[mirroring] Incorrect validation for multiple source ports and direction

[raphaelt-nvidia]

Description

There are two related issues which I am grouping together because the solution of one needs to consider the other.

1. The code in validate_mirror_session_config aims to prohibit the same source port appearing in more than one mirror session. I believe this is undesirable, because some HW allows mirroring the rx direction in one session and tx in another. Solving this would require checking both source port and direction of existing and new sessions.

2. The code mentioned above fails to prevent duplication of source ports when there is more than one of them. For example, suppose 'Ethernet12,Ethernet16,Ethernet20' are source ports in both an old and new session. validate_mirror_session_config breaks the new source list into its components using src_port.split(","), and each of these components is compared in interface_has_mirror_config to the entire list, which doesn't match and the command is accepted.

The combination of these two bugs results in a special case that works well by accident: when two sessions have the same list of source ports, one rx and the other tx. If someone were to fix #2 without fixing #1, this happy case would no longer work. That is why they need to be fixed together.

Steps to reproduce the issue:

Case #1

1. config mirror_session span add ms1 Ethernet0 Ethernet4 rx
2. config mirror_session span add ms2 Ethernet8 Ethernet4 tx

Case #2

1. config mirror_session span add ms1 Ethernet0 Ethernet12,Ethernet16,Ethernet20 rx
2. config mirror_session span add ms3 Ethernet24 Ethernet12,Ethernet16,Ethernet20 both

Describe the results you received:

Case #1

Error: Source Interface Ethernet4 already has mirror config

Case #2

No error at CLI level, errors in syncd

Describe the results you expected:

Case #1

Commands should be accepted at CLI level. If HW does not support mirroring tx and rx of same port in different sessions, this should be rejected with an error logged by orchagent.

Case #2

Reject command with a message that rx direction of at least one of the ports is already mirrored.

Output of show version:

SONiC Software Version: SONiC.build4.0-223f18f0f
Distribution: Debian 10.9
Kernel: 4.19.0-12-2-amd64
Build commit: 223f18f0f
Build date: Tue Jun 15 09:43:57 UTC 2021
Built by: stephens@arc-build-server-2

Platform: x86_64-mlnx_msn3420-r0
HwSKU: ACS-MSN3420
ASIC: mellanox
ASIC Count: 1
Serial Number: None
Model Number: None
Hardware Revision: None
Uptime: 13:23:59 up 5:10, 1 user, load average: 0.29, 0.28, 0.54

Output of show techsupport:

N/A as problem is fully diagnosed.

Additional information you deem important (e.g. issue happens only occasionally):

[zhangyanzhao]

Expect CLI level validation. @jamiedawsonyoung will take a look

[raphaelt-nvidia]

Any news?

[moshemos]

@jamiedawsonyoung as I saw you asked to take a look, any update?

[zhangyanzhao]

@jamiedawsonyoung Jamie assign this issue to you and please help to take a look. Let us know if you are not the right person anymore. Thanks.

[jamiedawsonyoung]

@jamiedawsonyoung Jamie assign this issue to you and please help to take a look. Let us know if you are not the right person anymore. Thanks.

Sorry, not me. Not sure how I got assigned here. I'm a design manager ☺️

[raphaelt-nvidia]

Rupesh Kumar of Broadcom is working on this and supplied ETA of 10th December.

[dprital]

Do we have an update on this issue ?

[zhangyanzhao]

@rupesh-k can you please help to provide an update? Thanks.

[rupesh-k]

Fix in progress. ETA 02/04

[raphaelt-nvidia]

@rupesh-k do you have an update?

[rupesh-k]

Adding CLI validation to check both port and direction. Will update once i raise PR.

[raphaelt-nvidia]

@rupesh-k , I see a comment linking this bug with PR #2053. AFAIK, that PR addresses bug #7989. Am I missing something?