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

[show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 file for FRR routing stack #884

Merged
merged 1 commit into from
Apr 21, 2020
Merged

[show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 file for FRR routing stack #884

merged 1 commit into from
Apr 21, 2020

Conversation

tsvanduyn
Copy link
Contributor

… file for FRR routing stack.

- What I did
I created new file bgp_frr_v4.py for the FRR routing-stack and changed the code to use this file instead of the pass through to vtysh for "show bgp". This now requires the use of "show ip bgp" like the bgp_quagga_v4.py file and which is standardized on most networking vendors.

- How I did it
I added the file bgp_frr_v4.py in the same directory as the bgp_quagga_v4.py file and updated the main.py for the show commands to use this file. This is the same as how we do bgp_quagga_v4.py today.

- How to verify it
If you are on a platform running FRR and you issue the "show bgp ?" command then it would be piped through to the FRR vtysh shell and show the help output for that command like this:

admin@str-s6000-acs-11:~$ show ver

SONiC Software Version: SONiC.master.254-a02255e2
Distribution: Debian 9.12
Kernel: 4.9.0-11-2-amd64
Build commit: a02255e2
Build date: Wed Apr 15 16:03:40 UTC 2020
Built by: johnar@jenkins-worker-11

Platform: x86_64-dell_s6000_s1220-r0
HwSKU: Force10-S6000
ASIC: broadcom
/usr/bin/decode-syseeprom : ERROR : Platform did not indicate serial number
Serial Number: 
Uptime: 18:29:47 up 5 days, 20:05,  1 user,  load average: 1.79, 1.80, 1.76

Docker images:
REPOSITORY                    TAG                   IMAGE ID            SIZE
docker-syncd-brcm             latest                72e9f881dfaa        436MB
docker-syncd-brcm             master.254-a02255e2   72e9f881dfaa        436MB
docker-nat                    latest                7868b81fde8e        310MB
docker-nat                    master.254-a02255e2   7868b81fde8e        310MB
docker-router-advertiser      latest                5bcbbdfc3a62        288MB
docker-router-advertiser      master.254-a02255e2   5bcbbdfc3a62        288MB
docker-platform-monitor       latest                695c3c3d38f6        335MB
docker-platform-monitor       master.254-a02255e2   695c3c3d38f6        335MB
docker-database               latest                cf28a5c45316        287MB
docker-database               master.254-a02255e2   cf28a5c45316        287MB
docker-orchagent              latest                b4c0cb7019b8        326MB
docker-orchagent              master.254-a02255e2   b4c0cb7019b8        326MB
docker-dhcp-relay             latest                29e5ec287632        297MB
docker-dhcp-relay             master.254-a02255e2   29e5ec287632        297MB
docker-sonic-telemetry        latest                ddb602ef7015        349MB
docker-sonic-telemetry        master.254-a02255e2   ddb602ef7015        349MB
docker-sonic-mgmt-framework   latest                f51f272754ea        427MB
docker-sonic-mgmt-framework   master.254-a02255e2   f51f272754ea        427MB
docker-fpm-frr                latest                9bd44bbb32e1        328MB
docker-fpm-frr                master.254-a02255e2   9bd44bbb32e1        328MB
docker-sflow                  latest                2ba9abcdb5f5        309MB
docker-sflow                  master.254-a02255e2   2ba9abcdb5f5        309MB
docker-iccpd                  latest                4ac6308a4232        309MB
docker-iccpd                  master.254-a02255e2   4ac6308a4232        309MB
docker-lldp-sv2               latest                32ed9f853e8d        305MB
docker-lldp-sv2               master.254-a02255e2   32ed9f853e8d        305MB
docker-snmp-sv2               latest                5b4d4135c2ba        345MB
docker-snmp-sv2               master.254-a02255e2   5b4d4135c2ba        345MB
docker-teamd                  latest                23de6e31c4e6        308MB
docker-teamd                  master.254-a02255e2   23de6e31c4e6        308MB

So you can see this device has the "docker-fpm-frr" container.
Before changes:

admin@str-s6000-acs-11:~$ show bgp -h
Usage: show bgp [OPTIONS] [BGP_ARGS]...

  Show BGP information

Options:
  --verbose       Enable verbose output
  -?, -h, --help  Show this message and exit.


While if I put the “show bgp ?” I get all the BGP Args because of the way this command is passed from the Sonic CLI to the FRR (vtysh).

admin@str-s6000-acs-11:~$ show bgp ?
  <cr>                  
  A.B.C.D               Network in the BGP routing table to display
  A.B.C.D/M             IPv4 prefix
  X:X::X:X              Network in the BGP routing table to display
  X:X::X:X/M            IPv6 prefix
  as-path-access-list   List AS path access lists
  attribute-info        List all bgp attribute information
  cidr-only             Display only routes with non-natural netmasks
  community             Display routes matching the communities
  community-info        List all bgp community information
  community-list        Display routes matching the community-list
  dampening             Display detailed information about dampening
  detail                Detailed information on flowspec entries
  extcommunity-list     List extended-community list
  filter-list           Display routes conforming to the filter-list
  import-check-table    BGP import check table
  ipv4                  Address Family
  ipv6                  Address Family
  json                  JavaScript Object Notation
  l2vpn                 Layer 2 Virtual Private Network
  large-community       Display routes matching the large-communities
  large-community-list  Display routes matching the large-community-list
  martian               martian next-hops
  memory                Global BGP memory statistics
  multicast             Address Family modifier
  neighbors             Detailed information on TCP and BGP neighbor connections
  nexthop               BGP nexthop table
  paths                 Path information
  peer-group            Detailed information on BGP peer groups
  peerhash              Display information about the BGP peerhash
  prefix-list           Display routes conforming to the prefix-list
  regexp                Display routes matching the AS path regular expression
  route-leak            Route leaking information
  route-map             Display routes matching the route-map
  statistics            BGP RIB advertisement statistics
  summary               Summary of BGP neighbor status
  unicast               Address Family modifier
  update-groups         Detailed info about dynamic update groups
  view                  BGP view
  views                 Show the defined BGP views
  vpn                   Address Family modifier
  vrf                   BGP VRF
  vrfs                  Show BGP VRFs

Once you make the change you would see this:

admin@str-s6000-acs-11:~$ show bgp ?
Usage: show [OPTIONS] COMMAND [ARGS]...

Error: No such command "bgp".
admin@str-s6000-acs-11:~$ 


admin@str-s6000-acs-11:~$ show bgp su
Usage: show [OPTIONS] COMMAND [ARGS]...

Error: No such command "bgp".
admin@str-s6000-acs-11:~$

As you can see the "show bgp ?" is no longer piped through to the FRR vtysh shell.
This behavior now mimics the behavior that we see on the bgp_quagga_v4.py routing-stack and forcing all help commands to go through the SONIC CLI. This would standardize the behavior of the two stacks and give the SONIC CLI more control.

Also the bgp_frr_v4.py file commands work as expected.

admin@str-s6000-acs-11:~$ show ip bgp summary 

IPv4 Unicast Summary:
BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0
BGP table version 1
RIB entries 1, using 184 bytes of memory
Peers 32, using 654 KiB of memory
Peer groups 2, using 128 bytes of memory

Neighbor        V         AS MsgRcvd MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd   NeighborName
10.0.0.1        4      65200       0       0        0    0    0    never       Active   ARISTA01T2
10.0.0.3        4      65200       0       0        0    0    0    never       Active   ARISTA02T2
10.0.0.5        4      65200       0       0        0    0    0    never       Active   ARISTA03T2
10.0.0.7        4      65200       0       0        0    0    0    never       Active   ARISTA04T2
10.0.0.9        4      65200       0       0        0    0    0    never       Active   ARISTA05T2
10.0.0.11       4      65200       0       0        0    0    0    never       Active   ARISTA06T2
10.0.0.13       4      65200       0       0        0    0    0    never       Active   ARISTA07T2
10.0.0.15       4      65200       0       0        0    0    0    never       Active   ARISTA08T2
10.0.0.17       4      65200       0       0        0    0    0    never       Active   ARISTA09T2
10.0.0.19       4      65200       0       0        0    0    0    never       Active   ARISTA10T2
10.0.0.21       4      65200       0       0        0    0    0    never       Active   ARISTA11T2
10.0.0.23       4      65200       0       0        0    0    0    never       Active   ARISTA12T2
10.0.0.25       4      65200       0       0        0    0    0    never       Active   ARISTA13T2
10.0.0.27       4      65200       0       0        0    0    0    never       Active   ARISTA14T2
10.0.0.29       4      65200       0       0        0    0    0    never       Active   ARISTA15T2
10.0.0.31       4      65200       0       0        0    0    0    never       Active   ARISTA16T2
10.0.0.33       4      64001       0       0        0    0    0    never       Active   ARISTA01T0
10.0.0.35       4      64002       0       0        0    0    0    never       Active   ARISTA02T0
10.0.0.37       4      64003       0       0        0    0    0    never       Active   ARISTA03T0
10.0.0.39       4      64004       0       0        0    0    0    never       Active   ARISTA04T0
10.0.0.41       4      64005       0       0        0    0    0    never       Active   ARISTA05T0
10.0.0.43       4      64006       0       0        0    0    0    never       Active   ARISTA06T0
10.0.0.45       4      64007       0       0        0    0    0    never       Active   ARISTA07T0
10.0.0.47       4      64008       0       0        0    0    0    never       Active   ARISTA08T0
10.0.0.49       4      64009       0       0        0    0    0    never       Active   ARISTA09T0
10.0.0.51       4      64010       0       0        0    0    0    never       Active   ARISTA10T0
10.0.0.53       4      64011       0       0        0    0    0    never       Active   ARISTA11T0
10.0.0.55       4      64012       0       0        0    0    0    never       Active   ARISTA12T0
10.0.0.57       4      64013       0       0        0    0    0    never       Active   ARISTA13T0
10.0.0.59       4      64014       0       0        0    0    0    never       Active   ARISTA14T0
10.0.0.61       4      64015       0       0        0    0    0    never       Active   ARISTA15T0
10.0.0.63       4      64016       0       0        0    0    0    never       Active   ARISTA16T0

Total number of neighbors 32
admin@str-s6000-acs-11:~$ show ip bgp -h
Usage: show ip bgp [OPTIONS] COMMAND [ARGS]...

  Show IPv4 BGP (Border Gateway Protocol) information

Options:
  -?, -h, --help  Show this message and exit.

Commands:
  neighbors  Show IP (IPv4) BGP neighbors
  summary    Show summarized information of IPv4 BGP state
admin@str-s6000-acs-11:~$

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

admin@str-s6000-acs-11:~$ show bgp ?
  <cr>                  
  A.B.C.D               Network in the BGP routing table to display
  A.B.C.D/M             IPv4 prefix
  X:X::X:X              Network in the BGP routing table to display
  X:X::X:X/M            IPv6 prefix
  as-path-access-list   List AS path access lists

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

admin@str-s6000-acs-11:~$ show bgp ?
Usage: show [OPTIONS] COMMAND [ARGS]...

Error: No such command "bgp".
admin@str-s6000-acs-11:~$

@lgtm-com
Copy link

lgtm-com bot commented Apr 21, 2020

This pull request introduces 2 alerts and fixes 1 when merging df8a53f into a87173b - view on LGTM.com

new alerts:

  • 2 for Module-level cyclic import

fixed alerts:

  • 1 for Module-level cyclic import

@pavel-shirshov pavel-shirshov self-requested a review April 21, 2020 21:03
@pavel-shirshov pavel-shirshov self-assigned this Apr 21, 2020
@pavel-shirshov pavel-shirshov requested a review from jleveque April 21, 2020 21:04
pavel-shirshov
pavel-shirshov approved these changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@pavel-shirshov pavel-shirshov 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 to me. Please wait Joe's review

@jleveque jleveque changed the title Updating BGP to use only 'show ip bgp' as the base and use bgp_frr_v4… [show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 file for FRR routing stack Apr 21, 2020
@jleveque
Copy link
Contributor

FYI, I rephrased the PR title.

@jleveque
Copy link
Contributor

@tsvanduyn: FYI, we have added a new "LGTM" PR checker, which checks for code issues. In the future, I suggest you fix the issues it finds. However, we are currently expecting these "Module-level cyclic import" issues that it flagged with the way we are currently importing separate files (e.g., bgp_frr_v4.py, bgp_quagga_v4.py, etc.) into our show/config main.py files. We need to investigate a way to restructure the code to allow for importing other files without these warnings, as I would like to split these large main.py files into separate files.

@jleveque jleveque merged commit c5a60ce into sonic-net:master Apr 21, 2020
abdosi pushed a commit that referenced this pull request Apr 27, 2020
@tsvanduyn @abdosi
[show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 fil…
bdcfe80 
…e for FRR routing stack (#884)

Co-authored-by: Travis Van Duyn <trvanduy@microsoft.com>
abdosi pushed a commit to abdosi/sonic-utilities that referenced this pull request Aug 4, 2020
@tsvanduyn @abdosi
[show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 fil…
c4c14f3 
…e for FRR routing stack (sonic-net#884)

Co-authored-by: Travis Van Duyn <trvanduy@microsoft.com>
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 28, 2022
@abdosi
[submodule update] sonic-utilities with PR's
827aab7 
[Vxlan] : adding show vnet/vxlan cmds (sonic-net#880)
[show][bgp] Use only 'show ip bgp' as the base and use bgp_frr_v4 file
for FRR routing stack (sonic-net#884)
[fast reboot] set a fast-reboot DB flag (sonic-net#887)
[show] Add 'ip/ipv6 bgp network' commands (sonic-net#888)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavel-shirshov pavel-shirshov pavel-shirshov approved these changes

@jleveque jleveque jleveque approved these changes

Assignees

@pavel-shirshov pavel-shirshov

Labels
Enhancement Included in 201911 Branch Request for 201911 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tsvanduyn @jleveque @pavel-shirshov @abdosi