From d555f3e904439620d5d05ae3b314a74c0516314a Mon Sep 17 00:00:00 2001 From: "G. Paul Ziemba" Date: Sun, 18 Mar 2018 15:08:45 -0700 Subject: [PATCH] bgpd: vpn-vrf-leaking new CLI: address Vivek's review comments + crash fix - vpn_leak_to_vpn_active(): check instance type - vpn_leak_prechange(): qualify with test for active - vpn_leak_postchange(): remove duplicated call to vpn_leak_from_vrf_update_all() - bgp_vty.c: Avoid null-pointer dereference for command "no rt vpn import" Signed-off-by: G. Paul Ziemba --- bgpd/bgp_mplsvpn.c | 3 +-- bgpd/bgp_mplsvpn.h | 26 +++++++++++++++++++------- bgpd/bgp_vty.c | 15 ++++++++------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 4916e95106..04dc60621e 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -738,7 +738,6 @@ void vpn_leak_from_vrf_withdraw_all(struct bgp *bgp_vpn, /* to */ safi_t safi = SAFI_MPLS_VPN; /* - * Walk vpn table, delete bi with parent == bgp_vrf * Walk vpn table, delete bi with bgp_orig == bgp_vrf */ for (prn = bgp_table_top(bgp_vpn->rib[afi][safi]); prn; @@ -1144,7 +1143,7 @@ static void vpn_policy_routemap_update(struct bgp *bgp, const char *rmap_name) char *mapname = bgp->vpn_policy[afi] .rmap_name[BGP_VPN_POLICY_DIR_FROMVPN]; - if (vpn_leak_from_vpn_active(bgp, afi, NULL) && + if (vpn_leak_from_vpn_active(bgp, afi, NULL) && mapname && !strcmp(rmap_name, mapname)) { diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index 4d7a858bec..d35568b838 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -81,6 +81,14 @@ extern void vpn_leak_zebra_vrf_label_withdraw(struct bgp *bgp, afi_t afi); static inline int vpn_leak_to_vpn_active(struct bgp *bgp_vrf, afi_t afi, const char **pmsg) { + if (bgp_vrf->inst_type != BGP_INSTANCE_TYPE_VRF + && bgp_vrf->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { + + if (pmsg) + *pmsg = "source bgp instance neither vrf nor default"; + return 0; + } + /* Is vrf configured to export to vpn? */ if (!CHECK_FLAG(bgp_vrf->af_flags[afi][SAFI_UNICAST], BGP_CONFIG_VRF_TO_MPLSVPN_EXPORT)) { @@ -110,7 +118,7 @@ static inline int vpn_leak_from_vpn_active(struct bgp *bgp_vrf, afi_t afi, const char **pmsg) { if (bgp_vrf->inst_type != BGP_INSTANCE_TYPE_VRF - && bgp_vrf->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { + && bgp_vrf->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { if (pmsg) *pmsg = "destination bgp instance neither vrf nor default"; @@ -136,10 +144,16 @@ static inline void vpn_leak_prechange(vpn_policy_direction_t direction, afi_t afi, struct bgp *bgp_vpn, struct bgp *bgp_vrf) { - if (direction == BGP_VPN_POLICY_DIR_FROMVPN) + if ((direction == BGP_VPN_POLICY_DIR_FROMVPN) && + vpn_leak_from_vpn_active(bgp_vrf, afi, NULL)) { + vpn_leak_to_vrf_withdraw_all(bgp_vrf, afi); - if (direction == BGP_VPN_POLICY_DIR_TOVPN) + } + if ((direction == BGP_VPN_POLICY_DIR_TOVPN) && + vpn_leak_to_vpn_active(bgp_vrf, afi, NULL)) { + vpn_leak_from_vrf_withdraw_all(bgp_vpn, bgp_vrf, afi); + } } static inline void vpn_leak_postchange(vpn_policy_direction_t direction, @@ -150,16 +164,14 @@ static inline void vpn_leak_postchange(vpn_policy_direction_t direction, vpn_leak_to_vrf_update_all(bgp_vrf, bgp_vpn, afi); if (direction == BGP_VPN_POLICY_DIR_TOVPN) { - if (bgp_vrf->vpn_policy[afi].tovpn_label - != bgp_vrf->vpn_policy[afi] + if (bgp_vrf->vpn_policy[afi].tovpn_label != + bgp_vrf->vpn_policy[afi] .tovpn_zebra_vrf_label_last_sent) { vpn_leak_zebra_vrf_label_update(bgp_vrf, afi); } vpn_leak_from_vrf_update_all(bgp_vpn, bgp_vrf, afi); } - if (direction == BGP_VPN_POLICY_DIR_TOVPN) - vpn_leak_from_vrf_update_all(bgp_vpn, bgp_vrf, afi); } extern void vpn_policy_routemap_event(const char *rmap_name); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 3aa8ceedaf..b14fffadb2 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -6180,7 +6180,7 @@ DEFPY (af_rd_vpn_export, int yes = 1; if (argv_find(argv, argc, "no", &idx)) - yes = 0; + yes = 0; if (yes) { ret = str2prefix_rd(rd_str, &prd); @@ -6247,7 +6247,7 @@ DEFPY (af_label_vpn_export, int yes = 1; if (argv_find(argv, argc, "no", &idx)) - yes = 0; + yes = 0; if (yes) label = label_val; /* rely on parser to force unsigned */ @@ -6303,7 +6303,7 @@ DEFPY (af_nexthop_vpn_export, int yes = 1; if (argv_find(argv, argc, "no", &idx)) - yes = 0; + yes = 0; if (yes) { if (!sockunion2hostprefix(nexthop_str, &p)) @@ -6388,7 +6388,7 @@ DEFPY (af_rt_vpn_imexport, int yes = 1; if (argv_find(argv, argc, "no", &idx)) - yes = 0; + yes = 0; ret = vpn_policy_getafi(vty, doafi); if (ret != CMD_SUCCESS) @@ -6434,7 +6434,8 @@ DEFPY (af_rt_vpn_imexport, vpn_leak_postchange(dir, afi, bgp_get_default(), bgp); } } - ecommunity_free(&ecom); + if (ecom) + ecommunity_free(&ecom); return CMD_SUCCESS; } @@ -6471,7 +6472,7 @@ DEFPY (af_route_map_vpn_imexport, int yes = 1; if (argv_find(argv, argc, "no", &idx)) - yes = 0; + yes = 0; ret = vpn_policy_getafi(vty, doafi); if (ret != CMD_SUCCESS) @@ -6539,7 +6540,7 @@ DEFPY (bgp_imexport_vpn, vpn_policy_direction_t dir; if (argv_find(argv, argc, "no", &idx)) - yes = 0; + yes = 0; if (BGP_INSTANCE_TYPE_VRF != bgp->inst_type && BGP_INSTANCE_TYPE_DEFAULT != bgp->inst_type) { -- 2.39.5