On Friday 22 April 2011 16:32:42 Matthias Benesch wrote:
> Hello,
>
> I am testing the USB-CDC NCM host driver. One issue that was found by
> one USB device stack provider, is that the current implementation is not
> sending ZLP or short packets if the dwNtbOutMaxSize reported by the
> device is greater than CDC_NCM_NTB_MAX_SIZE_TX (=16383 bytes).
>
> As written in the specification, ZLP shall not be sent "If exactly
> dwNtbInMaxSize or dwNtbOutMaxSize bytes are sent, and the size is a
> multiple of wMaxPacketSize for the given pipe".
>
> This requirement is covered in cdc_ncm.c after zero padding (line 858):
>
> /* final zero padding */
> cdc_ncm_zero_fill(skb_out->data, last_offset, offset, ctx->tx_max);
>
> /* store last offset */
> last_offset = offset;
>
> if ((last_offset < ctx->tx_max) && ((last_offset %
> le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0)) {
> /* force short packet */
> *(((u8 *)skb_out->data) + last_offset) = 0;
> last_offset++;
> }
>
> Unfortunately "ctx->tx_max" ist not equal to dwNtbOutMaxSize if
> dwNtbOutMaxSize is greater than CDC_NCM_NTB_MAX_SIZE_TX, because it will
> be reset to CDC_NCM_NTB_MAX_SIZE_TX in that case (see line 220 & 289).
>
> ctx->tx_max = le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize);
>
> ...
>
> /* verify maximum size of transmitted NTB in bytes */
> if ((ctx->tx_max <
> (CDC_NCM_MIN_HDR_SIZE + CDC_NCM_MIN_DATAGRAM_SIZE)) ||
> (ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX)) {
> pr_debug("Using default maximum transmit length=%d\n",
> CDC_NCM_NTB_MAX_SIZE_TX);
> ctx->tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
> }
>
> Therefore the short packet will not be sent although the USB device
> stack is still waiting for dwNtbOutMaxSize bytes of data to be filled by
> the host.
>
> In my opinion possible solutions are:
>
> 1. Set CDC_NCM_NTB_MAX_SIZE_TX to the maximum amount of bytes supported
> by NCM16 (=65535 bytes). In addition to that I disabled the zero
> padding, due to the big overhead of data. With this configuration I
> achieved a maximum stable throughput of 131 Mbit/s of transmitting data.
>
> 2. Enable "urb->transfer_flags |= URB_ZERO_PACKET;" within usbnet.c
> (line 1091) if ZLP shall be sent by the NCM implementation. This could
> be done by an additional flag which shall be set within the cdc_ncm.c if
> needed.
>
> /* don't assume the hardware handles USB_ZERO_PACKET
> * NOTE: strictly conforming cdc-ether devices should expect
> * the ZLP here, but ignore the one-byte packet.
> * NOTE2: CDC NCM specification is different from CDC ECM when
> * handling ZLP/short packets, so cdc_ncm driver will make short
> * packet itself if needed.
> */
> if (length % dev->maxpacket == 0) {
> if (!(info->flags & FLAG_SEND_ZLP)) {
> if (!(info->flags & FLAG_MULTI_PACKET)) {
> urb->transfer_buffer_length++;
> if (skb_tailroom(skb)) {
> skb->data[skb->len] = 0;
> __skb_put(skb, 1);
> }
> }
> } else
> urb->transfer_flags |= URB_ZERO_PACKET;
> }
Hi Matthias,
How about setting
#define CDC_NCM_NTB_MAX_SIZE_TX 16384 /* bytes */
to 16385 bytes. Then if the maximum is exceeded a short packet will always be
sent, without affecting performance with regard to zero padding?
--HPS
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
