On 1/30/25 2:43 PM, [email protected] wrote:
> On Tue, 2025-01-28 at 16:19 +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 <[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.
>
> Regarding the "get_nat_addresses", it can return LB VIPs only if you
> pass it a "lr_stateful_record" and that proved a bit troublesome. The
> invocation of this function goes
>
> en_routes_run (en-northd.c) -> build_parsed_routes (northd.c) ->
> parsed_routes_add_nat (northd.c) -> get_nat_addresses (northd.c)
>
> We could get the stateful record by calling
> 'engine_get_input_data("lr_stateful", node)' in the en_routes_run, but
> no other function in the "en-northd.c" does so. I tried it but the
> tests then fail on `ovn-nbctl --wait=hv sync`.
>
The problem is the incremental processing "en_routes" node in the I-P
DAG doesn't have "lr_stateful" as direct input. It only has these two
inputs:
engine_add_input(&en_routes, &en_bfd, NULL);
engine_add_input(&en_routes, &en_northd,
routes_northd_change_handler);
So calling engine_get_input_data("lr_stateful", node) will actually fail
an assertion if "node" == "en_routes".
struct engine_node *
engine_get_input(const char *input_name, struct engine_node *node)
{
size_t i;
for (i = 0; i < node->n_inputs; i++) {
if (!strcmp(node->inputs[i].node->name, input_name)) {
return node->inputs[i].node;
}
}
OVS_NOT_REACHED();
return NULL;
}
> I'm not that familiar with the northd engine nodes, but i assume that
> there's some deadlock situation going on. I could try to dig the root
> cause, but if the current approach is not problematic I'd maybe stick
> with that.
If we really want to access lr_stateful data we need to add a connection
in the I-P DAG from en_lr_stateful to en_routes, i.e., something like:
engine_add_input(&en_routes, &en_lr_stateful,
routes_lr_stateful_change_handler);
Where the handler takes care of the incremental processing of stateful
data (in this case it might be fine to just use engine_noop_handler()
because we don't need to reprocess anything).
Hope this helps.
Dumitru
>
> Martin.
>>
>> 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.
>>
>>> 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
