-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
bgpd: fix 4-byte AS display in bestpath-from-AS #1261
Conversation
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com> Before ====== cel-redxp-10# show ip bgp 20.1.3.0/24 BGP routing table entry for 20.1.3.0/24 Paths: (1 available, best #1, table Default-IP-Routing-Table) Advertised to non peer-group peers: top1(10.1.1.2) bottom0(20.1.2.2) 4294967292 20.1.2.2 from bottom0(20.1.2.2) (20.1.1.1) Origin IGP, metric 0, localpref 100, valid, external, bestpath-from-AS -4, best Community: 99:1 AddPath ID: RX 0, TX 92 Last update: Wed Sep 27 16:02:34 2017 cel-redxp-10# After ===== cel-redxp-10# show ip bgp 20.1.3.0/24 BGP routing table entry for 20.1.3.0/24 Paths: (1 available, best #1, table Default-IP-Routing-Table) Advertised to non peer-group peers: bottom0(20.1.2.2) 4294967292 20.1.2.2 from bottom0(20.1.2.2) (20.1.1.1) Origin IGP, metric 0, localpref 100, valid, external, bestpath-from-AS 4294967292, best Community: 99:1 AddPath ID: RX 0, TX 2 Last update: Wed Sep 27 16:07:09 2017 cel-redxp-10#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a similar issue this morning while testing #1237.
show ip route
can show negative metrics ^^
We're using %d
in too many places and it's very likely that we have more issues of this kind.
I wrote a small program to test printf:
#include <stdio.h>
#include <stdint.h>
int main(int argc, char *argv[])
{
printf("uint8_t\n");
printf("--------\n");
printf("int8_t (%%u): %u (real value 255)\n", (int8_t)255);
printf("int8_t (%%d): %d (real value 255)\n", (int8_t)255);
printf("uint8_t (%%u): %u (real value 255)\n", (uint8_t)255);
printf("uint8_t (%%d): %d (real value 255)\n", (uint8_t)255);
printf("\n");
printf("uint16_t\n");
printf("--------\n");
printf("int16_t (%%u): %u (real value 65535)\n", (int16_t)65535);
printf("int16_t (%%d): %d (real value 65535)\n", (int16_t)65535);
printf("uint16_t (%%u): %u (real value 65535)\n", (uint16_t)65535);
printf("uint16_t (%%d): %d (real value 65535)\n", (uint16_t)65535);
printf("\n");
printf("uint32_t\n");
printf("--------\n");
printf("int32_t (%%u): %u (real value 4294967295)\n", (int32_t)4294967295);
printf("int32_t (%%d): %d (real value 4294967295)\n", (int32_t)4294967295);
printf("uint32_t (%%u): %u (real value 4294967295)\n", (uint32_t)4294967295);
printf("uint32_t (%%d): %d (real value 4294967295)\n", (uint32_t)4294967295);
}
Surprisingly, for unsigned variables, using %d
or %u
only makes a difference for uint32_t:
$ ./printf
uint8_t
--------
int8_t (%u): 4294967295 (real value 255)
int8_t (%d): -1 (real value 255)
uint8_t (%u): 255 (real value 255)
uint8_t (%d): 255 (real value 255)
uint16_t
--------
int16_t (%u): 4294967295 (real value 65535)
int16_t (%d): -1 (real value 65535)
uint16_t (%u): 65535 (real value 65535)
uint16_t (%d): 65535 (real value 65535)
uint32_t
--------
int32_t (%u): 4294967295 (real value 4294967295)
int32_t (%d): -1 (real value 4294967295)
uint32_t (%u): 4294967295 (real value 4294967295)
uint32_t (%d): -1 (real value 4294967295)
That's probably why we don't see more negative values in other places. One day we need to analyze all uses of %d
in our codebase and fix the ones where %u
would be more appropriate.
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-1774/ This is a comment from an EXPERIMENTAL automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base79 Static Analyzer issues remaining.See details at |
bgpd: fix 4-byte AS display in bestpath-from-AS
Signed-off-by: Daniel Walton dwalton@cumulusnetworks.com
Before
cel-redxp-10# show ip bgp 20.1.3.0/24
BGP routing table entry for 20.1.3.0/24
Paths: (1 available, best #1, table Default-IP-Routing-Table)
Advertised to non peer-group peers:
top1(10.1.1.2) bottom0(20.1.2.2)
4294967292
20.1.2.2 from bottom0(20.1.2.2) (20.1.1.1)
Origin IGP, metric 0, localpref 100, valid, external,
bestpath-from-AS -4, best
Community: 99:1
AddPath ID: RX 0, TX 92
Last update: Wed Sep 27 16:02:34 2017
cel-redxp-10#
After
cel-redxp-10# show ip bgp 20.1.3.0/24
BGP routing table entry for 20.1.3.0/24
Paths: (1 available, best #1, table Default-IP-Routing-Table)
Advertised to non peer-group peers:
bottom0(20.1.2.2)
4294967292
20.1.2.2 from bottom0(20.1.2.2) (20.1.1.1)
Origin IGP, metric 0, localpref 100, valid, external,
bestpath-from-AS 4294967292, best
Community: 99:1
AddPath ID: RX 0, TX 2
Last update: Wed Sep 27 16:07:09 2017
cel-redxp-10#