On Wed, Feb 05, 2025 at 01:20:07PM +0100, Dumitru Ceara wrote:
> On 2/4/25 2:59 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 4 minor comments on this patch. If that's the only thing you
> change in v7 feel free to also add my ack:
>
> Acked-by: Dumitru Ceara <[email protected]>
Hi Dumitru,
thanks a lot. All of that will be part of v7.
Thank,
Felix
>
> Thanks,
> Dumitru
>
> > v5->v6:
> > * addressed review comments
> > v4->v5: skipped
> > 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 | 182 ++++++++++++++++++++++++++++++
> > northd/en-advertised-route-sync.h | 28 +++++
> > northd/en-northd-output.c | 9 ++
> > 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 | 19 ++++
> > tests/ovn-northd.at | 137 ++++++++++++++++++++++
> > 16 files changed, 445 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 2f0c965a7..0460868e4 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -38,6 +38,10 @@ Post v24.09.0
> > - Improved handling of IPv6 traffic by enabling address prefix tracking
> > in OVS for both IPv4 and IPv6 addresses, whenever possible, reducing
> > the amount of IPv6 datapath flows.
> > + - 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.
>
> Nit: there's no "Route" table in the SB anymore. This should be
> "Advertised_Route".
>
> >
> > OVN v24.09.0 - 13 Sep 2024
> > --------------------------
> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > index 75b5d1787..1b05f1463 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 57851fd63..d1cb4bc8f 100644
> > --- a/lib/ovn-util.c
> > +++ b/lib/ovn-util.c
> > @@ -1352,6 +1352,27 @@ 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));
> > +}
> > +
> > const struct sbrec_port_binding *
> > lport_lookup_by_name(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > const char *name)
> > @@ -1367,3 +1388,4 @@ lport_lookup_by_name(struct ovsdb_idl_index
> > *sbrec_port_binding_by_name,
> >
> > return retval;
> > }
> > +
> > diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> > index f2f70dd72..632235385 100644
> > --- a/lib/ovn-util.h
> > +++ b/lib/ovn-util.h
> > @@ -487,6 +487,8 @@ 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);
> > +
> > const struct sbrec_port_binding *lport_lookup_by_name(
> > struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > const char *name);
> > 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 c1641dbae..cebe33df5 100644
> > --- a/northd/automake.mk
> > +++ b/northd/automake.mk
> > @@ -38,6 +38,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..5126348a8
> > --- /dev/null
> > +++ b/northd/en-advertised-route-sync.c
> > @@ -0,0 +1,182 @@
> > +/*
> > + * Copyright (c) 2025, STACKIT GmbH & Co. KG
> > + *
> > + * 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)
>
> Nit: this should be indented so that it's aligned under the first
> argument from the line above it.
>
> > +{
> > + 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);
> > +
> > + 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;
> > +};
> > +
> > +/* Add a new entries to the to-be-advertised routes.
> > + * Takes ownership of ip_prefix. */
> > +static struct ar_entry *
> > +ar_add_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);
> > +
> > + 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)
> > +{
> > + 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);
> > + 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 (!route->od->dynamic_routing) {
> > + continue;
> > + }
> > +
> > + char *ip_prefix = normalize_v46_prefix(&route->prefix,
> > + route->plen);
>
> This fits on one line.
>
> > + route_e = ar_add_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..30e7cae1f
> > --- /dev/null
> > +++ b/northd/en-advertised-route-sync.h
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Copyright (c) 2025, STACKIT GmbH & Co. KG
> > + *
> > + * 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);
> > +
> > +#endif /* EN_ADVERTISED_ROUTE_SYNC_H */
> > diff --git a/northd/en-northd-output.c b/northd/en-northd-output.c
> > index 26c01d1e3..5516dd984 100644
> > --- a/northd/en-northd-output.c
> > +++ b/northd/en-northd-output.c
> > @@ -80,3 +80,12 @@ northd_output_ecmp_nexthop_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 71a0f6532..935088715 100644
> > --- a/northd/en-northd-output.h
> > +++ b/northd/en-northd-output.h
> > @@ -19,5 +19,7 @@ bool northd_output_fdb_aging_handler(struct engine_node
> > *node,
> > void *data OVS_UNUSED);
> > bool northd_output_ecmp_nexthop_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 4a1d4eac9..fa4a406a1 100644
> > --- a/northd/inc-proc-northd.c
> > +++ b/northd/inc-proc-northd.c
> > @@ -43,6 +43,7 @@
> > #include "en-sync-sb.h"
> > #include "en-sync-from-sb.h"
> > #include "en-ecmp-nexthop.h"
> > +#include "en-advertised-route-sync.h"
> > #include "unixctl.h"
> > #include "util.h"
> >
> > @@ -105,7 +106,8 @@ static unixctl_cb_func chassis_features_list;
> > 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(ecmp_nexthop, "ecmp_nexthop")
> > + SB_NODE(ecmp_nexthop, "ecmp_nexthop") \
> > + SB_NODE(advertised_route, "advertised_route")
> >
> > enum sb_engine_node {
> > #define SB_NODE(NAME, NAME_STR) SB_##NAME,
> > @@ -166,6 +168,7 @@ static ENGINE_NODE(bfd, "bfd");
> > static ENGINE_NODE(bfd_sync, "bfd_sync");
> > static ENGINE_NODE(ecmp_nexthop, "ecmp_nexthop");
> > static ENGINE_NODE(multicast_igmp, "multicast_igmp");
> > +static ENGINE_NODE(advertised_route_sync, "advertised_route_sync");
> >
> > void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> > struct ovsdb_idl_loop *sb)
> > @@ -275,6 +278,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> > engine_add_input(&en_ecmp_nexthop, &en_sb_mac_binding,
> > ecmp_nexthop_mac_binding_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);
> > @@ -364,6 +371,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> > northd_output_fdb_aging_handler);
> > engine_add_input(&en_northd_output, &en_ecmp_nexthop,
> > northd_output_ecmp_nexthop_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 542b40685..594060bdd 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -894,6 +894,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);
> > }
> > }
> > @@ -10699,7 +10701,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);
> >
> > @@ -10801,7 +10804,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) {
> > @@ -10827,11 +10830,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;
> > @@ -10953,7 +10957,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++) {
> > @@ -10982,14 +10987,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;
> > }
> > }
> > @@ -11213,13 +11218,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 68af275cb..1a843a627 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -360,6 +360,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;
> > @@ -707,7 +709,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;
> > @@ -738,7 +740,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 d3470d94c..9fad91df0 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -945,6 +945,10 @@ main(int argc, char *argv[])
> > ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> > &sbrec_ecmp_nexthop_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);
> > @@ -972,6 +976,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 d82f9872b..aad014cb3 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -2955,6 +2955,25 @@ 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
>
> This should be "Advertised_Route" instead.
>
> > + router. This includes:
> > + <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>
> > + </column>
> > </group>
> >
> > <group title="Common Columns">
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 7abd4d3f3..4c2e2713a 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14579,3 +14579,140 @@ wait_row_count Multicast_Group 0 name=239.0.1.68
> > OVN_CLEANUP([hv1], [hv2])
> > AT_CLEANUP
> > ])
> > +
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([dynamic-routing - sync to sb])
> > +AT_KEYWORDS([dynamic-routing])
> > +ovn_start
> > +
> > +# Adding a router - no route advertised.
> > +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
> > +
> > +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
> > +])
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
