Skip to content

support for not all DUT ports connected to a fanout switch#2517

Merged
lolyu merged 14 commits intosonic-net:masterfrom
sanmalho-git:discontiguous_ports
Dec 28, 2020
Merged

support for not all DUT ports connected to a fanout switch#2517
lolyu merged 14 commits intosonic-net:masterfrom
sanmalho-git:discontiguous_ports

Conversation

@sanmalho-git
Copy link
Contributor

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

Currently, it is required that all ports on DUT are in use and are connected to a fanout. However, there is a need to be able to run tests where all ports are not in use. Specifically, when dealing with

  • New Sonic devices
  • Sonic device with a front panel port used for in-band management
  • Chassis with lots of ports and multiple asics, were every port on every asic is not required to be covered
  • Chassis as a DUT, where the number of ports can be in hundreds
  • Expensive, high speed ports like 400G (hard to go from 400G down to 1/10G)

So, need to add support where not all DUT ports are connected to fanout and are thus not part of the testing.

How did you do it?

conn_graph_facts:

conn_graph_facts.py returns device_vlan_map_list. This used to be a dictionary with key being hostname and value being a list of vlanids for the fanout ports. Have modified this where the value instead of being a list of vlanids, it is a dictionary with key being the port_index and the value being the vlan id. This port_index is what gets put in the topology file. We get the port by looking at the host_port_vlans defined in the conn_graph for that device. This host_port_vlans has key being the Ethernet port (like Ethernet10) and value being a dictionary with key 'vlanlist' being a the list of fanout vlans. We check against all the ports 'vlanlist' to get the port on the DUT that connects to this fanout vlan, and then split on 'Ethernet' to get the port index.

For example - lets say on dut with hostname "dut1", we have port Ethernet10 connected to fanout w/ vlan 120, and Ethernet11 connected to fanout w/ vlan 121, then we would have:

  "host_port_vlans":
    {
      "Ethernet10": {
         "mode": "Access",
         "vlanids": "120",
         "vlanlist": [
            120
         ]
      },
      "Ethernet11": {
         "mode": "Access",
         "vlanids": "121",
         "vlanlist": [
            121
         ]
      }
   }

  "VlanList": [
     120,
     121
  ]

For vlan 120 in VlanList, we would iterate through host_port_vlans to find the port that has vlan 120 - in our case "Ethernet10". The port_index would then be "10". Similarly, for vlan 121, the port_index would be "Ethernet11".

So, returned device_vlan_map_list would be:

    "dut1" : {
       "10" : 120,
       "11" : 121
    }

vlan_port/kvm_port/mellanox_simx_port:

  • Updated to return (dut_fp_ports) a dictionary with key being the port index (same as in the topo file) and vlan being the port - instead of the just the list of ports.

bind/unbind vm_topology:

  • vlan_index is now a string in the dictionary of dut_fp_ports
  • updated regexp for checking valid vlan for multi-dut to be of the format '.@'

remove_dut_port.yml (bug fix):

  • set cmd to "remove" instead of "create" in vlan_port module call.

How did you verify/test it?

Tested against pizza box DUT with all DUT ports connected to a fanout, and also against another DUT where we have only 4 of the 52 ports connected to a different fanout.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@lgtm-com
Copy link

lgtm-com bot commented Nov 11, 2020

This pull request introduces 1 alert when merging 0f8b9b63e5d7623089902177c3c239bbb15c5a55 into 1317643 - view on LGTM.com

new alerts:

  • 1 for Unused import

@yxieca yxieca requested review from a team and lolyu November 12, 2020 16:28
@sanmalho-git sanmalho-git force-pushed the discontiguous_ports branch 3 times, most recently from 143479c to 212752e Compare November 19, 2020 18:45
@sanmalho-git
Copy link
Contributor Author

@lolyu can you please review these changes.

Copy link
Collaborator

@lolyu lolyu left a comment

Choose a reason for hiding this comment

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

Please fix the conflict.

@sanmalho-git
Copy link
Contributor Author

@lolyu - I have re-based and fixed the merge conflict. Also, the module_utils PR has been merged into master. So, moved the common code of port_alias_to_name_map between conn_graph_facts and minigraph_facts to module_utils/port_utils.py.

Can you please give it a final review before we merge this PR into master.

@lgtm-com
Copy link

lgtm-com bot commented Dec 16, 2020

This pull request introduces 2 alerts when merging 0b6cca041aa089844b773558e362a8e01995d606 into f5537b1 - view on LGTM.com

