[minigraph] set admin_status to down if port not in minigraph #6865
Conversation
src/sonic-config-engine/minigraph.py
Outdated
| @@ -1090,6 +1092,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw | |||
| port_speeds_default = {} | |||
| port_speed_png = {} | |||
| port_descriptions = {} | |||
| port_alias_list = [] | |||
There was a problem hiding this comment.
Do we need to create a new list or can we use existing port_alias_map?.
There was a problem hiding this comment.
port_alias_map contains aliases from config_db, so we cannot use it
src/sonic-config-engine/minigraph.py
Outdated
| @@ -1346,6 +1349,11 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw | |||
| for port in ports.values(): | |||
| port['pfc_asym'] = 'off' | |||
|
|
|||
| # make admin status to down if port not parsed from minigraph | |||
| for port in ports.values(): | |||
| if port['alias'] not in port_alias_list: | |||
There was a problem hiding this comment.
Does this impact multi-asic platforms where there are backplane ports? @arlakshm ?
dmytroxshevchuk
force-pushed
the
admin_status_down_if_port_not_in_mini
branch
from
3c24498 to
ec23e2b
Compare
dmytroxshevchuk
changed the title
dmytroxshevchuk
force-pushed
the
admin_status_down_if_port_not_in_mini
branch
10 times, most recently
from
b4e6bf2 to
bee186d
Compare
dmytroxshevchuk
changed the title
| @@ -810,7 +810,7 @@ | |||
| "index": "7", | |||
| "lanes": "26,27", | |||
| "description": "Eth7/2", | |||
| "admin_status": "up", | |||
| "admin_status": "down", | |||
There was a problem hiding this comment.
Why is the test modified to have all port set to "down"?. I would expect test some port down and some up.
There was a problem hiding this comment.
Ports described in minigraph.xml as EthernetInterface has up admin_status.
This ports:
Ethernet8, Ethernet9, Ethernet36, Ethernet98, Ethernet0, Ethernet6, Ethernet4, Ethernet109, Ethernet108, Ethernet18, Ethernet100, Ethernet34, Ethernet104, Ethernet106, Ethernet76, Ethernet74, Ethernet39, Ethernet72, Ethernet73, Ethernet70, Ethernet71, Ethernet32, Ethernet33, Ethernet16, Ethernet10, Ethernet11, Ethernet19, Ethernet112
have "admin_status": "up"
dmytroxshevchuk
force-pushed
the
admin_status_down_if_port_not_in_mini
branch
from
bee186d to
49484a7
Compare
| @@ -1364,6 +1364,11 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw | |||
| for port in ports.values(): | |||
| port['pfc_asym'] = 'off' | |||
|
|
|||
| # make admin status to down if port not parsed from minigraph | |||
| for port in ports.keys(): | |||
| if 'admin_status' in ports[port] and port not in port_speeds_default: | |||
There was a problem hiding this comment.
This check if 'admin_status' in ports[port]. What would be the value if its present? Where is that default value coming from?
Why I did it
After produce platform.json, hwsku.json on DUT we have issue with some ports on load_minigraph flow. Previous scripts in sonic took the list of ports from port_config.ini(than we can announce ports what we need), but for now sonic gets full pack of ports from platform.json/hwsku.json and sets configuration for ports parsed from minigraph. So we have all ports from config_db and only a part(ports announced in minigraph) from this pack will be configured. Another pack of ports not using and OperState will be in Down, however AdminStatus has Up state.
So sanity check will fall because of AdminStatus is Up and OperState is Down. More correct set AdminStatus to down.
How I did it
Current PR sets AdminStatus to Down state in load_minigraph flow it case when this port not exist in minigraph.
How to verify it
sonic-cfggen -m /ets/sonic/minigraph.xml --print-dataWhich release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)