diff mbox series

[ovs-dev,v1,7/8] northd: Sync SB Port bindings NAT column in a separate engine node.

Message ID 20230624064858.12198-1-numans@ovn.org
State Superseded
Headers show
Series northd: I-P for load balancer and lb groups | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Numan Siddique June 24, 2023, 6:48 a.m. UTC
From: Numan Siddique <numans@ovn.org>

A new engine node 'sync_to_sb_pb' is added within 'sync_to_sb'
node to sync NAT column of Port bindings table.  This separation
is required in order to add load balancer group I-P handling
in 'northd' engine node (which is handled in the next commit).

'sync_to_sb_pb' engine node can be later expanded to sync other
Port binding columns if required.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/en-sync-sb.c      |  31 +++++
 northd/en-sync-sb.h      |   4 +
 northd/inc-proc-northd.c |   8 +-
 northd/northd.c          | 243 +++++++++++++++++++++------------------
 northd/northd.h          |   2 +
 5 files changed, 174 insertions(+), 114 deletions(-)
diff mbox series

Patch

diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index fda0ca5a68..7c8be49209 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -240,6 +240,37 @@  en_sync_to_sb_lb_cleanup(void *data OVS_UNUSED)
 
 }
 
+void *
+en_sync_to_sb_pb_init(struct engine_node *node OVS_UNUSED,
+                      struct engine_arg *arg OVS_UNUSED)
+{
+    return NULL;
+}
+
+void
+en_sync_to_sb_pb_run(struct engine_node *node, void *data OVS_UNUSED)
+{
+    const struct engine_context *eng_ctx = engine_get_context();
+    struct northd_data *northd_data = engine_get_input_data("northd", node);
+
+    sync_pbs(eng_ctx->ovnsb_idl_txn, &northd_data->ls_ports);
+    engine_set_node_state(node, EN_UPDATED);
+}
+
+void
+en_sync_to_sb_pb_cleanup(void *data OVS_UNUSED)
+{
+
+}
+
+/* sync_to_sb_pb engine node functions.
+ * This engine node syncs the SB Port Bindings (partly)
+ * en_northd engine create the SB Port binding rows and
+ * updates most of the columns.
+ * This engine node updates the port binding columns which
+ * needs to be updated after northd engine is run.
+ */
+
 /* static functions. */
 static void
 sync_addr_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name,
diff --git a/northd/en-sync-sb.h b/northd/en-sync-sb.h
index b3bfb5454f..f1008915bd 100644
--- a/northd/en-sync-sb.h
+++ b/northd/en-sync-sb.h
@@ -21,4 +21,8 @@  void *en_sync_to_sb_lb_init(struct engine_node *, struct engine_arg *);
 void en_sync_to_sb_lb_run(struct engine_node *, void *data);
 void en_sync_to_sb_lb_cleanup(void *data);
 
+void *en_sync_to_sb_pb_init(struct engine_node *, struct engine_arg *);
+void en_sync_to_sb_pb_run(struct engine_node *, void *data);
+void en_sync_to_sb_pb_cleanup(void *data);
+
 #endif /* end of EN_SYNC_SB_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 546799b9e1..8f509ccdad 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -141,6 +141,7 @@  static ENGINE_NODE(sync_to_sb_addr_set, "sync_to_sb_addr_set");
 static ENGINE_NODE(fdb_aging, "fdb_aging");
 static ENGINE_NODE(fdb_aging_waker, "fdb_aging_waker");
 static ENGINE_NODE(sync_to_sb_lb, "sync_to_sb_lb");
+static ENGINE_NODE(sync_to_sb_pb, "sync_to_sb_pb");
 static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(northd_lb_data, "northd_lb_data");
 
 void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
@@ -212,13 +213,16 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_sync_to_sb_lb, &en_northd, NULL);
     engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer, NULL);
 
