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]

Message-ID: <CANn89iJQnSVZFp2XDgREN1QMtU4exOsnJq=5VzJ6tqTCJ7MH-g@mail.gmail.com>
Date:   Thu, 17 Dec 2020 19:07:56 +0100
From:   Eric Dumazet <edumazet@...gle.com>
To:     Ben Greear <greearb@...delatech.com>
Cc:     netdev <netdev@...r.kernel.org>
Subject: Re: net: tso: add UDP segmentation support: adds regression for ax200 upload

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.

>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@...delatech.com>
> Candela Technologies Inc  http://www.candelatech.com