bgpd: fix NULL dereference in vrf-vpn leak config if before default instance

Issue 2473

VRF-VPN route-leaking configuration commands assumed existence of
default bgp instance. If a non-default vrf configuration occurred
before the default vrf configuration, it would result in a (NULL)
dereference to the non-existent default vrf.

This change 1) detects non-existence of default vrf and skips corresponding
RIB updating that normally occurs during configuration changes of the
route-leaking parameters; and 2) when the default bgp instance is defined,
runs the deferred RIB updating.

In vpn_leak_to_vrf_update_all(), replace early return if bgp_vpn is NULL
with assert, as the former would lead to subtly wrong RIB contents. The
underlying issue that led to commit 73aed5841a1fe31e8a1f251dfcf87e1f2b1f2463
should be fixed by the above changes.

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

diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 3a854be534bf39ff17534db70e3213d91be5ae44..db2acf80588f96e3302483d0dd1dd96fc359bb28 100644 (file)
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -490,7 +490,7 @@ leak_update(
         * (only one hop back to ultimate parent for vrf-vpn-vrf scheme).
         * Using a loop here supports more complex intra-bgp import-export
         * schemes that could be implemented in the future.
-        * 
+        *
         */
        for (bi_ultimate = source_bi;
                bi_ultimate->extra && bi_ultimate->extra->parent;
@@ -1356,8 +1356,7 @@ void vpn_leak_to_vrf_update_all(struct bgp *bgp_vrf, /* to */
        struct bgp_node *prn;
        safi_t safi = SAFI_MPLS_VPN;
 
-       if (!bgp_vpn)
-               return;
+       assert(bgp_vpn);
 
        /*
         * Walk vpn table
@@ -2253,3 +2252,66 @@ vrf_id_t get_first_vrf_for_redirect_with_rt(struct ecommunity *eckey)
        }
        return VRF_UNKNOWN;
 }
+
+/*
+ * The purpose of this function is to process leaks that were deferred
+ * from earlier per-vrf configuration due to not-yet-existing default
+ * vrf, in other words, configuration such as:
+ *
+ *     router bgp MMM vrf FOO
+ *       address-family ipv4 unicast
+ *         rd vpn export 1:1
+ *       exit-address-family
+ *
+ *     router bgp NNN
+ *       ...
+ *
+ * This function gets called when the default instance ("router bgp NNN")
+ * is created.
+ */
+void vpn_leak_postchange_all(void)
+{
+       struct listnode *next;
+       struct bgp *bgp;
+       struct bgp *bgp_default = bgp_get_default();
+
+       assert(bgp_default);
+
+       /* First, do any exporting from VRFs to the single VPN RIB */
+       for (ALL_LIST_ELEMENTS_RO(bm->bgp, next, bgp)) {
+
+               if (bgp->inst_type != BGP_INSTANCE_TYPE_VRF)
+                       continue;
+
+               vpn_leak_postchange(
+                       BGP_VPN_POLICY_DIR_TOVPN,
+                       AFI_IP,
+                       bgp_default,
+                       bgp);
+
+               vpn_leak_postchange(
+                       BGP_VPN_POLICY_DIR_TOVPN,
+                       AFI_IP6,
+                       bgp_default,
+                       bgp);
+       }
+
+       /* Now, do any importing to VRFs from the single VPN RIB */
+       for (ALL_LIST_ELEMENTS_RO(bm->bgp, next, bgp)) {
+
+               if (bgp->inst_type != BGP_INSTANCE_TYPE_VRF)
+                       continue;
+
+               vpn_leak_postchange(
+                       BGP_VPN_POLICY_DIR_FROMVPN,
+                       AFI_IP,
+                       bgp_default,
+                       bgp);
+
+               vpn_leak_postchange(
+                       BGP_VPN_POLICY_DIR_FROMVPN,
+                       AFI_IP6,
+                       bgp_default,
+                       bgp);
+       }
+}

diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 384108dc0c475bc7af4ce81265705c146bfd3571..b0add40da9f94f0982ceaaa2785773ef09c9e98a 100644 (file)
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -182,6 +182,10 @@ static inline void vpn_leak_prechange(vpn_policy_direction_t direction,
                                      afi_t afi, struct bgp *bgp_vpn,
                                      struct bgp *bgp_vrf)
 {
+       /* Detect when default bgp instance is not (yet) defined by config */
+       if (!bgp_vpn)
+               return;
+
        if ((direction == BGP_VPN_POLICY_DIR_FROMVPN) &&
                vpn_leak_from_vpn_active(bgp_vrf, afi, NULL)) {
 
@@ -198,6 +202,10 @@ static inline void vpn_leak_postchange(vpn_policy_direction_t direction,
                                       afi_t afi, struct bgp *bgp_vpn,
                                       struct bgp *bgp_vrf)
 {
+       /* Detect when default bgp instance is not (yet) defined by config */
+       if (!bgp_vpn)
+               return;
+
        if (direction == BGP_VPN_POLICY_DIR_FROMVPN)
                vpn_leak_to_vrf_update_all(bgp_vrf, bgp_vpn, afi);
        if (direction == BGP_VPN_POLICY_DIR_TOVPN) {
@@ -216,4 +224,6 @@ extern void vpn_policy_routemap_event(const char *rmap_name);
 
 extern vrf_id_t get_first_vrf_for_redirect_with_rt(struct ecommunity *eckey);
 
+extern void vpn_leak_postchange_all(void);
+
 #endif /* _QUAGGA_BGP_MPLSVPN_H */

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 86f3f97c4934f7eb04197e50eea2c9d6baa2c5a8..88c310d9d5bcf6d398700e6b9d7ecef8bf3f3f3b 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -928,6 +928,14 @@ DEFUN_NOSH (router_bgp,
                        return CMD_WARNING_CONFIG_FAILED;
                }
 
+               /*
+                * If we just instantiated the default instance, complete
+                * any pending VRF-VPN leaking that was configured via
+                * earlier "router bgp X vrf FOO" blocks.
+                */
+               if (inst_type == BGP_INSTANCE_TYPE_DEFAULT)
+                       vpn_leak_postchange_all();
+
                /* Pending: handle when user tries to change a view to vrf n vv.
                 */
        }