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 <paulz@labn.net>

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 4916e95106d74ef05a2203d985ada6a1b2bbc75c..04dc60621e5e77db43a6ef7e36e6bec1ab2b9fb0 100644 (file)
--- 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 4d7a858becc46aeeed14727fb8a74d83e779b1ff..d35568b8385729179bc79c179ada30deea2d4935 100644 (file)
--- 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 3aa8ceedafc31c47dc71b8991a18d21d53ee1fd4..b14fffadb2ccb19e80f2c5ee857d4d0b10dedfd8 100644 (file)
--- 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) {