On Tue, Jan 28, 2025 at 11:25 AM Frode Nordahl <[email protected]> wrote:
>
> On Tue, Jan 28, 2025 at 5:03 PM Numan Siddique <[email protected]> wrote:
> >
> > On Tue, Jan 28, 2025 at 10:19 AM Frode Nordahl <[email protected]> wrote:
> > >
> > > Hello all,
> > >
> > > Apologies for the tardy response, I've been pursuing an interesting
> > > bug related to the "add_route" NAT option and routing IPv4 over IPv6
> > > next hop, and wanted to see that to an end before chiming in. Patches
> > > imminent.
> > >
> > > On Tue, Jan 28, 2025 at 1:50 PM Dumitru Ceara <[email protected]> wrote:
> > > >
> > > > On 1/28/25 1:33 PM, [email protected] wrote:
> > > > > Hi Frode/Numan/Dumitru,
> > > > > Thanks for the review and interest in this patch, I'll put my two
> > > > > cents
> > > > > to the discussion below.
> > > > >
> > > >
> > > > Hi Martin,
> > > >
> > > > > On Mon, 2025-01-27 at 11:57 +0100, Dumitru Ceara wrote:
> > > > >> 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?
> > > > >
> > > > > Yeah, "dynamic-routing-redistribute-lb-vips" is kinda mouthful, but if
> > > > > that's the prefix for every other related option, we should follow it.
> > > > >
> > > >
> > > > Alternatively we could try to change the dynamic-routing-redistribute-*
> > > > options and instead adding a boolean for each "redistribute" option we
> > > > could change "dynamic-routing-redistribute" to be a list of entities we
> > > > want to redistribute, e.g.:
> > > >
> > > > LR.options.dynamic-routing-redistribute="connected;nat;lb"
> > >
> > > On the topic of NAT and LB, as discovered during review, there is an
> > > oddly named get_nat_addresses() function that can retrieve both NAT
> > > and LB addresses.
> > >
> > > To save us the work of refactoring it, perhaps we could use a single
> > > keyword for both NAT and LB addresses? I mean, would you ever want one
> > > without the other?
> > >
> > > > What do you guys think?
> > > >
> > > > >>>>
> > > > >>>>
> > > > >>>>> 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.
> > > > >>
> > > > >
> > > > > I agree with everything that Dumitru very well summarized above, thank
> > > > > you. Just one question about the multi-node tests. The commit above,
> > > > > that Dumitru linked, also includes system tests for "routing-protocol-
> > > > > redirect". While it's not a multinode, it tests that process on
> > > > > "external network" can connect to BGP port on LRP, ensuring that BGP
> > > > > control plane is redirected to the specified LSP.
> > > > > Are we talking here about multinode version of that test, or a
> > > > > multinode test for leaking NAT/LB addresses to VRF? Because in my mind
> > > > > these are two distinct (although related) features.
> > > > >
> > > >
> > > > I think that ideally we should try to cover all supported cases in a
> > > > multi-node e2e test suite. I'm not sure we will be able to (time-wise)
> > > > though.
> > >
> > > We see the value of this too, and we are prepared to put some hands on
> > > that task. We have not done any multi node system tests before though,
> > > so your concerns about time are valid, we'll see how far we get! If
> > > you have any pointers that would be helpful (looping in MJ).
> > >
> > > >
> > > > >>> 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.
> > > > >
> > > > > This is a good point, I'll take a look at whether we need special
> > > > > handling for Distributed NAT in this patch. But as Dumitru said, these
> > > > > in-flight patch series for Fabric integration extend the support from
> > > > > just gateway routers to distributed routers as well. In that case, the
> > > > > FRR would have to be run on every compute node connected to the
> > > > > fabric/provider network. I have my own questions about the DR support,
> > > > > I'll raise those in the Fabric Integration series.
> > > > >
> > >
> > > In our target architecture we want to set up a gateway router on every
> > > chassis that speaks BGP, so the need for a routing protocol daemon to
> > > run on every chassis is not tied to the DGP support.
> > >
> > > We have a working POC with in-flight patches and OpenStack where we
> > > slip a gateway router in on every hypervisor, using OpenStack's
> > > "provider" network as a join LS.
> > >
> > > On the topic of distributed NAT what we have yet to verify is whether
> > > the filtering of advertised routes works as expected in the on-list
> > > patches. The original controller series made an attempt at only ever
> > > announcing NAT/LB addresses that actually had logical ports to
> > > instances local to its chassis. Meaning that a NAT address should only
> > > be announced by a single gateway router, which should avoid any
> > > concerns about asymmetric routing which does not work well with CT
> > > state.
> > >
> > > For LB's the story is a bit different, as you can have a single VIP
> > > with backends across multiple chassis, and given hashing is configured
> > > correctly in the fabric, it could be safe to have traffic balance
> > > across multiple chassis.
> >
> >
> > Thanks for the replies. With my initial glance at the patches, I didn't
> > see
> > special handling for dnat_and_snat entries with external_mac and
> > logical_port set.
> >
> > Let's say there are 2 NAT entries of type dnat_and_snat
> > D1 -> [ 172.16.0.100 (external_ip), 10.0.0.100 (private_ip) ,
> > EXTMAC1, sw0-port1]
> > D2 -> [ 172.16.0.110 (external_ip), 10.0.0.110 (private_ip) ]
> >
> > I'd expect the BGP route for D1 will be advertised from the compute
> > node C1 BGP speaker where sw0-port1 is claimed
> > and D2 will be advertised from the gateway chassis BGP speaker which
> > claims "cr-lrp-<>" port.
> >
> > Please correct me if my understanding is wrong.
>
> There are multiple approaches to this being discussed on the list, and
> we do not intend to associate a BGP speaker directly with a DGP.
>
> Our goal with this is to be able to support a physical network
> topology where chassis do not share layer 2 broadcast domains for
> their external connectivity (e.g it can work without EVPN). Another
> goal is to be able to direct traffic to its intended destination,
> avoiding "tromboning" of traffic through tunnels when not needed.
>
> So in your above example we would have a gateway router on C1 and
> another gateway router on C2, the BGP speaker on C1 would announce D1
> and the BGP speaker on C2 would announce D2.
>
> In the "provider" LS we have removed the localnet port.
>
> GWLRC1 has an LRP to the "provider" LS where it can reach GUESTLR
> dynamically through the use of LB/NAT "add_route" option, same setup
> on C2.
>
> The return path works with source routes so that instances with
> dnat_and_snat are set up to use the local GWLR as their default GW (we
> can probably make it more sophisticated through policy routing, or a
> future feature).
>
> I'll throw together a script that displays the setup in a pastebin, as
> that is probably easier to consume.
Thanks for the detailed explanation. A script would be really helpful.
Thanks
Numan
>
> --
> Frode Nordahl
>
> >
> > If my understanding is correct, then can you please help me
> > understand what MAC address C1 BGP speaker will use
> > in its logical switch port ? For the gateway chassis BGP speaker, I
> > presume it will use the router port mac.
> > Will both theC1 BGP speaker and gateway chassis BGP speaker use the
> > same router port MAC ? If so,
> > there would be mac flapping right ?
> >
> >
> > Thanks
> > Numan
> >
> > >
> > > > Ack, looking forward to reviews.
> > >
> > > Expect that to ramp up momentarily :)
> > >
> > > --
> > > Frode Nordahl
> > >
> > > > >>
> > > > >>>
> > > > >>> 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?
> > > > >
> > > > > Yeah, sounds good, I'll look into 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:
> > > > >
> > > > > Thanks for these two diffs below. I'll include them in v2.
> > > > >
> > > > >>>> 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
> > > >
> > > > Regards,
> > > > Dumitru
> > > >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
