From 689b510143d3ee779c78ca4c04153e3fb6df13ae Mon Sep 17 00:00:00 2001 From: Mitesh Kanjariya Date: Thu, 15 Mar 2018 03:29:50 -0700 Subject: [PATCH 1/4] bgpd: change advertise-subnet to a hidden command We have changed the flow in which we advertise the VNI subnet. We will mark this command as hidden for all future purposes. Signed-off-by: Mitesh Kanjariya --- bgpd/bgp_evpn_vty.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index 8fd7cb5d14..6cb5c9d8da 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -2692,10 +2692,10 @@ DEFUN (no_bgp_evpn_default_originate, return CMD_SUCCESS; } -DEFUN (bgp_evpn_advertise_vni_subnet, - bgp_evpn_advertise_vni_subnet_cmd, - "advertise-subnet", - "Advertise the subnet corresponding to VNI\n") +DEFUN_HIDDEN (bgp_evpn_advertise_vni_subnet, + bgp_evpn_advertise_vni_subnet_cmd, + "advertise-subnet", + "Advertise the subnet corresponding to VNI\n") { struct bgp *bgp_vrf = NULL; struct bgp *bgp = VTY_GET_CONTEXT(bgp); @@ -2715,11 +2715,11 @@ DEFUN (bgp_evpn_advertise_vni_subnet, return CMD_SUCCESS; } -DEFUN (no_bgp_evpn_advertise_vni_subnet, - no_bgp_evpn_advertise_vni_subnet_cmd, - "no advertise-subnet", - NO_STR - "Advertise All local VNIs\n") +DEFUN_HIDDEN (no_bgp_evpn_advertise_vni_subnet, + no_bgp_evpn_advertise_vni_subnet_cmd, + "no advertise-subnet", + NO_STR + "Advertise All local VNIs\n") { struct bgp *bgp = VTY_GET_CONTEXT(bgp); VTY_DECLVAR_CONTEXT_SUB(bgpevpn, vpn); From ee69da278d26aa46042302238ed6753022f42aa7 Mon Sep 17 00:00:00 2001 From: Mitesh Kanjariya Date: Fri, 2 Mar 2018 15:28:33 -0800 Subject: [PATCH 2/4] zebra: act on kernel notifications for remote neighbors as well There can be a race condition between kernel and frr as follows. Frr sends remote neigh notification. At the (almost) same time kernel might send a notification saying neigh is local. After processing this notifications, the state in frr is local while state in kernel is remote. This causes kernel and frr to be out of sync. This problem will be avoided if FRR acts on the kernel notifications for remote neighbors. When FRR sees a remote neighbor notification for a neighbor which it thinks is local, FRR will change the neigh state to remote. Ticket: CM-19923/CM-18830 Review: CCR-7222 Testing: Manual Signed-off-by: Vivek Venkatraman --- zebra/rt_netlink.c | 6 +- zebra/zebra_vxlan.c | 317 +++++++++++++++++++++++++++----------------- zebra/zebra_vxlan.h | 4 +- 3 files changed, 201 insertions(+), 126 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index df53a06bc2..fbaa9c4783 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -2220,11 +2220,11 @@ static int netlink_ipneigh_change(struct sockaddr_nl *snl, struct nlmsghdr *h, * in re-adding the neighbor if it is a valid "remote" neighbor. */ if (ndm->ndm_state & NUD_VALID) - return zebra_vxlan_local_neigh_add_update( + return zebra_vxlan_handle_kernel_neigh_update( ifp, link_if, &ip, &mac, ndm->ndm_state, ext_learned); - return zebra_vxlan_local_neigh_del(ifp, link_if, &ip); + return zebra_vxlan_handle_kernel_neigh_del(ifp, link_if, &ip); } if (IS_ZEBRA_DEBUG_KERNEL) @@ -2237,7 +2237,7 @@ static int netlink_ipneigh_change(struct sockaddr_nl *snl, struct nlmsghdr *h, /* Process the delete - it may result in re-adding the neighbor if it is * a valid "remote" neighbor. */ - return zebra_vxlan_local_neigh_del(ifp, link_if, &ip); + return zebra_vxlan_handle_kernel_neigh_del(ifp, link_if, &ip); } static int netlink_neigh_table(struct sockaddr_nl *snl, struct nlmsghdr *h, diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index fa8f837408..a2961428d7 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -1930,6 +1930,182 @@ static void zvni_gw_macip_add_for_vni_hash(struct hash_backet *backet, return; } +static int zvni_local_neigh_update(zebra_vni_t *zvni, + struct interface *ifp, + struct ipaddr *ip, + struct ethaddr *macaddr) +{ + char buf[ETHER_ADDR_STRLEN]; + char buf2[INET6_ADDRSTRLEN]; + zebra_neigh_t *n = NULL; + zebra_mac_t *zmac = NULL, *old_zmac = NULL; + + /* create a dummy MAC if the MAC is not already present */ + zmac = zvni_mac_lookup(zvni, macaddr); + if (!zmac) { + if (IS_ZEBRA_DEBUG_VXLAN) + zlog_debug( + "AUTO MAC %s created for neigh %s on VNI %u", + prefix_mac2str(macaddr, buf, sizeof(buf)), + ipaddr2str(ip, buf2, sizeof(buf2)), zvni->vni); + + zmac = zvni_mac_add(zvni, macaddr); + if (!zmac) { + zlog_warn("Failed to add MAC %s VNI %u", + prefix_mac2str(macaddr, buf, sizeof(buf)), + zvni->vni); + return -1; + } + + memset(&zmac->fwd_info, 0, sizeof(zmac->fwd_info)); + memset(&zmac->flags, 0, sizeof(uint32_t)); + SET_FLAG(zmac->flags, ZEBRA_MAC_AUTO); + } + + /* If same entry already exists, it might be a change or it might be a + * move from remote to local. + */ + n = zvni_neigh_lookup(zvni, ip); + if (n) { + if (CHECK_FLAG(n->flags, ZEBRA_NEIGH_LOCAL)) { + if (memcmp(n->emac.octet, macaddr->octet, + ETH_ALEN) == 0) { + /* Update any params and return - client doesn't + * care about a purely local change. + */ + n->ifindex = ifp->ifindex; + return 0; + } + + /* 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. + */ + zvni_neigh_send_del_to_client(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); + } + + /* Update the forwarding info. */ + n->ifindex = ifp->ifindex; + memcpy(&n->emac, macaddr, ETH_ALEN); + + /* Link to new MAC */ + listnode_add_sort(zmac->neigh_list, n); + + } else + /* Neighbor has moved from remote to local. */ + { + /* If MAC has changed, do the unlink/link */ + if (memcmp(n->emac.octet, macaddr->octet, + ETH_ALEN) != 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); + } + + /* Link to new MAC */ + memcpy(&n->emac, macaddr, ETH_ALEN); + listnode_add_sort(zmac->neigh_list, n); + } + + /* Mark appropriately */ + 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 { + /* New neighbor - create */ + n = zvni_neigh_add(zvni, ip, macaddr); + if (!n) { + zlog_err( + "Failed to add neighbor %s MAC %s intf %s(%u) -> VNI %u", + ipaddr2str(ip, buf2, sizeof(buf2)), + prefix_mac2str(macaddr, buf, sizeof(buf)), + ifp->name, ifp->ifindex, zvni->vni); + return -1; + } + /* 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)) { + if (IS_ZEBRA_DEBUG_VXLAN) + zlog_debug( + "Skipping neigh %s add to client as MAC %s is not local on VNI %u", + ipaddr2str(ip, buf2, sizeof(buf2)), + prefix_mac2str(macaddr, buf, sizeof(buf)), + zvni->vni); + + return 0; + } + + /* Inform BGP. */ + if (IS_ZEBRA_DEBUG_VXLAN) + zlog_debug("Neigh %s (MAC %s) is now ACTIVE on L2-VNI %u", + 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(zvni->vni, ip, macaddr, 0); +} + +static int zvni_remote_neigh_update(zebra_vni_t *zvni, + struct interface *ifp, + struct ipaddr *ip, + struct ethaddr *macaddr, + uint16_t state) +{ + char buf[ETHER_ADDR_STRLEN]; + char buf2[INET6_ADDRSTRLEN]; + zebra_neigh_t *n = NULL; + zebra_mac_t *zmac = NULL; + + /* If the neighbor is unknown, there is no further action. */ + n = zvni_neigh_lookup(zvni, ip); + if (!n) + return 0; + + /* If a remote entry, see if it needs to be refreshed */ + if (CHECK_FLAG(n->flags, ZEBRA_NEIGH_REMOTE)) { + if (state & NUD_STALE) + zvni_neigh_install(zvni, n); + } else { + /* We got a "remote" neighbor notification for an entry + * we think is local. This can happen in a multihoming + * scenario - but only if the MAC is already "remote". + * Just mark our entry as "remote". + */ + zmac = zvni_mac_lookup(zvni, macaddr); + if (!zmac || !CHECK_FLAG(zmac->flags, ZEBRA_MAC_REMOTE)) { + zlog_err( + "Ignore remote neigh %s (MAC %s) on L2-VNI %u " + "- MAC unknown or local", + ipaddr2str(&n->ip, buf2, sizeof(buf2)), + prefix_mac2str(macaddr, buf, sizeof(buf)), + zvni->vni); + return -1; + } + + UNSET_FLAG(n->flags, ZEBRA_NEIGH_LOCAL); + SET_FLAG(n->flags, ZEBRA_NEIGH_REMOTE); + n->r_vtep_ip = zmac->fwd_info.r_vtep_ip; + } + + return 0; +} + /* * Make hash key for MAC. */ @@ -4626,13 +4802,14 @@ void zebra_vxlan_print_vnis(struct vty *vty, struct zebra_vrf *zvrf, } /* - * Handle neighbor delete (on a VLAN device / L3 interface) from the - * kernel. This may result in either the neighbor getting deleted from - * our database or being re-added to the kernel (if it is a valid + * Handle neighbor delete notification from the kernel (on a VLAN device + * / L3 interface). This may result in either the neighbor getting deleted + * from our database or being re-added to the kernel (if it is a valid * remote neighbor). */ -int zebra_vxlan_local_neigh_del(struct interface *ifp, - struct interface *link_if, struct ipaddr *ip) +int zebra_vxlan_handle_kernel_neigh_del(struct interface *ifp, + struct interface *link_if, + struct ipaddr *ip) { char buf[INET6_ADDRSTRLEN]; char buf2[ETHER_ADDR_STRLEN]; @@ -4708,20 +4885,21 @@ int zebra_vxlan_local_neigh_del(struct interface *ifp, } /* - * Handle neighbor add or update (on a VLAN device / L3 interface) - * from the kernel. + * Handle neighbor add or update notification from the kernel (on a VLAN + * device / L3 interface). This is typically for a local neighbor but can + * also be for a remote neighbor (e.g., ageout notification). It could + * also be a "move" scenario. */ -int zebra_vxlan_local_neigh_add_update(struct interface *ifp, - struct interface *link_if, - struct ipaddr *ip, - struct ethaddr *macaddr, uint16_t state, - uint8_t ext_learned) +int zebra_vxlan_handle_kernel_neigh_update(struct interface *ifp, + struct interface *link_if, + struct ipaddr *ip, + struct ethaddr *macaddr, + uint16_t state, + uint8_t ext_learned) { char buf[ETHER_ADDR_STRLEN]; char buf2[INET6_ADDRSTRLEN]; zebra_vni_t *zvni = NULL; - zebra_neigh_t *n = NULL; - zebra_mac_t *zmac = NULL, *old_zmac = NULL; zebra_l3vni_t *zl3vni = NULL; /* check if this is a remote neigh entry corresponding to remote @@ -4746,114 +4924,11 @@ int zebra_vxlan_local_neigh_add_update(struct interface *ifp, ifp->ifindex, state, ext_learned ? "ext-learned " : "", zvni->vni); - /* create a dummy MAC if the MAC is not already present */ - zmac = zvni_mac_lookup(zvni, macaddr); - if (!zmac) { - if (IS_ZEBRA_DEBUG_VXLAN) - zlog_debug("AUTO MAC %s created for neigh %s on VNI %u", - prefix_mac2str(macaddr, buf, sizeof(buf)), - ipaddr2str(ip, buf2, sizeof(buf2)), - zvni->vni); + /* Is this about a local neighbor or a remote one? */ + if (!ext_learned) + return zvni_local_neigh_update(zvni, ifp, ip, macaddr); - zmac = zvni_mac_add(zvni, macaddr); - if (!zmac) { - zlog_warn("Failed to add MAC %s VNI %u", - prefix_mac2str(macaddr, buf, sizeof(buf)), - zvni->vni); - return -1; - } - - memset(&zmac->fwd_info, 0, sizeof(zmac->fwd_info)); - memset(&zmac->flags, 0, sizeof(uint32_t)); - SET_FLAG(zmac->flags, ZEBRA_MAC_AUTO); - } - - /* If same entry already exists, it might be a change or it might be a - * move from remote to local. - */ - n = zvni_neigh_lookup(zvni, ip); - if (n) { - if (CHECK_FLAG(n->flags, ZEBRA_NEIGH_LOCAL)) { - if (memcmp(n->emac.octet, macaddr->octet, ETH_ALEN) - == 0) { - /* Update any params and return - client doesn't - * care about a purely local change. - */ - n->ifindex = ifp->ifindex; - return 0; - } - - /* 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. - */ - zvni_neigh_send_del_to_client(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. - */ - { - /* TODO: Evaluate if we need to do anything here. */ - return 0; - } else - /* Neighbor has moved from remote to local. */ - { - 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); - if (!n) { - zlog_err( - "Failed to add neighbor %s MAC %s intf %s(%u) -> VNI %u", - ipaddr2str(ip, buf2, sizeof(buf2)), - prefix_mac2str(macaddr, buf, sizeof(buf)), - ifp->name, ifp->ifindex, zvni->vni); - return -1; - } - /* 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)) { - if (IS_ZEBRA_DEBUG_VXLAN) - zlog_debug( - "Skipping neigh %s add to client as MAC %s is not local on VNI %u", - ipaddr2str(ip, buf2, sizeof(buf2)), - prefix_mac2str(macaddr, buf, sizeof(buf)), - zvni->vni); - - return 0; - } - - /* Inform BGP. */ - if (IS_ZEBRA_DEBUG_VXLAN) - zlog_debug("neigh %s (MAC %s) is now ACTIVE on L2-VNI %u", - 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(zvni->vni, ip, macaddr, n->flags); + return zvni_remote_neigh_update(zvni, ifp, ip, macaddr, state); } diff --git a/zebra/zebra_vxlan.h b/zebra/zebra_vxlan.h index 16b01e6acd..6153c7d7e3 100644 --- a/zebra/zebra_vxlan.h +++ b/zebra/zebra_vxlan.h @@ -122,10 +122,10 @@ extern int zebra_vxlan_add_del_gw_macip(struct interface *ifp, struct prefix *p, extern int zebra_vxlan_svi_up(struct interface *ifp, struct interface *link_if); extern int zebra_vxlan_svi_down(struct interface *ifp, struct interface *link_if); -extern int zebra_vxlan_local_neigh_add_update( +extern int zebra_vxlan_handle_kernel_neigh_update( struct interface *ifp, struct interface *link_if, struct ipaddr *ip, struct ethaddr *macaddr, uint16_t state, uint8_t ext_learned); -extern int zebra_vxlan_local_neigh_del(struct interface *ifp, +extern int zebra_vxlan_handle_kernel_neigh_del(struct interface *ifp, struct interface *link_if, struct ipaddr *ip); extern int zebra_vxlan_local_mac_add_update(struct interface *ifp, From e9d2cbdebf26a264a3053abef9cfa9321e9a5cdd Mon Sep 17 00:00:00 2001 From: Mitesh Kanjariya Date: Mon, 26 Feb 2018 14:10:50 -0800 Subject: [PATCH 3/4] zebra: add EVPN learned neighbors as NUD_NOARP EVPN owns the remote neigh entries which are programed in the kernel. This entries should not age out and the only way to delete should be from EVPN. We should program these entries with NUD_NOARP instead of NUD_REACHABLE to avoid aging of this macs. Signed-off-by: Mitesh Kanjariya --- zebra/rt_netlink.c | 2 +- zebra/zebra_vxlan.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index fbaa9c4783..9fd8ecb27c 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -2412,7 +2412,7 @@ int kernel_del_mac(struct interface *ifp, vlanid_t vid, struct ethaddr *mac, int kernel_add_neigh(struct interface *ifp, struct ipaddr *ip, struct ethaddr *mac) { - return netlink_neigh_update2(ifp, ip, mac, NUD_REACHABLE, RTM_NEWNEIGH); + return netlink_neigh_update2(ifp, ip, mac, NUD_NOARP, RTM_NEWNEIGH); } int kernel_del_neigh(struct interface *ifp, struct ipaddr *ip) diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index a2961428d7..b851ba41d5 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -2079,8 +2079,10 @@ static int zvni_remote_neigh_update(zebra_vni_t *zvni, /* If a remote entry, see if it needs to be refreshed */ if (CHECK_FLAG(n->flags, ZEBRA_NEIGH_REMOTE)) { +#ifdef GNU_LINUX if (state & NUD_STALE) zvni_neigh_install(zvni, n); +#endif } else { /* We got a "remote" neighbor notification for an entry * we think is local. This can happen in a multihoming From 9fc1522cfbb0d87ed55d5c5f78eb82c84b81c9c8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 12 Apr 2018 09:20:20 -0400 Subject: [PATCH 4/4] zebra: Cleanup lines over 80 columns Cleanup warnings in lines over 80 columns. Signed-off-by: Donald Sharp --- zebra/zebra_vxlan.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index b851ba41d5..6e901a0457 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -2005,7 +2005,8 @@ static int zvni_local_neigh_update(zebra_vni_t *zvni, ETH_ALEN) != 0) { old_zmac = zvni_mac_lookup(zvni, &n->emac); if (old_zmac) { - listnode_delete(old_zmac->neigh_list, n); + listnode_delete(old_zmac->neigh_list, + n); zvni_deref_ip2mac(zvni, old_zmac, 0); } @@ -2091,12 +2092,10 @@ static int zvni_remote_neigh_update(zebra_vni_t *zvni, */ zmac = zvni_mac_lookup(zvni, macaddr); if (!zmac || !CHECK_FLAG(zmac->flags, ZEBRA_MAC_REMOTE)) { - zlog_err( - "Ignore remote neigh %s (MAC %s) on L2-VNI %u " - "- MAC unknown or local", - ipaddr2str(&n->ip, buf2, sizeof(buf2)), - prefix_mac2str(macaddr, buf, sizeof(buf)), - zvni->vni); + zlog_err("Ignore remote neigh %s (MAC %s) on L2-VNI %u - MAC unknown or local", + ipaddr2str(&n->ip, buf2, sizeof(buf2)), + prefix_mac2str(macaddr, buf, sizeof(buf)), + zvni->vni); return -1; }