zebra: Fix MAC change handling for a neighbor

When the MAC changes for a local neighbor, ensure that the neighbor data
structure as well as the link between the neighbor and MAC data structures
is updated correctly.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Mitesh Kanjariya <mitesh@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-17565
Reviewed By: CCR-6605
Testing Done: Manual, evpn-smoke

diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
index 5da773d7a3d3b986b753711298caa8c24f46801c..ad000e3f12c1d5762e970240e0a666364a162f53 100644 (file)
--- a/bgpd/bgp_evpn.c
+++ b/bgpd/bgp_evpn.c
@@ -2192,58 +2192,58 @@ char *bgp_evpn_label2str(mpls_label_t *label, char *buf, int len)
  * NOTE: We don't use prefix2str as the output here is a bit different.
  */
 void
-bgp_evpn_route2json (struct prefix_evpn *p, json_object *json)
+bgp_evpn_route2json(struct prefix_evpn *p, json_object *json)
 {
-  char buf1[ETHER_ADDR_STRLEN];
-  char buf2[PREFIX2STR_BUFFER];
+       char buf1[ETHER_ADDR_STRLEN];
+       char buf2[PREFIX2STR_BUFFER];
 
-  if (!json)
-    return;
+       if (!json)
+               return;
 
        if (p->prefix.route_type == BGP_EVPN_IMET_ROUTE) {
-               json_object_int_add (json, "routeType", p->prefix.route_type);
-               json_object_int_add (json, "ethTag", 0);
-               json_object_int_add (json,
-                                    "ipLen",
-                                    IS_EVPN_PREFIX_IPADDR_V4 (p) ?
+               json_object_int_add(json, "routeType", p->prefix.route_type);
+               json_object_int_add(json, "ethTag", 0);
+               json_object_int_add(json,
+                                   "ipLen",
+                                   IS_EVPN_PREFIX_IPADDR_V4 (p) ?
                                        IPV4_MAX_BITLEN : IPV6_MAX_BITLEN);
-               json_object_string_add (json, "ip",
-                                       inet_ntoa (p->prefix.ip.ipaddr_v4));
+               json_object_string_add(json, "ip",
+                                      inet_ntoa (p->prefix.ip.ipaddr_v4));
        }
        else if (p->prefix.route_type == BGP_EVPN_MAC_IP_ROUTE) {
                if (IS_EVPN_PREFIX_IPADDR_NONE(p)) {
-                 json_object_int_add (json, "routeType", p->prefix.route_type);
-                 json_object_int_add (json, "esi", 0); /* TODO: we don't support esi yet */
-                 json_object_int_add (json, "ethTag", 0);
-                 json_object_int_add (json, "macLen", 8 * ETH_ALEN);
-                 json_object_string_add (json, "mac",
-                                         prefix_mac2str (&p->prefix.mac,
-                                                         buf1, sizeof (buf1)));
+                 json_object_int_add(json, "routeType", p->prefix.route_type);
+                 json_object_int_add(json, "esi", 0); /* TODO: we don't support esi yet */
+                 json_object_int_add(json, "ethTag", 0);
+                 json_object_int_add(json, "macLen", 8 * ETH_ALEN);
+                 json_object_string_add(json, "mac",
+                                        prefix_mac2str (&p->prefix.mac,
+                                                        buf1, sizeof (buf1)));
                } else {
                        u_char family;
 
                        family = IS_EVPN_PREFIX_IPADDR_V4(p) ? \
                           AF_INET : AF_INET6;
 
-                       json_object_int_add (json, "routeType",
+                       json_object_int_add(json, "routeType",
                                             p->prefix.route_type);
-                       json_object_int_add (json, "esi", 0); /* TODO: we don't support esi yet */
-                       json_object_int_add (json, "ethTag", 0);
-                       json_object_int_add (json, "macLen",
-                                            8 * ETH_ALEN);
-                       json_object_string_add (json, "mac",
-                                               prefix_mac2str (&p->prefix.mac,
-                                                               buf1,
-                                                               sizeof (buf1)));
-                       json_object_int_add (json, "ipLen",
-                                            IS_EVPN_PREFIX_IPADDR_V4 (p) ?
+                       json_object_int_add(json, "esi", 0); /* TODO: we don't support esi yet */
+                       json_object_int_add(json, "ethTag", 0);
+                       json_object_int_add(json, "macLen",
+                                           8 * ETH_ALEN);
+                       json_object_string_add(json, "mac",
+                                              prefix_mac2str (&p->prefix.mac,
+                                                              buf1,
+                                                              sizeof (buf1)));
+                       json_object_int_add(json, "ipLen",
+                                           IS_EVPN_PREFIX_IPADDR_V4 (p) ?
                                                IPV4_MAX_BITLEN :
                                                IPV6_MAX_BITLEN);
-                       json_object_string_add (json, "ip",
-                                               inet_ntop (family,
-                                                       &p->prefix.ip.ip.addr,
-                                                       buf2,
-                                                       PREFIX2STR_BUFFER));
+                       json_object_string_add(json, "ip",
+                                              inet_ntop (family,
+                                                         &p->prefix.ip.ip.addr,
+                                                         buf2,
+                                                         PREFIX2STR_BUFFER));
                }
        } else {
                /* Currently, this is to cater to other AF_ETHERNET code. */
@@ -2683,10 +2683,7 @@ int bgp_filter_evpn_routes_upon_martian_nh_change(struct bgp *bgp)
                                        if (bgp_debug_update(ri->peer, &rn->p,
                                                             NULL, 1))
                                                zlog_debug(
-                                                       "%u: prefix %s with "
-                                                       "attr %s - DENIED"
-                                                       "due to martian or seld"
-                                                       "nexthop",
+                                                       "%u: prefix %s with attr %s - DENIED due to martian or self nexthop",
                                                        bgp->vrf_id,
                                                        prefix2str(
                                                                &rn->p,

diff --git a/bgpd/bgp_evpn.h b/bgpd/bgp_evpn.h
index 01474b09d85d09bea4d821fbfc8cf7c6cb99994b..985f41f58675c1ea084bbcca4086fae38e91cb1a 100644 (file)
--- a/bgpd/bgp_evpn.h
+++ b/bgpd/bgp_evpn.h
@@ -28,8 +28,7 @@
 extern void bgp_evpn_handle_router_id_update(struct bgp *bgp, int withdraw);
 extern char *bgp_evpn_label2str(mpls_label_t *label, char *buf, int len);
 extern char *bgp_evpn_route2str(struct prefix_evpn *p, char *buf, int len);
-extern void
-bgp_evpn_route2json (struct prefix_evpn *p, json_object *json);
+extern void bgp_evpn_route2json(struct prefix_evpn *p, json_object *json);
 extern void bgp_evpn_encode_prefix(struct stream *s, struct prefix *p,
                                   struct prefix_rd *prd, mpls_label_t *label,
                                   struct attr *attr, int addpath_encode,

diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c
index af34c3394611b90f08c03a74aa450557ca2506f6..a198e65128180e34673e7fa6ae0efd46ae65d981 100644 (file)
--- a/bgpd/bgp_evpn_vty.c
+++ b/bgpd/bgp_evpn_vty.c
@@ -76,8 +76,8 @@ static void display_import_rt(struct vty *vty, struct irt_node *irt,
        char rt_buf[RT_ADDRSTRLEN];
 
        if (json) {
-               json_rt = json_object_new_object ();
-               json_vnis = json_object_new_array ();
+               json_rt = json_object_new_object();
+               json_vnis = json_object_new_array();
        }
 
        /* TODO: This needs to go into a function */
@@ -521,6 +521,7 @@ static void show_vni_entry(struct hash_backet *backet, void *args[])
 
        if (json) {
                char vni_str[VNI_STR_LEN];
+
                json_object_object_add(json_vni, "exportRTs", json_export_rtl);
                snprintf(vni_str, VNI_STR_LEN, "%u", vpn->vni);
                json_object_object_add(json, vni_str, json_vni);

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index e9bf5743311ad6674be4d4b176b9bd8909056a19..a2a7d2ae1d424028ab2b5da148cbdd2309d07195 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -6246,7 +6246,8 @@ static void route_vty_out_route(struct prefix *p, struct vty *vty,
                            || (IN_CLASSB(destination) && p->prefixlen == 16)
                            || (IN_CLASSA(destination) && p->prefixlen == 8)
                            || p->u.prefix4.s_addr == 0) {
-                               /* When mask is natural, mask is not displayed. */
+                               /* When mask is natural,
+                                  mask is not displayed. */
                        } else
                                len += vty_out(vty, "/%d", p->prefixlen);
                } else {
@@ -6261,12 +6262,12 @@ static void route_vty_out_route(struct prefix *p, struct vty *vty,
                len = vty_out(vty, "%s", buf);
        } else if (p->family == AF_EVPN) {
 #if defined(HAVE_CUMULUS)
-        if (!json)
-          len = vty_out (vty, "%s",
-                         bgp_evpn_route2str((struct prefix_evpn *)p,
-                                           buf, BUFSIZ));
-        else
-          bgp_evpn_route2json ( (struct prefix_evpn *) p, json);
+       if (!json)
+               len = vty_out(vty, "%s",
+                             bgp_evpn_route2str((struct prefix_evpn *)p,
+                                                buf, BUFSIZ));
+       else
+               bgp_evpn_route2json((struct prefix_evpn *) p, json);
 #else
                prefix2str(p, buf, PREFIX_STRLEN);
                len = vty_out(vty, "%s", buf);
@@ -6393,7 +6394,7 @@ void route_vty_out(struct vty *vty, struct prefix *p, struct bgp_info *binfo,
                else
                        vty_out(vty, "%*s", 17, " ");
        } else {
-               route_vty_out_route (p, vty, json_path);
+               route_vty_out_route(p, vty, json_path);
        }
 
        /* Print attribute */
@@ -8379,9 +8380,9 @@ void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp,
                if (has_valid_label)
                        json_object_int_add(json, "localLabel", label);
 
-               json_object_string_add (json, "prefix",
-                                       prefix2str (p, prefix_str,
-                                                   sizeof (prefix_str)));
+               json_object_string_add(json, "prefix",
+                                      prefix2str(p, prefix_str,
+                                                 sizeof(prefix_str)));
        } else {
 #if defined(HAVE_CUMULUS)
                if (safi == SAFI_EVPN)

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 80daac236de210e760df33251586db2a30c3bf9f..5d44c0a1428484654915224f09956a338afad902 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -9040,8 +9040,7 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, u_char use_json,
                                        "    Hostname Capability:");
 
                                if (CHECK_FLAG(p->cap, PEER_CAP_HOSTNAME_ADV)) {
-                                       vty_out(vty, " advertised (name: %s, "
-                                               "domain name: %s)",
+                                       vty_out(vty, " advertised (name: %s,domain name: %s)",
                                                bgp->peer_self->hostname ?
                                                        bgp->peer_self->hostname
                                                        : "n/a",
@@ -9053,8 +9052,7 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, u_char use_json,
                                }
 
                                if (CHECK_FLAG(p->cap, PEER_CAP_HOSTNAME_RCV)) {
-                                       vty_out(vty, " received (name: %s, "
-                                               "domain name: %s)",
+                                       vty_out(vty, " received (name: %s,domain name: %s)",
                                                p->hostname ?
                                                        p->hostname : "n/a",
                                                p->domainname ?

diff --git a/zebra/zebra_l2.c b/zebra/zebra_l2.c
index 452bab003a1416bd552ac4aa19b47d0d6e965c1b..7281622e3333048e31108363773aeb1a211fbe2d 100644 (file)
--- a/zebra/zebra_l2.c
+++ b/zebra/zebra_l2.c
@@ -225,17 +225,17 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp,
 
        /* Set up or remove link with master */
        if (bridge_ifindex != IFINDEX_INTERNAL) {
-               zebra_l2_map_slave_to_bridge (&zif->brslave_info);
+               zebra_l2_map_slave_to_bridge(&zif->brslave_info);
                /* 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, ZEBRA_VXLIF_MASTER_CHANGE);
        } else if (old_bridge_ifindex != IFINDEX_INTERNAL) {
-               /* In the case of VxLAN, invoke the handler for EVPN. Note that
-                * this should be done *prior* to unmapping the interface from the
-                * bridge.
+               /* In the case of VxLAN, invoke the handler for EVPN.
+                * Note that this should be done *prior* to unmapping the interface
+                * from the bridge.
                 */
                if (zif->zif_type == ZEBRA_IF_VXLAN)
-                       zebra_vxlan_if_update (ifp, ZEBRA_VXLIF_MASTER_CHANGE);
-               zebra_l2_unmap_slave_from_bridge (&zif->brslave_info);
+                       zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_MASTER_CHANGE);
+               zebra_l2_unmap_slave_from_bridge(&zif->brslave_info);
        }
 }

diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c
index 2240d7c763a96a16c0fd448c008f250501a05283..4a5fc3849322372646930f4856b2f392d477e72a 100644 (file)
--- a/zebra/zebra_vxlan.c
+++ b/zebra/zebra_vxlan.c
@@ -1441,7 +1441,7 @@ static void zvni_gw_macip_del_for_vni_hash(struct hash_backet *backet,
        zif = ifp->info;
 
        /* If down or not mapped to a bridge, we're done. */
-       if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+       if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                return;
 
        zl2_info = zif->l2info.vxl;
@@ -1485,7 +1485,7 @@ static void zvni_gw_macip_add_for_vni_hash(struct hash_backet *backet,
        zif = ifp->info;
 
        /* If down or not mapped to a bridge, we're done. */
-       if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+       if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                return;
        zl2_info = zif->l2info.vxl;
 
@@ -2889,10 +2889,9 @@ int zebra_vxlan_local_neigh_add_update(struct interface *ifp,
        zebra_vni_t *zvni;
        zebra_neigh_t *n;
        struct zebra_vrf *zvrf;
-       zebra_mac_t *zmac;
+       zebra_mac_t *zmac, *old_zmac;
        char buf[ETHER_ADDR_STRLEN];
        char buf2[INET6_ADDRSTRLEN];
-       int send_upd = 1, send_del = 0;
 
        /* We are only interested in neighbors on an SVI that resides on top
         * of a VxLAN bridge.
@@ -2947,19 +2946,35 @@ int zebra_vxlan_local_neigh_add_update(struct interface *ifp,
                        if (memcmp(n->emac.octet, macaddr->octet,
                                   ETH_ALEN)
                            == 0) {
-                               if (n->ifindex == ifp->ifindex)
-                                       /* we're not interested in whatever has
-                                        * changed. */
-                                       return 0;
-                               /* client doesn't care about a purely local
-                                * change. */
-                               send_upd = 0;
-                       } else
-                               /* If the MAC has changed, issue a delete first
-                                * as this means a
-                                * different MACIP route.
+                               /* Update any params and return - client doesn't
+                                * care about a purely local change.
+                                */
+                               n->ifindex = ifp->ifindex;
+                               return 0;
+                       } else {
+                               /* If the MAC has changed,
+                                * need to issue a delete first
+                                * as this means a different MACIP route.
+                                * Also, need to do some unlinking/relinking.
                                 */
-                               send_del = 1;
+                               zvni_neigh_send_del_to_client(zvrf, zvni->vni,
+                                                             &n->ip, &n->emac,
+                                                             0);
+                               old_zmac = zvni_mac_lookup(zvni, &n->emac);
+                               if (old_zmac) {
+                                       listnode_delete(old_zmac->neigh_list,
+                                                       n);
+                                       zvni_deref_ip2mac(zvni, old_zmac, 0);
+                               }
+
+                               /* Set "local" forwarding info. */
+                               SET_FLAG(n->flags, ZEBRA_NEIGH_LOCAL);
+                               n->ifindex = ifp->ifindex;
+                               memcpy(&n->emac, macaddr, ETH_ALEN);
+
+                               /* Link to new MAC */
+                               listnode_add_sort(zmac->neigh_list, n);
+                       }
                } else if (ext_learned)
                /* The neighbor is remote and that is the notification we got.
                   */
@@ -2973,6 +2988,8 @@ int zebra_vxlan_local_neigh_add_update(struct interface *ifp,
                {
                        UNSET_FLAG(n->flags, ZEBRA_NEIGH_REMOTE);
                        n->r_vtep_ip.s_addr = 0;
+                       SET_FLAG(n->flags, ZEBRA_NEIGH_LOCAL);
+                       n->ifindex = ifp->ifindex;
                }
        } else {
                n = zvni_neigh_add(zvni, ip, macaddr);
@@ -2984,17 +3001,11 @@ int zebra_vxlan_local_neigh_add_update(struct interface *ifp,
                                ifp->name, ifp->ifindex, zvni->vni);
                        return -1;
                }
+               /* Set "local" forwarding info. */
+               SET_FLAG(n->flags, ZEBRA_NEIGH_LOCAL);
+               n->ifindex = ifp->ifindex;
        }
 
-       /* Issue delete for older info, if needed. */
-       if (send_del)
-               zvni_neigh_send_del_to_client(zvrf, zvni->vni, &n->ip, &n->emac,
-                                             0);
-
-       /* Set "local" forwarding info. */
-       SET_FLAG(n->flags, ZEBRA_NEIGH_LOCAL);
-       n->ifindex = ifp->ifindex;
-
        /* Before we program this in BGP, we need to check if MAC is locally
         * learnt as well */
        if (!CHECK_FLAG(zmac->flags, ZEBRA_MAC_LOCAL)) {
@@ -3008,23 +3019,20 @@ int zebra_vxlan_local_neigh_add_update(struct interface *ifp,
                return 0;
        }
 
-       /* Inform BGP if required. */
-       if (send_upd) {
-               if (IS_ZEBRA_DEBUG_VXLAN)
-                       zlog_debug(
-                               "%u: neigh %s (MAC %s) is now ACTIVE on VNI %u",
-                               ifp->vrf_id, ipaddr2str(ip, buf2, sizeof(buf2)),
-                               prefix_mac2str(macaddr, buf, sizeof(buf)),
-                               zvni->vni);
-
-               ZEBRA_NEIGH_SET_ACTIVE(n);
-               return zvni_neigh_send_add_to_client(zvrf, zvni->vni, ip,
-                                                    macaddr, 0);
-       }
+       /* Inform BGP. */
+       if (IS_ZEBRA_DEBUG_VXLAN)
+               zlog_debug(
+                       "%u: neigh %s (MAC %s) is now ACTIVE on VNI %u",
+                       ifp->vrf_id, ipaddr2str(ip, buf2, sizeof(buf2)),
+                       prefix_mac2str(macaddr, buf, sizeof(buf)),
+                       zvni->vni);
 
-       return 0;
+       ZEBRA_NEIGH_SET_ACTIVE(n);
+       return zvni_neigh_send_add_to_client(zvrf, zvni->vni, ip,
+                                            macaddr, 0);
 }
 
+
 /*
  * Handle message from client to delete a remote MACIP for a VNI.
  */
@@ -3096,7 +3104,7 @@ int zebra_vxlan_remote_macip_del(struct zserv *client, int sock, u_short length,
                zif = ifp->info;
 
                /* If down or not mapped to a bridge, we're done. */
-               if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+               if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                        continue;
 
                /* The remote VTEP specified is normally expected to exist, but
@@ -3246,7 +3254,7 @@ int zebra_vxlan_remote_macip_add(struct zserv *client, int sock, u_short length,
                zif = ifp->info;
 
                /* If down or not mapped to a bridge, we're done. */
-               if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+               if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                        continue;
 
                /* The remote VTEP specified should normally exist, but it is
@@ -3738,14 +3746,14 @@ int zebra_vxlan_remote_vtep_del(struct zserv *client, int sock, u_short length,
 
                ifp = zvni->vxlan_if;
                if (!ifp) {
-                 zlog_err ("VNI %u hash %p doesn't have intf upon remote VTEP DEL",
-                           zvni->vni, zvni);
-                 continue;
+                       zlog_err("VNI %u hash %p doesn't have intf upon remote VTEP DEL",
+                                zvni->vni, zvni);
+                       continue;
                }
                zif = ifp->info;
 
                /* If down or not mapped to a bridge, we're done. */
-               if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+               if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                        continue;
 
                /* If the remote VTEP does not exist, there's nothing more to
@@ -3817,10 +3825,11 @@ int zebra_vxlan_remote_vtep_add(struct zserv *client, int sock, u_short length,
                zif = ifp->info;
 
                /* If down or not mapped to a bridge, we're done. */
-               if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+               if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                        continue;
 
-               /* If the remote VTEP already exists, there's nothing more to do. */
+               /* If the remote VTEP already exists,
+                  there's nothing more to do. */
                if (zvni_vtep_find(zvni, &vtep_ip))
                        continue;
 
@@ -4366,7 +4375,7 @@ int zebra_vxlan_advertise_gw_macip(struct zserv *client, int sock,
                zif = ifp->info;
 
                /* If down or not mapped to a bridge, we're done. */
-               if (!if_is_operative (ifp) || !zif->brslave_info.br_if)
+               if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
                        return 0;
 
                zl2_info = zif->l2info.vxl;