diff mbox series

[ovs-dev,1/5] Add new table Load_Balancer in Southbound database.

Message ID 20201021072513.3750765-1-numans@ovn.org
State Superseded
Headers show
Series [ovs-dev,1/5] Add new table Load_Balancer in Southbound database. | expand

Commit Message

Numan Siddique Oct. 21, 2020, 7:25 a.m. UTC
From: Numan Siddique <numans@ovn.org>

This patch adds a new table 'Load_Balancer' in SB DB and syncs the Load_Balancer table rows
from NB DB to SB DB. An upcoming patch will make use of this table for handling the
load balancer hairpin traffic.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/ovn-northd.c   | 141 ++++++++++++++++++++++++++++++++++++++++++
 ovn-sb.ovsschema      |  27 +++++++-
 ovn-sb.xml            |  47 ++++++++++++++
 tests/ovn-northd.at   |  81 ++++++++++++++++++++++++
 utilities/ovn-sbctl.c |   3 +
 5 files changed, 297 insertions(+), 2 deletions(-)

Comments

Mark Michelson Oct. 23, 2020, 7:58 p.m. UTC | #1
On 10/21/20 3:25 AM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> This patch adds a new table 'Load_Balancer' in SB DB and syncs the Load_Balancer table rows
> from NB DB to SB DB. An upcoming patch will make use of this table for handling the
> load balancer hairpin traffic.
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>   northd/ovn-northd.c   | 141 ++++++++++++++++++++++++++++++++++++++++++
>   ovn-sb.ovsschema      |  27 +++++++-
>   ovn-sb.xml            |  47 ++++++++++++++
>   tests/ovn-northd.at   |  81 ++++++++++++++++++++++++
>   utilities/ovn-sbctl.c |   3 +
>   5 files changed, 297 insertions(+), 2 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 5b76868dfb..ea771afda1 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11896,6 +11896,136 @@ sync_dns_entries(struct northd_context *ctx, struct hmap *datapaths)
>       }
>       hmap_destroy(&dns_map);
>   }
> +
> +/*
> + * struct 'sync_lb_info' is used to sync the load balancer records between
> + * OVN Northbound db and Southbound db.
> + */
> +struct sync_lb_info {
> +    struct hmap_node hmap_node;
> +    const struct nbrec_load_balancer *nlb; /* LB record in the NB db. */
> +    const struct sbrec_load_balancer *slb; /* LB record in the SB db. */
> +
> +    /* Datapaths to which the LB entry is associated with it. */
> +    const struct sbrec_datapath_binding **sbs;
> +    size_t n_sbs;
> +};
> +
> +static inline struct sync_lb_info *
> +get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid *uuid)
> +{
> +    struct sync_lb_info *lb_info;
> +    size_t hash = uuid_hash(uuid);
> +    HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash, sync_lb_map) {
> +        if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
> +            return lb_info;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static void
> +sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
> +{
> +    struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
> +    struct ovn_datapath *od;
> +
> +    HMAP_FOR_EACH (od, key_node, datapaths) {
> +        if (!od->nbs || !od->nbs->n_load_balancer) {
> +            continue;
> +        }
> +
> +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> +            struct sync_lb_info *lb_info =
> +                get_sync_lb_info_from_hmap(
> +                    &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
> +
> +            if (!lb_info) {
> +                size_t hash = uuid_hash(
> +                    &od->nbs->load_balancer[i]->header_.uuid);
> +                lb_info = xzalloc(sizeof *lb_info);;
> +                lb_info->nlb = od->nbs->load_balancer[i];
> +                hmap_insert(&lb_map, &lb_info->hmap_node, hash);
> +            }
> +
> +            lb_info->n_sbs++;
> +            lb_info->sbs = xrealloc(lb_info->sbs,
> +                                    lb_info->n_sbs * sizeof *lb_info->sbs);
> +            lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
> +        }
> +    }
> +
> +    const struct sbrec_load_balancer *sbrec_lb, *next;
> +    SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next, ctx->ovnsb_idl) {
> +        const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
> +        struct uuid lb_uuid;
> +        if (!nb_lb_uuid || !uuid_from_string(&lb_uuid, nb_lb_uuid)) {
> +            sbrec_load_balancer_delete(sbrec_lb);
> +            continue;
> +        }
> +
> +        struct sync_lb_info *lb_info =
> +            get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
> +        if (lb_info) {
> +            lb_info->slb = sbrec_lb;
> +        } else {
> +            sbrec_load_balancer_delete(sbrec_lb);
> +        }
> +    }
> +
> +    struct sync_lb_info *lb_info;
> +    HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
> +        if (!lb_info->slb) {
> +            sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
> +            lb_info->slb = sbrec_lb;
> +            char *lb_id = xasprintf(
> +                UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
> +            const struct smap external_ids =
> +                SMAP_CONST1(&external_ids, "lb_id", lb_id);
> +            sbrec_load_balancer_set_external_ids(sbrec_lb, &external_ids);
> +            free(lb_id);
> +        }
> +
> +        /* Set the datapaths and other columns.  If nothing has changed, then
> +         * this will be a no-op.
> +         */
> +        sbrec_load_balancer_set_datapaths(
> +            lb_info->slb,
> +            (struct sbrec_datapath_binding **)lb_info->sbs,
> +            lb_info->n_sbs);
> +
> +        sbrec_load_balancer_set_name(lb_info->slb, lb_info->nlb->name);
> +        sbrec_load_balancer_set_vips(lb_info->slb, &lb_info->nlb->vips);
> +        sbrec_load_balancer_set_protocol(lb_info->slb, lb_info->nlb->protocol);
> +    }
> +
> +    HMAP_FOR_EACH (od, key_node, datapaths) {
> +        if (!od->nbs || !od->nbs->n_load_balancer) {
> +            continue;
> +        }
> +
> +        const struct sbrec_load_balancer **lbs =
> +            xmalloc(od->nbs->n_load_balancer * sizeof *lbs);
> +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> +            lb_info = get_sync_lb_info_from_hmap(
> +                &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
> +            ovs_assert(lb_info);
> +            lbs[i] = lb_info->slb;
> +        }
> +
> +        sbrec_datapath_binding_set_load_balancers(
> +            od->sb, (struct sbrec_load_balancer **)lbs,
> +            od->nbs->n_load_balancer);
> +        free(lbs);
> +    }
> +
> +    HMAP_FOR_EACH_POP (lb_info, hmap_node, &lb_map) {
> +        free(lb_info->sbs);
> +        free(lb_info);
> +    }
> +    hmap_destroy(&lb_map);
> +}
>   
>   static void
>   destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap *ports,
> @@ -12263,6 +12393,7 @@ ovnnb_db_run(struct northd_context *ctx,
>       sync_port_groups(ctx, &port_groups);
>       sync_meters(ctx);
>       sync_dns_entries(ctx, datapaths);
> +    sync_lb_entries(ctx, datapaths);
>       destroy_ovn_lbs(&lbs);
>       hmap_destroy(&lbs);
>   
> @@ -13022,6 +13153,8 @@ main(int argc, char *argv[])
>       ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_datapath_binding);
>       add_column_noalert(ovnsb_idl_loop.idl,
>                          &sbrec_datapath_binding_col_tunnel_key);
> +    add_column_noalert(ovnsb_idl_loop.idl,
> +                       &sbrec_datapath_binding_col_load_balancers);
>       add_column_noalert(ovnsb_idl_loop.idl,
>                          &sbrec_datapath_binding_col_external_ids);
>   
> @@ -13189,6 +13322,14 @@ main(int argc, char *argv[])
>       add_column_noalert(ovnsb_idl_loop.idl,
>                          &sbrec_service_monitor_col_external_ids);
>   
> +    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_load_balancer);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_datapaths);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_name);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_vips);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_protocol);
> +    add_column_noalert(ovnsb_idl_loop.idl,
> +                       &sbrec_load_balancer_col_external_ids);
> +
>       struct ovsdb_idl_index *sbrec_chassis_by_name
>           = chassis_index_create(ovnsb_idl_loop.idl);
>   
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index d1c506a22c..7db6c6a4dd 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>   {
>       "name": "OVN_Southbound",
> -    "version": "2.10.0",
> -    "cksum": "2548342632 22615",
> +    "version": "2.11.0",
> +    "cksum": "1470439925 23814",
>       "tables": {
>           "SB_Global": {
>               "columns": {
> @@ -152,6 +152,11 @@
>                        "type": {"key": {"type": "integer",
>                                         "minInteger": 1,
>                                         "maxInteger": 16777215}}},
> +                "load_balancers": {"type": {"key": {"type": "uuid",
> +                                                   "refTable": "Load_Balancer",
> +                                                   "refType": "weak"},
> +                                            "min": 0,
> +                                            "max": "unlimited"}},
>                   "external_ids": {
>                       "type": {"key": "string", "value": "string",
>                                "min": 0, "max": "unlimited"}}},
> @@ -447,6 +452,24 @@
>                       "type": {"key": "string", "value": "string",
>                                "min": 0, "max": "unlimited"}}},
>               "indexes": [["logical_port", "ip", "port", "protocol"]],
> +            "isRoot": true},
> +        "Load_Balancer": {
> +            "columns": {
> +                "name": {"type": "string"},
> +                "vips": {
> +                    "type": {"key": "string", "value": "string",
> +                             "min": 0, "max": "unlimited"}},
> +                "protocol": {
> +                    "type": {"key": {"type": "string",
> +                             "enum": ["set", ["tcp", "udp", "sctp"]]},
> +                             "min": 0, "max": 1}},
> +                "datapaths": {
> +                    "type": {"key": {"type": "uuid",
> +                                     "refTable": "Datapath_Binding"},
> +                             "min": 1, "max": "unlimited"}},
> +                "external_ids": {
> +                    "type": {"key": "string", "value": "string",
> +                             "min": 0, "max": "unlimited"}}},
>               "isRoot": true}
>       }
>   }
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 182ff0a8a2..8638fa7eb4 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2497,6 +2497,12 @@ tcp.flags = RST;
>         constructed for each supported encapsulation.
>       </column>
>   
> +    <column name="load_balancers">
> +      <p>
> +        Load balancers associated with the datapath.
> +      </p>
> +    </column>
> +
>       <group title="OVN_Northbound Relationship">
>         <p>
>           Each row in <ref table="Datapath_Binding"/> is associated with some
> @@ -4104,4 +4110,45 @@ tcp.flags = RST;
>         </column>
>       </group>
>     </table>
> +
> +  <table name="Load_Balancer">
> +    <p>
> +      Each row represents one load balancer and corresponds directly
> +      with the <ref table="Load_Balancer"/> table in the
> +      <ref db="OVN_Northbound"/> database.
> +    </p>

This isn't quite correct. Only load balancers associated with logical 
switches are represented in the southbound database. Load balancers used 
by logical routers and load balancers that are not used by any datapaths 
are not put in the southbound database. Your tests even ensure that this 
is the case.

