On Fri, Apr 25, 2025 at 05:22:10PM +0200, Stefan Sperling wrote:
> On Fri, Apr 25, 2025 at 12:23:25AM +0200, Mark Kettenis wrote:
> > Sadly this doesn't fix the panic I'm seeing on the x13s.
> > 
> > What I'm doing to provoke the panic is:
> > 
> > 1. Log in to the machine over ssh
> > 
> > 2. Run 'ifconfig qwx0 down'
> > 
> > 3. Pick up the laptop
> > 
> > 4. Run 'ifconfig qwx0 up'
> > 
> > There seems to be something really fishy going on with bringing the
> > interface down when there are packets in flight...
> 
> Thanks Mark! I could reproduce the crash on Z13 by following your recipe.
> 
> This diff, applied on top of the previous diff, seems to fix it for me.
> Can you confirm?

Here is a slightly improved version, which does not leave us hanging
in a non-INIT state in case the task code runs into some error case.

If a failure occurs, all we can do is what we did previously, forcefully
moving to INIT state in the kernel and hoping for the best.
And we must cancel the init_task which is scheduled in case of failure.

I've not seen any such errors occur during testing, but they are not
an impossible scenario.


M  sys/dev/ic/qwx.c  |  23+  3-

1 file changed, 23 insertions(+), 3 deletions(-)

commit - 733c622d5b43cf595e1a2ddfe07326238879463a
commit + d60a72e819acb4ac27fccdfde4bb06814eb39e49
blob - fcb17633d81d7f0734e5f38ac655d446d0659a33
blob + 782c41f10b6a1c6b22d074dc873f12bb770345f6
--- sys/dev/ic/qwx.c
+++ sys/dev/ic/qwx.c
@@ -341,8 +341,24 @@ qwx_stop(struct ifnet *ifp)
 	ifp->if_flags &= ~IFF_RUNNING;
 	ifq_clr_oactive(&ifp->if_snd);
 
-	sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
-	sc->ns_nstate = IEEE80211_S_INIT;
+	/*
+	 * Manually run the newstate task's code for switching to INIT state.
+	 * This reconfigures firmware state to stop scanning, or disassociate
+	 * from our current AP, and/or stop the VIF, etc.
+	 */
+	if (ic->ic_state != IEEE80211_S_INIT) {
+		sc->ns_nstate = IEEE80211_S_INIT;
+		sc->ns_arg = -1; /* do not send management frames */
+		refcnt_init(&sc->task_refs);
+		refcnt_take(&sc->task_refs);
+		qwx_newstate_task(sc);
+		if (ic->ic_state != IEEE80211_S_INIT) { /* task code failed */
+			task_del(systq, &sc->init_task);
+			sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
+		}
+		refcnt_finalize(&sc->task_refs, "qwxstop");
+	}
+
 	sc->scan.state = ATH11K_SCAN_IDLE;
 	sc->vdev_id_11d_scan = QWX_11D_INVALID_VDEV_ID;
 	sc->pdevs_active = 0;
@@ -899,6 +915,9 @@ qwx_newstate_task(void *arg)
 			}
 			/* FALLTHROUGH */
 		case IEEE80211_S_SCAN:
+			if (nstate < IEEE80211_S_SCAN)
+				qwx_scan_abort(sc);
+			break;
 		case IEEE80211_S_INIT:
 			break;
 		}
@@ -954,7 +973,8 @@ out:
 			task_add(systq, &sc->init_task);
 		else
 			sc->sc_newstate(ic, nstate, sc->ns_arg);
-	}
+	} else if (err == 0)
+		sc->sc_newstate(ic, nstate, sc->ns_arg);
 	refcnt_rele_wake(&sc->task_refs);
 	splx(s);
 }