diff mbox series

[ovs-dev,v2,2/2] northd: introduce ls_datapath_group column in lb sb db table

Message ID 09859a48a1e58ab9d49402a9f4a49d4b6ce4c9d0.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 ls_datapath_group column in the load_balancer table of the SB
sb and deprecate datapath_group one. This patch make the table symmetric
with lr_datapath_group column in the load_balancer table of SB db.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/chassis.c        |  8 ++++++++
 controller/lflow.c          | 10 ++++++++++
 controller/local_data.c     |  8 ++++++++
 controller/ovn-controller.c |  7 +++++++
 include/ovn/features.h      |  1 +
 northd/northd.c             | 39 +++++++++++++++++++++++++++++++------
 northd/northd.h             |  1 +
 ovn-sb.ovsschema            |  6 +++++-
 ovn-sb.xml                  |  6 ++++++
 tests/ovn-controller.at     |  4 ++++
 tests/ovn-northd.at         | 14 ++++++-------
 utilities/ovn-sbctl.c       | 13 +++++++++++++
 12 files changed, 103 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/controller/chassis.c b/controller/chassis.c
index ce88541ba..6454cf0e3 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -369,6 +369,7 @@  chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg,
     smap_replace(config, OVN_FEATURE_MAC_BINDING_TIMESTAMP, "true");
     smap_replace(config, OVN_FEATURE_CT_LB_RELATED, "true");
     smap_replace(config, OVN_FEATURE_FDB_TIMESTAMP, "true");
+    smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true");
 }
 
 /*
@@ -502,6 +503,12 @@  chassis_other_config_changed(const struct ovs_chassis_cfg *ovs_cfg,
         return true;
     }
 
+    if (!smap_get_bool(&chassis_rec->other_config,
+                       OVN_FEATURE_LS_DPG_COLUMN,
+                       false)) {
+        return true;
+    }
+
     return false;
 }
 
@@ -632,6 +639,7 @@  update_supported_sset(struct sset *supported)
     sset_add(supported, OVN_FEATURE_MAC_BINDING_TIMESTAMP);
     sset_add(supported, OVN_FEATURE_CT_LB_RELATED);
     sset_add(supported, OVN_FEATURE_FDB_TIMESTAMP);
+    sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN);
 }
 
 static void
diff --git a/controller/lflow.c b/controller/lflow.c
index 22faaf013..26b95911d 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1870,6 +1870,7 @@  add_lb_ct_snat_hairpin_vip_flow(const struct ovn_controller_lb *lb,
                                           local_datapaths, &match,
                                           &ofpacts, flow_table);
         }
+        /* datapath_group column is deprecated. */
         if (lb->slb->datapath_group) {
             for (size_t i = 0; i < lb->slb->datapath_group->n_datapaths; i++) {
                 add_lb_ct_snat_hairpin_for_dp(
@@ -1878,6 +1879,15 @@  add_lb_ct_snat_hairpin_vip_flow(const struct ovn_controller_lb *lb,
                     local_datapaths, &match, &ofpacts, flow_table);
             }
         }
+        if (lb->slb->ls_datapath_group) {
+            for (size_t i = 0;
+                 i < lb->slb->ls_datapath_group->n_datapaths; i++) {
+                add_lb_ct_snat_hairpin_for_dp(
+                    lb, !!lb_vip->vip_port,
+                    lb->slb->ls_datapath_group->datapaths[i],
+                    local_datapaths, &match, &ofpacts, flow_table);
+            }
+        }
     }
 
     ofpbuf_uninit(&ofpacts);
diff --git a/controller/local_data.c b/controller/local_data.c
index 01cfbdefe..3a7d3afeb 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -661,8 +661,16 @@  lb_is_local(const struct sbrec_load_balancer *sbrec_lb,
         }
     }
 
+    /* datapath_group column is deprecated. */
     struct sbrec_logical_dp_group *dp_group = sbrec_lb->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;
+        }
+    }
 