new alerts:

  • 2 for Unused import

@lolyu
Copy link
Collaborator

lolyu commented Dec 21, 2020

Retest this please

conn_graph_facts:
  conn_graph_facts.py returns device_vlan_map_list. This used to be a dictionary
  with key being hostname and value being a list of vlanids for the fanout ports.
  We have modified this where the value instead of being a list of vlanids, it is a dictionary
  with key being the port_index and the value being the vlan id. This port_index is what gets
  put in the topology file. We get the port
  by looking at the host_port_vlans defined in the conn_graph for that device. This
  host_port_vlans has key being the Ethernet port (like Ethernet10) and value being
  a dictionary with key 'vlanlist' being a the list of fanout vlans. We check against all the ports
  'vlanlist' to get the port on the DUT that connects to this fanout vlan, and then split on
  'Ethernet' to get the port index.

  For example - lets say on dut with hostname "dut1", we have port Ethernet10 connected to fanout w/ vlan 120, and
  Ethernet11 connected to fanout w/ vlan 121, then we would have:

  "host_port_vlans":
    {
      "Ethernet10": {
         "mode": "Access",
         "vlanids": "120",
         "vlanlist": [
            120
         ]
      },
      "Ethernet11": {
         "mode": "Access",
         "vlanids": "121",
         "vlanlist": [
            121
         ]
      }
   }

  "VlanList": [
     120,
     121
  ]

  For vlan 120 in VlanList, we would iterate through host_port_vlans to find the port
  that has vlan 120 - in our case "Ethernet10". The port_index would then be "10". Similarly,
  for vlan 121, the port_index would be "Ethernet11".

  So, returned device_vlan_map_list would be:
    "dut1" : {
       "10" : 120,
       "11" : 121
    }

- vlan_port/kvm_port/mellanox_simx_port:
   - Updated to return (dut_fp_ports) a dictionary with key being the port index (same as in the topo file)
     and vlan being the port - instead of the just the list of ports.

bind/unbind vm_topology:
   - vlan_index is now a string in the dictionary of dut_fp_ports
   - updated regexp for checking valid vlan for multi-dut to be of the format '<dutIndex>.<portIndex>@<ptfIndex>'

remove_dut_port.yml:
  - set cmd to "remove" instead of "create" in vlan_port module call.
                                                                                                                                                                                                                                                        }
- The dut_fp_ports is a dictionary with key being a string and not an integer
- We were using the port name like 'Ethernet10' and splitting in out 'Ethernet' to
  get the port_index in the device_vlan_map_list. This port_index is to correspond to
  what would be in the topology file. This would not work
  when ports are not consecutively names - like 'Ethernet0', 'Ethernet4', 'Ethernet8' .....
  In this scenario, the topology file would have host_interfaces/vlans still as 0,1,2,.....

  Fix is to get the port names based on 'hwksu' (like it is done for minigraph) and
  then use that list to get the port_index to be  put into device_vlan_map_list
…and conn_graph_facts to module_utils/port_utils

The port_alias_to_name_map is generated based on hwsku and is common
code between minigraph_facts and conn_graph_facts ansible module.

Now that module_utils are supported, added module_utils/port_utils.py to
have this common code. Modified the ansible modules to use this new
common code.
Since dut_fp_ports is a dictionary with key being the dut name, and the
value being a dictionary with key being the interface index as a string,
need to typecast the host_interfaces interface index as a string.
Copy link
Collaborator

@lolyu lolyu left a comment

Choose a reason for hiding this comment

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

LGTM

@lolyu
Copy link
Collaborator

lolyu commented Dec 28, 2020

Thanks, @sanmalho-git.

@lolyu lolyu merged commit d062c61 into sonic-net:master Dec 28, 2020
@sanmalho-git sanmalho-git deleted the discontiguous_ports branch January 6, 2021 17:43
neethajohn pushed a commit to neethajohn/sonic-mgmt that referenced this pull request Jan 8, 2021
[minigraph_facts] Fix conflicts

Move the port alias to name mapping parsing to
ansible/module_utils/port_utils.py that is introduced by the following
patch:
sonic-net#2517

