On Fri, Jul 26, 2024 at 1:20 PM Ales Musil <amu...@redhat.com> wrote:
>
> On Fri, Jul 26, 2024 at 5:51 PM Numan Siddique <num...@ovn.org> wrote:
>
> > On Thu, Jul 25, 2024 at 6:22 AM Ales Musil <amu...@redhat.com> wrote:
> > >
> > > Add support for limiting the CT zone usage per Ls, LR or LSP.
> > > When the limit is configured on logical switch it will also implicitly
> > > set limits for all ports in that logical switch. The port configuration
> > > can be overwritten individually and has priority over the whole logical
> > > switch configuration.
> > >
> > > The value 0 means unlimited, when the value is not specified it is
> > > derived from OvS default CT limit specified for given OvS datapath.
> > >
> > > Reported-at: https://bugzilla.redhat.com/2189924
> > > Signed-off-by: Ales Musil <amu...@redhat.com>
> >
> > Thanks for addressing the comments in v6.  The patch LGTM.
> >
> > Before applying/giving the Ack, I want to make sure that the system
> > test failure for this patch in the CI is a flake.
> > Can you please take a look ?
> >
> > https://github.com/ovsrobot/ovn/actions/runs/10092667039/job/27906915451#step:10:9299
> >
> > Thanks
> > Numan
> >
>
>
> It is a flake, for some reason the openbfdd fails to start from time to
> time.
>

Thanks for confirming.  I was a bit late in applying this patch series and
now it needs rebasing due to this commit -
https://github.com/ovn-org/ovn/commit/f2363f49f6a46e784b1d8dad33eee733aa09380c

I tried resolving the conflicts, but it was non trivial.  Can you
please rebase and submit ?

Numan

