Even though ovn-controller tries to set port_binding->chassis to NULL every time port_binding->virtual_parent is set to NULL for bindings of type="virtual", there's no way to enforce that an operator doesn't manually clear the "virtual_parent" column in the Southbound database.
In such scenario ovn-controller would crash because of trying to dereference the NULL port_binding->virtual_parent column. Add an extra check and release "virtual" port bindings that have "virtual_parent" NULL. Reported-at: https://bugzilla.redhat.com/1818844 CC: Numan Siddique <nusid...@redhat.com> Fixes: 054f4c85c413 ("Add a new logical switch port type - 'virtual'") Signed-off-by: Dumitru Ceara <dce...@redhat.com> --- controller/binding.c | 26 +++++++++++++++----------- tests/ovn.at | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index c3376e2..5ea12a8 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -625,22 +625,26 @@ consider_local_virtual_port(struct ovsdb_idl_index *sbrec_port_binding_by_name, const struct sbrec_chassis *chassis_rec, const struct sbrec_port_binding *binding_rec) { + if (binding_rec->virtual_parent) { + const struct sbrec_port_binding *parent = + lport_lookup_by_name(sbrec_port_binding_by_name, + binding_rec->virtual_parent); + if (parent && parent->chassis == chassis_rec) { + return; + } + } + /* pinctrl module takes care of binding the ports of type 'virtual'. * Release such ports if their virtual parents are no longer claimed by * this chassis. */ - const struct sbrec_port_binding *parent = - lport_lookup_by_name(sbrec_port_binding_by_name, - binding_rec->virtual_parent); - if (!parent || parent->chassis != chassis_rec) { - VLOG_INFO("Releasing lport %s from this chassis.", - binding_rec->logical_port); - if (binding_rec->encap) { - sbrec_port_binding_set_encap(binding_rec, NULL); - } - sbrec_port_binding_set_chassis(binding_rec, NULL); - sbrec_port_binding_set_virtual_parent(binding_rec, NULL); + VLOG_INFO("Releasing lport %s from this chassis.", + binding_rec->logical_port); + if (binding_rec->encap) { + sbrec_port_binding_set_encap(binding_rec, NULL); } + sbrec_port_binding_set_chassis(binding_rec, NULL); + sbrec_port_binding_set_virtual_parent(binding_rec, NULL); } static void diff --git a/tests/ovn.at b/tests/ovn.at index a846e5c..0135838 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -15010,6 +15010,24 @@ AT_CHECK([cat lflows.txt], [0], [dnl table=12(lr_in_arp_resolve ), priority=100 , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 50:54:00:00:00:03; next;) ]) +# Forcibly clear virtual_parent. ovn-controller should release the binding +# gracefully. +pb_uuid=$(ovn-sbctl --bare --columns _uuid find port_binding logical_port=sw0-vir) +ovn-sbctl clear port_binding $pb_uuid virtual_parent + +OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \ +logical_port=sw0-vir) = x]) + +# From sw0-p0 resend GARP for 10.0.0.10. hv1 should reclaim sw0-vir +# and sw0-p1 should be its virtual_parent. +send_garp 1 1 $eth_src $eth_dst $spa $tpa + +OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \ +logical_port=sw0-vir) = x$hv1_ch_uuid], [0], []) + +AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \ +logical_port=sw0-vir) = xsw0-p1]) + # From sw0-p3 send GARP for 10.0.0.10. hv1 should claim sw0-vir # and sw0-p3 should be its virtual_parent. eth_src=505400000005 -- 1.8.3.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
