bgpd: fix route-target display with as dotted format #13721

pguibert6WIND · 2023-06-07T15:36:23Z

The following command results in a wrong route-target display:

show running-config

[..]
route-map rmap permit 1
set extcommunity rt 1.45:55
exit
router bgp 65500
neighbor 1.1.1.1 remote-as 65500
address-family ipv4 unicast
neighbor 1.1.1.1 route-map rmap in

Observed output:

show bgp ipv4 3.3.3.3/32

[..]
Extended Community: RT:1.0.0.45:55

The decoding of the passed cli string assumes this is an IP address, whereas it is an AS number in dotted format. Consequently, the vty output will use the ip address encoding.

Count the number of dots in the extended community format. If a single dot number is detected, the AS format is passed, and used by the vty output.

Expected output:

show bgp ipv4 3.3.3.3/32

[..]
Extended Community: RT:65581:55

NetDEF-CI · 2023-06-07T19:01:11Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12081/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12081/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12081/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12081/artifact/TOPO9DEB10AMD64/TopotestDetails/

Successful on other platforms/tests

Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 i386 part 5
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 i386 part 0
Topotests debian 10 amd64 part 1
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 0
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 i386 part 9
Addresssanitizer topotests part 2
Static analyzer (clang)
Debian 9 deb pkg check
Topotests debian 10 amd64 part 5
Topotests Ubuntu 18.04 amd64 part 1
Ubuntu 18.04 deb pkg check
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests debian 10 amd64 part 7
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 arm8 part 6
Topotests Ubuntu 18.04 arm8 part 1
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 arm8 part 3
Topotests Ubuntu 18.04 amd64 part 2
Addresssanitizer topotests part 5
Topotests debian 10 amd64 part 4
Topotests Ubuntu 18.04 i386 part 3
Addresssanitizer topotests part 4
Topotests debian 10 amd64 part 8
Addresssanitizer topotests part 0
Topotests Ubuntu 18.04 arm8 part 9
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 3
Topotests debian 10 amd64 part 0
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 2
Topotests Ubuntu 18.04 amd64 part 8
Topotests Ubuntu 18.04 arm8 part 7
Topotests Ubuntu 18.04 i386 part 1
Topotests Ubuntu 18.04 amd64 part 4
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 amd64 part 9
Debian 10 deb pkg check
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 amd64 part 7
Topotests Ubuntu 18.04 amd64 part 5

NetDEF-CI · 2023-06-07T19:18:10Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12080/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-12080/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12080/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12080/artifact/TOPO9U18AMD64/TopotestDetails/

Successful on other platforms/tests

Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 i386 part 0
Addresssanitizer topotests part 3
Topotests debian 10 amd64 part 1
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 0
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 i386 part 9
Addresssanitizer topotests part 2
Static analyzer (clang)
Debian 9 deb pkg check
Topotests Ubuntu 18.04 amd64 part 1
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Ubuntu 18.04 deb pkg check
Topotests debian 10 amd64 part 5
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 arm8 part 6
Addresssanitizer topotests part 8
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 arm8 part 1
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 3
Addresssanitizer topotests part 4
Topotests debian 10 amd64 part 9
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 amd64 part 2
Topotests debian 10 amd64 part 4
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 8
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 3
Topotests Ubuntu 18.04 arm8 part 9
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 amd64 part 5
Ubuntu 20.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 2
Topotests Ubuntu 18.04 amd64 part 8
Topotests Ubuntu 18.04 i386 part 1
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 arm8 part 7
Debian 10 deb pkg check
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 amd64 part 7

Warnings Generated during build:

Checkout code: Successful with additional warnings

Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-12080/test

Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12080/artifact/TOPO9U18AMD64/TopotestLogs/log_topotests.txt
Topotests Ubuntu 18.04 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12080/artifact/TOPO9U18AMD64/TopotestDetails/

Report for bgp_ecommunity.c | 14 issues
===============================================
< ERROR: code indent should use tabs where possible
< #719: FILE: /tmp/f1-972940/bgp_ecommunity.c:719:
< ERROR: code indent should use tabs where possible
< #741: FILE: /tmp/f1-972940/bgp_ecommunity.c:741:
< WARNING: please, no spaces at the start of a line
< #741: FILE: /tmp/f1-972940/bgp_ecommunity.c:741:
< ERROR: code indent should use tabs where possible
< #745: FILE: /tmp/f1-972940/bgp_ecommunity.c:745:
< WARNING: please, no spaces at the start of a line
< #745: FILE: /tmp/f1-972940/bgp_ecommunity.c:745:
< ERROR: code indent should use tabs where possible
< #747: FILE: /tmp/f1-972940/bgp_ecommunity.c:747:
< WARNING: please, no spaces at the start of a line
< #747: FILE: /tmp/f1-972940/bgp_ecommunity.c:747:

ton31337 · 2023-06-08T07:04:33Z

Is this really an expected RT display format RT:65581:55:55?

pguibert6WIND · 2023-06-08T07:46:48Z

Is this really an expected RT display format RT:65581:55:55?

