On 2/6/25 3:19 PM, Felix Huettner via dev wrote:
> Here we expand the previous routes-sync engine node to not only
> advertise routes to the southbound table, but also learn received routes
> from this table.
>
> These routes are then passed to the same logic that connected and static
> routes are using for flow generation.
> However we prioritize these routes lower than connected or static routes
> as information in cluster (for the same prefix length) should always be
> more correct then learned routes.
> This is also consistent with the default behaviour of phyiscal routers.
>
> Acked-by: Dumitru Ceara <[email protected]>
> Signed-off-by: Felix Huettner <[email protected]>
> ---
Hi Felix,
> v6->v7:
> * addressed review comments
> v5->v6:
> * addressed review comments
> * changed option to "dynamic-routing-redistribute"
> v4->v5: skipped
> v2->v3:
> * A lot of minor review comments.
> * Support learning routes over other address families
>
> NEWS | 4 +
> TODO.rst | 5 +
> lib/stopwatch-names.h | 1 +
> northd/automake.mk | 2 +
> northd/en-ecmp-nexthop.c | 16 +-
> northd/en-learned-route-sync.c | 208 +++++++++++++++++++++++
> northd/en-learned-route-sync.h | 32 ++++
> northd/en-lflow.c | 5 +-
> northd/inc-proc-northd.c | 14 +-
> northd/northd.c | 301 ++++++++++++++++++++-------------
> northd/northd.h | 38 ++++-
> northd/ovn-northd.c | 1 +
> tests/ovn-northd.at | 253 +++++++++++++++++++++++----
> 13 files changed, 724 insertions(+), 156 deletions(-)
> create mode 100644 northd/en-learned-route-sync.c
> create mode 100644 northd/en-learned-route-sync.h
>
[...]
> diff --git a/northd/northd.h b/northd/northd.h
> index 06314730d..1fa0d6365 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -718,6 +718,8 @@ enum route_source {
> ROUTE_SOURCE_CONNECTED,
> /* The route is derived from a northbound static route entry. */
> ROUTE_SOURCE_STATIC,
> + /* The route is dynamically learned by an ovn-controller. */
> + ROUTE_SOURCE_LEARNED,
> };
>
> struct parsed_route {
> @@ -728,17 +730,51 @@ struct parsed_route {
> bool is_src_route;
> uint32_t route_table_id;
> uint32_t hash;
> - const struct nbrec_logical_router_static_route *route;
> bool ecmp_symmetric_reply;
> bool is_discard_route;
> const struct ovn_datapath *od;
> bool stale;
> struct sset ecmp_selection_fields;
> enum route_source source;
> + const struct ovsdb_idl_row *source_hint;
> char *lrp_addr_s;
> const struct ovn_port *out_port;
> };
>
> +/* Returns an independent clone of the provided parsed_route. The returned
> + * parsed_route will need to be freed using parsed_route_free. */
> +struct parsed_route *parsed_route_clone(const struct parsed_route *);
> +
> +/* Returns the hash to be used for a hmap of parsed_routes that are provided
> + * to the lflow engine node. */
> +size_t parsed_route_hash(const struct parsed_route *);
> +void parsed_route_free(struct parsed_route *);
> +
> +/* Adds a parsed_route to the provided routes hmap if it is not already
> + * in there.
> + * Takes ownership of the provided nexthop. All other parameters are cloned.
> + * Elements of the routes hmap need to be freed using parsed_route_free. */
Thanks for documenting these functions! However, the comments should go
in the C file.
With that addressed, my ack stands:
Acked-by: Dumitru Ceara <[email protected]>
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
