Good news - no more kernel panics on USB 3.0(xHCI), it’s fixed. Bad news - after 2-3h LTE modem lost local network connection via USB 3.0(cdce0). I have to remove modem and put it back to usb port - then local network connection between OpenBSD and modem back for 2-3h, sometimes 30-40 min. It looks like the same problem as kernel panic, but this time there is lost network connection via usb 3.0(xhci).
root@master[~]ping 192.168.8.1 PING 192.168.8.1 (192.168.8.1): 56 data bytes ping: sendmsg: Network is down ping: wrote 192.168.8.1 64 chars, ret=-1 ping: sendmsg: Network is down ping: wrote 192.168.8.1 64 chars, ret=-1 192.168.8.1 is default static IP on lte modem. Your changes in if_cdce.c 1.77 not completely fix the problem. > On 11 Jun 2020, at 11:13, Łukasz Lejtkowski <emig...@gmail.com> wrote: > > Hi Gerhard, > > Today I added Your patches to 6.7-stable and moved back LTE modem to USB 3.0. > So, just waiting for… nothing or kernel panic. I’ll let you know. > >> On 8 Jun 2020, at 19:13, Patrick Wildt <patr...@blueri.se >> <mailto:patr...@blueri.se>> wrote: >> >> On Mon, Jun 08, 2020 at 05:31:44PM +0200, Gerhard Roth wrote: >>> On 2020-05-25 13:19, Martin Pieuchot wrote: >>>> On 25/05/20(Mon) 12:56, Gerhard Roth wrote: >>>>> On 5/22/20 9:05 PM, Mark Kettenis wrote: >>>>>>> From: Łukasz Lejtkowski <emig...@gmail.com <mailto:emig...@gmail.com>> >>>>>>> Date: Fri, 22 May 2020 20:51:57 +0200 >>>>>>> >>>>>>> Probably power supply 12 V is broken. Showing 16,87 V(Fluke 179) - >>>>>>> too high. Should be 12,25-12,50 V. I replaced to the new one. >>>>>> >>>>>> That might be why the device stops responding. The fact that cleaning >>>>>> up from a failed USB transaction leads to this panic is a bug though. >>>>>> >>>>>> And somebody just posted a very similar panic with ure(4). Something >>>>>> in the network stack is holding a mutex when it shouldn't. >>>>> >>>>> I think that holding the mutex is ok. The bug is calling the stop >>>>> routine in case of errors. >>>>> >>>>> This is what common foo_start() does: >>>>> >>>>> m_head = ifq_deq_begin(&ifp->if_snd); >>>>> if (foo_encap(sc, m_head, 0)) { >>>>> ifq_deq_rollback(&ifp->if_snd, m_head); >>>>> ... >>>>> return; >>>>> } >>>>> ifq_deq_commit(&ifp->if_snd, m_head); >>>>> >>>>> Here, ifq_deq_begin() grabs a mutex and it is held while >>>>> calling foo_encap(). >>>>> >>>>> For USB network interfaces foo_encap() mostly does this: >>>>> >>>>> err = usbd_transfer(sc->sc_xfer); >>>>> if (err != USBD_IN_PROGRESS) { >>>>> foo_stop(sc); >>>>> return EIO; >>>>> } >>>>> >>>>> And foo_stop() calls usbd_abort_pipe() -> xhci_command_submit(), >>>>> which might sleep. >>>>> >>>>> How to fix? We could do the foo_encap() after the ifq_deq_commit(), >>>>> possibly dropping the current mbuf if encap fails (who cares >>>>> for the packets after foo_stop() anyway). >>>> >>>> That's the approach taken by drivers using ifq_dequeue(9) instead of >>>> ifq_deq_begin/commit(). >>>> >>>>> Or change all the drivers to follow the path that if_aue.c takes: >>>>> >>>>> err = usbd_transfer(c->aue_xfer); >>>>> if (err != USBD_IN_PROGRESS) { >>>>> ... >>>>> /* Stop the interface from process context. */ >>>>> usb_add_task(sc->aue_udev, &sc->aue_stop_task); >>>>> return (EIO); >>>>> } >>>> >>>> That's just trading the current problem for another one with higher >>>> complexity. >>>> >>>>> Any ideas, what's better? Or alternative proposals? >>>> >>>> Using ifq_dequeue(9) would have the advantage of unifying the code base. >>>> It introduces a behavior change. A simpler fix would be to call >>>> foo_stop() in the error path after ifq_deq_rollback(). >>>> >>> >>> Hi, >>> >>> two weeks passed any nobody objected Martin's proposal. So I thought, >>> we could try to move on this way. >>> >>> Gerhard >>> >> >> From what I remember from various discussions, the goal should be to >> check if there's a buffer free in the ring, then dequeue and send, and >> it it can't be sent out, then drop it. With USB apparently those >> drivers "always" have an open buffer, so we can just dequeue and send, >> like you do in this diff. And if it gets dropped, that's fine. >> >> That said, I think IFQ_DEQUEUE() is old compat code, and we actually >> nowadays prefer: >> >> m_head = ifq_dequeue(&ifp->if_snd); >> >> If you look at the define for IFQ_DEQUEUE() you'll see it's marked >> as compat code. If you look at a new driver, like ixl(4), you'll >> see that it also uses ifq_dequeue(). >> >> Sorry to to give you some work, but with that fixed: ok patrick@ >> >> Patrick >> >>> >>> Index: sys/dev/usb/if_axe.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_axe.c,v >>> retrieving revision 1.139 >>> diff -u -p -u -p -r1.139 if_axe.c >>> --- sys/dev/usb/if_axe.c 7 Jul 2019 06:40:10 -0000 1.139 >>> +++ sys/dev/usb/if_axe.c 8 Jun 2020 15:13:25 -0000 >>> @@ -1223,6 +1223,7 @@ axe_encap(struct axe_softc *sc, struct m >>> /* Transmit */ >>> err = usbd_transfer(c->axe_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->axe_mbuf = NULL; >>> axe_stop(sc); >>> return(EIO); >>> } >>> @@ -1246,16 +1247,15 @@ axe_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (axe_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> /* >>> * If there's a BPF listener, bounce a copy of this frame >>> Index: sys/dev/usb/if_axen.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_axen.c,v >>> retrieving revision 1.27 >>> diff -u -p -u -p -r1.27 if_axen.c >>> --- sys/dev/usb/if_axen.c 7 Jul 2019 06:40:10 -0000 1.27 >>> +++ sys/dev/usb/if_axen.c 8 Jun 2020 14:49:26 -0000 >>> @@ -1186,6 +1186,7 @@ axen_encap(struct axen_softc *sc, struct >>> /* Transmit */ >>> err = usbd_transfer(c->axen_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->axen_mbuf = NULL; >>> axen_stop(sc); >>> return EIO; >>> } >>> @@ -1209,16 +1210,15 @@ axen_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (axen_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> /* >>> * If there's a BPF listener, bounce a copy of this frame >>> Index: sys/dev/usb/if_cdce.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v >>> retrieving revision 1.76 >>> diff -u -p -u -p -r1.76 if_cdce.c >>> --- sys/dev/usb/if_cdce.c 14 Nov 2019 13:50:55 -0000 1.76 >>> +++ sys/dev/usb/if_cdce.c 8 Jun 2020 14:50:40 -0000 >>> @@ -375,18 +375,16 @@ cdce_start(struct ifnet *ifp) >>> if (usbd_is_dying(sc->cdce_udev) || ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (cdce_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> - >>> #if NBPFILTER > 0 >>> if (ifp->if_bpf) >>> bpf_mtap(ifp->if_bpf, m_head, BPF_DIRECTION_OUT); >>> @@ -422,6 +420,7 @@ cdce_encap(struct cdce_softc *sc, struct >>> 10000, cdce_txeof); >>> err = usbd_transfer(c->cdce_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->cdce_mbuf = NULL; >>> cdce_stop(sc); >>> return (EIO); >>> } >>> Index: sys/dev/usb/if_kue.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_kue.c,v >>> retrieving revision 1.89 >>> diff -u -p -u -p -r1.89 if_kue.c >>> --- sys/dev/usb/if_kue.c 2 Oct 2018 19:49:10 -0000 1.89 >>> +++ sys/dev/usb/if_kue.c 8 Jun 2020 14:52:27 -0000 >>> @@ -829,6 +829,7 @@ kue_send(struct kue_softc *sc, struct mb >>> if (err != USBD_IN_PROGRESS) { >>> printf("%s: kue_send error=%s\n", sc->kue_dev.dv_xname, >>> usbd_errstr(err)); >>> + c->kue_mbuf = NULL; >>> kue_stop(sc); >>> return (EIO); >>> } >>> @@ -852,17 +853,15 @@ kue_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (kue_send(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> #if NBPFILTER > 0 >>> /* >>> Index: sys/dev/usb/if_mos.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_mos.c,v >>> retrieving revision 1.40 >>> diff -u -p -u -p -r1.40 if_mos.c >>> --- sys/dev/usb/if_mos.c 7 Jul 2019 06:40:10 -0000 1.40 >>> +++ sys/dev/usb/if_mos.c 8 Jun 2020 15:15:42 -0000 >>> @@ -1097,6 +1097,7 @@ mos_encap(struct mos_softc *sc, struct m >>> /* Transmit */ >>> err = usbd_transfer(c->mos_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->mos_mbuf = NULL; >>> mos_stop(sc); >>> return(EIO); >>> } >>> @@ -1120,16 +1121,15 @@ mos_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (mos_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> /* >>> * If there's a BPF listener, bounce a copy of this frame >>> Index: sys/dev/usb/if_mue.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_mue.c,v >>> retrieving revision 1.7 >>> diff -u -p -u -p -r1.7 if_mue.c >>> --- sys/dev/usb/if_mue.c 7 Jul 2019 06:40:10 -0000 1.7 >>> +++ sys/dev/usb/if_mue.c 8 Jun 2020 14:55:07 -0000 >>> @@ -987,6 +987,7 @@ mue_encap(struct mue_softc *sc, struct m >>> /* Transmit */ >>> err = usbd_transfer(c->mue_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->mue_mbuf = NULL; >>> mue_stop(sc); >>> return(EIO); >>> } >>> @@ -1308,16 +1309,15 @@ mue_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (mue_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> /* If there's a BPF listener, bounce a copy of this frame to him. */ >>> #if NBPFILTER > 0 >>> Index: sys/dev/usb/if_smsc.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_smsc.c,v >>> retrieving revision 1.34 >>> diff -u -p -u -p -r1.34 if_smsc.c >>> --- sys/dev/usb/if_smsc.c 8 Apr 2020 09:49:32 -0000 1.34 >>> +++ sys/dev/usb/if_smsc.c 8 Jun 2020 15:02:20 -0000 >>> @@ -649,16 +649,15 @@ smsc_start(struct ifnet *ifp) >>> return; >>> } >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (smsc_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> #if NBPFILTER > 0 >>> if (ifp->if_bpf) >>> @@ -1400,6 +1399,7 @@ smsc_encap(struct smsc_softc *sc, struct >>> >>> err = usbd_transfer(c->sc_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->sc_mbuf = NULL; >>> smsc_stop(sc); >>> return (EIO); >>> } >>> Index: sys/dev/usb/if_ugl.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_ugl.c,v >>> retrieving revision 1.23 >>> diff -u -p -u -p -r1.23 if_ugl.c >>> --- sys/dev/usb/if_ugl.c 2 Oct 2018 19:49:10 -0000 1.23 >>> +++ sys/dev/usb/if_ugl.c 8 Jun 2020 15:16:09 -0000 >>> @@ -566,6 +566,7 @@ ugl_send(struct ugl_softc *sc, struct mb >>> if (err != USBD_IN_PROGRESS) { >>> printf("%s: ugl_send error=%s\n", sc->sc_dev.dv_xname, >>> usbd_errstr(err)); >>> + c->ugl_mbuf = NULL; >>> ugl_stop(sc); >>> return (EIO); >>> } >>> @@ -589,17 +590,15 @@ ugl_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (ugl_send(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> #if NBPFILTER > 0 >>> /* >>> Index: sys/dev/usb/if_upl.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_upl.c,v >>> retrieving revision 1.75 >>> diff -u -p -u -p -r1.75 if_upl.c >>> --- sys/dev/usb/if_upl.c 2 Oct 2018 19:49:10 -0000 1.75 >>> +++ sys/dev/usb/if_upl.c 8 Jun 2020 15:06:43 -0000 >>> @@ -546,6 +546,7 @@ upl_send(struct upl_softc *sc, struct mb >>> if (err != USBD_IN_PROGRESS) { >>> printf("%s: upl_send error=%s\n", sc->sc_dev.dv_xname, >>> usbd_errstr(err)); >>> + c->upl_mbuf = NULL; >>> upl_stop(sc); >>> return (EIO); >>> } >>> @@ -569,17 +570,15 @@ upl_start(struct ifnet *ifp) >>> if (ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (upl_send(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> #if NBPFILTER > 0 >>> /* >>> Index: sys/dev/usb/if_ure.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_ure.c,v >>> retrieving revision 1.14 >>> diff -u -p -u -p -r1.14 if_ure.c >>> --- sys/dev/usb/if_ure.c 10 Mar 2020 01:11:30 -0000 1.14 >>> +++ sys/dev/usb/if_ure.c 8 Jun 2020 15:12:53 -0000 >>> @@ -775,16 +775,15 @@ ure_start(struct ifnet *ifp) >>> break; >>> } >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> break; >>> >>> if (ure_encap(sc, m_head)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> break; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> #if NBPFILTER > 0 >>> if (ifp->if_bpf) >>> @@ -1933,6 +1932,7 @@ ure_encap(struct ure_softc *sc, struct m >>> >>> err = usbd_transfer(c->uc_xfer); >>> if (err != 0 && err != USBD_IN_PROGRESS) { >>> + c->uc_mbuf = NULL; >>> ure_stop(sc); >>> return (EIO); >>> } >>> Index: sys/dev/usb/if_urndis.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/if_urndis.c,v >>> retrieving revision 1.69 >>> diff -u -p -u -p -r1.69 if_urndis.c >>> --- sys/dev/usb/if_urndis.c 22 Jan 2019 18:06:05 -0000 1.69 >>> +++ sys/dev/usb/if_urndis.c 8 Jun 2020 15:14:57 -0000 >>> @@ -804,6 +804,7 @@ urndis_encap(struct urndis_softc *sc, st >>> /* Transmit */ >>> err = usbd_transfer(c->sc_xfer); >>> if (err != USBD_IN_PROGRESS) { >>> + c->sc_mbuf = NULL; >>> urndis_stop(sc); >>> return(EIO); >>> } >>> @@ -1190,16 +1191,15 @@ urndis_start(struct ifnet *ifp) >>> if (usbd_is_dying(sc->sc_udev) || ifq_is_oactive(&ifp->if_snd)) >>> return; >>> >>> - m_head = ifq_deq_begin(&ifp->if_snd); >>> + IFQ_DEQUEUE(&ifp->if_snd, m_head); >>> if (m_head == NULL) >>> return; >>> >>> if (urndis_encap(sc, m_head, 0)) { >>> - ifq_deq_rollback(&ifp->if_snd, m_head); >>> + m_freem(m_head); >>> ifq_set_oactive(&ifp->if_snd); >>> return; >>> } >>> - ifq_deq_commit(&ifp->if_snd, m_head); >>> >>> /* >>> * If there's a BPF listener, bounce a copy of this frame >
