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

Fixing 'show ip bgp neighbor <ip>' in frr unified config mode #3738

Merged
merged 2 commits into from
Feb 8, 2025

Conversation

kalash-nexthop
Copy link
Contributor

@kalash-nexthop kalash-nexthop commented Jan 28, 2025

Fixing show ip bgp neighbor neighbor_ip command in frr unified config mode.

fixes #3736

What I did

Currently this Cli fails for a valid neighbor in frr unified config mode, even when show ip bgp neighbors shows this neighbor correctly.

rtr1(bash)$ show ip bgp neighbor 10.1.0.2
Usage: show ip bgp neighbor [OPTIONS] [IPADDRESS] [[routes|advertised-
                            routes|received-routes]]
Try "show ip bgp neighbor -h" for help.

Error:  Bgp neighbor 10.1.0.2 not configured

The reason for this is that while trying to make sure neighbor exists in config_db, the code looks at BGP_NEIGHBOR table with key as neighbor_ip, whereas in frr unified config mode, the key is vrfname|neighbor_ip. I'm fixing this behavior.

A nice side effect of this fix is that neighbor name now shows up in show ip bgp summary output, which was previously missing when frr unified config mode was enabled.

How I did it

This PR fixes this issue by making sure if neighbor_ip is not present in the BGP_NEIGHBOR table, then it also checks for the entry using *|neighbor_ip as the key.
The modified config_db.json mock table forced me to fix another issue in show ip bgp summary where neighbor name was not being shown in unified config mgmt mode. I couldn't segregate that fix as otherwise the unit tests won't pass.

How to verify it

  • Verified manually that the changes work both with and without frr unified config mode.
  • Also verified in unit test by modifying the mocked config_db. The tests/mock_tables has mocked config_db json files, and I modified one v4 and one v6 neighbor (for both single and multi asic case) to have the key as default|ip. This makes sure that unit tests work in both the cases, when neighbor is present with neighbor_ip as the key, as well as when it is present with vrfname|neighbor_ip as the key.
  • The show_bgp_neighbor_test.py testcases fail without my change (with the new modified mock_tables/config_db.json) and pass with them.

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

rtr1(bash)$ show ip bgp neighbor 10.1.0.2
Usage: show ip bgp neighbor [OPTIONS] [IPADDRESS] [[routes|advertised-
                            routes|received-routes]]
Try "show ip bgp neighbor -h" for help.

Error:  Bgp neighbor 10.1.0.2 not configured

rtr1(bash)$ show ip bgp summary

IPv4 Unicast Summary:
BGP router identifier 10.0.0.1, local AS number 65100 vrf-id 0
BGP table version 5
RIB entries 5, using 640 bytes of memory
Peers 1, using 20592 KiB of memory
Peer groups 1, using 64 bytes of memory


Neighbhor      V     AS    MsgRcvd    MsgSent    TblVer    InQ    OutQ  Up/Down      State/PfxRcd  NeighborName
-----------  ---  -----  ---------  ---------  --------  -----  ------  ---------  --------------  --------------
10.1.0.2       4  65001       7168       7167         5      0       0  4d23h24m                2

Total number of neighbors 1

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

rtr1(bash)$ show ip bgp neighbors 10.1.0.2

BGP neighbor is 10.1.0.2, remote AS 65001, local AS 65100, external link
  Local Role: undefined
  Remote Role: undefined
 Description: rtr2
Hostname: sonic
 Member of peer-group peer4 for session parameters
  BGP version 4, remote router ID 10.0.0.3, local router ID 10.0.0.1
  BGP state = Established, up for 18:09:16
  Last read 00:00:16, Last write 00:00:16
  Hold time is 180 seconds, keepalive interval is 60 seconds
  Configured hold time is 180 seconds, keepalive interval is 60 seconds
  Configured tcp-mss is 0, synced tcp-mss is 1496
  Configured conditional advertisements interval is 60 seconds
...
...

rtr1(bash)$ show ip bgp sum

IPv4 Unicast Summary:
BGP router identifier 10.0.0.1, local AS number 65100 vrf-id 0
BGP table version 5
RIB entries 5, using 640 bytes of memory
Peers 1, using 20592 KiB of memory
Peer groups 1, using 64 bytes of memory


Neighbhor      V     AS    MsgRcvd    MsgSent    TblVer    InQ    OutQ  Up/Down      State/PfxRcd  NeighborName
-----------  ---  -----  ---------  ---------  --------  -----  ------  ---------  --------------  --------------
10.1.0.2       4  65001       7204       7203         5      0       0  5d00h00m                2  rtr2

Total number of neighbors 1

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

linux-foundation-easycla bot commented Jan 28, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…ng sure to get the neighbor key from config_db either in 'x.x.x.x' format or alternatively in 'default|x.x.x.x' format as is the case with unified mode
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kalash-nexthop
Copy link
Contributor Author

@FengPan-Frank May I request a review please?

Copy link
Contributor

@FengPan-Frank FengPan-Frank left a comment

Choose a reason for hiding this comment

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

LGTM

@FengPan-Frank FengPan-Frank merged commit f4e6e5b into sonic-net:master Feb 8, 2025
7 checks passed
@kalash-nexthop
Copy link
Contributor Author

Thanks @FengPan-Frank . Appreciate the review.

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.

[bug] 'show ip bgp neighbor <ip>' fails for valid neighbor in frr unified config mode
3 participants