Hello,
16.08.2016, 14:02, Felipe Balbi kirjoitti:
>
> Hi again,
>
> 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>
---
drivers/usb/gadget/function/f_ncm.c | 2 --
drivers/usb/gadget/function/u_ether.c | 47 +++++++++++++++++++++++++++++++----
2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 1f4f724..0c4c15f 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1636,8 +1636,6 @@ static struct usb_function_instance *ncm_alloc_inst(void)
return ERR_CAST(net);
}
- opts->net->features |= NETIF_F_SG;
-
config_group_init_type_name(&opts->func_inst.group, "", &ncm_func_type);
return &opts->func_inst;
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index a3f7e7c..81a98fc 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -347,7 +347,8 @@ clean:
rx_submit(dev, req, GFP_ATOMIC);
}
-static int prealloc(struct list_head *list, struct usb_ep *ep, unsigned n)
+static int prealloc(struct list_head *list, struct usb_ep *ep, unsigned n,
+ bool alloc_sg)
{
unsigned i;
struct usb_request *req;
@@ -363,6 +364,14 @@ static int prealloc(struct list_head *list, struct usb_ep *ep, unsigned n)
}
while (i--) {
req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+ if (alloc_sg && req) {
+ req->sg = kmalloc(sizeof(*req->sg) *
+ (MAX_SKB_FRAGS + 2), GFP_ATOMIC);
+ if (!req->sg) {
+ usb_ep_free_request(ep, req);
+ req = NULL;
+ }
+ }
if (!req)
return list_empty(list) ? -ENOMEM : 0;
list_add(&req->list, list);
@@ -376,6 +385,8 @@ extra:
next = req->list.next;
list_del(&req->list);
+ if (req->sg)
+ kfree(req->sg);
usb_ep_free_request(ep, req);
if (next == list)
@@ -391,10 +402,11 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n)
int status;
spin_lock(&dev->req_lock);
- status = prealloc(&dev->tx_reqs, link->in_ep, n);
+ status = prealloc(&dev->tx_reqs, link->in_ep, n,
+ dev->gadget->sg_supported);
if (status < 0)
goto fail;
- status = prealloc(&dev->rx_reqs, link->out_ep, n);
+ status = prealloc(&dev->rx_reqs, link->out_ep, n, false);
if (status < 0)
goto fail;
goto done;
@@ -563,7 +575,18 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
}
length = skb->len;
- req->buf = skb->data;
+ if (dev->gadget->sg_supported) {
+ BUG_ON(!req->sg);
+ sg_init_table(req->sg, MAX_SKB_FRAGS + 2);
+ req->num_sgs = skb_to_sgvec_nomark(skb, req->sg, 0, length);
+ req->buf = NULL;
+ } else {
+ BUG_ON(req->sg);
+ if (skb_linearize(skb))
+ goto drop;
+ req->buf = skb->data;
+ req->num_sgs = 0;
+ }
req->context = skb;
req->complete = tx_complete;
@@ -579,8 +602,16 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
* though any robust network rx path ignores extra padding.
* and some hardware doesn't like to write zlps.
*/
- if (req->zero && !dev->zlp && (length % in->maxpacket) == 0)
+ if (req->zero && !dev->zlp && (length % in->maxpacket) == 0) {
length++;
+ if (req->sg) {
+ static const char padding_byte[1] = {};
+ sg_set_buf(&req->sg[req->num_sgs++], padding_byte, 1);
+ }
+ }
+
+ if (req->num_sgs)
+ sg_mark_end(&req->sg[req->num_sgs - 1]);
req->length = length;
@@ -799,6 +830,8 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g,
SET_NETDEV_DEV(net, &g->dev);
SET_NETDEV_DEVTYPE(net, &gadget_type);
+ net->features |= NETIF_F_SG;
+
status = register_netdev(net);
if (status < 0) {
dev_dbg(&g->dev, "register_netdev failed, %d\n", status);
@@ -853,6 +886,8 @@ struct net_device *gether_setup_name_default(const char *netname)
net->ethtool_ops = &ops;
SET_NETDEV_DEVTYPE(net, &gadget_type);
+ net->features |= NETIF_F_SG;
+
return net;
}
EXPORT_SYMBOL_GPL(gether_setup_name_default);
@@ -1135,6 +1170,8 @@ void gether_disconnect(struct gether *link)
list_del(&req->list);
spin_unlock(&dev->req_lock);
+ if (req->sg)
+ kfree(req->sg);
usb_ep_free_request(link->in_ep, req);
spin_lock(&dev->req_lock);
}
--
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