> +
> +    <column name="name">
> +      A name for the load balancer.  This name has no special meaning or
> +      purpose other than to provide convenience for human interaction with
> +      the ovn-nb database.
> +    </column>
> +
> +    <column name="vips">
> +      A map of virtual IP addresses (and an optional port number with
> +      <code>:</code> as a separator) associated with this load balancer and
> +      their corresponding endpoint IP addresses (and optional port numbers
> +      with <code>:</code> as separators) separated by commas.
> +    </column>
> +
> +    <column name="protocol">
> +      <p>
> +        Valid protocols are <code>tcp</code>, <code>udp</code>, or
> +        <code>sctp</code>.  This column is useful when a port number is
> +        provided as part of the <code>vips</code> column.  If this column is
> +        empty and a port number is provided as part of <code>vips</code>
> +        column, OVN assumes the protocol to be <code>tcp</code>.
> +      </p>
> +    </column>
> +
> +    <column name="datapaths">
> +      Datapaths to which this load balancer applies to.
> +    </column>
> +
> +    <group title="Common Columns">
> +      <column name="external_ids">
> +        See <em>External IDs</em> at the beginning of this document.
> +      </column>
> +    </group>
> +  </table>
>   </database>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 50457c291a..3dd1920b79 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2130,3 +2130,84 @@ action=(reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is implici
>   ])
>   
>   AT_CLEANUP
> +
> +AT_SETUP([ovn -- NB to SB load balancer sync])
> +ovn_start
> +
> +ovn-nbctl --wait=hv lb-add lb0 10.0.0.10:80 10.0.0.4:8080
> +
> +AT_CHECK([ovn-nbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
> +1
> +])
> +
> +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
> +0
> +])
> +
> +ovn-nbctl ls-add sw0
> +ovn-nbctl --wait=hv ls-lb-add sw0 lb0
> +sw0_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw0)
> +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list load_balancer], [0], [dnl
> +lb0
> +10.0.0.10:80=10.0.0.4:8080
> +tcp
> +])
> +
> +lb0_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb0)
> +
> +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list load_balancer) = $sw0_sb_uuid])
> +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list datapath sw0) = $lb0_uuid])
> +
> +ovn-nbctl --wait=sb set load_balancer . vips:"10.0.0.20\:90"="20.0.0.4:8080,30.0.0.4:8080"
> +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list load_balancer], [0], [dnl
> +lb0
> +10.0.0.10:80=10.0.0.4:8080 10.0.0.20:90=20.0.0.4:8080,30.0.0.4:8080
> +tcp
> +])
> +
> +ovn-nbctl lr-add lr0
> +ovn-nbctl --wait=sb lr-lb-add lr0 lb0
> +
> +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list load_balancer) = $sw0_sb_uuid])
> +
> +ovn-nbctl ls-add sw1
> +ovn-nbctl --wait=sb ls-lb-add sw1 lb0
> +sw1_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw1)
> +
> +for i in $sw0_sb_uuid $sw1_sb_uuid; do echo $i >> dp_ids; done
> +for i in $(ovn-sbctl --bare --columns datapaths list load_balancer); do echo $i >> lb_dps; done
> +
> +cat dp_ids | sort > expout
> +AT_CHECK([cat lb_dps | sort], [0], [expout])
> +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list datapath sw1) = $lb0_uuid])
> +
> +ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80 20.0.0.50:8080 udp
> +
> +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
> +1
> +])
> +
> +ovn-nbctl --wait=sb lr-lb-add lr0 lb1
> +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
> +1
> +])
> +
> +ovn-nbctl --wait=sb ls-lb-add sw1 lb1
> +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list load_balancer lb1 ], [0], [dnl
> +lb1
> +10.0.0.30:80=20.0.0.50:8080
> +udp
> +])
> +
> +lb1_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb1)
> +
> +for i in $lb0_uuid $lb1_uuid; do echo $i >> lb_ids; done
> +cat lb_ids | sort > expout
> +
> +for i in $(ovn-sbctl --bare --columns load_balancers list datapath_binding sw1); do echo $i >> sw1_lbs; done
> +AT_CHECK([cat sw1_lbs | sort], [0], [expout])
> +
> +ovn-nbctl --wait=sb lb-del lb1
> +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list datapath_binding sw1) = $lb0_uuid])
> +
> +AT_CLEANUP
> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> index d3eec91332..e92e9187a5 100644
> --- a/utilities/ovn-sbctl.c
> +++ b/utilities/ovn-sbctl.c
> @@ -1415,6 +1415,9 @@ static const struct ctl_table_class tables[SBREC_N_TABLES] = {
>   
>       [SBREC_TABLE_HA_CHASSIS].row_ids[0]
>       = {&sbrec_ha_chassis_col_chassis, NULL, NULL},
> +
> +    [SBREC_TABLE_LOAD_BALANCER].row_ids[0]
> +    = {&sbrec_load_balancer_col_name, NULL, NULL},
>   };
>   
>   
>
Numan Siddique Oct. 23, 2020, 8:21 p.m. UTC | #2
On Sat, Oct 24, 2020, 1:29 AM Mark Michelson <mmichels@redhat.com> wrote:

> On 10/21/20 3:25 AM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > This patch adds a new table 'Load_Balancer' in SB DB and syncs the
> Load_Balancer table rows
> > from NB DB to SB DB. An upcoming patch will make use of this table for
> handling the
> > load balancer hairpin traffic.
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >   northd/ovn-northd.c   | 141 ++++++++++++++++++++++++++++++++++++++++++
> >   ovn-sb.ovsschema      |  27 +++++++-
> >   ovn-sb.xml            |  47 ++++++++++++++
> >   tests/ovn-northd.at   |  81 ++++++++++++++++++++++++
> >   utilities/ovn-sbctl.c |   3 +
> >   5 files changed, 297 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 5b76868dfb..ea771afda1 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -11896,6 +11896,136 @@ sync_dns_entries(struct northd_context *ctx,
> struct hmap *datapaths)
> >       }
> >       hmap_destroy(&dns_map);
> >   }
> > +
> > +/*
> > + * struct 'sync_lb_info' is used to sync the load balancer records
> between
> > + * OVN Northbound db and Southbound db.
> > + */
> > +struct sync_lb_info {
> > +    struct hmap_node hmap_node;
> > +    const struct nbrec_load_balancer *nlb; /* LB record in the NB db. */
> > +    const struct sbrec_load_balancer *slb; /* LB record in the SB db. */
> > +
> > +    /* Datapaths to which the LB entry is associated with it. */
> > +    const struct sbrec_datapath_binding **sbs;
> > +    size_t n_sbs;
> > +};
> > +
> > +static inline struct sync_lb_info *
> > +get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid *uuid)
> > +{
> > +    struct sync_lb_info *lb_info;
> > +    size_t hash = uuid_hash(uuid);
> > +    HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash, sync_lb_map) {
> > +        if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
> > +            return lb_info;
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +static void
> > +sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
> > +{
> > +    struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
> > +    struct ovn_datapath *od;
> > +
> > +    HMAP_FOR_EACH (od, key_node, datapaths) {
> > +        if (!od->nbs || !od->nbs->n_load_balancer) {
> > +            continue;
> > +        }
> > +
> > +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> > +            struct sync_lb_info *lb_info =
> > +                get_sync_lb_info_from_hmap(
> > +                    &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
> > +
> > +            if (!lb_info) {
> > +                size_t hash = uuid_hash(
> > +                    &od->nbs->load_balancer[i]->header_.uuid);
> > +                lb_info = xzalloc(sizeof *lb_info);;
> > +                lb_info->nlb = od->nbs->load_balancer[i];
> > +                hmap_insert(&lb_map, &lb_info->hmap_node, hash);
> > +            }
> > +
> > +            lb_info->n_sbs++;
> > +            lb_info->sbs = xrealloc(lb_info->sbs,
> > +                                    lb_info->n_sbs * sizeof
> *lb_info->sbs);
> > +            lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
> > +        }
> > +    }
> > +
> > +    const struct sbrec_load_balancer *sbrec_lb, *next;
> > +    SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next, ctx->ovnsb_idl) {
> > +        const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids,
> "lb_id");
> > +        struct uuid lb_uuid;
> > +        if (!nb_lb_uuid || !uuid_from_string(&lb_uuid, nb_lb_uuid)) {
> > +            sbrec_load_balancer_delete(sbrec_lb);
> > +            continue;
> > +        }
> > +
> > +        struct sync_lb_info *lb_info =
> > +            get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
> > +        if (lb_info) {
> > +            lb_info->slb = sbrec_lb;
> > +        } else {
> > +            sbrec_load_balancer_delete(sbrec_lb);
> > +        }
> > +    }
> > +
> > +    struct sync_lb_info *lb_info;
> > +    HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
> > +        if (!lb_info->slb) {
> > +            sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
> > +            lb_info->slb = sbrec_lb;
> > +            char *lb_id = xasprintf(
> > +                UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
> > +            const struct smap external_ids =
> > +                SMAP_CONST1(&external_ids, "lb_id", lb_id);
> > +            sbrec_load_balancer_set_external_ids(sbrec_lb,
> &external_ids);
> > +            free(lb_id);
> > +        }
> > +
> > +        /* Set the datapaths and other columns.  If nothing has
> changed, then
> > +         * this will be a no-op.
> > +         */
> > +        sbrec_load_balancer_set_datapaths(
> > +            lb_info->slb,
> > +            (struct sbrec_datapath_binding **)lb_info->sbs,
> > +            lb_info->n_sbs);
> > +
> > +        sbrec_load_balancer_set_name(lb_info->slb, lb_info->nlb->name);
> > +        sbrec_load_balancer_set_vips(lb_info->slb, &lb_info->nlb->vips);
> > +        sbrec_load_balancer_set_protocol(lb_info->slb,
> lb_info->nlb->protocol);
> > +    }
> > +
> > +    HMAP_FOR_EACH (od, key_node, datapaths) {
> > +        if (!od->nbs || !od->nbs->n_load_balancer) {
> > +            continue;
> > +        }
> > +
> > +        const struct sbrec_load_balancer **lbs =
> > +            xmalloc(od->nbs->n_load_balancer * sizeof *lbs);
> > +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> > +            lb_info = get_sync_lb_info_from_hmap(
> > +                &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
> > +            ovs_assert(lb_info);
> > +            lbs[i] = lb_info->slb;
> > +        }
> > +
> > +        sbrec_datapath_binding_set_load_balancers(
> > +            od->sb, (struct sbrec_load_balancer **)lbs,
> > +            od->nbs->n_load_balancer);
> > +        free(lbs);
> > +    }
> > +
> > +    HMAP_FOR_EACH_POP (lb_info, hmap_node, &lb_map) {
> > +        free(lb_info->sbs);
> > +        free(lb_info);
> > +    }
> > +    hmap_destroy(&lb_map);
> > +}
> >
> >   static void
> >   destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap *ports,
> > @@ -12263,6 +12393,7 @@ ovnnb_db_run(struct northd_context *ctx,
> >       sync_port_groups(ctx, &port_groups);
> >       sync_meters(ctx);
> >       sync_dns_entries(ctx, datapaths);
> > +    sync_lb_entries(ctx, datapaths);
> >       destroy_ovn_lbs(&lbs);
> >       hmap_destroy(&lbs);
> >
> > @@ -13022,6 +13153,8 @@ main(int argc, char *argv[])
> >       ovsdb_idl_add_table(ovnsb_idl_loop.idl,
> &sbrec_table_datapath_binding);
> >       add_column_noalert(ovnsb_idl_loop.idl,
> >                          &sbrec_datapath_binding_col_tunnel_key);
> > +    add_column_noalert(ovnsb_idl_loop.idl,
> > +                       &sbrec_datapath_binding_col_load_balancers);
> >       add_column_noalert(ovnsb_idl_loop.idl,
> >                          &sbrec_datapath_binding_col_external_ids);
> >
> > @@ -13189,6 +13322,14 @@ main(int argc, char *argv[])
> >       add_column_noalert(ovnsb_idl_loop.idl,
> >                          &sbrec_service_monitor_col_external_ids);
> >
> > +    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_load_balancer);
> > +    add_column_noalert(ovnsb_idl_loop.idl,
> &sbrec_load_balancer_col_datapaths);
> > +    add_column_noalert(ovnsb_idl_loop.idl,
> &sbrec_load_balancer_col_name);
> > +    add_column_noalert(ovnsb_idl_loop.idl,
> &sbrec_load_balancer_col_vips);
> > +    add_column_noalert(ovnsb_idl_loop.idl,
> &sbrec_load_balancer_col_protocol);
> > +    add_column_noalert(ovnsb_idl_loop.idl,
> > +                       &sbrec_load_balancer_col_external_ids);
> > +
> >       struct ovsdb_idl_index *sbrec_chassis_by_name
> >           = chassis_index_create(ovnsb_idl_loop.idl);
> >
> > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> > index d1c506a22c..7db6c6a4dd 100644
> > --- a/ovn-sb.ovsschema
> > +++ b/ovn-sb.ovsschema
> > @@ -1,7 +1,7 @@
> >   {
> >       "name": "OVN_Southbound",
> > -    "version": "2.10.0",
> > -    "cksum": "2548342632 22615",
> > +    "version": "2.11.0",
> > +    "cksum": "1470439925 23814",
> >       "tables": {
> >           "SB_Global": {
> >               "columns": {
> > @@ -152,6 +152,11 @@
> >                        "type": {"key": {"type": "integer",
> >                                         "minInteger": 1,
> >                                         "maxInteger": 16777215}}},
> > +                "load_balancers": {"type": {"key": {"type": "uuid",
> > +                                                   "refTable":
> "Load_Balancer",
> > +                                                   "refType": "weak"},
> > +                                            "min": 0,
> > +                                            "max": "unlimited"}},
> >                   "external_ids": {
> >                       "type": {"key": "string", "value": "string",
> >                                "min": 0, "max": "unlimited"}}},
> > @@ -447,6 +452,24 @@
> >                       "type": {"key": "string", "value": "string",
> >                                "min": 0, "max": "unlimited"}}},
> >               "indexes": [["logical_port", "ip", "port", "protocol"]],
> > +            "isRoot": true},
> > +        "Load_Balancer": {
> > +            "columns": {
> > +                "name": {"type": "string"},
> > +                "vips": {
> > +                    "type": {"key": "string", "value": "string",
> > +                             "min": 0, "max": "unlimited"}},
> > +                "protocol": {
> > +                    "type": {"key": {"type": "string",
> > +                             "enum": ["set", ["tcp", "udp", "sctp"]]},
> > +                             "min": 0, "max": 1}},
> > +                "datapaths": {
> > +                    "type": {"key": {"type": "uuid",
> > +                                     "refTable": "Datapath_Binding"},
> > +                             "min": 1, "max": "unlimited"}},
> > +                "external_ids": {
> > +                    "type": {"key": "string", "value": "string",
> > +                             "min": 0, "max": "unlimited"}}},
> >               "isRoot": true}
> >       }
> >   }
> > diff --git a/ovn-sb.xml b/ovn-sb.xml
> > index 182ff0a8a2..8638fa7eb4 100644
> > --- a/ovn-sb.xml
> > +++ b/ovn-sb.xml
> > @@ -2497,6 +2497,12 @@ tcp.flags = RST;
> >         constructed for each supported encapsulation.
> >       </column>
> >
> > +    <column name="load_balancers">
> > +      <p>
> > +        Load balancers associated with the datapath.
> > +      </p>
> > +    </column>
> > +
> >       <group title="OVN_Northbound Relationship">
> >         <p>
> >           Each row in <ref table="Datapath_Binding"/> is associated with
> some
> > @@ -4104,4 +4110,45 @@ tcp.flags = RST;
> >         </column>
> >       </group>
> >     </table>
> > +
> > +  <table name="Load_Balancer">
> > +    <p>
> > +      Each row represents one load balancer and corresponds directly
> > +      with the <ref table="Load_Balancer"/> table in the
> > +      <ref db="OVN_Northbound"/> database.
> > +    </p>
>
> This isn't quite correct. Only load balancers associated with logical
> switches are represented in the southbound database. Load balancers used
> by logical routers and load balancers that are not used by any datapaths
> are not put in the southbound database. Your tests even ensure that this
> is the case.
>

