Skip to content

Commit 7038ae7

Browse files
committed
mac-cache: Send broadcast re-ARP probes after a while.
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: 59c7361 ("pinctrl: Use unicast for MAC binding ARP probe.") Reported-at: https://redhat.atlassian.net/browse/FDP-4224 Assisted-by: Claude Opus 4.6, OpenCode Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ales Musil <amusil@redhat.com> (cherry picked from commit 47203af)
1 parent 1df2bbe commit 7038ae7

3 files changed

Lines changed: 119 additions & 1 deletion

File tree

controller/mac-cache.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ VLOG_DEFINE_THIS_MODULE(mac_cache);
3434
#define BUFFER_QUEUE_DEPTH 4
3535
#define BUFFERED_PACKETS_TIMEOUT_MS 10000
3636
#define BUFFERED_PACKETS_LOOKUP_MS 100
37+
#define PROBE_MULICAST_THRESHOLD 2
3738

3839
static uint32_t
3940
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,
178179
mb->data = mb_data;
179180
mb->sbrec = smb;
180181
mb->timestamp = timestamp;
182+
mb->arp_attempts = 0;
181183
mac_binding_update_log("Added", &mb_data, false, NULL, 0, 0);
182184
}
183185

@@ -908,6 +910,7 @@ mac_binding_probe_stats_run(struct vector *stats_vec, uint64_t *req_delay,
908910
"Not sending ARP/ND request for recently updated",
909911
&mb->data, true, threshold, stats->idle_age_ms,
910912
since_updated_ms);
913+
mb->arp_attempts = 0;
911914
continue;
912915
}
913916

@@ -954,16 +957,22 @@ mac_binding_probe_stats_run(struct vector *stats_vec, uint64_t *req_delay,
954957
}
955958

956959
if (!ipv6_addr_equals(&local, &in6addr_any)) {
960+
struct eth_addr eth_dst =
961+
mb->arp_attempts < PROBE_MULICAST_THRESHOLD
962+
? mb->data.mac
963+
: eth_addr_zero;
964+
957965
mac_binding_update_log("Sending ARP/ND request for active",
958966
&mb->data, true, threshold,
959967
stats->idle_age_ms, since_updated_ms);
960968

961969
send_self_originated_neigh_packet(probe_data->swconn,
962970
sbrec->datapath->tunnel_key,
963971
pb->tunnel_key, laddr.ea,
964-
mb->data.mac, &local,
972+
eth_dst, &local,
965973
&mb->data.ip,
966974
OFTABLE_LOCAL_OUTPUT);
975+
mb->arp_attempts++;
967976
}
968977

969978
destroy_lport_addresses(&laddr);

controller/mac-cache.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ struct mac_binding {
7878
const struct sbrec_mac_binding *sbrec;
7979
/* User specified timestamp (in ms) */
8080
long long timestamp;
81+
/* Number of re-ARP attempts for given entry. */
82+
size_t arp_attempts;
8183
};
8284

8385
struct fdb_data {

tests/ovn.at

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37563,6 +37563,113 @@ OVN_CLEANUP([hv1])
3756337563
AT_CLEANUP
3756437564
])
3756537565

