Re: [PATCH net] net: avoid 32 x truesize under-estimation for tiny skbs
From: Jason Wang <hidden>
Date: 2021-04-06 02:04:53
在 2021/4/2 下午7:00, Xuan Zhuo 写道:
On Fri, 2 Apr 2021 10:52:12 +0800, Jason Wang [off-list ref] wrote:quoted
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. ThanksDo 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
quoted hunk
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);