Hi Dumitru,

Thank you so much for making these changes and applying the patch.

Sragdhara

From: Dumitru Ceara <[email protected]>
Date: Friday, June 27, 2025 at 5:16 AM
To: Sragdhara Datta Chaudhuri <[email protected]>
Cc: [email protected] <[email protected]>, Lorenzo Bianconi 
<[email protected]>
Subject: Re: [ovs-dev] [PATCH OVN v2] ic: Advertise and learn routes for 
loadbalancer VIPs.
!-------------------------------------------------------------------|
  CAUTION: External Email

|-------------------------------------------------------------------!

On 6/12/25 11:09 PM, Lorenzo Bianconi via dev wrote:
>> In an interconnect topology, when a logical router (LR) is connected to a
>> transit-switch (TS), the static routes on the LR as well as routes for the
>> connected subnets get advertised to the other sites via the IC-SB.
>>
>> This commit adds two new nb_global options ic-route-adv-lb and
>> ic-route-learn-lb, which will help propagate the LB VIP based routes.
>> Default value for both the options is false. Also these options come into
>> effect only if the exisiting options ic-route-adv and ic-route-learn are
>> true.
>>
>> ic-route-adv-lb: If this is set to true, add routes in the IC-SB for the
>> VIPs associated with the LB instances linked to the logical router. These
>> routes would have the VIP as destination and IP of the LRP connected to TS
>> as next hop The origin would be set as "loadbalancer".
>>
>> ic-route-learn-lb: If this is set to true, routes with origin loadbalancer
>> are synced from the IC-SB and applied to the LR as learned route.
>>
>> This change makes services hosted behind LB in one site become accessible by
>> clients in other locations, via interconnect.
>>
>> Signed-off-by: Sragdhara Datta Chaudhuri <[email protected]>
>
> Acked-by: Lorenzo Bianconi <[email protected]>
>

Hi Sragdhara, Lorenzo,

Thanks for the patch and review!

>>
>> ---
>> v2: Address Lorenzo's comments.
>> ---
>>  ic/ovn-ic.c         | 198 +++++++++++++++++++++++++++--
>>  lib/ovn-util.h      |   1 +
>>  ovn-ic-sb.ovsschema |   7 +-
>>  ovn-ic-sb.xml       |  10 +-
>>  ovn-nb.xml          |  21 +++-
>>  tests/ovn-ic.at     | 295 ++++++++++++++++++++++++++++++++++++++++++++

This new feature misses a NEWS entry.

