[crm]: Fixed topology check and interface selection for test_crm_fdb_entry

[monipko]

Signed-off-by: Mykhailo Onipko [email protected]

Description of PR

Summary:
Fixes #
Removed hardcoded value "Ethernet0" and replaced by automatic selection depends on topo

Type of change

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

Approach

What is the motivation for this PR?

To add possibility run CRM test on t0-64 topo

How did you do it?

How did you verify/test it?

run test_crm_fdb_entry on t0-64

Any platform specific information?

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

Documentation

[bingwang-ms]

I'm not clear why this case can't run on topo without disabled interface. Could you help to clarify? Thanks!

[monipko]

I'm not clear why this case can't run on topo without disabled interface. Could you help to clarify? Thanks!

@bingwang-ms ,
Current revision of test_crm_fdb_entry limited by t0* topologies only.
As per test scenario - test creates new vlan and adds 1 port to it.
If test will try to add any port(not disabled one) in use(vlan or portchannel member) to the new created vlan it will cause orchagent crash(expected).

[bingwang-ms]

I'm not clear why this case can't run on topo without disabled interface. Could you help to clarify? Thanks!

@bingwang-ms ,
Current revision of test_crm_fdb_entry limited by t0* topologies only.
As per test scenario - test creates new vlan and adds 1 port to it.
If test will try to add any port(not disabled one) in use(vlan or portchannel member) to the new created vlan it will cause orchagent crash(expected).

Thanks. But things look different on my DUT. The test case will always adds Ethernet0 into a nrewly created vlan2, and the Ethernet0 is not disabled on my device.

admin@str-7260cx3-acs-2:~$ show int status
      Interface            Lanes    Speed    MTU    FEC         Alias             Vlan    Oper    Admin             Type    Asym PFC
---------------  ---------------  -------  -----  -----  ------------  ---------------  ------  -------  ---------------  ----------
      Ethernet0            77,78      50G   9100    N/A   Ethernet1/1            trunk      up       up  QSFP28 or later         off
......

And I saw no crash when adding Ethernet0 into vlan2.

admin@str-7260cx3-acs-2:~$ sudo config vlan add 2
admin@str-7260cx3-acs-2:~$ sudo config vlan member add 2 Ethernet0
admin@str-7260cx3-acs-2:~$ show vlan brief
+-----------+-----------------+-------------+----------------+-----------------------+
|   VLAN ID | IP Address      | Ports       | Port Tagging   | DHCP Helper Address   |
+===========+=================+=============+================+=======================+
|         2 |                 | Ethernet0   | tagged         |                       |
+-----------+-----------------+-------------+----------------+-----------------------+
|      1000 | 192.168.0.1/21  | Ethernet0   | untagged       | 192.0.0.1             |

So, is it an issue only on certain platform? Thanks

[monipko]

Thanks. But things look different on my DUT. The test case will always adds Ethernet0 into a nrewly created vlan2, and the Ethernet0 is not disabled on my device.
And I saw no crash when adding Ethernet0 into vlan2.
So, is it an issue only on certain platform? Thanks

Test case's author uses not configured (not participating in the topology) port to avoid possible influence of any traffic on CRM counters.

"Disabled" interface means disabled (not in use) by topology configuration and not disabled on the DUT.
Please compare topo_t0.yml and topo_t0-64.yml especially disabled_host_interfaces sections.

Then if you will try to add Ethernet0(PortChannel0001 member) into a newly created vlan2 on your DUT with applied t0-64 you will get expected crash: orchagent crash when config a PortChannel add into vlan member

[bingwang-ms]

Thanks. But things look different on my DUT. The test case will always adds Ethernet0 into a nrewly created vlan2, and the Ethernet0 is not disabled on my device.
And I saw no crash when adding Ethernet0 into vlan2.
So, is it an issue only on certain platform? Thanks

Test case author use not configured (not participating in the topology) port to avoid possible influence of any traffic on CRM counters.

"Disabled" interface means disabled (not in use) by topology configuration and not disabled on the DUT.
Please compare topo_t0.yml and topo_t0-64.yml especially disabled_host_interfaces sections.

Then if you will try to add Ethernet0(PortChannel0001 member) into a newly created vlan2 on your DUT with applied t0-64 you will get expected crash: orchagent crash when config a PortChannel add into vlan member

Thanks @monipko for your detailed clarification.
The DUT I'm testing against is topo_t0_116, which has no disabled_interface. And the Ethernet0 is already in vlan1000. If I didn't misunderstand, orchagent will crash if I added this interface to a new vlan2. But no crash was observed.
I think this case will be skipped on some topo without disabled_interfaces (like topo_t0_116) if this PR is merged. But the case passes on my DUT. Is this expected?

[bingwang-ms]

IMO, the crash in sonic-net/sonic-buildimage#5387 is because an interface with IP address is added to another vlan. Selecting a disabled interface is a good workaround, but it will result in this case be skipped on some topo. How about select an interface not in PortChannel?

[monipko]

IMO, the crash in Azure/sonic-buildimage#5387 is because an interface with IP address is added to another vlan. Selecting a disabled interface is a good workaround, but it will result in this case be skipped on some topo. How about select an interface not in PortChannel?

@bingwang-ms let me check and make corresponding changes

[monipko]

@bingwang-ms please take a look. Now test applicable for all t0-* topologies.
Port will be selected from the topo's host_interfaces(disabled or not) and not PortChannel members.

[monipko]

@lguohan @wangxin @yxieca do we have any obstacles for this not to be merged?

[bingwang-ms]

@lguohan @wangxin @yxieca do we have any obstacles for this not to be merged?

Sorry for left an open comment. Already reviewd and merged.