Re: [PATCH net-next 1/6] flow_offload: add flow_rule_no_unsupp_control_flags()

[Asbjørn Sloth Tønnesen]

Hi Baowen,

On 4/9/24 1:56 AM, Baowen Zheng wrote:
> On April 8, 2024 9:09 PM,  Asbjørn wrote:
>
> > 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@xxxxxxxxxxx>
> > ---
> > 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,
> > +
> Hi Asbjørn, thanks for your work, it makes sense for driver check. Will it better to name flags as "ctrl_flags" to make it more clear since it indicates the ctrl_flags in rule and you name it as control.flags in the following print message.

Good point, I will rename that argument in v2.

I copied the error message verbatim from sfc.

>                                                     struct
> > +netlink_ext_ack *extack) {
> > +       if (likely((flags & ~supp_flags) == 0))
> > +               return true;
> > +
> > +       NL_SET_ERR_MSG_FMT_MOD(extack,
> > +                              "Unsupported match on control.flags %#x",
> > +                              flags);
> > +
> > +       return false;
> > +}
> > +
> > struct flow_stats {
> >         u64     pkts;
> >         u64     bytes;
> > --
> This should not be included in this patch.

It's the default 3 lines of context, as is default in git format-patch.

--
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541