[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 18:42:35 UTC 2021
On Mon, Aug 2, 2021 at 2:03 PM Han Zhou <zhouhan at gmail.com> wrote:
>
> On Mon, Aug 2, 2021 at 8:03 AM Numan Siddique <numans at ovn.org> wrote:
> >
> >
> >
> >
> > 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.
>
> Well, it is different from the current behavior, in two ways:
> 1) The current implementation would flip ld->has_local_l3gateway of an
> existing ld from false to true, but not from true to false. (For a newly
> added ld, it is set to whatever the value is from the parameter). But your
> change would flip from true to false.
> 2) The current implementation flips the flag in the recursive function,
> which means it can flip for "ld" connected by the input, while your change
> would only flip in the first level call of local_datapath_add().
>
> However, I looked further at the impact of this change but figured out that
> the has_local_l3gateway is not really useful anymore, maybe from long time
> ago. The only usage now is in pinctrl.c but it seems the check is
> redundant. So I went ahead and removed it in this patch:
> https://patchwork.ozlabs.org/project/ovn/patch/20210802175626.2917748-1-hzhou@ovn.org/.
> Please take a look if this simplifies things.
Thanks. I'll take a look.
>
> Moreover, I don't see the benefit of refactoring the function
> add_local_datapath with callbacks. Do you have potential use in the future?
> I'd avoid callbacks if not necessary.
The only use case of adding the call back is to add the newly added
local datapath
to the tracked hash map. Since add_local_datapath__() recursively
calls the same
function, a datapath can be considered and added to the local datapath.
You'd prefer binding.c to pass the "hmap" of tracked_datapaths instead
to the function - local_datapath_add() of local_data.c ?
I've no strong preference. Let me know.
Thanks
Numan
>
> Thanks,
> Han
>
> >
> > 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
> >>
> >>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list