From 249072620110e0359e2b03de4cdcd3b48e1e4b06 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 17 Aug 2021 10:31:13 +0200 Subject: [PATCH 1/3] zebra: update zl3vni when bridge link refreshed in other namespaces When running bgp evpn rt5 setup with vrf namespace backend, once the BGP configuration loaded, some refresh like the config change of a vxlan interface is not taken into account. As consequence, the BGP l2vpn evpn entries are empty. This can happen by recreating vxlan interface like follows: ip netns exec cust1 ip li del vxlan1000 ip link add vxlan1000 type vxlan id 1000 dev loopback0 local 10.209.36.1 learning ip link set dev vxlan1000 mtu 9000 ip link set dev vxlan1000 netns cust1 ip netns exec cust1 bash ip link set dev vxlan1000 up ip link set dev vxlan1000 master br1000 Actually, changing learning attribute requires recreation, and this change needs to manually reload the frr configuration. The update mechanism in zebra about vxlan interface updates is already put in place, but it does not work well with namespace based vrf backend. The function zl3vni_from_svi() is then modified to parse all the interfaces of each namespace. Signed-off-by: Philippe Guibert --- zebra/zebra_vxlan.c | 102 ++++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 41 deletions(-) diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index aede4098b4..7ad129cecc 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -1798,51 +1798,22 @@ struct zebra_l3vni *zl3vni_from_vrf(vrf_id_t vrf_id) return zl3vni_lookup(zvrf->l3vni); } -/* - * Map SVI and associated bridge to a VNI. This is invoked upon getting - * neighbor notifications, to see if they are of interest. - */ -static struct zebra_l3vni *zl3vni_from_svi(struct interface *ifp, - struct interface *br_if) +static int zl3vni_from_svi_ns(struct ns *ns, void *_in_param, void **_p_zl3vni) { int found = 0; - vlanid_t vid = 0; - uint8_t bridge_vlan_aware = 0; - struct zebra_l3vni *zl3vni = NULL; - struct zebra_ns *zns = NULL; + struct zebra_ns *zns = ns->info; + struct zebra_l3vni **p_zl3vni = (struct zebra_l3vni **)_p_zl3vni; + struct zebra_from_svi_param *in_param = + (struct zebra_from_svi_param *)_in_param; struct route_node *rn = NULL; - struct zebra_if *zif = NULL; struct interface *tmp_if = NULL; - struct zebra_l2info_bridge *br = NULL; + struct zebra_if *zif = NULL; struct zebra_l2info_vxlan *vxl = NULL; - if (!br_if) - return NULL; + if (!in_param) + return NS_WALK_STOP; - /* Make sure the linked interface is a bridge. */ - if (!IS_ZEBRA_IF_BRIDGE(br_if)) - return NULL; - - /* Determine if bridge is VLAN-aware or not */ - zif = br_if->info; - assert(zif); - br = &zif->l2info.br; - bridge_vlan_aware = br->vlan_aware; - if (bridge_vlan_aware) { - struct zebra_l2info_vlan *vl; - - if (!IS_ZEBRA_IF_VLAN(ifp)) - return NULL; - - zif = ifp->info; - assert(zif); - vl = &zif->l2info.vl; - vid = vl->vid; - } - - /* See if this interface (or interface plus VLAN Id) maps to a VxLAN */ - /* TODO: Optimize with a hash. */ - zns = zebra_ns_lookup(NS_DEFAULT); + /* loop through all vxlan-interface */ for (rn = route_top(zns->if_table); rn; rn = route_next(rn)) { tmp_if = (struct interface *)rn->info; if (!tmp_if) @@ -1854,19 +1825,68 @@ static struct zebra_l3vni *zl3vni_from_svi(struct interface *ifp, continue; vxl = &zif->l2info.vxl; - if (zif->brslave_info.br_if != br_if) + if (zif->brslave_info.br_if != in_param->br_if) continue; - if (!bridge_vlan_aware || vxl->access_vlan == vid) { + if (!in_param->bridge_vlan_aware + || vxl->access_vlan == in_param->vid) { found = 1; break; } } if (!found) + return NS_WALK_CONTINUE; + + if (p_zl3vni) + *p_zl3vni = zl3vni_lookup(vxl->vni); + return NS_WALK_STOP; +} + +/* + * Map SVI and associated bridge to a VNI. This is invoked upon getting + * neighbor notifications, to see if they are of interest. + */ +static struct zebra_l3vni *zl3vni_from_svi(struct interface *ifp, + struct interface *br_if) +{ + struct zebra_l3vni *zl3vni = NULL; + struct zebra_if *zif = NULL; + struct zebra_l2info_bridge *br = NULL; + struct zebra_from_svi_param in_param = {}; + struct zebra_l3vni **p_zl3vni; + + if (!br_if) return NULL; - zl3vni = zl3vni_lookup(vxl->vni); + /* Make sure the linked interface is a bridge. */ + if (!IS_ZEBRA_IF_BRIDGE(br_if)) + return NULL; + in_param.br_if = br_if; + + /* Determine if bridge is VLAN-aware or not */ + zif = br_if->info; + assert(zif); + br = &zif->l2info.br; + in_param.bridge_vlan_aware = br->vlan_aware; + if (in_param.bridge_vlan_aware) { + struct zebra_l2info_vlan *vl; + + if (!IS_ZEBRA_IF_VLAN(ifp)) + return NULL; + + zif = ifp->info; + assert(zif); + vl = &zif->l2info.vl; + in_param.vid = vl->vid; + } + + /* See if this interface (or interface plus VLAN Id) maps to a VxLAN */ + /* TODO: Optimize with a hash. */ + + p_zl3vni = &zl3vni; + + ns_walk_func(zl3vni_from_svi_ns, (void *)&in_param, (void **)p_zl3vni); return zl3vni; } From c7620108890f58796b0155bb9ef3d65f0b39909f Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 17 Aug 2021 10:42:51 +0200 Subject: [PATCH 2/3] zebra: handle bridge mac address update in evpn contexts when running bgp evpn rt5 setup, the Rmac sent in BGP updates stands for the MAC address of the bridge interface. After having loaded frr configuration, the Rmac address is not refreshed. This issue can be easily reproduced by executing some commands: ip netns exec cust1 ip link set dev br1000 address 2e:ab:45:aa:bb:cc Actually, the BGP EVPN contexts are kept unchanged. That commit proposes to fix this by intercepting the mac address change, and refreshing the vxlan interfaces attached to te bridge interface that changed its MAC address. Signed-off-by: Philippe Guibert --- zebra/if_netlink.c | 34 ++++++++++++++++++++++++---------- zebra/zebra_l2.c | 36 +++++++++++++++++++++++++++++------- zebra/zebra_l2.h | 6 +++++- zebra/zebra_vxlan.c | 7 +++++++ zebra/zebra_vxlan.h | 1 + 5 files changed, 66 insertions(+), 18 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 8b3b788b72..658b862e62 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -1021,7 +1021,8 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (IS_ZEBRA_IF_BOND(ifp)) zebra_l2if_update_bond(ifp, true); if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp)) - zebra_l2if_update_bridge_slave(ifp, bridge_ifindex, ns_id); + zebra_l2if_update_bridge_slave(ifp, bridge_ifindex, ns_id, + ZEBRA_BRIDGE_NO_ACTION); else if (IS_ZEBRA_IF_BOND_SLAVE(ifp)) zebra_l2if_update_bond_slave(ifp, bond_ifindex, !!bypass); @@ -1644,9 +1645,9 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) ifp, linkinfo[IFLA_INFO_DATA], 1, link_nsid); if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp)) - zebra_l2if_update_bridge_slave(ifp, - bridge_ifindex, - ns_id); + zebra_l2if_update_bridge_slave( + ifp, bridge_ifindex, ns_id, + ZEBRA_BRIDGE_NO_ACTION); else if (IS_ZEBRA_IF_BOND_SLAVE(ifp)) zebra_l2if_update_bond_slave(ifp, bond_ifindex, !!bypass); @@ -1670,6 +1671,7 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if_handle_vrf_change(ifp, vrf_id); } else { bool was_bridge_slave, was_bond_slave; + uint8_t chgflags = ZEBRA_BRIDGE_NO_ACTION; /* Interface update. */ if (IS_ZEBRA_DEBUG_KERNEL) @@ -1711,6 +1713,8 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if_down(ifp); rib_update(RIB_UPDATE_KERNEL); } else if (if_is_operative(ifp)) { + bool mac_updated = false; + /* Must notify client daemons of new * interface status. */ if (IS_ZEBRA_DEBUG_KERNEL) @@ -1721,9 +1725,11 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) /* Update EVPN VNI when SVI MAC change */ - if (IS_ZEBRA_IF_VLAN(ifp) && - memcmp(old_hw_addr, ifp->hw_addr, - INTERFACE_HWADDR_MAX)) { + if (memcmp(old_hw_addr, ifp->hw_addr, + INTERFACE_HWADDR_MAX)) + mac_updated = true; + if (IS_ZEBRA_IF_VLAN(ifp) + && mac_updated) { struct interface *link_if; link_if = @@ -1733,6 +1739,13 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (link_if) zebra_vxlan_svi_up(ifp, link_if); + } else if (mac_updated + && IS_ZEBRA_IF_BRIDGE(ifp)) { + zlog_debug( + "Intf %s(%u) bridge changed MAC address", + name, ifp->ifindex); + chgflags = + ZEBRA_BRIDGE_MASTER_MAC_CHANGE; } } } else { @@ -1758,12 +1771,13 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) netlink_interface_update_l2info( ifp, linkinfo[IFLA_INFO_DATA], 0, link_nsid); + if (IS_ZEBRA_IF_BRIDGE(ifp)) + zebra_l2if_update_bridge(ifp, chgflags); if (IS_ZEBRA_IF_BOND(ifp)) zebra_l2if_update_bond(ifp, true); if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp) || was_bridge_slave) - zebra_l2if_update_bridge_slave(ifp, - bridge_ifindex, - ns_id); + zebra_l2if_update_bridge_slave( + ifp, bridge_ifindex, ns_id, chgflags); else if (IS_ZEBRA_IF_BOND_SLAVE(ifp) || was_bond_slave) zebra_l2if_update_bond_slave(ifp, bond_ifindex, !!bypass); diff --git a/zebra/zebra_l2.c b/zebra/zebra_l2.c index 71fac556e1..ae630314be 100644 --- a/zebra/zebra_l2.c +++ b/zebra/zebra_l2.c @@ -50,7 +50,8 @@ /* static function declarations */ /* Private functions */ -static void map_slaves_to_bridge(struct interface *br_if, int link) +static void map_slaves_to_bridge(struct interface *br_if, int link, + bool update_slave, uint8_t chgflags) { struct vrf *vrf; struct interface *ifp; @@ -79,9 +80,17 @@ static void map_slaves_to_bridge(struct interface *br_if, int link) br_slave = &zif->brslave_info; if (link) { - if (br_slave->bridge_ifindex == br_if->ifindex && - br_slave->ns_id == zns->ns_id) + if (br_slave->bridge_ifindex == br_if->ifindex + && br_slave->ns_id == zns->ns_id) { br_slave->br_if = br_if; + if (update_slave) { + zebra_l2if_update_bridge_slave( + ifp, + br_slave->bridge_ifindex, + br_slave->ns_id, + chgflags); + } + } } else { if (br_slave->br_if == br_if) br_slave->br_if = NULL; @@ -261,7 +270,7 @@ void zebra_l2_bridge_add_update(struct interface *ifp, memcpy(&zif->l2info.br, bridge_info, sizeof(*bridge_info)); /* Link all slaves to this bridge */ - map_slaves_to_bridge(ifp, 1); + map_slaves_to_bridge(ifp, 1, false, ZEBRA_BRIDGE_NO_ACTION); } /* @@ -270,7 +279,14 @@ void zebra_l2_bridge_add_update(struct interface *ifp, void zebra_l2_bridge_del(struct interface *ifp) { /* Unlink all slaves to this bridge */ - map_slaves_to_bridge(ifp, 0); + map_slaves_to_bridge(ifp, 0, false, ZEBRA_BRIDGE_NO_ACTION); +} + +void zebra_l2if_update_bridge(struct interface *ifp, uint8_t chgflags) +{ + if (!chgflags) + return; + map_slaves_to_bridge(ifp, 1, true, chgflags); } /* @@ -398,8 +414,8 @@ void zebra_l2_vxlanif_del(struct interface *ifp) * from a bridge before it can be mapped to another bridge. */ void zebra_l2if_update_bridge_slave(struct interface *ifp, - ifindex_t bridge_ifindex, - ns_id_t ns_id) + ifindex_t bridge_ifindex, ns_id_t ns_id, + uint8_t chgflags) { struct zebra_if *zif; ifindex_t old_bridge_ifindex; @@ -413,6 +429,12 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp, if (!zvrf) return; + if (zif->zif_type == ZEBRA_IF_VXLAN + && chgflags != ZEBRA_BRIDGE_NO_ACTION) { + if (ZEBRA_BRIDGE_MASTER_MAC_CHANGE) + zebra_vxlan_if_update(ifp, + ZEBRA_VXLIF_MASTER_MAC_CHANGE); + } old_bridge_ifindex = zif->brslave_info.bridge_ifindex; old_ns_id = zif->brslave_info.ns_id; if (old_bridge_ifindex == bridge_ifindex && diff --git a/zebra/zebra_l2.h b/zebra/zebra_l2.h index 6572f344c4..de833ebdbc 100644 --- a/zebra/zebra_l2.h +++ b/zebra/zebra_l2.h @@ -33,6 +33,9 @@ extern "C" { #endif +#define ZEBRA_BRIDGE_NO_ACTION (0) +#define ZEBRA_BRIDGE_MASTER_MAC_CHANGE (1 << 1) + /* zebra L2 interface information - bridge slave (linkage to bridge) */ struct zebra_l2info_brslave { ifindex_t bridge_ifindex; /* Bridge Master */ @@ -121,7 +124,7 @@ extern void zebra_l2_greif_del(struct interface *ifp); extern void zebra_l2_vxlanif_del(struct interface *ifp); extern void zebra_l2if_update_bridge_slave(struct interface *ifp, ifindex_t bridge_ifindex, - ns_id_t ns_id); + ns_id_t ns_id, uint8_t chgflags); extern void zebra_l2if_update_bond_slave(struct interface *ifp, ifindex_t bond_ifindex, bool bypass); @@ -130,6 +133,7 @@ extern void zebra_vlan_bitmap_compute(struct interface *ifp, extern void zebra_vlan_mbr_re_eval(struct interface *ifp, bitfield_t vlan_bitmap); extern void zebra_l2if_update_bond(struct interface *ifp, bool add); +extern void zebra_l2if_update_bridge(struct interface *ifp, uint8_t chgflags); #ifdef __cplusplus } diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index 7ad129cecc..c13c867d2a 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -5043,6 +5043,13 @@ int zebra_vxlan_if_update(struct interface *ifp, uint16_t chgflags) return 0; } + if ((chgflags & ZEBRA_VXLIF_MASTER_MAC_CHANGE) + && if_is_operative(ifp) && is_l3vni_oper_up(zl3vni)) { + zebra_vxlan_process_l3vni_oper_down(zl3vni); + zebra_vxlan_process_l3vni_oper_up(zl3vni); + return 0; + } + /* access-vlan change - process oper down, associate with new * svi_if and then process oper up again */ diff --git a/zebra/zebra_vxlan.h b/zebra/zebra_vxlan.h index 915e987b6b..464a8e5fc4 100644 --- a/zebra/zebra_vxlan.h +++ b/zebra/zebra_vxlan.h @@ -65,6 +65,7 @@ is_vxlan_flooding_head_end(void) #define ZEBRA_VXLIF_MASTER_CHANGE (1 << 1) #define ZEBRA_VXLIF_VLAN_CHANGE (1 << 2) #define ZEBRA_VXLIF_MCAST_GRP_CHANGE (1 << 3) +#define ZEBRA_VXLIF_MASTER_MAC_CHANGE (1 << 4) #define VNI_STR_LEN 32 From f56a15b5bd231c78456a2d340abbd0300a686e81 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 17 Aug 2021 10:56:32 +0200 Subject: [PATCH 3/3] zebra: refresh vxlan evpn contexts, when bridge interface goes up When using bgp evpn rt5 setup, after BGP configuration has been loaded, if the user attempts to detach and reattach the bridged vxlan interface from the bridge, then BGP loses its BGP EVPN contexts, and a refresh of BGP configuration is necessary to maintain consistency between linux configuration and BGP EVPN contexts (RIB). The following command can lead to inconsistency: ip netns exec cust1 ip link set dev vxlan1000 nomaster ip netns exec cust1 ip link set dev vxlan1000 master br1000 consecutive to the, BGP l2vpn evpn RIB is empty, and the way to solve this until now is to reconfigure EVPN like this: vrf cust1 no vni 1000 vni 1000 exit-vrf Actually, the link information is correctly handled. In fact, at the time of link event, the lower link status of the bridge interface was not yet up, thus preventing from establishing BGP EVPN contexts. In fact, when a bridge interface does not have any slave interface, the link status of the bridge interface is down. That change of status comes a bit after, and is not detected by slave interfaces, as this event is not intercepted. This commit intercepts the bridge link up event, and triggers a check on slaved vxlan interfaces. Signed-off-by: Philippe Guibert --- zebra/if_netlink.c | 3 +++ zebra/zebra_l2.c | 2 ++ zebra/zebra_l2.h | 1 + 3 files changed, 6 insertions(+) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 658b862e62..a78b7607a0 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -1756,6 +1756,9 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) "Intf %s(%u) has come UP", name, ifp->ifindex); if_up(ifp); + if (IS_ZEBRA_IF_BRIDGE(ifp)) + chgflags = + ZEBRA_BRIDGE_MASTER_UP; } else { if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug( diff --git a/zebra/zebra_l2.c b/zebra/zebra_l2.c index ae630314be..5a02149611 100644 --- a/zebra/zebra_l2.c +++ b/zebra/zebra_l2.c @@ -434,6 +434,8 @@ void zebra_l2if_update_bridge_slave(struct interface *ifp, if (ZEBRA_BRIDGE_MASTER_MAC_CHANGE) zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_MASTER_MAC_CHANGE); + if (ZEBRA_BRIDGE_MASTER_UP) + zebra_vxlan_if_update(ifp, ZEBRA_VXLIF_MASTER_CHANGE); } old_bridge_ifindex = zif->brslave_info.bridge_ifindex; old_ns_id = zif->brslave_info.ns_id; diff --git a/zebra/zebra_l2.h b/zebra/zebra_l2.h index de833ebdbc..98744f3c1f 100644 --- a/zebra/zebra_l2.h +++ b/zebra/zebra_l2.h @@ -35,6 +35,7 @@ extern "C" { #define ZEBRA_BRIDGE_NO_ACTION (0) #define ZEBRA_BRIDGE_MASTER_MAC_CHANGE (1 << 1) +#define ZEBRA_BRIDGE_MASTER_UP (1 << 2) /* zebra L2 interface information - bridge slave (linkage to bridge) */ struct zebra_l2info_brslave {