Parse rtattr #8629

donaldsharp · 2021-05-05T11:37:36Z

Cleanup usage of the parse_rtattr code in zebra.

a) in some places we memset tb before and in some places we don't. This is a crash waiting to happen. Move memset to inside function to ensure it always happens

b) if_netlink.c had a duplicate function that was a define. consolidate down to 1.

LabN-CI · 2021-05-05T13:23:34Z

Outdated results 🛑

Basic BGPD CI results: FAILURE

_	_
Result	FAILURE git merge/8629 `c5c213d` frr.github Build
Date	05/05/2021
Start	09:22:29
Finish	09:23:31
Run-Time	01:02
Total
Pass
Fail
Valgrind-Errors
Valgrind-Loss
Details	vncregress-2021-05-05-09:22:29.txt
Log	make-2021-05-05-09:22:29.out.bz2
Memory

For details, please contact louberger

donaldsharp · 2021-05-05T13:39:16Z

@louberger can you help out here?

NetDEF-CI · 2021-05-05T15:40:55Z

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-18830/

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.

CLANG Static Analyzer Summary

Github Pull Request 8629, comparing to Git base SHA 48c2039

Fixed warnings:

Logic error: Branch condition evaluates to a garbage value in bgp_pbr.c, function bgp_pbr_policyroute_remove_from_zebra_unit, line 1933
Logic error: Branch condition evaluates to a garbage value in bgp_pbr.c, function bgp_pbr_dump_entry, line 2222

New warnings:

Logic error: Branch condition evaluates to a garbage value in bgpd/bgp_pbr.c, function bgp_pbr_policyroute_remove_from_zebra_unit, line 1933
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18830/artifact/shared/static_analysis/report-214f53.html#EndPath
Logic error: Branch condition evaluates to a garbage value in bgpd/bgp_pbr.c, function bgp_pbr_dump_entry, line 2222
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18830/artifact/shared/static_analysis/report-58e857.html#EndPath
Logic error: Uninitialized argument value in zebra/if_netlink.c, function netlink_link_change, line 1599
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18830/artifact/shared/static_analysis/report-9ee9b1.html#EndPath
Logic error: Uninitialized argument value in zebra/if_netlink.c, function netlink_link_change, line 1484
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18830/artifact/shared/static_analysis/report-b03562.html#EndPath
Logic error: Uninitialized argument value in zebra/if_netlink.c, function netlink_interface, line 883
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18830/artifact/shared/static_analysis/report-c8bb62.html#EndPath

Static Analysis warning summary compared to base:

Fixed warnings: 2
New warnings: 5

5 Static Analyzer issues remaining.

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

idryzhov

Marking as not ready because of some new warnings.

LabN-CI · 2021-05-08T15:53:28Z

Outdated results 🛑

Basic BGPD CI results: FAILURE

_	_
Result	FAILURE git merge/8629 `c5c213d` frr.github Build
Date	05/08/2021
Start	11:52:23
Finish	11:53:25
Run-Time	01:02
Total
Pass
Fail
Valgrind-Errors
Valgrind-Loss
Details	vncregress-2021-05-08-11:52:23.txt
Log	make-2021-05-08-11:52:23.out.bz2
Memory

For details, please contact louberger

louberger · 2021-05-08T19:32:04Z

compile fails once rebased/merged to/with master:
frr-ci> make
true
make all-am
...
AR bgpd/libbgp.a
CCLD bgpd/bgp_btoa
CCLD ospfclient/ospfclient
CCLD zebra/zebra
zebra/if_netlink.o: In function netlink_extract_gre_info': .../zebra/if_netlink.c:576: undefined reference to parse_rtattr_nested'
collect2: error: ld returned 1 exit status
Makefile:8367: recipe for target 'zebra/zebra' failed
make[1]: *** [zebra/zebra] Error 1

Makefile:5514: recipe for target 'all' failed
make: *** [all] Error 2

In order to parse the netlink message into the `struct rtattr *tb[size]` it is assumed that the buffer is memset to 0 before the parsing. As such if you attempt to read a value that was not returned in the message you will not crash when you test for it. The code has places were we memset it and places where we don't. This *will* lead to crashes when the kernel changes. In our parsing routines let's have them memset instead of having to remember to do it pre pass in to the parser. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

