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,

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
    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 ?
    Is my understanding correct ?  If so,  this logical port will use
the SAME mac as the logical router port.
    What will be the type of this logical port ?
    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.

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.


Thanks
Numan


> > ---
> >  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