[ovs-dev] [PATCH ovn v8 1/8] ovn-controller: Store the local port bindings in the runtime data I-P state.
Numan Siddique
numans at ovn.org
Thu May 28 07:13:52 UTC 2020
On Wed, May 27, 2020 at 9:15 PM Dumitru Ceara <dceara at redhat.com> wrote:
> On 5/26/20 2:28 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,
>
> Maybe I'm wrong but won't your changes revert commit:
>
> 1cadfb515124 ("ovn-controller: Skip vport bindings done through OVS
> external_ids:iface-id.")
>
> Or at least I don't see the warning being logged anymore.
>
>
It doesn't revert the comment, but it doesn't log the warning anymore. It's
because of the way it is refactored now.
Lets say you have an ovs interface with external_ids:iface-id=sw0-vir
and the logical port 'sw0-vir' is of type 'virtual'.
When binding_run() calls build_local_bindings(), it creates a local_binding
for sw0-vir and the iface rec is set.
Later binding_run() for virtual port binding 'sw0-vir', calls
consider_virtual_lport()
and it looks for the local_binding for its parent. It never looks for
'sw0-vir' in local bindings.
I thought about this warning, but is it worth the lookup in the local
bindings shash to see if there
is a VIF interface for it ?
The VIF will be ignored like any other VIF configured with
external_ids:iface-id but with no
corresponding port binding row in SB DB.
If you want I can put it, but it's a waste of shash lookup for every
virtual port binding.
A few more comments inline.
>
> Thanks,
> Dumitru
>
> > ---
> > controller/binding.c | 1012 ++++++++++++++++++++++++-----------
> > controller/binding.h | 14 +-
> > controller/ovn-controller.c | 48 +-
> > tests/ovn.at | 36 ++
> > 4 files changed, 754 insertions(+), 356 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 79dc046d9..2997fcae8 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,
> > @@ -448,219 +407,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 +482,588 @@ 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 is sees the
>
> Nit: s/it is sees/it sees
>
Ack. Done.
>
> > + * 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 !pb->type[0] && pb->parent_port && pb->parent_port[0];
>
> Nit: I'd probably replace this with:
> 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)
> > +{
> > + if (!pb) {
> > + return;
> > + }
>
> We don't check for NULL pb in claim_lport(). And as far as I can see in
> all places where release_lport() is called the pb can't be NULL. I'd
> either check for NULL pb in both functions or skip the check completely
> as this is a static function and we control the way it's called.
>
I will remove the check for NULL in release_lport.
> > +
> > + 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);
>
> Indentation: this should be 4 spaces to the left.
>
I'll align it properly.
>
> > + }
> > + }
> > +
> > + 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 +1071,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 ?",
> > + 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 +1211,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 11fa2840d..6b759966b 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -958,6 +958,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,
> > @@ -969,6 +972,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 *
> > @@ -981,6 +986,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;
> > }
> >
> > @@ -992,6 +999,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) {
> > @@ -1000,6 +1008,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
> > @@ -1072,6 +1081,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
> > @@ -1095,12 +1106,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;
> > @@ -1129,35 +1144,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 8fa1a7e1b..d008dcb6c 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
> >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
More information about the dev
mailing list