>>  6 files changed, 517 insertions(+), 15 deletions(-)
>>
>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
>> index c8796680b..f1abdcb05 100644
>> --- a/ic/ovn-ic.c
>> +++ b/ic/ovn-ic.c
>> @@ -939,14 +939,17 @@ struct ic_route_info {
>>
>>      const struct nbrec_logical_router *nb_lr;
>>
>> -    /* Either nb_route or nb_lrp is set and the other one must be NULL.
>> +    /* One of nb_route, nb_lrp, nb_lb is set and the other ones must be 
>> NULL.
>>       * - For a route that is learned from IC-SB, or a static route that is
>>       *   generated from a route that is configured in NB, the "nb_route"
>>       *   is set.
>>       * - For a route that is generated from a direct-connect subnet of
>> -     *   a logical router port, the "nb_lrp" is set. */
>> +     *   a logical router port, the "nb_lrp" is set.
>> +     * - For a route that is generated from a load-balancer vip of
>> +     *   a logical router, the "nb_lb" is set. */
>>      const struct nbrec_logical_router_static_route *nb_route;
>>      const struct nbrec_logical_router_port *nb_lrp;
>> +    const struct nbrec_load_balancer *nb_lb;
>>  };
>>
>>  static uint32_t
>> @@ -1163,9 +1166,10 @@ add_to_routes_ad(struct hmap *routes_ad, const struct 
>> in6_addr prefix,
>>                   const struct nbrec_logical_router_port *nb_lrp,
>>                   const struct nbrec_logical_router_static_route *nb_route,
>>                   const struct nbrec_logical_router *nb_lr,
>> +                 const struct nbrec_load_balancer *nb_lb,
>>                   const char *route_tag)
>>  {
>> -    ovs_assert(nb_route || nb_lrp);
>> +    ovs_assert(nb_route || nb_lrp || nb_lb);
>>
>>      if (route_table == NULL) {
>>          route_table = "";
>> @@ -1184,6 +1188,7 @@ add_to_routes_ad(struct hmap *routes_ad, const struct 
>> in6_addr prefix,
>>          ic_route->route_table = route_table;
>>          ic_route->nb_lrp = nb_lrp;
>>          ic_route->nb_lr = nb_lr;
>> +        ic_route->nb_lb = nb_lb;
>>          ic_route->route_tag = route_tag;
>>          hmap_insert(routes_ad, &ic_route->node, hash);
>>      } else {
>> @@ -1193,6 +1198,9 @@ add_to_routes_ad(struct hmap *routes_ad, const struct 
>> in6_addr prefix,
>>          if (nb_route) {
>>              VLOG_WARN_RL(&rl, msg_fmt, origin, "route",
>>                           UUID_ARGS(&nb_route->header_.uuid));
>> +        } else if (nb_lb) {
>> +            VLOG_WARN_RL(&rl, msg_fmt, origin, "loadbalancer",
>> +                         UUID_ARGS(&nb_lb->header_.uuid));
>>          } else {
>>              VLOG_WARN_RL(&rl, msg_fmt, origin, "lrp",
>>                           UUID_ARGS(&nb_lrp->header_.uuid));
>> @@ -1248,7 +1256,8 @@ add_static_to_routes_ad(
>>      }
>>
>>      add_to_routes_ad(routes_ad, prefix, plen, nexthop, ROUTE_ORIGIN_STATIC,
>> -                     nb_route->route_table, NULL, nb_route, nb_lr, 
>> route_tag);
>> +                     nb_route->route_table, NULL, nb_route, nb_lr,
>> +                     NULL, route_tag);
>>  }
>>
>>  static void
>> @@ -1296,7 +1305,65 @@ add_network_to_routes_ad(struct hmap *routes_ad, 
>> const char *network,
>>
>>      /* directly-connected routes go to <main> route table */
>>      add_to_routes_ad(routes_ad, prefix, plen, nexthop, 
>> ROUTE_ORIGIN_CONNECTED,
>> -                     NULL, nb_lrp, NULL, nb_lr, route_tag);
>> +                     NULL, nb_lrp, NULL, nb_lr, NULL, route_tag);
>> +}
>> +
>> +static void
>> +add_lb_vip_to_routes_ad(struct hmap *routes_ad, const char *vip_key,
>> +                        const struct nbrec_load_balancer *nb_lb,
>> +                        const struct lport_addresses *nexthop_addresses,
>> +                        const struct smap *nb_options,
>> +                        const struct nbrec_logical_router *nb_lr,
>> +                        const char *route_tag)
>> +{
>> +    char *vip_str = NULL;
>> +    struct in6_addr vip_ip, nexthop;
>> +    uint16_t vip_port;
>> +    int addr_family;
>> +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> +
>> +    if (!ip_address_and_port_from_lb_key(vip_key, &vip_str, &vip_ip,
>> +                                         &vip_port, &addr_family)) {
>> +        VLOG_WARN_RL(&rl, "Route ad: Parsing failed for lb vip %s", 
>> vip_key);
>> +        return;
>> +    }
>> +    if (vip_str == NULL) {
>> +        return;
>> +    }
>> +    unsigned int plen = (addr_family == AF_INET) ? 32 : 128;
>> +    if (!route_need_advertise(NULL, &vip_ip, plen, nb_options)) {
>> +        VLOG_DBG("Route ad: skip lb vip %s.", vip_key);
>> +        goto out;
>> +    }
>> +    if (!get_nexthop_from_lport_addresses(IN6_IS_ADDR_V4MAPPED(&vip_ip),
>> +                                          nexthop_addresses,
>> +                                          &nexthop)) {
>> +        VLOG_WARN_RL(&rl, "Route ad: failed to get nexthop for lb vip");
>> +        goto out;
>> +    }
>> +
>> +    if (VLOG_IS_DBG_ENABLED()) {
>> +        struct ds msg = DS_EMPTY_INITIALIZER;
>> +
>> +        ds_put_format(&msg, "Adding lb vip route to <main> routing "
>> +                      "table: %s, nexthop ", vip_str);
>> +
>> +        if (IN6_IS_ADDR_V4MAPPED(&nexthop)) {
>> +            ds_put_format(&msg, IP_FMT,
>> +                          IP_ARGS(in6_addr_get_mapped_ipv4(&nexthop)));
>> +        } else {
>> +            ipv6_format_addr(&nexthop, &msg);
>> +        }
>> +
>> +        VLOG_DBG("%s", ds_cstr(&msg));
>> +        ds_destroy(&msg);
>> +    }
>> +
>> +    /* Lb vip routes go to <main> route table */
>> +    add_to_routes_ad(routes_ad, vip_ip, plen, nexthop, ROUTE_ORIGIN_LB,
>> +                     NULL, NULL, NULL, nb_lr, nb_lb, route_tag);
>> +out:
>> +    free(vip_str);
>>  }
>>
>>  static bool
>> @@ -1315,6 +1382,45 @@ route_has_local_gw(const struct nbrec_logical_router 
>> *lr,
>>      return false;
>>  }
>>
>> +static bool
>> +route_matches_local_lb(const struct nbrec_load_balancer *nb_lb,
>> +                       const char *ip_prefix)
>> +{
>> +    char *vip_str = NULL;
>> +    struct in6_addr vip_ip;
>> +    uint16_t vip_port;
>> +    int addr_family;
>> +    unsigned int plen;

Nit: Most of these can be moved lower, where it's used, to reduce scope.

>> +    char vip_cidr[50];
>> +    struct smap_node *node;
>> +    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> +
>> +    SMAP_FOR_EACH (node, &nb_lb->vips) {
>> +        if (node->key) {

Keys are always non-null.

>> +            if (ip_address_and_port_from_lb_key(node->key, &vip_str,
>> +                                                &vip_ip, &vip_port,
>> +                                                &addr_family)) {
>> +                if (vip_str) {

vip_str is always non-NULL here.

>> +                    plen = (addr_family == AF_INET) ? 32 : 128;
>> +                    snprintf(vip_cidr, sizeof(vip_cidr), "%s/%d",

Nit: should be "sizeof vip_cidr".

>> +                             vip_str, plen);
>> +                    if (!strcmp(vip_cidr, ip_prefix)) {

Comparing strings feels dangerous.  We should be comparing in6_addr structs.

>> +                       free(vip_str);
>> +                       return true;
>> +                    }

We leak vip_str here.

>> +                }
>> +            } else {
>> +                VLOG_WARN_RL(
>> +                    &rl,
>> +                    "Route learn: Parsing failed for local lb vip %s",
>> +                    node->key);
>> +            }
>> +        }
>> +    }
>> +    free(vip_str);

This should be moved in the loop above (see my previous comment).

>> +    return false;
>> +}
>> +

I took the liberty of rewriting this function as:

static bool
route_matches_local_lb(const struct nbrec_load_balancer *nb_lb,
                       const char *ip_prefix)
{
    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
    struct in6_addr prefix;
    unsigned int plen;

    if (!ip46_parse_cidr(ip_prefix, &prefix, &plen)) {
        return false;
    }

    struct smap_node *node;
    SMAP_FOR_EACH (node, &nb_lb->vips) {
        char *vip_str = NULL;
        struct in6_addr vip_ip;
        uint16_t vip_port;
        int addr_family;
        if (ip_address_and_port_from_lb_key(node->key, &vip_str,
                                            &vip_ip, &vip_port,
                                            &addr_family)) {
            if (IN6_IS_ADDR_V4MAPPED(&prefix) && addr_family == AF_INET) {
                ovs_be32 vip = in6_addr_get_mapped_ipv4(&vip_ip);
                ovs_be32 mask = be32_prefix_mask(plen);

                if ((vip & mask) == in6_addr_get_mapped_ipv4(&prefix)) {
                    free(vip_str);
                    return true;
                }
            } else if (!IN6_IS_ADDR_V4MAPPED(&prefix)
                       && addr_family == AF_INET6) {
                struct in6_addr mask = ipv6_create_mask(plen);
                struct in6_addr vip_prefix = ipv6_addr_bitand(&vip_ip, &mask);
                if (ipv6_addr_equals(&prefix, &vip_prefix)) {
                    free(vip_str);
                    return true;
                }
            }
            free(vip_str);
        } else {
            VLOG_WARN_RL(&rl,
                         "Route learn: Parsing failed for local lb vip %s",
                         node->key);
        }
    }
    return false;
}

>>  static bool
>>  route_need_learn(const struct nbrec_logical_router *lr,
>>                   const struct icsbrec_route *isb_route,
>> @@ -1330,6 +1436,11 @@ route_need_learn(const struct nbrec_logical_router 
>> *lr,
>>          return false;
>>      }
>>
>> +    if (!strcmp(isb_route->origin, ROUTE_ORIGIN_LB) &&
>> +        !smap_get_bool(nb_options, "ic-route-learn-lb", false)) {
>> +        return false;
>> +    }
>> +
>>      if (prefix_is_link_local(prefix, plen)) {
>>          return false;
>>      }
>> @@ -1344,6 +1455,29 @@ route_need_learn(const struct nbrec_logical_router 
>> *lr,
>>          return false;
>>      }
>>
>> +    for (int i = 0; i < lr->n_load_balancer; i++) {

Nit: size_t i

>> +        if (route_matches_local_lb(lr->load_balancer[i],
>> +                                   isb_route->ip_prefix)) {
>> +            VLOG_DBG("Skip learning %s (rtb:%s) route, as we've got local"
>> +                     " LB with matching VIP", isb_route->ip_prefix,
>> +                     isb_route->route_table);
>> +            return false;
>> +        }
>> +    }
>> +    for (int i = 0; i < lr->n_load_balancer_group; i++) {

Nit: size_t i

>> +        const struct nbrec_load_balancer_group *nb_lbg;
>> +        nb_lbg = lr->load_balancer_group[i];
>> +        for (int j = 0; j < nb_lbg->n_load_balancer; j++) {
>> +            if (route_matches_local_lb(nb_lbg->load_balancer[j],
>> +                                       isb_route->ip_prefix)) {
>> +                VLOG_DBG("Skip learning %s (rtb:%s) route, as we've got 
>> local"
>> +                         " LB with matching VIP", isb_route->ip_prefix,
>> +                         isb_route->route_table);
>> +                return false;
>> +            }
>> +        }
>> +    }
>> +
>>      return true;
>>  }
>>
>> @@ -1535,8 +1669,10 @@ ad_route_sync_external_ids(const struct ic_route_info 
>> *route_adv,
>>      const char *route_tag;
>>      smap_get_uuid(&isb_route->external_ids, "nb-id", &isb_ext_id);
>>      smap_get_uuid(&isb_route->external_ids, "lr-id", &isb_ext_lr_id);
>> -    nb_id = route_adv->nb_route ? route_adv->nb_route->header_.uuid
>> -                               : route_adv->nb_lrp->header_.uuid;
>> +    nb_id = route_adv->nb_lb ? route_adv->nb_lb->header_.uuid :
>> +            route_adv->nb_route ? route_adv->nb_route->header_.uuid :
>> +            route_adv->nb_lrp->header_.uuid;
>> +
>>      lr_id = route_adv->nb_lr->header_.uuid;
>>      if (!uuid_equals(&isb_ext_id, &nb_id)) {
>>          char *uuid_s = xasprintf(UUID_FMT, UUID_ARGS(&nb_id));
>> @@ -1696,6 +1832,40 @@ build_ts_routes_to_adv(struct ic_context *ctx,
>>                       lrp->name);
>>          }
>>      }
>> +
>> +    /* Check loadbalancers associated with the LR */
>> +    if (smap_get_bool(&nb_global->options, "ic-route-adv-lb", false)) {
>> +        for (int i = 0; i < lr->n_load_balancer; i++) {

Nit: size_t i

>> +            const struct nbrec_load_balancer *nb_lb = lr->load_balancer[i];
>> +            struct smap_node *node;
>> +            SMAP_FOR_EACH (node, &nb_lb->vips) {
>> +                if (node->key) {

Keys are always non-NULL.

>> +                    add_lb_vip_to_routes_ad(routes_ad, node->key, nb_lb,
>> +                                            ts_port_addrs,
>> +                                            &nb_global->options,
>> +                                            lr, route_tag);
>> +                }
>> +            }
>> +        }
>> +
>> +        for (int i = 0; i < lr->n_load_balancer_group; i++) {

Nit: size_t i

>> +            const struct nbrec_load_balancer_group *nb_lbg;
>> +            nb_lbg = lr->load_balancer_group[i];

Nit: Can be part of the declaration.

>> +            for (int j = 0; j < nb_lbg->n_load_balancer; j++) {

Nit: size_t j

>> +                const struct nbrec_load_balancer *nb_lb;
>> +                nb_lb = nb_lbg->load_balancer[j];

Nit: Can be part of the declaration.

>> +                struct smap_node *node;
>> +                SMAP_FOR_EACH (node, &nb_lb->vips) {
>> +                    if (node->key) {

Keys are always non-NULL.

>> +                        add_lb_vip_to_routes_ad(routes_ad, node->key, nb_lb,
>> +                                                ts_port_addrs,
>> +                                                &nb_global->options,
>> +                                                lr, route_tag);
>> +                    }
>> +                }
>> +            }
>> +        }
>> +    }
>>  }
>>
>>  static void
>> @@ -2213,6 +2383,10 @@ main(int argc, char *argv[])
>>                           &nbrec_logical_router_col_options);
>>      ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>>                           &nbrec_logical_router_col_external_ids);
>> +    ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>> +                         &nbrec_logical_router_col_load_balancer);
>> +    ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>> +                         &nbrec_logical_router_col_load_balancer_group);
>>
>>      ovsdb_idl_add_table(ovnnb_idl_loop.idl, 
>> &nbrec_table_logical_router_port);
>>      ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>> @@ -2252,6 +2426,16 @@ main(int argc, char *argv[])
>>      ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>>                           &nbrec_logical_switch_port_col_external_ids);
>>
>> +    ovsdb_idl_add_table(ovnnb_idl_loop.idl,
>> +                        &nbrec_table_load_balancer);
>> +    ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>> +                         &nbrec_load_balancer_col_vips);
>> +
>> +    ovsdb_idl_add_table(ovnnb_idl_loop.idl,
>> +                        &nbrec_table_load_balancer_group);
>> +    ovsdb_idl_add_column(ovnnb_idl_loop.idl,
>> +                         &nbrec_load_balancer_group_col_load_balancer);
>> +
>>      /* ovn-sb db. */
>>      struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
>>          ovsdb_idl_create(ovnsb_db, &sbrec_idl_class, false, true));
>> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
>> index 0fff9b463..5db7cb8d4 100644
>> --- a/lib/ovn-util.h
>> +++ b/lib/ovn-util.h
>> @@ -30,6 +30,7 @@
>>
>>  #define ROUTE_ORIGIN_CONNECTED "connected"
>>  #define ROUTE_ORIGIN_STATIC "static"
>> +#define ROUTE_ORIGIN_LB "loadbalancer"
>>
>>  #define ETH_CRC_LENGTH 4
>>  #define ETHERNET_OVERHEAD (ETH_HEADER_LEN + ETH_CRC_LENGTH)
>> diff --git a/ovn-ic-sb.ovsschema b/ovn-ic-sb.ovsschema
>> index 459c3833e..e92cf8f58 100644
>> --- a/ovn-ic-sb.ovsschema
>> +++ b/ovn-ic-sb.ovsschema
>> @@ -1,7 +1,7 @@
>>  {
>>      "name": "OVN_IC_Southbound",
>> -    "version": "2.0.0",
>> -    "cksum": "197748428 7081",
>> +    "version": "2.0.1",

This should actually be 2.1.0.

>> +    "cksum": "3470037528 7126",
>>      "tables": {
>>          "IC_SB_Global": {
>>              "columns": {
>> @@ -99,7 +99,8 @@
>>                  "nexthop": {"type": "string"},
>>                  "origin": {"type": {"key": {
>>                      "type": "string",
>> -                    "enum": ["set", ["connected", "static"]]}}},
>> +                    "enum": ["set",
>> +                             ["connected", "static", "loadbalancer"]]}}},
>>                  "external_ids": {
>>                      "type": {"key": "string", "value": "string",
>>                               "min": 0, "max": "unlimited"}}},
>> diff --git a/ovn-ic-sb.xml b/ovn-ic-sb.xml
>> index 5f0743dc3..8f98c7ebb 100644
>> --- a/ovn-ic-sb.xml
>> +++ b/ovn-ic-sb.xml
>> @@ -356,10 +356,12 @@
>>        </column>
>>
>>        <column name="origin">
>> -        Can be one of <code>connected</code> or <code>static</code>.  
>> Routes to
>> -        directly-connected subnets - LRP's CIDRs are inserted to OVN IC SB 
>> DB
>> -        with <code>connected</code> value in <ref column="origin"/>.  Static
>> -        routes are inserted to OVN IC SB DB with <code>static</code> value.
>> +        Can be one of <code>connected</code>, <code>static</code> or
>> +        <code>loadbalancer</code>.  Routes to directly-connected subnets -
>> +        LRP's CIDRs are inserted to OVN IC SB DB with <code>connected</code>
>> +        value in <ref column="origin"/>.  Static routes are inserted to OVN 
>> IC
>> +        SB DB with <code>static</code> value.  Routes for LB VIPs are 
>> inserted
>> +        in OVN IC SB DB with <code>loadbalancer</code> value.
>>          Next when route is learned to another AZ NB DB by ovn-ic, route 
>> origin
>>          is synced to <ref table="Logical_Router_Static_Route" 
>> column="options"
>>          key="origin"/>.
>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>> index 17346f228..f8a160041 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -437,7 +437,7 @@
>>            through the global <ref db="OVN_IC_Southbound"/> database.  Only
>>            routers with ports connected to interconnection transit switches
>>            participate in route advertisement.  For each of these routers, 
>> there
>> -          are two types of routes to be advertised:
>> +          are three types of routes to be advertised:
>>          </p>
>>
>>          <p>
>> @@ -450,6 +450,11 @@
>>            are considered as directly connected subnets on the router.
>>          </p>
>>
>> +        <p>
>> +          Thirdly, the vips of the loadbalancers associated with the logical
>> +          router are advertised.
>> +        </p>
>> +
>>          <p>
>>            Link local prefixes (IPv4 169.254.0.0/16 and IPv6 FE80::/10)
>>            are never advertised.
>> @@ -490,6 +495,20 @@
>>            <code>ic-route-learn</code> is <code>true</code>.
>>          </column>
>>
>> +        <column name="options" key="ic-route-adv-lb">
>> +          A boolean value that enables advertising routes for loadbalancer 
>> vips
>> +          to the global <ref db="OVN_IC_Southbound"/> database.  Default is
>> +          <code>false</code>.  This option takes effect only when option
>> +          <code>ic-route-adv</code> is <code>true</code>.
>> +        </column>
>> +
>> +        <column name="options" key="ic-route-learn-lb">
>> +          A boolean value that enables learning routes for loadbalancer
>> +          routes from the global <ref db="OVN_IC_Southbound"/> database.
>> +          Default is <code>false</code>.  This option takes effect only when
>> +          option <code>ic-route-learn</code> is <code>true</code>.
>> +        </column>
>> +
>>          <column name="options" key="ic-route-denylist">
>>            A string value contains a list of CIDRs delimited by ",".  A route
>>            will not be advertised or learned if the route's prefix belongs to
>> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
>> index c8276189c..ba4095a98 100644
>> --- a/tests/ovn-ic.at
>> +++ b/tests/ovn-ic.at
>> @@ -752,6 +752,301 @@ OVN_CLEANUP_IC([az1], [az2])
>>  AT_CLEANUP
>>  ])
>>
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([ovn-ic -- route sync -- loadbalancer])
>> +
>> +ovn_init_ic_db
>> +ovn-ic-nbctl ts-add ts1
>> +
>> +for i in 1 2; do
>> +    ovn_start az$i
>> +    ovn_as az$i
>> +    check ovn-ic-nbctl --wait=sb sync
>> +    # Enable route learning at AZ level
>> +    check ovn-nbctl set nb_global . options:ic-route-learn=true
>> +    # Enable route advertising at AZ level
>> +    check ovn-nbctl set nb_global . options:ic-route-adv=true
>> +
>> +    # Enable route advertising for loadbalancer VIP
>> +    check ovn-nbctl set nb_global . options:ic-route-adv-lb=true
>> +    # Enable route learning for loadbalancer VIP
>> +    check ovn-nbctl set nb_global . options:ic-route-learn-lb=true
>> +
>> +    # Create LRP and connect to TS
>> +    check ovn-nbctl lr-add lr$i
>> +    check ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i 
>> 169.254.100.$i/24 2001:db8:1::$i/64
>> +    check ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
>> +            -- lsp-set-addresses lsp-ts1-lr$i router \
>> +            -- lsp-set-type lsp-ts1-lr$i router \
>> +            -- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
>> +
>> +    # Create LB
>> +    check ovn-nbctl lb-add lb$i 200.1.$i.50:80 1.1.$i.1:80,1.1.$i.2:80
>> +    check ovn-nbctl lr-lb-add lr$i lb$i
>> +done
>> +
>> +for i in 1 2; do
>> +    OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep 
>> learned])
>> +done
>> +
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.50/32 origin=loadbalancer
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.2.50/32 origin=loadbalancer
>> +
>> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.2.50             169.254.100.2 dst-ip (learned)
>> +])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Add another few instances of LB in AZ1, some with same VIP IP.
>> +ovn_as az1 ovn-nbctl lb-add lb3 200.1.1.53:80 
>> 1.1.1.1:80,1.1.1.2:80,1.1.1.3:80
>> +ovn_as az1 ovn-nbctl lb-add lb4 200.1.1.53:8080 1.1.1.1:8080,1.1.1.2:8080
>> +ovn_as az1 ovn-nbctl lb-add lb5 200.1.1.54:80 1.1.1.1:80,1.1.1.2:80
>> +ovn_as az1 ovn-nbctl lr-lb-add lr1 lb3
>> +ovn_as az1 ovn-nbctl lr-lb-add lr1 lb4
>> +ovn_as az1 ovn-nbctl lr-lb-add lr1 lb5

