Download raw body.
uncm(4) corrections
The diff didn't apply.
$ patch -p0 -N -C < uncm-corrections.patch
...
Patching file sys/dev/usb/if_uncm.c using Plan A...
Hunk #1 succeeded at 403.
Hunk #2 succeeded at 482.
Hunk #3 succeeded at 546.
Hunk #4 succeeded at 787.
Hunk #5 succeeded at 814.
Hunk #6 succeeded at 1001.
Hunk #7 succeeded at 1013.
Hunk #8 succeeded at 1026.
Hunk #9 succeeded at 1042.
Hunk #10 succeeded at 1167.
Hunk #11 failed at 1220.
Hunk #12 succeeded at 1267 (offset 27 lines).
Hunk #13 succeeded at 1284 (offset 27 lines).
1 out of 13 hunks failed
done
See my comment below about DLBitRate / ULBitRate.
On Wed, Oct 14, 2026 at 01:02:15PM +0000, Jan Schreiber wrote:
> This patch does the following things:
>
> - removes unneeded splnet() calls
> - removes packages too big for a single NTB from the queue so they
> won't block
> - corrects the order for checking NCM_FORMATs. now the format
> check happens before the usage of the device supplied value
> - use the ifqueue(9) api to enqueue packages
> - up and down transfer speeds where mixed around (no change in
> behaviour but the debug messages will be correct now)
> - be more spec compliant for with checking the wNdpOutAlignment to be
> at least 4 and not == 0
> - close the pipe and then call usb_rem_wait_task(9)
>
> Tested locally with my Android device.
>
> Jan
>
>
> diff --git sys/dev/usb/if_uncm.c sys/dev/usb/if_uncm.c
> index 4c34080208f..5360c7bf938 100644
> --- sys/dev/usb/if_uncm.c
> +++ sys/dev/usb/if_uncm.c
> @@ -403,11 +403,13 @@ uncm_detach(struct device *self, int flags)
> if (ifp->if_flags & IFF_RUNNING)
> uncm_stop(sc);
>
> - usb_rem_wait_task(sc->sc_udev, &sc->sc_link_task);
> if (sc->sc_ctrl_pipe) {
> usbd_close_pipe(sc->sc_ctrl_pipe);
> sc->sc_ctrl_pipe = NULL;
> }
> +
> + usb_rem_wait_task(sc->sc_udev, &sc->sc_link_task);
> +
> if (ifp->if_softc != NULL) {
> ether_ifdetach(ifp);
> if_detach(ifp);
> @@ -480,10 +482,10 @@ uncm_ntb_setup(struct uncm_softc *sc)
> sc->sc_ndp_div = UGETW(np.wNdpOutDivisor);
> sc->sc_ndp_remainder = UGETW(np.wNdpOutPayloadRemainder);
>
> - if (!powerof2(sc->sc_align) || sc->sc_align == 0 ||
> + if (!powerof2(sc->sc_align) || sc->sc_align < 4 ||
> sc->sc_align >= sc->sc_tx_bufsz)
> sc->sc_align = sizeof(uint32_t);
> - if (!powerof2(sc->sc_ndp_div) || sc->sc_ndp_div == 0 ||
> + if (!powerof2(sc->sc_ndp_div) || sc->sc_ndp_div < 4 ||
> sc->sc_ndp_div >= sc->sc_tx_bufsz)
> sc->sc_ndp_div = sizeof(uint32_t);
> if (sc->sc_ndp_remainder >= sc->sc_ndp_div)
> @@ -544,10 +546,10 @@ uncm_ntb_setup_format(struct uncm_softc *sc)
> if (usbd_do_request(sc->sc_udev, &req, wFmt) != USBD_NORMAL_COMPLETION)
> goto fail;
> fmt = UGETW(wFmt);
> - if ((sc->sc_ncm_supported_formats & (1UL << fmt)) == 0)
> - goto fail;
> if (fmt != NCM_FORMAT_NTB16 && fmt != NCM_FORMAT_NTB32)
> goto fail;
> + if ((sc->sc_ncm_supported_formats & (1UL << fmt)) == 0)
> + goto fail;
> sc->sc_ncm_format = fmt;
>
> DPRINTF("%s: using NCM format %d, supported 0x%x\n",
> @@ -785,6 +787,15 @@ uncm_start(struct ifnet *ifp)
> break;
>
> mlen = maxoverhead + m->m_pkthdr.len;
> +
> + if (ndgram == 0 && offs + mlen > sc->sc_tx_bufsz) {
> + /* Drop packages that are too big for a NTB. */
> + ifq_deq_commit(&ifp->if_snd, m);
> + m_freem(m);
> + ifp->if_oerrors++;
> + DPRINTF("%s: packet too large for NTB\n", DEVNAM(sc));
> + continue;
> + }
> if ((sc->sc_maxdgram != 0 && ndgram >= sc->sc_maxdgram) ||
> (offs + len + mlen > sc->sc_tx_bufsz)) {
> ifq_deq_rollback(&ifp->if_snd, m);
> @@ -803,9 +814,11 @@ uncm_start(struct ifnet *ifp)
> }
> if (ml_empty(&sc->sc_tx_ml))
> return;
> - if (uncm_encap(sc, ndgram)) {
> - ifq_set_oactive(&ifp->if_snd);
> - ifp->if_timer = (2 * uncm_xfer_tout) / 1000;
> + ifq_set_oactive(&ifp->if_snd);
> + ifp->if_timer = (2 * uncm_xfer_tout) / 1000;
> + if (!uncm_encap(sc, ndgram)) {
> + ifp->if_timer = 0;
> + ifq_clr_oactive(&ifp->if_snd);
> }
> }
>
> @@ -988,10 +1001,7 @@ uncm_txeof(struct usbd_xfer *xfer, void *priv, usbd_status status)
> {
> struct uncm_softc *sc = priv;
> struct ifnet *ifp = GET_IFP(sc);
> - int s;
>
> - s = splnet();
> - ifq_clr_oactive(&ifp->if_snd);
> ifp->if_timer = 0;
>
> if (status != USBD_NORMAL_COMPLETION) {
> @@ -1003,10 +1013,9 @@ uncm_txeof(struct usbd_xfer *xfer, void *priv, usbd_status status)
> usbd_clear_endpoint_stall_async(sc->sc_tx_pipe);
> }
> }
> - if (ifq_empty(&ifp->if_snd) == 0)
> - uncm_start(ifp);
>
> - splx(s);
> + if (ifq_is_oactive(&ifp->if_snd))
> + ifq_restart(&ifp->if_snd);
> }
>
> /*
> @@ -1017,7 +1026,6 @@ void
> uncm_decap(struct uncm_softc *sc, struct usbd_xfer *xfer)
> {
> struct ifnet *ifp = GET_IFP(sc);
> - int s;
> void *buf;
> uint32_t len;
> char *dp;
> @@ -1034,7 +1042,6 @@ uncm_decap(struct uncm_softc *sc, struct usbd_xfer *xfer)
>
> usbd_get_xfer_status(xfer, NULL, &buf, &len, NULL);
> DPRINTFN(4, "%s: recv %d bytes\n", DEVNAM(sc), len);
> - s = splnet();
> if (len < sizeof(*hdr16))
> goto toosmall;
>
> @@ -1160,13 +1167,11 @@ uncm_decap(struct uncm_softc *sc, struct usbd_xfer *xfer)
> }
> done:
> if_input(ifp, &ml);
> - splx(s);
> return;
> toosmall:
> DPRINTF("%s: packet too small (%d)\n", DEVNAM(sc), len);
> fail:
> ifp->if_ierrors++;
> - splx(s);
> }
>
> void
> @@ -1215,8 +1220,8 @@ uncm_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
> break;
> case UCDC_N_CONNECTION_SPEED_CHANGE:
> if (total_len >= UCDC_NOTIFICATION_LENGTH + 8) {
> - down = UGETDW(&sc->sc_intr_msg.data[0]);
> - up = UGETDW(&sc->sc_intr_msg.data[4]);
> + up = UGETDW(&sc->sc_intr_msg.data[0]);
> + down = UGETDW(&sc->sc_intr_msg.data[4]);
Are you sure this swap is correct? I'm looking at Table 21 on page
25 from CDC120_with_Errata_and_ECN_through_Apr_3_2025.pdf where I
see:
Offset 0 DLBitRate downlink bit rate, as sent on the IN pipe
Offset 4 ULBitRate uplink bit rate, as sent on the OUT pipe
> sc->sc_baudrate = MIN(up, down);
> DPRINTFN(2, "%s: speed down %u up %u\n", DEVNAM(sc),
> down, up);
> @@ -1235,12 +1240,10 @@ uncm_link_task(void *arg)
> {
> struct uncm_softc *sc = arg;
> struct ifnet *ifp = GET_IFP(sc);
> - int s;
>
> if (usbd_is_dying(sc->sc_udev))
> return;
>
> - s = splnet();
> ifp->if_baudrate = sc->sc_baudrate;
> if (sc->sc_link != LINK_STATE_UNKNOWN &&
> ifp->if_link_state != sc->sc_link) {
> @@ -1254,7 +1257,6 @@ uncm_link_task(void *arg)
> if_link_state_change(ifp);
> if (LINK_STATE_IS_UP(sc->sc_link) &&
> (ifp->if_flags & IFF_RUNNING) && !ifq_empty(&ifp->if_snd))
> - uncm_start(ifp);
> + ifq_start(&ifp->if_snd);
> }
> - splx(s);
> }
>
--
Regards,
Mikolaj
uncm(4) corrections