bgpd: Use correct specifier to print 4-byte ASN #3726

kirankella · 2019-02-04T07:16:29Z

Summary

The display format of the Remote AS field in the output of
'show ip bgp peer-group' is signed int.
Corrected such occurances in bgp_vty.c

Components

bgpd

Signed-off-by: Kiran Kella [email protected]

…eer-group' Fix: Corrected the format to be unsigned int instead of signed int for the Remote AS.

NetDEF-CI · 2019-02-04T08:14:40Z

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Successful

Basic Tests: Failed

IPv4 ldp protocol on Ubuntu 16.04: Successful
Ubuntu 14.04 deb pkg check: Successful
Addresssanitizer topotest: Successful
Debian 8 deb pkg check: Successful
Topology tests on Ubuntu 16.04 amd64: Successful
IPv4 protocols on Ubuntu 14.04: Successful
Debian 9 deb pkg check: Successful
CentOS 7 rpm pkg check: Successful
Ubuntu 12.04 deb pkg check: Successful
Static analyzer (clang): Successful
Topology tests on Ubuntu 18.04 amd64: Successful
IPv6 protocols on Ubuntu 14.04: Successful
CentOS 6 rpm pkg check: Successful
Ubuntu 16.04 deb pkg check: Successful

Topotest tests on Ubuntu 16.04 i386: Failed

Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-6570/test

Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:

RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
*** defaultIntf: warning: lm has no interfaces

r2: ospfd crashed. Core file found - Backtrace follows:
[New LWP 16660]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/ospfd'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb7fb3d05 in ?? ()
#0  0xb7fb3d05 in ?? ()
#1  0x004b19e2 in ?? ()
#2  0xb7f66299 in thread_call () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#3  0xb7f40311 in frr_run () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#4  0x00499c26 in main ()
2019-02-03 23:54:32,873 ERROR: assert failed at "test_ospf_sr_topo1/test_memory_leak": 
r2: ospfd crashed. Core file found - Backtrace follows:
[New LWP 16660]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/ospfd'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb7fb3d05 in ?? ()
#0  0xb7fb3d05 in ?? ()
#1  0x004b19e2 in ?? ()
#2  0xb7f66299 in thread_call () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#3  0xb7f40311 in frr_run () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#4  0x00499c26 in main ()

RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument

see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6570/artifact/TOPOI386/ErrorLog/log_topotests.txt

Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6570/artifact/TOPOI386/MemoryLeaks/

CLANG Static Analyzer Summary

Github Pull Request 3726, comparing to Git base SHA 09d4653
Base image data for Git 09d4653 does not exist - compare skipped

4 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6570/artifact/shared/static_analysis/index.html

NetDEF-CI · 2019-02-04T14:43:54Z

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-FRRPULLREQ-6570/

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

CLANG Static Analyzer Summary

Github Pull Request 3726, comparing to Git base SHA 09d4653
Base image data for Git 09d4653 does not exist - compare skipped

4 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6570/artifact/shared/static_analysis/index.html

qlyoung · 2019-02-04T16:24:50Z

