Re: [dpdk-dev] [RFC v2] Generic flow director/filtering/classification API

[John Fastabend <john.fastabend@gmail.com>]

On 16-08-19 12:32 PM, Adrien Mazarguil wrote:
> Hi All,
> 
> Thanks to many for the positive and constructive feedback I've received so
> far. Here is the updated specification (v0.7) at last.
> 
> I've attempted to address as many comments as possible but could not
> process them all just yet. A new section "Future evolutions" has been
> added for the remaining topics.
> 
> This series adds rte_flow.h to the DPDK tree. Next time I will attempt to
> convert the specification as a documentation commit part of the patchset
> and actually implement API functions.
> 
> I think including the entire document here makes it easier to annotate on
> the ML, apologies in advance for the resulting traffic.
> 
> Finally I'm off for the next two weeks, do not expect replies from me in
> the meantime.
> 

Hopefully on vacation :)

[...]


> .. raw:: pdf
> 
>    PageBreak
> 
> +-------------------------------------------+
> | UDP payload matching                      |
> +===+=======================================+
> | 0 | Ethernet                              |
> +---+---------------------------------------+
> | 1 | IPv4                                  |
> +---+---------------------------------------+
> | 2 | UDP                                   |
> +---+-----+----------+--------------+-------+
> | 3 | RAW | ``spec`` | ``relative`` | 1     |
> |   |     |          +--------------+-------+
> |   |     |          | ``search``   | 1     |
> |   |     |          +--------------+-------+
> |   |     |          | ``offset``   | 10    |
> |   |     |          +--------------+-------+
> |   |     |          | ``limit``    | 0     |
> |   |     |          +--------------+-------+
> |   |     |          | ``length``   | 3     |
> |   |     |          +--------------+-------+
> |   |     |          | ``pattern``  | "foo" |
> +---+-----+----------+--------------+-------+
> | 4 | RAW | ``spec`` | ``relative`` | 1     |
> |   |     |          +--------------+-------+
> |   |     |          | ``search``   | 0     |
> |   |     |          +--------------+-------+
> |   |     |          | ``offset``   | 20    |
> |   |     |          +--------------+-------+
> |   |     |          | ``limit``    | 0     |
> |   |     |          +--------------+-------+
> |   |     |          | ``length``   | 3     |
> |   |     |          +--------------+-------+
> |   |     |          | ``pattern``  | "bar" |
> +---+-----+----------+--------------+-------+
> | 5 | RAW | ``spec`` | ``relative`` | 1     |
> |   |     |          +--------------+-------+
> |   |     |          | ``search``   | 0     |
> |   |     |          +--------------+-------+
> |   |     |          | ``offset``   | -29   |
> |   |     |          +--------------+-------+
> |   |     |          | ``limit``    | 0     |
> |   |     |          +--------------+-------+
> |   |     |          | ``length``   | 3     |
> |   |     |          +--------------+-------+
> |   |     |          | ``pattern``  | "baz" |
> +---+-----+----------+--------------+-------+
> 

Just an observation if you made 'offset' specified as an embedded RAW
field so that the offset could point at header length this would befully
generic. Although I guess its not practical as far as I know no hardware
would support the most general case.

> This translates to:
> 
> - Locate "foo" at least 10 bytes deep inside UDP payload.
> - Locate "bar" after "foo" plus 20 bytes.
> - Locate "baz" after "bar" minus 29 bytes.
> 
> Such a packet may be represented as follows (not to scale)::
> 
>  0                     >= 10 B           == 20 B
>  |                  |<--------->|     |<--------->|
>  |                  |           |     |           |
>  |-----|------|-----|-----|-----|-----|-----------|-----|------|
>  | ETH | IPv4 | UDP | ... | baz | foo | ......... | bar | .... |
>  |-----|------|-----|-----|-----|-----|-----------|-----|------|
>                           |                             |
>                           |<--------------------------->|
>                                       == 29 B
> 

[...]

> 
> Future evolutions
> =================
> 
> - Describing dedicated testpmd commands to control and validate this API.
> 
> - A method to optimize generic flow rules with specific pattern items and
>   action types generated on the fly by PMDs. DPDK will assign negative
>   numbers to these in order to not collide with the existing types. See
>   `Negative types`_.

Great thanks. As long as we build the core layer to support this then it
looks good to me.

> 
> - Adding specific egress pattern items and actions as described in `Traffic
>   direction`_.
> 
> - Optional software fallback when PMDs are unable to handle requested flow
>   rules so applications do not have to implement their own.

This is an interesting block. Would you presumably build this using the
existing support in DPDK or propose something else?

> 
> - Ranges in addition to bit-masks. Ranges are more generic in many ways as
>   they interpret values. For instance only ranges make sense to cover
>   several TCP or UDP ports. These will probably be defined on a pattern item
>   basis.
> 

Yep not needed at first but hardware does support this.




Thanks for doing this work, I'll look it over in a bit more detail over
the next few days but it looks like a reasonable base implementation to
me.

.John