[RFC]: raw packet filtering via tc-flower

[RFC]: raw packet filtering via tc-flower

[Ahmed Zaki]

Hello,

Following on the discussion in [1] regarding raw packet filtering via 
ethtool and ntuple. To recap, we wanted to enable the user to offload 
filtering and flow direction using binary patterns of extended lengths 
and masks (e.g. 512 bytes). The conclusion was that ethtool and ntuple 
are deemed legacy and are not the best approach.

After some internal discussions, tc-flower seems to be another 
possibility. In [2], the skbedit and queue-mapping is now supported on 
the rx and the user can offload flow direction to a specific rx queue.

Can we extend tc-flower to support raw packet filtering, for example:

# tc filter add dev $IFACE ingress protocol 802_3 flower \
    offset $OFF pattern $BYTES mask $MASK \
    action skbedit queue_mapping $RXQ_ID skip_sw

where offset, pattern and mask are new the flower args, $BYTES and $MASK 
could be up to 512 bytes.

Any feedback is welcome.

Thank you,
Ahmed


1: 
https://lore.kernel.org/netdev/459ef75b-69dc-4baa-b7e4-6393b0b358ce@intel.com/ 


2: 
https://lore.kernel.org/netdev/166633911976.52141.3907831602027668289.stgit@anambiarhost.jf.intel.com/

Re: [RFC]: raw packet filtering via tc-flower

[Jakub Kicinski]

On Wed, 21 Feb 2024 12:43:47 -0700 Ahmed Zaki wrote:
> Following on the discussion in [1] regarding raw packet filtering via 
> ethtool and ntuple. To recap, we wanted to enable the user to offload 
> filtering and flow direction using binary patterns of extended lengths 
> and masks (e.g. 512 bytes). The conclusion was that ethtool and ntuple 
> are deemed legacy and are not the best approach.
> 
> After some internal discussions, tc-flower seems to be another 
> possibility. In [2], the skbedit and queue-mapping is now supported on 
> the rx and the user can offload flow direction to a specific rx queue.
> 
> Can we extend tc-flower to support raw packet filtering, for example:
> 
> # tc filter add dev $IFACE ingress protocol 802_3 flower \
>     offset $OFF pattern $BYTES mask $MASK \
>     action skbedit queue_mapping $RXQ_ID skip_sw
> 
> where offset, pattern and mask are new the flower args, $BYTES and $MASK 
> could be up to 512 bytes.

Have you looked at cls_u32 offload?

Re: [RFC]: raw packet filtering via tc-flower

[Jiri Pirko]

Fri, Feb 23, 2024 at 03:40:45AM CET, kuba@kernel.org wrote:
>On Wed, 21 Feb 2024 12:43:47 -0700 Ahmed Zaki wrote:
>> Following on the discussion in [1] regarding raw packet filtering via 
>> ethtool and ntuple. To recap, we wanted to enable the user to offload 
>> filtering and flow direction using binary patterns of extended lengths 
>> and masks (e.g. 512 bytes). The conclusion was that ethtool and ntuple 
>> are deemed legacy and are not the best approach.
>> 
>> After some internal discussions, tc-flower seems to be another 
>> possibility. In [2], the skbedit and queue-mapping is now supported on 
>> the rx and the user can offload flow direction to a specific rx queue.
>> 
>> Can we extend tc-flower to support raw packet filtering, for example:
>> 
>> # tc filter add dev $IFACE ingress protocol 802_3 flower \
>>     offset $OFF pattern $BYTES mask $MASK \
>>     action skbedit queue_mapping $RXQ_ID skip_sw
>> 
>> where offset, pattern and mask are new the flower args, $BYTES and $MASK 
>> could be up to 512 bytes.
>
>Have you looked at cls_u32 offload?

Hmm, but why flower can't be extended this direction. I mean, it is very
convenient to match on well-defined fields. I can imagine that the
combination of match on well-defined fields and random chunks together
is completely valid use-case. Also, offloading of flower is
straightforward.

U32 is, well, not that convenient.

Re: [RFC]: raw packet filtering via tc-flower

[Edward Cree]

On 23/02/2024 09:51, Jiri Pirko wrote:
> Hmm, but why flower can't be extended this direction. I mean, it is very
> convenient to match on well-defined fields.

Flower is intrinsically tied to the flow dissector, both conceptually
 and in implementation.  I'm not sure it's appropriate for it to become
 a dumping ground for random vendor filtering extensions/capabilities.

> U32 is, well, not that convenient.

How about a new classifier that just does this raw matching?

> I can imagine that the
> combination of match on well-defined fields and random chunks together
> is completely valid use-case.

But is it likely to be something that hardware supports?  (Since the
 motivation for this feature is clearly the hardware offload — otherwise
 there are other mechanisms like BPF for arbitrary packet filtering.)

