diff mbox series

[ovs-dev,v2,1/2] northd: sync lb applied to logical routers in sb db lb table

Message ID d0ac298c9092841f4f97e34f3af637c28add6e2e.1689348133.git.lorenzo.bianconi@redhat.com
State Superseded
Headers show
Series sync lb applied to logical routers in sb db lb table | 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 July 14, 2023, 3:25 p.m. UTC
Introduce lr_datapath_group column in the load_balancer table of the SB
db.
Sync load_balancers applied to logical_routers to Load_Balancer table in
the SouthBound database.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2193323
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/local_data.c     |   8 +++
 controller/ovn-controller.c |   6 ++
 lib/lb.h                    |   3 +-
 northd/northd.c             |  81 ++++++++++++++++++-------
 ovn-sb.ovsschema            |   6 +-
 ovn-sb.xml                  |   6 ++
 tests/ovn-northd.at         |  17 +++++-
 tests/system-ovn.at         | 117 ++++++++++++++++++++++++++++++++++++
 8 files changed, 220 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/controller/local_data.c b/controller/local_data.c
index cf0b21bb1..01cfbdefe 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -670,5 +670,13 @@  lb_is_local(const struct sbrec_load_balancer *sbrec_lb,
         }
     }
 
+    dp_group = sbrec_lb->lr_datapath_group;
+    for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
+        if (get_local_datapath(local_datapaths,
+                               dp_group->datapaths[i]->tunnel_key)) {
+            return true;
+        }
+    }
+
     return false;
 }
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 96d01219e..55ad8e66f 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2717,6 +2717,12 @@  load_balancers_by_dp_init(const struct hmap *local_datapaths,
                                          lb->datapath_group->datapaths[i],
                                          lb, lbs);
         }
+        for (size_t i = 0; lb->lr_datapath_group
+                           && i < lb->lr_datapath_group->n_datapaths; i++) {
+            load_balancers_by_dp_add_one(local_datapaths,
+                                         lb->lr_datapath_group->datapaths[i],
+                                         lb, lbs);
+        }
     }
     return lbs;
 }
diff --git a/lib/lb.h b/lib/lb.h
index 23d8fc9e9..2456423cc 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -85,7 +85,8 @@  struct ovn_northd_lb {
     size_t n_nb_lr;
     unsigned long *nb_lr_map;
 
-    struct ovn_dp_group *dpg;
+    struct ovn_dp_group *ls_dpg;
+    struct ovn_dp_group *lr_dpg;
 };
 
 struct ovn_lb_vip {
diff --git a/northd/northd.c b/northd/northd.c
index b9605862e..443e61948 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4541,10 +4541,11 @@  ovn_dp_group_get_or_create(struct ovsdb_idl_txn *ovnsb_txn,
 static void
 sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
          const struct sbrec_load_balancer_table *sbrec_load_balancer_table,
-         struct ovn_datapaths *ls_datapaths, struct hmap *lbs)
+         struct ovn_datapaths *ls_datapaths,
+         struct ovn_datapaths *lr_datapaths, struct hmap *lbs)
 {
-    struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups);
-    size_t bitmap_len = ods_size(ls_datapaths);
+    struct hmap ls_dp_groups = HMAP_INITIALIZER(&ls_dp_groups);
+    struct hmap lr_dp_groups = HMAP_INITIALIZER(&lr_dp_groups);
     struct ovn_northd_lb *lb;
 
     /* Delete any stale SB load balancer rows and create datapath
@@ -4561,7 +4562,8 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
         }
 
         /* Delete any SB load balancer entries that refer to NB load balancers
-         * that don't exist anymore or are not applied to switches anymore.
+         * that don't exist anymore or are not applied to switches/routers
+         * anymore.
          *
          * There is also a special case in which duplicate LBs might be created
          * in the SB, e.g., due to the fact that OVSDB only ensures
@@ -4569,7 +4571,8 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
          * are not indexed in any way.
          */
         lb = ovn_northd_lb_find(lbs, &lb_uuid);
