From b682f6de5a4249c84f7a6467c1792d210385504f Mon Sep 17 00:00:00 2001 From: vivek Date: Sun, 13 Aug 2017 21:52:04 -0700 Subject: [PATCH] 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 Reviewed-by: Mitesh Kanjariya Reviewed-by: Donald Sharp Ticket: CM-17565 Reviewed By: CCR-6605 Testing Done: Manual, evpn-smoke --- bgpd/bgp_evpn.c | 75 +++++++++++++++---------------- bgpd/bgp_evpn.h | 3 +- bgpd/bgp_evpn_vty.c | 5 ++- bgpd/bgp_route.c | 23 +++++----- bgpd/bgp_vty.c | 6 +-- zebra/zebra_l2.c | 14 +++--- zebra/zebra_vxlan.c | 105 ++++++++++++++++++++++++-------------------- 7 files changed, 118 insertions(+), 113 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 5da773d7a3..ad000e3f12 100644 --- 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 01474b09d8..985f41f586 100644 --- 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 af34c33946..a198e65128 100644 --- 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 e9bf574331..a2a7d2ae1d 100644 --- 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 80daac236d..5d44c0a142 100644 --- 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 452bab003a..7281622e33 100644 --- 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 2240d7c763..4a5fc38493 100644 --- 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; -- 2.39.5