Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[minigraph] For egress ACL attaching to vlan, break them into vlan members #6895

Merged
merged 4 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/sonic-config-engine/minigraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,6 @@ def parse_dpg(dpg, hname):
dpg_ecmp_content['ipv4'] = ipv4_content
dpg_ecmp_content['ipv6'] = ipv6_content
vlanintfs = child.find(str(QName(ns, "VlanInterfaces")))
vlan_intfs = []
vlans = {}
vlan_members = {}
vlantype_name = ""
Expand Down Expand Up @@ -551,7 +550,7 @@ def parse_dpg(dpg, hname):
else:
vlan_members[(sonic_vlan_member_name, vmbr_list[i])] = {'tagging_mode': 'untagged'}

vlan_attributes = {'vlanid': vlanid}
vlan_attributes = {'vlanid': vlanid, 'members': vmbr_list }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

members fields should be removed from the config db.


# If this VLAN requires a DHCP relay agent, it will contain a <DhcpRelays> element
# containing a list of DHCP server IPs
Expand Down Expand Up @@ -579,7 +578,7 @@ def parse_dpg(dpg, hname):
aclname = aclintf.find(str(QName(ns, "OutAcl"))).text.upper().replace(" ", "_").replace("-", "_")
stage = "egress"
else:
system.exit("Error: 'AclInterface' must contain either an 'InAcl' or 'OutAcl' subelement.")
sys.exit("Error: 'AclInterface' must contain either an 'InAcl' or 'OutAcl' subelement.")
aclattach = aclintf.find(str(QName(ns, "AttachTo"))).text.split(';')
acl_intfs = []
is_mirror = False
Expand All @@ -596,7 +595,11 @@ def parse_dpg(dpg, hname):
# to LAG will be applied to all the LAG members internally by SAI/SDK
acl_intfs.append(member)
elif member in vlans:
acl_intfs.append(member)
# For egress ACL attaching to vlan, we break them into vlan members
if stage == "egress":
acl_intfs.extend(vlans[member]['members'])
else:
acl_intfs.append(member)
elif member in port_alias_map:
acl_intfs.append(port_alias_map[member])
# Give a warning if trying to attach ACL to a LAG member interface, correct way is to attach ACL to the LAG interface
Expand All @@ -620,9 +623,15 @@ def parse_dpg(dpg, hname):
acl_intfs.append(panel_port)
break
if acl_intfs:
# Remove duplications
dedup_intfs = []
for intf in acl_intfs:
if intf not in dedup_intfs:
dedup_intfs.append(intf)

acls[aclname] = {'policy_desc': aclname,
'stage': stage,
'ports': acl_intfs}
'ports': dedup_intfs}
if is_mirror:
acls[aclname]['type'] = 'MIRROR'
elif is_mirror_v6:
Expand Down
16 changes: 15 additions & 1 deletion src/sonic-config-engine/tests/t0-sample-graph.xml
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,20 @@
<MacAddress i:nil="true"/>
<SecondarySubnets/>
</VlanInterface>
<VlanInterface>
<Name>Vlan98</Name>
<AttachTo>fortyGigE0/100;PortChannel01;PortChannel03</AttachTo>
lguohan marked this conversation as resolved.
Show resolved Hide resolved
<NoDhcpRelay>False</NoDhcpRelay>
<StaticDHCPRelay>0.0.0.0/0</StaticDHCPRelay>
<FhrpProtoType i:nil="true"/>
<Type>UserDefinedL2Vlan</Type>
<DhcpRelays>192.0.0.1;192.0.0.2</DhcpRelays>
<VlanID>98</VlanID>
<Tag>98</Tag>
<Subnets/>
<MacAddress i:nil="true"/>
<SecondarySubnets/>
</VlanInterface>
</VlanInterfaces>
<IPInterfaces>
<IPInterface>
Expand Down Expand Up @@ -331,7 +345,7 @@
<Type>DataPlane</Type>
</AclInterface>
<AclInterface>
<AttachTo>PortChannel01;PortChannel02</AttachTo>
<AttachTo>PortChannel01;PortChannel02;Vlan98</AttachTo>
<OutAcl>DataAclEgress</OutAcl>
<Type>DataPlane</Type>
</AclInterface>
Expand Down
10 changes: 5 additions & 5 deletions src/sonic-config-engine/tests/test_cfggen.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def test_minigraph_acl(self):
"'DATAACLINGRESS': {'stage': 'ingress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04'], 'policy_desc': 'DATAACLINGRESS'}, "
"'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL', 'stage': 'ingress'}, "
"'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL', 'stage': 'ingress'}, "
"'DATAACLEGRESS': {'stage': 'egress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02'], 'policy_desc': 'DATAACLEGRESS'}, "
"'DATAACLEGRESS': {'stage': 'egress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02', 'Ethernet100', 'PortChannel03'], 'policy_desc': 'DATAACLEGRESS'}, "
"'EVERFLOWV6': {'stage': 'ingress', 'type': 'MIRRORV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4', 'Ethernet100'], 'policy_desc': 'EVERFLOWV6'}}"
)
)
Expand Down Expand Up @@ -207,10 +207,10 @@ def test_minigraph_vlans(self):
self.assertEqual(
utils.to_dict(output.strip()),
utils.to_dict(
"{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000'}, "
"'Vlan2001': {'alias': 'ab3', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2001'},"
"'Vlan2000': {'alias': 'ab2', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2000'},"
"'Vlan2020': {'alias': 'kk1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2020'}}"
"{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000', 'members': ['Ethernet8']}, "
"'Vlan2001': {'alias': 'ab3', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2001', 'members': ['Ethernet12']},"
"'Vlan2000': {'alias': 'ab2', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2000', 'members': ['Ethernet12']},"
"'Vlan2020': {'alias': 'kk1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2020', 'members': ['Ethernet12']}}"
)
)

Expand Down
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/test_minigraph_case.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def test_minigraph_vlans(self):
output = self.run_script(argument)
self.assertEqual(
utils.to_dict(output.strip()),
utils.to_dict("{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000', 'mac': '00:aa:bb:cc:dd:ee' }}")
utils.to_dict("{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000', 'mac': '00:aa:bb:cc:dd:ee', 'members': ['Ethernet8'] }}")
)

def test_minigraph_vlan_members(self):
Expand Down