virtio_net: Do not pull payload in skb->head
[ 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>
This commit is contained in:
parent
0d08bbce23
commit
a05fb4ac72
@ -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 = len;
|
/* Copy all frame if it fits skb->head, otherwise
|
||||||
if (copy > skb_tailroom(skb))
|
* we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
|
||||||
copy = skb_tailroom(skb);
|
*/
|
||||||
|
if (len <= skb_tailroom(skb))
|
||||||
|
copy = len;
|
||||||
|
else
|
||||||
|
copy = ETH_HLEN + metasize;
|
||||||
skb_put_data(skb, p, copy);
|
skb_put_data(skb, p, copy);
|
||||||
|
|
||||||
if (metasize) {
|
if (metasize) {
|
||||||
|
@ -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;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user