bgpd: add missing white-space between route short status and network … #15525

campescassiano · 2024-03-12T01:33:50Z

…columns

When running show ip bgp command, the 'route short status' and
'network' columns do not have white-space between them.

Old show:
    Network          Next Hop            Metric LocPrf Weight Path
 *>i1.1.1.1/32       10.1.12.111              0    100      0 i

New show:
     Network          Next Hop            Metric LocPrf Weight Path
 *>i 1.1.1.1/32       10.1.12.111              0    100      0 i

Added white-space to enhance readability between them.

ton31337 · 2024-03-12T06:23:02Z

How it looks if we have multiple flags for a prefix and if we don't have at all? For instance, best/suppressed/ibgp. Can you show the examples?

campescassiano · 2024-03-19T01:02:03Z

In the source code, we can verify that the route short status column can have at most 4 flags, each flag represented by its character if set or by white space, otherwise.

Here is a code snipped that prints the respective character for each flag that is set, or white space otherwise:

        //--> Snipped from: https://github.com/FRRouting/frr/blob/master/bgpd/bgp_route.c#L9005
	/* RPKI validation state */
	rpki_state =
		hook_call(bgp_rpki_prefix_status, path->peer, path->attr, p);

	if (rpki_state == RPKI_VALID)
		vty_out(vty, "V");
	else if (rpki_state == RPKI_INVALID)
		vty_out(vty, "I");
	else if (rpki_state == RPKI_NOTFOUND)
		vty_out(vty, "N");
	else
		vty_out(vty, " ");

	/* Route status display. */
	if (CHECK_FLAG(path->flags, BGP_PATH_REMOVED))
		vty_out(vty, "R");
	else if (CHECK_FLAG(path->flags, BGP_PATH_STALE))
		vty_out(vty, "S");
	else if (bgp_path_suppressed(path))
		vty_out(vty, "s");
	else if (CHECK_FLAG(path->flags, BGP_PATH_VALID)
		 && !CHECK_FLAG(path->flags, BGP_PATH_HISTORY))
		vty_out(vty, "*");
	else
		vty_out(vty, " ");

	/* Selected */
	if (CHECK_FLAG(path->flags, BGP_PATH_HISTORY))
		vty_out(vty, "h");
	else if (CHECK_FLAG(path->flags, BGP_PATH_DAMPED))
		vty_out(vty, "d");
	else if (CHECK_FLAG(path->flags, BGP_PATH_SELECTED))
		vty_out(vty, ">");
	else if (CHECK_FLAG(path->flags, BGP_PATH_MULTIPATH))
		vty_out(vty, "=");
	else
		vty_out(vty, " ");

	/* Internal route. */
	if (path->peer && (path->peer->as)
	    && (path->peer->as == path->peer->local_as))
		vty_out(vty, "i");
	else
		vty_out(vty, " ");

As you can see, it is ensured that each column that represents each flag will be either set or white-spaced.

So, the missing white space solves this problem, and follows the same idea from CISCO's show ip bgp command output.

To validate the fix, I've created two docker containers each being one router:

R1: running the fix
R2: running the master branch

Output of the R1 with the fix

R1# sh ip bgp
BGP table version is 6, local router ID is 1.1.1.1, vrf id 0
Default local pref 100, local AS 100
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

     Network          Next Hop            Metric LocPrf Weight Path
     1.1.1.1/32       0.0.0.0                  0         32768 i
 *>  191.168.100.110/32
                    0.0.0.0                  0         32768 i
 *>i 192.168.100.101/32
                    10.111.12.102            0    100      0 i

Displayed 3 routes and 3 total paths
R1#

Output of the R2 running the master branch

R2(config-router)# do sh ip bgp 
BGP table version is 6, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 100
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

    Network          Next Hop            Metric LocPrf Weight Path
    2.2.2.2/32       0.0.0.0                  0         32768 i
 *>i191.168.100.110/32
                    10.111.12.101            0    100      0 i
    192.168.100.100/32
                    0.0.0.0                  0         32768 i
 *> 192.168.100.101/32
                    0.0.0.0                  0         32768 i

Displayed  4 routes and 4 total paths

riw777

looks good ... do we need to have a transition period for this?

…columns When running `show ip bgp` command, the 'route short status' and 'network' columns do not have white-space between them. Old show: Network Next Hop Metric LocPrf Weight Path *>i1.1.1.1/32 10.1.12.111 0 100 0 i New show: Network Next Hop Metric LocPrf Weight Path *>i 1.1.1.1/32 10.1.12.111 0 100 0 i Added white-space to enhance readability between them. Signed-off-by: Cassiano Campes <[email protected]>