Signed-off-by: Longxiang Lyu <[email protected]>
bingwang-ms pushed a commit that referenced this pull request Jan 8, 2021
Description of PR
Fanout graph parsing was broken due to #2517
Following errors were seen while running pfcwd tests
E RunAnsibleModuleFail: run module conn_graph_facts failed, Ansible Results =>
E {
E "changed": false,
E "failed": true,
E "invocation": {
E "module_args": {
E "anchor": null,
E "filename": "/var/nejo/Networking-acs-sonic-mgmt/tests/common/fixtures/../../../ansible/files/starlab_connection_graph.xml",
E "filepath": null,
E "host": "str-7260cx3-acs-fan-05",
E "hosts": null
E }
E },
E "msg": "Did not find port for Ethernet23/1 in the ports based on hwsku 'Arista-7260CX3' for host str-7260cx3-acs-fan-05"
E }

How did you do it?
Parsing logic added in #2517 was for SONIC duts. Retained the old logic when dev type is FanoutLeaf

How did you verify/test it?
Ran one of the pfcwd tests and it passed
bingwang-ms pushed a commit to bingwang-ms/sonic-mgmt that referenced this pull request May 20, 2021
[minigraph_facts] Fix conflicts

Move the port alias to name mapping parsing to
ansible/module_utils/port_utils.py that is introduced by the following
patch:
sonic-net#2517

Signed-off-by: Longxiang Lyu <[email protected]>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
…t#12910)

