Hi, Felipe Balbi <balbi@xxxxxxxxxx> writes: > Jussi Kivilinna <jussi.kivilinna@xxxxxxxxxxx> writes: >>> Felipe Balbi <balbi@xxxxxxxxxx> writes: >>>>>>> Enabling SG allows enabling GSO (generic segmentation offload) feature >>>>>>> of linux networking layer. This increases TCP throughput with NCM >>>>>>> on Cortex-A15+USB3380 based device from 300 Mbit/s to 1.1 Gbit/s. >>>>>>> >>>>>>> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@xxxxxxxxxxx> >>>>>> >>>>>> this is AWESOME!! :-) But here's the thing, any chance we can build this >>>>>> in u_ether.c ? Also, NETIF_F_SG should be conditional on >>>>>> gadget->sg_supported so that we don't break UDCs that don't support >>>>>> sglists. >>>>>> >>>>> >>>>> Actually, no sglists are passed to UDC. Reason why this work >>>>> with minimal changes for NCM is that NCM does tx buffering >>>>> in its wrap function.. 'ncm_wrap_ntb' copies input skbuffs to >>>>> larger skbuff, so enabling SG is only matter of changing that >>>>> skbuff data copy from 'memcpy' to 'skb_copy_bits' (and changing >>>>> CRC calculation work with skbuff fragments). Since NCM already >>>>> does copying, SG can be enabled for NCM without extra overhead. >>>> >>>> aha, understood. Now what if we skip copying altogether? If we have an >>>> sg and a UDC that supports sg (gadget->sg_supported = 1), then we can >>>> avoid copying, right? >>> >>> perhaps this is the crud of the change (still need to check for >>> sg_supported)? >>> >>> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c >>> index 5f562c1ec795..f3497cba32ec 100644 >>> --- a/drivers/usb/gadget/function/u_ether.c >>> +++ b/drivers/usb/gadget/function/u_ether.c >>> @@ -235,7 +235,7 @@ rx_submit(struct eth_dev *dev, struct usb_request *req, gfp_t gfp_flags) >>> */ >>> skb_reserve(skb, NET_IP_ALIGN); >>> >>> - req->buf = skb->data; >>> + req->num_sgs = skb_to_sgvec(skb, req->sg, 0, skb->len); >>> req->length = size; >>> req->complete = rx_complete; >>> req->context = skb; >>> >> >> I did experiment with this, but cannot fully test it without >> sg_supported UDC. Also, wrapper functions need to be reviewed >> to make sure they work with non-linear skbuffs. Seems that EEM >> wrapper had problem with SG, probably because it tries to add >> CRC at the skbuff tail. Below is patch which I've tested for >> now. It applies on top of the earlier NCM SG patch. >> >> -Jussi >> >> --- >> [PATCH] [RFC] usb: gadget: u_ether: enable NETIF_F_SG for networking gadgets >> >> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@xxxxxxxxxxx> > > I tested this patch on top of previous two patches. It works fine for a > while, but then starts failing. No idea yet why it fails. I'll try to > debug this for a while longer. nothing interesting on dwc3's traces, but I see pending softirq: [ 28.978622] IPv6: ADDRCONF(NETDEV_UP): usb0: link is not ready [ 39.633641] configfs-gadget gadget: high-speed config #1: c [ 39.650761] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready [ 39.701440] NOHZ: local_softirq_pending 08 [ 39.721752] NOHZ: local_softirq_pending 08 [ 40.228248] NOHZ: local_softirq_pending 08 [ 40.610264] NOHZ: local_softirq_pending 08 [ 41.233350] NOHZ: local_softirq_pending 08 [ 41.233577] NOHZ: local_softirq_pending 08 [ 41.233729] NOHZ: local_softirq_pending 08 [ 41.241269] NOHZ: local_softirq_pending 08 [ 41.294759] NOHZ: local_softirq_pending 08 [ 41.350932] NOHZ: local_softirq_pending 08 I don't get why our networking gadgets are always so flakey. Guess I should really dig details about the networking stack to fix these bugs once and for all :-( -- balbi
Attachment:
signature.asc
Description: PGP signature
