On Mon, Apr 12, 2021 at 10:40:20AM +0200, Greg Kroah-Hartman wrote:
From: Eric Dumazet edumazet@google.com
[ Upstream commit 0f6925b3e8da0dbbb52447ca8a8b42b371aac7db ]
Xuan Zhuo reported that commit 3226b158e67c ("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 3226b158e67c ("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: 3226b158e67c ("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 Signed-off-by: Sasha Levin sashal@kernel.org
Note that an issue related to this patch was recently reported. It's quite possible that the root cause is a bug elsewhere in the kernel, but it probably makes sense to defer the backport until we know more ...
drivers/net/virtio_net.c | 10 +++++++--- include/linux/virtio_net.h | 14 +++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index b67460864b3c..d8ee001d8e8e 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -406,9 +406,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, offset += hdr_padded_len; p += hdr_padded_len;
- copy = len;
- if (copy > skb_tailroom(skb))
copy = skb_tailroom(skb);
- /* 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;- else
copy = ETH_HLEN + metasize;if (metasize) { diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index 98775d7fa696..b465f8f3e554 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -65,14 +65,18 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, skb_reset_mac_header(skb); if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
u16 start = __virtio16_to_cpu(little_endian, hdr->csum_start);u16 off = __virtio16_to_cpu(little_endian, hdr->csum_offset);
u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start);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)) return -EINVAL; p_off = skb_transport_offset(skb) + thlen;
if (p_off > skb_headlen(skb))
if (!pskb_may_pull(skb, p_off)) return -EINVAL;@@ -102,14 +106,14 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, } 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) return -EINVAL;skb_set_transport_header(skb, keys.control.thoff); } else if (gso_type) { p_off = thlen;
if (p_off > skb_headlen(skb))
if (!pskb_may_pull(skb, p_off)) return -EINVAL;-- 2.30.2