When a logical_port is deleted and added back, in two sb transactions being handled within one ovn-controller loop, some flows belonging to the deleted pb (such as flows in OFTABLE_CHK_IN_PORT_SEC) were not properly deleted.
Reported-at: https://issues.redhat.com/browse/FDP-873 Signed-off-by: Xavier Simonart <[email protected]> --- controller/lflow.c | 3 +++ controller/local_data.c | 26 ++++++++++++++++++++++++-- tests/ovn.at | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 13c3a0d73..987de3f06 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -2287,6 +2287,9 @@ lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb, * port binding'uuid', then this function should handle it properly. */ ofctrl_remove_flows(l_ctx_out->flow_table, &pb->header_.uuid); + if (sbrec_port_binding_is_deleted(pb)) { + return true; + } if (pb->n_port_security && shash_find(l_ctx_in->binding_lports, pb->logical_port)) { diff --git a/controller/local_data.c b/controller/local_data.c index f889fb76b..9ee5d9171 100644 --- a/controller/local_data.c +++ b/controller/local_data.c @@ -359,15 +359,37 @@ tracked_datapath_lport_add(const struct sbrec_port_binding *pb, } /* Check if the lport is already present or not. - * If it is already present, then just update the 'pb' field. */ + * If it is already present, then check whether it is the same pb. + * We might have two different pb with the same logical_port if it was + * deleted and added back within the same loop. + * If the same pb was already present, just update the 'pb' field. + * Otherwise, add the second pb */ struct tracked_lport *lport = shash_find_data(&tracked_dp->lports, pb->logical_port); if (!lport) { lport = xmalloc(sizeof *lport); shash_add(&tracked_dp->lports, pb->logical_port, lport); + } else if (pb != lport->pb) { + bool found = false; + /* There is at least another pb with the same logical_port. + * However, our pb might already be shash_added (e.g. pb1 deleted, pb2 + * added, pb2 deleted). This is not really optimal, but this loop + * only runs in a very uncommon race condition (same logical port + * deleted and added within same loop */ + struct shash_node *node; + SHASH_FOR_EACH (node, &tracked_dp->lports) { + lport = (struct tracked_lport *) node->data; + if (lport->pb == pb) { + found = true; + break; + } + } + if (!found) { + lport = xmalloc(sizeof *lport); + shash_add(&tracked_dp->lports, pb->logical_port, lport); + } } - lport->pb = pb; lport->tracked_type = tracked_type; } diff --git a/tests/ovn.at b/tests/ovn.at index d7f01169c..4835612ce 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -39233,3 +39233,43 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([Port Deleted and added back]) +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 +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 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 2001::3" +check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 2001::3" + +OVN_POPULATE_ARP +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +# Delete sw0-p1 +sleep_controller hv1 +check ovn-nbctl --wait=sb lsp-del sw0-p1 + +# Add back sw0-p1 but without any address set. +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 +wake_up_controller hv1 + +CHECK_FLOWS_AFTER_RECOMPUTE([hv1], [hv1]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) -- 2.31.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