Updated the topotests to contemplate the new `show ip bgp` outputs. Signed-off-by: Cassiano Campes <[email protected]>

Topotests changed to consider the extra wite space added in "show ip bgp" output. Signed-off-by: Cassiano Campes <[email protected]>

campescassiano · 2024-03-20T17:15:31Z

@riw777 and @ton31337 please check once again. I adjusted the CI tests to address the new changes. All tests are now passing.

ton31337 · 2024-03-21T07:31:40Z

Still, I'm yet concerned about the output with various flags. Also, I'm curious how it looks if we have RPKI enabled for this BGP instance. Can you test and show the outputs?

campescassiano · 2024-03-21T22:29:35Z

@ton31337 , I am not a BGP expert, so I could not find an easy way to test the RPKI.

Thus, I took a different way to test it, I decided to use GDB to manually set the flag that I could not reach via configuration.

Below I explain the: (1) scenario; (2) configurations used; (3) shows without using the gdb; (4) the show where gdb sets the flag.

Scenario

I tested it using two docker containers, where R1 is the container building from source; and R2 is the container with the original FRR 9.1 version.

R1 version

This one is where I built from source with the proposed patch:

R1# sh ver 
FRRouting 10.1-dev (R1) on Linux(6.5.0-26-generic).
Copyright 1996-2005 Kunihiro Ishiguro, et al.
configured with:
    '--with-clippy=./build-clippy/lib/clippy' '--sysconfdir=/etc/frr' '--sbindir=${prefix}/lib/frr' '--localstatedir=/var/run/frr' '--prefix=/usr' '--enable-user=frr' '--enable-group=frr' '--enable-vty-group=frrvty' '--disable-doc' '--disable-capabilities' '--enable-rpki' '--disable-grpc'

R2 version

This one is by adding the frr mirror on the sources.list and installing via apt-get install frr frr-pythontools.

R3# sh ver
FRRouting 9.1 (R3) on Linux(6.5.0-26-generic).
Copyright 1996-2005 Kunihiro Ishiguro, et al.
configured with:
    '--build=x86_64-linux-gnu' '--prefix=/usr' '--includedir=${prefix}/include' '--mandir=${prefix}/share/man' '--infodir=${prefix}/share/info' '--sysconfdir=/etc' '--localstatedir=/var' '--disable-option-checking' '--disable-silent-rules' '--libdir=${prefix}/lib/x86_64-linux-gnu' '--libexecdir=${prefix}/lib/x86_64-linux-gnu' '--disable-maintainer-mode' '--localstatedir=/var/run/frr' '--sbindir=/usr/lib/frr' '--sysconfdir=/etc/frr' '--with-vtysh-pager=/usr/bin/pager' '--libdir=/usr/lib/x86_64-linux-gnu/frr' '--with-moduledir=/usr/lib/x86_64-linux-gnu/frr/modules' '--disable-dependency-tracking' '--enable-rpki' '--disable-scripting' '--enable-pim6d' '--with-libpam' '--enable-doc' '--enable-doc-html' '--enable-snmp' '--enable-fpm' '--disable-protobuf' '--disable-zeromq' '--enable-ospfapi' '--enable-bgp-vnc' '--enable-multipath=256' '--enable-user=frr' '--enable-group=frr' '--enable-vty-group=frrvty' '--enable-configfile-mask=0640' '--enable-logfile-mask=0640' 'build_alias=x86_64-linux-gnu' 'PYTHON=python3'

R1 Config

R1# sh run 
Building configuration...

Current configuration:
!
frr version 10.1-dev
frr defaults traditional
hostname R1
log syslog informational
no ipv6 forwarding
service integrated-vtysh-config
!
interface eth0
 ip address 10.111.1.101/24
exit
!
interface eth1
 ip address 10.111.12.101/24
 ip ospf area 0
 mpls enable
exit
!
interface lo
 ip address 1.1.1.1/32
 ip ospf area 0
 mpls enable
exit
!
router bgp 200
 bgp router-id 1.1.1.1
 neighbor 2.2.2.2 remote-as 200
 neighbor 2.2.2.2 update-source lo
 !
 address-family ipv4 unicast
  network 10.111.1.0/24
 exit-address-family
exit
!
router ospf
 ospf router-id 1.1.1.1
 mpls ldp-sync
