On 3/29/21 3:21 PM, [email protected] wrote: > From: Numan Siddique <[email protected]> > > When a port binding type changes from type 'A' to type 'B', then > there are many code paths in the existing binding.c which results > in crashes due to use-after-free or NULL references. > > Below crashes are seen when a container lport is changed to a normal > lport and then deleted. > > *** > (gdb) bt > 0 in raise () from /lib64/libc.so.6 > 1 in abort () from /lib64/libc.so.6 > 2 ovs_abort_valist ("%s: assertion %s failed in %s()") at lib/util.c:419 > 3 vlog_abort_valist ("%s: assertion %s failed in %s()") at lib/vlog.c:1249 > 4 vlog_abort ("%s: assertion %s failed in %s()") at lib/vlog.c:1263 > 5 ovs_assert_failure (where="lib/ovsdb-idl.c:4653", > function="ovsdb_idl_txn_write__", > condition="row->new_datum != NULL") at lib/util.c:86 > 6 ovsdb_idl_txn_write__ () at lib/ovsdb-idl.c:4695 > 7 ovsdb_idl_txn_write_clone () at lib/ovsdb-idl.c:4757 > 8 sbrec_port_binding_set_chassis () at lib/ovn-sb-idl.c:25946 > 9 release_lport () at controller/binding.c:971 > 10 release_local_binding_children () at controller/binding.c:1039 > 11 release_local_binding () at controller/binding.c:1056 > 12 consider_iface_release (iface_rec=.. > iface_id="bb43e818-b2ee-4329-b67e-218556580056") at controller/binding.c:1880 > 13 binding_handle_ovs_interface_changes () at controller/binding.c:1998 > 14 runtime_data_ovs_interface_handler () at > controller/ovn-controller.c:1481 > 15 engine_compute () at lib/inc-proc-eng.c:306 > 16 engine_run_node () at lib/inc-proc-eng.c:352 > 17 engine_run () at lib/inc-proc-eng.c:377 > 18 main () at controller/ovn-controller.c:2826 > > The present code creates a 'struct local_binding' instance for a > container lport and adds this object to the parent local binding > children list. And if the container lport is changed to a normal > vif, then there is no way to access the local binding object created > earlier. This patch fixes these type of issues by refactoring the > 'local binding' code of binding.c. This patch now creates only one > instance of 'struct local_binding' for every OVS interface with > external_ids:iface-id set. A new structure 'struct binding_lport' is > added which is created for a VIF, container and virtual port bindings > and is stored in 'binding_lports' shash. 'struct local_binding' now > maintains a list of binding_lports which it maps to. > > When a container lport is changed to a normal lport, we now can > easily access the 'binding_lport' object of the container lport > fron the 'binding_lports' shash. > > A new debug unixctl command is added - debug/dump-local-bindings, > which dumps the local bindings stored by the ovn-controller. This > command is also used in the test cases to validate that ovn-controller > maintains proper local bindings. > > Reported-by: Dumitru Ceara <[email protected]> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1936328 > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1936331 > > Co-authored-by: Dumitru Ceara <[email protected]> > [[email protected] contributed to the test cases which helped in reproducing > the crashes.] > Signed-off-by: Dumitru Ceara <[email protected]> > Signed-off-by: Numan Siddique <[email protected]> > ---
Hi Numan, I think the changes are OK, I hope we didn't miss any cases though. As Mark mentioned on an older revision, the additional tests do give a certain level of confidence. I'd recommend removing the "co-authored-by" tag above as you've been adding and improving the tests a lot since we first hit this issue and since v1 was posted. That would also allow me to: Acked-by: Dumitru Ceara <[email protected]> I do have some minor nits/comments below, please have a look at them before pushing this change. Regards, Dumitru > v4 -> v5 > ---- > * Addressed review comments from Mark and Dumitru. > * Added a new debug unixctl command - debug/dump-local-bindings > * Added more test cases. > > v3 -> v4 > --- > * -> Addressed some of the review comments from Dumitru. > Added another test case to test the scenarion when a container > port changes its parent from one to another port. > > v2 -> v3 > ---- > * Fixed a crash seen when a container port is changed to normal port > and then deleted in the same transaction. Added the relevant test case > for this. > > v1 -> v2 > ---- > * Fixed a crash seen when 2 container ports are changed to normal ports > in the same transaction. Added the relevant test case for this. > > controller/binding.c | 1114 +++++++++++++++++++++++------------ > controller/binding.h | 36 +- > controller/ovn-controller.c | 44 +- > controller/physical.c | 13 +- > tests/ovn-controller.at | 80 +++ > tests/ovn.at | 640 ++++++++++++++++++++ > 6 files changed, 1517 insertions(+), 410 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 4e6c756969..b4dc6982c9 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -597,6 +597,23 @@ remove_local_lport_ids(const struct sbrec_port_binding > *pb, > } > } > > +/* Corresponds to each Port_Binding.type. */ > +enum en_lport_type { > + LP_UNKNOWN, > + LP_VIF, > + LP_CONTAINER, > + LP_PATCH, > + LP_L3GATEWAY, > + LP_LOCALNET, > + LP_LOCALPORT, > + LP_L2GATEWAY, > + LP_VTEP, > + LP_CHASSISREDIRECT, > + LP_VIRTUAL, > + LP_EXTERNAL, > + LP_REMOTE > +}; > + > /* Local bindings. binding.c module binds the logical port (represented by > * Port_Binding rows) and sets the 'chassis' column when it sees the > * OVS interface row (of type "" or "internal") with the > @@ -608,134 +625,180 @@ remove_local_lport_ids(const struct > sbrec_port_binding *pb, > * 'struct local_binding' is used. A shash of these local bindings is > * maintained with the 'external_ids:iface-id' as the key to the shash. > * > - * struct local_binding (defined in binding.h) has 3 main fields: > - * - type > - * - OVS interface row object > - * - Port_Binding row object > - * > - * An instance of 'struct local_binding' can be one of 3 types. > - * > - * BT_VIF: Represent a local binding for an OVS interface of > - * type "" or "internal" with the external_ids:iface-id > - * set. > - * > - * This can be a > - * * probable local binding - external_ids:iface-id is > - * set, but the corresponding Port_Binding row is not > - * created or is not visible to the local ovn-controller > - * instance. > - * > - * * a local binding - external_ids:iface-id is set and > - * which is already bound to the corresponding Port_Binding > - * row. > - * > - * It maintains a list of children > - * (of type BT_CONTAINER/BT_VIRTUAL) if any. > - * > - * BT_CONTAINER: Represents a local binding which has a parent of type > - * BT_VIF. Its Port_Binding row's 'parent' column is set to > - * its parent's Port_Binding. It shares the OVS interface > row > - * with the parent. > - * Each ovn-controller when it sees a container > Port_Binding, > - * it creates 'struct local_binding' for the parent > - * Port_Binding and for its even if the OVS interface row > for > - * the parent is not present. > - * > - * BT_VIRTUAL: Represents a local binding which has a parent of type BT_VIF. > - * Its Port_Binding type is "virtual" and it shares the OVS > - * interface row with the parent. > - * Port_Binding of type "virtual" is claimed by pinctrl module > - * when it sees the ARP packet from the parent's VIF. > - * > + * struct local_binding has 3 main fields: > + * - name : 'external_ids:iface-id' of the OVS interface (key). > + * - OVS interface row object. > + * - List of 'binding_lport' objects with the primary lport > + * in the front of the list (if present). > * > * An object of 'struct local_binding' is created: > - * - For each interface that has iface-id configured with the type - > BT_VIF. > - * > - * - For each container Port Binding (of type BT_CONTAINER) and its > - * parent Port_Binding (of type BT_VIF), no matter if > - * they are bound to this chassis i.e even if OVS interface row for the > - * parent is not present. > + * - For each interface that has external_ids:iface-id configured. > * > - * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its > parent > - * is bound to this chassis. > + * - For each port binding (also referred as lport) of type 'LP_VIF' > + * if it is a parent lport of container lports even if there is no > + * corresponding OVS interface. > */ > +struct local_binding { > + char *name; > + const struct ovsrec_interface *iface; > + struct ovs_list binding_lports; > +}; > > -static struct local_binding * > -local_binding_create(const char *name, const struct ovsrec_interface *iface, > - const struct sbrec_port_binding *pb, > - enum local_binding_type type) > -{ > - struct local_binding *lbinding = xzalloc(sizeof *lbinding); > - lbinding->name = xstrdup(name); > - lbinding->type = type; > - lbinding->pb = pb; > - lbinding->iface = iface; > - shash_init(&lbinding->children); > - return lbinding; > -} > - > -static void > -local_binding_add(struct shash *local_bindings, struct local_binding > *lbinding) > -{ > - shash_add(local_bindings, lbinding->name, lbinding); > -} > +/* This structure represents a logical port (or port binding) > + * which is associated with 'struct local_binding'. > + * > + * An instance of 'struct binding_lport' is created for a logical port > + * - If the OVS interface's iface-id corresponds to the logical port. > + * - If it is a container or virtual logical port and its parent > + * has a 'local binding'. > + * > + */ > +struct binding_lport { > + struct ovs_list list_node; /* Node in local_binding.binding_lports. */ > > -static void > -local_binding_destroy(struct local_binding *lbinding) > -{ > - local_bindings_destroy(&lbinding->children); > + char *name; > + const struct sbrec_port_binding *pb; > + struct local_binding *lbinding; > + enum en_lport_type type; > +}; > > - free(lbinding->name); > - free(lbinding); > -} > +static struct local_binding *local_binding_create( > + const char *name, const struct ovsrec_interface *); > +static void local_binding_add(struct shash *local_bindings, > + struct local_binding *); > +static struct local_binding *local_binding_find( > + struct shash *local_bindings, const char *name); > +static void local_binding_destroy(struct local_binding *, > + struct shash *binding_lports); > +static void local_binding_delete(struct local_binding *, > + struct shash *local_bindings, > + struct shash *binding_lports); > +static struct binding_lport *local_binding_add_lport( > + struct shash *binding_lports, > + struct local_binding *, > + const struct sbrec_port_binding *, > + enum en_lport_type); > +static struct binding_lport *local_binding_get_primary_lport( > + struct local_binding *); > +static bool local_binding_handle_stale_binding_lports( > + struct local_binding *lbinding, struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, struct hmap *qos_map); > + > +static struct binding_lport *binding_lport_create( > + const struct sbrec_port_binding *, > + struct local_binding *, enum en_lport_type); > +static void binding_lport_destroy(struct binding_lport *); > +static void binding_lport_delete(struct shash *binding_lports, > + struct binding_lport *); > +static void binding_lport_add(struct shash *binding_lports, > + struct binding_lport *); > +static struct binding_lport *binding_lport_find( > + struct shash *binding_lports, const char *lport_name); > +static const struct sbrec_port_binding *binding_lport_get_parent_pb( > + struct binding_lport *b_lprt); > +static struct binding_lport *binding_lport_check_and_cleanup( > + struct binding_lport *, struct shash *b_lports); > + > +static char *get_lport_type_str(enum en_lport_type lport_type); > > void > -local_bindings_init(struct shash *local_bindings) > +local_binding_data_init(struct local_binding_data *lbinding_data) > { > - shash_init(local_bindings); > + shash_init(&lbinding_data->bindings); > + shash_init(&lbinding_data->lports); > } > > void > -local_bindings_destroy(struct shash *local_bindings) > +local_binding_data_destroy(struct local_binding_data *lbinding_data) > { > struct shash_node *node, *next; > - SHASH_FOR_EACH_SAFE (node, next, local_bindings) { > + > + SHASH_FOR_EACH_SAFE (node, next, &lbinding_data->lports) { > + struct binding_lport *b_lport = node->data; > + binding_lport_destroy(b_lport); > + shash_delete(&lbinding_data->lports, node); > + } > + > + SHASH_FOR_EACH_SAFE (node, next, &lbinding_data->bindings) { > struct local_binding *lbinding = node->data; > - local_binding_destroy(lbinding); > - shash_delete(local_bindings, node); > + local_binding_destroy(lbinding, &lbinding_data->lports); > + shash_delete(&lbinding_data->bindings, node); > } > > - shash_destroy(local_bindings); > + shash_destroy(&lbinding_data->lports); > + shash_destroy(&lbinding_data->bindings); > } > > -static > -void local_binding_delete(struct shash *local_bindings, > - struct local_binding *lbinding) > +const struct sbrec_port_binding * > +local_binding_get_primary_pb(struct shash *local_bindings, const char > *pb_name) > { > - shash_find_and_delete(local_bindings, lbinding->name); > - local_binding_destroy(lbinding); > -} > + struct local_binding *lbinding = > + local_binding_find(local_bindings, pb_name); > + struct binding_lport *b_lport = > local_binding_get_primary_lport(lbinding); > > -static void > -local_binding_add_child(struct local_binding *lbinding, > - struct local_binding *child) > -{ > - local_binding_add(&lbinding->children, child); > - child->parent = lbinding; > + return b_lport ? b_lport->pb : NULL; > } > > -static struct local_binding * > -local_binding_find_child(struct local_binding *lbinding, > - const char *child_name) > +void > +binding_dump_local_bindings(struct local_binding_data *lbinding_data, > + struct ds *out_data) > { > - return local_binding_find(&lbinding->children, child_name); > -} > + const struct shash_node **nodes; > + size_t i, n; > > -static void > -local_binding_delete_child(struct local_binding *lbinding, > - struct local_binding *child) > -{ > - shash_find_and_delete(&lbinding->children, child->name); > + nodes = shash_sort(&lbinding_data->bindings); > + n = shash_count(&lbinding_data->bindings); Nit: size_t n = shash_count(&lbinding_data->bindings); > + > + ds_put_cstr(out_data, "Local bindings:\n"); > + for (i = 0; i < n; i++) { Nit: 'i' could be defined here instead of at the top of the function. > + const struct shash_node *node = nodes[i]; > + struct local_binding *lbinding = node->data; > + size_t num_lports = ovs_list_size(&lbinding->binding_lports); > + ds_put_format(out_data, "name: [%s], OVS interface name : [%s], " > + "num binding lports : [%"PRIuSIZE"]\n", > + lbinding->name, > + lbinding->iface ? lbinding->iface->name : "NULL", > + num_lports); > + > + if (num_lports) { > + struct binding_lport *b_lport; > + size_t j = 0; > + size_t num_c_lports = 0; > + struct shash child_lports = SHASH_INITIALIZER(&child_lports); > + bool primary_lport = false; Tiny nit: we could use reverse xmas tree here :) Also, we don't need num_c_lports, we can just use shash_is_empty() and shash_count(). > + LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { > + if (j == 0 && b_lport->type == LP_VIF) { > + ds_put_format(out_data, "primary lport : [%s]\n", > + b_lport->name); > + primary_lport = true; Would it make the code simpler if 'primary_lport' would actually be a 'struct binding_lport *' pointing to the first list element, if that's of type LP_VIF? We could then display the primary port information in a single place, below, where we now check "if (!primary_lport)". > + } else { > + shash_add(&child_lports, b_lport->name, b_lport); > + num_c_lports++; > + } > + j++; > + } > + > + if (!primary_lport) { > + ds_put_format(out_data, "no primary lport\n"); > + } > + if (num_c_lports) { > + const struct shash_node **c_nodes = > + shash_sort(&child_lports); > + for (j = 0; j < num_c_lports; j++) { > + b_lport = c_nodes[j]->data; > + ds_put_format(out_data, "child lport[%"PRIuSIZE"] : > [%s], " > + "type : [%s]\n", j + 1, b_lport->name, > + get_lport_type_str(b_lport->type)); > + } > + free(c_nodes); > + } > + shash_destroy(&child_lports); > + } > + > + ds_put_cstr(out_data, "----------------------------------------\n"); > + } > + > + free(nodes); > } > > static bool > @@ -744,12 +807,6 @@ is_lport_vif(const struct sbrec_port_binding *pb) > return !pb->type[0]; > } > > -static bool > -is_lport_container(const struct sbrec_port_binding *pb) > -{ > - return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0]; > -} > - > static struct tracked_binding_datapath * > tracked_binding_datapath_create(const struct sbrec_datapath_binding *dp, > bool is_new, > @@ -818,26 +875,13 @@ binding_tracked_dp_destroy(struct hmap > *tracked_datapaths) > hmap_destroy(tracked_datapaths); > } > > -/* Corresponds to each Port_Binding.type. */ > -enum en_lport_type { > - LP_UNKNOWN, > - LP_VIF, > - LP_PATCH, > - LP_L3GATEWAY, > - LP_LOCALNET, > - LP_LOCALPORT, > - LP_L2GATEWAY, > - LP_VTEP, > - LP_CHASSISREDIRECT, > - LP_VIRTUAL, > - LP_EXTERNAL, > - LP_REMOTE > -}; > - > static enum en_lport_type > get_lport_type(const struct sbrec_port_binding *pb) > { > if (is_lport_vif(pb)) { > + if (pb->parent_port && pb->parent_port[0]) { > + return LP_CONTAINER; > + } > return LP_VIF; > } else if (!strcmp(pb->type, "patch")) { > return LP_PATCH; > @@ -864,6 +908,41 @@ get_lport_type(const struct sbrec_port_binding *pb) > return LP_UNKNOWN; > } > > +static char * > +get_lport_type_str(enum en_lport_type lport_type) > +{ > + switch (lport_type) { > + case LP_VIF: > + return "VIF"; > + case LP_CONTAINER: > + return "CONTAINER"; > + case LP_VIRTUAL: > + return "VIRTUAL"; > + case LP_PATCH: > + return "PATCH"; > + case LP_CHASSISREDIRECT: > + return "CHASSISREDIRECT"; > + case LP_L3GATEWAY: > + return "L3GATEWAT"; > + case LP_LOCALNET: > + return "PATCH"; > + case LP_LOCALPORT: > + return "LOCALPORT"; > + case LP_L2GATEWAY: > + return "L2GATEWAY"; > + case LP_EXTERNAL: > + return "EXTERNAL"; > + case LP_REMOTE: > + return "REMOTE"; > + case LP_VTEP: > + return "VTEP"; > + case LP_UNKNOWN: > + return "UNKNOWN"; > + } > + > + OVS_NOT_REACHED(); > +} > + > /* For newly claimed ports, if 'notify_up' is 'false': > * - set the 'pb.up' field to true if 'pb' has no 'parent_pb'. > * - set the 'pb.up' field to true if 'parent_pb.up' is 'true' (e.g., for > @@ -991,14 +1070,15 @@ release_lport(const struct sbrec_port_binding *pb, > bool sb_readonly, > static bool > is_lbinding_set(struct local_binding *lbinding) > { > - return lbinding && lbinding->pb && lbinding->iface; > + return lbinding && lbinding->iface; > } > > static bool > -is_lbinding_this_chassis(struct local_binding *lbinding, > - const struct sbrec_chassis *chassis) > +is_binding_lport_this_chassis(struct binding_lport *b_lport, > + const struct sbrec_chassis *chassis) > { > - return lbinding && lbinding->pb && lbinding->pb->chassis == chassis; > + return (b_lport && b_lport->pb && chassis && > + b_lport->pb->chassis == chassis); > } > > static bool > @@ -1010,15 +1090,14 @@ can_bind_on_this_chassis(const struct sbrec_chassis > *chassis_rec, > || !strcmp(requested_chassis, chassis_rec->hostname); > } > > -/* Returns 'true' if the 'lbinding' has children of type BT_CONTAINER, > +/* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER, > * 'false' otherwise. */ > static bool > is_lbinding_container_parent(struct local_binding *lbinding) > { > - struct shash_node *node; > - SHASH_FOR_EACH (node, &lbinding->children) { > - struct local_binding *l = node->data; > - if (l->type == BT_CONTAINER) { > + struct binding_lport *b_lport; > + LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { > + if (b_lport->type == LP_CONTAINER) { > return true; > } > } > @@ -1027,66 +1106,39 @@ is_lbinding_container_parent(struct local_binding > *lbinding) > } > > static bool > -release_local_binding_children(const struct sbrec_chassis *chassis_rec, > - struct local_binding *lbinding, > - bool sb_readonly, > - struct hmap *tracked_dp_bindings) > +release_binding_lport(const struct sbrec_chassis *chassis_rec, > + struct binding_lport *b_lport, bool sb_readonly, > + struct hmap *tracked_dp_bindings) > { > - struct shash_node *node; > - SHASH_FOR_EACH (node, &lbinding->children) { > - struct local_binding *l = node->data; > - if (is_lbinding_this_chassis(l, chassis_rec)) { > - if (!release_lport(l->pb, sb_readonly, tracked_dp_bindings)) { > - return false; > - } > + if (is_binding_lport_this_chassis(b_lport, chassis_rec)) { > + if (!release_lport(b_lport->pb, sb_readonly, tracked_dp_bindings)) { > + return false; > } > - > - /* Clear the local bindings' 'iface'. */ > - l->iface = NULL; > } > > return true; > } > > -static bool > -release_local_binding(const struct sbrec_chassis *chassis_rec, > - struct local_binding *lbinding, bool sb_readonly, > - struct hmap *tracked_dp_bindings) > -{ > - if (!release_local_binding_children(chassis_rec, lbinding, > - sb_readonly, tracked_dp_bindings)) { > - return false; > - } > - > - bool retval = true; > - if (is_lbinding_this_chassis(lbinding, chassis_rec)) { > - retval = release_lport(lbinding->pb, sb_readonly, > tracked_dp_bindings); > - } > - > - lbinding->pb = NULL; > - lbinding->iface = NULL; > - return retval; > -} > - > static bool > consider_vif_lport_(const struct sbrec_port_binding *pb, > bool can_bind, const char *vif_chassis, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out, > - struct local_binding *lbinding, > + struct binding_lport *b_lport, > struct hmap *qos_map) > { > - bool lbinding_set = is_lbinding_set(lbinding); > + bool lbinding_set = b_lport && is_lbinding_set(b_lport->lbinding); > + > if (lbinding_set) { > if (can_bind) { > /* We can claim the lport. */ > const struct sbrec_port_binding *parent_pb = > - lbinding->parent ? lbinding->parent->pb : NULL; > + binding_lport_get_parent_pb(b_lport); > > if (!claim_lport(pb, parent_pb, b_ctx_in->chassis_rec, > - lbinding->iface, !b_ctx_in->ovnsb_idl_txn, > - !lbinding->parent, > - b_ctx_out->tracked_dp_bindings)){ > + b_lport->lbinding->iface, > + !b_ctx_in->ovnsb_idl_txn, > + !parent_pb, b_ctx_out->tracked_dp_bindings)){ > return false; > } > > @@ -1098,7 +1150,7 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, > b_ctx_out->tracked_dp_bindings); > update_local_lport_ids(pb, b_ctx_out); > update_local_lports(pb->logical_port, b_ctx_out); > - if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) { > + if (b_lport->lbinding->iface && qos_map && > b_ctx_in->ovs_idl_txn) { > get_qos_params(pb, qos_map); > } > } else { > @@ -1136,16 +1188,19 @@ consider_vif_lport(const struct sbrec_port_binding > *pb, > vif_chassis); > > if (!lbinding) { > - lbinding = local_binding_find(b_ctx_out->local_bindings, > + lbinding = local_binding_find(&b_ctx_out->lbinding_data->bindings, > pb->logical_port); > } > > + struct binding_lport *b_lport = NULL; > if (lbinding) { > - lbinding->pb = pb; > + struct shash *binding_lports = > + &b_ctx_out->lbinding_data->lports; > + b_lport = local_binding_add_lport(binding_lports, lbinding, pb, > LP_VIF); > } > > return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, > - b_ctx_out, lbinding, qos_map); > + b_ctx_out, b_lport, qos_map); > } > > static bool > @@ -1154,9 +1209,9 @@ consider_container_lport(const struct > sbrec_port_binding *pb, > struct binding_ctx_out *b_ctx_out, > struct hmap *qos_map) > { > + struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings; > struct local_binding *parent_lbinding; > - parent_lbinding = local_binding_find(b_ctx_out->local_bindings, > - pb->parent_port); > + parent_lbinding = local_binding_find(local_bindings, pb->parent_port); > > if (!parent_lbinding) { > /* There is no local_binding for parent port. Create it > @@ -1171,54 +1226,61 @@ consider_container_lport(const struct > sbrec_port_binding *pb, > * we want the these container ports also be claimed by the > * chassis. > * */ > - parent_lbinding = local_binding_create(pb->parent_port, NULL, NULL, > - BT_VIF); > - local_binding_add(b_ctx_out->local_bindings, parent_lbinding); > + parent_lbinding = local_binding_create(pb->parent_port, NULL); > + local_binding_add(local_bindings, parent_lbinding); > } > > - struct local_binding *container_lbinding = > - local_binding_find_child(parent_lbinding, pb->logical_port); > + struct shash *binding_lports = &b_ctx_out->lbinding_data->lports; > + struct binding_lport *container_b_lport = > + local_binding_add_lport(binding_lports, parent_lbinding, pb, > + LP_CONTAINER); > > - if (!container_lbinding) { > - container_lbinding = local_binding_create(pb->logical_port, > - parent_lbinding->iface, > - pb, BT_CONTAINER); > - local_binding_add_child(parent_lbinding, container_lbinding); > - } else { > - ovs_assert(container_lbinding->type == BT_CONTAINER); > - container_lbinding->pb = pb; > - container_lbinding->iface = parent_lbinding->iface; > - } > + struct binding_lport *parent_b_lport = > + binding_lport_find(binding_lports, pb->parent_port); > > - if (!parent_lbinding->pb) { > - parent_lbinding->pb = lport_lookup_by_name( > + bool can_consider_c_lport = true; > + if (!parent_b_lport || !parent_b_lport->pb) { > + const struct sbrec_port_binding *parent_pb = lport_lookup_by_name( > b_ctx_in->sbrec_port_binding_by_name, pb->parent_port); > > - if (parent_lbinding->pb) { > + if (parent_pb && get_lport_type(parent_pb) == LP_VIF) { > /* Its possible that the parent lport is not considered yet. > * So call consider_vif_lport() to process it first. */ > - consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, > + consider_vif_lport(parent_pb, b_ctx_in, b_ctx_out, > parent_lbinding, qos_map); > + parent_b_lport = binding_lport_find(binding_lports, > + pb->parent_port); > } else { > - /* The parent lport doesn't exist. Call release_lport() to > - * release the container lport, if it was bound earlier. */ > - if (is_lbinding_this_chassis(container_lbinding, > - b_ctx_in->chassis_rec)) { > - return release_lport(pb, !b_ctx_in->ovnsb_idl_txn, > - b_ctx_out->tracked_dp_bindings); > - } > + /* The parent lport doesn't exist. Cannot consider the container > + * lport for binding. */ > + can_consider_c_lport = false; > + } > + } > > - return true; > + if (parent_b_lport && parent_b_lport->type != LP_VIF) { > + can_consider_c_lport = false; > + } > + > + if (!can_consider_c_lport) { > + /* Call release_lport() to release the container lport, > + * if it was bound earlier. */ > + if (is_binding_lport_this_chassis(container_b_lport, > + b_ctx_in->chassis_rec)) { > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn, > + b_ctx_out->tracked_dp_bindings); > } > + > + return true; > } > > - const char *vif_chassis = smap_get(&parent_lbinding->pb->options, > + ovs_assert(parent_b_lport && parent_b_lport->pb); > + const char *vif_chassis = smap_get(&parent_b_lport->pb->options, > "requested-chassis"); > bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, > vif_chassis); > > return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, > b_ctx_out, > - container_lbinding, qos_map); > + container_b_lport, qos_map); > } > > static bool > @@ -1227,46 +1289,47 @@ consider_virtual_lport(const struct > sbrec_port_binding *pb, > struct binding_ctx_out *b_ctx_out, > struct hmap *qos_map) > { > - struct local_binding * parent_lbinding = > - pb->virtual_parent ? local_binding_find(b_ctx_out->local_bindings, > + struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings; > + struct local_binding *parent_lbinding = > + pb->virtual_parent ? local_binding_find(local_bindings, > pb->virtual_parent) > : NULL; > > - if (parent_lbinding && !parent_lbinding->pb) { > - parent_lbinding->pb = lport_lookup_by_name( > - b_ctx_in->sbrec_port_binding_by_name, pb->virtual_parent); > - > - if (parent_lbinding->pb) { > - /* Its possible that the parent lport is not considered yet. > - * So call consider_vif_lport() to process it first. */ > - consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, > - parent_lbinding, qos_map); > - } > - } > - > + struct binding_lport *virtual_b_lport = NULL; > /* Unlike container lports, we don't have to create parent_lbinding if > * it is NULL. This is because, if parent_lbinding is not present, it > * means the virtual port can't bind in this chassis. > * Note: pinctrl module binds the virtual lport when it sees ARP > * packet from the parent lport. */ > - struct local_binding *virtual_lbinding = NULL; > - if (is_lbinding_this_chassis(parent_lbinding, b_ctx_in->chassis_rec)) { > - virtual_lbinding = > - local_binding_find_child(parent_lbinding, pb->logical_port); > - if (!virtual_lbinding) { > - virtual_lbinding = local_binding_create(pb->logical_port, > - parent_lbinding->iface, > - pb, BT_VIRTUAL); > - local_binding_add_child(parent_lbinding, virtual_lbinding); > - } else { > - ovs_assert(virtual_lbinding->type == BT_VIRTUAL); > - virtual_lbinding->pb = pb; > - virtual_lbinding->iface = parent_lbinding->iface; > + if (parent_lbinding) { > + struct shash *binding_lports = &b_ctx_out->lbinding_data->lports; > + > + struct binding_lport *parent_b_lport = > + binding_lport_find(binding_lports, pb->virtual_parent); > + > + if (!parent_b_lport || !parent_b_lport->pb) { > + const struct sbrec_port_binding *parent_pb = > lport_lookup_by_name( > + b_ctx_in->sbrec_port_binding_by_name, pb->virtual_parent); > + > + if (parent_pb && get_lport_type(parent_pb) == LP_VIF) { > + /* Its possible that the parent lport is not considered yet. > + * So call consider_vif_lport() to process it first. */ > + consider_vif_lport(parent_pb, b_ctx_in, b_ctx_out, > + parent_lbinding, qos_map); > + } > + } > + > + parent_b_lport = local_binding_get_primary_lport(parent_lbinding); > + if (is_binding_lport_this_chassis(parent_b_lport, > + b_ctx_in->chassis_rec)) { > + virtual_b_lport = > + local_binding_add_lport(binding_lports, parent_lbinding, pb, > + LP_VIRTUAL); > } > } > > return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, > - virtual_lbinding, qos_map); > + virtual_b_lport, qos_map); > } > > /* Considers either claiming the lport or releasing the lport > @@ -1407,6 +1470,8 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, > continue; > } > > + struct shash *local_bindings = > + &b_ctx_out->lbinding_data->bindings; > for (j = 0; j < port_rec->n_interfaces; j++) { > const struct ovsrec_interface *iface_rec; > > @@ -1416,11 +1481,10 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, > > if (iface_id && ofport > 0) { > struct local_binding *lbinding = > - local_binding_find(b_ctx_out->local_bindings, iface_id); > + local_binding_find(local_bindings, iface_id); > if (!lbinding) { > - lbinding = local_binding_create(iface_id, iface_rec, > NULL, > - BT_VIF); > - local_binding_add(b_ctx_out->local_bindings, lbinding); > + lbinding = local_binding_create(iface_id, iface_rec); > + local_binding_add(local_bindings, lbinding); > } else { > static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(1, 5); > @@ -1431,7 +1495,6 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, > "configuration on interface [%s]", > lbinding->iface->name, iface_rec->name, > iface_rec->name); > - ovs_assert(lbinding->type == BT_VIF); > } > > update_local_lports(iface_id, b_ctx_out); > @@ -1494,11 +1557,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct > binding_ctx_out *b_ctx_out) > break; > > case LP_VIF: > - if (is_lport_container(pb)) { > - consider_container_lport(pb, b_ctx_in, b_ctx_out, > qos_map_ptr); > - } else { > - consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, > qos_map_ptr); > - } > + consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, qos_map_ptr); > + break; > + > + case LP_CONTAINER: > + consider_container_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); > break; > > case LP_VIRTUAL: > @@ -1799,39 +1862,45 @@ consider_iface_claim(const struct ovsrec_interface > *iface_rec, > update_local_lports(iface_id, b_ctx_out); > smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); > > - struct local_binding *lbinding = > - local_binding_find(b_ctx_out->local_bindings, iface_id); > + struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings; > + struct shash *binding_lports = &b_ctx_out->lbinding_data->lports; > + struct local_binding *lbinding = local_binding_find(local_bindings, > + iface_id); > > if (!lbinding) { > - lbinding = local_binding_create(iface_id, iface_rec, NULL, BT_VIF); > - local_binding_add(b_ctx_out->local_bindings, lbinding); > + lbinding = local_binding_create(iface_id, iface_rec); > + local_binding_add(local_bindings, lbinding); > } else { > lbinding->iface = iface_rec; > } > > - if (!lbinding->pb || strcmp(lbinding->name, lbinding->pb->logical_port)) > { > - lbinding->pb = lport_lookup_by_name( > - b_ctx_in->sbrec_port_binding_by_name, lbinding->name); > - if (lbinding->pb && !strcmp(lbinding->pb->type, "virtual")) { > - lbinding->pb = NULL; > + struct binding_lport *b_lport = > local_binding_get_primary_lport(lbinding); > + const struct sbrec_port_binding *pb = NULL; > + if (!b_lport) { > + pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, > + lbinding->name); > + if (pb && get_lport_type(pb) == LP_VIF) { > + b_lport = local_binding_add_lport(binding_lports, lbinding, pb, > + LP_VIF); > } > } > > - if (lbinding->pb) { > - if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out, > - lbinding, qos_map)) { > - return false; > - } > + if (!b_lport) { > + /* There is no binding lport for this local binding. */ > + return true; > + } > + > + if (!consider_vif_lport(b_lport->pb, b_ctx_in, b_ctx_out, > + lbinding, qos_map)) { > + return false; > } > > + Nit: extra line not needed. > /* Update the child local_binding's iface (if any children) and try to > * claim the container lbindings. */ > - struct shash_node *node; > - SHASH_FOR_EACH (node, &lbinding->children) { > - struct local_binding *child = node->data; > - child->iface = iface_rec; > - if (child->type == BT_CONTAINER) { > - if (!consider_container_lport(child->pb, b_ctx_in, b_ctx_out, > + LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { > + if (b_lport->type == LP_CONTAINER) { > + if (!consider_container_lport(b_lport->pb, b_ctx_in, b_ctx_out, > qos_map)) { > return false; > } > @@ -1862,32 +1931,42 @@ consider_iface_release(const struct ovsrec_interface > *iface_rec, > struct binding_ctx_out *b_ctx_out) > { > struct local_binding *lbinding; > - lbinding = local_binding_find(b_ctx_out->local_bindings, > - iface_id); > - if (is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec)) { > + struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings; > + struct shash *binding_lports = &b_ctx_out->lbinding_data->lports; > + > + lbinding = local_binding_find(local_bindings, iface_id); > + struct binding_lport *b_lport = > local_binding_get_primary_lport(lbinding); > + if (is_binding_lport_this_chassis(b_lport, b_ctx_in->chassis_rec)) { > struct local_datapath *ld = > get_local_datapath(b_ctx_out->local_datapaths, > - lbinding->pb->datapath->tunnel_key); > + b_lport->pb->datapath->tunnel_key); > if (ld) { > - remove_pb_from_local_datapath(lbinding->pb, > - b_ctx_in->chassis_rec, > - b_ctx_out, ld); > + remove_pb_from_local_datapath(b_lport->pb, > + b_ctx_in->chassis_rec, > + b_ctx_out, ld); > } > > - /* Note: release_local_binding() resets lbinding->pb and > - * lbinding->iface. > - * Cannot access these members of lbinding after this call. */ > - if (!release_local_binding(b_ctx_in->chassis_rec, lbinding, > - !b_ctx_in->ovnsb_idl_txn, > - b_ctx_out->tracked_dp_bindings)) { > - return false; > + /* Release the primary binding lport and other children lports if > + * any. */ > + LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { > + if (!release_binding_lport(b_ctx_in->chassis_rec, b_lport, > + !b_ctx_in->ovnsb_idl_txn, > + b_ctx_out->tracked_dp_bindings)) { > + return false; > + } > } > + > + } > + > + if (lbinding) { > + /* Clear the iface of the local binding. */ > + lbinding->iface = NULL; > } > > /* Check if the lbinding has children of type PB_CONTAINER. > * If so, don't delete the local_binding. */ > if (lbinding && !is_lbinding_container_parent(lbinding)) { > - local_binding_delete(b_ctx_out->local_bindings, lbinding); > + local_binding_delete(lbinding, local_bindings, binding_lports); > } > > remove_local_lports(iface_id, b_ctx_out); > @@ -2088,56 +2167,35 @@ handle_deleted_lport(const struct sbrec_port_binding > *pb, > } > } > > -static struct local_binding * > -get_lbinding_for_lport(const struct sbrec_port_binding *pb, > - enum en_lport_type lport_type, > - struct binding_ctx_out *b_ctx_out) > -{ > - ovs_assert(lport_type == LP_VIF || lport_type == LP_VIRTUAL); > - > - if (lport_type == LP_VIF && !is_lport_container(pb)) { > - return local_binding_find(b_ctx_out->local_bindings, > pb->logical_port); > - } > - > - struct local_binding *parent_lbinding = NULL; > - > - if (lport_type == LP_VIRTUAL) { > - if (pb->virtual_parent) { > - parent_lbinding = local_binding_find(b_ctx_out->local_bindings, > - pb->virtual_parent); > - } > - } else { > - if (pb->parent_port) { > - parent_lbinding = local_binding_find(b_ctx_out->local_bindings, > - pb->parent_port); > - } > - } > - > - return parent_lbinding > - ? local_binding_find(&parent_lbinding->children, pb->logical_port) > - : NULL; > -} > - > static bool > handle_deleted_vif_lport(const struct sbrec_port_binding *pb, > enum en_lport_type lport_type, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > { > - struct local_binding *lbinding = > - get_lbinding_for_lport(pb, lport_type, b_ctx_out); > + struct local_binding *lbinding = NULL; > + bool bound = false; > > - if (lbinding) { > - lbinding->pb = NULL; > - /* The port_binding 'pb' is deleted. So there is no need to > - * clear the 'chassis' column of 'pb'. But we need to do > - * for the local_binding's children. */ > - if (lbinding->type == BT_VIF && > - !release_local_binding_children( > - b_ctx_in->chassis_rec, lbinding, > - !b_ctx_in->ovnsb_idl_txn, > - b_ctx_out->tracked_dp_bindings)) { > - return false; > + struct shash *binding_lports = &b_ctx_out->lbinding_data->lports; > + struct binding_lport *b_lport = binding_lport_find(binding_lports, > pb->logical_port); > + if (b_lport) { > + lbinding = b_lport->lbinding; > + bound = is_binding_lport_this_chassis(b_lport, > b_ctx_in->chassis_rec); > + > + /* Remove b_lport from local_binding. */ > + binding_lport_delete(binding_lports, b_lport); > + } > + > + if (bound && lbinding && lport_type == LP_VIF) { > + /* We need to release the container/virtual binding lports (if any) > if > + * deleted 'pb' type is LP_VIF. */ > + struct binding_lport *c_lport; > + LIST_FOR_EACH (c_lport, list_node, &lbinding->binding_lports) { > + if (!release_binding_lport(b_ctx_in->chassis_rec, c_lport, > + !b_ctx_in->ovnsb_idl_txn, > + b_ctx_out->tracked_dp_bindings)) { > + return false; > + } > } > } > > @@ -2147,18 +2205,8 @@ handle_deleted_vif_lport(const struct > sbrec_port_binding *pb, > * it from local_lports if there is a VIF entry. > * consider_iface_release() takes care of removing from the local_lports > * when the interface change happens. */ > - if (is_lport_container(pb)) { > + if (lport_type == LP_CONTAINER) { > remove_local_lports(pb->logical_port, b_ctx_out); > - > - /* If the container port is removed we should also remove it from > - * its parent's children set. > - */ > - if (lbinding) { > - if (lbinding->parent) { > - local_binding_delete_child(lbinding->parent, lbinding); > - } > - local_binding_destroy(lbinding); > - } > } > > handle_deleted_lport(pb, b_ctx_in, b_ctx_out); > @@ -2177,7 +2225,7 @@ handle_updated_vif_lport(const struct > sbrec_port_binding *pb, > > if (lport_type == LP_VIRTUAL) { > handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map); > - } else if (lport_type == LP_VIF && is_lport_container(pb)) { > + } else if (lport_type == LP_CONTAINER) { > handled = consider_container_lport(pb, b_ctx_in, b_ctx_out, qos_map); > } else { > handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, qos_map); > @@ -2189,14 +2237,14 @@ handle_updated_vif_lport(const struct > sbrec_port_binding *pb, > > bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); > > - if (lport_type == LP_VIRTUAL || > - (lport_type == LP_VIF && is_lport_container(pb)) || > + if (lport_type == LP_VIRTUAL || lport_type == LP_CONTAINER || > claimed == now_claimed) { > return true; > } > > - struct local_binding *lbinding = > - local_binding_find(b_ctx_out->local_bindings, pb->logical_port); > + struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings; > + struct local_binding *lbinding = local_binding_find(local_bindings, > + pb->logical_port); > > /* If the ovs port backing this binding previously was removed in the > * meantime, we won't have a local_binding for it. > @@ -2206,12 +2254,11 @@ handle_updated_vif_lport(const struct > sbrec_port_binding *pb, > return true; > } > > - struct shash_node *node; > - SHASH_FOR_EACH (node, &lbinding->children) { > - struct local_binding *child = node->data; > - if (child->type == BT_CONTAINER) { > - handled = consider_container_lport(child->pb, b_ctx_in, > b_ctx_out, > - qos_map); > + struct binding_lport *b_lport; > + LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { > + if (b_lport->type == LP_CONTAINER) { > + handled = consider_container_lport(b_lport->pb, b_ctx_in, > + b_ctx_out, qos_map); > if (!handled) { > return false; > } > @@ -2256,12 +2303,25 @@ 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) { > - if (is_lport_container(pb)) { > - shash_add(&deleted_container_pbs, pb->logical_port, pb); > - } else { > - shash_add(&deleted_vif_pbs, pb->logical_port, pb); > + struct binding_lport *b_lport = > + binding_lport_find(&b_ctx_out->lbinding_data->lports, > + pb->logical_port); > + if (b_lport) { > + /* If the 'b_lport->type' and 'lport_type' don't match, then > update > + * the b_lport->type to the updated 'lport_type'. The function > + * binding_lport_check_and_cleanup() will cleanup the 'b_lport' > + * if required. */ > + if (b_lport->type != lport_type) { > + b_lport->type = lport_type; > } > + b_lport = binding_lport_check_and_cleanup( > + b_lport, &b_ctx_out->lbinding_data->lports); > + } > + > + if (lport_type == LP_VIF) { > + shash_add(&deleted_vif_pbs, pb->logical_port, pb); > + } else if (lport_type == LP_CONTAINER) { > + shash_add(&deleted_container_pbs, pb->logical_port, pb); > } else if (lport_type == LP_VIRTUAL) { > shash_add(&deleted_virtual_pbs, pb->logical_port, pb); > } else { > @@ -2272,7 +2332,7 @@ binding_handle_port_binding_changes(struct > binding_ctx_in *b_ctx_in, > 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, > + handled = handle_deleted_vif_lport(node->data, LP_CONTAINER, > b_ctx_in, > b_ctx_out); > shash_delete(&deleted_container_pbs, node); > if (!handled) { > @@ -2326,12 +2386,33 @@ delete_done: > > enum en_lport_type lport_type = get_lport_type(pb); > > + struct binding_lport *b_lport = > + binding_lport_find(&b_ctx_out->lbinding_data->lports, > + pb->logical_port); > + if (b_lport) { > + ovs_assert(b_lport->pb == pb); > + > + if (b_lport->type != lport_type) { > + b_lport->type = lport_type; > + } > + > + if (b_lport->lbinding) { > + handled = local_binding_handle_stale_binding_lports( > + b_lport->lbinding, b_ctx_in, b_ctx_out, qos_map_ptr); > + if (!handled) { > + /* Backout from the handling. */ > + break; > + } > + } > + } > + > struct local_datapath *ld = > get_local_datapath(b_ctx_out->local_datapaths, > pb->datapath->tunnel_key); > > switch (lport_type) { > case LP_VIF: > + case LP_CONTAINER: > case LP_VIRTUAL: > handled = handle_updated_vif_lport(pb, lport_type, b_ctx_in, > b_ctx_out, qos_map_ptr); > @@ -2468,11 +2549,11 @@ binding_init(void) > * available. > */ > void > -binding_seqno_run(struct shash *local_bindings) > +binding_seqno_run(struct local_binding_data *lbinding_data) > { > const char *iface_id; > const char *iface_id_next; > - > + struct shash *local_bindings = &lbinding_data->bindings; > SSET_FOR_EACH_SAFE (iface_id, iface_id_next, > &binding_iface_released_set) { > struct shash_node *lb_node = shash_find(local_bindings, iface_id); > > @@ -2508,16 +2589,17 @@ binding_seqno_run(struct shash *local_bindings) > * If so, then this is a newly bound interface, make sure we reset > the > * Port_Binding 'up' field and the OVS Interface 'external-id'. > */ > - if (lb && lb->pb && lb->iface) { > + struct binding_lport *b_lport = local_binding_get_primary_lport(lb); > + if (lb && b_lport && lb->iface) { > new_ifaces = true; > > if (smap_get(&lb->iface->external_ids, OVN_INSTALLED_EXT_ID)) { > ovsrec_interface_update_external_ids_delkey( > lb->iface, OVN_INSTALLED_EXT_ID); > } > - if (lb->pb->n_up) { > + if (b_lport->pb->n_up) { > bool up = false; > - sbrec_port_binding_set_up(lb->pb, &up, 1); > + sbrec_port_binding_set_up(b_lport->pb, &up, 1); > } > simap_put(&binding_iface_seqno_map, lb->name, new_seqno); > } > @@ -2542,12 +2624,13 @@ binding_seqno_run(struct shash *local_bindings) > * available. > */ > void > -binding_seqno_install(struct shash *local_bindings) > +binding_seqno_install(struct local_binding_data *lbinding_data) > { > struct ofctrl_acked_seqnos *acked_seqnos = > ofctrl_acked_seqnos_get(binding_seq_type_pb_cfg); > struct simap_node *node; > struct simap_node *node_next; > + struct shash *local_bindings = &lbinding_data->bindings; > > SIMAP_FOR_EACH_SAFE (node, node_next, &binding_iface_seqno_map) { > struct shash_node *lb_node = shash_find(local_bindings, node->name); > @@ -2557,7 +2640,8 @@ binding_seqno_install(struct shash *local_bindings) > } > > struct local_binding *lb = lb_node->data; > - if (!lb->pb || !lb->iface) { > + struct binding_lport *b_lport = local_binding_get_primary_lport(lb); > + if (!b_lport || !lb->iface) { > goto del_seqno; > } > > @@ -2568,14 +2652,12 @@ binding_seqno_install(struct shash *local_bindings) > ovsrec_interface_update_external_ids_setkey(lb->iface, > OVN_INSTALLED_EXT_ID, > "true"); > - if (lb->pb->n_up) { > + if (b_lport->pb->n_up) { > bool up = true; > > - sbrec_port_binding_set_up(lb->pb, &up, 1); > - struct shash_node *child_node; > - SHASH_FOR_EACH (child_node, &lb->children) { > - struct local_binding *lb_child = child_node->data; > - sbrec_port_binding_set_up(lb_child->pb, &up, 1); > + sbrec_port_binding_set_up(b_lport->pb, &up, 1); > + LIST_FOR_EACH (b_lport, list_node, &lb->binding_lports) { > + sbrec_port_binding_set_up(b_lport->pb, &up, 1); > } > } > > @@ -2591,3 +2673,305 @@ binding_seqno_flush(void) > { > simap_clear(&binding_iface_seqno_map); > } > + > +/* Static functions for local_lbindind and binding_lport. */ > +static struct local_binding * > +local_binding_create(const char *name, const struct ovsrec_interface *iface) > +{ > + struct local_binding *lbinding = xzalloc(sizeof *lbinding); > + lbinding->name = xstrdup(name); > + lbinding->iface = iface; > + ovs_list_init(&lbinding->binding_lports); > + > + return lbinding; > +} > + > +static struct local_binding * > +local_binding_find(struct shash *local_bindings, const char *name) > +{ > + return shash_find_data(local_bindings, name); > +} > + > +static void > +local_binding_add(struct shash *local_bindings, struct local_binding > *lbinding) > +{ > + shash_add(local_bindings, lbinding->name, lbinding); > +} > + > +static void > +local_binding_destroy(struct local_binding *lbinding, > + struct shash *binding_lports) > +{ > + struct binding_lport *b_lport; > + LIST_FOR_EACH_POP (b_lport, list_node, &lbinding->binding_lports) { > + b_lport->lbinding = NULL; > + binding_lport_delete(binding_lports, b_lport); > + } > + > + free(lbinding->name); > + free(lbinding); > +} > + > +static void > +local_binding_delete(struct local_binding *lbinding, > + struct shash *local_bindings, > + struct shash *binding_lports) > +{ > + shash_find_and_delete(local_bindings, lbinding->name); > + local_binding_destroy(lbinding, binding_lports); > +} > + > +/* Returns the primary binding lport if present in lbinding's > + * binding lports list. A binding lport is considered primary > + * if binding lport's type is LP_VIF and the name matches > + * with the 'lbinding'. > + */ > +static struct binding_lport * > +local_binding_get_primary_lport(struct local_binding *lbinding) > +{ > + if (!lbinding) { > + return NULL; > + } > + > + if (!ovs_list_is_empty(&lbinding->binding_lports)) { > + struct binding_lport *b_lport = NULL; > + b_lport = CONTAINER_OF(ovs_list_front(&lbinding->binding_lports), > + struct binding_lport, list_node); > + > + if (b_lport->type == LP_VIF && > + !strcmp(lbinding->name, b_lport->name)) { > + return b_lport; > + } > + } > + > + return NULL; > +} > + > +static struct binding_lport * > +local_binding_add_lport(struct shash *binding_lports, > + struct local_binding *lbinding, > + const struct sbrec_port_binding *pb, > + enum en_lport_type b_type) > +{ > + struct binding_lport *b_lport = > + binding_lport_find(binding_lports, pb->logical_port); > + bool add_to_lport_list = false; > + if (!b_lport) { > + b_lport = binding_lport_create(pb, lbinding, b_type); > + binding_lport_add(binding_lports, b_lport); > + add_to_lport_list = true; > + } else if (b_lport->lbinding != lbinding) { > + add_to_lport_list = true; > + if (!ovs_list_is_empty(&b_lport->list_node)) { > + ovs_list_remove(&b_lport->list_node); > + } > + b_lport->lbinding = lbinding; > + b_lport->type = b_type; > + } > + > + if (add_to_lport_list) { > + if (b_type == LP_VIF) { > + ovs_list_push_front(&lbinding->binding_lports, > &b_lport->list_node); > + } else { > + ovs_list_push_back(&lbinding->binding_lports, > &b_lport->list_node); > + } > + } > + > + return b_lport; > +} > + > +/* This function handles the stale binding lports of 'lbinding' if 'lbinding' > + * doesn't have a primary binding lport. > + */ > +static bool > +local_binding_handle_stale_binding_lports(struct local_binding *lbinding, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, > + struct hmap *qos_map) > +{ > + /* Check if this lbinding has a primary binding_lport or not. */ > + struct binding_lport *p_lport = > local_binding_get_primary_lport(lbinding); > + if (p_lport) { > + /* Nothing to be done. */ > + return true; > + } > + > + bool handled = true; > + struct binding_lport *b_lport, *next; > + const struct sbrec_port_binding *pb; > + LIST_FOR_EACH_SAFE (b_lport, next, list_node, &lbinding->binding_lports) > { > + /* Get the lport type again from the pb. Its possible that the > + * pb type has changed. */ > + enum en_lport_type pb_lport_type = get_lport_type(b_lport->pb); > + if (b_lport->type == LP_VIRTUAL && pb_lport_type == LP_VIRTUAL) { > + pb = b_lport->pb; > + binding_lport_delete(&b_ctx_out->lbinding_data->lports, > + b_lport); > + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out, > qos_map); > + } else if (b_lport->type == LP_CONTAINER && > + pb_lport_type == LP_CONTAINER) { > + /* For container lport, binding_lport is preserved so that when > + * the parent port is created, it can be considered. > + * consider_container_lport() creates the binding_lport for the > parent > + * port (with iface set to NULL). */ > + handled = consider_container_lport(b_lport->pb, b_ctx_in, > b_ctx_out, qos_map); > + } else { > + /* This can happen when the lport type changes from one type > + * to another. Eg. from normal lport to external. Release the > + * lport if it was claimed earlier and delete the b_lport. */ > + handled = release_binding_lport(b_ctx_in->chassis_rec, b_lport, > + !b_ctx_in->ovnsb_idl_txn, > + b_ctx_out->tracked_dp_bindings); > + binding_lport_delete(&b_ctx_out->lbinding_data->lports, > + b_lport); > + } > + > + if (!handled) { > + return false; > + } > + } > + > + return handled; > +} > + > +static struct binding_lport * > +binding_lport_create(const struct sbrec_port_binding *pb, > + struct local_binding *lbinding, > + enum en_lport_type type) > +{ > + struct binding_lport *b_lport = xzalloc(sizeof *b_lport); > + b_lport->name = xstrdup(pb->logical_port); > + b_lport->pb = pb; > + b_lport->type = type; > + b_lport->lbinding = lbinding; > + ovs_list_init(&b_lport->list_node); > + > + return b_lport; > +} > + > +static void > +binding_lport_add(struct shash *binding_lports, struct binding_lport > *b_lport) > +{ > + shash_add(binding_lports, b_lport->pb->logical_port, b_lport); > +} > + > +static struct binding_lport * > +binding_lport_find(struct shash *binding_lports, const char *lport_name) > +{ > + if (!lport_name) { > + return NULL; > + } > + > + return shash_find_data(binding_lports, lport_name); > +} > + > +static void > +binding_lport_destroy(struct binding_lport *b_lport) > +{ > + if (!ovs_list_is_empty(&b_lport->list_node)) { > + ovs_list_remove(&b_lport->list_node); > + } > + > + free(b_lport->name); > + free(b_lport); > +} > + > +static void > +binding_lport_delete(struct shash *binding_lports, > + struct binding_lport *b_lport) > +{ > + shash_find_and_delete(binding_lports, b_lport->name); > + binding_lport_destroy(b_lport); > +} > + > + > +static const struct sbrec_port_binding * > +binding_lport_get_parent_pb(struct binding_lport *b_lport) > +{ > + if (!b_lport) { > + return NULL; > + } > + > + if (b_lport->type == LP_VIF) { > + return NULL; > + } > + > + struct local_binding *lbinding = b_lport->lbinding; > + ovs_assert(lbinding); > + > + struct binding_lport *parent_b_lport = > + local_binding_get_primary_lport(lbinding); > + > + return parent_b_lport ? parent_b_lport->pb : NULL; > +} > + > +/* This function checks and cleans up the 'b_lport' if it is > + * not in the correct state. > + * > + * If the 'b_lport' type is LP_VIF, then its name and its lbinding->name > + * should match. Otherwise this should be cleaned up. > + * > + * If the 'b_lport' type is LP_CONTAINER, then its parent_port name should > + * be the same as its lbinding's name. Otherwise this should be > + * cleaned up. > + * > + * If the 'b_lport' type is LP_VIRTUAL, then its virtual parent name > + * should be the same as its lbinding's name. Otherwise this > + * should be cleaned up. > + * > + * If the 'b_lport' type is not LP_VIF, LP_CONTAINER or LP_VIRTUAL, it > + * should be cleaned up. This can happen if the CMS changes > + * the port binding type. > + */ > +static struct binding_lport * > +binding_lport_check_and_cleanup(struct binding_lport *b_lport, > + struct shash *binding_lports) > +{ > + bool cleanup_blport = false; > + > + if (!b_lport->lbinding) { > + cleanup_blport = true; > + goto cleanup; > + } > + > + switch (b_lport->type) { > + case LP_VIF: > + if (strcmp(b_lport->name, b_lport->lbinding->name)) { > + cleanup_blport = true; > + } > + break; > + > + case LP_CONTAINER: > + if (strcmp(b_lport->pb->parent_port, b_lport->lbinding->name)) { > + cleanup_blport = true; > + } > + break; > + > + case LP_VIRTUAL: > + if (!b_lport->pb->virtual_parent || > + strcmp(b_lport->pb->virtual_parent, b_lport->lbinding->name)) { > + cleanup_blport = true; > + } > + break; > + > + case LP_PATCH: > + case LP_LOCALPORT: > + case LP_VTEP: > + case LP_L2GATEWAY: > + case LP_L3GATEWAY: > + case LP_CHASSISREDIRECT: > + case LP_EXTERNAL: > + case LP_LOCALNET: > + case LP_REMOTE: > + case LP_UNKNOWN: > + cleanup_blport = true; > + } > + > +cleanup: > + if (cleanup_blport) { > + binding_lport_delete(binding_lports, b_lport); > + return NULL; > + } > + > + return b_lport; > +} > diff --git a/controller/binding.h b/controller/binding.h > index c9ebef4b17..4fc9ef207b 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -36,6 +36,7 @@ struct sbrec_chassis; > struct sbrec_port_binding_table; > struct sset; > struct sbrec_port_binding; > +struct ds; > > struct binding_ctx_in { > struct ovsdb_idl_txn *ovnsb_idl_txn; > @@ -56,7 +57,7 @@ struct binding_ctx_in { > > struct binding_ctx_out { > struct hmap *local_datapaths; > - struct shash *local_bindings; > + struct local_binding_data *lbinding_data; > > /* sset of (potential) local lports. */ > struct sset *local_lports; > @@ -86,28 +87,16 @@ struct binding_ctx_out { > struct hmap *tracked_dp_bindings; > }; > > -enum local_binding_type { > - BT_VIF, > - BT_CONTAINER, > - BT_VIRTUAL > +struct local_binding_data { > + struct shash bindings; > + struct shash lports; > }; > > -struct local_binding { > - char *name; > - enum local_binding_type type; > - const struct ovsrec_interface *iface; > - const struct sbrec_port_binding *pb; > - > - /* shash of 'struct local_binding' representing children. */ > - struct shash children; > - struct local_binding *parent; > -}; > +void local_binding_data_init(struct local_binding_data *); > +void local_binding_data_destroy(struct local_binding_data *); > > -static inline struct local_binding * > -local_binding_find(struct shash *local_bindings, const char *name) > -{ > - return shash_find_data(local_bindings, name); > -} > +const struct sbrec_port_binding *local_binding_get_primary_pb( > + struct shash *local_bindings, const char *pb_name); > > /* Represents a tracked binding logical port. */ > struct tracked_binding_lport { > @@ -128,8 +117,6 @@ bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct sbrec_port_binding_table *, > const struct sbrec_chassis *); > > -void local_bindings_init(struct shash *local_bindings); > -void local_bindings_destroy(struct shash *local_bindings); > bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, > struct binding_ctx_out *); > bool binding_handle_port_binding_changes(struct binding_ctx_in *, > @@ -137,7 +124,8 @@ bool binding_handle_port_binding_changes(struct > binding_ctx_in *, > void binding_tracked_dp_destroy(struct hmap *tracked_datapaths); > > void binding_init(void); > -void binding_seqno_run(struct shash *local_bindings); > -void binding_seqno_install(struct shash *local_bindings); > +void binding_seqno_run(struct local_binding_data *lbinding_data); > +void binding_seqno_install(struct local_binding_data *lbinding_data); > void binding_seqno_flush(void); > +void binding_dump_local_bindings(struct local_binding_data *, struct ds *); > #endif /* controller/binding.h */ > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index ea0b677bdc..7ee11dffe2 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -81,6 +81,7 @@ static unixctl_cb_func cluster_state_reset_cmd; > static unixctl_cb_func debug_pause_execution; > static unixctl_cb_func debug_resume_execution; > static unixctl_cb_func debug_status_execution; > +static unixctl_cb_func debug_dump_local_bindings; > static unixctl_cb_func lflow_cache_flush_cmd; > static unixctl_cb_func lflow_cache_show_stats_cmd; > static unixctl_cb_func debug_delay_nb_cfg_report; > @@ -1182,8 +1183,7 @@ struct ed_type_runtime_data { > /* Contains "struct local_datapath" nodes. */ > struct hmap local_datapaths; > > - /* Contains "struct local_binding" nodes. */ > - struct shash local_bindings; > + struct local_binding_data lbinding_data; > > /* Contains the name of each logical port resident on the local > * hypervisor. These logical ports include the VIFs (and their child > @@ -1222,9 +1222,9 @@ struct ed_type_runtime_data { > * | | Interface and Port Binding changes store the | > * | @tracked_dp_bindings | changed datapaths (datapaths added/removed from | > * | | local_datapaths) and changed port bindings | > - * | | (added/updated/deleted in 'local_bindings'). | > + * | | (added/updated/deleted in 'lbinding_data'). | > * | | So any changes to the runtime data - | > - * | | local_datapaths and local_bindings is captured | > + * | | local_datapaths and lbinding_data is captured | > * | | here. | > * ------------------------------------------------------------------------ > * | | This is a bool which represents if the runtime | > @@ -1251,7 +1251,7 @@ struct ed_type_runtime_data { > * > * --------------------------------------------------------------------- > * | local_datapaths | The changes to these runtime data is captured in | > - * | local_bindings | the @tracked_dp_bindings indirectly and hence it | > + * | lbinding_data | the @tracked_dp_bindings indirectly and hence it | > * | local_lport_ids | is not tracked explicitly. | > * --------------------------------------------------------------------- > * | local_iface_ids | This is used internally within the runtime data | > @@ -1294,7 +1294,7 @@ en_runtime_data_init(struct engine_node *node > OVS_UNUSED, > sset_init(&data->active_tunnels); > sset_init(&data->egress_ifaces); > smap_init(&data->local_iface_ids); > - local_bindings_init(&data->local_bindings); > + local_binding_data_init(&data->lbinding_data); > > /* Init the tracked data. */ > hmap_init(&data->tracked_dp_bindings); > @@ -1322,7 +1322,7 @@ en_runtime_data_cleanup(void *data) > free(cur_node); > } > hmap_destroy(&rt_data->local_datapaths); > - local_bindings_destroy(&rt_data->local_bindings); > + local_binding_data_destroy(&rt_data->lbinding_data); > hmapx_destroy(&rt_data->ct_updated_datapaths); > } > > @@ -1405,7 +1405,7 @@ init_binding_ctx(struct engine_node *node, > b_ctx_out->local_lport_ids_changed = false; > b_ctx_out->non_vif_ports_changed = false; > b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; > - b_ctx_out->local_bindings = &rt_data->local_bindings; > + b_ctx_out->lbinding_data = &rt_data->lbinding_data; > b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; > b_ctx_out->tracked_dp_bindings = NULL; > b_ctx_out->local_lports_changed = NULL; > @@ -1449,7 +1449,7 @@ en_runtime_data_run(struct engine_node *node, void > *data) > free(cur_node); > } > hmap_clear(local_datapaths); > - local_bindings_destroy(&rt_data->local_bindings); > + local_binding_data_destroy(&rt_data->lbinding_data); > sset_destroy(local_lports); > sset_destroy(local_lport_ids); > sset_destroy(active_tunnels); > @@ -1460,7 +1460,7 @@ en_runtime_data_run(struct engine_node *node, void > *data) > sset_init(active_tunnels); > sset_init(&rt_data->egress_ifaces); > smap_init(&rt_data->local_iface_ids); > - local_bindings_init(&rt_data->local_bindings); > + local_binding_data_init(&rt_data->lbinding_data); > hmapx_clear(&rt_data->ct_updated_datapaths); > } > > @@ -1822,7 +1822,7 @@ static void init_physical_ctx(struct engine_node *node, > p_ctx->local_lports = &rt_data->local_lports; > p_ctx->ct_zones = ct_zones; > p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; > - p_ctx->local_bindings = &rt_data->local_bindings; > + p_ctx->local_bindings = &rt_data->lbinding_data.bindings; > p_ctx->ct_updated_datapaths = &rt_data->ct_updated_datapaths; > } > > @@ -2688,7 +2688,8 @@ main(int argc, char *argv[]) > engine_get_internal_data(&en_flow_output); > struct ed_type_ct_zones *ct_zones_data = > engine_get_internal_data(&en_ct_zones); > - struct ed_type_runtime_data *runtime_data = NULL; > + struct ed_type_runtime_data *runtime_data = > + engine_get_internal_data(&en_runtime_data);; Nit: one ';' too many. > > ofctrl_init(&flow_output_data->group_table, > &flow_output_data->meter_table, > @@ -2741,6 +2742,11 @@ main(int argc, char *argv[]) > unixctl_command_register("debug/delay-nb-cfg-report", "SECONDS", 1, 1, > debug_delay_nb_cfg_report, > &delay_nb_cfg_report); > > + unixctl_command_register("debug/dump-local-bindings", "", 0, 0, > + debug_dump_local_bindings, > + &runtime_data->lbinding_data); > + runtime_data = NULL; I think it's fine to remove this line, next time we use 'runtime_data' we reinitialize it anyway. > + > unsigned int ovs_cond_seqno = UINT_MAX; > unsigned int ovnsb_cond_seqno = UINT_MAX; > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > @@ -2958,7 +2964,7 @@ main(int argc, char *argv[]) > ovnsb_cond_seqno, > ovnsb_expected_cond_seqno)); > if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) { > - binding_seqno_run(&runtime_data->local_bindings); > + binding_seqno_run(&runtime_data->lbinding_data); > } > > flow_output_data = engine_get_data(&en_flow_output); > @@ -2971,7 +2977,7 @@ main(int argc, char *argv[]) > } > ofctrl_seqno_run(ofctrl_get_cur_cfg()); > if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) { > - binding_seqno_install(&runtime_data->local_bindings); > + binding_seqno_install(&runtime_data->lbinding_data); > } > } > > @@ -3411,3 +3417,13 @@ debug_delay_nb_cfg_report(struct unixctl_conn *conn, > int argc OVS_UNUSED, > unixctl_command_reply(conn, "no delay for nb_cfg report."); > } > } > + > +static void > +debug_dump_local_bindings(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void > *local_bindings) > +{ > + struct ds binding_data = DS_EMPTY_INITIALIZER; > + binding_dump_local_bindings(local_bindings, &binding_data); > + unixctl_command_reply(conn, ds_cstr(&binding_data)); > + ds_destroy(&binding_data); > +} > diff --git a/controller/physical.c b/controller/physical.c > index fa5d0d692d..874d1ee270 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -1839,20 +1839,19 @@ physical_handle_ovs_iface_changes(struct physical_ctx > *p_ctx, > continue; > } > > - const struct local_binding *lb = > - local_binding_find(p_ctx->local_bindings, iface_id); > - > - if (!lb || !lb->pb) { > + const struct sbrec_port_binding *lb_pb = > + local_binding_get_primary_pb(p_ctx->local_bindings, iface_id); > + if (!lb_pb) { > continue; > } > > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > if (ovsrec_interface_is_deleted(iface_rec)) { > - ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); > + ofctrl_remove_flows(flow_table, &lb_pb->header_.uuid); > simap_find_and_delete(&localvif_to_ofport, iface_id); > } else { > if (!ovsrec_interface_is_new(iface_rec)) { > - ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); > + ofctrl_remove_flows(flow_table, &lb_pb->header_.uuid); > } > > simap_put(&localvif_to_ofport, iface_id, ofport); > @@ -1860,7 +1859,7 @@ physical_handle_ovs_iface_changes(struct physical_ctx > *p_ctx, > p_ctx->mff_ovn_geneve, p_ctx->ct_zones, > p_ctx->active_tunnels, > p_ctx->local_datapaths, > - lb->pb, p_ctx->chassis, > + lb_pb, p_ctx->chassis, > flow_table, &ofpacts); > } > } > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index 02212ed9c5..a6024468e7 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -469,3 +469,83 @@ OVS_WAIT_UNTIL([ > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +# Test that changes of a port binding from one type to another doesn'that > +# result in any ovn-controller asserts or crashes. > +AT_SETUP([ovn-controller - port binding type change handling]) > +AT_KEYWORDS([ovn]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +check ovn-nbctl ls-add ls1 -- lsp-add ls1 lsp1 > + > +as hv1 > +check ovs-vsctl \ > + -- add-port br-int vif1 \ > + -- set Interface vif1 external_ids:iface-id=lsp1 > + > +# ovn-controller should bind the interface. > +wait_for_ports_up > +hv_uuid=$(fetch_column Chassis _uuid name=hv1) > +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[lsp1]], OVS interface name : [[vif1]], num binding lports : [[1]] > +primary lport : [[lsp1]] > +---------------------------------------- > +]) > + > +# pause ovn-northd > +check as northd ovn-appctl -t ovn-northd pause > +check as northd-backup ovn-appctl -t ovn-northd pause > + > +as northd ovn-appctl -t ovn-northd status > +as northd-backup ovn-appctl -t ovn-northd status > + > +for type in patch chassisredirect l3gateway localnet localport l2gateway \ > +virtual external remote vtep > +do > + for update_type in patch chassisredirect l3gateway localnet localport > l2gateway \ > +virtual external remote vtep > + do I think you missed 'patch' type. Also, I think this can be simplified to avoid duplication if we use arrays, for example: pb_types=(patch chassisredirect l3gateway localnet localport l2gateway virtual external remote vtep) for type in ${pb_types[[@]]} do for update_type in ${pb_types[[@]]} do ... > + check ovn-sbctl set port_binding lsp1 type=$type > + check as hv1 ovs-vsctl set open . external_ids:ovn-cms-options=$type > + OVS_WAIT_UNTIL([test $type = $(ovn-sbctl get chassis . > other_config:ovn-cms-options)]) > + > + AT_CHECK([as hv1 ovn-appctl -t ovn-controller > debug/dump-local-bindings], [0], [dnl > +Local bindings: > +name: [[lsp1]], OVS interface name : [[vif1]], num binding lports : [[0]] > +---------------------------------------- > +]) > + > + echo "Updating to $update_type from $type" > + check ovn-sbctl set port_binding lsp1 type=$update_type > + check as hv1 ovs-vsctl set open . > external_ids:ovn-cms-options=$update_type > + OVS_WAIT_UNTIL([test $update_type = $(ovn-sbctl get chassis . > other_config:ovn-cms-options)]) > + > + AT_CHECK([as hv1 ovn-appctl -t ovn-controller > debug/dump-local-bindings], [0], [dnl > +Local bindings: > +name: [[lsp1]], OVS interface name : [[vif1]], num binding lports : [[0]] > +---------------------------------------- > +]) > + # Set the port binding type back to VIF. > + check ovn-sbctl set port_binding lsp1 type=\"\" > + check as hv1 ovs-vsctl set open . external_ids:ovn-cms-options=foo > + OVS_WAIT_UNTIL([test foo = $(ovn-sbctl get chassis . > other_config:ovn-cms-options)]) > + > + AT_CHECK([as hv1 ovn-appctl -t ovn-controller > debug/dump-local-bindings], [0], [dnl > +Local bindings: > +name: [[lsp1]], OVS interface name : [[vif1]], num binding lports : [[1]] > +primary lport : [[lsp1]] > +---------------------------------------- > +]) > + done > +done > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > diff --git a/tests/ovn.at b/tests/ovn.at > index 391a8bcd93..73cc8bf025 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -25216,3 +25216,643 @@ AT_CHECK([cat hv2_offlows_table72.txt | grep -v > NXST], [1], [dnl > OVN_CLEANUP([hv1], [hv2]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- container port changed to normal port and then deleted]) > +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 > +ovs-vsctl -- add-port br-int vm1 > + > +check ovn-nbctl ls-add ls > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl lsp-add ls vm-cont vm1 1 > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > + > +wait_for_ports_up > + > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl clear logical_switch_port vm-cont parent_name > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=foo > +check ovn-nbctl lsp-del vm-cont > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not asserted. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +wait_column "false" nb:Logical_Switch_Port up name=vm1 > + > +check ovn-nbctl lsp-add ls vm-cont1 vm1 1 > +check ovn-nbctl lsp-add ls vm-cont2 vm1 2 > + > +check ovn-nbctl --wait=sb lsp-del vm1 > + > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl clear logical_switch_port vm-cont1 parent_name > +check ovn-nbctl clear logical_switch_port vm-cont2 parent_name > + > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +check ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not crashed. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl set logical_switch_port vm-cont1 parent_name=vm1 > +check ovn-nbctl --wait=sb set logical_switch_port vm-cont2 parent_name=vm1 > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > + > +wait_for_ports_up > + > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl --wait=sb lsp-del vm1 > +check ovn-nbctl clear logical_switch_port vm-cont1 parent_name > +check ovn-nbctl --wait=sb clear logical_switch_port vm-cont2 parent_name > +check ovn-nbctl lsp-del vm-cont1 > +check ovn-nbctl --wait=sb lsp-del vm-cont2 > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +check ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not crashed. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl lsp-add ls vm-cont1 vm1 1 > +check ovn-nbctl lsp-add ls vm-cont2 vm1 2 > + > +wait_for_ports_up > + > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl clear logical_switch_port vm-cont1 parent_name > +check ovn-nbctl --wait=sb clear logical_switch_port vm-cont2 parent_name > +check ovn-nbctl lsp-del vm-cont1 > +check ovn-nbctl lsp-del vm-cont2 > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +check ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not crashed. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +check ovn-nbctl lsp-add ls vm-cont1 vm1 1 > +check ovn-nbctl lsp-add ls vm-cont2 vm1 2 > + > +wait_for_ports_up > + > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl clear logical_switch_port vm-cont1 parent_name > +check ovn-nbctl --wait=sb clear logical_switch_port vm-cont2 parent_name > + > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=foo > + > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +wait_column "false" nb:Logical_Switch_Port up name=vm1 > +wait_column "false" nb:Logical_Switch_Port up name=vm-cont1 > +wait_column "false" nb:Logical_Switch_Port up name=vm-cont2 > + > +check ovn-nbctl set logical_switch_port vm-cont1 parent_name=vm1 > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > +check ovn-nbctl --wait=sb set logical_switch_port vm-cont2 parent_name=vm1 > + > +wait_for_ports_up > + > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl clear logical_switch_port vm-cont1 parent_name > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm-cont1 > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +wait_column "false" nb:Logical_Switch_Port up name=vm1 > +wait_column "true" nb:Logical_Switch_Port up name=vm-cont1 > +wait_column "false" nb:Logical_Switch_Port up name=vm-cont2 > + > +check ovn-nbctl --wait=sb set logical_switch_port vm-cont2 > parent_name=vm-cont1 > +check ovn-nbctl --wait=sb set logical_switch_port vm1 parent_name=vm-cont1 > + > +wait_for_ports_up > + > +# Delete vm1, vm-cont1 and vm-cont2 and recreate again. > +check ovn-nbctl lsp-del vm1 > +check ovn-nbctl lsp-del vm-cont1 > +check ovn-nbctl --wait=hv lsp-del vm-cont2 > + > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl lsp-add ls vm-cont1 vm1 1 > +check ovn-nbctl lsp-add ls vm-cont2 vm1 2 > + > +wait_for_ports_up > + > +# Make vm1 as a child port of some non existent lport - foo. vm1, vm1-cont1 > and > +# vm1-cont2 should be released. > +check ovn-nbctl --wait=sb set logical_switch_port vm1 parent_name=bar > +wait_column "false" nb:Logical_Switch_Port up name=vm1 > +wait_column "false" nb:Logical_Switch_Port up name=vm-cont1 > +wait_column "false" nb:Logical_Switch_Port up name=vm-cont2 > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- container port changed from one parent to another]) > +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 > +ovs-vsctl -- add-port br-int vm1 -- set interface vm1 ofport-request=1 > +ovs-vsctl -- add-port br-int vm2 -- set interface vm1 ofport-request=2 > + > +check ovn-nbctl ls-add ls > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl lsp-add ls vm1-cont vm1 1 > +check ovn-nbctl lsp-add ls vm2 > +check ovn-nbctl lsp-add ls vm2-cont vm2 2 > + > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > +check as hv1 ovs-vsctl set Interface vm2 external_ids:iface-id=vm2 > + > +wait_for_ports_up > + > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c dl_vlan=1], > [0], [dnl > +1 > +]) > + > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c dl_vlan=2], > [0], [dnl > +1 > +]) > + > +# change the parent of vm1-cont to vm2. > +as hv1 ovn-appctl -t ovn-controller vlog/set dbg > +check ovn-nbctl --wait=sb set logical_switch_port vm1-cont parent_name=vm2 \ > +-- set logical_switch_port vm1-cont tag_request=3 > + > +wait_for_ports_up > + > +check ovn-nbctl --wait=hv sync > + > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c dl_vlan=1], > [1], [dnl > +0 > +]) > + > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c dl_vlan=2], > [0], [dnl > +1 > +]) > + > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c dl_vlan=3], > [0], [dnl > +1 > +]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- container port use-after-free test]) > +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 > +ovs-vsctl -- add-port br-int vm1 > + > +check ovn-nbctl ls-add ls > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl lsp-add ls vm-cont vm1 1 > +check ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > +check ovn-nbctl clear logical_switch_port vm-cont parent_name > +check ovs-vsctl set Interface vm1 external_ids:iface-id=foo > +check ovn-nbctl lsp-del vm-cont > +check ovn-nbctl ls-del ls > +check ovn-nbctl ls-add ls > +check ovn-nbctl lsp-add ls vm1 > +check ovn-nbctl lsp-add ls vm-cont vm1 1 > +check ovs-vsctl set Interface vm1 external_ids:iface-id=vm1 > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl clear logical_switch_port vm-cont parent_name > +check ovn-nbctl lsp-del vm-cont > +check as hv1 ovn-appctl -t ovn-controller debug/resume > +check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=foo > + > +ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not asserted. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +wait_column "false" nb:Logical_Switch_Port up name=vm1 > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > +# Test that OVS.external_ids:iface-id doesn't affect non-VIF port bindings. > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- Non-VIF ports incremental processing]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.10 > + > +check ovn-nbctl ls-add ls1 -- lsp-add ls1 lsp1 > + > +as hv1 > +check ovs-vsctl \ > + -- add-port br-int vif1 \ > + -- set Interface vif1 external_ids:iface-id=lsp1 > + > +# ovn-controller should bind the interface. > +wait_for_ports_up > +hv_uuid=$(fetch_column Chassis _uuid name=hv1) > +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 > + > +# Change the port type to router, ovn-controller should release it. > +check ovn-nbctl --wait=hv lsp-set-type lsp1 router > +check_column "" Port_Binding chassis logical_port=lsp1 > + > +# Clear port type, ovn-controller should rebind it. > +check ovn-nbctl --wait=hv lsp-set-type lsp1 '' > +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 > + > +# Change the port type to localnet, ovn-controller should release it. > +check ovn-nbctl --wait=hv lsp-set-type lsp1 localnet > +check_column "" Port_Binding chassis logical_port=lsp1 > + > +# Clear port type, ovn-controller should rebind it. > +check ovn-nbctl --wait=hv lsp-set-type lsp1 '' > +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 > + > +# Change the port type to localport, ovn-controller should release it. > +check ovn-nbctl --wait=hv lsp-set-type lsp1 localport > +check_column "" Port_Binding chassis logical_port=lsp1 > + > +# Clear port type, ovn-controller should rebind it. > +check ovn-nbctl --wait=hv lsp-set-type lsp1 '' > +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 > + > +# Change the port type to localnet and then delete it. > +# ovn-controller should handle this properly. > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl --wait=sb lsp-set-type lsp1 localport > +check ovn-nbctl --wait=sb lsp-del lsp1 > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +check ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not asserted. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +check ovn-nbctl lsp-add ls1 lsp1 > +wait_for_ports_up > + > +# Change the port type to virtual and then delete it. > +# ovn-controller should handle this properly. > +check as hv1 ovn-appctl -t ovn-controller debug/pause > +check ovn-nbctl --wait=sb lsp-set-type lsp1 virtual > +check ovn-nbctl --wait=sb lsp-del lsp1 > +check as hv1 ovn-appctl -t ovn-controller debug/resume > + > +check ovn-nbctl --wait=hv sync > + > +# Make sure that ovn-controller has not asserted. > +AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > +# Tests that ovn-controller creates local bindings correctly by running > +# ovn-appctl -t ovn-controller debug/dump-local-bindings. > +# Ideally this test case should have been a unit test case. > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- ovn-controller local bindings]) > +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 > +ovs-vsctl -- add-port br-int hv1-vm1 > + > +sim_add hv2 > +as hv2 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.2 > +ovs-vsctl -- add-port br-int hv2-vm1 > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lsp-add sw0 sw0p1 > +check ovn-nbctl lsp-add sw0 sw0p2 > + > +check as hv1 ovs-vsctl set interface hv1-vm1 external_ids:iface-id=sw0p1 > +check as hv2 ovs-vsctl set interface hv2-vm1 external_ids:iface-id=sw0p2 > + > +wait_for_ports_up > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv1-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p1]] > +---------------------------------------- > +]) > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +]) > + > +# Create an ovs interface in hv1 > +check as hv1 ovs-vsctl add-port br-int hv1-vm2 -- set interface hv1-vm2 > external_ids:iface-id=sw1p1 > +check ovn-nbctl --wait=hv sync > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv1-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p1]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[0]] > +---------------------------------------- > +]) > + > +# Create lport sw1p1 > +check ovn-nbctl ls-add sw1 -- lsp-add sw1 sw1p1 > + > +wait_for_ports_up > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv1-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p1]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[1]] > +primary lport : [[sw1p1]] > +---------------------------------------- > +]) > + > +# Swap sw0p1 and sw0p2. > +check as hv1 ovs-vsctl set interface hv1-vm1 external_ids:iface-id=sw0p2 > +check as hv2 ovs-vsctl set interface hv2-vm1 external_ids:iface-id=sw0p1 > + > +check ovn-nbctl --wait=hv sync > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p2]], OVS interface name : [[hv1-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[1]] > +primary lport : [[sw1p1]] > +---------------------------------------- > +]) > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv2-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p1]] > +---------------------------------------- > +]) > + > +# Create child port for sw0p1 > +check ovn-nbctl --wait=hv lsp-add sw0 sw0p1-c1 sw0p1 1 > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +primary lport : [[sw0p1]] > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv1-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[1]] > +primary lport : [[sw1p1]] > +---------------------------------------- > +]) > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv2-vm1]], num binding lports : [[2]] > +primary lport : [[sw0p1]] > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +---------------------------------------- > +]) > + > +# Create another child port for sw0p1 > +check ovn-nbctl --wait=hv lsp-add sw0 sw0p1-c2 sw0p1 2 > + > +wait_for_ports_up > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[3]] > +primary lport : [[sw0p1]] > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv1-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[1]] > +primary lport : [[sw1p1]] > +---------------------------------------- > +]) > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv2-vm1]], num binding lports : [[3]] > +primary lport : [[sw0p1]] > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +]) > + > +# Swap sw0p1 and sw0p2 again. > +check as hv1 ovs-vsctl set interface hv1-vm1 external_ids:iface-id=sw0p1 > +check as hv2 ovs-vsctl set interface hv2-vm1 external_ids:iface-id=sw0p2 > + > +check ovn-nbctl --wait=hv sync > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[hv1-vm1]], num binding lports : [[3]] > +primary lport : [[sw0p1]] > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[1]] > +primary lport : [[sw1p1]] > +---------------------------------------- > +]) > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[3]] > +primary lport : [[sw0p1]] > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +]) > + > +# Make sw0p1 as child port of non existent lport - foo > +check ovn-nbctl --wait=hv set logical_switch_port sw0p1 parent_name=foo > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[hv1-vm1]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw1p1]], OVS interface name : [[hv1-vm2]], num binding lports : [[1]] > +primary lport : [[sw1p1]] > +---------------------------------------- > +]) > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +]) > + > +# Change the lport type of sw0p2 to different types and make sure that > +# local bindings are correct. > + > +hv2_uuid=$(fetch_column Chassis _uuid name=hv2) > +check_column "$hv2_uuid" Port_Binding chassis logical_port=sw0p2 > + > +# Change the port type to router, ovn-controller should release it. > +check ovn-nbctl --wait=hv lsp-set-type sw0p2 router > +check_column "" Port_Binding chassis logical_port=sw0p2 > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[0]] > +---------------------------------------- > +]) > + > +# change the port type to external from router. > +check ovn-nbctl --wait=hv lsp-set-type sw0p2 external > +check_column "" Port_Binding chassis logical_port=sw0p2 > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[0]] > +---------------------------------------- > +]) > + > +# change the port type to localnet from external. > +check ovn-nbctl --wait=hv lsp-set-type sw0p2 localnet > +check_column "" Port_Binding chassis logical_port=sw0p2 > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[0]] > +---------------------------------------- > +]) > + > +# change the port type to localport from localnet. > +check ovn-nbctl --wait=hv lsp-set-type sw0p2 localnet > +check_column "" Port_Binding chassis logical_port=sw0p2 > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[0]] > +---------------------------------------- > +]) > + > +# change the port type back to vif. > +check ovn-nbctl --wait=hv lsp-set-type sw0p2 "" > +wait_column "$hv2_uuid" Port_Binding chassis logical_port=sw0p2 > + > +AT_CHECK([as hv2 ovn-appctl -t ovn-controller debug/dump-local-bindings], > [0], [dnl > +Local bindings: > +name: [[foo]], OVS interface name : [[NULL]], num binding lports : [[1]] > +no primary lport > +child lport[[1]] : [[sw0p1]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p1]], OVS interface name : [[NULL]], num binding lports : [[2]] > +no primary lport > +child lport[[1]] : [[sw0p1-c1]], type : [[CONTAINER]] > +child lport[[2]] : [[sw0p1-c2]], type : [[CONTAINER]] > +---------------------------------------- > +name: [[sw0p2]], OVS interface name : [[hv2-vm1]], num binding lports : [[1]] > +primary lport : [[sw0p2]] > +---------------------------------------- > +]) > + > +OVN_CLEANUP([hv1], [hv2]) > +AT_CLEANUP > +]) > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
