On Wed, Jan 25, 2017 at 07:48:18PM +1000, Martin Pieuchot wrote:
> On 25/01/17(Wed) 10:36, Stefan Sperling wrote:
> > On Tue, Jan 24, 2017 at 03:10:34PM -0500, mabi wrote:
> > > Hi Stefan
> > > Thanks for your input. It looks like the g2k16 modifications to the athn
> > > code from awolk@ did not make it into the 6.0 release. So there is still
> > > hope for 6.1 ;-)
> >
> > There was a rabbit hole this diff by Adam fell into. I don't know what
> > the current status of this is. Adam might know more.
>
> The diff should go in, it doesn't make things worse.
>
Changes from g2k16 will not prevent the timeouts but will help by removing the
need to manually kick the netstart scripts when the timeout happens.
I recall the diff was put on hold as we still found it crashing in some cases,
from the undeadly report:
---
The fourth one was in the athn driver itself. The interface was half cleaned up
(the fields of the ifp data structure were freed but not the interface itself)
so when the watchdog tried to access it caused the crash.
---
One of the diff from testing had this guard in place:
$ cat /home/mulander/athn-watchdog.6.diff
Index: if_athn_usb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
retrieving revision 1.42
diff -u -p -r1.42 if_athn_usb.c
--- if_athn_usb.c 11 Dec 2015 16:07:02 -0000 1.42
+++ if_athn_usb.c 4 Sep 2016 18:48:14 -0000
@@ -2098,13 +2098,17 @@ void
athn_usb_watchdog(struct ifnet *ifp)
{
struct athn_softc *sc = ifp->if_softc;
+ struct ieee80211com *ic = &sc->sc_ic;
ifp->if_timer = 0;
if (sc->sc_tx_timer > 0) {
if (--sc->sc_tx_timer == 0) {
printf("%s: device timeout\n", sc->sc_dev.dv_xname);
- /* athn_usb_init(ifp); XXX needs a process context! */
+ if (ic->ic_bss == NULL)
+ return;
+ athn_usb_stop(ifp);
+ athn_usb_init(ifp);
ifp->if_oerrors++;
return;
}
the ic->ic_bss being null doing stop resulted in further crashing. Though it was
agreed that adding guards likes that in the watchdog is not wanted in the
watchdog handler. The final diff is just a athn_usb_stop/athn_usb_init in the
watchdog itself, it got mixed feedback. Don't remember who exactly took which
stance but the general opinions were;
- it should go in, doesn't make things worse
- let's wait for other changes in the stack
I decided to wait out and I guess the diff just bit rotted on my disk :)
Here is the final one that just restarts the interface. I have been running it
since September on most of my snapshots, stopped applying it around December
when I was travelling with a different usb dongle (ural0).
OK's to commit?
Index: if_athn_usb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
retrieving revision 1.45
diff -u -p -r1.45 if_athn_usb.c
--- if_athn_usb.c 22 Jan 2017 10:17:39 -0000 1.45
+++ if_athn_usb.c 25 Jan 2017 22:52:10 -0000
@@ -2104,7 +2104,8 @@ athn_usb_watchdog(struct ifnet *ifp)
if (sc->sc_tx_timer > 0) {
if (--sc->sc_tx_timer == 0) {
printf("%s: device timeout\n", sc->sc_dev.dv_xname);
- /* athn_usb_init(ifp); XXX needs a process context! */
+ athn_usb_stop(ifp);
+ athn_usb_init(ifp);
ifp->if_oerrors++;
return;
}