if_netlink.c created it's on nested parsing #define which is identical to netlink_parse_rtattr_nested. Consolidate on one instead of having this duality. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

donaldsharp · 2021-05-12T00:07:53Z

@idryzhov I have addressed the issues

LabN-CI · 2021-05-12T00:56:28Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/8629 `c9d842c`
Date	05/11/2021
Start	20:30:51
Finish	20:56:24
Run-Time	25:33
Total	1815
Pass	1815
Fail	0
Valgrind-Errors
Valgrind-Loss
Details	vncregress-2021-05-11-20:30:51.txt
Log	autoscript-2021-05-11-20:32:01.log.bz2
Memory	497 484 422

For details, please contact louberger

NetDEF-CI · 2021-05-12T04:49:47Z

Continuous Integration Result: FAILED

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

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 3: Failed (click for details)

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

Topology Tests failed for Topotests debian 10 amd64 part 3:

r1: Daemon bgpd not running

From frr r1 bgpd log file:
2021/05/12 00:57:48 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:48 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv4 neighbor 192.168.0.2 graceful-restart json
2021/05/12 00:57:48 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:48 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv4 neighbor 192.168.0.2 graceful-restart 
2021/05/12 00:57:49 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:49 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv6 neighbor fd00::2 graceful-restart json
2021/05/12 00:57:49 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:49 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv6 neighbor fd00::2 graceful-restart 
2021/05/12 00:57:49 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:50 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:50 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:50 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:56 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:56 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:56 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:56 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:57:56 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:58:02 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:58:02 BGP: [M7Q4P-46WDR] vty[??]@# do write memory


r2: Daemon bgpd not running

From frr r2 bgpd log file:
2021/05/12 00:59:26 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:26 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv4 neighbor 192.168.0.1 graceful-restart json
2021/05/12 00:59:26 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:26 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv4 neighbor 192.168.0.1 graceful-restart 
2021/05/12 00:59:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:27 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv6 neighbor fd00::1 graceful-restart json
2021/05/12 00:59:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:27 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv6 neighbor fd00::1 graceful-restart 
2021/05/12 00:59:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:28 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:28 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:34 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:34 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:34 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:34 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:35 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:41 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 00:59:41 BGP: [M7Q4P-46WDR] vty[??]@# do write memory


r1: Daemon bgpd not running

From frr r1 bgpd log file:
2021/05/12 01:01:18 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:18 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv4 neighbor 192.168.0.2 graceful-restart json
2021/05/12 01:01:18 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:18 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv4 neighbor 192.168.0.2 graceful-restart 
2021/05/12 01:01:19 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:19 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv6 neighbor fd00::2 graceful-restart json
2021/05/12 01:01:19 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:19 BGP: [M7Q4P-46WDR] vty[??]@# show bgp ipv6 neighbor fd00::2 graceful-restart 
2021/05/12 01:01:20 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:20 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:20 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:20 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:26 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:26 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:27 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:33 BGP: [M7Q4P-46WDR] vty[??]@> enable
2021/05/12 01:01:33 BGP: [M7Q4P-46WDR] vty[??]@# do write memory


2021-05-12 01:03:09,998 ERROR: assert failed at "test_bgp_gr_functionality_topo1/test_BGP_GR_TC_46_p1": Testcase test_BGP_GR_TC_46_p1 :Failed 
   Error [DUT: r1]: localGrMode is not correct Expected: Restart*, Found: Helper
assert '[DUT: r1]: localGrMode is not correct Expected: Restart*, Found: Helper' is True

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

Successful on other platforms/tests

