[ovs-dev] [PATCH ovn v10 1/8] ovn-controller: Store the local port bindings in the runtime data I-P state.

Dumitru Ceara dceara at redhat.com
Mon Jun 8 15:34:27 UTC 2020


On 6/8/20 3:49 PM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> This patch adds a new data structure - 'struct local_binding' which represents
> a probable local port binding. A new object of this structure is creared for
> each interface present in the integration bridge (br-int) with the
> external_ids:iface-id set. This struct has 2 main fields
>  - 'struct ovsrec_interface *' and 'struct sbrec_port_binding *'. These fields
> are updated during port claim and release.
> 
> A shash of the local bindings is maintained with the 'iface-id' as the hash key
> in the runtime data of the incremental processing engine.
> 
> This patch helps in the upcoming patch to add incremental processing support
> for OVS interface and SB port binding changes.
> 
> This patch also refactors the port binding code by adding a port binding function
> for each port binding type.
> 
> Acked-by: Mark Michelson <mmichels at redhat.com>
> Signed-off-by: Numan Siddique <numans at ovn.org>

Hi Numan,

I spotted a small typo (please see below) otherwise this patch looks
good to me. With that addressed:

Acked-by: Dumitru Ceara <dceara at redhat.com>

Thanks,
Dumitru

> ---
>  controller/binding.c        | 1020 ++++++++++++++++++++++++-----------
>  controller/binding.h        |   14 +-
>  controller/ovn-controller.c |   48 +-
>  tests/ovn.at                |   36 ++
>  4 files changed, 756 insertions(+), 362 deletions(-)
> 
> diff --git a/controller/binding.c b/controller/binding.c
> index 79dc046d9..6a13d1a0e 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -69,47 +69,6 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type);
>  }
>  
> -static void
> -get_local_iface_ids(const struct ovsrec_bridge *br_int,
> -                    struct shash *lport_to_iface,
> -                    struct sset *local_lports,
> -                    struct sset *egress_ifaces)
> -{
> -    int i;
> -
> -    for (i = 0; i < br_int->n_ports; i++) {
> -        const struct ovsrec_port *port_rec = br_int->ports[i];
> -        const char *iface_id;
> -        int j;
> -
> -        if (!strcmp(port_rec->name, br_int->name)) {
> -            continue;
> -        }
> -
> -        for (j = 0; j < port_rec->n_interfaces; j++) {
> -            const struct ovsrec_interface *iface_rec;
> -
> -            iface_rec = port_rec->interfaces[j];
> -            iface_id = smap_get(&iface_rec->external_ids, "iface-id");
> -            int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
> -
> -            if (iface_id && ofport > 0) {
> -                shash_add(lport_to_iface, iface_id, iface_rec);
> -                sset_add(local_lports, iface_id);
> -            }
> -
> -            /* Check if this is a tunnel interface. */
> -            if (smap_get(&iface_rec->options, "remote_ip")) {
> -                const char *tunnel_iface
> -                    = smap_get(&iface_rec->status, "tunnel_egress_iface");
> -                if (tunnel_iface) {
> -                    sset_add(egress_ifaces, tunnel_iface);
> -                }
> -            }
> -        }
> -    }
> -}
> -
>  static void
>  add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
>                       struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> @@ -403,13 +362,12 @@ destroy_qos_map(struct hmap *qos_map)
>  
>  static void
>  update_local_lport_ids(struct sset *local_lport_ids,
> -                       const struct sbrec_port_binding *binding_rec)
> +                       const struct sbrec_port_binding *pb)
>  {
> -        char buf[16];
> -        snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
> -                 binding_rec->datapath->tunnel_key,
> -                 binding_rec->tunnel_key);
> -        sset_add(local_lport_ids, buf);
> +    char buf[16];
> +    snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
> +             pb->datapath->tunnel_key, pb->tunnel_key);
> +    sset_add(local_lport_ids, buf);
>  }
>  
>  /*
> @@ -448,219 +406,6 @@ sbrec_get_port_encap(const struct sbrec_chassis *chassis_rec,
>      return best_encap;
>  }
>  
> -static bool
> -is_our_chassis(const struct sbrec_chassis *chassis_rec,
> -               const struct sbrec_port_binding *binding_rec,
> -               const struct sset *active_tunnels,
> -               const struct ovsrec_interface *iface_rec,
> -               const struct sset *local_lports)
> -{
> -    /* Ports of type "virtual" should never be explicitly bound to an OVS
> -     * port in the integration bridge. If that's the case, ignore the binding
> -     * and log a warning.
> -     */
> -    if (iface_rec && !strcmp(binding_rec->type, "virtual")) {
> -        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> -        VLOG_WARN_RL(&rl,
> -                     "Virtual port %s should not be bound to OVS port %s",
> -                     binding_rec->logical_port, iface_rec->name);
> -        return false;
> -    }
> -
> -    bool our_chassis = false;
> -    if (iface_rec
> -        || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> -            sset_contains(local_lports, binding_rec->parent_port))) {
> -        /* This port is in our chassis unless it is a localport. */
> -        our_chassis = strcmp(binding_rec->type, "localport");
> -    } else if (!strcmp(binding_rec->type, "l2gateway")) {
> -        const char *chassis_id = smap_get(&binding_rec->options,
> -                                          "l2gateway-chassis");
> -        our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
> -    } else if (!strcmp(binding_rec->type, "chassisredirect") ||
> -               !strcmp(binding_rec->type, "external")) {
> -        our_chassis = ha_chassis_group_contains(binding_rec->ha_chassis_group,
> -                                                chassis_rec) &&
> -                      ha_chassis_group_is_active(binding_rec->ha_chassis_group,
> -                                                 active_tunnels, chassis_rec);
> -    } else if (!strcmp(binding_rec->type, "l3gateway")) {
> -        const char *chassis_id = smap_get(&binding_rec->options,
> -                                          "l3gateway-chassis");
> -        our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
> -    }
> -
> -    return our_chassis;
> -}
> -
> -/* Updates 'binding_rec' and if the port binding is local also updates the
> - * local datapaths and ports.
> - * Updates and returns the array of local virtual ports that will require
> - * additional processing.
> - */
> -static const struct sbrec_port_binding **
> -consider_local_datapath(const struct sbrec_port_binding *binding_rec,
> -                        const struct ovsrec_interface *iface_rec,
> -                        struct binding_ctx_in *b_ctx_in,
> -                        struct binding_ctx_out *b_ctx_out,
> -                        struct hmap *qos_map,
> -                        const struct sbrec_port_binding **vpbs,
> -                        size_t *n_vpbs, size_t *n_allocated_vpbs)
> -{
> -    bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, binding_rec,
> -                                      b_ctx_in->active_tunnels, iface_rec,
> -                                      b_ctx_out->local_lports);
> -    if (iface_rec
> -        || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> -            sset_contains(b_ctx_out->local_lports,
> -                          binding_rec->parent_port))) {
> -        if (binding_rec->parent_port && binding_rec->parent_port[0]) {
> -            /* Add child logical port to the set of all local ports. */
> -            sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> -        }
> -        add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> -                           b_ctx_in->sbrec_port_binding_by_datapath,
> -                           b_ctx_in->sbrec_port_binding_by_name,
> -                           binding_rec->datapath, false,
> -                           b_ctx_out->local_datapaths);
> -        if (iface_rec && qos_map && b_ctx_in->ovs_idl_txn) {
> -            get_qos_params(binding_rec, qos_map);
> -        }
> -    } else if (!strcmp(binding_rec->type, "l2gateway")) {
> -        if (our_chassis) {
> -            sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> -            add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> -                               b_ctx_in->sbrec_port_binding_by_datapath,
> -                               b_ctx_in->sbrec_port_binding_by_name,
> -                               binding_rec->datapath, false,
> -                               b_ctx_out->local_datapaths);
> -        }
> -    } else if (!strcmp(binding_rec->type, "chassisredirect")) {
> -        if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
> -                                      b_ctx_in->chassis_rec)) {
> -            add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> -                               b_ctx_in->sbrec_port_binding_by_datapath,
> -                               b_ctx_in->sbrec_port_binding_by_name,
> -                               binding_rec->datapath, false,
> -                               b_ctx_out->local_datapaths);
> -        }
> -    } else if (!strcmp(binding_rec->type, "l3gateway")) {
> -        if (our_chassis) {
> -            add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> -                               b_ctx_in->sbrec_port_binding_by_datapath,
> -                               b_ctx_in->sbrec_port_binding_by_name,
> -                               binding_rec->datapath, true,
> -                               b_ctx_out->local_datapaths);
> -        }
> -    } else if (!strcmp(binding_rec->type, "localnet")) {
> -        /* Add all localnet ports to local_lports so that we allocate ct zones
> -         * for them. */
> -        sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> -        if (qos_map && b_ctx_in->ovs_idl_txn) {
> -            get_qos_params(binding_rec, qos_map);
> -        }
> -    } else if (!strcmp(binding_rec->type, "external")) {
> -        if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
> -                                      b_ctx_in->chassis_rec)) {
> -            add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> -                               b_ctx_in->sbrec_port_binding_by_datapath,
> -                               b_ctx_in->sbrec_port_binding_by_name,
> -                               binding_rec->datapath, false,
> -                               b_ctx_out->local_datapaths);
> -        }
> -    }
> -
> -    if (our_chassis
> -        || !strcmp(binding_rec->type, "patch")
> -        || !strcmp(binding_rec->type, "localport")
> -        || !strcmp(binding_rec->type, "vtep")
> -        || !strcmp(binding_rec->type, "localnet")) {
> -        update_local_lport_ids(b_ctx_out->local_lport_ids, binding_rec);
> -    }
> -
> -    ovs_assert(b_ctx_in->ovnsb_idl_txn);
> -    const char *vif_chassis = smap_get(&binding_rec->options,
> -                                           "requested-chassis");
> -    bool can_bind = !vif_chassis || !vif_chassis[0]
> -                    || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name)
> -                    || !strcmp(vif_chassis, b_ctx_in->chassis_rec->hostname);
> -
> -    if (can_bind && our_chassis) {
> -        if (binding_rec->chassis != b_ctx_in->chassis_rec) {
> -            if (binding_rec->chassis) {
> -                VLOG_INFO("Changing chassis for lport %s from %s to %s.",
> -                          binding_rec->logical_port,
> -                          binding_rec->chassis->name,
> -                          b_ctx_in->chassis_rec->name);
> -            } else {
> -                VLOG_INFO("Claiming lport %s for this chassis.",
> -                          binding_rec->logical_port);
> -            }
> -            for (int i = 0; i < binding_rec->n_mac; i++) {
> -                VLOG_INFO("%s: Claiming %s",
> -                          binding_rec->logical_port, binding_rec->mac[i]);
> -            }
> -            sbrec_port_binding_set_chassis(binding_rec, b_ctx_in->chassis_rec);
> -        }
> -        /* Check if the port encap binding, if any, has changed */
> -        struct sbrec_encap *encap_rec =
> -            sbrec_get_port_encap(b_ctx_in->chassis_rec, iface_rec);
> -        if (encap_rec && binding_rec->encap != encap_rec) {
> -            sbrec_port_binding_set_encap(binding_rec, encap_rec);
> -        }
> -    } else if (binding_rec->chassis == b_ctx_in->chassis_rec) {
> -        if (!strcmp(binding_rec->type, "virtual")) {
> -            if (*n_vpbs == *n_allocated_vpbs) {
> -                vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof *vpbs);
> -            }
> -            vpbs[(*n_vpbs)] = binding_rec;
> -            (*n_vpbs)++;
> -        } else {
> -            VLOG_INFO("Releasing lport %s from this chassis.",
> -                      binding_rec->logical_port);
> -            if (binding_rec->encap) {
> -                sbrec_port_binding_set_encap(binding_rec, NULL);
> -            }
> -            sbrec_port_binding_set_chassis(binding_rec, NULL);
> -        }
> -    } else if (our_chassis) {
> -        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> -        VLOG_INFO_RL(&rl,
> -                     "Not claiming lport %s, chassis %s "
> -                     "requested-chassis %s",
> -                     binding_rec->logical_port, b_ctx_in->chassis_rec->name,
> -                     vif_chassis);
> -    }
> -
> -    return vpbs;
> -}
> -
> -static void
> -consider_local_virtual_port(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> -                            const struct sbrec_chassis *chassis_rec,
> -                            const struct sbrec_port_binding *binding_rec)
> -{
> -    if (binding_rec->virtual_parent) {
> -        const struct sbrec_port_binding *parent =
> -            lport_lookup_by_name(sbrec_port_binding_by_name,
> -                                 binding_rec->virtual_parent);
> -        if (parent && parent->chassis == chassis_rec) {
> -            return;
> -        }
> -    }
> -
> -    /* pinctrl module takes care of binding the ports of type 'virtual'.
> -     * Release such ports if their virtual parents are no longer claimed by
> -     * this chassis.
> -     */
> -    VLOG_INFO("Releasing lport %s from this chassis.",
> -              binding_rec->logical_port);
> -    if (binding_rec->encap) {
> -        sbrec_port_binding_set_encap(binding_rec, NULL);
> -    }
> -    sbrec_port_binding_set_chassis(binding_rec, NULL);
> -    sbrec_port_binding_set_virtual_parent(binding_rec, NULL);
> -}
> -
>  static void
>  add_localnet_egress_interface_mappings(
>          const struct sbrec_port_binding *port_binding,
> @@ -736,6 +481,585 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec,
>      ld->localnet_port = binding_rec;
>  }
>  
> +/* 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
> + * external_ids:iface-id=<logical_port name> set.
> + *
> + * This module also manages the other port_bindings.
> + *
> + * To better manage the local bindings with the associated OVS interfaces,
> + * '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 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.
> + *
> + *  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.
> + *
> + */
> +enum local_binding_type {
> +    BT_VIF,
> +    BT_CONTAINER,
> +    BT_VIRTUAL
> +};
> +
> +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;
> +};
> +
> +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);
> +}
> +
> +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_destroy(struct local_binding *lbinding)
> +{
> +    local_bindings_destroy(&lbinding->children);
> +
> +    free(lbinding->name);
> +    free(lbinding);
> +}
> +
> +void
> +local_bindings_init(struct shash *local_bindings)
> +{
> +    shash_init(local_bindings);
> +}
> +
> +void
> +local_bindings_destroy(struct shash *local_bindings)
> +{
> +    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);
> +}
> +
> +static void
> +local_binding_add_child(struct local_binding *lbinding,
> +                        struct local_binding *child)
> +{
> +    local_binding_add(&lbinding->children, child);
> +}
> +
> +static struct local_binding *
> +local_binding_find_child(struct local_binding *lbinding,
> +                         const char *child_name)
> +{
> +    return local_binding_find(&lbinding->children, child_name);
> +}
> +
> +static bool
> +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];
> +}
> +
> +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)) {
> +        return LP_VIF;
> +    } else if (!strcmp(pb->type, "patch")) {
> +        return LP_PATCH;
> +    } else if (!strcmp(pb->type, "chassisredirect")) {
> +        return LP_CHASSISREDIRECT;
> +    } else if (!strcmp(pb->type, "l3gateway")) {
> +        return LP_L3GATEWAY;
> +    } else if (!strcmp(pb->type, "localnet")) {
> +        return LP_LOCALNET;
> +    } else if (!strcmp(pb->type, "localport")) {
> +        return LP_LOCALPORT;
> +    } else if (!strcmp(pb->type, "l2gateway")) {
> +        return LP_L2GATEWAY;
> +    } else if (!strcmp(pb->type, "virtual")) {
> +        return LP_VIRTUAL;
> +    } else if (!strcmp(pb->type, "external")) {
> +        return LP_EXTERNAL;
> +    } else if (!strcmp(pb->type, "remote")) {
> +        return LP_REMOTE;
> +    } else if (!strcmp(pb->type, "vtep")) {
> +        return LP_VTEP;
> +    }
> +
> +    return LP_UNKNOWN;
> +}
> +
> +static void
> +claim_lport(const struct sbrec_port_binding *pb,
> +            const struct sbrec_chassis *chassis_rec,
> +            const struct ovsrec_interface *iface_rec)
> +{
> +    if (pb->chassis != chassis_rec) {
> +        if (pb->chassis) {
> +            VLOG_INFO("Changing chassis for lport %s from %s to %s.",
> +                    pb->logical_port, pb->chassis->name,
> +                    chassis_rec->name);
> +        } else {
> +            VLOG_INFO("Claiming lport %s for this chassis.", pb->logical_port);
> +        }
> +        for (int i = 0; i < pb->n_mac; i++) {
> +            VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]);
> +        }
> +
> +        sbrec_port_binding_set_chassis(pb, chassis_rec);
> +    }
> +
> +    /* Check if the port encap binding, if any, has changed */
> +    struct sbrec_encap *encap_rec =
> +        sbrec_get_port_encap(chassis_rec, iface_rec);
> +    if (encap_rec && pb->encap != encap_rec) {
> +        sbrec_port_binding_set_encap(pb, encap_rec);
> +    }
> +}
> +
> +static void
> +release_lport(const struct sbrec_port_binding *pb)
> +{
> +    VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port);
> +    if (pb->encap) {
> +        sbrec_port_binding_set_encap(pb, NULL);
> +    }
> +    sbrec_port_binding_set_chassis(pb, NULL);
> +
> +    if (pb->virtual_parent) {
> +        sbrec_port_binding_set_virtual_parent(pb, NULL);
> +    }
> +}
> +
> +static bool
> +is_lbinding_set(struct local_binding *lbinding)
> +{
> +    return lbinding && lbinding->pb && lbinding->iface;
> +}
> +
> +static bool
> +is_lbinding_this_chassis(struct local_binding *lbinding,
> +                         const struct sbrec_chassis *chassis)
> +{
> +    return lbinding && lbinding->pb && lbinding->pb->chassis == chassis;
> +}
> +
> +static bool
> +can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec,
> +                         const char *requested_chassis)
> +{
> +    return !requested_chassis || !requested_chassis[0]
> +           || !strcmp(requested_chassis, chassis_rec->name)
> +           || !strcmp(requested_chassis, chassis_rec->hostname);
> +}
> +
> +static void
> +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 hmap *qos_map)
> +{
> +    bool lbinding_set = is_lbinding_set(lbinding);
> +    if (lbinding_set) {
> +        if (can_bind) {
> +            /* We can claim the lport. */
> +            claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface);
> +
> +            add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> +                               b_ctx_in->sbrec_port_binding_by_datapath,
> +                               b_ctx_in->sbrec_port_binding_by_name,
> +                               pb->datapath, false,
> +                               b_ctx_out->local_datapaths);
> +            update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
> +            if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) {
> +                get_qos_params(pb, qos_map);
> +            }
> +        } else {
> +            /* We could, but can't claim the lport. */
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +                VLOG_INFO_RL(&rl,
> +                             "Not claiming lport %s, chassis %s "
> +                             "requested-chassis %s",
> +                             pb->logical_port,
> +                             b_ctx_in->chassis_rec->name,
> +                             vif_chassis);
> +        }
> +    }
> +
> +    if (pb->chassis == b_ctx_in->chassis_rec) {
> +        /* Release the lport if there is no lbinding. */
> +        if (!lbinding_set || !can_bind) {
> +            release_lport(pb);
> +        }
> +    }
> +
> +}
> +
> +static void
> +consider_vif_lport(const struct sbrec_port_binding *pb,
> +                   struct binding_ctx_in *b_ctx_in,
> +                   struct binding_ctx_out *b_ctx_out,
> +                   struct local_binding *lbinding,
> +                   struct hmap *qos_map)
> +{
> +    const char *vif_chassis = smap_get(&pb->options, "requested-chassis");
> +    bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
> +                                             vif_chassis);
> +
> +    if (!lbinding) {
> +        lbinding = local_binding_find(b_ctx_out->local_bindings,
> +                                      pb->logical_port);
> +    }
> +
> +    if (lbinding) {
> +        lbinding->pb = pb;
> +    }
> +
> +    consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
> +                        b_ctx_out, lbinding, qos_map);
> +}
> +
> +static void
> +consider_container_lport(const struct sbrec_port_binding *pb,
> +                         struct binding_ctx_in *b_ctx_in,
> +                         struct binding_ctx_out *b_ctx_out,
> +                         struct hmap *qos_map)
> +{
> +    struct local_binding *parent_lbinding;
> +    parent_lbinding = local_binding_find(b_ctx_out->local_bindings,
> +                                         pb->parent_port);
> +
> +    if (parent_lbinding && !parent_lbinding->pb) {
> +        parent_lbinding->pb = lport_lookup_by_name(
> +            b_ctx_in->sbrec_port_binding_by_name, pb->parent_port);
> +
> +        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);
> +        }
> +    }
> +
> +    if (!parent_lbinding || !parent_lbinding->pb) {
> +        /* Call release_lport, to release the container lport, if
> +         * it was bound earlier. */
> +        if (pb->chassis == b_ctx_in->chassis_rec) {
> +            release_lport(pb);
> +        }
> +        return;
> +    }
> +
> +    struct local_binding *container_lbinding =
> +        local_binding_find_child(parent_lbinding, pb->logical_port);
> +    ovs_assert(!container_lbinding);
> +
> +    container_lbinding = local_binding_create(pb->logical_port,
> +                                              parent_lbinding->iface,
> +                                              pb, BT_CONTAINER);
> +    local_binding_add_child(parent_lbinding, container_lbinding);
> +
> +    const char *vif_chassis = smap_get(&parent_lbinding->pb->options,
> +                                       "requested-chassis");
> +    bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
> +                                             vif_chassis);
> +
> +    consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out,
> +                        container_lbinding, qos_map);
> +}
> +
> +static void
> +consider_virtual_lport(const struct sbrec_port_binding *pb,
> +                       struct binding_ctx_in *b_ctx_in,
> +                       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,
> +                                                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 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);
> +        ovs_assert(!virtual_lbinding);
> +        virtual_lbinding = local_binding_create(pb->logical_port,
> +                                                parent_lbinding->iface,
> +                                                pb, BT_VIRTUAL);
> +        local_binding_add_child(parent_lbinding, virtual_lbinding);
> +    }
> +
> +    return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out,
> +                               virtual_lbinding, qos_map);
> +}
> +
> +/* Considers either claiming the lport or releasing the lport
> + * for non VIF lports.
> + */
> +static void
> +consider_nonvif_lport_(const struct sbrec_port_binding *pb,
> +                       bool our_chassis,
> +                       bool has_local_l3gateway,
> +                       struct binding_ctx_in *b_ctx_in,
> +                       struct binding_ctx_out *b_ctx_out)
> +{
> +    ovs_assert(b_ctx_in->ovnsb_idl_txn);
> +    if (our_chassis) {
> +        sset_add(b_ctx_out->local_lports, pb->logical_port);
> +        add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> +                           b_ctx_in->sbrec_port_binding_by_datapath,
> +                           b_ctx_in->sbrec_port_binding_by_name,
> +                           pb->datapath, has_local_l3gateway,
> +                           b_ctx_out->local_datapaths);
> +
> +        update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
> +        claim_lport(pb, b_ctx_in->chassis_rec, NULL);
> +    } else if (pb->chassis == b_ctx_in->chassis_rec) {
> +        release_lport(pb);
> +    }
> +}
> +
> +static void
> +consider_l2gw_lport(const struct sbrec_port_binding *pb,
> +                    struct binding_ctx_in *b_ctx_in,
> +                    struct binding_ctx_out *b_ctx_out)
> +{
> +    const char *chassis_id = smap_get(&pb->options, "l2gateway-chassis");
> +    bool our_chassis = chassis_id && !strcmp(chassis_id,
> +                                             b_ctx_in->chassis_rec->name);
> +
> +    consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out);
> +}
> +
> +static void
> +consider_l3gw_lport(const struct sbrec_port_binding *pb,
> +                    struct binding_ctx_in *b_ctx_in,
> +                    struct binding_ctx_out *b_ctx_out)
> +{
> +    const char *chassis_id = smap_get(&pb->options, "l3gateway-chassis");
> +    bool our_chassis = chassis_id && !strcmp(chassis_id,
> +                                             b_ctx_in->chassis_rec->name);
> +
> +    consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out);
> +}
> +
> +static void
> +consider_localnet_lport(const struct sbrec_port_binding *pb,
> +                        struct binding_ctx_in *b_ctx_in,
> +                        struct binding_ctx_out *b_ctx_out,
> +                        struct hmap *qos_map)
> +{
> +    /* Add all localnet ports to local_lports so that we allocate ct zones
> +     * for them. */
> +    sset_add(b_ctx_out->local_lports, pb->logical_port);
> +    if (qos_map && b_ctx_in->ovs_idl_txn) {
> +        get_qos_params(pb, qos_map);
> +    }
> +
> +    update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
> +}
> +
> +static void
> +consider_ha_lport(const struct sbrec_port_binding *pb,
> +                  struct binding_ctx_in *b_ctx_in,
> +                  struct binding_ctx_out *b_ctx_out)
> +{
> +    bool our_chassis = false;
> +    bool is_ha_chassis = ha_chassis_group_contains(pb->ha_chassis_group,
> +                                                   b_ctx_in->chassis_rec);
> +    our_chassis = is_ha_chassis &&
> +                  ha_chassis_group_is_active(pb->ha_chassis_group,
> +                                             b_ctx_in->active_tunnels,
> +                                             b_ctx_in->chassis_rec);
> +
> +    if (is_ha_chassis && !our_chassis) {
> +        /* If the chassis_rec is part of ha_chassis_group associated with
> +         * the port_binding 'pb', we need to add to the local_datapath
> +         * in even if its not active.
> +         *
> +         * If the chassis is active, consider_nonvif_lport_() takes care
> +         * of adding the datapath of this 'pb' to local datapaths.
> +         * */
> +        add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> +                           b_ctx_in->sbrec_port_binding_by_datapath,
> +                           b_ctx_in->sbrec_port_binding_by_name,
> +                           pb->datapath, false,
> +                           b_ctx_out->local_datapaths);
> +    }
> +
> +    consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out);
> +}
> +
> +static void
> +consider_cr_lport(const struct sbrec_port_binding *pb,
> +                  struct binding_ctx_in *b_ctx_in,
> +                  struct binding_ctx_out *b_ctx_out)
> +{
> +    consider_ha_lport(pb, b_ctx_in, b_ctx_out);
> +}
> +
> +static void
> +consider_external_lport(const struct sbrec_port_binding *pb,
> +                        struct binding_ctx_in *b_ctx_in,
> +                        struct binding_ctx_out *b_ctx_out)
> +{
> +    return consider_ha_lport(pb, b_ctx_in, b_ctx_out);
> +}
> +
> +/*
> + * Builds local_bindings from the OVS interfaces.
> + */
> +static void
> +build_local_bindings(struct binding_ctx_in *b_ctx_in,
> +                     struct binding_ctx_out *b_ctx_out)
> +{
> +    int i;
> +    for (i = 0; i < b_ctx_in->br_int->n_ports; i++) {
> +        const struct ovsrec_port *port_rec = b_ctx_in->br_int->ports[i];
> +        const char *iface_id;
> +        int j;
> +
> +        if (!strcmp(port_rec->name, b_ctx_in->br_int->name)) {
> +            continue;
> +        }
> +
> +        for (j = 0; j < port_rec->n_interfaces; j++) {
> +            const struct ovsrec_interface *iface_rec;
> +
> +            iface_rec = port_rec->interfaces[j];
> +            iface_id = smap_get(&iface_rec->external_ids, "iface-id");
> +            int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
> +
> +            if (iface_id && ofport > 0) {
> +                struct local_binding *lbinding =
> +                    local_binding_find(b_ctx_out->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);
> +                } else {
> +                    static struct vlog_rate_limit rl =
> +                        VLOG_RATE_LIMIT_INIT(1, 5);
> +                    VLOG_WARN_RL(
> +                        &rl,
> +                        "Invalid configuration: iface-id is configured on "
> +                        "interfaces : [%s] and [%s]. Ignoring the "
> +                        "configuration on interface [%s]",
> +                        lbinding->iface->name, iface_rec->name,
> +                        iface_rec->name);
> +                    ovs_assert(lbinding->type == BT_VIF);
> +                }
> +
> +                sset_add(b_ctx_out->local_lports, iface_id);
> +            }
> +
> +            /* Check if this is a tunnel interface. */
> +            if (smap_get(&iface_rec->options, "remote_ip")) {
> +                const char *tunnel_iface
> +                    = smap_get(&iface_rec->status, "tunnel_egress_iface");
> +                if (tunnel_iface) {
> +                    sset_add(b_ctx_out->egress_ifaces, tunnel_iface);
> +                }
> +            }
> +        }
> +    }
> +}
> +
>  void
>  binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
>  {
> @@ -743,106 +1067,135 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
>          return;
>      }
>  
> -    const struct sbrec_port_binding *binding_rec;
>      struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
> -    struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
> -    struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
>      struct hmap qos_map;
>  
>      hmap_init(&qos_map);
>      if (b_ctx_in->br_int) {
> -        get_local_iface_ids(b_ctx_in->br_int, &lport_to_iface,
> -                            b_ctx_out->local_lports, &egress_ifaces);
> +        build_local_bindings(b_ctx_in, b_ctx_out);
>      }
>  
> -    /* Array to store pointers to local virtual ports. It is populated by
> -     * consider_local_datapath.
> -     */
> -    const struct sbrec_port_binding **vpbs = NULL;
> -    size_t n_vpbs = 0;
> -    size_t n_allocated_vpbs = 0;
> +    struct hmap *qos_map_ptr =
> +        !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL;
> +
> +    struct ovs_list localnet_lports = OVS_LIST_INITIALIZER(&localnet_lports);
> +
> +    struct localnet_lport {
> +        struct ovs_list list_node;
> +        const struct sbrec_port_binding *pb;
> +    };
>  
>      /* Run through each binding record to see if it is resident on this
>       * chassis and update the binding accordingly.  This includes both
> -     * directly connected logical ports and children of those ports.
> -     * Virtual ports are just added to vpbs array and will be processed
> -     * later. This is special case for virtual ports is needed in order to
> -     * make sure we update the virtual_parent port bindings first.
> +     * directly connected logical ports and children of those ports
> +     * (which also includes virtual ports).
>       */
> -    SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
> +    const struct sbrec_port_binding *pb;
> +    SBREC_PORT_BINDING_TABLE_FOR_EACH (pb,
>                                         b_ctx_in->port_binding_table) {
> -        const struct ovsrec_interface *iface_rec
> -            = shash_find_data(&lport_to_iface, binding_rec->logical_port);
> -        vpbs =
> -            consider_local_datapath(binding_rec,
> -                                    iface_rec, b_ctx_in, b_ctx_out,
> -                                    sset_is_empty(&egress_ifaces) ? NULL :
> -                                    &qos_map,
> -                                    vpbs, &n_vpbs, &n_allocated_vpbs);
> -    }
> -
> -    /* Now also update the virtual ports in case their parent ports were
> -     * updated above.
> -     */
> -    for (size_t i = 0; i < n_vpbs; i++) {
> -        consider_local_virtual_port(b_ctx_in->sbrec_port_binding_by_name,
> -                                    b_ctx_in->chassis_rec, vpbs[i]);
> +        enum en_lport_type lport_type = get_lport_type(pb);
> +
> +        switch (lport_type) {
> +        case LP_PATCH:
> +        case LP_LOCALPORT:
> +        case LP_VTEP:
> +            update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
> +            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);
> +            }
> +            break;
> +
> +        case LP_VIRTUAL:
> +            consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr);
> +            break;
> +
> +        case LP_L2GATEWAY:
> +            consider_l2gw_lport(pb, b_ctx_in, b_ctx_out);
> +            break;
> +
> +        case LP_L3GATEWAY:
> +            consider_l3gw_lport(pb, b_ctx_in, b_ctx_out);
> +            break;
> +
> +        case LP_CHASSISREDIRECT:
> +            consider_cr_lport(pb, b_ctx_in, b_ctx_out);
> +            break;
> +
> +        case LP_EXTERNAL:
> +            consider_external_lport(pb, b_ctx_in, b_ctx_out);
> +            break;
> +
> +        case LP_LOCALNET: {
> +            consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr);
> +            struct localnet_lport *lnet_lport = xmalloc(sizeof *lnet_lport);
> +            lnet_lport->pb = pb;
> +            ovs_list_push_back(&localnet_lports, &lnet_lport->list_node);
> +            break;
> +        }
> +
> +        case LP_REMOTE:
> +            /* Nothing to be done for REMOTE type. */
> +            break;
> +
> +        case LP_UNKNOWN: {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +            VLOG_WARN_RL(&rl,
> +                         "Unknown port binding type [%s] for port binding "
> +                         "[%s]. Does ovn-controller needs update ?",

Typo: s/needs update/need update.

> +                         pb->type, pb->logical_port);
> +            break;
> +        }
> +        }
>      }
> -    free(vpbs);
>  
>      add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table,
>                              &bridge_mappings);
>  
> -    /* Run through each binding record to see if it is a localnet port
> +    /* Run through each localnet lport list to see if it is a localnet port
>       * on local datapaths discovered from above loop, and update the
>       * corresponding local datapath accordingly. */
> -    SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
> -                                       b_ctx_in->port_binding_table) {
> -        if (!strcmp(binding_rec->type, "localnet")) {
> -            consider_localnet_port(binding_rec, &bridge_mappings,
> -                                   &egress_ifaces, b_ctx_out->local_datapaths);
> -        }
> +    struct localnet_lport *lnet_lport;
> +    LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) {
> +        consider_localnet_port(lnet_lport->pb, &bridge_mappings,
> +                               b_ctx_out->egress_ifaces,
> +                               b_ctx_out->local_datapaths);
> +        free(lnet_lport);
>      }
> +
>      shash_destroy(&bridge_mappings);
>  
> -    if (!sset_is_empty(&egress_ifaces)
> +    if (!sset_is_empty(b_ctx_out->egress_ifaces)
>          && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table,
> -                        b_ctx_in->qos_table, &egress_ifaces)) {
> +                        b_ctx_in->qos_table, b_ctx_out->egress_ifaces)) {
>          const char *entry;
> -        SSET_FOR_EACH (entry, &egress_ifaces) {
> +        SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
>              setup_qos(entry, &qos_map);
>          }
>      }
>  
> -    shash_destroy(&lport_to_iface);
> -    sset_destroy(&egress_ifaces);
>      destroy_qos_map(&qos_map);
>  }
>  
>  /* Returns true if port-binding changes potentially require flow changes on
>   * the current chassis. Returns false if we are sure there is no impact. */
>  bool
> -binding_evaluate_port_binding_changes(
> -        const struct sbrec_port_binding_table *pb_table,
> -        const struct ovsrec_bridge *br_int,
> -        const struct sbrec_chassis *chassis_rec,
> -        struct sset *active_tunnels,
> -        struct sset *local_lports)
> +binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in,
> +                                      struct binding_ctx_out *b_ctx_out)
>  {
> -    if (!chassis_rec) {
> +    if (!b_ctx_in->chassis_rec) {
>          return true;
>      }
>  
>      bool changed = false;
>  
>      const struct sbrec_port_binding *binding_rec;
> -    struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
> -    struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
> -    if (br_int) {
> -        get_local_iface_ids(br_int, &lport_to_iface, local_lports,
> -                            &egress_ifaces);
> -    }
> -    SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, pb_table) {
> +    SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec,
> +                                               b_ctx_in->port_binding_table) {
>          /* XXX: currently OVSDB change tracking doesn't support getting old
>           * data when the operation is update, so if a port-binding moved from
>           * this chassis to another, there is no easy way to find out the
> @@ -854,24 +1207,35 @@ binding_evaluate_port_binding_changes(
>           *   interface table will be updated, which will trigger recompute.
>           *
>           * - If the port is not a regular VIF, always trigger recompute. */
> -        if (binding_rec->chassis == chassis_rec) {
> +        if (binding_rec->chassis == b_ctx_in->chassis_rec) {
>              changed = true;
>              break;
>          }
>  
> -        const struct ovsrec_interface *iface_rec
> -            = shash_find_data(&lport_to_iface, binding_rec->logical_port);
> -        if (is_our_chassis(chassis_rec, binding_rec, active_tunnels, iface_rec,
> -                           local_lports) ||
> -            (strcmp(binding_rec->type, "") && strcmp(binding_rec->type,
> -                                                     "remote"))) {
> +        if (!strcmp(binding_rec->type, "remote")) {
> +            continue;
> +        }
> +
> +        if (strcmp(binding_rec->type, "")) {
> +            changed = true;
> +            break;
> +        }
> +
> +        struct local_binding *lbinding = NULL;
> +        if (!binding_rec->parent_port || !binding_rec->parent_port[0]) {
> +            lbinding = local_binding_find(b_ctx_out->local_bindings,
> +                                          binding_rec->logical_port);
> +        } else {
> +            lbinding = local_binding_find(b_ctx_out->local_bindings,
> +                                          binding_rec->parent_port);
> +        }
> +
> +        if (lbinding) {
>              changed = true;
>              break;
>          }
>      }
>  
> -    shash_destroy(&lport_to_iface);
> -    sset_destroy(&egress_ifaces);
>      return changed;
>  }
>  
> diff --git a/controller/binding.h b/controller/binding.h
> index d0b8331af..9affc9a96 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -31,6 +31,8 @@ struct ovsrec_open_vswitch_table;
>  struct sbrec_chassis;
>  struct sbrec_port_binding_table;
>  struct sset;
> +struct sbrec_port_binding;
> +struct shash;
>  
>  struct binding_ctx_in {
>      struct ovsdb_idl_txn *ovnsb_idl_txn;
> @@ -51,8 +53,10 @@ struct binding_ctx_in {
>  
>  struct binding_ctx_out {
>      struct hmap *local_datapaths;
> +    struct shash *local_bindings;
>      struct sset *local_lports;
>      struct sset *local_lport_ids;
> +    struct sset *egress_ifaces;
>  };
>  
>  void binding_register_ovs_idl(struct ovsdb_idl *);
> @@ -60,11 +64,9 @@ void binding_run(struct binding_ctx_in *, struct binding_ctx_out *);
>  bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                       const struct sbrec_port_binding_table *,
>                       const struct sbrec_chassis *);
> -bool binding_evaluate_port_binding_changes(
> -        const struct sbrec_port_binding_table *,
> -        const struct ovsrec_bridge *br_int,
> -        const struct sbrec_chassis *,
> -        struct sset *active_tunnels,
> -        struct sset *local_lports);
> +bool binding_evaluate_port_binding_changes(struct binding_ctx_in *,
> +                                           struct binding_ctx_out *);
>  
> +void local_bindings_init(struct shash *local_bindings);
> +void local_bindings_destroy(struct shash *local_bindings);
>  #endif /* controller/binding.h */
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 1cbdda1a9..61d34157a 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -957,6 +957,9 @@ struct ed_type_runtime_data {
>      /* Contains "struct local_datapath" nodes. */
>      struct hmap local_datapaths;
>  
> +    /* Contains "struct local_binding" nodes. */
> +    struct shash local_bindings;
> +
>      /* Contains the name of each logical port resident on the local
>       * hypervisor.  These logical ports include the VIFs (and their child
>       * logical ports, if any) that belong to VMs running on the hypervisor,
> @@ -968,6 +971,8 @@ struct ed_type_runtime_data {
>       * <datapath-tunnel-key>_<port-tunnel-key> */
>      struct sset local_lport_ids;
>      struct sset active_tunnels;
> +
> +    struct sset egress_ifaces;
>  };
>  
>  static void *
> @@ -980,6 +985,8 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED,
>      sset_init(&data->local_lports);
>      sset_init(&data->local_lport_ids);
>      sset_init(&data->active_tunnels);
> +    sset_init(&data->egress_ifaces);
> +    local_bindings_init(&data->local_bindings);
>      return data;
>  }
>  
> @@ -991,6 +998,7 @@ en_runtime_data_cleanup(void *data)
>      sset_destroy(&rt_data->local_lports);
>      sset_destroy(&rt_data->local_lport_ids);
>      sset_destroy(&rt_data->active_tunnels);
> +    sset_destroy(&rt_data->egress_ifaces);
>      struct local_datapath *cur_node, *next_node;
>      HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
>                          &rt_data->local_datapaths) {
> @@ -999,6 +1007,7 @@ en_runtime_data_cleanup(void *data)
>          free(cur_node);
>      }
>      hmap_destroy(&rt_data->local_datapaths);
> +    local_bindings_destroy(&rt_data->local_bindings);
>  }
>  
>  static void
> @@ -1071,6 +1080,8 @@ init_binding_ctx(struct engine_node *node,
>      b_ctx_out->local_datapaths = &rt_data->local_datapaths;
>      b_ctx_out->local_lports = &rt_data->local_lports;
>      b_ctx_out->local_lport_ids = &rt_data->local_lport_ids;
> +    b_ctx_out->egress_ifaces = &rt_data->egress_ifaces;
> +    b_ctx_out->local_bindings = &rt_data->local_bindings;
>  }
>  
>  static void
> @@ -1094,12 +1105,16 @@ en_runtime_data_run(struct engine_node *node, void *data)
>              free(cur_node);
>          }
>          hmap_clear(local_datapaths);
> +        local_bindings_destroy(&rt_data->local_bindings);
>          sset_destroy(local_lports);
>          sset_destroy(local_lport_ids);
>          sset_destroy(active_tunnels);
> +        sset_destroy(&rt_data->egress_ifaces);
>          sset_init(local_lports);
>          sset_init(local_lport_ids);
>          sset_init(active_tunnels);
> +        sset_init(&rt_data->egress_ifaces);
> +        local_bindings_init(&rt_data->local_bindings);
>      }
>  
>      struct binding_ctx_in b_ctx_in;
> @@ -1128,35 +1143,12 @@ static bool
>  runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
>  {
>      struct ed_type_runtime_data *rt_data = data;
> -    struct sset *local_lports = &rt_data->local_lports;
> -    struct sset *active_tunnels = &rt_data->active_tunnels;
> -
> -    struct ovsrec_open_vswitch_table *ovs_table =
> -        (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
> -            engine_get_input("OVS_open_vswitch", node));
> -    struct ovsrec_bridge_table *bridge_table =
> -        (struct ovsrec_bridge_table *)EN_OVSDB_GET(
> -            engine_get_input("OVS_bridge", node));
> -    const char *chassis_id = chassis_get_id();
> -    const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
> -
> -    ovs_assert(br_int && chassis_id);
> -
> -    struct ovsdb_idl_index *sbrec_chassis_by_name =
> -        engine_ovsdb_node_get_index(
> -                engine_get_input("SB_chassis", node),
> -                "name");
> -
> -    const struct sbrec_chassis *chassis
> -        = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
> -    ovs_assert(chassis);
> -
> -    struct sbrec_port_binding_table *pb_table =
> -        (struct sbrec_port_binding_table *)EN_OVSDB_GET(
> -            engine_get_input("SB_port_binding", node));
> +    struct binding_ctx_in b_ctx_in;
> +    struct binding_ctx_out b_ctx_out;
> +    init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
>  
> -    bool changed = binding_evaluate_port_binding_changes(
> -        pb_table, br_int, chassis, active_tunnels, local_lports);
> +    bool changed = binding_evaluate_port_binding_changes(&b_ctx_in,
> +                                                         &b_ctx_out);
>  
>      return !changed;
>  }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 15b40ca1e..8743e0efb 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -8606,6 +8606,42 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet
>  # expected packet at VM1
>  OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1])
>  
> +# Test binding of parent and container ports.
> +ovn-nbctl lsp-set-options vm1 requested-chassis=foo
> +
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)])
> +
> +ovn-nbctl clear logical_switch_port vm1 options
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)])
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)])
> +
> +as hv1 ovs-vsctl set interface vm1 external_ids:iface-id=foo
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)])
> +
> +as hv1 ovs-vsctl set interface vm1 external_ids:iface-id=vm1
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)])
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)])
> +
> +ovn-nbctl lsp-del vm1
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)])
> +
> +ovn-nbctl lsp-add mgmt vm1
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)])
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)])
> +
> +as hv1 ovs-vsctl del-port vm1
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)])
> +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)])
> +
>  OVN_CLEANUP([hv1],[hv2])
>  
>  AT_CLEANUP
> 



More information about the dev mailing list