From 1877c8b464dcad3f240527453d92f53d966af234 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 16 Apr 2025 14:45:41 +0200 Subject: [PATCH 1/2] ovs: slightly improve _delete_interface() Add comments, and move variables inside the block where they are used. (cherry picked from commit 78a4e5cf3bd2b8cba19c22826c1a4b99d3d7ef0e) --- src/core/devices/ovs/nm-ovsdb.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/core/devices/ovs/nm-ovsdb.c b/src/core/devices/ovs/nm-ovsdb.c index 2dbc842a73..0ab7c05f8e 100644 --- a/src/core/devices/ovs/nm-ovsdb.c +++ b/src/core/devices/ovs/nm-ovsdb.c @@ -1374,54 +1374,57 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) nm_auto_decref_json json_t *bridges = NULL; nm_auto_decref_json json_t *new_bridges = NULL; gboolean bridges_changed; - gboolean ports_changed; - gboolean interfaces_changed; - int pi; - int ii; bridges = json_array(); new_bridges = json_array(); bridges_changed = FALSE; + /* Loop over all bridges */ g_hash_table_iter_init(&iter, priv->bridges); while (g_hash_table_iter_next(&iter, (gpointer) &ovs_bridge, NULL)) { - nm_auto_decref_json json_t *ports = NULL; - nm_auto_decref_json json_t *new_ports = NULL; + nm_auto_decref_json json_t *ports = NULL; + nm_auto_decref_json json_t *new_ports = NULL; + gboolean ports_changed = FALSE; + int pi; ports = json_array(); new_ports = json_array(); - ports_changed = FALSE; + /* Add the bridge UUID to the list of known bridges for the "expect" condition */ json_array_append_new(bridges, json_pack("[s,s]", "uuid", ovs_bridge->bridge_uuid)); + /* Loop over all bridge's ports */ for (pi = 0; pi < ovs_bridge->ports->len; pi++) { nm_auto_decref_json json_t *interfaces = NULL; nm_auto_decref_json json_t *new_interfaces = NULL; + gboolean interfaces_changed = FALSE; + int ii; interfaces = json_array(); new_interfaces = json_array(); port_uuid = g_ptr_array_index(ovs_bridge->ports, pi); ovs_port = g_hash_table_lookup(priv->ports, &port_uuid); + /* Add the port UUID to the list of known bridge port for the "expect" condition */ json_array_append_new(ports, json_pack("[s,s]", "uuid", port_uuid)); - interfaces_changed = FALSE; - if (!ovs_port) { /* This would be a violation of ovsdb's reference integrity (a bug). */ _LOGW("Unknown port '%s' in bridge '%s'", port_uuid, ovs_bridge->bridge_uuid); continue; } + /* Loop over all port's interfaces */ for (ii = 0; ii < ovs_port->interfaces->len; ii++) { interface_uuid = g_ptr_array_index(ovs_port->interfaces, ii); ovs_interface = g_hash_table_lookup(priv->interfaces, &interface_uuid); + /* Add the interface UUID to the list of known port interfaces for the "expect" condition */ json_array_append_new(interfaces, json_pack("[s,s]", "uuid", interface_uuid)); if (ovs_interface) { if (nm_streq(ovs_interface->name, ifname)) { - /* skip the interface */ + /* We are deleting this interface, don't count it */ interfaces_changed = TRUE; continue; } @@ -1430,32 +1433,42 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) _LOGW("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid); } + /* Add the interface to the list of new interfaces to set on the port */ json_array_append_new(new_interfaces, json_pack("[s,s]", "uuid", interface_uuid)); } if (json_array_size(new_interfaces) == 0) { + /* The port no longer has any interface. Don't add it to "new_ports" and set + * ports_changed=TRUE, so that it will be deleted. */ ports_changed = TRUE; } else { if (interfaces_changed) { + /* An interface needs to be deleted from this port */ _expect_port_interfaces(params, ovs_port->name, interfaces); _set_port_interfaces(params, ovs_port->name, new_interfaces); } + /* The port is still alive */ json_array_append_new(new_ports, json_pack("[s,s]", "uuid", port_uuid)); } } if (json_array_size(new_ports) == 0) { + /* The bridge no longer has any port. Don't add it to "new_bridges" and set + * bridges_changed=TRUE, so that it will be deleted. */ bridges_changed = TRUE; } else { if (ports_changed) { + /* A port needs to be deleted from this bridge */ _expect_bridge_ports(params, ovs_bridge->name, ports); _set_bridge_ports(params, ovs_bridge->name, new_ports); } + /* The bridge is still alive */ json_array_append_new(new_bridges, json_pack("[s,s]", "uuid", ovs_bridge->bridge_uuid)); } } if (bridges_changed) { + /* A port needs to be deleted from this bridge */ _expect_ovs_bridges(params, priv->db_uuid, bridges); _set_ovs_bridges(params, priv->db_uuid, new_bridges); } From db2f88bc732812eb97e589a3b76b6817bd132609 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 16 Apr 2025 15:16:13 +0200 Subject: [PATCH 2/2] ovs: only keep bridges and ports with NM interfaces attached If a OVS bridge created via NM has a port created externally, when the bridge connections goes down then NM detaches the NM-created port. However, it finds that the bridge still has a port (the external one) and so it doesn't remove the bridge from ovsdb. This is a problem, because it means that an explicity deactivation of the bridge leaves the bridge up. To fix this, only track the number of port in the bridge actually created by NM. Also, leave alone bridges not created by NM. (cherry picked from commit 476c89b6f2cd514fca8797bdc503eff60dc3db18) --- src/core/devices/ovs/nm-ovsdb.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/core/devices/ovs/nm-ovsdb.c b/src/core/devices/ovs/nm-ovsdb.c index 0ab7c05f8e..84467896d5 100644 --- a/src/core/devices/ovs/nm-ovsdb.c +++ b/src/core/devices/ovs/nm-ovsdb.c @@ -1384,19 +1384,27 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) while (g_hash_table_iter_next(&iter, (gpointer) &ovs_bridge, NULL)) { nm_auto_decref_json json_t *ports = NULL; nm_auto_decref_json json_t *new_ports = NULL; + guint num_nm_ports = 0; gboolean ports_changed = FALSE; int pi; - ports = json_array(); - new_ports = json_array(); + ports = json_array(); + new_ports = json_array(); /* Add the bridge UUID to the list of known bridges for the "expect" condition */ json_array_append_new(bridges, json_pack("[s,s]", "uuid", ovs_bridge->bridge_uuid)); + if (!ovs_bridge->connection_uuid) { + /* Externally created, don't touch it */ + json_array_append_new(new_bridges, json_pack("[s,s]", "uuid", ovs_bridge->bridge_uuid)); + continue; + } + /* Loop over all bridge's ports */ for (pi = 0; pi < ovs_bridge->ports->len; pi++) { - nm_auto_decref_json json_t *interfaces = NULL; - nm_auto_decref_json json_t *new_interfaces = NULL; + nm_auto_decref_json json_t *interfaces = NULL; + nm_auto_decref_json json_t *new_interfaces = NULL; + guint num_nm_interfaces = 0; gboolean interfaces_changed = FALSE; int ii; @@ -1428,6 +1436,8 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) interfaces_changed = TRUE; continue; } + if (ovs_interface->connection_uuid) + num_nm_interfaces++; } else { /* This would be a violation of ovsdb's reference integrity (a bug). */ _LOGW("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid); @@ -1437,8 +1447,8 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) json_array_append_new(new_interfaces, json_pack("[s,s]", "uuid", interface_uuid)); } - if (json_array_size(new_interfaces) == 0) { - /* The port no longer has any interface. Don't add it to "new_ports" and set + if (num_nm_interfaces == 0) { + /* The port no longer has any NM interface. Don't add it to "new_ports" and set * ports_changed=TRUE, so that it will be deleted. */ ports_changed = TRUE; } else { @@ -1449,11 +1459,13 @@ _delete_interface(NMOvsdb *self, json_t *params, const char *ifname) } /* The port is still alive */ json_array_append_new(new_ports, json_pack("[s,s]", "uuid", port_uuid)); + if (ovs_port->connection_uuid) + num_nm_ports++; } } - if (json_array_size(new_ports) == 0) { - /* The bridge no longer has any port. Don't add it to "new_bridges" and set + if (num_nm_ports == 0) { + /* The bridge no longer has any NM port. Don't add it to "new_bridges" and set * bridges_changed=TRUE, so that it will be deleted. */ bridges_changed = TRUE; } else {