From owner-svn-src-all@freebsd.org Wed May 27 18:32:13 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C0FEC2CB4C2; Wed, 27 May 2020 18:32:13 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49XKDs4gqRz3d52; Wed, 27 May 2020 18:32:13 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 818EB18FE2; Wed, 27 May 2020 18:32:13 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 04RIWDZ5065296; Wed, 27 May 2020 18:32:13 GMT (envelope-from adrian@FreeBSD.org) Received: (from adrian@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 04RIWDpH065294; Wed, 27 May 2020 18:32:13 GMT (envelope-from adrian@FreeBSD.org) Message-Id: <202005271832.04RIWDpH065294@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: adrian set sender to adrian@FreeBSD.org using -f From: Adrian Chadd Date: Wed, 27 May 2020 18:32:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r361560 - head/sys/net80211 X-SVN-Group: head X-SVN-Commit-Author: adrian X-SVN-Commit-Paths: head/sys/net80211 X-SVN-Commit-Revision: 361560 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 May 2020 18:32:13 -0000 Author: adrian Date: Wed May 27 18:32:12 2020 New Revision: 361560 URL: https://svnweb.freebsd.org/changeset/base/361560 Log: [net80211] Fix interrupted scan logic and ticks comparison The scan task refactoring stuff circa 2014-2016 broke the blocking task into a taskqueue with some async bits, but it apparently broke scans being interrupted by traffic. Notably - the new "field" SCAN_PAUSE sets both SCAN_INTERRUPT and SCAN_CANCEL, and a bunch of existing code was checking for SCAN_CANCEL only and breaking the scan. Unfortunately it was then (a) cancelling the scan entirely and (b) not notifying userland that scan was done. So: * Update the calls to scan_end() to only pass in 1 (saying the scan is complete) if SCAN_CANCEL is set WITHOUT SCAN_INTERRUPT. If both are set then yes, the scan is interrupted, but it isn't canceled - it's just paused. * Update the "did the scan flags change whilst the driver was called" logic to check for canceled scans, not interrupted scans. * The "scan done" logic now explicitly checks for either interrupted or completed scans. This accounts for the situation where a scan is being aborted via traffic but it ALSO happens to have finished (ie the last channel was checked.) This doesn't ENTIRELY fix scanning as the resume function is broken due to incorrect ticks math. Thus, the second half of this patch changes the ieee80211_ticks_*() macros to use int instead of long, matching the logic that the TCP code does with ticks and handles wrapping / negative ticks values. If cast to long then the wrapping math wouldn't work right (ie, if ticks was actually negative, ie, after the system has been up for a while.) This allows contbgscan() to correctly calculate if a scan should continue based on ticks and ic->ic_lastdata . Reviewed by: bz Differential Revision: https://reviews.freebsd.org/D25031 Modified: head/sys/net80211/ieee80211_freebsd.h head/sys/net80211/ieee80211_scan_sw.c Modified: head/sys/net80211/ieee80211_freebsd.h ============================================================================== --- head/sys/net80211/ieee80211_freebsd.h Wed May 27 18:26:10 2020 (r361559) +++ head/sys/net80211/ieee80211_freebsd.h Wed May 27 18:32:12 2020 (r361560) @@ -254,9 +254,9 @@ void ieee80211_vap_destroy(struct ieee80211vap *); #define ticks_to_msecs(t) TICKS_2_MSEC(t) #define ticks_to_secs(t) ((t) / hz) -#define ieee80211_time_after(a,b) ((long)(b) - (long)(a) < 0) +#define ieee80211_time_after(a,b) ((int)(b) - (int)(a) < 0) #define ieee80211_time_before(a,b) ieee80211_time_after(b,a) -#define ieee80211_time_after_eq(a,b) ((long)(a) - (long)(b) >= 0) +#define ieee80211_time_after_eq(a,b) ((int)(a) - (int)(b) >= 0) #define ieee80211_time_before_eq(a,b) ieee80211_time_after_eq(b,a) struct mbuf *ieee80211_getmgtframe(uint8_t **frm, int headroom, int pktlen); Modified: head/sys/net80211/ieee80211_scan_sw.c ============================================================================== --- head/sys/net80211/ieee80211_scan_sw.c Wed May 27 18:26:10 2020 (r361559) +++ head/sys/net80211/ieee80211_scan_sw.c Wed May 27 18:32:12 2020 (r361560) @@ -675,19 +675,32 @@ scan_curchan_task(void *arg, int pending) struct ieee80211com *ic = ss->ss_ic; struct ieee80211_channel *chan; unsigned long maxdwell; - int scandone; + int scandone, scanstop; IEEE80211_LOCK(ic); end: + /* + * Note: only /end/ the scan if we're CANCEL rather than + * CANCEL+INTERRUPT (ie, 'PAUSE'). + * + * We can stop the scan if we hit cancel, but we shouldn't + * call scan_end(ss, 1) if we're just PAUSEing the scan. + */ scandone = (ss->ss_next >= ss->ss_last) || - (ss_priv->ss_iflags & ISCAN_CANCEL) != 0; + ((ss_priv->ss_iflags & ISCAN_PAUSE) == ISCAN_CANCEL); + scanstop = (ss->ss_next >= ss->ss_last) || + ((ss_priv->ss_iflags & ISCAN_CANCEL) != 0); IEEE80211_DPRINTF(ss->ss_vap, IEEE80211_MSG_SCAN, - "%s: loop start; scandone=%d\n", + "%s: loop start; scandone=%d, scanstop=%d, ss_iflags=0x%x, ss_next=%u, ss_last=%u\n", __func__, - scandone); + scandone, + scanstop, + (uint32_t) ss_priv->ss_iflags, + (uint32_t) ss->ss_next, + (uint32_t) ss->ss_last); - if (scandone || (ss->ss_flags & IEEE80211_SCAN_GOTPICK) || + if (scanstop || (ss->ss_flags & IEEE80211_SCAN_GOTPICK) || (ss_priv->ss_iflags & ISCAN_ABORT) || ieee80211_time_after(ticks + ss->ss_mindwell, ss_priv->ss_scanend)) { ss_priv->ss_iflags &= ~ISCAN_RUNNING; @@ -787,11 +800,12 @@ scan_end(struct ieee80211_scan_state *ss, int scandone * Since a cancellation may have occurred during one of the * driver calls (whilst unlocked), update scandone. */ - if (scandone == 0 && (ss_priv->ss_iflags & ISCAN_CANCEL) != 0) { + if ((scandone == 0) && ((ss_priv->ss_iflags & ISCAN_PAUSE) == ISCAN_CANCEL)) { /* XXX printf? */ if_printf(vap->iv_ifp, - "%s: OOPS! scan cancelled during driver call (1)!\n", - __func__); + "%s: OOPS! scan cancelled during driver call (1) (ss_iflags=0x%x)!\n", + __func__, + ss_priv->ss_iflags); scandone = 1; } @@ -856,11 +870,12 @@ scan_end(struct ieee80211_scan_state *ss, int scandone * Since a cancellation may have occurred during one of the * driver calls (whilst unlocked), update scandone. */ - if (scandone == 0 && (ss_priv->ss_iflags & ISCAN_CANCEL) != 0) { + if (scandone == 0 && (ss_priv->ss_iflags & ISCAN_PAUSE) == ISCAN_CANCEL) { /* XXX printf? */ if_printf(vap->iv_ifp, - "%s: OOPS! scan cancelled during driver call (2)!\n", - __func__); + "%s: OOPS! scan cancelled during driver call (2) (ss_iflags=0x%x)!\n", + __func__, + ss_priv->ss_iflags); scandone = 1; } @@ -900,11 +915,18 @@ scan_done(struct ieee80211_scan_state *ss, int scandon */ if ((vap->iv_flags_ext & IEEE80211_FEXT_SCAN_OFFLOAD) == 0) vap->iv_sta_ps(vap, 0); - if (ss->ss_next >= ss->ss_last) + if (ss->ss_next >= ss->ss_last) { + IEEE80211_DPRINTF(vap, IEEE80211_MSG_SCAN, + "%s: Dropping out of scan; ss_next=%u, ss_last=%u\n", + __func__, + (uint32_t) ss->ss_next, + (uint32_t) ss->ss_last); ic->ic_flags_ext &= ~IEEE80211_FEXT_BGSCAN; + } /* send 'scan done' event if not interrupted due to traffic. */ - if (!(ss_priv->ss_iflags & ISCAN_INTERRUPT)) + if (!(ss_priv->ss_iflags & ISCAN_INTERRUPT) || + (ss->ss_next >= ss->ss_last)) ieee80211_notify_scan_done(vap); } ss_priv->ss_iflags &= ~(ISCAN_PAUSE | ISCAN_ABORT);