From patchwork Mon Jun 15 19:45:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Greear X-Patchwork-Id: 6611981 Return-Path: X-Original-To: patchwork-ath10k@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B3F929F326 for ; Mon, 15 Jun 2015 19:46:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B226B205F5 for ; Mon, 15 Jun 2015 19:45:59 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AD4A1205E2 for ; Mon, 15 Jun 2015 19:45:58 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Z4aKK-000291-AQ; Mon, 15 Jun 2015 19:45:48 +0000 Received: from merlin.infradead.org ([2001:4978:20e::2]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Z4aKI-00028b-PK for ath10k@bombadil.infradead.org; Mon, 15 Jun 2015 19:45:46 +0000 Received: from mail2.candelatech.com ([208.74.158.173]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Z4aKG-0003dr-H2 for ath10k@lists.infradead.org; Mon, 15 Jun 2015 19:45:45 +0000 Received: from [192.168.100.236] (unknown [50.251.239.81]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mail2.candelatech.com (Postfix) with ESMTPSA id AB00640B4B8; Mon, 15 Jun 2015 12:45:07 -0700 (PDT) Message-ID: <557F2B43.8060409@candelatech.com> Date: Mon, 15 Jun 2015 12:45:07 -0700 From: Ben Greear Organization: Candela Technologies User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Michal Kazior Subject: Re: Question on beacon-miss offloading. References: <5579F87F.4070601@candelatech.com> <557A1BE9.7070305@candelatech.com> <557AFC9D.3020109@candelatech.com> <557EF0B0.7070008@candelatech.com> In-Reply-To: <557EF0B0.7070008@candelatech.com> X-Enigmail-Version: 1.5.2 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150615_154544_712915_2ACD6E22 X-CRM114-Status: GOOD ( 29.27 ) X-Spam-Score: -2.4 (--) Cc: "linux-wireless@vger.kernel.org" , ath10k X-BeenThere: ath10k@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "ath10k" Errors-To: ath10k-bounces+patchwork-ath10k=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 06/15/2015 08:35 AM, Ben Greear wrote: > On 06/14/2015 10:36 PM, Michal Kazior wrote: >> On 12 June 2015 at 17:37, Ben Greear wrote: >>> On 06/11/2015 11:03 PM, Michal Kazior wrote: >>>> On 12 June 2015 at 01:38, Ben Greear wrote: >>>>> On 06/11/2015 02:07 PM, Ben Greear wrote: >>>>>> In my ath10k CT firmware, I am disabling the beacon-miss offloading >>>>>> to save space and because it will not work with lots of virtual >>>>>> stations. >>>>>> >>>>>> But, it must be that I need some way to tell the stack that this >>>>>> feature is not enabled, because when suddenly kill my AP, then >>>>>> the ath10k station connected to it shows endless 'beacon loss' events >>>>>> in 'iw events' output, but it never actually loses connection. >>>>>> >>>>>> Stock firmware works fine, so probably I just need to disable >>>>>> some feature flag when registering the ath10k hardware >>>>>> when using CT firmware. >>>>>> >>>>>> With stock firmware, I see a quick dissassociation due to inactivity. >>>>>> >>>>>> I am having poor luck finding how a driver tells the stack >>>>>> it has beacon miss offload or not, so, does anyone know how >>>>>> this is controlled? >>>>> >>>>> I still am not sure why stock firmware works, but it appears >>>>> the reason mine is failing is that the ACK status for mgt frames >>>>> is always set to TRUE since the ath10k wmi-mgt-tx API is so >>>>> lame. So, mac80211 does a probe, ath10k lies and says it was >>>>> acked, and mac80211 then things all is well for another few >>>>> seconds. >>>> >>>> mac80211 shouldn't do a Probe Req to an AP on beacon loss because >>>> ath10k advertises it supports tx-status report. Hence mac80211 should >>>> use NullFunc frames which shouldn't go through wmi-mgmt-tx but htt >>>> tx-frm. >>>> >>>> But then again: NullFunc status reporting via htt tx-frm was broken on >>>> 10.1 if memory serves right. I believe it was fixed in 10.2 or 10.2.4. >>>> >>>> This problem has been effectively obscured on stock 10.1 by the >>>> offloaded beacon miss. >>> >>> Thanks for the hint. I was able to fix my firmware to properly >>> return htt tx status, and now it appears to work properly. >>> >>> A quick throughput test works as well, so hopefully no regressions. >>> >>> I guess the NulFunc related comment is incorrect for 10.1 stock firmware? >>> >>> Maybe some others? >>> >>> static void ath10k_tx_htt(struct ath10k *ar, struct sk_buff *skb) >>> { >>> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; >>> int ret = 0; >>> >>> if (ar->htt.target_version_major >= 3) { >>> /* Since HTT 3.0 there is no separate mgmt tx command */ >>> ret = ath10k_htt_tx(&ar->htt, skb); >>> goto exit; >>> } >>> >>> if (ieee80211_is_mgmt(hdr->frame_control)) { >>> if (test_bit(ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX, >>> ar->fw_features)) { >>> if (skb_queue_len(&ar->wmi_mgmt_tx_queue) >= >>> ATH10K_MAX_NUM_MGMT_PENDING) { >>> ath10k_warn(ar, "reached WMI management transmit queue limit\n"); >>> ret = -EBUSY; >>> goto exit; >>> } >>> >>> skb_queue_tail(&ar->wmi_mgmt_tx_queue, skb); >>> ieee80211_queue_work(ar->hw, &ar->wmi_mgmt_tx_work); >>> } else { >>> ret = ath10k_htt_mgmt_tx(&ar->htt, skb); >>> } >>> } else if (!test_bit(ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX, >>> ar->fw_features) && >>> ieee80211_is_nullfunc(hdr->frame_control)) { >>> /* FW does not report tx status properly for NullFunc frames >>> * unless they are sent through mgmt tx path. mac80211 sends >>> * those frames when it detects link/beacon loss and depends >>> * on the tx status to be correct. */ >>> ret = ath10k_htt_mgmt_tx(&ar->htt, skb); >>> } else { >>> ret = ath10k_htt_tx(&ar->htt, skb); >>> } >> >> The NullFunc workaround was originally done for 999.999.0.636 but >> should be true for 10.1 as well with the sole exception the latter >> doesn't have htt-mgmt-tx to workaround the problem. > > Is it correct to say that this logic is completely broken for stock 10.1 firmware > because null-func frames are not management frames, so the packet goes out the > ath10k_htt_mgmt_tx call, and stock 10.1 does not properly do tx status > for htt-tx frames? > > > And slightly different question: Once I put in proper htt ACK reporting into > my firmware, I notice that stations often loose connectivity when they are > idle. I captured a sniff, and it appears the null-func packets are sent, > but the header is not requesting an explicit ACK, and the AP does not ACK it, > so connection is lost. > > I guess that the null-func frames should be requesting explicit ACK? This patch below (with firmware fix to make it actually pay attention to this special TID in station mode), fixes the null-func to be non-qos frames and get immediate ack. This in turn seems to fix the disconnect issue I was facing, though I need to test some more to be sure. This seem reasonable, maybe even for upstream? [greearb@ben-dt2 ath10k]$ git diff Thanks, Ben diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 6ca48e3..610447e 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -2213,6 +2213,9 @@ static u8 ath10k_tx_h_get_tid(struct ieee80211_hdr *hdr) if (ieee80211_is_mgmt(hdr->frame_control)) return HTT_DATA_TX_EXT_TID_MGMT; + if (ieee80211_is_nullfunc(hdr->frame_control)) + return HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST; + if (!ieee80211_is_data_qos(hdr->frame_control)) return HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST;