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

Reply via email to