Skip to content

Commit

Permalink
[sonic-cfggen]: Protect config_db.json from minigraph misconfig (#1727)
Browse files Browse the repository at this point in the history 
* Add noise config for PortChannel & EthernetInterface in simple-sample-graph.xml

* Add noise config for PORTCHANNEL_INTERFACE in simple-sample-graph.xml

Signed-off-by: Wenda <wenni@microsoft.com>

* Add noice config for DEVICE_NEIGHBOR in t0-sample-graph.xml

Add unit test against introducing ports not existing in port_config.ini
into DEVICE_NEIGHBOR

Signed-off-by: Wenda <wenni@microsoft.com>

* DeviceInterfaceLink in minigraph.xml can contain port not existing in
port_config.ini but contraining non-zero Bandwidth attribute

Add noice config in simple-sample-graph.xml to capture the case that
such a port is leaked into config_db.json

Signed-off-by: Wenda <wenni@microsoft.com>

* Protect PORTCHANNEL from ports not existing in port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Protect PORTCHANNEL_INTERFACE from portchannels containing ports not
existing in port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Protect DEVICE_NEIGHBOR from ports not existing in port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Add noise config Ethernet1 in DeviceInterfaceLinks in simple-sample-graph.xml as it is in PortChannel1001

Signed-off-by: Wenda <wenni@microsoft.com>

* Add noise config Ethernet1 in DeviceInterfaceLinks in simple-sample-graph.xml as it is in PortChannel1001

Signed-off-by: Wenda <wenni@microsoft.com>

* Protect PORTCHANNEL from ports not existing in port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Protect PORTCHANNEL_INTERFACE from portchannels containing ports not
existing in port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Protect DEVICE_NEIGHBOR from ports not existing in port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Correct space in minigraph.py

Signed-off-by: Wenda <wenni@microsoft.com>

* Does not allow non-port_config.ini port to get into the port list

Signed-off-by: Wenda <wenni@microsoft.com>

* Check PORTCHANNEL against PORT list only if port_config_file exists

Signed-off-by: Wenda <wenni@microsoft.com>

* Correct format

Signed-off-by: Wenda <wenni@microsoft.com>

* print warning when a port coming from DeviceInterfaceLink is not in
port_config.ini

Signed-off-by: Wenda <wenni@microsoft.com>

* Change Ethernet1 and 2 to fortyGigE0/1 and 2,respectively

Signed-off-by: Wenda <wenni@microsoft.com>

* Change Ethernet1 and 2 to fortyGigE0/1 and 2,respectively

Signed-off-by: Wenda <wenni@microsoft.com>

* print warning when ignoring ports, portchannels, portchannel interfaces, and
device neighbors

Update t0-sample-graph.xml with interface name 'fortyGigE0/2' and the
ACL_TABLE output

Signed-off-by: Wenda <wenni@microsoft.com>
  • Loading branch information
@wendani @lguohan
wendani authored and lguohan committed May 24, 2018
1 parent 6fc38af commit d32c043
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 6 deletions.
37 changes: 32 additions & 5 deletions src/sonic-config-engine/minigraph.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ def parse_xml(filename, platform=None, port_config_file=None):
(port_speeds_default, port_descriptions) = parse_deviceinfo(child, hwsku)

current_device = [devices[key] for key in devices if key.lower() == hostname.lower()][0]
results = {}
results = {}
results['DEVICE_METADATA'] = {'localhost': {
'bgp_asn': bgp_asn,
'deployment_id': deployment_id,
Expand Down Expand Up @@ -447,7 +447,6 @@ def parse_xml(filename, platform=None, port_config_file=None):

results['INTERFACE'] = phyport_intfs
results['VLAN_INTERFACE'] = vlan_intfs
results['PORTCHANNEL_INTERFACE'] = pc_intfs

for port_name in port_speeds_default:
# ignore port not in port_config.ini
Expand All @@ -457,9 +456,11 @@ def parse_xml(filename, platform=None, port_config_file=None):
ports.setdefault(port_name, {})['speed'] = port_speeds_default[port_name]

for port_name in port_speed_png:
# if port_name is not in port_config.ini, still consider it.
# and later swss will pick up and behave on-demand port break-up.
# if on-deman port break-up is not supported on a specific platform, swss will return error.
# not consider port not in port_config.ini
if port_name not in ports:
print >> sys.stderr, "Warning: ignore interface '%s' as it is not in the port_config.ini" % port_name
continue

ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name]

for port_name, port in ports.items():
Expand All @@ -474,10 +475,36 @@ def parse_xml(filename, platform=None, port_config_file=None):
ports.setdefault(port_name, {})['description'] = port_descriptions[port_name]

results['PORT'] = ports

if port_config_file:
port_set = set(ports.keys())
for (pc_name, mbr_map) in pcs.items():
# remove portchannels that contain ports not existing in port_config.ini
# when port_config.ini exists
if not set(mbr_map['members']).issubset(port_set):
print >> sys.stderr, "Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name
del pcs[pc_name]

results['PORTCHANNEL'] = pcs


for pc_intf in pc_intfs.keys():
# remove portchannels not in PORTCHANNEL dictionary
if pc_intf[0] not in pcs:
print >> sys.stderr, "Warning: ignore '%s' interface '%s' as '%s' is not in the valid PortChannel list" % (pc_intf[0], pc_intf[1], pc_intf[0])
del pc_intfs[pc_intf]

results['PORTCHANNEL_INTERFACE'] = pc_intfs

results['VLAN'] = vlans
results['VLAN_MEMBER'] = vlan_members

for nghbr in neighbors.keys():
# remove port not in port_config.ini
if nghbr not in ports:
print >> sys.stderr, "Warning: ignore interface '%s' in DEVICE_NEIGHBOR as it is not in the port_config.ini" % nghbr
del neighbors[nghbr]

results['DEVICE_NEIGHBOR'] = neighbors
results['DEVICE_NEIGHBOR_METADATA'] = { key:devices[key] for key in devices if key.lower() != hostname.lower() }
results['SYSLOG_SERVER'] = dict((item, {}) for item in syslog_servers)
Expand Down
52 changes: 51 additions & 1 deletion src/sonic-config-engine/tests/simple-sample-graph.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@
<AttachTo>fortyGigE0/4</AttachTo>
<SubInterface/>
</PortChannel>
<PortChannel>
<Name>PortChannel1001</Name>
<AttachTo>fortyGigE0/1;fortyGigE0/2</AttachTo>
<SubInterface/>
</PortChannel>
</PortChannelInterfaces>
<VlanInterfaces>
<VlanInterface>
Expand All @@ -147,6 +152,16 @@
<AttachTo>PortChannel01</AttachTo>
<Prefix>FC00::71/126</Prefix>
</IPInterface>
<IPInterface>
<Name i:nil="true"/>
<AttachTo>PortChannel1001</AttachTo>
<Prefix>10.0.0.57/31</Prefix>
</IPInterface>
<IPInterface>
<Name i:Name="true"/>
<AttachTo>PortChannel1001</AttachTo>
<Prefix>FC00::72/126</Prefix>
</IPInterface>
<IPInterface>
<Name i:nil="true"/>
<AttachTo>fortyGigE0/0</AttachTo>
Expand Down Expand Up @@ -193,6 +208,28 @@
<StartPort>fortyGigE0/8</StartPort>
<Validate>true</Validate>
</DeviceLinkBase>
<DeviceLinkBase i:type="DeviceInterfaceLink">
<ElementType>DeviceInterfaceLink</ElementType>
<AutoNegotiation>true</AutoNegotiation>
<Bandwidth>10000</Bandwidth>
<EndDevice>switch-t0</EndDevice>
<EndPort>fortyGigE0/1</EndPort>
<FlowControl>true</FlowControl>
<StartDevice>ARISTA05T1</StartDevice>
<StartPort>Ethernet1/32</StartPort>
<Validate>true</Validate>
</DeviceLinkBase>
<DeviceLinkBase i:type="DeviceInterfaceLink">
<ElementType>DeviceInterfaceLink</ElementType>
<AutoNegotiation>true</AutoNegotiation>
<Bandwidth>10000</Bandwidth>
<EndDevice>switch-t0</EndDevice>
<EndPort>fortyGigE0/2</EndPort>
<FlowControl>true</FlowControl>
<StartDevice>ARISTA06T1</StartDevice>
<StartPort>Ethernet1/33</StartPort>
<Validate>true</Validate>
</DeviceLinkBase>
</DeviceInterfaceLinks>
<Devices>
<Device i:type="ToRRouter">
Expand Down Expand Up @@ -240,7 +277,20 @@
<EnableAutoNegotiation>true</EnableAutoNegotiation>
<EnableFlowControl>true</EnableFlowControl>
<Index>1</Index>
<InterfaceName>fortyGigE0/1</InterfaceName>
<InterfaceName>Ethernet1</InterfaceName>
<InterfaceType i:nil="true"/>
<MultiPortsInterface>false</MultiPortsInterface>
<PortName>0</PortName>
<Priority>0</Priority>
<Speed>10000</Speed>
</a:EthernetInterface>
<a:EthernetInterface>
<ElementType>DeviceInterface</ElementType>
<AlternateSpeeds i:nil="true"/>
<EnableAutoNegotiation>true</EnableAutoNegotiation>
<EnableFlowControl>true</EnableFlowControl>
<Index>1</Index>
<InterfaceName>Ethernet2</InterfaceName>
<InterfaceType i:nil="true"/>
<MultiPortsInterface>false</MultiPortsInterface>
<PortName>0</PortName>
Expand Down
11 changes: 11 additions & 0 deletions src/sonic-config-engine/tests/t0-sample-graph.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,17 @@
<StartDevice>switch-t0</StartDevice>
<StartPort>fortyGigE0/124</StartPort>
</DeviceLinkBase>
<DeviceLinkBase>
<ElementType>DeviceInterfaceLink</ElementType>
<AutoNegotiation>true</AutoNegotiation>
<Bandwidth>10000</Bandwidth>
<EndDevice>switch-t0</EndDevice>
<EndPort>fortyGigE0/2</EndPort>
<FlowControl>true</FlowControl>
<StartDevice>ARISTA05T1</StartDevice>
<StartPort>Ethernet1/33</StartPort>
<Validate>true</Validate>
</DeviceLinkBase>
</DeviceInterfaceLinks>
<Devices>
<Device i:type="ToRRouter">
Expand Down
11 changes: 11 additions & 0 deletions src/sonic-config-engine/tests/test_cfggen.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ def test_minigraph_acl(self):
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v ACL_TABLE'
output = self.run_script(argument, True)
self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n"
"Warning: ignore interface 'fortyGigE0/2' as it is not in the port_config.ini\n"
"Warning: ignore interface 'fortyGigE0/2' in DEVICE_NEIGHBOR as it is not in the port_config.ini\n"
"{'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'},"
" 'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'},"
" 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124']},"
Expand Down Expand Up @@ -130,6 +132,15 @@ def test_minigraph_neighbors(self):
output = self.run_script(argument)
self.assertEqual(output.strip(), "{'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}")

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'}, "
"'Ethernet120': {'name': 'ARISTA03T1', 'port': 'Ethernet1/1'}}")

def test_minigraph_bgp(self):
argument = '-m "' + self.sample_graph_bgp_speaker + '" -p "' + self.port_config + '" -v "BGP_NEIGHBOR[\'10.0.0.59\']"'
output = self.run_script(argument)
Expand Down

0 comments on commit d32c043

Please sign in to comment.