Message-ID: <20240628124409.GD783093@kernel.org>
Date: Fri, 28 Jun 2024 13:44:09 +0100
From: Simon Horman <horms@...nel.org>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	michal.swiatkowski@...ux.intel.com, przemyslaw.kitszel@...el.com
Subject: Re: [PATCH iwl-next 5/6] ice: Optimize switch recipe creation

On Tue, Jun 18, 2024 at 04:11:56PM +0200, Marcin Szycik wrote:
> Currently when creating switch recipes, switch ID is always added as the
> first word in every recipe. There are only 5 words in a recipe, so one
> word is always wasted. This is also true for the last recipe, which stores
> result indexes (in case of chain recipes). Therefore the maximum usable
> length of a chain recipe is 4 * 4 = 16 words. 4 words in a recipe, 4
> recipes that can be chained (using a 5th one for result indexes).
> 
> Current max size chained recipe:
> 0: smmmm
> 1: smmmm
> 2: smmmm
> 3: smmmm
> 4: srrrr
> 
> Where:
> s - switch ID
> m - regular match (e.g. ipv4 src addr, udp dst port, etc.)
> r - result index
> 
> Switch ID does not actually need to be present in every recipe, only in one
> of them (in case of chained recipe). This frees up to 8 extra words:
> 3 from recipes in the middle (because first recipe still needs to have
> switch ID), and 5 from one extra recipe (because now the last recipe also
> does not have switch ID, so it can chain 1 more recipe).
> 
> Max size chained recipe after changes:
> 0: smmmm
> 1: Mmmmm
> 2: Mmmmm
> 3: Mmmmm
> 4: MMMMM
> 5: Rrrrr
> 
> Extra usable words available after this change are highlighted with capital
> letters.
> 
> Changing how switch ID is added is not straightforward, because it's not a
> regular lookup. Its FV index and mask can't be determined based on protocol
> + offset pair read from package and instead need to be added manually.
> 
> Additionally, change how result indexes are added. Currently they are
> always inserted in a new recipe at the end. Example for 13 words, (with
> above optimization, switch ID being one of the words):
> 0: smmmm
> 1: mmmmm
> 2: mmmxx
> 3: rrrxx
> 
> Where:
> x - unused word
> 
> In this and some other cases, the result indexes can be moved just after
> last matches because there are unused words, saving one recipe. Example
> for 13 words after both optimizations:
> 0: smmmm
> 1: mmmmm
> 2: mmmrr
> 
> Note how one less result index is needed in this case, because the last
> recipe does not need to "link" to itself.
> 
> There are cases when adding an additional recipe for result indexes cannot
> be avoided. In that cases result indexes are all put in the last recipe.
> Example for 14 words after both optimizations:
> 0: smmmm
> 1: mmmmm
> 2: mmmmx
> 3: rrrxx
> 
> With these two changes, recipes/rules are more space efficient, allowing
> more to be created in total.
> 
> Co-developed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@...ux.intel.com>

I appreciate the detailed description above, it is very helpful.
After a number of readings of this patch - it is complex -
I was unable to find anything wrong. And I do like both the simplification
and better hw utilisation that this patch (set) brings.

So from that perspective:

Reviewed-by: Simon Horman <horms@...nel.org>

I would say, however, that it might have been easier to review
if somehow this patch was broken up into smaller pieces.
I appreciate that, in a sense, that is what the other patches
of this series do. But nonetheless... it is complex.

...

Powered by blists - more mailing lists