-        if (!lb || !lb->n_nb_ls || !hmapx_add(&existing_lbs, lb)) {
+        if (!lb || (!lb->n_nb_ls && !lb->n_nb_lr) ||
+            !hmapx_add(&existing_lbs, lb)) {
             sbrec_load_balancer_delete(sbrec_lb);
             continue;
         }
@@ -4577,11 +4580,20 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
         lb->slb = sbrec_lb;
 
         /* Find or create datapath group for this load balancer. */
-        lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, &dp_groups,
-                                             lb->slb->datapath_group,
-                                             lb->n_nb_ls, lb->nb_ls_map,
-                                             bitmap_len, true,
-                                             ls_datapaths, NULL);
+        if (lb->n_nb_ls) {
+            lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &ls_dp_groups,
+                                                    lb->slb->datapath_group,
+                                                    lb->n_nb_ls, lb->nb_ls_map,
+                                                    ods_size(ls_datapaths),
+                                                    true, ls_datapaths, NULL);
+        }
+        if (lb->n_nb_lr) {
+            lb->lr_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &lr_dp_groups,
+                                                    lb->slb->lr_datapath_group,
+                                                    lb->n_nb_lr, lb->nb_lr_map,
+                                                    ods_size(lr_datapaths),
+                                                    false, NULL, lr_datapaths);
+        }
     }
     hmapx_destroy(&existing_lbs);
 
@@ -4589,7 +4601,7 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
      * the SB load balancer columns. */
     HMAP_FOR_EACH (lb, hmap_node, lbs) {
 
-        if (!lb->n_nb_ls) {
+        if (!lb->n_nb_ls && !lb->n_nb_lr) {
             continue;
         }
 
@@ -4612,19 +4624,33 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
         }
 
         /* Find or create datapath group for this load balancer. */
-        if (!lb->dpg) {
-            lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, &dp_groups,
-                                                 lb->slb->datapath_group,
-                                                 lb->n_nb_ls, lb->nb_ls_map,
-                                                 bitmap_len, true,
-                                                 ls_datapaths, NULL);
+        if (!lb->ls_dpg && lb->n_nb_ls) {
+            lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &ls_dp_groups,
+                                                    lb->slb->datapath_group,
+                                                    lb->n_nb_ls, lb->nb_ls_map,
+                                                    ods_size(ls_datapaths),
+                                                    true, ls_datapaths, NULL);
+        }
+        if (!lb->lr_dpg && lb->n_nb_lr) {
+            lb->lr_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &lr_dp_groups,
+                                                    lb->slb->lr_datapath_group,
+                                                    lb->n_nb_lr, lb->nb_lr_map,
+                                                    ods_size(lr_datapaths),
+                                                    false, NULL, lr_datapaths);
         }
 
         /* Update columns. */
         sbrec_load_balancer_set_name(lb->slb, lb->nlb->name);
         sbrec_load_balancer_set_vips(lb->slb, ovn_northd_lb_get_vips(lb));
         sbrec_load_balancer_set_protocol(lb->slb, lb->nlb->protocol);
-        sbrec_load_balancer_set_datapath_group(lb->slb, lb->dpg->dp_group);
+        if (lb->ls_dpg) {
+            sbrec_load_balancer_set_datapath_group(lb->slb,
+                                                   lb->ls_dpg->dp_group);
+        }
+        if (lb->lr_dpg) {
+            sbrec_load_balancer_set_lr_datapath_group(lb->slb,
+                                                      lb->lr_dpg->dp_group);
+        }
         sbrec_load_balancer_set_options(lb->slb, &options);
         /* Clearing 'datapaths' column, since 'dp_group' is in use. */
         sbrec_load_balancer_set_datapaths(lb->slb, NULL, 0);
@@ -4632,11 +4658,17 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
     }
 
     struct ovn_dp_group *dpg;
-    HMAP_FOR_EACH_POP (dpg, node, &dp_groups) {
+    HMAP_FOR_EACH_POP (dpg, node, &ls_dp_groups) {
+        bitmap_free(dpg->bitmap);
+        free(dpg);
+    }
+    hmap_destroy(&ls_dp_groups);
+
+    HMAP_FOR_EACH_POP (dpg, node, &lr_dp_groups) {
         bitmap_free(dpg->bitmap);
         free(dpg);
     }
-    hmap_destroy(&dp_groups);
+    hmap_destroy(&lr_dp_groups);
 
     /* Datapath_Binding.load_balancers is not used anymore, it's still in the
      * schema for compatibility reasons.  Reset it to empty, just in case.
@@ -4645,6 +4677,13 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
     HMAP_FOR_EACH (od, key_node, &ls_datapaths->datapaths) {
         ovs_assert(od->nbs);
 
+        if (od->sb->n_load_balancers) {
+            sbrec_datapath_binding_set_load_balancers(od->sb, NULL, 0);
+        }
+    }
+    HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
+        ovs_assert(od->nbr);
+
         if (od->sb->n_load_balancers) {
             sbrec_datapath_binding_set_load_balancers(od->sb, NULL, 0);
         }
@@ -17581,7 +17620,7 @@  ovnnb_db_run(struct northd_input *input_data,
     ovn_update_ipv6_prefix(&data->lr_ports);
 
     sync_lbs(ovnsb_txn, input_data->sbrec_load_balancer_table,
-             &data->ls_datapaths, &data->lbs);
+             &data->ls_datapaths, &data->lr_datapaths, &data->lbs);
     sync_port_groups(ovnsb_txn, input_data->sbrec_port_group_table,
                      &data->port_groups);
     sync_meters(ovnsb_txn, input_data->nbrec_meter_table,
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index 7b20aa21b..a66db998f 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Southbound",
     "version": "20.27.4",
-    "cksum": "3194181501 30531",
+    "cksum": "146375056 30745",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -534,6 +534,10 @@ 
                     {"type": {"key": {"type": "uuid",
                                       "refTable": "Logical_DP_Group"},
                               "min": 0, "max": 1}},
+                "lr_datapath_group":
+                    {"type": {"key": {"type": "uuid",
+                                      "refTable": "Logical_DP_Group"},
+                              "min": 0, "max": 1}},
                 "options": {
                      "type": {"key": "string",
                               "value": "string",
diff --git a/ovn-sb.xml b/ovn-sb.xml
index f9dd13602..f9b7d309d 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4884,6 +4884,12 @@  tcp.flags = RST;
       means that the same load balancer applies to all datapaths in a group.
     </column>
 
+    <column name="lr_datapath_group">
+      The group of logical router datapaths to which this load balancer
+      applies to. This means that the same load balancer applies to all
+      datapaths in a group.
+    </column>
+
     <group title="Load_Balancer options">
     <column name="options" key="hairpin_snat_ip">
       IP to be used as source IP for packets that have been hair-pinned after
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 3e06f14c9..dcf6aa794 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2836,6 +2836,15 @@  sb:load_balancer vips,protocol name=lbg0
 
 check ovn-nbctl lr-add lr0 -- add logical_router lr0 load_balancer_group $lbg
 check ovn-nbctl --wait=sb lr-lb-add lr0 lb0
+check_row_count sb:load_balancer 2
+
+lr0_sb_uuid=$(fetch_column datapath_binding _uuid external_ids:name=lr0)
+lb0_lr_dp_group=$(fetch_column sb:load_balancer lr_datapath_group name=lb0)
+
+AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find Logical_DP_Group dnl
+                    | grep -A1 $lb0_lr_dp_group | tail -1], [0], [dnl
+$lr0_sb_uuid
+])
 
 echo
 echo "__file__:__line__: Check that SB lb0 has only sw0 in datapaths column."
@@ -2881,7 +2890,13 @@  check_row_count sb:load_balancer 2
 lbg1=$(fetch_column nb:load_balancer _uuid name=lbg1)
 check ovn-nbctl add load_balancer_group $lbg load_balancer $lbg1
 check ovn-nbctl --wait=sb lr-lb-add lr0 lb1
-check_row_count sb:load_balancer 3
+check_row_count sb:load_balancer 4
+
+lb1_lr_dp_group=$(fetch_column sb:load_balancer lr_datapath_group name=lb1)
+AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find Logical_DP_Group dnl
+                    | grep -A1 $lb1_lr_dp_group | tail -1], [0], [dnl
+$lr0_sb_uuid
+])
 
 echo
 echo "__file__:__line__: Associate lb1 to sw1 and check that lb1 is created in SB DB."
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index f09dddd99..2cb867ce0 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -11632,3 +11632,120 @@  OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ct_flush on logical router load balancer])
+AT_KEYWORDS([ct-lr-flush])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+start_daemon ovn-controller
+
+check ovn-nbctl lr-add R1
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add public
+
+check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
+check ovn-nbctl lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24
+
+check ovn-nbctl set logical_router R1 options:chassis=hv1
+
+check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
+    type=router options:router-port=rp-sw0 \
+    -- lsp-set-addresses sw0-rp router
+
+check ovn-nbctl lsp-add sw0 sw0-vm \
+    -- lsp-set-addresses sw0-vm "00:00:01:01:02:04 192.168.1.2/24"
+
+check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port public-rp \
+    type=router options:router-port=rp-public \
+    -- lsp-set-addresses public-rp router
+
+check ovn-nbctl lsp-add public public-vm \
+   -- lsp-set-addresses public-vm "00:00:02:01:02:04 172.16.1.2/24"
+
+ADD_NAMESPACES(sw0-vm)
+ADD_VETH(sw0-vm, sw0-vm, br-int, "192.168.1.2/24", "00:00:01:01:02:04", \
+         "192.168.1.1")
+OVS_WAIT_UNTIL([test "$(ip netns exec sw0-vm ip a | grep fe80 | grep tentative)" = ""])
+
+ADD_NAMESPACES(public-vm)
+ADD_VETH(public-vm, public-vm, br-int, "172.16.1.2/24", "00:00:02:01:02:04", \
+         "172.16.1.1")
+
+OVS_WAIT_UNTIL([test "$(ip netns exec public-vm ip a | grep fe80 | grep tentative)" = ""])
+
+# Start webservers in 'server'.
+OVS_START_L7([sw0-vm], [http])
+
+# Create a load balancer and associate to R1
+check ovn-nbctl lb-add lb1 172.16.1.150:80 192.168.1.2:80 \
+    -- set load_balancer lb1 options:ct_flush="true"
+check ovn-nbctl lr-lb-add R1 lb1
+
+check ovn-nbctl --wait=hv sync
+
+for i in $(seq 1 5); do
+    echo Request $i
+    NS_CHECK_EXEC([public-vm], [wget 172.16.1.150 -t 5 -T 1 --retry-connrefused -v -o wget$i.log])
+done
+
+OVS_WAIT_FOR_OUTPUT([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.150) | wc -l ], [0], [dnl
+1
+])
+
+check ovn-nbctl lb-del lb1
+
+OVS_WAIT_FOR_OUTPUT([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.150) | wc -l ], [0], [dnl
+0
+])
+
+check ovn-nbctl lb-add lb2 172.16.1.151:80 192.168.1.2:80
+check ovn-nbctl lr-lb-add R1 lb2
+
+check ovn-nbctl --wait=hv sync
+
+for i in $(seq 1 5); do
+    echo Request $i
+    NS_CHECK_EXEC([public-vm], [wget 172.16.1.151 -t 5 -T 1 --retry-connrefused -v -o wget$i.log])
+done
+
+OVS_WAIT_FOR_OUTPUT([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.151) | wc -l ], [0], [dnl
+1
+])
+
+check ovn-nbctl lb-del lb2
+
+OVS_WAIT_FOR_OUTPUT([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.151) | wc -l ], [0], [dnl
+1
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/Failed to acquire.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])