zebra: Check both IPv4 and IPv6 NH before removing RMAC #14808

leonshaw · 2023-11-16T02:13:47Z

IPv4 and IPv4-mapped IPv6 address of a VTEP is used as nexthops of EVPN routes. Thus should check for both IPv4 and IPv6 before removing RMAC. Otherwise, withdrawing the last route in one address family breaks the other.

leonshaw · 2023-11-16T03:10:17Z

BTW, the nexthop IP of EVPN type 5 route is an intermediate value to resolve to L2 neighbor (RMAC + VTEP) in VxLAN domain, thus not necessarily the same as VTEP IP. Linux rtnetlink also has RTA_VIA, so it's possible to use IPv4 nexthop in IPv6 route. Not sure if there's a way to specify L2 nexthop directly.

ton31337 · 2023-11-16T12:52:23Z

zebra/zebra_vxlan.c

+	struct ipaddr *ipv4_vtep, *ipv6_nh;
+
+	memset(&tmp_ip, 0, sizeof(tmp_ip));
+	if (vtep_ip->ipa_type == IPADDR_V6) {


Shouldn't we check if IPv4 is REALLY mapped? Like using IS_MAPPED_IPV6().

Theoretically, yes. But currently only IPv4 VTEP is supported, and the nexthop is either IPv4 or IPv4-mapped. I think we can keep this assumption for now (like in zebra_vxlan_evpn_vrf_route_add), and handle other cases when adding support for IPv6 VTEP.

Yes, but at some level it's a bit safer (when VTEPs could be IPv6?) and more understandable looking at the code.

Regarding IPv6 VTEPs, things are different because the addresses can't be simply taken as nexthops of IPv4 routes. We should either allocate intermediate v4 addresses or use dataplane-dependent mechanisms like RTA_VIA (see my comment above).
Anyway, I'm adding this validation.

ton31337

Can we also have a topotest for this behavior?

leonshaw · 2023-11-17T02:09:14Z

Will add a topotest.

ton31337 · 2023-11-19T18:38:09Z

tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py

+    logger.info("==== Remove IPv6 network on R2")
+    result = apply_raw_config(tgen, config_no_ipv6)
+    assert result is True, "Failed to remove IPv6 network on R2, Error: {} ".format(result)
+    topotest.sleep(4, "waiting 4 seconds for bgp convergence")


Nah, sleep is not an acceptable option, can we use something here with retries?

Replaced sleep with verify_bgp_rib.

tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py

chiragshah6 · 2023-12-06T05:21:49Z

tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py

    output = tgen.gears["r1"].vtysh_cmd("show bgp vrf r1-vrf-101", isjson=False)
    logger.info("==== result from show bgp vrf r1-vrf-101 ")
    logger.info(output)
    output = tgen.gears["r1"].vtysh_cmd("show ip route vrf r1-vrf-101", isjson=False)
    logger.info("==== result from show ip route vrf r1-vrf-101")
    logger.info(output)
+    output = tgen.gears["r1"].vtysh_cmd("show ipv6 route vrf r1-vrf-101", isjson=False)


It would be best to check json form output against the static collected json output

chiragshah6 · 2023-12-06T05:28:57Z

tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py

    output = tgen.gears["r1"].vtysh_cmd("show evpn vni detail", isjson=False)
    logger.info("==== result from show evpn vni detail")
    logger.info(output)
    output = tgen.gears["r1"].vtysh_cmd("show evpn next-hops vni all", isjson=False)
    logger.info("==== result from show evpn next-hops vni all")
    logger.info(output)
    output = tgen.gears["r1"].vtysh_cmd("show evpn rmac vni all", isjson=False)
-    logger.info("==== result from show evpn next-hops vni all")
+    logger.info("==== result from show evpn rmac vni all")


can you check show specific rmac json output can compare number remote vteps:
Example:

show evpn rmac vni 8888 mac 44:38:39:ff:ff:33 json { "routerMac":"44:38:39:ff:ff:33", "vtepIp":"27.0.0.7", "nexthops":[ "27.0.0.7", <<<<< "::ffff:1b00:7" <<<<<ipv6mappedv4 address ] }

The IPv4-mapped IPv6 address is used as next-hop of overlay IPv6 route, not VTEP, so it won't be shown here.

chiragshah6 · 2023-12-06T06:10:44Z

zebra/zebra_vxlan.c

+	} else {
+		ipv4_vtep = vtep_ip;
+		ipv6_nh = &tmp_ip;
+		ipv6_nh->ipa_type = IPADDR_V6;


why convert v4 vtep/nh address to v6 ?

When installing EVPN overlay routes, VTEP address is used as v4 next-hop, while v4-mapped v6 address is used as v6 next-hop. So should check for both v4 and v6 next-hops when deleting.

IPv4 and IPv4-mapped IPv6 address of a VTEP is used as nexthops of EVPN routes. Thus should check for both IPv4 and IPv6 before removing RMAC. Otherwise, withdrawing the last route in one address family breaks the other. Signed-off-by: Xiao Liang <shaw.leon@gmail.com>

Note that withdrawing IPv6 route should not affect IPv4. Signed-off-by: Xiao Liang <shaw.leon@gmail.com>

chiragshah6 · 2023-12-08T18:20:17Z

@leonshaw Is there any issue file explaining the situation where RMAC is removed? If there are more data available along with some of the show commands outputs assist to shade light on the problem.

leonshaw · 2023-12-11T03:16:24Z

@chiragshah6
See the topotest in question for example. R2 advertises EVPN type-5 routes of both IPv4 and IPv6 prefixes under VTEP 192.168.100.41. R1 then use the VTEP address as IPv4 next-hop, and IPv4-mapped IPv6 address as IPv6 next-hop (which eventually resolve to underlay VxLAN encap via ARP/ND table and L2 FDB). These two next-hops share one RMAC.

# ip -4 route
192.168.101.41 via 192.168.100.41 dev bridge-101 proto bgp metric 20 onlink
# ip -6 route
fd00::2 via ::ffff:192.168.100.41 dev bridge-101 proto bgp metric 20 onlink pref medium
# ip neigh
192.168.100.41 dev bridge-101 lladdr ca:45:f5:59:9e:55 extern_learn  NOARP proto zebra
::ffff:192.168.100.41 dev bridge-101 lladdr ca:45:f5:59:9e:55 extern_learn  NOARP proto zebra
# bridge fdb
ca:45:f5:59:9e:55 dev vxlan-101 dst 192.168.100.41 link-netnsid 0 self extern_learn

r1# sh evpn next-hops vni 101
Number of NH Neighbors known for this VNI: 2
IP              RMAC
192.168.100.41  ca:45:f5:59:9e:55
::ffff:c0a8:6429 ca:45:f5:59:9e:55
r1# sh evpn rmac vni 101
Number of Remote RMACs known for this VNI: 1
MAC               Remote VTEP
ca:45:f5:59:9e:55 192.168.100.41

When the IPv6 route is withdrawn, without the changes of this PR, the RMAC will be deleted because the IPv6 next-hop is gone and the RMAC is considered unreferenced.

r1# sh evpn next-hops vni 101
Number of NH Neighbors known for this VNI: 1
IP              RMAC
192.168.100.41  ca:45:f5:59:9e:55
r1# sh evpn rmac vni 101

With this change, zebra will find there's still an IPv4 next-hop, thus the RMAC is preserved.

r1# show evpn next-hops vni 101
Number of NH Neighbors known for this VNI: 1
IP              RMAC
192.168.100.41  82:eb:2d:49:62:fa
r1# show evpn rmac vni 101
Number of Remote RMACs known for this VNI: 1
MAC               Remote VTEP
82:eb:2d:49:62:fa 192.168.100.41

github-actions · 2024-06-09T01:54:24Z

This PR is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this PR closed.

leonshaw · 2024-06-09T08:12:04Z

@chiragshah6 What to do with this PR now?

ton31337 · 2024-07-22T09:53:51Z

Superseded by #16341.

frrbot bot added the zebra label Nov 16, 2023

github-actions bot added size/M master labels Nov 16, 2023

ton31337 reviewed Nov 16, 2023

View reviewed changes

frrbot bot added the tests Topotests, make check, etc label Nov 17, 2023

github-actions bot added size/L and removed size/M labels Nov 17, 2023

leonshaw force-pushed the fix/evpn-ipv6-nh branch from 7378f43 to 15acbc6 Compare November 17, 2023 12:59

ton31337 reviewed Nov 19, 2023

View reviewed changes

leonshaw force-pushed the fix/evpn-ipv6-nh branch 2 times, most recently from 9d56e94 to 9cb30d1 Compare November 20, 2023 03:20

ton31337 approved these changes Nov 21, 2023

View reviewed changes

donaldsharp self-requested a review November 21, 2023 16:28

chiragshah6 requested changes Dec 6, 2023

View reviewed changes

chiragshah6 reviewed Dec 6, 2023

View reviewed changes

leonshaw force-pushed the fix/evpn-ipv6-nh branch from 9cb30d1 to e745e7d Compare December 7, 2023 03:43

tests: Add IPv6 network adv/withdraw case in bgp_evpn_rt5 topotest

279a917

Note that withdrawing IPv6 route should not affect IPv4. Signed-off-by: Xiao Liang <shaw.leon@gmail.com>

leonshaw force-pushed the fix/evpn-ipv6-nh branch from e745e7d to 279a917 Compare December 7, 2023 03:44

leonshaw requested a review from chiragshah6 December 7, 2023 06:43

github-actions bot added the autoclose label Jun 9, 2024

github-actions bot removed the autoclose label Jun 10, 2024

leonshaw mentioned this pull request Jul 6, 2024

zebra: evpn: not coerce VTEP IP to IPv4 in nh_list #16341

Merged

ton31337 closed this Jul 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zebra: Check both IPv4 and IPv6 NH before removing RMAC #14808

zebra: Check both IPv4 and IPv6 NH before removing RMAC #14808

leonshaw commented Nov 16, 2023

leonshaw commented Nov 16, 2023

ton31337 Nov 16, 2023

leonshaw Nov 17, 2023

ton31337 Nov 17, 2023

leonshaw Nov 17, 2023

ton31337 left a comment

leonshaw commented Nov 17, 2023

ton31337 Nov 19, 2023

leonshaw Nov 20, 2023

chiragshah6 Dec 6, 2023

leonshaw Dec 7, 2023

chiragshah6 Dec 6, 2023

leonshaw Dec 7, 2023

chiragshah6 Dec 6, 2023 •

edited

Loading

leonshaw Dec 7, 2023

chiragshah6 commented Dec 8, 2023

leonshaw commented Dec 11, 2023

github-actions bot commented Jun 9, 2024

leonshaw commented Jun 9, 2024

ton31337 commented Jul 22, 2024

zebra: Check both IPv4 and IPv6 NH before removing RMAC #14808

zebra: Check both IPv4 and IPv6 NH before removing RMAC #14808

Conversation

leonshaw commented Nov 16, 2023

leonshaw commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ton31337 left a comment

Choose a reason for hiding this comment

leonshaw commented Nov 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chiragshah6 Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chiragshah6 commented Dec 8, 2023

leonshaw commented Dec 11, 2023

github-actions bot commented Jun 9, 2024

leonshaw commented Jun 9, 2024

ton31337 commented Jul 22, 2024

chiragshah6 Dec 6, 2023 •

edited

Loading