Skip to content

MSTP Traps for Control Packet#1853

Closed
ridahanif96 wants to merge 4 commits intoopencomputeproject:masterfrom
ridahanif96:mstp_traps
Closed

MSTP Traps for Control Packet#1853
ridahanif96 wants to merge 4 commits intoopencomputeproject:masterfrom
ridahanif96:mstp_traps

Conversation

@ridahanif96
Copy link

@ridahanif96 ridahanif96 commented Aug 18, 2023

This PR brings MSTP Trap for control packets. Modified saihostif.h for adding MSTP Trap

This is based on MSTP Support in SONIC

Signed-off-by: Rida Hanif [email protected]

ridahanif96 and others added 4 commits August 18, 2023 14:37
Signed-off-by: Rida Hanif <[email protected]>
Signed-off-by: Rida Hanif <[email protected]>
Signed-off-by: Rida Hanif <[email protected]>
Signed-off-by: Rida Hanif <[email protected]>
@ridahanif96 ridahanif96 marked this pull request as ready for review August 18, 2023 09:46
@ridahanif96
Copy link
Author

ridahanif96 commented Aug 18, 2023

@rck-innovium Hi, this PR is created in reference to MSTP Traps. During the community review it was discussed that we don't need this trap as switch internal STP trap will be enough to handle control packet traps. Can you please help with this. Thanks.

@rlhui
Copy link
Collaborator

rlhui commented Aug 18, 2023

@rck-innovium Hi, this PR is created in reference to MSTP Traps. During the community review it was discussed that we don't need this trap as switch internal STP trap will be enough to handle control packet traps. Can you please help with this. Thanks.

Hi @ridahanif96 , what exact help is needed from @rck-innovium ? thanks.

@ridahanif96
Copy link
Author

@rck-innovium Hi, this PR is created in reference to MSTP Traps. During the community review it was discussed that we don't need this trap as switch internal STP trap will be enough to handle control packet traps. Can you please help with this. Thanks.

Hi @ridahanif96 , what exact help is needed from @rck-innovium ? thanks.

Hi @rlhui , thanks for help.
Earlier, we purpose a separte trap for MSTP Control packet. Later, during HLD review @rck-innovium suggested that there is no need for this new trap.We can use SAI_HOSTIF_TRAP_TYPE_STP, as DMAC and LLC fields are identical for STP and MSTP. If we create a separate trap types for STP and MSTP , it would require the hardware to parse/match the BPDU protocol identifier.
We also agree with this, just created this PR for getting more suggestions from @rck-innovium so we can proceed further with our work as per suggestions/guidance.

@ridahanif96 ridahanif96 marked this pull request as draft August 19, 2023 08:35
Copy link
Contributor

@rck-innovium rck-innovium left a comment

Choose a reason for hiding this comment

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

@ridahanif96 

Yes. We should be able to use the existing STP trap for STP, RSTP and MSTP.

Can you please explain about the other change in this PR about SAI_BRIDGE_PORT_TAGGING_MODE_HYBRID.

@ridahanif96
Copy link
Author

@ridahanif96

Yes. We should be able to use the existing STP trap for STP, RSTP and MSTP.

Can you please explain about the other change in this PR about SAI_BRIDGE_PORT_TAGGING_MODE_HYBRID.

Thanks @rck-innovium , I will close this PR. The other change in this PR about SAI_BRIDGE_PORT_TAGGING_MODE_HYBRID is added by mistake with this PR. Actually we have another PR related to feature Switchport mode hybrid. In that PR we proposed a change in saibridge.h for bridge tagging mode hybrid to support both tagged and untagged traffic.

@ridahanif96
Copy link
Author

It has been decided in SAI group call (08/24/23) that We should be able to use the existing STP trap for STP, RSTP and MSTP. Therefore changes proposed by this PR will not be required. As discussed in SAI call, this PR will be closed. Thanks!

@ridahanif96
Copy link
Author

Closing PR!

@ridahanif96 ridahanif96 closed this Nov 9, 2023
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.

4 participants