]> git.puffer.fish Git - mirror/frr.git/commitdiff
zebra: fix for unexpected fdb entry showing up during ifdown/ifup events
authorsharathr <sharathr@cumulusnetworks.com>
Fri, 8 Oct 2021 14:27:50 +0000 (07:27 -0700)
committerStephen Worley <sworley@nvidia.com>
Mon, 13 Feb 2023 23:12:05 +0000 (18:12 -0500)
Ticket: 2674793
Testing Done:  precommit, evpn-min and evpn-smoke

The problem in this case is whenever we are triggering ifdown
followed by ifup of bridge, we see that remote mac entries
are programmed with vlan-1 in the fdb from zebra and never cleaned up.
bridge has vlan_default_pvid 1 which means any port that gets added
will initially have vlan 1 which then gets deleted by ifupdown2 and
the proper vlan gets added.

The problem lies in zebra where we are not cleaning up the remote
macs during vlan change.

Fix is to uninstall the remote macs and then install them
during vlan change.

Signed-off-by: Stephen Worley <sworley@nvidia.com>
zebra/zebra_evpn.c
zebra/zebra_evpn.h
zebra/zebra_l2.c
zebra/zebra_l2.h
zebra/zebra_vxlan.h
zebra/zebra_vxlan_if.c
zebra/zebra_vxlan_if.h

index 77b028ad64f04993b9489daa65e51176e7c77131..bbe53adb2f1ab62ec3fb6eb91dadf03752b81a96 100644 (file)
@@ -943,10 +943,25 @@ struct interface *zebra_evpn_map_to_macvlan(struct interface *br_if,
        return tmp_if;
 }
 
+/*
+ * Uninstall MAC hash entry - called upon access VLAN change.
+ */
+static void zebra_evpn_uninstall_mac_hash(struct hash_bucket *bucket,
+                                         void *ctxt)
+{
+       struct zebra_mac *mac;
+       struct mac_walk_ctx *wctx = ctxt;
+
+       mac = (struct zebra_mac *)bucket->data;
+
+       if (CHECK_FLAG(mac->flags, ZEBRA_MAC_REMOTE))
+               zebra_evpn_rem_mac_uninstall(wctx->zevpn, mac, false);
+}
+
 /*
  * Install MAC hash entry - called upon access VLAN change.
  */
-void zebra_evpn_install_mac_hash(struct hash_bucket *bucket, void *ctxt)
+static void zebra_evpn_install_mac_hash(struct hash_bucket *bucket, void *ctxt)
 {
        struct zebra_mac *mac;
        struct mac_walk_ctx *wctx = ctxt;
@@ -957,6 +972,44 @@ void zebra_evpn_install_mac_hash(struct hash_bucket *bucket, void *ctxt)
                zebra_evpn_rem_mac_install(wctx->zevpn, mac, false);
 }
 
+/*
+ * Uninstall remote MAC entries for this EVPN.
+ */
+void zebra_evpn_rem_mac_uninstall_all(struct zebra_evpn *zevpn)
+{
+       struct mac_walk_ctx wctx;
+
+       if (!zevpn->mac_table)
+               return;
+
+       memset(&wctx, 0, sizeof(struct mac_walk_ctx));
+       wctx.zevpn = zevpn;
+       wctx.uninstall = 1;
+       wctx.upd_client = 0;
+       wctx.flags = ZEBRA_MAC_REMOTE;
+
+       hash_iterate(zevpn->mac_table, zebra_evpn_uninstall_mac_hash, &wctx);
+}
+
+/*
+ * Install remote MAC entries for this EVPN.
+ */
+void zebra_evpn_rem_mac_install_all(struct zebra_evpn *zevpn)
+{
+       struct mac_walk_ctx wctx;
+
+       if (!zevpn->mac_table)
+               return;
+
+       memset(&wctx, 0, sizeof(struct mac_walk_ctx));
+       wctx.zevpn = zevpn;
+       wctx.uninstall = 0;
+       wctx.upd_client = 0;
+       wctx.flags = ZEBRA_MAC_REMOTE;
+
+       hash_iterate(zevpn->mac_table, zebra_evpn_install_mac_hash, &wctx);
+}
+
 /*
  * Read and populate local MACs and neighbors corresponding to this EVPN.
  */