That's the intention. Since hairpin lflows are added only to the logical
switch datapaths.


Thanks
Numan



> > +
> > +    <column name="name">
> > +      A name for the load balancer.  This name has no special meaning or
> > +      purpose other than to provide convenience for human interaction
> with
> > +      the ovn-nb database.
> > +    </column>
> > +
> > +    <column name="vips">
> > +      A map of virtual IP addresses (and an optional port number with
> > +      <code>:</code> as a separator) associated with this load balancer
> and
> > +      their corresponding endpoint IP addresses (and optional port
> numbers
> > +      with <code>:</code> as separators) separated by commas.
> > +    </column>
> > +
> > +    <column name="protocol">
> > +      <p>
> > +        Valid protocols are <code>tcp</code>, <code>udp</code>, or
> > +        <code>sctp</code>.  This column is useful when a port number is
> > +        provided as part of the <code>vips</code> column.  If this
> column is
> > +        empty and a port number is provided as part of <code>vips</code>
> > +        column, OVN assumes the protocol to be <code>tcp</code>.
> > +      </p>
> > +    </column>
> > +
> > +    <column name="datapaths">
> > +      Datapaths to which this load balancer applies to.
> > +    </column>
> > +
> > +    <group title="Common Columns">
> > +      <column name="external_ids">
> > +        See <em>External IDs</em> at the beginning of this document.
> > +      </column>
> > +    </group>
> > +  </table>
> >   </database>
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 50457c291a..3dd1920b79 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2130,3 +2130,84 @@ action=(reg0 = 0; reject { /* eth.dst <->
> eth.src; ip.dst <-> ip.src; is implici
> >   ])
> >
> >   AT_CLEANUP
> > +
> > +AT_SETUP([ovn -- NB to SB load balancer sync])
> > +ovn_start
> > +
> > +ovn-nbctl --wait=hv lb-add lb0 10.0.0.10:80 10.0.0.4:8080
> > +
> > +AT_CHECK([ovn-nbctl --bare --columns _uuid list load_balancer | wc -l],
> [0], [dnl
> > +1
> > +])
> > +
> > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l],
> [0], [dnl
> > +0
> > +])
> > +
> > +ovn-nbctl ls-add sw0
> > +ovn-nbctl --wait=hv ls-lb-add sw0 lb0
> > +sw0_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw0)
> > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
> load_balancer], [0], [dnl
> > +lb0
> > +10.0.0.10:80=10.0.0.4:8080
> > +tcp
> > +])
> > +
> > +lb0_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb0)
> > +
> > +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list
> load_balancer) = $sw0_sb_uuid])
> > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
> datapath sw0) = $lb0_uuid])
> > +
> > +ovn-nbctl --wait=sb set load_balancer . vips:"10.0.0.20\:90"="
> 20.0.0.4:8080,30.0.0.4:8080"
> > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
> load_balancer], [0], [dnl
> > +lb0
> > +10.0.0.10:80=10.0.0.4:8080 10.0.0.20:90=20.0.0.4:8080,30.0.0.4:8080
> > +tcp
> > +])
> > +
> > +ovn-nbctl lr-add lr0
> > +ovn-nbctl --wait=sb lr-lb-add lr0 lb0
> > +
> > +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list
> load_balancer) = $sw0_sb_uuid])
> > +
> > +ovn-nbctl ls-add sw1
> > +ovn-nbctl --wait=sb ls-lb-add sw1 lb0
> > +sw1_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw1)
> > +
> > +for i in $sw0_sb_uuid $sw1_sb_uuid; do echo $i >> dp_ids; done
> > +for i in $(ovn-sbctl --bare --columns datapaths list load_balancer); do
> echo $i >> lb_dps; done
> > +
> > +cat dp_ids | sort > expout
> > +AT_CHECK([cat lb_dps | sort], [0], [expout])
> > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
> datapath sw1) = $lb0_uuid])
> > +
> > +ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80 20.0.0.50:8080 udp
> > +
> > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l],
> [0], [dnl
> > +1
> > +])
> > +
> > +ovn-nbctl --wait=sb lr-lb-add lr0 lb1
> > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l],
> [0], [dnl
> > +1
> > +])
> > +
> > +ovn-nbctl --wait=sb ls-lb-add sw1 lb1
> > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
> load_balancer lb1 ], [0], [dnl
> > +lb1
> > +10.0.0.30:80=20.0.0.50:8080
> > +udp
> > +])
> > +
> > +lb1_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb1)
> > +
> > +for i in $lb0_uuid $lb1_uuid; do echo $i >> lb_ids; done
> > +cat lb_ids | sort > expout
> > +
> > +for i in $(ovn-sbctl --bare --columns load_balancers list
> datapath_binding sw1); do echo $i >> sw1_lbs; done
> > +AT_CHECK([cat sw1_lbs | sort], [0], [expout])
> > +
> > +ovn-nbctl --wait=sb lb-del lb1
> > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
> datapath_binding sw1) = $lb0_uuid])
> > +
> > +AT_CLEANUP
> > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> > index d3eec91332..e92e9187a5 100644
> > --- a/utilities/ovn-sbctl.c
> > +++ b/utilities/ovn-sbctl.c
> > @@ -1415,6 +1415,9 @@ static const struct ctl_table_class
> tables[SBREC_N_TABLES] = {
> >
> >       [SBREC_TABLE_HA_CHASSIS].row_ids[0]
> >       = {&sbrec_ha_chassis_col_chassis, NULL, NULL},
> > +
> > +    [SBREC_TABLE_LOAD_BALANCER].row_ids[0]
> > +    = {&sbrec_load_balancer_col_name, NULL, NULL},
> >   };
> >
> >
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Mark Michelson Oct. 23, 2020, 8:59 p.m. UTC | #3
On 10/23/20 4:21 PM, Numan Siddique wrote:
> 
> 
> On Sat, Oct 24, 2020, 1:29 AM Mark Michelson <mmichels@redhat.com 
> <mailto:mmichels@redhat.com>> wrote:
> 
>     On 10/21/20 3:25 AM, numans@ovn.org <mailto:numans@ovn.org> wrote:
>      > From: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>>
>      >
>      > This patch adds a new table 'Load_Balancer' in SB DB and syncs
>     the Load_Balancer table rows
>      > from NB DB to SB DB. An upcoming patch will make use of this
>     table for handling the
>      > load balancer hairpin traffic.
>      >
>      > Signed-off-by: Numan Siddique <numans@ovn.org
>     <mailto:numans@ovn.org>>
>      > ---
>      >   northd/ovn-northd.c   | 141
>     ++++++++++++++++++++++++++++++++++++++++++
>      >   ovn-sb.ovsschema      |  27 +++++++-
>      >   ovn-sb.xml            |  47 ++++++++++++++
>      >   tests/ovn-northd.at <http://ovn-northd.at>   |  81
>     ++++++++++++++++++++++++
>      >   utilities/ovn-sbctl.c |   3 +
>      >   5 files changed, 297 insertions(+), 2 deletions(-)
>      >
>      > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>      > index 5b76868dfb..ea771afda1 100644
>      > --- a/northd/ovn-northd.c
>      > +++ b/northd/ovn-northd.c
>      > @@ -11896,6 +11896,136 @@ sync_dns_entries(struct northd_context
>     *ctx, struct hmap *datapaths)
>      >       }
>      >       hmap_destroy(&dns_map);
>      >   }
>      > +
>      > +/*
>      > + * struct 'sync_lb_info' is used to sync the load balancer
>     records between
>      > + * OVN Northbound db and Southbound db.
>      > + */
>      > +struct sync_lb_info {
>      > +    struct hmap_node hmap_node;
>      > +    const struct nbrec_load_balancer *nlb; /* LB record in the
>     NB db. */
>      > +    const struct sbrec_load_balancer *slb; /* LB record in the
>     SB db. */
>      > +
>      > +    /* Datapaths to which the LB entry is associated with it. */
>      > +    const struct sbrec_datapath_binding **sbs;
>      > +    size_t n_sbs;
>      > +};
>      > +
>      > +static inline struct sync_lb_info *
>      > +get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid
>     *uuid)
>      > +{
>      > +    struct sync_lb_info *lb_info;
>      > +    size_t hash = uuid_hash(uuid);
>      > +    HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash,
>     sync_lb_map) {
>      > +        if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
>      > +            return lb_info;
>      > +        }
>      > +    }
>      > +
>      > +    return NULL;
>      > +}
>      > +
>      > +static void
>      > +sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
>      > +{
>      > +    struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
>      > +    struct ovn_datapath *od;
>      > +
>      > +    HMAP_FOR_EACH (od, key_node, datapaths) {
>      > +        if (!od->nbs || !od->nbs->n_load_balancer) {
>      > +            continue;
>      > +        }
>      > +
>      > +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
>      > +            struct sync_lb_info *lb_info =
>      > +                get_sync_lb_info_from_hmap(
>      > +                    &lb_map,
>     &od->nbs->load_balancer[i]->header_.uuid);
>      > +
>      > +            if (!lb_info) {
>      > +                size_t hash = uuid_hash(
>      > +                    &od->nbs->load_balancer[i]->header_.uuid);
>      > +                lb_info = xzalloc(sizeof *lb_info);;
>      > +                lb_info->nlb = od->nbs->load_balancer[i];
>      > +                hmap_insert(&lb_map, &lb_info->hmap_node, hash);
>      > +            }
>      > +
>      > +            lb_info->n_sbs++;
>      > +            lb_info->sbs = xrealloc(lb_info->sbs,
>      > +                                    lb_info->n_sbs * sizeof
>     *lb_info->sbs);
>      > +            lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
>      > +        }
>      > +    }
>      > +
>      > +    const struct sbrec_load_balancer *sbrec_lb, *next;
>      > +    SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next,
>     ctx->ovnsb_idl) {
>      > +        const char *nb_lb_uuid =
>     smap_get(&sbrec_lb->external_ids, "lb_id");
>      > +        struct uuid lb_uuid;
>      > +        if (!nb_lb_uuid || !uuid_from_string(&lb_uuid,
>     nb_lb_uuid)) {
>      > +            sbrec_load_balancer_delete(sbrec_lb);
>      > +            continue;
>      > +        }
>      > +
>      > +        struct sync_lb_info *lb_info =
>      > +            get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
>      > +        if (lb_info) {
>      > +            lb_info->slb = sbrec_lb;
>      > +        } else {
>      > +            sbrec_load_balancer_delete(sbrec_lb);
>      > +        }
>      > +    }
>      > +
>      > +    struct sync_lb_info *lb_info;
>      > +    HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
>      > +        if (!lb_info->slb) {
>      > +            sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
>      > +            lb_info->slb = sbrec_lb;
>      > +            char *lb_id = xasprintf(
>      > +                UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
>      > +            const struct smap external_ids =
>      > +                SMAP_CONST1(&external_ids, "lb_id", lb_id);
>      > +            sbrec_load_balancer_set_external_ids(sbrec_lb,
>     &external_ids);
>      > +            free(lb_id);
>      > +        }
>      > +
>      > +        /* Set the datapaths and other columns.  If nothing has
>     changed, then
>      > +         * this will be a no-op.
>      > +         */
>      > +        sbrec_load_balancer_set_datapaths(
>      > +            lb_info->slb,
>      > +            (struct sbrec_datapath_binding **)lb_info->sbs,
>      > +            lb_info->n_sbs);
>      > +
>      > +        sbrec_load_balancer_set_name(lb_info->slb,
>     lb_info->nlb->name);
>      > +        sbrec_load_balancer_set_vips(lb_info->slb,
>     &lb_info->nlb->vips);
>      > +        sbrec_load_balancer_set_protocol(lb_info->slb,
>     lb_info->nlb->protocol);
>      > +    }
>      > +
>      > +    HMAP_FOR_EACH (od, key_node, datapaths) {
>      > +        if (!od->nbs || !od->nbs->n_load_balancer) {
>      > +            continue;
>      > +        }
>      > +
>      > +        const struct sbrec_load_balancer **lbs =
>      > +            xmalloc(od->nbs->n_load_balancer * sizeof *lbs);
>      > +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
>      > +            lb_info = get_sync_lb_info_from_hmap(
>      > +                &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
>      > +            ovs_assert(lb_info);
>      > +            lbs[i] = lb_info->slb;
>      > +        }
>      > +
>      > +        sbrec_datapath_binding_set_load_balancers(
>      > +            od->sb, (struct sbrec_load_balancer **)lbs,
>      > +            od->nbs->n_load_balancer);
>      > +        free(lbs);
>      > +    }
>      > +
>      > +    HMAP_FOR_EACH_POP (lb_info, hmap_node, &lb_map) {
>      > +        free(lb_info->sbs);
>      > +        free(lb_info);
>      > +    }
>      > +    hmap_destroy(&lb_map);
>      > +}
>      >
>      >   static void
>      >   destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap
>     *ports,
>      > @@ -12263,6 +12393,7 @@ ovnnb_db_run(struct northd_context *ctx,
>      >       sync_port_groups(ctx, &port_groups);
>      >       sync_meters(ctx);
>      >       sync_dns_entries(ctx, datapaths);
>      > +    sync_lb_entries(ctx, datapaths);
>      >       destroy_ovn_lbs(&lbs);
>      >       hmap_destroy(&lbs);
>      >
>      > @@ -13022,6 +13153,8 @@ main(int argc, char *argv[])
>      >       ovsdb_idl_add_table(ovnsb_idl_loop.idl,
>     &sbrec_table_datapath_binding);
>      >       add_column_noalert(ovnsb_idl_loop.idl,
>      >                          &sbrec_datapath_binding_col_tunnel_key);
>      > +    add_column_noalert(ovnsb_idl_loop.idl,
>      > +                       &sbrec_datapath_binding_col_load_balancers);
>      >       add_column_noalert(ovnsb_idl_loop.idl,
>      >                          &sbrec_datapath_binding_col_external_ids);
>      >
>      > @@ -13189,6 +13322,14 @@ main(int argc, char *argv[])
>      >       add_column_noalert(ovnsb_idl_loop.idl,
>      >                          &sbrec_service_monitor_col_external_ids);
>      >
>      > +    ovsdb_idl_add_table(ovnsb_idl_loop.idl,
>     &sbrec_table_load_balancer);
>      > +    add_column_noalert(ovnsb_idl_loop.idl,
>     &sbrec_load_balancer_col_datapaths);
>      > +    add_column_noalert(ovnsb_idl_loop.idl,
>     &sbrec_load_balancer_col_name);
>      > +    add_column_noalert(ovnsb_idl_loop.idl,
>     &sbrec_load_balancer_col_vips);
>      > +    add_column_noalert(ovnsb_idl_loop.idl,
>     &sbrec_load_balancer_col_protocol);
>      > +    add_column_noalert(ovnsb_idl_loop.idl,
>      > +                       &sbrec_load_balancer_col_external_ids);
>      > +
>      >       struct ovsdb_idl_index *sbrec_chassis_by_name
>      >           = chassis_index_create(ovnsb_idl_loop.idl);
>      >
>      > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
>      > index d1c506a22c..7db6c6a4dd 100644
>      > --- a/ovn-sb.ovsschema
>      > +++ b/ovn-sb.ovsschema
>      > @@ -1,7 +1,7 @@
>      >   {
>      >       "name": "OVN_Southbound",
>      > -    "version": "2.10.0",
>      > -    "cksum": "2548342632 22615",
>      > +    "version": "2.11.0",
>      > +    "cksum": "1470439925 23814",
>      >       "tables": {
>      >           "SB_Global": {
>      >               "columns": {
>      > @@ -152,6 +152,11 @@
>      >                        "type": {"key": {"type": "integer",
>      >                                         "minInteger": 1,
>      >                                         "maxInteger": 16777215}}},
>      > +                "load_balancers": {"type": {"key": {"type": "uuid",
>      > +                                                   "refTable":
>     "Load_Balancer",
>      > +                                                   "refType":
>     "weak"},
>      > +                                            "min": 0,
>      > +                                            "max": "unlimited"}},
>      >                   "external_ids": {
>      >                       "type": {"key": "string", "value": "string",
>      >                                "min": 0, "max": "unlimited"}}},
>      > @@ -447,6 +452,24 @@
>      >                       "type": {"key": "string", "value": "string",
>      >                                "min": 0, "max": "unlimited"}}},
>      >               "indexes": [["logical_port", "ip", "port",
>     "protocol"]],
>      > +            "isRoot": true},
>      > +        "Load_Balancer": {
>      > +            "columns": {
>      > +                "name": {"type": "string"},
>      > +                "vips": {
>      > +                    "type": {"key": "string", "value": "string",
>      > +                             "min": 0, "max": "unlimited"}},
>      > +                "protocol": {
>      > +                    "type": {"key": {"type": "string",
>      > +                             "enum": ["set", ["tcp", "udp",
>     "sctp"]]},
>      > +                             "min": 0, "max": 1}},
>      > +                "datapaths": {
>      > +                    "type": {"key": {"type": "uuid",
>      > +                                     "refTable":
>     "Datapath_Binding"},
>      > +                             "min": 1, "max": "unlimited"}},
>      > +                "external_ids": {
>      > +                    "type": {"key": "string", "value": "string",
>      > +                             "min": 0, "max": "unlimited"}}},
>      >               "isRoot": true}
>      >       }
>      >   }
>      > diff --git a/ovn-sb.xml b/ovn-sb.xml
>      > index 182ff0a8a2..8638fa7eb4 100644
>      > --- a/ovn-sb.xml
>      > +++ b/ovn-sb.xml
>      > @@ -2497,6 +2497,12 @@ tcp.flags = RST;
>      >         constructed for each supported encapsulation.
>      >       </column>
>      >
>      > +    <column name="load_balancers">
>      > +      <p>
>      > +        Load balancers associated with the datapath.
>      > +      </p>
>      > +    </column>
>      > +
>      >       <group title="OVN_Northbound Relationship">
>      >         <p>
>      >           Each row in <ref table="Datapath_Binding"/> is
>     associated with some
>      > @@ -4104,4 +4110,45 @@ tcp.flags = RST;
>      >         </column>
>      >       </group>
>      >     </table>
>      > +
>      > +  <table name="Load_Balancer">
>      > +    <p>
>      > +      Each row represents one load balancer and corresponds directly
>      > +      with the <ref table="Load_Balancer"/> table in the
>      > +      <ref db="OVN_Northbound"/> database.
>      > +    </p>
> 
>     This isn't quite correct. Only load balancers associated with logical
>     switches are represented in the southbound database. Load balancers
>     used
>     by logical routers and load balancers that are not used by any
>     datapaths
>     are not put in the southbound database. Your tests even ensure that
>     this
>     is the case.
> 
> 
> That's the intention. Since hairpin lflows are added only to the logical 
> switch datapaths.
> 
> 
> Thanks
> Numan

