Messages in this thread | | | Date | Tue, 9 Apr 2024 11:13:22 +0000 | Subject | Re: [PATCH net-next 1/6] flow_offload: add flow_rule_no_unsupp_control_flags() | From | Asbjørn Sloth Tønnesen <> |
| |
Hi Louis,
On 4/9/24 8:40 AM, Louis Peens wrote: > 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. I agree, I will update the naming in v2:
flow_rule_no_unsupp_control_flags => flow_rule_is_supp_control_flags flow_rule_no_control_flags + s/no/has/ => flow_rule_has_control_flags flow_rule_match_no_control_flags + s/no/has/ => flow_rule_match_has_control_flags
-- Best regards Asbjørn Sloth Tønnesen Network Engineer Fiberby - AS42541
| |