diff mbox series

[ovs-dev,RFC,v3,1/4] northd: Introduce ECMP_Nexthop table in SB db.

Message ID 6980e3633fe62568c5a4fb08492bc620da10b05d.1731342268.git.lorenzo.bianconi@redhat.com
State New
Headers show
Series Introduce ECMP_nexthop monitor in ovn-controller | 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

Lorenzo Bianconi Nov. 11, 2024, 4:30 p.m. UTC
Introduce ECMP_Nexthop table in the SB db in order to track active
ecmp-symmetric-reply connections and flush stale ones. ECMP_Nexthop
table contains ip and mac address for each active nexthop.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 northd/en-northd.c       | 29 ++++++++++++++++++++
 northd/en-northd.h       |  4 +++
 northd/inc-proc-northd.c |  9 ++++++-
 northd/northd.c          | 58 +++++++++++++++++++++++++++++++++++++++-
 northd/northd.h          |  6 +++++
 ovn-sb.ovsschema         | 17 ++++++++++--
 ovn-sb.xml               | 31 +++++++++++++++++++++
 tests/ovn-northd.at      | 33 ++++++++++++++++++-----
 8 files changed, 176 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/northd/en-northd.c b/northd/en-northd.c
index 24ed31517..165af44a0 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -404,6 +404,23 @@  en_bfd_sync_run(struct engine_node *node, void *data)
     engine_set_node_state(node, EN_UPDATED);
 }
 
+void
+en_ecmp_nexthop_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);
+    struct static_routes_data *static_routes_data =
+        engine_get_input_data("static_routes", node);
+    const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table =
+        EN_OVSDB_GET(engine_get_input("SB_ecmp_nexthop", node));
+
+    build_ecmp_nexthop_table(eng_ctx->ovnsb_idl_txn,
+                             &northd_data->lr_ports,
+                             &static_routes_data->parsed_routes,
+                             sbrec_ecmp_nexthop_table);
+    engine_set_node_state(node, EN_UPDATED);
+}
+
 void
 *en_northd_init(struct engine_node *node OVS_UNUSED,
                 struct engine_arg *arg OVS_UNUSED)
@@ -454,6 +471,13 @@  void
     return data;
 }
 
+void *
+en_ecmp_nexthop_init(struct engine_node *node OVS_UNUSED,
+                     struct engine_arg *arg OVS_UNUSED)
+{
+    return NULL;
+}
+
 void
 en_northd_cleanup(void *data)
 {
@@ -526,3 +550,8 @@  en_bfd_sync_cleanup(void *data)
 {
     bfd_sync_destroy(data);
 }
+
+void
+en_ecmp_nexthop_cleanup(void *data OVS_UNUSED)
+{
+}
diff --git a/northd/en-northd.h b/northd/en-northd.h
index 631a7c17a..2666cc67e 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -42,5 +42,9 @@  bool bfd_sync_northd_change_handler(struct engine_node *node,
                                     void *data OVS_UNUSED);
 void en_bfd_sync_run(struct engine_node *node, void *data);
 void en_bfd_sync_cleanup(void *data OVS_UNUSED);
+void en_ecmp_nexthop_run(struct engine_node *node, void *data);
+void *en_ecmp_nexthop_init(struct engine_node *node OVS_UNUSED,
+                           struct engine_arg *arg OVS_UNUSED);
+void en_ecmp_nexthop_cleanup(void *data);
 
 #endif /* EN_NORTHD_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 8c834facb..8e16fde80 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -103,7 +103,8 @@  static unixctl_cb_func chassis_features_list;
     SB_NODE(fdb, "fdb") \
     SB_NODE(static_mac_binding, "static_mac_binding") \
     SB_NODE(chassis_template_var, "chassis_template_var") \
-    SB_NODE(logical_dp_group, "logical_dp_group")
+    SB_NODE(logical_dp_group, "logical_dp_group") \
+    SB_NODE(ecmp_nexthop, "ecmp_nexthop")
 
 enum sb_engine_node {
 #define SB_NODE(NAME, NAME_STR) SB_##NAME,
@@ -162,6 +163,7 @@  static ENGINE_NODE(route_policies, "route_policies");
 static ENGINE_NODE(static_routes, "static_routes");
 static ENGINE_NODE(bfd, "bfd");
 static ENGINE_NODE(bfd_sync, "bfd_sync");
+static ENGINE_NODE(ecmp_nexthop, "ecmp_nexthop");
 
 void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                           struct ovsdb_idl_loop *sb)
@@ -264,6 +266,10 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_bfd_sync, &en_route_policies, NULL);
     engine_add_input(&en_bfd_sync, &en_northd, bfd_sync_northd_change_handler);
 
+    engine_add_input(&en_ecmp_nexthop, &en_sb_ecmp_nexthop, NULL);
+    engine_add_input(&en_ecmp_nexthop, &en_northd, NULL);
+    engine_add_input(&en_ecmp_nexthop, &en_static_routes, NULL);
+
     engine_add_input(&en_sync_meters, &en_nb_acl, NULL);
     engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
     engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