By default when as-notation mode is standard, the whole uint32_t value should be displayed.
I agree that an other pull request should be done so that the display is related to the as notation mode of the bgp instance.

NetDEF-CI · 2023-06-08T08:31:25Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12081/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

ton31337 · 2023-06-08T17:09:45Z

Is this really an expected RT display format RT:65581:55:55?

By default when as-notation mode is standard, the whole uint32_t value should be displayed. I agree that an other pull request should be done so that the display is related to the as notation mode of the bgp instance.

Can we fix this in this PR at once and verify with a topotest?

pguibert6WIND · 2023-06-08T17:20:20Z

Is this really an expected RT display format RT:65581:55:55?

By default when as-notation mode is standard, the whole uint32_t value should be displayed. I agree that an other pull request should be done so that the display is related to the as notation mode of the bgp instance.

Can we fix this in this PR at once and verify with a topotest?

I dont see the point in doing it in two steps.
the bgp pointer is not reachable in the function, and it would require more than usual changes.
I prefer looking at it in a second step, if you don't mind

ton31337 · 2023-06-13T07:51:47Z

To double-check: This RT:65581:55:55 should be shown as RT:1.45:55, correct?

pguibert6WIND · 2023-06-13T18:02:44Z

To double-check: This RT:65581:55:55 should be shown as RT:1.45:55, correct?
65581

yes.

# show running-config
router bgp 65581 vrf vrf1 as-notation dot
 neighbor 1.1.1.1 remote-as 65581
!
#show bgp vrf vrf1 summary
IPv4 Unicast Summary (VRF vrf1):
BGP router identifier 0.0.0.0, local AS number 65581 vrf-id -1
BGP table version 0
RIB entries 0, using 0 bytes of memory
Peers 1, using 88 KiB of memory

Neighbor        V         AS   MsgRcvd   MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd   PfxSnt Desc
1.1.1.1         4       1.45         0         0        0    0    0    never         Idle        0 N/A

ton31337 · 2023-06-13T19:04:00Z

Yeah, but show bgp ipv4 3.3.3.3/32?

NetDEF-CI · 2023-06-19T18:58:39Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12382/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12382/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-12382/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12382/artifact/TOPO9DEB10AMD64/TopotestDetails/

Successful on other platforms/tests

Topotests Ubuntu 18.04 amd64 part 3
Topotests Ubuntu 18.04 arm8 part 7
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 i386 part 2
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 arm8 part 8
Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 amd64 part 5
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 i386 part 1
Topotests Ubuntu 18.04 amd64 part 0
Topotests Ubuntu 18.04 i386 part 8
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 7
Debian 10 deb pkg check
Addresssanitizer topotests part 3
Topotests debian 10 amd64 part 8
Topotests Ubuntu 18.04 i386 part 5
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 arm8 part 9
Topotests Ubuntu 18.04 arm8 part 2
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 amd64 part 4
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 9
Topotests debian 10 amd64 part 7
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 9
Topotests Ubuntu 18.04 amd64 part 8
Addresssanitizer topotests part 8
Topotests debian 10 amd64 part 2
Static analyzer (clang)
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 i386 part 0
Addresssanitizer topotests part 6
Ubuntu 20.04 deb pkg check
Topotests debian 10 amd64 part 1
Topotests Ubuntu 18.04 amd64 part 1
Topotests Ubuntu 18.04 arm8 part 6
Topotests Ubuntu 18.04 arm8 part 1
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 3
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 amd64 part 6
Addresssanitizer topotests part 4
Debian 9 deb pkg check
Topotests Ubuntu 18.04 arm8 part 3
Topotests debian 10 amd64 part 5
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests debian 10 amd64 part 4

ton31337

If you could add a topotest with an expected dotted format checking, that would be good. In the future it would be handy when changing/refactoring something to catch this too.

github-actions · 2023-06-27T10:11:45Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

riw777

looks okay ... waiting on @ton31337 's comments

ton31337 · 2023-08-01T09:20:52Z

@pguibert6WIND ping

riw777 · 2023-10-03T12:57:47Z

@pguibert6WIND -- any progress on this one?

pguibert6WIND · 2023-10-03T19:40:54Z

@pguibert6WIND -- any progress on this one?

sorry, need help for rebase.
if not possible, pls wait.

ton31337 · 2023-10-10T06:06:41Z

What do you mean need help for rebase? :)

pguibert6WIND · 2023-12-04T10:14:37Z

What do you mean need help for rebase? :)

there is a conflict, and I need time to resolve this.

ton31337 · 2024-01-06T18:51:42Z

@pguibert6WIND could you rebase and push?

pguibert6WIND · 2024-01-09T16:24:05Z

@pguibert6WIND could you rebase and push?

done

pguibert6WIND · 2024-01-10T08:16:07Z

ci:rerun

ton31337 · 2024-01-11T09:34:27Z

@pguibert6WIND how should the output be for show bgp ipv4 ...?

Extended Community: RT:1.0.0.45:55

