- dev - lists.frrouting.org

New Defects reported by Coverity Scan for freerangerouting/frr
by scan-admin＠coverity.com 29 Sep '17

29 Sep '17

Hi, Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 5 new defect(s) introduced to freerangerouting/frr found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 5 of 5 defect(s) ** CID 1457194: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 102 in ip_route_blackhole() ________________________________________________________________________________________________________ *** CID 1457194: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 102 in ip_route_blackhole() 96 #if 8 /* anything to parse? */ 97 int _i; 98 #if 1 /* anything that can fail? */ 99 unsigned _fail = 0, _failcnt = 0; 100 #endif 101 const char *no = NULL; >>> CID 1457194: Null pointer dereferences (FORWARD_NULL) >>> Assigning: "prefix" = "NULL". 102 const char *prefix = NULL; 103 struct in_addr mask = { INADDR_ANY }; 104 const char *mask_str = NULL; 105 const char *flag = NULL; 106 long tag = 0; 107 const char *tag_str = NULL; ** CID 1457193: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 489 in ipv6_route_address_interface() ________________________________________________________________________________________________________ *** CID 1457193: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 489 in ipv6_route_address_interface() 483 int _i; 484 #if 1 /* anything that can fail? */ 485 unsigned _fail = 0, _failcnt = 0; 486 #endif 487 const char *no = NULL; 488 struct prefix_ipv6 prefix = { }; >>> CID 1457193: Null pointer dereferences (FORWARD_NULL) >>> Assigning: "prefix_str" = "NULL". 489 const char *prefix_str = NULL; 490 struct prefix_ipv6 from = { }; 491 const char *from_str = NULL; 492 struct in6_addr gate = IN6ADDR_ANY_INIT; 493 const char *gate_str = NULL; 494 const char *ifname = NULL; ** CID 1457192: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 195 in ip_route_address_interface() ________________________________________________________________________________________________________ *** CID 1457192: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 195 in ip_route_address_interface() 189 #if 9 /* anything to parse? */ 190 int _i; 191 #if 1 /* anything that can fail? */ 192 unsigned _fail = 0, _failcnt = 0; 193 #endif 194 const char *no = NULL; >>> CID 1457192: Null pointer dereferences (FORWARD_NULL) >>> Assigning: "prefix" = "NULL". 195 const char *prefix = NULL; 196 struct in_addr mask = { INADDR_ANY }; 197 const char *mask_str = NULL; 198 struct in_addr gate = { INADDR_ANY }; 199 const char *gate_str = NULL; 200 const char *ifname = NULL; ** CID 1457191: Uninitialized variables (UNINIT) /pimd/pim_cmd.c: 6505 in interface_no_ip_pim_boundary_oil() ________________________________________________________________________________________________________ *** CID 1457191: Uninitialized variables (UNINIT) /pimd/pim_cmd.c: 6505 in interface_no_ip_pim_boundary_oil() 6499 "Define multicast boundary\n" 6500 "Filter OIL by group using prefix list\n" 6501 "Prefix list to filter OIL with") 6502 { 6503 VTY_DECLVAR_CONTEXT(interface, iif); 6504 struct pim_interface *pim_ifp; >>> CID 1457191: Uninitialized variables (UNINIT) >>> Declaring variable "idx" without initializer. 6505 int idx; 6506 6507 argv_find(argv, argc, "WORD", &idx); 6508 6509 PIM_GET_PIM_INTERFACE(pim_ifp, iif); 6510 ** CID 1457190: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 393 in ipv6_route_blackhole() ________________________________________________________________________________________________________ *** CID 1457190: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_vty_clippy.c: 393 in ipv6_route_blackhole() 387 int _i; 388 #if 1 /* anything that can fail? */ 389 unsigned _fail = 0, _failcnt = 0; 390 #endif 391 const char *no = NULL; 392 struct prefix_ipv6 prefix = { }; >>> CID 1457190: Null pointer dereferences (FORWARD_NULL) >>> Assigning: "prefix_str" = "NULL". 393 const char *prefix_str = NULL; 394 struct prefix_ipv6 from = { }; 395 const char *from_str = NULL; 396 const char *flag = NULL; 397 long tag = 0; 398 const char *tag_str = NULL; ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "frr(a)lists.nox.tf", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

Modify ZEBRA_ROUTE_KERNEL priority/metric read in linux
by Donald Sharp 27 Sep '17

27 Sep '17