As the vendor behind this, one hopes Intel can comment on both the
 hardware and the use-case side of this question.

Re: [RFC]: raw packet filtering via tc-flower

[Jiri Pirko]

Fri, Feb 23, 2024 at 01:07:06PM CET, ecree.xilinx@gmail.com wrote:
>On 23/02/2024 09:51, Jiri Pirko wrote:
>> Hmm, but why flower can't be extended this direction. I mean, it is very
>> convenient to match on well-defined fields.
>
>Flower is intrinsically tied to the flow dissector, both conceptually
> and in implementation.  I'm not sure it's appropriate for it to become
> a dumping ground for random vendor filtering extensions/capabilities.

Nope, the extension of dissector would be clean, one timer. Just add
support for offset+len based dissection. Also, the fact that flower uses
flow_dissector is only because DaveM requested that back in the days I
pushed flower. The original implementation was not using flow_dissector.
The dissection backing should not block flower extension. We can always
replace it if needed and convenient (maybe the criteria is met already).


>
>> U32 is, well, not that convenient.
>
>How about a new classifier that just does this raw matching?

That's u32 basically, isn't it?


>
>> I can imagine that the
>> combination of match on well-defined fields and random chunks together
>> is completely valid use-case.
>
>But is it likely to be something that hardware supports?  (Since the

Yeah, I know couple of ASICs that support this, driver names are mlx5
and mlxsw :)


> motivation for this feature is clearly the hardware offload — otherwise
> there are other mechanisms like BPF for arbitrary packet filtering.)
>
>As the vendor behind this, one hopes Intel can comment on both the
> hardware and the use-case side of this question.

Re: [RFC]: raw packet filtering via tc-flower

[Edward Cree]

On 23/02/2024 12:22, Jiri Pirko wrote:
> Nope, the extension of dissector would be clean, one timer. Just add
> support for offset+len based dissection.

... until someone else comes along with another kind of filtering and
 wants _that_ in flower for the same reasons.

>> How about a new classifier that just does this raw matching?
> 
> That's u32 basically, isn't it?

Well, u32 has all the extra complications around hashtables, links,
 permoff... I guess you could have helpers in the kernel to stitch
 'const' u32 filters into raw matches for drivers that only offload
 that and reject anything else; and tc userspace could have syntactic
 sugar to transform Ahmed's offset/pattern/mask into the appropriate
 u32 matches under the hood.

Re: [RFC]: raw packet filtering via tc-flower

[Jamal Hadi Salim]

On Fri, Feb 23, 2024 at 7:36 AM Edward Cree <ecree.xilinx@gmail.com> wrote:
>
> On 23/02/2024 12:22, Jiri Pirko wrote:
> > Nope, the extension of dissector would be clean, one timer. Just add
> > support for offset+len based dissection.
>
> ... until someone else comes along with another kind of filtering and
>  wants _that_ in flower for the same reasons.
>
> >> How about a new classifier that just does this raw matching?
> >
> > That's u32 basically, isn't it?
>
> Well, u32 has all the extra complications around hashtables, links,
>  permoff... I guess you could have helpers in the kernel to stitch
>  'const' u32 filters into raw matches for drivers that only offload
>  that and reject anything else; and tc userspace could have syntactic
>  sugar to transform Ahmed's offset/pattern/mask into the appropriate
>  u32 matches under the hood.

u32 has a DSL that deals with parsing as well, which includes dealing
with variable packet offsets etc. That is a necessary ingredient if
you want to do pragmatic parsing (example how do you point to TCP
ports if the IP header has options etc).
This flower extension, if it wants to do something equivalent, would
have to adopt some of that language.
For the record, I am not against this being part of flower i just dont
think it's a simple offset/value/mask. u32 for example decided that it
will work with fixed 4 byte lengths to simplify, etc..
I wouldnt trust user space to do the right thing either otherwise
syzkaller will be feasting on this ... I empathise that using flower
will impose an operational challenge of having to use two classifiers
(but I want to point out that it already does offloads).

cheers,
jamal

Re: [RFC]: raw packet filtering via tc-flower

[Edward Cree]

On 23/02/2024 13:32, Jamal Hadi Salim wrote:
> u32 has a DSL that deals with parsing as well, which includes dealing
> with variable packet offsets etc. That is a necessary ingredient if
> you want to do pragmatic parsing (example how do you point to TCP
> ports if the IP header has options etc).

My understanding (Ahmed can correct me) is that the proposed raw
 filtering here would not support variable packet offsets at all.
That is precisely why I consider it a narrow hack for specialised
 use-cases and thus oppose its addition to cls_flower.

Re: [RFC]: raw packet filtering via tc-flower

[Ahmed Zaki]

