On 30 Jan 2025, at 11:26, [email protected] wrote:
> On Thu, 2025-01-30 at 10:43 +0100, Eelco Chaudron wrote: >> >> >> 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(). > > Oh yeah, I just followed indentation levels from previous change :D > Should I post new version or can it be fixed on merge/upstream commit? > Don’t worry, I’ll fix it on commit. //Eelco >> >>> 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