is bad, and should be a numeric one (a not like below), right?

Extended Community: RT:1.45:55

This is OK output, correct?:

Extended Community: RT:65581:55

riw777 · 2024-02-20T14:46:18Z

I think we're just waiting on the output for this one ... to make certain it's correct? Can we get this cleaned up so we can push it?

The following command results in a wrong route-target display: > # show running-config > [..] > route-map rmap permit 1 > set extcommunity rt 1.45:55 > exit > router bgp 1.45 as-notation plain > neighbor 192.0.2.1 remote-as 65500 > address-family ipv4 unicast > network 192.0.2.2/32 route-map rmap > Observed output: > # show bgp ipv4 192.0.2.2/32 > [..] > Extended Community: RT:1.0.0.45:55 > The decoding of the passed cli string assumes this is an IP address, whereas it is an AS number in dotted format. Consequently, the vty output will use the ip address encoding. Count the number of dots in the extended community format. If a single dot number is detected, the AS format is passed, and used by the vty output. After fix: > > # show bgp ipv4 192.0.2.2/32 > [..] > Extended Community: RT:65581:55 > For remind, AS 65581 and AS 1.45 are a unique AS number. > show bgp neighbor > BGP neighbor is 192.0.2.1, remote AS 65500, local AS 65581, external link > [..] Signed-off-by: Philippe Guibert <[email protected]>

pguibert6WIND · 2024-02-27T13:31:45Z

@pguibert6WIND how should the output be for show bgp ipv4 ...?
Extended Community: RT:1.0.0.45:55
is bad, and should be a numeric one (a not like below), right?
Extended Community: RT:1.45:55
This is OK output, correct?:
Extended Community: RT:65581:55

Today, the Route Target displayed in BGP prefix does not follow the BGP instance ASDOT format.
so today, the output will be displayed as before (RT:65581:55).

The fix about displaying RT with appropriate format will be done on a separate pull request.

frrbot bot added bgp bugfix labels Jun 7, 2023

github-actions bot added master size/M labels Jun 7, 2023

pguibert6WIND force-pushed the route_target_wrong_display branch from 52de42a to f6a5460 Compare June 7, 2023 15:50

fdumontet6WIND approved these changes Jun 19, 2023

View reviewed changes

pguibert6WIND force-pushed the route_target_wrong_display branch from f6a5460 to d792d50 Compare June 19, 2023 15:58

ton31337 reviewed Jun 21, 2023

View reviewed changes

github-actions bot added the conflicts label Jun 27, 2023

riw777 approved these changes Jul 11, 2023

View reviewed changes

pguibert6WIND force-pushed the route_target_wrong_display branch from d792d50 to 3416619 Compare December 4, 2023 12:58

github-actions bot removed the conflicts label Dec 4, 2023

pguibert6WIND force-pushed the route_target_wrong_display branch from 3416619 to f30088a Compare January 9, 2024 16:23

pguibert6WIND force-pushed the route_target_wrong_display branch from f30088a to 3612da3 Compare January 10, 2024 17:35

pguibert6WIND force-pushed the route_target_wrong_display branch from 3612da3 to 7c1480f Compare February 27, 2024 13:30

riw777 merged commit 0f3923d into FRRouting:master Feb 27, 2024
9 checks passed

bgpd: fix route-target display with as dotted format #13721

bgpd: fix route-target display with as dotted format #13721

Conversation

pguibert6WIND commented Jun 7, 2023 • edited Loading

show running-config

show bgp ipv4 3.3.3.3/32

show bgp ipv4 3.3.3.3/32

NetDEF-CI commented Jun 7, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

NetDEF-CI commented Jun 7, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Warnings Generated during build:

ton31337 commented Jun 8, 2023

pguibert6WIND commented Jun 8, 2023

NetDEF-CI commented Jun 8, 2023 • edited Loading

Continuous Integration Result: SUCCESSFUL

ton31337 commented Jun 8, 2023

pguibert6WIND commented Jun 8, 2023

ton31337 commented Jun 13, 2023

pguibert6WIND commented Jun 13, 2023

ton31337 commented Jun 13, 2023

NetDEF-CI commented Jun 19, 2023

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

ton31337 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 27, 2023

riw777 left a comment

Choose a reason for hiding this comment

ton31337 commented Aug 1, 2023

riw777 commented Oct 3, 2023

pguibert6WIND commented Oct 3, 2023

ton31337 commented Oct 10, 2023

pguibert6WIND commented Dec 4, 2023

ton31337 commented Jan 6, 2024

pguibert6WIND commented Jan 9, 2024

pguibert6WIND commented Jan 10, 2024

ton31337 commented Jan 11, 2024

riw777 commented Feb 20, 2024

pguibert6WIND commented Feb 27, 2024

pguibert6WIND commented Jun 7, 2023 •

edited

Loading

NetDEF-CI commented Jun 7, 2023 •

edited

Loading

NetDEF-CI commented Jun 7, 2023 •

edited

Loading

NetDEF-CI commented Jun 8, 2023 •

edited

Loading