On 2020/4/27 下午10:32, Jesper Dangaard Brouer wrote:
On Mon, 27 Apr 2020 15:21:02 +0800
Jason Wang<jasowang@xxxxxxxxxx> wrote:
On 2020/4/23 上午12:09, Jesper Dangaard Brouer wrote:
The virtio_net driver is running inside the guest-OS. There are two
XDP receive code-paths in virtio_net, namely receive_small() and
receive_mergeable(). The receive_big() function does not support XDP.
In receive_small() the frame size is available in buflen. The buffer
backing these frames are allocated in add_recvbuf_small() with same
size, except for the headroom, but tailroom have reserved room for
skb_shared_info. The headroom is encoded in ctx pointer as a value.
In receive_mergeable() the frame size is more dynamic. There are two
basic cases: (1) buffer size is based on a exponentially weighted
moving average (see DECLARE_EWMA) of packet length. Or (2) in case
virtnet_get_headroom() have any headroom then buffer size is
PAGE_SIZE. The ctx pointer is this time used for encoding two values;
the buffer len "truesize" and headroom. In case (1) if the rx buffer
size is underestimated, the packet will have been split over more
buffers (num_buf info in virtio_net_hdr_mrg_rxbuf placed in top of
buffer area). If that happens the XDP path does a xdp_linearize_page
operation.
Cc: Jason Wang<jasowang@xxxxxxxxxx>
Signed-off-by: Jesper Dangaard Brouer<brouer@xxxxxxxxxx>
---
drivers/net/virtio_net.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 11f722460513..1df3676da185 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -689,6 +689,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
xdp.data_end = xdp.data + len;
xdp.data_meta = xdp.data;
xdp.rxq = &rq->xdp_rxq;
+ xdp.frame_sz = buflen;
orig_data = xdp.data;
act = bpf_prog_run_xdp(xdp_prog, &xdp);
stats->xdp_packets++;
@@ -797,10 +798,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
int offset = buf - page_address(page);
struct sk_buff *head_skb, *curr_skb;
struct bpf_prog *xdp_prog;
- unsigned int truesize;
+ unsigned int truesize = mergeable_ctx_to_truesize(ctx);
unsigned int headroom = mergeable_ctx_to_headroom(ctx);
- int err;
unsigned int metasize = 0;
+ unsigned int frame_sz;
+ int err;
head_skb = NULL;
stats->bytes += len - vi->hdr_len;
@@ -821,6 +823,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
if (unlikely(hdr->hdr.gso_type))
goto err_xdp;
+ /* Buffers with headroom use PAGE_SIZE as alloc size,
+ * see add_recvbuf_mergeable() + get_mergeable_buf_len()
+ */
+ frame_sz = headroom ? PAGE_SIZE : truesize;
+
/* This happens when rx buffer size is underestimated
* or headroom is not enough because of the buffer
* was refilled before XDP is set. This should only
@@ -834,6 +841,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
page, offset,
VIRTIO_XDP_HEADROOM,
&len);
+ frame_sz = PAGE_SIZE;
Should this be PAGE_SIZE - SKB_DATA_ALIGN(sizeof(struct skb_shared_info))?
No, frame_sz include the SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) length.
Ok, consider mergeable buffer path depends on the truesize which is
encoded in ctx.
It looks to the the calculation in add_recvfbuf_mergeable() is wrong, we
need count both headroom and tailroom there.
We probably need the attached 2 patches to fix this.
(untested, will test it tomorrow).
Thanks
>From c2778eb8ee4b7558bccb53f2fc7f1b0aaf1fcb58 Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@xxxxxxxxxx>
Date: Tue, 28 Apr 2020 11:37:39 +0800
Subject: [PATCH 2/2] virtio-net: fix the XDP truesize calculation for
mergeable buffers
We should not exclude headroom and tailroom when XDP is set. So this
patch fixes this by initializing the truesize from PAGE_SIZE when XDP
is set.
Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
---
drivers/net/virtio_net.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9bdaf2425e6e..f9ba5275e447 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1172,7 +1172,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
char *buf;
void *ctx;
int err;
- unsigned int len, hole;
+ unsigned int len, hole, truesize;
/* Extra tailroom is needed to satisfy XDP's assumption. This
* means rx frags coalescing won't work, but consider we've
@@ -1182,6 +1182,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
if (unlikely(!skb_page_frag_refill(len + room, alloc_frag, gfp)))
return -ENOMEM;
+ truesize = headroom ? PAGE_SIZE: len;
buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
buf += headroom; /* advance address leaving hole at front of pkt */
get_page(alloc_frag->page);
@@ -1193,11 +1194,12 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
* the current buffer.
*/
len += hole;
+ truesize += hole;
alloc_frag->offset += hole;
}
sg_init_one(rq->sg, buf, len);
- ctx = mergeable_len_to_ctx(len, headroom);
+ ctx = mergeable_len_to_ctx(truesize, headroom);
err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
if (err < 0)
put_page(virt_to_head_page(buf));
--
2.20.1
>From 307ac87e823fde059be3bb5a7bdd3ffd3b18521d Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@xxxxxxxxxx>
Date: Tue, 28 Apr 2020 11:31:47 +0800
Subject: [PATCH 1/2] virtio-net: don't reserve space for vnet header for XDP
We tried to reserve space for vnet header before
xdp.data_hard_start. But this is useless since the packet could be
modified by XDP which may invalidate the information stored in the
header and there's no way for XDP to know the existence of the vnet
header currently.
So let's just not reserve space for vnet header in this case.
Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
---
drivers/net/virtio_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 2fe7a3188282..9bdaf2425e6e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -681,8 +681,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
page = xdp_page;
}
- xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len;
- xdp.data = xdp.data_hard_start + xdp_headroom;
+ xdp.data_hard_start = buf + VIRTNET_RX_PAD;
+ xdp.data = xdp.data_hard_start + xdp_headroom + vi->hdr_len;;
xdp_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + len;
xdp.rxq = &rq->xdp_rxq;
@@ -837,7 +837,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
* the descriptor on if we get an XDP_TX return code.
*/
data = page_address(xdp_page) + offset;
- xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
+ xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM;
xdp.data = data + vi->hdr_len;
xdp_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + (len - vi->hdr_len);
--
2.20.1
