Unfortunately the previous fix wasn't enough, when we call
"sbrec_static_mac_binding_table_get_for_uuid()" it will still return
row even if was marked as deleted in the current transaction. This
would cause the Static MAC binding to be removed in one txn and added
in other one, causing traffic disruptions. To properly handle that
update the datapath reference instead. This should do the update
in a single transaction instead.
Fixes: e8a0ade51cc1 ("northd: Remove stale Static MAC Bindings during SB
datapath re-creation.")
Suggested-by: Ilya Maximets <i.maxim...@ovn.org>
Signed-off-by: Ales Musil <amu...@redhat.com>
---
northd/northd.c | 61 ++++++++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 36 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index e0a329a17..3456e2918 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2867,19 +2867,6 @@ 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
@@ -2889,8 +2876,11 @@ cleanup_mac_bindings(
{
const struct sbrec_mac_binding *b;
SBREC_MAC_BINDING_TABLE_FOR_EACH_SAFE (b, sbrec_mac_binding_table) {
- if (mac_binding_is_stale(lr_datapaths, lr_ports, b->datapath,
- b->logical_port)) {
+ 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)) {
sbrec_mac_binding_delete(b);
}
}
@@ -18860,34 +18850,19 @@ 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,
- const struct hmap *lr_datapaths, const struct hmap *lr_ports)
+ 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
- * which there is not a NB Logical_Router_Port of the same name or
- * entries which SB Datapath_Binding reference is considered
- * stale/removed. */
- const struct nbrec_static_mac_binding *nb_smb;
+ struct hmapx stale = HMAPX_INITIALIZER(&stale);
const struct sbrec_static_mac_binding *sb_smb;
SBREC_STATIC_MAC_BINDING_TABLE_FOR_EACH_SAFE (sb_smb,
sbrec_static_mb_table) {
- nb_smb = nbrec_static_mac_binding_table_get_for_uuid(
- nbrec_static_mb_table, &sb_smb->header_.uuid);
- if (!nb_smb) {
- sbrec_static_mac_binding_delete(sb_smb);
- continue;
- }
-
- if (mac_binding_is_stale(lr_datapaths, lr_ports, sb_smb->datapath,
- sb_smb->logical_port)) {
- sbrec_static_mac_binding_delete(sb_smb);
- }
+ hmapx_add(&stale, CONST_CAST(void *, sb_smb));
}
/* Create/Update SB Static_MAC_Binding entries with corresponding values
* from NB Static_MAC_Binding entries. */
- NBREC_STATIC_MAC_BINDING_TABLE_FOR_EACH (
- nb_smb, nbrec_static_mb_table) {
+ const struct nbrec_static_mac_binding *nb_smb;
+ NBREC_STATIC_MAC_BINDING_TABLE_FOR_EACH (nb_smb, nbrec_static_mb_table) {
struct ovn_port *op = ovn_port_find(lr_ports, nb_smb->logical_port);
if (op && op->nbrp) {
struct ovn_datapath *od = op->od;
@@ -18909,6 +18884,11 @@ build_static_mac_binding_table(
sbrec_static_mac_binding_set_datapath(mb, od->sdp->sb_dp);
} else {
/* Update existing entry if there is a change*/
+ if (mb->datapath != od->sdp->sb_dp) {
+ sbrec_static_mac_binding_set_datapath(
+ mb, od->sdp->sb_dp);
+ }
+
if (strcmp(mb->mac, nb_smb->mac)) {
sbrec_static_mac_binding_set_mac(mb, nb_smb->mac);
}
@@ -18927,10 +18907,19 @@ build_static_mac_binding_table(
sbrec_static_mac_binding_set_override_dynamic_mac(mb,
nb_smb->override_dynamic_mac);
}
+
+ hmapx_find_and_delete(&stale, mb);
}
}
}
}
+
+ /* Cleanup SB Static_MAC_Binding entries which are no longer needed. */
+ struct hmapx_node *node;
+ HMAPX_FOR_EACH (node, &stale) {
+ sbrec_static_mac_binding_delete(node->data);
+ }
+ hmapx_destroy(&stale);
}
static void
@@ -19209,7 +19198,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_datapaths.datapaths, &data->lr_ports);
+ &data->lr_ports);
stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
--
2.50.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