index b07c6515b2cf9f68dede6cf279d798bfba5b6b87..8218b77213ad6fe3b9fa51eb2609ee9ee03b487e 100644 (file)
@@ -189,7 +189,8 @@ struct zebra_evpn *zebra_evpn_from_svi(struct interface *ifp,
                                       struct interface *br_if);
 struct interface *zebra_evpn_map_to_macvlan(struct interface *br_if,
                                            struct interface *svi_if);
-void zebra_evpn_install_mac_hash(struct hash_bucket *bucket, void *ctxt);
+void zebra_evpn_rem_mac_install_all(struct zebra_evpn *zevpn);
+void zebra_evpn_rem_mac_uninstall_all(struct zebra_evpn *zevpn);
 void zebra_evpn_read_mac_neigh(struct zebra_evpn *zevpn, struct interface *ifp);
 unsigned int zebra_evpn_hash_keymake(const void *p);
 bool zebra_evpn_hash_cmp(const void *p1, const void *p2);
index c43fe49026e6ad47293e4955db43f50631199159..c5d4c58852e945f40ac1b06ae748672b91bcf0ab 100644 (file)
@@ -347,8 +347,8 @@ void zebra_l2_vxlanif_add_update(struct interface *ifp,
                                 struct zebra_l2info_vxlan *vxlan_info, int add)
 {
        struct zebra_if *zif;
-       struct in_addr old_vtep_ip;
        uint16_t chgflags = 0;
+       struct zebra_vxlan_if_update_ctx ctx;
 
        zif = ifp->info;
        assert(zif);
@@ -359,14 +359,16 @@ void zebra_l2_vxlanif_add_update(struct interface *ifp,
                return;
        }
 
-       old_vtep_ip = zif->l2info.vxl.vtep_ip;
+       memset(&ctx, 0, sizeof(ctx));
+       ctx.old_vtep_ip = zif->l2info.vxl.vtep_ip;
 
-       if (!IPV4_ADDR_SAME(&old_vtep_ip, &vxlan_info->vtep_ip)) {
+       if (!IPV4_ADDR_SAME(&ctx.old_vtep_ip, &vxlan_info->vtep_ip)) {
                chgflags |= ZEBRA_VXLIF_LOCAL_IP_CHANGE;
                zif->l2info.vxl.vtep_ip = vxlan_info->vtep_ip;
        }
 
        if (IS_ZEBRA_VXLAN_IF_VNI(zif)) {
+               ctx.old_vni = vxlan_info->vni_info.vni;
                if (!IPV4_ADDR_SAME(&zif->l2info.vxl.vni_info.vni.mcast_grp,
                                    &vxlan_info->vni_info.vni.mcast_grp)) {
                        chgflags |= ZEBRA_VXLIF_MCAST_GRP_CHANGE;
@@ -375,8 +377,10 @@ void zebra_l2_vxlanif_add_update(struct interface *ifp,
                }
        }
 
-       if (chgflags)
-               zebra_vxlan_if_update(ifp, chgflags);
+       if (chgflags) {
+               ctx.chgflags = chgflags;
+               zebra_vxlan_if_update(ifp, &ctx);
+       }
 }
 
 /*
@@ -388,6 +392,7 @@ void zebra_l2_vxlanif_update_access_vlan(struct interface *ifp,
        struct zebra_if *zif;
        vlanid_t old_access_vlan;
        struct zebra_vxlan_vni *vni;
+       struct zebra_vxlan_if_update_ctx ctx;
 
 
        zif = ifp->info;
@@ -401,12 +406,15 @@ void zebra_l2_vxlanif_update_access_vlan(struct interface *ifp,
        if (old_access_vlan == access_vlan)
                return;
 
+       memset(&ctx, 0, sizeof(ctx));
        vni = zebra_vxlan_if_vni_find(zif, 0);
+       ctx.old_vni = *vni;
+       ctx.chgflags = ZEBRA_VXLIF_VLAN_CHANGE;
        vni->access_vlan = access_vlan;
 
        zebra_evpn_vl_vxl_deref(old_access_vlan, vni->vni, zif);
        zebra_evpn_vl_vxl_ref(access_vlan, vni->vni, zif);
-       zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_VLAN_CHANGE);
+       zebra_vxlan_if_update(ifp, &ctx);
 }
 
 /*
@@ -435,6 +443,9 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp,
        ifindex_t old_bridge_ifindex;
        ns_id_t old_ns_id;
        struct zebra_vrf *zvrf;
+       struct zebra_vxlan_if_update_ctx ctx;
+
+       memset(&ctx, 0, sizeof(ctx));
 
        zif = ifp->info;
        assert(zif);
@@ -445,11 +456,14 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp,
 
        if (zif->zif_type == ZEBRA_IF_VXLAN
            && chgflags != ZEBRA_BRIDGE_NO_ACTION) {
-               if (chgflags & ZEBRA_BRIDGE_MASTER_MAC_CHANGE)
-                       zebra_vxlan_if_update(ifp,
-                                             ZEBRA_VXLIF_MASTER_MAC_CHANGE);
-               if (chgflags & ZEBRA_BRIDGE_MASTER_UP)
-                       zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_MASTER_CHANGE);
+               if (chgflags & ZEBRA_BRIDGE_MASTER_MAC_CHANGE) {
+                       ctx.chgflags = ZEBRA_VXLIF_MASTER_MAC_CHANGE;
+                       zebra_vxlan_if_update(ifp, &ctx);
+               }
+               if (chgflags & ZEBRA_BRIDGE_MASTER_UP) {
+                       ctx.chgflags = ZEBRA_VXLIF_MASTER_CHANGE;
+                       zebra_vxlan_if_update(ifp, &ctx);
+               }
        }
        old_bridge_ifindex = zif->brslave_info.bridge_ifindex;
        old_ns_id = zif->brslave_info.ns_id;
@@ -457,6 +471,9 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp,
            old_ns_id == zif->brslave_info.ns_id)
                return;
 
+       ctx.chgflags = ZEBRA_VXLIF_MASTER_CHANGE;
+
+
        zif->brslave_info.ns_id = ns_id;
        zif->brslave_info.bridge_ifindex = bridge_ifindex;
        /* Set up or remove link with master */
@@ -464,7 +481,7 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp,
                zebra_l2_map_slave_to_bridge(&zif->brslave_info, zvrf->zns);
                /* In the case of VxLAN, invoke the handler for EVPN. */
                if (zif->zif_type == ZEBRA_IF_VXLAN)
-                       zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_MASTER_CHANGE);
+                       zebra_vxlan_if_update(ifp, &ctx);
                if (zif->es_info.es)
                        zebra_evpn_es_local_br_port_update(zif);
        } else if (old_bridge_ifindex != IFINDEX_INTERNAL) {
@@ -474,7 +491,7 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp,
                 * to unmapping the interface from the bridge.
                 */
                if (zif->zif_type == ZEBRA_IF_VXLAN)
-                       zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_MASTER_CHANGE);
+                       zebra_vxlan_if_update(ifp, &ctx);
                if (zif->es_info.es)
                        zebra_evpn_es_local_br_port_update(zif);
                zebra_l2_unmap_slave_from_bridge(&zif->brslave_info);
