Re: [PATCH v2 net-next 06/13] net: dsa: felix: create a template for the DSA tags on xmit

[Florian Fainelli <f.fainelli@gmail.com>]

On 5/30/2020 4:51 AM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> With this patch we try to kill 2 birds with 1 stone.
> 
> First of all, some switches that use tag_ocelot.c don't have the exact
> same bitfield layout for the DSA tags. The destination ports field is
> different for Seville VSC9953 for example. So the choices are to either
> duplicate tag_ocelot.c into a new tag_seville.c (sub-optimal) or somehow
> take into account a supposed ocelot->dest_ports_offset when packing this
> field into the DSA injection header (again not ideal).
> 
> Secondly, tag_ocelot.c already needs to memset a 128-bit area to zero
> and call some packing() functions of dubious performance in the
> fastpath. And most of the values it needs to pack are pretty much
> constant (BYPASS=1, SRC_PORT=CPU, DEST=port index). So it would be good
> if we could improve that.
> 
> The proposed solution is to allocate a memory area per port at probe
> time, initialize that with the statically defined bits as per chip
> hardware revision, and just perform a simpler memcpy in the fastpath.
> 
> Other alternatives have been analyzed, such as:
> - Create a separate tag_seville.c: too much code duplication for just 1
>   bit field difference.

If this is really the only difference, we could have added a device ID
or something that would allow tag_ocelot.c to differentiate Seville from
Felix and just have a conditional for using the right definition.

The solution proposed here is okay and scales beyond a single bit field
difference. Maybe this will open up the door for consolidating the
various Microchip KSZ tag implementations at some point.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian