When the VIF plug provider's prepare callback returns false, meaning the port is not ready to be plugged, consider_plug_lport__() destroys its port context and returns true without setting *vif_plug_port_ctx_ptr. Both call sites in consider_plug_lport() then dereference the NULL context (vif_plug_port_ctx->vif_plug_port_ctx_out.name), crashing ovn-controller with a SIGSEGV.
This is easily reproduced with the representor provider when a requested VF cannot be resolved while the VF representors are being recreated, for example while the compute host is rebooting. Fix it by making consider_plug_lport() defer and return early when consider_plug_lport__() returns true without producing an output context, mirroring how consider_unplug_iface() already handles a false return from the provider's prepare callback. Add a vif-plug-dummy-return-false option to the dummy provider so tests can exercise this path, and a regression test verifying that ovn-controller defers plugging instead of crashing. Assisted-by: OpenCode (deepseek/deepseek-pro-latest) Signed-off-by: MJ Ponsonby <[email protected]> --- controller/vif-plug.c | 10 +++++ lib/vif-plug-providers/dummy/vif-plug-dummy.c | 7 +++ tests/ovn.at | 44 +++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/controller/vif-plug.c b/controller/vif-plug.c index 8a463f6e1..5aaf1af51 100644 --- a/controller/vif-plug.c +++ b/controller/vif-plug.c @@ -438,6 +438,11 @@ consider_plug_lport(const struct sbrec_port_binding *pb, vif_plug_ctx_in)) { return false; } + if (!vif_plug_port_ctx) { + /* The VIF plug provider asked to defer plugging/updating + * this lport, nothing to do this iteration. */ + return true; + } ret = consider_plug_lport_update__(vif_plug, &iface_external_ids, pb, lbinding->iface, @@ -450,6 +455,11 @@ consider_plug_lport(const struct sbrec_port_binding *pb, vif_plug_ctx_in)) { return false; } + if (!vif_plug_port_ctx) { + /* The VIF plug provider asked to defer plugging/updating + * this lport, nothing to do this iteration. */ + return true; + } const struct ovsrec_interface *iface_rec = iface_lookup_by_name(vif_plug_ctx_in->ovsrec_interface_by_name, diff --git a/lib/vif-plug-providers/dummy/vif-plug-dummy.c b/lib/vif-plug-providers/dummy/vif-plug-dummy.c index bc8953e98..23092cd6d 100644 --- a/lib/vif-plug-providers/dummy/vif-plug-dummy.c +++ b/lib/vif-plug-providers/dummy/vif-plug-dummy.c @@ -70,6 +70,13 @@ vif_plug_dummy_port_prepare(const struct vif_plug_port_ctx_in *ctx_in, VLOG_DBG("vif_plug_dummy_port_prepare: %s", ctx_in->lport_name); if (ctx_in->op_type == PLUG_OP_CREATE) { + const char *return_false = smap_get( + &ctx_in->lport_options, "vif-plug-dummy-return-false"); + if (return_false && !strcmp(return_false, "true")) { + VLOG_DBG("vif_plug_dummy_port_prepare: %s - returning false as " + "requested", ctx_in->lport_name); + return false; + } size_t lport_name_len = strlen(ctx_in->lport_name); ctx_out->name = xzalloc(IFNAMSIZ); memcpy(ctx_out->name, ctx_in->lport_name, diff --git a/tests/ovn.at b/tests/ovn.at index 780149467..2965c7dc0 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -44042,6 +44042,50 @@ OVN_CLEANUP([hv1]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller - vif-plug provider returns false]) +AT_KEYWORDS([vif-plug]) + +ovn_start + +net_add n1 +sim_add hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +check ovn-nbctl ls-add lsw0 + +check ovn-nbctl lsp-add lsw0 lsp1 +check ovn-nbctl lsp-set-addresses lsp1 "f0:00:00:00:00:01 172.16.0.101" + +# Ask the dummy provider to signal "not ready" from its prepare callback. +# ovn-controller must defer plugging and keep running instead of crashing. +check ovn-nbctl lsp-set-options lsp1 \ + requested-chassis=hv1 \ + vif-plug-type=dummy \ + vif-plug-dummy-return-false=true + +OVS_WAIT_UNTIL([ + grep -q "Not plugging/updating lport lsp1 on direction from VIF plug provider." hv1/ovn-controller.log +]) + +# The controller must still be alive and the port must not be plugged. +AT_CHECK([as hv1 ovs-appctl -t ovn-controller version >/dev/null], [0], [], [ignore]) +AT_CHECK([as hv1 ovs-vsctl get Interface lsp1 _uuid], [1], [], [ignore]) + +# Once the provider is ready again, the port gets plugged. +check ovn-nbctl lsp-set-options lsp1 \ + requested-chassis=hv1 \ + vif-plug-type=dummy + +OVS_WAIT_UNTIL([ + as hv1 ovs-vsctl get Interface lsp1 _uuid >/dev/null 2>&1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) + AT_SETUP([Patch ports not owned by OVN]) AT_KEYWORDS([ovn]) -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