exit
!
mpls ldp
 router-id 1.1.1.1
 ordered-control
 !
 address-family ipv4
  discovery transport-address 1.1.1.1
  !
  interface eth1
  exit
  !
 exit-address-family
 !
exit
!
end

R2 Config

R2# sh run 
Building configuration...

Current configuration:
!
frr version 9.1
frr defaults traditional
hostname R2
log syslog informational
no ipv6 forwarding
service integrated-vtysh-config
!
interface eth0
 ip address 10.111.12.102/24
 ip ospf area 0
 mpls enable
exit
!
interface eth1
 ip address 10.111.23.102/24
 mpls enable
exit
!
interface lo
 ip address 2.2.2.2/32
 ip ospf area 0
 mpls enable
exit
!
router bgp 200
 bgp router-id 2.2.2.2
 neighbor 1.1.1.1 remote-as 200
 neighbor 1.1.1.1 update-source lo
 !
 address-family ipv4 unicast
  network 10.111.23.0/24
 exit-address-family
exit
!
router ospf
 ospf router-id 2.2.2.2
 mpls ldp-sync
exit
!
mpls ldp
 router-id 2.2.2.2
 ordered-control
 !
 address-family ipv4
  discovery transport-address 2.2.2.2
  !
  interface eth0
  exit
  !
  interface eth1
  exit
  !
 exit-address-family
 !
exit
!
rpki
exit
!
end

R1 show ip bgp output

R1# sh ip bgp 
BGP table version is 4, local router ID is 1.1.1.1, vrf id 0
Default local pref 100, local AS 200
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

     Network          Next Hop            Metric LocPrf Weight Path
 *>  10.111.1.0/24    0.0.0.0                  0         32768 i
 *>i 10.111.23.0/24   2.2.2.2                  0    100      0 i

Displayed 2 routes and 2 total paths
R1#

R2 show ip bgp output

R2# sh ip bgp 
BGP table version is 4, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 200
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

    Network          Next Hop            Metric LocPrf Weight Path
 *>i10.111.1.0/24    1.1.1.1                  0    100      0 i
 *> 10.111.23.0/24   0.0.0.0                  0         32768 i

Displayed  2 routes and 2 total paths
R2#

Show on R1 using GDB to set the flags manually

As I explained earlier here each column represents one flag.

What I did was:

attach gdb on bgpd process;
add a breakpoint at bgp_route.c:9005 line;
execute the show ip bgp
when breakpoint is reached I then manually set the rpki_state to one of the four values (shown below)

// source: https://github.com/FRRouting/frr/blob/master/bgpd/bgp_rpki.h
enum rpki_states {
	RPKI_NOT_BEING_USED,
	RPKI_VALID,
	RPKI_NOTFOUND,
	RPKI_INVALID
};

Obs.: Notice that the other flags are already set by the config present on R1 and R2, only RPKI flag was changed via gdb.

show ip bgp setting by hand to RPKI_VALID and the gdb steps:

R1# sh ip bgp 
BGP table version is 4, local router ID is 1.1.1.1, vrf id 0
Default local pref 100, local AS 200
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

     Network          Next Hop            Metric LocPrf Weight Path
V*>  10.111.1.0/24    0.0.0.0                  0         32768 i
V*>i 10.111.23.0/24   2.2.2.2                  0    100      0 i

Displayed 2 routes and 2 total paths
R1# 


<!----- GDB logs below ----->

(gdb) b bgp_route.c:9009
Breakpoint 3 at 0x5cf9c94fdf60: file bgpd/bgp_route.c, line 9009.
(gdb) c
Continuing.

Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca5699d0, p=p@entry=0x5cf9ca5698e0, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_VALID
(gdb) c
Continuing.

Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca56b6b0, p=p@entry=0x5cf9c9f4ca40, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_VALID
(gdb) c
Continuing.
(gdb)

show ip bgp setting by hand to RPKI_INVALID and the gdb steps:

R1# sh ip bgp 
BGP table version is 4, local router ID is 1.1.1.1, vrf id 0
Default local pref 100, local AS 200
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

     Network          Next Hop            Metric LocPrf Weight Path
I*>  10.111.1.0/24    0.0.0.0                  0         32768 i
I*>i 10.111.23.0/24   2.2.2.2                  0    100      0 i

Displayed 2 routes and 2 total paths
R1# 

<!----- GDB logs below ----->

