- dev - lists.frrouting.org

[cmaster-next] [PATCH] bgpd: Remove dead function
by Donald Sharp 05 Dec '16

05 Dec '16

peer_afc_set is never called. Remove. Signed-off-by: Donald Sharp <sharpd(a)cumulusnetworks.com> (cherry picked from commit 1d60d3c12bb66e5db09c06a772ace203bed80cba) --- bgpd/bgpd.c | 9 --------- bgpd/bgpd.h | 1 - 2 files changed, 10 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7f92310..dc63550 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1870,15 +1870,6 @@ peer_deactivate (struct peer *peer, afi_t afi, safi_t safi) return ret; } -int -peer_afc_set (struct peer *peer, afi_t afi, safi_t safi, int enable) -{ - if (enable) - return peer_activate (peer, afi, safi); - else - return peer_deactivate (peer, afi, safi); -} - static void peer_nsf_stop (struct peer *peer) { diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index dd6a0fd..4bcbb41 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1265,7 +1265,6 @@ extern int peer_group_listen_range_add(struct peer_group *, struct prefix *); extern int peer_activate (struct peer *, afi_t, safi_t); extern int peer_deactivate (struct peer *, afi_t, safi_t); -extern int peer_afc_set (struct peer *, afi_t, safi_t, int); extern int peer_group_bind (struct bgp *, union sockunion *, struct peer *, struct peer_group *, as_t *); -- 2.5.5

2 3

[cmaster-next] RD/RT discussion from Technical Meeting Yesterday
by Donald Sharp 02 Dec '16

02 Dec '16

During the meeting we discussed option #6 as the viable option. Reasoning: Closely modeled the ietf way of thinking Here it is in a broad outline: vrf <VRFNAME1> ip route … <---- Vrf specific Route router bgp XXX ! (address-family ipv4) network 1.2.3.0/24 route-map foo neighbor ... ! CE session redistribute ospf ! CE setup for OSPF ! no special parameters on redistribute needed; VRF name is already OK <here> - never RD/RT router ospf <INSTANCENUM> ! (is this under vrf config?) redistribute bgp ... router bgp AAA ! core instance address-family vpnv4 | vpnv6 | evpn Network <vpnv4> or <RT2/RT5> config [ rd <> ( could be used) ?] neighbor ... ! PE session vrf <VRFNAME1> rt {import|export|both} RTLIST rd {automatic|VALUE} network <> [<rd>] ! <- most used hopefully router bgp BBB ! core instance vrf <VRFNAME2> rt {import|export|both} RTLIST ! can have value overlap rd {automatic|VALUE} address-family vpnv4 | evpn Network <vpnv4> or <RT2/RT5> config neighbor ... ! PE session Questions asked: For VRF's would we extend this to MAC VRF's also? donald

3 2

[cmaster-next] [PATCH] lib, zebra: Minimize display of link-params sub data
by Donald Sharp 02 Dec '16

02 Dec '16

When link-params is configured it auto starts displaying 6000-02# conf t dell-s6000-02(config)# int swp1 dell-s6000-02(config-if)# link-params dell-s6000-02(config-link-params)# admin-grp 0x12345678 dell-s6000-02(config-link-params)# end dell-s6000-02# show run interface swp1 link-params enable metric 0 <----Remove the bw lines max-bw 1.25e+06 max-rsv-bw 1.25e+06 unrsv-bw 0 1.25e+06 unrsv-bw 1 1.25e+06 unrsv-bw 2 1.25e+06 unrsv-bw 3 1.25e+06 unrsv-bw 4 1.25e+06 unrsv-bw 5 1.25e+06 unrsv-bw 6 1.25e+06 unrsv-bw 7 1.25e+06 admin-grp 305419896 exit-link-params ! I'd like to reduce this to: interface enp0s3 ip igmp ip pim sm link-params enable admin-grp 0x12345678 <----- Fix this to be what we entered exit-link-params ! (cherry picked from commit fc256e310c9d231e8bc1e0a147009463c95ab19f) --- lib/if.c | 10 +++++----- lib/if.h | 1 + zebra/interface.c | 13 +++++++------ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/if.c b/lib/if.c index df53f8d..e745dc9 100644 --- a/lib/if.c +++ b/lib/if.c @@ -1366,14 +1366,14 @@ if_link_params_get (struct interface *ifp) iflp->te_metric = ifp->metric; /* Compute default bandwidth based on interface */ - int bw = (float)((ifp->bandwidth ? ifp->bandwidth : DEFAULT_BANDWIDTH) - * TE_KILO_BIT / TE_BYTE); + iflp->default_bw = ((ifp->bandwidth ? ifp->bandwidth : DEFAULT_BANDWIDTH) + * TE_KILO_BIT / TE_BYTE); /* Set Max, Reservable and Unreserved Bandwidth */ - iflp->max_bw = bw; - iflp->max_rsv_bw = bw; + iflp->max_bw = iflp->default_bw; + iflp->max_rsv_bw = iflp->default_bw; for (i = 0; i < MAX_CLASS_TYPE; i++) - iflp->unrsv_bw[i] = bw; + iflp->unrsv_bw[i] = iflp->default_bw; /* Update Link parameters status */ iflp->lp_status = LP_TE | LP_MAX_BW | LP_MAX_RSV_BW | LP_UNRSV_BW; diff --git a/lib/if.h b/lib/if.h index 7fdd46d..0041a1a 100644 --- a/lib/if.h +++ b/lib/if.h @@ -174,6 +174,7 @@ struct if_stats struct if_link_params { u_int32_t lp_status; /* Status of Link Parameters: */ u_int32_t te_metric; /* Traffic Engineering metric */ + float default_bw; float max_bw; /* Maximum Bandwidth */ float max_rsv_bw; /* Maximum Reservable Bandwidth */ float unrsv_bw[MAX_CLASS_TYPE]; /* Unreserved Bandwidth per Class Type (8) */ diff --git a/zebra/interface.c b/zebra/interface.c index b87f61f..f7ec650 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -2787,20 +2787,21 @@ link_params_config_write (struct vty *vty, struct interface *ifp) vty_out (vty, " link-params%s", VTY_NEWLINE); vty_out(vty, " enable%s", VTY_NEWLINE); - if (IS_PARAM_SET(iflp, LP_TE)) + if (IS_PARAM_SET(iflp, LP_TE) && iflp->te_metric != (u_int32_t)ifp->metric) vty_out(vty, " metric %u%s",iflp->te_metric, VTY_NEWLINE); - if (IS_PARAM_SET(iflp, LP_MAX_BW)) + if (IS_PARAM_SET(iflp, LP_MAX_BW) && iflp->max_bw != iflp->default_bw) vty_out(vty, " max-bw %g%s", iflp->max_bw, VTY_NEWLINE); - if (IS_PARAM_SET(iflp, LP_MAX_RSV_BW)) + if (IS_PARAM_SET(iflp, LP_MAX_RSV_BW) && iflp->max_rsv_bw != iflp->default_bw) vty_out(vty, " max-rsv-bw %g%s", iflp->max_rsv_bw, VTY_NEWLINE); if (IS_PARAM_SET(iflp, LP_UNRSV_BW)) { for (i = 0; i < 8; i++) - vty_out(vty, " unrsv-bw %d %g%s", - i, iflp->unrsv_bw[i], VTY_NEWLINE); + if (iflp->unrsv_bw[i] != iflp->default_bw) + vty_out(vty, " unrsv-bw %d %g%s", + i, iflp->unrsv_bw[i], VTY_NEWLINE); } if (IS_PARAM_SET(iflp, LP_ADM_GRP)) - vty_out(vty, " admin-grp %u%s", iflp->admin_grp, VTY_NEWLINE); + vty_out(vty, " admin-grp 0x%x%s", iflp->admin_grp, VTY_NEWLINE); if (IS_PARAM_SET(iflp, LP_DELAY)) { vty_out(vty, " delay %u", iflp->av_delay); -- 2.5.5

2 7

[cmaster-next] [PATCH] bgpd: Fix route node unlock when clearing adj-out
by Donald Sharp 02 Dec '16

02 Dec '16

From: vivek <vivek(a)cumulusnetworks.com> When clearing the adj-out for a subgroup (e.g., upon peer going down), ensure that the adj-out is removed before unlocking the route node that it points to, otherwise, there is a possibility that the route node may be prematurely freed. Signed-off-by: Vivek Venkatraman <vivek(a)cumulusnetworks.com> Ticket: CM-13690 Reviewed By: None Testing Done: bgp-smoke, resilient-hash tests (cherry picked from commit c3d7d35f3b7152c9e50e52c3cfd60b0dc52df703) --- bgpd/bgp_updgrp_adv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index c485e61..a95a11b 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -573,8 +573,8 @@ subgroup_clear_table (struct update_subgroup *subgrp) SUBGRP_FOREACH_ADJ_SAFE (subgrp, aout, taout) { - bgp_unlock_node (aout->rn); bgp_adj_out_remove_subgroup (aout->rn, aout, subgrp); + bgp_unlock_node (aout->rn); } } -- 2.5.5

