On Fri, Jun 28, 2024 at 03:39:27PM +0200, Marcin Szycik wrote: > > > On 28.06.2024 14:44, Simon Horman wrote: > > 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.swiatkow...@linux.intel.com> > >> Signed-off-by: Michal Swiatkowski <michal.swiatkow...@linux.intel.com> > >> Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com> > >> Signed-off-by: Marcin Szycik <marcin.szy...@linux.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 <ho...@kernel.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. > > Yeah... it is a bit of a revolution, and unfortunately I don't think much of > if could be separated into other patches. Maybe functions like > fill_recipe_template() and bookkeep_recipe() would be good candidates. > If there will be another version, I'll try to separate some of it.
Understood. TBH, I couldn't think of a great way to split it either.
