On Tue, 2025-01-28 at 17:42 +0100, Felix Huettner wrote: > Hi everyone, > > i still need to review the actual patch, but i wanted to join the > discussion. > > On Tue, Jan 28, 2025 at 04:19:37PM +0100, Frode Nordahl 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 <dce...@redhat.com> > > wrote: > > > > > > On 1/28/25 1:33 PM, martin.kal...@canonical.com 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 > > > > > > <fnord...@ubuntu.com> > > > > > > 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 > > > > > > > <martin.kal...@canonical.com> 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 <fnord...@ubuntu.com> > > > > > > > > Signed-off-by: Frode Nordahl <fnord...@ubuntu.com> > > > > > > > > > > > > > > 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? > > "dynamic-routing-redistribute" looks like the shorter version. > However i see an advantage in the individual "dynamic-routing- > static", > etc. options as they could allow us to use more values than just > "true" > and "false". E.g. they could in the future be used to filter for a > given > prefix and only announce that. > > But that is just an idea on a potential benefit. I don't actually see > a > reason to currently built that.
It occurred to me that we could also use `dynamic-routing- nat`/`dynamic-routing-lb-vips` names. This would be both shorter and kept the door open for non-boolean values in the future. Martin. > > > > > > What do you guys think? > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Martin Kalcok > > > > > > > > <martin.kal...@canonical.com> > > > > > > > > > > > > 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). > > That would be awesome if you would work on that. I guess we could > then > split this up again so that everyone builds a test validating their > individual use case. Then we get a broad coverage of this feature. > > > > > > > > > > > > 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. > > If you also add a tracked_port during advertised_route_table_sync > then > you should get different route priorities based on if the port is > local > or not. > That should then be useable by frr to decide if to announce a given > route or not. > > Would that maybe solve that usecase? > > Thanks a lot, > Felix > > > > > 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. > > > > > 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
