From owner-p4-projects@FreeBSD.ORG Sun Oct 3 09:42:09 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 731B41065679; Sun, 3 Oct 2010 09:42:09 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 208001065674 for ; Sun, 3 Oct 2010 09:42:09 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from skunkworks.freebsd.org (skunkworks.freebsd.org [IPv6:2001:4f8:fff6::2d]) by mx1.freebsd.org (Postfix) with ESMTP id 0C5728FC1D for ; Sun, 3 Oct 2010 09:42:09 +0000 (UTC) Received: from skunkworks.freebsd.org (localhost [127.0.0.1]) by skunkworks.freebsd.org (8.14.4/8.14.4) with ESMTP id o939g8qS038031 for ; Sun, 3 Oct 2010 09:42:08 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by skunkworks.freebsd.org (8.14.4/8.14.4/Submit) id o939g87U038028 for perforce@freebsd.org; Sun, 3 Oct 2010 09:42:08 GMT (envelope-from hselasky@FreeBSD.org) Date: Sun, 3 Oct 2010 09:42:08 GMT Message-Id: <201010030942.o939g87U038028@skunkworks.freebsd.org> X-Authentication-Warning: skunkworks.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 184381 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Oct 2010 09:42:09 -0000 http://p4web.freebsd.org/@@184381?ac=10 Change 184381 by hselasky@hselasky_laptop001 on 2010/10/03 09:41:31 USB network (NCM driver): - correct the ethernet payload remainder which must be post-offseted by -14 bytes instead of 0 bytes. This is not very clearly defined in the NCM specification. - add development feature about limiting the maximum datagram count in each NCM payload. - zero-pad alignment data - add TX-interval tuning sysctl Affected files ... .. //depot/projects/usb/src/sys/dev/usb/net/if_cdce.c#29 edit .. //depot/projects/usb/src/sys/dev/usb/net/if_cdcereg.h#7 edit .. //depot/projects/usb/src/sys/dev/usb/usb_cdc.h#16 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/net/if_cdce.c#29 (text+ko) ==== @@ -110,10 +110,13 @@ #ifdef USB_DEBUG static int cdce_debug = 0; +static int cdce_tx_interval = 0; SYSCTL_NODE(_hw_usb, OID_AUTO, cdce, CTLFLAG_RW, 0, "USB CDC-Ethernet"); SYSCTL_INT(_hw_usb_cdce, OID_AUTO, debug, CTLFLAG_RW, &cdce_debug, 0, "Debug level"); +SYSCTL_INT(_hw_usb_cdce, OID_AUTO, interval, CTLFLAG_RW, &cdce_tx_interval, 0, + "NCM transmit interval in ms"); #endif static const struct usb_config cdce_config[CDCE_N_TRANSFER] = { @@ -192,7 +195,7 @@ .if_index = 0, .frames = CDCE_NCM_TX_FRAMES_MAX, .bufsize = (CDCE_NCM_TX_FRAMES_MAX * CDCE_NCM_TX_MAXLEN), - .flags = {.pipe_bof = 1,.force_short_xfer = 1,}, + .flags = {.pipe_bof = 1,}, .callback = cdce_ncm_bulk_write_callback, .timeout = 10000, /* 10 seconds */ .usb_mode = USB_MODE_DUAL, /* both modes */ @@ -294,9 +297,22 @@ { struct usb_ncm_parameters temp; struct usb_device_request req; - uDWord value; + struct usb_ncm_func_descriptor *ufd; + uint8_t value[8]; int err; + ufd = usbd_find_descriptor(sc->sc_ue.ue_udev, NULL, + sc->sc_ifaces_index[1], UDESC_CS_INTERFACE, 0 - 1, + UCDC_NCM_FUNC_DESC_SUBTYPE, 0 - 1); + + /* verify length of NCM functional descriptor */ + if (ufd != NULL) { + if (ufd->bLength < sizeof(*ufd)) + ufd = NULL; + else + DPRINTFN(1, "Found NCM functional descriptor.\n"); + } + req.bmRequestType = UT_READ_CLASS_INTERFACE; req.bRequest = UCDC_NCM_GET_NTB_PARAMETERS; USETW(req.wValue, 0); @@ -317,17 +333,19 @@ sc->sc_ncm.tx_remainder = UGETW(temp.wNdpOutPayloadRemainder); sc->sc_ncm.tx_modulus = UGETW(temp.wNdpOutDivisor); sc->sc_ncm.tx_struct_align = UGETW(temp.wNdpOutAlignment); + sc->sc_ncm.tx_nframe = UGETW(temp.wNtbOutMaxDatagrams); } else { sc->sc_ncm.rx_max = UGETDW(temp.dwNtbOutMaxSize); sc->sc_ncm.tx_max = UGETDW(temp.dwNtbInMaxSize); sc->sc_ncm.tx_remainder = UGETW(temp.wNdpInPayloadRemainder); sc->sc_ncm.tx_modulus = UGETW(temp.wNdpInDivisor); sc->sc_ncm.tx_struct_align = UGETW(temp.wNdpInAlignment); + sc->sc_ncm.tx_nframe = UGETW(temp.wNtbOutMaxDatagrams); } /* Verify maximum receive length */ - if (err || (sc->sc_ncm.rx_max < 32) || + if ((sc->sc_ncm.rx_max < 32) || (sc->sc_ncm.rx_max > CDCE_NCM_RX_MAXLEN)) { DPRINTFN(1, "Using default maximum receive length\n"); sc->sc_ncm.rx_max = CDCE_NCM_RX_MAXLEN; @@ -335,7 +353,7 @@ /* Verify maximum transmit length */ - if (err || (sc->sc_ncm.tx_max < 32) || + if ((sc->sc_ncm.tx_max < 32) || (sc->sc_ncm.tx_max > CDCE_NCM_TX_MAXLEN)) { DPRINTFN(1, "Using default maximum transmit length\n"); sc->sc_ncm.tx_max = CDCE_NCM_TX_MAXLEN; @@ -347,7 +365,7 @@ * - not greater than the maximum transmit length * - not less than four bytes */ - if (err || (sc->sc_ncm.tx_struct_align < 4) || + if ((sc->sc_ncm.tx_struct_align < 4) || (sc->sc_ncm.tx_struct_align != ((-sc->sc_ncm.tx_struct_align) & sc->sc_ncm.tx_struct_align)) || (sc->sc_ncm.tx_struct_align >= sc->sc_ncm.tx_max)) { @@ -361,7 +379,7 @@ * - not greater than the maximum transmit length * - not less than four bytes */ - if (err || (sc->sc_ncm.tx_modulus < 4) || + if ((sc->sc_ncm.tx_modulus < 4) || (sc->sc_ncm.tx_modulus != ((-sc->sc_ncm.tx_modulus) & sc->sc_ncm.tx_modulus)) || (sc->sc_ncm.tx_modulus >= sc->sc_ncm.tx_max)) { @@ -371,11 +389,30 @@ /* Verify that the payload remainder */ - if (err || (sc->sc_ncm.tx_remainder >= sc->sc_ncm.tx_modulus)) { + if ((sc->sc_ncm.tx_remainder >= sc->sc_ncm.tx_modulus)) { DPRINTFN(1, "Using default transmit remainder: 0 bytes\n"); sc->sc_ncm.tx_remainder = 0; } + /* + * Offset the TX remainder so that IP packet payload starts at + * the tx_modulus. This is not too clear in the specification. + */ + + sc->sc_ncm.tx_remainder = + (sc->sc_ncm.tx_remainder - ETHER_HDR_LEN) & + (sc->sc_ncm.tx_modulus - 1); + + /* Verify max datagrams */ + + if ((sc->sc_ncm.tx_nframe == 0) || + (sc->sc_ncm.tx_nframe > (CDCE_NCM_SUBFRAMES_MAX - 1))) { + DPRINTFN(1, "Using default max " + "subframes: %u units\n", CDCE_NCM_SUBFRAMES_MAX - 1); + /* need to reserve one entry for zero padding */ + sc->sc_ncm.tx_nframe = (CDCE_NCM_SUBFRAMES_MAX - 1); + } + /* Additional configuration, will fail in device side mode, which is OK. */ req.bmRequestType = UT_WRITE_CLASS_INTERFACE; @@ -383,8 +420,17 @@ USETW(req.wValue, 0); req.wIndex[0] = sc->sc_ifaces_index[1]; req.wIndex[1] = 0; - USETW(req.wLength, 4); - USETDW(value, sc->sc_ncm.rx_max); + + if ((ufd != NULL) && + (ufd->bmNetworkCapabilities & UCDC_NCM_CAP_MAX_DGRAM)) { + USETW(req.wLength, 8); + USETDW(value, sc->sc_ncm.rx_max); + USETW(value + 4, (CDCE_NCM_SUBFRAMES_MAX - 1)); + USETW(value + 6, 0); + } else { + USETW(req.wLength, 4); + USETDW(value, sc->sc_ncm.rx_max); + } err = usbd_do_request_flags(sc->sc_ue.ue_udev, NULL, &req, &value, 0, NULL, 1000 /* ms */); @@ -567,7 +613,7 @@ } else { - bzero(sc->sc_ue.ue_eaddr, sizeof(sc->sc_ue.ue_eaddr)); + memset(sc->sc_ue.ue_eaddr, 0, sizeof(sc->sc_ue.ue_eaddr)); for (i = 0; i != (ETHER_ADDR_LEN * 2); i++) { @@ -983,6 +1029,18 @@ } #if CDCE_HAVE_NCM +static void +cdce_ncm_tx_zero(struct usb_page_cache *pc, + uint32_t start, uint32_t end) +{ + if (start >= CDCE_NCM_TX_MAXLEN) + return; + if (end > CDCE_NCM_TX_MAXLEN) + end = CDCE_NCM_TX_MAXLEN; + + usbd_frame_zero(pc, start, end - start); +} + static uint8_t cdce_ncm_fill_tx_frames(struct usb_xfer *xfer, uint8_t index) { @@ -993,7 +1051,8 @@ uint32_t rem; uint32_t offset; uint32_t last_offset; - uint32_t n; + uint16_t n; + uint8_t retval; usbd_xfer_set_frame_offset(xfer, index * CDCE_NCM_TX_MAXLEN, index); @@ -1003,11 +1062,17 @@ /* Store last valid offset before alignment */ last_offset = offset; - /* Align offset correctly */ - offset = sc->sc_ncm.tx_remainder - - ((0UL - offset) & (0UL - sc->sc_ncm.tx_modulus)); + /* Align offset */ + offset = CDCE_NCM_ALIGN(sc->sc_ncm.tx_remainder, + offset, sc->sc_ncm.tx_modulus); + + /* Zero pad */ + cdce_ncm_tx_zero(pc, last_offset, offset); + + /* buffer full */ + retval = 2; - for (n = 0; n != CDCE_NCM_SUBFRAMES_MAX; n++) { + for (n = 0; n != sc->sc_ncm.tx_nframe; n++) { /* check if end of transmit buffer is reached */ @@ -1020,8 +1085,11 @@ IFQ_DRV_DEQUEUE(&(ifp->if_snd), m); - if (m == NULL) + if (m == NULL) { + /* buffer not full */ + retval = 1; break; + } if (m->m_pkthdr.len > rem) { if (n == 0) { @@ -1047,9 +1115,12 @@ /* Store last valid offset before alignment */ last_offset = offset; - /* Align offset correctly */ - offset = sc->sc_ncm.tx_remainder - - ((0UL - offset) & (0UL - sc->sc_ncm.tx_modulus)); + /* Align offset */ + offset = CDCE_NCM_ALIGN(sc->sc_ncm.tx_remainder, + offset, sc->sc_ncm.tx_modulus); + + /* Zero pad */ + cdce_ncm_tx_zero(pc, last_offset, offset); /* * If there's a BPF listener, bounce a copy @@ -1067,7 +1138,7 @@ } if (n == 0) - return (1); + return (0); rem = (sizeof(sc->sc_ncm.dpt) + (4 * n) + 4); @@ -1079,8 +1150,22 @@ USETW(sc->sc_ncm.dp[n].wFrameIndex, 0); } + offset = last_offset; + + /* Align offset */ + offset = CDCE_NCM_ALIGN(0, offset, CDCE_NCM_TX_MINLEN); + + /* Optimise, save bandwidth and force short termination */ + if (offset >= sc->sc_ncm.tx_max) + offset = sc->sc_ncm.tx_max; + else + offset ++; + + /* Zero pad */ + cdce_ncm_tx_zero(pc, last_offset, offset); + /* set frame length */ - usbd_xfer_set_frame_len(xfer, index, last_offset); + usbd_xfer_set_frame_len(xfer, index, offset); /* Fill out 16-bit header */ sc->sc_ncm.hdr.dwSignature[0] = 'N'; @@ -1088,7 +1173,7 @@ sc->sc_ncm.hdr.dwSignature[2] = 'M'; sc->sc_ncm.hdr.dwSignature[3] = 'H'; USETW(sc->sc_ncm.hdr.wHeaderLength, sizeof(sc->sc_ncm.hdr)); - USETW(sc->sc_ncm.hdr.wBlockLength, last_offset); + USETW(sc->sc_ncm.hdr.wBlockLength, offset); USETW(sc->sc_ncm.hdr.wSequence, sc->sc_ncm.tx_seq); USETW(sc->sc_ncm.hdr.wDptIndex, sizeof(sc->sc_ncm.hdr)); @@ -1106,7 +1191,7 @@ sizeof(sc->sc_ncm.dpt)); usbd_copy_in(pc, sizeof(sc->sc_ncm.hdr) + sizeof(sc->sc_ncm.dpt), &(sc->sc_ncm.dp), sizeof(sc->sc_ncm.dp)); - return (0); + return (retval); } static void @@ -1115,6 +1200,7 @@ struct cdce_softc *sc = usbd_xfer_softc(xfer); struct ifnet *ifp = uether_getifp(&sc->sc_ue); uint16_t x; + uint8_t temp; int actlen; int aframes; @@ -1128,11 +1214,19 @@ case USB_ST_SETUP: for (x = 0; x != CDCE_NCM_TX_FRAMES_MAX; x++) { - if (cdce_ncm_fill_tx_frames(xfer, x)) + temp = cdce_ncm_fill_tx_frames(xfer, x); + if (temp == 0) + break; + if (temp == 1) { + x++; break; + } } if (x != 0) { +#ifdef USB_DEBUG + usbd_xfer_set_interval(xfer, cdce_tx_interval); +#endif usbd_xfer_set_frames(xfer, x); usbd_transfer_submit(xfer); } ==== //depot/projects/usb/src/sys/dev/usb/net/if_cdcereg.h#7 (text+ko) ==== @@ -38,7 +38,8 @@ #define CDCE_FRAMES_MAX 8 /* units */ #define CDCE_IND_SIZE_MAX 32 /* bytes */ -#define CDCE_NCM_TX_MAXLEN 2048UL /* bytes */ +#define CDCE_NCM_TX_MINLEN 512 /* bytes, must be power of two */ +#define CDCE_NCM_TX_MAXLEN (1UL << 14) /* bytes */ #define CDCE_NCM_TX_FRAMES_MAX 8 /* units */ #define CDCE_NCM_RX_MAXLEN (1UL << 14) /* bytes */ @@ -46,6 +47,10 @@ #define CDCE_NCM_SUBFRAMES_MAX 32 /* units */ +#define CDCE_NCM_ALIGN(rem,off,mod) \ + ((uint32_t)(((uint32_t)(rem)) - \ + ((uint32_t)((-(uint32_t)(off)) & (-(uint32_t)(mod)))))) + #ifndef CDCE_HAVE_NCM #define CDCE_HAVE_NCM 1 #endif @@ -68,6 +73,7 @@ uint16_t tx_modulus; uint16_t tx_struct_align; uint16_t tx_seq; + uint16_t tx_nframe; }; struct cdce_softc { ==== //depot/projects/usb/src/sys/dev/usb/usb_cdc.h#16 (text+ko) ==== @@ -241,6 +241,7 @@ #define UCDC_NCM_CAP_ENCAP 0x04 #define UCDC_NCM_CAP_MAX_DATA 0x08 #define UCDC_NCM_CAP_CRCMODE 0x10 +#define UCDC_NCM_CAP_MAX_DGRAM 0x20 } __packed; /* Communications interface specific class request codes */ @@ -276,7 +277,7 @@ uWord wNdpOutDivisor; uWord wNdpOutPayloadRemainder; uWord wNdpOutAlignment; - uWord wReserved26; + uWord wNtbOutMaxDatagrams; } __packed; /* Communications interface specific class notification codes */