Am 22.04.2011 22:45, schrieb Hans Petter Selasky:
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
Hi Hans Petter,
Thanks for the tip. I have not thought about a easy solution like that.
I just tested to set
#define CDC_NCM_NTB_MAX_SIZE_TX 16384 /* bytes */
to 16383 bytes.
I have to say that I am not quite familar with the DMA engines of USB
chipsets. But is it usefull to to pad zeros up to the 16385 bytes if the
USB device has reported a dwNtbOutMaxSize greater than that? Because
within the source code is written that "it would be more efficient for
USB HS mobile device with DMA engine to receive a full size NTB, than
canceling DMA transfer and receiving a short packet." But this will not
be reached, due to the fact that we are not zero padding up to
dwNtbOutMaxSize.
What is the minimum usefull amount of zero padding in that case? Is it
maybe possible to just fill zeros up to the multiple of wMaxPacketSize?
Or to do not use zero padding in case of dwNtbOutMaxSize >
CDC_NCM_NTB_MAX_SIZE_TX?
BR, Matthias
--
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
