[ovs-dev] [PATCH ovn v4] binding: Fix the crashes seen when port binding type changes.
Mark Michelson
mmichels at redhat.com
Wed Mar 24 20:46:15 UTC 2021
Hi Numan,
I have some minor findings below. They're the sort of thing that could
be folded into the patch when you merge it.
I'm not giving an ack on this. The reason isn't because I don't approve,
it's more that I'm not comfortable enough with this code to understand
fully if I should be giving it an ack. The test cases help to make it
seem good though.
I understand the general gist of this: Create a common local_binding
type that then contains a list of binding_lports representing the parent
and child ports. I just don't feel comfortable knowing that every
possible case has been covered.
On 3/23/21 11:55 AM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
>
> 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.
>
> Reported-by: Dumitru Ceara <dceara at redhat.com>
> 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 <dceara at redhat.com>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> Signed-off-by: Numan Siddique <numans at ovn.org>
> ---
>
> 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 | 973 +++++++++++++++++++++++-------------
> controller/binding.h | 32 +-
> controller/ovn-controller.c | 25 +-
> controller/physical.c | 13 +-
> tests/ovn.at | 304 +++++++++++
> 5 files changed, 949 insertions(+), 398 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 4e6c756969..0e3e4aad91 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,116 @@ 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
> + * struct local_binding has 3 main fields:
> + * - name
I may be overly nitpicky here, but I think you should explain that the
name field corresponds to the key described in the previous paragraph.
> * - 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.
> - *
> + * - 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 interface that has iface-id configured.
> *
> - * - 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 '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.
> */
>
> -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;
> -}
> +struct local_binding {
> + char *name;
> + const struct ovsrec_interface *iface;
> + struct ovs_list binding_lports;
> +};
>
> -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 *, enum en_lport_type, struct shash *b_lports);
>
> 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) {
> - struct local_binding *lbinding = node->data;
> - local_binding_destroy(lbinding);
> - shash_delete(local_bindings, node);
> - }
>
> - shash_destroy(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);
> + }
>
> -static
> -void local_binding_delete(struct shash *local_bindings,
> - struct local_binding *lbinding)
> -{
> - shash_find_and_delete(local_bindings, lbinding->name);
> - local_binding_destroy(lbinding);
> -}
> + SHASH_FOR_EACH_SAFE (node, next, &lbinding_data->bindings) {
> + struct local_binding *lbinding = node->data;
> + local_binding_destroy(lbinding, &lbinding_data->lports);
> + shash_delete(&lbinding_data->bindings, node);
> + }
>
> -static void
> -local_binding_add_child(struct local_binding *lbinding,
> - struct local_binding *child)
> -{
> - local_binding_add(&lbinding->children, child);
> - child->parent = lbinding;
> + shash_destroy(&lbinding_data->lports);
> + shash_destroy(&lbinding_data->bindings);
> }
>
> -static struct local_binding *
> -local_binding_find_child(struct local_binding *lbinding,
> - const char *child_name)
> +const struct sbrec_port_binding *
> +local_binding_get_primary_pb(struct shash *local_bindings, const char *name)
> {
> - return local_binding_find(&lbinding->children, child_name);
> -}
> + struct local_binding *lbinding = local_binding_find(local_bindings, name);
> + struct binding_lport *b_lport = local_binding_get_primary_lport(lbinding);
>
> -static void
> -local_binding_delete_child(struct local_binding *lbinding,
> - struct local_binding *child)
> -{
> - shash_find_and_delete(&lbinding->children, child->name);
> + return b_lport ? b_lport->pb : NULL;
> }
>
> static bool
> @@ -818,26 +817,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 (is_lport_container(pb)) {
> + return LP_CONTAINER;
> + }
> return LP_VIF;
> } else if (!strcmp(pb->type, "patch")) {
> return LP_PATCH;
> @@ -991,14 +977,14 @@ 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 && b_lport->pb->chassis == chassis;
> }
>
> static bool
> @@ -1010,15 +996,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 +1012,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) : false;
> +
> 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 +1056,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 +1094,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 +1115,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,40 +1132,35 @@ 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(
> + 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,
> + * 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);
> }
>
> @@ -1212,13 +1168,14 @@ consider_container_lport(const struct sbrec_port_binding *pb,
> }
> }
>
> - 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 +1184,47 @@ consider_virtual_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 =
> - pb->virtual_parent ? local_binding_find(b_ctx_out->local_bindings,
> + 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 +1365,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 +1376,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 +1390,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 +1452,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 +1757,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;
> }
>
> +
> /* 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 +1826,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 +2062,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 +2100,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 +2120,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 +2132,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 +2149,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 +2198,20 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
>
> 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) {
> + binding_lport_check_and_cleanup(
> + b_lport, lport_type,
> + &b_ctx_out->lbinding_data->lports);
> + /* Do not access b_lport after this. It may have been freed. */
Some of the mystery can be removed here by doing:
b_lport = binding_lport_check_and_cleanup();
This way, if b_lport was freed, then it will be NULL, and if it was not
freed it will be non-NULL.
> + }
> +
> 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);
> - }
> + 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 +2222,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 +2276,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 +2439,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 +2479,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 +2514,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 +2530,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 +2542,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 +2563,292 @@ 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 can change. */
> + 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;
> +}
> +
> +static struct binding_lport *
> +binding_lport_check_and_cleanup(struct binding_lport *b_lport,
> + enum en_lport_type lport_type,
> + struct shash *binding_lports)
> +{
> + if (b_lport->type != lport_type) {
> + b_lport->type = lport_type;
> + }
> +
> + 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..b8a1a5d796 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -56,7 +56,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 +86,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 *name);
>
> /* Represents a tracked binding logical port. */
> struct tracked_binding_lport {
> @@ -137,7 +125,7 @@ bool binding_handle_port_binding_changes(struct binding_ctx_in *,
It's somewhere in the fold up there^, but you need to remove the
local_bindings_init() and local_bindings_destroy() function declarations
from binding.h. With this change, they are no longer defined in binding.c
> 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);
> #endif /* controller/binding.h */
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 5dd643f52b..7e00c2e270 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1182,8 +1182,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 +1221,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 +1250,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 +1293,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 +1321,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 +1404,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 +1448,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 +1459,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 +1821,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;
> }
>
> @@ -2955,7 +2954,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);
> @@ -2968,7 +2967,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);
> }
> }
>
> 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.at b/tests/ovn.at
> index b751d6db2e..9a3b5d8225 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -25161,3 +25161,307 @@ 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
> +
> +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
> +])
>
More information about the dev
mailing list