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.

Sign up for GitHub

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.py] Update minigraph parsing logic to include only active ports for mirror tables #3592

Merged
merged 2 commits into from
Oct 17, 2019
Merged

[minigraph.py] Update minigraph parsing logic to include only active ports for mirror tables #3592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
202099c
[minigraph.py] Update minigraph parsing logic to include only active …
daall Oct 11, 2019
9d84a8d
Incorporate PR comments
daall Oct 17, 2019
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
53 changes: 47 additions & 6 deletions src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
# Default Virtual Network Index (VNI)
vni_default = 8000

###############################################################################
#
# Minigraph parsing functions
#
###############################################################################

class minigraph_encoder(json.JSONEncoder):
def default(self, obj):
if isinstance(obj, (
Expand Down Expand Up @@ -274,15 +280,17 @@ def parse_dpg(dpg, hname):
if member.lower().startswith('erspanv6'):
is_mirror_v6 = True
else:
is_mirror = True;
# Erspan session will be attached to all front panel ports,
# if panel ports is a member port of LAG, should add the LAG
# to acl table instead of the panel ports
is_mirror = True
# Erspan session will be attached to all front panel ports
# initially. If panel ports is a member port of LAG, then
# the LAG will be added to acl table instead of the panel
# ports. Non-active ports will be removed from this list
# later after the rest of the minigraph has been parsed.
acl_intfs = pc_intfs[:]
for panel_port in port_alias_map.values():
if panel_port not in intfs_inpc:
acl_intfs.append(panel_port)
break;
break
if acl_intfs:
acls[aclname] = {'policy_desc': aclname,
'ports': acl_intfs}
Expand Down Expand Up @@ -510,6 +518,39 @@ def parse_spine_chassis_fe(results, vni, lo_intfs, phyport_intfs, pc_intfs, pc_m
# Enslave the port channel interface to a Vnet
pc_intfs[pc_intf] = {'vnet_name': chassis_vnet}

###############################################################################
#
# Post-processing functions
#
###############################################################################

def filter_acl_mirror_table_bindings(acls, neighbors, port_channels):
"""
Filters out inactive front-panel ports from the binding list for mirror
ACL tables. We define an "active" port as one that is a member of a
port channel or one that is connected to a neighboring device.
"""

for acl_table, group_params in acls.iteritems():
group_type = group_params.get('type', None)

if group_type != 'MIRROR' and group_type != 'MIRRORV6':
continue

active_ports = [ port for port in group_params.get('ports', []) if port in neighbors.keys() or port in port_channels ]

if not active_ports:
print >> sys.stderr, 'Warning: mirror table {} in ACL_TABLE does not have any ports bound to it'.format(acl_table)

acls[acl_table]['ports'] = active_ports

return acls

###############################################################################
#
# Main functions
#
###############################################################################

def parse_xml(filename, platform=None, port_config_file=None):
root = ET.parse(filename).getroot()
Expand Down Expand Up @@ -729,7 +770,7 @@ def parse_xml(filename, platform=None, port_config_file=None):
results['NTP_SERVER'] = dict((item, {}) for item in ntp_servers)
results['TACPLUS_SERVER'] = dict((item, {'priority': '1', 'tcp_port': '49'}) for item in tacacs_servers)

results['ACL_TABLE'] = acls
results['ACL_TABLE'] = filter_acl_mirror_table_bindings(acls, neighbors, pcs)

# Do not configure the minigraph's mirror session, which is currently unused
# mirror_sessions = {}
Expand Down
7 changes: 7 additions & 0 deletions src/sonic-config-engine/tests/t0-sample-graph.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,13 @@
<StartPort>Ethernet1/33</StartPort>
<Validate>true</Validate>
</DeviceLinkBase>
<DeviceLinkBase>
<ElementType>DeviceInterfaceLink</ElementType>
<EndDevice>Servers0</EndDevice>
<EndPort>eth0</EndPort>
<StartDevice>switch-t0</StartDevice>
<StartPort>fortyGigE0/4</StartPort>
</DeviceLinkBase>
</DeviceInterfaceLinks>
<Devices>
<Device i:type="ToRRouter">
Expand Down
11 changes: 9 additions & 2 deletions src/sonic-config-engine/tests/test_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ def test_render_template(self):
output = self.run_script(argument)
self.assertEqual(output.strip(), 'value1\nvalue2')

# FIXME: This test depends heavily on the ordering of the interfaces and
# it is not at all intuitive what that ordering should be. Could make it
# more robust by adding better parsing logic.
def test_minigraph_acl(self):
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v ACL_TABLE'
output = self.run_script(argument, True)
Expand All @@ -98,11 +101,11 @@ def test_minigraph_acl(self):
"Warning: ignore interface 'fortyGigE0/2' in DEVICE_NEIGHBOR as it is not in the port_config.ini\n"
"{'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04']}, "
"'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL'}, "
"'EVERFLOW': {'type': 'MIRROR', 'policy_desc': 'EVERFLOW', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet24', 'Ethernet40', 'Ethernet20', 'Ethernet44', 'Ethernet48', 'Ethernet28', 'Ethernet96', 'Ethernet92', 'Ethernet76', 'Ethernet72', 'Ethernet52', 'Ethernet80', 'Ethernet56', 'Ethernet32', 'Ethernet16', 'Ethernet36', 'Ethernet12', 'Ethernet60', 'Ethernet8', 'Ethernet4', 'Ethernet0', 'Ethernet64', 'Ethernet68', 'Ethernet84', 'Ethernet88', 'Ethernet108', 'Ethernet104', 'Ethernet100']}, "
"'EVERFLOW': {'type': 'MIRROR', 'policy_desc': 'EVERFLOW', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4']}, "
"'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT'}, "
"'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'}, "
"'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'}, "
"'EVERFLOWV6': {'type': 'MIRRORV6', 'policy_desc': 'EVERFLOWV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet24', 'Ethernet40', 'Ethernet20', 'Ethernet44', 'Ethernet48', 'Ethernet28', 'Ethernet96', 'Ethernet92', 'Ethernet76', 'Ethernet72', 'Ethernet52', 'Ethernet80', 'Ethernet56', 'Ethernet32', 'Ethernet16', 'Ethernet36', 'Ethernet12', 'Ethernet60', 'Ethernet8', 'Ethernet4', 'Ethernet0', 'Ethernet64', 'Ethernet68', 'Ethernet84', 'Ethernet88', 'Ethernet108', 'Ethernet104', 'Ethernet100']}}")
"'EVERFLOWV6': {'type': 'MIRRORV6', 'policy_desc': 'EVERFLOWV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4']}}")

# everflow portion is not used
# def test_minigraph_everflow(self):
Expand Down Expand Up @@ -160,13 +163,17 @@ def test_minigraph_neighbors(self):
output = self.run_script(argument)
self.assertEqual(output.strip(), "{'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}")

# FIXME: This test depends heavily on the ordering of the interfaces and
# it is not at all intuitive what that ordering should be. Could make it
# more robust by adding better parsing logic.
def test_minigraph_extra_neighbors(self):
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v DEVICE_NEIGHBOR'
output = self.run_script(argument)
self.assertEqual(output.strip(), \
"{'Ethernet116': {'name': 'ARISTA02T1', 'port': 'Ethernet1/1'}, "
"'Ethernet124': {'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}, "
"'Ethernet112': {'name': 'ARISTA01T1', 'port': 'Ethernet1/1'}, "
"'Ethernet4': {'name': 'Servers0', 'port': 'eth0'}, "
"'Ethernet120': {'name': 'ARISTA03T1', 'port': 'Ethernet1/1'}}")

def test_minigraph_port_description(self):
Expand Down