Re: [PATCH net v2] net: sparx5: flower: fix fragment flags handling

[Jacob Keller]

On 4/10/2024 2:52 AM, Asbjørn Sloth Tønnesen wrote:
> I noticed that only 3 out of the 4 input bits were used,
> mt.key->flags & FLOW_DIS_IS_FRAGMENT was never checked.
> 
> In order to avoid a complicated maze, I converted it to
> use a 16 byte mapping table.
> 
> As shown in the table below the old heuristics doesn't
> always do the right thing, ie. when FLOW_DIS_IS_FRAGMENT=1/1
> then it used to only match follow-up fragment packets.
> 
> Here are all the combinations, and their resulting new/old
> VCAP key/mask filter:
> 
>   /- FLOW_DIS_IS_FRAGMENT (key/mask)
>   |    /- FLOW_DIS_FIRST_FRAG (key/mask)
>   |    |    /-- new VCAP fragment (key/mask)
>   v    v    v    v- old VCAP fragment (key/mask)
> 
>  0/0  0/0  -/-  -/-     impossible (due to entry cond. on mask)
>  0/0  0/1  -/-  0/3 !!  invalid (can't match non-fragment + follow-up frag)
>  0/0  1/0  -/-  -/-     impossible (key > mask)
>  0/0  1/1  1/3  1/3     first fragment
> 
>  0/1  0/0  0/3  3/3 !!  not fragmented
>  0/1  0/1  0/3  3/3 !!  not fragmented (+ not first fragment)
>  0/1  1/0  -/-  -/-     impossible (key > mask)
>  0/1  1/1  -/-  1/3 !!  invalid (non-fragment and first frag)
> 
>  1/0  0/0  -/-  -/-     impossible (key > mask)
>  1/0  0/1  -/-  -/-     impossible (key > mask)
>  1/0  1/0  -/-  -/-     impossible (key > mask)
>  1/0  1/1  -/-  -/-     impossible (key > mask)
> 
>  1/1  0/0  1/1  3/3 !!  some fragment
>  1/1  0/1  3/3  3/3     follow-up fragment
>  1/1  1/0  -/-  -/-     impossible (key > mask)
>  1/1  1/1  1/3  1/3     first fragment
> 
> In the datasheet the VCAP fragment values are documented as:
>  0 = no fragment
>  1 = initial fragment
>  2 = suspicious fragment
>  3 = valid follow-up fragment
> 
> Result: 3 combinations match the old behavior,
>         3 combinations have been corrected,
>         2 combinations are now invalid, and fail,
>         8 combinations are impossible.
> 

Appreciate the detailed analysis with the lookup table. This is a bit
more opaque but a maze of combinations would be even less readable and
maintainable.

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>