Make sure we remove the stale Static MAC Bindings if there is SB
datapath re-creation in a single transaction. Otherwise, northd
would be stuck in a commit failed loop due to "referential integrity
violation" as the static mac binding would still reference the old
datapath.
At the same time unify the check if dynamic/static mac binding is
stale as it was different up until now. Both of them use the same
columns as an indication of logical port and datapath.
Fixes: b22684d4e37c ("Add a northbound interface to program MAC_Binding table")
Reported-at: https://issues.redhat.com/browse/FDP-1623
Signed-off-by: Ales Musil <amu...@redhat.com>
---
v2: Replace the Fixes tags.
Remove the redundant check for op->nbrp.
Update the commit message with more details.
---
northd/northd.c | 28 +++++++++++++++++++---------
tests/ovn-northd.at | 8 ++++++++
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 015f30a35..4b5b6584e 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2867,6 +2867,19 @@ common:
}
}
+static bool
+mac_binding_is_stale(const struct hmap *lr_datapaths,
+ const struct hmap *lr_ports,
+ const struct sbrec_datapath_binding *dp,
+ const char *logical_port)
+{
+ const struct ovn_datapath *od =
+ ovn_datapath_from_sbrec(NULL, lr_datapaths, dp);
+
+ return !od || ovn_datapath_is_stale(od) ||
+ !ovn_port_find(lr_ports, logical_port);
+}
+
/* Remove mac_binding entries that refer to logical_ports which are
* deleted. */
static void
@@ -2876,11 +2889,8 @@ cleanup_mac_bindings(
{
const struct sbrec_mac_binding *b;
SBREC_MAC_BINDING_TABLE_FOR_EACH_SAFE (b, sbrec_mac_binding_table) {
- const struct ovn_datapath *od =
- ovn_datapath_from_sbrec(NULL, lr_datapaths, b->datapath);
-
- if (!od || ovn_datapath_is_stale(od) ||
- !ovn_port_find(lr_ports, b->logical_port)) {
+ if (mac_binding_is_stale(lr_datapaths, lr_ports, b->datapath,
+ b->logical_port)) {
sbrec_mac_binding_delete(b);
}
}
@@ -18850,7 +18860,7 @@ build_static_mac_binding_table(
struct ovsdb_idl_txn *ovnsb_txn,
const struct nbrec_static_mac_binding_table *nbrec_static_mb_table,
const struct sbrec_static_mac_binding_table *sbrec_static_mb_table,
- struct hmap *lr_ports)
+ const struct hmap *lr_datapaths, const struct hmap *lr_ports)
{
/* Cleanup SB Static_MAC_Binding entries which do not have corresponding
* NB Static_MAC_Binding entries, and SB Static_MAC_Binding entries for
@@ -18866,8 +18876,8 @@ build_static_mac_binding_table(
continue;
}
- struct ovn_port *op = ovn_port_find(lr_ports, nb_smb->logical_port);
- if (!op || !op->nbrp || !op->od || !op->od->sdp->sb_dp) {
+ if (mac_binding_is_stale(lr_datapaths, lr_ports, sb_smb->datapath,
+ sb_smb->logical_port)) {
sbrec_static_mac_binding_delete(sb_smb);
}
}
@@ -19187,7 +19197,7 @@ ovnnb_db_run(struct northd_input *input_data,
build_static_mac_binding_table(ovnsb_txn,
input_data->nbrec_static_mac_binding_table,
input_data->sbrec_static_mac_binding_table,
- &data->lr_ports);
+ &data->lr_datapaths.datapaths, &data->lr_ports);
stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 11bbb211d..1a4910924 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -8518,6 +8518,14 @@ wait_row_count sb:Static_MAC_Binding 1
logical_port=lr-lrp ip=1.1.1.2 mac="00\:0
check ovn-nbctl lr-del lr
wait_row_count sb:Static_MAC_Binding 0
+dnl Also check for router (and router port) being recreated in the same
+dnl transaction.
+check ovn-nbctl lr-add lr -- lrp-add lr lr-lrp 00:00:00:00:00:01 1.1.1.1/24
+wait_row_count sb:Static_MAC_Binding 1 logical_port=lr-lrp ip=1.1.1.2
mac="00\:00\:00\:00\:00\:02"
+check ovn-nbctl lrp-del lr-lrp -- lr-del lr -- lr-add lr -- lrp-add lr lr-lrp
00:00:00:00:00:01 1.1.1.1/24
+wait_row_count sb:Static_MAC_Binding 1
+check ovn-nbctl --wait=sb sync
+
AT_CLEANUP
])
--
2.50.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