Right, I figure this is a case where the code is correct, but the 
documentation is misleading. The wording of "corresponds directly with 
the Load_Balancer table" makes it sound like all northbound load 
balancers are represented in the southbound database. I think what you 
were meaning was that each individual southbound load balancers has a 
corresponding northbound load balancer. It still would be good to 
indicate in the docs that only northbound load balancers used by logical 
switches have corresponding southbound load balancers.

> 
> 
> 
>      > +
>      > +    <column name="name">
>      > +      A name for the load balancer.  This name has no special
>     meaning or
>      > +      purpose other than to provide convenience for human
>     interaction with
>      > +      the ovn-nb database.
>      > +    </column>
>      > +
>      > +    <column name="vips">
>      > +      A map of virtual IP addresses (and an optional port number
>     with
>      > +      <code>:</code> as a separator) associated with this load
>     balancer and
>      > +      their corresponding endpoint IP addresses (and optional
>     port numbers
>      > +      with <code>:</code> as separators) separated by commas.
>      > +    </column>
>      > +
>      > +    <column name="protocol">
>      > +      <p>
>      > +        Valid protocols are <code>tcp</code>, <code>udp</code>, or
>      > +        <code>sctp</code>.  This column is useful when a port
>     number is
>      > +        provided as part of the <code>vips</code> column.  If
>     this column is
>      > +        empty and a port number is provided as part of
>     <code>vips</code>
>      > +        column, OVN assumes the protocol to be <code>tcp</code>.
>      > +      </p>
>      > +    </column>
>      > +
>      > +    <column name="datapaths">
>      > +      Datapaths to which this load balancer applies to.
>      > +    </column>
>      > +
>      > +    <group title="Common Columns">
>      > +      <column name="external_ids">
>      > +        See <em>External IDs</em> at the beginning of this document.
>      > +      </column>
>      > +    </group>
>      > +  </table>
>      >   </database>
>      > diff --git a/tests/ovn-northd.at <http://ovn-northd.at>
>     b/tests/ovn-northd.at <http://ovn-northd.at>
>      > index 50457c291a..3dd1920b79 100644
>      > --- a/tests/ovn-northd.at <http://ovn-northd.at>
>      > +++ b/tests/ovn-northd.at <http://ovn-northd.at>
>      > @@ -2130,3 +2130,84 @@ action=(reg0 = 0; reject { /* eth.dst <->
>     eth.src; ip.dst <-> ip.src; is implici
>      >   ])
>      >
>      >   AT_CLEANUP
>      > +
>      > +AT_SETUP([ovn -- NB to SB load balancer sync])
>      > +ovn_start
>      > +
>      > +ovn-nbctl --wait=hv lb-add lb0 10.0.0.10:80
>     <http://10.0.0.10:80> 10.0.0.4:8080 <http://10.0.0.4:8080>
>      > +
>      > +AT_CHECK([ovn-nbctl --bare --columns _uuid list load_balancer |
>     wc -l], [0], [dnl
>      > +1
>      > +])
>      > +
>      > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer |
>     wc -l], [0], [dnl
>      > +0
>      > +])
>      > +
>      > +ovn-nbctl ls-add sw0
>      > +ovn-nbctl --wait=hv ls-lb-add sw0 lb0
>      > +sw0_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw0)
>      > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
>     load_balancer], [0], [dnl
>      > +lb0
>      > +10.0.0.10:80=10.0.0.4:8080 <http://10.0.0.4:8080>
>      > +tcp
>      > +])
>      > +
>      > +lb0_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb0)
>      > +
>      > +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list
>     load_balancer) = $sw0_sb_uuid])
>      > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
>     datapath sw0) = $lb0_uuid])
>      > +
>      > +ovn-nbctl --wait=sb set load_balancer .
>     vips:"10.0.0.20\:90"="20.0.0.4:8080
>     <http://20.0.0.4:8080>,30.0.0.4:8080 <http://30.0.0.4:8080>"
>      > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
>     load_balancer], [0], [dnl
>      > +lb0
>      > +10.0.0.10:80=10.0.0.4:8080 <http://10.0.0.4:8080>
>     10.0.0.20:90=20.0.0.4:8080 <http://20.0.0.4:8080>,30.0.0.4:8080
>     <http://30.0.0.4:8080>
>      > +tcp
>      > +])
>      > +
>      > +ovn-nbctl lr-add lr0
>      > +ovn-nbctl --wait=sb lr-lb-add lr0 lb0
>      > +
>      > +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list
>     load_balancer) = $sw0_sb_uuid])
>      > +
>      > +ovn-nbctl ls-add sw1
>      > +ovn-nbctl --wait=sb ls-lb-add sw1 lb0
>      > +sw1_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw1)
>      > +
>      > +for i in $sw0_sb_uuid $sw1_sb_uuid; do echo $i >> dp_ids; done
>      > +for i in $(ovn-sbctl --bare --columns datapaths list
>     load_balancer); do echo $i >> lb_dps; done
>      > +
>      > +cat dp_ids | sort > expout
>      > +AT_CHECK([cat lb_dps | sort], [0], [expout])
>      > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
>     datapath sw1) = $lb0_uuid])
>      > +
>      > +ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80
>     <http://10.0.0.30:80> 20.0.0.50:8080 <http://20.0.0.50:8080> udp
>      > +
>      > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer |
>     wc -l], [0], [dnl
>      > +1
>      > +])
>      > +
>      > +ovn-nbctl --wait=sb lr-lb-add lr0 lb1
>      > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer |
>     wc -l], [0], [dnl
>      > +1
>      > +])
>      > +
>      > +ovn-nbctl --wait=sb ls-lb-add sw1 lb1
>      > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
>     load_balancer lb1 ], [0], [dnl
>      > +lb1
>      > +10.0.0.30:80=20.0.0.50:8080 <http://20.0.0.50:8080>
>      > +udp
>      > +])
>      > +
>      > +lb1_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb1)
>      > +
>      > +for i in $lb0_uuid $lb1_uuid; do echo $i >> lb_ids; done
>      > +cat lb_ids | sort > expout
>      > +
>      > +for i in $(ovn-sbctl --bare --columns load_balancers list
>     datapath_binding sw1); do echo $i >> sw1_lbs; done
>      > +AT_CHECK([cat sw1_lbs | sort], [0], [expout])
>      > +
>      > +ovn-nbctl --wait=sb lb-del lb1
>      > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
>     datapath_binding sw1) = $lb0_uuid])
>      > +
>      > +AT_CLEANUP
>      > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
>      > index d3eec91332..e92e9187a5 100644
>      > --- a/utilities/ovn-sbctl.c
>      > +++ b/utilities/ovn-sbctl.c
>      > @@ -1415,6 +1415,9 @@ static const struct ctl_table_class
>     tables[SBREC_N_TABLES] = {
>      >
>      >       [SBREC_TABLE_HA_CHASSIS].row_ids[0]
>      >       = {&sbrec_ha_chassis_col_chassis, NULL, NULL},
>      > +
>      > +    [SBREC_TABLE_LOAD_BALANCER].row_ids[0]
>      > +    = {&sbrec_load_balancer_col_name, NULL, NULL},
>      >   };
>      >
>      >
>      >
> 
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org <mailto:dev@openvswitch.org>
>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique Oct. 26, 2020, 8:57 a.m. UTC | #4
On Sat, Oct 24, 2020 at 2:30 AM Mark Michelson <mmichels@redhat.com> wrote:
>
> On 10/23/20 4:21 PM, Numan Siddique wrote:
> >
> >
> > On Sat, Oct 24, 2020, 1:29 AM Mark Michelson <mmichels@redhat.com
> > <mailto:mmichels@redhat.com>> wrote:
> >
> >     On 10/21/20 3:25 AM, numans@ovn.org <mailto:numans@ovn.org> wrote:
> >      > From: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>>
> >      >
> >      > This patch adds a new table 'Load_Balancer' in SB DB and syncs
> >     the Load_Balancer table rows
> >      > from NB DB to SB DB. An upcoming patch will make use of this
> >     table for handling the
> >      > load balancer hairpin traffic.
> >      >
> >      > Signed-off-by: Numan Siddique <numans@ovn.org
> >     <mailto:numans@ovn.org>>
> >      > ---
> >      >   northd/ovn-northd.c   | 141
> >     ++++++++++++++++++++++++++++++++++++++++++
> >      >   ovn-sb.ovsschema      |  27 +++++++-
> >      >   ovn-sb.xml            |  47 ++++++++++++++
> >      >   tests/ovn-northd.at <http://ovn-northd.at>   |  81
> >     ++++++++++++++++++++++++
> >      >   utilities/ovn-sbctl.c |   3 +
> >      >   5 files changed, 297 insertions(+), 2 deletions(-)
> >      >
> >      > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> >      > index 5b76868dfb..ea771afda1 100644
> >      > --- a/northd/ovn-northd.c
> >      > +++ b/northd/ovn-northd.c
> >      > @@ -11896,6 +11896,136 @@ sync_dns_entries(struct northd_context
> >     *ctx, struct hmap *datapaths)
> >      >       }
> >      >       hmap_destroy(&dns_map);
> >      >   }
> >      > +
> >      > +/*
> >      > + * struct 'sync_lb_info' is used to sync the load balancer
> >     records between
> >      > + * OVN Northbound db and Southbound db.
> >      > + */
> >      > +struct sync_lb_info {
> >      > +    struct hmap_node hmap_node;
> >      > +    const struct nbrec_load_balancer *nlb; /* LB record in the
> >     NB db. */
> >      > +    const struct sbrec_load_balancer *slb; /* LB record in the
> >     SB db. */
> >      > +
> >      > +    /* Datapaths to which the LB entry is associated with it. */
> >      > +    const struct sbrec_datapath_binding **sbs;
> >      > +    size_t n_sbs;
> >      > +};
> >      > +
> >      > +static inline struct sync_lb_info *
> >      > +get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid
> >     *uuid)
> >      > +{
> >      > +    struct sync_lb_info *lb_info;
> >      > +    size_t hash = uuid_hash(uuid);
> >      > +    HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash,
> >     sync_lb_map) {
> >      > +        if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
> >      > +            return lb_info;
> >      > +        }
> >      > +    }
> >      > +
> >      > +    return NULL;
> >      > +}
> >      > +
> >      > +static void
> >      > +sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
> >      > +{
> >      > +    struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
> >      > +    struct ovn_datapath *od;
> >      > +
> >      > +    HMAP_FOR_EACH (od, key_node, datapaths) {
> >      > +        if (!od->nbs || !od->nbs->n_load_balancer) {
> >      > +            continue;
> >      > +        }
> >      > +
> >      > +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> >      > +            struct sync_lb_info *lb_info =
> >      > +                get_sync_lb_info_from_hmap(
> >      > +                    &lb_map,
> >     &od->nbs->load_balancer[i]->header_.uuid);
> >      > +
> >      > +            if (!lb_info) {
> >      > +                size_t hash = uuid_hash(
> >      > +                    &od->nbs->load_balancer[i]->header_.uuid);
> >      > +                lb_info = xzalloc(sizeof *lb_info);;
> >      > +                lb_info->nlb = od->nbs->load_balancer[i];
> >      > +                hmap_insert(&lb_map, &lb_info->hmap_node, hash);
> >      > +            }
> >      > +
> >      > +            lb_info->n_sbs++;
> >      > +            lb_info->sbs = xrealloc(lb_info->sbs,
> >      > +                                    lb_info->n_sbs * sizeof
> >     *lb_info->sbs);
> >      > +            lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
> >      > +        }
> >      > +    }
> >      > +
> >      > +    const struct sbrec_load_balancer *sbrec_lb, *next;
> >      > +    SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next,
> >     ctx->ovnsb_idl) {
> >      > +        const char *nb_lb_uuid =
> >     smap_get(&sbrec_lb->external_ids, "lb_id");
> >      > +        struct uuid lb_uuid;
> >      > +        if (!nb_lb_uuid || !uuid_from_string(&lb_uuid,
> >     nb_lb_uuid)) {
> >      > +            sbrec_load_balancer_delete(sbrec_lb);
> >      > +            continue;
> >      > +        }
> >      > +
> >      > +        struct sync_lb_info *lb_info =
> >      > +            get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
> >      > +        if (lb_info) {
> >      > +            lb_info->slb = sbrec_lb;
> >      > +        } else {
> >      > +            sbrec_load_balancer_delete(sbrec_lb);
> >      > +        }
> >      > +    }
> >      > +
> >      > +    struct sync_lb_info *lb_info;
> >      > +    HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
> >      > +        if (!lb_info->slb) {
> >      > +            sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
> >      > +            lb_info->slb = sbrec_lb;
> >      > +            char *lb_id = xasprintf(
> >      > +                UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
> >      > +            const struct smap external_ids =
> >      > +                SMAP_CONST1(&external_ids, "lb_id", lb_id);
> >      > +            sbrec_load_balancer_set_external_ids(sbrec_lb,
> >     &external_ids);
> >      > +            free(lb_id);
> >      > +        }
> >      > +
> >      > +        /* Set the datapaths and other columns.  If nothing has
> >     changed, then
> >      > +         * this will be a no-op.
> >      > +         */
> >      > +        sbrec_load_balancer_set_datapaths(
> >      > +            lb_info->slb,
> >      > +            (struct sbrec_datapath_binding **)lb_info->sbs,
> >      > +            lb_info->n_sbs);
> >      > +
> >      > +        sbrec_load_balancer_set_name(lb_info->slb,
> >     lb_info->nlb->name);
> >      > +        sbrec_load_balancer_set_vips(lb_info->slb,
> >     &lb_info->nlb->vips);
> >      > +        sbrec_load_balancer_set_protocol(lb_info->slb,
> >     lb_info->nlb->protocol);
> >      > +    }
> >      > +
> >      > +    HMAP_FOR_EACH (od, key_node, datapaths) {
> >      > +        if (!od->nbs || !od->nbs->n_load_balancer) {
> >      > +            continue;
> >      > +        }
> >      > +
> >      > +        const struct sbrec_load_balancer **lbs =
> >      > +            xmalloc(od->nbs->n_load_balancer * sizeof *lbs);
> >      > +        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> >      > +            lb_info = get_sync_lb_info_from_hmap(
> >      > +                &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
> >      > +            ovs_assert(lb_info);
> >      > +            lbs[i] = lb_info->slb;
> >      > +        }
> >      > +
> >      > +        sbrec_datapath_binding_set_load_balancers(
> >      > +            od->sb, (struct sbrec_load_balancer **)lbs,
> >      > +            od->nbs->n_load_balancer);
> >      > +        free(lbs);
> >      > +    }
> >      > +
> >      > +    HMAP_FOR_EACH_POP (lb_info, hmap_node, &lb_map) {
> >      > +        free(lb_info->sbs);
> >      > +        free(lb_info);
> >      > +    }
> >      > +    hmap_destroy(&lb_map);
> >      > +}
> >      >
> >      >   static void
> >      >   destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap
> >     *ports,
> >      > @@ -12263,6 +12393,7 @@ ovnnb_db_run(struct northd_context *ctx,
> >      >       sync_port_groups(ctx, &port_groups);
> >      >       sync_meters(ctx);
> >      >       sync_dns_entries(ctx, datapaths);
> >      > +    sync_lb_entries(ctx, datapaths);
> >      >       destroy_ovn_lbs(&lbs);
> >      >       hmap_destroy(&lbs);
> >      >
> >      > @@ -13022,6 +13153,8 @@ main(int argc, char *argv[])
> >      >       ovsdb_idl_add_table(ovnsb_idl_loop.idl,
> >     &sbrec_table_datapath_binding);
> >      >       add_column_noalert(ovnsb_idl_loop.idl,
> >      >                          &sbrec_datapath_binding_col_tunnel_key);
> >      > +    add_column_noalert(ovnsb_idl_loop.idl,
> >      > +                       &sbrec_datapath_binding_col_load_balancers);
> >      >       add_column_noalert(ovnsb_idl_loop.idl,
> >      >                          &sbrec_datapath_binding_col_external_ids);
> >      >
> >      > @@ -13189,6 +13322,14 @@ main(int argc, char *argv[])
> >      >       add_column_noalert(ovnsb_idl_loop.idl,
> >      >                          &sbrec_service_monitor_col_external_ids);
> >      >
> >      > +    ovsdb_idl_add_table(ovnsb_idl_loop.idl,
> >     &sbrec_table_load_balancer);
> >      > +    add_column_noalert(ovnsb_idl_loop.idl,
> >     &sbrec_load_balancer_col_datapaths);
> >      > +    add_column_noalert(ovnsb_idl_loop.idl,
> >     &sbrec_load_balancer_col_name);
> >      > +    add_column_noalert(ovnsb_idl_loop.idl,
> >     &sbrec_load_balancer_col_vips);
> >      > +    add_column_noalert(ovnsb_idl_loop.idl,
> >     &sbrec_load_balancer_col_protocol);
> >      > +    add_column_noalert(ovnsb_idl_loop.idl,
> >      > +                       &sbrec_load_balancer_col_external_ids);
> >      > +
> >      >       struct ovsdb_idl_index *sbrec_chassis_by_name
> >      >           = chassis_index_create(ovnsb_idl_loop.idl);
> >      >
> >      > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> >      > index d1c506a22c..7db6c6a4dd 100644
> >      > --- a/ovn-sb.ovsschema
> >      > +++ b/ovn-sb.ovsschema
> >      > @@ -1,7 +1,7 @@
> >      >   {
> >      >       "name": "OVN_Southbound",
> >      > -    "version": "2.10.0",
> >      > -    "cksum": "2548342632 22615",
> >      > +    "version": "2.11.0",
> >      > +    "cksum": "1470439925 23814",
> >      >       "tables": {
> >      >           "SB_Global": {
> >      >               "columns": {
> >      > @@ -152,6 +152,11 @@
> >      >                        "type": {"key": {"type": "integer",
> >      >                                         "minInteger": 1,
> >      >                                         "maxInteger": 16777215}}},
> >      > +                "load_balancers": {"type": {"key": {"type": "uuid",
> >      > +                                                   "refTable":
> >     "Load_Balancer",
> >      > +                                                   "refType":
> >     "weak"},
> >      > +                                            "min": 0,
> >      > +                                            "max": "unlimited"}},
> >      >                   "external_ids": {
> >      >                       "type": {"key": "string", "value": "string",
> >      >                                "min": 0, "max": "unlimited"}}},
> >      > @@ -447,6 +452,24 @@
> >      >                       "type": {"key": "string", "value": "string",
> >      >                                "min": 0, "max": "unlimited"}}},
> >      >               "indexes": [["logical_port", "ip", "port",
> >     "protocol"]],
> >      > +            "isRoot": true},
> >      > +        "Load_Balancer": {
> >      > +            "columns": {
> >      > +                "name": {"type": "string"},
> >      > +                "vips": {
> >      > +                    "type": {"key": "string", "value": "string",
> >      > +                             "min": 0, "max": "unlimited"}},
> >      > +                "protocol": {
> >      > +                    "type": {"key": {"type": "string",
> >      > +                             "enum": ["set", ["tcp", "udp",
> >     "sctp"]]},
> >      > +                             "min": 0, "max": 1}},
> >      > +                "datapaths": {
> >      > +                    "type": {"key": {"type": "uuid",
> >      > +                                     "refTable":
> >     "Datapath_Binding"},
> >      > +                             "min": 1, "max": "unlimited"}},
> >      > +                "external_ids": {
> >      > +                    "type": {"key": "string", "value": "string",
> >      > +                             "min": 0, "max": "unlimited"}}},
> >      >               "isRoot": true}
> >      >       }
> >      >   }
> >      > diff --git a/ovn-sb.xml b/ovn-sb.xml
> >      > index 182ff0a8a2..8638fa7eb4 100644
> >      > --- a/ovn-sb.xml
> >      > +++ b/ovn-sb.xml
> >      > @@ -2497,6 +2497,12 @@ tcp.flags = RST;
> >      >         constructed for each supported encapsulation.
> >      >       </column>
> >      >
> >      > +    <column name="load_balancers">
> >      > +      <p>
> >      > +        Load balancers associated with the datapath.
> >      > +      </p>
> >      > +    </column>
> >      > +
> >      >       <group title="OVN_Northbound Relationship">
> >      >         <p>
> >      >           Each row in <ref table="Datapath_Binding"/> is
> >     associated with some
> >      > @@ -4104,4 +4110,45 @@ tcp.flags = RST;
> >      >         </column>
> >      >       </group>
> >      >     </table>
> >      > +
> >      > +  <table name="Load_Balancer">
> >      > +    <p>
> >      > +      Each row represents one load balancer and corresponds directly
> >      > +      with the <ref table="Load_Balancer"/> table in the
> >      > +      <ref db="OVN_Northbound"/> database.
> >      > +    </p>
> >
> >     This isn't quite correct. Only load balancers associated with logical
> >     switches are represented in the southbound database. Load balancers
> >     used
> >     by logical routers and load balancers that are not used by any
> >     datapaths
> >     are not put in the southbound database. Your tests even ensure that
> >     this
> >     is the case.
> >
> >
> > That's the intention. Since hairpin lflows are added only to the logical
> > switch datapaths.
> >
> >
> > Thanks
> > Numan
>
> Right, I figure this is a case where the code is correct, but the
> documentation is misleading. The wording of "corresponds directly with
> the Load_Balancer table" makes it sound like all northbound load
> balancers are represented in the southbound database. I think what you
> were meaning was that each individual southbound load balancers has a
> corresponding northbound load balancer. It still would be good to
> indicate in the docs that only northbound load balancers used by logical
> switches have corresponding southbound load balancers.