+    engine_add_input(&en_sync_to_sb_pb, &en_northd, NULL);
+
     /* en_sync_to_sb engine node syncs the SB database tables from
      * the NB database tables.
-     * Right now this engine syncs the SB Address_Set table and
-     * SB Load_Balancer table.
+     * Right now this engine syncs the SB Address_Set table,
+     * SB Load_Balancer table and (partly) SB Port_Binding table.
      */
     engine_add_input(&en_sync_to_sb, &en_sync_to_sb_addr_set, NULL);
     engine_add_input(&en_sync_to_sb, &en_sync_to_sb_lb, NULL);
+    engine_add_input(&en_sync_to_sb, &en_sync_to_sb_pb, NULL);
 
     engine_add_input(&en_sync_from_sb, &en_northd,
                      sync_from_sb_northd_handler);
diff --git a/northd/northd.c b/northd/northd.c
index 3183057bd9..f8adcad763 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3472,8 +3472,6 @@  ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
         ds_destroy(&s);
 
         sbrec_port_binding_set_external_ids(op->sb, &op->nbrp->external_ids);
-
-        sbrec_port_binding_set_nat_addresses(op->sb, NULL, 0);
     } else {
         if (!lsp_is_router(op->nbsp)) {
             uint32_t queue_id = smap_get_int(
@@ -3589,116 +3587,6 @@  ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
             } else {
                 sbrec_port_binding_set_options(op->sb, NULL);
             }
-            const char *nat_addresses = smap_get(&op->nbsp->options,
-                                           "nat-addresses");
-            size_t n_nats = 0;
-            char **nats = NULL;
-            bool l3dgw_ports = op->peer && op->peer->od &&
-                               op->peer->od->n_l3dgw_ports;
-            if (nat_addresses && !strcmp(nat_addresses, "router")) {
-                if (op->peer && op->peer->od
-                    && (chassis || op->peer->od->n_l3dgw_ports)) {
-                    bool exclude_lb_vips = smap_get_bool(&op->nbsp->options,
-                            "exclude-lb-vips-from-garp", false);
-                    nats = get_nat_addresses(op->peer, &n_nats, false,
-                                             !exclude_lb_vips);
-                }
-            } else if (nat_addresses && (chassis || l3dgw_ports)) {
-                struct lport_addresses laddrs;
-                if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
-                    static struct vlog_rate_limit rl =
-                        VLOG_RATE_LIMIT_INIT(1, 1);
-                    VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
-                } else {
-                    destroy_lport_addresses(&laddrs);
-                    n_nats = 1;
-                    nats = xcalloc(1, sizeof *nats);
-                    struct ds nat_addr = DS_EMPTY_INITIALIZER;
-                    ds_put_format(&nat_addr, "%s", nat_addresses);
-                    if (l3dgw_ports) {
-                        const struct ovn_port *l3dgw_port = (
-                            is_l3dgw_port(op->peer)
-                            ? op->peer
-                            : op->peer->od->l3dgw_ports[0]);
-                        ds_put_format(&nat_addr, " is_chassis_resident(%s)",
-                            l3dgw_port->cr_port->json_key);
-                    }
-                    nats[0] = xstrdup(ds_cstr(&nat_addr));
-                    ds_destroy(&nat_addr);
-                }
-            }
-
-            /* Add the router mac and IPv4 addresses to
-             * Port_Binding.nat_addresses so that GARP is sent for these
-             * IPs by the ovn-controller on which the distributed gateway
-             * router port resides if:
-             *
-             * -  op->peer has 'reside-on-redirect-chassis' set and the
-             *    the logical router datapath has distributed router port.
-             *
-             * -  op->peer is distributed gateway router port.
-             *
-             * -  op->peer's router is a gateway router and op has a localnet
-             *    port.
-             *
-             * Note: Port_Binding.nat_addresses column is also used for
-             * sending the GARPs for the router port IPs.
-             * */
-            bool add_router_port_garp = false;
-            if (op->peer && op->peer->nbrp && op->peer->od->n_l3dgw_ports) {
-                if (is_l3dgw_port(op->peer)) {
-                    add_router_port_garp = true;
-                } else if (smap_get_bool(&op->peer->nbrp->options,
-                               "reside-on-redirect-chassis", false)) {
-                    if (op->peer->od->n_l3dgw_ports == 1) {
-                        add_router_port_garp = true;
-                    } else {
-                        static struct vlog_rate_limit rl =
-                            VLOG_RATE_LIMIT_INIT(1, 1);
-                        VLOG_WARN_RL(&rl, "\"reside-on-redirect-chassis\" is "
-                                     "set on logical router port %s, which "
-                                     "is on logical router %s, which has %"
-                                     PRIuSIZE" distributed gateway ports. This"
-                                     "option can only be used when there is "
-                                     "a single distributed gateway port.",
-                                     op->peer->key, op->peer->od->nbr->name,
-                                     op->peer->od->n_l3dgw_ports);
-                    }
-                }
-            } else if (chassis && op->od->n_localnet_ports) {
-                add_router_port_garp = true;
-            }
-
-            if (add_router_port_garp) {
-                struct ds garp_info = DS_EMPTY_INITIALIZER;
-                ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);
-
-                for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
-                     i++) {
-                    ds_put_format(&garp_info, " %s",
-                                  op->peer->lrp_networks.ipv4_addrs[i].addr_s);
-                }
-
-                if (op->peer->od->n_l3dgw_ports) {
-                    const struct ovn_port *l3dgw_port = (
-                        is_l3dgw_port(op->peer)
-                        ? op->peer
-                        : op->peer->od->l3dgw_ports[0]);
-                    ds_put_format(&garp_info, " is_chassis_resident(%s)",
-                                  l3dgw_port->cr_port->json_key);
-                }
-
-                n_nats++;
-                nats = xrealloc(nats, (n_nats * sizeof *nats));
-                nats[n_nats - 1] = ds_steal_cstr(&garp_info);
-                ds_destroy(&garp_info);
-            }
-            sbrec_port_binding_set_nat_addresses(op->sb,
-                                                 (const char **) nats, n_nats);
-            for (size_t i = 0; i < n_nats; i++) {
-                free(nats[i]);
-            }
-            free(nats);
         }
 
         sbrec_port_binding_set_parent_port(op->sb, op->nbsp->parent_name);
