On 2/25/22 13:44, Han Zhou wrote:
On Fri, Feb 25, 2022 at 9:15 AM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

At the moment ovs meters are reconfigured by ovn just when a
meter is allocated or removed while updates for an already
allocated meter are ignored. This issue can be easily verified
with the following reproducer:

$ovn-nbctl meter-add meter0 drop 10 pktps
$ovn-nbctl --log --meter=meter0 acl-add sw0 to-lport 1000 'tcp.dst == 80'
drop
$ovn-nbctl --may-exist meter-add meter0 drop 20 pktps
$ovs-ofctl -O OpenFlow15 dump-meters br-int

In order to fix the issue introduce incremental processing for ovn
metering configured through sb db meter/meter-band tables and remove
meter extend table management (extend table is now considering just
meters created through the set_meter() action since we do not have
an entry in the sb db for them).

Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1939524
Acked-by: Mark Michelson <mmich...@redhat.com>

Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>

Thanks Lorenzo for fixing this. I am concerned with sending messages to OVS
directly within the I-P engine. That has been the responsibility of ofctrl
module, and the engine was supposed to compute the output data only. Sorry
for the late response, but can we avoid this?

Hi Han, I didn't realize when reviewing this that this was something we were not supposed to do. Just to be certain I understand, the idea here would be for the engine node to calculate the new data, then pass that data as an input to ofctrl_put()?


Thanks,
Han

---
  controller/lflow.c          |  28 +----
  controller/lflow.h          |   1 +
  controller/ofctrl.c         |  34 ++----
  controller/ofctrl.h         |   6 +-
  controller/ovn-controller.c | 230 +++++++++++++++++++++++++++++++++++-
  include/ovn/actions.h       |   1 +
  lib/actions.c               |   7 +-
  lib/ovn-util.c              |   9 ++
  lib/ovn-util.h              |  15 +++
  tests/ovn-controller.at     |   6 +-
  tests/ovn-performance.at    |  22 ++++
  tests/ovn.at                |  69 ++++++++++-
  tests/system-ovn.at         |  17 +++
  13 files changed, 382 insertions(+), 63 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index e169edef1..83cbc5b87 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1083,27 +1083,6 @@ lflow_handle_changed_ref(enum ref_type ref_type,
const char *ref_name,
      return ret;
  }

-static void
-lflow_parse_ctrl_meter(const struct sbrec_logical_flow *lflow,
-                       struct ovn_extend_table *meter_table,
-                       uint32_t *meter_id)
-{
-    ovs_assert(meter_id);
-    *meter_id = NX_CTLR_NO_METER;
-
-    if (lflow->controller_meter) {
-        *meter_id = ovn_extend_table_assign_id(meter_table,
-                                               lflow->controller_meter,
-                                               lflow->header_.uuid);
-        if (*meter_id == EXT_TABLE_ID_INVALID) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
-            VLOG_WARN_RL(&rl, "Unable to assign id for meter: %s",
-                         lflow->controller_meter);
-            return;
-        }
-    }
-}
-
  static void
  add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
                            const struct local_datapath *ldp,
