From patchwork Mon Dec 14 12:42:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emmanuel Grumbach X-Patchwork-Id: 7844131 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CCEBEBEEE1 for ; Mon, 14 Dec 2015 12:43:03 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 163F9202F8 for ; Mon, 14 Dec 2015 12:42:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C8085202A1 for ; Mon, 14 Dec 2015 12:42:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752832AbbLNMmy (ORCPT ); Mon, 14 Dec 2015 07:42:54 -0500 Received: from mail-lf0-f45.google.com ([209.85.215.45]:36597 "EHLO mail-lf0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752825AbbLNMmw (ORCPT ); Mon, 14 Dec 2015 07:42:52 -0500 Received: by lfed137 with SMTP id d137so68325422lfe.3 for ; Mon, 14 Dec 2015 04:42:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=T+FIuncmpg8DSqhxIsZlY4YGAoQPy5Z0jzhjIwYwxtg=; b=vNYYtuq1WZUeaSDTNLJwF8yxoTXTUOkRFFyUIiyLsExXrZdfTQH6mlNJdXUzZV2Bsh b/VjZ4F++5tNaxKujCFLimSFqrgJdv6ZN2fqJ440BbHcOtG/wKcBw/5YG/EmMNjR0Dvv PgcpyPS3b5XdpMO6EOVFJk3dVJh5MbK2pbUGCaDGDIn9kEb6naRMbN2s7kHfsD2y+Ket 9Z9cs/xx5ehTN+RJNLCLd+H7UIq+Vd2sBTMu8Xc86BsrMg44cXbDJceIwMQWPHiCqYj3 x5V13f997nLHK/fbOdyp+f0tUVVlv52hO/psPnf4AyUFWhZFM+lTdSO4wOVKu3ctM2FP 0EQQ== MIME-Version: 1.0 X-Received: by 10.25.141.70 with SMTP id p67mr13205452lfd.151.1450096970921; Mon, 14 Dec 2015 04:42:50 -0800 (PST) Received: by 10.114.77.231 with HTTP; Mon, 14 Dec 2015 04:42:50 -0800 (PST) In-Reply-To: References: <20151203220522.GB11138@jessup.stsp.name> <20151211200205.GV25635@ted.stsp.name> Date: Mon, 14 Dec 2015 14:42:50 +0200 Message-ID: Subject: Re: iwlwifi A-MSDU tx From: Emmanuel Grumbach To: Stefan Sperling , Emmanuel Grumbach Cc: linux-wireless Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, T_TVD_MIME_EPI,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Dec 12, 2015 at 10:54 PM, Emmanuel Grumbach wrote: > On Fri, Dec 11, 2015 at 10:02 PM, Stefan Sperling wrote: >> Hi Emmanuel, >> >> Thanks a bunch, this worked out quite well for me. >> I got what I was aiming for out of this already but I still have some >> additional questions if you don't mind. > > Happy to hear that. > >> >> Do you know of a good way to generate A-MSDUs? >> One way I found is to run: >> top -d .01 >> on the AP and watching this top over SSH from my associated OpenBSD client. > > iperf? :) > > You can also load iwlwifi with 11n_disable=2. This will prevent A-MPDU > if you don't want to have A-MSDU in A-MPDU at that stage. > >> >> It seems only locally generated packets will trigger A-MSDUs. >> I suppose forwarded frames always fit the interface MTU and don't >> trigger A-MSDUs. Is there a way to test this with forwarded traffic? > > Hmm. Good question. A-MSDUs are created each time we get a large send > from the network stack. I think that this relies on the socket > occupancy which means that you are right. I don't think we can test > this with forwarded traffic for now. > >> >> One issue I discovered is that OpenBSD (with my partly uncommitted 11n >> patch set) cannot decrypt encrypted A-MSDUs sent by iwlwifi. >> Frames carrying normal MSDUs have a CCMP header and are decrypted correctly. >> But frames carrying A-MSDUs have a wep/tkip header (wireshark shows >> "WEP parameters" instead of CCMP parameters") and decryption fails on >> OpenBSD in ieee80211_ccmp_decrypt() because the ExtIV bit is not set. >> It doesn't look like the source of this problem is at OpenBSD's end since >> CCMP is required for HT. So in case the firmware doesn't produce CCMP for >> A-MSDUs, this would be a problem in the firmware. Can you confirm this? >> I also tried running iwlwifi in software crypto mode but the AP would >> not send A-MSDUs at all in that case. > > SW crypto won't work for A-MSDU since it'll refuse LSO which in turn > disables A-MSDU. > I don't really touch the wifi header, I just duplicate it if needed, > so I don't see off the top of my head where the problem could be. > Thanks for reporting it. > I found the problem. Patch attached. >> >> On the receiving OpenBSD side I'm currently limited to 4k A-MSDU and it >> turns out the AP won't send more than 2 subframes per A-MSDU in this case. > > with default MTU, you won't get more than that. > >> To test with more than 2 subframes I applied the following crude patch which >> makes the AP send more subframes but crashes the firmware after the frame >> is sent. The driver recovers fine and traffic keeps flowing but that's >> not pretty. Do you have a better idea? If not, no problem. I mainly did >> this to confirm that OpenBSD won't crash handling A-MSDU with more than >> 2 subframes, which it won't. > > ouch. No :) What you should really do is to reduce the MTU to 500 bytes. > iperf -M will do. > >> >> Thanks again, >> Stefan >> >> diff --git a/drivers/net/wireless/iwlwifi/mvm/tx.c b/drivers/net/wireless/iwlwifi/mvm/tx.c >> index 7ece5c1..b3d562d 100644 >> --- a/drivers/net/wireless/iwlwifi/mvm/tx.c >> +++ b/drivers/net/wireless/iwlwifi/mvm/tx.c >> @@ -448,7 +448,7 @@ static int iwl_mvm_tx_tso(struct iwl_mvm *mvm, struct sk_buff *skb, >> struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta); >> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); >> struct ieee80211_hdr *hdr = (void *)skb->data; >> - unsigned int mss = skb_shinfo(skb)->gso_size; >> + unsigned int mss = skb_shinfo(skb)->gso_size / 2; >> struct sk_buff *tmp, *next; >> char cb[sizeof(skb->cb)]; >> unsigned int num_subframes, tcp_payload_len, subf_len, max_amsdu_len; >> >> On Fri, Dec 04, 2015 at 12:30:52PM +0200, Emmanuel Grumbach wrote: >>> Adding the right mailing list this time. >>> >>> On Fri, Dec 4, 2015 at 10:18 AM, Emmanuel Grumbach wrote: >>> > On Fri, Dec 4, 2015 at 9:35 AM, Emmanuel Grumbach wrote: >>> >> Hi, >>> >> >>> >> On Fri, Dec 4, 2015 at 12:05 AM, Stefan Sperling wrote: >>> >>> Hi Emmanuel, >>> >>> >>> >>> As part of implementing 802.11n support in OpenBSD I'm looking for >>> >>> an AP which sends A-MSDUs. Preferrably under software control rather >>> >>> than firmware. >>> >>> >>> >>> I found your iwlwifi A-MSDU patches at >>> >>> http://marc.info/?l=linux-wireless&m=144553311122495&w=2 >>> >>> and http://marc.info/?l=linux-wireless&m=144553311822496&w=2 >>> >>> >>> >>> Would these patches make it possible to run an AP with an 5300 or 7260 >>> >>> card and send A-MSDUs? That would be great as I have the necessary hardware. >>> >> >>> >> 7260 will go. Not 5300. >>> >> Note that this code simulates Tx TCP Csum offload. I did that to write >>> >> the code while we still don't have the hardware that has this >>> >> capability. But for testing purposes, it should do the work. The >>> >> testing teams have reported that AP mode didn't work quite well with >>> >> this and I haven't taken the time to look into yet, but if you see >>> >> issues, please report them or even better: fix them :) >>> >> >>> >>> >>> >>> Which Linux tree do these patches apply to? I've tried the following >>> >>> but had no luck in getting these patches applied without rejects: >>> >>> >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-next.git >>> >>> >>> >> >>> >> Right. These patches had a long internal review cycle. They are now >>> >> merge in our internal tree. >>> >> You can find them in our internal tree [1] (backport based). I will >>> >> push them soon in iwlwifi-next.git. >>> > >>> > I forgot to mention that you need to change the define of >>> > IWL_MVM_SW_TX_CSUM_OFFLOAD to 1. >>> > >>> >> >>> >> [1] - https://git.kernel.org/cgit/linux/kernel/git/iwlwifi/backport-iwlwifi.git/ >>> >> Please read https://wireless.wiki.kernel.org/en/users/drivers/iwlwifi/core_release#how_to_install_the_driver. >>> >> >>> >>> Thanks, >>> >>> Stefan From 9ef0db4a69297bcc001c8d2588801bc489834b59 Mon Sep 17 00:00:00 2001 From: Emmanuel Grumbach Date: Mon, 14 Dec 2015 14:31:59 +0200 Subject: [PATCH] [BUGFIX] iwlwifi: pcie: correctly handle the IV in A-MSDU [SQUASH] The IV needs to be copied before it is pulled from the skb. This prevented TSO to work properly with iv_len != 0. type=bugfix bug=not-tracked fixes=I9d02be5b8349120e73d8997d91182aec2f42290f Change-Id: Ieab88fdc18cd90066ff571ea2bd11ba79ea6f040 Signed-off-by: Emmanuel Grumbach --- drivers/net/wireless/iwlwifi/pcie/tx.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/pcie/tx.c b/drivers/net/wireless/iwlwifi/pcie/tx.c index fd36eea..14d5ffd 100644 --- a/drivers/net/wireless/iwlwifi/pcie/tx.c +++ b/drivers/net/wireless/iwlwifi/pcie/tx.c @@ -1977,15 +1977,10 @@ static int iwl_fill_data_tbs_amsdu(struct iwl_trans *trans, struct sk_buff *skb, &dev_cmd->hdr, IWL_HCMD_SCRATCHBUF_SIZE + tb1_len, NULL, 0); - /* - * Pull the ieee80211 header + IV to be able to use TSO core, - * we will restore it for the tx_status flow. - */ - skb_pull(skb, hdr_len + iv_len); ip_hdrlen = skb_transport_header(skb) - skb_network_header(skb); snap_ip_tcp_hdrlen = IEEE80211_CCMP_HDR_LEN + ip_hdrlen + tcp_hdrlen(skb); - total_len = skb->len - snap_ip_tcp_hdrlen; + total_len = skb->len - snap_ip_tcp_hdrlen - hdr_len - iv_len; amsdu_pad = 0; /* total amount of header we may need for this A-MSDU */ @@ -2000,9 +1995,15 @@ static int iwl_fill_data_tbs_amsdu(struct iwl_trans *trans, struct sk_buff *skb, get_page(hdr_page->page); start_hdr = hdr_page->pos; info->driver_data[IWL_TRANS_FIRST_DRIVER_DATA] = hdr_page->page; - memcpy(hdr_page->pos, skb->data, iv_len); + memcpy(hdr_page->pos, skb->data + hdr_len, iv_len); hdr_page->pos += iv_len; + /* + * Pull the ieee80211 header + IV to be able to use TSO core, + * we will restore it for the tx_status flow. + */ + skb_pull(skb, hdr_len + iv_len); + tso_start(skb, &tso); while (total_len) { -- 2.5.0