leandrof/linux
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

virtio_net: Do not pull payload in skb->head

Browse Source 
Xuan Zhuo reported that commit 3226b158e6 ("net: avoid 32 x truesize
under-estimation for tiny skbs") brought  a ~10% performance drop.

The reason for the performance drop was that GRO was forced
to chain sk_buff (using skb_shinfo(skb)->frag_list), which
uses more memory but also cause packet consumers to go over
a lot of overhead handling all the tiny skbs.

It turns out that virtio_net page_to_skb() has a wrong strategy :
It allocates skbs with GOOD_COPY_LEN (128) bytes in skb->head, then
copies 128 bytes from the page, before feeding the packet to GRO stack.

This was suboptimal before commit 3226b158e6 ("net: avoid 32 x truesize
under-estimation for tiny skbs") because GRO was using 2 frags per MSS,
meaning we were not packing MSS with 100% efficiency.

Fix is to pull only the ethernet header in page_to_skb()

Then, we change virtio_net_hdr_to_skb() to pull the missing
headers, instead of assuming they were already pulled by callers.

This fixes the performance regression, but could also allow virtio_net
to accept packets with more than 128bytes of headers.

Many thanks to Xuan Zhuo for his report, and his tests/help.

Fixes: 3226b158e6 ("net: avoid 32 x truesize under-estimation for tiny skbs")
Reported-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Link: https://www.spinics.net/lists/netdev/msg731397.html
Co-Developed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Eric Dumazet 2021-04-02 06:26:02 -07:00 committed by David S. Miller
parent b25b343db0
commit 0f6925b3e8
2 changed files with 16 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
drivers/net/virtio_net.c
View File

@ -406,9 +406,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
offset += hdr_padded_len; offset += hdr_padded_len;
p += hdr_padded_len; p += hdr_padded_len;
/* Copy all frame if it fits skb->head, otherwise
* we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
*/
if (len <= skb_tailroom(skb))
copy = len; copy = len;
if (copy > skb_tailroom(skb)) else
copy = skb_tailroom(skb); copy = ETH_HLEN + metasize;
skb_put_data(skb, p, copy); skb_put_data(skb, p, copy);
if (metasize) { if (metasize) {

14
include/linux/virtio_net.h
View File

@ -65,14 +65,18 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
skb_reset_mac_header(skb); skb_reset_mac_header(skb);
if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
u16 start = __virtio16_to_cpu(little_endian, hdr->csum_start); u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start);
u16 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset);
u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16));
if (!pskb_may_pull(skb, needed))
return -EINVAL;
if (!skb_partial_csum_set(skb, start, off)) if (!skb_partial_csum_set(skb, start, off))
return -EINVAL; return -EINVAL;
p_off = skb_transport_offset(skb) + thlen; p_off = skb_transport_offset(skb) + thlen;
if (p_off > skb_headlen(skb)) if (!pskb_may_pull(skb, p_off))
return -EINVAL; return -EINVAL;
} else { } else {
/* gso packets without NEEDS_CSUM do not set transport_offset. /* gso packets without NEEDS_CSUM do not set transport_offset.
@ -102,14 +106,14 @@ retry:
} }
p_off = keys.control.thoff + thlen; p_off = keys.control.thoff + thlen;
if (p_off > skb_headlen(skb) || if (!pskb_may_pull(skb, p_off) ||
keys.basic.ip_proto != ip_proto) keys.basic.ip_proto != ip_proto)
return -EINVAL; return -EINVAL;
skb_set_transport_header(skb, keys.control.thoff); skb_set_transport_header(skb, keys.control.thoff);
} else if (gso_type) { } else if (gso_type) {
p_off = thlen; p_off = thlen;
if (p_off > skb_headlen(skb)) if (!pskb_may_pull(skb, p_off))
return -EINVAL; return -EINVAL;
} }
} }