Ack.

I will just update the documentation as

<p>
  Each row represents a load balancer.
</p>

Let me know if you have any comments.

Thanks
Numan


>
> >
> >
> >
> >      > +
> >      > +    <column name="name">
> >      > +      A name for the load balancer.  This name has no special
> >     meaning or
> >      > +      purpose other than to provide convenience for human
> >     interaction with
> >      > +      the ovn-nb database.
> >      > +    </column>
> >      > +
> >      > +    <column name="vips">
> >      > +      A map of virtual IP addresses (and an optional port number
> >     with
> >      > +      <code>:</code> as a separator) associated with this load
> >     balancer and
> >      > +      their corresponding endpoint IP addresses (and optional
> >     port numbers
> >      > +      with <code>:</code> as separators) separated by commas.
> >      > +    </column>
> >      > +
> >      > +    <column name="protocol">
> >      > +      <p>
> >      > +        Valid protocols are <code>tcp</code>, <code>udp</code>, or
> >      > +        <code>sctp</code>.  This column is useful when a port
> >     number is
> >      > +        provided as part of the <code>vips</code> column.  If
> >     this column is
> >      > +        empty and a port number is provided as part of
> >     <code>vips</code>
> >      > +        column, OVN assumes the protocol to be <code>tcp</code>.
> >      > +      </p>
> >      > +    </column>
> >      > +
> >      > +    <column name="datapaths">
> >      > +      Datapaths to which this load balancer applies to.
> >      > +    </column>
> >      > +
> >      > +    <group title="Common Columns">
> >      > +      <column name="external_ids">
> >      > +        See <em>External IDs</em> at the beginning of this document.
> >      > +      </column>
> >      > +    </group>
> >      > +  </table>
> >      >   </database>
> >      > diff --git a/tests/ovn-northd.at <http://ovn-northd.at>
> >     b/tests/ovn-northd.at <http://ovn-northd.at>
> >      > index 50457c291a..3dd1920b79 100644
> >      > --- a/tests/ovn-northd.at <http://ovn-northd.at>
> >      > +++ b/tests/ovn-northd.at <http://ovn-northd.at>
> >      > @@ -2130,3 +2130,84 @@ action=(reg0 = 0; reject { /* eth.dst <->
> >     eth.src; ip.dst <-> ip.src; is implici
> >      >   ])
> >      >
> >      >   AT_CLEANUP
> >      > +
> >      > +AT_SETUP([ovn -- NB to SB load balancer sync])
> >      > +ovn_start
> >      > +
> >      > +ovn-nbctl --wait=hv lb-add lb0 10.0.0.10:80
> >     <http://10.0.0.10:80> 10.0.0.4:8080 <http://10.0.0.4:8080>
> >      > +
> >      > +AT_CHECK([ovn-nbctl --bare --columns _uuid list load_balancer |
> >     wc -l], [0], [dnl
> >      > +1
> >      > +])
> >      > +
> >      > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer |
> >     wc -l], [0], [dnl
> >      > +0
> >      > +])
> >      > +
> >      > +ovn-nbctl ls-add sw0
> >      > +ovn-nbctl --wait=hv ls-lb-add sw0 lb0
> >      > +sw0_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw0)
> >      > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
> >     load_balancer], [0], [dnl
> >      > +lb0
> >      > +10.0.0.10:80=10.0.0.4:8080 <http://10.0.0.4:8080>
> >      > +tcp
> >      > +])
> >      > +
> >      > +lb0_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb0)
> >      > +
> >      > +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list
> >     load_balancer) = $sw0_sb_uuid])
> >      > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
> >     datapath sw0) = $lb0_uuid])
> >      > +
> >      > +ovn-nbctl --wait=sb set load_balancer .
> >     vips:"10.0.0.20\:90"="20.0.0.4:8080
> >     <http://20.0.0.4:8080>,30.0.0.4:8080 <http://30.0.0.4:8080>"
> >      > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
> >     load_balancer], [0], [dnl
> >      > +lb0
> >      > +10.0.0.10:80=10.0.0.4:8080 <http://10.0.0.4:8080>
> >     10.0.0.20:90=20.0.0.4:8080 <http://20.0.0.4:8080>,30.0.0.4:8080
> >     <http://30.0.0.4:8080>
> >      > +tcp
> >      > +])
> >      > +
> >      > +ovn-nbctl lr-add lr0
> >      > +ovn-nbctl --wait=sb lr-lb-add lr0 lb0
> >      > +
> >      > +AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list
> >     load_balancer) = $sw0_sb_uuid])
> >      > +
> >      > +ovn-nbctl ls-add sw1
> >      > +ovn-nbctl --wait=sb ls-lb-add sw1 lb0
> >      > +sw1_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw1)
> >      > +
> >      > +for i in $sw0_sb_uuid $sw1_sb_uuid; do echo $i >> dp_ids; done
> >      > +for i in $(ovn-sbctl --bare --columns datapaths list
> >     load_balancer); do echo $i >> lb_dps; done
> >      > +
> >      > +cat dp_ids | sort > expout
> >      > +AT_CHECK([cat lb_dps | sort], [0], [expout])
> >      > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
> >     datapath sw1) = $lb0_uuid])
> >      > +
> >      > +ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80
> >     <http://10.0.0.30:80> 20.0.0.50:8080 <http://20.0.0.50:8080> udp
> >      > +
> >      > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer |
> >     wc -l], [0], [dnl
> >      > +1
> >      > +])
> >      > +
> >      > +ovn-nbctl --wait=sb lr-lb-add lr0 lb1
> >      > +AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer |
> >     wc -l], [0], [dnl
> >      > +1
> >      > +])
> >      > +
> >      > +ovn-nbctl --wait=sb ls-lb-add sw1 lb1
> >      > +AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list
> >     load_balancer lb1 ], [0], [dnl
> >      > +lb1
> >      > +10.0.0.30:80=20.0.0.50:8080 <http://20.0.0.50:8080>
> >      > +udp
> >      > +])
> >      > +
> >      > +lb1_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb1)
> >      > +
> >      > +for i in $lb0_uuid $lb1_uuid; do echo $i >> lb_ids; done
> >      > +cat lb_ids | sort > expout
> >      > +
> >      > +for i in $(ovn-sbctl --bare --columns load_balancers list
> >     datapath_binding sw1); do echo $i >> sw1_lbs; done
> >      > +AT_CHECK([cat sw1_lbs | sort], [0], [expout])
> >      > +
> >      > +ovn-nbctl --wait=sb lb-del lb1
> >      > +AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list
> >     datapath_binding sw1) = $lb0_uuid])
> >      > +
> >      > +AT_CLEANUP
> >      > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> >      > index d3eec91332..e92e9187a5 100644
> >      > --- a/utilities/ovn-sbctl.c
> >      > +++ b/utilities/ovn-sbctl.c
> >      > @@ -1415,6 +1415,9 @@ static const struct ctl_table_class
> >     tables[SBREC_N_TABLES] = {
> >      >
> >      >       [SBREC_TABLE_HA_CHASSIS].row_ids[0]
> >      >       = {&sbrec_ha_chassis_col_chassis, NULL, NULL},
> >      > +
> >      > +    [SBREC_TABLE_LOAD_BALANCER].row_ids[0]
> >      > +    = {&sbrec_load_balancer_col_name, NULL, NULL},
> >      >   };
> >      >
> >      >
> >      >
> >
> >     _______________________________________________
> >     dev mailing list
> >     dev@openvswitch.org <mailto:dev@openvswitch.org>
> >     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 5b76868dfb..ea771afda1 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11896,6 +11896,136 @@  sync_dns_entries(struct northd_context *ctx, struct hmap *datapaths)
     }
     hmap_destroy(&dns_map);
 }
+
+/*
+ * struct 'sync_lb_info' is used to sync the load balancer records between
+ * OVN Northbound db and Southbound db.
+ */
+struct sync_lb_info {
+    struct hmap_node hmap_node;
+    const struct nbrec_load_balancer *nlb; /* LB record in the NB db. */
+    const struct sbrec_load_balancer *slb; /* LB record in the SB db. */
+
+    /* Datapaths to which the LB entry is associated with it. */
+    const struct sbrec_datapath_binding **sbs;
+    size_t n_sbs;
+};
+
+static inline struct sync_lb_info *
+get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid *uuid)
+{
+    struct sync_lb_info *lb_info;
+    size_t hash = uuid_hash(uuid);
+    HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash, sync_lb_map) {
+        if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
+            return lb_info;
+        }
+    }
+
+    return NULL;
+}
+
+static void
+sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
+{
+    struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
+    struct ovn_datapath *od;
+
+    HMAP_FOR_EACH (od, key_node, datapaths) {
+        if (!od->nbs || !od->nbs->n_load_balancer) {
+            continue;
+        }
+
+        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
+            struct sync_lb_info *lb_info =
+                get_sync_lb_info_from_hmap(
+                    &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
+
+            if (!lb_info) {
+                size_t hash = uuid_hash(
+                    &od->nbs->load_balancer[i]->header_.uuid);
+                lb_info = xzalloc(sizeof *lb_info);;
+                lb_info->nlb = od->nbs->load_balancer[i];
+                hmap_insert(&lb_map, &lb_info->hmap_node, hash);
+            }
+
+            lb_info->n_sbs++;
+            lb_info->sbs = xrealloc(lb_info->sbs,
+                                    lb_info->n_sbs * sizeof *lb_info->sbs);
+            lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
+        }
+    }
+
+    const struct sbrec_load_balancer *sbrec_lb, *next;
+    SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next, ctx->ovnsb_idl) {
+        const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
+        struct uuid lb_uuid;
+        if (!nb_lb_uuid || !uuid_from_string(&lb_uuid, nb_lb_uuid)) {
+            sbrec_load_balancer_delete(sbrec_lb);
+            continue;
+        }
+
+        struct sync_lb_info *lb_info =
+            get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
+        if (lb_info) {
+            lb_info->slb = sbrec_lb;
+        } else {
+            sbrec_load_balancer_delete(sbrec_lb);
+        }
+    }
+
+    struct sync_lb_info *lb_info;
+    HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
+        if (!lb_info->slb) {
+            sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
+            lb_info->slb = sbrec_lb;
+            char *lb_id = xasprintf(
+                UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
+            const struct smap external_ids =
+                SMAP_CONST1(&external_ids, "lb_id", lb_id);
+            sbrec_load_balancer_set_external_ids(sbrec_lb, &external_ids);
+            free(lb_id);
+        }
+
+        /* Set the datapaths and other columns.  If nothing has changed, then
+         * this will be a no-op.
+         */
+        sbrec_load_balancer_set_datapaths(
+            lb_info->slb,
+            (struct sbrec_datapath_binding **)lb_info->sbs,
+            lb_info->n_sbs);
+
+        sbrec_load_balancer_set_name(lb_info->slb, lb_info->nlb->name);
+        sbrec_load_balancer_set_vips(lb_info->slb, &lb_info->nlb->vips);
+        sbrec_load_balancer_set_protocol(lb_info->slb, lb_info->nlb->protocol);
+    }
+
+    HMAP_FOR_EACH (od, key_node, datapaths) {
+        if (!od->nbs || !od->nbs->n_load_balancer) {
+            continue;
+        }
+
+        const struct sbrec_load_balancer **lbs =
+            xmalloc(od->nbs->n_load_balancer * sizeof *lbs);
+        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
+            lb_info = get_sync_lb_info_from_hmap(
+                &lb_map, &od->nbs->load_balancer[i]->header_.uuid);
+            ovs_assert(lb_info);
+            lbs[i] = lb_info->slb;
+        }
+
+        sbrec_datapath_binding_set_load_balancers(
+            od->sb, (struct sbrec_load_balancer **)lbs,
+            od->nbs->n_load_balancer);
+        free(lbs);
+    }
+
+    HMAP_FOR_EACH_POP (lb_info, hmap_node, &lb_map) {
+        free(lb_info->sbs);
+        free(lb_info);
+    }
+    hmap_destroy(&lb_map);
+}
 
 static void
 destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap *ports,
