Subject: Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx
 queues
From:   Arend Van Spriel <arend.vanspriel@broadcom.com>
To:     linux-wireless@vger.kernel.org, johannes@sipsolutions.net,
        Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Felix Fietkau <nbd@nbd.name>,
        =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@redhat.com>
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>
 <16a1bd77c20.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
Message-ID: <2e440cc1-d085-ff35-819c-d639cda3d894@broadcom.com>
Date:   Tue, 16 Apr 2019 09:34:01 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <16a1bd77c20.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 4/14/2019 2:34 PM, Arend Van Spriel wrote:
> 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.

Getting back to this in an attempt to clarify my question. So from what 
Felix is saying the NETIF_F_LLTX flag is not deprecated, but restricted. 
What I would like to know is what exactly is required from a driver to 
allow the use of this flag.

Regards,
Arend