On 1/29/25 12:15 PM, Felix Huettner via dev wrote:
> This engine node determines the routes that the ovn-controller should
> export.
>
> Co-Authored-By: Frode Nordahl <[email protected]>
> Signed-off-by: Frode Nordahl <[email protected]>
> Signed-off-by: Felix Huettner <[email protected]>
> ---
Hi Felix, Frode,
Overall this patch looks correct to me. I have some minor comments for
the next revision, please see below.
> v3->v4:
> - addressed review comments.
>
> TODO.rst | 1 +
> controller/automake.mk | 4 +-
> controller/local_data.c | 7 +-
> controller/local_data.h | 1 +
> controller/ovn-controller.c | 191 ++++++++++++++++++++++++++++++++-
> controller/route.c | 203 ++++++++++++++++++++++++++++++++++++
> controller/route.h | 68 ++++++++++++
> tests/automake.mk | 1 +
> 8 files changed, 473 insertions(+), 3 deletions(-)
> create mode 100644 controller/route.c
> create mode 100644 controller/route.h
>
> diff --git a/TODO.rst b/TODO.rst
> index 3426497a7..b3ab43232 100644
> --- a/TODO.rst
> +++ b/TODO.rst
> @@ -97,6 +97,7 @@ OVN To-do List
> * ovn-controller Incremental processing
>
> * Implement I-P for datapath groups.
> + * Implement I-P for route exchange relevant ports.
>
> * ovn-northd parallel logical flow processing
>
> diff --git a/controller/automake.mk b/controller/automake.mk
> index bb0bf2d33..a6a2c517a 100644
> --- a/controller/automake.mk
> +++ b/controller/automake.mk
> @@ -51,7 +51,9 @@ controller_ovn_controller_SOURCES = \
> controller/ct-zone.h \
> controller/ct-zone.c \
> controller/ovn-dns.c \
> - controller/ovn-dns.h
> + controller/ovn-dns.h \
> + controller/route.h \
> + controller/route.c
>
> controller_ovn_controller_LDADD = lib/libovn.la
> $(OVS_LIBDIR)/libopenvswitch.la
> man_MANS += controller/ovn-controller.8
> diff --git a/controller/local_data.c b/controller/local_data.c
> index 69a1b775f..4aee39d6b 100644
> --- a/controller/local_data.c
> +++ b/controller/local_data.c
> @@ -414,14 +414,19 @@ tracked_datapath_lport_add(const struct
> sbrec_port_binding *pb,
> }
>
> void
> -tracked_datapaths_destroy(struct hmap *tracked_datapaths)
> +tracked_datapaths_clear(struct hmap *tracked_datapaths)
> {
> struct tracked_datapath *t_dp;
> HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) {
> shash_destroy_free_data(&t_dp->lports);
> free(t_dp);
> }
> +}
>
> +void
> +tracked_datapaths_destroy(struct hmap *tracked_datapaths)
> +{
> + tracked_datapaths_clear(tracked_datapaths);
> hmap_destroy(tracked_datapaths);
> }
>
> diff --git a/controller/local_data.h b/controller/local_data.h
> index ab8e789a5..1d60dada8 100644
> --- a/controller/local_data.h
> +++ b/controller/local_data.h
> @@ -131,6 +131,7 @@ struct tracked_datapath *tracked_datapath_find(
> void tracked_datapath_lport_add(const struct sbrec_port_binding *,
> enum en_tracked_resource_type,
> struct hmap *tracked_datapaths);
> +void tracked_datapaths_clear(struct hmap *tracked_datapaths);
> void tracked_datapaths_destroy(struct hmap *tracked_datapaths);
>
> /* Maps from a chassis to the OpenFlow port number of the tunnel that can be
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 854e54854..6ffa283a7 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -88,6 +88,7 @@
> #include "lib/dns-resolve.h"
> #include "ct-zone.h"
> #include "ovn-dns.h"
> +#include "route.h"
>
> VLOG_DEFINE_THIS_MODULE(main);
>
> @@ -247,6 +248,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT(&igmp);
> struct ovsdb_idl_condition chprv = OVSDB_IDL_CONDITION_INIT(&chprv);
> struct ovsdb_idl_condition tv = OVSDB_IDL_CONDITION_INIT(&tv);
> + struct ovsdb_idl_condition ar = OVSDB_IDL_CONDITION_INIT(&ar);
>
> /* Always monitor all logical datapath groups. Otherwise, DPG updates may
> * be received *after* the lflows using it are seen by ovn-controller.
> @@ -254,6 +256,11 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> * avoid the unnecessarily extra wake-ups of ovn-controller. */
> ovsdb_idl_condition_add_clause_true(&ldpg);
>
> + /* Always monitor advertised routes. Otherwise, once we claim a port on
> + * startup we do not yet know the routes to advertise and might wrongly
> + * delete already installed ones. */
> + ovsdb_idl_condition_add_clause_true(&ar);
> +
This problem is just at startup if I understand correctly. We handled
similar cases for other Southbound records in a different way. See the:
if (chassis) {
[...]
} else {
/* During initialization, we monitor all records in Chassis_Private so
* that we don't try to recreate existing ones. */
ovsdb_idl_condition_add_clause_true(&chprv);
/* Also, to avoid traffic disruption (e.g., conntrack flushing for
* zones that are used by OVN but not yet known due to the SB initial
* contents not being available), monitor all port bindings
* connected to gateways; they might be claimed as soon as the
* chassis is available.
*/
sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l2gateway");
sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l3gateway");
}
Can't we do the same thing for advertised routes?
> if (monitor_all) {
> ovsdb_idl_condition_add_clause_true(&pb);
> ovsdb_idl_condition_add_clause_true(&lf);
> @@ -381,6 +388,7 @@ out:;
> sb_table_set_req_mon_condition(ovnsb_idl, igmp_group, &igmp),
> sb_table_set_req_mon_condition(ovnsb_idl, chassis_private, &chprv),
> sb_table_set_opt_mon_condition(ovnsb_idl, chassis_template_var, &tv),
> + sb_table_set_opt_mon_condition(ovnsb_idl, advertised_route, &ar),
> };
>
> unsigned int expected_cond_seqno = 0;
> @@ -400,6 +408,7 @@ out:;
> ovsdb_idl_condition_destroy(&igmp);
> ovsdb_idl_condition_destroy(&chprv);
> ovsdb_idl_condition_destroy(&tv);
> + ovsdb_idl_condition_destroy(&ar);
> return expected_cond_seqno;
> }
>
> @@ -865,7 +874,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> SB_NODE(fdb, "fdb") \
> SB_NODE(meter, "meter") \
> SB_NODE(static_mac_binding, "static_mac_binding") \
> - SB_NODE(chassis_template_var, "chassis_template_var")
> + SB_NODE(chassis_template_var, "chassis_template_var") \
> + SB_NODE(advertised_route, "advertised_route")
>
> enum sb_engine_node {
> #define SB_NODE(NAME, NAME_STR) SB_##NAME,
> @@ -4812,6 +4822,172 @@ pflow_lflow_output_sb_chassis_handler(struct
> engine_node *node,
> return true;
> }
>
> +struct ed_type_route {
> + struct ovsdb_idl *ovnsb_idl;
> + /* Contains struct tracked_datapath entries for local datapaths subject
> to
> + * route exchange. */
> + struct hmap tracked_route_datapaths;
> + /* Contains struct advertise_datapath_entry */
> + struct hmap announce_routes;
> +};
> +
> +static void
> +en_route_run(struct engine_node *node, void *data)
> +{
> + struct ed_type_route *re_data = data;
> + route_cleanup(&re_data->announce_routes);
Nit: For better "code grouping" I'd move the route_cleanup() call just
before tracked_datapaths_clear() below.
> +
> + const struct ovsrec_open_vswitch_table *ovs_table =
> + EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
> + const char *chassis_id = get_ovs_chassis_id(ovs_table);
> + ovs_assert(chassis_id);
> +
> + struct ovsdb_idl_index *sbrec_chassis_by_name =
> + engine_ovsdb_node_get_index(
> + engine_get_input("SB_chassis", node),
> + "name");
> + const struct sbrec_chassis *chassis
> + = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
> + ovs_assert(chassis);
> +
> + struct ovsdb_idl_index *sbrec_port_binding_by_name =
> + engine_ovsdb_node_get_index(
> + engine_get_input("SB_port_binding", node),
> + "name");
> + struct ed_type_runtime_data *rt_data =
> + engine_get_input_data("runtime_data", node);
> +
> + struct route_ctx_in r_ctx_in = {
> + .ovnsb_idl = re_data->ovnsb_idl,
> + .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
> + .chassis = chassis,
> + .active_tunnels = &rt_data->active_tunnels,
> + .local_datapaths = &rt_data->local_datapaths,
> + .local_lports = &rt_data->local_lports,
> + };
> +
> + struct route_ctx_out r_ctx_out = {
> + .tracked_re_datapaths = &re_data->tracked_route_datapaths,
> + .announce_routes = &re_data->announce_routes,
> + };
> +
> + tracked_datapaths_clear(r_ctx_out.tracked_re_datapaths);
> +
Nit: I'd remove this newline.
> + route_run(&r_ctx_in, &r_ctx_out);
> +
Nit: this one too.
> + engine_set_node_state(node, EN_UPDATED);
> +}
> +
> +
> +static void *
> +en_route_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg)
> +{
> + struct ed_type_route *data = xzalloc(sizeof *data);
> +
> + hmap_init(&data->tracked_route_datapaths);
> + hmap_init(&data->announce_routes);
> + data->ovnsb_idl = arg->sb_idl;
> +
> + return data;
> +}
> +
> +static void
> +en_route_cleanup(void *data)
> +{
> + struct ed_type_route *re_data = data;
> +
> + tracked_datapaths_destroy(&re_data->tracked_route_datapaths);
> + route_cleanup(&re_data->announce_routes);
> + hmap_destroy(&re_data->announce_routes);
> +}
> +
> +static bool
> +route_runtime_data_handler(struct engine_node *node, void *data)
> +{
> + struct ed_type_route *re_data = data;
> + struct ed_type_runtime_data *rt_data =
> + engine_get_input_data("runtime_data", node);
> +
> + if (!rt_data->tracked) {
> + return false;
> + }
> +
> + struct tracked_datapath *t_dp;
> + HMAP_FOR_EACH (t_dp, node, &rt_data->tracked_dp_bindings) {
> + struct tracked_datapath *re_t_dp =
> + tracked_datapath_find(&re_data->tracked_route_datapaths,
> t_dp->dp);
> +
> + if (re_t_dp) {
> + /* XXX: Until we get I-P support for route exchange we need to
> + * request recompute. */
> + return false;
> + }
> +
> + struct shash_node *shash_node;
> + SHASH_FOR_EACH (shash_node, &t_dp->lports) {
> + struct tracked_lport *lport = shash_node->data;
> + if (route_exchange_relevant_port(lport->pb)) {
> + /* XXX: Until we get I-P support for route exchange we need
> to
> + * request recompute. */
> + return false;
> + }
> + }
> + }
> +
> + return true;
> +}
> +
> +static bool
> +route_sb_port_binding_data_handler(struct engine_node *node, void *data)
> +{
> + struct ed_type_route *re_data = data;
> + const struct sbrec_port_binding_table *pb_table =
> + EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
> +
> + const struct sbrec_port_binding *sbrec_pb;
> + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (sbrec_pb, pb_table) {
> + struct tracked_datapath *re_t_dp =
> + tracked_datapath_find(&re_data->tracked_route_datapaths,
> + sbrec_pb->datapath);
> + if (re_t_dp) {
> + /* XXX: Until we get I-P support for route exchange we need to
> + * request recompute. */
> + return false;
> + }
> +
> + if (route_exchange_relevant_port(sbrec_pb)) {
> + /* XXX: Until we get I-P support for route exchange we need to
> + * request recompute. */
> + return false;
> + }
> +
> + }
> + return true;
> +}
> +
> +static bool
> +route_sb_advertised_route_data_handler(struct engine_node *node, void *data)
> +{
> + struct ed_type_route *re_data = data;
> + const struct sbrec_advertised_route_table *advertised_route_table =
> + EN_OVSDB_GET(engine_get_input("SB_advertised_route", node));
> +
> + const struct sbrec_advertised_route *sbrec_route;
> + SBREC_ADVERTISED_ROUTE_TABLE_FOR_EACH_TRACKED (sbrec_route,
> + advertised_route_table) {
> + struct tracked_datapath *re_t_dp =
> + tracked_datapath_find(&re_data->tracked_route_datapaths,
> + sbrec_route->datapath);
> + if (re_t_dp) {
> + /* XXX: Until we get I-P support for route exchange we need to
> + * request recompute. */
> + return false;
> + }
> + }
> + return true;
> +}
> +
> /* Returns false if the northd internal version stored in SB_Global
> * and ovn-controller internal version don't match.
> */
> @@ -5103,6 +5279,7 @@ main(int argc, char *argv[])
> ENGINE_NODE(mac_cache, "mac_cache");
> ENGINE_NODE(bfd_chassis, "bfd_chassis");
> ENGINE_NODE(dns_cache, "dns_cache");
> + ENGINE_NODE(route, "route");
>
> #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
> SB_NODES
> @@ -5125,6 +5302,15 @@ main(int argc, char *argv[])
> engine_add_input(&en_lb_data, &en_runtime_data,
> lb_data_runtime_data_handler);
>
> + engine_add_input(&en_route, &en_ovs_open_vswitch, NULL);
> + engine_add_input(&en_route, &en_sb_chassis, NULL);
> + engine_add_input(&en_route, &en_sb_port_binding,
> + route_sb_port_binding_data_handler);
> + engine_add_input(&en_route, &en_runtime_data,
> + route_runtime_data_handler);
> + engine_add_input(&en_route, &en_sb_advertised_route,
> + route_sb_advertised_route_data_handler);
> +
> engine_add_input(&en_addr_sets, &en_sb_address_set,
> addr_sets_sb_address_set_handler);
> engine_add_input(&en_port_groups, &en_sb_port_group,
> @@ -5310,6 +5496,9 @@ main(int argc, char *argv[])
> controller_output_mac_cache_handler);
> engine_add_input(&en_controller_output, &en_bfd_chassis,
> controller_output_bfd_chassis_handler);
> + /* This is just temporary until the route output is actually used. */
> + engine_add_input(&en_controller_output, &en_route,
> + controller_output_bfd_chassis_handler);
>
> struct engine_arg engine_arg = {
> .sb_idl = ovnsb_idl_loop.idl,
> diff --git a/controller/route.c b/controller/route.c
> new file mode 100644
> index 000000000..49e76ee73
> --- /dev/null
> +++ b/controller/route.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright (c) 2024, Canonical, Ltd.
> + * 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 <net/if.h>
> +
> +#include "openvswitch/hmap.h"
> +#include "openvswitch/vlog.h"
> +
> +#include "lib/ovn-sb-idl.h"
> +
> +#include "binding.h"
> +#include "ha-chassis.h"
> +#include "local_data.h"
> +#include "route.h"
> +
> +
Nit: one newline too many.
> +VLOG_DEFINE_THIS_MODULE(exchange);
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> +
> +/* While the linux kernel can handle 2^32 routing tables, only so many can
> fit
> + * in the corresponding VRF interface name. */
> +#define MAX_TABLE_ID 1000000000
> +
> +bool
> +route_exchange_relevant_port(const struct sbrec_port_binding *pb)
> +{
> + return pb && smap_get_bool(&pb->options, "dynamic-routing", false);
> +}
> +
> +uint32_t
> +advertise_route_hash(const struct in6_addr *dst, unsigned int plen)
> +{
> + uint32_t hash = hash_bytes(dst->s6_addr, 16, 0);
> + return hash_int(plen, hash);
> +}
> +
> +static const struct sbrec_port_binding*
> +find_route_exchange_pb(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> + const struct sbrec_chassis *chassis,
> + const struct sset *active_tunnels,
> + const struct sbrec_port_binding *pb)
> +{
> + if (!pb) {
> + return NULL;
> + }
> + if (route_exchange_relevant_port(pb)) {
> + return pb;
> + }
> + const char *crp = smap_get(&pb->options, "chassis-redirect-port");
> + if (!crp) {
> + return NULL;
> + }
> + if (!lport_is_chassis_resident(sbrec_port_binding_by_name, chassis,
> + active_tunnels, crp)) {
> + return NULL;
> + }
> + const struct sbrec_port_binding *crpbp = lport_lookup_by_name(
> + sbrec_port_binding_by_name, crp);
> + if (route_exchange_relevant_port(crpbp)) {
> + return crpbp;
> + }
> + return NULL;
> +}
> +
> +static void
> +advertise_datapath_cleanup(struct advertise_datapath_entry *ad)
> +{
> + struct advertise_route_entry *ar;
> + HMAP_FOR_EACH_SAFE (ar, node, &ad->routes) {
> + hmap_remove(&ad->routes, &ar->node);
> + free(ar);
> + }
> + hmap_destroy(&ad->routes);
> + sset_destroy(&ad->bound_ports);
> + free(ad);
> +}
> +
> +static struct advertise_datapath_entry*
> +advertise_datapath_find(const struct hmap *datapaths,
> + const struct sbrec_datapath_binding *db)
> +{
> + struct advertise_datapath_entry *ade;
> + HMAP_FOR_EACH_WITH_HASH (ade, node, db->tunnel_key, datapaths) {
> + if (ade->db == db) {
> + return ade;
> + }
> + }
> + return NULL;
> +}
> +
> +void
> +route_run(struct route_ctx_in *r_ctx_in,
> + struct route_ctx_out *r_ctx_out)
> +{
> + struct advertise_datapath_entry *ad;
> + const struct local_datapath *ld;
> +
> + HMAP_FOR_EACH (ld, hmap_node, r_ctx_in->local_datapaths) {
> + if (!ld->n_peer_ports || ld->is_switch) {
> + continue;
> + }
> +
> + ad = xzalloc(sizeof(*ad));
> + ad->db = ld->datapath;
> + hmap_init(&ad->routes);
> + sset_init(&ad->bound_ports);
> +
> + /* This is a LR datapath, find LRPs with route exchange options
> + * that are bound locally. */
> + for (size_t i = 0; i < ld->n_peer_ports; i++) {
> + const struct sbrec_port_binding *local_peer
> + = ld->peer_ports[i].local;
> + const struct sbrec_port_binding *repb = find_route_exchange_pb(
> + r_ctx_in->sbrec_port_binding_by_name,
> + r_ctx_in->chassis,
> + r_ctx_in->active_tunnels,
> + local_peer);
> + if (!repb) {
> + continue;
> + }
> +
> + ad->maintain_vrf |= smap_get_bool(
> + &repb->options, "dynamic-routing-maintain-vrf", false);
> + sset_add(&ad->bound_ports, local_peer->logical_port);
> + }
> +
> + if (sset_is_empty(&ad->bound_ports)) {
> + advertise_datapath_cleanup(ad);
> + continue;
> + }
> + tracked_datapath_add(ld->datapath, TRACKED_RESOURCE_NEW,
> + r_ctx_out->tracked_re_datapaths);
> +
> + /* While tunnel_key would most likely never be negative, the compiler
> + * has opinions if we don't check before using it in snprintf below.
> */
> + if (ld->datapath->tunnel_key < 0 ||
> + ld->datapath->tunnel_key > MAX_TABLE_ID) {
This seems unnecessary. The SB schema sets a maximum for the datapath
binding tunnel_key ('"maxInteger": 16777215'). I guess we don't need
MAX_TABLE_ID at all if we remove the check.
> + VLOG_WARN_RL(&rl,
> + "skip route sync for datapath "UUID_FMT", "
> + "tunnel_key %"PRIi64" would make VRF interface name
> "
> + "overflow.",
> + UUID_ARGS(&ld->datapath->header_.uuid),
> + ld->datapath->tunnel_key);
> + goto cleanup;
> + }
> +
> + hmap_insert(r_ctx_out->announce_routes, &ad->node,
> ad->db->tunnel_key);
> + continue;
> +
> +cleanup:
> + advertise_datapath_cleanup(ad);
> + }
> +
> + const struct sbrec_advertised_route *route;
> + SBREC_ADVERTISED_ROUTE_FOR_EACH (route, r_ctx_in->ovnsb_idl) {
If you use the SBREC_ADVERTISED_ROUTE_TABLE_FOR_EACH() macro instead
then you don't need to pass the IDL pointer. We do that in all places
in ovn-controller; we pass a pointer to the table instead. See "struct
binding_ctx_in.port_binding_table" for an example.
> + struct in6_addr prefix;
> + unsigned int plen;
> + if (!ip46_parse_cidr(route->ip_prefix, &prefix, &plen)) {
> + VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in route "
> + UUID_FMT, route->ip_prefix,
> + UUID_ARGS(&route->header_.uuid));
> + continue;
> + }
> +
> + ad = advertise_datapath_find(r_ctx_out->announce_routes,
> + route->datapath);
> + if (!ad) {
> + continue;
> + }
> +
Let's do the ip_prefix parsing here instead, only if the datapath is
configured to advertise anything.
> + struct advertise_route_entry *ar = xzalloc(sizeof(*ar));
s/sizeof(*ar)/sizeof *ar/
Also, xmalloc is fine. We're writing all fields immediately anyway.
> + hmap_insert(&ad->routes, &ar->node,
> + advertise_route_hash(&prefix, plen));
Nit: I know it doesn't matter in this case but it feels weird to already
insert a partially initialized struct into the hmap. Can we do the
hmap_insert() two lines lower?
> + ar->addr = prefix;
> + ar->plen = plen;
> + }
> +}
> +
> +void
> +route_cleanup(struct hmap *announce_routes)
> +{
> + struct advertise_datapath_entry *ad;
> + HMAP_FOR_EACH_SAFE (ad, node, announce_routes) {
> + hmap_remove(announce_routes, &ad->node);
Nit: HMAP_FOR_EACH_POP is shorter.
> + advertise_datapath_cleanup(ad);
> + }
> +}
> diff --git a/controller/route.h b/controller/route.h
> new file mode 100644
> index 000000000..6fc5963a5
> --- /dev/null
> +++ b/controller/route.h
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (c) 2024, Canonical, Ltd.
> + * 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 ROUTE_H
> +#define ROUTE_H 1
> +
> +#include <stdbool.h>
> +#include <netinet/in.h>
> +#include "openvswitch/hmap.h"
> +#include "sset.h"
> +
> +struct hmap;
> +struct ovsdb_idl_index;
> +struct sbrec_chassis;
> +struct sbrec_port_binding;
> +
> +struct route_ctx_in {
> + struct ovsdb_idl *ovnsb_idl;
> + struct ovsdb_idl_index *sbrec_port_binding_by_name;
> + const struct sbrec_chassis *chassis;
> + const struct sset *active_tunnels;
> + const struct hmap *local_datapaths;
> + const struct sset *local_lports;
> +};
> +
> +struct route_ctx_out {
> + struct hmap *tracked_re_datapaths;
> + /* Contains struct advertise_datapath_entry */
> + struct hmap *announce_routes;
> +};
> +
> +struct advertise_datapath_entry {
> + struct hmap_node node;
> + const struct sbrec_datapath_binding *db;
> + bool maintain_vrf;
> + struct hmap routes;
> +
> + /* The name of the port bindings locally bound for this datapath and
> + * running route exchange logic. */
> + struct sset bound_ports;
> +};
> +
> +struct advertise_route_entry {
> + struct hmap_node node;
> + struct in6_addr addr;
> + unsigned int plen;
> +};
> +
> +bool route_exchange_relevant_port(const struct sbrec_port_binding *pb);
> +uint32_t advertise_route_hash(const struct in6_addr *dst, unsigned int plen);
'advertise_route_hash()` can be static inside route.c. I don't think we
use it anywhere else (I also checked the remaining patches in the series).
> +void route_run(struct route_ctx_in *, struct route_ctx_out *);
> +void route_cleanup(struct hmap *announce_routes);
> +
> +#endif /* ROUTE_H */
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 3899c9e80..9244532fa 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -305,6 +305,7 @@ tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \
> controller/ovsport.$(OBJEXT) \
> controller/patch.$(OBJEXT) \
> controller/vif-plug.$(OBJEXT) \
> + controller/route.$(OBJEXT) \
Nit: Could you please move this one line higher? In this makefile
(most) source files are listed in alphabetical order. We should
probably see if we need to fix that up everywhere but in this specific
case it looks a bit nicer if we don't break the ordering. :)
> northd/ipam.$(OBJEXT)
>
> # Python tests.
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