@@ -12263,6 +12393,7 @@  ovnnb_db_run(struct northd_context *ctx,
     sync_port_groups(ctx, &port_groups);
     sync_meters(ctx);
     sync_dns_entries(ctx, datapaths);
+    sync_lb_entries(ctx, datapaths);
     destroy_ovn_lbs(&lbs);
     hmap_destroy(&lbs);
 
@@ -13022,6 +13153,8 @@  main(int argc, char *argv[])
     ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_datapath_binding);
     add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_datapath_binding_col_tunnel_key);
+    add_column_noalert(ovnsb_idl_loop.idl,
+                       &sbrec_datapath_binding_col_load_balancers);
     add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_datapath_binding_col_external_ids);
 
@@ -13189,6 +13322,14 @@  main(int argc, char *argv[])
     add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_service_monitor_col_external_ids);
 
+    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_load_balancer);
+    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_datapaths);
+    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_name);
+    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_vips);
+    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_load_balancer_col_protocol);
+    add_column_noalert(ovnsb_idl_loop.idl,
+                       &sbrec_load_balancer_col_external_ids);
+
     struct ovsdb_idl_index *sbrec_chassis_by_name
         = chassis_index_create(ovnsb_idl_loop.idl);
 
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index d1c506a22c..7db6c6a4dd 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Southbound",
-    "version": "2.10.0",
-    "cksum": "2548342632 22615",
+    "version": "2.11.0",
+    "cksum": "1470439925 23814",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -152,6 +152,11 @@ 
                      "type": {"key": {"type": "integer",
                                       "minInteger": 1,
                                       "maxInteger": 16777215}}},
