[minigraph.py] generate port description for every port#2395
[minigraph.py] generate port description for every port#2395liat-grozovik merged 4 commits intosonic-net:masterfrom
Conversation
Signed-off-by: Mykola Faryma <mykolaf@mellanox.com>
Signed-off-by: Mykola Faryma <mykolaf@mellanox.com>
src/sonic-config-engine/minigraph.py
Outdated
| try: | ||
| neigh_host = neighbors[port_name]['name'] | ||
| neigh_port = neighbors[port_name]['port'] | ||
| port['description'] = neigh_host + ':' + neigh_port |
There was a problem hiding this comment.
I don't agree with your change here. Port description shouldn't be coming from port_config.ini and certainly shouldn't default to neigh_host + neigh_port. The port_config.ini shouldn't have a description mandatory or otherwise.
There was a problem hiding this comment.
This is not encouraging to specify port description via port_config.ini.
As for default value I'd be glad to hear some suggestions.
My concern is not matching the descr advertised by lldp and snmp. The neighbor info and port name is the defaults that lldp is using.
Let's start a discussion
There was a problem hiding this comment.
The description should be derived solely on information available locally and without being a function of the remote system or neighbor. If the port description exists in config_db.json/redis, pick that. Otherwise pick the alias (which is optional in sonic and may or may not match the interface name). Otherwise pick the interface name.
There was a problem hiding this comment.
Hi @nikos-github , can you comment/approve this?
…neighbor data Signed-off-by: Mykola Faryma <mykolaf@mellanox.com>
|
Made to 201811 branch on 2/14/2019 |
* [minigraph.py] generate mandatory default port description Signed-off-by: Mykola Faryma <mykolaf@mellanox.com> * use port name as default description * [config-engine] update test exaple output Signed-off-by: Mykola Faryma <mykolaf@mellanox.com> * [minigraph.py] use alias/port name as default description instead of neighbor data Signed-off-by: Mykola Faryma <mykolaf@mellanox.com>
Update sonic-utilities submodule pointer to include the following: * 94a3436 [show] vnet endpoint [ip/ipv6] command (sonic-net#2342) ([sonic-net#2421](sonic-net/sonic-utilities#2421)) * 84a0712 [VxLAN]Fix Vxlan delete command to throw error when there are references ([sonic-net#2404](sonic-net/sonic-utilities#2404)) * 1341f58 [202012] [generate_dump]: Enhance show techsupport for cisco-8000 platform ([sonic-net#2395](sonic-net/sonic-utilities#2395)) Signed-off-by: dprital <drorp@nvidia.com>
Update sonic-utilities submodule pointer to include the following: 94a3436 [show] vnet endpoint [ip/ipv6] command (2342) (#2421) 84a0712 [VxLAN]Fix Vxlan delete command to throw error when there are references (#2404) 1341f58 [202012] [generate_dump]: Enhance show techsupport for cisco-8000 platform (#2395) Signed-off-by: dprital <drorp@nvidia.com>
Signed-off-by: Mykola Faryma mykolaf@mellanox.com
- What I did
Add logic to always generate port description for PORT. The goal is to provide the description in one place for snmp, lldp and user. (previously lldpmgr generated its own port description)
- Why I did it
My goal is for each port to have some description. The concern here is that currently lldpd will advertise SONiC ports with some default description of it's own, not matching the one in DB, used by snmp & shown to user.
We will the description generated in one place and will configure the lldpd accordingly. #2396
- How I did it
The logic is the following:
If the port description is specified in port_config.ini/minigraph - use it.
If the minigraph has info about the neighbor for this port - create description in form "{neighbor_host}:{neighbor_port}"
Else assign it to port name.
- How to verify it
sonic-cfggen -m /etc/sonic/minigraph.xml -p /usr/share/sonic/device/x86_64-mlnx_msn2100-r0/ACS-MSN2100/port_config.ini --print-data
port_config.ini
- Description for the changelog
generate mandatory default port description
- A picture of a cute animal (not mandatory but encouraged)