When incrementally processing changes, child Port Bindings (container and virtual ports) must be deleted first, before their parents, because they need to be removed from their parent's children hash to avoid parents with children with NULL port bindings.
Signed-off-by: Dumitru Ceara <[email protected]> --- controller/binding.c | 75 +++++++++++++++++++++++++++++++++++++++++++------- tests/ovn.at | 18 ++++++++++++ 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index ce092a0..d44f635 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -2183,13 +2183,26 @@ bool binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { - bool handled = true; + /* Run the tracked port binding loop twice to ensure correctness: + * 1. First to handle deleted changes. This is split in four sub-parts + * because child local bindings must be cleaned up first: + * a. Container ports first. + * b. Then virtual ports. + * c. Then regular VIFs. + * d. Last other ports. + * 2. Second to handle add/update changes. + */ + struct shash deleted_container_pbs = + SHASH_INITIALIZER(&deleted_container_pbs); + struct shash deleted_virtual_pbs = + SHASH_INITIALIZER(&deleted_virtual_pbs); + struct shash deleted_vif_pbs = + SHASH_INITIALIZER(&deleted_vif_pbs); + struct shash deleted_other_pbs = + SHASH_INITIALIZER(&deleted_other_pbs); const struct sbrec_port_binding *pb; + bool handled = true; - /* Run the tracked port binding loop twice. One to handle deleted - * changes. And another to handle add/update changes. - * This will ensure correctness. - */ SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, b_ctx_in->port_binding_table) { if (!sbrec_port_binding_is_deleted(pb)) { @@ -2197,18 +2210,60 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, } enum en_lport_type lport_type = get_lport_type(pb); - if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) { - handled = handle_deleted_vif_lport(pb, lport_type, b_ctx_in, - b_ctx_out); + + if (lport_type == LP_VIF) { + if (is_lport_container(pb)) { + shash_add(&deleted_container_pbs, pb->logical_port, pb); + } else { + shash_add(&deleted_vif_pbs, pb->logical_port, pb); + } + } else if (lport_type == LP_VIRTUAL) { + shash_add(&deleted_virtual_pbs, pb->logical_port, pb); } else { - handle_deleted_lport(pb, b_ctx_in, b_ctx_out); + shash_add(&deleted_other_pbs, pb->logical_port, pb); + } + } + + struct shash_node *node; + struct shash_node *node_next; + SHASH_FOR_EACH_SAFE (node, node_next, &deleted_container_pbs) { + handled = handle_deleted_vif_lport(node->data, LP_VIF, b_ctx_in, + b_ctx_out); + shash_delete(&deleted_container_pbs, node); + if (!handled) { + goto delete_done; } + } + SHASH_FOR_EACH_SAFE (node, node_next, &deleted_virtual_pbs) { + handled = handle_deleted_vif_lport(node->data, LP_VIRTUAL, b_ctx_in, + b_ctx_out); + shash_delete(&deleted_virtual_pbs, node); if (!handled) { - break; + goto delete_done; + } + } + + SHASH_FOR_EACH_SAFE (node, node_next, &deleted_vif_pbs) { + handled = handle_deleted_vif_lport(node->data, LP_VIF, b_ctx_in, + b_ctx_out); + shash_delete(&deleted_vif_pbs, node); + if (!handled) { + goto delete_done; } } + SHASH_FOR_EACH_SAFE (node, node_next, &deleted_other_pbs) { + handle_deleted_lport(node->data, b_ctx_in, b_ctx_out); + shash_delete(&deleted_other_pbs, node); + } + +delete_done: + shash_destroy(&deleted_container_pbs); + shash_destroy(&deleted_virtual_pbs); + shash_destroy(&deleted_vif_pbs); + shash_destroy(&deleted_other_pbs); + if (!handled) { return false; } diff --git a/tests/ovn.at b/tests/ovn.at index 6718326..8f88424 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22323,6 +22323,24 @@ check ovn-nbctl lsp-add ls2 lsp-cont1 lsp1 1 check ovn-nbctl --wait=hv sync check_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch +AS_BOX([delete both OVN VIF and OVN container port]) +as hv1 ovn-appctl -t ovn-controller debug/pause +check ovn-nbctl lsp-del lsp1 \ + -- lsp-del lsp-cont1 +check ovn-nbctl --wait=sb sync +as hv1 ovn-appctl -t ovn-controller debug/resume + +AS_BOX([readd both OVN VIF and OVN container port]) +as hv1 ovn-appctl -t ovn-controller debug/pause +check ovn-nbctl lsp-add ls1 lsp1 \ + -- lsp-add ls2 lsp-cont1 lsp1 1 +check ovn-nbctl --wait=sb sync +as hv1 ovn-appctl -t ovn-controller debug/resume + +check ovn-nbctl --wait=hv sync +wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch +wait_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch + OVN_CLEANUP([hv1]) AT_CLEANUP _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
