[PATCH v2] usb: xhci: Link TRB must not occur within a USB payload burst
Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB
can only occur at a boundary between underlying USB frames (512 bytes for 480M).
If this isn't done the USB frames aren't formatted correctly and, for example,
the USB3 ethernet ax88179_178a card will stop sending (while still receiving)
when running a netperf tcp transmit test with (say) and 8k buffer.
This should be a candidate for stable, the ax88179_178a driver defaults to
gso and tso enabled so it passes a lot of fragmented skb to the USB stack.
Signed-off-by: David Laight
---
Changes for v2:
1) Only act on bulk endpoints.
While isoc endpoints could suffer from the same problem it is much less
likely and can't be fixed by adding NOP TRBs (they would stop data being
sent in the poll interval).
2) When writing the NOP TRB use the count of TRBs instead of scanning for
the link TRB.
drivers/usb/host/xhci-ring.c | 53 ++--
1 file changed, 51 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5480215..c1342dc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2927,8 +2927,57 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
}
while (1) {
- if (room_on_ring(xhci, ep_ring, num_trbs))
- break;
+ if (room_on_ring(xhci, ep_ring, num_trbs)) {
+ union xhci_trb *trb = ep_ring->enqueue;
+ unsigned int usable = ep_ring->enq_seg->trbs +
+ TRBS_PER_SEGMENT - 1 - trb;
+ u32 nop_cmd;
+
+ /*
+* Section 4.11.7.1 TD Fragments states that a link
+* TRB must only occur at the boundary between
+* data bursts (eg 512 bytes for 480M).
+* While it is possible to split a large fragment
+* we don't know the size yet.
+* Simplest solution is to fill the trb before the
+* LINK with nop commands.
+*/
+ if (num_trbs == 1 || num_trbs <= usable || usable == 0)
+ break;
+
+ if (ep_ring->type != TYPE_BULK)
+ /*
+* While isoc transfers might have a buffer that
+* crosses a 64k boundary it is unlikely.
+* Since we can't add NOPs without generating
+* gaps in the traffic just hope it never
+* happens at the end of the ring.
+* This could be fixed by writing a LINK TRB
+* instead of the first NOP - however the
+* TRB_TYPE_LINK_LE32() calls would all need
+* changing to check the ring length. */
+ break;
+
+ if (num_trbs >= TRBS_PER_SEGMENT) {
+ xhci_err(xhci, "Too many fragments %d, max
%d\n",
+ num_trbs, TRBS_PER_SEGMENT - 1);
+ return -ENOMEM;
+ }
+
+ nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
+ ep_ring->cycle_state);
+ ep_ring->num_trbs_free -= usable;
+ do {
+ trb->generic.field[0] = 0;
+ trb->generic.field[1] = 0;
+ trb->generic.field[2] = 0;
+ trb->generic.field[3] = nop_cmd;
+ trb++;
+ } while (--usable);
+ ep_ring->enqueue = trb;
+ if (room_on_ring(xhci, ep_ring, num_trbs))
+ break;
+ }
if (ep_ring == xhci->cmd_ring) {
xhci_err(xhci, "Do not support expand command ring\n");
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: xhci: Link TRB must not occur within a USB payload burst
On Mon, Nov 11, 2013 at 12:26:54PM -, David Laight wrote:
>
> Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB
> can only occur at a boundary between underlying USB frames (512 bytes for
> 480M).
>
> If this isn't done the USB frames aren't formatted correctly and, for example,
> the USB3 ethernet ax88179_178a card will stop sending (while still receiving)
> when running a netperf tcp transmit test with (say) and 8k buffer.
Can you send the exact command line you used to cause the stall? I'd
like to reproduce this with my USB 3.0 ethernet adapter.
Thanks,
Sarah Sharp
>
> This should be a candidate for stable, the ax88179_178a driver defaults to
> gso and tso enabled so it passes a lot of fragmented skb to the USB stack.
>
> Signed-off-by: David Laight
> ---
>
> Changes for v2:
>
> 1) Only act on bulk endpoints.
>While isoc endpoints could suffer from the same problem it is much less
>likely and can't be fixed by adding NOP TRBs (they would stop data being
>sent in the poll interval).
>
> 2) When writing the NOP TRB use the count of TRBs instead of scanning for
>the link TRB.
>
> drivers/usb/host/xhci-ring.c | 53
> ++--
> 1 file changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5480215..c1342dc 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2927,8 +2927,57 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
> xhci_ring *ep_ring,
> }
>
> while (1) {
> - if (room_on_ring(xhci, ep_ring, num_trbs))
> - break;
> + if (room_on_ring(xhci, ep_ring, num_trbs)) {
> + union xhci_trb *trb = ep_ring->enqueue;
> + unsigned int usable = ep_ring->enq_seg->trbs +
> + TRBS_PER_SEGMENT - 1 - trb;
> + u32 nop_cmd;
> +
> + /*
> + * Section 4.11.7.1 TD Fragments states that a link
> + * TRB must only occur at the boundary between
> + * data bursts (eg 512 bytes for 480M).
> + * While it is possible to split a large fragment
> + * we don't know the size yet.
> + * Simplest solution is to fill the trb before the
> + * LINK with nop commands.
> + */
> + if (num_trbs == 1 || num_trbs <= usable || usable == 0)
> + break;
> +
> + if (ep_ring->type != TYPE_BULK)
> + /*
> + * While isoc transfers might have a buffer that
> + * crosses a 64k boundary it is unlikely.
> + * Since we can't add NOPs without generating
> + * gaps in the traffic just hope it never
> + * happens at the end of the ring.
> + * This could be fixed by writing a LINK TRB
> + * instead of the first NOP - however the
> + * TRB_TYPE_LINK_LE32() calls would all need
> + * changing to check the ring length. */
> + break;
> +
> + if (num_trbs >= TRBS_PER_SEGMENT) {
> + xhci_err(xhci, "Too many fragments %d, max
> %d\n",
> + num_trbs, TRBS_PER_SEGMENT - 1);
> + return -ENOMEM;
> + }
> +
> + nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
> + ep_ring->cycle_state);
> + ep_ring->num_trbs_free -= usable;
> + do {
> + trb->generic.field[0] = 0;
> + trb->generic.field[1] = 0;
> + trb->generic.field[2] = 0;
> + trb->generic.field[3] = nop_cmd;
> + trb++;
> + } while (--usable);
> + ep_ring->enqueue = trb;
> + if (room_on_ring(xhci, ep_ring, num_trbs))
> + break;
> + }
>
> if (ep_ring == xhci->cmd_ring) {
> xhci_err(xhci, "Do not support expand command ring\n");
> --
> 1.8.1.2
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] usb: xhci: Link TRB must not occur within a USB payload burst
> From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] > > On Mon, Nov 11, 2013 at 12:26:54PM -, David Laight wrote: > > > > Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB > > can only occur at a boundary between underlying USB frames (512 bytes for > > 480M). > > > > If this isn't done the USB frames aren't formatted correctly and, for > > example, > > the USB3 ethernet ax88179_178a card will stop sending (while still > > receiving) > > when running a netperf tcp transmit test with (say) and 8k buffer. > > Can you send the exact command line you used to cause the stall? I'd > like to reproduce this with my USB 3.0 ethernet adapter. Make sure you are running 3.12.0 or later. The usbnet code to support segmentation isn't in anything much earlier and the as88179_178a driver needs to have TSO enabled. I'm using netperf 2.6 (compiled from source) running the netserver on the system with the USB ethernet. The ethernet is directly connected to an e1000 PCIe card on the other system (in a random subnet). The control connection is using the main LAN. I'm running netperf from a script that can loop through various parameters. The TCP receive test should be from: control="main LAN address of remote system" target="USB3 address of remote system" output=REQUEST_SIZE,PROTOCOL,ELAPSED_TIME,TRANSACTION_RATE,LOCAL_SEND_THROUGHPUT,LOCAL_RECV_THROUGHPUT,REMOTE_RECV_THROUGHPUT,THROUGHPUT_UNITS netperf -l 60 -f B -D5 -t omni -H $control -- \ -T tcp -H $target -D -d in -r 8192 -O $output I'd added some diagnostic prints to every TRB setup so I could see the fragment sizes, ring slot addresses and the flags. You should see URB for almost 64k split into 3 or 4 parts. When it stopped it was always just after a LINK TRB that had been mid-TD. The USB packets are still processed, but the TCP packets don't get transmitted. If you have a USB3 analyser I think you'll see an unexpected short packet if you split a data TRB across the link TRB on a non-1k boundary. I then wrote the patch to see if it would make a difference - it did. David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
