This is beneficial in a few ways:
- first, it reduces the number of different types of data the northd
I-P node has to process.
- it turns out the northd I-P node (whose recompute is rather costly)
doesn't really depend on meters or ACLs.
- prepares the ground for a pure I-P implementation for ACLs, with a
simple/clear dependency between NB.ACL and the lflow I-P node.
From a meters synchronization perspective this commit doesn't change any
of the existing behavior and logic. It mostly just moves the meters
related code out of northd.c and into en-meters.c.
Signed-off-by: Dumitru Ceara <[email protected]>
---
lib/stopwatch-names.h | 1
northd/automake.mk | 2
northd/en-lflow.c | 5 +
northd/en-meters.c | 281 ++++++++++++++++++++++++++++++++++++++++++++++
northd/en-meters.h | 44 +++++++
northd/en-northd.c | 6 -
northd/inc-proc-northd.c | 14 ++
northd/northd.c | 235 +-------------------------------------
northd/northd.h | 4 -
northd/ovn-northd.c | 3
tests/ovn-northd.at | 36 ++++++
11 files changed, 390 insertions(+), 241 deletions(-)
create mode 100644 northd/en-meters.c
create mode 100644 northd/en-meters.h
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index de6fca4ccc..98535fff5a 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -30,5 +30,6 @@
#define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
#define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
#define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
+#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
#endif
diff --git a/northd/automake.mk b/northd/automake.mk
index b17f1fdb54..f52766de0c 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -12,6 +12,8 @@ northd_ovn_northd_SOURCES = \
northd/en-northd.h \
northd/en-lflow.c \
northd/en-lflow.h \
+ northd/en-meters.c \
+ northd/en-meters.h \
northd/en-northd-output.c \
northd/en-northd-output.h \
northd/en-sync-sb.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 28ab1c67fb..0886d4c5ca 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -20,6 +20,7 @@
#include "en-lflow.h"
#include "en-northd.h"
+#include "en-meters.h"
#include "lib/inc-proc-eng.h"
#include "northd.h"
@@ -35,6 +36,8 @@ lflow_get_input_data(struct engine_node *node,
struct lflow_input *lflow_input)
{
struct northd_data *northd_data = engine_get_input_data("northd", node);
+ struct sync_meters_data *sync_meters_data =
+ engine_get_input_data("sync_meters", node);
lflow_input->nbrec_bfd_table =
EN_OVSDB_GET(engine_get_input("NB_bfd", node));
lflow_input->sbrec_bfd_table =
@@ -56,7 +59,7 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->ls_ports = &northd_data->ls_ports;
lflow_input->lr_ports = &northd_data->lr_ports;
lflow_input->port_groups = &northd_data->port_groups;
- lflow_input->meter_groups = &northd_data->meter_groups;
+ lflow_input->meter_groups = &sync_meters_data->meter_groups;
lflow_input->lbs = &northd_data->lbs;
lflow_input->features = &northd_data->features;
lflow_input->ovn_internal_version_changed =
diff --git a/northd/en-meters.c b/northd/en-meters.c
new file mode 100644
index 0000000000..aabd002b62
--- /dev/null
+++ b/northd/en-meters.c
@@ -0,0 +1,281 @@
+/*
+ * Copyright (c) 2023, 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>
+
+#include "openvswitch/vlog.h"
+#include "stopwatch.h"
+
+#include "en-meters.h"
+#include "lib/stopwatch-names.h"
+
+VLOG_DEFINE_THIS_MODULE(en_meters);
+
+static void build_meter_groups(struct shash *meter_group,
+ const struct nbrec_meter_table *);
+static void sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
+ const struct nbrec_meter_table *,
+ const struct nbrec_acl_table *,
+ const struct sbrec_meter_table *,
+ struct shash *meter_groups);
+
+void
+*en_sync_meters_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct sync_meters_data *data = xmalloc(sizeof *data);
+
+ *data = (struct sync_meters_data) {
+ .meter_groups = SHASH_INITIALIZER(&data->meter_groups),
+ };
+ return data;
+}
+
+void
+en_sync_meters_cleanup(void *data_)
+{
+ struct sync_meters_data *data = data_;
+
+ shash_destroy(&data->meter_groups);
+}
+
+void
+en_sync_meters_run(struct engine_node *node, void *data_)
+{
+ struct sync_meters_data *data = data_;
+
+ const struct nbrec_acl_table *acl_table =
+ EN_OVSDB_GET(engine_get_input("NB_acl", node));
+
+ const struct nbrec_meter_table *nb_meter_table =
+ EN_OVSDB_GET(engine_get_input("NB_meter", node));
+
+ const struct sbrec_meter_table *sb_meter_table =
+ EN_OVSDB_GET(engine_get_input("SB_meter", node));
+
+ const struct engine_context *eng_ctx = engine_get_context();
+
+ stopwatch_start(SYNC_METERS_RUN_STOPWATCH_NAME, time_msec());
+
+ build_meter_groups(&data->meter_groups, nb_meter_table);
+
+ sync_meters(eng_ctx->ovnsb_idl_txn, nb_meter_table, acl_table,
+ sb_meter_table, &data->meter_groups);
+
+ stopwatch_stop(SYNC_METERS_RUN_STOPWATCH_NAME, time_msec());
+ engine_set_node_state(node, EN_UPDATED);
+}
+
+const struct nbrec_meter*
+fair_meter_lookup_by_name(const struct shash *meter_groups,
+ const char *meter_name)
+{
+ const struct nbrec_meter *nb_meter =
+ meter_name ? shash_find_data(meter_groups, meter_name) : NULL;
+ if (nb_meter) {
+ return (nb_meter->fair && *nb_meter->fair) ? nb_meter : NULL;
+ }
+ return NULL;
+}
+
+struct band_entry {
+ int64_t rate;
+ int64_t burst_size;
+ const char *action;
+};
+
+static int
+band_cmp(const void *band1_, const void *band2_)
+{
+ const struct band_entry *band1p = band1_;
+ const struct band_entry *band2p = band2_;
+
+ if (band1p->rate != band2p->rate) {
+ return band1p->rate - band2p->rate;
+ } else if (band1p->burst_size != band2p->burst_size) {
+ return band1p->burst_size - band2p->burst_size;
+ } else {
+ return strcmp(band1p->action, band2p->action);
+ }
+}
+
+static bool
+bands_need_update(const struct nbrec_meter *nb_meter,
+ const struct sbrec_meter *sb_meter)
+{
+ if (nb_meter->n_bands != sb_meter->n_bands) {
+ return true;
+ }
+
+ /* Place the Northbound entries in sorted order. */
+ struct band_entry *nb_bands;
+ nb_bands = xmalloc(sizeof *nb_bands * nb_meter->n_bands);
+ for (size_t i = 0; i < nb_meter->n_bands; i++) {
+ struct nbrec_meter_band *nb_band = nb_meter->bands[i];
+
+ nb_bands[i].rate = nb_band->rate;
+ nb_bands[i].burst_size = nb_band->burst_size;
+ nb_bands[i].action = nb_band->action;
+ }
+ qsort(nb_bands, nb_meter->n_bands, sizeof *nb_bands, band_cmp);
+
+ /* Place the Southbound entries in sorted order. */
+ struct band_entry *sb_bands;
+ sb_bands = xmalloc(sizeof *sb_bands * sb_meter->n_bands);
+ for (size_t i = 0; i < sb_meter->n_bands; i++) {
+ struct sbrec_meter_band *sb_band = sb_meter->bands[i];
+
+ sb_bands[i].rate = sb_band->rate;
+ sb_bands[i].burst_size = sb_band->burst_size;
+ sb_bands[i].action = sb_band->action;
+ }
+ qsort(sb_bands, sb_meter->n_bands, sizeof *sb_bands, band_cmp);
+
+ bool need_update = false;
+ for (size_t i = 0; i < nb_meter->n_bands; i++) {
+ if (band_cmp(&nb_bands[i], &sb_bands[i])) {
+ need_update = true;
+ break;
+ }
+ }
+
+ free(nb_bands);
+ free(sb_bands);
+
+ return need_update;
+}
+
+static void
+sync_meters_iterate_nb_meter(struct ovsdb_idl_txn *ovnsb_txn,
+ const char *meter_name,
+ const struct nbrec_meter *nb_meter,
+ struct shash *sb_meters,
+ struct sset *used_sb_meters)
+{
+ const struct sbrec_meter *sb_meter;
+ bool new_sb_meter = false;
+
+ sb_meter = shash_find_data(sb_meters, meter_name);
+ if (!sb_meter) {
+ sb_meter = sbrec_meter_insert(ovnsb_txn);
+ sbrec_meter_set_name(sb_meter, meter_name);
+ shash_add(sb_meters, sb_meter->name, sb_meter);
+ new_sb_meter = true;
+ }
+ sset_add(used_sb_meters, meter_name);
+
+ if (new_sb_meter || bands_need_update(nb_meter, sb_meter)) {
+ struct sbrec_meter_band **sb_bands;
+ sb_bands = xcalloc(nb_meter->n_bands, sizeof *sb_bands);
+ for (size_t i = 0; i < nb_meter->n_bands; i++) {
+ const struct nbrec_meter_band *nb_band = nb_meter->bands[i];
+
+ sb_bands[i] = sbrec_meter_band_insert(ovnsb_txn);
+
+ sbrec_meter_band_set_action(sb_bands[i], nb_band->action);
+ sbrec_meter_band_set_rate(sb_bands[i], nb_band->rate);
+ sbrec_meter_band_set_burst_size(sb_bands[i],
+ nb_band->burst_size);
+ }
+ sbrec_meter_set_bands(sb_meter, sb_bands, nb_meter->n_bands);
+ free(sb_bands);
+ }
+
+ sbrec_meter_set_unit(sb_meter, nb_meter->unit);
+}
+
+static void
+sync_acl_fair_meter(struct ovsdb_idl_txn *ovnsb_txn,
+ struct shash *meter_groups,
+ const struct nbrec_acl *acl, struct shash *sb_meters,
+ struct sset *used_sb_meters)
+{
+ const struct nbrec_meter *nb_meter =
+ fair_meter_lookup_by_name(meter_groups, acl->meter);
+
+ if (!nb_meter) {
+ return;
+ }
+
+ char *meter_name = alloc_acl_log_unique_meter_name(acl);
+ sync_meters_iterate_nb_meter(ovnsb_txn, meter_name, nb_meter, sb_meters,
+ used_sb_meters);
+ free(meter_name);
+}
+
+static void
+build_meter_groups(struct shash *meter_groups,
+ const struct nbrec_meter_table *nb_meter_table)
+{
+ const struct nbrec_meter *nb_meter;
+
+ shash_clear(meter_groups);
+ NBREC_METER_TABLE_FOR_EACH (nb_meter, nb_meter_table) {
+ shash_add(meter_groups, nb_meter->name, nb_meter);
+ }
+}
+
+/* Each entry in the Meter and Meter_Band tables in OVN_Northbound have
+ * a corresponding entries in the Meter and Meter_Band tables in
+ * OVN_Southbound. Additionally, ACL logs that use fair meters have
+ * a private copy of its meter in the SB table.
+ */
+static void
+sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
+ const struct nbrec_meter_table *nbrec_meter_table,
+ const struct nbrec_acl_table *nbrec_acl_table,
+ const struct sbrec_meter_table *sbrec_meter_table,
+ struct shash *meter_groups)
+{
+ struct shash sb_meters = SHASH_INITIALIZER(&sb_meters);
+ struct sset used_sb_meters = SSET_INITIALIZER(&used_sb_meters);
+
+ const struct sbrec_meter *sb_meter;
+ SBREC_METER_TABLE_FOR_EACH (sb_meter, sbrec_meter_table) {
+ shash_add(&sb_meters, sb_meter->name, sb_meter);
+ }
+
+ const struct nbrec_meter *nb_meter;
+ NBREC_METER_TABLE_FOR_EACH (nb_meter, nbrec_meter_table) {
+ sync_meters_iterate_nb_meter(ovnsb_txn, nb_meter->name, nb_meter,
+ &sb_meters, &used_sb_meters);
+ }
+
+ /*
+ * In addition to creating Meters in the SB from the block above, check
+ * and see if additional rows are needed to get ACLs logs individually
+ * rate-limited.
+ */
+ const struct nbrec_acl *acl;
+ NBREC_ACL_TABLE_FOR_EACH (acl, nbrec_acl_table) {
+ sync_acl_fair_meter(ovnsb_txn, meter_groups, acl,
+ &sb_meters, &used_sb_meters);
+ }
+
+ const char *used_meter;
+ SSET_FOR_EACH_SAFE (used_meter, &used_sb_meters) {
+ shash_find_and_delete(&sb_meters, used_meter);
+ sset_delete(&used_sb_meters, SSET_NODE_FROM_NAME(used_meter));
+ }
+ sset_destroy(&used_sb_meters);
+
+ struct shash_node *node;
+ SHASH_FOR_EACH_SAFE (node, &sb_meters) {
+ sbrec_meter_delete(node->data);
+ shash_delete(&sb_meters, node);
+ }
+ shash_destroy(&sb_meters);
+}
diff --git a/northd/en-meters.h b/northd/en-meters.h
new file mode 100644
index 0000000000..a1743282e3
--- /dev/null
+++ b/northd/en-meters.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2023, 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 EN_METERS_H
+#define EN_METERS_H 1
+
+#include "openvswitch/shash.h"
+
+#include "lib/inc-proc-eng.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/ovn-sb-idl.h"
+
+struct sync_meters_data {
+ struct shash meter_groups;
+};
+
+void *en_sync_meters_init(struct engine_node *, struct engine_arg *);
+void en_sync_meters_cleanup(void *data);
+void en_sync_meters_run(struct engine_node *, void *data);
+
+const struct nbrec_meter *fair_meter_lookup_by_name(
+ const struct shash *meter_groups,
+ const char *meter_name);
+
+static inline char*
+alloc_acl_log_unique_meter_name(const struct nbrec_acl *acl)
+{
+ return xasprintf("%s__" UUID_FMT,
+ acl->meter, UUID_ARGS(&acl->header_.uuid));
+}
+
+#endif /* EN_ACL_H */
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 044fa70190..cc05efdbbc 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -76,10 +76,6 @@ northd_get_input_data(struct engine_node *node,
EN_OVSDB_GET(engine_get_input("NB_load_balancer_group", node));
input_data->nbrec_port_group_table =
EN_OVSDB_GET(engine_get_input("NB_port_group", node));
- input_data->nbrec_meter_table =
- EN_OVSDB_GET(engine_get_input("NB_meter", node));
- input_data->nbrec_acl_table =
- EN_OVSDB_GET(engine_get_input("NB_acl", node));
input_data->nbrec_static_mac_binding_table =
EN_OVSDB_GET(engine_get_input("NB_static_mac_binding", node));
input_data->nbrec_chassis_template_var_table =
@@ -107,8 +103,6 @@ northd_get_input_data(struct engine_node *node,
EN_OVSDB_GET(engine_get_input("SB_service_monitor", node));
input_data->sbrec_port_group_table =
EN_OVSDB_GET(engine_get_input("SB_port_group", node));
- input_data->sbrec_meter_table =
- EN_OVSDB_GET(engine_get_input("SB_meter", node));
input_data->sbrec_dns_table =
EN_OVSDB_GET(engine_get_input("SB_dns", node));
input_data->sbrec_ip_multicast_table =
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index d328deb222..1de0551b3d 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -33,6 +33,7 @@
#include "en-northd.h"
#include "en-lflow.h"
#include "en-northd-output.h"
+#include "en-meters.h"
#include "en-sync-sb.h"
#include "en-sync-from-sb.h"
#include "unixctl.h"
@@ -135,6 +136,7 @@ static ENGINE_NODE(lflow, "lflow");
static ENGINE_NODE(mac_binding_aging, "mac_binding_aging");
static ENGINE_NODE(mac_binding_aging_waker, "mac_binding_aging_waker");
static ENGINE_NODE(northd_output, "northd_output");
+static ENGINE_NODE(sync_meters, "sync_meters");
static ENGINE_NODE(sync_to_sb, "sync_to_sb");
static ENGINE_NODE(sync_to_sb_addr_set, "sync_to_sb_addr_set");
static ENGINE_NODE(fdb_aging, "fdb_aging");
@@ -148,10 +150,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_northd, &en_nb_port_group, NULL);
engine_add_input(&en_northd, &en_nb_load_balancer, NULL);
engine_add_input(&en_northd, &en_nb_load_balancer_group, NULL);
- engine_add_input(&en_northd, &en_nb_acl, NULL);
engine_add_input(&en_northd, &en_nb_logical_router, NULL);
engine_add_input(&en_northd, &en_nb_mirror, NULL);
- engine_add_input(&en_northd, &en_nb_meter, NULL);
engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL);
engine_add_input(&en_northd, &en_nb_chassis_template_var, NULL);
@@ -188,7 +188,13 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_fdb_aging, &en_northd, NULL);
engine_add_input(&en_fdb_aging, &en_fdb_aging_waker, NULL);
+ engine_add_input(&en_sync_meters, &en_nb_acl, NULL);
+ engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
+ engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
+
engine_add_input(&en_lflow, &en_nb_bfd, NULL);
+ engine_add_input(&en_lflow, &en_nb_acl, NULL);
+ engine_add_input(&en_lflow, &en_sync_meters, NULL);
engine_add_input(&en_lflow, &en_sb_bfd, NULL);
engine_add_input(&en_lflow, &en_sb_logical_flow, NULL);
engine_add_input(&en_lflow, &en_sb_multicast_group, NULL);
@@ -204,9 +210,11 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
/* en_sync_to_sb engine node syncs the SB database tables from
* the NB database tables.
- * Right now this engine only syncs the SB Address_Set table.
+ * Right now this engine syncs the SB Address_Set table and
+ * SB Meter/Meter_Band tables.
*/
engine_add_input(&en_sync_to_sb, &en_sync_to_sb_addr_set, NULL);
+ engine_add_input(&en_sync_to_sb, &en_sync_meters, NULL);
engine_add_input(&en_sync_from_sb, &en_northd,
sync_from_sb_northd_handler);
diff --git a/northd/northd.c b/northd/northd.c
index 13f2ae0565..48fb44a8be 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -21,6 +21,7 @@
#include "bitmap.h"
#include "coverage.h"
#include "dirs.h"
+#include "en-meters.h"
#include "ipam.h"
#include "openvswitch/dynamic-string.h"
#include "hash.h"
@@ -4980,25 +4981,22 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct
hmap *ls_ports,
}
static bool
-check_ls_changes_other_than_lsp(const struct nbrec_logical_switch *ls)
+check_ls_changes_other_than_lsp_acl(const struct nbrec_logical_switch *ls)
{
/* Check if the columns are changed in this row. */
enum nbrec_logical_switch_column_id col;
for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) {
- if (nbrec_logical_switch_is_updated(ls, col) &&
- col != NBREC_LOGICAL_SWITCH_COL_PORTS) {
+ if (nbrec_logical_switch_is_updated(ls, col)) {
+ if (col == NBREC_LOGICAL_SWITCH_COL_ACLS ||
+ col == NBREC_LOGICAL_SWITCH_COL_PORTS) {
+ continue;
+ }
return true;
}
}
/* Check if the referenced rows are changed.
XXX: Need a better OVSDB IDL interface for this check. */
- for (size_t i = 0; i < ls->n_acls; i++) {
- if (nbrec_acl_row_get_seqno(ls->acls[i],
- OVSDB_IDL_CHANGE_MODIFY) > 0) {
- return true;
- }
- }
if (ls->copp && nbrec_copp_row_get_seqno(ls->copp,
OVSDB_IDL_CHANGE_MODIFY) > 0) {
return true;
@@ -5106,7 +5104,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
}
/* Now only able to handle lsp changes. */
- if (check_ls_changes_other_than_lsp(changed_ls)) {
+ if (check_ls_changes_other_than_lsp_acl(changed_ls)) {
goto fail;
}
@@ -6987,25 +6985,6 @@ build_acl_hints(struct ovn_datapath *od,
}
}
-static const struct nbrec_meter*
-fair_meter_lookup_by_name(const struct shash *meter_groups,
- const char *meter_name)
-{
- const struct nbrec_meter *nb_meter =
- meter_name ? shash_find_data(meter_groups, meter_name) : NULL;
- if (nb_meter) {
- return (nb_meter->fair && *nb_meter->fair) ? nb_meter : NULL;
- }
- return NULL;
-}
-
-static char*
-alloc_acl_log_unique_meter_name(const struct nbrec_acl *acl)
-{
- return xasprintf("%s__" UUID_FMT,
- acl->meter, UUID_ARGS(&acl->header_.uuid));
-}
-
static void
build_acl_log_meter(struct ds *actions, const struct nbrec_acl *acl,
const struct shash *meter_groups)
@@ -16608,183 +16587,6 @@ sync_port_groups(struct ovsdb_idl_txn *ovnsb_txn,
shash_destroy(&sb_port_groups);
}
-struct band_entry {
- int64_t rate;
- int64_t burst_size;
- const char *action;
-};
-
-static int
-band_cmp(const void *band1_, const void *band2_)
-{
- const struct band_entry *band1p = band1_;
- const struct band_entry *band2p = band2_;
-
- if (band1p->rate != band2p->rate) {
- return band1p->rate - band2p->rate;
- } else if (band1p->burst_size != band2p->burst_size) {
- return band1p->burst_size - band2p->burst_size;
- } else {
- return strcmp(band1p->action, band2p->action);
- }
-}
-
-static bool
-bands_need_update(const struct nbrec_meter *nb_meter,
- const struct sbrec_meter *sb_meter)
-{
- if (nb_meter->n_bands != sb_meter->n_bands) {
- return true;
- }
-
- /* Place the Northbound entries in sorted order. */
- struct band_entry *nb_bands;
- nb_bands = xmalloc(sizeof *nb_bands * nb_meter->n_bands);
- for (size_t i = 0; i < nb_meter->n_bands; i++) {
- struct nbrec_meter_band *nb_band = nb_meter->bands[i];
-
- nb_bands[i].rate = nb_band->rate;
- nb_bands[i].burst_size = nb_band->burst_size;
- nb_bands[i].action = nb_band->action;
- }
- qsort(nb_bands, nb_meter->n_bands, sizeof *nb_bands, band_cmp);
-
- /* Place the Southbound entries in sorted order. */
- struct band_entry *sb_bands;
- sb_bands = xmalloc(sizeof *sb_bands * sb_meter->n_bands);
- for (size_t i = 0; i < sb_meter->n_bands; i++) {
- struct sbrec_meter_band *sb_band = sb_meter->bands[i];
-
- sb_bands[i].rate = sb_band->rate;
- sb_bands[i].burst_size = sb_band->burst_size;
- sb_bands[i].action = sb_band->action;
- }
- qsort(sb_bands, sb_meter->n_bands, sizeof *sb_bands, band_cmp);
-
- bool need_update = false;
- for (size_t i = 0; i < nb_meter->n_bands; i++) {
- if (band_cmp(&nb_bands[i], &sb_bands[i])) {
- need_update = true;
- break;
- }
- }
-
- free(nb_bands);
- free(sb_bands);
-
- return need_update;
-}
-
-static void
-sync_meters_iterate_nb_meter(struct ovsdb_idl_txn *ovnsb_txn,
- const char *meter_name,
- const struct nbrec_meter *nb_meter,
- struct shash *sb_meters,
- struct sset *used_sb_meters)
-{
- const struct sbrec_meter *sb_meter;
- bool new_sb_meter = false;
-
- sb_meter = shash_find_data(sb_meters, meter_name);
- if (!sb_meter) {
- sb_meter = sbrec_meter_insert(ovnsb_txn);
- sbrec_meter_set_name(sb_meter, meter_name);
- shash_add(sb_meters, sb_meter->name, sb_meter);
- new_sb_meter = true;
- }
- sset_add(used_sb_meters, meter_name);
-
- if (new_sb_meter || bands_need_update(nb_meter, sb_meter)) {
- struct sbrec_meter_band **sb_bands;
- sb_bands = xcalloc(nb_meter->n_bands, sizeof *sb_bands);
- for (size_t i = 0; i < nb_meter->n_bands; i++) {
- const struct nbrec_meter_band *nb_band = nb_meter->bands[i];
-
- sb_bands[i] = sbrec_meter_band_insert(ovnsb_txn);
-
- sbrec_meter_band_set_action(sb_bands[i], nb_band->action);
- sbrec_meter_band_set_rate(sb_bands[i], nb_band->rate);
- sbrec_meter_band_set_burst_size(sb_bands[i],
- nb_band->burst_size);
- }
- sbrec_meter_set_bands(sb_meter, sb_bands, nb_meter->n_bands);
- free(sb_bands);
- }
-
- sbrec_meter_set_unit(sb_meter, nb_meter->unit);
-}
-
-static void
-sync_acl_fair_meter(struct ovsdb_idl_txn *ovnsb_txn,
- struct shash *meter_groups,
- const struct nbrec_acl *acl, struct shash *sb_meters,
- struct sset *used_sb_meters)
-{
- const struct nbrec_meter *nb_meter =
- fair_meter_lookup_by_name(meter_groups, acl->meter);
-
- if (!nb_meter) {
- return;
- }
-
- char *meter_name = alloc_acl_log_unique_meter_name(acl);
- sync_meters_iterate_nb_meter(ovnsb_txn, meter_name, nb_meter, sb_meters,
- used_sb_meters);
- free(meter_name);
-}
-
-/* Each entry in the Meter and Meter_Band tables in OVN_Northbound have
- * a corresponding entries in the Meter and Meter_Band tables in
- * OVN_Southbound. Additionally, ACL logs that use fair meters have
- * a private copy of its meter in the SB table.
- */
-static void
-sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
- const struct nbrec_meter_table *nbrec_meter_table,
- const struct nbrec_acl_table *nbrec_acl_table,
- const struct sbrec_meter_table *sbrec_meter_table,
- struct shash *meter_groups)
-{
- struct shash sb_meters = SHASH_INITIALIZER(&sb_meters);
- struct sset used_sb_meters = SSET_INITIALIZER(&used_sb_meters);
-
- const struct sbrec_meter *sb_meter;
- SBREC_METER_TABLE_FOR_EACH (sb_meter, sbrec_meter_table) {
- shash_add(&sb_meters, sb_meter->name, sb_meter);
- }
-
- const struct nbrec_meter *nb_meter;
- NBREC_METER_TABLE_FOR_EACH (nb_meter, nbrec_meter_table) {
- sync_meters_iterate_nb_meter(ovnsb_txn, nb_meter->name, nb_meter,
- &sb_meters, &used_sb_meters);
- }
-
- /*
- * In addition to creating Meters in the SB from the block above, check
- * and see if additional rows are needed to get ACLs logs individually
- * rate-limited.
- */
- const struct nbrec_acl *acl;
- NBREC_ACL_TABLE_FOR_EACH (acl, nbrec_acl_table) {
- sync_acl_fair_meter(ovnsb_txn, meter_groups, acl,
- &sb_meters, &used_sb_meters);
- }
-
- const char *used_meter;
- SSET_FOR_EACH_SAFE (used_meter, &used_sb_meters) {
- shash_find_and_delete(&sb_meters, used_meter);
- sset_delete(&used_sb_meters, SSET_NODE_FROM_NAME(used_meter));
- }
- sset_destroy(&used_sb_meters);
-
- struct shash_node *node;
- SHASH_FOR_EACH_SAFE (node, &sb_meters) {
- sbrec_meter_delete(node->data);
- shash_delete(&sb_meters, node);
- }
- shash_destroy(&sb_meters);
-}
-
static bool
mirror_needs_update(const struct nbrec_mirror *nb_mirror,
const struct sbrec_mirror *sb_mirror)
@@ -17254,16 +17056,6 @@ build_mcast_groups(const struct sbrec_igmp_group_table
*sbrec_igmp_group_table,
}
}
-static void
-build_meter_groups(const struct nbrec_meter_table *nbrec_meter_table,
- struct shash *meter_groups)
-{
- const struct nbrec_meter *nb_meter;
- NBREC_METER_TABLE_FOR_EACH (nb_meter, nbrec_meter_table) {
- shash_add(meter_groups, nb_meter->name, nb_meter);
- }
-}
-
static const struct nbrec_static_mac_binding *
static_mac_binding_by_port_ip(
const struct nbrec_static_mac_binding_table *nbrec_static_mb_table,
@@ -17405,7 +17197,6 @@ northd_init(struct northd_data *data)
hmap_init(&data->ls_ports);
hmap_init(&data->lr_ports);
hmap_init(&data->port_groups);
- shash_init(&data->meter_groups);
hmap_init(&data->lbs);
hmap_init(&data->lb_groups);
ovs_list_init(&data->lr_list);
@@ -17443,12 +17234,6 @@ northd_destroy(struct northd_data *data)
hmap_destroy(&data->port_groups);
- struct shash_node *node;
- SHASH_FOR_EACH_SAFE (node, &data->meter_groups) {
- shash_delete(&data->meter_groups, node);
- }
- shash_destroy(&data->meter_groups);
-
/* XXX Having to explicitly clean up macam here
* is a bit strange. We don't explicitly initialize
* macam in this module, but this is the logical place
@@ -17587,7 +17372,6 @@ ovnnb_db_run(struct northd_input *input_data,
build_ip_mcast(ovnsb_txn, input_data->sbrec_ip_multicast_table,
input_data->sbrec_ip_mcast_by_dp,
&data->ls_datapaths.datapaths);
- build_meter_groups(input_data->nbrec_meter_table, &data->meter_groups);
build_static_mac_binding_table(ovnsb_txn,
input_data->nbrec_static_mac_binding_table,
input_data->sbrec_static_mac_binding_table,
@@ -17602,9 +17386,6 @@ ovnnb_db_run(struct northd_input *input_data,
&data->ls_datapaths, &data->lbs);
sync_port_groups(ovnsb_txn, input_data->sbrec_port_group_table,
&data->port_groups);
- sync_meters(ovnsb_txn, input_data->nbrec_meter_table,
- input_data->nbrec_acl_table, input_data->sbrec_meter_table,
- &data->meter_groups);
sync_mirrors(ovnsb_txn, input_data->nbrec_mirror_table,
input_data->sbrec_mirror_table);
sync_dns_entries(ovnsb_txn, input_data->sbrec_dns_table,
diff --git a/northd/northd.h b/northd/northd.h
index f3e63b1e1a..dcc3acabd5 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -31,8 +31,6 @@ struct northd_input {
const struct nbrec_load_balancer_group_table
*nbrec_load_balancer_group_table;
const struct nbrec_port_group_table *nbrec_port_group_table;
- const struct nbrec_meter_table *nbrec_meter_table;
- const struct nbrec_acl_table *nbrec_acl_table;
const struct nbrec_static_mac_binding_table
*nbrec_static_mac_binding_table;
const struct nbrec_chassis_template_var_table
@@ -50,7 +48,6 @@ struct northd_input {
const struct sbrec_load_balancer_table *sbrec_load_balancer_table;
const struct sbrec_service_monitor_table *sbrec_service_monitor_table;
const struct sbrec_port_group_table *sbrec_port_group_table;
- const struct sbrec_meter_table *sbrec_meter_table;
const struct sbrec_dns_table *sbrec_dns_table;
const struct sbrec_ip_multicast_table *sbrec_ip_multicast_table;
const struct sbrec_static_mac_binding_table
@@ -109,7 +106,6 @@ struct northd_data {
struct hmap ls_ports;
struct hmap lr_ports;
struct hmap port_groups;
- struct shash meter_groups;
struct hmap lbs;
struct hmap lb_groups;
struct ovs_list lr_list;
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 4fa1b039ea..45362f4f93 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -836,6 +836,9 @@ main(int argc, char *argv[])
ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
&sbrec_multicast_group_columns[i]);
}
+ for (size_t i = 0; i < SBREC_METER_N_COLUMNS; i++) {
+ ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_meter_columns[i]);
+ }
unixctl_command_register("sb-connection-status", "", 0, 0,
ovn_conn_show, ovnsb_idl_loop.idl);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d5be3be75b..dc8bf253f1 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9,6 +9,8 @@ m4_define([_DUMP_DB_TABLES], [
ovn-sbctl list logical_flow >> $1
ovn-sbctl list port_binding >> $1
ovn-sbctl list address_set >> $1
+ ovn-sbctl list meter >> $1
+ ovn-sbctl list meter_band >> $1
])
# CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -9679,6 +9681,40 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ACL/Meter incremental processing - no northd recompute])
+ovn_start
+
+check_recompute_counter() {
+ northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats
northd recompute)
+ AT_CHECK([test x$northd_recomp = x$1])
+
+ northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats
northd recompute)
+ AT_CHECK([test x$northd_recomp = x$1])
+
+ sync_meters_recomp=$(as northd ovn-appctl -t NORTHD_TYPE
inc-engine/show-stats sync_meters recompute)
+ AT_CHECK([test x$sync_meters_recomp = x$3])
+}
