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. 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]> --- 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 | 49 +++++++++++++++++++++++++++ tests/ovn.at | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) diff --git a/northd/northd.c b/northd/northd.c index 2cb69f9aa..daaa89a3c 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -4560,6 +4560,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, && (vector_len(&od->router_ports) == hmap_count(&od->ports))); struct ovn_port *op; + struct ovs_list exist_virtual_ports; + ovs_list_init(&exist_virtual_ports); HMAP_FOR_EACH (op, dp_node, &od->ports) { op->visited = false; } @@ -4629,6 +4631,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(&exist_virtual_ports, &op->list); } op->visited = true; } @@ -4675,6 +4679,51 @@ 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_ports; + sset_init(&created_ports); + + struct sset deleted_ports; + sset_init(&deleted_ports); + + HMAPX_FOR_EACH (hmapx_node, &trk_lsps->created) { + op = hmapx_node->data; + sset_add(&created_ports, op->nbsp->name); + } + + HMAPX_FOR_EACH (hmapx_node, &trk_lsps->deleted) { + op = hmapx_node->data; + sset_add(&deleted_ports, op->nbsp->name); + } + + LIST_FOR_EACH_POP (op, list, &exist_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_ports, vparent)) { + add_op_to_northd_tracked_ports(&trk_lsps->updated, op); + break; + } + if (sset_find(&deleted_ports, vparent)) { + add_op_to_northd_tracked_ports(&trk_lsps->updated, op); + break; + } + } + free(tokstr); + } + + sset_destroy(&created_ports); + sset_destroy(&deleted_ports); + return true; fail: diff --git a/tests/ovn.at b/tests/ovn.at index fadc62a76..44226a7ac 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -43651,3 +43651,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