index 0e196a851fa06e3bfc210af7e5042c4b583db86d..e1ce13b163d5fd65960e969118e65140c122a023 100644 (file)
@@ -108,6 +108,13 @@ struct zebra_vxlan_if_vlan_ctx {
        struct zebra_vxlan_vni *vni;
 };
 
+struct zebra_vxlan_if_update_ctx {
+       uint16_t chgflags;
+       struct in_addr old_vtep_ip;
+       struct zebra_vxlan_vni old_vni;
+       struct hash *old_vni_table;
+};
+
 struct zebra_vxlan_if_ctx {
        /* input */
        struct zebra_if *zif;
index c1b53075f97940196dfe37babaa4fe7b75536e7d..b33c215c7359b922395b195f0ea5dd36de543992 100644 (file)
@@ -191,7 +191,8 @@ extern int zebra_vxlan_check_readd_vtep(struct interface *ifp, vni_t vni,
 extern int zebra_vxlan_if_up(struct interface *ifp);
 extern int zebra_vxlan_if_down(struct interface *ifp);
 extern int zebra_vxlan_if_add(struct interface *ifp);
-extern int zebra_vxlan_if_update(struct interface *ifp, uint16_t chgflags);
+extern int zebra_vxlan_if_update(struct interface *ifp,
+                                struct zebra_vxlan_if_update_ctx *ctx);
 extern int zebra_vxlan_if_del(struct interface *ifp);
 extern int zebra_vxlan_process_vrf_vni_cmd(struct zebra_vrf *zvrf, vni_t vni,
                                           char *err, int err_str_sz,
index 57fdc19535866e026d5e7cb802b355dd9ef7488b..31cef767dec48c39f9ccf801b25cd62b1b5d8270 100644 (file)
@@ -175,9 +175,11 @@ static int zebra_vxlan_if_del_vni(struct interface *ifp,
 
 static int zebra_vxlan_if_update_vni(struct interface *ifp,
                                     struct zebra_vxlan_vni *vnip,
-                                    uint16_t chgflags)
+                                    struct zebra_vxlan_if_update_ctx *ctx)
 {
        vni_t vni;
+       uint16_t chgflags;
+       vlanid_t access_vlan;
        struct zebra_if *zif;
        struct zebra_l2info_vxlan *vxl;
        struct zebra_evpn *zevpn;
@@ -193,6 +195,7 @@ static int zebra_vxlan_if_update_vni(struct interface *ifp,
        assert(zif);
        vxl = &zif->l2info.vxl;
        vni = vnip->vni;
+       chgflags = ctx->chgflags;
 
        zl3vni = zl3vni_lookup(vni);
        if (zl3vni) {
@@ -299,8 +302,12 @@ static int zebra_vxlan_if_update_vni(struct interface *ifp,
                        /* Remove all existing local neigh and MACs for this VNI
                         * (including from BGP)
                         */
+                       access_vlan = vnip->access_vlan;
+                       vnip->access_vlan = ctx->old_vni.access_vlan;
                        zebra_evpn_neigh_del_all(zevpn, 0, 1, DEL_LOCAL_MAC);
                        zebra_evpn_mac_del_all(zevpn, 0, 1, DEL_LOCAL_MAC);
+                       zebra_evpn_rem_mac_uninstall_all(zevpn);
+                       vnip->access_vlan = access_vlan;
                }
 
                if (zevpn->local_vtep_ip.s_addr != vxl->vtep_ip.s_addr ||
@@ -347,15 +354,11 @@ static int zebra_vxlan_if_update_vni(struct interface *ifp,
                if (chgflags & ZEBRA_VXLIF_MASTER_CHANGE)
                        zebra_evpn_read_mac_neigh(zevpn, ifp);
                else if (chgflags & ZEBRA_VXLIF_VLAN_CHANGE) {
-                       struct mac_walk_ctx m_wctx;
                        struct neigh_walk_ctx n_wctx;
 
                        zebra_evpn_read_mac_neigh(zevpn, ifp);
 
-                       memset(&m_wctx, 0, sizeof(m_wctx));
-                       m_wctx.zevpn = zevpn;
-                       hash_iterate(zevpn->mac_table,
-                                    zebra_evpn_install_mac_hash, &m_wctx);
+                       zebra_evpn_rem_mac_install_all(zevpn);
 
                        memset(&n_wctx, 0, sizeof(n_wctx));
                        n_wctx.zevpn = zevpn;
@@ -493,15 +496,15 @@ static int zebra_vxlan_if_add_update_vni(struct zebra_if *zif,
                                         struct zebra_vxlan_vni *vni,
                                         void *ctxt)
 {
-       struct hash *old_vni_table;
        struct zebra_vxlan_vni vni_tmp;
+       struct zebra_vxlan_if_update_ctx *ctx;
        struct zebra_vxlan_vni *old_vni = NULL;
 
-       old_vni_table = (struct hash *)ctxt;
+       ctx = (struct zebra_vxlan_if_update_ctx *)ctxt;
        memcpy(&vni_tmp, vni, sizeof(*vni));
 
-       old_vni = hash_release(old_vni_table, &vni_tmp);
-       if (!old_vni) {
+       if ((hashcount(ctx->old_vni_table) == 0) ||
+           !(old_vni = hash_release(ctx->old_vni_table, &vni_tmp))) {
                if (IS_ZEBRA_DEBUG_VXLAN)
                        zlog_debug("vxlan %s adding vni(%d, %d)",
                                   zif->ifp->name, vni->vni, vni->access_vlan);
@@ -510,6 +513,9 @@ static int zebra_vxlan_if_add_update_vni(struct zebra_if *zif,
                return 0;
        }
 
+       ctx->old_vni = *old_vni;
+       ctx->chgflags = ZEBRA_VXLIF_VLAN_CHANGE;
+
        /* copy mcast group from old_vni as thats not being changed here */
        vni->mcast_grp = old_vni->mcast_grp;
 
@@ -524,8 +530,7 @@ static int zebra_vxlan_if_add_update_vni(struct zebra_if *zif,
                zebra_evpn_vl_vxl_deref(old_vni->access_vlan, old_vni->vni,
                                        zif);
                zebra_evpn_vl_vxl_ref(vni->access_vlan, vni->vni, zif);
-               zebra_vxlan_if_update_vni(zif->ifp, vni,
-                                         ZEBRA_VXLIF_VLAN_CHANGE);
+               zebra_vxlan_if_update_vni(zif->ifp, vni, ctx);
                zebra_vxlan_vni_free(old_vni);
        }
 
@@ -536,10 +541,10 @@ static int zebra_vxlan_if_vni_entry_update_callback(struct zebra_if *zif,
                                                    struct zebra_vxlan_vni *vni,
                                                    void *ctxt)
 {
-       uint16_t *chgflags;
+       struct zebra_vxlan_if_update_ctx *ctx;
 
-       chgflags = (uint16_t *)ctxt;
-       return zebra_vxlan_if_update_vni(zif->ifp, vni, *chgflags);
+       ctx = (struct zebra_vxlan_if_update_ctx *)ctxt;
+       return zebra_vxlan_if_update_vni(zif->ifp, vni, ctx);
 }
 
 static int zebra_vxlan_if_vni_entry_del_callback(struct zebra_if *zif,
@@ -770,31 +775,32 @@ int zebra_vxlan_if_vni_table_add_update(struct interface *ifp,
                                        struct hash *vni_table)
 {
        struct zebra_if *zif;
-       struct hash *old_vni_table = NULL;
        struct zebra_vxlan_vni_info *vni_info;
+       struct zebra_vxlan_if_update_ctx ctx;
 
        zif = (struct zebra_if *)ifp->info;
 
        vni_info = VNI_INFO_FROM_ZEBRA_IF(zif);
 
-       old_vni_table = vni_info->vni_table;
+       memset(&ctx, 0, sizeof(ctx));
+       ctx.old_vni_table = vni_info->vni_table;
        vni_info->vni_table = vni_table;
 
-       zebra_vxlan_if_vni_iterate(zif, zebra_vxlan_if_add_update_vni,
-                                  old_vni_table);
+       zebra_vxlan_if_vni_iterate(zif, zebra_vxlan_if_add_update_vni, &ctx);
 
        /* release kernel deleted vnis */
-       if (old_vni_table) {
-               if (hashcount(old_vni_table)) {
+       if (ctx.old_vni_table) {
+               if (hashcount(ctx.old_vni_table)) {
                        /* UGLY HACK: Put back the old table so that delete of
                         * MACs goes through and then flip back.
                         */
-                       vni_info->vni_table = old_vni_table;
-                       hash_iterate(old_vni_table, zebra_vxlan_if_vni_clean,
-                                    zif);
+                       vni_info->vni_table = ctx.old_vni_table;
+                       hash_iterate(ctx.old_vni_table,
+                                    zebra_vxlan_if_vni_clean, zif);
                        vni_info->vni_table = vni_table;
                }
-               zebra_vxlan_vni_table_destroy(old_vni_table);
+               zebra_vxlan_vni_table_destroy(ctx.old_vni_table);
+               ctx.old_vni_table = NULL;
        }
 
        return 0;
@@ -805,6 +811,7 @@ int zebra_vxlan_if_vni_mcast_group_update(struct interface *ifp, vni_t vni_id,
 {
        struct zebra_if *zif;
        struct zebra_vxlan_vni *vni;
+       struct zebra_vxlan_if_update_ctx ctx;
 
        zif = (struct zebra_if *)ifp->info;
 
@@ -815,13 +822,16 @@ int zebra_vxlan_if_vni_mcast_group_update(struct interface *ifp, vni_t vni_id,
        if (!vni)
                return 0;
 
+       memset(&ctx, 0, sizeof(ctx));
+       ctx.old_vni.mcast_grp = vni->mcast_grp;
+       ctx.chgflags = ZEBRA_VXLIF_MCAST_GRP_CHANGE;
+
        if (mcast_group)
                vni->mcast_grp = *mcast_group;
        else
                memset(&vni->mcast_grp, 0, sizeof(vni->mcast_grp));
 
-       return zebra_vxlan_if_update_vni(ifp, vni,
-                                        ZEBRA_VXLIF_MCAST_GRP_CHANGE);
+       return zebra_vxlan_if_update_vni(ifp, vni, &ctx);
 }
 
 int zebra_vxlan_if_vni_down(struct interface *ifp, struct zebra_vxlan_vni *vnip)
@@ -1061,7 +1071,8 @@ int zebra_vxlan_if_del(struct interface *ifp)
 /*
  * Handle VxLAN interface update - change to tunnel IP, master or VLAN.
  */
-int zebra_vxlan_if_update(struct interface *ifp, uint16_t chgflags)
+int zebra_vxlan_if_update(struct interface *ifp,
+                         struct zebra_vxlan_if_update_ctx *ctx)
 {
        struct zebra_if *zif;
        struct zebra_vxlan_vni_info *vni_info;
@@ -1071,11 +1082,11 @@ int zebra_vxlan_if_update(struct interface *ifp, uint16_t chgflags)
 
        if (IS_ZEBRA_VXLAN_IF_VNI(zif)) {
                vni_info = VNI_INFO_FROM_ZEBRA_IF(zif);
-               return zebra_vxlan_if_update_vni(ifp, &vni_info->vni, chgflags);
+               return zebra_vxlan_if_update_vni(ifp, &vni_info->vni, ctx);
        }
 
        zebra_vxlan_if_vni_iterate(
-               zif, zebra_vxlan_if_vni_entry_update_callback, &chgflags);
+               zif, zebra_vxlan_if_vni_entry_update_callback, ctx);
 
        return 0;
 }
index e5ee6dc2d9042102e75a0efcaf788a5498c144f7..7d2ddb2d8d237323780445d263d69e0132083161 100644 (file)
@@ -80,7 +80,8 @@ extern int zebra_vxlan_if_vni_table_add_update(struct interface *ifp,
 extern int zebra_vxlan_if_vni_update(struct interface *ifp,
                                     struct zebra_vxlan_vni *vni,
                                     uint16_t chgflags);
-extern int zebra_vxlan_if_update(struct interface *ifp, uint16_t chgflags);
+extern int zebra_vxlan_if_update(struct interface *ifp,
+                                struct zebra_vxlan_if_update_ctx *ctx);
 extern int zebra_vxlan_if_vni_add(struct interface *ifp,
                                  struct zebra_vxlan_vni *vni);
 extern int zebra_vxlan_if_add(struct interface *ifp);