bgpd/bgp_vty.c

 			conf->as);
 	} else if (conf->as_type == AS_INTERNAL) {
-		vty_out(vty, "\nBGP peer-group %s, remote AS %d\n", group->name,
+		vty_out(vty, "\nBGP peer-group %s, remote AS %u\n", group->name,


The format specifier needs to be PRIu32, not u

qlyoung · 2019-02-04T16:57:25Z

Good catch! Thought we'd fixed all these but apparently not 🙂

We also need you to sign off and reformat your commit message to match our project guidelines.

Seeing as this issue is present on master as well, I've gone ahead and submitted this - that has the correct PRIu32 specifier and is an example of how to sign off and title your commit messages. If you'd like to force push this PR to make the changes we can merge it then.

kirankella · 2019-02-04T17:51:31Z

Good catch! Thought we'd fixed all these but apparently not 🙂

We also need you to sign off and reformat your commit message to match our project guidelines.

Seeing as this issue is present on master as well, I've gone ahead and submitted this - that has the correct PRIu32 specifier and is an example of how to sign off and title your commit messages. If you'd like to force push this PR to make the changes we can merge it then.

Changes done as suggested. Thanks for the review.

NetDEF-CI · 2019-02-04T18:49:49Z

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-FRRPULLREQ-6582/

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

Warnings Generated during build:

Checkout code: Successful with additional warnings:

Report for bgp_vty.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #11408: FILE: /tmp/f1-12322/bgp_vty.c:11408:
< WARNING: line over 80 characters
< #11411: FILE: /tmp/f1-12322/bgp_vty.c:11411:

CLANG Static Analyzer Summary

Github Pull Request 3726, comparing to Git base SHA 09d4653
Base image data for Git 09d4653 does not exist - compare skipped

4 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6582/artifact/shared/static_analysis/index.html

qlyoung · 2019-02-04T20:18:53Z

@kirankella - you still need to amend the commit messages to follow the standards I linked and sign off on your code. Otherwise we can't accept it.

qlyoung · 2019-02-05T16:57:45Z

@kirankella The commit messages, not the PR title.

Please use the doc I linked you.
http://docs.frrouting.org/projects/dev-guide/en/latest/workflow.html#coding-practices-style

kirankella · 2019-02-05T17:47:48Z

@kirankella The commit messages, not the PR title.

Please use the doc I linked you.
http://docs.frrouting.org/projects/dev-guide/en/latest/workflow.html#coding-practices-style

@qlyoung Thanks for the links. I tried to meet to the guidelines. Can you please help in correcting me the commit message, since this is the first time i am submitting PR on frr? What am i missing?

qlyoung · 2019-02-05T19:39:10Z

You need to amend your commits, put the top level directory of the change in front of the subject line (as you did with the title of this PR), and when you amend use the -s flag to add a Signed-off-by line.

Take a look at any of the non-merge commits in this project's history if you need examples.

When you changed the title of this PR, it didn't change the commits. You have to do that with git, you can't do it with github. Notice how the commit messages are still wrong:
https://github.com/FRRouting/frr/pull/3726/commits

In show ip bgp peer-group, the 4-byte ASN is printed as signed int. Corrected the format specifier in such places in bgp_vty.c Signed-off-by: Kiran Kella <[email protected]>

…la/frr into show_bgp_peer_group_asn

kirankella · 2019-02-06T07:28:23Z

You need to amend your commits, put the top level directory of the change in front of the subject line (as you did with the title of this PR), and when you amend use the -s flag to add a Signed-off-by line.

Take a look at any of the non-merge commits in this project's history if you need examples.

When you changed the title of this PR, it didn't change the commits. You have to do that with git, you can't do it with github. Notice how the commit messages are still wrong:
https://github.com/FRRouting/frr/pull/3726/commits

Thanks much @qlyoung. I did amend the commit with -s option to meet the guidelines.

NetDEF-CI · 2019-02-06T08:29:17Z

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-FRRPULLREQ-6598/

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

Warnings Generated during build:

Checkout code: Successful with additional warnings:

Report for bgp_vty.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #11408: FILE: /tmp/f1-32735/bgp_vty.c:11408:
< WARNING: line over 80 characters
< #11411: FILE: /tmp/f1-32735/bgp_vty.c:11411:

CLANG Static Analyzer Summary

Github Pull Request 3726, comparing to Git base SHA 09d4653
Base image data for Git 09d4653 does not exist - compare skipped

4 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6598/artifact/shared/static_analysis/index.html

LabN-CI · 2019-02-06T09:09:13Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/3726 `64458b7`
Date	02/06/2019
Start	03:45:12
Finish	04:08:45
Run-Time	23:33
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-02-06-03:45:12.txt
Log	autoscript-2019-02-06-03:45:49.log.bz2
Memory

For details, please contact louberger

qlyoung · 2019-02-06T19:26:50Z

@kirankella I fixed your patches and merged them manually.
759f630

Thanks for your contribution!

4-byte ASN is not displayed correctly in the output of 'show ip bgp p…

2b01cc1

…eer-group' Fix: Corrected the format to be unsigned int instead of signed int for the Remote AS.

kirankella mentioned this pull request Feb 4, 2019

4-byte ASN displayed incorrectly as signed int in 'show ip bgp peer-group' sonic-net/sonic-frr#13

Closed

qlyoung requested changes Feb 4, 2019

View reviewed changes

qlyoung mentioned this pull request Feb 4, 2019

bgpd: use correct specifier to print asn #3727

Merged

kirankella changed the title ~~4-byte ASN is not displayed correctly in the output of 'show ip bgp peer-group' command~~ bgpd: 4-byte ASN is not displayed correctly in the output of 'show ip bgp peer-group' command Feb 4, 2019

kirankella changed the title ~~bgpd: 4-byte ASN is not displayed correctly in the output of 'show ip bgp peer-group' command~~ bgpd: Use correct specifier to print 4-byte ASN in show ip bgp peer-group Feb 4, 2019

Incorporated review comments for proper specification.

8d1b1f3

kirankella changed the title ~~bgpd: Use correct specifier to print 4-byte ASN in show ip bgp peer-group~~ bgpd: Use correct specifier to print 4-byte ASN Feb 5, 2019

kirankella added 2 commits February 6, 2019 12:51

bgpd: Use correct format specifier for printing 4-byte ASN

29f4cf7

In show ip bgp peer-group, the 4-byte ASN is printed as signed int. Corrected the format specifier in such places in bgp_vty.c Signed-off-by: Kiran Kella <[email protected]>

Merge branch 'show_bgp_peer_group_asn' of https://github.com/kirankel…

64458b7

…la/frr into show_bgp_peer_group_asn

qlyoung closed this Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bgpd: Use correct specifier to print 4-byte ASN #3726

bgpd: Use correct specifier to print 4-byte ASN #3726

kirankella commented Feb 4, 2019 •

edited

Loading

NetDEF-CI commented Feb 4, 2019

NetDEF-CI commented Feb 4, 2019

qlyoung Feb 4, 2019

qlyoung commented Feb 4, 2019 •

edited

Loading

kirankella commented Feb 4, 2019

NetDEF-CI commented Feb 4, 2019

qlyoung commented Feb 4, 2019

qlyoung commented Feb 5, 2019

kirankella commented Feb 5, 2019 •

edited

Loading

qlyoung commented Feb 5, 2019 •

edited

Loading

kirankella commented Feb 6, 2019

NetDEF-CI commented Feb 6, 2019

LabN-CI commented Feb 6, 2019

qlyoung commented Feb 6, 2019

bgpd: Use correct specifier to print 4-byte ASN #3726

bgpd: Use correct specifier to print 4-byte ASN #3726

Conversation

kirankella commented Feb 4, 2019 • edited Loading

Summary

Components

NetDEF-CI commented Feb 4, 2019

Continuous Integration Result: FAILED

Get source and apply patch from patchwork: Successful

Building Stage: Successful

Basic Tests: Failed

Topotest tests on Ubuntu 16.04 i386: Failed

Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6570/artifact/TOPOI386/MemoryLeaks/

CLANG Static Analyzer Summary

4 Static Analyzer issues remaining.

NetDEF-CI commented Feb 4, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

4 Static Analyzer issues remaining.

qlyoung Feb 4, 2019

Choose a reason for hiding this comment

qlyoung commented Feb 4, 2019 • edited Loading

kirankella commented Feb 4, 2019

NetDEF-CI commented Feb 4, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

Checkout code: Successful with additional warnings:

CLANG Static Analyzer Summary

4 Static Analyzer issues remaining.

qlyoung commented Feb 4, 2019

qlyoung commented Feb 5, 2019

kirankella commented Feb 5, 2019 • edited Loading

qlyoung commented Feb 5, 2019 • edited Loading

kirankella commented Feb 6, 2019

NetDEF-CI commented Feb 6, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

Checkout code: Successful with additional warnings:

CLANG Static Analyzer Summary

4 Static Analyzer issues remaining.

LabN-CI commented Feb 6, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

qlyoung commented Feb 6, 2019

kirankella commented Feb 4, 2019 •

edited

Loading

qlyoung commented Feb 4, 2019 •

edited

Loading

kirankella commented Feb 5, 2019 •

edited

Loading

qlyoung commented Feb 5, 2019 •

edited

Loading