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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 31 May 2020 00:39:24 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>,
        radu-andrei.bulie@....com, fido_max@...ox.ru,
        Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v2 net-next 06/13] net: dsa: felix: create a template for
 the DSA tags on xmit

Hi Florian,

On Sun, 31 May 2020 at 00:31, Florian Fainelli <f.fainelli@...il.com> wrote:
>
>
>
> On 5/30/2020 4:51 AM, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@....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@...il.com>
> --
> Florian

Yes, but with a check for device id in the fast path, the xmit
performance would have been slightly worse, and this way it is
slightly better. Again, not the magnitude is important here (which is
marginal either way), but the sign :)

Thanks,
-Vladimir