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?
Thanks,
Martin.
>
> > 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
