netdev - Re: [RFC PATCH 02/13] net: dsa: implement a central TX reallocation procedure

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 for Android: free password hash cracker in your pocket

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

Message-ID: <af11fa94-a62e-62c3-9fe2-8bb47b26eabe@gmail.com>
Date:   Sat, 17 Oct 2020 17:37:11 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        Christian Eggers <ceggers@...i.de>,
        Kurt Kanzenbach <kurt@...utronix.de>
Subject: Re: [RFC PATCH 02/13] net: dsa: implement a central TX reallocation
 procedure



On 10/17/2020 3:17 PM, Vladimir Oltean wrote:
> On Sat, Oct 17, 2020 at 03:11:52PM -0700, Florian Fainelli wrote:
>>> 	slave_dev->needed_headroom += master->needed_headroom;
>>> 	slave_dev->needed_tailroom += master->needed_tailroom;
>>
>> Not positive you need that because you may be account for more head or tail
>> room than necessary.
>>
>> For instance with tag_brcm.c and systemport.c we need 4 bytes of head room
>> for the Broadcom tag and an additional 8 bytes for pushing the transmit
>> status block descriptor in front of the Ethernet frame about to be
>> transmitted. These additional 8 bytes are a requirement of the DSA master
>> here and exist regardless of DSA being used, but we should not be
>> propagating them to the DSA slave.
> 
> And that's exactly what I'm trying to do here, do you see any problem
> with it? Basically I'm telling the network stack to allocate skbs with
> large enough headroom and tailroom so that reallocations will not be
> necessary for its entire TX journey. Not in DSA and not in the
> systemport either. That's the exact reason why the VLAN driver does this
> too, as far as I understand. Doing this trick also has the benefit that
> it works with stacked DSA devices too. The real master has a headroom
> of, say, 16 bytes, the first-level switch has 16 bytes, and the
> second-level switch has 16 more bytes. So when you inject an skb into
> the second-level switch (the one with the user ports that applications
> will use), the skb will be reallocated only once, with a new headroom of
> 16 * 3 bytes, instead of potentially 3 times (incrementally, first for
> 16, then for 32, then for 48). Am I missing something?
> 

That is fine with me, given that we can resolve most of the TX path 
ahead of time, I suppose we should indeed take advantage of that 
knowledge. Thanks!
-- 
Florian