On Mon, Feb 03, 2025 at 05:09:01PM +0100, Dumitru Ceara wrote:
> On 2/3/25 5:06 PM, [email protected] wrote:
> > On Mon, 2025-02-03 at 16:47 +0100, Felix Huettner wrote:
> >> On Mon, Feb 03, 2025 at 04:23:00PM +0100, Dumitru Ceara wrote:
> >>> On 2/3/25 4:12 PM, [email protected] wrote:
> >>>> On Fri, 2025-01-31 at 12:49 +0100, Dumitru Ceara wrote:
> >>>>> On 1/30/25 10:51 PM, Dumitru Ceara wrote:
> >>>>>> On 1/21/25 4:47 PM, Felix Huettner via dev wrote:
> >>>>>>> in order to exchange routes between OVN and the network
> >>>>>>> fabric we
> >>>>>>> use the new Advertised_Route sb table. Northd here
> >>>>>>> advertises all
> >>>>>>> routes
> >>>>>>> where the user explicitly opted-in.
> >>>>>>>
> >>>>>>> ovn-controller will later use this table to share these
> >>>>>>> routes to
> >>>>>>> the
> >>>>>>> outside.
> >>>>>>>
> >>>>>>> Signed-off-by: Felix Huettner
> >>>>>>> <[email protected]>
> >>>>>>> ---
> >>>>>>
> >>>>>> Hi Felix,
> >>>>>>
> >>>>>> I have a few small comments below.  Aside from those this
> >>>>>> patch
> >>>>>> looks in
> >>>>>> good shape to me.
> >>>>>>
> >>>>>>> v3->v4:
> >>>>>>>   * fix use after free
> >>>>>>> v2->v3:
> >>>>>>>   * A lot of minor review comments.
> >>>>>>>   * Sync logic reworked to no longer need a "stale" field.
> >>>>>>>   * Stop watching Advertised_Route table for changes.
> >>>>>>>
> >>>>>>>  NEWS                              |   4 +
> >>>>>>>  ic/ovn-ic.c                       |  21 ----
> >>>>>>>  lib/ovn-util.c                    |  22 ++++
> >>>>>>>  lib/ovn-util.h                    |   2 +
> >>>>>>>  lib/stopwatch-names.h             |   1 +
> >>>>>>>  northd/automake.mk                |   2 +
> >>>>>>>  northd/en-advertised-route-sync.c | 186
> >>>>>>> ++++++++++++++++++++++++++++++
> >>>>>>>  northd/en-advertised-route-sync.h |  29 +++++
> >>>>>>>  northd/en-northd-output.c         |   8 ++
> >>>>>>>  northd/en-northd-output.h         |   2 +
> >>>>>>>  northd/inc-proc-northd.c          |  11 +-
> >>>>>>>  northd/northd.c                   |  29 +++--
> >>>>>>>  northd/northd.h                   |   6 +-
> >>>>>>>  northd/ovn-northd.c               |   5 +
> >>>>>>>  ovn-nb.xml                        |  13 +++
> >>>>>>>  tests/ovn-northd.at               | 137
> >>>>>>> ++++++++++++++++++++++
> >>>>>>>  16 files changed, 443 insertions(+), 35 deletions(-)
> >>>>>>>  create mode 100644 northd/en-advertised-route-sync.c
> >>>>>>>  create mode 100644 northd/en-advertised-route-sync.h
> >>>>>>>
> >>>>>>> diff --git a/NEWS b/NEWS
> >>>>>>> index 72c5a6339..4ff158014 100644
> >>>>>>> --- a/NEWS
> >>>>>>> +++ b/NEWS
> >>>>>>> @@ -28,6 +28,10 @@ Post v24.09.0
> >>>>>>>         this option was not set).  TLS ciphersuites for
> >>>>>>> TLSv1.3
> >>>>>>> and later can
> >>>>>>>         be configured via --ssl-ciphersuites (--ssl-ciphers
> >>>>>>> only
> >>>>>>> applies to
> >>>>>>>         TLSv1.2 and earlier).
> >>>>>>> +   - Dynamic Routing:
> >>>>>>> +     * Add the option "dynamic-routing" to Logical
> >>>>>>> Routers. If
> >>>>>>> set to true all
> >>>>>>> +       static and connected routes attached to the router
> >>>>>>> are
> >>>>>>> shared to the
> >>>>>>> +       southbound "Route" table for sharing outside of
> >>>>>>> OVN.
> >>>>>>>  
> >>>>>>>  OVN v24.09.0 - 13 Sep 2024
> >>>>>>>  --------------------------
> >>>>>>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> >>>>>>> index 8320cbea5..948c17f9b 100644
> >>>>>>> --- a/ic/ovn-ic.c
> >>>>>>> +++ b/ic/ovn-ic.c
> >>>>>>> @@ -1008,27 +1008,6 @@
> >>>>>>> get_nexthop_from_lport_addresses(bool
> >>>>>>> is_v4,
> >>>>>>>      return true;
> >>>>>>>  }
> >>>>>>>  
> >>>>>>> -static bool
> >>>>>>> -prefix_is_link_local(struct in6_addr *prefix, unsigned int
> >>>>>>> plen)
> >>>>>>> -{
> >>>>>>> -    if (IN6_IS_ADDR_V4MAPPED(prefix)) {
> >>>>>>> -        /* Link local range is "169.254.0.0/16". */
> >>>>>>> -        if (plen < 16) {
> >>>>>>> -            return false;
> >>>>>>> -        }
> >>>>>>> -        ovs_be32 lla;
> >>>>>>> -        inet_pton(AF_INET, "169.254.0.0", &lla);
> >>>>>>> -        return ((in6_addr_get_mapped_ipv4(prefix) &
> >>>>>>> htonl(0xffff0000)) == lla);
> >>>>>>> -    }
> >>>>>>> -
> >>>>>>> -    /* ipv6, link local range is "fe80::/10". */
> >>>>>>> -    if (plen < 10) {
> >>>>>>> -        return false;
> >>>>>>> -    }
> >>>>>>> -    return (((prefix->s6_addr[0] & 0xff) == 0xfe) &&
> >>>>>>> -            ((prefix->s6_addr[1] & 0xc0) == 0x80));
> >>>>>>> -}
> >>>>>>> -
> >>>>>>>  static bool
> >>>>>>>  prefix_is_deny_listed(const struct smap *nb_options,
> >>>>>>>                        struct in6_addr *prefix,
> >>>>>>> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> >>>>>>> index b78bdbfa1..ed847517a 100644
> >>>>>>> --- a/lib/ovn-util.c
> >>>>>>> +++ b/lib/ovn-util.c
> >>>>>>> @@ -1351,3 +1351,25 @@ ovn_update_swconn_at(struct rconn
> >>>>>>> *swconn,
> >>>>>>> const char *target,
> >>>>>>>  
> >>>>>>>      return notify;
> >>>>>>>  }
> >>>>>>> +
> >>>>>>> +bool
> >>>>>>> +prefix_is_link_local(const struct in6_addr *prefix,
> >>>>>>> unsigned int
> >>>>>>> plen)
> >>>>>>> +{
> >>>>>>> +    if (IN6_IS_ADDR_V4MAPPED(prefix)) {
> >>>>>>> +        /* Link local range is "169.254.0.0/16". */
> >>>>>>> +        if (plen < 16) {
> >>>>>>> +            return false;
> >>>>>>> +        }
> >>>>>>> +        ovs_be32 lla;
> >>>>>>> +        inet_pton(AF_INET, "169.254.0.0", &lla);
> >>>>>>> +        return ((in6_addr_get_mapped_ipv4(prefix) &
> >>>>>>> htonl(0xffff0000)) == lla);
> >>>>>>> +    }
> >>>>>>> +
> >>>>>>> +    /* ipv6, link local range is "fe80::/10". */
> >>>>>>> +    if (plen < 10) {
> >>>>>>> +        return false;
> >>>>>>> +    }
> >>>>>>> +    return (((prefix->s6_addr[0] & 0xff) == 0xfe) &&
> >>>>>>> +            ((prefix->s6_addr[1] & 0xc0) == 0x80));
> >>>>>>> +}
> >>>>>>> +
> >>>>>>
> >>>>>> Nit: one newline too many.
> >>>>>>
> >>>>>>> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> >>>>>>> index 899bd9d12..31c2c68df 100644
> >>>>>>> --- a/lib/ovn-util.h
> >>>>>>> +++ b/lib/ovn-util.h
> >>>>>>> @@ -487,4 +487,6 @@ void ovn_exit_args_finish(struct
> >>>>>>> ovn_exit_args *exit_args);
> >>>>>>>  bool ovn_update_swconn_at(struct rconn *swconn, const char
> >>>>>>> *target,
> >>>>>>>                            int probe_interval, const char
> >>>>>>> *where);
> >>>>>>>  
> >>>>>>> +bool prefix_is_link_local(const struct in6_addr *prefix,
> >>>>>>> unsigned int plen);
> >>>>>>> +
> >>>>>>>  #endif /* OVN_UTIL_H */
> >>>>>>> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
> >>>>>>> index 660c653fb..dc4129ee5 100644
> >>>>>>> --- a/lib/stopwatch-names.h
> >>>>>>> +++ b/lib/stopwatch-names.h
> >>>>>>> @@ -34,5 +34,6 @@
> >>>>>>>  #define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
> >>>>>>>  #define LR_STATEFUL_RUN_STOPWATCH_NAME "lr_stateful"
> >>>>>>>  #define LS_STATEFUL_RUN_STOPWATCH_NAME "ls_stateful"
> >>>>>>> +#define ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME
> >>>>>>> "advertised_route_sync"
> >>>>>>>  
> >>>>>>>  #endif
> >>>>>>> diff --git a/northd/automake.mk b/northd/automake.mk
> >>>>>>> index 6566ad299..a2797237a 100644
> >>>>>>> --- a/northd/automake.mk
> >>>>>>> +++ b/northd/automake.mk
> >>>>>>> @@ -34,6 +34,8 @@ northd_ovn_northd_SOURCES = \
> >>>>>>>       northd/en-ls-stateful.h \
> >>>>>>>       northd/en-sampling-app.c \
> >>>>>>>       northd/en-sampling-app.h \
> >>>>>>> +     northd/en-advertised-route-sync.c \
> >>>>>>> +     northd/en-advertised-route-sync.h \
> >>>>>>>       northd/inc-proc-northd.c \
> >>>>>>>       northd/inc-proc-northd.h \
> >>>>>>>       northd/ipam.c \
> >>>>>>> diff --git a/northd/en-advertised-route-sync.c b/northd/en-
> >>>>>>> advertised-route-sync.c
> >>>>>>> new file mode 100644
> >>>>>>> index 000000000..6ddd15512
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/northd/en-advertised-route-sync.c
> >>>>>>> @@ -0,0 +1,186 @@
> >>>>>>> +/*
> >>>>>>> + * Copyright (c) 2024, STACKIT GmbH & Co. KG
> >>>>>>
> >>>>>> Nit: 2025
> >>>>>>
> >>>>>>> + *
> >>>>>>> + * Licensed under the Apache License, Version 2.0 (the
> >>>>>>> "License");
> >>>>>>> + * you may not use this file except in compliance with the
> >>>>>>> License.
> >>>>>>> + * You may obtain a copy of the License at:
> >>>>>>> + *
> >>>>>>> + *     http://www.apache.org/licenses/LICENSE-2.0
> >>>>>>> + *
> >>>>>>> + * Unless required by applicable law or agreed to in
> >>>>>>> writing,
> >>>>>>> software
> >>>>>>> + * distributed under the License is distributed on an "AS
> >>>>>>> IS"
> >>>>>>> BASIS,
> >>>>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
> >>>>>>> express
> >>>>>>> or implied.
> >>>>>>> + * See the License for the specific language governing
> >>>>>>> permissions and
> >>>>>>> + * limitations under the License.
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> +#include <config.h>
> >>>>>>> +
> >>>>>>> +#include "stopwatch.h"
> >>>>>>> +#include "northd.h"
> >>>>>>> +
> >>>>>>> +#include "en-advertised-route-sync.h"
> >>>>>>> +#include "lib/stopwatch-names.h"
> >>>>>>> +#include "openvswitch/hmap.h"
> >>>>>>> +#include "ovn-util.h"
> >>>>>>> +
> >>>>>>> +static void
> >>>>>>> +advertised_route_table_sync(
> >>>>>>> +    struct ovsdb_idl_txn *ovnsb_txn,
> >>>>>>> +    const struct sbrec_advertised_route_table
> >>>>>>> *sbrec_advertised_route_table,
> >>>>>>> +    const struct hmap *parsed_routes);
> >>>>>>> +
> >>>>>>> +void
> >>>>>>> +*en_advertised_route_sync_init(struct engine_node *node
> >>>>>>> OVS_UNUSED,
> >>>>>>> +                     struct engine_arg *arg OVS_UNUSED)
> >>>>>>> +{
> >>>>>>> +    return NULL;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +void
> >>>>>>> +en_advertised_route_sync_cleanup(void *data OVS_UNUSED)
> >>>>>>> +{
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +void
> >>>>>>> +en_advertised_route_sync_run(struct engine_node *node,
> >>>>>>> void
> >>>>>>> *data OVS_UNUSED)
> >>>>>>> +{
> >>>>>>> +    struct routes_data *routes_data
> >>>>>>> +        = engine_get_input_data("routes", node);
> >>>>>>> +    const struct engine_context *eng_ctx =
> >>>>>>> engine_get_context();
> >>>>>>> +    const struct sbrec_advertised_route_table
> >>>>>>> *sbrec_advertised_route_table =
> >>>>>>> +       
> >>>>>>> EN_OVSDB_GET(engine_get_input("SB_advertised_route",
> >>>>>>> node));
> >>>>>>> +
> >>>>>>> +   
> >>>>>>> stopwatch_start(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME,
> >>>>>>> time_msec());
> >>>>>>> +
> >>>>>>> +    advertised_route_table_sync(eng_ctx->ovnsb_idl_txn,
> >>>>>>> +                      sbrec_advertised_route_table,
> >>>>>>> +                      &routes_data->parsed_routes);
> >>>>>>
> >>>>>> Nit: indent under the parenthesis, i.e.:
> >>>>>>
> >>>>>>     advertised_route_table_sync(eng_ctx->ovnsb_idl_txn,
> >>>>>>                                 sbrec_advertised_route_table,
> >>>>>>                                 &routes_data->parsed_routes);
> >>>>>>
> >>>>>>> +
> >>>>>>> +   
> >>>>>>> stopwatch_stop(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME,
> >>>>>>> time_msec());
> >>>>>>> +    engine_set_node_state(node, EN_UPDATED);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +struct ar_entry {
> >>>>>>> +    struct hmap_node hmap_node;
> >>>>>>> +
> >>>>>>> +    const struct sbrec_datapath_binding *sb_db;
> >>>>>>> +
> >>>>>>> +    const struct sbrec_port_binding *logical_port;
> >>>>>>> +    char *ip_prefix;
> >>>>>>> +};
> >>>>>>> +
> >>>>>>> +static struct ar_entry *
> >>>>>>> +ar_alloc_entry(struct hmap *routes,
> >>>>>>> +               const struct sbrec_datapath_binding *sb_db,
> >>>>>>> +               const struct sbrec_port_binding
> >>>>>>> *logical_port,
> >>>>>>> +               char *ip_prefix)
> >>>>>>> +{
> >>>>>>> +    struct ar_entry *route_e = xzalloc(sizeof *route_e);
> >>>>>>> +
> >>>>>>> +    route_e->sb_db = sb_db;
> >>>>>>> +    route_e->logical_port = logical_port;
> >>>>>>> +    route_e->ip_prefix = ip_prefix;
> >>>>>>> +    uint32_t hash = uuid_hash(&sb_db->header_.uuid);
> >>>>>>> +    hash = hash_string(logical_port->logical_port, hash);
> >>>>>>> +    hash = hash_string(ip_prefix, hash);
> >>>>>>> +    hmap_insert(routes, &route_e->hmap_node, hash);
> >>>>>>> +
> >>>>>
> >>>>> This does more than just allocating the entry.  Let's call the
> >>>>> function
> >>>>> ar_add_entry() or similar.  What do you think?
> >>>>
> >>>> Hi Felix, Dumitru,
> >>>
> >>> Hi Martin,
> >>>
> >>>> I have one more question/comment for ar_alloc_entry.
> >>>> If you 'free' memory at pointer passed as the '*ip_prefix' after
> >>>> calling this function, northd will crash some time later when
> >>>> trying to
> >>>> serialize request to SB. Example of code that would cause the
> >>>> crash:
> >>>>
> >>>>     char *ip_prefix = normalize_v46_prefix(&route->prefix,
> >>>>                                            route->plen);
> >>>>     ar_alloc_entry(route_map,
> >>>>                   route->od->sb,
> >>>>                   route->out_port->sb,
> >>>>                   ip_prefix,
> >>>>                   tracked_pb);
> >>>>     free(ip_prefix);
> >>>>
> >>>> This caught me quite off guard. Could the function copy the value
> >>>> from
> >>>> `*ip_prefix` instead? Or alternatively add a comment that
> >>>> outlines this
> >>>> behavior.
> >>>>
> >>>
> >>> It kind of depends what semantics we want.  In the current form
> >>> ar_alloc_entry() takes ownership of the ip_prefix you pass it.  So
> >>> freeing ip_prefix outside afterwards would break the semantics.
> >>
> >> Hi Martin, Hi Dumitru,
> >>
> >> i wanted to take the ownership for ar_alloc_entry (or ar_add_entry)
> >> because we generally need to convert something to a string first.
> >> As we don't need this string afterwards i found that duplicating it
> >> seems quite unnecessary.
> >>
> >> Thanks a lot,
> >> Felix
> > 
> > Thanks for the explanation,
> > I don't have strong opinion on the ownership (whether to take it or
> > not), but i think that if it stays this way, a comment would go a long
> > way to avoid issues for future travelers.
> 
> That's a fair ask, +1.

Sounds good, i'll add one.

> 
> >>
> >>>
> >>>> I also noticed that in later patches in this series, in function
> >>>> `publish_host_routes`, there's code like this:
> >>>>
> >>>>     struct ovn_port_routable_addresses addrs = get_op_addresses(
> >>>>         port->peer, lr_stateful_rec, false);
> >>>>     for (size_t i = 0; i < addrs.n_addrs; i++) {
> >>>>         publish_lport_addresses(sync_routes, route->od->sb,
> >>>>                                 route->out_port,
> >>>>                                 &addrs.laddrs[i],
> >>>>                                 port->peer);
> >>>>     }
> >>>>     destroy_routable_addresses(&addrs);
> >>>>
> >>>> Here, the `&addrs.laddrs[i]` are passed to
> >>>> `publish_lport_addresses`
> >>>> which eventually passes `&addrs.laddrs[i]->ipv4_addrs[]` into
> >>>> `ar_alloc_entry` and then it destroys the `&addrs`. I'm bit
> >>>> usnsure if
> >>>> it's the way that `destroy_routable_addresses` destroys `&addrs`,
> >>>> or if
> >>>> it's just accidental, but in this case the north is not crashing.
> >>>>
> >>>
> >>> The difference in this case is that the call is:
> >>>
> >>>         const struct ipv4_netaddr *addr = &addresses-
> >>>> ipv4_addrs[i];
> >>>         char *addr_s = xasprintf("%s/32", addr->addr_s);
> >>>         ar_alloc_entry(sync_routes, sb_db, logical_port->sb,
> >>>                        addr_s, tracking_port->sb);
> >>>
> >>> So ar_alloc_entry() is getting again full ownership of the addr_s
> >>> we
> >>> built (and allocated) with the xasprintf() call.
> > 
> > Oh right, thanks for pointing that out, I missed the effect of
> > xasprintf.
> > 
> > 
> >>>
> >>> Hope this helps.
> >>>
> > It did, thanks ;)
> > 
> > Martin.
> > 
> >>> Dumitru
> >>>
> >>>> Martin.
> >>>>
> >>>>>
> >>>>>>> +    return route_e;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static struct ar_entry *
> >>>>>>> +ar_find(struct hmap *route_map,
> >>>>>>> +                    const struct sbrec_datapath_binding
> >>>>>>> *sb_db,
> >>>>>>> +                    const struct sbrec_port_binding
> >>>>>>> *logical_port,
> >>>>>>> +                    const char *ip_prefix)
> >>>>>>
> >>>>>> Nit: indentation
> >>>>>>
> >>>>>>> +{
> >>>>>>> +    struct ar_entry *route_e;
> >>>>>>> +    uint32_t hash;
> >>>>>>> +
> >>>>>>> +    hash = uuid_hash(&sb_db->header_.uuid);
> >>>>>>> +    hash = hash_string(logical_port->logical_port, hash);
> >>>>>>> +    hash = hash_string(ip_prefix, hash);
> >>>>>>> +    HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash,
> >>>>>>> route_map) {
> >>>>>>> +        if (!uuid_equals(&sb_db->header_.uuid,
> >>>>>>> +                         &route_e->sb_db->header_.uuid)) {
> >>>>>>> +            continue;
> >>>>>>> +        }
> >>>>>>> +
> >>>>>>> +        if (!uuid_equals(&logical_port->header_.uuid,
> >>>>>>> +                         &route_e->logical_port-
> >>>>>>>> header_.uuid))
> >>>>>>> {
> >>>>>>> +            continue;
> >>>>>>> +        }
> >>>>>>> +
> >>>>>>> +        if (strcmp(ip_prefix, route_e->ip_prefix)) {
> >>>>>>> +            continue;
> >>>>>>> +        }
> >>>>>>> +
> >>>>>>> +        return route_e;
> >>>>>>> +    }
> >>>>>>> +
> >>>>>>> +    return NULL;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static void
> >>>>>>> +ar_entry_free(struct ar_entry *route_e)
> >>>>>>> +{
> >>>>>>> +    free(route_e->ip_prefix);
> >>>>>>> +    free(route_e);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static void
> >>>>>>> +advertised_route_table_sync(
> >>>>>>> +    struct ovsdb_idl_txn *ovnsb_txn,
> >>>>>>> +    const struct sbrec_advertised_route_table
> >>>>>>> *sbrec_advertised_route_table,
> >>>>>>> +    const struct hmap *parsed_routes)
> >>>>>>> +{
> >>>>>>> +    struct hmap sync_routes =
> >>>>>>> HMAP_INITIALIZER(&sync_routes);
> >>>>>>> +
> >>>>>>
> >>>>>> Nit: no need for an empty line IMO.
> >>>>>>
> >>>>>>> +    const struct parsed_route *route;
> >>>>>>> +
> >>>>>>> +    struct ar_entry *route_e;
> >>>>>>> +    const struct sbrec_advertised_route *sb_route;
> >>>>>>> +    HMAP_FOR_EACH (route, key_node, parsed_routes) {
> >>>>>>> +        if (route->is_discard_route) {
> >>>>>>> +            continue;
> >>>>>>> +        }
> >>>>>>> +        if (prefix_is_link_local(&route->prefix, route-
> >>>>>>>> plen)) {
> >>>>>>> +            continue;
> >>>>>>> +        }
> >>>>>>> +        if (!smap_get_bool(&route->od->nbr->options,
> >>>>>>> "dynamic-
> >>>>>>> routing",
> >>>>>>> +                           false)) {
> >>>>>>> +            continue;
> >>>>>>> +        }
> >>>>>>
> >>>>>> This should be:
> >>>>>>
> >>>>>>         if (!route->od->dynamic_routing) {
> >>>>>>             continue;
> >>>>>>         }
> >>>>>>
> >>>>>> instead.
> >>>>>>
> >>>>>>> +
> >>>>>>> +        char *ip_prefix = normalize_v46_prefix(&route-
> >>>>>>>> prefix,
> >>>>>>> +                                               route-
> >>>>>>>> plen);
> >>>>>>> +        route_e = ar_alloc_entry(&sync_routes, route->od-
> >>>>>>>> sb,
> >>>>>>> +                                 route->out_port->sb,
> >>>>>>> ip_prefix);
> >>>>>>> +    }
> >>>>>>> +
> >>>>>>> +    SBREC_ADVERTISED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route,
> >>>>>>> +                                               
> >>>>>>> sbrec_advertised_route_table) {
> >>>>>>> +        route_e = ar_find(&sync_routes, sb_route-
> >>>>>>>> datapath,
> >>>>>>> +                          sb_route->logical_port,
> >>>>>>> +                          sb_route->ip_prefix);
> >>>>>>> +        if (route_e) {
> >>>>>>> +          hmap_remove(&sync_routes, &route_e->hmap_node);
> >>>>>>> +          ar_entry_free(route_e);
> >>>>>>> +        } else {
> >>>>>>> +          sbrec_advertised_route_delete(sb_route);
> >>>>>>> +        }
> >>>>>>> +    }
> >>>>>>> +
> >>>>>>> +    HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) {
> >>>>>>> +        const struct sbrec_advertised_route *sr =
> >>>>>>> +            sbrec_advertised_route_insert(ovnsb_txn);
> >>>>>>> +        sbrec_advertised_route_set_datapath(sr, route_e-
> >>>>>>>> sb_db);
> >>>>>>> +        sbrec_advertised_route_set_logical_port(sr,
> >>>>>>> route_e-
> >>>>>>>> logical_port);
> >>>>>>> +        sbrec_advertised_route_set_ip_prefix(sr, route_e-
> >>>>>>>> ip_prefix);
> >>>>>>> +        ar_entry_free(route_e);
> >>>>>>> +    }
> >>>>>>> +
> >>>>>>> +    hmap_destroy(&sync_routes);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> diff --git a/northd/en-advertised-route-sync.h b/northd/en-
> >>>>>>> advertised-route-sync.h
> >>>>>>> new file mode 100644
> >>>>>>> index 000000000..bd27ecefa
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/northd/en-advertised-route-sync.h
> >>>>>>> @@ -0,0 +1,29 @@
> >>>>>>> +/*
> >>>>>>> + * Copyright (c) 2024, STACKIT GmbH & Co. KG
> >>>>>>
> >>>>>> Nit: 2025
> >>>>>>
> >>>>>>> + *
> >>>>>>> + * Licensed under the Apache License, Version 2.0 (the
> >>>>>>> "License");
> >>>>>>> + * you may not use this file except in compliance with the
> >>>>>>> License.
> >>>>>>> + * You may obtain a copy of the License at:
> >>>>>>> + *
> >>>>>>> + *     http://www.apache.org/licenses/LICENSE-2.0
> >>>>>>> + *
> >>>>>>> + * Unless required by applicable law or agreed to in
> >>>>>>> writing,
> >>>>>>> software
> >>>>>>> + * distributed under the License is distributed on an "AS
> >>>>>>> IS"
> >>>>>>> BASIS,
> >>>>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
> >>>>>>> express
> >>>>>>> or implied.
> >>>>>>> + * See the License for the specific language governing
> >>>>>>> permissions and
> >>>>>>> + * limitations under the License.
> >>>>>>> + */
> >>>>>>> +#ifndef EN_ADVERTISED_ROUTE_SYNC_H
> >>>>>>> +#define EN_ADVERTISED_ROUTE_SYNC_H 1
> >>>>>>> +
> >>>>>>> +#include "lib/inc-proc-eng.h"
> >>>>>>> +
> >>>>>>> +struct advertised_route_sync_data {
> >>>>>>> +};
> >>>>>>> +
> >>>>>>> +void *en_advertised_route_sync_init(struct engine_node *,
> >>>>>>> struct
> >>>>>>> engine_arg *);
> >>>>>>> +void en_advertised_route_sync_cleanup(void *data);
> >>>>>>> +void en_advertised_route_sync_run(struct engine_node *,
> >>>>>>> void
> >>>>>>> *data);
> >>>>>>> +
> >>>>>>> +
> >>>>>>
> >>>>>> Nit: one empty line too many.
> >>>>>>
> >>>>>>> +#endif /* EN_ADVERTISED_ROUTE_SYNC_H */
> >>>>>>> diff --git a/northd/en-northd-output.c b/northd/en-northd-
> >>>>>>> output.c
> >>>>>>> index 98098d974..555ed2b9e 100644
> >>>>>>> --- a/northd/en-northd-output.c
> >>>>>>> +++ b/northd/en-northd-output.c
> >>>>>>> @@ -72,3 +72,11 @@ northd_output_fdb_aging_handler(struct
> >>>>>>> engine_node *node,
> >>>>>>>      engine_set_node_state(node, EN_UPDATED);
> >>>>>>>      return true;
> >>>>>>>  }
> >>>>>>> +
> >>>>>>> +bool
> >>>>>>> +northd_output_advertised_route_sync_handler(struct
> >>>>>>> engine_node
> >>>>>>> *node,
> >>>>>>> +                                            void *data
> >>>>>>> OVS_UNUSED)
> >>>>>>> +{
> >>>>>>> +    engine_set_node_state(node, EN_UPDATED);
> >>>>>>> +    return true;
> >>>>>>> +}
> >>>>>>> diff --git a/northd/en-northd-output.h b/northd/en-northd-
> >>>>>>> output.h
> >>>>>>> index 5f577b89c..00391ace3 100644
> >>>>>>> --- a/northd/en-northd-output.h
> >>>>>>> +++ b/northd/en-northd-output.h
> >>>>>>> @@ -17,5 +17,7 @@ bool
> >>>>>>> northd_output_mac_binding_aging_handler(struct engine_node
> >>>>>>> *node,
> >>>>>>>                                               void *data
> >>>>>>> OVS_UNUSED);
> >>>>>>>  bool northd_output_fdb_aging_handler(struct engine_node
> >>>>>>> *node,
> >>>>>>>                                       void *data
> >>>>>>> OVS_UNUSED);
> >>>>>>> +bool northd_output_advertised_route_sync_handler(struct
> >>>>>>> engine_node *node,
> >>>>>>> +                                                 void
> >>>>>>> *data
> >>>>>>> OVS_UNUSED);
> >>>>>>>  
> >>>>>>>  #endif
> >>>>>>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-
> >>>>>>> northd.c
> >>>>>>> index 6e0aa04c4..12956a5de 100644
> >>>>>>> --- a/northd/inc-proc-northd.c
> >>>>>>> +++ b/northd/inc-proc-northd.c
> >>>>>>> @@ -41,6 +41,7 @@
> >>>>>>>  #include "en-sampling-app.h"
> >>>>>>>  #include "en-sync-sb.h"
> >>>>>>>  #include "en-sync-from-sb.h"
> >>>>>>> +#include "en-advertised-route-sync.h"
> >>>>>>>  #include "unixctl.h"
> >>>>>>>  #include "util.h"
> >>>>>>>  
> >>>>>>> @@ -102,7 +103,8 @@ static unixctl_cb_func
> >>>>>>> chassis_features_list;
> >>>>>>>      SB_NODE(fdb, "fdb") \
> >>>>>>>      SB_NODE(static_mac_binding, "static_mac_binding") \
> >>>>>>>      SB_NODE(chassis_template_var, "chassis_template_var")
> >>>>>>> \
> >>>>>>> -    SB_NODE(logical_dp_group, "logical_dp_group")
> >>>>>>> +    SB_NODE(logical_dp_group, "logical_dp_group") \
> >>>>>>> +    SB_NODE(advertised_route, "advertised_route")
> >>>>>>>  
> >>>>>>>  enum sb_engine_node {
> >>>>>>>  #define SB_NODE(NAME, NAME_STR) SB_##NAME,
> >>>>>>> @@ -161,6 +163,7 @@ static ENGINE_NODE(route_policies,
> >>>>>>> "route_policies");
> >>>>>>>  static ENGINE_NODE(routes, "routes");
> >>>>>>>  static ENGINE_NODE(bfd, "bfd");
> >>>>>>>  static ENGINE_NODE(bfd_sync, "bfd_sync");
> >>>>>>> +static ENGINE_NODE(advertised_route_sync,
> >>>>>>> "advertised_route_sync");
> >>>>>>>  
> >>>>>>>  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >>>>>>>                            struct ovsdb_idl_loop *sb)
> >>>>>>> @@ -263,6 +266,10 @@ void inc_proc_northd_init(struct
> >>>>>>> ovsdb_idl_loop *nb,
> >>>>>>>      engine_add_input(&en_bfd_sync, &en_route_policies,
> >>>>>>> NULL);
> >>>>>>>      engine_add_input(&en_bfd_sync, &en_northd,
> >>>>>>> bfd_sync_northd_change_handler);
> >>>>>>>  
> >>>>>>> +    engine_add_input(&en_advertised_route_sync,
> >>>>>>> &en_routes,
> >>>>>>> NULL);
> >>>>>>> +    engine_add_input(&en_advertised_route_sync,
> >>>>>>> &en_sb_advertised_route,
> >>>>>>> +                     NULL);
> >>>>>>> +
> >>>>>>>      engine_add_input(&en_sync_meters, &en_nb_acl, NULL);
> >>>>>>>      engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
> >>>>>>>      engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
> >>>>>>> @@ -344,6 +351,8 @@ void inc_proc_northd_init(struct
> >>>>>>> ovsdb_idl_loop *nb,
> >>>>>>>                      
> >>>>>>> northd_output_mac_binding_aging_handler);
> >>>>>>>      engine_add_input(&en_northd_output, &en_fdb_aging,
> >>>>>>>                       northd_output_fdb_aging_handler);
> >>>>>>> +    engine_add_input(&en_northd_output,
> >>>>>>> &en_advertised_route_sync,
> >>>>>>> +                    
> >>>>>>> northd_output_advertised_route_sync_handler);
> >>>>>>>  
> >>>>>>>      struct engine_arg engine_arg = {
> >>>>>>>          .nb_idl = nb->idl,
> >>>>>>> diff --git a/northd/northd.c b/northd/northd.c
> >>>>>>> index 74014de32..1c32ef8f8 100644
> >>>>>>> --- a/northd/northd.c
> >>>>>>> +++ b/northd/northd.c
> >>>>>>> @@ -912,6 +912,8 @@ join_datapaths(const struct
> >>>>>>> nbrec_logical_switch_table *nbrec_ls_table,
> >>>>>>>          if (smap_get(&od->nbr->options, "chassis")) {
> >>>>>>>              od->is_gw_router = true;
> >>>>>>>          }
> >>>>>>> +        od->dynamic_routing = smap_get_bool(&od->nbr-
> >>>>>>>> options,
> >>>>>>> +                                            "dynamic-
> >>>>>>> routing",
> >>>>>>> false);
> >>>>>>>          ovs_list_push_back(lr_list, &od->lr_list);
> >>>>>>>      }
> >>>>>>>  }
> >>>>>>> @@ -11127,7 +11129,8 @@ route_hash(struct parsed_route
> >>>>>>> *route)
> >>>>>>>  }
> >>>>>>>  
> >>>>>>>  static bool
> >>>>>>> -find_static_route_outport(struct ovn_datapath *od, const
> >>>>>>> struct
> >>>>>>> hmap *lr_ports,
> >>>>>>> +find_static_route_outport(const struct ovn_datapath *od,
> >>>>>>> +    const struct hmap *lr_ports,
> >>>>>>>      const struct nbrec_logical_router_static_route *route,
> >>>>>>> bool
> >>>>>>> is_ipv4,
> >>>>>>>      const char **p_lrp_addr_s, struct ovn_port
> >>>>>>> **p_out_port);
> >>>>>>>  
> >>>>>>> @@ -11229,7 +11232,7 @@ parsed_route_add(const struct
> >>>>>>> ovn_datapath *od,
> >>>>>>>      new_pr->route_table_id = route_table_id;
> >>>>>>>      new_pr->is_src_route = is_src_route;
> >>>>>>>      new_pr->hash = route_hash(new_pr);
> >>>>>>> -    new_pr->nbr = od->nbr;
> >>>>>>> +    new_pr->od = od;
> >>>>>>>      new_pr->ecmp_symmetric_reply = ecmp_symmetric_reply;
> >>>>>>>      new_pr->is_discard_route = is_discard_route;
> >>>>>>>      if (!is_discard_route) {
> >>>>>>> @@ -11255,11 +11258,12 @@ parsed_route_add(const struct
> >>>>>>> ovn_datapath *od,
> >>>>>>>  }
> >>>>>>>  
> >>>>>>>  static void
> >>>>>>> -parsed_routes_add_static(struct ovn_datapath *od, const
> >>>>>>> struct
> >>>>>>> hmap *lr_ports,
> >>>>>>> -                  const struct
> >>>>>>> nbrec_logical_router_static_route
> >>>>>>> *route,
> >>>>>>> -                  const struct hmap *bfd_connections,
> >>>>>>> -                  struct hmap *routes, struct simap
> >>>>>>> *route_tables,
> >>>>>>> -                  struct hmap *bfd_active_connections)
> >>>>>>> +parsed_routes_add_static(const struct ovn_datapath *od,
> >>>>>>> +                         const struct hmap *lr_ports,
> >>>>>>> +                         const struct
> >>>>>>> nbrec_logical_router_static_route *route,
> >>>>>>> +                         const struct hmap
> >>>>>>> *bfd_connections,
> >>>>>>> +                         struct hmap *routes, struct simap
> >>>>>>> *route_tables,
> >>>>>>> +                         struct hmap
> >>>>>>> *bfd_active_connections)
> >>>>>>>  {
> >>>>>>>      /* Verify that the next hop is an IP address with an
> >>>>>>> all-
> >>>>>>> ones mask. */
> >>>>>>>      struct in6_addr *nexthop = NULL;
> >>>>>>> @@ -11381,7 +11385,8 @@ parsed_routes_add_static(struct
> >>>>>>> ovn_datapath *od, const struct hmap *lr_ports,
> >>>>>>>  }
> >>>>>>>  
> >>>>>>>  static void
> >>>>>>> -parsed_routes_add_connected(struct ovn_datapath *od, const
> >>>>>>> struct ovn_port *op,
> >>>>>>> +parsed_routes_add_connected(const struct ovn_datapath *od,
> >>>>>>> +                            const struct ovn_port *op,
> >>>>>>>                              struct hmap *routes)
> >>>>>>>  {
> >>>>>>>      for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs;
> >>>>>>> i++) {
> >>>>>>> @@ -11410,14 +11415,14 @@
> >>>>>>> parsed_routes_add_connected(struct
> >>>>>>> ovn_datapath *od, const struct ovn_port *op,
> >>>>>>>  }
> >>>>>>>  
> >>>>>>>  void
> >>>>>>> -build_parsed_routes(struct ovn_datapath *od, const struct
> >>>>>>> hmap
> >>>>>>> *lr_ports,
> >>>>>>> +build_parsed_routes(const struct ovn_datapath *od, const
> >>>>>>> struct
> >>>>>>> hmap *lr_ports,
> >>>>>>>                      const struct hmap *bfd_connections,
> >>>>>>> struct
> >>>>>>> hmap *routes,
> >>>>>>>                      struct simap *route_tables,
> >>>>>>>                      struct hmap *bfd_active_connections)
> >>>>>>>  {
> >>>>>>>      struct parsed_route *pr;
> >>>>>>>      HMAP_FOR_EACH (pr, key_node, routes) {
> >>>>>>> -        if (pr->nbr == od->nbr) {
> >>>>>>> +        if (pr->od == od) {
> >>>>>>>              pr->stale = true;
> >>>>>>>          }
> >>>>>>>      }
> >>>>>>> @@ -11641,13 +11646,15 @@ build_route_match(const struct
> >>>>>>> ovn_port
> >>>>>>> *op_inport, uint32_t rtb_id,
> >>>>>>>  
> >>>>>>>  /* Output: p_lrp_addr_s and p_out_port. */
> >>>>>>>  static bool
> >>>>>>> -find_static_route_outport(struct ovn_datapath *od, const
> >>>>>>> struct
> >>>>>>> hmap *lr_ports,
> >>>>>>> +find_static_route_outport(const struct ovn_datapath *od,
> >>>>>>> +    const struct hmap *lr_ports,
> >>>>>>>      const struct nbrec_logical_router_static_route *route,
> >>>>>>> bool
> >>>>>>> is_ipv4,
> >>>>>>>      const char **p_lrp_addr_s, struct ovn_port
> >>>>>>> **p_out_port)
> >>>>>>>  {
> >>>>>>>      const char *lrp_addr_s = NULL;
> >>>>>>>      struct ovn_port *out_port = NULL;
> >>>>>>>      if (route->output_port) {
> >>>>>>> +        /* XXX: we should be able to use &od->ports
> >>>>>>> instead of
> >>>>>>> lr_ports. */
> >>>>>>>          out_port = ovn_port_find(lr_ports, route-
> >>>>>>>> output_port);
> >>>>>>>          if (!out_port) {
> >>>>>>>              static struct vlog_rate_limit rl =
> >>>>>>> VLOG_RATE_LIMIT_INIT(5, 1);
> >>>>>>> diff --git a/northd/northd.h b/northd/northd.h
> >>>>>>> index 9457a7be6..98a17d733 100644
> >>>>>>> --- a/northd/northd.h
> >>>>>>> +++ b/northd/northd.h
> >>>>>>> @@ -367,6 +367,8 @@ struct ovn_datapath {
> >>>>>>>  
> >>>>>>>      /* router datapath has a logical port with redirect-
> >>>>>>> type set
> >>>>>>> to bridged. */
> >>>>>>>      bool redirect_bridged;
> >>>>>>> +    /* nbr has the option "dynamic-routing" set to true.
> >>>>>>> */
> >>>>>>> +    bool dynamic_routing;
> >>>>>>>  
> >>>>>>>      struct ovn_port **localnet_ports;
> >>>>>>>      size_t n_localnet_ports;
> >>>>>>> @@ -714,7 +716,7 @@ struct parsed_route {
> >>>>>>>      const struct nbrec_logical_router_static_route *route;
> >>>>>>>      bool ecmp_symmetric_reply;
> >>>>>>>      bool is_discard_route;
> >>>>>>> -    const struct nbrec_logical_router *nbr;
> >>>>>>> +    const struct ovn_datapath *od;
> >>>>>>>      bool stale;
> >>>>>>>      struct sset ecmp_selection_fields;
> >>>>>>>      enum route_source source;
> >>>>>>> @@ -745,7 +747,7 @@ void northd_indices_create(struct
> >>>>>>> northd_data
> >>>>>>> *data,
> >>>>>>>  
> >>>>>>>  void route_policies_init(struct route_policies_data *);
> >>>>>>>  void route_policies_destroy(struct route_policies_data *);
> >>>>>>> -void build_parsed_routes(struct ovn_datapath *, const
> >>>>>>> struct
> >>>>>>> hmap *,
> >>>>>>> +void build_parsed_routes(const struct ovn_datapath *,
> >>>>>>> const
> >>>>>>> struct hmap *,
> >>>>>>>                           const struct hmap *, struct hmap
> >>>>>>> *,
> >>>>>>> struct simap *,
> >>>>>>>                           struct hmap *);
> >>>>>>>  uint32_t get_route_table_id(struct simap *, const char *);
> >>>>>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> >>>>>>> index aa4c57663..17d6145be 100644
> >>>>>>> --- a/northd/ovn-northd.c
> >>>>>>> +++ b/northd/ovn-northd.c
> >>>>>>> @@ -941,6 +941,10 @@ main(int argc, char *argv[])
> >>>>>>>          ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> >>>>>>>                              
> >>>>>>> &sbrec_logical_dp_group_columns[i]);
> >>>>>>>      }
> >>>>>>> +    for (size_t i = 0; i <
> >>>>>>> SBREC_ADVERTISED_ROUTE_N_COLUMNS;
> >>>>>>> i++) {
> >>>>>>> +        ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> >>>>>>> +  ��                         
> >>>>>>> &sbrec_advertised_route_columns[i]);
> >>>>>>> +    }
> >>>>>>>  
> >>>>>>>      unixctl_command_register("sb-connection-status", "",
> >>>>>>> 0, 0,
> >>>>>>>                               ovn_conn_show,
> >>>>>>> ovnsb_idl_loop.idl);
> >>>>>>> @@ -968,6 +972,7 @@ main(int argc, char *argv[])
> >>>>>>>      stopwatch_create(LR_NAT_RUN_STOPWATCH_NAME, SW_MS);
> >>>>>>>      stopwatch_create(LR_STATEFUL_RUN_STOPWATCH_NAME,
> >>>>>>> SW_MS);
> >>>>>>>      stopwatch_create(LS_STATEFUL_RUN_STOPWATCH_NAME,
> >>>>>>> SW_MS);
> >>>>>>> +   
> >>>>>>> stopwatch_create(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME,
> >>>>>>> SW_MS);
> >>>>>>>  
> >>>>>>>      /* Initialize incremental processing engine for ovn-
> >>>>>>> northd
> >>>>>>> */
> >>>>>>>      inc_proc_northd_init(&ovnnb_idl_loop,
> >>>>>>> &ovnsb_idl_loop);
> >>>>>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
> >>>>>>> index 24ef12f3b..718d0126e 100644
> >>>>>>> --- a/ovn-nb.xml
> >>>>>>> +++ b/ovn-nb.xml
> >>>>>>> @@ -2946,6 +2946,19 @@ or
> >>>>>>>          option is not present the limit is not set and the
> >>>>>>> zone
> >>>>>>> limit is
> >>>>>>>          derived from OvS default datapath limit.
> >>>>>>>        </column>
> >>>>>>> +
> >>>>>>> +      <column name="options" key="dynamic-routing"
> >>>>>>> type='{"type": "boolean"}'>
> >>>>>>> +        If set to <code>true</code> then this <ref
> >>>>>>> table="Logical_Router"/>
> >>>>>>> +        can participate in dynamic routing with components
> >>>>>>> outside of OVN.
> >>>>>>> +
> >>>>>>> +        It will synchronize all routes to the soutbound
> >>>>>>> +        <ref table="Route" db="OVN_SB"/> table that are
> >>>>>>> relevant
> >>>>>>> for the
> >>>>>>> +        router. This includes:
> >>>>>>> +        * all "connected" routes implicitly created by
> >>>>>>> networks
> >>>>>>> associated with
> >>>>>>> +          this Logical Router
> >>>>>>> +        * all <ref table="Logical_Router_Static_Route"/>
> >>>>>>> that
> >>>>>>> are applied to
> >>>>>>> +          this Logical Router
> >>>>>>
> >>>>>> This doesn't render that nicely.  The resulting man page is
> >>>>>> content
> >>>>>> is
> >>>>>> something like:
> >>>>>>
> >>>>>>               includes:  * all "connected" routes implicitly
> >>>>>> created by
> >>>>>>               networks associated with this Logical Router *
> >>>>>> all
> >>>>>>               Logical_Router_Static_Route that are applied to
> >>>>>> this
> >>>>>>               Logical Router
> >>>>>>
> >>>>>> If you change this block to:
> >>>>>>
> >>>>>>         <ul>
> >>>>>>           <li>
> >>>>>>             all "connected" routes implicitly created by
> >>>>>> networks
> >>>>>>             associated with this Logical Router
> >>>>>>           </li>
> >>>>>>           <li>
> >>>>>>             all <ref table="Logical_Router_Static_Route"/>
> >>>>>> that are
> >>>>>>             applied to this Logical Router
> >>>>>>           </li>
> >>>>>>         </ul>
> >>>>>>
> >>>>>> The generated man page looks slightly better IMO:
> >>>>>>
> >>>>>>               table that are relevant for the router. This
> >>>>>> includes:
> >>>>>>
> >>>>>>               •      all  "connected" routes implicitly
> >>>>>> created by
> >>>>>>                      networks associated with this Logical
> >>>>>> Router
> >>>>>>
> >>>>>>               •      all Logical_Router_Static_Route that are
> >>>>>> applied
> >>>>>>                      to this Logical Router
> >>>>>>
> >>>>>>> +      </column>
> >>>>>>>      </group>
> >>>>>>>  
> >>>>>>>      <group title="Common Columns">
> >>>>>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> >>>>>>> index edfd5764b..30ea23502 100644
> >>>>>>> --- a/tests/ovn-northd.at
> >>>>>>> +++ b/tests/ovn-northd.at
> >>>>>>> @@ -14419,3 +14419,140 @@ AT_CHECK([ovn-sbctl lflow-list S1
> >>>>>>> |
> >>>>>>> grep ls_out_acl_action | grep priority=500 |
> >>>>>>>  
> >>>>>>>  AT_CLEANUP
> >>>>>>>  ])
> >>>>>>> +
> >>>>>>> +OVN_FOR_EACH_NORTHD_NO_HV([
> >>>>>>> +AT_SETUP([dynamic-routing - sync to sb])
> >>>>>>> +AT_KEYWORDS([dynamic-routing])
> >>>>>>> +ovn_start
> >>>>>>> +
> >>>>>>> +# adding a router - still nothing here
> >>>>>>
> >>>>>> Nit: comments should be sentences.
> >>>>>>
> >>>>>>> +check ovn-nbctl lr-add lr0
> >>>>>>> +check ovn-nbctl --wait=sb set Logical_Router lr0
> >>>>>>> option:dynamic-
> >>>>>>> routing=true
> >>>>>>> +check_row_count Advertised_Route 0
> >>>>>>> +datapath=$(fetch_column datapath_binding _uuid
> >>>>>>> external_ids:name=lr0)
> >>>>>>> +
> >>>>>>> +# adding a LRP adds a route entry for the associated
> >>>>>>> network
> >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0
> >>>>>>> 00:00:00:00:ff:01
> >>>>>>> 10.0.0.1/24
> >>>>>>> +pb=$(fetch_column port_binding _uuid logical_port=lr0-sw0)
> >>>>>>> +check_row_count Advertised_Route 1
> >>>>>>> +check_column 10.0.0.0/24 Advertised_Route ip_prefix
> >>>>>>> datapath=$datapath logical_port=$pb
> >>>>>>> +
> >>>>>>> +# adding a second LRP adds an additional route entry
> >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw1
> >>>>>>> 00:00:00:00:ff:02
> >>>>>>> 10.0.1.1/24
> >>>>>>> +pb2=$(fetch_column port_binding _uuid logical_port=lr0-
> >>>>>>> sw1)
> >>>>>>> +check_row_count Advertised_Route 2
> >>>>>>> +check_column 10.0.0.0/24 Advertised_Route ip_prefix
> >>>>>>> datapath=$datapath logical_port=$pb
> >>>>>>> +check_column 10.0.1.0/24 Advertised_Route ip_prefix
> >>>>>>> datapath=$datapath logical_port=$pb2
> >>>>>>> +
> >>>>>>> +# adding a static route adds an additional entry
> >>>>>>> +check ovn-nbctl --wait=sb lr-route-add lr0 192.168.0.0/24
> >>>>>>> 10.0.0.10
> >>>>>>> +check_row_count Advertised_Route 3
> >>>>>>> +check_row_count Advertised_Route 2 logical_port=$pb
> >>>>>>> +check_row_count Advertised_Route 1 logical_port=$pb
> >>>>>>> ip_prefix=192.168.0.0/24
> >>>>>>> +
> >>>>>>> +# adding an ipv6 LRP adds an addition route entry
> >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw2
> >>>>>>> 00:00:00:00:ff:03
> >>>>>>> 2001:db8::1/64 fe80::1/64
> >>>>>>> +pb3=$(fetch_column port_binding _uuid logical_port=lr0-
> >>>>>>> sw2)
> >>>>>>> +check_row_count Advertised_Route 4
> >>>>>>> +check_row_count Advertised_Route 2 logical_port=$pb
> >>>>>>> +check_row_count Advertised_Route 1 logical_port=$pb
> >>>>>>> ip_prefix=192.168.0.0/24
> >>>>>>> +check_column 10.0.1.0/24 Advertised_Route ip_prefix
> >>>>>>> datapath=$datapath logical_port=$pb2
> >>>>>>> +check_column 2001:db8::/64 Advertised_Route ip_prefix
> >>>>>>> datapath=$datapath logical_port=$pb3
> >>>>>>> +
> >>>>>>> +# 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 4
> >>>>>>> +
> >>>>>>> +# removing the lrp used for the static route removes both
> >>>>>>> route
> >>>>>>> entries
> >>>>>>> +check ovn-nbctl --wait=sb lrp-del lr0-sw0
> >>>>>>> +check_row_count Advertised_Route 2
> >>>>>>> +check_row_count Advertised_Route 1 logical_port=$pb2
> >>>>>>> +check_row_count Advertised_Route 1 logical_port=$pb3
> >>>>>>> +
> >>>>>>> +# removing the lr will remove all routes
> >>>>>>> +check ovn-nbctl --wait=sb lr-del lr0
> >>>>>>> +check_row_count Advertised_Route 0
> >>>>>>> +
> >>>>>>> +AT_CLEANUP
> >>>>>>> +])
> >>>>>>> +
> >>>>>>> +OVN_FOR_EACH_NORTHD_NO_HV([
> >>>>>>> +AT_SETUP([dynamic-routing incremental processing])
> >>>>>>> +AT_KEYWORDS([dynamic-routing])
> >>>>>>> +ovn_start
> >>>>>>> +
> >>>>>>> +# Test I-P for dynamic-routing.
> >>>>>>> +# Presently ovn-northd has no I-P for Advertised_Route.
> >>>>>>> +# Wait for sb to be connected before clearing stats.
> >>>>>>> +check ovn-nbctl --wait=sb sync
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl lr-add lr0
> >>>>>>> +check ovn-nbctl --wait=sb set Logical_Router lr0
> >>>>>>> option:dynamic-
> >>>>>>> routing=true
> >>>>>>> +
> >>>>>>> +check_engine_stats northd recompute nocompute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> >>>>>>
> >>>>>> CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) also waits for logical
> >>>>>> ports to
> >>>>>> be
> >>>>>> up.  We have no ports in the configuration at this point. 
> >>>>>> This can
> >>>>>> be a
> >>>>>> plain:
> >>>>>>
> >>>>>> CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0
> >>>>>>> 00:00:00:00:ff:01
> >>>>>>> 10.0.0.1/24
> >>>>>>> +check_engine_stats northd recompute compute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw1
> >>>>>>> 00:00:00:00:ff:02
> >>>>>>> 10.0.1.1/24
> >>>>>>> +check_engine_stats northd recompute compute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb lr-route-add lr0 192.168.0.0/24
> >>>>>>> 10.0.0.10
> >>>>>>> +check_engine_stats northd recompute nocompute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw2
> >>>>>>> 00:00:00:00:ff:03
> >>>>>>> 2001:db8::1/64 fe80::1/64
> >>>>>>> +check_engine_stats northd recompute compute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb remove Logical_Router lr0 option
> >>>>>>> dynamic-routing
> >>>>>>> +check_engine_stats northd recompute nocompute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb set Logical_Router lr0
> >>>>>>> option:dynamic-
> >>>>>>> routing=true
> >>>>>>> +check_engine_stats northd recompute nocompute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb lrp-del lr0-sw0
> >>>>>>> +check_engine_stats northd recompute compute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-
> >>>>>>> stats
> >>>>>>> +check ovn-nbctl --wait=sb lr-del lr0
> >>>>>>> +check_engine_stats northd recompute nocompute
> >>>>>>> +check_engine_stats routes recompute nocompute
> >>>>>>> +check_engine_stats advertised_route_sync recompute
> >>>>>>> nocompute
> >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >>>>>>> +
> >>>>>>> +AT_CLEANUP
> >>>>>>> +])
> >>>>>>
> >>>>>> Regards,
> >>>>>> Dumitru
> >>>>>>
> >>>>>
> >>>>
> >>>
> > 
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to