On 6/27/25 3:58 PM, Mark Michelson via dev wrote:
> This is similar to a recent change that refactored datapath syncing.
> This works in a very similar way.
>
> * Input nodes create type-agnostic ovn_unpaired_port_bindings. These are
> fed into the en-port-binding-pair node.
> * The en-port-binding-pair node ensures that a southbound Port_Binding
> is created for each unpaired port binding. Any remaining soutbound
> Port_Bindings are deleted.
> * Type-specific nodes then convert the paired port bindings into
> type-specific paired port bindings that can be consumed by other
> nodes.
>
> However, there are some important differences to note between this and
> the datapath sync patch:
>
> * This patch opts for the word "pair" instead of "sync". This is because
> en-port-binding-pair ensures that all southbound Port_Bindings are
> paired with some northbound structure. However, the columns in the
> southobund Port_Binding are not all synced with their northd
> counterpart. Other engine nodes are responsible for fully syncing the
> data.
> * Not all southbound Port_Bindings have a corresponding northbound row.
> Therefore, the engine nodes that create unpaired port bindings pass an
> opaque cookie pointer to the pairing node instead of a database row.
> * Port bindings tend to be identified by name. This means the code has
> to have several safeguards in place to catch scenarios such as a port
> "moving" from one datapath to another, or a port being deleted and
> re-added quickly.
> * Unlike with the datapath syncing code, this required changes to
> northd's incremental processing since northd was already incrementally
> processing some northbound logical switch port changes.
>
> Signed-off-by: Mark Michelson <[email protected]>
> ---
Hi Mark,
As with patch 3/4, I really like what this patch does! It must have
been very hard to untangle all dependencies, thanks for the cleanup!
I have a few minor comments inline.
> v8 -> v9:
> * Rebased.
> * Changed some file names to be in alignment with others in the
> directory.
> * Fixed memory leak of port tunnel ids.
> * Added a missing "static" to a structure in
> en-port-binding-logical-switch-port.c.
>
> v7 -> v8:
> * Rebased.
>
> v6 -> v7:
> * Rebased.
> * Made some recommended changes to formatting (combining if statements,
> fixing indentation, etc.)
> * Switched from an array of router DPGs in the chassisredirect code
> to a vector.
> * Fixed several memory leaks in the chassisredirect code.
> * Fixed a crash in northd.c in the incremental case where a northbound
> logical switch port did not have a corresponding paired logical
> switch port to find due to tunnel key issues.
> * Ensured that we only compare a requested tunnel key against the
> VXLAN multicast minimum when VXLAN is actually enabled.
>
> v5 -> v6:
> * Rebased.
> * All calls to xzalloc() in this patch have been changed to xmalloc()
> or xcalloc().
> * Candidate paired port bindings use a vector instead of a list now.
> * Chassisredirect ports have been simplified a great deal.
> * This version introduces an enum for port binding types to make type
> checking more efficient.
> * This patch abandons the attempt to blindly pull in unpaired port
> binding maps based on their positions in the engine node inputs
> array. Instead, we now explicitly pull them in by input name and
> assign them to array positions based on their port binding type.
>
> v4 -> v5:
> * Rebased.
> * Fixed some formatting anomalies in mirror port syncing.
> * Fixed checkpatch warnings.
>
> v3 -> v4:
> * Rebased.
> * Addressed newly-added mirror port functionality.
> * Fixed many memory leaks.
>
> v3: This is the first version with this patch present.
> ---
>
> TODO.rst | 12 +
> northd/automake.mk | 14 +-
> northd/en-global-config.c | 3 +
> northd/en-global-config.h | 1 +
> northd/en-northd.c | 49 +-
> northd/en-port-binding-chassisredirect.c | 329 ++++++++
> northd/en-port-binding-chassisredirect.h | 50 ++
> northd/en-port-binding-logical-router-port.c | 176 +++++
> northd/en-port-binding-logical-router-port.h | 47 ++
> northd/en-port-binding-logical-switch-port.c | 229 ++++++
> northd/en-port-binding-logical-switch-port.h | 48 ++
> northd/en-port-binding-mirror.c | 191 +++++
> northd/en-port-binding-mirror.h | 48 ++
> northd/en-port-binding-pair.c | 473 ++++++++++++
> northd/en-port-binding-pair.h | 34 +
> northd/inc-proc-northd.c | 54 +-
> northd/northd.c | 751 +++++--------------
> northd/northd.h | 17 +-
> northd/port-binding-pair.c | 77 ++
> northd/port-binding-pair.h | 108 +++
> 20 files changed, 2125 insertions(+), 586 deletions(-)
> create mode 100644 northd/en-port-binding-chassisredirect.c
> create mode 100644 northd/en-port-binding-chassisredirect.h
> create mode 100644 northd/en-port-binding-logical-router-port.c
> create mode 100644 northd/en-port-binding-logical-router-port.h
> create mode 100644 northd/en-port-binding-logical-switch-port.c
> create mode 100644 northd/en-port-binding-logical-switch-port.h
> create mode 100644 northd/en-port-binding-mirror.c
> create mode 100644 northd/en-port-binding-mirror.h
> create mode 100644 northd/en-port-binding-pair.c
> create mode 100644 northd/en-port-binding-pair.h
> create mode 100644 northd/port-binding-pair.c
> create mode 100644 northd/port-binding-pair.h
>
> diff --git a/TODO.rst b/TODO.rst
> index 78962bb92..60ae155c5 100644
> --- a/TODO.rst
> +++ b/TODO.rst
> @@ -168,3 +168,15 @@ OVN To-do List
> ovn\_synced\_logical_router and ovn\_synced\_logical\_switch. This will
> allow for the eventual removal of the ovn\_datapath structure from the
> codebase.
> +
> +* Port Binding sync nodes
> +
> + * Southbound Port bindings are synced across three engine nodes:
> + - en_port_binding_pair
> + - en_northd
> + - en_sync_to_sb
> + It would be easier to work with if these were combined into a
> + single node instead.
> +
> + * Add incremental processing to the en-port-binding-pair node, as
> + well as derivative nodes.
> diff --git a/northd/automake.mk b/northd/automake.mk
> index 45ca0337f..a71ac1b4d 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -54,6 +54,16 @@ northd_ovn_northd_SOURCES = \
> northd/en-learned-route-sync.h \
> northd/en-group-ecmp-route.c \
> northd/en-group-ecmp-route.h \
> + northd/en-port-binding-logical-router-port.c \
> + northd/en-port-binding-logical-router-port.h \
> + northd/en-port-binding-logical-switch-port.c \
> + northd/en-port-binding-logical-switch-port.h \
> + northd/en-port-binding-chassisredirect.c \
> + northd/en-port-binding-chassisredirect.h \
> + northd/en-port-binding-mirror.c \
> + northd/en-port-binding-mirror.h \
> + northd/en-port-binding-pair.c \
> + northd/en-port-binding-pair.h \
> northd/inc-proc-northd.c \
> northd/inc-proc-northd.h \
> northd/ipam.c \
> @@ -61,7 +71,9 @@ northd_ovn_northd_SOURCES = \
> northd/lflow-mgr.c \
> northd/lflow-mgr.h \
> northd/lb.c \
> - northd/lb.h
> + northd/lb.h \
> + northd/port-binding-pair.c \
> + northd/port-binding-pair.h
> northd_ovn_northd_LDADD = \
> lib/libovn.la \
> $(OVSDB_LIBDIR)/libovsdb.la \
> diff --git a/northd/en-global-config.c b/northd/en-global-config.c
> index 5a0b4c600..dd38ea4b4 100644
> --- a/northd/en-global-config.c
> +++ b/northd/en-global-config.c
> @@ -148,6 +148,9 @@ en_global_config_run(struct engine_node *node , void
> *data)
> config_data->max_dp_tunnel_id =
> get_ovn_max_dp_key_local(config_data->vxlan_mode, ic_vxlan_mode);
>
> + uint8_t pb_tunnel_bits = config_data->vxlan_mode ? 12 : 16;
> + config_data->max_pb_tunnel_id = (1u << (pb_tunnel_bits - 1)) - 1;
> +
> char *max_tunid = xasprintf("%d", config_data->max_dp_tunnel_id);
> smap_replace(options, "max_tunid", max_tunid);
> free(max_tunid);
> diff --git a/northd/en-global-config.h b/northd/en-global-config.h
> index 55a1e420b..dbb06151c 100644
> --- a/northd/en-global-config.h
> +++ b/northd/en-global-config.h
> @@ -51,6 +51,7 @@ struct ed_type_global_config {
>
> bool vxlan_mode;
> uint32_t max_dp_tunnel_id;
> + uint32_t max_pb_tunnel_id;
>
> bool tracked;
> struct global_config_tracked_data tracked_data;
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 0c66a6700..02be1968e 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -125,6 +125,19 @@ northd_get_input_data(struct engine_node *node,
>
> input_data->synced_lrs =
> engine_get_input_data("datapath_synced_logical_router", node);
> +
> + input_data->paired_lsps =
> + engine_get_input_data("port_binding_paired_logical_switch_port",
> node);
> +
> + input_data->paired_lrps =
> + engine_get_input_data("port_binding_paired_logical_router_port",
> node);
> +
> + input_data->paired_crps =
> + engine_get_input_data("port_binding_paired_chassisredirect_port",
> + node);
> +
> + input_data->paired_mirrors =
> + engine_get_input_data("port_binding_paired_mirror", node);
> }
>
> enum engine_node_state
> @@ -501,42 +514,6 @@ en_northd_clear_tracked_data(void *data_)
> destroy_northd_data_tracked_changes(data);
> }
>
> -enum engine_input_handler_result
> -northd_sb_fdb_change_handler(struct engine_node *node, void *data)
> -{
> - struct northd_data *nd = data;
> - const struct sbrec_fdb_table *sbrec_fdb_table =
> - EN_OVSDB_GET(engine_get_input("SB_fdb", node));
> -
> - /* check if changed rows are stale and delete them */
> - const struct sbrec_fdb *fdb_e, *fdb_prev_del = NULL;
> - SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb_e, sbrec_fdb_table) {
> - if (sbrec_fdb_is_deleted(fdb_e)) {
> - continue;
> - }
> -
> - if (fdb_prev_del) {
> - sbrec_fdb_delete(fdb_prev_del);
> - }
> -
> - fdb_prev_del = fdb_e;
> - struct ovn_datapath *od
> - = ovn_datapath_find_by_key(&nd->ls_datapaths.datapaths,
> - fdb_e->dp_key);
> - if (od) {
> - if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) {
> - fdb_prev_del = NULL;
> - }
> - }
> - }
> -
> - if (fdb_prev_del) {
> - sbrec_fdb_delete(fdb_prev_del);
> - }
> -
> - return EN_HANDLED_UNCHANGED;
> -}
> -
> void
> en_route_policies_cleanup(void *data)
> {
> diff --git a/northd/en-port-binding-chassisredirect.c
> b/northd/en-port-binding-chassisredirect.c
> new file mode 100644
> index 000000000..eda97af5c
> --- /dev/null
> +++ b/northd/en-port-binding-chassisredirect.c
> @@ -0,0 +1,329 @@
> +/*
> + * 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
> + *
> + * 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 "inc-proc-eng.h"
> +#include "ovn-nb-idl.h"
> +#include "en-datapath-logical-router.h"
> +#include "en-datapath-logical-switch.h"
> +#include "en-port-binding-chassisredirect.h"
> +#include "port-binding-pair.h"
> +#include "ovn-util.h"
> +#include "vec.h"
> +
> +#include "openvswitch/vlog.h"
> +
> +#include "hmapx.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_port_binding_chassisredirect_port);
> +
> +struct router_dgps {
> + const struct ovn_synced_logical_router *lr;
> + struct vector dgps;
> +};
> +
> +static struct router_dgps *
> +router_dgps_alloc(const struct ovn_synced_logical_router *lr)
> +{
> + struct router_dgps *rdgps = xmalloc(sizeof *rdgps);
> + rdgps->lr = lr;
> + rdgps->dgps =
> + VECTOR_EMPTY_INITIALIZER(const struct nbrec_logical_router_port *);
> +
Nit: designated initializers.
> + return rdgps;
> +}
> +
> +static void
> +router_dgps_free(struct router_dgps *rdgps)
> +{
> + vector_destroy(&rdgps->dgps);
> + free(rdgps);
> +}
> +
> +struct switch_localnets {
> + const struct ovn_synced_logical_switch *ls;
> + size_t n_localnet_ports;
> +};
> +
> +struct port_router_dgps {
> + const struct nbrec_logical_router_port *nbrp;
> + struct router_dgps *r;
> +};
> +
> +static struct port_router_dgps *
> +port_router_dgps_alloc(const struct nbrec_logical_router_port *nbrp,
> + struct router_dgps *r)
> +{
> + struct port_router_dgps *p_dgps = xmalloc(sizeof *p_dgps);
> + p_dgps->nbrp = nbrp;
> + p_dgps->r = r;
> +
Nit: designated initializers.
> + return p_dgps;
> +}
> +
> +void *
> +en_port_binding_chassisredirect_port_init(struct engine_node *node
> OVS_UNUSED,
> + struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_unpaired_port_binding_map *map = xmalloc(sizeof *map);
> + ovn_unpaired_port_binding_map_init(map, NULL);
> + return map;
> +}
> +
> +struct chassisredirect_port {
> + char *name;
> + union {
> + const struct nbrec_logical_router_port *nbrp;
> + const struct nbrec_logical_switch_port *nbsp;
> + };
> + enum ovn_datapath_type dp_type;
> +};
> +
> +static struct chassisredirect_port *
> +chassisredirect_router_port_alloc(const struct nbrec_logical_router_port
> *nbrp)
> +{
> + struct chassisredirect_port *crp = xmalloc(sizeof *crp);
> + crp->name = ovn_chassis_redirect_name(nbrp->name);
> + crp->nbrp = nbrp;
> + crp->dp_type = DP_ROUTER;
Nit: designated initializers.
> +
> + return crp;
> +}
> +
> +static struct chassisredirect_port *
> +chassisredirect_switch_port_alloc(const struct nbrec_logical_switch_port
> *nbsp)
> +{
> + struct chassisredirect_port *crp = xmalloc(sizeof *crp);
> + crp->name = ovn_chassis_redirect_name(nbsp->name);
> + crp->nbsp = nbsp;
> + crp->dp_type = DP_SWITCH;
> +
Nit: designated initializers.
> + return crp;
> +}
> +
> +static void
> +chassisredirect_port_free(struct chassisredirect_port *crp)
> +{
> + free(crp->name);
> + free(crp);
> +}
> +
> +static void
> +chassisredirect_port_binding_map_destroy(
> + struct ovn_unpaired_port_binding_map *map)
> +{
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &map->ports) {
> + struct ovn_unpaired_port_binding *upb = node->data;
> + chassisredirect_port_free(upb->cookie);
Instead of doing this here should we just register a "cookie_cleanup"
callback? Similar to what you do for "sb_is_valid". This applies to
all types of ports.
Otherwise the shash nodes contain pointers to freed data after this
loop. I know we don't access them because we destroy the map
immediately after but it just feels awkward.
> + }
> + ovn_unpaired_port_binding_map_destroy(map);
> +}
> +
> +enum engine_node_state
> +en_port_binding_chassisredirect_port_run(struct engine_node *node, void
> *data)
> +{
> + const struct ovn_synced_logical_router_map *lr_map =
> + engine_get_input_data("datapath_synced_logical_router", node);
> + const struct ovn_synced_logical_switch_map *ls_map =
> + engine_get_input_data("datapath_synced_logical_switch", node);
> +
> + struct ovn_unpaired_port_binding_map *map = data;
> +
> + chassisredirect_port_binding_map_destroy(map);
> + ovn_unpaired_port_binding_map_init(map, NULL);
Nit: It feels a bit weird to pass NULL here. We have to look at the
implementation to see that this will be translated to "default_callbacks".
Should we expose "struct ovn_unpaired_port_binding_map_callbacks
default_callbacks"?
> +
> + struct shash ports = SHASH_INITIALIZER(&ports);
> + const struct ovn_synced_logical_router *lr;
> + struct hmapx all_rdgps = HMAPX_INITIALIZER(&all_rdgps);
> + HMAP_FOR_EACH (lr, hmap_node, &lr_map->synced_routers) {
> + if (smap_get(&lr->nb->options, "chassis")) {
> + /* If the logical router has the chassis option set,
> + * then we ignore any ports that have gateway_chassis
> + * or ha_chassis_group options set.
> + */
> + continue;
> + }
> + struct router_dgps *rdgps = router_dgps_alloc(lr);
> + hmapx_add(&all_rdgps, rdgps);
> + const struct nbrec_logical_router_port *nbrp;
> + for (size_t i = 0; i < lr->nb->n_ports; i++) {
> + nbrp = lr->nb->ports[i];
> + if (nbrp->ha_chassis_group || nbrp->n_gateway_chassis) {
> + vector_push(&rdgps->dgps, &nbrp);
> + shash_add(&ports, nbrp->name,
> + port_router_dgps_alloc(nbrp, rdgps));
> + }
> + }
> + }
> +
> + struct hmapx all_localnets = HMAPX_INITIALIZER(&all_localnets);
> + const struct ovn_synced_logical_switch *ls;
> + HMAP_FOR_EACH (ls, hmap_node, &ls_map->synced_switches) {
> + struct switch_localnets *localnets = xmalloc(sizeof *localnets);
> + localnets->ls = ls;
> + localnets->n_localnet_ports = 0;
Nit: designated initializers.
> + hmapx_add(&all_localnets, localnets);
> + for (size_t i = 0; i < ls->nb->n_ports; i++) {
> + const struct nbrec_logical_switch_port *nbsp = ls->nb->ports[i];
> + if (!strcmp(nbsp->type, "localnet")) {
> + localnets->n_localnet_ports++;
> + }
> + }
> + }
> +
> + /* All logical router DGPs need corresponding chassisredirect ports
> + * made
> + */
> + struct hmapx_node *hmapx_node;
> + HMAPX_FOR_EACH (hmapx_node, &all_rdgps) {
> + struct router_dgps *rdgps = hmapx_node->data;
> + struct ovn_unpaired_port_binding *upb;
> + const struct nbrec_logical_router_port *nbrp;
> + VECTOR_FOR_EACH (&rdgps->dgps, nbrp) {
> + struct chassisredirect_port *crp =
> + chassisredirect_router_port_alloc(nbrp);
> + upb = ovn_unpaired_port_binding_alloc(0, crp->name,
> + PB_CHASSISREDIRECT_PORT,
> + crp, rdgps->lr->sb);
> + shash_add(&map->ports, crp->name, upb);
> + }
> + }
> +
> + /* Logical switch ports that are peered with DGPs need chassisredirect
> + * ports created if
> + * 1. The DGP it is paired with is the only one on its router, and
> + * 2. There are no localnet ports on the switch
> + */
> + HMAPX_FOR_EACH (hmapx_node, &all_localnets) {
> + struct switch_localnets *localnets = hmapx_node->data;
> + if (localnets->n_localnet_ports > 0) {
> + continue;
> + }
> + for (size_t i = 0; i < localnets->ls->nb->n_ports; i++) {
> + const struct nbrec_logical_switch_port *nbsp =
> + localnets->ls->nb->ports[i];
> + if (strcmp(nbsp->type, "router")) {
> + continue;
> + }
> + const char *peer_name = smap_get( ->options, "router-port");
> + if (!peer_name) {
> + continue;
> + }
> + struct port_router_dgps *prdgps = shash_find_data(&ports,
> + peer_name);
> + if (!prdgps) {
> + continue;
> + }
> + if (vector_len(&prdgps->r->dgps) > 1) {
> + continue;
> + }
> + struct ovn_unpaired_port_binding *upb;
> + struct chassisredirect_port *crp =
> + chassisredirect_switch_port_alloc(nbsp);
> + upb = ovn_unpaired_port_binding_alloc(0, crp->name,
> + PB_CHASSISREDIRECT_PORT,
> + crp, localnets->ls->sb);
> + shash_add(&map->ports, crp->name, upb);
> + }
> + }
> +
> + HMAPX_FOR_EACH (hmapx_node, &all_rdgps) {
> + router_dgps_free(hmapx_node->data);
> + }
> + hmapx_destroy(&all_rdgps);
> + shash_destroy_free_data(&ports);
> + HMAPX_FOR_EACH (hmapx_node, &all_localnets) {
> + free(hmapx_node->data);
> + }
> + hmapx_destroy(&all_localnets);
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_chassisredirect_port_cleanup(void *data)
> +{
> + struct ovn_unpaired_port_binding_map *map = data;
> + chassisredirect_port_binding_map_destroy(map);
> +}
> +
> +
> +static void
> +ovn_paired_chassisredirect_port_map_init(
> + struct ovn_paired_chassisredirect_port_map *map)
> +{
> + shash_init(&map->paired_chassisredirect_ports);
> +}
> +
> +static void
> +ovn_paired_chassisredirect_port_map_destroy(
> + struct ovn_paired_chassisredirect_port_map *map)
> +{
> + shash_destroy_free_data(&map->paired_chassisredirect_ports);
> +}
> +
> +void *
> +en_port_binding_paired_chassisredirect_port_init(
> + struct engine_node *node OVS_UNUSED, struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_paired_chassisredirect_port_map *map = xmalloc(sizeof *map);
> + ovn_paired_chassisredirect_port_map_init(map);
> + return map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_paired_chassisredirect_port_run(struct engine_node *node,
> + void *data)
> +{
> + const struct ovn_paired_port_bindings *pbs =
> + engine_get_input_data("port_binding_pair", node);
> + struct ovn_paired_chassisredirect_port_map *map = data;
> +
> + ovn_paired_chassisredirect_port_map_destroy(map);
> + ovn_paired_chassisredirect_port_map_init(map);
> +
> + struct ovn_paired_port_binding *port;
> + LIST_FOR_EACH (port, list_node, &pbs->paired_pbs) {
> + if (port->type != PB_CHASSISREDIRECT_PORT) {
> + continue;
> + }
> +
> + const struct chassisredirect_port *cr_port = port->cookie;
> + struct ovn_paired_chassisredirect_port *paired_cr_port;
> + paired_cr_port = xmalloc(sizeof *paired_cr_port);
> + paired_cr_port->name = cr_port->name;
> + paired_cr_port->sb = port->sb_pb;
> + paired_cr_port->dp_type = cr_port->dp_type;
Nit: designated initializers.
> + if (paired_cr_port->dp_type == DP_SWITCH) {
> + paired_cr_port->primary_nbsp = cr_port->nbsp;
> + } else {
> + paired_cr_port->primary_nbrp = cr_port->nbrp;
> + }
> + shash_add(&map->paired_chassisredirect_ports, cr_port->name,
> + paired_cr_port);
> + }
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_paired_chassisredirect_port_cleanup(void *data)
> +{
> + struct ovn_paired_chassisredirect_port_map *map = data;
> +
> + ovn_paired_chassisredirect_port_map_destroy(map);
> +}
> diff --git a/northd/en-port-binding-chassisredirect.h
> b/northd/en-port-binding-chassisredirect.h
> new file mode 100644
> index 000000000..f1477cbbf
> --- /dev/null
> +++ b/northd/en-port-binding-chassisredirect.h
> @@ -0,0 +1,50 @@
> +/*
> + * 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
> + *
> + * 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_PORT_BINDING_CHASSISREDIRECT_PORT_H
> +#define EN_PORT_BINDING_CHASSISREDIRECT_PORT_H
> +
> +#include "lib/inc-proc-eng.h"
> +#include "datapath-sync.h"
> +#include "openvswitch/shash.h"
> +
> +void *en_port_binding_chassisredirect_port_init(struct engine_node *,
> + struct engine_arg *);
> +
> +enum engine_node_state en_port_binding_chassisredirect_port_run(
> + struct engine_node *, void *data);
> +void en_port_binding_chassisredirect_port_cleanup(void *data);
> +
> +struct ovn_paired_chassisredirect_port {
> + const char *name;
> + union {
> + const struct nbrec_logical_switch_port *primary_nbsp;
> + const struct nbrec_logical_router_port *primary_nbrp;
> + };
> + enum ovn_datapath_type dp_type;
> + const struct sbrec_port_binding *sb;
> +};
> +
> +struct ovn_paired_chassisredirect_port_map {
> + struct shash paired_chassisredirect_ports;
> +};
> +
> +void *en_port_binding_paired_chassisredirect_port_init(struct engine_node *,
> + struct engine_arg *);
> +enum engine_node_state en_port_binding_paired_chassisredirect_port_run(
> + struct engine_node *, void *data);
> +void en_port_binding_paired_chassisredirect_port_cleanup(void *data);
> +#endif /* EN_PORT_BINDING_CHASSISREDIRECT_PORT_H */
> diff --git a/northd/en-port-binding-logical-router-port.c
> b/northd/en-port-binding-logical-router-port.c
> new file mode 100644
> index 000000000..ba2080e61
> --- /dev/null
> +++ b/northd/en-port-binding-logical-router-port.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
> + *
> + * 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/vlog.h"
> +#include "util.h"
> +
> +#include "inc-proc-eng.h"
> +#include "ovn-nb-idl.h"
> +#include "port-binding-pair.h"
> +#include "en-datapath-logical-router.h"
> +#include "en-port-binding-logical-router-port.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_port_binding_logical_router_port);
> +
> +struct logical_router_port_cookie {
> + const struct nbrec_logical_router_port *nbrp;
> + const struct ovn_synced_logical_router *router;
> +};
> +
> +static struct logical_router_port_cookie *
> +logical_router_port_cookie_alloc(const struct nbrec_logical_router_port
> *nbrp,
> + const struct ovn_synced_logical_router *lr)
> +{
> + struct logical_router_port_cookie *cookie = xmalloc(sizeof *cookie);
> + cookie->nbrp = nbrp;
> + cookie->router = lr;
Nit: the same designated initializers comment as I had on patch 3/4.
> +
> + return cookie;
> +}
> +
> +static void
> +logical_router_port_cookie_free(struct logical_router_port_cookie *cookie)
> +{
> + free(cookie);
> +}
> +
> +static void
> +unpaired_logical_router_port_map_destroy(
> + struct ovn_unpaired_port_binding_map *map)
> +{
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &map->ports) {
> + struct ovn_unpaired_port_binding *upb = node->data;
> + logical_router_port_cookie_free(upb->cookie);
> + }
> + ovn_unpaired_port_binding_map_destroy(map);
> +}
> +
> +void *
> +en_port_binding_logical_router_port_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_unpaired_port_binding_map *map = xmalloc(sizeof *map);
> + ovn_unpaired_port_binding_map_init(map, NULL);
> + return map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_logical_router_port_run(struct engine_node *node, void *data)
> +{
> + const struct ovn_synced_logical_router_map *lr_map =
> + engine_get_input_data("datapath_synced_logical_router", node);
> +
> + struct ovn_unpaired_port_binding_map *map = data;
> +
> + unpaired_logical_router_port_map_destroy(map);
> + ovn_unpaired_port_binding_map_init(map, NULL);
> +
> + const struct ovn_synced_logical_router *paired_lr;
> + HMAP_FOR_EACH (paired_lr, hmap_node, &lr_map->synced_routers) {
> + const struct nbrec_logical_router_port *nbrp;
> + for (size_t i = 0; i < paired_lr->nb->n_ports; i++) {
> + nbrp = paired_lr->nb->ports[i];
> + uint32_t requested_tunnel_key = smap_get_int(&nbrp->options,
> + "requested-tnl-key",
> + 0);
> + struct logical_router_port_cookie *cookie =
> + logical_router_port_cookie_alloc(nbrp, paired_lr);
> + struct ovn_unpaired_port_binding *upb;
> + upb = ovn_unpaired_port_binding_alloc(requested_tunnel_key,
> + nbrp->name,
> + PB_ROUTER_PORT,
> + cookie,
> + paired_lr->sb);
> + smap_clone(&upb->external_ids, &nbrp->external_ids);
> + if (!shash_add_once(&map->ports, nbrp->name, upb)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> + VLOG_WARN_RL(&rl, "duplicate logical router port %s",
> + nbrp->name);
> + }
> + }
> + }
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_logical_router_port_cleanup(void *data)
> +{
> + struct ovn_unpaired_port_binding_map *map = data;
> + unpaired_logical_router_port_map_destroy(map);
> +}
> +
> +static void
> +paired_logical_router_port_map_init(
> + struct ovn_paired_logical_router_port_map *router_port_map)
> +{
> + shash_init(&router_port_map->paired_router_ports);
> +}
> +
> +static void
> +paired_logical_router_port_map_destroy(
> + struct ovn_paired_logical_router_port_map *router_port_map)
> +{
> + shash_destroy_free_data(&router_port_map->paired_router_ports);
> +}
> +
> +void *
> +en_port_binding_paired_logical_router_port_init(
> + struct engine_node *node OVS_UNUSED, struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_paired_logical_router_port_map *router_port_map;
> + router_port_map = xmalloc(sizeof *router_port_map);
> + paired_logical_router_port_map_init(router_port_map);
> +
> + return router_port_map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_paired_logical_router_port_run(struct engine_node *node,
> + void *data)
> +{
> + const struct ovn_paired_port_bindings *pbs =
> + engine_get_input_data("port_binding_pair", node);
> + struct ovn_paired_logical_router_port_map *router_port_map = data;
> +
> + paired_logical_router_port_map_destroy(router_port_map);
> + paired_logical_router_port_map_init(router_port_map);
> +
> + struct ovn_paired_port_binding *spb;
> + LIST_FOR_EACH (spb, list_node, &pbs->paired_pbs) {
> + if (spb->type != PB_ROUTER_PORT) {
> + continue;
> + }
> + const struct logical_router_port_cookie *cookie = spb->cookie;
> + struct ovn_paired_logical_router_port *lrp = xmalloc(sizeof *lrp);
> + lrp->nb = cookie->nbrp;
> + lrp->router = cookie->router;
> + lrp->sb = spb->sb_pb;
Nit: designated initializers.
> + shash_add(&router_port_map->paired_router_ports, lrp->nb->name, lrp);
> + }
> +
> + return EN_UPDATED;
> +}
> +
> +void en_port_binding_paired_logical_router_port_cleanup(void *data)
> +{
> + struct ovn_paired_logical_router_port_map *map = data;
> + paired_logical_router_port_map_destroy(map);
> +}
> diff --git a/northd/en-port-binding-logical-router-port.h
> b/northd/en-port-binding-logical-router-port.h
> new file mode 100644
> index 000000000..156a25da6
> --- /dev/null
> +++ b/northd/en-port-binding-logical-router-port.h
> @@ -0,0 +1,47 @@
> +/*
> + * 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
> + *
> + * 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_PORT_BINDING_LOGICAL_ROUTER_PORT_H
> +#define EN_PORT_BINDING_LOGICAL_ROUTER_PORT_H
> +
> +#include "lib/inc-proc-eng.h"
> +#include "openvswitch/shash.h"
> +
> +void *en_port_binding_logical_router_port_init(struct engine_node *,
> + struct engine_arg *);
Nit: indentation.
> +
> +enum engine_node_state en_port_binding_logical_router_port_run(
> + struct engine_node *, void *data);
> +void en_port_binding_logical_router_port_cleanup(void *data);
> +
> +struct ovn_paired_logical_router_port {
> + const struct nbrec_logical_router_port *nb;
> + const struct sbrec_port_binding *sb;
> + const struct ovn_synced_logical_router *router;
> +};
> +
> +struct ovn_paired_logical_router_port_map {
> + struct shash paired_router_ports;
> +};
> +
> +void *en_port_binding_paired_logical_router_port_init(struct engine_node *,
> + struct engine_arg *);
> +
> +enum engine_node_state en_port_binding_paired_logical_router_port_run(
> + struct engine_node *, void *data);
> +void en_port_binding_paired_logical_router_port_cleanup(void *data);
> +
> +#endif /* EN_PORT_BINDING_LOGICAL_ROUTER_PORT_H */
> diff --git a/northd/en-port-binding-logical-switch-port.c
> b/northd/en-port-binding-logical-switch-port.c
> new file mode 100644
> index 000000000..47b1f0057
> --- /dev/null
> +++ b/northd/en-port-binding-logical-switch-port.c
> @@ -0,0 +1,229 @@
> +/*
> + * 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
> + *
> + * 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/vlog.h"
> +#include "util.h"
> +
> +#include "inc-proc-eng.h"
> +#include "ovn-nb-idl.h"
> +#include "ovn-sb-idl.h"
> +#include "port-binding-pair.h"
> +#include "en-datapath-logical-switch.h"
> +#include "en-port-binding-logical-switch-port.h"
> +#include "northd.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_port_binding_logical_switch_port);
> +
> +struct logical_switch_port_cookie {
> + const struct nbrec_logical_switch_port *nbsp;
> + const struct ovn_synced_logical_switch *sw;
> +};
> +
> +static struct logical_switch_port_cookie *
> +logical_switch_port_cookie_alloc(const struct nbrec_logical_switch_port
> *nbsp,
> + const struct ovn_synced_logical_switch *sw)
> +{
> + struct logical_switch_port_cookie *cookie = xmalloc(sizeof *cookie);
> + cookie->nbsp = nbsp;
> + cookie->sw = sw;
Nit: designated initializers.
> + return cookie;
> +}
> +
> +static void
> +logical_switch_port_cookie_free(struct logical_switch_port_cookie *cookie)
> +{
> + free(cookie);
> +}
> +
> +static bool
> +switch_port_sb_is_valid(const struct sbrec_port_binding *sb_pb,
> + const struct ovn_unpaired_port_binding *upb)
> +{
> + const struct logical_switch_port_cookie *cookie = upb->cookie;
> +
> + bool update_sbrec = false;
> + if (lsp_is_type_changed(sb_pb, cookie->nbsp, &update_sbrec)
> + && update_sbrec) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static struct ovn_unpaired_port_binding_map_callbacks switch_port_callbacks
> = {
> + .sb_is_valid = switch_port_sb_is_valid,
> +};
> +
> +static void
> +unpaired_logical_switch_port_map_destroy(
> + struct ovn_unpaired_port_binding_map *map)
> +{
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &map->ports) {
> + struct ovn_unpaired_port_binding *upb = node->data;
> + logical_switch_port_cookie_free(upb->cookie);
> + }
> + ovn_unpaired_port_binding_map_destroy(map);
> +}
> +
> +void *
> +en_port_binding_logical_switch_port_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_unpaired_port_binding_map *map = xmalloc(sizeof *map);
> + ovn_unpaired_port_binding_map_init(map, &switch_port_callbacks);
> + return map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_logical_switch_port_run(struct engine_node *node, void *data)
> +{
> + const struct ovn_synced_logical_switch_map *ls_map =
> + engine_get_input_data("datapath_synced_logical_switch", node);
> +
> + struct ovn_unpaired_port_binding_map *map = data;
> +
> + unpaired_logical_switch_port_map_destroy(map);
> + ovn_unpaired_port_binding_map_init(map, &switch_port_callbacks);
> +
> + const struct ovn_synced_logical_switch *paired_ls;
> + HMAP_FOR_EACH (paired_ls, hmap_node, &ls_map->synced_switches) {
> + const struct nbrec_logical_switch_port *nbsp;
> + for (size_t i = 0; i < paired_ls->nb->n_ports; i++) {
> + nbsp = paired_ls->nb->ports[i];
> + uint32_t requested_tunnel_key = smap_get_int( ->options,
> + "requested-tnl-key",
> + 0);
> + struct logical_switch_port_cookie *cookie =
> + logical_switch_port_cookie_alloc(nbsp, paired_ls);
> + struct ovn_unpaired_port_binding *upb;
> + upb = ovn_unpaired_port_binding_alloc(requested_tunnel_key,
> + nbsp->name,
> + PB_SWITCH_PORT,
> + cookie,
> + paired_ls->sb);
> + smap_clone(&upb->external_ids,  ->external_ids);
> + const char *name = smap_get( ->external_ids,
> + "neutron:port_name");
> + if (name && name[0]) {
> + smap_add(&upb->external_ids, "name", name);
> + }
> + if (!shash_add_once(&map->ports, nbsp->name, upb)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> + VLOG_WARN_RL(&rl, "duplicate logical port %s", nbsp->name);
> + }
> + }
> + }
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_logical_switch_port_cleanup(void *data)
> +{
> + struct ovn_unpaired_port_binding_map *map = data;
> + unpaired_logical_switch_port_map_destroy(map);
> +}
> +
> +static void
> +paired_logical_switch_port_map_init(
> + struct ovn_paired_logical_switch_port_map *switch_port_map)
> +{
> + shash_init(&switch_port_map->paired_switch_ports);
> +}
> +
> +static void
> +paired_logical_switch_port_map_destroy(
> + struct ovn_paired_logical_switch_port_map *switch_port_map)
> +{
> + shash_destroy_free_data(&switch_port_map->paired_switch_ports);
> +}
> +
> +void *
> +en_port_binding_paired_logical_switch_port_init(
> + struct engine_node *node OVS_UNUSED, struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_paired_logical_switch_port_map *switch_port_map;
> + switch_port_map = xmalloc(sizeof *switch_port_map);
> + paired_logical_switch_port_map_init(switch_port_map);
> +
> + return switch_port_map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_paired_logical_switch_port_run(struct engine_node *node,
> + void *data)
> +{
> + const struct ovn_paired_port_bindings *pbs =
> + engine_get_input_data("port_binding_pair", node);
> + struct ovn_paired_logical_switch_port_map *switch_port_map = data;
> +
> + paired_logical_switch_port_map_destroy(switch_port_map);
> + paired_logical_switch_port_map_init(switch_port_map);
> +
> + struct ovn_paired_port_binding *spb;
> + LIST_FOR_EACH (spb, list_node, &pbs->paired_pbs) {
> + if (spb->type != PB_SWITCH_PORT) {
> + continue;
> + }
> + const struct logical_switch_port_cookie *cookie = spb->cookie;
> + struct ovn_paired_logical_switch_port *lsw = xmalloc(sizeof *lsw);
> + lsw->nb = cookie->nbsp;
> + lsw->sw = cookie->sw;
> + lsw->sb = spb->sb_pb;
Nit: designated initializers.
> + shash_add(&switch_port_map->paired_switch_ports, lsw->nb->name, lsw);
> +
> + /* This deals with a special case where a logical switch port is
> + * removed and added back very quickly. The sequence of events is as
> + * follows:
> + * 1) NB Logical_Switch_Port "lsp" is added to the NB DB.
> + * 2) en-port-binding-pair creates a corresponding SB Port_Binding.
> + * 3) The user binds the port to a hypervisor.
> + * 4) ovn-controller on the hypervisor sets the SB Port_Binding "up"
> + * column to "true".
> + * 5) ovn-northd sets the Logical_Switch_Port "up" column to "true".
> + * 6) A user deletes and then re-adds "lsp" back to the NB
> + * Logical_Switch_Port column very quickly, so quickly that we
> + * do not detect the deletion at all.
> + * 7) The new northbound Logical_Switch_Port has its "up" column
> + * empty (i.e. not "true") since it is new.
> + * 8) The pairing code matches the new Logical_Switch_Port with the
> + * existing Port_Binding for "lsp" since the pairing code matches
> + * using the name of the Logical_Switch_Port.
> + *
> + * At this point, the SB Port_Binding's "up" column is set "true",
> + * but the NB Logical_Switch_Port's "up" column is not. We need to
> + * ensure the NB Logical_Switch_Port's "up" column is set to "true"
> + * as well.
> + *
> + * In most cases, setting the NB Logical_Switch_Port "up" column to
> + * true is accomplished when changes on the SB Port_Binding are
> + * detected. But in this rare case, there is no SB Port_Binding
> + * change, so the "up" column is unserviced.
> + */
> + lsp_set_up(lsw->sb, lsw->nb);
> + }
> +
> + return EN_UPDATED;
> +}
> +
> +void en_port_binding_paired_logical_switch_port_cleanup(void *data)
> +{
> + struct ovn_paired_logical_switch_port_map *map = data;
> + paired_logical_switch_port_map_destroy(map);
> +}
> diff --git a/northd/en-port-binding-logical-switch-port.h
> b/northd/en-port-binding-logical-switch-port.h
> new file mode 100644
> index 000000000..9ef32ce88
> --- /dev/null
> +++ b/northd/en-port-binding-logical-switch-port.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
> + *
> + * 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_PORT_BINDING_LOGICAL_SWITCH_PORT_H
> +#define EN_PORT_BINDING_LOGICAL_SWITCH_PORT_H
> +
> +#include "lib/inc-proc-eng.h"
> +#include "openvswitch/shash.h"
> +
> +
> +void *en_port_binding_logical_switch_port_init(struct engine_node *,
> + struct engine_arg *);
> +
> +enum engine_node_state en_port_binding_logical_switch_port_run(
> + struct engine_node *, void *data);
> +void en_port_binding_logical_switch_port_cleanup(void *data);
> +
> +struct ovn_paired_logical_switch_port {
> + const struct nbrec_logical_switch_port *nb;
> + const struct sbrec_port_binding *sb;
> + const struct ovn_synced_logical_switch *sw;
> +};
> +
> +struct ovn_paired_logical_switch_port_map {
> + struct shash paired_switch_ports;
> +};
> +
> +void *en_port_binding_paired_logical_switch_port_init(struct engine_node *,
> + struct engine_arg *);
> +
> +enum engine_node_state en_port_binding_paired_logical_switch_port_run(
> + struct engine_node *, void *data);
> +void en_port_binding_paired_logical_switch_port_cleanup(void *data);
> +
> +#endif /* EN_PORT_BINDING_LOGICAL_SWITCH_PORT_H */
> diff --git a/northd/en-port-binding-mirror.c b/northd/en-port-binding-mirror.c
> new file mode 100644
> index 000000000..000975987
> --- /dev/null
> +++ b/northd/en-port-binding-mirror.c
> @@ -0,0 +1,191 @@
> +/*
> + * 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
> + *
> + * 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 "ovn-util.h"
> +#include "lib/inc-proc-eng.h"
> +#include "ovn-nb-idl.h"
> +#include "en-datapath-logical-switch.h"
> +#include "en-port-binding-mirror.h"
> +#include "port-binding-pair.h"
> +#include "northd.h"
> +#include "openvswitch/vlog.h"
> +
> +#define MIRROR_PORT_TYPE "mirror"
> +
> +VLOG_DEFINE_THIS_MODULE(en_port_binding_mirror);
> +
> +struct mirror_port {
> + char *name;
> + const char *sink;
> + const struct nbrec_logical_switch_port *nbsp;
> +};
> +
> +static struct mirror_port *
> +mirror_port_alloc(const struct sbrec_datapath_binding *sb, const char *sink,
> + const struct nbrec_logical_switch_port *nbsp)
> +{
> + struct mirror_port *mp = xmalloc(sizeof *mp);
> + mp->name = ovn_mirror_port_name(ovn_datapath_name(sb), sink);
> + mp->sink = sink;
> + mp->nbsp = nbsp;
> +
Nit: designated initializers.
> + return mp;
> +}
> +
> +static void
> +mirror_port_free(struct mirror_port *mp)
> +{
> + free(mp->name);
> + free(mp);
> +}
> +
> +static void
> +unpaired_mirror_map_destroy(struct ovn_unpaired_port_binding_map *map)
> +{
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &map->ports) {
> + struct ovn_unpaired_port_binding *upb = node->data;
> + mirror_port_free(upb->cookie);
> + }
> + ovn_unpaired_port_binding_map_destroy(map);
> +}
> +
> +void *
> +en_port_binding_mirror_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg OVS_UNUSED)
> +{
> + struct ovn_unpaired_port_binding_map *map = xmalloc(sizeof *map);
> + ovn_unpaired_port_binding_map_init(map, NULL);
> + return map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_mirror_run(struct engine_node *node, void *data)
> +{
> + const struct ovn_synced_logical_switch_map *ls_map =
> + engine_get_input_data("datapath_synced_logical_switch", node);
> + struct ovn_unpaired_port_binding_map *map = data;
> +
> + unpaired_mirror_map_destroy(map);
> + ovn_unpaired_port_binding_map_init(map, NULL);
> +
> + /* Typically, we'd use an ovsdb_idl_index to search for a specific record
> + * based on a column value. However, we currently are not monitoring
> + * the Logical_Switch_Port table at all in ovn-northd. Introducing
> + * this monitoring is likely more computationally intensive than
> + * making an on-the-fly sset of logical switch port names.
> + */
> + struct sset all_switch_ports = SSET_INITIALIZER(&all_switch_ports);
> + const struct ovn_synced_logical_switch *ls;
> + HMAP_FOR_EACH (ls, hmap_node, &ls_map->synced_switches) {
> + for (size_t i = 0; i < ls->nb->n_ports; i++) {
> + sset_add(&all_switch_ports, ls->nb->ports[i]->name);
> + }
> + }
> +
> + HMAP_FOR_EACH (ls, hmap_node, &ls_map->synced_switches) {
> + for (size_t i = 0; i < ls->nb->n_ports; i++) {
> + const struct nbrec_logical_switch_port *nbsp = ls->nb->ports[i];
> + for (size_t j = 0; j < nbsp->n_mirror_rules; j++) {
> + struct nbrec_mirror *nb_mirror = nbsp->mirror_rules[j];
> + if (strcmp(nb_mirror->type, "lport")) {
> + continue;
> + }
> + if (!sset_find(&all_switch_ports, nb_mirror->sink)) {
> + continue;
> + }
> + struct mirror_port *mp = mirror_port_alloc(ls->sb,
> + nb_mirror->sink,
> + nbsp);
> + struct ovn_unpaired_port_binding *upb;
> + upb = ovn_unpaired_port_binding_alloc(0, mp->name,
> + PB_MIRROR_PORT, mp,
> + ls->sb);
> + shash_add(&map->ports, mp->name, upb);
> + }
> + }
> + }
> + sset_destroy(&all_switch_ports);
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_mirror_cleanup(void *data)
> +{
> + struct ovn_unpaired_port_binding_map *map = data;
> + unpaired_mirror_map_destroy(map);
> +}
> +
> +static void
> +ovn_paired_mirror_map_init(
> + struct ovn_paired_mirror_map *map)
> +{
> + shash_init(&map->paired_mirror_ports);
> +}
> +
> +static void
> +ovn_paired_mirror_map_destroy(
> + struct ovn_paired_mirror_map *map)
> +{
> + shash_destroy_free_data(&map->paired_mirror_ports);
> +}
> +
> +void *
> +en_port_binding_paired_mirror_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg OVS_UNUSED)
> +{
> + struct ovn_paired_mirror_map *map = xmalloc(sizeof *map);
> + ovn_paired_mirror_map_init(map);
> + return map;
> +}
> +
> +enum engine_node_state
> +en_port_binding_paired_mirror_run(struct engine_node *node,
> + void *data)
> +{
> + const struct ovn_paired_port_bindings *pbs =
> + engine_get_input_data("port_binding_pair", node);
> + struct ovn_paired_mirror_map *map = data;
> +
> + ovn_paired_mirror_map_destroy(map);
> + ovn_paired_mirror_map_init(map);
> +
> + struct ovn_paired_port_binding *port;
> + LIST_FOR_EACH (port, list_node, &pbs->paired_pbs) {
> + if (port->type != PB_MIRROR_PORT) {
> + continue;
> + }
> + const struct mirror_port *mp = port->cookie;
> + struct ovn_paired_mirror *opm = xmalloc(sizeof *opm);
> + opm->name = mp->name;
> + opm->sink = mp->sink;
> + opm->sb = port->sb_pb;
> + opm->nbsp = mp->nbsp;
Nit: designated initializers.
> + shash_add(&map->paired_mirror_ports, opm->name, opm);
> + }
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_paired_mirror_cleanup(void *data)
> +{
> + struct ovn_paired_mirror_map *map = data;
> +
> + ovn_paired_mirror_map_destroy(map);
> +}
> +
> diff --git a/northd/en-port-binding-mirror.h b/northd/en-port-binding-mirror.h
> new file mode 100644
> index 000000000..a4bf2645a
> --- /dev/null
> +++ b/northd/en-port-binding-mirror.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
> + *
> + * 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_PORT_BINDING_MIRROR_H
> +#define EN_PORT_BINDING_MIRROR_H
> +
> +#include "lib/inc-proc-eng.h"
> +#include "openvswitch/shash.h"
> +
> +void *en_port_binding_mirror_init(struct engine_node *,
> + struct engine_arg *);
> +
> +enum engine_node_state en_port_binding_mirror_run(struct engine_node *,
> + void *data);
> +void en_port_binding_mirror_cleanup(void *data);
> +
> +struct ovn_paired_mirror {
> + const char *name;
> + const char *sink;
> + const struct nbrec_logical_switch_port *nbsp;
> + const struct sbrec_port_binding *sb;
> +};
> +
> +struct ovn_paired_mirror_map {
> + struct shash paired_mirror_ports;
> +};
> +
> +void *en_port_binding_paired_mirror_init(struct engine_node *,
> + struct engine_arg *);
> +
> +enum engine_node_state en_port_binding_paired_mirror_run(struct engine_node
> *,
> + void *data);
> +void en_port_binding_paired_mirror_cleanup(void *data);
> +
> +#endif /* EN_PORT_BINDING_MIRROR_H */
> diff --git a/northd/en-port-binding-pair.c b/northd/en-port-binding-pair.c
> new file mode 100644
> index 000000000..61c8af86b
> --- /dev/null
> +++ b/northd/en-port-binding-pair.c
> @@ -0,0 +1,473 @@
> +/*
> + * 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
> + *
> + * 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-port-binding-pair.h"
> +#include "en-global-config.h"
> +#include "port-binding-pair.h"
> +#include "ovn-sb-idl.h"
> +#include "mcast-group-index.h"
> +#include "vec.h"
> +
> +#include "simap.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(port_binding_pair);
> +
> +void *
> +en_port_binding_pair_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *args OVS_UNUSED)
> +{
> + struct ovn_paired_port_bindings *paired_port_bindings
> + = xmalloc(sizeof *paired_port_bindings);
> + ovs_list_init(&paired_port_bindings->paired_pbs);
> + hmap_init(&paired_port_bindings->tunnel_key_maps);
Nit: designatedd initializers.
> +
> + return paired_port_bindings;
> +}
> +
> +static struct ovn_unpaired_port_binding *
> +find_unpaired_port_binding(const struct ovn_unpaired_port_binding_map **maps,
> + const struct sbrec_port_binding *sb_pb)
> +{
> + const struct ovn_unpaired_port_binding_map *map;
> +
> + for (size_t i = 0; i < PB_MAX; i++) {
> + map = maps[i];
> + struct ovn_unpaired_port_binding *upb;
> + upb = shash_find_data(&map->ports, sb_pb->logical_port);
> + if (upb && map->cb->sb_is_valid(sb_pb, upb)) {
> + return upb;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +struct tunnel_key_map {
> + struct hmap_node hmap_node;
> + uint32_t datapath_tunnel_key;
> + struct hmap port_tunnel_keys;
> +};
> +
> +static struct tunnel_key_map *
> +find_tunnel_key_map(uint32_t datapath_tunnel_key,
> + const struct hmap *tunnel_key_maps)
> +{
> + uint32_t hash = hash_int(datapath_tunnel_key, 0);
> + struct tunnel_key_map *key_map;
> + HMAP_FOR_EACH_WITH_HASH (key_map, hmap_node, hash, tunnel_key_maps) {
> + if (key_map->datapath_tunnel_key == datapath_tunnel_key) {
> + return key_map;
> + }
> + }
> + return NULL;
> +}
> +
> +static struct tunnel_key_map *
> +alloc_tunnel_key_map(uint32_t datapath_tunnel_key,
> + struct hmap *tunnel_key_maps)
> +{
> + uint32_t hash = hash_int(datapath_tunnel_key, 0);
> + struct tunnel_key_map *key_map;
> +
> + key_map = xmalloc(sizeof *key_map);
> + key_map->datapath_tunnel_key = datapath_tunnel_key;
> + hmap_init(&key_map->port_tunnel_keys);
> + hmap_insert(tunnel_key_maps, &key_map->hmap_node, hash);
> +
> + return key_map;
> +
> +}
> +
> +static struct tunnel_key_map *
> +find_or_alloc_tunnel_key_map(const struct sbrec_datapath_binding *sb_dp,
> + struct hmap *tunnel_key_maps)
> +{
> + struct tunnel_key_map *key_map = find_tunnel_key_map(sb_dp->tunnel_key,
> + tunnel_key_maps);
> + if (!key_map) {
> + key_map = alloc_tunnel_key_map(sb_dp->tunnel_key, tunnel_key_maps);
> + }
> + return key_map;
> +}
> +
> +static void
> +tunnel_key_maps_destroy(struct hmap *tunnel_key_maps)
> +{
> + struct tunnel_key_map *key_map;
> + HMAP_FOR_EACH_POP (key_map, hmap_node, tunnel_key_maps) {
> + ovn_destroy_tnlids(&key_map->port_tunnel_keys);
> + free(key_map);
> + }
> + hmap_destroy(tunnel_key_maps);
> +}
> +
> +struct candidate_spb {
> + struct ovn_paired_port_binding *spb;
> + uint32_t requested_tunnel_key;
> + uint32_t existing_tunnel_key;
> + struct tunnel_key_map *tunnel_key_map;
> + bool tunnel_key_assigned;
> +};
> +
> +static void
> +reset_port_binding_pair_data(
> + struct ovn_paired_port_bindings *paired_port_bindings)
> +{
> + /* Free the old paired port_bindings */
> + struct ovn_paired_port_binding *spb;
> + LIST_FOR_EACH_POP (spb, list_node, &paired_port_bindings->paired_pbs) {
> + free(spb);
> + }
> + tunnel_key_maps_destroy(&paired_port_bindings->tunnel_key_maps);
> +
> + hmap_init(&paired_port_bindings->tunnel_key_maps);
> + ovs_list_init(&paired_port_bindings->paired_pbs);
> +}
> +
> +static struct ovn_paired_port_binding *
> +ovn_paired_port_binding_alloc(const struct sbrec_port_binding *sb_pb,
> + const struct ovn_unpaired_port_binding *upb)
> +{
> + struct ovn_paired_port_binding *spb;
> + spb = xmalloc(sizeof *spb);
> + spb->sb_pb = sb_pb;
> + spb->cookie = upb->cookie;
> + spb->type = upb->type;
Nit: designated initializers.
> + sbrec_port_binding_set_external_ids(sb_pb, &upb->external_ids);
> + sbrec_port_binding_set_logical_port(sb_pb, upb->name);
> +
> + return spb;
> +}
> +
> +static void
> +get_candidate_pbs_from_sb(
> + const struct sbrec_port_binding_table *sb_pb_table,
> + const struct ovn_unpaired_port_binding_map **input_maps,
> + struct hmap *tunnel_key_maps, struct vector *candidate_spbs,
> + struct simap *visited)
> +{
> + const struct sbrec_port_binding *sb_pb;
> + const struct ovn_unpaired_port_binding *upb;
> + SBREC_PORT_BINDING_TABLE_FOR_EACH_SAFE (sb_pb, sb_pb_table) {
> + upb = find_unpaired_port_binding(input_maps, sb_pb);
> + if (!upb) {
> + sbrec_port_binding_delete(sb_pb);
> + continue;
> + }
> +
> + if (!uuid_equals(&upb->sb_dp->header_.uuid,
> + &sb_pb->datapath->header_.uuid)) {
> + /* A matching unpaired port was found for this port binding, but
> it
> + * has moved to a different datapath. Delete the old SB port
> + * binding so that a new one will be created later when we
> traverse
> + * unpaired port bindings later.
> + */
> + sbrec_port_binding_delete(sb_pb);
> + continue;
> + }
> +
> + if (!simap_put(visited, sb_pb->logical_port, upb->type)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_INFO_RL(
> + &rl, "deleting port_binding "UUID_FMT" with "
> + "duplicate name %s",
> + UUID_ARGS(&sb_pb->header_.uuid), sb_pb->logical_port);
> + sbrec_port_binding_delete(sb_pb);
> + continue;
> + }
> + struct candidate_spb candidate = {
> + .spb = ovn_paired_port_binding_alloc(sb_pb, upb),
> + .requested_tunnel_key = upb->requested_tunnel_key,
> + .existing_tunnel_key = sb_pb->tunnel_key,
> + .tunnel_key_map =
> + find_or_alloc_tunnel_key_map(upb->sb_dp, tunnel_key_maps),
> + };
> + vector_push(candidate_spbs, &candidate);
> + }
> +}
> +
> +static void
> +get_candidate_pbs_from_nb(
> + struct ovsdb_idl_txn *ovnsb_idl_txn,
> + const struct ovn_unpaired_port_binding_map **input_maps,
> + struct hmap *tunnel_key_maps,
> + struct vector *candidate_spbs,
> + struct simap *visited)
> +{
> + for (size_t i = 0; i < PB_MAX; i++) {
> + const struct ovn_unpaired_port_binding_map *map = input_maps[i];
> + struct shash_node *shash_node;
> + SHASH_FOR_EACH (shash_node, &map->ports) {
> + const struct ovn_unpaired_port_binding *upb = shash_node->data;
> + struct simap_node *visited_node = simap_find(visited, upb->name);
> + if (visited_node) {
> + if (upb->type != visited_node->data) {
> + static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5,
> +
> 1);
> + VLOG_WARN_RL(&rl, "duplicate logical port %s",
> upb->name);
> + }
> + continue;
> + } else {
> + /* Add the port to "visited" to help with detection of
> + * duplicated port names across different types of ports.
> + */
> + simap_put(visited, upb->name, upb->type);
> + }
> + const struct sbrec_port_binding *sb_pb;
> + sb_pb = sbrec_port_binding_insert(ovnsb_idl_txn);
> + struct candidate_spb candidate = {
> + .spb = ovn_paired_port_binding_alloc(sb_pb, upb),
> + .requested_tunnel_key = upb->requested_tunnel_key,
> + .existing_tunnel_key = sb_pb->tunnel_key,
> + .tunnel_key_map =
> + find_or_alloc_tunnel_key_map(upb->sb_dp,
> tunnel_key_maps),
> + };
> + vector_push(candidate_spbs, &candidate);
> + }
> + }
> +}
> +
> +static void
> +pair_requested_tunnel_keys(struct vector *candidate_spbs,
> + struct ovs_list *paired_pbs,
> + bool vxlan_mode)
> +{
> + struct candidate_spb *candidate;
> + VECTOR_FOR_EACH_PTR (candidate_spbs, candidate) {
> + if (!candidate->requested_tunnel_key) {
> + continue;
> + }
> + if (vxlan_mode &&
> + candidate->requested_tunnel_key >= OVN_VXLAN_MIN_MULTICAST) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for port %s"
> + " is incompatible with VXLAN",
> + candidate->requested_tunnel_key,
> + candidate->spb->sb_pb->logical_port);
> + continue;
> + }
> +
> + if (!ovn_add_tnlid(&candidate->tunnel_key_map->port_tunnel_keys,
> + candidate->requested_tunnel_key)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "Logical port_binding %s requests same "
> + "tunnel key %"PRIu32" as another logical "
> + "port_binding on the same datapath",
> + candidate->spb->sb_pb->logical_port,
> + candidate->requested_tunnel_key);
> + continue;
> + }
> + sbrec_port_binding_set_tunnel_key(candidate->spb->sb_pb,
> + candidate->requested_tunnel_key);
> + candidate->tunnel_key_assigned = true;
> + ovs_list_push_back(paired_pbs, &candidate->spb->list_node);
> + }
> +}
> +
> +static void
> +pair_existing_tunnel_keys(struct vector *candidate_spbs,
> + struct ovs_list *paired_pbs)
> +{
> + struct candidate_spb *candidate;
> + VECTOR_FOR_EACH_PTR (candidate_spbs, candidate) {
> + if (!candidate->existing_tunnel_key ||
> + candidate->tunnel_key_assigned) {
> + continue;
> + }
> + /* Existing southbound pb. If this key is available,
> + * reuse it.
> + */
> + if (ovn_add_tnlid(&candidate->tunnel_key_map->port_tunnel_keys,
> + candidate->existing_tunnel_key)) {
> + candidate->tunnel_key_assigned = true;
> + ovs_list_push_back(paired_pbs, &candidate->spb->list_node);
> + }
> + }
> +}
> +
> +static void
> +pair_new_tunnel_keys(struct vector *candidate_spbs,
> + struct ovs_list *paired_pbs,
> + uint32_t max_pb_tunnel_id)
> +{
> + uint32_t hint = 0;
> + struct candidate_spb *candidate;
> + VECTOR_FOR_EACH_PTR (candidate_spbs, candidate) {
> + if (candidate->tunnel_key_assigned) {
> + continue;
> + }
> + uint32_t tunnel_key =
> + ovn_allocate_tnlid(&candidate->tunnel_key_map->port_tunnel_keys,
> + "port", 1, max_pb_tunnel_id,
> + &hint);
> + if (!tunnel_key) {
> + continue;
> + }
> + sbrec_port_binding_set_tunnel_key(candidate->spb->sb_pb,
> + tunnel_key);
> + candidate->tunnel_key_assigned = true;
> + ovs_list_push_back(paired_pbs, &candidate->spb->list_node);
> + }
> +}
> +
> +static void
> +free_unpaired_candidates(struct vector *candidate_spbs)
> +{
> + struct candidate_spb *candidate;
> + VECTOR_FOR_EACH_PTR (candidate_spbs, candidate) {
> + if (candidate->tunnel_key_assigned) {
> + continue;
> + }
> + sbrec_port_binding_delete(candidate->spb->sb_pb);
> + free(candidate->spb);
> + }
> +}
> +
> +static void
> +cleanup_stale_fdb_entries(const struct sbrec_fdb_table *sbrec_fdb_table,
> + struct hmap *tunnel_key_maps)
> +{
> + const struct sbrec_fdb *fdb_e;
> + SBREC_FDB_TABLE_FOR_EACH_SAFE (fdb_e, sbrec_fdb_table) {
> + bool delete = true;
> + struct tunnel_key_map *map = find_tunnel_key_map(fdb_e->dp_key,
> + tunnel_key_maps);
> + if (map &&
> + ovn_tnlid_present(&map->port_tunnel_keys, fdb_e->port_key)) {
> + delete = false;
> + }
> +
> + if (delete) {
> + sbrec_fdb_delete(fdb_e);
> + }
> + }
> +}
> +
> +enum engine_node_state
> +en_port_binding_pair_run(struct engine_node *node , void *data)
> +{
> + const struct sbrec_port_binding_table *sb_pb_table =
> + EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
> + const struct sbrec_fdb_table *sb_fdb_table =
> + EN_OVSDB_GET(engine_get_input("SB_fdb", node));
> + const struct ed_type_global_config *global_config =
> + engine_get_input_data("global_config", node);
> + const struct ovn_unpaired_port_binding_map *lsp_map =
> + engine_get_input_data("port_binding_logical_switch_port", node);
> + const struct ovn_unpaired_port_binding_map *lrp_map =
> + engine_get_input_data("port_binding_logical_router_port", node);
> + const struct ovn_unpaired_port_binding_map *crp_map =
> + engine_get_input_data("port_binding_chassisredirect_port", node);
> + const struct ovn_unpaired_port_binding_map *mp_map =
> + engine_get_input_data("port_binding_mirror", node);
> +
> + const struct ovn_unpaired_port_binding_map *input_maps[PB_MAX];
> +
> + input_maps[PB_SWITCH_PORT] = lsp_map;
> + input_maps[PB_ROUTER_PORT] = lrp_map;
> + input_maps[PB_CHASSISREDIRECT_PORT] = crp_map;
> + input_maps[PB_MIRROR_PORT] = mp_map;
> +
> + size_t num_ports = 0;
> + for (size_t i = 0; i < PB_MAX; i++) {
> + ovs_assert(input_maps[i]);
> + num_ports += shash_count(&input_maps[i]->ports);
> + }
> +
> + struct ovn_paired_port_bindings *paired_port_bindings = data;
> + reset_port_binding_pair_data(paired_port_bindings);
> +
> + struct simap visited = SIMAP_INITIALIZER(&visited);
> + struct vector candidate_spbs =
> + VECTOR_CAPACITY_INITIALIZER(struct candidate_spb, num_ports);
> + get_candidate_pbs_from_sb(sb_pb_table, input_maps,
> + &paired_port_bindings->tunnel_key_maps,
> + &candidate_spbs, &visited);
> +
> + const struct engine_context *eng_ctx = engine_get_context();
> + get_candidate_pbs_from_nb(eng_ctx->ovnsb_idl_txn, input_maps,
> + &paired_port_bindings->tunnel_key_maps,
> + &candidate_spbs, &visited);
> +
> + simap_destroy(&visited);
> +
> + pair_requested_tunnel_keys(&candidate_spbs,
> + &paired_port_bindings->paired_pbs,
> + global_config->vxlan_mode);
> + pair_existing_tunnel_keys(&candidate_spbs,
> + &paired_port_bindings->paired_pbs);
> + pair_new_tunnel_keys(&candidate_spbs, &paired_port_bindings->paired_pbs,
> + global_config->max_pb_tunnel_id);
> +
> + cleanup_stale_fdb_entries(sb_fdb_table,
> + &paired_port_bindings->tunnel_key_maps);
> +
> + free_unpaired_candidates(&candidate_spbs);
> + vector_destroy(&candidate_spbs);
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_port_binding_pair_cleanup(void *data)
> +{
> + struct ovn_paired_port_bindings *paired_port_bindings = data;
> + struct ovn_paired_port_binding *spb;
> +
> + LIST_FOR_EACH_POP (spb, list_node, &paired_port_bindings->paired_pbs) {
> + free(spb);
> + }
> + tunnel_key_maps_destroy(&paired_port_bindings->tunnel_key_maps);
> +}
> +
> +enum engine_input_handler_result
> +port_binding_fdb_change_handler(struct engine_node *node, void *data)
> +{
> + struct ovn_paired_port_bindings *paired_port_bindings = data;
> + const struct sbrec_fdb_table *sbrec_fdb_table =
> + EN_OVSDB_GET(engine_get_input("SB_fdb", node));
> +
> + /* check if changed rows are stale and delete them */
> + const struct sbrec_fdb *fdb_e, *fdb_prev_del = NULL;
> + SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb_e, sbrec_fdb_table) {
> + if (sbrec_fdb_is_deleted(fdb_e)) {
> + continue;
> + }
> +
> + if (fdb_prev_del) {
> + sbrec_fdb_delete(fdb_prev_del);
> + }
> +
> + fdb_prev_del = fdb_e;
> + struct tunnel_key_map *tunnel_key_map =
> + find_tunnel_key_map(fdb_e->dp_key,
> + &paired_port_bindings->tunnel_key_maps);
> + if (tunnel_key_map &&
> + ovn_tnlid_present(&tunnel_key_map->port_tunnel_keys,
> + fdb_e->port_key)) {
> + fdb_prev_del = NULL;
> + }
> + }
> +
> + if (fdb_prev_del) {
> + sbrec_fdb_delete(fdb_prev_del);
> + }
> +
> + return EN_HANDLED_UNCHANGED;
> +}
> diff --git a/northd/en-port-binding-pair.h b/northd/en-port-binding-pair.h
> new file mode 100644
> index 000000000..9b9417487
> --- /dev/null
> +++ b/northd/en-port-binding-pair.h
> @@ -0,0 +1,34 @@
> +/*
> + * 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
> + *
> + * 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_PORT_BINDING_PAIR_H
> +#define EN_PORT_BINDING_PAIR_H
> +
> +#include "inc-proc-eng.h"
> +
> +void *en_port_binding_pair_init(struct engine_node *node,
> + struct engine_arg *args);
> +
> +
Nit: no need for the second newline here.
> +enum engine_node_state en_port_binding_pair_run(struct engine_node *node,
> + void *data);
> +
Nit: no need for the newline here.
> +void en_port_binding_pair_cleanup(void *data);
> +
> +enum engine_input_handler_result
> +port_binding_fdb_change_handler(struct engine_node *, void *data);
> +
> +#endif /* EN_PORT_BINDING_PAIR_H */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 4627a053c..7aa4bdf3f 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -50,6 +50,11 @@
> #include "en-datapath-logical-router.h"
> #include "en-datapath-logical-switch.h"
> #include "en-datapath-sync.h"
> +#include "en-port-binding-logical-router-port.h"
> +#include "en-port-binding-logical-switch-port.h"
> +#include "en-port-binding-chassisredirect.h"
> +#include "en-port-binding-mirror.h"
> +#include "en-port-binding-pair.h"
> #include "unixctl.h"
> #include "util.h"
>
> @@ -187,6 +192,15 @@ 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);
> +static ENGINE_NODE(port_binding_logical_router_port);
> +static ENGINE_NODE(port_binding_logical_switch_port);
> +static ENGINE_NODE(port_binding_chassisredirect_port);
> +static ENGINE_NODE(port_binding_mirror);
> +static ENGINE_NODE(port_binding_paired_logical_router_port);
> +static ENGINE_NODE(port_binding_paired_logical_switch_port);
> +static ENGINE_NODE(port_binding_paired_chassisredirect_port);
> +static ENGINE_NODE(port_binding_paired_mirror);
> +static ENGINE_NODE(port_binding_pair);
>
> void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> struct ovsdb_idl_loop *sb)
> @@ -232,6 +246,36 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_datapath_synced_logical_switch, &en_datapath_sync,
> NULL);
>
> + engine_add_input(&en_port_binding_logical_switch_port,
> + &en_datapath_synced_logical_switch, NULL);
> + engine_add_input(&en_port_binding_logical_router_port,
> + &en_datapath_synced_logical_router, NULL);
> + engine_add_input(&en_port_binding_chassisredirect_port,
> + &en_datapath_synced_logical_switch, NULL);
> + engine_add_input(&en_port_binding_chassisredirect_port,
> + &en_datapath_synced_logical_router, NULL);
> + engine_add_input(&en_port_binding_mirror,
> + &en_datapath_synced_logical_switch, NULL);
> + engine_add_input(&en_port_binding_pair,
> + &en_port_binding_logical_switch_port, NULL);
> + engine_add_input(&en_port_binding_pair,
> + &en_port_binding_logical_router_port, NULL);
> + engine_add_input(&en_port_binding_pair,
> + &en_port_binding_chassisredirect_port, NULL);
> + engine_add_input(&en_port_binding_pair, &en_port_binding_mirror, NULL);
> + engine_add_input(&en_port_binding_pair, &en_sb_port_binding, NULL);
> + engine_add_input(&en_port_binding_pair, &en_global_config, NULL);
> + engine_add_input(&en_port_binding_pair, &en_sb_fdb,
> + port_binding_fdb_change_handler);
> + engine_add_input(&en_port_binding_paired_logical_router_port,
> + &en_port_binding_pair, NULL);
> + engine_add_input(&en_port_binding_paired_logical_switch_port,
> + &en_port_binding_pair, NULL);
> + engine_add_input(&en_port_binding_paired_chassisredirect_port,
> + &en_port_binding_pair, NULL);
> + engine_add_input(&en_port_binding_paired_mirror, &en_port_binding_pair,
> + 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,7 +291,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_northd, &en_sb_service_monitor, NULL);
> 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_fdb, engine_noop_handler);
Nit: I know we don't do it everywhere but in general it's nice if we
have a comment explaining why a noop_handler is fine. In this case,
AFAICT, that's because we only use the sb_fdb note to do indexed lookups
in the IDL.
> engine_add_input(&en_northd, &en_global_config,
> northd_global_config_handler);
>
> @@ -288,6 +332,14 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_noop_handler);
> engine_add_input(&en_northd, &en_datapath_synced_logical_switch,
> engine_noop_handler);
> + engine_add_input(&en_northd, &en_port_binding_paired_logical_router_port,
> + engine_noop_handler);
> + engine_add_input(&en_northd, &en_port_binding_paired_logical_switch_port,
> + engine_noop_handler);
> + engine_add_input(&en_northd,
> &en_port_binding_paired_chassisredirect_port,
> + engine_noop_handler);
> + engine_add_input(&en_northd, &en_port_binding_paired_mirror,
> + engine_noop_handler);
>
> engine_add_input(&en_lr_nat, &en_northd, lr_nat_northd_handler);
>
> diff --git a/northd/northd.c b/northd/northd.c
> index faa520ecd..93d03843c 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -54,6 +54,10 @@
> #include "en-sampling-app.h"
> #include "en-datapath-logical-switch.h"
> #include "en-datapath-logical-router.h"
> +#include "en-port-binding-logical-switch-port.h"
> +#include "en-port-binding-logical-router-port.h"
> +#include "en-port-binding-chassisredirect.h"
> +#include "en-port-binding-mirror.h"
> #include "lib/ovn-parallel-hmap.h"
> #include "ovn/actions.h"
> #include "ovn/features.h"
> @@ -464,7 +468,7 @@ od_has_lb_vip(const struct ovn_datapath *od)
> }
> }
>
> -static const char *
> +const char *
> ovn_datapath_name(const struct sbrec_datapath_binding *sb)
> {
> return smap_get_def(&sb->external_ids, "name", "");
> @@ -495,8 +499,6 @@ ovn_datapath_create(struct hmap *datapaths, const struct
> uuid *key,
> od->sb = sb;
> od->nbs = nbs;
> od->nbr = nbr;
> - hmap_init(&od->port_tnlids);
> - od->port_key_hint = 0;
> hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key));
> od->lr_group = NULL;
> hmap_init(&od->ports);
> @@ -528,7 +530,6 @@ ovn_datapath_destroy(struct hmap *datapaths, struct
> ovn_datapath *od)
> * private list and once we've exited that function it is not safe to
> * use it. */
> hmap_remove(datapaths, &od->key_node);
> - ovn_destroy_tnlids(&od->port_tnlids);
> destroy_ipam_info(&od->ipam_info);
> vector_destroy(&od->router_ports);
> vector_destroy(&od->ls_peers);
> @@ -989,6 +990,7 @@ ovn_port_create(struct hmap *ports, const char *key,
> op->sb = sb;
> ovn_port_set_nb(op, nbsp, nbrp);
> op->primary_port = op->cr_port = NULL;
> + op->tunnel_key = sb->tunnel_key;
> hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
> op->has_attached_lport_mirror = false;
>
> @@ -1001,10 +1003,6 @@ ovn_port_create(struct hmap *ports, const char *key,
> static void
> ovn_port_cleanup(struct ovn_port *port)
> {
> - if (port->tunnel_key) {
> - ovs_assert(port->od);
> - ovn_free_tnlid(&port->od->port_tnlids, port->tunnel_key);
> - }
> for (int i = 0; i < port->n_lsp_addrs; i++) {
> destroy_lport_addresses(&port->lsp_addrs[i]);
> }
> @@ -1081,12 +1079,6 @@ ovn_port_find(const struct hmap *ports, const char
> *name)
> return ovn_port_find__(ports, name, false);
> }
>
> -static struct ovn_port *
> -ovn_port_find_bound(const struct hmap *ports, const char *name)
> -{
> - return ovn_port_find__(ports, name, true);
> -}
> -
> static bool
> lsp_is_clone_to_unknown(const struct nbrec_logical_switch_port *nbsp)
> {
> @@ -1151,7 +1143,7 @@ lsp_disable_arp_nd_rsp(const struct
> nbrec_logical_switch_port *nbsp)
> return smap_get_bool( ->options, "disable_arp_nd_rsp", false);
> }
>
> -static bool
> +bool
> lsp_is_type_changed(const struct sbrec_port_binding *sb,
> const struct nbrec_logical_switch_port *nbsp,
> bool *update_sbrec)
> @@ -1887,105 +1879,35 @@ parse_lsp_addrs(struct ovn_port *op)
> }
> }
>
> -static void
> -create_mirror_port(struct ovn_port *op, struct hmap *ports,
> - struct ovs_list *both_dbs, struct ovs_list *nb_only,
> - const struct nbrec_mirror *nb_mirror)
> -{
> - char *mp_name = ovn_mirror_port_name(ovn_datapath_name(op->od->sb),
> - nb_mirror->sink);
> - struct ovn_port *mp = ovn_port_find(ports, mp_name);
> - struct ovn_port *target_port = ovn_port_find(ports, nb_mirror->sink);
> -
> - if (!target_port) {
> - goto clear;
> - }
> -
> - if (!mp) {
> - mp = ovn_port_create(ports, mp_name, op->nbsp, NULL, NULL);
> - ovs_list_push_back(nb_only, &mp->list);
> - } else if (mp->sb) {
> - ovn_port_set_nb(mp, op->nbsp, NULL);
> - ovs_list_remove(&mp->list);
> - ovs_list_push_back(both_dbs, &mp->list);
> - } else {
> - goto clear;
> - }
> -
> - mp->mirror_target_port = target_port;
> - mp->od = op->od;
> -
> - op->has_attached_lport_mirror = true;
> -clear:
> - free(mp_name);
> +static struct ovn_port *
> +create_mirror_port(struct ovn_port *source,
> + struct ovn_port *sink, const char *mirror_port_name,
> + struct hmap *ports,
> + const struct sbrec_port_binding *sb_pb)
> +{
> + struct ovn_port *mp = ovn_port_create(ports, mirror_port_name,
> + source->nbsp, NULL, sb_pb);
> + ovn_port_set_nb(mp, source->nbsp, NULL);
> + mp->mirror_target_port = sink;
> + mp->od = source->od;
> + source->has_attached_lport_mirror = true;
> + return mp;
> }
>
> static struct ovn_port *
> join_logical_ports_lsp(struct hmap *ports,
> - struct ovs_list *nb_only, struct ovs_list *both,
> struct ovn_datapath *od,
> const struct nbrec_logical_switch_port *nbsp,
> + const struct sbrec_port_binding *sb_pb,
> const char *name,
> unsigned long *queue_id_bitmap,
> - struct hmap *tag_alloc_table,
> - struct hmapx *mirror_attached_ports)
> -{
> - struct ovn_port *op = ovn_port_find_bound(ports, name);
> - if (op && (op->od || op->nbsp || op->nbrp)) {
> - static struct vlog_rate_limit rl
> - = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "duplicate logical port %s", name);
> - return NULL;
> - } else if (op && (!op->sb || op->sb->datapath == od->sb)) {
> - /*
> - * Handle cases where lport type was explicitly changed
> - * in the NBDB, in such cases:
> - * 1. remove the current sbrec of the affected lport from
> - * the port_binding table.
> - *
> - * 2. create a new sbrec with the same logical_port as the
> - * deleted lport and add it to the nb_only list which
> - * will make the northd handle this lport as a new
> - * created one and recompute everything that is needed
> - * for this lport.
> - *
> - * This change will affect container/virtual lport type
> - * changes only for now, this change is needed in
> - * contaier/virtual lport cases to avoid port type
> - * conflicts in the ovn-controller when the user clears
> - * the parent_port field in the container lport or updated
> - * the lport type.
> - *
> - */
> - bool update_sbrec = false;
> - if (op->sb && lsp_is_type_changed(op->sb, nbsp,
> - &update_sbrec)
> - && update_sbrec) {
> - ovs_list_remove(&op->list);
> - sbrec_port_binding_delete(op->sb);
> - ovn_port_destroy(ports, op);
> - op = ovn_port_create(ports, name, nbsp,
> - NULL, NULL);
> - ovs_list_push_back(nb_only, &op->list);
> - } else {
> - ovn_port_set_nb(op, nbsp, NULL);
> - ovs_list_remove(&op->list);
> -
> - uint32_t queue_id = smap_get_int(&op->sb->options,
> - "qdisc_queue_id", 0);
> - if (queue_id) {
> - bitmap_set1(queue_id_bitmap, queue_id);
> - }
> -
> - ovs_list_push_back(both, &op->list);
> -
> - /* This port exists due to a SB binding, but should
> - * not have been initialized fully. */
> - ovs_assert(!op->n_lsp_addrs && !op->n_ps_addrs);
> - }
> - } else {
> - op = ovn_port_create(ports, name, nbsp, NULL, NULL);
> - ovs_list_push_back(nb_only, &op->list);
> + struct hmap *tag_alloc_table)
> +{
> + struct ovn_port *op = ovn_port_create(ports, name, nbsp, NULL, sb_pb);
> + uint32_t queue_id = smap_get_int(&op->sb->options,
> + "qdisc_queue_id", 0);
> + if (queue_id) {
> + bitmap_set1(queue_id_bitmap, queue_id);
> }
>
> if (lsp_is_localnet(nbsp)) {
> @@ -2005,47 +1927,23 @@ join_logical_ports_lsp(struct hmap *ports,
> hmap_insert(&od->ports, &op->dp_node,
> hmap_node_hash(&op->key_node));
>
> - if (nbsp->n_mirror_rules) {
> - hmapx_add(mirror_attached_ports, op);
> - }
> -
> tag_alloc_add_existing_tags(tag_alloc_table, nbsp);
> return op;
> }
>
> static struct ovn_port*
> join_logical_ports_lrp(struct hmap *ports,
> - struct ovs_list *nb_only, struct ovs_list *both,
> struct hmapx *dgps,
> struct ovn_datapath *od,
> const struct nbrec_logical_router_port *nbrp,
> + const struct sbrec_port_binding *sb_pb,
> const char *name, struct lport_addresses
> *lrp_networks)
> {
> if (!lrp_networks->n_ipv4_addrs && !lrp_networks->n_ipv6_addrs) {
> return NULL;
> }
>
> - struct ovn_port *op = ovn_port_find_bound(ports, name);
> - if (op && (op->od || op->nbsp || op->nbrp)) {
> - static struct vlog_rate_limit rl
> - = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "duplicate logical router port %s",
> - name);
> - destroy_lport_addresses(lrp_networks);
> - return NULL;
> - } else if (op && (!op->sb || op->sb->datapath == od->sb)) {
> - ovn_port_set_nb(op, NULL, nbrp);
> - ovs_list_remove(&op->list);
> - ovs_list_push_back(both, &op->list);
> -
> - /* This port exists but should not have been
> - * initialized fully. */
> - ovs_assert(!op->lrp_networks.n_ipv4_addrs
> - && !op->lrp_networks.n_ipv6_addrs);
> - } else {
> - op = ovn_port_create(ports, name, NULL, nbrp, NULL);
> - ovs_list_push_back(nb_only, &op->list);
> - }
> + struct ovn_port *op = ovn_port_create(ports, name, NULL, nbrp, sb_pb);
>
> op->lrp_networks = *lrp_networks;
> op->od = od;
> @@ -2099,128 +1997,123 @@ join_logical_ports_lrp(struct hmap *ports,
>
>
> static struct ovn_port *
> -create_cr_port(struct ovn_port *op, struct hmap *ports,
> - struct ovs_list *both_dbs, struct ovs_list *nb_only)
> +create_cr_port(struct ovn_port *op, const char *name, struct hmap *ports,
> + const struct sbrec_port_binding *sb_pb)
> {
> - char *redirect_name = ovn_chassis_redirect_name(
> - op->nbsp ? op->nbsp->name : op->nbrp->name);
> -
> - struct ovn_port *crp = ovn_port_find(ports, redirect_name);
> - if (crp && crp->sb && crp->sb->datapath == op->od->sb) {
> - ovn_port_set_nb(crp, op->nbsp, op->nbrp);
> - ovs_list_remove(&crp->list);
> - ovs_list_push_back(both_dbs, &crp->list);
> - } else {
> - crp = ovn_port_create(ports, redirect_name,
> - op->nbsp, op->nbrp, NULL);
> - ovs_list_push_back(nb_only, &crp->list);
> - }
> + struct ovn_port *crp = ovn_port_create(ports, name, op->nbsp, op->nbrp,
> + sb_pb);
>
> crp->primary_port = op;
> op->cr_port = crp;
> crp->od = op->od;
> - free(redirect_name);
>
> return crp;
> }
>
> -/* Returns true if chassis resident port needs to be created for
> - * op's peer logical switch. False otherwise.
> - *
> - * Chassis resident port needs to be created if the following
> - * conditionsd are met:
> - * - op is a distributed gateway port
> - * - op is the only distributed gateway port attached to its
> - * router
> - * - op's peer logical switch has no localnet ports.
> - */
> -static bool
> -peer_needs_cr_port_creation(struct ovn_port *op)
> -{
> - if ((op->nbrp->n_gateway_chassis || op->nbrp->ha_chassis_group)
> - && vector_len(&op->od->l3dgw_ports) == 1 && op->peer &&
> op->peer->nbsp
> - && vector_is_empty(&op->peer->od->localnet_ports)) {
> - return true;
> - }
> -
> - return false;
> -}
> -
> static void
> -join_mirror_ports(struct ovn_port *op,
> - const struct nbrec_logical_switch_port *nbsp,
> - struct hmap *ports, struct ovs_list *both,
> - struct ovs_list *nb_only)
> +join_logical_ports(
> + struct hmap *ls_datapaths, struct hmap *lr_datapaths,
> + const struct ovn_paired_logical_switch_port_map *paired_lsps,
> + const struct ovn_paired_logical_router_port_map *paired_lrps,
> + const struct ovn_paired_chassisredirect_port_map *paired_crps,
> + const struct ovn_paired_mirror_map *paired_mirrors,
> + struct hmap *ls_ports, struct hmap *lr_ports,
> + unsigned long *queue_id_bitmap,
> + struct hmap *tag_alloc_table)
> {
> - /* Create mirror targets port bindings if there any mirror
> - * with lport type attached to this port. */
> - for (size_t j = 0; j < op->nbsp->n_mirror_rules; j++) {
> - struct nbrec_mirror *mirror = nbsp->mirror_rules[j];
> - if (!strcmp(mirror->type, "lport")) {
> - create_mirror_port(op, ports, both, nb_only, mirror);
> + struct ovn_datapath *od;
> + struct hmapx dgps = HMAPX_INITIALIZER(&dgps);
> +
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &paired_lrps->paired_router_ports) {
> + struct ovn_paired_logical_router_port *slrp = node->data;
> + od = ovn_datapath_from_sbrec(ls_datapaths, lr_datapaths,
> + slrp->router->sb);
> + if (!od) {
> + /* This can happen if the router is not enabled */
> + continue;
> }
> + struct lport_addresses lrp_networks;
> + if (!extract_lrp_networks(slrp->nb, &lrp_networks)) {
> + static struct vlog_rate_limit rl
> + = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "bad 'mac' %s", slrp->nb->mac);
> + continue;
> + }
> +
> + join_logical_ports_lrp(lr_ports, &dgps, od, slrp->nb, slrp->sb,
> + slrp->nb->name, &lrp_networks);
> }
> -}
>
> -static void
> -join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table,
> - struct hmap *ls_datapaths, struct hmap *lr_datapaths,
> - struct hmap *ports, unsigned long *queue_id_bitmap,
> - struct hmap *tag_alloc_table, 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);
> + SHASH_FOR_EACH (node, &paired_lsps->paired_switch_ports) {
> + struct ovn_paired_logical_switch_port *slsp = node->data;
> + od = ovn_datapath_from_sbrec(ls_datapaths, lr_datapaths,
> + slsp->sw->sb);
>
> - const struct sbrec_port_binding *sb;
> - SBREC_PORT_BINDING_TABLE_FOR_EACH (sb, sbrec_pb_table) {
> - struct ovn_port *op = ovn_port_create(ports, sb->logical_port,
> - NULL, NULL, sb);
> - ovs_list_push_back(sb_only, &op->list);
> + ovs_assert(od);
> + join_logical_ports_lsp(ls_ports, od, slsp->nb, slsp->sb,
> + slsp->nb->name, queue_id_bitmap,
> + tag_alloc_table);
> }
>
> - struct ovn_datapath *od;
> - struct hmapx dgps = HMAPX_INITIALIZER(&dgps);
> - struct hmapx mirror_attached_ports =
> - HMAPX_INITIALIZER(&mirror_attached_ports);
> - HMAP_FOR_EACH (od, key_node, lr_datapaths) {
> - ovs_assert(od->nbr);
> - for (size_t i = 0; i < od->nbr->n_ports; i++) {
> - const struct nbrec_logical_router_port *nbrp
> - = od->nbr->ports[i];
> -
> - struct lport_addresses lrp_networks;
> - if (!extract_lrp_networks(nbrp, &lrp_networks)) {
> - static struct vlog_rate_limit rl
> - = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "bad 'mac' %s", nbrp->mac);
> - continue;
> - }
> - join_logical_ports_lrp(ports, nb_only, both, &dgps,
> - od, nbrp,
> - nbrp->name, &lrp_networks);
> + SHASH_FOR_EACH (node, &paired_crps->paired_chassisredirect_ports) {
> + struct ovn_paired_chassisredirect_port *crp = node->data;
> + struct ovn_port *primary_port;
> + if (crp->dp_type == DP_ROUTER) {
> + primary_port = ovn_port_find(lr_ports, crp->primary_nbrp->name);
> + create_cr_port(primary_port, crp->name, lr_ports, crp->sb);
> + } else {
> + primary_port = ovn_port_find(ls_ports, crp->primary_nbsp->name);
> + create_cr_port(primary_port, crp->name, ls_ports, crp->sb);
> }
> }
>
> - HMAP_FOR_EACH (od, key_node, ls_datapaths) {
> - ovs_assert(od->nbs);
> - for (size_t i = 0; i < od->nbs->n_ports; i++) {
> - const struct nbrec_logical_switch_port *nbsp
> - = od->nbs->ports[i];
> - join_logical_ports_lsp(ports, nb_only, both, od, nbsp,
> - nbsp->name, queue_id_bitmap,
> - tag_alloc_table, &mirror_attached_ports);
> + SHASH_FOR_EACH (node, &paired_mirrors->paired_mirror_ports) {
> + struct ovn_paired_mirror *mirror = node->data;
> + struct ovn_port *source_port =
> + ovn_port_find(ls_ports, mirror->nbsp->name);
> + struct ovn_port *sink_port =
> + ovn_port_find(ls_ports, mirror->sink);
> + if (!sink_port) {
> + continue;
> }
> + create_mirror_port(source_port, sink_port, mirror->name, ls_ports,
> + mirror->sb);
> }
>
> /* Connect logical router ports, and logical switch ports of type
> "router",
> * to their peers. As well as logical switch ports of type "switch" to
> * theirs. */
> +
> struct ovn_port *op;
> - HMAP_FOR_EACH (op, key_node, ports) {
> - if (op->nbsp && lsp_is_router(op->nbsp) && !op->primary_port) {
> - struct ovn_port *peer = ovn_port_get_peer(ports, op);
> + HMAP_FOR_EACH (op, key_node, lr_ports) {
> + if (op->nbrp->peer && !is_cr_port(op)) {
> + struct ovn_port *peer = ovn_port_find(lr_ports, op->nbrp->peer);
> + if (!peer) {
> + continue;
> + }
> + if (peer->nbrp && peer->nbrp->peer &&
> + !strcmp(op->nbrp->name, peer->nbrp->peer)) {
> + /* We only configure LRP peers if each LRP has the other as
> + * its peer. */
> + op->peer = peer;
> + } else if (peer->nbsp) {
> + /* An ovn_port for a switch port of type "router" does have
> + * a router port as its peer (see the case above for
> + * "router" ports), but this is set via options:router-port
> + * in Logical_Switch_Port and does not involve the
> + * Logical_Router_Port's 'peer' column. */
> + static struct vlog_rate_limit rl =
> + VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "Bad configuration: The peer of router "
> + "port %s is a switch port", op->key);
> + }
> + }
> + }
> +
> + HMAP_FOR_EACH (op, key_node, ls_ports) {
> + if (lsp_is_router(op->nbsp) && !op->primary_port) {
> + struct ovn_port *peer = ovn_port_get_peer(lr_ports, op);
> if (!peer || !peer->nbrp) {
> continue;
> }
> @@ -2276,21 +2169,21 @@ join_logical_ports(const struct
> sbrec_port_binding_table *sbrec_pb_table,
> arp_proxy, op->nbsp->name);
> }
> }
> - } else if (op->nbsp && op->nbsp->peer && lsp_is_switch(op->nbsp)) {
> + } else if (op->nbsp->peer && lsp_is_switch(op->nbsp)) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> - struct ovn_port *peer = ovn_port_find(ports, op->nbsp->peer);
> + struct ovn_port *peer = ovn_port_find(ls_ports, op->nbsp->peer);
>
> if (!peer) {
> continue;
> }
>
> - if (peer->nbrp || (peer->nbsp && lsp_is_router(peer->nbsp))) {
> + if (lsp_is_router(peer->nbsp)) {
> VLOG_WARN_RL(&rl, "Bad configuration: The peer of switch "
> "port %s is a router port", op->key);
> continue;
> }
>
> - if (!peer->nbsp || !lsp_is_switch(peer->nbsp)) {
> + if (!lsp_is_switch(peer->nbsp)) {
> /* Common case. Likely the manual configuration is not
> * finished yet. */
> continue;
> @@ -2305,26 +2198,6 @@ join_logical_ports(const struct
> sbrec_port_binding_table *sbrec_pb_table,
> }
>
> op->peer = peer;
> - } else if (op->nbrp && op->nbrp->peer && !is_cr_port(op)) {
> - struct ovn_port *peer = ovn_port_find(ports, op->nbrp->peer);
> - if (peer) {
> - if (peer->nbrp && peer->nbrp->peer &&
> - !strcmp(op->nbrp->name, peer->nbrp->peer)) {
> - /* We only configure LRP peers if each LRP has the other
> as
> - * its peer. */
> - op->peer = peer;
> - } else if (peer->nbsp) {
> - /* An ovn_port for a switch port of type "router" does
> have
> - * a router port as its peer (see the case above for
> - * "router" ports), but this is set via
> options:router-port
> - * in Logical_Switch_Port and does not involve the
> - * Logical_Router_Port's 'peer' column. */
> - static struct vlog_rate_limit rl =
> - VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "Bad configuration: The peer of router
> "
> - "port %s is a switch port", op->key);
> - }
> - }
> }
> }
>
> @@ -2335,11 +2208,6 @@ join_logical_ports(const struct
> sbrec_port_binding_table *sbrec_pb_table,
> ovs_assert(op->nbrp);
> ovs_assert(op->nbrp->ha_chassis_group ||
> op->nbrp->n_gateway_chassis);
>
> - /* Additional "derived" ovn_port crp represents the instance of op on
> - * the gateway chassis. */
> - struct ovn_port *crp = create_cr_port(op, ports, both, nb_only);
> - ovs_assert(crp);
> -
> /* Add to l3dgw_ports in od, for later use during flow creation. */
> vector_push(&od->l3dgw_ports, &op);
>
> @@ -2350,41 +2218,16 @@ join_logical_ports(const struct
> sbrec_port_binding_table *sbrec_pb_table,
> }
> }
>
> -
> - /* Create chassisredirect port for the distributed gateway port's (DGP)
> - * peer if
> - * - DGP's router has only one DGP and
> - * - Its peer is a logical switch port and
> - * - Its peer's logical switch has no localnet ports
> - *
> - * This is required to support
> - * - NAT via geneve (for the overlay provider networks) and
> - * - to centralize routing on the gateway chassis for the traffic
> - * destined to the DGP's networks.
> - *
> - * Future enhancement: Support 'centralizerouting' for all the DGP's
> - * of a logical router.
> - * */
> - HMAPX_FOR_EACH (hmapx_node, &dgps) {
> - op = hmapx_node->data;
> - if (peer_needs_cr_port_creation(op)) {
> - create_cr_port(op->peer, ports, both, nb_only);
> - }
> - }
> hmapx_destroy(&dgps);
>
> - HMAPX_FOR_EACH (hmapx_node, &mirror_attached_ports) {
> - op = hmapx_node->data;
> - if (op && op->nbsp) {
> - join_mirror_ports(op, op->nbsp, ports, both, nb_only);
> - }
> - }
> - hmapx_destroy(&mirror_attached_ports);
> -
> /* Wait until all ports have been connected to add to IPAM since
> * it relies on proper peers to be set
> */
> - HMAP_FOR_EACH (op, key_node, ports) {
> + HMAP_FOR_EACH (op, key_node, ls_ports) {
> + ipam_add_port_addresses(op->od, op);
> + }
> +
> + HMAP_FOR_EACH (op, key_node, lr_ports) {
> ipam_add_port_addresses(op->od, op);
> }
> }
> @@ -2788,15 +2631,6 @@ copy_gw_chassis_from_nbrp_to_sbpb(
> free(sb_ha_chassis);
> }
>
> -static const char*
> -op_get_name(const struct ovn_port *op)
> -{
> - ovs_assert(op->nbsp || op->nbrp);
> - const char *name = op->nbsp ? op->nbsp->name
> - : op->nbrp->name;
> - return name;
> -}
> -
> static void
> ovn_update_ipv6_prefix(struct hmap *lr_ports)
> {
> @@ -3057,8 +2891,6 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
> const char *addresses = ds_cstr(&s);
> sbrec_port_binding_set_mac(op->sb, &addresses, 1);
> ds_destroy(&s);
> -
> - sbrec_port_binding_set_external_ids(op->sb, &op->nbrp->external_ids);
> } else {
> if (op->mirror_target_port) {
> /* In case of using a lport mirror, we establish a port binding
> @@ -3267,15 +3099,6 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
> op->sb, (const char **) op->nbsp->port_security,
> op->nbsp->n_port_security);
>
> - struct smap ids = SMAP_INITIALIZER(&ids);
> - smap_clone(&ids, &op->nbsp->external_ids);
> - const char *name = smap_get(&ids, "neutron:port_name");
> - if (name && name[0]) {
> - smap_add(&ids, "name", name);
> - }
> - sbrec_port_binding_set_external_ids(op->sb, &ids);
> - smap_destroy(&ids);
> -
> if (!op->nbsp->n_mirror_rules) {
> /* Nothing is set. Clear mirror_rules from pb. */
> sbrec_port_binding_set_mirror_rules(op->sb, NULL, 0);
> @@ -3333,27 +3156,6 @@ cleanup_sb_ha_chassis_groups(
> }
> }
>
> -static void
> -cleanup_stale_fdb_entries(const struct sbrec_fdb_table *sbrec_fdb_table,
> - struct hmap *ls_datapaths)
> -{
> - const struct sbrec_fdb *fdb_e;
> - SBREC_FDB_TABLE_FOR_EACH_SAFE (fdb_e, sbrec_fdb_table) {
> - bool delete = true;
> - struct ovn_datapath *od
> - = ovn_datapath_find_by_key(ls_datapaths, fdb_e->dp_key);
> - if (od) {
> - if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) {
> - delete = false;
> - }
> - }
> -
> - if (delete) {
> - sbrec_fdb_delete(fdb_e);
> - }
> - }
> -}
> -
> static void
> delete_fdb_entries(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
> uint32_t dp_key, uint32_t port_key)
> @@ -4130,64 +3932,6 @@ sync_pbs_for_northd_changed_ovn_ports(
> return true;
> }
>
> -static bool
> -ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key)
> -{
> - bool added = ovn_add_tnlid(&op->od->port_tnlids, tunnel_key);
> - if (added) {
> - op->tunnel_key = tunnel_key;
> - if (tunnel_key > op->od->port_key_hint) {
> - op->od->port_key_hint = tunnel_key;
> - }
> - }
> - return added;
> -}
> -
> -/* Returns false if the requested key is confict with another allocated key,
> so
> - * that the I-P engine can fallback to recompute if needed; otherwise return
> - * true (even if the key is not allocated). */
> -static bool
> -ovn_port_assign_requested_tnl_id(struct ovn_port *op)
> -{
> - const struct smap *options = (op->nbsp
> - ? &op->nbsp->options
> - : &op->nbrp->options);
> - uint32_t tunnel_key = smap_get_int(options, "requested-tnl-key", 0);
> - if (tunnel_key) {
> - if (vxlan_mode && tunnel_key >= OVN_VXLAN_MIN_MULTICAST) {
> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> - VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for port %s "
> - "is incompatible with VXLAN",
> - tunnel_key, op_get_name(op));
> - return true;
> - }
> - if (!ovn_port_add_tnlid(op, tunnel_key)) {
> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> - VLOG_WARN_RL(&rl, "Logical %s port %s requests same tunnel key "
> - "%"PRIu32" as another LSP or LRP",
> - op->nbsp ? "switch" : "router",
> - op_get_name(op), tunnel_key);
> - return false;
> - }
> - }
> - return true;
> -}
> -
> -static bool
> -ovn_port_allocate_key(struct ovn_port *op)
> -{
> - if (!op->tunnel_key) {
> - uint8_t key_bits = vxlan_mode ? 12 : 16;
> - op->tunnel_key = ovn_allocate_tnlid(&op->od->port_tnlids, "port",
> - 1, (1u << (key_bits - 1)) - 1,
> - &op->od->port_key_hint);
> - if (!op->tunnel_key) {
> - return false;
> - }
> - }
> - return true;
> -}
> -
> /* Updates the southbound Port_Binding table so that it contains the logical
> * switch ports specified by the northbound database.
> *
> @@ -4196,17 +3940,19 @@ ovn_port_allocate_key(struct ovn_port *op)
> * datapaths. */
> static void
> build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> - const struct sbrec_port_binding_table *sbrec_port_binding_table,
> const struct sbrec_mirror_table *sbrec_mirror_table,
> const struct sbrec_mac_binding_table *sbrec_mac_binding_table,
> const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table,
> 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,
> + const struct ovn_paired_logical_switch_port_map *paired_lsps,
> + const struct ovn_paired_logical_router_port_map *paired_lrps,
> + const struct ovn_paired_chassisredirect_port_map *paired_crps,
> + const struct ovn_paired_mirror_map *paired_mirrors,
> struct hmap *ls_datapaths, struct hmap *lr_datapaths,
> struct hmap *ls_ports, struct hmap *lr_ports)
> {
> - struct ovs_list sb_only, nb_only, both;
> /* XXX: Add tag_alloc_table and queue_id_bitmap as part of northd_data
> * to improve I-P. */
> struct hmap tag_alloc_table = HMAP_INITIALIZER(&tag_alloc_table);
> @@ -4217,107 +3963,37 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> struct sset active_ha_chassis_grps =
> SSET_INITIALIZER(&active_ha_chassis_grps);
>
> - /* Borrow ls_ports for joining NB and SB for both LSPs and LRPs.
> - * We will split them later. */
> - struct hmap *ports = ls_ports;
> - join_logical_ports(sbrec_port_binding_table, ls_datapaths, lr_datapaths,
> - ports, queue_id_bitmap,
> - &tag_alloc_table, &sb_only, &nb_only, &both);
> -
> - /* Purge stale Mac_Bindings if ports are deleted. */
> - bool remove_mac_bindings = !ovs_list_is_empty(&sb_only);
> + join_logical_ports(ls_datapaths, lr_datapaths,
> + paired_lsps, paired_lrps, paired_crps,
> + paired_mirrors, ls_ports, lr_ports, queue_id_bitmap,
> + &tag_alloc_table);
>
> - /* Assign explicitly requested tunnel ids first. */
> struct ovn_port *op;
> - LIST_FOR_EACH (op, list, &both) {
> - ovn_port_assign_requested_tnl_id(op);
> - }
> - LIST_FOR_EACH (op, list, &nb_only) {
> - ovn_port_assign_requested_tnl_id(op);
> - }
> -
> - /* Keep nonconflicting tunnel IDs that are already assigned. */
> - LIST_FOR_EACH (op, list, &both) {
> - if (!op->tunnel_key) {
> - ovn_port_add_tnlid(op, op->sb->tunnel_key);
> - }
> - }
> -
> - /* Assign new tunnel ids where needed. */
> - LIST_FOR_EACH_SAFE (op, list, &both) {
> - if (!ovn_port_allocate_key(op)) {
> - sbrec_port_binding_delete(op->sb);
> - ovs_list_remove(&op->list);
> - ovn_port_destroy(ports, op);
> - }
> - }
> - LIST_FOR_EACH_SAFE (op, list, &nb_only) {
> - if (!ovn_port_allocate_key(op)) {
> - ovs_list_remove(&op->list);
> - ovn_port_destroy(ports, op);
> - }
> - }
> -
> - /* For logical ports that are in both databases, update the southbound
> - * record based on northbound data.
> - * For logical ports that are in NB database, do any tag allocation
> - * needed. */
> - LIST_FOR_EACH_SAFE (op, list, &both) {
> - /* When reusing stale Port_Bindings, make sure that stale
> - * Mac_Bindings are purged.
> - */
> - if (op->od->sb != op->sb->datapath) {
> - remove_mac_bindings = true;
> - }
> - if (op->nbsp) {
> - tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp);
> - }
> + /* For logical ports, update the southbound record based on northbound
> + * data.
> + * For logical switch ports, do any tag allocation needed.
> + */
> + HMAP_FOR_EACH (op, key_node, ls_ports) {
> + tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp);
> ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
> sbrec_chassis_by_hostname,
> sbrec_ha_chassis_grp_by_name,
> sbrec_mirror_table,
> op, queue_id_bitmap,
> &active_ha_chassis_grps);
> - op->od->is_transit_router |= is_transit_router_port(op);
> - ovs_list_remove(&op->list);
> }
>
> - /* Add southbound record for each unmatched northbound record. */
> - LIST_FOR_EACH_SAFE (op, list, &nb_only) {
> - op->sb = sbrec_port_binding_insert(ovnsb_txn);
> + HMAP_FOR_EACH (op, key_node, lr_ports) {
> ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
> sbrec_chassis_by_hostname,
> sbrec_ha_chassis_grp_by_name,
> sbrec_mirror_table,
> op, queue_id_bitmap,
> &active_ha_chassis_grps);
> - sbrec_port_binding_set_logical_port(op->sb, op->key);
> op->od->is_transit_router |= is_transit_router_port(op);
> - ovs_list_remove(&op->list);
> - }
> -
> - /* Delete southbound records without northbound matches. */
> - if (!ovs_list_is_empty(&sb_only)) {
> - LIST_FOR_EACH_SAFE (op, list, &sb_only) {
> - ovs_list_remove(&op->list);
> - sbrec_port_binding_delete(op->sb);
> - ovn_port_destroy(ports, op);
> - }
> }
>
> - /* Move logical router ports to lr_ports, and logical switch ports will
> - * remain in ports/ls_ports. */
> - HMAP_FOR_EACH_SAFE (op, key_node, ports) {
> - if (!op->nbrp) {
> - continue;
> - }
> - hmap_remove(ports, &op->key_node);
> - hmap_insert(lr_ports, &op->key_node, op->key_node.hash);
> - }
> -
> - if (remove_mac_bindings) {
> - cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths,
> lr_ports);
> - }
> + cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths, lr_ports);
>
> tag_alloc_destroy(&tag_alloc_table);
> bitmap_free(queue_id_bitmap);
> @@ -4468,67 +4144,39 @@ ovn_port_find_in_datapath(struct ovn_datapath *od,
> return NULL;
> }
>
> -static bool
> +static void
> ls_port_init(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn,
> struct ovn_datapath *od,
> - const struct sbrec_port_binding *sb,
> const struct sbrec_mirror_table *sbrec_mirror_table,
> struct ovsdb_idl_index *sbrec_chassis_by_name,
> struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> {
> op->od = od;
> parse_lsp_addrs(op);
> - /* Assign explicitly requested tunnel ids first. */
> - if (!ovn_port_assign_requested_tnl_id(op)) {
> - return false;
> - }
> - /* Keep nonconflicting tunnel IDs that are already assigned. */
> - if (sb) {
> - if (!op->tunnel_key) {
> - ovn_port_add_tnlid(op, sb->tunnel_key);
> - }
> - }
> - /* Assign new tunnel ids where needed. */
> - if (!ovn_port_allocate_key(op)) {
> - return false;
> - }
> - /* Create new binding, if needed. */
> - if (sb) {
> - op->sb = sb;
> - } else {
> - /* XXX: the new SB port_binding will change in IDL, so need to handle
> - * SB port_binding updates incrementally to achieve end-to-end
> - * incremental processing. */
> - op->sb = sbrec_port_binding_insert(ovnsb_txn);
> - sbrec_port_binding_set_logical_port(op->sb, op->key);
> - }
> ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
> sbrec_chassis_by_hostname, NULL,
> sbrec_mirror_table,
> op, NULL, NULL);
> - return true;
> }
>
> static struct ovn_port *
> ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports,
> const char *key, const struct nbrec_logical_switch_port *nbsp,
> struct ovn_datapath *od,
> + const struct sbrec_port_binding *sb,
> const struct sbrec_mirror_table *sbrec_mirror_table,
> struct ovsdb_idl_index *sbrec_chassis_by_name,
> struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> {
> struct ovn_port *op = ovn_port_create(ls_ports, key, nbsp, NULL,
> - NULL);
> + sb);
> hmap_insert(&od->ports, &op->dp_node, hmap_node_hash(&op->key_node));
> - if (!ls_port_init(op, ovnsb_txn, od, NULL, sbrec_mirror_table,
> - sbrec_chassis_by_name, sbrec_chassis_by_hostname)) {
> - ovn_port_destroy(ls_ports, op);
> - return NULL;
> - }
> + ls_port_init(op, ovnsb_txn, od, sbrec_mirror_table,
> + sbrec_chassis_by_name, sbrec_chassis_by_hostname);
>
> return op;
> }
>
> -static bool
> +static void
> ls_port_reinit(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn,
> const struct nbrec_logical_switch_port *nbsp,
> struct ovn_datapath *od,
> @@ -4539,10 +4187,11 @@ ls_port_reinit(struct ovn_port *op, struct
> ovsdb_idl_txn *ovnsb_txn,
> {
> ovn_port_cleanup(op);
> op->sb = sb;
> + op->tunnel_key = sb->tunnel_key;
> ovn_port_set_nb(op, nbsp, NULL);
> op->primary_port = op->cr_port = NULL;
> - return ls_port_init(op, ovnsb_txn, od, sb, sbrec_mirror_table,
> - sbrec_chassis_by_name, sbrec_chassis_by_hostname);
> + ls_port_init(op, ovnsb_txn, od, sbrec_mirror_table,
> + sbrec_chassis_by_name, sbrec_chassis_by_hostname);
> }
>
> /* Returns true if the logical switch has changes which can be
> @@ -4559,6 +4208,7 @@ ls_changes_can_be_handled(
> {
> /* 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)) {
> if (col == NBREC_LOGICAL_SWITCH_COL_ACLS ||
> @@ -4713,15 +4363,27 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>
> /* Compare the individual ports in the old and new Logical Switches */
> for (size_t j = 0; j < changed_ls->n_ports; ++j) {
> - struct nbrec_logical_switch_port *new_nbsp = changed_ls->ports[j];
> - op = ovn_port_find_in_datapath(od, new_nbsp);
> + const struct ovn_paired_logical_switch_port *paired_lsp =
> + shash_find_data(&ni->paired_lsps->paired_switch_ports,
> + changed_ls->ports[j]->name);
> + if (!paired_lsp) {
> + /* The switch has a port that was not synced to the southbound
> + * database for some reason, likely due to tunnel key issues.
> + * As far as northd is concerned, this port does not exist and
> + * requires no processing. Just skip it.
> + */
> + continue;
> + }
> +
> + op = ovn_port_find_in_datapath(od, paired_lsp->nb);
>
> if (!op) {
> - if (!lsp_can_be_inc_processed(new_nbsp)) {
> + if (!lsp_can_be_inc_processed(paired_lsp->nb)) {
> goto fail;
> }
> op = ls_port_create(ovnsb_idl_txn, &nd->ls_ports,
> - new_nbsp->name, new_nbsp, od,
> + paired_lsp->nb->name, paired_lsp->nb, od,
> + paired_lsp->sb,
> ni->sbrec_mirror_table,
> ni->sbrec_chassis_by_name,
> ni->sbrec_chassis_by_hostname);
> @@ -4729,16 +4391,15 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> goto fail;
> }
> add_op_to_northd_tracked_ports(&trk_lsps->created, op);
> - } else if (ls_port_has_changed(new_nbsp)) {
> + } else if (ls_port_has_changed(paired_lsp->nb)) {
> /* Existing port updated */
> bool temp = false;
> - if (lsp_is_type_changed(op->sb, new_nbsp, &temp) ||
> + if (lsp_is_type_changed(op->sb, paired_lsp->nb, &temp) ||
> !op->lsp_can_be_inc_processed ||
> - !lsp_can_be_inc_processed(new_nbsp)) {
> + !lsp_can_be_inc_processed(paired_lsp->nb)) {
> goto fail;
> }
> - const struct sbrec_port_binding *sb = op->sb;
> - if (sset_contains(&nd->svc_monitor_lsps, new_nbsp->name)) {
> + if (sset_contains(&nd->svc_monitor_lsps, paired_lsp->nb->name)) {
> /* This port is used for svc monitor, which may be impacted
> * by this change. Fallback to recompute. */
> goto fail;
> @@ -4750,7 +4411,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> goto fail;
> }
> if (!check_lsp_is_up &&
> - !check_lsp_changes_other_than_up(new_nbsp)) {
> + !check_lsp_changes_other_than_up(paired_lsp->nb)) {
> /* If the only change is the "up" column while the
> * "ignore_lsp_down" is set to true, just ignore this
> * change. */
> @@ -4759,17 +4420,11 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> }
>
> uint32_t old_tunnel_key = op->tunnel_key;
> - if (!ls_port_reinit(op, ovnsb_idl_txn,
> - new_nbsp,
> - od, sb, ni->sbrec_mirror_table,
> - ni->sbrec_chassis_by_name,
> - ni->sbrec_chassis_by_hostname)) {
> - if (sb) {
> - sbrec_port_binding_delete(sb);
> - }
> - ovn_port_destroy(&nd->ls_ports, op);
> - goto fail;
> - }
> + ls_port_reinit(op, ovnsb_idl_txn,
> + paired_lsp->nb,
> + od, paired_lsp->sb, ni->sbrec_mirror_table,
> + ni->sbrec_chassis_by_name,
> + ni->sbrec_chassis_by_hostname);
> add_op_to_northd_tracked_ports(&trk_lsps->updated, op);
>
> if (old_tunnel_key != op->tunnel_key) {
> @@ -4794,7 +4449,6 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> add_op_to_northd_tracked_ports(&trk_lsps->deleted, op);
> hmap_remove(&nd->ls_ports, &op->key_node);
> hmap_remove(&od->ports, &op->dp_node);
> - sbrec_port_binding_delete(op->sb);
> delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port, od->tunnel_key,
> op->tunnel_key);
> if (op->has_attached_lport_mirror ||
> @@ -19191,13 +18845,16 @@ ovnnb_db_run(struct northd_input *input_data,
> &data->ls_datapaths, &data->lr_datapaths,
> &data->lb_datapaths_map,
> &data->lb_group_datapaths_map);
> build_ports(ovnsb_txn,
> - input_data->sbrec_port_binding_table,
> input_data->sbrec_mirror_table,
> input_data->sbrec_mac_binding_table,
> input_data->sbrec_ha_chassis_group_table,
> input_data->sbrec_chassis_by_name,
> input_data->sbrec_chassis_by_hostname,
> input_data->sbrec_ha_chassis_grp_by_name,
> + input_data->paired_lsps,
> + input_data->paired_lrps,
> + input_data->paired_crps,
> + input_data->paired_mirrors,
> &data->ls_datapaths.datapaths, &data->lr_datapaths.datapaths,
> &data->ls_ports, &data->lr_ports);
> build_lb_port_related_data(ovnsb_txn,
> @@ -19231,10 +18888,7 @@ ovnnb_db_run(struct northd_input *input_data,
> sync_template_vars(ovnsb_txn,
> input_data->nbrec_chassis_template_var_table,
> input_data->sbrec_chassis_template_var_table);
>
> - cleanup_stale_fdb_entries(input_data->sbrec_fdb_table,
> - &data->ls_datapaths.datapaths);
> stopwatch_stop(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
> -
> }
>
> /* Stores the set of chassis which references an ha_chassis_group.
> @@ -19426,6 +19080,25 @@ handle_cr_port_binding_changes(const struct
> sbrec_port_binding *sb,
> }
> }
>
> +void
> +lsp_set_up(const struct sbrec_port_binding *pb,
> + const struct nbrec_logical_switch_port *lsp)
> +{
> + bool up = false;
> +
> + if (lsp_is_router(lsp) || lsp_is_switch(lsp)) {
> + up = true;
> + } else if (pb->chassis) {
> + up = !smap_get_bool(&pb->chassis->other_config, "is-remote", false)
> + ? pb->n_up && pb->up[0]
> + : true;
> + }
> +
> + if (!lsp->up || *lsp->up != up) {
> + nbrec_logical_switch_port_set_up(lsp, &up, 1);
> + }
> +}
> +
> /* Handle changes to the 'chassis' column of the 'Port_Binding' table. When
> * this column is not empty, it means we need to set the corresponding
> logical
> * port as 'up' in the northbound DB. */
> @@ -19474,25 +19147,13 @@ handle_port_binding_changes(struct ovsdb_idl_txn
> *ovnsb_txn,
> continue;
> }
>
> - bool up = false;
> -
> - if (lsp_is_router(op->nbsp) || lsp_is_switch(op->nbsp)) {
> - up = true;
> - } else if (sb->chassis) {
> - up = !smap_get_bool(&sb->chassis->other_config, "is-remote",
> false)
> - ? sb->n_up && sb->up[0]
> - : true;
> - }
> -
> - if (!op->nbsp->up || *op->nbsp->up != up) {
> - nbrec_logical_switch_port_set_up(op->nbsp, &up, 1);
> - }
> + lsp_set_up(sb, op->nbsp);
>
> /* ovn-controller will update 'Port_Binding.up' only if it was
> * explicitly set to 'false'.
> */
> if (!op->sb->n_up) {
> - up = false;
> + bool up = false;
> sbrec_port_binding_set_up(op->sb, &up, 1);
> }
>
> diff --git a/northd/northd.h b/northd/northd.h
> index 484ea1be2..ac4499ef7 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -76,6 +76,12 @@ struct northd_input {
> const struct ovn_synced_logical_switch_map *synced_lses;
> const struct ovn_synced_logical_router_map *synced_lrs;
>
> + /* Paired port binding inputs. */
> + const struct ovn_paired_logical_switch_port_map *paired_lsps;
> + const struct ovn_paired_logical_router_port_map *paired_lrps;
> + const struct ovn_paired_chassisredirect_port_map *paired_crps;
> + const struct ovn_paired_mirror_map *paired_mirrors;
> +
> /* Indexes */
> struct ovsdb_idl_index *sbrec_chassis_by_name;
> struct ovsdb_idl_index *sbrec_chassis_by_hostname;
> @@ -378,9 +384,6 @@ struct ovn_datapath {
> /* Logical switch data. */
> struct vector router_ports; /* Vector of struct ovn_port *. */
>
> - struct hmap port_tnlids;
> - uint32_t port_key_hint;
> -
> bool has_unknown;
> bool has_vtep_lports;
> bool has_arp_proxy_port;
> @@ -829,6 +832,8 @@ void ovnsb_db_run(struct ovsdb_idl_txn *ovnnb_txn,
> const struct sbrec_ha_chassis_group_table *,
> struct hmap *ls_ports,
> struct hmap *lr_ports);
> +void lsp_set_up(const struct sbrec_port_binding *pb,
> + const struct nbrec_logical_switch_port *lsp);
> bool northd_handle_ls_changes(struct ovsdb_idl_txn *,
> const struct northd_input *,
> struct northd_data *);
> @@ -1041,4 +1046,10 @@ struct ovn_port_routable_addresses get_op_addresses(
>
> void destroy_routable_addresses(struct ovn_port_routable_addresses *ra);
>
> +bool lsp_is_type_changed(const struct sbrec_port_binding *sb,
> + const struct nbrec_logical_switch_port *nbsp,
> + bool *update_sbrec);
> +
Nit: no need for newline here.
> +const char *
> +ovn_datapath_name(const struct sbrec_datapath_binding *sb);
> #endif /* NORTHD_H */
> diff --git a/northd/port-binding-pair.c b/northd/port-binding-pair.c
> new file mode 100644
> index 000000000..a55e02920
> --- /dev/null
> +++ b/northd/port-binding-pair.c
> @@ -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
> + *
> + * 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 "port-binding-pair.h"
> +
> +struct ovn_unpaired_port_binding *
> +ovn_unpaired_port_binding_alloc(uint32_t requested_tunnel_key,
> + const char *name,
> + enum ovn_port_type type,
> + void *cookie,
> + const struct sbrec_datapath_binding *sb_dp)
> +{
> + struct ovn_unpaired_port_binding *pb = xmalloc(sizeof *pb);
> + pb->requested_tunnel_key = requested_tunnel_key;
> + pb->name = name;
> + pb->type = type;
> + pb->cookie = cookie;
> + pb->sb_dp = sb_dp;
> + smap_init(&pb->external_ids);
Nit: designated initializers.
> +
> + return pb;
> +}
> +
> +void
> +ovn_unpaired_port_binding_destroy(struct ovn_unpaired_port_binding *pb)
> +{
> + smap_destroy(&pb->external_ids);
> +}
> +
> +static bool
> +default_sb_is_valid(const struct sbrec_port_binding *sb_pb OVS_UNUSED,
> + const struct ovn_unpaired_port_binding *upb OVS_UNUSED)
> +{
> + return true;
> +}
> +
> +static struct ovn_unpaired_port_binding_map_callbacks default_callbacks = {
> + .sb_is_valid = default_sb_is_valid,
> +};
> +
> +void
> +ovn_unpaired_port_binding_map_init(
> + struct ovn_unpaired_port_binding_map *map,
> + const struct ovn_unpaired_port_binding_map_callbacks *cb)
> +{
> + shash_init(&map->ports);
> + map->cb = cb ? cb : &default_callbacks;
> +}
> +
> +void
> +ovn_unpaired_port_binding_map_destroy(
> + struct ovn_unpaired_port_binding_map *map)
> +{
> + struct ovn_unpaired_port_binding *pb;
> + struct shash_node *node;
> + SHASH_FOR_EACH_SAFE (node, &map->ports) {
> + pb = node->data;
> + shash_delete(&map->ports, node);
> + ovn_unpaired_port_binding_destroy(pb);
> + free(pb);
> + }
> + shash_destroy(&map->ports);
> +}
> diff --git a/northd/port-binding-pair.h b/northd/port-binding-pair.h
> new file mode 100644
> index 000000000..31a818da0
> --- /dev/null
> +++ b/northd/port-binding-pair.h
> @@ -0,0 +1,108 @@
> +/* 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
> + *
> + * 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 PORT_BINDING_PAIR_H
> +#define PORT_BINDING_PAIR_H 1
> +
> +#include "openvswitch/hmap.h"
> +#include "openvswitch/list.h"
> +#include "openvswitch/shash.h"
> +#include "smap.h"
> +
> +/* Port Binding pairing API. This file consists of utility functions
> + * that can be used when pairing northbound port types (e.g.
> + * Logical_Router_Port and Logical_Switch_Port) to southbound Port_Bindings.
> + *
> + * The basic flow of data is as such.
> + * 1. A northbound type is converted into an ovn_unpaired_port_binding.
> + * All ovn_unpaired_port_bindings are placed into an
> ovn_unpaired_datapath_map.
> + * 2. The en_port_binding_pair node takes all of the maps in as input and
> + * pairs them with southbound port bindings. This includes allocating
> + * tunnel keys across all ports. The output of this node is
> + * ovn_paired_port_bindings, which contains a list of all paired port
> bindings.
> + * 3. A northbound type-aware node then takes the ovn_paired_port_bindings,
> + * and decodes the generic paired port bindings back into a type-specific
> + * version (e.g. ovn_paired_logical_router_port). Later nodes can then
> consume
> + * these type-specific paired port binding types in order to perform
> + * further processing.
> + *
> + * It is important to note that this code pairs northbound ports to
> southbound
> + * port bindings, but it does not 100% sync them. The following fields are
> + * synced between the northbound port and the southbound Port_Binding:
> + * - logical_port
> + * - tunnel_key
> + * - external_ids
> + *
> + * Two later incremental engine nodes sync the rest of the fields on the Port
> + * Binding. en_northd syncs the vast majority of the data. Then finally,
> + * en_sync_to_sb syncs the nat_addresses of the Port_Binding.
> + */
> +
> +enum ovn_port_type {
> + PB_SWITCH_PORT,
> + PB_ROUTER_PORT,
> + PB_CHASSISREDIRECT_PORT,
> + PB_MIRROR_PORT,
> + PB_MAX,
> +};
> +
> +struct ovn_unpaired_port_binding {
> + uint32_t requested_tunnel_key;
> + struct smap external_ids;
> + void *cookie;
> + const char *name;
> + enum ovn_port_type type;
> + const struct sbrec_datapath_binding *sb_dp;
> +};
> +
> +struct sbrec_port_binding;
> +struct ovn_unpaired_port_binding_map_callbacks {
> + bool (*sb_is_valid)(const struct sbrec_port_binding *sp_pb,
> + const struct ovn_unpaired_port_binding *upb);
> +};
> +
> +struct ovn_unpaired_port_binding_map {
> + struct shash ports;
> + const struct ovn_unpaired_port_binding_map_callbacks *cb;
> +};
> +
> +struct sbrec_port_binding;
> +struct ovn_paired_port_binding {
> + struct ovs_list list_node;
> + const void *cookie;
> + enum ovn_port_type type;
> + const struct sbrec_port_binding *sb_pb;
> +};
> +
> +struct ovn_paired_port_bindings {
> + struct ovs_list paired_pbs;
> + struct hmap tunnel_key_maps;
> +};
> +
> +struct ovn_unpaired_port_binding *ovn_unpaired_port_binding_alloc(
> + uint32_t requested_tunnel_key, const char *name,
> + enum ovn_port_type type,
> + void *cookie,
> + const struct sbrec_datapath_binding *sb_dp);
> +
> +void ovn_unpaired_port_binding_destroy(struct ovn_unpaired_port_binding *pb);
> +
> +void ovn_unpaired_port_binding_map_init(
> + struct ovn_unpaired_port_binding_map *map,
> + const struct ovn_unpaired_port_binding_map_callbacks *cb);
> +void ovn_unpaired_port_binding_map_destroy(
> + struct ovn_unpaired_port_binding_map *map);
> +
> +#endif /* PORT_BINDING_PAIR_H */
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev