On 1/26/25 4:49 AM, Numan Siddique wrote:
> On Sat, Jan 25, 2025 at 5:52 PM Frode Nordahl <[email protected]> wrote:
>>
>> Hello,
>>
>> Note that this email is sent at a time convenient to me, please don't
>> feel obliged to read nor respond to it until a time convenient for
>> you.
>>
>> The timing of this bit of time skewed work is because I anticipated
>> the output to be a new patch set for which we would have a desire to
>> try to get within soft freeze. It became apparent that it would likely
>> fit within the context of the patch set you diligently posted on
>> friday, so I'll leave the result of it as a review comment instead.
>>
>> On Thu, Jan 23, 2025 at 3:00 PM Martin Kalcok
>> <[email protected]> wrote:
>>>
>>> This change builds on top of the new "dynamic routing" OVN feature
>>> that allows advertising routes to the fabric network. When LR option
>>> "dynamic-routing" is set on the router, following two new LRP options
>>> become available:
>>>
>>> * redistribute-nat - When set to "true", ovn-controller will advertise
>>>                      routes for external NAT IPs valid for the LRP.
>>> * redistribute-lb-vips - When set to "true", ovn-controller will advertise
>>>                          host routes to LB VIPs via the LRP.
>>>
>>> Co-authored-by: Frode Nordahl <[email protected]>
>>> Signed-off-by: Frode Nordahl <[email protected]>
>>
>> Thanks! Good to have these options resurrected and adapted to the
>> current state of the fabric integration patch epic.
>>
>> After having read up a bit on the current state of review of our
>> sibling options "dynmic-routing", "dynamic-routing-connected" and
>> "dynamic-routing-static", I wonder if we need to make the nat and LB
>> options have similar prefixes?
>>
>>
>>> Signed-off-by: Martin Kalcok <[email protected]>
> 
> Hi Frode/Martin,
> 

Hi Numan,

Just a disclaimer before I reply to the rest: I didn't review these two
patches properly yet.

> I've a few questions related to the entire BGP series.  Can you please
> help answer my questions?
> 
> 1.  If I understand, a logical port in the provider logical switch
> (with localnet port) needs to be created

I have this branch in my repo where I applied the previous versions of
Frode/Felix's BGP series and there's a test there that shows how things
are configured:

https://github.com/dceara/ovn/blob/80edb08e6dfd5d3b5e19288d7cc277639d85a8ae/tests/system-ovn.at#L13945-L13969

But IMO, the switch where the BGP control plane (FRR or something else)
is connected doesn't have to be a "provider" logical switch.  The BGP
control plane can run behind any LSP on any logical switch that's
connected to a logical router port that has
options:routing-protocols=BGP,BFD set.

All we need to do is ensure that BGP packets (control traffic) reach
that logical router port.

In fact, in some cases we might _not even need to use the
routing-protocols option_ as long as the CMS ensures itself that BGP
packets (control traffic) reach the FRR instance in some other (out of
band) way.

>     and this port will be used to bind the VIF for the "frr" to
> establish a bgp (unnumbered) session
>     with its neighbor (which I assume will be the leaf switch) to
> advertise the routes right ?

That's one option yes.  But it can be any BGP speaker in general.  Even
another FRR (or other control plane) running behind a different OVN
logical router (port).

>     Is my understanding correct ?  If so,  this logical port will use
> the SAME mac as the logical router port.

Yes that's going to be required.

>     What will be the type of this logical port ?

Regular VIF, but because the VIF and LRP share mac address we had to
take extra care and did:

https://github.com/ovn-org/ovn/commit/370527673c2b35c1b79d90a4e5052177e593a699#diff-97e16400e2bcbb4b65f7f3b8f2c05e9e8e56148df77719b71d60f235e3bcc0edR14058-R14083

>     Does the patch series add any system tests and create this logical port ?
>     As I commented in another patch, adding end-to-end multi node
> system tests would be very full.
> 

+1 I agree with this completely.

> 2.  Do this patch series advertise routes for the dnat_and_snat's with
> external_mac and logical_port set ?
>     Or does it only support gateway routers ?  and not distributed
> router with a DGP ?
>      If it is supported,  I assume, frr needs to be run on all the
> compute nodes where the logical_port
>     specified in the dnat_and_snat's reside.
> 

I think that's true for all BGP support being added by the current
in-flight series.

> 
> Thanks
> Numan
> 

Regards,
Dumitru

