bgpd: vpn-vrf routemaps: no leak if named route-map not defined

given a configuration such as this:

    router bgp 7777 vrf A
	address-family ipv4 unicast
	    route-map vpn import FOO
	    import vpn

or this:

    router bgp 7777 vrf A
	address-family ipv4 unicast
	    rd vpn export 1:3
	    rt vpn export 1:100
	    route-map vpn export FOO
	    export vpn

Previous code allowed leaking if the named FOO route-map was not defined.

Since the configuration is logically incomplete, if a route-map is named
for "vpn export" or "vpn import" but is not defined, leaking should not
occur until the route-map is defined.

This changeset implements the correct behavior.

Signed-off-by: G. Paul Ziemba <paulz@labn.net>

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 08aaed6577438899f461c7ef2952b99601a97d5e..d0e881f854f309545f2a2f87bc7015ebe4963082 100644 (file)
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -1315,6 +1315,9 @@ void vpn_leak_to_vrf_update_all(struct bgp *bgp_vrf, /* to */
        }
 }
 
+/*
+ * This function is called for definition/deletion/change to a route-map
+ */
 static void vpn_policy_routemap_update(struct bgp *bgp, const char *rmap_name)
 {
        int debug = BGP_DEBUG(vpn, VPN_LEAK_RMAP_EVENT);
@@ -1331,9 +1334,8 @@ static void vpn_policy_routemap_update(struct bgp *bgp, const char *rmap_name)
 
        for (afi = 0; afi < AFI_MAX; ++afi) {
 
-               if (vpn_leak_to_vpn_active(bgp, afi, NULL)
-                   && bgp->vpn_policy[afi].rmap_name[BGP_VPN_POLICY_DIR_TOVPN]
-                   && !strcmp(rmap_name,
+               if (bgp->vpn_policy[afi].rmap_name[BGP_VPN_POLICY_DIR_TOVPN]
+                       && !strcmp(rmap_name,
                               bgp->vpn_policy[afi]
                                       .rmap_name[BGP_VPN_POLICY_DIR_TOVPN])) {
 
@@ -1349,23 +1351,22 @@ static void vpn_policy_routemap_update(struct bgp *bgp, const char *rmap_name)
                                zlog_debug("%s: after vpn_leak_prechange",
                                           __func__);
 
-                       if (!rmap)
-                               bgp->vpn_policy[afi]
-                                       .rmap[BGP_VPN_POLICY_DIR_TOVPN] = NULL;
+                       /* in case of definition/deletion */
+                       bgp->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_TOVPN] =
+                               rmap;
 
                        vpn_leak_postchange(BGP_VPN_POLICY_DIR_TOVPN, afi,
                                            bgp_get_default(), bgp);
+
                        if (debug)
                                zlog_debug("%s: after vpn_leak_postchange",
                                           __func__);
                }
 
-               char *mapname = bgp->vpn_policy[afi]
-                       .rmap_name[BGP_VPN_POLICY_DIR_FROMVPN];
-
-               if (vpn_leak_from_vpn_active(bgp, afi, NULL) &&
-                       mapname &&
-                       !strcmp(rmap_name, mapname))  {
+               if (bgp->vpn_policy[afi].rmap_name[BGP_VPN_POLICY_DIR_FROMVPN]
+                       && !strcmp(rmap_name,
+                               bgp->vpn_policy[afi]
+                               .rmap_name[BGP_VPN_POLICY_DIR_FROMVPN]))  {
 
                        if (debug) {
                                zlog_debug("%s: rmap \"%s\" matches vrf-policy fromvpn for as %d afi %s",
@@ -1376,11 +1377,9 @@ static void vpn_policy_routemap_update(struct bgp *bgp, const char *rmap_name)
                        vpn_leak_prechange(BGP_VPN_POLICY_DIR_FROMVPN, afi,
                                           bgp_get_default(), bgp);
 
-                       if (!rmap) {
-                               bgp->vpn_policy[afi]
-                                       .rmap[BGP_VPN_POLICY_DIR_FROMVPN] =
-                                       NULL;
-                       }
+                       /* in case of definition/deletion */
+                       bgp->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_FROMVPN] =
+                                       rmap;
 
                        vpn_leak_postchange(BGP_VPN_POLICY_DIR_FROMVPN, afi,
                                            bgp_get_default(), bgp);

diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 739855949830f71451c380a6786abbfcbfa6d5d2..c13030c6c8c34c455325c36cf13f6ac73069b5fd 100644 (file)
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -113,6 +113,14 @@ static inline int vpn_leak_to_vpn_active(struct bgp *bgp_vrf, afi_t afi,
                return 0;
        }
 
+       /* Is a route-map specified, but not defined? */
+       if (bgp_vrf->vpn_policy[afi].rmap_name[BGP_VPN_POLICY_DIR_TOVPN] &&
+               !bgp_vrf->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_TOVPN]) {
+               if (pmsg)
+                       *pmsg = "route-map tovpn named but not defined";
+               return 0;
+       }
+
        /* Is there an "auto" export label that isn't allocated yet? */
        if (CHECK_FLAG(bgp_vrf->vpn_policy[afi].flags,
                BGP_VPN_POLICY_TOVPN_LABEL_AUTO) &&
@@ -144,11 +152,21 @@ static inline int vpn_leak_from_vpn_active(struct bgp *bgp_vrf, afi_t afi,
                        *pmsg = "import not set";
                return 0;
        }
+
+       /* Is there an RT list set? */
        if (!bgp_vrf->vpn_policy[afi].rtlist[BGP_VPN_POLICY_DIR_FROMVPN]) {
                if (pmsg)
                        *pmsg = "rtlist fromvpn not defined";
                return 0;
        }
+
+       /* Is a route-map specified, but not defined? */
+       if (bgp_vrf->vpn_policy[afi].rmap_name[BGP_VPN_POLICY_DIR_FROMVPN] &&
+               !bgp_vrf->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_FROMVPN]) {
+               if (pmsg)
+                       *pmsg = "route-map fromvpn named but not defined";
+               return 0;
+       }
        return 1;
 }