@@ -334,6 +340,7 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_sync_from_sb, &en_sb_port_binding, NULL);
     engine_add_input(&en_sync_from_sb, &en_sb_ha_chassis_group, NULL);
 
+    engine_add_input(&en_northd_output, &en_ecmp_nexthop, NULL);
     engine_add_input(&en_northd_output, &en_sync_from_sb, NULL);
     engine_add_input(&en_northd_output, &en_sync_to_sb,
                      northd_output_sync_to_sb_handler);
diff --git a/northd/northd.c b/northd/northd.c
index 832ff1758..acaa101ec 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10720,6 +10720,60 @@  build_bfd_map(const struct nbrec_bfd_table *nbrec_bfd_table,
     }
 }
 
+void
+build_ecmp_nexthop_table(
+        struct ovsdb_idl_txn *ovnsb_txn,
+        struct hmap *lr_ports, struct hmap *routes,
+        const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table)
+{
+    if (!ovnsb_txn) {
+        return;
+    }
+
+    struct sset nb_nexthops_sset = SSET_INITIALIZER(&nb_nexthops_sset);
+    struct sset sb_nexthops_sset = SSET_INITIALIZER(&sb_nexthops_sset);
+
+    const struct sbrec_ecmp_nexthop *sb_ecmp_nexthop;
+    SBREC_ECMP_NEXTHOP_TABLE_FOR_EACH (sb_ecmp_nexthop,
+                                       sbrec_ecmp_nexthop_table) {
+        sset_add(&sb_nexthops_sset, sb_ecmp_nexthop->nexthop);
+    }
+
+    struct parsed_route *pr;
+    HMAP_FOR_EACH (pr, key_node, routes) {
+        if (!pr->ecmp_symmetric_reply) {
+            continue;
+        }
+
+        if (!pr->out_port) {
+            continue;
+        }
+
+        struct ovn_port *out_port = ovn_port_find(lr_ports, pr->out_port->key);
+        if (!out_port || !out_port->sb) {
+            continue;
+        }
+
+        const struct nbrec_logical_router_static_route *r = pr->route;
+        if (!sset_contains(&sb_nexthops_sset, r->nexthop)) {
+            sb_ecmp_nexthop = sbrec_ecmp_nexthop_insert(ovnsb_txn);
+            sbrec_ecmp_nexthop_set_nexthop(sb_ecmp_nexthop, r->nexthop);
+            sbrec_ecmp_nexthop_set_port(sb_ecmp_nexthop, out_port->sb);
+        }
+        sset_add(&nb_nexthops_sset, r->nexthop);
+    }
+
+    SBREC_ECMP_NEXTHOP_TABLE_FOR_EACH_SAFE (sb_ecmp_nexthop,
+                                            sbrec_ecmp_nexthop_table) {
+        if (!sset_contains(&nb_nexthops_sset, sb_ecmp_nexthop->nexthop)) {
+            sbrec_ecmp_nexthop_delete(sb_ecmp_nexthop);
+        }
+    }
+
+    sset_destroy(&nb_nexthops_sset);
+    sset_destroy(&sb_nexthops_sset);
+}
+
 /* Returns a string of the IP address of the router port 'op' that
  * overlaps with 'ip_s".  If one is not found, returns NULL.
  *
@@ -11160,10 +11214,11 @@  parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports,
     }
 
     /* Verify that ip_prefix and nexthop are on the same network. */
+    struct ovn_port *out_port = NULL;
     if (!is_discard_route &&
         !find_static_route_outport(od, lr_ports, route,
                                    IN6_IS_ADDR_V4MAPPED(&prefix),
-                                   NULL, NULL)) {
+                                   NULL, &out_port)) {
         return;
     }
 
@@ -11206,6 +11261,7 @@  parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports,
     new_pr->hash = route_hash(new_pr);
     new_pr->route = route;
     new_pr->nbr = od->nbr;
+    new_pr->out_port = out_port;
     new_pr->ecmp_symmetric_reply = smap_get_bool(&route->options,
                                                  "ecmp_symmetric_reply",
                                                  false);
diff --git a/northd/northd.h b/northd/northd.h
index c1442ff40..e6338e60b 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -703,6 +703,7 @@  struct parsed_route {
     uint32_t route_table_id;
     uint32_t hash;
     const struct nbrec_logical_router_static_route *route;
+    struct ovn_port *out_port;
     bool ecmp_symmetric_reply;
     bool is_discard_route;
     const struct nbrec_logical_router *nbr;
@@ -746,6 +747,11 @@  void bfd_destroy(struct bfd_data *);
 void bfd_sync_init(struct bfd_sync_data *);
 void bfd_sync_destroy(struct bfd_sync_data *);
 
+void build_ecmp_nexthop_table(
+        struct ovsdb_idl_txn *ovnsb_txn,
+        struct hmap *lr_ports, struct hmap *routes,
+        const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table);
+
 struct lflow_table;
 struct lr_stateful_tracked_data;
 struct ls_stateful_tracked_data;
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index 73abf2c8d..864cb0ed6 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Southbound",
-    "version": "20.37.0",
-    "cksum": "1950136776 31493",
+    "version": "20.38.0",
+    "cksum": "466210938 32119",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -610,6 +610,19 @@ 
                                           "refTable": "Datapath_Binding"}}}},
             "indexes": [["logical_port", "ip"]],
             "isRoot": true},
