On Wed, Nov 4, 2026 at 2:09 PM MJ Ponsonby via dev <[email protected]>
wrote:
> 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.
>
> Fixes: 820a711d3008 ("lib: Add infrastructure for VIF plug providers.")
> Assisted-by: DeepSeek V4 Pro, OpenCode
> Signed-off-by: MJ Ponsonby <[email protected]>
> ---
> controller/vif-plug.c | 6 +++
> lib/vif-plug-providers/dummy/vif-plug-dummy.c | 6 +++
> tests/ovn.at | 44 +++++++++++++++++++
> 3 files changed, 56 insertions(+)
>
> diff --git a/controller/vif-plug.c b/controller/vif-plug.c
> index 8a463f6e1..5a1c056c8 100644
> --- a/controller/vif-plug.c
> +++ b/controller/vif-plug.c
> @@ -438,6 +438,9 @@ consider_plug_lport(const struct sbrec_port_binding
> *pb,
> vif_plug_ctx_in)) {
> return false;
> }
> + if (!vif_plug_port_ctx) {
> + return true;
> + }
>
> ret = consider_plug_lport_update__(vif_plug,
> &iface_external_ids,
> pb, lbinding->iface,
> @@ -450,6 +453,9 @@ consider_plug_lport(const struct sbrec_port_binding
> *pb,
> vif_plug_ctx_in)) {
> return false;
> }
> + if (!vif_plug_port_ctx) {
> + 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..29acbc0e1 100644
> --- a/lib/vif-plug-providers/dummy/vif-plug-dummy.c
> +++ b/lib/vif-plug-providers/dummy/vif-plug-dummy.c
> @@ -70,6 +70,12 @@ 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) {
> + if (smap_get_bool(&ctx_in->lport_options,
> + "vif-plug-dummy-return-false", false)) {
> + 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
>
>
Thank you MJ,
applied to main and backported down to 26.03.
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
