Download raw body.
uncm(4) cleanup
Hi Jan.
I ran this diff on -current, however with and without below diff, I see
no carrier on uncm0 interface.
$ dmesg | grep -e ^uncm
uncm0 at uhub1 port 1 configuration 1 interface 0 "Google Pixel 7a" rev 2.10/5.10 addr 6
uncm0: address 26:d3:84:63:d2:cb
$ ifconfig uncm0
uncm0: flags=a48843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST,AUTOCONF6TEMP,AUTOCONF6,AUTOCONF4> mtu 1500
lladdr 26:d3:84:63:d2:cb
index 3 priority 0 llprio 3
status: no carrier
inet6 fe80::24d3:84ff:fe63:d2cb%uncm0 prefixlen 64 scopeid 0x3
// with the below diff
$ sysctl -n kern.version
OpenBSD 8.0-current (GENERIC.MP) #11: Sat Oct 10 12:22:21 UTC 2026
root@pc1.home.local:/home/mkucharski/openbsd/src/sys/arch/amd64/compile/GENERIC.MP
// or vanilla kernel
$ sysctl -n kern.version
OpenBSD 8.0-current (GENERIC.MP) #86: Sat Oct 10 02:01:31 MDT 2026
deraadt@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
On the Android side USB tethering is turned on.
On Fri, Oct 09, 2026 at 01:46:40PM +0000, Jan Schreiber wrote:
> After dlg@ send me a link to his inital implementation of a cdc ncm
> driver I took some time fixing up some whitespace, added a macro for
> more readability and added some missing status checks.
>
> Jan
>
> diff --git sys/dev/usb/if_uncm.c sys/dev/usb/if_uncm.c
> index 626fb552dff..1579235bd07 100644
> --- sys/dev/usb/if_uncm.c
> +++ sys/dev/usb/if_uncm.c
> @@ -108,6 +108,13 @@ struct uncm_softc {
>
> #define UNCM_RX_NTB_MAX 16384
>
> +#define UNCM_NTB16_MINSZ (sizeof(struct ncm_header16) + \
> + sizeof(struct ncm_pointer16) + \
> + sizeof(struct ncm_pointer16_dgram))
> +#define UNCM_NTB32_MINSZ (sizeof(struct ncm_header32) + \
> + sizeof(struct ncm_pointer32) + \
> + sizeof(struct ncm_pointer32_dgram))
> +
> int uncm_match(struct device *, void *, void *);
> void uncm_attach(struct device *, struct device *, void *);
> int uncm_detach(struct device *, int);
> @@ -192,16 +199,17 @@ uncm_attach(struct device *parent, struct device *self, void *aux)
> sc->sc_ctrl_ifaceno = uaa->ifaceno;
> sc->sc_maxpktlen = ETHER_MAX_LEN - ETHER_CRC_LEN;
> sc->sc_link = LINK_STATE_UNKNOWN;
> +
> ml_init(&sc->sc_tx_ml);
> usb_init_task(&sc->sc_link_task, uncm_link_task, sc,
> USB_TASK_TYPE_GENERIC);
> +
> usbd_desc_iter_init(sc->sc_udev, &iter);
> while ((desc = usbd_desc_iter_next(&iter))) {
> if (desc->bDescriptorType == UDESC_IFACE_ASSOC) {
> ad = (usb_interface_assoc_descriptor_t *)desc;
> if (ad->bFirstInterface == uaa->ifaceno &&
> ad->bInterfaceCount > 1) {
> - /* Fall back to the interface following control. */
> data_ifaceno = uaa->ifaceno + 1;
> }
> continue;
> @@ -245,6 +253,7 @@ uncm_attach(struct device *parent, struct device *self, void *aux)
> break;
> }
> }
> +
> if (data_ifaceno == -1) {
> printf("%s: no data interface number\n", DEVNAM(sc));
> goto fail;
> @@ -260,6 +269,7 @@ uncm_attach(struct device *parent, struct device *self, void *aux)
> usbd_claim_iface(sc->sc_udev, i);
> }
> }
> +
> if (sc->sc_data_iface == NULL) {
> printf("%s: no data interface found\n", DEVNAM(sc));
> goto fail;
> @@ -275,6 +285,7 @@ uncm_attach(struct device *parent, struct device *self, void *aux)
> UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN)
> ctrl_ep = ed->bEndpointAddress;
> }
> +
> if (ctrl_ep == -1) {
> printf("%s: missing interrupt endpoint\n", DEVNAM(sc));
> goto fail;
> @@ -316,6 +327,7 @@ uncm_attach(struct device *parent, struct device *self, void *aux)
> UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_OUT)
> sc->sc_tx_ep = ed->bEndpointAddress;
> }
> +
> if (sc->sc_rx_ep == -1 || sc->sc_tx_ep == -1) {
> printf("%s: missing bulk endpoints\n", DEVNAM(sc));
> goto fail;
> @@ -438,12 +450,14 @@ uncm_ntb_setup(struct uncm_softc *sc)
> {
> usb_device_request_t req;
> struct ncm_ntb_parameters np;
> + int toosmall = 0;
>
> req.bmRequestType = UT_READ_CLASS_INTERFACE;
> req.bRequest = NCM_GET_NTB_PARAMETERS;
> USETW(req.wValue, 0);
> USETW(req.wIndex, sc->sc_ctrl_ifaceno);
> USETW(req.wLength, sizeof(np));
> +
> if (usbd_do_request(sc->sc_udev, &req, &np) == USBD_NORMAL_COMPLETION &&
> UGETW(np.wLength) == sizeof(np)) {
> sc->sc_rx_bufsz = MIN(UGETDW(np.dwNtbInMaxSize), UINT16_MAX);
> @@ -476,19 +490,17 @@ uncm_ntb_setup(struct uncm_softc *sc)
> sc->sc_ncm_supported_formats = NCM_FORMAT_NTB16_MASK;
> }
>
> - if (sc->sc_rx_bufsz < sizeof(struct ncm_header16) +
> - sizeof(struct ncm_pointer16) +
> - sizeof(struct ncm_pointer16_dgram) ||
> - sc->sc_tx_bufsz < sizeof(struct ncm_header16) +
> - sizeof(struct ncm_pointer16) +
> - sizeof(struct ncm_pointer16_dgram) ||
> - ((sc->sc_ncm_supported_formats & NCM_FORMAT_NTB32_MASK) &&
> - (sc->sc_rx_bufsz < sizeof(struct ncm_header32) +
> - sizeof(struct ncm_pointer32) +
> - sizeof(struct ncm_pointer32_dgram) ||
> - sc->sc_tx_bufsz < sizeof(struct ncm_header32) +
> - sizeof(struct ncm_pointer32) +
> - sizeof(struct ncm_pointer32_dgram)))) {
> + if (sc->sc_rx_bufsz < UNCM_NTB16_MINSZ ||
> + sc->sc_tx_bufsz < UNCM_NTB16_MINSZ)
> + toosmall = 1;
> +
> + if (sc->sc_ncm_supported_formats & NCM_FORMAT_NTB32_MASK) {
> + if (sc->sc_rx_bufsz < UNCM_NTB32_MINSZ ||
> + sc->sc_tx_bufsz < UNCM_NTB32_MINSZ)
> + toosmall = 1;
> + }
> +
> + if (toosmall) {
> DPRINTF("%s: invalid NTB size %d/%d\n", DEVNAM(sc),
> sc->sc_rx_bufsz, sc->sc_tx_bufsz);
> sc->sc_ncm_supported_formats = 0;
> @@ -549,6 +561,7 @@ uncm_ntb_setup_input_size(struct uncm_softc *sc)
> USETW(req.wValue, 0);
> USETW(req.wIndex, sc->sc_ctrl_ifaceno);
> USETW(req.wLength, sizeof(dwSize));
> +
> if (usbd_do_request(sc->sc_udev, &req, dwSize) ==
> USBD_NORMAL_COMPLETION)
> sc->sc_rx_bufsz = UNCM_RX_NTB_MAX;
> @@ -566,11 +579,11 @@ uncm_alloc_xfers(struct uncm_softc *sc)
> sc->sc_rx_bufsz);
> }
>
> - if (sc->sc_tx_xfer == NULL) {
> + if (sc->sc_tx_xfer == NULL) {
> if ((sc->sc_tx_xfer = usbd_alloc_xfer(sc->sc_udev)) != NULL)
> sc->sc_tx_buf = usbd_alloc_buffer(sc->sc_tx_xfer,
> sc->sc_tx_bufsz);
> - }
> + }
>
> return (sc->sc_rx_buf != NULL && sc->sc_tx_buf != NULL);
> }
> @@ -584,13 +597,13 @@ uncm_free_xfers(struct uncm_softc *sc)
> sc->sc_rx_buf = NULL;
> }
>
> - if (sc->sc_tx_xfer != NULL) {
> + if (sc->sc_tx_xfer != NULL) {
> usbd_free_xfer(sc->sc_tx_xfer);
> sc->sc_tx_xfer = NULL;
> sc->sc_tx_buf = NULL;
> }
>
> - ml_purge(&sc->sc_tx_ml);
> + ml_purge(&sc->sc_tx_ml);
> }
>
> void
> @@ -713,6 +726,7 @@ uncm_padding(void *buf, size_t bufsz, int offs, int alignment, int remainder)
> nb = uncm_align(bufsz, offs, alignment, remainder);
> if (nb > 0)
> memset(buf + offs, 0, nb);
> +
> return nb;
> }
>
> @@ -757,7 +771,7 @@ uncm_start(struct ifnet *ifp)
> if (m == NULL)
> break;
>
> - mlen = maxoverhead + m->m_pkthdr.len;
> + mlen = maxoverhead + m->m_pkthdr.len;
> if ((sc->sc_maxdgram != 0 && ndgram >= sc->sc_maxdgram) ||
> (offs + len + mlen > sc->sc_tx_bufsz)) {
> ifq_deq_rollback(&ifp->if_snd, m);
> @@ -852,7 +866,7 @@ uncm_encap(struct uncm_softc *sc, int ndgram)
> int offs = 0, plen = 0;
> int dgoffs = 0, poffs;
> struct mbuf *m;
> - usbd_status err;
> + usbd_status err;
>
> KASSERT(sc->sc_ncm_format == NCM_FORMAT_NTB16 ||
> sc->sc_ncm_format == NCM_FORMAT_NTB32);
> @@ -1051,9 +1065,11 @@ uncm_decap(struct uncm_softc *sc, struct usbd_xfer *xfer)
>
> if ((uint64_t)len < ptroff + sizeof(*ptr16))
> goto toosmall;
> +
> ptr16 = (struct ncm_pointer16 *)(buf + ptroff);
> psig = UGETDW(ptr16->dwSignature);
> ptrlen = UGETW(ptr16->wLength);
> +
> if ((uint64_t)len < (uint64_t)ptrlen + ptroff)
> goto toosmall;
>
> @@ -1152,18 +1168,23 @@ uncm_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
> return;
>
> if (status != USBD_NORMAL_COMPLETION) {
> + if (status == USBD_NOT_STARTED || status == USBD_CANCELLED)
> + return;
> DPRINTF("%s: notification error: %s\n", DEVNAM(sc),
> usbd_errstr(status));
> if (status == USBD_STALLED)
> usbd_clear_endpoint_stall_async(sc->sc_ctrl_pipe);
> return;
> }
> +
> usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL);
> +
> if (total_len < UCDC_NOTIFICATION_LENGTH) {
> DPRINTF("%s: short notification (%d<%d)\n", DEVNAM(sc),
> total_len, UCDC_NOTIFICATION_LENGTH);
> return;
> }
> +
> if (sc->sc_intr_msg.bmRequestType != UCDC_NOTIFICATION) {
> DPRINTF("%s: unexpected notification (type=0x%02x)\n",
> DEVNAM(sc), sc->sc_intr_msg.bmRequestType);
> @@ -1213,7 +1234,8 @@ uncm_link_task(void *arg)
> if (ifp->if_flags & IFF_DEBUG)
> log(LOG_DEBUG, "%s: link state changed from %s to %s\n",
> DEVNAM(sc),
> - LINK_STATE_IS_UP(ifp->if_link_state) ? "up" : "down",
> + LINK_STATE_IS_UP(ifp->if_link_state) ?
> + "up" : "down",
> LINK_STATE_IS_UP(sc->sc_link) ? "up" : "down");
> ifp->if_link_state = sc->sc_link;
> if_link_state_change(ifp);
> @@ -1223,4 +1245,3 @@ uncm_link_task(void *arg)
> }
> splx(s);
> }
> -
>
>
--
Regards,
Mikolaj
uncm(4) cleanup