@@ -4692,6 +4580,137 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
     }
 }
 
+/* Sync the SB Port bindings which needs to be updated.
+ * Presently it syncs the nat column of port bindings corresponding to
+ * the logical switch ports. */
+void sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports)
+{
+    ovs_assert(ovnsb_idl_txn);
+
+    struct ovn_port *op;
+    HMAP_FOR_EACH (op, key_node, ls_ports) {
+        if (lsp_is_router(op->nbsp)) {
+            const char *chassis = NULL;
+            if (op->peer && op->peer->od && op->peer->od->nbr) {
+                chassis = smap_get(&op->peer->od->nbr->options, "chassis");
+            }
+
+            const char *nat_addresses = smap_get(&op->nbsp->options,
+                                                 "nat-addresses");
+            size_t n_nats = 0;
+            char **nats = NULL;
+            bool l3dgw_ports = op->peer && op->peer->od &&
+                               op->peer->od->n_l3dgw_ports;
+            if (nat_addresses && !strcmp(nat_addresses, "router")) {
+                if (op->peer && op->peer->od
+                    && (chassis || op->peer->od->n_l3dgw_ports)) {
+                    bool exclude_lb_vips = smap_get_bool(&op->nbsp->options,
+                            "exclude-lb-vips-from-garp", false);
+                    nats = get_nat_addresses(op->peer, &n_nats, false,
+                                             !exclude_lb_vips);
+                }
+            } else if (nat_addresses && (chassis || l3dgw_ports)) {
+                struct lport_addresses laddrs;
+                if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
+                    static struct vlog_rate_limit rl =
+                        VLOG_RATE_LIMIT_INIT(1, 1);
+                    VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
+                } else {
+                    destroy_lport_addresses(&laddrs);
+                    n_nats = 1;
+                    nats = xcalloc(1, sizeof *nats);
+                    struct ds nat_addr = DS_EMPTY_INITIALIZER;
+                    ds_put_format(&nat_addr, "%s", nat_addresses);
+                    if (l3dgw_ports) {
+                        const struct ovn_port *l3dgw_port = (
+                            is_l3dgw_port(op->peer)
+                            ? op->peer
+                            : op->peer->od->l3dgw_ports[0]);
+                        ds_put_format(&nat_addr, " is_chassis_resident(%s)",
+                            l3dgw_port->cr_port->json_key);
+                    }
+                    nats[0] = xstrdup(ds_cstr(&nat_addr));
+                    ds_destroy(&nat_addr);
+                }
+            }
+
+            /* Add the router mac and IPv4 addresses to
+             * Port_Binding.nat_addresses so that GARP is sent for these
+             * IPs by the ovn-controller on which the distributed gateway
+             * router port resides if:
+             *
+             * -  op->peer has 'reside-on-redirect-chassis' set and the
+             *    the logical router datapath has distributed router port.
+             *
+             * -  op->peer is distributed gateway router port.
+             *
+             * -  op->peer's router is a gateway router and op has a localnet
+             *    port.
+             *
+             * Note: Port_Binding.nat_addresses column is also used for
+             * sending the GARPs for the router port IPs.
+             * */
+            bool add_router_port_garp = false;
+            if (op->peer && op->peer->nbrp && op->peer->od->n_l3dgw_ports) {
+                if (is_l3dgw_port(op->peer)) {
+                    add_router_port_garp = true;
+                } else if (smap_get_bool(&op->peer->nbrp->options,
+                               "reside-on-redirect-chassis", false)) {
+                    if (op->peer->od->n_l3dgw_ports == 1) {
+                        add_router_port_garp = true;
+                    } else {
+                        static struct vlog_rate_limit rl =
+                            VLOG_RATE_LIMIT_INIT(1, 1);
+                        VLOG_WARN_RL(&rl, "\"reside-on-redirect-chassis\" is "
+                                     "set on logical router port %s, which "
+                                     "is on logical router %s, which has %"
+                                     PRIuSIZE" distributed gateway ports. This"
+                                     "option can only be used when there is "
+                                     "a single distributed gateway port.",
+                                     op->peer->key, op->peer->od->nbr->name,
+                                     op->peer->od->n_l3dgw_ports);
+                    }
+                }
+            } else if (chassis && op->od->n_localnet_ports) {
+                add_router_port_garp = true;
+            }
+
+            if (add_router_port_garp) {
+                struct ds garp_info = DS_EMPTY_INITIALIZER;
+                ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);
+
+                for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
+                     i++) {
+                    ds_put_format(&garp_info, " %s",
+                                  op->peer->lrp_networks.ipv4_addrs[i].addr_s);
+                }
+
+                if (op->peer->od->n_l3dgw_ports) {
+                    const struct ovn_port *l3dgw_port = (
+                        is_l3dgw_port(op->peer)
+                        ? op->peer
+                        : op->peer->od->l3dgw_ports[0]);
+                    ds_put_format(&garp_info, " is_chassis_resident(%s)",
+                                  l3dgw_port->cr_port->json_key);
+                }
+
+                n_nats++;
+                nats = xrealloc(nats, (n_nats * sizeof *nats));
+                nats[n_nats - 1] = ds_steal_cstr(&garp_info);
+                ds_destroy(&garp_info);
+            }
+            sbrec_port_binding_set_nat_addresses(op->sb,
+                                                 (const char **) nats, n_nats);
+            for (size_t i = 0; i < n_nats; i++) {
+                free(nats[i]);
+            }
+            free(nats);
+        } else {
+            sbrec_port_binding_set_nat_addresses(op->sb, NULL, 0);
+        }
+    }
+}
+
 static bool
 ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key)
 {
diff --git a/northd/northd.h b/northd/northd.h
index 3afa4eaff2..a79cc470c3 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -377,4 +377,6 @@  const char *northd_get_svc_monitor_mac(void);
 void sync_lbs(struct ovsdb_idl_txn *, const struct sbrec_load_balancer_table *,
               struct ovn_datapaths *ls_datapaths, struct hmap *lbs);
 
+void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports);
+
 #endif /* NORTHD_H */