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.

Sign up for GitHub

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

Jump to bottom

Make 'show ip bgp summary' work even when we don't have any peer groups #3739

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

Make 'show ip bgp summary' work even when we don't have any peer groups #3739

wants to merge 2 commits into from

Conversation

kalash-nexthop
Copy link

@kalash-nexthop kalash-nexthop commented Jan 28, 2025 •
edited
Loading

Fix show ip bgp summary output when no peer groups are configured

fixes #3737

What I did

Currently when we don't have any peer groups configured, show ip bgp summary fails

sonic(bash)$ show ip bgp summary
Usage: show ip bgp summary [OPTIONS]
Try "show ip bgp summary -h" for help.

Error: peerGroupCount missing in the bgp_summary

vtysh gives more meaningful output in this case:

sonic(bash)$ vtysh

Hello, this is FRRouting (version 10.0.1).
Copyright 1996-2005 Kunihiro Ishiguro, et al.

2025/01/27 20:21:52 [YDG3W-JND95] FD Limit set: 1048576 is stupidly large.  Is this what you intended?  Consider using
 --limit-fds also limiting size to 100000
sonic# show ip bgp summary

IPv4 Unicast Summary:
BGP router identifier 10.0.0.3, local AS number 65001 VRF default vrf-id 0
BGP table version 48
RIB entries 3, using 384 bytes of memory
Peers 1, using 20 KiB of memory

Neighbor        V         AS   MsgRcvd   MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd   PfxSnt Desc
rtr1(10.1.0.1)  4      65100     14459     14624       48    0    0 3d18h59m            0        2 rtr1

Total number of neighbors 1

Changing show ip bgp summary to give such meaningful output in this case as well.

How I did it

Avoid getting KeyError while accessing cmd_output['peerGroupCount'] by using cmd_output.get('peerGroupCount', 0).

How to verify it

  1. Manually verified it on a SONiC dut.

  2. The unit tests fail without my changes and pass with them.

Please note that I'm printing 0 peer groups (Peer groups 0, using 0 bytes of memory) instead of removing the entire line, just in case some existing user scripts always expect this line to be present in the output.

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

sonic(bash)$ show ip bgp summary
Usage: show ip bgp summary [OPTIONS]
Try "show ip bgp summary -h" for help.

Error: peerGroupCount missing in the bgp_summary

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

sonic(bash)$ show ip bgp sum

IPv4 Unicast Summary:
BGP router identifier 10.0.0.3, local AS number 65001 vrf-id 0
BGP table version 48
RIB entries 3, using 384 bytes of memory
Peers 1, using 20592 KiB of memory
Peer groups 0, using 0 bytes of memory


Neighbhor      V     AS    MsgRcvd    MsgSent    TblVer    InQ    OutQ  Up/Down      State/PfxRcd  NeighborName
-----------  ---  -----  ---------  ---------  --------  -----  ------  ---------  --------------  --------------
10.1.0.1       4  65100      14691      14856        48      0       0  3d22h51m                0

Total number of neighbors 1
sonic(bash)$

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kalash-nexthop kalash-nexthop changed the title Make 'show ip bgp summary' work even when we don't have any peer grou… Make 'show ip bgp summary' work even when we don't have any peer groups Jan 29, 2025
@kalash-nexthop kalash-nexthop force-pushed the show-bgp-summ-no-peergroup branch from b4a6f53 to 6f104b8 Compare January 30, 2025 00:03
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

FengPan-Frank
FengPan-Frank approved these changes Jan 30, 2025
View reviewed changes
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

@kalash-nexthop
Copy link
Author

LGTM

Thanks Feng! Much appreciated.

@kalash-nexthop
Copy link
Author

kalash-nexthop commented Jan 30, 2025 •
edited
Loading

@FengPan-Frank This is my first sonic PR and I don't know how to proceed with merge now (my apologies but I will get familiar with the merge process as part of this PR). Would you or someone in your team (with write permissions to this repo) do the PR merge for me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FengPan-Frank FengPan-Frank FengPan-Frank approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bug] 'show ip bgp summary' command doesn't work when no peer-group is configured
3 participants
@kalash-nexthop @mssonicbld @FengPan-Frank