@@ -1126,8 +1105,10 @@ add_matches_to_flow_table(const struct
sbrec_logical_flow *lflow,
       * controller.
       */
      uint32_t ctrl_meter_id = NX_CTLR_NO_METER;
-    lflow_parse_ctrl_meter(lflow, l_ctx_out->meter_table,
-                           &ctrl_meter_id);
+    if (lflow->controller_meter) {
+        ctrl_meter_id =
ovn_controller_get_meter_id(l_ctx_in->meter_table,
+
  lflow->controller_meter);
+    }

      /* Encode OVN logical actions into OpenFlow. */
      uint64_t ofpacts_stub[1024 / 8];
@@ -1153,6 +1134,7 @@ add_matches_to_flow_table(const struct
sbrec_logical_flow *lflow,
          .fdb_ptable = OFTABLE_GET_FDB,
          .fdb_lookup_ptable = OFTABLE_LOOKUP_FDB,
          .ctrl_meter_id = ctrl_meter_id,
+        .meter_hash = l_ctx_in->meter_table,
      };
      ovnacts_encode(ovnacts->data, ovnacts->size, &ep, &ofpacts);

diff --git a/controller/lflow.h b/controller/lflow.h
index d61733bc2..4760c5f62 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -153,6 +153,7 @@ struct lflow_ctx_in {
      const struct sset *active_tunnels;
      const struct sset *related_lport_ids;
      const struct hmap *chassis_tunnels;
+    const struct shash *meter_table;
  };

  struct lflow_ctx_out {
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 19aa787f9..2f484c662 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -369,8 +369,6 @@ static enum mf_field_id mff_ovn_geneve;
   * is restarted, even if there is no change in the desired flow table. */
  static bool need_reinstall_flows;

-static ovs_be32 queue_msg(struct ofpbuf *);
-
  static struct ofpbuf *encode_flow_mod(struct ofputil_flow_mod *);

  static struct ofpbuf *encode_group_mod(const struct ofputil_group_mod *);
@@ -822,7 +820,7 @@ ofctrl_get_cur_cfg(void)
      return cur_cfg;
  }

-static ovs_be32
+ovs_be32
  queue_msg(struct ofpbuf *msg)
  {
      const struct ofp_header *oh = msg->data;
@@ -1978,29 +1976,16 @@ add_meter_string(struct ovn_extend_table_info
*m_desired,
      free(meter_string);
  }

-static void
-add_meter(struct ovn_extend_table_info *m_desired,
-          const struct sbrec_meter_table *meter_table,
-          struct ovs_list *msgs)
+void
+meter_create_msg(struct ovs_list *msgs,
+                 const struct sbrec_meter *sb_meter,
+                 int cmd, int id)
  {
-    const struct sbrec_meter *sb_meter;
-    SBREC_METER_TABLE_FOR_EACH (sb_meter, meter_table) {
-        if (!strcmp(m_desired->name, sb_meter->name)) {
-            break;
-        }
-    }
-
-    if (!sb_meter) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-        VLOG_ERR_RL(&rl, "could not find meter named \"%s\"",
m_desired->name);
-        return;
-    }
-
      struct ofputil_meter_mod mm;
-    mm.command = OFPMC13_ADD;
-    mm.meter.meter_id = m_desired->table_id;
-    mm.meter.flags = OFPMF13_STATS;
+    mm.command = cmd;
+    mm.meter.meter_id = id;

+    mm.meter.flags = OFPMF13_STATS;
      if (!strcmp(sb_meter->unit, "pktps")) {
          mm.meter.flags |= OFPMF13_PKTPS;
      } else {
@@ -2329,7 +2314,6 @@ void
  ofctrl_put(struct ovn_desired_flow_table *lflow_table,
             struct ovn_desired_flow_table *pflow_table,
             struct shash *pending_ct_zones,
-           const struct sbrec_meter_table *meter_table,
             uint64_t req_cfg,
             bool lflows_changed,
             bool pflows_changed)
@@ -2414,8 +2398,6 @@ ofctrl_put(struct ovn_desired_flow_table
*lflow_table,
              /* The "set-meter" action creates a meter entry name that
               * describes the meter itself. */
              add_meter_string(m_desired, &msgs);
-        } else {
-            add_meter(m_desired, meter_table, &msgs);
          }
      }

diff --git a/controller/ofctrl.h b/controller/ofctrl.h
index ad8f4be65..7c1844a72 100644
--- a/controller/ofctrl.h
+++ b/controller/ofctrl.h
@@ -29,6 +29,7 @@ struct match;
  struct ofpbuf;
  struct ovsrec_bridge;
  struct sbrec_meter_table;
+struct sbrec_meter;
  struct shash;

  struct ovn_desired_flow_table {
@@ -55,7 +56,6 @@ enum mf_field_id ofctrl_get_mf_field_id(void);
  void ofctrl_put(struct ovn_desired_flow_table *lflow_table,
                  struct ovn_desired_flow_table *pflow_table,
                  struct shash *pending_ct_zones,
-                const struct sbrec_meter_table *,
                  uint64_t nb_cfg,
                  bool lflow_changed,
                  bool pflow_changed);
@@ -145,5 +145,9 @@ void ofctrl_check_and_add_flow_metered(struct
ovn_desired_flow_table *,
  bool ofctrl_is_connected(void);
  void ofctrl_set_probe_interval(int probe_interval);
  void ofctrl_get_memory_usage(struct simap *usage);
+ovs_be32 queue_msg(struct ofpbuf *msg);
+void meter_create_msg(struct ovs_list *msgs,
+                      const struct sbrec_meter *sb_meter,
+                      int cmd, int id);

  #endif /* controller/ofctrl.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index e4b8b1bdd..805f5dbf3 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -76,6 +76,8 @@
  #include "stopwatch.h"
  #include "lib/inc-proc-eng.h"
  #include "hmapx.h"
+#include "openvswitch/ofp-util.h"
+#include "openvswitch/ofp-meter.h"

  VLOG_DEFINE_THIS_MODULE(main);

@@ -968,7 +970,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
      SB_NODE(dhcpv6_options, "dhcpv6_options") \
      SB_NODE(dns, "dns") \
      SB_NODE(load_balancer, "load_balancer") \
-    SB_NODE(fdb, "fdb")
+    SB_NODE(fdb, "fdb") \
+    SB_NODE(meter, "meter")

  enum sb_engine_node {
  #define SB_NODE(NAME, NAME_STR) SB_##NAME,
@@ -1543,6 +1546,221 @@ addr_sets_sb_address_set_handler(struct
engine_node *node, void *data)
      return true;
  }

+struct ed_type_meter {
+    unsigned long *ids;
+    struct shash meter_sets;
+};
+
+static void *
+en_meter_init(struct engine_node *node OVS_UNUSED,
+              struct engine_arg *arg OVS_UNUSED)
+{
+    struct ed_type_meter *m = xzalloc(sizeof *m);
+    m->ids = bitmap_allocate(MAX_METER_ID);
+    bitmap_set1(m->ids, 0); /* table id 0 is invalid. */
+    shash_init(&m->meter_sets);
+
+    return m;
+}
+
+static bool
+en_meter_data_check_bands(struct ed_meter_data *md,
+                          const struct sbrec_meter *sb_meter)
+{
+    if (md->n_bands != sb_meter->n_bands) {
+        return true;
+    }
+
+    for (int i = 0; i < sb_meter->n_bands; i++) {
+        int j;
+        for (j = 0; j < md->n_bands; j++) {
+            if (sb_meter->bands[i]->rate == md->bands[j].rate &&
+                sb_meter->bands[i]->burst_size ==
md->bands[j].burst_size) {
+                break;
+            }
+        }
+        if (j == md->n_bands) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+static void
+en_meter_data_update(struct ed_meter_data *md,
+                     const struct sbrec_meter *sb_meter)
+{
+    free(md->bands);
+    md->n_bands = sb_meter->n_bands;
+    md->bands = xcalloc(md->n_bands, sizeof *md->bands);
+    for (int i = 0; i < sb_meter->n_bands; i++) {
+        md->bands[i].rate = sb_meter->bands[i]->rate;
+        md->bands[i].burst_size = sb_meter->bands[i]->burst_size;
+    }
+}
+
+static struct ed_meter_data *
+en_meter_data_alloc(struct ed_type_meter *m,
+                    const struct sbrec_meter *sb_meter)
+{
+    uint32_t id = bitmap_scan(m->ids, 0, 1, MAX_METER_ID + 1);
+    if (id == MAX_METER_ID + 1) {
+        static struct vlog_rate_limit rl =
+            VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_ERR_RL(&rl, "%"PRIu32" out of meter ids.", id);
+        return NULL;
+    }
+
+    struct ed_meter_data *md = xzalloc(sizeof *md);
+    bitmap_set1(m->ids, id);
+    md->id = id;
+    en_meter_data_update(md, sb_meter);
+    shash_add(&m->meter_sets, sb_meter->name, md);
+
+    return md;
+}
+
+static void
+en_meter_data_destroy(struct ed_type_meter *m, struct ed_meter_data *md)
+{
+    bitmap_set0(m->ids, md->id);
+    free(md->bands);
+    free(md);
+}
+
+static void
+en_meter_send_msgs(struct ovs_list *msgs)
+{
+    if (ovs_list_is_empty(msgs)) {
+        return;
+    }
+
+    struct ofpbuf *barrier =
ofputil_encode_barrier_request(OFP15_VERSION);
+    ovs_list_push_back(msgs, &barrier->list_node);
+    /* Queue the messages. */
+    struct ofpbuf *msg;
+    LIST_FOR_EACH_POP (msg, list_node, msgs) {
+        queue_msg(msg);
+    }
+}
+
+static void
+en_meter_delete_msg(struct ovs_list *msgs, uint32_t id)
+{
+    struct ofputil_meter_mod mm = {
+        .command = OFPMC13_DELETE,
+        .meter = { .meter_id = id },
+    };
+    struct ofpbuf *msg = ofputil_encode_meter_mod(OFP15_VERSION, &mm);
+    ovs_list_push_back(msgs, &msg->list_node);
+}
+
+static void
+en_meter_cleanup(void *data)
+{
+    struct ed_type_meter *m = data;
+
+    struct shash_node *node, *next;
+    SHASH_FOR_EACH_SAFE (node, next, &m->meter_sets) {
+        struct ed_meter_data *md = node->data;
+        shash_delete(&m->meter_sets, node);
+        en_meter_data_destroy(m, md);
+    }
+    shash_destroy(&m->meter_sets);
+    bitmap_free(m->ids);
+}
+
+static void
+en_meter_run(struct engine_node *node, void *data)
+{
+    struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
+    const struct sbrec_meter *sb_meter;
+    struct ed_type_meter *m = data;
+    struct ed_meter_data *md;
+
+    struct sbrec_meter_table *m_table =
+        (struct sbrec_meter_table *)EN_OVSDB_GET(
+            engine_get_input("SB_meter", node));
+
+    /* remove stale entries */
+    struct shash_node *iter, *next;
+    SHASH_FOR_EACH_SAFE (iter, next, &m->meter_sets) {
+        bool found = false;
+        SBREC_METER_TABLE_FOR_EACH (sb_meter, m_table) {
+            if (!strcmp(sb_meter->name, iter->name)) {
+                found = true;
+                break;
+            }
+        }
+        if (!found) {
+            md = iter->data;
+            shash_delete(&m->meter_sets, iter);
+            en_meter_delete_msg(&msgs, md->id);
+            en_meter_data_destroy(m, md);
+        }
+    }
+
+    /* add new entries or update current ones */
+    SBREC_METER_TABLE_FOR_EACH (sb_meter, m_table) {
+        md = shash_find_data(&m->meter_sets, sb_meter->name);
+        if (md) {
+            if (en_meter_data_check_bands(md, sb_meter)) {
+                en_meter_data_update(md, sb_meter);
+                meter_create_msg(&msgs, sb_meter, OFPMC13_MODIFY,
md->id);
+            }
+        } else {
+            md = en_meter_data_alloc(m, sb_meter);
+            if (md) {
+                meter_create_msg(&msgs, sb_meter, OFPMC13_ADD, md->id);
+            }
+        }
+    }
+
+    en_meter_send_msgs(&msgs);
+    engine_set_node_state(node, EN_UPDATED);
+}
+
+static bool
+meter_sb_meter_handler(struct engine_node *node, void *data)
+{
+    struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
+    struct ed_type_meter *m = data;
+    struct sbrec_meter_table *m_table =
+        (struct sbrec_meter_table *)EN_OVSDB_GET(
+            engine_get_input("SB_meter", node));
+
+    const struct sbrec_meter *iter;
+    SBREC_METER_TABLE_FOR_EACH_TRACKED (iter, m_table) {
+        struct ed_meter_data *md;
+        if (sbrec_meter_is_deleted(iter)) {
+            md = shash_find_and_delete(&m->meter_sets, iter->name);
+            if (md) {
+                en_meter_delete_msg(&msgs, md->id);
+                en_meter_data_destroy(m, md);
+                engine_set_node_state(node, EN_UPDATED);
+            }
+        } else {
+            if (sbrec_meter_is_new(iter)) {
+                md = en_meter_data_alloc(m, iter);
+                if (md) {
+                    meter_create_msg(&msgs, iter, OFPMC13_ADD, md->id);
+                    engine_set_node_state(node, EN_UPDATED);
+                }
+            } else {
+                md = shash_find_data(&m->meter_sets, iter->name);
+                if (md) {
+                    en_meter_data_update(md, iter);
+                }
+                meter_create_msg(&msgs, iter, OFPMC13_MODIFY, md->id);
+            }
+        }
+    }
+    en_meter_send_msgs(&msgs);
+
+    return true;
+}
+
  struct ed_type_port_groups{
      /* A copy of SB port_groups, each converted as a sset for efficient
lport
       * lookup. */
@@ -2298,6 +2516,10 @@ init_lflow_ctx(struct engine_node *node,
          (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
              engine_get_input("OVS_open_vswitch", node));

+    struct ed_type_meter *meter_data =
+        engine_get_input_data("meter", node);
+    struct shash *meter_sets = &meter_data->meter_sets;
+
      const char *chassis_id = get_ovs_chassis_id(ovs_table);
      const struct sbrec_chassis *chassis = NULL;
      struct ovsdb_idl_index *sbrec_chassis_by_name =
@@ -2348,6 +2570,7 @@ init_lflow_ctx(struct engine_node *node,
      l_ctx_in->active_tunnels = &rt_data->active_tunnels;
      l_ctx_in->related_lport_ids = &rt_data->related_lports.lport_ids;
      l_ctx_in->chassis_tunnels = &non_vif_data->chassis_tunnels;
+    l_ctx_in->meter_table = meter_sets;

      l_ctx_out->flow_table = &fo->flow_table;
      l_ctx_out->group_table = &fo->group_table;
@@ -3222,6 +3445,7 @@ main(int argc, char *argv[])
      ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lflow_output,
"logical_flow_output");
      ENGINE_NODE(flow_output, "flow_output");
      ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets");
+    ENGINE_NODE(meter, "meter");
      ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups");

  #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
@@ -3243,6 +3467,8 @@ main(int argc, char *argv[])
      engine_add_input(&en_port_groups, &en_runtime_data,
                       port_groups_runtime_data_handler);

+    engine_add_input(&en_meter, &en_sb_meter, meter_sb_meter_handler);
+
      engine_add_input(&en_non_vif_data, &en_ovs_open_vswitch, NULL);
      engine_add_input(&en_non_vif_data, &en_ovs_bridge, NULL);
      engine_add_input(&en_non_vif_data, &en_sb_chassis, NULL);
@@ -3287,6 +3513,7 @@ main(int argc, char *argv[])
                       lflow_output_runtime_data_handler);
      engine_add_input(&en_lflow_output, &en_non_vif_data,
                       NULL);
+    engine_add_input(&en_lflow_output, &en_meter, NULL);

      engine_add_input(&en_lflow_output, &en_sb_multicast_group,
                       lflow_output_sb_multicast_group_handler);
@@ -3768,7 +3995,6 @@ main(int argc, char *argv[])
                          ofctrl_put(&lflow_output_data->flow_table,
                                     &pflow_output_data->flow_table,
                                     &ct_zones_data->pending,
-
sbrec_meter_table_get(ovnsb_idl_loop.idl),
                                     ofctrl_seqno_get_req_cfg(),
                                     engine_node_changed(&en_lflow_output),

engine_node_changed(&en_pflow_output));
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0641b927e..bed24d9d1 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -799,6 +799,7 @@ struct ovnact_encode_params {
                                  * 'lookup_fdb' to resubmit. */
      uint32_t ctrl_meter_id;     /* Meter to be used if the resulting flow
                                     sends packets to controller. */
+    const struct shash *meter_hash;
  };

  void ovnacts_encode(const struct ovnact[], size_t ovnacts_len,
diff --git a/lib/actions.c b/lib/actions.c
index 5d3caaf2b..f7f5aae52 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -3411,10 +3411,9 @@ encode_LOG(const struct ovnact_log *log,
  {
      uint32_t meter_id = NX_CTLR_NO_METER;

-    if (log->meter) {
-        meter_id = ovn_extend_table_assign_id(ep->meter_table,
log->meter,
-                                              ep->lflow_uuid);
-        if (meter_id == EXT_TABLE_ID_INVALID) {
+    if (ep->meter_hash && log->meter) {
+        meter_id = ovn_controller_get_meter_id(ep->meter_hash,
log->meter);
+        if (meter_id == MAX_METER_ID + 1) {
              VLOG_WARN("Unable to assign id for log meter: %s",
log->meter);
              return;
          }
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index a22ae84fe..b7e9d9b2c 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -30,6 +30,8 @@
  #include "socket-util.h"
  #include "svec.h"
  #include "unixctl.h"
+#include "controller/ovn-controller.h"
+#include "openvswitch/ofp-actions.h"

  VLOG_DEFINE_THIS_MODULE(ovn_util);

@@ -806,3 +808,10 @@ get_bridge(const struct ovsrec_bridge_table
*bridge_table, const char *br_name)
      }
      return NULL;
  }
+
+uint32_t
+ovn_controller_get_meter_id(const struct shash *meter_sets, const char
*name)
+{
+    struct ed_meter_data *md = shash_find_data(meter_sets, name);
+    return md ? md->id : NX_CTLR_NO_METER;
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 1fe91ba99..c5bb21d2d 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -303,5 +303,20 @@ struct ovsrec_bridge_table;
  const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table
*,
                                         const char *br_name);

+#define MAX_METER_ID    (MAX_EXT_TABLE_ID / 2)
+struct ed_meter_band_data {
+    int64_t burst_size;
+    int64_t rate;
+};
+
+struct ed_meter_data {
+    uint32_t id; /* ovs meter id */
+    struct ed_meter_band_data *bands;
+    size_t n_bands;
+};
+
+struct shash;
+uint32_t ovn_controller_get_meter_id(const struct shash *meter_sets,
+                                     const char *name);

  #endif /* OVN_UTIL_H */
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index f5e5448a4..6e4c24f0f 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -725,7 +725,7 @@ check ovn-nbctl meter-add event-elb drop 100 pktps 10
  check ovn-nbctl --wait=hv copp-add copp0 event-elb event-elb
  check ovn-nbctl --wait=hv ls-copp-add copp0 ls1

-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.0f | grep -q meter_id=32768])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.0f | grep -q meter_id=1])

  check ovn-nbctl copp-del copp0
  AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
@@ -738,13 +738,13 @@ check ovn-nbctl --wait=hv copp-add copp1 reject
acl-meter
  check ovn-nbctl ls-copp-add copp1 ls1
  check ovn-nbctl --wait=hv acl-add ls1 from-lport 1002 'inport == "lsp1"
&& ip && udp' reject

-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.16 | grep -q meter_id=32768])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.16 | grep -q meter_id=1])

  # arp metering
  check ovn-nbctl meter-add arp-meter drop 200 pktps 0
  check ovn-nbctl --wait=hv copp-add copp2 arp-resolve arp-meter
  check ovn-nbctl --wait=hv lr-copp-add copp2 lr1
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.00 | grep -q meter_id=32769])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.00 | grep -q meter_id=2])

  OVN_CLEANUP([hv1])
  AT_CLEANUP
diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at
index 10341ad72..aed1cb53a 100644
--- a/tests/ovn-performance.at
+++ b/tests/ovn-performance.at
@@ -543,6 +543,28 @@ OVN_CONTROLLER_EXPECT_HIT(
      [as hv3 ovs-vsctl set interface vgw3
external-ids:ovn-egress-iface=true]
  )

+ovn-nbctl --wait=hv meter-add meter0 drop 100 pktps 10
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+    [hv1 hv2 hv3 hv4], [lflow_run],
+    [ovn-nbctl --wait=hv copp-add copp0 arp meter0]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+    [hv1 hv2 hv3 hv4], [lflow_run],
+    [ovn-nbctl --wait=hv lr-copp-add copp0 lr1]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+    [hv1 hv2 hv3 hv4], [lflow_run],
+    [ovn-nbctl --wait=hv --may-exist meter-add meter0 drop 200 pktps 10]
+)
+
+OVN_CONTROLLER_EXPECT_HIT(
+    [hv1 hv2 hv3 hv4], [lflow_run],
+    [ovn-nbctl --wait=hv meter-del meter0]
+)
+
  for i in 1 2; do
      j=$((i%2 + 1))
      lp=lp$i
diff --git a/tests/ovn.at b/tests/ovn.at
index 7adf6003d..37bd4fce7 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9158,9 +9158,10 @@ echo "Meter duration: $d_secs"
  AT_SKIP_IF([test $d_secs -gt 9])

  # Print some information that may help debugging.