37566+
OVN_FOR_EACH_NORTHD([
37567+
AT_SETUP([MAC binding aging - probing unicast to broadcast transition])
37568+
CHECK_SCAPY
37569+
ovn_start
37570+
37571+
aging_th=5
37572+
net_add n1
37573+
sim_add hv1
37574+
as hv1
37575+
check ovs-vsctl add-br br-phys
37576+
ovn_attach n1 br-phys 192.168.0.1
37577+
ovn-appctl -t ovn-controller vlog/set mac_cache:file:dbg pinctrl:file:dbg
37578+
37579+
check ovn-nbctl \
37580+
-- ls-add ls1 \
37581+
-- lr-add lr \
37582+
-- set logical_router lr options:mac_binding_age_threshold=$aging_th \
37583+
-- lrp-add lr lr-ls1 00:00:00:00:10:00 10.10.10.1/24 42.42.42.1/24 \
37584+
fd11::1/64 fd12::1/64 \
37585+
-- lsp-add-router-port ls1 ls1-lr lr-ls1 \
37586+
-- lsp-add ls1 vif1 \
37587+
-- lsp-set-addresses vif1 "unknown"
37588+
37589+
check ovs-vsctl \
37590+
-- add-port br-int vif1 \
37591+
-- set interface vif1 external-ids:iface-id=vif1 \
37592+
options:tx_pcap=hv1/vif1-tx.pcap options:rxq_pcap=hv1/vif1-rx.pcap
37593+
37594+
OVN_POPULATE_ARP
37595+
wait_for_ports_up
37596+
check ovn-nbctl --wait=hv sync
37597+
37598+
# Wait for pinctrl thread to be connected.
37599+
OVS_WAIT_UNTIL([grep pinctrl hv1/ovn-controller.log | grep -q connected])
37600+
37601+
# Create one IPv4 and one IPv6 MAC binding.
37602+
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
37603+
wait_row_count mac_binding 1 ip="10.10.10.100" logical_port="lr-ls1"
37604+
37605+
send_na hv1 vif1 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1
37606+
wait_row_count mac_binding 1 ip=\"fd11::64\" logical_port=\"lr-ls1\"
37607+
37608+
# The first 2 probes are unicast (arp_attempts 0-1); the entry then falls
37609+
# back to broadcast ARP / multicast NS.
37610+
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
37611+
OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_bcast])
37612+
37613+
dump_ns 33:33:ff:00:00:64 00:00:00:00:10:00 ff02::1:ff00:64 fd11::1 fd11::64 > expected_mcast
37614+
OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_mcast])
37615+
37616+
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
37617+
OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_ucast_v4])
37618+
37619+
dump_ns 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1 fd11::64 > expected_ucast_v6
37620+
OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_ucast_v6])
37621+
37622+
# Verify the reset-to-zero mechanism using a distinct pair of neighbours
37623+
# kept alive (never re-created) for the whole check. A single entry emits
37624+
# at most ARP_BROADCAST_THRESHOLD (2) unicast probes before falling back to
37625+
# broadcast, so a 4th unicast probe proves arp_attempts was reset.
37626+
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
37627+
wait_row_count mac_binding 1 ip="10.10.10.101" logical_port="lr-ls1"
37628+
v4_uuid=$(fetch_column Mac_Binding _uuid ip=10.10.10.101)
37629+
37630+
send_na hv1 vif1 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1
37631+
wait_row_count mac_binding 1 ip=\"fd11::65\" logical_port=\"lr-ls1\"
37632+
v6_uuid=$(fetch_column Mac_Binding _uuid ip=\"fd11::65\")
37633+
37634+
# Keep the entries active (Tx towards them) and let a couple of unicast
37635+
# probes go out (arp_attempts reaches 1, below the broadcast threshold).
37636+
send_udp hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a 10.10.10.101 42.42.42.100
37637+
send_udp6 hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a fd11::65 fd12::100
37638+
37639+
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_101.pkt
37640+
dump_ns 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1 fd11::65 > ucast_v6_65.pkt
37641+
OVS_WAIT_UNTIL([test $($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap | \
37642+
grep -Fc "$(cat ucast_v4_101.pkt)") -ge 2])
37643+
OVS_WAIT_UNTIL([test $($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap | \
37644+
grep -Fc "$(cat ucast_v6_65.pkt)") -ge 2])
37645+
37646+
# The neighbours answer, refreshing the rows in place (resetting
37647+
# arp_attempts). Confirm the timestamps advanced.
37648+
v4_ts=$(fetch_column Mac_Binding timestamp ip=10.10.10.101)
37649+
v6_ts=$(fetch_column Mac_Binding timestamp ip=\"fd11::65\")
37650+
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
37651+
send_na hv1 vif1 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1
37652+
OVS_WAIT_UNTIL([test $(fetch_column Mac_Binding timestamp ip=10.10.10.101) -gt $v4_ts])
37653+
OVS_WAIT_UNTIL([test $(fetch_column Mac_Binding timestamp ip=\"fd11::65\") -gt $v6_ts])
37654+
37655+
# Probing must restart from unicast: wait for a 4th unicast ARP/NS probe
37656+
# while the rows are still the original ones (same UUID, never re-created).
37657+
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
37658+
dump_ns 00:00:00:00:10:1b 00:00:00:00:10:00 fd11::65 fd11::1 fd11::65 > ucast_v6.pkt
37659+
OVS_WAIT_UNTIL([
37660+
send_udp hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a 10.10.10.101 42.42.42.100
37661+
send_udp6 hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a fd11::65 fd12::100
37662+
test "$(fetch_column Mac_Binding _uuid ip=10.10.10.101)" = "$v4_uuid" && \
37663+
test "$(fetch_column Mac_Binding _uuid ip=\"fd11::65\")" = "$v6_uuid" && \
37664+
test $($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap | \
37665+
grep -Fc "$(cat ucast_v4.pkt)") -ge 4 && \
37666+
test $($PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap | \
37667+
grep -Fc "$(cat ucast_v6.pkt)") -ge 4])
37668+
37669+
OVN_CLEANUP([hv1])
37670+
AT_CLEANUP
37671+
])
37672+
3756637673
OVN_FOR_EACH_NORTHD([
3756737674
AT_SETUP([MAC binding aging - probing distributed GW router])
3756837675
CHECK_SCAPY

0 commit comments

Comments
 (0)