[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240409132043.gaf5heor5vx42j3j@DEN-DL-M70577>
Date: Tue, 9 Apr 2024 13:20:43 +0000
From: Daniel Machon <daniel.machon@...rochip.com>
To: Asbjørn Sloth Tønnesen <ast@...erby.net>
CC: Steen Hegelund <Steen.Hegelund@...rochip.com>, Lars Povlsen
<lars.povlsen@...rochip.com>, "David S. Miller" <davem@...emloft.net>, "Eric
Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, <UNGLinuxDriver@...rochip.com>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
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@...rochip.com>
Powered by blists - more mailing lists