> 
>>> ---
>>>  NEWS                              |   4 +
>>>  northd/en-advertised-route-sync.c |  11 +
>>>  northd/inc-proc-northd.c          |   4 +
>>>  northd/northd.c                   |  98 +++++++-
>>>  northd/northd.h                   |   4 +
>>>  ovn-nb.xml                        |  31 +++
>>>  tests/system-ovn.at               | 379 ++++++++++++++++++++++++++++++
>>>  7 files changed, 530 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index f526013f1..ad5b74b2e 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -24,6 +24,10 @@ Post v24.09.0
>>>       a lower priority than static routes.
>>>     - Add the option "dynamic-routing-connected-as-host-routes" to LRPs. If 
>>> set
>>>       to true then connected routes are announced as individual host routes.
>>> +   - Add 'redistribute-lb-vips' LRP option. If set to true, the LRP can be 
>>> used
>>> +     to advertise host paths to the Load Balancer VIPs associated with the 
>>> LR.
>>> +   - Add 'redistribute-nat' LRP option. If set to true, the LRP can be used
>>> +     to advertise external NAT IPs associated with it.
>>>
>>>  OVN v24.09.0 - 13 Sep 2024
>>>  --------------------------
>>> diff --git a/northd/en-advertised-route-sync.c 
>>> b/northd/en-advertised-route-sync.c
>>> index 065c73861..b6786b3af 100644
>>> --- a/northd/en-advertised-route-sync.c
>>> +++ b/northd/en-advertised-route-sync.c
>>> @@ -421,9 +421,20 @@ advertised_route_table_sync(
>>>                                          "dynamic-routing-static")) {
>>>              continue;
>>>          }
>>> +        if (route->source == ROUTE_SOURCE_NAT &&
>>> +                !smap_get_bool(&route->out_port->nbrp->options,
>>> +                               "redistribute-nat", false)) {
>>> +                continue;
>>> +        }
>>> +        if (route->source == ROUTE_SOURCE_LB &&
>>> +                !smap_get_bool(&route->out_port->nbrp->options,
>>> +                               "redistribute-lb-vips", false)) {
>>> +                continue;
>>> +        }
>>>
>>>          char *ip_prefix = normalize_v46_prefix(&route->prefix,
>>>                                                 route->plen);
>>> +
>>>          ar_sync_to_sb(ovnsb_txn, &sync_routes,
>>>                           route->od->sb,
>>>                           route->out_port->sb,
>>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
>>> index ab500a86a..36e1d9993 100644
>>> --- a/northd/inc-proc-northd.c
>>> +++ b/northd/inc-proc-northd.c
>>> @@ -262,6 +262,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>>>      engine_add_input(&en_routes, &en_bfd, NULL);
>>>      engine_add_input(&en_routes, &en_northd,
>>>                       routes_northd_change_handler);
>>> +    engine_add_input(&en_routes, &en_lr_nat,
>>> +                     NULL);
>>> +    engine_add_input(&en_routes, &en_lb_data,
>>> +                     NULL);
>>>
>>>      engine_add_input(&en_bfd_sync, &en_bfd, NULL);
>>>      engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL);
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index 23b0769fe..e86208ef8 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -11435,6 +11435,96 @@ parsed_routes_add_connected(const struct 
>>> ovn_datapath *od,
>>>      }
>>>  }
>>>
>>> +static void
>>> +parsed_routes_add_nat(const struct ovn_datapath *od,
>>> +                      const struct ovn_port *op,
>>> +                      struct hmap *routes)
>>> +{
>>> +    if (!op->nbrp || !smap_get_bool(&op->nbrp->options,
>>> +                                    "redistribute-nat", false)) {
>>> +        return;
>>> +    }
>>> +
>>> +    size_t n_nats = 0;
>>> +    char **nats = NULL;
>>> +    nats = get_nat_addresses(op, &n_nats, false, false, NULL, true);
>>> +
>>> +    for (size_t i = 0; i < n_nats; i++) {
>>> +        struct lport_addresses *laddrs = xzalloc(sizeof *laddrs);
>>> +        int ofs = 0;
>>> +        extract_addresses(nats[i], laddrs, &ofs);
>>> +        for (int j = 0; j < laddrs->n_ipv4_addrs; j++) {
>>> +            struct ipv4_netaddr *addr = &laddrs->ipv4_addrs[j];
>>> +            struct in6_addr prefix;
>>> +            ip46_parse(addr->network_s, &prefix);
>>> +
>>> +            parsed_route_add(od, NULL, &prefix, addr->plen,
>>> +                             false, addr->addr_s, op,
>>> +                             0, false,
>>> +                             false, NULL, ROUTE_SOURCE_NAT,
>>> +                             &op->nbrp->header_, routes);
>>> +        }
>>> +        for (int j = 0; j < laddrs->n_ipv6_addrs; j++) {
>>> +            struct ipv6_netaddr *addr = &laddrs->ipv6_addrs[j];
>>> +            parsed_route_add(od, NULL, &addr->addr, addr->plen,
>>> +                             false, addr->addr_s, op,
>>> +                             0, false,
>>> +                             false, NULL, ROUTE_SOURCE_NAT,
>>> +                             &op->nbrp->header_, routes);
>>> +        }
>>> +        destroy_lport_addresses(laddrs);
>>> +        free(nats[i]);
>>> +    }
>>> +    free(nats);
>>> +}
>>> +
>>> +static void
>>> +parsed_routes_add_lb(const struct ovn_datapath *od,
>>> +                     const struct ovn_port *op,
>>> +                     struct hmap *routes)
>>> +{
>>> +    if (!op->nbrp || !smap_get_bool(&op->nbrp->options,
>>> +                                    "redistribute-lb-vips", false)) {
>>> +        return;
>>> +    }
>>> +
>>
>> The get_nat_addresses() function appears to have support for also
>> retrieving LB addresses, and it appears to do it by consuming already
>> populated ssets. It also supports filtering on Load Balancers with the
>> "add_route" option set, which can be useful for retrieving indirectly
>> connected resources (see further discussion below).
>>
>> Should we consider switching to it?
>>
>>> +    for (size_t i = 0; i < od->nbr->n_load_balancer; i++) {
>>> +        struct ovn_northd_lb *lb = ovn_northd_lb_create(
>>> +                                        od->nbr->load_balancer[i]);
>>> +        for (size_t j = 0; j < lb->n_vips; j++) {
>>> +            const struct ovn_lb_vip *lb_vip = &lb->vips[j];
>>> +            if (find_lport_address(&op->lrp_networks, lb_vip->vip_str)) {
>>> +                int plen = lb_vip->address_family == AF_INET ? 32 : 128;
>>> +                parsed_route_add(od, NULL, &lb_vip->vip, plen,
>>> +                                 false, lb_vip->vip_str, op,
>>> +                                 0, false,
>>> +                                 false, NULL, ROUTE_SOURCE_LB,
>>> +                                 &op->nbrp->header_, routes);
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    for (size_t i = 0; i < od->nbr->n_load_balancer_group; i++) {
>>> +        struct nbrec_load_balancer_group *lb_group =
>>> +            od->nbr->load_balancer_group[i];
>>> +        for (size_t j = 0; j < lb_group->n_load_balancer; j++) {
>>> +            struct ovn_northd_lb *lb =
>>> +                ovn_northd_lb_create(lb_group->load_balancer[j]);
>>> +            for (size_t k = 0; k < lb->n_vips; k++) {
>>> +                const struct ovn_lb_vip *lb_vip = &lb->vips[k];
>>> +                if (find_lport_address(&op->lrp_networks, 
>>> lb_vip->vip_str)) {
>>> +                    int plen = lb_vip->address_family == AF_INET ? 32 : 
>>> 128;
>>> +                    parsed_route_add(od, NULL, &lb_vip->vip, plen,
>>> +                                     false, lb_vip->vip_str, op,
>>> +                                     0, false,
>>> +                                     false, NULL, ROUTE_SOURCE_LB,
>>> +                                     &op->nbrp->header_, routes);
>>> +                }
>>> +            }
>>> +        }
>>> +    }
>>> +}
>>> +
>>>  void
>>>  build_parsed_routes(const struct ovn_datapath *od, const struct hmap 
>>> *lr_ports,
>>>                       const struct hmap *bfd_connections, struct hmap 
>>> *routes,
>>> @@ -11457,6 +11547,8 @@ build_parsed_routes(const struct ovn_datapath *od, 
>>> const struct hmap *lr_ports,
>>>      const struct ovn_port *op;
>>>      HMAP_FOR_EACH (op, dp_node, &od->ports) {
>>>          parsed_routes_add_connected(od, op, routes);
>>> +        parsed_routes_add_nat(od, op, routes);
>>> +        parsed_routes_add_lb(od, op, routes);
>>>      }
>>
>> We did not get to finish the end to end review of the state of the
>> epic in context of downstream use with CMSs such as OpenStack before
>> you posted these patches, so I know that support for NAT and LB
>> "add_route" option was not on the radar at that point in time.
>>
>> After having reviewed it in this context I think we would benefit
>> greatly from adding support for that here, something like:
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 7057bb03a..bb29013ef 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -11438,7 +11438,8 @@ parsed_routes_add_connected(const struct
>> ovn_datapath *od,
>>  static void
>>  parsed_routes_add_nat(const struct ovn_datapath *od,
>>                        const struct ovn_port *op,
>> -                      struct hmap *routes)
>> +                      struct hmap *routes,
>> +                      bool routable_only)
>>  {
>>      if (!op->nbrp || !smap_get_bool(&op->nbrp->options,
>>                                      "redistribute-nat", false)) {
>> @@ -11447,7 +11448,7 @@ parsed_routes_add_nat(const struct ovn_datapath *od,
>>
>>      size_t n_nats = 0;
>>      char **nats = NULL;
>> -    nats = get_nat_addresses(op, &n_nats, false, false, NULL, true);
>> +    nats = get_nat_addresses(op, &n_nats, routable_only, false, NULL, true);
>>
>>      for (size_t i = 0; i < n_nats; i++) {
>>          struct lport_addresses *laddrs = xzalloc(sizeof *laddrs);
>> @@ -11481,7 +11482,8 @@ parsed_routes_add_nat(const struct ovn_datapath *od,
>>  static void
>>  parsed_routes_add_lb(const struct ovn_datapath *od,
>>                       const struct ovn_port *op,
>> -                     struct hmap *routes)
>> +                     struct hmap *routes,
>> +                     bool routable_only)
>>  {
>>      if (!op->nbrp || !smap_get_bool(&op->nbrp->options,
>>                                      "redistribute-lb-vips", false)) {
>> @@ -11547,8 +11549,19 @@ build_parsed_routes(const struct ovn_datapath
>> *od, const struct hmap *lr_ports,
>>      const struct ovn_port *op;
>>      HMAP_FOR_EACH (op, dp_node, &od->ports) {
>>          parsed_routes_add_connected(od, op, routes);
>> -        parsed_routes_add_nat(od, op, routes);
>> -        parsed_routes_add_lb(od, op, routes);
>> +        parsed_routes_add_nat(od, op, routes, false);
>> +        parsed_routes_add_lb(od, op, routes, false);
>> +    }
>> +
>> +    for (size_t i = 0; od->is_gw_router && i < od->n_ls_peers; i++) {
>> +        for (size_t j = 0; j < od->ls_peers[i]->n_router_ports; j++) {
>> +            struct ovn_port *router_port;
>> +
>> +            router_port = od->ls_peers[i]->router_ports[j]->peer;
>> +
>> +            parsed_routes_add_nat(od, router_port, routes, true);
>> +            parsed_routes_add_lb(od, router_port, routes, true);
>> +        }
>>      }
>>
>>      HMAP_FOR_EACH_SAFE (pr, key_node, routes) {
>>
>> ---
>>
>> The original controller side patches did controller-side filtering
>> based on the instances behind the addresses actually being hosted on
>> the local chassis, and this would still be relevant to consume
>> something like the above, ensuring that /32 routes in the
>> Advertise_Route table is only announced by the one gateway router
>> local to the backing instance.
>>
>> A test case showing more detail on topology is added below.
>>
>>>      HMAP_FOR_EACH_SAFE (pr, key_node, routes) {
>>> @@ -11638,6 +11730,8 @@ route_source_to_offset(enum route_source source)
>>>  {
>>>      switch (source) {
>>>          case ROUTE_SOURCE_CONNECTED:
>>> +        case ROUTE_SOURCE_NAT:
>>> +        case ROUTE_SOURCE_LB:
>>>              return ROUTE_PRIO_OFFSET_CONNECTED;
>>>          case ROUTE_SOURCE_STATIC:
>>>              return ROUTE_PRIO_OFFSET_STATIC;
>>> @@ -13915,7 +14009,9 @@ build_route_flows_for_lrouter(
>>>      struct parsed_route *route;
>>>      HMAP_FOR_EACH_WITH_HASH (route, key_node, uuid_hash(&od->key),
>>>                               parsed_routes) {
>>> -        if (route->source == ROUTE_SOURCE_CONNECTED) {
>>> +        if (route->source == ROUTE_SOURCE_CONNECTED ||
>>> +                route->source == ROUTE_SOURCE_NAT ||
>>> +                route->source == ROUTE_SOURCE_LB) {
>>>              unique_routes_add(&unique_routes, route);
>>>              continue;
>>>          }
>>> diff --git a/northd/northd.h b/northd/northd.h
>>> index 3bc6f6f04..117b7421f 100644
>>> --- a/northd/northd.h
>>> +++ b/northd/northd.h
>>> @@ -702,6 +702,10 @@ enum route_source {
>>>      ROUTE_SOURCE_CONNECTED,
>>>      /* The route is derived from a northbound static route entry. */
>>>      ROUTE_SOURCE_STATIC,
>>> +    /* Host route generated from NAT's external IP. */
>>> +    ROUTE_SOURCE_NAT,
>>> +    /* Host route generated from LB's external IP. */
>>> +    ROUTE_SOURCE_LB,
>>>      /* the route is learned by an ovn-controller */
>>>      ROUTE_SOURCE_LEARNED,
>>>  };
>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>>> index c5f182f24..417088a3a 100644
>>> --- a/ovn-nb.xml
>>> +++ b/ovn-nb.xml
>>> @@ -2961,6 +2961,10 @@ or
>>>                 table="Logical_Router_Port"/>
>>>          * <ref column="options" key="dynamic-routing-static"
>>>                 table="Logical_Router_Port"/>
>>> +        * <ref column="options" key="redistribute-lb-vips"
>>> +               table="Logical_Router_Port"/>
>>> +        * <ref column="options" key="redistribute-nat"
>>> +               table="Logical_Router_Port"/>
>>>        </column>
>>>
>>>        <column name="options" key="dynamic-routing-connected"
>>> @@ -3798,6 +3802,33 @@ or
>>>            This allows a single chassis to learn different routes on 
>>> separate
>>>            LRPs bound to this chassis.
>>>        </column>
>>> +
>>> +      <column name="options" key="redistribute-lb-vips"
>>> +              type='{"type": "boolean"}'>
>>> +        <p>
>>> +          Only relevant if <ref column="options" key="dynamic-routing"
>>> +          table="Logical_Router"/> on the respective Logical_Router is set
>>> +          to <code>true</code>.
>>> +
>>> +          If this option is <code>true</code>, northd will create host 
>>> route
>>> +          entries in the southbound <ref table="Advertised_Route"
>>> +          db="OVN_Southbound"/> table, associated with this LRP, for each 
>>> LB
>>> +          VIP.
>>> +        </p>
>>> +      </column>
>>> +
>>> +      <column name="options" key="redistribute-nat" type='{"type": 
>>> "boolean"}'>
>>> +        <p>
>>> +          Only relevant if <ref column="options" key="dynamic-routing"
>>> +          table="Logical_Router"/> on the respective Logical_Router is set
>>> +          to <code>true</code>.
>>> +
>>> +          If this option is <code>true</code>, northd will create host 
>>> route
>>> +          entries in the southbound <ref table="Advertised_Route"
>>> +          db="OVN_Southbound"/> table, for external IP addresses of NAT 
>>> rules
>>> +          associated with this LRP.
>>> +        </p>
>>> +      </column>
>>>      </group>
>>>
>>>      <group title="Attachment">
>>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>>> index 9dddfc399..1bcab802f 100644
>>> --- a/tests/system-ovn.at
>>> +++ b/tests/system-ovn.at
>>> @@ -15283,3 +15283,382 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>>>  AT_CLEANUP
>>>  ])
>>>
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([route-exchange for LB VIPs with gateway router IPv4])
>>> +AT_KEYWORDS([route-exchange])
>>> +
>>> +CHECK_VRF()
>>> +CHECK_CONNTRACK()
>>> +CHECK_CONNTRACK_NAT()
>>> +ovn_start
>>> +OVS_TRAFFIC_VSWITCHD_START()
>>> +ADD_BR([br-int])
>>> +ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone])
>>> +
>>> +# Set external-ids in br-int needed for ovn-controller
>>> +ovs-vsctl \
>>> +        -- set Open_vSwitch . external-ids:system-id=hv1 \
>>> +        -- set Open_vSwitch . 
>>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>>> +        -- set bridge br-int fail-mode=secure 
>>> other-config:disable-in-band=true
>>> +
>>> +# Start ovn-controller
>>> +start_daemon ovn-controller
>>> +
>>> +ovn-appctl vlog/set route_exchange
>>> +check ovn-nbctl -- lr-add R1 \
>>> +                -- set Logical_Router R1 options:requested-tnl-key=1000 
>>> options:dynamic-routing=true
>>> +
>>> +check ovn-nbctl ls-add sw0
>>> +check ovn-nbctl ls-add public
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1000:.*UP], [1])
>>> +
>>> +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
>>> +check ovn-nbctl -- lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24 \
>>> +                -- lrp-set-options rp-public \
>>> +                       maintain-vrf=true \
>>> +                       redistribute-lb-vips=true
>>> +
>>> +check ovn-nbctl set logical_router R1 options:chassis=hv1
>>> +
>>> +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
>>> +    type=router options:router-port=rp-sw0 \
>>> +    -- lsp-set-addresses sw0-rp router
>>> +
>>> +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port 
>>> public-rp \
>>> +    type=router options:router-port=rp-public \
>>> +    -- lsp-set-addresses public-rp router
>>> +
>>> +check ovs-vsctl set Open_vSwitch . 
>>> external-ids:ovn-bridge-mappings=phynet:br-ext
>>> +
>>> +check ovn-nbctl lsp-add public public1 \
>>> +        -- lsp-set-addresses public1 unknown \
>>> +        -- lsp-set-type public1 localnet \
>>> +        -- lsp-set-options public1 network_name=phynet
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([test `ip route show table 1000 | wc -l` -eq 1], [1])
>>> +
>>> +
>>> +# Create a load balancer and associate to R1
>>> +check ovn-nbctl lb-add lb1 172.16.1.150:80 172.16.1.100:80
>>> +check ovn-nbctl lr-lb-add R1 lb1
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1000:.*UP])
>>> +AT_CHECK([test `ip route show table 1000 | wc -l` -eq 1])
>>> +AT_CHECK([ip route show table 1000 | grep -q 172.16.1.150])
>>> +
>>> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>> +
>>> +# Ensure system resources are cleaned up
>>> +AT_CHECK([ip link | grep -q ovnvrf1000:.*UP], [1])
>>> +AT_CHECK([test `ip route show table 1000 | wc -l` -eq 1], [1])
>>> +
>>> +as ovn-sb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as ovn-nb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as northd
>>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>>> +
>>> +as
>>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>>> +/Failed to acquire.*/d
>>> +/connection dropped.*/d"])
>>> +AT_CLEANUP
>>> +])
>>> +
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([route-exchange for LB VIPs with gateway router IPv6])
>>> +AT_KEYWORDS([route-exchange])
>>> +
>>> +CHECK_VRF()
>>> +CHECK_CONNTRACK()
>>> +CHECK_CONNTRACK_NAT()
>>> +ovn_start
>>> +OVS_TRAFFIC_VSWITCHD_START()
>>> +ADD_BR([br-int])
>>> +ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone])
>>> +
>>> +# Set external-ids in br-int needed for ovn-controller
>>> +ovs-vsctl \
>>> +        -- set Open_vSwitch . external-ids:system-id=hv1 \
>>> +        -- set Open_vSwitch . 
>>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>>> +        -- set bridge br-int fail-mode=secure 
>>> other-config:disable-in-band=true
>>> +
>>> +# Start ovn-controller
>>> +start_daemon ovn-controller
>>> +
>>> +ovn-appctl vlog/set route_exchange
>>> +check ovn-nbctl -- lr-add R1 \
>>> +                -- set Logical_Router R1 options:requested-tnl-key=1001 
>>> options:dynamic-routing=true
>>> +
>>> +check ovn-nbctl ls-add sw0
>>> +check ovn-nbctl ls-add public
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1001:.*UP], [1])
>>> +
>>> +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 2001:db8:100::1/64
>>> +check ovn-nbctl -- lrp-add R1 rp-public 00:00:02:01:02:03 
>>> 2001:db8:1001::1/64 \
>>> +                -- lrp-set-options rp-public \
>>> +                       maintain-vrf=true \
>>> +                       redistribute-lb-vips=true
>>> +
>>> +check ovn-nbctl set logical_router R1 options:chassis=hv1
>>> +
>>> +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
>>> +    type=router options:router-port=rp-sw0 \
>>> +    -- lsp-set-addresses sw0-rp router
>>> +
>>> +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port 
>>> public-rp \
>>> +    type=router options:router-port=rp-public \
>>> +    -- lsp-set-addresses public-rp router
>>> +
>>> +check ovs-vsctl set Open_vSwitch . 
>>> external-ids:ovn-bridge-mappings=phynet:br-ext
>>> +
>>> +check ovn-nbctl lsp-add public public1 \
>>> +        -- lsp-set-addresses public1 unknown \
>>> +        -- lsp-set-type public1 localnet \
>>> +        -- lsp-set-options public1 network_name=phynet
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([test `ip -6 route show table 1001 | wc -l` -eq 1], [1])
>>> +
>>> +# Create a load balancer and associate to R1
>>> +check ovn-nbctl lb-add lb1 [[2001:db8:1001::150]]:80 
>>> [[2001:db8:1001::100]]:80
>>> +check ovn-nbctl lr-lb-add R1 lb1
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1001:.*UP])
>>> +AT_CHECK([test `ip -6 route show table 1001 | wc -l` -eq 1])
>>> +AT_CHECK([ip -6 route show table 1001 | grep -q 2001:db8:1001::150])
>>> +
>>> +
>>> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>> +
>>> +# Ensure system resources are cleaned up
>>> +AT_CHECK([ip link | grep -q ovnvrf1001:.*UP], [1])
>>> +AT_CHECK([test `ip -6 route show table 1001 | wc -l` -eq 1], [1])
>>> +
>>> +as ovn-sb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as ovn-nb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as northd
>>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>>> +
>>> +as
>>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>>> +/Failed to acquire.*/d
>>> +/connection dropped.*/d"])
>>> +AT_CLEANUP
>>> +])
>>> +
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([route-exchange for DNAT and DNAT_AND_SNAT with gateway router 
>>> IPv4])
>>> +AT_KEYWORDS([route-exchange])
>>> +
>>> +CHECK_VRF()
>>> +CHECK_CONNTRACK()
>>> +CHECK_CONNTRACK_NAT()
>>> +ovn_start
>>> +OVS_TRAFFIC_VSWITCHD_START()
>>> +ADD_BR([br-int])
>>> +ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone])
>>> +
>>> +# Set external-ids in br-int needed for ovn-controller
>>> +ovs-vsctl \
>>> +        -- set Open_vSwitch . external-ids:system-id=hv1 \
>>> +        -- set Open_vSwitch . 
>>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>>> +        -- set bridge br-int fail-mode=secure 
>>> other-config:disable-in-band=true
>>> +
>>> +# Start ovn-controller
>>> +start_daemon ovn-controller
>>> +
>>> +ovn-appctl vlog/set route_exchange
>>> +check ovn-nbctl -- lr-add R1 \
>>> +                -- set Logical_Router R1 options:requested-tnl-key=1002 
>>> options:dynamic-routing=true
>>> +
>>> +check ovn-nbctl ls-add sw0
>>> +check ovn-nbctl ls-add public
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1002:.*UP], [1])
>>> +
>>> +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
>>> +check ovn-nbctl -- lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24 \
>>> +                -- lrp-set-options rp-public \
>>> +                       maintain-vrf=true \
>>> +                       redistribute-nat=true
>>> +
>>> +check ovn-nbctl set logical_router R1 options:chassis=hv1
>>> +
>>> +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
>>> +    type=router options:router-port=rp-sw0 \
>>> +    -- lsp-set-addresses sw0-rp router
>>> +
>>> +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port 
>>> public-rp \
>>> +    type=router options:router-port=rp-public \
>>> +    -- lsp-set-addresses public-rp router
>>> +
>>> +check ovs-vsctl set Open_vSwitch . 
>>> external-ids:ovn-bridge-mappings=phynet:br-ext
>>> +
>>> +check ovn-nbctl lsp-add public public1 \
>>> +        -- lsp-set-addresses public1 unknown \
>>> +        -- lsp-set-type public1 localnet \
>>> +        -- lsp-set-options public1 network_name=phynet
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([test `ip route show table 1002 | wc -l` -eq 2], [1])
>>> +
>>> +# Create dnat_and_snat, dnat rules in R1
>>> +check ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.10 192.168.1.10
>>> +check ovn-nbctl lr-nat-add R1 dnat 172.16.1.11 192.168.1.11
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1002:.*UP])
>>> +AT_CHECK([test `ip route show table 1002 | wc -l` -eq 2])
>>> +AT_CHECK([ip route show table 1002 | grep -q 172.16.1.10])
>>> +AT_CHECK([ip route show table 1002 | grep -q 172.16.1.11])
>>> +
>>> +
>>> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>> +
>>> +# Ensure system resources are cleaned up
>>> +AT_CHECK([ip link | grep -q ovnvrf1000:.*UP], [1])
>>> +AT_CHECK([test `ip route show table 1002 | wc -l` -eq 1], [1])
>>> +
>>> +as ovn-sb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as ovn-nb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as northd
>>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>>> +
>>> +as
>>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>>> +/Failed to acquire.*/d
>>> +/connection dropped.*/d"])
>>> +AT_CLEANUP
>>> +])
>>> +
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([route-exchange for DNAT and DNAT_AND_SNAT with gateway router 
>>> IPv6])
>>> +AT_KEYWORDS([route-exchange])
>>> +
>>> +CHECK_VRF()
>>> +CHECK_CONNTRACK()
>>> +CHECK_CONNTRACK_NAT()
>>> +ovn_start
>>> +OVS_TRAFFIC_VSWITCHD_START()
>>> +ADD_BR([br-int])
>>> +ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone])
>>> +
>>> +# Set external-ids in br-int needed for ovn-controller
>>> +ovs-vsctl \
>>> +        -- set Open_vSwitch . external-ids:system-id=hv1 \
>>> +        -- set Open_vSwitch . 
>>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>>> +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>>> +        -- set bridge br-int fail-mode=secure 
>>> other-config:disable-in-band=true
>>> +
>>> +# Start ovn-controller
>>> +start_daemon ovn-controller
>>> +
>>> +ovn-appctl vlog/set route_exchange
>>> +check ovn-nbctl -- lr-add R1 \
>>> +                -- set Logical_Router R1 options:requested-tnl-key=1003 
>>> options:dynamic-routing=true
>>> +
>>> +check ovn-nbctl ls-add sw0
>>> +check ovn-nbctl ls-add public
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1003:.*UP], [1])
>>> +
>>> +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 2001:db8:100::1/64
>>> +check ovn-nbctl -- lrp-add R1 rp-public 00:00:02:01:02:03 
>>> 2001:db8:1003::1/64 \
>>> +                -- lrp-set-options rp-public \
>>> +                       maintain-vrf=true \
>>> +                       redistribute-nat=true
>>> +
>>> +check ovn-nbctl set logical_router R1 options:chassis=hv1
>>> +
>>> +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
>>> +    type=router options:router-port=rp-sw0 \
>>> +    -- lsp-set-addresses sw0-rp router
>>> +
>>> +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port 
>>> public-rp \
>>> +    type=router options:router-port=rp-public \
>>> +    -- lsp-set-addresses public-rp router
>>> +
>>> +check ovs-vsctl set Open_vSwitch . 
>>> external-ids:ovn-bridge-mappings=phynet:br-ext
>>> +
>>> +check ovn-nbctl lsp-add public public1 \
>>> +        -- lsp-set-addresses public1 unknown \
>>> +        -- lsp-set-type public1 localnet \
>>> +        -- lsp-set-options public1 network_name=phynet
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([test `ip -6 route show table 1003 | wc -l` -eq 2], [1])
>>> +
>>> +# Create dnat_and_snat, dnat rules in R1
>>> +check ovn-nbctl lr-nat-add R1 \
>>> +    dnat_and_snat 2001:db8:1003::150 2001:db8:100::100
>>> +check ovn-nbctl lr-nat-add R1 \
>>> +    dnat 2001:db8:1003::151 2001:db8:100::100
>>> +
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +AT_CHECK([ip link | grep -q ovnvrf1003:.*UP])
>>> +AT_CHECK([test `ip -6 route show table 1003 | wc -l` -eq 2])
>>> +AT_CHECK([ip -6 route show table 1003 | grep -q 2001:db8:1003::150])
>>> +AT_CHECK([ip -6 route show table 1003 | grep -q 2001:db8:1003::151])
>>> +
>>> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>> +
>>> +# Ensure system resources are cleaned up
>>> +AT_CHECK([ip link | grep -q ovnvrf1003:.*UP], [1])
>>> +AT_CHECK([test `ip -6 route show table 1003 | wc -l` -eq 2], [1])
>>> +
>>> +as ovn-sb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as ovn-nb
>>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> +
>>> +as northd
>>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>>> +
>>> +as
>>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>>> +/Failed to acquire.*/d
>>> +/connection dropped.*/d"])
>>> +AT_CLEANUP
>>> +])
>>> +
>>> --
>>> 2.43.0
>>
>> We could also have a unit test for this for quicker iteration, I wrote
>> one while working on the diff injected above:
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index 47efd8258..676db246d 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -14678,3 +14678,137 @@ AT_CHECK([ovn-sbctl --columns ip_prefix
>> --bare find Advertised_Route datapath=$d
>>  AT_CLEANUP
>>  ])
>>
>> +OVN_FOR_EACH_NORTHD_NO_HV([
>> +AT_SETUP([dynamic-routing - nat sync to sb])
>> +AT_KEYWORDS([dynamic-routing])
>> +ovn_start
>> +
>> +# Start with GW router and a single LRP
>> +check ovn-nbctl lr-add lr0
>> +check ovn-nbctl \
>> +    -- \
>> +    set Logical_Router lr0 options:dynamic-routing=true \
>> +                           options:chassis=hv1
>> +check ovn-nbctl --wait=sb \
>> +    -- \
>> +    lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
>> +
>> +check_row_count Advertised_Route 0
>> +
>> +datapath=$(ovn-sbctl --bare --columns _uuid list datapath_binding lr0)
>> +pb=$(ovn-sbctl --bare --columns _uuid list port_binding lr0-sw0)
>> +
>> +# Adding LRP dynamic-routing-nat option and NAT rule advertises a route 
>> entry
>> +check ovn-nbctl --wait=sb \
>> +    -- \
>> +    lrp-set-options lr0-sw0 redistribute-nat=true \
>> +    -- \
>> +    lr-nat-add lr0 dnat_and_snat 172.16.1.10 192.168.1.10
>> +
>> +ovn-nbctl list NAT
>> +ovn-sbctl list Advertised_Route
>> +ovn-sbctl lflow-list
>> +
>> +check_row_count Advertised_Route 1
>> +# XXX the missing /32 in the ip_prefix below is probably incorrect?
>> +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Advertised_Route
>> datapath=$datapath logical_port=$pb], [0], [dnl
>> +172.16.1.10
>> +])
>> +
>> +# Add LR with distributed LRP connected to GW router through join LS
>> +check ovn-nbctl \
>> +    -- \
>> +    lrp-add lr0 lr0-join 00:00:00:00:ff:02 10.42.0.1/24 \
>> +    -- \
>> +    ls-add ls-join \
>> +    -- \
>> +    lsp-add ls-join lsp-join-to-lr0 \
>> +    -- \
>> +    lsp-set-type lsp-join-to-lr0 router \
>> +    -- \
>> +    lsp-set-options lsp-join-to-lr0 router-port=lr0-join \
>> +    -- \
>> +    lsp-set-addresses lsp-join-to-lr0 router \
>> +    -- \
>> +    lr-add lr-guest0 \
>> +    -- \
>> +    lrp-add lr-guest0 lrp-guest0-sw0 00:00:00:00:fe:01 10.51.0.1/24 \
>> +    -- \
>> +    lrp-add lr-guest0 lrp-guest0-join 00:00:00:00:fe:02 10.42.0.2/24 \
>> +    -- \
>> +    lrp-set-options lrp-guest0-join redistribute-nat=true \
>> +    -- \
>> +    lsp-add ls-join lsp-join-to-guest0 \
>> +    -- \
>> +    lsp-set-type lsp-join-to-guest0 router \
>> +    -- \
>> +    lsp-set-options lsp-join-to-guest0 router-port=lrp-guest0-join \
>> +    -- \
>> +    lrp-set-gateway-chassis lrp-guest0-join hv1
>> +
>> +pb2=$(ovn-sbctl --bare --columns _uuid list port_binding lrp-guest0-join)
>> +
>> +check ovn-nbctl --wait=sb \
>> +    --add-route lr-nat-add lr-guest0 dnat_and_snat 172.16.2.10 192.168.2.10
>> +
>> +check_row_count Advertised_Route 2
>> +# XXX the missing /32 in the ip_prefix below is probably incorrect?
>> +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Advertised_Route
>> datapath=$datapath logical_port=$pb], [0], [dnl
>> +172.16.1.10
>> +])
>> +# XXX the missing /32 in the ip_prefix below is probably incorrect?
>> +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Advertised_Route
>> datapath=$datapath logical_port=$pb2], [0], [dnl
>> +172.16.2.10
>> +])
>> +
>> +# Add nonlocal LR with distributed LRP connected to GW router through join 
>> LS
>> +check ovn-nbctl \
>> +    -- \
>> +    lr-add lr-guest1 \
>> +    -- \
>> +    lrp-add lr-guest1 lrp-guest1-sw0 00:00:00:00:fd:01 10.51.1.1/24 \
>> +    -- \
>> +    lrp-add lr-guest1 lrp-guest1-join 00:00:00:00:fd:02 10.42.0.3/24 \
>> +    -- \
>> +    lrp-set-options lrp-guest1-join redistribute-nat=true \
>> +    -- \
>> +    lsp-add ls-join lsp-join-to-guest1 \
>> +    -- \
>> +    lsp-set-type lsp-join-to-guest1 router \
>> +    -- \
>> +    lsp-set-options lsp-join-to-guest1 router-port=lrp-guest1-join \
>> +    -- \
>> +    lrp-set-gateway-chassis lrp-guest1-join nonlocalhv
>> +
>> +pb3=$(ovn-sbctl --bare --columns _uuid list port_binding lrp-guest1-join)
>> +
>> +check ovn-nbctl --wait=sb \
>> +    --add-route lr-nat-add lr-guest1 dnat_and_snat 172.16.3.10 192.168.3.10
>> +check_row_count Advertised_Route 3
>> +# XXX the missing /32 in the ip_prefix below is probably incorrect?
>> +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Advertised_Route
>> datapath=$datapath logical_port=$pb], [0], [dnl
>> +172.16.1.10
>> +])
>> +# XXX the missing /32 in the ip_prefix below is probably incorrect?
>> +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Advertised_Route
>> datapath=$datapath logical_port=$pb2], [0], [dnl
>> +172.16.2.10
>> +])
>> +# XXX the missing /32 in the ip_prefix below is probably incorrect?
>> +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Advertised_Route
>> datapath=$datapath logical_port=$pb3], [0], [dnl
>> +172.16.3.10
>> +])
>> +
>> +# removing the option:dynamic-routing removes all routes
>> +check ovn-nbctl --wait=sb remove Logical_Router lr0 option dynamic-routing
>> +check_row_count Advertised_Route 0
>> +
>> +# and setting it again adds them again
>> +check ovn-nbctl --wait=sb set Logical_Router lr0 option:dynamic-routing=true
>> +check_row_count Advertised_Route 3
>> +
>> +# removing the lr will remove all routes
>> +check ovn-nbctl --wait=sb lr-del lr0
>> +check_row_count Advertised_Route 0
>> +
>> +AT_CLEANUP
>> +])
>> ---
>>
>> --
>> Frode Nordahl
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

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

Reply via email to