[ovs-dev] [PATCH ovn] binding: Fix potential crash when binding_seqno_run is skipped.
Dumitru Ceara
dceara at redhat.com
Thu Feb 18 16:42:58 UTC 2021
The prerequisite for binding_seqno_run() is that it has to be called
while there are valid SB and OVS transactions available.
ovn-controller's main() function respects that but that means that there
is a small window (when OVS transaction is still in progress) when port
bindings might be deleted from the Southbound.
When binding_seqno_run() was finally called, it was asserting that the
Port_bindings to still exist. This check is too restrictive and the
solution is to just skip bindings that have become partial (i.e., no
SB.Port_Binding or no OVS.Interface).
Fixes: 4d3cb42b076b ("binding: Set Logical_Switch_Port.up when all OVS flows are installed.")
Reported-at: https://bugzilla.redhat.com/1930030
Reported-by: Numan Siddique <numans at ovn.org>
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
Note: the window to reproduce the issue is quite narrow so I didn't
manage to write a test that would hit the issue consistently.
---
controller/binding.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index efaa109..2b19fd0 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2498,13 +2498,15 @@ binding_seqno_run(struct shash *local_bindings)
SSET_FOR_EACH_SAFE (iface_id, iface_id_next, &binding_iface_bound_set) {
struct shash_node *lb_node = shash_find(local_bindings, iface_id);
- if (lb_node) {
- /* This is a newly bound interface, make sure we reset the
- * Port_Binding 'up' field and the OVS Interface 'external-id'.
- */
- struct local_binding *lb = lb_node->data;
+ struct local_binding *lb = lb_node ? lb_node->data : NULL;
- ovs_assert(lb->pb && lb->iface);
+ /* Make sure the binding is still complete, i.e., both SB port_binding
+ * and OVS interface still exist.
+ *
+ * If so, then this is a newly bound interface, make sure we reset the
+ * Port_Binding 'up' field and the OVS Interface 'external-id'.
+ */
+ if (lb && lb->pb && lb->iface) {
new_ifaces = true;
if (smap_get(&lb->iface->external_ids, OVN_INSTALLED_EXT_ID)) {
--
1.8.3.1
More information about the dev
mailing list