On 9/18/26 9:23 AM, Jaygue Lee wrote:
> if_status_mgr_release_iface() only moves an interface to OIF_MARK_DOWN,
> and therefore only clears Port_Binding "up", for interfaces this
> ovn-controller instance claimed itself. Ports that were already bound to
> this chassis when ovn-controller started are adopted without being
> tracked, so when the recompute path releases one of them, for example
> after a hypervisor crash where the VM's tap is gone but the Port_Binding
> still points at this chassis, the chassis is cleared but "up" stays true.
>
> That leaves the port unbound but up, a state that cannot happen
> otherwise. Consumers that only look at "up", such as ovn-northd when it
> decides whether a load balancer service monitor may stay "online", keep
> treating the port as up although nothing probes the backend anymore, so
> traffic is still sent to it.
>
> Set "up" to false in release_lport() when the port is not tracked by
> if-status, matching what the tracked path does.
>
> CC: Dumitru Ceara <[email protected]>
> Fixes: 5c3371922994 ("if-status: Add OVS interface status management module.")
> Assisted-by: Claude Opus 5, Claude Code
> Signed-off-by: Jaygue Lee <[email protected]>
> ---
Hi Jaygue, Mairtin,
Thanks for the fix and review!
> controller/binding.c | 8 ++++++++
> tests/ovn-controller.at | 42 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index c2bce665e7..0a53fac5ce 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -1585,6 +1585,14 @@ release_lport(const struct sbrec_port_binding *pb,
> if (!release_lport_main_chassis(pb, sb_readonly, if_mgr)) {
> return false;
> }
> + /* A port claimed by a previous ovn-controller instance is not
> + * tracked by if-status, which would otherwise set it down. */
> + if (!sb_readonly && pb->n_up && pb->up[0] &&
> + !if_status_mgr_iface_is_present(if_mgr, pb->logical_port)) {
> + bool up = false;
> + sbrec_port_binding_set_up(pb, &up, 1);
> + VLOG_INFO("Setting lport %s down in Southbound",
> pb->logical_port);
> + }
I'm not sure I agree with the fix though. I think it belongs in
if_status_mgr_release_iface() doesn't it?
> } else if (is_additional_chassis(pb, chassis_rec)) {
> if (!release_lport_additional_chassis(pb, chassis_rec, sb_readonly))
> {
> return false;
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index a7b79fc670..1e0e865016 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -3070,6 +3070,48 @@ OVN_CLEANUP([hv1])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-controller - released untracked port is set down])
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 \
> +"00:00:00:00:00:01 10.0.0.1"
> +check ovs-vsctl -- add-port br-int hv1-vif1 -- \
> + set interface hv1-vif1 external-ids:iface-id=sw0-p1
> +hv1_uuid=$(fetch_column Chassis _uuid name=hv1)
> +wait_row_count Port_Binding 1 logical_port=sw0-p1 chassis=$hv1_uuid 'up=true'
> +wait_row_count nb:Logical_Switch_Port 1 name=sw0-p1 'up=true'
Nit: this should be "wait_for_ports_up sw0-p1" instead of the two lines
above.
> +
> +# Stop ovn-controller without releasing anything (as after a crash), remove
> +# the VIF while it is stopped (the VM did not come back) and start it again.
> +# The port is still bound to hv1 in the SB but this ovn-controller instance
> +# never claimed it, so if-status does not track it.
> +check ovn-appctl -t ovn-controller exit --restart
> +check ovs-vsctl del-port br-int hv1-vif1
> +start_daemon ovn-controller
> +
> +# The port must be released and set down.
> +wait_row_count Port_Binding 1 logical_port=sw0-p1 'chassis=[[]]' 'up=false'
> +wait_row_count nb:Logical_Switch_Port 1 name=sw0-p1 'up=false'
> +
> +# Adding the VIF back claims the port again as usual.
> +check ovs-vsctl -- add-port br-int hv1-vif1 -- \
> + set interface hv1-vif1 external-ids:iface-id=sw0-p1
> +wait_row_count Port_Binding 1 logical_port=sw0-p1 chassis=$hv1_uuid 'up=true'
> +wait_row_count nb:Logical_Switch_Port 1 name=sw0-p1 'up=true'
> +
> +OVN_CLEANUP([hv1
> +/Trying to release unknown interface sw0-p1/d])
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD([
> AT_SETUP([Encap enforce local_ip])
> ovn_start
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
