Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue#2583 : Fix for VXLAN tunnel port FDBs not displayed in 'show mac' #2584

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

praveenraja1
Copy link

What I did

The 'fdbshow' script did not have the logic to check and use 'SAI_FDB_ENTRY_ATTR_ENDPOINT_IP' which used for FDB entries learnt on tunnel port, it tried to look up 'br_port_id' from oid_map which was failing.
Added logic to use 'SAI_FDB_ENTRY_ATTR_ENDPOINT_IP' for vxlan tunnel entries and fallback to the port_id lookup for normal fdb thats leant on ports.
Tested with a combination of VXLAN tunnel and normal port macs.

How I did it

Added logic to logic to look for 'SAI_FDB_ENTRY_ATTR_ENDPOINT_IP' in the ASIC_DB entry and use it for 'if_name ' , if it doesnt exists then legacy code flow is maintained.

How to verify it

#2583

Has been raised for this.

'show mac' with VXLAN tunnel fdbs present.

Previous command output (if the output of a command-line utility has changed)

root@sonic:~# show mac
No. Vlan MacAddress Port Type


1      77  0C:48:C6:C3:30:F8  Ethernet24  Dynamic

Total number of entries 1
root@sonic:~#

New command output (if the output of a command-line utility has changed)

root@sonic:~# show mac
No. Vlan MacAddress Port Type


1      30  72:E5:BE:90:A6:1C  1.1.1.1     Static
2      30  F6:2C:C6:50:FF:F2  1.1.1.1     Static
3      30  B2:14:7E:B4:05:3C  1.1.1.1     Static
4      77  0C:48:C6:C3:30:F8  Ethernet24  Dynamic

Total number of entries 4
root@sonic:~#

Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

looks good, please add unit tests

Copy link
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

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

The remote MACs are supposed to be displayed through the command show vxlan remotemac as per behavior today.
"show mac" command displays only local mac. Why do we need to change show mac?
@srj102 @adyeung Can someone from Broadcom please provide feedback?

@praveenraja1
Copy link
Author

show vxlan remotemac

The remote MACs are supposed to be displayed through the command show vxlan remotemac as per behavior today. "show mac" command displays only local mac. Why do we need to change show mac? @srj102 @adyeung Can someone from Broadcom please provide feedback?

Thanks for the info, i do see that 'show vxlan remotemac' displays all the macs learnt on vxlan tunnel. Will wait for the reply from Broadcom and then discard this review if this is not required.

@srj102
Copy link
Contributor

srj102 commented Jan 9, 2023

currently show vxlan remotemac displays all the MACs from the VXLAN_FDB APP DB table.

There is no command which gives a global view of all the macs learnt on the switch.
This enhancement helps with the global view.

we could also add an option to display only the local macs,
show mac -local.

@praveenraja1
Copy link
Author

currently show vxlan remotemac displays all the MACs from the VXLAN_FDB APP DB table.

There is no command which gives a global view of all the macs learnt on the switch. This enhancement helps with the global view.

we could also add an option to display only the local macs, show mac -local.

Added this option as requested.
Show_fdb_test.txt

@maipbui
Copy link
Contributor

maipbui commented Jun 29, 2023

@praveenraja1 Please merge latest master code to trigger Semgrep checker.

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.

5 participants