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