Messages in this thread | | | Date | Wed, 10 Apr 2024 15:20:15 -0700 | Subject | Re: [PATCH net v2] net: sparx5: flower: fix fragment flags handling | From | 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@intel.com>
| |