+        "ECMP_Nexthop": {
+            "columns": {
+                "nexthop": {"type": "string"},
+                "port": {"type": {"key": {"type": "uuid",
+                                          "refTable": "Port_Binding",
+                                          "refType": "strong"},
+                                   "min": 0, "max": 1}},
+                "mac": {"type": "string"},
+                "external_ids": {
+                    "type": {"key": "string", "value": "string",
+                             "min": 0, "max": "unlimited"}}},
+            "indexes": [["nexthop", "port"]],
+            "isRoot": true},
         "Chassis_Template_Var": {
             "columns": {
                 "chassis": {"type": "string"},
diff --git a/ovn-sb.xml b/ovn-sb.xml
index ea4adc1c3..ea1d484a7 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -5217,4 +5217,35 @@  tcp.flags = RST;
       The set of variable values for a given chassis.
     </column>
   </table>
+
+  <table name="ECMP_Nexthop">
+    <p>
+      Each record in this table represents an active ECMP route committed by
+      <code>ovn-northd</code> to <code>ovs</code> connection-tracking table.
+      <code>ECMP_Nexthop</code> table is used by <code>ovn-controller</code>
+      to track active ct entries and to flush stale ones.
+    </p>
+    <column name="nexthop">
+      <p>
+        Nexthop IP address for this ECMP route. Nexthop IP address should
+        be the IP address of a connected router port or the IP address of
+        an external device used as nexthop for the given destination.
+      </p>
+    </column>
+    <column name="port">
+      <p>
+        The reference to <ref table="Port_Binding"/> table for the port used
+        to connect to the configured next-hop.
+      </p>
+    </column>
+    <column name="mac">
+      <p>
+        Nexthop mac address.
+      </p>
+    </column>
+
+    <column name="external_ids">
+      See <em>External IDs</em> at the beginning of this document.
+    </column>
+  </table>
 </database>
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 4e5b3b172..15959a972 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3879,7 +3879,7 @@  wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \
 check_engine_stats northd norecompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -3894,7 +3894,7 @@  wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100
 check_engine_stats northd norecompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -3906,7 +3906,7 @@  check_engine_stats northd recompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats static_routes recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -3922,7 +3922,7 @@  check_engine_stats northd recompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats static_routes recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -3934,7 +3934,7 @@  check_engine_stats northd recompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats route_policies recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -3969,7 +3969,7 @@  check_engine_stats northd recompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats static_routes recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -3984,7 +3984,7 @@  check_engine_stats northd recompute nocompute
 check_engine_stats bfd recompute nocompute
 check_engine_stats route_policies recompute nocompute
 check_engine_stats lflow recompute nocompute
-check_engine_stats northd_output norecompute compute
+check_engine_stats northd_output recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
@@ -6809,6 +6809,10 @@  check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
 
 # ECMP flows will be added even if there is only one next-hop.
 check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.10
+check_row_count ECMP_Nexthop 1
+uuid=$(fetch_column Port_Binding _uuid logical_port=lr0-public)
+check_column 192.168.0.10 ECMP_Nexthop nexthop
+check_column "$uuid" ECMP_Nexthop port
 
 ovn-sbctl dump-flows lr0 > lr0flows
 
@@ -6828,6 +6832,13 @@  AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | ovn_strip_lflows], [0], [dn
 ])
 
 check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.20
+check_row_count ECMP_Nexthop 2
+AT_CHECK([ovn-sbctl --columns nexthop --bare find ECMP_Nexthop nexthop=192.168.0.10], [0], [dnl
+192.168.0.10
+])
+AT_CHECK([ovn-sbctl --columns nexthop --bare find ECMP_Nexthop nexthop=192.168.0.20], [0], [dnl
+192.168.0.20
+])
 
 ovn-sbctl dump-flows lr0 > lr0flows
 AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], [dnl
@@ -6857,6 +6868,13 @@  AT_CHECK([grep -e "lr_in_arp_resolve.*ecmp" lr0flows | ovn_strip_lflows], [0], [
 
 # add ecmp route with wrong nexthop
 check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.1.20
+check_row_count ECMP_Nexthop 2
+AT_CHECK([ovn-sbctl --columns nexthop --bare find ECMP_Nexthop nexthop=192.168.0.10], [0], [dnl
+192.168.0.10
+])
+AT_CHECK([ovn-sbctl --columns nexthop --bare find ECMP_Nexthop nexthop=192.168.0.20], [0], [dnl
+192.168.0.20
+])
 
 ovn-sbctl dump-flows lr0 > lr0flows
 AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], [dnl
@@ -6876,6 +6894,7 @@  AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192.
 
 check ovn-nbctl lr-route-del lr0
 wait_row_count nb:Logical_Router_Static_Route 0
+check_row_count ECMP_Nexthop 0
 
 check ovn-nbctl --wait=sb lr-route-add lr0 1.0.0.0/24 192.168.0.10
 ovn-sbctl dump-flows lr0 > lr0flows