On Mon, Dec 21, 2020 at 7:46 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote: > > On Sat, Dec 19, 2020 at 5:55 PM Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote: > > > > On 12/19/20 7:18 AM, Johannes Berg wrote: > > > On Fri, 2020-12-18 at 12:16 -0800, Jakub Kicinski wrote: > > >> On Thu, 17 Dec 2020 12:40:26 -0800 Ben Greear wrote: > > >>> On 12/17/20 10:20 AM, Eric Dumazet wrote: > > >>>> On Thu, Dec 17, 2020 at 7:13 PM Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote: > > >>>>> It is the iwlwifi/mvm logic that supports ax200. > > >>>> > > >>>> Let me ask again : > > >>>> > > >>>> I see two different potential call points : > > >>>> > > >>>> drivers/net/wireless/intel/iwlwifi/pcie/tx.c:1529: > > >>>> tso_build_hdr(skb, hdr_page->pos, &tso, data_left, !total_len); > > >>>> drivers/net/wireless/intel/iwlwifi/queue/tx.c:427: > > >>>> tso_build_hdr(skb, hdr_page->pos, &tso, data_left, !total_len); > > >>>> > > >>>> To the best of your knowledge, which one would be used in your case ? > > >>>> > > >>>> Both are horribly complex, I do not want to spend time studying two > > >>>> implementations. > > >>> > > >>> It is the queue/tx.c code that executes on my system, verified with > > >>> printk. > > >> > > >> Not sure why Intel's not on CC here. > > > > > > Heh :) > > > > > > Let's also add linux-wireless. > > > > > >> Luca, is the ax200 TSO performance regression with recent kernel on your > > >> radar? > > > > > > It wasn't on mine for sure, so far. But it's supposed to be Christmas > > > vacation, so haven't checked our bug tracker etc. I see Emmanuel was at > > > least looking at the bug report, but not sure what else happened yet. > > > > Not to bitch and moan too much, but even the most basic of testing would > > have shown this, how can testing be so poor on the ax200 driver? > > > > It even shows up with the out-of-tree ax200 driver. > > > > > Off the top of my head, I don't really see the issue. Does anyone have > > > the ability to capture the frames over the air (e.g. with another AX200 > > > in monitor mode, load the driver with amsdu_size=3 module parameter to > > > properly capture A-MSDUs)? > > > > I can do that at some point, and likely it could be reproduced with an /n or /ac > > AP and those are a lot easier to sniff. > > > > Thanks, > > Ben > > > > > > -- > > Ben Greear <greearb@xxxxxxxxxxxxxxx> > > Candela Technologies Inc http://www.candelatech.com > > It seems the problem comes from some skbs reaching the driver with > gso_type == 0, > meaning skb_is_gso_tcp() is fuzzy. (net/core/tso.c is only one of the > skb_is_gso_tcp() users) > > Local TCP stack should provide either SKB_GSO_TCPV4 or SKB_GSO_TCPV6 > for GSO packets. > > So maybe the issue is coming from traffic coming from a VM through a > tun device or something, > and our handling of GSO_ROBUST / DODGY never cared about setting > SKB_GSO_TCPV4 or SKB_GSO_TCPV6 if not already given by user space ? > > Or a plain bug somewhere, possibly overwriting gso_type with 0 or garbage... Oh well, iwl_mvm_tx_tso_segment() 'builds' a fake gso packet. I suspect this will fix the issue : diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c index a983c215df310776ffe67f3b3ffa203eab609bfc..e7ad6367c88de4aff700c630d850760d1d3bf011 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c @@ -773,6 +773,7 @@ iwl_mvm_tx_tso_segment(struct sk_buff *skb, unsigned int num_subframes, next = skb_gso_segment(skb, netdev_flags); skb_shinfo(skb)->gso_size = mss; + skb_shinfo(skb)->gso_type = ipv4 ? SKB_GSO_TCPV4 : SKB_GSO_TCPV6; if (WARN_ON_ONCE(IS_ERR(next))) return -EINVAL; else if (next)
