netdev - Re: net: tso: add UDP segmentation support: adds regression for ax200 upload

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:   Thu, 17 Dec 2020 10:13:23 -0800
From:   Ben Greear <greearb@...delatech.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     netdev <netdev@...r.kernel.org>
Subject: Re: net: tso: add UDP segmentation support: adds regression for ax200
 upload


On 12/17/2020 10:07 AM, Eric Dumazet wrote:
> On Thu, Dec 17, 2020 at 6:56 PM Ben Greear <greearb@...delatech.com> wrote:
>> On 12/17/20 2:11 AM, Eric Dumazet wrote:
>>> On Thu, Dec 17, 2020 at 12:59 AM Ben Greear <greearb@...delatech.com> wrote:
>>>> On 12/16/20 3:09 PM, Ben Greear wrote:
>>>>> Hello Eric,
>>>>>
>>>>> The patch below evidently causes TCP throughput to be about 50Mbps instead of 700Mbps
>>>>> when using ax200 to upload tcp traffic.
>>>>>
>>>>> When I disable TSO, performance goes back up to around 700Mbps.
>>>> As a followup, when I revert the patch, upload speed goes to ~900Mbps,
>>>> so even better than just disabling TSO (I left TSO enabled after reverting the patch).
>>>>
>>>> Thanks,
>>>> Ben
>>>>
>>> Thanks for the report !
>>>
>>> It seems drivers/net/wireless/intel/iwlwifi/pcie/tx.c:iwl_fill_data_tbs_amsdu()
>>> calls tso_build_hdr() with extra bytes (SNAP header),
>>> it is not yet clear to me what is broken :/
>> Your patch is guessing tcp vs udp by looking at header length
>> from what I could tell.  So if something uses a different size,
>> it probably gets confused?
> I do not think so, my patch selects TCP vs UDP by using standard GSO
> helper skb_is_gso_tcp(skb)
>
> tso->tlen is initialized from tso_start() :
>
> int tlen = skb_is_gso_tcp(skb) ? tcp_hdrlen(skb) : sizeof(struct udphdr);
>
> tso->tlen = tlen;
>
> Maybe for some reason skb_is_gso_tcp(skb) returns false in your case,
> some debugging would help.
>
>>> Can you confirm which driver is used for ax200 ?
>>>
>>> I see tso_build_hdr() also being used from
>>> drivers/net/wireless/intel/iwlwifi/queue/tx.c
>> I tested against the un-modified ax200 5.10.0 kernel driver, and it has the issue.
>>
>> The ax200 backports release/core56 driver acts a bit different (poorer performance over all than
>> in-kernel driver), but has similar upstream issues that are mitigated by
>> disabling TSO.
> Sorry, I can not find ax200 driver.

It is the iwlwifi/mvm logic that supports ax200.

Thanks,

Ben