(gdb) c
Continuing.

Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca56b6b0, p=p@entry=0x5cf9c9f4ca40, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_VALID
(gdb) c
Continuing.
(gdb) 
Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca5699d0, p=p@entry=0x5cf9ca5698e0, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_INVALID
(gdb) c
Continuing.

Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca56b6b0, p=p@entry=0x5cf9c9f4ca40, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_INVALID
(gdb) c
Continuing.

show ip bgp setting by hand to RPKI_NOTFOUND and the gdb steps:

R1# sh ip bgp 
BGP table version is 4, local router ID is 1.1.1.1, vrf id 0
Default local pref 100, local AS 200
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

     Network          Next Hop            Metric LocPrf Weight Path
N*>  10.111.1.0/24    0.0.0.0                  0         32768 i
N*>i 10.111.23.0/24   2.2.2.2                  0    100      0 i

Displayed 2 routes and 2 total paths
R1# 

<!----- GDB logs below ----->

(gdb) c
Continuing.
(gdb) 
Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca5699d0, p=p@entry=0x5cf9ca5698e0, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_NOTFOUND
(gdb) c
Continuing.

Thread 1 "bgpd" hit Breakpoint 3, route_vty_short_status_out (vty=vty@entry=0x5cf9ca54c700, path=path@entry=0x5cf9ca56b6b0, p=p@entry=0x5cf9c9f4ca40, json_path=json_path@entry=0x0) at bgpd/bgp_route.c:9009
9009		if (rpki_state == RPKI_VALID)
(gdb) set rpki_state = RPKI_NOTFOUND
(gdb) c
Continuing.

ton31337

LGTM

riw777

looks good

frrbot bot added the bgp label Mar 12, 2024

github-actions bot added master size/XS labels Mar 12, 2024

campescassiano marked this pull request as draft March 12, 2024 01:35

campescassiano force-pushed the ccs/bugfix/show-ip-bgp branch 2 times, most recently from 41982b1 to a9634e7 Compare March 19, 2024 01:38

github-actions bot added size/M and removed size/XS labels Mar 19, 2024

campescassiano force-pushed the ccs/bugfix/show-ip-bgp branch 2 times, most recently from 362f0a0 to 357f512 Compare March 19, 2024 11:03

riw777 approved these changes Mar 19, 2024

View reviewed changes

campescassiano marked this pull request as ready for review March 19, 2024 14:23

campescassiano added 3 commits March 19, 2024 16:36

bgpd: Updated topotests with new changes

c490a8f

Updated the topotests to contemplate the new `show ip bgp` outputs. Signed-off-by: Cassiano Campes <[email protected]>

bgpd: Adjusted topotests to reflect the new "show ip bgp" output

6859414

Topotests changed to consider the extra wite space added in "show ip bgp" output. Signed-off-by: Cassiano Campes <[email protected]>

campescassiano force-pushed the ccs/bugfix/show-ip-bgp branch from 357f512 to 6859414 Compare March 19, 2024 19:36

ton31337 approved these changes Mar 22, 2024

View reviewed changes

campescassiano requested a review from riw777 March 26, 2024 11:33

riw777 approved these changes Mar 26, 2024

View reviewed changes

riw777 merged commit 67aaa4b into FRRouting:master Mar 26, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bgpd: add missing white-space between route short status and network … #15525

bgpd: add missing white-space between route short status and network … #15525

campescassiano commented Mar 12, 2024

ton31337 commented Mar 12, 2024

campescassiano commented Mar 19, 2024

riw777 left a comment

campescassiano commented Mar 20, 2024

ton31337 commented Mar 21, 2024

campescassiano commented Mar 21, 2024

ton31337 left a comment

riw777 left a comment

bgpd: add missing white-space between route short status and network … #15525

bgpd: add missing white-space between route short status and network … #15525

Conversation

campescassiano commented Mar 12, 2024

ton31337 commented Mar 12, 2024

campescassiano commented Mar 19, 2024

Output of the R1 with the fix

Output of the R2 running the master branch

riw777 left a comment

Choose a reason for hiding this comment

campescassiano commented Mar 20, 2024

ton31337 commented Mar 21, 2024

campescassiano commented Mar 21, 2024

Scenario

R1 version

R2 version

R1 Config

R2 Config

R1 show ip bgp output

R2 show ip bgp output

Show on R1 using GDB to set the flags manually

ton31337 left a comment

Choose a reason for hiding this comment

riw777 left a comment

Choose a reason for hiding this comment