Messages in this thread Patch in this message | | | Date | Tue, 9 Apr 2024 10:40:51 +0200 | From | Louis Peens <> | Subject | Re: [PATCH net-next 1/6] flow_offload: add flow_rule_no_unsupp_control_flags() |
| |
On Mon, Apr 08, 2024 at 01:09:19PM +0000, Asbjørn Sloth Tønnesen wrote: > [Some people who received this message don't often get email from ast@fiberby.net. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > This helper can be used by drivers to check for the > presence of unsupported control flags. > > It mirrors the existing check done in sfc: > drivers/net/ethernet/sfc/tc.c +276 > > This is aimed at drivers, which implements some control flags. > > This should also be used by drivers that implement all > current flags, so that future flags will be unsupported > by default. > > Only compile-tested. > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> > --- > include/net/flow_offload.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h > index 314087a5e1818..c1317b14da08c 100644 > --- a/include/net/flow_offload.h > +++ b/include/net/flow_offload.h > @@ -449,6 +449,28 @@ static inline bool flow_rule_match_key(const struct flow_rule *rule, > return dissector_uses_key(rule->match.dissector, key); > } > > +/** > + * flow_rule_no_unsupp_control_flags() - check for unsupported control flags > + * @supp_flags: flags supported by driver > + * @flags: flags present in rule > + * @extack: The netlink extended ACK for reporting errors. > + * > + * Returns true if only supported control flags are set, false otherwise. > + */ > +static inline bool flow_rule_no_unsupp_control_flags(const u32 supp_flags, > + const u32 flags, > + struct netlink_ext_ack *extack) Thanks for the change Asbjørn, I like the series in general. I do have some nitpicking with the naming of this function, the double negative makes it a bit hard to read. Especially where it gets used, where it then reads as: 'if not no unsupported'
I think something like: 'if not supported' or 'if unsupported'
will read much better - personally I think the first option is the best, otherwise you might end up with 'if not unsupported', which is also weird.
Some possible suggestions I can think of: flow_rule_control_flags_is_supp() flow_rule_is_supp_control_flags() flow_rule_check_supp_control_flags() or perhaps some even better variant of this. I hope it's not just me, if that's the case please feel free to ignore.
| |