Re: usbnet transmit path problems
On Thu, 2013-09-12 at 09:56 +0800, Ming Lei wrote: On Thu, Sep 12, 2013 at 12:05 AM, David Laight david.lai...@aculab.com wrote: Patches are always welcome, :-) Indeed, I think your patch, if no better alternatives come up soon, should be taken. None of this is helping me sort out why netperf udp rr tests with burst 19 are losing all the packets at once :-( If you can share your test case, guys in list may help you to troubleshoot it, :-) Indeed. Regards Oliver -- 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: usbnet transmit path problems
On Mon, Sep 16, 2013 at 4:13 PM, Oliver Neukum oli...@neukum.org wrote: On Thu, 2013-09-12 at 09:56 +0800, Ming Lei wrote: On Thu, Sep 12, 2013 at 12:05 AM, David Laight david.lai...@aculab.com wrote: Patches are always welcome, :-) Indeed, I think your patch, if no better alternatives come up soon, should be taken. OK, will submit it later. Thanks, -- Ming Lei -- 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: usbnet transmit path problems
On Wed, Sep 11, 2013 at 8:56 PM, David Laight david.lai...@aculab.com wrote: 2) If 'length % dev-maxpacket == 0' for a multi-fragment packet then the extra byte isn't added correctly (the code probably falls off the end of the scatter-gather list). Indeed. Ming Lei, should usbnet handle this in the sg case or better leave it to the subdriver you introduced this for? Is the ZLP issue a problem with the host or with the target? Sorry, what do you mean the ZLP issue here? I understand Oliver thinks one commit from me may break ZLP handling, are you discussing this problem? If not, could you explain it in a bit detail? If it is a host problem then the necessity comes from the host, but the fix needs to be target dependant. If it is a common target problem then generic code can apply a common fix. All usbnet device should have sent one ZLP in case the size of bulk out transfer can be divided by max packet size, but the one byte transfer might be introduced for avoiding some target problem (can't deal with zlp well), as said by David, see below discussion: http://marc.info/?l=linux-usbm=127067487604112w=2 AFICT there are at least 3 fixes: 1) Extend the ethernet frame by one byte and hope the receiving system doesn't object to the padding. This is probably the only option if tx_fixup() doesn't add a header. 2) Put the ethernet frame length in the header and have the target discard the added pad byte (ax88179_178a.c). 3) Add a second zero-length frame in the same USB data block (ax88172a.c). Why do we need the above 3 fixes? The patch in my last email can fix the problem which is introduced recently, can't it? Only the third requires that tx_fixup() append to the packet. For the other two actual pad can be added by usbnet. IMO, it should be handled by usbnet, could you comment on below patch? Seems excessive to kmalloc() one byte! It isn't strange, many usb drivers have to do that(maybe kmalloc two or three, or four bytes) since the buffer is involved into DMA. If you can't assume that the 'dev' structure itself can be dma'd from allocate the extra byte in the sg list. It is better to always obey rule of DMA-API, so don't do that since one extra kmalloc() per device is needed, even though we can allocate only one global buffer for this purpose. Thanks, -- Ming Lei -- 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: usbnet transmit path problems
On Wed, Sep 11, 2013 at 8:56 PM, David Laight david.lai...@aculab.com wrote: 2) If 'length % dev-maxpacket == 0' for a multi-fragment packet then the extra byte isn't added correctly (the code probably falls off the end of the scatter-gather list). Indeed. Ming Lei, should usbnet handle this in the sg case or better leave it to the subdriver you introduced this for? Is the ZLP issue a problem with the host or with the target? Sorry, what do you mean the ZLP issue here? I understand Oliver thinks one commit from me may break ZLP handling, are you discussing this problem? If not, could you explain it in a bit detail? I was thinking of the general ZLP problem. If it is a host problem then the necessity comes from the host, but the fix needs to be target dependant. If it is a common target problem then generic code can apply a common fix. All usbnet device should have sent one ZLP in case the size of bulk out transfer can be divided by max packet size, but the one byte transfer might be introduced for avoiding some target problem (can't deal with zlp well), as said by David, see below discussion: http://marc.info/?l=linux-usbm=127067487604112w=2 AFAICT the code avoids sending a zero length packet (that would terminate a USB bulk transfer packet) by increasing the length of the bulk packet by (at least) one byte. AFICT there are at least 3 fixes: 1) Extend the ethernet frame by one byte and hope the receiving system doesn't object to the padding. This is probably the only option if tx_fixup() doesn't add a header. 2) Put the ethernet frame length in the header and have the target discard the added pad byte (ax88179_178a.c). 3) Add a second zero-length frame in the same USB data block (ax88172a.c). Why do we need the above 3 fixes? The patch in my last email can fix the problem which is introduced recently, can't it? I meant there are 3 ways of avoiding the ZLP, each driver will pick one of them. I've just looked at all the drivers in net/usb. It doesn't look like they all handle fragmented skb, shared skb, or ZLP properly. A lot of common code could be removed if usbnet knew the size of the header and allocated it before calling tx_fixup(). None of this is helping me sort out why netperf udp rr tests with burst 19 are losing all the packets at once :-( 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
Re: usbnet transmit path problems
On Thu, Sep 12, 2013 at 12:05 AM, David Laight david.lai...@aculab.com wrote: On Wed, Sep 11, 2013 at 8:56 PM, David Laight david.lai...@aculab.com wrote: 2) If 'length % dev-maxpacket == 0' for a multi-fragment packet then the extra byte isn't added correctly (the code probably falls off the end of the scatter-gather list). Indeed. Ming Lei, should usbnet handle this in the sg case or better leave it to the subdriver you introduced this for? Is the ZLP issue a problem with the host or with the target? Sorry, what do you mean the ZLP issue here? I understand Oliver thinks one commit from me may break ZLP handling, are you discussing this problem? If not, could you explain it in a bit detail? I was thinking of the general ZLP problem. IMO the current approach should be general enough. If it is a host problem then the necessity comes from the host, but the fix needs to be target dependant. If it is a common target problem then generic code can apply a common fix. All usbnet device should have sent one ZLP in case the size of bulk out transfer can be divided by max packet size, but the one byte transfer might be introduced for avoiding some target problem (can't deal with zlp well), as said by David, see below discussion: http://marc.info/?l=linux-usbm=127067487604112w=2 AFAICT the code avoids sending a zero length packet (that would terminate a USB bulk transfer packet) by increasing the length of the bulk packet by (at least) one byte. No, the code on the link supports URB_ZERO_PACKET which should be the decent approach for the problem. AFICT there are at least 3 fixes: 1) Extend the ethernet frame by one byte and hope the receiving system doesn't object to the padding. This is what usbnet is doing at default if both FLAG_SEND_ZLP and FLAG_MULTI_PACKET aren't set. This is probably the only option if tx_fixup() doesn't add a header. 2) Put the ethernet frame length in the header and have the target discard the added pad byte (ax88179_178a.c). That is probably because the target need the padding flag, so that it can skip the one byte padding frame. 3) Add a second zero-length frame in the same USB data block (ax88172a.c). Actually it is a 4byte frame added by the subdriver, instead of adding one zlp. Since the subdriver adds the termination packet by itself, usbnet won't handle the case at all. There is no much difference among the 3 appoaches, all of which take the padding trick, and the difference is that if padding flag is required, and if the padding length is one byte or others, and both are device-dependent. So current approach is still general enough to cover all cases, isn't it? Why do we need the above 3 fixes? The patch in my last email can fix the problem which is introduced recently, can't it? I meant there are 3 ways of avoiding the ZLP, each driver will pick one of them. All the 3 ways are basically same or very similar, and the difference is that drivers may need the padding flag or take different length termination packet. I've just looked at all the drivers in net/usb. It doesn't look like they all handle fragmented skb, shared skb, or ZLP properly. A lot of common code could be removed if usbnet knew the size of the header and allocated it before calling tx_fixup(). Patches are always welcome, :-) None of this is helping me sort out why netperf udp rr tests with burst 19 are losing all the packets at once :-( If you can share your test case, guys in list may help you to troubleshoot it, :-) Thanks, -- Ming Lei -- 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
