On Thu, Aug 27, 2026 at 9:44 PM Jacob Tanenbaum <[email protected]> wrote:
> > > On Thu, Aug 27, 2026 at 9:29 AM Ales Musil via dev < > [email protected]> wrote: > >> On Thu, Aug 27, 2026 at 3:12 PM Ales Musil <[email protected]> wrote: >> >> > The re-ARP probes were using unicast to check if the host is still >> > alive to prevent the entry from aging out. The unicast works fine >> > and prevents unnecessary floods, however, there is a case when the >> > MAC address could have been changed without OVN learning about that. >> > In that case the unicast won't be ever responded to and the only way >> > to refresh that entry is to wait for it to age out. Send broadcast >> > after two unicast attempts with the timing that gives us usually >> > 2 unicast probes and 2 broadcast, we will age out if neither of them >> > is responded to. >> > >> > Fixes: 59c7361e2613 ("pinctrl: Use unicast for MAC binding ARP probe.") >> > Reported-at: https://redhat.atlassian.net/browse/FDP-4224 >> > Assisted-by: Claude Opus 4.6, OpenCode >> > Signed-off-by: Ales Musil <[email protected]> >> > --- >> > >> >> Ah I noticed a few wrong numbers in the comments. >> I'll fix that in v2 or during the merge, whichever comes first. >> >> >> > controller/mac-cache.c | 11 ++++- >> > controller/mac-cache.h | 2 + >> > tests/ovn.at | 102 +++++++++++++++++++++++++++++++++++++++++ >> > 3 files changed, 114 insertions(+), 1 deletion(-) >> > >> > diff --git a/controller/mac-cache.c b/controller/mac-cache.c >> > index 8f100a29d..39ab13754 100644 >> > --- a/controller/mac-cache.c >> > +++ b/controller/mac-cache.c >> > @@ -34,6 +34,7 @@ VLOG_DEFINE_THIS_MODULE(mac_cache); >> > #define BUFFER_QUEUE_DEPTH 4 >> > #define BUFFERED_PACKETS_TIMEOUT_MS 10000 >> > #define BUFFERED_PACKETS_LOOKUP_MS 100 >> > +#define ARP_BROADCAST_THRESHOLD 2 >> > > Nit: Is the name a little misleading? this threshold applies to both ARP > and ND. > Fair, updated in v2. > > >> > >> > static uint32_t >> > mac_binding_data_hash(const struct mac_binding_data *mb_data); >> > @@ -178,6 +179,7 @@ mac_binding_add(struct hmap *map, struct >> > mac_binding_data mb_data, >> > mb->data = mb_data; >> > mb->sbrec = smb; >> > mb->timestamp = timestamp; >> > + mb->arp_attempts = 0; >> > mac_binding_update_log("Added", &mb_data, false, NULL, 0, 0); >> > } >> > >> > @@ -908,6 +910,7 @@ mac_binding_probe_stats_run(struct vector >> *stats_vec, >> > uint64_t *req_delay, >> > "Not sending ARP/ND request for recently updated", >> > &mb->data, true, threshold, stats->idle_age_ms, >> > since_updated_ms); >> > + mb->arp_attempts = 0; >> > continue; >> > } >> > >> > @@ -954,6 +957,11 @@ mac_binding_probe_stats_run(struct vector >> *stats_vec, >> > uint64_t *req_delay, >> > } >> > >> > if (!ipv6_addr_equals(&local, &in6addr_any)) { >> > + struct eth_addr eth_dst = >> > + mb->arp_attempts < ARP_BROADCAST_THRESHOLD >> > + ? mb->data.mac >> > + : eth_addr_zero; >> > + >> > mac_binding_update_log("Sending ARP/ND request for active", >> > &mb->data, true, threshold, >> > stats->idle_age_ms, >> since_updated_ms); >> > @@ -961,9 +969,10 @@ mac_binding_probe_stats_run(struct vector >> *stats_vec, >> > uint64_t *req_delay, >> > send_self_originated_neigh_packet(probe_data->swconn, >> > >> sbrec->datapath->tunnel_key, >> > pb->tunnel_key, laddr.ea, >> > - mb->data.mac, &local, >> > + eth_dst, &local, >> > &mb->data.ip, >> > OFTABLE_LOCAL_OUTPUT); >> > + mb->arp_attempts++; >> > } >> > >> > destroy_lport_addresses(&laddr); >> > diff --git a/controller/mac-cache.h b/controller/mac-cache.h >> > index 8abea60c7..bf9afaf3a 100644 >> > --- a/controller/mac-cache.h >> > +++ b/controller/mac-cache.h >> > @@ -78,6 +78,8 @@ struct mac_binding { >> > const struct sbrec_mac_binding *sbrec; >> > /* User specified timestamp (in ms) */ >> > long long timestamp; >> > + /* Number of re-ARP attempts for given entry. */ >> > + size_t arp_attempts; >> > }; >> > >> > struct fdb_data { >> > diff --git a/tests/ovn.at b/tests/ovn.at >> > index a88a077c6..e169fc607 100644 >> > --- a/tests/ovn.at >> > +++ b/tests/ovn.at >> > @@ -37563,6 +37563,108 @@ OVN_CLEANUP([hv1]) >> > AT_CLEANUP >> > ]) >> > >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([MAC binding aging - probing unicast to broadcast transition]) >> > +CHECK_SCAPY >> > +ovn_start >> > + >> > +aging_th=5 >> > +net_add n1 >> > +sim_add hv1 >> > +as hv1 >> > +check ovs-vsctl add-br br-phys >> > +ovn_attach n1 br-phys 192.168.0.1 >> > +ovn-appctl -t ovn-controller vlog/set mac_cache:file:dbg >> pinctrl:file:dbg >> > + >> > +check ovn-nbctl >> > \ >> > + -- ls-add ls1 >> > \ >> > + -- lr-add lr >> > \ >> > + -- set logical_router lr >> options:mac_binding_age_threshold=$aging_th >> > \ >> > + -- lrp-add lr lr-ls1 00:00:00:00:10:00 10.10.10.1/24 42.42.42.1/24 >> > \ >> > + fd11::1/64 fd12::1/64 >> > \ >> > + -- lsp-add-router-port ls1 ls1-lr lr-ls1 >> > \ >> > + -- lsp-add ls1 vif1 >> > \ >> > + -- lsp-set-addresses vif1 "unknown" >> > + >> > +check ovs-vsctl >> > \ >> > + -- add-port br-int vif1 >> > \ >> > + -- set interface vif1 external-ids:iface-id=vif1 >> > \ >> > + options:tx_pcap=hv1/vif1-tx.pcap options:rxq_pcap=hv1/vif1-rx.pcap >> > + >> > +OVN_POPULATE_ARP >> > +wait_for_ports_up >> > +check ovn-nbctl --wait=hv sync >> > + >> > +# Wait for pinctrl thread to be connected. >> > +OVS_WAIT_UNTIL([grep pinctrl hv1/ovn-controller.log | grep -q >> connected]) >> > + >> > +# Create one IPv4 and one IPv6 MAC binding. >> > +send_garp hv1 vif1 2 00:00:00:00:10:1a ff:ff:ff:ff:ff:ff 10.10.10.100 >> > 10.10.10.100 >> > +wait_row_count mac_binding 1 ip="10.10.10.100" logical_port="lr-ls1" >> > + >> > +send_na hv1 vif1 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1 >> > +wait_row_count mac_binding 1 ip=\"fd11::64\" logical_port=\"lr-ls1\" >> > + >> > +# The first 3 probes are unicast (arp_attempts 0-2); the entry then >> falls >> > >> >> Should be 2 and (0-1). >> >> >> > +# back to broadcast ARP / multicast NS. >> > +dump_arp 1 00:00:00:00:10:00 ff:ff:ff:ff:ff:ff 10.10.10.1 10.10.10.100 >> > 00:00:00:00:00:00 > expected_bcast >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_bcast]) >> > + >> > +dump_ns 33:33:ff:00:00:64 00:00:00:00:10:00 ff02::1:ff00:64 fd11::1 >> > fd11::64 > expected_mcast >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_mcast]) >> > + >> > +dump_arp 1 00:00:00:00:10:00 00:00:00:00:10:1a 10.10.10.1 10.10.10.100 >> > 00:00:00:00:10:1a > expected_ucast_v4 >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_ucast_v4]) >> > + >> > +dump_ns 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1 fd11::64 > >> > expected_ucast_v6 >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_ucast_v6]) >> > + >> > +# Verify the reset-to-zero mechanism using a distinct pair of >> neighbours >> > +# kept alive (never re-created) for the whole check. A single entry >> emits >> > +# at most ARP_BROADCAST_THRESHOLD (3) unicast probes before falling >> back >> > to >> > >> >> Should be (2). >> >> >> > +# broadcast, so a 4th unicast probe proves arp_attempts was reset. >> > +send_garp hv1 vif1 2 00:00:00:00:10:1b ff:ff:ff:ff:ff:ff 10.10.10.101 >> > 10.10.10.101 >> > +wait_row_count mac_binding 1 ip="10.10.10.101" logical_port="lr-ls1" >> > +v4_uuid=$(fetch_column Mac_Binding _uuid ip=10.10.10.101) >> > + >> > +send_na hv1 vif1 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1 >> > +wait_row_count mac_binding 1 ip=\"fd11::65\" logical_port=\"lr-ls1\" >> > +v6_uuid=$(fetch_column Mac_Binding _uuid ip=\"fd11::65\") >> > + >> > +# Keep the entries active (Tx towards them) and let a couple of unicast >> > +# probes go out (arp_attempts reaches 2, below the broadcast >> threshold). >> > >> >> Should be 1. >> >> >> > +send_udp hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a 10.10.10.101 >> > 42.42.42.100 >> > +send_udp6 hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a fd11::65 >> fd12::100 >> > +OVS_WAIT_UNTIL([test $(grep -c "Sending ARP/ND.*ip: 10.10.10.101" >> > hv1/ovn-controller.log) -ge 2]) >> > +OVS_WAIT_UNTIL([test $(grep -c "Sending ARP/ND.*ip: fd11::65" >> > hv1/ovn-controller.log) -ge 2]) >> > + >> > +# The neighbours answer, refreshing the rows in place (resetting >> > +# arp_attempts). Confirm the timestamps advanced. >> > +v4_ts=$(fetch_column Mac_Binding timestamp ip=10.10.10.101) >> > +v6_ts=$(fetch_column Mac_Binding timestamp ip=\"fd11::65\") >> > +send_garp hv1 vif1 2 00:00:00:00:10:1b 00:00:00:00:10:00 10.10.10.101 >> > 10.10.10.1 >> > +send_na hv1 vif1 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1 >> > +OVS_WAIT_UNTIL([test $(fetch_column Mac_Binding timestamp >> > ip=10.10.10.101) -gt $v4_ts]) >> > +OVS_WAIT_UNTIL([test $(fetch_column Mac_Binding timestamp >> > ip=\"fd11::65\") -gt $v6_ts]) >> > + >> > +# Probing must restart from unicast: wait for a 4th unicast ARP/NS >> probe >> > +# while the rows are still the original ones (same UUID, never >> > re-created). >> > +dump_arp 1 00:00:00:00:10:00 00:00:00:00:10:1b 10.10.10.1 10.10.10.101 >> > 00:00:00:00:10:1b > ucast_v4.pkt >> > +dump_ns 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1 fd11::65 > >> > ucast_v6.pkt >> > +OVS_WAIT_UNTIL([ >> > + send_udp hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a 10.10.10.101 >> > 42.42.42.100 >> > + send_udp6 hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a fd11::65 >> > fd12::100 >> > + test "$(fetch_column Mac_Binding _uuid ip=10.10.10.101)" = >> "$v4_uuid" >> > && \ >> > + test "$(fetch_column Mac_Binding _uuid ip=\"fd11::65\")" = >> "$v6_uuid" >> > && \ >> > + test $($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" >> hv1/vif1-tx.pcap >> > | \ >> > + grep -Fc "$(cat ucast_v4.pkt)") -ge 4 && \ >> > + test $($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" >> hv1/vif1-tx.pcap >> > | \ >> > + grep -Fc "$(cat ucast_v6.pkt)") -ge 4]) >> > + >> > +OVN_CLEANUP([hv1]) >> > +AT_CLEANUP >> > +]) >> > + >> > OVN_FOR_EACH_NORTHD([ >> > AT_SETUP([MAC binding aging - probing distributed GW router]) >> > CHECK_SCAPY >> > -- >> > 2.55.0 >> > >> > >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
