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. > > > > 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
