Messages in this thread | | | Date | Tue, 9 Apr 2024 13:20:43 +0000 | From | Daniel Machon <> | Subject | Re: [PATCH net] net: sparx5: flower: fix fragment flags handling |
| |
> 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. > > It should now be aligned with how FLOW_DIS_IS_FRAGMENT > and FLOW_DIS_FIRST_FRAG is set in __skb_flow_dissect() in > net/core/flow_dissector.c > > Since the VCAP fragment values are not a bitfield, we have > to ignore the suspicious fragment value, eg. when matching > on any kind of fragment with FLOW_DIS_IS_FRAGMENT=1/1. > > Only compile tested, and logic tested in userspace, as I > unfortunately don't have access to this switch chip (yet).
Ran VCAP test-suites - seems good. Thanks!
Tested-by: Daniel Machon <daniel.machon@microchip.com>
| |