On Thu, 2025-01-23 at 12:54 -0500, Aaron Conole wrote: > Eelco Chaudron <[email protected]> writes: > > > On 21 Jan 2025, at 14:23, [email protected] wrote: > > > > > The github robot didn't seem to take into consideration `base- > > > commit` > > > and `prerequisite-patch-id` tags. However now that the > > > prerequisite > > > series is (mostly) merge into main, perhaps just re-triggering CI > > > could > > > be enough. > > > > Copying Aaron as I do not think the robot supports ‘rerequisite- > > patch-id’, and ‘base-commit’ tags. > > We don't have such support in OVS side. > > You will need to resubmit the series to get it to rebase on what is > merged. > > We did have a discussion on the ML years ago and the conclusion at > that > time was that we shouldn't have series which depend on other series > in-flight. That is because if the base series needs major rework, > both > will need to be resubmitted. So it doesn't exactly speed up much. > > Hope it helps.
Thanks for the explanation Aaron, I did ended up with posting v4. The original was just an attempt to indicate dependency for (hopefully) easier review. But I think you are right, if the CI ran and passed with the indicated dependencies, and if that dependency series got changed afterwards, it could create false sense of "passing CI". Martin. > > > > Recheck-request: github-robot > > > > > > On Tue, 2025-01-21 at 11:44 +0100, 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". > > > > > > > > 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. > > > > > > > > [0] > > > > https://mail.openvswitch.org/pipermail/ovs-dev/2025-January/419383.html > > > > [1] > > > > https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ > > > > > > > > CC: Frode Nordahl <[email protected]> > > > > Fixes: 91fc51106cfe ("route-table: Support parsing multipath > > > > routes.") > > > > Signed-off-by: Martin Kalcok <[email protected]> > > > > --- > > > > lib/route-table.c | 28 ++++++++++++++++++++++++++-- > > > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/lib/route-table.c b/lib/route-table.c > > > > index 6dfb364a4..ecc7fd7e5 100644 > > > > --- a/lib/route-table.c > > > > +++ b/lib/route-table.c > > > > @@ -226,6 +226,23 @@ 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 +467,20 @@ route_table_parse__(struct ofpbuf *buf, > > > > size_t > > > > ofs, > > > > ofpbuf_uninit(&mp_buf); > > > > } > > > > } > > > > - if (!attrs[RTA_OIF] && !attrs[RTA_GATEWAY] > > > > - && !attrs[RTA_VIA] && !attrs[RTA_MULTIPATH]) { > > > > + if (route_type_needs_nexthop(rtm->rtm_type) && > > > > !attrs[RTA_OIF] > > > > + && !attrs[RTA_GATEWAY] && !attrs[RTA_VIA] > > > > + && !attrs[RTA_MULTIPATH]) { > > > > 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 > > > > case that > > > > + * 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; > > > > > > > > base-commit: caed64d1685409d1f9b53b35621a08ad6588200c > > > > prerequisite-patch-id: 9d606dcf57f9213291028029f2a71a43f346ce1e > > > > prerequisite-patch-id: b61606e7f32fede1bd34248957f67d72040be326 > > > > prerequisite-patch-id: 907ead49b46ccf4b2e81e9f49f535788e592348b > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
