From:   Arend Van Spriel <arend.vanspriel@broadcom.com>
To:     Felix Fietkau <nbd@nbd.name>,
        =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= <toke@redhat.com>,
        <linux-wireless@vger.kernel.org>
CC:     <johannes@sipsolutions.net>,
        Herbert Xu <herbert@gondor.apana.org.au>
Date:   Sun, 14 Apr 2019 14:34:28 +0200
Message-ID: <16a1bd77c20.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
In-Reply-To: <e912d736-a3ad-1e21-0cdd-6a065df96fbb@nbd.name>
References: <20190316170634.13125-1-nbd@nbd.name>
 <20190316170634.13125-5-nbd@nbd.name>
 <87sgvmvg9g.fsf@toke.dk>
 <773e4dff-29fd-22b4-e4bc-cd5a94c66dc2@broadcom.com>
 <e912d736-a3ad-1e21-0cdd-6a065df96fbb@nbd.name>
User-Agent: AquaMail/1.19.0-1434 (build: 101900002)
Subject: Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On April 14, 2019 1:19:49 PM Felix Fietkau <nbd@nbd.name> wrote:

> On 2019-04-14 11:44, Arend Van Spriel wrote:
>> + Herbert
>> 
>> On 3/16/2019 7:14 PM, Toke Høiland-Jørgensen wrote:
>>> Felix Fietkau <nbd@nbd.name> writes:
>>> 
>>>> When using iTXQ, tx sequence number allocation and statistics are run at
>>>> dequeue time. Because of that, it is safe to enable NETIF_F_LLTX, which
>>>> allows tx handlers to run on multiple CPUs in parallel.
>>> 
>>> Cool, didn't know about that flag.
>> 
>> It is water under the bridge as this patch got applied already, but I
>> stumbled upon it just recently and didn't know about that flag either.
>> So I looked for more information about it and found the definition [1],
>> but the comment seemed important enough to send this reply.
>> 
>> 	NETIF_F_LLTX_BIT,	/* LockLess TX - deprecated. Please */
>> 				/* do not use LLTX in new drivers */
>> 
>> Here is the commit that marked it deprecated:
>> 
>> commit e24eb521fbf2a350ce879dfc1d8e56d4ffa2aa22
>> Author: Christian Borntraeger <borntraeger@de.ibm.com>
>> Date:   Tue Sep 25 19:42:02 2007 -0700
>> 
>>      [NET]: note that NETIF_F_LLTX is deprecated
>> 
>> So I am not sure we should really do this in mac80211. Maybe Herbert can
>> comment although it has been over a decade ago.
> There is a lot of comparable code that also uses this flag, e.g.
> batman-adv, bridge, vlan, various tunnel implementations. I think
> mac80211 fits well with those kinds of use cases.

Ok. As said I was not sure so I can/will not argue.

> If I remember correctly, the deprecation was added to avoid quirky
> custom locking schemes in ethernet drivers.

What do you mean by "quirky custom locking schemes"? You mean that TX path 
would use driver data that should actually be accessed under some lock?

When seeing the deprecated comment I wanted to know the why and was hoping 
the commit message would divulge. It just mentions it is not needed. So now 
I am curious as to why it wouldn't be needed especially as you say there 
are (valid) use-cases in the kernel today.

Regards,
Arend