+    dp_group = sbrec_lb->ls_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)) {
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 55ad8e66f..06b9137c0 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2711,12 +2711,19 @@  load_balancers_by_dp_init(const struct hmap *local_datapaths,
             load_balancers_by_dp_add_one(local_datapaths,
                                          lb->datapaths[i], lb, lbs);
         }
+        /* datapath_group column is deprecated. */
         for (size_t i = 0; lb->datapath_group
                            && i < lb->datapath_group->n_datapaths; i++) {
             load_balancers_by_dp_add_one(local_datapaths,
                                          lb->datapath_group->datapaths[i],
                                          lb, lbs);
         }
+        for (size_t i = 0; lb->ls_datapath_group
+                           && i < lb->ls_datapath_group->n_datapaths; i++) {
+            load_balancers_by_dp_add_one(local_datapaths,
+                                         lb->ls_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,
diff --git a/include/ovn/features.h b/include/ovn/features.h
index de8f1f548..031e452df 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -26,6 +26,7 @@ 
 #define OVN_FEATURE_MAC_BINDING_TIMESTAMP "mac-binding-timestamp"
 #define OVN_FEATURE_CT_LB_RELATED "ovn-ct-lb-related"
 #define OVN_FEATURE_FDB_TIMESTAMP "fdb-timestamp"
+#define OVN_FEATURE_LS_DPG_COLUMN "ls-dpg-column"
 
 /* OVS datapath supported features.  Based on availability OVN might generate
  * different types of openflows.
diff --git a/northd/northd.c b/northd/northd.c
index 443e61948..5fae85d55 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -490,6 +490,15 @@  build_chassis_features(const struct sbrec_chassis_table *sbrec_chassis_table,
             chassis_features->fdb_timestamp) {
             chassis_features->fdb_timestamp = false;
         }
+
+        bool ls_dpg_column =
+            smap_get_bool(&chassis->other_config,
+                          OVN_FEATURE_LS_DPG_COLUMN,
+                          false);
+        if (!ls_dpg_column &&
+            chassis_features->ls_dpg_column) {
+            chassis_features->ls_dpg_column = false;
+        }
     }
 }
 
@@ -4542,7 +4551,8 @@  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 ovn_datapaths *lr_datapaths, struct hmap *lbs)
+         struct ovn_datapaths *lr_datapaths, struct hmap *lbs,
+         struct chassis_features *chassis_features)
 {
     struct hmap ls_dp_groups = HMAP_INITIALIZER(&ls_dp_groups);
     struct hmap lr_dp_groups = HMAP_INITIALIZER(&lr_dp_groups);
@@ -4581,8 +4591,12 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
 
         /* Find or create datapath group for this load balancer. */
         if (lb->n_nb_ls) {
+            struct sbrec_logical_dp_group *ls_datapath_group
+                = chassis_features->ls_dpg_column
+                    ? lb->slb->ls_datapath_group
+                    : lb->slb->datapath_group; /* deprecated */
             lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &ls_dp_groups,
-                                                    lb->slb->datapath_group,
+                                                    ls_datapath_group,
                                                     lb->n_nb_ls, lb->nb_ls_map,
                                                     ods_size(ls_datapaths),
                                                     true, ls_datapaths, NULL);
@@ -4625,8 +4639,12 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
 
         /* Find or create datapath group for this load balancer. */
         if (!lb->ls_dpg && lb->n_nb_ls) {
+            struct sbrec_logical_dp_group *ls_datapath_group
+                = chassis_features->ls_dpg_column
+                    ? lb->slb->ls_datapath_group
+                    : lb->slb->datapath_group; /* deprecated */
             lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &ls_dp_groups,
-                                                    lb->slb->datapath_group,
+                                                    ls_datapath_group,
                                                     lb->n_nb_ls, lb->nb_ls_map,
                                                     ods_size(ls_datapaths),
                                                     true, ls_datapaths, NULL);
@@ -4644,8 +4662,15 @@  sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
         sbrec_load_balancer_set_vips(lb->slb, ovn_northd_lb_get_vips(lb));
         sbrec_load_balancer_set_protocol(lb->slb, lb->nlb->protocol);
         if (lb->ls_dpg) {
-            sbrec_load_balancer_set_datapath_group(lb->slb,
-                                                   lb->ls_dpg->dp_group);
+            if (chassis_features->ls_dpg_column) {
+                sbrec_load_balancer_set_ls_datapath_group(
+                        lb->slb, lb->ls_dpg->dp_group);
+                sbrec_load_balancer_set_datapath_group(lb->slb, NULL);
+            } else {
+                /* datapath_group column is deprecated. */
+                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,
@@ -17435,6 +17460,7 @@  northd_init(struct northd_data *data)
         .mac_binding_timestamp = true,
         .ct_lb_related = true,
         .fdb_timestamp = true,
+        .ls_dpg_column = true,
     };
     data->ovn_internal_version_changed = false;
     sset_init(&data->svc_monitor_lsps);
@@ -17620,7 +17646,8 @@  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->lr_datapaths, &data->lbs);
+             &data->ls_datapaths, &data->lr_datapaths, &data->lbs,
+             &data->features);
     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/northd/northd.h b/northd/northd.h
index f3e63b1e1..f692dd952 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -73,6 +73,7 @@  struct chassis_features {
     bool mac_binding_timestamp;
     bool ct_lb_related;
     bool fdb_timestamp;
+    bool ls_dpg_column;
 };
 
 /* A collection of datapaths. E.g. all logical switch datapaths, or all
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index a66db998f..d7ac91345 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Southbound",
     "version": "20.27.4",
-    "cksum": "146375056 30745",
+    "cksum": "3110293831 30959",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -534,6 +534,10 @@ 
                     {"type": {"key": {"type": "uuid",
                                       "refTable": "Logical_DP_Group"},
                               "min": 0, "max": 1}},
+                "ls_datapath_group":
+                    {"type": {"key": {"type": "uuid",
+                                      "refTable": "Logical_DP_Group"},
+                              "min": 0, "max": 1}},
                 "lr_datapath_group":
                     {"type": {"key": {"type": "uuid",
                                       "refTable": "Logical_DP_Group"},
diff --git a/ovn-sb.xml b/ovn-sb.xml
index f9b7d309d..d4e311036 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4880,6 +4880,12 @@  tcp.flags = RST;
     </column>
 
     <column name="datapath_group">
+      Deprecated. The group of datapaths to which this load balancer applies
+      to. This means that the same load balancer applies to all datapaths in
+      a group.
+    </column>
+
+    <column name="ls_datapath_group">
       The group of datapaths to which this load balancer applies to.  This
       means that the same load balancer applies to all datapaths in a group.
     </column>
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 28c13234c..0e803ea18 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -526,6 +526,10 @@  OVS_WAIT_UNTIL([
     test "$(ovn-sbctl get chassis hv1 other_config:port-up-notif)" = '"true"'
 ])
 
+OVS_WAIT_UNTIL([
+    test "$(ovn-sbctl get chassis hv1 other_config:ls-dpg-column)" = '"true"'
+])
+
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index dcf6aa794..b29f59cd5 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2799,7 +2799,7 @@  lbg0_uuid=$(fetch_column sb:load_balancer _uuid name=lbg0)
 echo
 echo "__file__:__line__: Check that SB lb0 has sw0 in datapaths column."
 
-lb0_dp_group=$(fetch_column sb:load_balancer datapath_group name=lb0)
+lb0_dp_group=$(fetch_column sb:load_balancer ls_datapath_group name=lb0)
 AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find Logical_DP_Group dnl
                     | grep -A1 $lb0_dp_group | tail -1], [0], [dnl
 $sw0_sb_uuid
@@ -2810,7 +2810,7 @@  check_column "" sb:datapath_binding load_balancers external_ids:name=sw0
 echo
 echo "__file__:__line__: Check that SB lbg0 has sw0 in datapaths column."
 
-lbg0_dp_group=$(fetch_column sb:load_balancer datapath_group name=lbg0)
+lbg0_dp_group=$(fetch_column sb:load_balancer ls_datapath_group name=lbg0)
 AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find Logical_DP_Group dnl
                     | grep -A1 $lbg0_dp_group | tail -1], [0], [dnl
 $sw0_sb_uuid
@@ -2913,7 +2913,7 @@  echo "__file__:__line__: Check that SB lbg1 has vips and protocol columns are se
 check_column "20.0.0.30:80=20.0.0.50:8080 udp" sb:load_balancer vips,protocol name=lbg1
 
 lb1_uuid=$(fetch_column sb:load_balancer _uuid name=lb1)
-lb1_dp_group=$(fetch_column sb:load_balancer datapath_group name=lb1)
+lb1_dp_group=$(fetch_column sb:load_balancer ls_datapath_group name=lb1)
 
 echo
 echo "__file__:__line__: Check that SB lb1 has sw1 in datapaths column."
@@ -2924,7 +2924,7 @@  $sw1_sb_uuid
 ])
 
 lbg1_uuid=$(fetch_column sb:load_balancer _uuid name=lbg1)
-lbg1_dp_group=$(fetch_column sb:load_balancer datapath_group name=lbg1)
+lbg1_dp_group=$(fetch_column sb:load_balancer ls_datapath_group name=lbg1)
 
 echo
 echo "__file__:__line__: Check that SB lbg1 has sw0 and sw1 in datapaths column."
@@ -5972,9 +5972,9 @@  ovn_start
 check ovn-nbctl ls-add ls -- lb-add lb1 10.0.0.1:80 10.0.0.2:80 -- ls-lb-add ls lb1
 check ovn-nbctl --wait=sb sync
 
-dps=$(fetch_column Load_Balancer datapath_group)
+dps=$(fetch_column Load_Balancer ls_datapath_group)
 nlb=$(fetch_column nb:Load_Balancer _uuid)
-AT_CHECK([ovn-sbctl create Load_Balancer name=lb1 datapath_group="$dps" external_ids="lb_id=$nlb"], [0], [ignore])
+AT_CHECK([ovn-sbctl create Load_Balancer name=lb1 ls_datapath_group="$dps" external_ids="lb_id=$nlb"], [0], [ignore])
 
 check ovn-nbctl --wait=sb sync
 check_row_count Load_Balancer 1
@@ -8643,7 +8643,7 @@  AT_CHECK([grep "ls_in_lb " S1flows | sed 's/table=../table=??/' | sort], [0], [d
 
 ovn-sbctl get datapath S0 _uuid > dp_uuids
 ovn-sbctl get datapath S1 _uuid >> dp_uuids
-lb_dp_group=$(ovn-sbctl --bare --columns datapath_group find Load_Balancer name=lb0)
+lb_dp_group=$(ovn-sbctl --bare --columns ls_datapath_group find Load_Balancer name=lb0)
 AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find Logical_DP_Group dnl
                     | grep -A1 $lb_dp_group | tail -1 | tr ' ' '\n' | sort], [0], [dnl
 $(cat dp_uuids | sort)
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index 3948cae3f..f1f8c2b42 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -396,7 +396,9 @@  pre_get_info(struct ctl_context *ctx)
     ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
 
     ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_datapaths);
+    /* datapath_group column is deprecated. */
     ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_datapath_group);
+    ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_ls_datapath_group);
     ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_vips);
     ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_name);
     ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_protocol);