+                "load_balancers": {"type": {"key": {"type": "uuid",
+                                                   "refTable": "Load_Balancer",
+                                                   "refType": "weak"},
+                                            "min": 0,
+                                            "max": "unlimited"}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
@@ -447,6 +452,24 @@ 
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
             "indexes": [["logical_port", "ip", "port", "protocol"]],
+            "isRoot": true},
+        "Load_Balancer": {
+            "columns": {
+                "name": {"type": "string"},
+                "vips": {
+                    "type": {"key": "string", "value": "string",
+                             "min": 0, "max": "unlimited"}},
+                "protocol": {
+                    "type": {"key": {"type": "string",
+                             "enum": ["set", ["tcp", "udp", "sctp"]]},
+                             "min": 0, "max": 1}},
+                "datapaths": {
+                    "type": {"key": {"type": "uuid",
+                                     "refTable": "Datapath_Binding"},
+                             "min": 1, "max": "unlimited"}},
+                "external_ids": {
+                    "type": {"key": "string", "value": "string",
+                             "min": 0, "max": "unlimited"}}},
             "isRoot": true}
     }
 }
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 182ff0a8a2..8638fa7eb4 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -2497,6 +2497,12 @@  tcp.flags = RST;
       constructed for each supported encapsulation.
     </column>
 
