[ovs-dev] [PATCH ovn v3 1/5] controller: Move 'struct local_datapath' to a separate file.

Numan Siddique numans at ovn.org
Mon Aug 2 15:03:39 UTC 2021


On Mon, Aug 2, 2021, 1:54 AM Han Zhou <zhouhan at gmail.com> wrote:

> On Tue, Jul 27, 2021 at 7:47 PM <numans at ovn.org> wrote:
> >
> > From: Numan Siddique <numans at ovn.org>
> >
> > This would uncomplicate the binding.c code a bit.  The tracking
> > data and the related functions are also moved to the file - local_data.c.
> > This would help in an upcoming patch.
> >
> > Signed-off-by: Numan Siddique <numans at ovn.org>
> > ---
> >  controller/automake.mk      |   4 +-
> >  controller/binding.c        | 365 +++++++----------------------------
> >  controller/binding.h        |  13 --
> >  controller/lflow.c          |   1 +
> >  controller/local_data.c     | 369 ++++++++++++++++++++++++++++++++++++
> >  controller/local_data.h     | 119 ++++++++++++
> >  controller/lport.c          |  40 ++++
> >  controller/lport.h          |   7 +-
> >  controller/ovn-controller.c |  56 ++----
> >  controller/ovn-controller.h |  34 ----
> >  controller/patch.c          |   1 +
> >  controller/physical.c       |   1 +
> >  controller/pinctrl.c        |   1 +
> >  13 files changed, 623 insertions(+), 388 deletions(-)
> >  create mode 100644 controller/local_data.c
> >  create mode 100644 controller/local_data.h
> >
> > diff --git a/controller/automake.mk b/controller/automake.mk
> > index 2f6c50890..41f907d6e 100644
> > --- a/controller/automake.mk
> > +++ b/controller/automake.mk
> > @@ -33,7 +33,9 @@ controller_ovn_controller_SOURCES = \
> >         controller/physical.c \
> >         controller/physical.h \
> >         controller/mac-learn.c \
> > -       controller/mac-learn.h
> > +       controller/mac-learn.h \
> > +       controller/local_data.c \
> > +       controller/local_data.h
> >
> >  controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/
> libopenvswitch.la
> >  man_MANS += controller/ovn-controller.8
> > diff --git a/controller/binding.c b/controller/binding.c
> > index d50f3affa..bc47b8c0e 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -14,13 +14,8 @@
> >   */
> >
> >  #include <config.h>
> > -#include "binding.h"
> > -#include "ha-chassis.h"
> > -#include "if-status.h"
> > -#include "lflow.h"
> > -#include "lport.h"
> > -#include "patch.h"
> >
> > +/* OVS includes. */
> >  #include "lib/bitmap.h"
> >  #include "openvswitch/poll-loop.h"
> >  #include "lib/sset.h"
> > @@ -29,9 +24,18 @@
> >  #include "lib/vswitch-idl.h"
> >  #include "openvswitch/hmap.h"
> >  #include "openvswitch/vlog.h"
> > +
> > +/* OVN includes. */
> > +#include "binding.h"
> > +#include "ha-chassis.h"
> > +#include "if-status.h"
> > +#include "lflow.h"
> >  #include "lib/chassis-index.h"
> >  #include "lib/ovn-sb-idl.h"
> > +#include "local_data.h"
> > +#include "lport.h"
> >  #include "ovn-controller.h"
> > +#include "patch.h"
> >
> >  VLOG_DEFINE_THIS_MODULE(binding);
> >
> > @@ -76,106 +80,27 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> >      ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type);
> >  }
> >
> > -static struct tracked_binding_datapath *tracked_binding_datapath_create(
> > -    const struct sbrec_datapath_binding *,
> > -    bool is_new, struct hmap *tracked_dps);
> > -static struct tracked_binding_datapath *tracked_binding_datapath_find(
> > -    struct hmap *, const struct sbrec_datapath_binding *);
> > -static void tracked_binding_datapath_lport_add(
> > -    const struct sbrec_port_binding *, struct hmap *tracked_datapaths);
> >  static void update_lport_tracking(const struct sbrec_port_binding *pb,
> > -                                  struct hmap *tracked_dp_bindings);
> > -
> > -static void
> > -add_local_datapath__(struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> > -                     struct ovsdb_idl_index
> *sbrec_port_binding_by_datapath,
> > -                     struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > -                     const struct sbrec_datapath_binding *datapath,
> > -                     bool has_local_l3gateway, int depth,
> > -                     struct hmap *local_datapaths,
> > -                     struct hmap *tracked_datapaths)
> > -{
> > -    uint32_t dp_key = datapath->tunnel_key;
> > -    struct local_datapath *ld = get_local_datapath(local_datapaths,
> dp_key);
> > -    if (ld) {
> > -        if (has_local_l3gateway) {
> > -            ld->has_local_l3gateway = true;
> > -        }
> > -        return;
> > -    }
> > +                                  struct hmap *tracked_dp_bindings,
> > +                                  bool claimed);
> >
> > -    ld = xzalloc(sizeof *ld);
> > -    hmap_insert(local_datapaths, &ld->hmap_node, dp_key);
> > -    ld->datapath = datapath;
> > -    ld->localnet_port = NULL;
> > -    shash_init(&ld->external_ports);
> > -    ld->has_local_l3gateway = has_local_l3gateway;
> > -
> > -    if (tracked_datapaths) {
> > -        struct tracked_binding_datapath *tdp =
> > -            tracked_binding_datapath_find(tracked_datapaths, datapath);
> > -        if (!tdp) {
> > -            tracked_binding_datapath_create(datapath, true,
> tracked_datapaths);
> > -        } else {
> > -            /* Its possible that there is already an entry in tracked
> datapaths
> > -             * for this 'datapath'. tracked_binding_datapath_lport_add()
> may
> > -             * have created it. Since the 'datapath' is added to the
> > -             * local datapaths, set 'tdp->is_new' to true so that the
> flows
> > -             * for this datapath are programmed properly.
> > -             * */
> > -            tdp->is_new = true;
> > -        }
> > -    }
> > +struct local_datpath_added_aux {
> > +    bool has_local_l3gateway;
> > +    struct hmap *tracked_datapaths;
> > +};
> >
> > -    if (depth >= 100) {
> > -        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > -        VLOG_WARN_RL(&rl, "datapaths nested too deep");
> > -        return;
> > +/* This function is called by local_datapath_add() if a new
> local_datapath
> > + * is created. */
> > +static void
> > +local_datapath_added(struct local_datapath *ld, void *aux)
> > +{
> > +    struct local_datpath_added_aux *aux_ = aux;
> > +    if (aux_->tracked_datapaths) {
> > +        tracked_datapath_add(ld->datapath, TRACKED_RESOURCE_NEW,
> > +                             aux_->tracked_datapaths);
> >      }
> >
> > -    struct sbrec_port_binding *target =
> > -
>  sbrec_port_binding_index_init_row(sbrec_port_binding_by_datapath);
> > -    sbrec_port_binding_index_set_datapath(target, datapath);
> > -
> > -    const struct sbrec_port_binding *pb;
> > -    SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
> > -                                       sbrec_port_binding_by_datapath) {
> > -        if (!strcmp(pb->type, "patch") || !strcmp(pb->type,
> "l3gateway")) {
> > -            const char *peer_name = smap_get(&pb->options, "peer");
> > -            if (peer_name) {
> > -                const struct sbrec_port_binding *peer;
> > -
> > -                peer = lport_lookup_by_name(sbrec_port_binding_by_name,
> > -                                            peer_name);
> > -
> > -                if (peer && peer->datapath) {
> > -                    if (!strcmp(pb->type, "patch")) {
> > -                        /* Add the datapath to local datapath only for
> patch
> > -                         * ports. For l3gateway ports, since gateway
> router
> > -                         * resides on one chassis, we don't need to add.
> > -                         * Otherwise, all other chassis might create
> patch
> > -                         * ports between br-int and the provider bridge.
> */
> > -
>  add_local_datapath__(sbrec_datapath_binding_by_key,
> > -
> sbrec_port_binding_by_datapath,
> > -                                             sbrec_port_binding_by_name,
> > -                                             peer->datapath, false,
> > -                                             depth + 1, local_datapaths,
> > -                                             tracked_datapaths);
> > -                    }
> > -                    ld->n_peer_ports++;
> > -                    if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
> > -                        ld->peer_ports =
> > -                            x2nrealloc(ld->peer_ports,
> > -                                       &ld->n_allocated_peer_ports,
> > -                                       sizeof *ld->peer_ports);
> > -                    }
> > -                    ld->peer_ports[ld->n_peer_ports - 1].local = pb;
> > -                    ld->peer_ports[ld->n_peer_ports - 1].remote = peer;
> > -                }
> > -            }
> > -        }
> > -    }
> > -    sbrec_port_binding_index_destroy_row(target);
> > +    ld->has_local_l3gateway = aux_->has_local_l3gateway;
> >  }
> >
> >  static void
> > @@ -186,11 +111,19 @@ add_local_datapath(struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> >                     bool has_local_l3gateway, struct hmap
> *local_datapaths,
> >                     struct hmap *tracked_datapaths)
> >  {
> > -    add_local_datapath__(sbrec_datapath_binding_by_key,
> > -                         sbrec_port_binding_by_datapath,
> > -                         sbrec_port_binding_by_name,
> > -                         datapath, has_local_l3gateway, 0,
> local_datapaths,
> > -                         tracked_datapaths);
> > +    struct local_datpath_added_aux aux = {
> > +        .has_local_l3gateway = has_local_l3gateway,
> > +        .tracked_datapaths = tracked_datapaths,
> > +    };
> > +
> > +    struct local_datapath *ld =
> > +        local_datapath_add(local_datapaths, datapath,
> > +                           sbrec_datapath_binding_by_key,
> > +                           sbrec_port_binding_by_datapath,
> > +                           sbrec_port_binding_by_name,
> > +                           local_datapath_added,
> > +                           &aux);
> > +    ld->has_local_l3gateway = has_local_l3gateway;
>

Hi Han,

"has_local_l3gateway" is updated here after every call to
"local_datapath_add()".  This ensures that
this patch doesn't change the functionality when compared to the present
code.


>  }
> >
> >  static void
> > @@ -559,7 +492,8 @@ update_related_lport(const struct sbrec_port_binding
> *pb,
> >
> >          if (b_ctx->tracked_dp_bindings) {
> >              /* Add the 'pb' to the tracked_datapaths. */
> > -            tracked_binding_datapath_lport_add(pb,
> b_ctx->tracked_dp_bindings);
> > +            tracked_datapath_lport_add(pb, TRACKED_RESOURCE_NEW,
> > +                                       b_ctx->tracked_dp_bindings);
> >          }
> >      }
> >      sset_add(&b_ctx->related_lports->lport_names, pb->logical_port);
> > @@ -582,7 +516,8 @@ remove_related_lport(const struct sbrec_port_binding
> *pb,
> >
> >          if (b_ctx->tracked_dp_bindings) {
> >              /* Add the 'pb' to the tracked_datapaths. */
> > -            tracked_binding_datapath_lport_add(pb,
> b_ctx->tracked_dp_bindings);
> > +            tracked_datapath_lport_add(pb, TRACKED_RESOURCE_REMOVED,
> > +                                       b_ctx->tracked_dp_bindings);
> >          }
> >      }
> >  }
> > @@ -927,74 +862,6 @@ is_lport_vif(const struct sbrec_port_binding *pb)
> >      return !pb->type[0];
> >  }
> >
> > -static struct tracked_binding_datapath *
> > -tracked_binding_datapath_create(const struct sbrec_datapath_binding *dp,
> > -                                bool is_new,
> > -                                struct hmap *tracked_datapaths)
> > -{
> > -    struct tracked_binding_datapath *t_dp = xzalloc(sizeof *t_dp);
> > -    t_dp->dp = dp;
> > -    t_dp->is_new = is_new;
> > -    shash_init(&t_dp->lports);
> > -    hmap_insert(tracked_datapaths, &t_dp->node,
> uuid_hash(&dp->header_.uuid));
> > -    return t_dp;
> > -}
> > -
> > -static struct tracked_binding_datapath *
> > -tracked_binding_datapath_find(struct hmap *tracked_datapaths,
> > -                              const struct sbrec_datapath_binding *dp)
> > -{
> > -    struct tracked_binding_datapath *t_dp;
> > -    size_t hash = uuid_hash(&dp->header_.uuid);
> > -    HMAP_FOR_EACH_WITH_HASH (t_dp, node, hash, tracked_datapaths) {
> > -        if (uuid_equals(&t_dp->dp->header_.uuid, &dp->header_.uuid)) {
> > -            return t_dp;
> > -        }
> > -    }
> > -
> > -    return NULL;
> > -}
> > -
> > -static void
> > -tracked_binding_datapath_lport_add(const struct sbrec_port_binding *pb,
> > -                                   struct hmap *tracked_datapaths)
> > -{
> > -    if (!tracked_datapaths) {
> > -        return;
> > -    }
> > -
> > -    struct tracked_binding_datapath *tracked_dp =
> > -        tracked_binding_datapath_find(tracked_datapaths, pb->datapath);
> > -    if (!tracked_dp) {
> > -        tracked_dp = tracked_binding_datapath_create(pb->datapath,
> false,
> > -                                                     tracked_datapaths);
> > -    }
> > -
> > -    /* Check if the lport is already present or not.
> > -     * If it is already present, then just update the 'pb' field. */
> > -    struct tracked_binding_lport *lport =
> > -        shash_find_data(&tracked_dp->lports, pb->logical_port);
> > -
> > -    if (!lport) {
> > -        lport = xmalloc(sizeof *lport);
> > -        shash_add(&tracked_dp->lports, pb->logical_port, lport);
> > -    }
> > -
> > -    lport->pb = pb;
> > -}
> > -
> > -void
> > -binding_tracked_dp_destroy(struct hmap *tracked_datapaths)
> > -{
> > -    struct tracked_binding_datapath *t_dp;
> > -    HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) {
> > -        shash_destroy_free_data(&t_dp->lports);
> > -        free(t_dp);
> > -    }
> > -
> > -    hmap_destroy(tracked_datapaths);
> > -}
> > -
> >  static enum en_lport_type
> >  get_lport_type(const struct sbrec_port_binding *pb)
> >  {
> > @@ -1129,7 +996,7 @@ claim_lport(const struct sbrec_port_binding *pb,
> >          sbrec_port_binding_set_chassis(pb, chassis_rec);
> >
> >          if (tracked_datapaths) {
> > -            update_lport_tracking(pb, tracked_datapaths);
> > +            update_lport_tracking(pb, tracked_datapaths, true);
> >          }
> >      }
> >
> > @@ -1178,7 +1045,7 @@ release_lport(const struct sbrec_port_binding *pb,
> bool sb_readonly,
> >          sbrec_port_binding_set_virtual_parent(pb, NULL);
> >      }
> >
> > -    update_lport_tracking(pb, tracked_datapaths);
> > +    update_lport_tracking(pb, tracked_datapaths, false);
> >      if_status_mgr_release_iface(if_mgr, pb->logical_port);
> >      VLOG_INFO("Releasing lport %s from this chassis.",
> pb->logical_port);
> >      return true;
> > @@ -1828,42 +1695,6 @@ binding_cleanup(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> >      return !any_changes;
> >  }
> >
> > -static const struct sbrec_port_binding *
> > -get_peer_lport__(const struct sbrec_port_binding *pb,
> > -                 struct binding_ctx_in *b_ctx_in)
> > -{
> > -    const char *peer_name = smap_get(&pb->options, "peer");
> > -
> > -    if (!peer_name) {
> > -        return NULL;
> > -    }
> > -
> > -    const struct sbrec_port_binding *peer;
> > -    peer = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name,
> > -                                peer_name);
> > -    return (peer && peer->datapath) ? peer : NULL;
> > -}
> > -
> > -static const struct sbrec_port_binding *
> > -get_l3gw_peer_lport(const struct sbrec_port_binding *pb,
> > -                    struct binding_ctx_in *b_ctx_in)
> > -{
> > -    if (strcmp(pb->type, "l3gateway")) {
> > -        return NULL;
> > -    }
> > -    return get_peer_lport__(pb, b_ctx_in);
> > -}
> > -
> > -static const struct sbrec_port_binding *
> > -get_peer_lport(const struct sbrec_port_binding *pb,
> > -               struct binding_ctx_in *b_ctx_in)
> > -{
> > -    if (strcmp(pb->type, "patch")) {
> > -        return NULL;
> > -    }
> > -    return get_peer_lport__(pb, b_ctx_in);
> > -}
> > -
> >  /* This function adds the local datapath of the 'peer' of
> >   * lport 'pb' to the local datapaths if it is not yet added.
> >   */
> > @@ -1873,61 +1704,16 @@ add_local_datapath_peer_port(const struct
> sbrec_port_binding *pb,
> >                               struct binding_ctx_out *b_ctx_out,
> >                               struct local_datapath *ld)
> >  {
> > -    const struct sbrec_port_binding *peer;
> > -    peer = get_peer_lport(pb, b_ctx_in);
> > -
> > -    if (!peer) {
> > -        return;
> > -    }
> > -
> > -    bool present = false;
> > -    for (size_t i = 0; i < ld->n_peer_ports; i++) {
> > -        if (ld->peer_ports[i].local == pb) {
> > -            present = true;
> > -            break;
> > -        }
> > -    }
> > -
> > -    if (!present) {
> > -        ld->n_peer_ports++;
> > -        if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
> > -            ld->peer_ports =
> > -                x2nrealloc(ld->peer_ports,
> > -                           &ld->n_allocated_peer_ports,
> > -                           sizeof *ld->peer_ports);
> > -        }
> > -        ld->peer_ports[ld->n_peer_ports - 1].local = pb;
> > -        ld->peer_ports[ld->n_peer_ports - 1].remote = peer;
> > -    }
> > -
> > -    struct local_datapath *peer_ld =
> > -        get_local_datapath(b_ctx_out->local_datapaths,
> > -                           peer->datapath->tunnel_key);
> > -    if (!peer_ld) {
> > -        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,
> > -                             peer->datapath, false,
> > -                             1, b_ctx_out->local_datapaths,
> > -                             b_ctx_out->tracked_dp_bindings);
> > -        return;
> > -    }
> > -
> > -    for (size_t i = 0; i < peer_ld->n_peer_ports; i++) {
> > -        if (peer_ld->peer_ports[i].local == peer) {
> > -            return;
> > -        }
> > -    }
> > +    struct local_datpath_added_aux aux = {
> > +        .has_local_l3gateway = false,
> > +        .tracked_datapaths = b_ctx_out->tracked_dp_bindings,
> > +    };
> >
> > -    peer_ld->n_peer_ports++;
> > -    if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) {
> > -        peer_ld->peer_ports =
> > -            x2nrealloc(peer_ld->peer_ports,
> > -                        &peer_ld->n_allocated_peer_ports,
> > -                        sizeof *peer_ld->peer_ports);
> > -    }
> > -    peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer;
> > -    peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb;
> > +    local_datapath_add_peer_port(pb,
> b_ctx_in->sbrec_datapath_binding_by_key,
> > +
> b_ctx_in->sbrec_port_binding_by_datapath,
> > +                                 b_ctx_in->sbrec_port_binding_by_name,
> > +                                 ld, b_ctx_out->local_datapaths,
> > +                                 local_datapath_added, &aux);
> >  }
> >
> >  static void
> > @@ -1935,34 +1721,7 @@ remove_local_datapath_peer_port(const struct
> sbrec_port_binding *pb,
> >                                  struct local_datapath *ld,
> >                                  struct hmap *local_datapaths)
> >  {
> > -    size_t i = 0;
> > -    for (i = 0; i < ld->n_peer_ports; i++) {
> > -        if (ld->peer_ports[i].local == pb) {
> > -            break;
> > -        }
> > -    }
> > -
> > -    if (i == ld->n_peer_ports) {
> > -        return;
> > -    }
> > -
> > -    const struct sbrec_port_binding *peer = ld->peer_ports[i].remote;
> > -
> > -    /* Possible improvement: We can shrink the allocated peer ports
> > -     * if (ld->n_peer_ports < ld->n_allocated_peer_ports / 2).
> > -     */
> > -    ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports -
> 1].local;
> > -    ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports -
> 1].remote;
> > -    ld->n_peer_ports--;
> > -
> > -    struct local_datapath *peer_ld =
> > -        get_local_datapath(local_datapaths, peer->datapath->tunnel_key);
> > -    if (peer_ld) {
> > -        /* Remove the peer port from the peer datapath. The peer
> > -         * datapath also tries to remove its peer lport, but that would
> > -         * be no-op. */
> > -        remove_local_datapath_peer_port(peer, peer_ld, local_datapaths);
> > -    }
> > +   local_datapath_remove_peer_port(pb, ld, local_datapaths);
> >  }
> >
> >  static void
> > @@ -1995,13 +1754,16 @@ remove_pb_from_local_datapath(const struct
> sbrec_port_binding *pb,
> >
> >  static void
> >  update_lport_tracking(const struct sbrec_port_binding *pb,
> > -                      struct hmap *tracked_dp_bindings)
> > +                      struct hmap *tracked_dp_bindings,
> > +                      bool claimed)
> >  {
> >      if (!tracked_dp_bindings) {
> >          return;
> >      }
> >
> > -    tracked_binding_datapath_lport_add(pb, tracked_dp_bindings);
> > +    tracked_datapath_lport_add(
> > +        pb, claimed ? TRACKED_RESOURCE_NEW : TRACKED_RESOURCE_REMOVED,
> > +        tracked_dp_bindings);
> >  }
> >
> >  /* Considers the ovs iface 'iface_rec' for claiming.
> > @@ -2315,7 +2077,7 @@ handle_deleted_lport(const struct
> sbrec_port_binding *pb,
> >      /* If the binding is not local, if 'pb' is a L3 gateway port, we
> should
> >       * remove its peer, if that one is local.
> >       */
> > -    pb = get_l3gw_peer_lport(pb, b_ctx_in);
> > +    pb = lport_get_l3gw_peer(pb, b_ctx_in->sbrec_port_binding_by_name);
> >      if (pb) {
> >          ld = get_local_datapath(b_ctx_out->local_datapaths,
> >                                  pb->datapath->tunnel_key);
> > @@ -2598,7 +2360,10 @@ delete_done:
> >                       * */
> >                      const struct sbrec_port_binding *peer;
> >                      struct local_datapath *peer_ld = NULL;
> > -                    peer = get_peer_lport(pb, b_ctx_in);
> > +                    peer =
> > +                        lport_get_peer(pb,
> > +
> b_ctx_in->sbrec_port_binding_by_name);
> > +
> >                      if (peer) {
> >                          peer_ld =
> >
>  get_local_datapath(b_ctx_out->local_datapaths,
> > diff --git a/controller/binding.h b/controller/binding.h
> > index 77197e742..b1717bd2b 100644
> > --- a/controller/binding.h
> > +++ b/controller/binding.h
> > @@ -121,19 +121,6 @@ void local_binding_set_up(struct shash
> *local_bindings, const char *pb_name,
> >  void local_binding_set_down(struct shash *local_bindings, const char
> *pb_name,
> >                              bool sb_readonly, bool ovs_readonly);
> >
> > -/* Represents a tracked binding logical port. */
> > -struct tracked_binding_lport {
> > -    const struct sbrec_port_binding *pb;
> > -};
> > -
> > -/* Represent a tracked binding datapath. */
> > -struct tracked_binding_datapath {
> > -    struct hmap_node node;
> > -    const struct sbrec_datapath_binding *dp;
> > -    bool is_new;
> > -    struct shash lports; /* shash of struct tracked_binding_lport. */
> > -};
> > -
> >  void binding_register_ovs_idl(struct ovsdb_idl *);
> >  void binding_run(struct binding_ctx_in *, struct binding_ctx_out *);
> >  bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index 67bc20203..2e4582009 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -18,6 +18,7 @@
> >  #include "coverage.h"
> >  #include "ha-chassis.h"
> >  #include "lflow-cache.h"
> > +#include "local_data.h"
> >  #include "lport.h"
> >  #include "ofctrl.h"
> >  #include "openvswitch/dynamic-string.h"
> > diff --git a/controller/local_data.c b/controller/local_data.c
> > new file mode 100644
> > index 000000000..1dc8c45a1
> > --- /dev/null
> > +++ b/controller/local_data.c
> > @@ -0,0 +1,369 @@
> > +/* Copyright (c) 2021, Red Hat, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#include <config.h>
> > +
> > +/* OVS includes. */
> > +#include "include/openvswitch/json.h"
> > +#include "lib/hmapx.h"
> > +#include "lib/util.h"
> > +#include "openvswitch/vlog.h"
> > +
> > +/* OVN includes. */
> > +#include "lib/ovn-util.h"
> > +#include "lib/ovn-sb-idl.h"
> > +#include "local_data.h"
> > +#include "lport.h"
> > +
> > +VLOG_DEFINE_THIS_MODULE(ldata);
> > +
> > +static struct local_datapath *local_datapath_add__(
> > +    struct hmap *local_datapaths,
> > +    const struct sbrec_datapath_binding *,
> > +    struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > +    int depth,
> > +    void (*datapath_added)(struct local_datapath *,
> > +                           void *aux),
> > +    void *aux);
> > +
> > +static struct tracked_datapath *tracked_datapath_create(
> > +    const struct sbrec_datapath_binding *dp,
> > +    enum en_tracked_resource_type tracked_type,
> > +    struct hmap *tracked_datapaths);
> > +
> > +struct local_datapath *
> > +get_local_datapath(const struct hmap *local_datapaths, uint32_t
> tunnel_key)
> > +{
> > +    struct hmap_node *node = hmap_first_with_hash(local_datapaths,
> tunnel_key);
> > +    return (node
> > +            ? CONTAINER_OF(node, struct local_datapath, hmap_node)
> > +            : NULL);
> > +}
> > +
> > +struct local_datapath *
> > +local_datapath_alloc(const struct sbrec_datapath_binding *dp)
> > +{
> > +    struct local_datapath *ld = xzalloc(sizeof *ld);
> > +    ld->datapath = dp;
> > +    ld->is_switch = datapath_is_switch(dp);
> > +    shash_init(&ld->external_ports);
> > +    return ld;
> > +}
> > +
> > +void
> > +local_datapaths_destroy(struct hmap *local_datapaths)
> > +{
> > +    struct local_datapath *ld;
> > +    HMAP_FOR_EACH_POP (ld, hmap_node, local_datapaths) {
> > +        local_datapath_destroy(ld);
> > +    }
> > +
> > +    hmap_destroy(local_datapaths);
> > +}
> > +
> > +void
> > +local_datapath_destroy(struct local_datapath *ld)
> > +{
> > +    free(ld->peer_ports);
> > +    shash_destroy(&ld->external_ports);
> > +    free(ld);
> > +}
> > +
> > +struct local_datapath *
> > +local_datapath_add(struct hmap *local_datapaths,
> > +                   const struct sbrec_datapath_binding *dp,
> > +                   struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> > +                   struct ovsdb_idl_index
> *sbrec_port_binding_by_datapath,
> > +                   struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > +                   void (*datapath_added_cb)(
> > +                         struct local_datapath *ld,
> > +                         void *aux),
> > +                   void *aux)
> > +{
> > +    return local_datapath_add__(local_datapaths, dp,
> > +                                sbrec_datapath_binding_by_key,
> > +                                sbrec_port_binding_by_datapath,
> > +                                sbrec_port_binding_by_name, 0,
> > +                                datapath_added_cb, aux);
> > +}
> > +
> > +void
> > +local_datapath_add_peer_port(
> > +    const struct sbrec_port_binding *pb,
> > +    struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > +    struct local_datapath *ld,
> > +    struct hmap *local_datapaths,
> > +    void (*datapath_added_cb)(
> > +                         struct local_datapath *ld,
> > +                         void *aux),
> > +    void *aux)
> > +{
> > +    const struct sbrec_port_binding *peer;
> > +    peer = lport_get_peer(pb, sbrec_port_binding_by_name);
> > +
> > +    if (!peer) {
> > +        return;
> > +    }
> > +
> > +    bool present = false;
> > +    for (size_t i = 0; i < ld->n_peer_ports; i++) {
> > +        if (ld->peer_ports[i].local == pb) {
> > +            present = true;
> > +            break;
> > +        }
> > +    }
> > +
> > +    if (!present) {
> > +        ld->n_peer_ports++;
> > +        if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
> > +            ld->peer_ports =
> > +                x2nrealloc(ld->peer_ports,
> > +                           &ld->n_allocated_peer_ports,
> > +                           sizeof *ld->peer_ports);
> > +        }
> > +        ld->peer_ports[ld->n_peer_ports - 1].local = pb;
> > +        ld->peer_ports[ld->n_peer_ports - 1].remote = peer;
> > +    }
> > +
> > +    struct local_datapath *peer_ld =
> > +        get_local_datapath(local_datapaths,
> > +                           peer->datapath->tunnel_key);
> > +    if (!peer_ld) {
> > +        local_datapath_add__(local_datapaths, peer->datapath,
> > +                             sbrec_datapath_binding_by_key,
> > +                             sbrec_port_binding_by_datapath,
> > +                             sbrec_port_binding_by_name, 1,
> > +                             datapath_added_cb, aux);
> > +        return;
> > +    }
> > +
> > +    for (size_t i = 0; i < peer_ld->n_peer_ports; i++) {
> > +        if (peer_ld->peer_ports[i].local == peer) {
> > +            return;
> > +        }
> > +    }
> > +
> > +    peer_ld->n_peer_ports++;
> > +    if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) {
> > +        peer_ld->peer_ports =
> > +            x2nrealloc(peer_ld->peer_ports,
> > +                        &peer_ld->n_allocated_peer_ports,
> > +                        sizeof *peer_ld->peer_ports);
> > +    }
> > +    peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer;
> > +    peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb;
> > +}
> > +
> > +void
> > +local_datapath_remove_peer_port(const struct sbrec_port_binding *pb,
> > +                                struct local_datapath *ld,
> > +                                struct hmap *local_datapaths)
> > +{
> > +    size_t i = 0;
> > +    for (i = 0; i < ld->n_peer_ports; i++) {
> > +        if (ld->peer_ports[i].local == pb) {
> > +            break;
> > +        }
> > +    }
> > +
> > +    if (i == ld->n_peer_ports) {
> > +        return;
> > +    }
> > +
> > +    const struct sbrec_port_binding *peer = ld->peer_ports[i].remote;
> > +
> > +    /* Possible improvement: We can shrink the allocated peer ports
> > +     * if (ld->n_peer_ports < ld->n_allocated_peer_ports / 2).
> > +     */
> > +    ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports -
> 1].local;
> > +    ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports -
> 1].remote;
> > +    ld->n_peer_ports--;
> > +
> > +    struct local_datapath *peer_ld =
> > +        get_local_datapath(local_datapaths, peer->datapath->tunnel_key);
> > +    if (peer_ld) {
> > +        /* Remove the peer port from the peer datapath. The peer
> > +         * datapath also tries to remove its peer lport, but that would
> > +         * be no-op. */
> > +        local_datapath_remove_peer_port(peer, peer_ld, local_datapaths);
> > +    }
> > +}
> > +
> > +/* track datapath functions. */
> > +struct tracked_datapath *
> > +tracked_datapath_add(const struct sbrec_datapath_binding *dp,
> > +                     enum en_tracked_resource_type tracked_type,
> > +                     struct hmap *tracked_datapaths)
> > +{
> > +    struct tracked_datapath *t_dp =
> > +        tracked_datapath_find(tracked_datapaths, dp);
> > +    if (!t_dp) {
> > +        t_dp = tracked_datapath_create(dp, tracked_type,
> tracked_datapaths);
> > +    } else {
> > +        t_dp->tracked_type = tracked_type;
> > +    }
> > +
> > +    return t_dp;
> > +}
> > +
> > +struct tracked_datapath *
> > +tracked_datapath_find(struct hmap *tracked_datapaths,
> > +                      const struct sbrec_datapath_binding *dp)
> > +{
> > +    struct tracked_datapath *t_dp;
> > +    size_t hash = uuid_hash(&dp->header_.uuid);
> > +    HMAP_FOR_EACH_WITH_HASH (t_dp, node, hash, tracked_datapaths) {
> > +        if (uuid_equals(&t_dp->dp->header_.uuid, &dp->header_.uuid)) {
> > +            return t_dp;
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +void
> > +tracked_datapath_lport_add(const struct sbrec_port_binding *pb,
> > +                           enum en_tracked_resource_type tracked_type,
> > +                           struct hmap *tracked_datapaths)
> > +{
> > +    struct tracked_datapath *tracked_dp =
> > +        tracked_datapath_find(tracked_datapaths, pb->datapath);
> > +    if (!tracked_dp) {
> > +        tracked_dp = tracked_datapath_create(pb->datapath,
> > +                                             TRACKED_RESOURCE_UPDATED,
> > +                                             tracked_datapaths);
> > +    }
> > +
> > +    /* Check if the lport is already present or not.
> > +     * If it is already present, then just update the 'pb' field. */
> > +    struct tracked_lport *lport =
> > +        shash_find_data(&tracked_dp->lports, pb->logical_port);
> > +
> > +    if (!lport) {
> > +        lport = xmalloc(sizeof *lport);
> > +        shash_add(&tracked_dp->lports, pb->logical_port, lport);
> > +    }
> > +
> > +    lport->pb = pb;
> > +    lport->tracked_type = tracked_type;
> > +}
> > +
> > +void
> > +tracked_datapaths_destroy(struct hmap *tracked_datapaths)
> > +{
> > +    struct tracked_datapath *t_dp;
> > +    HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) {
> > +        shash_destroy_free_data(&t_dp->lports);
> > +        free(t_dp);
> > +    }
> > +
> > +    hmap_destroy(tracked_datapaths);
> > +}
> > +
> > +/* static functions. */
> > +static struct local_datapath *
> > +local_datapath_add__(struct hmap *local_datapaths,
> > +                     const struct sbrec_datapath_binding *dp,
> > +                     struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> > +                     struct ovsdb_idl_index
> *sbrec_port_binding_by_datapath,
> > +                     struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > +                     int depth,
> > +                     void (*datapath_added_cb)(
> > +                           struct local_datapath *ld,
> > +                           void *aux),
> > +                     void *aux)
> > +{
> > +    uint32_t dp_key = dp->tunnel_key;
> > +    struct local_datapath *ld = get_local_datapath(local_datapaths,
> dp_key);
> > +    if (ld) {
> > +        return ld;
>
> Hi Numan, I had a comment here for the previous version: "This changes the
> original behavior that when the ld is found it updates the
> has_local_l3gateway before returning. Would this cause a problem?"
> You confirmed it, but there seems to be no change in this version. Did I
> miss anything?
>
>
Hi Han,

Thanks for the reviews.

Please see the comment above and let me know if that is not correct.
This patch makes sure that "has_local_l3gateway" is updated with the
latest value after every call to local_datapath_add() and the caller in
binding.c does this
and not this module.

Thanks
Numan



> > +    }
> > +
> > +    ld = local_datapath_alloc(dp);
> > +    hmap_insert(local_datapaths, &ld->hmap_node, dp_key);
> > +    ld->datapath = dp;
> > +
> > +    if (datapath_added_cb) {
> > +        datapath_added_cb(ld, aux);
> > +    }
> > +
> > +    if (depth >= 100) {
> > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > +        VLOG_WARN_RL(&rl, "datapaths nested too deep");
> > +        return ld;
> > +    }
> > +
> > +    struct sbrec_port_binding *target =
> > +
>  sbrec_port_binding_index_init_row(sbrec_port_binding_by_datapath);
> > +    sbrec_port_binding_index_set_datapath(target, dp);
> > +
> > +    const struct sbrec_port_binding *pb;
> > +    SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
> > +                                       sbrec_port_binding_by_datapath) {
> > +        if (!strcmp(pb->type, "patch") || !strcmp(pb->type,
> "l3gateway")) {
> > +            const char *peer_name = smap_get(&pb->options, "peer");
> > +            if (peer_name) {
> > +                const struct sbrec_port_binding *peer;
> > +
> > +                peer = lport_lookup_by_name(sbrec_port_binding_by_name,
> > +                                            peer_name);
> > +
> > +                if (peer && peer->datapath) {
> > +                    if (!strcmp(pb->type, "patch")) {
> > +                        /* Add the datapath to local datapath only for
> patch
> > +                         * ports. For l3gateway ports, since gateway
> router
> > +                         * resides on one chassis, we don't need to add.
> > +                         * Otherwise, all other chassis might create
> patch
> > +                         * ports between br-int and the provider bridge.
> */
> > +                        local_datapath_add__(local_datapaths,
> peer->datapath,
> > +
> sbrec_datapath_binding_by_key,
> > +
> sbrec_port_binding_by_datapath,
> > +                                             sbrec_port_binding_by_name,
> > +                                             depth + 1,
> datapath_added_cb,
> > +                                             aux);
> > +                    }
> > +                    ld->n_peer_ports++;
> > +                    if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
> > +                        ld->peer_ports =
> > +                            x2nrealloc(ld->peer_ports,
> > +                                       &ld->n_allocated_peer_ports,
> > +                                       sizeof *ld->peer_ports);
> > +                    }
> > +                    ld->peer_ports[ld->n_peer_ports - 1].local = pb;
> > +                    ld->peer_ports[ld->n_peer_ports - 1].remote = peer;
> > +                }
> > +            }
> > +        }
> > +    }
> > +    sbrec_port_binding_index_destroy_row(target);
> > +    return ld;
> > +}
> > +
> > +static struct tracked_datapath *
> > +tracked_datapath_create(const struct sbrec_datapath_binding *dp,
> > +                        enum en_tracked_resource_type tracked_type,
> > +                        struct hmap *tracked_datapaths)
> > +{
> > +    struct tracked_datapath *t_dp = xzalloc(sizeof *t_dp);
> > +    t_dp->dp = dp;
> > +    t_dp->tracked_type = tracked_type;
> > +    shash_init(&t_dp->lports);
> > +    hmap_insert(tracked_datapaths, &t_dp->node,
> uuid_hash(&dp->header_.uuid));
> > +    return t_dp;
> > +}
> > diff --git a/controller/local_data.h b/controller/local_data.h
> > new file mode 100644
> > index 000000000..e29fe928c
> > --- /dev/null
> > +++ b/controller/local_data.h
> > @@ -0,0 +1,119 @@
> > +/* Copyright (c) 2021, Red Hat, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#ifndef LOCAL_DATA_H
> > +#define LOCAL_DATA_H 1
> > +
> > +/* OVS includes. */
> > +#include "include/openvswitch/shash.h"
> > +#include "lib/smap.h"
> > +
> > +struct sbrec_datapath_binding;
> > +struct sbrec_port_binding;
> > +struct ovsdb_idl_index;
> > +
> > +/* A logical datapath that has some relevance to this hypervisor.  A
> logical
> > + * datapath D is relevant to hypervisor H if:
> > + *
> > + *     - Some VIF or l2gateway or l3gateway port in D is located on H.
> > + *
> > + *     - D is reachable over a series of hops across patch ports,
> starting from
> > + *       a datapath relevant to H.
> > + *
> > + * The 'hmap_node''s hash value is 'datapath->tunnel_key'. */
> > +struct local_datapath {
> > +    struct hmap_node hmap_node;
> > +    const struct sbrec_datapath_binding *datapath;
> > +    bool is_switch;
> > +
> > +    /* The localnet port in this datapath, if any (at most one is
> allowed). */
> > +    const struct sbrec_port_binding *localnet_port;
> > +
> > +    /* True if this datapath contains an l3gateway port located on this
> > +     * hypervisor. */
> > +    bool has_local_l3gateway;
> > +
> > +    struct {
> > +        const struct sbrec_port_binding *local;
> > +        const struct sbrec_port_binding *remote;
> > +    } *peer_ports;
> > +
> > +    size_t n_peer_ports;
> > +    size_t n_allocated_peer_ports;
> > +
> > +    struct shash external_ports;
> > +};
> > +
> > +struct local_datapath *local_datapath_alloc(
> > +    const struct sbrec_datapath_binding *);
> > +struct local_datapath *get_local_datapath(const struct hmap *,
> > +                                          uint32_t tunnel_key);
> > +struct local_datapath * local_datapath_add(
> > +    struct hmap *local_datapaths, const struct sbrec_datapath_binding *,
> > +    struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > +    void (*datapath_added)(struct local_datapath *, void *aux),
> > +    void *aux);
> > +
> > +void local_datapaths_destroy(struct hmap *local_datapaths);
> > +void local_datapath_destroy(struct local_datapath *ld);
> > +void local_datapath_add_peer_port(
> > +    const struct sbrec_port_binding *pb,
> > +    struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > +    struct local_datapath *ld,
> > +    struct hmap *local_datapaths,
> > +    void (*datapath_added_cb)(
> > +                         struct local_datapath *ld,
> > +                         void *aux),
> > +    void *aux);
> > +
> > +void local_datapath_remove_peer_port(const struct sbrec_port_binding
> *pb,
> > +                                     struct local_datapath *ld,
> > +                                     struct hmap *local_datapaths);
> > +
> > +enum en_tracked_resource_type {
> > +    TRACKED_RESOURCE_NEW,
> > +    TRACKED_RESOURCE_REMOVED,
> > +    TRACKED_RESOURCE_UPDATED
> > +};
> > +
> > +/* Represents a tracked logical port. */
> > +struct tracked_lport {
> > +    const struct sbrec_port_binding *pb;
> > +    enum en_tracked_resource_type tracked_type;
> > +};
> > +
> > +/* Represent a tracked datapath. */
> > +struct tracked_datapath {
> > +    struct hmap_node node;
> > +    const struct sbrec_datapath_binding *dp;
> > +    enum en_tracked_resource_type tracked_type;
> > +    struct shash lports; /* shash of struct tracked_binding_lport. */
> > +};
> > +
> > +struct tracked_datapath * tracked_datapath_add(
> > +    const struct sbrec_datapath_binding *, enum
> en_tracked_resource_type,
> > +    struct hmap *tracked_datapaths);
> > +struct tracked_datapath *tracked_datapath_find(
> > +    struct hmap *tracked_datapaths, const struct sbrec_datapath_binding
> *);
> > +void tracked_datapath_lport_add(const struct sbrec_port_binding *,
> > +                                enum en_tracked_resource_type,
> > +                                struct hmap *tracked_datapaths);
> > +void tracked_datapaths_destroy(struct hmap *tracked_datapaths);
> > +
> > +#endif /* controller/local_data.h */
> > diff --git a/controller/lport.c b/controller/lport.c
> > index 478fcfd82..25b4ef200 100644
> > --- a/controller/lport.c
> > +++ b/controller/lport.c
> > @@ -23,6 +23,10 @@
> >  #include "lib/ovn-sb-idl.h"
> >  VLOG_DEFINE_THIS_MODULE(lport);
> >
> > +static const struct sbrec_port_binding *get_peer_lport(
> > +    const struct sbrec_port_binding *pb,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name);
> > +
> >  const struct sbrec_port_binding *
> >  lport_lookup_by_name(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >                       const char *name)
> > @@ -84,6 +88,26 @@ lport_is_chassis_resident(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> >      }
> >  }
> >
> > +const struct sbrec_port_binding *
> > +lport_get_peer(const struct sbrec_port_binding *pb,
> > +               struct ovsdb_idl_index *sbrec_port_binding_by_name)
> > +{
> > +    if (strcmp(pb->type, "patch")) {
> > +        return NULL;
> > +    }
> > +    return get_peer_lport(pb, sbrec_port_binding_by_name);
> > +}
> > +
> > +const struct sbrec_port_binding *
> > +lport_get_l3gw_peer(const struct sbrec_port_binding *pb,
> > +                    struct ovsdb_idl_index *sbrec_port_binding_by_name)
> > +{
> > +    if (strcmp(pb->type, "l3gateway")) {
> > +        return NULL;
> > +    }
> > +    return get_peer_lport(pb, sbrec_port_binding_by_name);
> > +}
> > +
> >  const struct sbrec_datapath_binding *
> >  datapath_lookup_by_key(struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> >                         uint64_t dp_key)
> > @@ -120,3 +144,19 @@ mcgroup_lookup_by_dp_name(
> >
> >      return retval;
> >  }
> > +
> > +static const struct sbrec_port_binding *
> > +get_peer_lport(const struct sbrec_port_binding *pb,
> > +               struct ovsdb_idl_index *sbrec_port_binding_by_name)
> > +{
> > +    const char *peer_name = smap_get(&pb->options, "peer");
> > +
> > +    if (!peer_name) {
> > +        return NULL;
> > +    }
> > +
> > +    const struct sbrec_port_binding *peer;
> > +    peer = lport_lookup_by_name(sbrec_port_binding_by_name,
> > +                                peer_name);
> > +    return (peer && peer->datapath) ? peer : NULL;
> > +}
> > diff --git a/controller/lport.h b/controller/lport.h
> > index 345efc184..43b3d714d 100644
> > --- a/controller/lport.h
> > +++ b/controller/lport.h
> > @@ -54,5 +54,10 @@ lport_is_chassis_resident(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> >                            const struct sbrec_chassis *chassis,
> >                            const struct sset *active_tunnels,
> >                            const char *port_name);
> > -
> > +const struct sbrec_port_binding *lport_get_peer(
> > +    const struct sbrec_port_binding *,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name);
> > +const struct sbrec_port_binding *lport_get_l3gw_peer(
> > +    const struct sbrec_port_binding *,
> > +    struct ovsdb_idl_index *sbrec_port_binding_by_name);
> >  #endif /* controller/lport.h */
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 3a9bdbc97..317e9f967 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -39,6 +39,7 @@
> >  #include "lflow.h"
> >  #include "lflow-cache.h"
> >  #include "lib/vswitch-idl.h"
> > +#include "local_data.h"
> >  #include "lport.h"
> >  #include "memory.h"
> >  #include "ofctrl.h"
> > @@ -138,15 +139,6 @@ struct pending_pkt {
> >  /* Registered ofctrl seqno type for nb_cfg propagation. */
> >  static size_t ofctrl_seq_type_nb_cfg;
> >
> > -struct local_datapath *
> > -get_local_datapath(const struct hmap *local_datapaths, uint32_t
> tunnel_key)
> > -{
> > -    struct hmap_node *node = hmap_first_with_hash(local_datapaths,
> tunnel_key);
> > -    return (node
> > -            ? CONTAINER_OF(node, struct local_datapath, hmap_node)
> > -            : NULL);
> > -}
> > -
> >  uint32_t
> >  get_tunnel_type(const char *name)
> >  {
> > @@ -1075,14 +1067,14 @@ struct ed_type_runtime_data {
> >   *
> >   *
>  ------------------------------------------------------------------------
> >   * |                      | This is a hmap of
>     |
> > - * |                      | 'struct tracked_binding_datapath' defined in
>    |
> > - * |                      | binding.h. Runtime data handlers for OVS
>    |
> > + * |                      | 'struct tracked_datapath' defined in
>    |
> > + * |                      | ldata.h. Runtime data handlers for OVS
>    |
> >   * |                      | Interface and Port Binding changes store the
>    |
> >   * | @tracked_dp_bindings | changed datapaths (datapaths added/removed
> from |
> >   * |                      | local_datapaths) and changed port bindings
>    |
> > - * |                      | (added/updated/deleted in 'lbinding_data').
>    |
> > + * |                      | (added/updated/deleted in 'lbinding_data').
>     |
> >   * |                      | So any changes to the runtime data -
>    |
> > - * |                      | local_datapaths and lbinding_data is
> captured  |
> > + * |                      | local_datapaths and lbinding_data is
> captured   |
> >   * |                      | here.
>     |
> >   *
>  ------------------------------------------------------------------------
> >   * |                      | This is a bool which represents if the
> runtime  |
> > @@ -1109,7 +1101,7 @@ struct ed_type_runtime_data {
> >   *
> >   *
> ---------------------------------------------------------------------
> >   * | local_datapaths  | The changes to these runtime data is captured in
> |
> > - * | lbinding_data   | the @tracked_dp_bindings indirectly and hence it
> |
> > + * | lbinding_data   | the @tracked_dp_bindings indirectly and hence it
>  |
> >   * | local_lport_ids  | is not tracked explicitly.
> |
> >   *
> ---------------------------------------------------------------------
> >   * | local_iface_ids  | This is used internally within the runtime data
>  |
> > @@ -1134,7 +1126,7 @@ en_runtime_data_clear_tracked_data(void *data_)
> >  {
> >      struct ed_type_runtime_data *data = data_;
> >
> > -    binding_tracked_dp_destroy(&data->tracked_dp_bindings);
> > +    tracked_datapaths_destroy(&data->tracked_dp_bindings);
> >      hmap_init(&data->tracked_dp_bindings);
> >      data->local_lports_changed = false;
> >      data->tracked = false;
> > @@ -1172,15 +1164,7 @@ en_runtime_data_cleanup(void *data)
> >      sset_destroy(&rt_data->active_tunnels);
> >      sset_destroy(&rt_data->egress_ifaces);
> >      smap_destroy(&rt_data->local_iface_ids);
> > -    struct local_datapath *cur_node, *next_node;
> > -    HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> > -                        &rt_data->local_datapaths) {
> > -        free(cur_node->peer_ports);
> > -        shash_destroy(&cur_node->external_ports);
> > -        hmap_remove(&rt_data->local_datapaths, &cur_node->hmap_node);
> > -        free(cur_node);
> > -    }
> > -    hmap_destroy(&rt_data->local_datapaths);
> > +    local_datapaths_destroy(&rt_data->local_datapaths);
> >      shash_destroy_free_data(&rt_data->local_active_ports_ipv6_pd);
> >      shash_destroy_free_data(&rt_data->local_active_ports_ras);
> >      local_binding_data_destroy(&rt_data->lbinding_data);
> > @@ -1292,14 +1276,7 @@ en_runtime_data_run(struct engine_node *node, void
> *data)
> >          /* don't cleanup since there is no data yet */
> >          first_run = false;
> >      } else {
> > -        struct local_datapath *cur_node, *next_node;
> > -        HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> local_datapaths) {
> > -            free(cur_node->peer_ports);
> > -            shash_destroy(&cur_node->external_ports);
> > -            hmap_remove(local_datapaths, &cur_node->hmap_node);
> > -            free(cur_node);
> > -        }
> > -        hmap_clear(local_datapaths);
> > +        local_datapaths_destroy(local_datapaths);
> >          shash_clear_free_data(local_active_ipv6_pd);
> >          shash_clear_free_data(local_active_ras);
> >          local_binding_data_destroy(&rt_data->lbinding_data);
> > @@ -1308,6 +1285,7 @@ en_runtime_data_run(struct engine_node *node, void
> *data)
> >          sset_destroy(active_tunnels);
> >          sset_destroy(&rt_data->egress_ifaces);
> >          smap_destroy(&rt_data->local_iface_ids);
> > +        hmap_init(local_datapaths);
> >          sset_init(local_lports);
> >          related_lports_init(&rt_data->related_lports);
> >          sset_init(active_tunnels);
> > @@ -1749,12 +1727,12 @@ port_groups_runtime_data_handler(struct
> engine_node *node, void *data)
> >                                                   pg_sb->name);
> >          ovs_assert(pg_lports);
> >
> > -        struct tracked_binding_datapath *tdp;
> > +        struct tracked_datapath *tdp;
> >          bool need_update = false;
> >          HMAP_FOR_EACH (tdp, node, &rt_data->tracked_dp_bindings) {
> >              struct shash_node *shash_node;
> >              SHASH_FOR_EACH (shash_node, &tdp->lports) {
> > -                struct tracked_binding_lport *lport = shash_node->data;
> > +                struct tracked_lport *lport = shash_node->data;
> >                  if (sset_contains(pg_lports, lport->pb->logical_port)) {
> >                      /* At least one local port-binding change is related
> to the
> >                       * port_group, so the port_group_cs_local needs
> update. */
> > @@ -1907,9 +1885,9 @@ ct_zones_runtime_data_handler(struct engine_node
> *node, void *data OVS_UNUSED)
> >      }
> >
> >      struct hmap *tracked_dp_bindings = &rt_data->tracked_dp_bindings;
> > -    struct tracked_binding_datapath *tdp;
> > +    struct tracked_datapath *tdp;
> >      HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) {
> > -        if (tdp->is_new) {
> > +        if (tdp->tracked_type == TRACKED_RESOURCE_NEW) {
> >              /* A new datapath has been added. Fall back to full
> recompute. */
> >              return false;
> >          }
> > @@ -2431,9 +2409,9 @@ lflow_output_runtime_data_handler(struct
> engine_node *node,
> >      struct ed_type_lflow_output *fo = data;
> >      init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out);
> >
> > -    struct tracked_binding_datapath *tdp;
> > +    struct tracked_datapath *tdp;
> >      HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) {
> > -        if (tdp->is_new) {
> > +        if (tdp->tracked_type == TRACKED_RESOURCE_NEW) {
> >              if (!lflow_add_flows_for_datapath(tdp->dp, &l_ctx_in,
> >                                                &l_ctx_out)) {
> >                  return false;
> > @@ -2441,7 +2419,7 @@ lflow_output_runtime_data_handler(struct
> engine_node *node,
> >          } else {
> >              struct shash_node *shash_node;
> >              SHASH_FOR_EACH (shash_node, &tdp->lports) {
> > -                struct tracked_binding_lport *lport = shash_node->data;
> > +                struct tracked_lport *lport = shash_node->data;
> >                  if (!lflow_handle_flows_for_lport(lport->pb, &l_ctx_in,
> >                                                    &l_ctx_out)) {
> >                      return false;
> > diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h
> > index b864ed0fa..578588305 100644
> > --- a/controller/ovn-controller.h
> > +++ b/controller/ovn-controller.h
> > @@ -40,40 +40,6 @@ struct ct_zone_pending_entry {
> >      enum ct_zone_pending_state state;
> >  };
> >
> > -/* A logical datapath that has some relevance to this hypervisor.  A
> logical
> > - * datapath D is relevant to hypervisor H if:
> > - *
> > - *     - Some VIF or l2gateway or l3gateway port in D is located on H.
> > - *
> > - *     - D is reachable over a series of hops across patch ports,
> starting from
> > - *       a datapath relevant to H.
> > - *
> > - * The 'hmap_node''s hash value is 'datapath->tunnel_key'. */
> > -struct local_datapath {
> > -    struct hmap_node hmap_node;
> > -    const struct sbrec_datapath_binding *datapath;
> > -
> > -    /* The localnet port in this datapath, if any (at most one is
> allowed). */
> > -    const struct sbrec_port_binding *localnet_port;
> > -
> > -    /* True if this datapath contains an l3gateway port located on this
> > -     * hypervisor. */
> > -    bool has_local_l3gateway;
> > -
> > -    struct {
> > -        const struct sbrec_port_binding *local;
> > -        const struct sbrec_port_binding *remote;
> > -    } *peer_ports;
> > -
> > -    size_t n_peer_ports;
> > -    size_t n_allocated_peer_ports;
> > -
> > -    struct shash external_ports;
> > -};
> > -
> > -struct local_datapath *get_local_datapath(const struct hmap *,
> > -                                          uint32_t tunnel_key);
> > -
> >  const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table
> *,
> >                                         const char *br_name);
> >
> > diff --git a/controller/patch.c b/controller/patch.c
> > index e54b56354..a661025da 100644
> > --- a/controller/patch.c
> > +++ b/controller/patch.c
> > @@ -20,6 +20,7 @@
> >  #include "hash.h"
> >  #include "lflow.h"
> >  #include "lib/vswitch-idl.h"
> > +#include "local_data.h"
> >  #include "lport.h"
> >  #include "openvswitch/hmap.h"
> >  #include "openvswitch/vlog.h"
> > diff --git a/controller/physical.c b/controller/physical.c
> > index 00cf2ca35..188606927 100644
> > --- a/controller/physical.c
> > +++ b/controller/physical.c
> > @@ -21,6 +21,7 @@
> >  #include "flow.h"
> >  #include "ha-chassis.h"
> >  #include "lflow.h"
> > +#include "local_data.h"
> >  #include "lport.h"
> >  #include "chassis.h"
> >  #include "lib/bundle.h"
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index 8e4c4d18c..9e448e3c7 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -25,6 +25,7 @@
> >  #include "encaps.h"
> >  #include "flow.h"
> >  #include "ha-chassis.h"
> > +#include "local_data.h"
> >  #include "lport.h"
> >  #include "mac-learn.h"
> >  #include "nx-match.h"
> > --
> > 2.31.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
>


More information about the dev mailing list