Recheck-request: github-robot-_ovn-kubernetes Rosemarie O'Riorden
Lowell, MA, United States [email protected] On Mon, Aug 11, 2025 at 5:13 PM Rosemarie O'Riorden <[email protected]> wrote: > Previously, if there was a logical switch with a virtual port that had a > virtual parent port configured, and then the parent port was deleted and > recreated, the parent port would not be reclaimed by the chassis and > thus would not behave as the parent port, breaking traffic. > > This is because virtual ports were not being updated if their parents > were recently created or deleted. > > So long as virtual ports have a parent port configured, there should be > no issue deleting and recreating the parent. It should continue working > as the parent as soon as it's up again. > > With this change, the parent port will function as the parent even after > deletion and recreation. The chassis will properly reclaim the virtual > port. > > Fixes: b337750e45be ("northd: Incremental processing of VIF changes in > 'northd' node.") > Reported-at: https://issues.redhat.com/browse/FDP-710 > Co-authored-by: Mohammad Heib <[email protected]> > Signed-off-by: Mohammad Heib <[email protected]> > Signed-off-by: Rosemarie O'Riorden <[email protected]> > --- > v4: > - Added Fixes tag. > - Combined two ssets for deleted and created ports into one. > - Nest looping through list under condition that sset is not empty. > - Only destory sset if not empty, otherwise unnecessary. > - Spacing, ordering, and other syntax nits. > > v3: > - Fixed algorithm to use an sset and loop through created ports, only > updating them if their virtual parent was recently created or > deleted. > - Added case for deleted parents, as just mentioned. > - Fixed send_garp() syntax in test and remove redefinition. > - Added wait for port binding to disappear in test. > - Added more virtual parents to test. > - Fixed various syntax issues/nits. > > v2: Fixed memory leak by freeing op_smap and tokstr. > --- > northd/northd.c | 45 +++++++++++++++++++++++++ > tests/ovn.at | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 134 insertions(+) > > diff --git a/northd/northd.c b/northd/northd.c > index 2cb69f9aa..742d39a72 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -4559,7 +4559,10 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn > *ovnsb_idl_txn, > bool ls_had_only_router_ports = (!vector_is_empty(&od->router_ports) > && (vector_len(&od->router_ports) == hmap_count(&od->ports))); > > + struct ovs_list existing_virtual_ports; > struct ovn_port *op; > + > + ovs_list_init(&existing_virtual_ports); > HMAP_FOR_EACH (op, dp_node, &od->ports) { > op->visited = false; > } > @@ -4629,6 +4632,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn > *ovnsb_idl_txn, > delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port, > od->tunnel_key, old_tunnel_key); > } > + } else if (!strcmp(op->nbsp->type, "virtual")) { > + ovs_list_push_back(&existing_virtual_ports, &op->list); > } > op->visited = true; > } > @@ -4675,6 +4680,46 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn > *ovnsb_idl_txn, > } > } > > + /* Update old virtual ports that have newly created or newly deleted > + * VIF as parent port. This code handles cases where the virtual port > was > + * created before the parent port or when the parent port was > recreated. > + */ > + struct hmapx_node *hmapx_node; > + > + struct sset created_or_deleted_ports; > + sset_init(&created_or_deleted_ports); > + > + HMAPX_FOR_EACH (hmapx_node, &trk_lsps->created) { > + op = hmapx_node->data; > + sset_add(&created_or_deleted_ports, op->nbsp->name); > + } > + > + HMAPX_FOR_EACH (hmapx_node, &trk_lsps->deleted) { > + op = hmapx_node->data; > + sset_add(&created_or_deleted_ports, op->nbsp->name); > + } > + > + if (!sset_is_empty(&created_or_deleted_ports)) { > + LIST_FOR_EACH_POP (op, list, &existing_virtual_ports) { > + const char *virtual_parents = > + smap_get_def(&op->nbsp->options, > + "virtual-parents", ""); > + char *tokstr = xstrdup(virtual_parents); > + char *save_ptr = NULL; > + char *vparent; > + > + for (vparent = strtok_r(tokstr, ",", &save_ptr); vparent != > NULL; > + vparent = strtok_r(NULL, ",", &save_ptr)) { > + if (sset_find(&created_or_deleted_ports, vparent)) { > + add_op_to_northd_tracked_ports(&trk_lsps->updated, > op); > + break; > + } > + } > + free(tokstr); > + } > + sset_destroy(&created_or_deleted_ports); > + } > + > return true; > > fail: > diff --git a/tests/ovn.at b/tests/ovn.at > index bcf352ab3..9e16ba4f1 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -43767,3 +43767,92 @@ AT_CHECK([grep -q "Postponing virtual lport > sw0-vir" hv3/ovn-controller.log], [1 > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([virtual port - parent port re-create]) > +AT_KEYWORDS([virtual ports]) > +ovn_start > + > +net_add n1 > + > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +check ovn-appctl vlog/set dbg > +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ > + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap \ > + ofport-request=1 > + > +check ovs-appctl -t ovn-controller vlog/set dbg > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lsp-add sw0 sw0-p1 > +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3 > 1000::3" > +check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3 > 10.0.0.10 1000::3" > + > +check ovn-nbctl lsp-add sw0 sw0-vir > +check ovn-nbctl lsp-set-addresses sw0-vir "50:54:00:00:00:10 10.0.0.10" > +check ovn-nbctl lsp-set-port-security sw0-vir "50:54:00:00:00:10 > 10.0.0.10" > +check ovn-nbctl lsp-set-type sw0-vir virtual > +check ovn-nbctl set logical_switch_port sw0-vir > options:virtual-ip=10.0.0.10 > +check ovn-nbctl set logical_switch_port sw0-vir > options:virtual-parents=sw0-p1,sw0-p2,sw0-p3,sw0-p4 > + > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > +hv1_ch_uuid=$(ovn-sbctl --bare --columns _uuid find chassis name="hv1") > + > +# From sw0-p1 send GARP for 10.0.0.10. hv1 should claim sw0-vir > +# and sw0-p1 should be its virtual_parent. > +eth_src=50:54:00:00:00:03 > +eth_dst=ff:ff:ff:ff:ff:ff > +spa=10.0.0.10 > +tpa=10.0.0.10 > +send_garp hv1 hv1-vif1 1 $eth_src $eth_dst $spa $tpa > + > +OVS_WAIT_UNTIL([test 1 = $(cat hv1/ovn-controller.log | \ > + grep "pinctrl received packet-in" | \ > + grep opcode=BIND_VPORT | \ > + grep OF_Table_ID=$(ovn-debug lflow-stage-to-oftable ls_in_arp_rsp) | \ > + wc -l)]) > + > +wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$hv1_ch_uuid > +check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1 > +wait_for_ports_up sw0-vir > + > +# Delete parent port. > +check ovn-nbctl lsp-del sw0-p1 > +check ovn-nbctl --wait=hv sync > +wait_row_count Port_Binding 0 logical_port=sw0-vir chassis=$hv1_ch_uuid > + > +# Re-add parent port. > +check ovn-nbctl lsp-add sw0 sw0-p1 > +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3 > 1000::3" > +check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3 > 10.0.0.10 1000::3" > + > +wait_for_ports_up > + > +# Must sleep for vport-backoff duration, otherwise ovn-controller will > +# increase the backoff and not claim the port. > +sleep_time=$(ovn-sbctl get Port_Binding sw0-vir options:vport-backoff) > +sleep $(echo "$sleep_time" | tr -d '"' | awk '{print $1/1000}') > + > +# From sw0-p1 send GARP for 10.0.0.10. hv1 should claim sw0-vir > +# and sw0-p1 should be its virtual_parent. > +send_garp hv1 hv1-vif1 1 $eth_src $eth_dst $spa $tpa > + > +OVS_WAIT_UNTIL([test 2 = $(cat hv1/ovn-controller.log | \ > + grep "pinctrl received packet-in" | \ > + grep opcode=BIND_VPORT | \ > + grep OF_Table_ID=$(ovn-debug lflow-stage-to-oftable ls_in_arp_rsp) | \ > + wc -l)]) > + > +wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$hv1_ch_uuid > +check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1 > +wait_for_ports_up sw0-vir > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > -- > 2.50.0 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