All - This PR: https://github.com/FRRouting/frr/pull/1237 has received a request that we spend a bit of time disseminating the problem and this proposed fix for more feedback. The Problem: Linux kernel VRF table lookups follow this behavior: Lookup up in the table prefix A. If no route is found fall back to the main default table and lookup prefix A again. This behavior is considered 'surprising' to people who expect the behavior of VRF's to stop looking when nothing is found in the corresponding table. The linux kernel coders documented that a default unreachable route with a high metric should be installed into the VRF table to prevent the fallback mechanism from being invoked in the kernel. Prior to mid-August this was not that big of a deal as that default unreachable routes were being ignored by rt_netlink.c. As such when a zebra( or higher ) installed a default route, zebra would not have knowledge of the default unreachable route and so zebra would make the decision to just install the default route. Since zebra installs routes into the kernel with a metric of 20, the new default route would be used. So After mid August, zebra started seeing the default unreachable kernel route. This default unreachable received an admin distance of 0. As such when FRR attempted to install another default route it would fail the admin distance check and not be installed. The Proposal: Since there is no good way to specify admin distance to/from the kernel currently, let's interpret the metric/priority for ZEBRA_ROUTE_KERNEL routes a bit differently admin distance == High order byte of metric passed from kernel metric == Low 3 bytes of metric passed from kernel What this allows: The vrf creator scripts( something like iproute2 ) can be modified to use a metric that would correspond to an admin distance of 255. This way when the default unreachable is read we translate the metric to Admin distance = 255 metric = <some value> This allows zebra to allow static routes and routing protocols to override the default unreachable and it allows people who do not modify the metric values for normal ( ip route .... commands ) for their routes to behave like they think it should (the kernel should win) This does not affect *bsd because most bsd's have a uint8_t metric and we will not change behavior their. donald

1 1

IPv6 Multihop BGP & Vendor specific capability 131
by David Hooton 27 Sep '17

27 Sep '17

Hi All, I’m running 3.0-rc2 and appear to have found a bug in the IPv6 multihop BGP code. The box is configured to peer with a 3rd party ASR9000 and can’t seem to keep the session up. It will download ~30,000 routes then reset the session with the following errors in the logs. Sep 27 19:19:45 ord-285eec43a74305dd bgpd[16943]: bgp_mp_reach_parse: (2001:XX00:100::30:XXX) Failed to read NLRI Sep 27 19:19:45 ord-285eec43a74305dd bgpd[16943]: %NOTIFICATION: sent to neighbor 2001:XX00:100::30:XXX 3/1 (UPDATE Message Error/Malformed Attribute List) 0 bytes Sep 27 19:19:45 ord-285eec43a74305dd bgpd[16943]: 2001:XX00:100::30:XXX: Attribute MP_REACH_NLRI, parse error Sep 27 19:19:45 ord-285eec43a74305dd bgpd[16943]: %ADJCHANGE: neighbor 2001:XX00:100::30:XXX(Unknown) in vrf Default Down BGP Notification send Sep 27 19:19:47 ord-285eec43a74305dd bgpd[16943]: 2001:XX00:100::30:XXX Vendor specific capability 131 Sep 27 19:19:47 ord-285eec43a74305dd bgpd[16943]: %ADJCHANGE: neighbor 2001:XX00:100::30:XXX(Unknown) in vrf Default Up Sep 27 19:19:49 ord-285eec43a74305dd bgpd[16943]: bgp_mp_reach_parse: (2001:XX00:100::30:XXX) Failed to read NLRI Sep 27 19:19:49 ord-285eec43a74305dd bgpd[16943]: %NOTIFICATION: sent to neighbor 2001:XX00:100::30:XXX 3/1 (UPDATE Message Error/Malformed Attribute List) 0 bytes Sep 27 19:19:49 ord-285eec43a74305dd bgpd[16943]: 2001:XX00:100::30:XXX: Attribute MP_REACH_NLRI, parse error Sep 27 19:19:49 ord-285eec43a74305dd bgpd[16943]: %ADJCHANGE: neighbor 2001:XX00:100::30:XXX(Unknown) in vrf Default Down BGP Notification send A single hop BGP session seems to work fine, it is just the multihop sessions which won’t stay up. Any suggestions? Kind Regards, David Hooton Founder | Ordnance Cloud Scale, Carrier Grade P: +61415850000<tel:+61415850000> T: @dave_hooton W: ordnance.co

2 2

New Defects reported by Coverity Scan for freerangerouting/frr
by scan-admin＠coverity.com 27 Sep '17

27 Sep '17