1 0

[cmaster-next] [PATCH] ospfd: Fix json Crash with inactive timer
by Donald Sharp 02 Dec '16

02 Dec '16

When nbr->t_inactivity is not active, and you do a show json over the neighbor it will crash ospfd. Fix the code so it prints out -1 when the timer is inactive. Ticket:CM-13835 Signed-off-by: Donald Sharp <sharpd(a)cumulusnetworks.com> Reviewed-by: Don Slice <dslice(a)cumulusnetworks.com> Reviewed-by: Daniel Walton <dwalton(a)cumulusnetworks.com> (cherry picked from commit 78d8fcb9623ab4d9cebf6187a451448e056a84bf) --- ospfd/ospf_vty.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 43a4434..4eeb5c1 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -4590,10 +4590,15 @@ show_ip_ospf_neighbor_detail_sub (struct vty *vty, struct ospf_interface *oi, /* Show Router Dead interval timer. */ if (use_json) { - struct timeval res = tv_sub (nbr->t_inactivity->u.sands, recent_relative_time ()); - unsigned long time_store = 0; - time_store = (1000 * res.tv_sec) + (res.tv_usec / 1000); - json_object_int_add(json_sub, "routerDeadIntervalTimerDueMsec", time_store); + if (nbr->t_inactivity) + { + struct timeval res = tv_sub (nbr->t_inactivity->u.sands, recent_relative_time ()); + unsigned long time_store = 0; + time_store = (1000 * res.tv_sec) + (res.tv_usec / 1000); + json_object_int_add(json_sub, "routerDeadIntervalTimerDueMsec", time_store); + } + else + json_object_int_add(json_sub, "routerDeadIntervalTimerDueMsec", -1); } else vty_out (vty, " Dead timer due in %s%s", -- 2.5.5

1 0

[cmaster-next] CLI plans
by David Lamparter 01 Dec '16

01 Dec '16

Hi all, Quentin and I just had a chat about future CLI plans. Here's a summary for everyone else: - the parser for CLI commands currently merges them straight into the common graph of all CLI commands. This'll be split off into an extra step: (pseudocode) graph = graph_new() command_parse_format(graph, cmd->str) command_merge_graph(graph, target_graph) The idea here is that the separate merge step makes it easier to * handle errors * undo the installation * do other graph transformations before merging - I'm finishing up on keyword specification {keyword OPTION|other A.B.C.D} - it's mostly done and works. There's nothing special for keywords, they're just selections that loop on themselves, allowing multiple uses. - for keyword arg lists one can use by-name references to *fixed* args (e.g. argv_find("keyword")). - by-name references to *variables* (WORD, A.B.C.D) will be pending until we get the merge split off, after which this is a new graph transformation step. - Quentin is looking at tests/, the testcli case is exhibiting some "interesting" behaviour. They're mostly help-string related and some of these may end up just being changes in design. - I have autocomplete for {interface, route-map, etc.} names mostly done, but only in-daemon, not vtysh. I'll push that out; adding vtysh support is a matter of adding a communication channel for completions, e.g. a hidden command. Whoever wants to can do that :) - Help string bloat reduction is complicated and we don't quite have a plan there yet. - Improving the way "no" commands work is on the long-term horizon. For now they need another DEFUN in most cases. Cheers, -David

1 0

[cmaster-next] [PATCH] bgpd: fix invalid memory access in peer_free()
by Renato Westphal 01 Dec '16

01 Dec '16

We shoult not call bgp_unlock() before calling bgp_delete_connected_nexthop() in the peer_free() function. Otherwise, if bgp->lock reaches zero, bgp_free() is called and peer->bgp becomes an invalid pointer in the bgp_delete_connected_nexthop() function. To fix this, move the call to bgp_unlock() to the end of peer_free(). Bug exposed by commit 37d361e ("bgpd: plug several memleaks"). Signed-off-by: Renato Westphal <renato(a)opensourcerouting.org> --- bgpd/bgpd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 22d4dd8..d5aff84 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1019,8 +1019,6 @@ peer_free (struct peer *peer) { assert (peer->status == Deleted); - bgp_unlock(peer->bgp); - /* this /ought/ to have been done already through bgp_stop earlier, * but just to be sure.. */ @@ -1092,6 +1090,8 @@ peer_free (struct peer *peer) bfd_info_free(&(peer->bfd_info)); + bgp_unlock(peer->bgp); + memset (peer, 0, sizeof (struct peer)); XFREE (MTYPE_BGP_PEER, peer); -- 1.9.1

3 3

[cmaster-next] [PATCH 0/2] Minor stable/2.0 patches
by Donald Sharp 01 Dec '16

01 Dec '16

These 2 patches fix issue #7. Donald Sharp (2): ospfd: Fix memory leak when not using json pimd: Fix large integer display of drpriority ospfd/ospf_vty.c | 6 ++++-- pimd/pim_vty.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) -- 2.5.5

2 1

[cmaster-next] link-params sub mode
by Donald Sharp 01 Dec '16

01 Dec '16

Olivier - We've noticed that link-params creates allot of config lines when you configure any part of it. Would you mind if I change the behavior to this: When link-params is configured it auto starts displaying 6000-02# conf t dell-s6000-02(config)# int swp1 dell-s6000-02(config-if)# link-params dell-s6000-02(config-link-params)# admin-grp 0x12345678 dell-s6000-02(config-link-params)# end dell-s6000-02# show run interface swp1 link-params enable metric 0 <----Remove the metric and bw lines max-bw 1.25e+06 max-rsv-bw 1.25e+06 unrsv-bw 0 1.25e+06 unrsv-bw 1 1.25e+06 unrsv-bw 2 1.25e+06 unrsv-bw 3 1.25e+06 unrsv-bw 4 1.25e+06 unrsv-bw 5 1.25e+06 unrsv-bw 6 1.25e+06 unrsv-bw 7 1.25e+06 admin-grp 305419896 exit-link-params ! I'd like to reduce this to: interface swp1 link-params enable admin-grp 0x12345678 <----- Fix this to be what we entered exit-link-params ! Finally do we need to display the enable line as well? donald

3 8

[cmaster-next] Topology Tests added to CI - and new BGPd crash discovered
by Martin Winter 01 Dec '16

01 Dec '16

(Testing the new mailing list as well) First of all: I finally got around and added some framework for Topology Tests based on Mininet to the CI testbed. As Mininet is all based on Python, the whole framework is based on pytest. At this time, there are 2 tests in it: - OSPFv3 simple topology - BGP Multiview topology The git for this setup is here: https://git.netdef.org/scm/netdef/topotests.git There is no proprietary code in it - all can be easy replicated on a Ubuntu 16.04 (I have not tested any other Distros and Mininet requires Linux). Just follow the directions in the README inside the git and you should be able to replicate the setup. The idea is to have some test setup which can be shared and anyone can add tests (The CI system runs all the tests in this git) Happy to hear any feedback on this setup… As a side note, the BGP Multiview test crashes BGPd on startup. Because of this, I have NOT YET enabled the test results parsing in the CI system (or all builds would fail at this time) If you look at the the latest build on our CI system, you’ll see a crash in BGPd See https://ci1.netdef.org/browse/QUAGGA-CMASTERNEXT-TOPOU1604-106 Seems to be a crash in the VNC code. (cc’ing Lou to make sure he see this…) Again, please provide feedback. The idea is to add many more topologies to the CI system, but would like to hear some feedback on the setup. - Martin

3 2