On 2024-02-23 6:44 a.m., Edward Cree wrote:
> On 23/02/2024 13:32, Jamal Hadi Salim wrote:
>> u32 has a DSL that deals with parsing as well, which includes dealing
>> with variable packet offsets etc. That is a necessary ingredient if
>> you want to do pragmatic parsing (example how do you point to TCP
>> ports if the IP header has options etc).
> 
> My understanding (Ahmed can correct me) is that the proposed raw
>   filtering here would not support variable packet offsets at all.
> That is precisely why I consider it a narrow hack for specialised
>   use-cases and thus oppose its addition to cls_flower.


Intel's DDP (NVM) comes with default parser tables that contain all the 
supported protocol definitions. In order to use RSS or flow director on 
any of these protocol/field that is not defined in ethtool/tc, we 
usually need to submit patches for kernel, PF and even virtchannel and 
vf drivers if we want support on the VF.

While Intel's hardware supports programming the parser IP stage (and 
that would allow mixed protocol field + binary matching/arbitrary 
offset), for now we want to support something like DPDK's raw filtering:

https://doc.dpdk.org/dts/test_plans/iavf_fdir_protocol_agnostic_flow_test_plan.html#test-case-1-vf-fdir-mac-ipv4-udp


What we had in mind is offloading based on exclusive binary matching, 
not mixed protocol field + binary matching. Also, as in my original 
example, may be restrict the protocol to 802_3, so all parsing starts at 
MAC hdr which would make the offset calculations much easier.

Please advice what is the best way forward, flower vs u32, new filter, 
..etc.

Thanks
Ahmed, Sridhar, Amritha

Re: [RFC]: raw packet filtering via tc-flower

[Jakub Kicinski]

On Mon, 26 Feb 2024 07:40:55 -0700 Ahmed Zaki wrote:
> Intel's DDP (NVM) comes with default parser tables that contain all the 
> supported protocol definitions. In order to use RSS or flow director on 
> any of these protocol/field that is not defined in ethtool/tc, we 
> usually need to submit patches for kernel, PF and even virtchannel and 
> vf drivers if we want support on the VF.
> 
> While Intel's hardware supports programming the parser IP stage (and 
> that would allow mixed protocol field + binary matching/arbitrary 
> offset), for now we want to support something like DPDK's raw filtering:
> 
> https://doc.dpdk.org/dts/test_plans/iavf_fdir_protocol_agnostic_flow_test_plan.html#test-case-1-vf-fdir-mac-ipv4-udp
> 
> 
> What we had in mind is offloading based on exclusive binary matching, 
> not mixed protocol field + binary matching. Also, as in my original 
> example, may be restrict the protocol to 802_3, so all parsing starts at 
> MAC hdr which would make the offset calculations much easier.
> 
> Please advice what is the best way forward, flower vs u32, new filter, 
> ..etc.

I vote for u32. We can always add a new filter. But if one already
exists which fully covers the functionality we shouldn't add a new
one until we know the exact pain points, IOW have tried the existing.

If we do add a new filter, I think this should be part of the P4
classifier. With the parsing tree instantiated from the device side
and filters added by the user..

Re: [RFC]: raw packet filtering via tc-flower

[Jamal Hadi Salim]

On Mon, Feb 26, 2024 at 10:03 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 26 Feb 2024 07:40:55 -0700 Ahmed Zaki wrote:
> > Intel's DDP (NVM) comes with default parser tables that contain all the
> > supported protocol definitions. In order to use RSS or flow director on
> > any of these protocol/field that is not defined in ethtool/tc, we
> > usually need to submit patches for kernel, PF and even virtchannel and
> > vf drivers if we want support on the VF.
> >
> > While Intel's hardware supports programming the parser IP stage (and
> > that would allow mixed protocol field + binary matching/arbitrary
> > offset), for now we want to support something like DPDK's raw filtering:
> >
> > https://doc.dpdk.org/dts/test_plans/iavf_fdir_protocol_agnostic_flow_test_plan.html#test-case-1-vf-fdir-mac-ipv4-udp
> >
> >
> > What we had in mind is offloading based on exclusive binary matching,
> > not mixed protocol field + binary matching. Also, as in my original
> > example, may be restrict the protocol to 802_3, so all parsing starts at
> > MAC hdr which would make the offset calculations much easier.
> >
> > Please advice what is the best way forward, flower vs u32, new filter,
> > ..etc.
>
> I vote for u32. We can always add a new filter. But if one already
> exists which fully covers the functionality we shouldn't add a new
> one until we know the exact pain points, IOW have tried the existing.
>

Yes, u32 is the most "ready". No need to patch the classifier code
(unlike flower) and the dev ops exists already (and has been used by
drivers in the past).

> If we do add a new filter, I think this should be part of the P4
> classifier. With the parsing tree instantiated from the device side
> and filters added by the user..

P4 will open much bigger opportunities for DDP imo (but some people
would claim i am a little tiny biased ;->).

cheers,
jamal