| Message ID | 09e0bf49191df67af33b1a57bf9e4b6c1bd18515.1687184886.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | sync lb applied to logical routers in sb db lb table | expand |
| 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 | fail | github build: failed |
On Mon, Jun 19, 2023 at 4:43 PM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > Introduce ls_datapath_group column in the load_balancer table of the SB > db 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> > Hi Lorenzo, I have few comments down below. > --- > 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 --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 80e836e44..196e8e7e7 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -2699,12 +2699,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..d0a6f871c 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" > The name should be aligned with other features s/_/-/. > > /* 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 2203d6c4a..9837aacc4 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; > + } > } > } > > @@ -4498,7 +4507,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); > @@ -4526,6 +4536,7 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn, > * are not indexed in any way. > */ > lb = ovn_northd_lb_find(lbs, &lb_uuid); > + > nit: Extra space if (!lb || (!lb->n_nb_ls && !lb->n_nb_lr) || > !hmapx_add(&existing_lbs, lb)) { > sbrec_load_balancer_delete(sbrec_lb); > @@ -4536,8 +4547,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); > @@ -4580,8 +4595,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); > @@ -4599,8 +4618,14 @@ 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); > + } else { > + /* datapath_group column is deprecated. */ > + sbrec_load_balancer_set_datapath_group( > + lb->slb, lb->ls_dpg->dp_group); > + } > We should always clear the second column so we don't accidentally end up with some duplications that do now make any sense. > } > if (lb->lr_dpg) { > sbrec_load_balancer_set_lr_datapath_group(lb->slb, > @@ -17261,6 +17286,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); > @@ -17446,7 +17472,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..23cf3ea11 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 963a41bf6..3ef4b0c15 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2774,7 +2774,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 > @@ -2785,7 +2785,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 > @@ -2888,7 +2888,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." > @@ -2899,7 +2899,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." > @@ -5947,9 +5947,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 > @@ -8618,7 +8618,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"); > -- > 2.41.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales
> On Mon, Jun 19, 2023 at 4:43 PM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> wrote: > > > Introduce ls_datapath_group column in the load_balancer table of the SB > > db 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> > > > > Hi Lorenzo, > > I have few comments down below. Hi Ales, thx for the review. > > > > --- > > 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 --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 80e836e44..196e8e7e7 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -2699,12 +2699,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..d0a6f871c 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" > > > > The name should be aligned with other features s/_/-/. ack, I will fix it > > > > > > /* 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 2203d6c4a..9837aacc4 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; > > + } > > } > > } > > > > @@ -4498,7 +4507,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); > > @@ -4526,6 +4536,7 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn, > > * are not indexed in any way. > > */ > > lb = ovn_northd_lb_find(lbs, &lb_uuid); > > + > > > > nit: Extra space > > if (!lb || (!lb->n_nb_ls && !lb->n_nb_lr) || ack, I will fix it > > !hmapx_add(&existing_lbs, lb)) { > > sbrec_load_balancer_delete(sbrec_lb); > > @@ -4536,8 +4547,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); > > @@ -4580,8 +4595,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); > > @@ -4599,8 +4618,14 @@ 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); > > + } else { > > + /* datapath_group column is deprecated. */ > > + sbrec_load_balancer_set_datapath_group( > > + lb->slb, lb->ls_dpg->dp_group); > > + } > > > > We should always clear the second column so we don't accidentally end up > with some > duplications that do now make any sense. ack, I will fix it Regards, Lorenzo > > > > } > > if (lb->lr_dpg) { > > sbrec_load_balancer_set_lr_datapath_group(lb->slb, > > @@ -17261,6 +17286,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); > > @@ -17446,7 +17472,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..23cf3ea11 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 963a41bf6..3ef4b0c15 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -2774,7 +2774,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 > > @@ -2785,7 +2785,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 > > @@ -2888,7 +2888,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." > > @@ -2899,7 +2899,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." > > @@ -5947,9 +5947,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 > > @@ -8618,7 +8618,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"); > > -- > > 2.41.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > Thanks, > Ales > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > amusil@redhat.com IM: amusil > <https://red.ht/sig>
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 80e836e44..196e8e7e7 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2699,12 +2699,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..d0a6f871c 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 2203d6c4a..9837aacc4 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; + } } } @@ -4498,7 +4507,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); @@ -4526,6 +4536,7 @@ 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 && !lb->n_nb_lr) || !hmapx_add(&existing_lbs, lb)) { sbrec_load_balancer_delete(sbrec_lb); @@ -4536,8 +4547,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); @@ -4580,8 +4595,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); @@ -4599,8 +4618,14 @@ 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); + } 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, @@ -17261,6 +17286,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); @@ -17446,7 +17472,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..23cf3ea11 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 963a41bf6..3ef4b0c15 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2774,7 +2774,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 @@ -2785,7 +2785,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 @@ -2888,7 +2888,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." @@ -2899,7 +2899,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." @@ -5947,9 +5947,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 @@ -8618,7 +8618,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");
Introduce ls_datapath_group column in the load_balancer table of the SB db 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(-)