This introduces a new southbound table called ACL_ID for storing
persistent ACL conntrack IDs. These IDs are generated by ovn-northd for
any ACL that is of type "allow-established".
Signed-off-by: Mark Michelson <mmich...@redhat.com>
---
northd/automake.mk | 2 +
northd/en-acl-ids.c | 206 +++++++++++++++++++++++++++++++++++++++
northd/en-acl-ids.h | 17 ++++
northd/en-northd.c | 6 ++
northd/inc-proc-northd.c | 15 ++-
northd/northd.c | 3 +
northd/northd.h | 4 +
northd/ovn-northd.c | 4 +
ovn-sb.ovsschema | 14 ++-
ovn-sb.xml | 13 +++
tests/ovn.at | 39 ++++++++
11 files changed, 319 insertions(+), 4 deletions(-)
create mode 100644 northd/en-acl-ids.c
create mode 100644 northd/en-acl-ids.h
diff --git a/northd/automake.mk b/northd/automake.mk
index 6566ad299..d46dfd763 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -34,6 +34,8 @@ northd_ovn_northd_SOURCES = \
northd/en-ls-stateful.h \
northd/en-sampling-app.c \
northd/en-sampling-app.h \
+ northd/en-acl-ids.c \
+ northd/en-acl-ids.h \
northd/inc-proc-northd.c \
northd/inc-proc-northd.h \
northd/ipam.c \
diff --git a/northd/en-acl-ids.c b/northd/en-acl-ids.c
new file mode 100644
index 000000000..897cc2da1
--- /dev/null
+++ b/northd/en-acl-ids.c
@@ -0,0 +1,206 @@
+/*
+ * 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 "en-acl-ids.h"
+#include "lib/uuidset.h"
+#include "lib/ovn-sb-idl.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/bitmap.h"
+
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(northd_acl_ids);
+
+enum id_state {
+ /* The ID represents a new northbound ACL that has
+ * not yet been synced to the southbound DB
+ */
+ ID_NEW,
+ /* The ID represents an ACL ID that has been synced
+ * with the southbound DB already
+ */
+ ID_SYNCED,
+ /* The ID represents a deleted NB ACL that also needs
+ * to be removed from the southbound DB
+ */
+ ID_INACTIVE,
+};
+
+struct acl_id {
+ struct hmap_node node;
+ int64_t id;
+ struct uuid nb_acl_uuid;
+ enum id_state state;
+};
+
+#define MAX_ACL_ID 65535
+
+struct acl_id_data {
+ struct hmap ids;
+ unsigned long *id_bitmap;
+};
+
+static void
+acl_id_data_init(struct acl_id_data *id_data)
+{
+ hmap_init(&id_data->ids);
+ id_data->id_bitmap = bitmap_allocate(MAX_ACL_ID);
+}
+
+static struct acl_id_data *
+acl_id_data_alloc(void)
+{
+ struct acl_id_data *id_data = xzalloc(sizeof *id_data);
+ acl_id_data_init(id_data);
+
+ return id_data;
+}
+
+static void
+acl_id_data_destroy(struct acl_id_data *id_data)
+{
+ struct acl_id *acl_id;
+ HMAP_FOR_EACH_POP (acl_id, node, &id_data->ids) {
+ free(acl_id);
+ }
+ hmap_destroy(&id_data->ids);
+ bitmap_free(id_data->id_bitmap);
+}
+
+void *
+en_acl_id_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct acl_id_data *id_data = acl_id_data_alloc();
+ return id_data;
+}
+
+static void
+add_acl_id(struct hmap *id_map, int64_t id, enum id_state state,
+ const struct uuid *acl_uuid)
+{
+ struct acl_id *acl_id = xzalloc(sizeof *acl_id);
+ acl_id->id = id;
+ acl_id->state = state;
+ acl_id->nb_acl_uuid = *acl_uuid;
+ hmap_insert(id_map, &acl_id->node, uuid_hash(acl_uuid));
+}
+
+void
+en_acl_id_run(struct engine_node *node, void *data)
+{
+ const struct engine_context *eng_ctx = engine_get_context();
+ if (!eng_ctx->ovnnb_idl_txn || !eng_ctx->ovnsb_idl_txn) {
+ return;
+ }
+
+ const struct nbrec_acl_table *nb_acl_table =
+ EN_OVSDB_GET(engine_get_input("NB_acl", node));
+ const struct sbrec_acl_id_table *sb_acl_id_table =
+ EN_OVSDB_GET(engine_get_input("SB_acl_id", node));
+ struct uuidset visited = UUIDSET_INITIALIZER(&visited);
+ struct acl_id_data *id_data = data;
+
+ acl_id_data_destroy(id_data);
+ acl_id_data_init(id_data);
+
+ const struct nbrec_acl *nb_acl;
+ const struct sbrec_acl_id *sb_id;
+ SBREC_ACL_ID_TABLE_FOR_EACH (sb_id, sb_acl_id_table) {
+ nb_acl = nbrec_acl_table_get_for_uuid(nb_acl_table, &sb_id->nb_acl);
+ if (nb_acl && !strcmp(nb_acl->action, "allow-established")) {
+ bitmap_set1(id_data->id_bitmap, sb_id->id);
+ uuidset_insert(&visited, &sb_id->nb_acl);
+ add_acl_id(&id_data->ids, sb_id->id, ID_SYNCED, &sb_id->nb_acl);
+ } else {
+ /* NB ACL is deleted or has changed action type. This
+ * ID is no longer active.
+ */
+ add_acl_id(&id_data->ids, sb_id->id, ID_INACTIVE, &sb_id->nb_acl);
+ }
+ }
+
+ size_t scan_start = 1;
+ size_t scan_end = MAX_ACL_ID;
+ NBREC_ACL_TABLE_FOR_EACH (nb_acl, nb_acl_table) {
+ if (uuidset_find_and_delete(&visited, &nb_acl->header_.uuid)) {
+ continue;
+ }
+ if (strcmp(nb_acl->action, "allow-established")) {
+ continue;
+ }
+ int64_t new_id = bitmap_scan(id_data->id_bitmap, 0,
+ scan_start, scan_end + 1);
+ if (new_id == scan_end + 1) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "Exhausted all ACL IDs");
+ break;
+ }
+ add_acl_id(&id_data->ids, new_id, ID_NEW, &nb_acl->header_.uuid);
+ bitmap_set1(id_data->id_bitmap, new_id);
+ scan_start = new_id + 1;
+ }
+
+ engine_set_node_state(node, EN_UPDATED);
+ uuidset_destroy(&visited);
+}
+
+void
+en_acl_id_cleanup(void *data)
+{
+ acl_id_data_destroy(data);
+}
+
+static const struct sbrec_acl_id *
+acl_id_lookup_by_id(struct ovsdb_idl_index *sbrec_acl_id_by_id,
+ int64_t id)
+{
+ struct sbrec_acl_id *target = sbrec_acl_id_index_init_row(
+ sbrec_acl_id_by_id);
+ sbrec_acl_id_index_set_id(target, id);
+
+ struct sbrec_acl_id *retval = sbrec_acl_id_index_find(
+ sbrec_acl_id_by_id, target);
+
+ sbrec_acl_id_index_destroy_row(target);
+
+ return retval;
+}
+
+void sync_acl_ids(const struct acl_id_data *id_data,
+ struct ovsdb_idl_txn *ovnsb_txn,
+ struct ovsdb_idl_index *sbrec_acl_id_by_id)
+{
+ struct acl_id *acl_id;
+ const struct sbrec_acl_id *sb_id;
+ HMAP_FOR_EACH (acl_id, node, &id_data->ids) {
+ switch (acl_id->state) {
+ case ID_NEW:
+ sb_id = sbrec_acl_id_insert(ovnsb_txn);
+ sbrec_acl_id_set_id(sb_id, acl_id->id);
+ sbrec_acl_id_set_nb_acl(sb_id, acl_id->nb_acl_uuid);
+ break;
+ case ID_INACTIVE:
+ sb_id = acl_id_lookup_by_id(sbrec_acl_id_by_id, acl_id->id);
+ if (sb_id) {
+ sbrec_acl_id_delete(sb_id);
+ }
+ break;
+ case ID_SYNCED:
+ break;
+ }
+ }
+}
diff --git a/northd/en-acl-ids.h b/northd/en-acl-ids.h
new file mode 100644
index 000000000..8b60b3c7c
--- /dev/null
+++ b/northd/en-acl-ids.h
@@ -0,0 +1,17 @@
+#ifndef EN_ACL_IDS_H
+#define EN_ACL_IDS_H
+
+#include <config.h>
+#include <stdbool.h>
+
+#include "lib/inc-proc-eng.h"
+
+bool northd_acl_id_handler(struct engine_node *node, void *data);
+void *en_acl_id_init(struct engine_node *, struct engine_arg *);
+void en_acl_id_run(struct engine_node *, void *data);
+void en_acl_id_cleanup(void *data);
+
+struct acl_id_data;
+void sync_acl_ids(const struct acl_id_data *, struct ovsdb_idl_txn *,
+ struct ovsdb_idl_index *sbrec_acl_id_by_id);
+#endif
diff --git a/northd/en-northd.c b/northd/en-northd.c
index c7d1ebcb3..5545fe7a6 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -61,6 +61,10 @@ northd_get_input_data(struct engine_node *node,
engine_ovsdb_node_get_index(
engine_get_input("SB_fdb", node),
"sbrec_fdb_by_dp_and_port");
+ input_data->sbrec_acl_id_by_id =
+ engine_ovsdb_node_get_index(
+ engine_get_input("SB_acl_id", node),
+ "sbrec_acl_id_by_id");
input_data->nbrec_logical_switch_table =
EN_OVSDB_GET(engine_get_input("NB_logical_switch", node));
@@ -110,6 +114,8 @@ northd_get_input_data(struct engine_node *node,
input_data->svc_monitor_mac = global_config->svc_monitor_mac;
input_data->svc_monitor_mac_ea = global_config->svc_monitor_mac_ea;
input_data->features = &global_config->features;
+
+ input_data->acl_id_data = engine_get_input_data("acl_id", node);
}
void
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 6e0aa04c4..43abf042d 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -41,6 +41,7 @@
#include "en-sampling-app.h"
#include "en-sync-sb.h"
#include "en-sync-from-sb.h"
+#include "en-acl-ids.h"
#include "unixctl.h"
#include "util.h"
@@ -102,7 +103,8 @@ static unixctl_cb_func chassis_features_list;
SB_NODE(fdb, "fdb") \
SB_NODE(static_mac_binding, "static_mac_binding") \
SB_NODE(chassis_template_var, "chassis_template_var") \
- SB_NODE(logical_dp_group, "logical_dp_group")
+ SB_NODE(logical_dp_group, "logical_dp_group") \
+ SB_NODE(acl_id, "acl_id")
enum sb_engine_node {
#define SB_NODE(NAME, NAME_STR) SB_##NAME,
@@ -161,6 +163,7 @@ static ENGINE_NODE(route_policies, "route_policies");
static ENGINE_NODE(routes, "routes");
static ENGINE_NODE(bfd, "bfd");
static ENGINE_NODE(bfd_sync, "bfd_sync");
+static ENGINE_NODE(acl_id, "acl_id");
void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
struct ovsdb_idl_loop *sb)
@@ -186,6 +189,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
global_config_sb_chassis_handler);
engine_add_input(&en_global_config, &en_sampling_app, NULL);
+ engine_add_input(&en_acl_id, &en_nb_acl, NULL);
+ engine_add_input(&en_acl_id, &en_sb_acl_id, NULL);
+
engine_add_input(&en_northd, &en_nb_mirror, NULL);
engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL);
engine_add_input(&en_northd, &en_nb_chassis_template_var, NULL);
@@ -201,8 +207,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL);
engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL);
engine_add_input(&en_northd, &en_sb_fdb, northd_sb_fdb_change_handler);
+ engine_add_input(&en_northd, &en_sb_acl_id, NULL);
engine_add_input(&en_northd, &en_global_config,
northd_global_config_handler);
+ engine_add_input(&en_northd, &en_acl_id, NULL);
/* northd engine node uses the sb mac binding table to
* cleanup mac binding entries for deleted logical ports
@@ -364,6 +372,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
= mac_binding_by_datapath_index_create(sb->idl);
struct ovsdb_idl_index *fdb_by_dp_key =
ovsdb_idl_index_create1(sb->idl, &sbrec_fdb_col_dp_key);
+ struct ovsdb_idl_index *sbrec_acl_id_by_id =
+ ovsdb_idl_index_create1(sb->idl, &sbrec_acl_id_col_id);
engine_init(&en_northd_output, &engine_arg);
@@ -388,6 +398,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_ovsdb_node_add_index(&en_sb_fdb,
"fdb_by_dp_key",
fdb_by_dp_key);
+ engine_ovsdb_node_add_index(&en_sb_acl_id,
+ "sbrec_acl_id_by_id",
+ sbrec_acl_id_by_id);
struct ovsdb_idl_index *sbrec_address_set_by_name
= ovsdb_idl_index_create1(sb->idl, &sbrec_address_set_col_name);
diff --git a/northd/northd.c b/northd/northd.c
index 0aae627a2..8dab88f62 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -50,6 +50,7 @@
#include "en-lr-stateful.h"
#include "en-ls-stateful.h"
#include "en-sampling-app.h"
+#include "en-acl-ids.h"
#include "lib/ovn-parallel-hmap.h"
#include "ovn/actions.h"
#include "ovn/features.h"
@@ -19120,6 +19121,8 @@ ovnnb_db_run(struct northd_input *input_data,
&data->ls_datapaths.datapaths);
sync_template_vars(ovnsb_txn, input_data->nbrec_chassis_template_var_table,
input_data->sbrec_chassis_template_var_table);
+ sync_acl_ids(input_data->acl_id_data, ovnsb_txn,
+ input_data->sbrec_acl_id_by_id);
cleanup_stale_fdb_entries(input_data->sbrec_fdb_table,
&data->ls_datapaths.datapaths);
diff --git a/northd/northd.h b/northd/northd.h
index d60c944df..bccb1c5d8 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -63,12 +63,16 @@ struct northd_input {
struct eth_addr svc_monitor_mac_ea;
const struct chassis_features *features;
+ /* ACL ID inputs. */
+ const struct acl_id_data *acl_id_data;
+
/* Indexes */
struct ovsdb_idl_index *sbrec_chassis_by_name;
struct ovsdb_idl_index *sbrec_chassis_by_hostname;
struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name;
struct ovsdb_idl_index *sbrec_ip_mcast_by_dp;
struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port;
+ struct ovsdb_idl_index *sbrec_acl_id_by_id;
};
/* A collection of datapaths. E.g. all logical switch datapaths, or all
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index fb29c3c21..bbb321248 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -896,6 +896,10 @@ main(int argc, char *argv[])
ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
&sbrec_logical_dp_group_columns[i]);
}
+ for (size_t i = 0; i < SBREC_ACL_ID_N_COLUMNS; i++) {
+ ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
+ &sbrec_acl_id_columns[i]);
+ }
unixctl_command_register("sb-connection-status", "", 0, 0,
ovn_conn_show, ovnsb_idl_loop.idl);
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index 73abf2c8d..837aee620 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
- "version": "20.37.0",
- "cksum": "1950136776 31493",
+ "version": "20.38.0",
+ "cksum": "4077385391 31851",
"tables": {
"SB_Global": {
"columns": {
@@ -617,6 +617,14 @@
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}}},
"indexes": [["chassis"]],
- "isRoot": true}
+ "isRoot": true},
+ "ACL_ID": {
+ "columns": {
+ "id": {"type": {"key": {"type": "integer",
+ "minInteger": 0,
+ "maxInteger": 32767}}},
+ "nb_acl" : {"type": {"key": {"type": "uuid"}}}},
+ "indexes": [["id"]],
+ "isRoot": true}
}
}
diff --git a/ovn-sb.xml b/ovn-sb.xml
index ea4adc1c3..a3d71e100 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -5217,4 +5217,17 @@ tcp.flags = RST;
The set of variable values for a given chassis.
</column>
</table>
+
+ <table name="ACL_ID">
+ <p>
+ Each record represents an identifier that <code>ovn-northd</code> needs
+ to synchronize with instances of <code>ovn-controller</code>.
+ </p>
+ <column name="id">
+ The actual identifier being synchronized.
+ </column>
+ <column name="nb_acl">
+ The Northbound ACL UUID for which this ID corresponds.
+ </column>
+ </table>
</database>
diff --git a/tests/ovn.at b/tests/ovn.at
index 2fdf1a88c..53e6712cc 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -39761,3 +39761,42 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+AT_SETUP([ACL Conntrack ID propagation])
+ovn_start
+
+dnl In this test, we want to ensure that southbound ACL_ID
+dnl entries are created for northbound ACLs of type "allow-established".
+dnl
+dnl If an ACL is of a different type, or if an ACL is deleted,
+dnl then there should be no soutbhound ACL_ID.
+
+check ovn-nbctl ls-add sw
+check ovn-nbctl --wait=sb acl-add sw from-lport 1000 1 allow-related
+acl_uuid=$(fetch_column nb:ACL _uuid priority=1000)
+
+dnl The ACL is not allow-established, so SBDB should have no rows.
+wait_row_count ACL_ID 0
+
+dnl When we change to allow-established, the SBDB should pick it up.
+check ovn-nbctl --wait=sb set ACL $acl_uuid action=allow-established
+
+wait_row_count ACL_ID 1
+
+dnl Setting to a new action should remove the row from the SBDB.
+check ovn-nbctl --wait=sb set ACL $acl_uuid action=drop
+
+wait_row_count ACL_ID 0
+
+dnl Set back to allow-established and make sure it shows up in the SBDB.
+check ovn-nbctl --wait=sb set ACL $acl_uuid action=allow-established
+
+wait_row_count ACL_ID 1
+
+dnl Delete the ACL and ensure the SBDB entry is deleted.
+check ovn-nbctl --wait=sb acl-del sw from-lport 1000 1
+
+wait_row_count ACL_ID 0
+
+AT_CLEANUP
+])
--
2.45.2
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
