Re: [PATCH net] net: avoid 32 x truesize under-estimation for tiny skbs

[Jason Wang]

在 2021/4/2 下午7:00, Xuan Zhuo 写道:
On Fri, 2 Apr 2021 10:52:12 +0800, Jason Wang <jasow...@redhat.com> wrote:
So I wonder something like the following like this help. We know the
frag size, that means, if we know there's sufficient tailroom we can use
build_skb() without reserving dedicated room for skb_shared_info.

Thanks


Do you mean so?

I have also considered this scenario, although build_skb is not always used, but
it is also very good for the right situation.


Something like this. Would you mind to post a formal patch to net-next 
with some perf numbers?

Thanks



Thanks.

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index bb4ea9dbc16b..3db207c67422 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -383,17 +383,12 @@ static struct sk_buff *page_to_skb(struct virtnet_info 
*vi,
  {
        struct sk_buff *skb;
        struct virtio_net_hdr_mrg_rxbuf *hdr;
-       unsigned int copy, hdr_len, hdr_padded_len;
-       char *p;
+       unsigned int copy, hdr_len, hdr_padded_len, shinfo_size;
+       char *p, *hdr_p;

        p = page_address(page) + offset;
+       hdr_p = p;

-       /* copy small packet so we can reuse these pages for small data */
-       skb = napi_alloc_skb(&rq->napi, GOOD_COPY_LEN);
-       if (unlikely(!skb))
-               return NULL;
-
-       hdr = skb_vnet_hdr(skb);

        hdr_len = vi->hdr_len;
        if (vi->mergeable_rx_bufs)
@@ -401,27 +396,57 @@ static struct sk_buff *page_to_skb(struct virtnet_info 
*vi,
        else
                hdr_padded_len = sizeof(struct padded_vnet_hdr);

-       /* hdr_valid means no XDP, so we can copy the vnet header */
-       if (hdr_valid)
-               memcpy(hdr, p, hdr_len);
-
        len -= hdr_len;
        offset += hdr_padded_len;
        p += hdr_padded_len;

-       copy = len;
-       if (copy > skb_tailroom(skb))
-               copy = skb_tailroom(skb);
-       skb_put_data(skb, p, copy);
+       shinfo_size = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+
+       if (truesize - len - hdr_len >= shinfo_size && len > GOOD_COPY_LEN) {
+               skb = build_skb(p, truesize);
+               if (unlikely(!skb))
+                       return NULL;
+
+               skb_put(skb, len);
+
+               /* hdr_valid means no XDP, so we can copy the vnet header */
+               if (hdr_valid) {
+                       hdr = skb_vnet_hdr(skb);
+                       memcpy(hdr, hdr_p, hdr_len);
+               }
+
+               if (metasize) {
+                       __skb_pull(skb, metasize);
+                       skb_metadata_set(skb, metasize);
+               }
+
+               return skb;
+       } else {
+               /* copy small packet so we can reuse these pages for small data 
*/
+               skb = napi_alloc_skb(&rq->napi, GOOD_COPY_LEN);
+               if (unlikely(!skb))
+                       return NULL;
+
+               copy = len;
+               if (copy > skb_tailroom(skb))
+                       copy = skb_tailroom(skb);
+               skb_put_data(skb, p, copy);
+
+               len -= copy;
+               offset += copy;
+       }
+
+       hdr = skb_vnet_hdr(skb);
+
+       /* hdr_valid means no XDP, so we can copy the vnet header */
+       if (hdr_valid)
+               memcpy(hdr, hdr_p, hdr_len);

        if (metasize) {
                __skb_pull(skb, metasize);
                skb_metadata_set(skb, metasize);
        }

-       len -= copy;
-       offset += copy;
-
        if (vi->mergeable_rx_bufs) {
                if (len)
                        skb_add_rx_frag(skb, 0, page, offset, len, truesize);