Topotests debian 10 amd64 part 0
Topotests Ubuntu 18.04 i386 part 8
Topotests debian 10 amd64 part 5
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 arm8 part 4
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 9
Topotests Ubuntu 18.04 amd64 part 7
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 amd64 part 9
Addresssanitizer topotests part 7
Fedora 29 rpm pkg check
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 amd64 part 4
Topotests Ubuntu 18.04 arm8 part 5
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 i386 part 7
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 4
Addresssanitizer topotests part 4
Topotests Ubuntu 18.04 amd64 part 1
Static analyzer (clang)
Topotests debian 10 amd64 part 6
IPv6 protocols on Ubuntu 18.04
Topotests Ubuntu 18.04 amd64 part 0
Addresssanitizer topotests part 1
CentOS 7 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 2
Topotests Ubuntu 18.04 amd64 part 3
Topotests Ubuntu 18.04 i386 part 9
Topotests Ubuntu 18.04 i386 part 4
Topotests Ubuntu 18.04 arm8 part 3
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 arm8 part 6
IPv4 protocols on Ubuntu 18.04
Topotests Ubuntu 18.04 arm8 part 1
Addresssanitizer topotests part 3
Topotests debian 10 amd64 part 8
Topotests debian 10 amd64 part 7
Ubuntu 16.04 deb pkg check
Topotests Ubuntu 18.04 amd64 part 5
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 arm8 part 8
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 i386 part 1
Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 amd64 part 6
Ubuntu 20.04 deb pkg check
Topotests debian 10 amd64 part 1
IPv4 ldp protocol on Ubuntu 18.04
Debian 9 deb pkg check
Topotests Ubuntu 18.04 arm8 part 7
Topotests Ubuntu 18.04 i386 part 0
Topotests debian 10 amd64 part 9
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 arm8 part 2
Debian 10 deb pkg check
Topotests Ubuntu 18.04 amd64 part 8
Addresssanitizer topotests part 2

idryzhov · 2021-05-12T08:38:05Z

CI failure unrelated, merging.

polychaeta added the zebra label May 5, 2021

idryzhov approved these changes May 5, 2021

View reviewed changes

idryzhov requested changes May 5, 2021

View reviewed changes

donaldsharp added 2 commits May 11, 2021 20:05

zebra: Consolidate on 1 function netlink_parse_rattr_nested

c9d842c

if_netlink.c created it's on nested parsing #define which is identical to netlink_parse_rtattr_nested. Consolidate on one instead of having this duality. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

donaldsharp force-pushed the parse_rtattr branch from c5c213d to c9d842c Compare May 12, 2021 00:06

idryzhov approved these changes May 12, 2021

View reviewed changes

idryzhov merged commit 17daea8 into FRRouting:master May 12, 2021

donaldsharp deleted the parse_rtattr branch June 23, 2021 11:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse rtattr #8629

Parse rtattr #8629

donaldsharp commented May 5, 2021

LabN-CI commented May 5, 2021 •

edited

Loading

Basic BGPD CI results: FAILURE

donaldsharp commented May 5, 2021

NetDEF-CI commented May 5, 2021 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

Fixed warnings:

New warnings:

Static Analysis warning summary compared to base:

5 Static Analyzer issues remaining.

idryzhov left a comment

LabN-CI commented May 8, 2021 •

edited

Loading

Basic BGPD CI results: FAILURE

louberger commented May 8, 2021

donaldsharp commented May 12, 2021

LabN-CI commented May 12, 2021

NetDEF-CI commented May 12, 2021

idryzhov commented May 12, 2021

Parse rtattr #8629

Parse rtattr #8629

Conversation

donaldsharp commented May 5, 2021

LabN-CI commented May 5, 2021 • edited Loading

Basic BGPD CI results: FAILURE

donaldsharp commented May 5, 2021

NetDEF-CI commented May 5, 2021 • edited Loading

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

Fixed warnings:

New warnings:

Static Analysis warning summary compared to base:

5 Static Analyzer issues remaining.

idryzhov left a comment

Choose a reason for hiding this comment

LabN-CI commented May 8, 2021 • edited Loading

Basic BGPD CI results: FAILURE

louberger commented May 8, 2021

donaldsharp commented May 12, 2021

LabN-CI commented May 12, 2021

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented May 12, 2021

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

idryzhov commented May 12, 2021

LabN-CI commented May 5, 2021 •

edited

Loading

NetDEF-CI commented May 5, 2021 •

edited

Loading

LabN-CI commented May 8, 2021 •

edited

Loading