On 6/18/26 3:03 PM, Dumitru Ceara wrote: > On 6/16/26 11:54 AM, Ales Musil via dev wrote: >> Extend physical_consider_evpn_arp() to also install flows in >> the dedicated EVPN ARP side table (OFTABLE_EVPN_ARP_LOOKUP, >> table 113) for the switch datapath. These flows are matched >> by the chk_evpn_arp() action added in the previous commit. >> >> Each flow matches on metadata (switch dp_key) and IP address >> (reg0 for IPv4, xxreg0 for IPv6). On a hit, it loads the >> resolved MAC into eth.dst and sets MLF_EVPN_LOOKUP_BIT. >> >> The flows use the same UUID as the router-side neighbor flows, >> so ofctrl_remove_flows() in physical_handle_evpn_arp_changes() >> automatically removes both router-side and switch-side flows >> when an EVPN ARP entry is deleted. >> >> Assisted-by: Claude Opus 4.6, Claude Code >> Signed-off-by: Ales Musil <[email protected]> >> --- > > Hi Ales, > > Thanks for the patch! > >> controller/physical.c | 44 +++++++++++++++++++++++++++++-- >> tests/system-ovn.at | 60 +++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 102 insertions(+), 2 deletions(-) >> >> diff --git a/controller/physical.c b/controller/physical.c >> index 7584d6065..89ff2785e 100644 >> --- a/controller/physical.c >> +++ b/controller/physical.c >> @@ -3689,6 +3689,8 @@ physical_consider_evpn_fdb(const struct evpn_fdb *fdb, >> static void >> physical_consider_evpn_arp(const struct hmap *local_datapaths, >> const struct evpn_arp *arp, >> + struct ofpbuf *ofpacts, >> + struct match *match, >> struct ovn_desired_flow_table *flow_table) >> { >> /* Walk connected OVN routers and install neighbor flows for the ARPs >> @@ -3706,6 +3708,38 @@ physical_consider_evpn_arp(const struct hmap >> *local_datapaths, >> consider_neighbor_flow(remote_pb, &arp->flow_uuid, &arp->ip, >> arp->mac, >> flow_table, arp->priority, false); >> } >> + >> + /* Install EVPN ARP lookup flows in the dedicated side table for the >> + * switch datapath. These flows are matched by the chk_evpn_arp() >> + * action. On a hit they load the resolved MAC into eth.dst >> + * and set MLF_EVPN_LOOKUP_BIT. */ > > While reviewing the code I realized this might create issues when used > in combination with dynamic-routing-arp-prefer-local=true. > > Any traffic routed by an ovn router connected to this logical switch > will prefer SB.MAC_Binding mac addresses it might have for the next-hop > IP X. > > But if X is also learned through evpn on this logical switch, > potentially with a different MAC address than what we have in the > SB.MAC_Binding, traffic from VIFs attached to the switch destined to X > will go to a different MAC. > > Do we want to document this? Can we address it in any sensible way? >
After some more thinking: this would definitely be a misconfig. There shouldn't be two hosts (different MACs) using the same IP on the same broadcast domain. It's fine to keep it as is. > Kind of related context: > https://redhat.atlassian.net/browse/FDP-4000 > >> + const struct in6_addr *ip = &arp->ip; >> + struct eth_addr mac = arp->mac; >> + const struct local_datapath *ldp = arp->ldp; >> + >> + match_init_catchall(match); >> + match_set_metadata(match, htonll(ldp->datapath->tunnel_key)); >> + >> + if (IN6_IS_ADDR_V4MAPPED(ip)) { >> + ovs_be32 ip4 = in6_addr_get_mapped_ipv4(ip); >> + match_set_reg(match, 0, ntohl(ip4)); >> + } else { >> + ovs_be128 value; >> + memcpy(&value, ip, sizeof(value)); >> + match_set_xxreg(match, 0, ntoh128(value)); >> + } >> + >> + ofpbuf_clear(ofpacts); >> + >> + /* Load resolved MAC into eth.dst. */ >> + put_load_bytes(mac.ea, sizeof mac.ea, >> + MFF_ETH_DST, 0, 48, ofpacts); >> + /* Set the EVPN lookup result bit. */ >> + put_load(1, MFF_LOG_FLAGS, MLF_EVPN_LOOKUP_BIT, 1, ofpacts); >> + >> + ofctrl_add_flow(flow_table, OFTABLE_EVPN_ARP_LOOKUP, arp->priority, >> + arp->flow_uuid.parts[0], match, ofpacts, >> + &arp->flow_uuid); >> } >> >> static void >> @@ -3753,7 +3787,8 @@ physical_eval_evpn_flows(const struct physical_ctx >> *ctx, >> >> const struct evpn_arp *arp; >> HMAP_FOR_EACH (arp, hmap_node, ctx->evpn_arps) { >> - physical_consider_evpn_arp(ctx->local_datapaths, arp, flow_table); >> + physical_consider_evpn_arp(ctx->local_datapaths, arp, ofpacts, >> + &match, flow_table); >> } >> } >> >> @@ -3967,14 +4002,19 @@ physical_handle_evpn_arp_changes(const struct hmap >> *local_datapaths, >> const struct hmapx *updated_arps, >> const struct uuidset *removed_arps) >> { >> + struct ofpbuf ofpacts; >> + ofpbuf_init(&ofpacts, 0); >> + struct match match = MATCH_CATCHALL_INITIALIZER; >> >> const struct hmapx_node *node; >> HMAPX_FOR_EACH (node, updated_arps) { >> const struct evpn_arp *arp = node->data; >> >> ofctrl_remove_flows(flow_table, &arp->flow_uuid); >> - physical_consider_evpn_arp(local_datapaths, arp, flow_table); >> + physical_consider_evpn_arp(local_datapaths, arp, &ofpacts, >> + &match, flow_table); >> } >> + ofpbuf_uninit(&ofpacts); >> >> const struct uuidset_node *uuidset_node; >> UUIDSET_FOR_EACH (uuidset_node, removed_arps) { >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 35df0ec2f..1f0f17cb7 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -18528,6 +18528,66 @@ ovn-appctl evpn/vtep-arp-list > arp_after >> >> check diff -q arp_before arp_after >> >> +AS_BOX([EVPN ARP/ND side table flows]) >> +dnl Verify that EVPN ARP entries also produce flows in the dedicated >> +dnl side table (OFTABLE_EVPN_ARP_LOOKUP) for the switch datapath. >> +dnl These flows match on metadata+IP and load the resolved MAC into >> +dnl eth.dst while setting the EVPN lookup bit (reg10[25]). >> +dnl Verify we have 6 flows: 3 IPv4 + 3 IPv6. >> +AT_CHECK([test "$(ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP >> | \ >> + grep -c priority)" = "6"]) >> + >> +dnl Verify each IPv4 EVPN entry has a side table flow. >> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep "reg0=0xac100132" | grep -q "metadata=0x$dp_key"]) >> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep "reg0=0xac10013c" | grep -q "metadata=0x$dp_key"]) >> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep "reg0=0xac100146" | grep -q "metadata=0x$dp_key"]) >> + >> +dnl Verify each IPv6 EVPN entry has a side table flow. >> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep "reg0=0x1720016" | grep "reg3=0x50" | grep -q >> "metadata=0x$dp_key"]) >> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep "reg0=0x1720016" | grep "reg3=0x60" | grep -q >> "metadata=0x$dp_key"]) >> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep "reg0=0x1720016" | grep "reg3=0x70" | grep -q >> "metadata=0x$dp_key"]) > > It might be nice to also check the mac addresses in the 6 flows above. > >> + >> +dnl Verify recompute stability for side table flows. >> +ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep priority | awk '{print $[7], $[8]}' | sort > evpn_side_before >> + >> +check ovn-appctl inc-engine/recompute >> +check ovn-nbctl --wait=hv sync >> + >> +ovs-ofctl dump-flows br-int table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep priority | awk '{print $[7], $[8]}' | sort > evpn_side_after >> +AT_CHECK([diff -q evpn_side_before evpn_side_after]) >> + >> +dnl Verify side table flows are removed when EVPN entries are removed. >> +check ip neigh del dev $BR_NAME 172.16.1.50 >> +check ip neigh del dev $BR_NAME 172.16.1.60 >> +check ip neigh del dev $BR_NAME 172.16.1.70 >> + >> +check ip -6 neigh del dev $BR_NAME 172:16::50 >> +check ip -6 neigh del dev $BR_NAME 172:16::60 >> +check ip -6 neigh del dev $BR_NAME 172:16::70 >> + >> +OVS_WAIT_UNTIL([test "$(ovs-ofctl dump-flows br-int >> table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep -c priority)" = "0"]) >> + >> +dnl Re-add the EVPN entries for subsequent tests. >> +check ip neigh add dev $BR_NAME 172.16.1.50 lladdr f0:00:0f:16:10:50 nud >> noarp extern_learn >> +check ip neigh add dev $BR_NAME 172.16.1.60 lladdr f0:00:0f:16:10:60 nud >> noarp extern_learn >> +check ip neigh add dev $BR_NAME 172.16.1.70 lladdr f0:00:0f:16:10:70 nud >> noarp extern_learn >> + >> +check ip -6 neigh add dev $BR_NAME 172:16::50 lladdr f0:00:0f:16:10:50 nud >> noarp extern_learn >> +check ip -6 neigh add dev $BR_NAME 172:16::60 lladdr f0:00:0f:16:10:60 nud >> noarp extern_learn >> +check ip -6 neigh add dev $BR_NAME 172:16::70 lladdr f0:00:0f:16:10:70 nud >> noarp extern_learn >> + >> +OVS_WAIT_UNTIL([test "$(ovs-ofctl dump-flows br-int >> table=OFTABLE_EVPN_ARP_LOOKUP | \ >> + grep -c priority)" = "6"]) >> + >> # Remove remote workload ARP entries and check ovn-controller's state. >> check ip neigh del dev $BR_NAME 172.16.1.50 >> check ip neigh del dev $BR_NAME 172.16.1.60 > > Regards, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