Hi, Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 1 new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 1457145: Memory - corruptions (ARRAY_VS_SINGLETON) /lib/plist.c: 726 in prefix_list_apply_which_prefix() ________________________________________________________________________________________________________ *** CID 1457145: Memory - corruptions (ARRAY_VS_SINGLETON) /lib/plist.c: 726 in prefix_list_apply_which_prefix() 720 721 if (--depth) { 722 if (!table->entries[*byte].next_table) 723 break; 724 725 table = table->entries[*byte].next_table; >>> CID 1457145: Memory - corruptions (ARRAY_VS_SINGLETON) >>> Using "byte" as an array. This might corrupt or misinterpret adjacent memory locations. 726 byte++; 727 continue; 728 } 729 730 for (pentry = table->entries[*byte].final_chain; pentry; 731 pentry = pentry->next_best) { ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "frr(a)lists.nox.tf", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

New Defects reported by Coverity Scan for freerangerouting/frr
by scan-admin＠coverity.com 23 Sep '17

23 Sep '17

Hi, Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 1 new defect(s) introduced to freerangerouting/frr found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 1457040: Possible Control flow issues (DEADCODE) /zebra/rt_netlink.c: 1346 in netlink_route_multipath() ________________________________________________________________________________________________________ *** CID 1457040: Possible Control flow issues (DEADCODE) /zebra/rt_netlink.c: 1346 in netlink_route_multipath() 1340 else { 1341 req.r.rtm_table = RT_TABLE_UNSPEC; 1342 addattr32(&req.n, sizeof req, RTA_TABLE, re->table); 1343 } 1344 1345 if (discard) >>> CID 1457040: Possible Control flow issues (DEADCODE) >>> Execution cannot reach this statement: "goto skip;". 1346 goto skip; 1347 1348 if (re->mtu || re->nexthop_mtu) { 1349 char buf[NL_PKT_BUF_SIZE]; 1350 struct rtattr *rta = (void *)buf; 1351 u_int32_t mtu = re->mtu; ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "frr(a)lists.nox.tf", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

List of Maintainers
by Donald Sharp 19 Sep '17

19 Sep '17

All -- During todays technical meeting we discussed formalizing the list of maintainers on github -> frrouting.org website list. The current list is( in no particular order ): Russ White Daniel Walton Bingen Eguzkitza Renato Westphal Christian Franke Martin Winter David Lamparter JR Rivers Lou Berger Donnie Savage Quentin Young Olivier Dugeon Vincent Jardin Donald Sharp Jafar Al-Gharaibeh Nicolas Dichtel Philippe Guibert 1) If you are on this list and no-longer want to be, please feel free to let the TSC know and we will remove you 2) If you are not on this list and would like to be, please feel free to let the TSC know Thanks! donald

1 0

Babel Admin Distance
by Donald Sharp 13 Sep '17

13 Sep '17

All - Renato and I noticed that zebra was not properly handling Admin distance correctly. As part of cleaenup we created this PR: https://github.com/FRRouting/frr/pull/1160 Which has this commit: https://github.com/FRRouting/frr/pull/1160/commits/c710b277cfde65fab8ae071f… During discussion about this the other night, I guessed at an admin distance of 100 for Babel. In conversation this morning with Don Slice he indicated that a number of 100 might be a bit low as that do we really trust Babel more than OSPF? Do people have opinions on what the Admin Distance should be set too? thanks! donald

4 5

Some TSC discussion during next weeks Technical Meeting(9/19)
by Donald Sharp 12 Sep '17

12 Sep '17

All - During next weeks Technical Meeting I'd like to carve out 10-15 minutes for a TSC discussion related to our Maintainers List and the upcoming yearly October elections. If you are interested in this conversation please take a moment to ensure your schedule is clear. I'd like to discuss these items: 1) Consolidation of github Maintainers and frrouting.org maintainers lists. 2) Do we need to modify the nomination process? Opening of Nominees? 3) Selection of a person to be in charge of voting in Oct. If you would like to attend and do not regularly attend please feel free to send me an email either here or on slack so I can get you added to the fun. Thanks! donald

1 0

Re: [dev] [PATCH] bgpd: fix length calculation of multi-segment AS_PATH UPDATE messages
by Donald Sharp 09 Sep '17

09 Sep '17