+    <column name="load_balancers">
+      <p>
+        Load balancers associated with the datapath.
+      </p>
+    </column>
+
     <group title="OVN_Northbound Relationship">
       <p>
         Each row in <ref table="Datapath_Binding"/> is associated with some
@@ -4104,4 +4110,45 @@  tcp.flags = RST;
       </column>
     </group>
   </table>
+
+  <table name="Load_Balancer">
+    <p>
+      Each row represents one load balancer and corresponds directly
+      with the <ref table="Load_Balancer"/> table in the
+      <ref db="OVN_Northbound"/> database.
+    </p>
+
+    <column name="name">
+      A name for the load balancer.  This name has no special meaning or
+      purpose other than to provide convenience for human interaction with
+      the ovn-nb database.
+    </column>
+
+    <column name="vips">
+      A map of virtual IP addresses (and an optional port number with
+      <code>:</code> as a separator) associated with this load balancer and
+      their corresponding endpoint IP addresses (and optional port numbers
+      with <code>:</code> as separators) separated by commas.
+    </column>
+
+    <column name="protocol">
+      <p>
+        Valid protocols are <code>tcp</code>, <code>udp</code>, or
+        <code>sctp</code>.  This column is useful when a port number is
+        provided as part of the <code>vips</code> column.  If this column is
+        empty and a port number is provided as part of <code>vips</code>
+        column, OVN assumes the protocol to be <code>tcp</code>.
+      </p>
+    </column>
+
+    <column name="datapaths">
+      Datapaths to which this load balancer applies to.
+    </column>
+
+    <group title="Common Columns">
+      <column name="external_ids">
+        See <em>External IDs</em> at the beginning of this document.
+      </column>
+    </group>
+  </table>
 </database>
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 50457c291a..3dd1920b79 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2130,3 +2130,84 @@  action=(reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is implici
 ])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- NB to SB load balancer sync])
+ovn_start
+
+ovn-nbctl --wait=hv lb-add lb0 10.0.0.10:80 10.0.0.4:8080
+
+AT_CHECK([ovn-nbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
+1
+])
+
+AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
+0
+])
+
+ovn-nbctl ls-add sw0
+ovn-nbctl --wait=hv ls-lb-add sw0 lb0
+sw0_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw0)
+AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list load_balancer], [0], [dnl
+lb0
+10.0.0.10:80=10.0.0.4:8080
+tcp
+])
+
+lb0_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb0)
+
+AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list load_balancer) = $sw0_sb_uuid])
+AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list datapath sw0) = $lb0_uuid])
+
+ovn-nbctl --wait=sb set load_balancer . vips:"10.0.0.20\:90"="20.0.0.4:8080,30.0.0.4:8080"
+AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list load_balancer], [0], [dnl
+lb0
+10.0.0.10:80=10.0.0.4:8080 10.0.0.20:90=20.0.0.4:8080,30.0.0.4:8080
+tcp
+])
+
+ovn-nbctl lr-add lr0
+ovn-nbctl --wait=sb lr-lb-add lr0 lb0
+
+AT_CHECK([test $(ovn-sbctl --bare --columns datapaths list load_balancer) = $sw0_sb_uuid])
+
+ovn-nbctl ls-add sw1
+ovn-nbctl --wait=sb ls-lb-add sw1 lb0
+sw1_sb_uuid=$(ovn-sbctl --bare --columns _uuid list datapath sw1)
+
+for i in $sw0_sb_uuid $sw1_sb_uuid; do echo $i >> dp_ids; done
+for i in $(ovn-sbctl --bare --columns datapaths list load_balancer); do echo $i >> lb_dps; done
+
+cat dp_ids | sort > expout
+AT_CHECK([cat lb_dps | sort], [0], [expout])
+AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list datapath sw1) = $lb0_uuid])
+
+ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80 20.0.0.50:8080 udp
+
+AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
+1
+])
+
+ovn-nbctl --wait=sb lr-lb-add lr0 lb1
+AT_CHECK([ovn-sbctl --bare --columns _uuid list load_balancer | wc -l], [0], [dnl
+1
+])
+
+ovn-nbctl --wait=sb ls-lb-add sw1 lb1
+AT_CHECK([ovn-sbctl --bare --columns name,vips,protocol list load_balancer lb1 ], [0], [dnl
+lb1
+10.0.0.30:80=20.0.0.50:8080
+udp
+])
+
+lb1_uuid=$(ovn-sbctl --bare --columns _uuid list load_balancer lb1)
+
+for i in $lb0_uuid $lb1_uuid; do echo $i >> lb_ids; done
+cat lb_ids | sort > expout
+
+for i in $(ovn-sbctl --bare --columns load_balancers list datapath_binding sw1); do echo $i >> sw1_lbs; done
+AT_CHECK([cat sw1_lbs | sort], [0], [expout])
+
+ovn-nbctl --wait=sb lb-del lb1
+AT_CHECK([test $(ovn-sbctl --bare --columns load_balancers list datapath_binding sw1) = $lb0_uuid])
+
+AT_CLEANUP
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index d3eec91332..e92e9187a5 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -1415,6 +1415,9 @@  static const struct ctl_table_class tables[SBREC_N_TABLES] = {
 
     [SBREC_TABLE_HA_CHASSIS].row_ids[0]
     = {&sbrec_ha_chassis_col_chassis, NULL, NULL},
+
+    [SBREC_TABLE_LOAD_BALANCER].row_ids[0]
+    = {&sbrec_load_balancer_col_name, NULL, NULL},
 };