When installing a new flow then its idle_age will start ticking from 0.
We previously treated this as the flow being used and bumped the MAC_Binding
timestamp. However if the flow has just been freshly installed it was
never used and therefore should not be bumped.
This is especially important for chassis that add the flows newly
because they became part of the relevant datapath.
In case of a large enough mac_binding_age_threshold and if the datapath
is added and removed across chassis faster than the
mac_binding_age_threshold this can actually lead to MAC_Binding that are
never removed even though they are inactive.
We also need to fix the test "MAC binding aging - probing GW router
Dynamic Neigh" since it previously relied on the fact that a newly
created flow directly triggers an update. This pushed the time of the
first probe request sufficiently for the test to work. As we now no
longer do this we can at most wait for 'cooldown_period' as afterwards a
probe might be send immediately.
Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
controller/mac-cache.c | 6 ++++
tests/ovn.at | 66 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/controller/mac-cache.c b/controller/mac-cache.c
index fcf5fa7d4..53286a8fe 100644
--- a/controller/mac-cache.c
+++ b/controller/mac-cache.c
@@ -346,6 +346,9 @@ void
mac_binding_stats_process_flow_stats(struct vector *stats_vec,
struct ofputil_flow_stats *ofp_stats)
{
+ if (ofp_stats->idle_age == ofp_stats->duration_sec) {
+ return;
+ }
struct mac_cache_stats stats = (struct mac_cache_stats) {
.idle_age_ms = ofp_stats->idle_age * 1000,
.data.mb = (struct mac_binding_data) {
@@ -450,6 +453,9 @@ void
fdb_stats_process_flow_stats(struct vector *stats_vec,
struct ofputil_flow_stats *ofp_stats)
{
+ if (ofp_stats->idle_age == ofp_stats->duration_sec) {
+ return;
+ }
struct mac_cache_stats stats = (struct mac_cache_stats) {
.idle_age_ms = ofp_stats->idle_age * 1000,
.data.fdb = (struct fdb_data) {
diff --git a/tests/ovn.at b/tests/ovn.at
index 292ca0dae..81fbfc629 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -36578,6 +36578,68 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([MAC binding aging - unused entries])
+ovn_start
+
+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=4 \
+ -- lrp-add lr lr-ls1 00:00:00:00:10:00 192.168.10.1/24 \
+ -- lsp-add ls1 ls1-lr \
+ -- lsp-set-type ls1-lr router \
+ -- lsp-set-addresses ls1-lr router \
+ -- lsp-set-options ls1-lr router-port=lr-ls1 \
+ -- lsp-add ls1 vif1 \
+ -- lsp-set-addresses vif1 "00:00:00:00:10:10 192.168.10.10"
+
+check ovs-vsctl \
+ -- add-port br-int vif1 \
+ -- set interface vif1 external-ids:iface-id=vif1
+
+OVN_POPULATE_ARP
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+dnl Wait for pinctrl thread to be connected.
+OVS_WAIT_UNTIL([grep pinctrl hv1/ovn-controller.log | grep -q connected])
+
+dnl Create the MAC_Binding directly in the database to ensure it is never used.
+dnl The timestamp is set to now.
+dp=$(fetch_column Datapath_Binding _uuid external_ids:name=lr)
+now=$(date +%s)000
+check_uuid ovn-sbctl create MAC_Binding datapath=$dp logical_port=lr-ls1 \
+ ip=192.168.10.20 'mac="00:00:00:11:22:33"' timestamp=$now
+
+dnl Wait until the flow is installed locally
+OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE | grep
192.168.10.20])
+
+dnl Wait long enough to give ovn-controller the chance to decide if the entry
+dnl should be bumped or not.
+sleep 2
+
+dnl The entry should still be there and the timestamp should not have been
+dnl updated
+check_row_count mac_binding 1 ip="192.168.10.20"
+new_ts=$(fetch_column MAC_Binding timestamp ip="192.168.10.20")
+check test "$now" = "$new_ts"
+
+dnl After waiting until the age threshold the entry should be deleted
+sleep 3
+check_row_count mac_binding 0 ip="192.168.10.20"
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD([
AT_SETUP([MAC binding aging - probing])
AT_SKIP_IF([test $HAVE_SCAPY = no])
@@ -36844,7 +36906,7 @@ OVS_WAIT_UNTIL([$(ovs-ofctl dump-flows br-int
table=OFTABLE_MAC_BINDING | \
n_arp=$(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE |awk
'/arp_spa=192.168.20.2/{print substr($4,11,1)}')
# Check GW router does not send any ARP requests in this case.
OVS_WAIT_UNTIL([test $(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE
| \
- awk '/nw_src=192.168.10.100/{print substr($6,10,1)}')
-ge $((aging_th/2))])
+ awk '/nw_src=192.168.10.100/{print substr($6,10,1)}')
-ge $((aging_th*3/16))])
AT_CHECK([test $(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE |awk
'/arp_spa=192.168.20.2/{print substr($4,11,1)}') -eq $n_arp])
send_imcp_echo_req hv1 public 00:00:00:00:10:00 00:00:00:00:10:1a 192.168.20.2
192.168.10.100
@@ -36865,7 +36927,7 @@ check_row_count mac_binding 1 mac=\"00:00:00:00:30:00\"
ip=\"fd12::2\"
n_ipv6=$(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE |awk
'/ipv6_src=fd12::2/{print substr($4,11,1)}')
OVS_WAIT_UNTIL([test $(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE
| \
- awk '/ipv6_src=fd12::2/{print substr($6,10,1)}') -ge
$((aging_th/2))])
+ awk '/ipv6_src=fd12::2/{print substr($6,10,1)}') -ge
$((aging_th*3/16))])
# Check GW router does not send any NS requests in this case.
AT_CHECK([test $(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE |awk
'/ipv6_src=fd12::2/{print substr($4,11,1)}') -eq $n_ipv6])
send_icmp6_echo_req hv1 public 00:00:00:00:10:00 00:00:00:00:50:01 fd12::2
fd11::64
base-commit: b6ee938b9f87a428ca9ebd90894c49c3c3353d23
--
2.43.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