utilities:
* 252910a 2022-11-29 | [drop counters] Fix CLI script for unconfigured PGs (sonic-net#2518) (HEAD -> 202205) [Lior Avramov]
* f2bf7ed 2022-11-30 | Change show kube command default value of insecure key to True (sonic-net#2517) [lixiaoyuner]
* 0a030ce 2022-12-01 | [QoS] Introduce delay to the qos reload flow (sonic-net#2503) [DavidZagury]
* d2fa21c 2022-11-02 | Disable "tag as local" when reboot (sonic-net#2451) [lixiaoyuner]

platform-daemon:
* 5532070 2022-12-01 | Remove the argument that is causing the xcvrd to crash (sonic-net#318) (HEAD -> 202205) [Vivek]

Signed-off-by: Ying Xie <[email protected]>

Signed-off-by: Ying Xie <[email protected]>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
sonic-net#13202

advance sonic-utilities submodule for 202211 branch

34428157 - (HEAD, origin/202211) Revert "Optimize the execution time of the 'show techsupport' script to 5-10%, (Qos config change sonic-net#2504)" (6 days ago) [stormliang]
c3bd01f6 - Revert "[generate_dump] Optimize the execution time of 'show techsupport' CLI by parallel function execution ([201811][Devices] Add new device CIG CS6436-56P sonic-net#2512)" (6 days ago) [stormliang]
5a326d8b - [Mellanox] Change severity to NOTICE in Mellanox buffer migrator when unable to fetch DEVICE_METADATA due to empty CONFIG_DB during initialization ([warm boot] cherry-pick PR sonic-net#2538 and advance related sub-modules in 201811 branch sonic-net#2569) (2 weeks ago) [Stephen Sun]
50b36ef3 - Fix issue: unconfigured PGs are displayed in watermarkstat ([docker-lldp]: fix several issues in lldpd docker sonic-net#2556) (2 weeks ago) [Stephen Sun]
a9fd2a79 - [Command Ref] Add doc for syslog rate limit ([sub module] move sairedis and swss to 201811 branch sonic-net#2508) (2 weeks ago) [Junchao-Mellanox]
80546ff3 - [generate_dump] Optimize the execution time of 'show techsupport' CLI by parallel function execution ([201811][Devices] Add new device CIG CS6436-56P sonic-net#2512) (2 weeks ago) [Vadym Hlushko]
6649ca8a - [timer.unit.j2] use wanted-by in timer unit ([201803] [services] Restart SwSS service upon unexpected critical process exit sonic-net#2546) (2 weeks ago) [Stepan Blyshchak]
dd23d0ef - Fixes [Sub-If|VRF] Unbind sub-interface from VRF is failed sonic-net#12170: Delete subinterface and recreate the subinterface in ([VLAN] "show mac" doesn't work when interface added to vlan as tagged member sonic-net#2513) (2 weeks ago) [Preetham]
236749d3 - [db_migrator] Fix migration of Loopback data: handle all Loopback interfaces (DellEMC S6000 xcvrd support sonic-net#2560) (2 weeks ago) [Vaibhav Hemant Dixit]
5762d814 - Optimize the execution time of the 'show techsupport' script to 5-10%, (Qos config change sonic-net#2504) (2 weeks ago) [Vadym Hlushko]
d3c3e368 - [muxcable][show] update show mux tunnel-route to separate ASIC and kernel into two columns (build errors on branch 201811 for centec platform sonic-net#2553) (2 weeks ago) [Jing Zhang]
c98648a1 - [show]Fix show route return code on error (Dell SMF driver hwmon number reorder fix for Dell S6100/Z9100 sonic-net#2542) (2 weeks ago) [Sudharsan Dhamal Gopalarathnam]
01374673 - [route_check]: Ignore ASIC only SOC IPs (Added new SN3700/SN3700C Mellanox platforms sonic-net#2548) (2 weeks ago) [Lawrence Lee]
d2967805 - YANG Validation for ConfigDB Updates: WARM_RESTART, SFLOW_SESSION, SFLOW, VXLAN_TUNNEL, VXLAN_EVPN_NVO, VXLAN_TUNNEL_MAP, MGMT_VRF_CONFIG, CABLE_LENGTH, VRF tables ([submodule 201811] advance sairedis and swss submodule for 201811 branch sonic-net#2526) (2 weeks ago) [isabelmsft]
88b01ffd - [db_migrator] Remove import of swsssdk as it is not supported in master ([build]: apply proxy setting to curl. sonic-net#2544) (2 weeks ago) [Vaibhav Hemant Dixit]
4ae970c6 - Support syslog rate limit configuration for containers and host (Move FRR from 4.0 to 6.0.2 and make new frr version and pkg compile sonic-net#2454) (2 weeks ago) [Junchao-Mellanox]
608ed147 - [generate_dump] [Mellanox] Fix the duplicate dfw dump collection problem by adding symlinks ('show vlan config' is not displaying the VLAN members, after the clear config and reload with default l2 configuration. sonic-net#2536) (2 weeks ago) [Vivek]
bdc2599f - [config] Add check in config interface ip command to block if the interface is portchannel member ([sub module] advance sonic-swss sub module sonic-net#2539) (2 weeks ago) [Sudharsan Dhamal Gopalarathnam]
cff4fed5 - [system-health] Improve code structure of system health CLIs ([sub-module] advance sonic-swss sub-module sonic-net#2453) (2 weeks ago) [Junchao-Mellanox]
488e5714 - Transceiver eeprom dom CLI modification to show output from TRANSCEIVER_DOM_THRESHOLD table (Fix for KeyError: 'DEVICE_NEIGHBOR' when executing 'show interfaces neighbor expected' command sonic-net#2535) (2 weeks ago) [mihirpat1]
07ca5def - sonic-utilities: Update config reload() to verify formatting of an input file ([ntp]: Do not disable reader for error ENOBUFS sonic-net#2529) (2 weeks ago) [Caitlin Choate]
f0f083a2 - [GCU] Add RemoveCreateOnlyDependency Validator/Generator (Enabling Fast-reboot command in s6100 loaded with T0 topo getting Failed unmounting /host error sonic-net#2500) (2 weeks ago) [jingwenxie]
eca0253c - [QoS] Introduce delay to the qos reload flow (Config reload/load_minigraph not clearing State DB sonic-net#2503) (2 weeks ago) [DavidZagury]
35158ee0 - Use github code scanning instead of LGTM ([sub module] sub module sonic-swss-common tracking 201811 branch sonic-net#2530) (2 weeks ago) [Liu Shilong]
682b5cee - Change show kube command default value of insecure key to True ([submodule] update sonic-snmpagent sonic-net#2517) (2 weeks ago) [lixiaoyuner]
ce19e631 - Add db_migrator_constants.py script to setup.py (Add device data for Arista 7060PX/DX4-32 sonic-net#2534) (2 weeks ago) [Vaibhav Hemant Dixit]
0d0c2693 - [drop counters] Fix CLI script for unconfigured PGs ([config] Do not fail for minigraphs which do not have neighbors listed in <Devices> section sonic-net#2518) (2 weeks ago) [Lior Avramov]
2c69d0fd - Update vrf add, del commands for duplicate/non-existing VRFs (solve package build dependency issue sonic-net#2467) (2 weeks ago) [Muhammad Danish]
efc09280 - Port 202012 DB migration changes to newer branches ([vs]: Force10-S6000 buffer settings for virtual switch sonic-net#2515) (2 weeks ago) [Vaibhav Hemant Dixit]
70a15aaa - [VXLAN]Fixing traceback in show remotemac when mac moves during command execution ([build] When generating image version, handle case where current commit has no reachable tags sonic-net#2506) (2 weeks ago) [Sudharsan Dhamal Gopalarathnam]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants