- dev - lists.frrouting.org

[cmaster-next] [PATCH] bgpd: Fix 'show ip bgp' to not change it's output
by Donald Sharp 05 Dec '16

05 Dec '16

For unicast routes do not change the output of the 'show ip bgp' command. This will allow people with existing scripts to continue to work. Signed-off-by: Donald Sharp <sharpd(a)cumulusnetworks.com> --- bgpd/bgp_route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 23ecefa..c2c6304 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -6329,7 +6329,7 @@ route_vty_out (struct vty *vty, struct prefix *p, vty_out (vty, "%s", VTY_NEWLINE); #if ENABLE_BGP_VNC /* prints an additional line, indented, with VNC info, if present */ - if ((safi == SAFI_MPLS_VPN) || (safi == SAFI_ENCAP) || (safi == SAFI_UNICAST)) + if ((safi == SAFI_MPLS_VPN) || (safi == SAFI_ENCAP)) rfapi_vty_out_vncinfo(vty, p, binfo, safi); #endif } -- 2.7.4

2 1

[cmaster-next] [PATCH] bgpd: Remove nexthop for peer only for "real" peer
by Donald Sharp 05 Dec '16

05 Dec '16

From: vivek <vivek(a)cumulusnetworks.com> During connection establishment, there is a separate peer structure created for the doppelganger (for incoming connection). When this is deleted after the connection has established, take care to ensure that the nexthop entry for the peer is not deleted. Fixes: f9164b1d74f6a20d69d7ef10d2e39b4ae7996cbf Signed-off-by: Vivek Venkatraman <vivek(a)cumulusnetworks.com> Ticket: CM-13875 Reviewed By: None Testing Done: Manual (cherry picked from commit 4f2bc892cbddbf36bd5e1b2f36c33260af614b33) --- bgpd/bgpd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7675df2..7f92310 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1972,7 +1972,8 @@ peer_delete (struct peer *peer) bgp_fsm_change_status (peer, Deleted); /* Remove from NHT */ - bgp_unlink_nexthop_by_peer (peer); + if (CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) + bgp_unlink_nexthop_by_peer (peer); /* Password configuration */ if (peer->password) -- 2.5.5

2 1

[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 03 Dec '16

03 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 02 Dec '16

02 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