On 5/26/25 03:27, Ales Musil wrote:
On Mon, May 12, 2025 at 10:59 PM Mark Michelson via dev
<ovs-dev@openvswitch.org <mailto:ovs-dev@openvswitch.org>> wrote:
In current OVN, the en-northd node (via code in northd.c) takes full
control of syncing logical switches and logical routers with southbound
datapath bindings. This is fine, so long as:
1) These are the only datapath types to sync.
2) northd will always be the arbiter of datapath syncing.
However, future commits will introduce new types of datapaths. These are
not good fits for the en-northd node, since they have completely
independent processing rules, and trying to shoehorn them into a struct
ovn_datapath would be wasteful.
This patch introduces a new way of syncing datapaths. Each datapath type
has a node that is responsible for creating an ovn_unsynced_datapath_map
for the type of datapath it creates. Then a type-agnostic datapath
syncing node syncs these datapaths with the southbound Datapath_Binding
type. Then, these synced datapaths are fed back into type-specific
datapath nodes, which translate these synced datapaths into specific
types.
Nodes can then use these as inputs if they need synced datapaths (i.e. a
northbound type with its corresponding southbound type). In this case,
en_northd uses the synced logical switch and logical router types in
order to create its ovn_datapath structures.
Doing this will provide an easy way to sync new datapath types to the
southbound database.
Signed-off-by: Mark Michelson <mmich...@redhat.com
<mailto:mmich...@redhat.com>>
---
Hi Mark,
thank you for working on this. I like the approach in general, but
I have some comments/suggestions down below.
And thank you for taking the time to review this! I plan to take all of
your suggestions. I have some comments in-line below if you're
interested in having a look.
v4 -> v5:
* Rebased.
v3 -> v4:
* Rebased.
v2 -> v3:
* Rebased
* Fixed a typo in datapath_sync.h
v1 -> v2:
* Many formatting fixes (added newlines, removed unnecessary local
variables, removed unnecessary parameter names, etc.).
* Clarified language in TODO.rst regarding the eventual removal of the
ovn_datapath structure.
* Use ovsdb_idl_row's type field to find the northbound type when
converting from generic synced datapath to logical switch or logical
router.
* Fixed several memory leaks.
* Switched to allocating the input_maps dynamically in
en-datapath-sync
since sanitizers complain about stack-allocated variable-length
arrays.
* Changed name of function that deletes remaining candidate synced
datapaths to make it less confusing
(delete_candidates_with_no_tunnel_keys -> delete_candidates).
* Used engine_noop_handler instead of defining new stubs that return
true.
Any review comments not addressed in this version should have been
addressed by my responses to earlier reviews.
---
TODO.rst | 12 +
northd/automake.mk <http://automake.mk> | 8 +
northd/datapath_sync.c | 57 +++++
northd/datapath_sync.h | 77 ++++++
northd/en-datapath-logical-router.c | 175 ++++++++++++++
northd/en-datapath-logical-router.h | 49 ++++
northd/en-datapath-logical-switch.c | 176 ++++++++++++++
northd/en-datapath-logical-switch.h | 48 ++++
northd/en-datapath-sync.c | 319 +++++++++++++++++++++++++
northd/en-datapath-sync.h | 27 +++
northd/en-global-config.c | 11 +
northd/en-northd.c | 6 +
northd/inc-proc-northd.c | 40 ++++
northd/northd.c | 354 +++-------------------------
northd/northd.h | 6 +-
15 files changed, 1045 insertions(+), 320 deletions(-)
create mode 100644 northd/datapath_sync.c
create mode 100644 northd/datapath_sync.h
create mode 100644 northd/en-datapath-logical-router.c
create mode 100644 northd/en-datapath-logical-router.h
create mode 100644 northd/en-datapath-logical-switch.c
create mode 100644 northd/en-datapath-logical-switch.h
create mode 100644 northd/en-datapath-sync.c
create mode 100644 northd/en-datapath-sync.h
diff --git a/TODO.rst b/TODO.rst
index e50b1bd76..78962bb92 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -156,3 +156,15 @@ OVN To-do List
monitoring conditions to update before we actually try to
learn routes.
Otherwise we could try to add duplicated Learned_Routes and
the ovnsb
commit would fail.
+
+* Datapath sync nodes
+
+ * Add incremental processing to the en-datapath-logical-switch,
+ en-datapath-logical-router, en-datapath-sync, and possibly the
+ en-datapath-synced-logical-router and
en-datapath-synced-logical-switch
+ nodes.
+
+ * Migrate data stored in the ovn\_datapath structure to
+ ovn\_synced\_logical_router and ovn\_synced\_logical\_switch.
This will
+ allow for the eventual removal of the ovn\_datapath structure
from the
+ codebase.
diff --git a/northd/automake.mk <http://automake.mk>
b/northd/automake.mk <http://automake.mk>
index 9a7165529..bf9978dd2 100644
--- a/northd/automake.mk <http://automake.mk>
+++ b/northd/automake.mk <http://automake.mk>
@@ -3,11 +3,19 @@ bin_PROGRAMS += northd/ovn-northd
northd_ovn_northd_SOURCES = \
northd/aging.c \
northd/aging.h \
+ northd/datapath_sync.c \
+ northd/datapath_sync.h \
northd/debug.c \
northd/debug.h \
northd/northd.c \
northd/northd.h \
northd/ovn-northd.c \
+ northd/en-datapath-logical-switch.c \
+ northd/en-datapath-logical-switch.h \
+ northd/en-datapath-logical-router.c \
+ northd/en-datapath-logical-router.h \
+ northd/en-datapath-sync.c \
+ northd/en-datapath-sync.h \
northd/en-ecmp-nexthop.c \
northd/en-ecmp-nexthop.h \
northd/en-global-config.c \
diff --git a/northd/datapath_sync.c b/northd/datapath_sync.c
new file mode 100644
index 000000000..fcffb71aa
--- /dev/null
+++ b/northd/datapath_sync.c
@@ -0,0 +1,57 @@
+/* Copyright (c) 2025, 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
<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 "datapath_sync.h"
+
+struct ovn_unsynced_datapath *
+ovn_unsynced_datapath_alloc(const char *name, uint32_t
requested_tunnel_key,
+ const struct ovsdb_idl_row *nb_row)
+{
+ struct ovn_unsynced_datapath *dp = xzalloc(sizeof *dp);
nit: No need for xzalloc.
+ dp->name = xstrdup(name);
+ dp->requested_tunnel_key = requested_tunnel_key;
+ dp->nb_row = nb_row;
+ smap_init(&dp->external_ids);
+
+ return dp;
+}
+
+void
+ovn_unsynced_datapath_destroy(struct ovn_unsynced_datapath *dp)
+{
+ free(dp->name);
+ smap_destroy(&dp->external_ids);
+}
+
+void
+ovn_unsynced_datapath_map_init(struct ovn_unsynced_datapath_map *map,
+ const char *sb_key_name)
+{
+ hmap_init(&map->dps);
+ map->sb_key_name = sb_key_name;
+}
+
+void
+ovn_unsynced_datapath_map_destroy(struct ovn_unsynced_datapath_map
*map)
+{
+ struct ovn_unsynced_datapath *dp;
+ HMAP_FOR_EACH_POP (dp, hmap_node, &map->dps) {
+ ovn_unsynced_datapath_destroy(dp);
+ free(dp);
+ }
+ hmap_destroy(&map->dps);
+}
diff --git a/northd/datapath_sync.h b/northd/datapath_sync.h
new file mode 100644
index 000000000..78512f7d4
--- /dev/null
+++ b/northd/datapath_sync.h
@@ -0,0 +1,77 @@
+/* Copyright (c) 2025, 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
<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 DATAPATH_SYNC_H
+#define DATAPATH_SYNC_H 1
+
+#include "openvswitch/hmap.h"
+#include "openvswitch/list.h"
+#include "smap.h"
+
+/* Datapath syncing API. This file consists of utility functions
+ * that can be used when syncing northbound datapath types (e.g.
+ * Logical_Router and Logical_Switch) to southbound Datapath_Bindings.
+ *
+ * The basic flow of data is as such.
+ * 1. A northbound type is converted into an ovn_unsynced_datapath.
+ * All ovn_unsynced_datapaths are placed into an
ovn_unsynced_datapath_map.
+ * 2. The en_datapath_sync node takes all of the maps in as input and
+ * syncs them with southbound datapath bindings. This includes
allocating
+ * tunnel keys across all datapath types. The output of this node is
+ * ovn_synced_datapaths, which contains a list of all synced datapaths.
+ * 3. A northbound type-aware node then takes the ovn_synced_datapaths,
+ * and decodes the generic synced datapaths back into a type-specific
+ * version (e.g. ovn_synced_logical_router). Later nodes can then
consume
+ * these type-specific synced datapath types in order to perform
+ * further processing.
+ */
+
+/* Represents a datapath from the northbound database
+ * that has not yet been synced with the southbound database.
+ */
+struct ovn_unsynced_datapath {
+ struct hmap_node hmap_node;
+ char *name;
+ uint32_t requested_tunnel_key;
+ struct smap external_ids;
+ const struct ovsdb_idl_row *nb_row;
+};
+
+struct ovn_unsynced_datapath_map {
+ /* ovn_unsynced_datapath */
+ struct hmap dps;
+ const char *sb_key_name;
+};
+
+struct ovn_synced_datapath {
+ struct ovs_list list_node;
+ const struct ovsdb_idl_row *nb_row;
+ const struct sbrec_datapath_binding *sb_dp;
+};
+
+struct ovn_synced_datapaths {
+ struct ovs_list synced_dps;
+};
+
+struct ovn_unsynced_datapath *ovn_unsynced_datapath_alloc(
+ const char *name, uint32_t requested_tunnel_key,
+ const struct ovsdb_idl_row *nb_row);
+void ovn_unsynced_datapath_destroy(struct ovn_unsynced_datapath *dp);
+
+void ovn_unsynced_datapath_map_init(struct
ovn_unsynced_datapath_map *map,
+ const char *sb_key_name);
+void ovn_unsynced_datapath_map_destroy(struct
ovn_unsynced_datapath_map *map);
+
+#endif /* DATAPATH_SYNC_H */
diff --git a/northd/en-datapath-logical-router.c
b/northd/en-datapath-logical-router.c
new file mode 100644
index 000000000..343b2eec9
--- /dev/null
+++ b/northd/en-datapath-logical-router.c
@@ -0,0 +1,175 @@
+/*
+ * Copyright (c) 2025, 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
<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/hmap.h"
+#include "openvswitch/util.h"
+
+#include "ovn-nb-idl.h"
+#include "aging.h"
+#include "datapath_sync.h"
+#include "en-datapath-logical-router.h"
+#include "ovn-util.h"
+
+#define LR_SB_KEY_NAME "logical-router"
+
+void *
+en_datapath_logical_router_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *args OVS_UNUSED)
+{
+ struct ovn_unsynced_datapath_map *map = xzalloc(sizeof *map);
nit: No need for xzalloc.
+ ovn_unsynced_datapath_map_init(map, LR_SB_KEY_NAME);
+ return map;
+}
+
+enum engine_node_state
+en_datapath_logical_router_run(struct engine_node *node , void *data)
+{
+ const struct nbrec_logical_router_table *nb_lr_table =
+ EN_OVSDB_GET(engine_get_input("NB_logical_router", node));
+
+ struct ovn_unsynced_datapath_map *map = data;
+
+ ovn_unsynced_datapath_map_destroy(map);
+ ovn_unsynced_datapath_map_init(map, LR_SB_KEY_NAME);
+
+ const struct nbrec_logical_router *nbr;
+ NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH (nbr, nb_lr_table) {
+ struct ovn_unsynced_datapath *dp;
+ dp = ovn_unsynced_datapath_alloc(nbr->name,
+ smap_get_int(&nbr->options,
+
"requested-tnl-key", 0),
+ &nbr->header_);
+
+ smap_add_format(&dp->external_ids, LR_SB_KEY_NAME, UUID_FMT,
+ UUID_ARGS(&nbr->header_.uuid));
+ smap_add(&dp->external_ids, "name", dp->name);
+ const char *neutron_router = smap_get(&nbr->options,
+ "neutron:router_name");
+ if (neutron_router && neutron_router[0]) {
+ smap_add(&dp->external_ids, "name2", neutron_router);
+ }
+
+ int64_t ct_zone_limit = ovn_smap_get_llong(&nbr->options,
+ "ct-zone-limit",
-1);
+ if (ct_zone_limit > 0) {
+ smap_add_format(&dp->external_ids, "ct-zone-limit",
"%"PRId64,
+ ct_zone_limit);
+ }
+
+ int nat_default_ct = smap_get_int(&nbr->options,
+ "snat-ct-zone", -1);
+ if (nat_default_ct >= 0) {
+ smap_add_format(&dp->external_ids, "snat-ct-zone", "%d",
+ nat_default_ct);
+ }
+
+ bool learn_from_arp_request =
+ smap_get_bool(&nbr->options,
"always_learn_from_arp_request",
+ true);
+ if (!learn_from_arp_request) {
+ smap_add(&dp->external_ids,
"always_learn_from_arp_request",
+ "false");
+ }
+
+ /* For timestamp refreshing, the smallest threshold of the
option is
+ * set to SB to make sure all entries are refreshed in time.
+ * This approach simplifies processing in ovn-controller,
but it
+ * may be enhanced, if necessary, to parse the complete
CIDR-based
+ * threshold configurations to SB to reduce unnecessary
refreshes. */
+ uint32_t age_threshold = min_mac_binding_age_threshold(
+ smap_get(&nbr->options,
+
"mac_binding_age_threshold"));
+ if (age_threshold) {
+ smap_add_format(&dp->external_ids,
"mac_binding_age_threshold",
+ "%u", age_threshold);
+ }
+
+ hmap_insert(&map->dps, &dp->hmap_node,
uuid_hash(&nbr->header_.uuid));
+ }
+
+ return EN_UPDATED;
+}
+
+
+void
+en_datapath_logical_router_cleanup(void *data)
+{
+ struct ovn_unsynced_datapath_map *map = data;
+ ovn_unsynced_datapath_map_destroy(map);
+}
+
+static void
+synced_logical_router_map_init(
+ struct ovn_synced_logical_router_map *router_map)
+{
+ hmap_init(&router_map->synced_routers);
+}
+
+static void
+synced_logical_router_map_destroy(
+ struct ovn_synced_logical_router_map *router_map)
+{
+ struct ovn_synced_logical_router *lr;
+ HMAP_FOR_EACH_POP (lr, hmap_node, &router_map->synced_routers) {
+ free(lr);
+ }
+ hmap_destroy(&router_map->synced_routers);
+}
+
+void *
+en_datapath_synced_logical_router_init(struct engine_node *node
OVS_UNUSED,
+ struct engine_arg *args
OVS_UNUSED)
+{
+ struct ovn_synced_logical_router_map *router_map;
+ router_map = xzalloc(sizeof *router_map);
nit: No need for xzalloc.
+ synced_logical_router_map_init(router_map);
+
+ return router_map;
+}
+
+enum engine_node_state
+en_datapath_synced_logical_router_run(struct engine_node *node ,
void *data)
+{
+ const struct ovn_synced_datapaths *dps =
+ engine_get_input_data("datapath_sync", node);
+ struct ovn_synced_logical_router_map *router_map = data;
+
+ synced_logical_router_map_destroy(router_map);
+ synced_logical_router_map_init(router_map);
+
+ struct ovn_synced_datapath *sdp;
+ LIST_FOR_EACH (sdp, list_node, &dps->synced_dps) {
+ if (sdp->nb_row->table->class_ !=
&nbrec_table_logical_router) {
+ continue;
+ }
+ struct ovn_synced_logical_router *lr = xzalloc(sizeof *lr);
nit: No need for xzalloc.
+ lr->nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_router,
+ header_);
+ lr->sb = sdp->sb_dp;
+ hmap_insert(&router_map->synced_routers, &lr->hmap_node,
+ uuid_hash(&lr->nb->header_.uuid));
+ }
+
+ return EN_UPDATED;
+}
+
+void en_datapath_synced_logical_router_cleanup(void *data)
+{
+ struct ovn_synced_logical_router_map *router_map = data;
+ synced_logical_router_map_destroy(router_map);
+}
diff --git a/northd/en-datapath-logical-router.h
b/northd/en-datapath-logical-router.h
new file mode 100644
index 000000000..587de8393
--- /dev/null
+++ b/northd/en-datapath-logical-router.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2025, 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
<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_DATAPATH_LOGICAL_ROUTER_H
+#define EN_DATAPATH_LOGICAL_ROUTER_H
+
+#include "lib/inc-proc-eng.h"
+#include "openvswitch/hmap.h"
+
+
+void *en_datapath_logical_router_init(struct engine_node *,
+ struct engine_arg *);
+
+enum engine_node_state en_datapath_logical_router_run(struct
engine_node *,
+ void *data);
+void en_datapath_logical_router_cleanup(void *data);
+
+struct ovn_synced_logical_router {
+ struct hmap_node hmap_node;
+ const struct nbrec_logical_router *nb;
+ const struct sbrec_datapath_binding *sb;
+};
+
+struct ovn_synced_logical_router_map {
+ struct hmap synced_routers;
+};
+
+void *en_datapath_synced_logical_router_init(struct engine_node *,
+ struct engine_arg *);
+
+enum engine_node_state en_datapath_synced_logical_router_run(
+ struct engine_node *, void *data);
+
+void en_datapath_synced_logical_router_cleanup(void *data);
+
+#endif /* EN_DATAPATH_LOGICAL_ROUTER_H */
diff --git a/northd/en-datapath-logical-switch.c
b/northd/en-datapath-logical-switch.c
new file mode 100644
index 000000000..6395b1552
--- /dev/null
+++ b/northd/en-datapath-logical-switch.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright (c) 2025, 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
<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/hmap.h"
+#include "openvswitch/util.h"
+#include "openvswitch/vlog.h"
+
+#include "inc-proc-eng.h"
+#include "ovn-nb-idl.h"
+#include "datapath_sync.h"
+#include "en-datapath-logical-switch.h"
+#include "en-global-config.h"
+#include "ovn-util.h"
+
+#define LS_SB_KEY_NAME "logical-switch"
+
+VLOG_DEFINE_THIS_MODULE(en_datapath_logical_switch);
+
+void *
+en_datapath_logical_switch_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *args OVS_UNUSED)
+{
+ struct ovn_unsynced_datapath_map *map = xzalloc(sizeof *map);
nit: No need for xzalloc.
+ ovn_unsynced_datapath_map_init(map, LS_SB_KEY_NAME);
+ return map;
+}
+
+enum engine_node_state
+en_datapath_logical_switch_run(struct engine_node *node , void *data)
+{
+ const struct nbrec_logical_switch_table *nb_ls_table =
+ EN_OVSDB_GET(engine_get_input("NB_logical_switch", node));
+ const struct ed_type_global_config *global_config =
+ engine_get_input_data("global_config", node);
+
+ struct ovn_unsynced_datapath_map *map = data;
+
+ ovn_unsynced_datapath_map_destroy(map);
+ ovn_unsynced_datapath_map_init(map, LS_SB_KEY_NAME);
+
+ const struct nbrec_logical_switch *nbs;
+ NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbs, nb_ls_table) {
+ uint32_t requested_tunnel_key =
smap_get_int(&nbs->other_config,
+
"requested-tnl-key", 0);
+ const char *ts = smap_get(&nbs->other_config, "interconn-ts");
+
+ if (!ts && global_config->vxlan_mode &&
+ requested_tunnel_key >= 1 << 12) {
+ static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for datapath %s
is "
+ "incompatible with VXLAN",
requested_tunnel_key,
+ nbs->name);
+ requested_tunnel_key = 0;
+ }
+
+ struct ovn_unsynced_datapath *dp;
+ dp = ovn_unsynced_datapath_alloc(nbs->name,
requested_tunnel_key,
+ &nbs->header_);
+
+ smap_init(&dp->external_ids);
+ smap_add_format(&dp->external_ids, LS_SB_KEY_NAME, UUID_FMT,
+ UUID_ARGS(&nbs->header_.uuid));
+ smap_add(&dp->external_ids, "name", dp->name);
+ const char *neutron_network = smap_get(&nbs->other_config,
+ "neutron:network_name");
+ if (neutron_network && neutron_network[0]) {
+ smap_add(&dp->external_ids, "name2", neutron_network);
+ }
+
+ int64_t ct_zone_limit = ovn_smap_get_llong(&nbs->other_config,
+ "ct-zone-limit",
-1);
+ if (ct_zone_limit > 0) {
+ smap_add_format(&dp->external_ids, "ct-zone-limit",
"%"PRId64,
+ ct_zone_limit);
+ }
+
+ if (ts) {
+ smap_add(&dp->external_ids, "interconn-ts", ts);
+ }
+
+ uint32_t age_threshold = smap_get_uint(&nbs->other_config,
+ "fdb_age_threshold", 0);
+ if (age_threshold) {
+ smap_add_format(&dp->external_ids, "fdb_age_threshold",
+ "%u", age_threshold);
+ }
+
+ hmap_insert(&map->dps, &dp->hmap_node,
uuid_hash(&nbs->header_.uuid));
+ }
+
+ return EN_UPDATED;
+}
+
+
+void
+en_datapath_logical_switch_cleanup(void *data)
+{
+ struct ovn_unsynced_datapath_map *map = data;
+ ovn_unsynced_datapath_map_destroy(map);
+}
+
+static void
+synced_logical_switch_map_init(
+ struct ovn_synced_logical_switch_map *switch_map)
+{
+ hmap_init(&switch_map->synced_switches);
+}
+
+static void
+synced_logical_switch_map_destroy(
+ struct ovn_synced_logical_switch_map *switch_map)
+{
+ struct ovn_synced_logical_switch *ls;
+ HMAP_FOR_EACH_POP (ls, hmap_node, &switch_map->synced_switches) {
+ free(ls);
+ }
+ hmap_destroy(&switch_map->synced_switches);
+}
+
+void *
+en_datapath_synced_logical_switch_init(struct engine_node *node
OVS_UNUSED,
+ struct engine_arg *args
OVS_UNUSED)
+{
+ struct ovn_synced_logical_switch_map *switch_map;
+ switch_map = xzalloc(sizeof *switch_map);
nit: No need for xzalloc.
+ synced_logical_switch_map_init(switch_map);
+
+ return switch_map;
+}
+
+enum engine_node_state
+en_datapath_synced_logical_switch_run(struct engine_node *node ,
void *data)
+{
+ const struct ovn_synced_datapaths *dps =
+ engine_get_input_data("datapath_sync", node);
+ struct ovn_synced_logical_switch_map *switch_map = data;
+
+ synced_logical_switch_map_destroy(switch_map);
+ synced_logical_switch_map_init(switch_map);
+
+ struct ovn_synced_datapath *sdp;
+ LIST_FOR_EACH (sdp, list_node, &dps->synced_dps) {
+ if (sdp->nb_row->table->class_ !=
&nbrec_table_logical_switch) {
+ continue;
+ }
+ struct ovn_synced_logical_switch *lsw = xzalloc(sizeof *lsw);
+ lsw->nb = CONTAINER_OF(sdp->nb_row, struct
nbrec_logical_switch,
+ header_);
+ lsw->sb = sdp->sb_dp;
+ hmap_insert(&switch_map->synced_switches, &lsw->hmap_node,
+ uuid_hash(&lsw->nb->header_.uuid));
+ }
+
+ return EN_UPDATED;
+}
+
+void en_datapath_synced_logical_switch_cleanup(void *data)
+{
+ struct ovn_synced_logical_switch_map *switch_map = data;
+ synced_logical_switch_map_destroy(switch_map);
+}
diff --git a/northd/en-datapath-logical-switch.h
b/northd/en-datapath-logical-switch.h
new file mode 100644
index 000000000..1190b7be8
--- /dev/null
+++ b/northd/en-datapath-logical-switch.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2025, 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
<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_DATAPATH_LOGICAL_SWITCH_H
+#define EN_DATAPATH_LOGICAL_SWITCH_H
+
+#include "lib/inc-proc-eng.h"
+#include "openvswitch/hmap.h"
+
+
+void *en_datapath_logical_switch_init(struct engine_node *,
+ struct engine_arg *);
+
+enum engine_node_state en_datapath_logical_switch_run(struct
engine_node *,
+ void *data);
+void en_datapath_logical_switch_cleanup(void *data);
+
+struct ovn_synced_logical_switch {
+ struct hmap_node hmap_node;
+ const struct nbrec_logical_switch *nb;
+ const struct sbrec_datapath_binding *sb;
+};
+
+struct ovn_synced_logical_switch_map {
+ struct hmap synced_switches;
+};
+
+void *en_datapath_synced_logical_switch_init(struct engine_node *,
+ struct engine_arg *);
+
+enum engine_node_state en_datapath_synced_logical_switch_run(
+ struct engine_node *, void *data);
+void en_datapath_synced_logical_switch_cleanup(void *data);
+
+#endif /* EN_DATAPATH_LOGICAL_SWITCH_H */
diff --git a/northd/en-datapath-sync.c b/northd/en-datapath-sync.c
new file mode 100644
index 000000000..15e4ddc06
--- /dev/null
+++ b/northd/en-datapath-sync.c
@@ -0,0 +1,319 @@
+/*
+ * Copyright (c) 2025, 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
<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 "uuidset.h"
+
+#include "en-datapath-sync.h"
+#include "en-global-config.h"
+#include "datapath_sync.h"
+#include "ovn-sb-idl.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(datapath_sync);
+
+void *
+en_datapath_sync_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *args OVS_UNUSED)
+{
+ struct ovn_synced_datapaths *synced_datapaths
+ = xzalloc(sizeof *synced_datapaths);
nit: No need for xzalloc.
+ ovs_list_init(&synced_datapaths->synced_dps);
+
+ return synced_datapaths;
+}
+
+static struct ovn_unsynced_datapath *
+find_unsynced_datapath(const struct ovn_unsynced_datapath_map **maps,
+ size_t n_maps,
+ const struct sbrec_datapath_binding *sb_dp,
+ const char **map_key)
+{
+ struct uuid key;
+ const struct ovn_unsynced_datapath_map *map;
+ bool found_map = false;
+ *map_key = NULL;
+ for (size_t i = 0; i < n_maps; i++) {
+ map = maps[i];
+ if (smap_get_uuid(&sb_dp->external_ids, map->sb_key_name,
&key)) {
+ found_map = true;
+ break;
+ }
+ }
+
+ if (!found_map) {
+ return NULL;
+ }
+ ovs_assert(map);
+ *map_key = map->sb_key_name;
+
+ uint32_t hash = uuid_hash(&key);
+ struct ovn_unsynced_datapath *dp;
+ HMAP_FOR_EACH_WITH_HASH (dp, hmap_node, hash, &map->dps) {
+ if (uuid_equals(&key, &dp->nb_row->uuid)) {
+ return dp;
+ }
+ }
+
+ return NULL;
+}
The lookup is a bit unfortunate because we need to walk through
the map array for each SB datapath binding. I have a suggestion below
which would hopefully help with that.
+
+struct candidate_sdp {
+ struct ovs_list list_node;
+ struct ovn_synced_datapath *sdp;
+ uint32_t requested_tunnel_key;
+ uint32_t existing_tunnel_key;
+};
+
+static struct candidate_sdp *
+candidate_sdp_alloc(const struct ovn_unsynced_datapath *udp,
+ const struct sbrec_datapath_binding *sb_dp)
+{
+ struct ovn_synced_datapath *sdp;
+ sdp = xzalloc(sizeof *sdp);
nit: No need for xzalloc.
+ sdp->sb_dp = sb_dp;
+ sdp->nb_row = udp->nb_row;
+ sbrec_datapath_binding_set_external_ids(sb_dp, &udp->external_ids);
+
+ struct candidate_sdp *candidate;
+ candidate = xzalloc(sizeof *candidate);
nit: No need for xzalloc.
+ candidate->sdp = sdp;
+ candidate->requested_tunnel_key = udp->requested_tunnel_key;
+ candidate->existing_tunnel_key = sdp->sb_dp->tunnel_key;
+
+ return candidate;
+}
+
+static void
+reset_synced_datapaths(struct ovn_synced_datapaths *synced_datapaths)
+{
+ struct ovn_synced_datapath *sdp;
+ LIST_FOR_EACH_POP (sdp, list_node, &synced_datapaths->synced_dps) {
+ free(sdp);
+ }
+ ovs_list_init(&synced_datapaths->synced_dps);
+}
+
+static void
+create_synced_datapath_candidates_from_sb(
+ const struct sbrec_datapath_binding_table *sb_dp_table,
+ struct uuidset *visited,
+ const struct ovn_unsynced_datapath_map **input_maps,
+ size_t n_input_maps,
+ struct ovs_list *candidate_sdps)
+{
+ const struct sbrec_datapath_binding *sb_dp;
+ SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_SAFE (sb_dp, sb_dp_table) {
+ const char *map_key;
+ struct ovn_unsynced_datapath *udp;
+ udp = find_unsynced_datapath(input_maps, n_input_maps, sb_dp,
+ &map_key);
+ if (!udp) {
+ sbrec_datapath_binding_delete(sb_dp);
+ continue;
+ }
+
+ if (uuidset_find(visited, &udp->nb_row->uuid)) {
+ static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_INFO_RL(
+ &rl, "deleting Datapath_Binding "UUID_FMT" with "
+ "duplicate external-ids:%s "UUID_FMT,
+ UUID_ARGS(&sb_dp->header_.uuid), map_key,
+ UUID_ARGS(&udp->nb_row->uuid));
+ sbrec_datapath_binding_delete(sb_dp);
+ continue;
+ }
+
+ struct candidate_sdp *candidate;
+ candidate = candidate_sdp_alloc(udp, sb_dp);
+ ovs_list_push_back(candidate_sdps, &candidate->list_node);
+ uuidset_insert(visited, &udp->nb_row->uuid);
+ }
+}
+
+static void
+create_synced_datapath_candidates_from_nb(
+ const struct ovn_unsynced_datapath_map **input_maps,
+ size_t n_input_maps,
+ struct ovsdb_idl_txn *ovnsb_idl_txn,
+ struct uuidset *visited,
+ struct ovs_list *candidate_sdps)
+{
+ for (size_t i = 0; i < n_input_maps; i++) {
+ const struct ovn_unsynced_datapath_map *map = input_maps[i];
+ struct ovn_unsynced_datapath *udp;
+ HMAP_FOR_EACH (udp, hmap_node, &map->dps) {
+ if (uuidset_find(visited, &udp->nb_row->uuid)) {
+ continue;
+ }
+ struct sbrec_datapath_binding *sb_dp;
+ sb_dp = sbrec_datapath_binding_insert(ovnsb_idl_txn);
+ struct candidate_sdp *candidate;
+ candidate = candidate_sdp_alloc(udp, sb_dp);
+ ovs_list_push_back(candidate_sdps, &candidate->list_node);
+ }
+ }
+}
+
+static void
+assign_requested_tunnel_keys(struct ovs_list *candidate_sdps,
+ struct hmap *dp_tnlids,
+ struct ovn_synced_datapaths
*synced_datapaths)
+{
+ struct candidate_sdp *candidate;
+ LIST_FOR_EACH_SAFE (candidate, list_node, candidate_sdps) {
+ if (!candidate->requested_tunnel_key) {
+ continue;
+ }
+ if (ovn_add_tnlid(dp_tnlids,
candidate->requested_tunnel_key)) {
+ static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "Logical datapath "UUID_FMT" requests
same "
+ "tunnel key %"PRIu32" as another logical
datapath",
+ UUID_ARGS(&candidate->sdp->nb_row->uuid),
+ candidate->requested_tunnel_key);
+ }
+ sbrec_datapath_binding_set_tunnel_key(candidate->sdp->sb_dp,
+
candidate->requested_tunnel_key);
+ ovs_list_remove(&candidate->list_node);
+ ovs_list_push_back(&synced_datapaths->synced_dps,
+ &candidate->sdp->list_node);
+ free(candidate);
+ }
+}
+
+static void
+assign_existing_tunnel_keys(struct ovs_list *candidate_sdps,
+ struct hmap *dp_tnlids,
+ struct ovn_synced_datapaths
*synced_datapaths)
+{
+ struct candidate_sdp *candidate;
+ LIST_FOR_EACH_SAFE (candidate, list_node, candidate_sdps) {
+ if (!candidate->existing_tunnel_key) {
+ continue;
+ }
+ /* Existing southbound DP. If this key is available,
+ * reuse it.
+ */
+ if (ovn_add_tnlid(dp_tnlids, candidate->existing_tunnel_key)) {
+ ovs_list_remove(&candidate->list_node);
+ ovs_list_push_back(&synced_datapaths->synced_dps,
+ &candidate->sdp->list_node);
+ free(candidate);
+ }
+ }
+}
+
+static void
+allocate_tunnel_keys(struct ovs_list *candidate_sdps,
+ struct hmap *dp_tnlids,
+ uint32_t max_dp_tunnel_id,
+ struct ovn_synced_datapaths *synced_datapaths)
+{
+ uint32_t hint = 0;
+ struct candidate_sdp *candidate;
+ LIST_FOR_EACH_SAFE (candidate, list_node, candidate_sdps) {
+ uint32_t tunnel_key =
+ ovn_allocate_tnlid(dp_tnlids, "datapath",
OVN_MIN_DP_KEY_LOCAL,
+ max_dp_tunnel_id, &hint);
+ if (!tunnel_key) {
+ continue;
+ }
+ sbrec_datapath_binding_set_tunnel_key(candidate->sdp->sb_dp,
+ tunnel_key);
+ ovs_list_remove(&candidate->list_node);
+ ovs_list_push_back(&synced_datapaths->synced_dps,
+ &candidate->sdp->list_node);
+ free(candidate);
+ }
+}
+
+static void
+delete_candidates(struct ovs_list *candidate_sdps)
+{
+ struct candidate_sdp *candidate;
+ LIST_FOR_EACH_POP (candidate, list_node, candidate_sdps) {
+ sbrec_datapath_binding_delete(candidate->sdp->sb_dp);
+ free(candidate->sdp);
+ free(candidate);
+ }
+}
+
+enum engine_node_state
+en_datapath_sync_run(struct engine_node *node , void *data)
+{
+ const struct sbrec_datapath_binding_table *sb_dp_table =
+ EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node));
+ const struct ed_type_global_config *global_config =
+ engine_get_input_data("global_config", node);
+ /* The inputs are:
+ * * Some number of input maps.
+ * * Southbound Datapath Binding table.
+ * * Global config data.
+ *
+ * Therefore, the number of inputs - 2 is the number of input
+ * maps from the datapath-specific nodes.
+ */
+ size_t n_input_maps = node->n_inputs - 2;
+ const struct ovn_unsynced_datapath_map **input_maps =
+ xmalloc(n_input_maps * sizeof *input_maps);
+ struct ovn_synced_datapaths *synced_datapaths = data;
This is really fragile, any change of order or "unexpected" extra
input will break this assumption and it might not be obvious why the
tests are failing.
The following suggestion should also address the point about
"find_unsynced_datapath". I think we should add type to
Datapath_Binding, this would allow us to distinguish the maps right
away. We could just use enum with a static array and make the lookup
faster. The downside is the we need to handle upgrades, but we could
just recreate SB entries that don't have any type with appropriate
one which would be just a one-time thing.
It's funny that you mention this. In my lflow sync refactor, this is one
of the first changes I made, since it's incredibly inconvenient to have
different keys for different datapath types. I can fold that change into
this patch instead.
I also had wondered about upgrades, but it's pretty easy to work around
that issue. We just have to understand that the new external-ids:type
may not be present. If it's not, then delete the current SB
datapath_binding and replace it with one that has the expected
external-ids:type key. Any engine nodes that are "downstream" from the
datapath_sync node can then be reasonably sure that the type is present
in the datapath_binding since they are working exclusively with synced
datapaths.
IMO with the plan to introduce more datapath types it makes sense to
be able to tell right away which NB counterpart it corresponds to.
I know we have the external_ids identification, but that requires lookup
extra lookup for each type potentially.
+
+ for (size_t i = 0; i < n_input_maps; i++) {
+ input_maps[i] = engine_get_data(node->inputs[i].node);
+ }
+
+ reset_synced_datapaths(synced_datapaths);
+
+ struct uuidset visited = UUIDSET_INITIALIZER(&visited);
+ struct ovs_list candidate_sdps =
OVS_LIST_INITIALIZER(&candidate_sdps);
It might be actually better to use vector here, the tunnel functions
will need
a small adjustment if that will be the case.
The nice thing about using the candidate list is that as the candidates
become synced, I can remove them from the list. Then when I call a new
function, I have a reduced list of candidates from before. When all is
done, the remaining list items can be freed.
Vectors can't safely have elements removed mid-traversal, so I guess I
would need to add a boolean to indicate whether the candidate has been
synced or not. Then in each successive loop, I'd need to filter
candidates based on the value of this boolean. It's not a bad idea, and
it could potentially be good for memory cohesion to allocate a vector
up-front. I'll see what I can do.
On another note, another change I made in the lflow sync refactor series
is to implement ovn_synced_datapaths as a vector instead of a list. Keep
in mind that the vector change had not been merged when I first posted
this series :)
+ create_synced_datapath_candidates_from_sb(sb_dp_table, &visited,
+ input_maps, n_input_maps,
+ &candidate_sdps);
+
+ const struct engine_context *eng_ctx = engine_get_context();
+ create_synced_datapath_candidates_from_nb(input_maps, n_input_maps,
+
eng_ctx->ovnsb_idl_txn, &visited,
+ &candidate_sdps);
+ uuidset_destroy(&visited);
+
+ struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
+ assign_requested_tunnel_keys(&candidate_sdps, &dp_tnlids,
+ synced_datapaths);
+ assign_existing_tunnel_keys(&candidate_sdps, &dp_tnlids,
synced_datapaths);
+ allocate_tunnel_keys(&candidate_sdps, &dp_tnlids,
+ global_config->max_dp_tunnel_id,
synced_datapaths);
+
+ /* Any remaining candidates could not be synced due to tunnel key
+ * issues and need to be deleted.
+ */
+ delete_candidates(&candidate_sdps);
+
+ ovn_destroy_tnlids(&dp_tnlids);
+ free(input_maps);
+
+ return EN_UPDATED;
+}
+
+void en_datapath_sync_cleanup(void *data)
+{
+ struct ovn_synced_datapaths *synced_datapaths = data;
+ struct ovn_synced_datapath *sdp;
+
+ LIST_FOR_EACH_POP (sdp, list_node, &synced_datapaths->synced_dps) {
+ free(sdp);
+ }
+}
diff --git a/northd/en-datapath-sync.h b/northd/en-datapath-sync.h
new file mode 100644
index 000000000..3b3262304
--- /dev/null
+++ b/northd/en-datapath-sync.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2025, 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
<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_DATAPATH_SYNC_H
+#define EN_DATAPATH_SYNC_H
+
+#include "inc-proc-eng.h"
+
+void *en_datapath_sync_init(struct engine_node *,
+ struct engine_arg *);
+enum engine_node_state en_datapath_sync_run(struct engine_node *,
void *data);
+void en_datapath_sync_cleanup(void *data);
+
+#endif /* EN_DATAPATH_SYNC_H */
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index 11513e31e..7204462ee 100644
--- a/northd/en-global-config.c
+++ b/northd/en-global-config.c
@@ -63,6 +63,17 @@ en_global_config_init(struct engine_node *node
OVS_UNUSED,
return data;
}
+static uint32_t
+get_ovn_max_dp_key_local(bool vxlan_mode, bool vxlan_ic_mode)
+{
+ if (vxlan_mode) {
+ /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for VXLAN mode. */
+ return vxlan_ic_mode ? OVN_MAX_DP_VXLAN_KEY_LOCAL
+ : OVN_MAX_DP_VXLAN_KEY;
+ }
+ return vxlan_ic_mode ? OVN_MAX_DP_VXLAN_KEY_LOCAL :
OVN_MAX_DP_KEY_LOCAL;
+}
+
enum engine_node_state
en_global_config_run(struct engine_node *node , void *data)
{
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 02a27aac2..c4573f88f 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -117,6 +117,12 @@ northd_get_input_data(struct engine_node *node,
input_data->svc_monitor_mac_ea =
global_config->svc_monitor_mac_ea;
input_data->features = &global_config->features;
input_data->vxlan_mode = global_config->vxlan_mode;
+
+ input_data->synced_lses =
+ engine_get_input_data("datapath_synced_logical_switch", node);
+
+ input_data->synced_lrs =
+ engine_get_input_data("datapath_synced_logical_router", node);
}
enum engine_node_state
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index b1e4994a4..bdc6c48df 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -47,6 +47,9 @@
#include "en-advertised-route-sync.h"
#include "en-learned-route-sync.h"
#include "en-group-ecmp-route.h"
+#include "en-datapath-logical-router.h"
+#include "en-datapath-logical-switch.h"
+#include "en-datapath-sync.h"
#include "unixctl.h"
#include "util.h"
@@ -179,6 +182,11 @@ static ENGINE_NODE(advertised_route_sync);
static ENGINE_NODE(learned_route_sync, CLEAR_TRACKED_DATA);
static ENGINE_NODE(dynamic_routes);
static ENGINE_NODE(group_ecmp_route, CLEAR_TRACKED_DATA);
+static ENGINE_NODE(datapath_logical_router);
+static ENGINE_NODE(datapath_logical_switch);
+static ENGINE_NODE(datapath_synced_logical_router);
+static ENGINE_NODE(datapath_synced_logical_switch);
+static ENGINE_NODE(datapath_sync);
void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
struct ovsdb_idl_loop *sb)
@@ -209,6 +217,21 @@ void inc_proc_northd_init(struct ovsdb_idl_loop
*nb,
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_datapath_logical_switch,
&en_nb_logical_switch, NULL);
+ engine_add_input(&en_datapath_logical_switch,
&en_global_config, NULL);
+
+ engine_add_input(&en_datapath_logical_router,
&en_nb_logical_router, NULL);
+
+ engine_add_input(&en_datapath_sync,
&en_datapath_logical_switch, NULL);
+ engine_add_input(&en_datapath_sync,
&en_datapath_logical_router, NULL);
+ engine_add_input(&en_datapath_sync, &en_sb_datapath_binding, NULL);
+ engine_add_input(&en_datapath_sync, &en_global_config, NULL);
+
+ engine_add_input(&en_datapath_synced_logical_router,
&en_datapath_sync,
+ NULL);
+ engine_add_input(&en_datapath_synced_logical_switch,
&en_datapath_sync,
+ NULL);
+
engine_add_input(&en_northd, &en_nb_mirror, NULL);
engine_add_input(&en_northd, &en_nb_mirror_rule, NULL);
engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL);
@@ -247,6 +270,23 @@ void inc_proc_northd_init(struct ovsdb_idl_loop
*nb,
northd_nb_logical_router_handler);
engine_add_input(&en_northd, &en_lb_data, northd_lb_data_handler);
+ /* Currently, northd handles logical router and switch changes
in nodes
+ * that read directly from the northbound logical tables. Those
nodes
+ * will trigger a recompute if conditions on changed logical
routers
+ * or logical switches cannot be handled. From en-northd's
perspective,
+ * synced logical switch and router changes are always handled.
+ *
+ * Once datapath syncing has incremental processing added, then
+ * en-northd can move its logical router and switch change
handling to
+ * handlers defined here, and there will be no need for
en_northd to
+ * read directly from the northbound database for incremental
handling
+ * of these types.
+ */
+ engine_add_input(&en_northd, &en_datapath_synced_logical_router,
+ engine_noop_handler);
+ engine_add_input(&en_northd, &en_datapath_synced_logical_switch,
+ engine_noop_handler);
+
engine_add_input(&en_lr_nat, &en_northd, lr_nat_northd_handler);
engine_add_input(&en_lr_stateful, &en_northd,
lr_stateful_northd_handler);
diff --git a/northd/northd.c b/northd/northd.c
index b0e957b30..47b54dbd9 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -52,6 +52,8 @@
#include "en-ls-stateful.h"
#include "en-multicast.h"
#include "en-sampling-app.h"
+#include "en-datapath-logical-switch.h"
+#include "en-datapath-logical-router.h"
#include "lib/ovn-parallel-hmap.h"
#include "ovn/actions.h"
#include "ovn/features.h"
@@ -96,8 +98,6 @@ static bool default_acl_drop;
* and ports tunnel key allocation (12 bits for each instead of
default 16). */
static bool vxlan_mode;
-static bool vxlan_ic_mode;
-
#define MAX_OVN_TAGS 4096
@@ -482,6 +482,8 @@ struct lrouter_group {
struct hmapx tmp_ha_ref_chassis;
};
+static void init_mcast_info_for_datapath(struct ovn_datapath *od);
+
static struct ovn_datapath *
ovn_datapath_create(struct hmap *datapaths, const struct uuid *key,
const struct nbrec_logical_switch *nbs,
@@ -503,6 +505,8 @@ ovn_datapath_create(struct hmap *datapaths,
const struct uuid *key,
od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
od->l3dgw_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
+ od->tunnel_key = sb->tunnel_key;
+ init_mcast_info_for_datapath(od);
return od;
}
@@ -745,87 +749,6 @@ store_mcast_info_for_switch_datapath(const
struct sbrec_ip_multicast *sb,
}
}
-static void
-ovn_datapath_update_external_ids(struct ovn_datapath *od)
-{
- /* Get the logical-switch or logical-router UUID to set in
- * external-ids. */
- char uuid_s[UUID_LEN + 1];
- sprintf(uuid_s, UUID_FMT, UUID_ARGS(&od->key));
- const char *key = od->nbs ? "logical-switch" : "logical-router";
-
- /* Get names to set in external-ids. */
- const char *name = od->nbs ? od->nbs->name : od->nbr->name;
- const char *name2 = (od->nbs
- ? smap_get(&od->nbs->external_ids,
- "neutron:network_name")
- : smap_get(&od->nbr->external_ids,
- "neutron:router_name"));
-
- /* Set external-ids. */
- struct smap ids = SMAP_INITIALIZER(&ids);
- smap_add(&ids, key, uuid_s);
- smap_add(&ids, "name", name);
- if (name2 && name2[0]) {
- 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");
- if (ts) {
- smap_add(&ids, "interconn-ts", ts);
- }
-
- uint32_t age_threshold = smap_get_uint(&od->nbs->other_config,
- "fdb_age_threshold", 0);
- if (age_threshold) {
- smap_add_format(&ids, "fdb_age_threshold",
- "%u", age_threshold);
- }
- }
-
- /* Set snat-ct-zone */
- if (od->nbr) {
- int nat_default_ct = smap_get_int(&od->nbr->options,
- "snat-ct-zone", -1);
- if (nat_default_ct >= 0) {
- smap_add_format(&ids, "snat-ct-zone", "%d",
nat_default_ct);
- }
-
- bool learn_from_arp_request =
- smap_get_bool(&od->nbr->options,
"always_learn_from_arp_request",
- true);
- if (!learn_from_arp_request) {
- smap_add(&ids, "always_learn_from_arp_request", "false");
- }
-
- /* For timestamp refreshing, the smallest threshold of the
option is
- * set to SB to make sure all entries are refreshed in time.
- * XXX: This approach simplifies processing in
ovn-controller, but it
- * may be enhanced, if necessary, to parse the complete
CIDR-based
- * threshold configurations to SB to reduce unnecessary
refreshes. */
- uint32_t age_threshold = min_mac_binding_age_threshold(
- smap_get(&od->nbr->options,
-
"mac_binding_age_threshold"));
- if (age_threshold) {
- smap_add_format(&ids, "mac_binding_age_threshold",
- "%u", age_threshold);
- }
- }
-
- sbrec_datapath_binding_set_external_ids(od->sb, &ids);
- smap_destroy(&ids);
-}
-
static enum dynamic_routing_redistribute_mode
parse_dynamic_routing_redistribute(
const struct smap *options,
@@ -877,170 +800,6 @@ parse_dynamic_routing_redistribute(
return out;
}
-static void
-join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
- const struct nbrec_logical_router_table *nbrec_lr_table,
- const struct sbrec_datapath_binding_table
*sbrec_dp_table,
- struct ovsdb_idl_txn *ovnsb_txn,
- struct hmap *datapaths, struct ovs_list *sb_only,
- struct ovs_list *nb_only, struct ovs_list *both)
-{
- ovs_list_init(sb_only);
- ovs_list_init(nb_only);
- ovs_list_init(both);
-
- const struct sbrec_datapath_binding *sb;
- SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_SAFE (sb, sbrec_dp_table) {
- struct uuid key;
- if (!smap_get_uuid(&sb->external_ids, "logical-switch",
&key) &&
- !smap_get_uuid(&sb->external_ids, "logical-router",
&key)) {
- ovsdb_idl_txn_add_comment(
- ovnsb_txn,
- "deleting Datapath_Binding "UUID_FMT" that lacks "
- "external-ids:logical-switch and "
- "external-ids:logical-router",
- UUID_ARGS(&sb->header_.uuid));
- sbrec_datapath_binding_delete(sb);
- continue;
- }
-
- if (ovn_datapath_find_(datapaths, &key)) {
- static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(5, 1);
- VLOG_INFO_RL(
- &rl, "deleting Datapath_Binding "UUID_FMT" with "
- "duplicate external-ids:logical-switch/router
"UUID_FMT,
- UUID_ARGS(&sb->header_.uuid), UUID_ARGS(&key));
- sbrec_datapath_binding_delete(sb);
- continue;
- }
-
- struct ovn_datapath *od = ovn_datapath_create(datapaths, &key,
- NULL, NULL, sb);
- ovs_list_push_back(sb_only, &od->list);
- }
-
- vxlan_ic_mode = false;
- const struct nbrec_logical_switch *nbs;
- NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbs, nbrec_ls_table) {
- struct ovn_datapath *od = ovn_datapath_find_(datapaths,
-
&nbs->header_.uuid);
- if (od) {
- od->nbs = nbs;
- ovs_list_remove(&od->list);
- ovs_list_push_back(both, &od->list);
- ovn_datapath_update_external_ids(od);
- } else {
- od = ovn_datapath_create(datapaths, &nbs->header_.uuid,
- nbs, NULL, NULL);
- ovs_list_push_back(nb_only, &od->list);
- }
-
- init_ipam_info_for_datapath(od);
- init_mcast_info_for_datapath(od);
-
- if (smap_get_bool(&nbs->other_config, "ic-vxlan_mode",
false)) {
- vxlan_ic_mode = true;
- }
- }
-
- const struct nbrec_logical_router *nbr;
- NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH (nbr, nbrec_lr_table) {
- if (!lrouter_is_enabled(nbr)) {
- continue;
- }
-
- struct ovn_datapath *od = ovn_datapath_find_(datapaths,
-
&nbr->header_.uuid);
- if (od) {
- if (!od->nbs) {
- od->nbr = nbr;
- ovs_list_remove(&od->list);
- ovs_list_push_back(both, &od->list);
- ovn_datapath_update_external_ids(od);
- } else {
- /* Can't happen! */
- static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(5, 1);
- VLOG_WARN_RL(&rl,
- "duplicate UUID "UUID_FMT" in
OVN_Northbound",
- UUID_ARGS(&nbr->header_.uuid));
- continue;
- }
- } else {
- od = ovn_datapath_create(datapaths, &nbr->header_.uuid,
- NULL, nbr, NULL);
- ovs_list_push_back(nb_only, &od->list);
- }
- init_mcast_info_for_datapath(od);
- if (smap_get(&od->nbr->options, "chassis")) {
- od->is_gw_router = true;
- }
- od->dynamic_routing = smap_get_bool(&od->nbr->options,
- "dynamic-routing", false);
- od->dynamic_routing_redistribute =
- parse_dynamic_routing_redistribute(&od->nbr->options,
DRRM_NONE,
- od->nbr->name);
- }
-}
-
-
-uint32_t
-get_ovn_max_dp_key_local(bool _vxlan_mode, bool _vxlan_ic_mode)
-{
- if (_vxlan_mode) {
- /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for VXLAN mode. */
- return _vxlan_ic_mode ? OVN_MAX_DP_VXLAN_KEY_LOCAL
- : OVN_MAX_DP_VXLAN_KEY;
- }
- return _vxlan_ic_mode ? OVN_MAX_DP_VXLAN_KEY_LOCAL :
OVN_MAX_DP_KEY_LOCAL;
-}
-
-static void
-ovn_datapath_allocate_key(struct hmap *datapaths, struct hmap
*dp_tnlids,
- struct ovn_datapath *od, uint32_t *hint)
-{
- if (!od->tunnel_key) {
- od->tunnel_key = ovn_allocate_tnlid(dp_tnlids, "datapath",
- OVN_MIN_DP_KEY_LOCAL,
- get_ovn_max_dp_key_local(vxlan_mode, vxlan_ic_mode), hint);
- if (!od->tunnel_key) {
- if (od->sb) {
- sbrec_datapath_binding_delete(od->sb);
- }
- ovs_list_remove(&od->list);
- ovn_datapath_destroy(datapaths, od);
- }
- }
-}
-
-static void
-ovn_datapath_assign_requested_tnl_id(
- struct hmap *dp_tnlids, struct ovn_datapath *od)
-{
- const struct smap *other_config = (od->nbs
- ? &od->nbs->other_config
- : &od->nbr->options);
- uint32_t tunnel_key = smap_get_int(other_config,
"requested-tnl-key", 0);
- if (tunnel_key) {
- const char *interconn_ts = smap_get(other_config,
"interconn-ts");
- if (!interconn_ts && vxlan_mode && tunnel_key >= 1 << 12) {
- static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for datapath %s
is "
- "incompatible with VXLAN", tunnel_key,
- od->nbs ? od->nbs->name : od->nbr->name);
- return;
- }
- if (ovn_add_tnlid(dp_tnlids, tunnel_key)) {
- od->tunnel_key = tunnel_key;
- } else {
- static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_WARN_RL(&rl, "Logical %s %s requests same tunnel key "
- "%"PRIu32" as another logical switch or
router",
- od->nbs ? "switch" : "router",
- od->nbs ? od->nbs->name : od->nbr->name,
tunnel_key);
- }
- }
-}
-
static void
ods_build_array_index(struct ovn_datapaths *datapaths)
{
@@ -1060,82 +819,43 @@ ods_build_array_index(struct ovn_datapaths
*datapaths)
}
}
-/* Updates the southbound Datapath_Binding table so that it
contains the
- * logical switches and routers specified by the northbound database.
- *
- * Initializes 'datapaths' to contain a "struct ovn_datapath" for
every logical
- * switch and router. */
+/* Initializes 'ls_datapaths' to contain a "struct ovn_datapath"
for every
+ * logical switch, and initializes 'lr_datapaths' to contain a
+ * "struct ovn_datapath" for every logical router.
+ */
static void
-build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
- const struct nbrec_logical_switch_table
*nbrec_ls_table,
- const struct nbrec_logical_router_table
*nbrec_lr_table,
- const struct sbrec_datapath_binding_table
*sbrec_dp_table,
+build_datapaths(const struct ovn_synced_logical_switch_map *ls_map,
+ const struct ovn_synced_logical_router_map *lr_map,
struct ovn_datapaths *ls_datapaths,
struct ovn_datapaths *lr_datapaths)
{
- struct ovs_list sb_only, nb_only, both;
-
- struct hmap *datapaths = &ls_datapaths->datapaths;
- join_datapaths(nbrec_ls_table, nbrec_lr_table, sbrec_dp_table,
ovnsb_txn,
- datapaths, &sb_only, &nb_only, &both);
-
- /* Assign explicitly requested tunnel ids first. */
- struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
- struct ovn_datapath *od;
- LIST_FOR_EACH (od, list, &both) {
- ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od);
- }
- LIST_FOR_EACH (od, list, &nb_only) {
- ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od);
- }
-
- /* Keep nonconflicting tunnel IDs that are already assigned. */
- LIST_FOR_EACH (od, list, &both) {
- if (!od->tunnel_key && ovn_add_tnlid(&dp_tnlids,
od->sb->tunnel_key)) {
- od->tunnel_key = od->sb->tunnel_key;
- }
- }
-
- /* Assign new tunnel ids where needed. */
- uint32_t hint = 0;
- LIST_FOR_EACH_SAFE (od, list, &both) {
- ovn_datapath_allocate_key(datapaths, &dp_tnlids, od, &hint);
- }
- LIST_FOR_EACH_SAFE (od, list, &nb_only) {
- ovn_datapath_allocate_key(datapaths, &dp_tnlids, od, &hint);
+ struct ovn_synced_logical_switch *ls;
+ HMAP_FOR_EACH (ls, hmap_node, &ls_map->synced_switches) {
+ struct ovn_datapath *od =
+ ovn_datapath_create(&ls_datapaths->datapaths,
+ &ls->nb->header_.uuid,
+ ls->nb, NULL, ls->sb);
+ init_ipam_info_for_datapath(od);
}
- /* Sync tunnel ids from nb to sb. */
- LIST_FOR_EACH (od, list, &both) {
- if (od->sb->tunnel_key != od->tunnel_key) {
- sbrec_datapath_binding_set_tunnel_key(od->sb,
od->tunnel_key);
+ struct ovn_synced_logical_router *lr;
+ HMAP_FOR_EACH (lr, hmap_node, &lr_map->synced_routers) {
+ if (!lrouter_is_enabled(lr->nb)) {
+ continue;
}
- ovn_datapath_update_external_ids(od);
- }
- LIST_FOR_EACH (od, list, &nb_only) {
- od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
- ovn_datapath_update_external_ids(od);
- sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
- }
- ovn_destroy_tnlids(&dp_tnlids);
- /* Delete southbound records without northbound matches. */
- LIST_FOR_EACH_SAFE (od, list, &sb_only) {
- ovs_list_remove(&od->list);
- sbrec_datapath_binding_delete(od->sb);
- ovn_datapath_destroy(datapaths, od);
- }
-
- /* Move lr datapaths to lr_datapaths, and ls datapaths will
- * remain in datapaths/ls_datapaths. */
- HMAP_FOR_EACH_SAFE (od, key_node, datapaths) {
- if (!od->nbr) {
- ovs_assert(od->nbs);
- continue;
+ struct ovn_datapath *od =
+ ovn_datapath_create(&lr_datapaths->datapaths,
+ &lr->nb->header_.uuid,
+ NULL, lr->nb, lr->sb);
+ if (smap_get(&od->nbr->options, "chassis")) {
+ od->is_gw_router = true;
}
- hmap_remove(datapaths, &od->key_node);
- hmap_insert(&lr_datapaths->datapaths, &od->key_node,
- od->key_node.hash);
+ od->dynamic_routing = smap_get_bool(&od->nbr->options,
+ "dynamic-routing", false);
+ od->dynamic_routing_redistribute =
+ parse_dynamic_routing_redistribute(&od->nbr->options,
DRRM_NONE,
+ od->nbr->name);
}
ods_build_array_index(ls_datapaths);
@@ -19304,10 +19024,8 @@ ovnnb_db_run(struct northd_input *input_data,
vxlan_mode = input_data->vxlan_mode;
- build_datapaths(ovnsb_txn,
- input_data->nbrec_logical_switch_table,
- input_data->nbrec_logical_router_table,
- input_data->sbrec_datapath_binding_table,
+ build_datapaths(input_data->synced_lses,
+ input_data->synced_lrs,
&data->ls_datapaths,
&data->lr_datapaths);
build_lb_datapaths(input_data->lbs, input_data->lbgrps,
diff --git a/northd/northd.h b/northd/northd.h
index 69143cd7a..e5a9cc775 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -70,6 +70,10 @@ struct northd_input {
/* ACL ID inputs. */
const struct acl_id_data *acl_id_data;
+ /* Synced datapath inputs. */
+ const struct ovn_synced_logical_switch_map *synced_lses;
+ const struct ovn_synced_logical_router_map *synced_lrs;
+
/* Indexes */
struct ovsdb_idl_index *sbrec_chassis_by_name;
struct ovsdb_idl_index *sbrec_chassis_by_hostname;
@@ -966,8 +970,6 @@ lr_has_multiple_gw_ports(const struct
ovn_datapath *od)
return vector_len(&od->l3dgw_ports) > 1 && !od->is_gw_router;
}
-uint32_t get_ovn_max_dp_key_local(bool _vxlan_mode, bool ic_mode);
-
/* Returns true if the logical router port 'enabled' column is
empty or
* set to true. Otherwise, returns false. */
static inline bool
--
2.47.0
_______________________________________________
dev mailing list
d...@openvswitch.org <mailto:d...@openvswitch.org>
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