>
> > > ---
> > > v6: Rebase on top of latest main.
> > >     Addressed comments from Mark:
> > >     - Fix spelling error in the documenation.
> > >     Addressed comments from Numan:
> > >     - Avoid looping over all interface and loop over local ones only.
> > >     - Do not loop over interfaces when the DP limit didn't change.
> > > v5: Rebase on top of latest main.
> > >     Avoid OvS CT zone lookup in every loop of pending commit.
> > > v4: Rebase on top of latest main.
> > >     Change naming of the ct_zone_limit_sync to avoid potential confusion
> > as suggested by Lorenzo.
> > > v3: Rebase on top of latest main.
> > > ---
> > >  NEWS                        |   3 +
> > >  controller/ct-zone.c        | 180 ++++++++++++++++++++++++++++++++----
> > >  controller/ct-zone.h        |  15 ++-
> > >  controller/ovn-controller.c |  21 +++--
> > >  lib/ovn-util.c              |  17 ++++
> > >  lib/ovn-util.h              |   3 +
> > >  northd/northd.c             |   8 ++
> > >  ovn-nb.xml                  |  29 ++++++
> > >  tests/ovn-controller.at     |  99 ++++++++++++++++++++
> > >  9 files changed, 346 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 72933b5b9..4c711d1ef 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -42,6 +42,9 @@ Post v24.03.0
> > >      true, the traffic destined to a logical port of a provider logical
> > switch
> > >      (having a localnet port) will be tunnelled instead of sending it
> > via the
> > >      localnet port.
> > > +  - Add support for CT zone limit that can be specified per LR
> > > +    (options:ct-zone-limit), LS (other_config:ct-zone-limit) or LSP
> > > +    (options:ct-zone-limit).
> > >
> > >  OVN v24.03.0 - 01 Mar 2024
> > >  --------------------------
> > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c
> > > index ab0eec9d0..1aab7ccef 100644
> > > --- a/controller/ct-zone.c
> > > +++ b/controller/ct-zone.c
> > > @@ -15,6 +15,7 @@
> > >
> > >  #include <config.h>
> > >
> > > +#include "binding.h"
> > >  #include "ct-zone.h"
> > >  #include "local_data.h"
> > >  #include "openvswitch/vlog.h"
> > > @@ -34,6 +35,16 @@ static bool ct_zone_assign_unused(struct ct_zone_ctx
> > *ctx,
> > >  static bool ct_zone_remove(struct ct_zone_ctx *ctx, const char *name);
> > >  static void ct_zone_add(struct ct_zone_ctx *ctx, const char *name,
> > >                          uint16_t zone, bool set_pending);
> > > +static void
> > > +ct_zone_limits_update_per_dp(struct ct_zone_ctx *ctx,
> > > +                             const struct local_datapath *local_dp,
> > > +                             const struct shash *local_lports,
> > > +                             const char *name);
> > > +static bool ct_zone_limit_update(struct ct_zone_ctx *ctx, const char
> > *name,
> > > +                                 int64_t limit);
> > > +static int64_t ct_zone_get_dp_limit(const struct sbrec_datapath_binding
> > *dp);
> > > +static int64_t ct_zone_get_pb_limit(const struct sbrec_port_binding
> > *pb);
> > > +static int64_t ct_zone_limit_normalize(int64_t limit);
> > >
> > >  void
> > >  ct_zone_ctx_init(struct ct_zone_ctx *ctx)
> > > @@ -210,11 +221,24 @@ ct_zones_update(const struct sset *local_lports,
> > >
> > >  void
> > >  ct_zones_commit(const struct ovsrec_bridge *br_int,
> > > -                struct shash *pending_ct_zones)
> > > +                const struct ovsrec_datapath *ovs_dp,
> > > +                struct ovsdb_idl_txn *ovs_idl_txn,
> > > +                struct ct_zone_ctx *ctx)
> > >  {
> > > +    if (shash_is_empty(&ctx->pending)) {
> > > +        return;
> > > +    }
> > > +
> > > +    struct ovsrec_ct_zone **all_zones =
> > > +            xzalloc(sizeof *all_zones * (MAX_CT_ZONES + 1));
> > > +    for (size_t i = 0; i < ovs_dp->n_ct_zones; i++) {
> > > +        all_zones[ovs_dp->key_ct_zones[i]] = ovs_dp->value_ct_zones[i];
> > > +    }
> > > +
> > >      struct shash_node *iter;
> > > -    SHASH_FOR_EACH (iter, pending_ct_zones) {
> > > +    SHASH_FOR_EACH (iter, &ctx->pending) {
> > >          struct ct_zone_pending_entry *ctzpe = iter->data;
> > > +        struct ct_zone *ct_zone = &ctzpe->ct_zone;
> > >
> > >          /* The transaction is open, so any pending entries in the
> > >           * CT_ZONE_DB_QUEUED must be sent and any in CT_ZONE_DB_QUEUED
> > > @@ -226,7 +250,7 @@ ct_zones_commit(const struct ovsrec_bridge *br_int,
> > >
> > >          char *user_str = xasprintf("ct-zone-%s", iter->name);
> > >          if (ctzpe->add) {
> > > -            char *zone_str = xasprintf("%"PRIu16, ctzpe->ct_zone.zone);
> > > +            char *zone_str = xasprintf("%"PRIu16, ct_zone->zone);
> > >              struct smap_node *node =
> > >                      smap_get_node(&br_int->external_ids, user_str);
> > >              if (!node || strcmp(node->value, zone_str)) {
> > > @@ -241,8 +265,22 @@ ct_zones_commit(const struct ovsrec_bridge *br_int,
> > >          }
> > >          free(user_str);
> > >
> > > +        struct ovsrec_ct_zone *ovs_zone = all_zones[ct_zone->zone];
> > > +        if ((!ctzpe->add || ct_zone->limit < 0) && ovs_zone) {
> > > +            ovsrec_datapath_update_ct_zones_delkey(ovs_dp,
> > ct_zone->zone);
> > > +        } else if (ctzpe->add && ct_zone->limit >= 0) {
> > > +            if (!ovs_zone) {
> > > +                ovs_zone = ovsrec_ct_zone_insert(ovs_idl_txn);
> > > +                ovsrec_datapath_update_ct_zones_setkey(ovs_dp,
> > ct_zone->zone,
> > > +                                                       ovs_zone);
> > > +            }
> > > +            ovsrec_ct_zone_set_limit(ovs_zone, &ct_zone->limit, 1);
> > > +        }
> > > +
> > >          ctzpe->state = CT_ZONE_DB_SENT;
> > >      }
> > > +
> > > +    free(all_zones);
> > >  }
> > >
> > >  void
> > > @@ -261,9 +299,21 @@ ct_zones_pending_clear_commited(struct shash
> > *pending)
> > >  /* Returns "true" when there is no need for full recompute. */
> > >  bool
> > >  ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
> > > -                         const struct sbrec_datapath_binding *dp)
> > > +                         const struct local_datapath *local_dp,
> > > +                         const struct shash *local_lports)
> > >  {
> > > -    int req_snat_zone = ct_zone_get_snat(dp);
> > > +    const char *name = smap_get(&local_dp->datapath->external_ids,
> > "name");
> > > +    if (!name) {
> > > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> > > +        VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"'
> > skipping"
> > > +                    "zone check.",
> > > +                    UUID_ARGS(&local_dp->datapath->header_.uuid));
> > > +        return true;
> > > +    }
> > > +
> > > +    ct_zone_limits_update_per_dp(ctx, local_dp, local_lports, name);
> > > +
> > > +    int req_snat_zone = ct_zone_get_snat(local_dp->datapath);
> > >      if (req_snat_zone == -1) {
> > >          /* datapath snat ct zone is not set.  This condition will also
> > hit
> > >           * when CMS clears the snat-ct-zone for the logical router.
> > > @@ -273,14 +323,6 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
> > >          return true;
> > >      }
> > >
> > > -    const char *name = smap_get(&dp->external_ids, "name");
> > > -    if (!name) {
> > > -        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> > > -        VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"'
> > skipping"
> > > -                    "zone check.", UUID_ARGS(&dp->header_.uuid));
> > > -        return true;
> > > -    }
> > > -
> > >      /* Check if the requested snat zone has changed for the datapath
> > >       * or not.  If so, then fall back to full recompute of
> > >       * ct_zone engine. */
> > > @@ -304,14 +346,18 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
> > >
> > >  /* Returns "true" if there was an update to the context. */
> > >  bool
> > > -ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name,
> > > +ct_zone_handle_port_update(struct ct_zone_ctx *ctx,
> > > +                           const struct sbrec_port_binding *pb,
> > >                             bool updated, int *scan_start)
> > >  {
> > > -    struct shash_node *node = shash_find(&ctx->current, name);
> > > -    if (updated && !node) {
> > > -        ct_zone_assign_unused(ctx, name, scan_start);
> > > +    struct shash_node *node = shash_find(&ctx->current,
> > pb->logical_port);
> > > +    if (updated) {
> > > +        if (!node) {
> > > +            ct_zone_assign_unused(ctx, pb->logical_port, scan_start);
> > > +        }
> > > +        ct_zone_limit_update(ctx, pb->logical_port,
> > ct_zone_get_pb_limit(pb));
> > >          return true;
> > > -    } else if (!updated && node && ct_zone_remove(ctx, node->name)) {
> > > +    } else if (node && ct_zone_remove(ctx, node->name)) {
> > >          return true;
> > >      }
> > >
> > > @@ -325,6 +371,25 @@ ct_zone_find_zone(const struct shash *ct_zones,
> > const char *name)
> > >      return ct_zone ? ct_zone->zone : 0;
> > >  }
> > >
> > > +void
> > > +ct_zones_limits_sync(struct ct_zone_ctx *ctx,
> > > +                     const struct hmap *local_datapaths,
> > > +                     const struct shash *local_lports)
> > > +{
> > > +    const struct local_datapath *ld;
> > > +    HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
> > > +        const char *name = smap_get(&ld->datapath->external_ids,
> > "name");
> > > +        if (!name) {
> > > +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> > 1);
> > > +            VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' "
> > > +                        "skipping zone assignment.",
> > > +                        UUID_ARGS(&ld->datapath->header_.uuid));
> > > +            continue;
> > > +        }
> > > +
> > > +        ct_zone_limits_update_per_dp(ctx, ld, local_lports, name);
> > > +    }
> > > +}
> > >
> > >  static bool
> > >  ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name,
> > > @@ -377,7 +442,10 @@ ct_zone_add(struct ct_zone_ctx *ctx, const char
> > *name, uint16_t zone,
> > >          shash_add(&ctx->current, name, ct_zone);
> > >      }
> > >
> > > -    ct_zone->zone = zone;
> > > +    *ct_zone = (struct ct_zone) {
> > > +        .zone = zone,
> > > +        .limit = -1,
> > > +    };
> > >
> > >      if (set_pending) {
> > >          ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED,
> > > @@ -460,6 +528,7 @@ ct_zone_restore(const struct
> > sbrec_datapath_binding_table *dp_table,
> > >
> > >          struct ct_zone ct_zone = {
> > >              .zone = zone,
> > > +            .limit = -1,
> > >          };
> > >          /* Make sure we remove the uuid one in the next OvS DB commit
> > without
> > >           * flush. */
> > > @@ -475,3 +544,76 @@ ct_zone_restore(const struct
> > sbrec_datapath_binding_table *dp_table,
> > >      ct_zone_add(ctx, current_name, zone, false);
> > >      free(new_name);
> > >  }
> > > +
> > > +static void
> > > +ct_zone_limits_update_per_dp(struct ct_zone_ctx *ctx,
> > > +                             const struct local_datapath *local_dp,
> > > +                             const struct shash *local_lports,
> > > +                             const char *name)
> > > +{
> > > +
> > > +    int64_t dp_limit = ct_zone_get_dp_limit(local_dp->datapath);
> > > +    char *dnat = alloc_nat_zone_key(name, "dnat");
> > > +    char *snat = alloc_nat_zone_key(name, "snat");
> > > +
> > > +    bool zone_updated = ct_zone_limit_update(ctx, dnat, dp_limit);
> > > +    zone_updated |= ct_zone_limit_update(ctx, snat, dp_limit);
> > > +
> > > +    if (local_dp->is_switch && zone_updated) {
> > > +        const struct shash_node *node;
> > > +        SHASH_FOR_EACH (node, local_lports) {
> > > +            const struct binding_lport *lport = node->data;
> > > +
> > > +            if (lport->pb->datapath != local_dp->datapath) {
> > > +                continue;
> > > +            }
> > > +
> > > +            ct_zone_limit_update(ctx, lport->name,
> > > +                                 ct_zone_get_pb_limit(lport->pb));
> > > +        }
> > > +    }
> > > +
> > > +    free(dnat);
> > > +    free(snat);
> > > +}
> > > +
> > > +static bool
> > > +ct_zone_limit_update(struct ct_zone_ctx *ctx, const char *name, int64_t
> > limit)
> > > +{
> > > +    struct ct_zone *ct_zone = shash_find_data(&ctx->current, name);
> > > +
> > > +    if (!ct_zone || ct_zone->limit == limit) {
> > > +        return false;
> > > +    }
> > > +
> > > +    ct_zone->limit = limit;
> > > +    /* Add pending entry only for DB store to avoid flushing the zone.
> > */
> > > +    ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED, ct_zone,
> > true, name);
> > > +    VLOG_DBG("setting ct zone %"PRIu16" limit to %"PRId64,
> > > +             ct_zone->zone, ct_zone->limit);
> > > +
> > > +    return true;
> > > +}
> > > +
> > > +static int64_t
> > > +ct_zone_get_dp_limit(const struct sbrec_datapath_binding *dp)
> > > +{
> > > +    int64_t limit = ovn_smap_get_llong(&dp->external_ids,
> > "ct-zone-limit", -1);
> > > +    return ct_zone_limit_normalize(limit);
> > > +}
> > > +
> > > +static int64_t
> > > +ct_zone_get_pb_limit(const struct sbrec_port_binding *pb)
> > > +{
> > > +    int64_t dp_limit = ovn_smap_get_llong(&pb->datapath->external_ids,
> > > +                                          "ct-zone-limit", -1);
> > > +    int64_t limit = ovn_smap_get_llong(&pb->options,
> > > +                                       "ct-zone-limit", dp_limit);
> > > +    return ct_zone_limit_normalize(limit);
> > > +}
> > > +
> > > +static int64_t
> > > +ct_zone_limit_normalize(int64_t limit)
> > > +{
> > > +    return limit >= 0 && limit <= UINT32_MAX ? limit : -1;
> > > +}
> > > diff --git a/controller/ct-zone.h b/controller/ct-zone.h
> > > index a7c2011a1..e74af1d66 100644
> > > --- a/controller/ct-zone.h
> > > +++ b/controller/ct-zone.h
> > > @@ -22,6 +22,7 @@
> > >  #include "openvswitch/hmap.h"
> > >  #include "openvswitch/shash.h"
> > >  #include "openvswitch/types.h"
> > > +#include "local_data.h"
> > >  #include "ovn-sb-idl.h"
> > >  #include "simap.h"
> > >  #include "vswitch-idl.h"
> > > @@ -43,6 +44,7 @@ struct ct_zone_ctx {
> > >
> > >  struct ct_zone {
> > >      uint16_t zone;
> > > +    int64_t limit;
> > >  };
> > >
> > >  /* States to move through when a new conntrack zone has been allocated.
> > */
> > > @@ -70,12 +72,19 @@ void ct_zones_update(const struct sset *local_lports,
> > >                       const struct hmap *local_datapaths,
> > >                       struct ct_zone_ctx *ctx);
> > >  void ct_zones_commit(const struct ovsrec_bridge *br_int,
> > > -                     struct shash *pending_ct_zones);
> > > +                     const struct ovsrec_datapath *ovs_dp,
> > > +                     struct ovsdb_idl_txn *ovs_idl_txn,
> > > +                     struct ct_zone_ctx *ctx);
> > >  void ct_zones_pending_clear_commited(struct shash *pending);
> > >  bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
> > > -                              const struct sbrec_datapath_binding *dp);
> > > -bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char
> > *name,
> > > +                              const struct local_datapath *local_dp,
> > > +                              const struct shash *local_lports);
> > > +bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx,
> > > +                                const struct sbrec_port_binding *pb,
> > >                                  bool updated, int *scan_start);
> > >  uint16_t ct_zone_find_zone(const struct shash *ct_zones, const char
> > *name);
> > > +void ct_zones_limits_sync(struct ct_zone_ctx *ctx,
> > > +                          const struct hmap *local_datapaths,
> > > +                          const struct shash *local_lports);
> > >
> > >  #endif /* controller/ct-zone.h */
> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > > index aee558f9a..28f4fe407 100644
> > > --- a/controller/ovn-controller.c
> > > +++ b/controller/ovn-controller.c
> > > @@ -795,6 +795,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > >      ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_private_key);
> > >      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_datapath);
> > >      ovsdb_idl_add_column(ovs_idl, &ovsrec_datapath_col_capabilities);
> > > +    ovsdb_idl_add_column(ovs_idl, &ovsrec_datapath_col_ct_zones);
> > >      ovsdb_idl_add_table(ovs_idl,
> > &ovsrec_table_flow_sample_collector_set);
> > >      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_qos);
> > >      ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_other_config);
> > > @@ -804,6 +805,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > >      ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config);
> > >      ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids);
> > >      ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_link_state);
> > > +    ovsdb_idl_add_table(ovs_idl, &ovsrec_table_ct_zone);
> > > +    ovsdb_idl_add_column(ovs_idl, &ovsrec_ct_zone_col_limit);
> > >
> > >      chassis_register_ovs_idl(ovs_idl);
> > >      encaps_register_ovs_idl(ovs_idl);
> > > @@ -2227,6 +2230,8 @@ en_ct_zones_run(struct engine_node *node, void
> > *data)
> > >      ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int);
> > >      ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths,
> > >                      &ct_zones_data->ctx);
> > > +    ct_zones_limits_sync(&ct_zones_data->ctx, &rt_data->local_datapaths,
> > > +                         &rt_data->lbinding_data.lports);
> > >
> > >      ct_zones_data->recomputed = true;
> > >      engine_set_node_state(node, EN_UPDATED);
> > > @@ -2246,8 +2251,9 @@ ct_zones_datapath_binding_handler(struct
> > engine_node *node, void *data)
> > >          EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node));
> > >
> > >      SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) {
> > > -        if (!get_local_datapath(&rt_data->local_datapaths,
> > > -                                dp->tunnel_key)) {
> > > +        const struct local_datapath *local_dp=
> > > +                get_local_datapath(&rt_data->local_datapaths,
> > dp->tunnel_key);
> > > +        if (!local_dp) {
> > >              continue;
> > >          }
> > >
> > > @@ -2257,7 +2263,8 @@ ct_zones_datapath_binding_handler(struct
> > engine_node *node, void *data)
> > >              return false;
> > >          }
> > >
> > > -        if (!ct_zone_handle_dp_update(&ct_zones_data->ctx, dp)) {
> > > +        if (!ct_zone_handle_dp_update(&ct_zones_data->ctx, local_dp,
> > > +                                      &rt_data->lbinding_data.lports)) {
> > >              return false;
> > >          }
> > >      }
> > > @@ -2306,8 +2313,8 @@ ct_zones_runtime_data_handler(struct engine_node
> > *node, void *data)
> > >                      t_lport->tracked_type == TRACKED_RESOURCE_NEW ||
> > >                      t_lport->tracked_type == TRACKED_RESOURCE_UPDATED;
> > >              updated |= ct_zone_handle_port_update(&ct_zones_data->ctx,
> > > -
> > t_lport->pb->logical_port,
> > > -                                                  port_updated,
> > &scan_start);
> > > +                                                  t_lport->pb,
> > port_updated,
> > > +                                                  &scan_start);
> > >          }
> > >      }
> > >
> > > @@ -5560,8 +5567,8 @@ main(int argc, char *argv[])
> > >                          if (ct_zones_data) {
> > >
> > stopwatch_start(CT_ZONE_COMMIT_STOPWATCH_NAME,
> > >                                              time_msec());
> > > -                            ct_zones_commit(br_int,
> > > -
> > &ct_zones_data->ctx.pending);
> > > +                            ct_zones_commit(br_int, br_int_dp,
> > ovs_idl_txn,
> > > +                                            &ct_zones_data->ctx);
> > >
> > stopwatch_stop(CT_ZONE_COMMIT_STOPWATCH_NAME,
> > >                                             time_msec());
> > >                          }
> > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> > > index 58e941193..1ad347419 100644
> > > --- a/lib/ovn-util.c
> > > +++ b/lib/ovn-util.c
> > > @@ -816,6 +816,23 @@ str_tolower(const char *orig)
> > >      return copy;
> > >  }
> > >
> > > +/* This is a wrapper function which get the value associated with 'key'
> > in
> > > + * 'smap' and converts it to a long long. If 'key' is not in 'smap' or a
> > > + * valid unsigned integer can't be parsed from its value, returns 'def'.
> > > + */
> > > +long long
> > > +ovn_smap_get_llong(const struct smap *smap, const char *key, long long
> > def)
> > > +{
> > > +    const char *value = smap_get(smap, key);
> > > +    long long ll_value;
> > > +
> > > +    if (!value || !str_to_llong(value, 10, &ll_value)) {
> > > +        return def;
> > > +    }
> > > +
> > > +    return ll_value;
> > > +}
> > > +
> > >  /* For a 'key' of the form "IP:port" or just "IP", sets 'port',
> > >   * 'ip_address' and 'ip' ('struct in6_addr' IPv6 or IPv4 mapped
> > address).
> > >   * The caller must free() the memory allocated for 'ip_address'.
> > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> > > index f75b821b6..ae971ce5a 100644
> > > --- a/lib/ovn-util.h
> > > +++ b/lib/ovn-util.h
> > > @@ -211,6 +211,9 @@ char *normalize_v46_prefix(const struct in6_addr
> > *prefix, unsigned int plen);
> > >   */
> > >  char *str_tolower(const char *orig);
> > >
> > > +long long ovn_smap_get_llong(const struct smap *smap, const char *key,
> > > +                             long long def);
> > > +
> > >  /* OVN daemon options. Taken from ovs/lib/daemon.h. */
> > >  #define OVN_DAEMON_OPTION_ENUMS                     \
> > >      OVN_OPT_DETACH,                                 \
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index 5b50ea191..325b412db 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -741,6 +741,14 @@ ovn_datapath_update_external_ids(struct
> > ovn_datapath *od)
> > >          smap_add(&ids, "name2", name2);
> > >      }
> > >
> > > +    int64_t ct_zone_limit = ovn_smap_get_llong(od->nbs ?
> > > +                                               &od->nbs->other_config :
> > > +                                               &od->nbr->options,
> > > +                                               "ct-zone-limit", -1);
> > > +    if (ct_zone_limit > 0) {
> > > +        smap_add_format(&ids, "ct-zone-limit", "%"PRId64,
> > ct_zone_limit);
> > > +    }
> > > +
> > >      /* Set interconn-ts. */
> > >      if (od->nbs) {
> > >          const char *ts = smap_get(&od->nbs->other_config,
> > "interconn-ts");
> > > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > > index 0f9a1005a..7087c0cf2 100644
> > > --- a/ovn-nb.xml
> > > +++ b/ovn-nb.xml
> > > @@ -747,6 +747,17 @@
> > >          this timeout will be automatically removed. The value defaults
> > >          to 0, which means disabled.
> > >        </column>
> > > +
> > > +      <column name="other_config" key="ct-zone-limit"
> > > +              type='{"type": "integer", "minInteger": 0, "maxInteger":
> > 4294967295}'>
> > > +        CT zone <code>limit</code> value for given
> > > +        <ref table="Logical_Switch"/>. This value will be propagated to
> > all
> > > +        <ref table="Logical_Switch_Port"/> when configured, but can be
> > > +        overwritten individually per <ref
> > table="Logical_Switch_Port"/>. The
> > > +        value 0 means unlimited. When the option is not present the
> > limit
> > > +        is not set and the zone limit is derived from OvS default
> > datapath
> > > +        limit.
> > > +      </column>
> > >      </group>
> > >
> > >      <group title="IP Multicast Snooping Options">
> > > @@ -1148,6 +1159,16 @@
> > >            <code>false</code>.
> > >          </column>
> > >
> > > +        <column name="options" key="ct-zone-limit"
> > > +                type='{"type": "integer", "minInteger": 0,
> > "maxInteger": 4294967295}'>
> > > +          CT zone <code>limit</code> value for given
> > > +          <ref table="Logical_Switch_Port"/>. This value has priority
> > over
> > > +          limit specified on <ref table="Logical_Switch"/> when
> > configured.
> > > +          The value 0 means unlimited. When the option is not present
> > the limit
> > > +          is not set and the zone limit is derived from OvS default
> > datapath
> > > +          limit.
> > > +        </column>
> > > +
> > >        </group>
> > >
> > >        <group title="Options for localnet ports">
> > > @@ -2811,6 +2832,14 @@ or
> > >          </p>
> > >
> > >        </column>
> > > +
> > > +      <column name="options" key="ct-zone-limit"
> > > +              type='{"type": "integer", "minInteger": 0, "maxInteger":
> > 4294967295}'>
> > > +        CT zone <code>limit</code> value for given
> > > +        <ref table="Logical_Router"/>. The value 0 means unlimited,
> > when the
> > > +        option is not present the limit is not set and the zone limit is
> > > +        derived from OvS default datapath limit.
> > > +      </column>
> > >      </group>
> > >
> > >      <group title="Common Columns">
> > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> > > index 9cb099e68..cc308468b 100644
> > > --- a/tests/ovn-controller.at
> > > +++ b/tests/ovn-controller.at
> > > @@ -3127,3 +3127,102 @@ OVS_WAIT_UNTIL([grep -q 'tcp:127.0.0.1:1235:
> > connected' hv1/ovn-controller.log])
> > >
> > >  OVN_CLEANUP([hv1])
> > >  AT_CLEANUP
> > > +
> > > +OVN_FOR_EACH_NORTHD([
> > > +AT_SETUP([ovn-controller - CT zone limit])
> > > +ovn_start
> > > +
> > > +net_add n1
> > > +sim_add hv1
> > > +as hv1
> > > +check ovs-vsctl add-br br-phys
> > > +ovn_attach n1 br-phys 192.168.0.1
> > > +
> > > +check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone
> > > +
> > > +check ovs-vsctl add-port br-int lsp \
> > > +    -- set Interface lsp external-ids:iface-id=lsp
> > > +
> > > +check ovn-nbctl lr-add lr
> > > +
> > > +check ovn-nbctl ls-add ls
> > > +check ovn-nbctl lsp-add ls ls-lr
> > > +check ovn-nbctl lsp-set-type ls-lr router
> > > +check ovn-nbctl lsp-set-addresses ls-lr router
> > > +check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1
> > > +
> > > +check ovn-nbctl lsp-add ls lsp
> > > +check ovn-nbctl lsp-set-addresses lsp "00:00:00:00:00:02 10.0.0.2"
> > > +
> > > +check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1
> > > +check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1
> > > +
> > > +wait_for_ports_up
> > > +check ovn-nbctl --wait=hv sync
> > > +
> > > +get_zone_num() {
> > > +    output=$1
> > > +    name=$2
> > > +
> > > +    printf "$output" | grep $name | cut -d ' ' -f 2
> > > +}
> > > +
> > > +check_ovs_ct_limit() {
> > > +    zone=$1
> > > +    limit=$2
> > > +
> > > +    AT_CHECK_UNQUOTED([ovs-appctl dpctl/ct-get-limits zone=$zone | sed
> > "s/count=.*/count=?/;s/default limit=.*/default limit=?/" | sort], [0], [dnl
> > > +default limit=?
> > > +zone=$zone,limit=$limit,count=?
> > > +])
> > > +}
> > > +
> > > +wait_ovs_ct_limit_count() {
> > > +    count=$1
> > > +
> > > +    OVS_WAIT_UNTIL([test $count -eq $(ovs-vsctl --no-headings
> > --format=table list CT_Zone | wc -l)])
> > > +}
> > > +
> > > +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list)
> > > +lr_dnat=$(get_zone_num "$ct_zones" lr_dnat)
> > > +lr_snat=$(get_zone_num "$ct_zones" lr_snat)
> > > +
> > > +ls_dnat=$(get_zone_num "$ct_zones" ls_dnat)
> > > +ls_snat=$(get_zone_num "$ct_zones" ls_snat)
> > > +
> > > +lsp=$(get_zone_num "$ct_zones" lsp)
> > > +
> > > +wait_ovs_ct_limit_count 0
> > > +
> > > +check ovn-nbctl --wait=hv set Logical_Router lr options:ct-zone-limit=5
> > > +wait_ovs_ct_limit_count 2
> > > +check_ovs_ct_limit $lr_dnat 5
> > > +check_ovs_ct_limit $lr_snat 5
> > > +
> > > +check ovn-nbctl --wait=hv remove Logical_Router lr options ct-zone-limit
> > > +wait_ovs_ct_limit_count 0
> > > +
> > > +check ovn-nbctl --wait=hv set Logical_Switch ls
> > other_config:ct-zone-limit=10
> > > +wait_ovs_ct_limit_count 3
> > > +check_ovs_ct_limit $ls_dnat 10
> > > +check_ovs_ct_limit $ls_snat 10
> > > +check_ovs_ct_limit $lsp 10
> > > +
> > > +check ovn-nbctl --wait=hv set Logical_Switch_Port lsp
> > options:ct-zone-limit=5
> > > +wait_ovs_ct_limit_count 3
> > > +check_ovs_ct_limit $ls_dnat 10
> > > +check_ovs_ct_limit $ls_snat 10
> > > +check_ovs_ct_limit $lsp 5
> > > +
> > > +check ovn-nbctl --wait=hv remove Logical_Switch_Port lsp options
> > ct-zone-limit
> > > +wait_ovs_ct_limit_count 3
> > > +check_ovs_ct_limit $ls_dnat 10
> > > +check_ovs_ct_limit $ls_snat 10
> > > +check_ovs_ct_limit $lsp 10
> > > +
> > > +check ovn-nbctl --wait=hv remove Logical_Switch ls other_config
> > ct-zone-limit
> > > +wait_ovs_ct_limit_count 0
> > > +
> > > +OVN_CLEANUP([hv1])
> > > +AT_CLEANUP
> > > +])
> > > --
> > > 2.45.2
> > >
> > > _______________________________________________
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
> >
> Thanks,
> Ales
>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA <https://www.redhat.com>
>
> amu...@redhat.com
> <https://red.ht/sig>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to