netdev - Re: [PATCH RFC 5/6] net: dsa: microchip: Update tag

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:   Wed, 5 Dec 2018 10:52:13 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>, Tristram.Ha@...rochip.com
Cc:     Pavel Machek <pavel@....cz>, UNGLinuxDriver@...rochip.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH RFC 5/6] net: dsa: microchip: Update tag_ksz.c to access
 switch driver

On 12/5/18 10:18 AM, Andrew Lunn wrote:
> On Wed, Dec 05, 2018 at 07:00:38PM +0100, Andrew Lunn wrote:
>> On Mon, Dec 03, 2018 at 03:34:56PM -0800, Tristram.Ha@...rochip.com wrote:
>>> From: Tristram Ha <Tristram.Ha@...rochip.com>
>>>
>>> Update tag_ksz.c to access switch driver's tail tagging operations.
>>
>> Hi Tristram
>>
>> Humm, i'm not sure we want this, the tagging spit into two places.  I
>> need to take a closer look at the previous patch, to see why it cannot
>> be done here.
> 
> O.K, i think i get what is going on.
> 
> I would however implement it differently.
> 
> One net/dsa/tag_X.c file can export two dsa_device_ops structures,
> allowing you to share common code for the two taggers. You could call
> these DSA_TAG_PROTO_KSZ_1_BYTE, and DSA_TAG_PROTO_KSZ_2_BYTE, and the
> .get_tag_protocol call would then return the correct one for the
> switch.

Agreed, that is what is done by net/dsa/tag_brcm.c because there are two
formats for the Broadcom tag:

- TAG_BRCM: the 4-bytes Broadcom tag is between MAC SA and Ethertype
- TAG_BRCM_PREPEND: the 4-bytes Broadcom tag is before the MAC DA

And the code to process them is basically using relative offsets from
the start of the frame to access correct data.

This is done largely for performance reasons because we have 1/2
Gigabit/secs capable CPU ports and so we want to avoid as little cache
trashing as possible and immediately get the right rcv() function to
process the packets.

> 
> It might also be possible to merge in tag_trailer, or at least share
> some code.
> 
> What i don't yet understand is how you are passing PTP information
> around. The commit messages need to explain that, since it is not
> obvious, and it is the first time we have needed PTP info in a tag
> driver.
> 
> 	Andrew
> 


-- 
Florian