@@ -932,10 +934,15 @@  cmd_lflow_list_load_balancers(struct ctl_context *ctx, struct vconn *vconn,
                     break;
                 }
             }
+            /* datapath_group column is deprecated. */
             if (lb->datapath_group && !dp_found) {
                 dp_found = datapath_group_contains_datapath(lb->datapath_group,
                                                             datapath);
             }
+            if (lb->ls_datapath_group && !dp_found) {
+                dp_found = datapath_group_contains_datapath(
+                        lb->ls_datapath_group, datapath);
+            }
             if (!dp_found) {
                 continue;
             }
@@ -954,11 +961,17 @@  cmd_lflow_list_load_balancers(struct ctl_context *ctx, struct vconn *vconn,
                 print_vflow_datapath_name(lb->datapaths[i], true,
                                           &ctx->output);
             }
+            /* datapath_group column is deprecated. */
             for (size_t i = 0; lb->datapath_group
                                && i < lb->datapath_group->n_datapaths; i++) {
                 print_vflow_datapath_name(lb->datapath_group->datapaths[i],
                                           true, &ctx->output);
             }
+            for (size_t i = 0; lb->ls_datapath_group
+                    && i < lb->ls_datapath_group->n_datapaths; i++) {
+                print_vflow_datapath_name(lb->ls_datapath_group->datapaths[i],
+                                          true, &ctx->output);
+            }
         }
 
         ds_put_cstr(&ctx->output, "\n  vips:\n");