Andreas - Thanks for taking the time and solving this AS_PATH issue! I'm about to submit this patch for a Pull Request into FRR. I'm attributing the patch to you, before I can do so I need your 'Signed-off-by:...'. I'm including the relevant part of our documentation to see what we think it signifies: `Signed-off-by:` this is a developer's certification that he or she has the right to submit the patch for inclusion into the project. It is an agreement to the Developer's Certificate of Origin (below). Code without a proper signoff can not and will not be merged. If you are unfamiliar with this process, you should read the [official policy at kernel.org](http://www.kernel.org/doc/Documentation/SubmittingPatches) and you might find this article about [participating in the Linux community on the Linux Foundation website](http://www.linuxfoundation.org/content/how-participate-linux-commu… to be a helpful resource. In short, when you sign off on a commit, you assert your agreement to all of the following: > Developer's Certificate of Origin 1.1 > > By making a contribution to this project, I certify that: > > (a) The contribution was created in whole or in part by me and I > have the right to submit it under the open source license > indicated in the file; or > > (b) The contribution is based upon previous work that, to the best > of my knowledge, is covered under an appropriate open source > license and I have the right under that license to submit that > work with modifications, whether created in whole or in part > by me, under the same open source license (unless I am > permitted to submit under a different license), as indicated > in the file; or > > (c) The contribution was provided directly to me by some other > person who certified (a), (b) or (c) and I have not modified > it. > > (d) I understand and agree that this project and the contribution > are public and that a record of the contribution (including all > personal information I submit with it, including my sign-off) is > maintained indefinitely and may be redistributed consistent with > this project or the open source license(s) involved. thanks! donald On Thu, Sep 7, 2017 at 10:54 PM, Andreas Jaggi via dev <dev(a)lists.frrouting.org> wrote: > _______________________________________________ > dev mailing list > dev(a)lists.frrouting.org > https://lists.frrouting.org/listinfo/dev > > > ---------- Forwarded message ---------- > From: Andreas Jaggi <aj(a)open.ch> > To: quagga-dev(a)lists.quagga.net, dev(a)lists.frrouting.org > Cc: > Bcc: > Date: Fri, 8 Sep 2017 04:53:37 +0200 > Subject: [PATCH] bgpd: fix length calculation of multi-segment AS_PATH UPDATE messages > Hi > > There is a bug in the length calculation of mutli-segment AS_PATH UPDATE messages, which can make bgpd send out malformed BGP UPDATE packets and in turn causes that BGP sessions are reset. > > This thread on quagga-users shows how the problem manifests: https://lists.quagga.net/pipermail/quagga-users/2017-September/014830.html > > In our case we had an UPDATE message having an AS_PATH with 567 entries which was split into three segments (255+255+57). > This results in an AS_PATH attribute of the following length: 2274 bytes > (eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 57*4 segment length) > The AS_PATH attribute was written properly into the message, but the length written in the path attribute header by bgpd was 3294 bytes instead of 2274 (thus resulting in an invalid/malformed BGP UPDATE message). > > Turns out that the aspath_put function wrongly double-accumulates the written bytes and then ends up with these 3294 bytes instead of 2274. > (eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 510*4 segment length + 2 bytes segment header + 57*4 segment length). > > > The following patch (against the master branch of Quagga) fixes the length calculation in aspath_put: > > diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c > index b7af5e88..d813bfba 100644 > --- a/bgpd/bgp_aspath.c > +++ b/bgpd/bgp_aspath.c > @@ -903,7 +903,7 @@ aspath_put (struct stream *s, struct aspath *as, int use32bit ) > assegment_header_put (s, seg->type, AS_SEGMENT_MAX); > assegment_data_put (s, seg->as, AS_SEGMENT_MAX, use32bit); > written += AS_SEGMENT_MAX; > - bytes += ASSEGMENT_SIZE (written, use32bit); > + bytes += ASSEGMENT_SIZE (AS_SEGMENT_MAX, use32bit); > } > > /* write the final segment, probably is also the first */ > > > It looks like this bug has been slumbering for 10 years since its introduction in 2007 when it was added with the AS4 support changes (commit 0b2aa3a0). > > Which leads us to the following affected versions: > Quagga: 0.99.10 - 0.99.24.1, 1.0.*, 1.1.0, 1.1.1, 1.2.0, 1.2.1 > FRR: 2.0, 3.0-rc1 > > > Cheers > Andreas > > > -- > andreas jaggi > lead engineer network services > > open systems ag > raeffelstrasse 29 > ch-8045 zurich > t: +41 58 100 10 10 > f: +41 58 100 10 11 > aj(a)open.ch > > http://www.open.ch > >

2 1

About API interface in FRRouting
by Wing Lin 08 Sep '17

08 Sep '17

Hi, I have been studying FRRouting recently. And I want to use FRRouting in my code, so I have one question below. Is there any API interface in FRRouting exposed to users? Looking forward to your reply. Thanks!

2 1