linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Chris Chiu <chiu@endlessm.com>, linux-wireless@vger.kernel.org
Cc: linux@endlessm.com
Subject: Re: [PATCH] mac80211: fix Tx BA session stuck issue during sw scanning
Date: Tue, 29 Nov 2016 13:59:29 +0100	[thread overview]
Message-ID: <1480424369.10012.6.camel@sipsolutions.net> (raw)
In-Reply-To: <1479887968-17473-1-git-send-email-chiu@endlessm.com> (sfid-20161123_085941_969147_2A29A624)

On Wed, 2016-11-23 at 15:59 +0800, Chris Chiu wrote:
> ieee80211_iface_work() will check if sw scanning is in progress
> before handling block ack session. In our case, the RTL8821AE
> operate in station mode, when tx session expired, DELBA packet
> stuck during sw scanning and so do other data packets.
> 
> ieee80211_scan_state_decision() will take lots of time in
> SCAN_SUSPEND/SCAN_RESUME state due to !tx_empty or bad_latency.
> Then the sw scanning mostly take > 20 seconds to finish or even
> worse in our case RTL8821AE have 37 channels for 2G+5G to scan
> and tx stalls ~300 seconds. AP side still thinks the connection
> is alive because it still receives the QoS NULL packet from STA.
> So the link state will never change but actually no data tx/rx
> during this long time.
> 
> This commit tries to send out packet in SCAN_SUSPEND state so the
> sw scanning can complete more efficiently and less affect on Block
> Ack session handling. Verified on RTL8821AE for > 30000 pings and
> no Tx BA session stuck observed.

The premise seems fairly reasonable, although I'm a little worried that
if so much new traffic is coming in we never finish the scan suspend?
Actually, the queues are still stopped, so it's only management frames
that can come in, so that should be ok?

> +	test_and_clear_bit(SCAN_SUSPEND_SCANNING, &local->scanning);
> 

That makes no sense, you're not checking the return value, just clear
the bit without test.
 
> @@ -844,6 +846,8 @@ static void ieee80211_scan_state_suspend(struct
> ieee80211_local *local,
>  	/* disable PS */
>  	ieee80211_offchannel_return(local);
>  
> +	__set_bit(SCAN_SUSPEND_SCANNING, &local->scanning);

Why are you using the non-atomic version here, vs. the atomic one
above?

johannes

  reply	other threads:[~2016-11-29 12:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-23  7:59 [PATCH] mac80211: fix Tx BA session stuck issue during sw scanning Chris Chiu
2016-11-29 12:59 ` Johannes Berg [this message]
2016-11-29 13:38   ` Arend Van Spriel
2016-12-01 11:24   ` Chris Chiu
2016-12-05 14:57     ` Johannes Berg
2016-12-05 16:56       ` Chris Chiu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1480424369.10012.6.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=chiu@endlessm.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@endlessm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).