-AT_CHECK([as hv ovs-appctl -t ovn-controller meter-table-list], [0], [dnl
-http-rl1: 32768
-http-rl2: 32769
+
+AT_CHECK([as hv ovs-ofctl dump-meters br-int -O OpenFlow13 | grep
meter], [0], [dnl
+meter=1 pktps stats bands=
+meter=2 pktps stats bands=
  ])
  as hv ovs-ofctl -O OpenFlow13 meter-stats br-int

@@ -9223,9 +9224,12 @@ ovn-nbctl acl-del lsw0 to-lport 1000 'tcp.dst==80'
  ovn-nbctl --wait=hv sync
  AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br-int | grep meter], [0],
[ignore], [ignore])

-# Delete acl2, meter should be deleted in OVS
  ovn-nbctl acl-del lsw0 to-lport 1000 'tcp.dst==81'
  ovn-nbctl --wait=hv sync
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br-int | grep meter], [0],
[ignore], [ignore])
+
+ovn-nbctl meter-del http-rl1
+ovn-nbctl --wait=hv sync
  AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br-int | grep meter], [1])

  OVN_CLEANUP([hv])
@@ -29647,3 +29651,60 @@ OVS_WAIT_UNTIL([
  OVN_CLEANUP([hv1],[hv2])
  AT_CLEANUP
  ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - check meters update])
+AT_KEYWORDS([meters-update])
+
+ovn_start
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.10
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lsp-add sw0 lsp
+
+as hv1 ovs-vsctl \
+    -- add-port br-int vif1 \
+    -- set Interface vif1 external_ids:iface-id=lsp
+
+# Wait for port to be bound.
+wait_row_count Chassis 1 name=hv1
+ch=$(fetch_column Chassis _uuid name=hv1)
+wait_row_count Port_Binding 1 logical_port=lsp chassis=$ch
+
+# Add a new meter
+check ovn-nbctl meter-add meter0 drop 10 pktps
+check ovn-nbctl --log --severity=alert --meter=meter0 \
+                --name=http acl-add sw0 to-lport 1000 'tcp.dst == 80'
drop
+check ovn-nbctl --wait=hv sync
+
+AT_CHECK([as hv1 ovs-ofctl -OOpenFlow15 dump-meters br-int | grep -q
rate=10], [0])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep -q meter_id=1], [0])
+
+# Update existing meter
+check ovn-nbctl --may-exist meter-add meter0 drop 20 pktps
+check ovn-nbctl --wait=hv sync
+
+AT_CHECK([as hv1 ovs-ofctl -OOpenFlow15 dump-meters br-int | grep -q
rate=20], [0])
+
+# Add a new meter
+check ovn-nbctl meter-add meter1 drop 20 pktps
+check ovn-nbctl --log --severity=alert --meter=meter1 \
+                --name=dns acl-add sw0 to-lport 1000 'udp.dst == 53' drop
+check ovn-nbctl --wait=hv sync
+AT_CHECK([as hv1 ovs-ofctl -OOpenFlow15 dump-meters br-int | grep -q
rate=20], [0])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep -q meter_id=2], [0])
+
+# Remove meter0
+check ovn-nbctl meter-del meter0
+check ovn-nbctl --wait=hv sync
+AT_CHECK([as hv1 ovs-ofctl -OOpenFlow15 dump-meters br-int | grep -q
rate=10], [1])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep -q meter_id=1], [1])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 2dcd7e906..e6c317083 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6661,8 +6661,25 @@ OVS_WAIT_UNTIL([
      test "${n_reject}" = "2"
  ])
  kill $(pidof tcpdump)
+rm -f reject.pcap
+
+# Let's update the meter
+NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &])
+check ovn-nbctl --may-exist meter-add acl-meter drop 10 pktps 0
+ip netns exec sw01 scapy -H <<-EOF
+p = IP(src="192.168.1.2", dst="192.168.1.1") / UDP(dport = 12345) /
Raw(b"X"*64)
+send (p, iface='sw01', loop = 0, verbose = 0, count = 100)
+EOF

+# 10pps + 1 burst size
+OVS_WAIT_UNTIL([
+    n_reject=$(grep unreachable reject.pcap | wc -l)
+    test "${n_reject}" = "20"
+])
+
+kill $(pidof tcpdump)
  rm -f reject.pcap
+
  NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &])
  check ovn-nbctl --wait=hv copp-del copp0 reject

--
2.35.1

_______________________________________________
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


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to