Missing "check " prefix (this applies to multiple places in this test).

>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.53/32 origin=loadbalancer
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.54/32 origin=loadbalancer
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.53])
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.54])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +               200.1.1.54             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Remove one LB.
>> +ovn_as az1 ovn-nbctl lr-lb-del lr1 lb5
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.54])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Add load balancer group.
>> +ovn_as az1 ovn-nbctl lb-add lb6 250.1.1.11:80 1.1.1.1:80,1.1.1.2:80
>> +ovn_as az1 ovn-nbctl lb-add lb7 250.1.1.12:80 1.1.1.1:80,1.1.1.2:80
>> +lb6=$(ovn_as az1 fetch_column nb:load_balancer _uuid name=lb6)
>> +lb7=$(ovn_as az1 fetch_column nb:load_balancer _uuid name=lb7)
>> +
>> +ovn_as az1
>> +lbg=$(ovn-nbctl create load_balancer_group name=lbg -- \
>> +  add load_balancer_group lbg load_balancer $lb6 -- \
>> +  add load_balancer_group lbg load_balancer $lb7)
>> +ovn_as az1 ovn-nbctl add logical_router lr1 load_balancer_group $lbg
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 250.1.1.11])
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 250.1.1.12])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +               250.1.1.11             169.254.100.1 dst-ip (learned)
>> +               250.1.1.12             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Remove the LB group from LR.
>> +ovn_as az1 ovn-nbctl remove logical_router lr1 load_balancer_group $lbg
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 250.1.1.11])
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 250.1.1.12])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Disable LB route-advertising for AZ1.
>> +ovn_as az1 ovn-nbctl set nb_global . options:ic-route-adv-lb=false
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [])
>> +
>> +# Enable LB route-advertising back for AZ1.
>> +ovn_as az1 ovn-nbctl set nb_global . options:ic-route-adv-lb=true
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.50/32 origin=loadbalancer
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.53/32 origin=loadbalancer
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.50])
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.53])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Disable LB route-learning for AZ2.
>> +ovn_as az2 ovn-nbctl set nb_global . options:ic-route-learn-lb=false
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.50/32 origin=loadbalancer
>> +wait_row_count ic-sb:Route 1 ip_prefix=200.1.1.53/32 origin=loadbalancer
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [])
>> +
>> +# Enable LB route-learning back for AZ2.
>> +ovn_as az2 ovn-nbctl set nb_global . options:ic-route-learn-lb=true
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.50])
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.53])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Add IPv6 LB.
>> +ovn_as az1 ovn-nbctl lb-add lb_v6 [[4242::1]]:80 "[[4242::2]]:80"
>> +ovn_as az1 ovn-nbctl lr-lb-add lr1 lb_v6
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 4242])
>> +
>> +AT_CHECK([ovn-ic-sbctl list route | grep 4242 -A 2], [0], [dnl
>> +ip_prefix           : "4242::1/128"
>> +nexthop             : "2001:db8:1::1"
>> +origin              : loadbalancer
>> +])
>> +
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +
>> +IPv6 Routes
>> +Route Table <main>:
>> +                  4242::1             2001:db8:1::1 dst-ip (learned)
>> +])
>> +
>> +# Remove IPv6 LB
>> +ovn_as az1 ovn-nbctl lr-lb-del lr1 lb_v6
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 4242])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Configure same LB VIP locally as in remote az. Ensure learnt route is 
>> removed.
>> +ovn_as az2 ovn-nbctl lb-add lb_temp 200.1.1.50:80 1.1.1.1:80,1.1.1.2:80
>> +ovn_as az2 ovn-nbctl lr-lb-add lr2 lb_temp
>> +wait_row_count ic-sb:Route 2 ip_prefix=200.1.1.50/32 origin=loadbalancer
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.50])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +ovn_as az2 ovn-nbctl lr-lb-del lr2 lb_temp
>> +ovn_as az2 ovn-nbctl lb-del lb_temp
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.1.50])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Configure static route in remote az. Route would be learnt. Add local LB 
>> VIP
>> +# same as static route prefix. Ensure route is removed.
>> +ovn_as az1 ovn-nbctl lr-route-add lr1 200.1.3.55 169.1.1.1
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.3.55])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +               200.1.3.55             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +ovn_as az2 ovn-nbctl lb-add lb_temp 200.1.3.55:80 1.1.1.1:80,1.1.1.2:80
>> +ovn_as az2 ovn-nbctl lr-lb-add lr2 lb_temp
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.3.55])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +ovn_as az2 ovn-nbctl lr-lb-del lr2 lb_temp
>> +ovn_as az2 ovn-nbctl lb-del lb_temp
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.3.55])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +               200.1.3.55             169.254.100.1 dst-ip (learned)
>> +])
>> +ovn_as az1 ovn-nbctl lr-route-del lr1 200.1.3.55 169.1.1.1
>> +OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.3.55])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +])
>> +
>> +# Create a third az
>> +ovn_start az3
>> +ovn_as az3
>> +check ovn-ic-nbctl --wait=sb sync
>> +check ovn-nbctl set nb_global . options:ic-route-learn=true
>> +check ovn-nbctl set nb_global . options:ic-route-adv=true
>> +
>> +check ovn-nbctl set nb_global . options:ic-route-adv-lb=true
>> +check ovn-nbctl set nb_global . options:ic-route-learn-lb=true
>> +
>> +check ovn-nbctl lr-add lr3
>> +check ovn-nbctl lrp-add lr3 lrp-lr3-ts1 aa:aa:aa:aa:aa:03 169.254.100.3/24 
>> 2001:db8:1::3/64
>> +check ovn-nbctl lsp-add ts1 lsp-ts1-lr3 \
>> +        -- lsp-set-addresses lsp-ts1-lr3 router \
>> +        -- lsp-set-type lsp-ts1-lr3 router \
>> +        -- lsp-set-options lsp-ts1-lr3 router-port=lrp-lr3-ts1
>> +
>> +# Create LB in 3rd az and check that the route propagates to az1 LR
>> +ovn_as az3 ovn-nbctl lb-add lb3 200.1.3.50:80 1.1.3.1:80,1.1.3.2:80
>> +ovn_as az3 ovn-nbctl lr-lb-add lr3 lb3
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep 200.1.3.50])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned)
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +               200.1.3.50             169.254.100.3 dst-ip (learned)
>> +])
>> +
>> +# Create LB in az3 with same VIP as az1, check for ECMP route in az2 LR
>> +ovn_as az3 ovn-nbctl lb-add lb4 200.1.1.50:80 1.1.3.1:80,1.1.3.2:80
>> +ovn_as az3 ovn-nbctl lr-lb-add lr3 lb4
>> +OVS_WAIT_UNTIL([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | 
>> grep ecmp])
>> +AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr2], [0], [dnl
>> +IPv4 Routes
>> +Route Table <main>:
>> +               200.1.1.50             169.254.100.1 dst-ip (learned) ecmp
>> +               200.1.1.50             169.254.100.3 dst-ip (learned) ecmp
>> +               200.1.1.53             169.254.100.1 dst-ip (learned)
>> +               200.1.3.50             169.254.100.3 dst-ip (learned)
>> +])
>> +
>> +OVN_CLEANUP_IC([az1], [az2])
>> +
>> +AT_CLEANUP
>> +])
>> +
>>  OVN_FOR_EACH_NORTHD([
>>  AT_SETUP([ovn-ic -- route sync -- IPv6 route tables])
>>  AT_KEYWORDS([IPv6-route-sync])
>> --
>> 2.39.3

I rebased the patch, addressed all the comments I listed above and applied
the patch to main.  I also added Sragdhara to the AUTHORS.rst file.

Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to