On 23 Jan 2025, at 16:20, Martin Kalcok wrote:
> A recent commit added condition to "route_table_parse__" function that
> causes it to throw an error when parsing route without "nexthop
> information" (either RTA_OIF, RTA_GATEWAY, RTA_VIA or RTA_MULTIPATH). While
> this requirement is reasonable for regular routes, there are some route types
> that don't need nexthop. We intend to use one of these types,
> (RTN_BLACKHOLE)[0], in OVN for route advertising .
>
> This change does not enforce the above-mentioned condition for those special
> route types that don't require "nexthop information".
>
> [0] https://mail.openvswitch.org/pipermail/ovs-dev/2025-January/419383.html
>
> CC: Frode Nordahl <[email protected]>
> Fixes: 91fc51106cfe ("route-table: Support parsing multipath routes.")
> Signed-off-by: Martin Kalcok <[email protected]>
One small nit below, but we can change this on commit (we missed it in an
earlier patch).
Acked-by: Eelco Chaudron <[email protected]>
> ---
> v4:
> * Fix code style.
> * Rebased on current 'main' after the requirement patchset[1] was merged.
>
> v3:
> * Fix typo in arguments for the "route_type_needs_nexthop" function
>
> v2:
> * Ensure that the list of nexthops is cleared if the route does not have
> them.
> * The function for determining whether a route requires nexthop now takes
> route_type (unsigned char) as an argument directly. Previously it
> took a pointer to rtmsg struct.
> * The patch is rebased on top of the in-flight patch series by Frode[1]
> * Fixed typos.
>
> [1]
> https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
>
> lib/route-table.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/lib/route-table.c b/lib/route-table.c
> index 6dfb364a4..08d37bbfd 100644
> --- a/lib/route-table.c
> +++ b/lib/route-table.c
> @@ -226,6 +226,24 @@ route_table_reset(void)
> }
> }
>
> +/* Returns true if the given route requires nexthop information (output
> + * interface, nexthop IP, ...). Returns false for special route types
> + * that don't need this information. */
> +static bool
> +route_type_needs_nexthop(unsigned char rtmsg_type)
> +{
> + switch (rtmsg_type) {
> + case RTN_BLACKHOLE:
> + case RTN_THROW:
> + case RTN_UNREACHABLE:
> + case RTN_PROHIBIT:
> + return false;
> +
> + default:
> + return true;
> + }
> +}
> +
> static int
> route_table_parse__(struct ofpbuf *buf, size_t ofs,
> const struct nlmsghdr *nlmsg,
> @@ -450,13 +468,20 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
> ofpbuf_uninit(&mp_buf);
> }
> }
> - if (!attrs[RTA_OIF] && !attrs[RTA_GATEWAY]
> + if (route_type_needs_nexthop(rtm->rtm_type)
> + && !attrs[RTA_OIF] && !attrs[RTA_GATEWAY]
> && !attrs[RTA_VIA] && !attrs[RTA_MULTIPATH]) {
The && on the next two lines should be aligned to the r from
route_type_needs_nexthop().
> VLOG_DBG_RL(&rl, "route message needs an RTA_OIF, RTA_GATEWAY, "
> "RTA_VIA or RTA_MULTIPATH attribute");
> goto error_out;
> }
> /* Add any additional RTA attribute processing before RTA_MULTIPATH.
> */
> +
> + /* Ensure that the change->rd->nexthops list is cleared in cases when
> + * the route does not need a next hop. */
> + if (!route_type_needs_nexthop(rtm->rtm_type)) {
> + route_data_destroy_nexthops__(&change->rd);
> + }
> } else {
> VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
> goto error_out;
> --
> 2.43.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
