| Message ID | 20240618075643.24867-10-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio-net: support AF_XDP zero copy | expand |
On Tue, 2024-06-18 at 15:56 +0800, Xuan Zhuo wrote: > Support AF-XDP for merge mode. > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 139 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 139 insertions(+) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 06608d696e2e..cfa106aa8039 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -504,6 +504,10 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > struct net_device *dev, > unsigned int *xdp_xmit, > struct virtnet_rq_stats *stats); > +static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb, > + struct sk_buff *curr_skb, > + struct page *page, void *buf, > + int len, int truesize); > > static bool is_xdp_frame(void *ptr) > { > @@ -1128,6 +1132,139 @@ static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct > } > } > > +static void xsk_drop_follow_bufs(struct net_device *dev, > + struct receive_queue *rq, > + u32 num_buf, > + struct virtnet_rq_stats *stats) > +{ > + struct xdp_buff *xdp; > + u32 len; > + > + while (num_buf-- > 1) { Why do you skip the last buffer? I thought it should be dropped, too?!? Thanks! Paolo
On Thu, 20 Jun 2024 12:37:43 +0200, Paolo Abeni <pabeni@redhat.com> wrote: > On Tue, 2024-06-18 at 15:56 +0800, Xuan Zhuo wrote: > > Support AF-XDP for merge mode. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > --- > > drivers/net/virtio_net.c | 139 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 139 insertions(+) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index 06608d696e2e..cfa106aa8039 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -504,6 +504,10 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > > struct net_device *dev, > > unsigned int *xdp_xmit, > > struct virtnet_rq_stats *stats); > > +static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb, > > + struct sk_buff *curr_skb, > > + struct page *page, void *buf, > > + int len, int truesize); > > > > static bool is_xdp_frame(void *ptr) > > { > > @@ -1128,6 +1132,139 @@ static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct > > } > > } > > > > +static void xsk_drop_follow_bufs(struct net_device *dev, > > + struct receive_queue *rq, > > + u32 num_buf, > > + struct virtnet_rq_stats *stats) > > +{ > > + struct xdp_buff *xdp; > > + u32 len; > > + > > + while (num_buf-- > 1) { > > Why do you skip the last buffer? I thought it should be dropped, too?!? Here, we just need to drop the follow bufs (num_buf - 1). The first one have be dropped. Thanks. > > Thanks! > > Paolo >
On Tue, Jun 18, 2024 at 3:57 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > > Support AF-XDP for merge mode. > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 139 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 139 insertions(+) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 06608d696e2e..cfa106aa8039 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -504,6 +504,10 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > struct net_device *dev, > unsigned int *xdp_xmit, > struct virtnet_rq_stats *stats); > +static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb, > + struct sk_buff *curr_skb, > + struct page *page, void *buf, > + int len, int truesize); > > static bool is_xdp_frame(void *ptr) > { > @@ -1128,6 +1132,139 @@ static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct > } > } > > +static void xsk_drop_follow_bufs(struct net_device *dev, > + struct receive_queue *rq, > + u32 num_buf, > + struct virtnet_rq_stats *stats) > +{ > + struct xdp_buff *xdp; > + u32 len; > + > + while (num_buf-- > 1) { > + xdp = virtqueue_get_buf(rq->vq, &len); > + if (unlikely(!xdp)) { > + pr_debug("%s: rx error: %d buffers missing\n", > + dev->name, num_buf); > + DEV_STATS_INC(dev, rx_length_errors); > + break; > + } > + u64_stats_add(&stats->bytes, len); > + xsk_buff_free(xdp); > + } > +} > + > +static int xsk_append_merge_buffer(struct virtnet_info *vi, > + struct receive_queue *rq, > + struct sk_buff *head_skb, > + u32 num_buf, > + struct virtio_net_hdr_mrg_rxbuf *hdr, > + struct virtnet_rq_stats *stats) > +{ > + struct sk_buff *curr_skb; > + struct xdp_buff *xdp; > + u32 len, truesize; > + struct page *page; > + void *buf; > + > + curr_skb = head_skb; > + > + while (--num_buf) { > + buf = virtqueue_get_buf(rq->vq, &len); > + if (unlikely(!buf)) { > + pr_debug("%s: rx error: %d buffers out of %d missing\n", > + vi->dev->name, num_buf, > + virtio16_to_cpu(vi->vdev, > + hdr->num_buffers)); > + DEV_STATS_INC(vi->dev, rx_length_errors); > + return -EINVAL; > + } > + > + u64_stats_add(&stats->bytes, len); > + > + xdp = buf_to_xdp(vi, rq, buf, len); > + if (!xdp) > + goto err; > + > + buf = napi_alloc_frag(len); So we don't do this for non xsk paths. Any reason we can't reuse the existing codes? Thanks
On Fri, 28 Jun 2024 10:19:44 +0800, Jason Wang <jasowang@redhat.com> wrote: > On Tue, Jun 18, 2024 at 3:57 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > > > > Support AF-XDP for merge mode. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > --- > > drivers/net/virtio_net.c | 139 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 139 insertions(+) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index 06608d696e2e..cfa106aa8039 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -504,6 +504,10 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > > struct net_device *dev, > > unsigned int *xdp_xmit, > > struct virtnet_rq_stats *stats); > > +static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb, > > + struct sk_buff *curr_skb, > > + struct page *page, void *buf, > > + int len, int truesize); > > > > static bool is_xdp_frame(void *ptr) > > { > > @@ -1128,6 +1132,139 @@ static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct > > } > > } > > > > +static void xsk_drop_follow_bufs(struct net_device *dev, > > + struct receive_queue *rq, > > + u32 num_buf, > > + struct virtnet_rq_stats *stats) > > +{ > > + struct xdp_buff *xdp; > > + u32 len; > > + > > + while (num_buf-- > 1) { > > + xdp = virtqueue_get_buf(rq->vq, &len); > > + if (unlikely(!xdp)) { > > + pr_debug("%s: rx error: %d buffers missing\n", > > + dev->name, num_buf); > > + DEV_STATS_INC(dev, rx_length_errors); > > + break; > > + } > > + u64_stats_add(&stats->bytes, len); > > + xsk_buff_free(xdp); > > + } > > +} > > + > > +static int xsk_append_merge_buffer(struct virtnet_info *vi, > > + struct receive_queue *rq, > > + struct sk_buff *head_skb, > > + u32 num_buf, > > + struct virtio_net_hdr_mrg_rxbuf *hdr, > > + struct virtnet_rq_stats *stats) > > +{ > > + struct sk_buff *curr_skb; > > + struct xdp_buff *xdp; > > + u32 len, truesize; > > + struct page *page; > > + void *buf; > > + > > + curr_skb = head_skb; > > + > > + while (--num_buf) { > > + buf = virtqueue_get_buf(rq->vq, &len); > > + if (unlikely(!buf)) { > > + pr_debug("%s: rx error: %d buffers out of %d missing\n", > > + vi->dev->name, num_buf, > > + virtio16_to_cpu(vi->vdev, > > + hdr->num_buffers)); > > + DEV_STATS_INC(vi->dev, rx_length_errors); > > + return -EINVAL; > > + } > > + > > + u64_stats_add(&stats->bytes, len); > > + > > + xdp = buf_to_xdp(vi, rq, buf, len); > > + if (!xdp) > > + goto err; > > + > > + buf = napi_alloc_frag(len); > > So we don't do this for non xsk paths. Any reason we can't reuse the > existing codes? Do you mean this code: while (--num_buf) { int num_skb_frags; -> buf = virtnet_rq_get_buf(rq, &len, &ctx); if (unlikely(!buf)) { pr_debug("%s: rx error: %d buffers out of %d missing\n", dev->name, num_buf, virtio16_to_cpu(vi->vdev, hdr->num_buffers)); DEV_STATS_INC(dev, rx_length_errors); goto err_buf; } u64_stats_add(&stats->bytes, len); page = virt_to_head_page(buf); -> truesize = mergeable_ctx_to_truesize(ctx); -> headroom = mergeable_ctx_to_headroom(ctx); -> tailroom = headroom ? sizeof(struct skb_shared_info) : 0; -> room = SKB_DATA_ALIGN(headroom + tailroom); -> if (unlikely(len > truesize - room)) { -> pr_debug("%s: rx error: len %u exceeds truesize %lu\n", -> dev->name, len, (unsigned long)(truesize - room)); -> DEV_STATS_INC(dev, rx_length_errors); -> goto err_skb; -> } curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page, buf, len, truesize); if (!curr_skb) goto err_skb; } The code lines that are marked are differ. The same logic is separated to function virtnet_skb_append_frag(). So the code is similitude, but we can not merge them. Thanks. > > Thanks >
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 06608d696e2e..cfa106aa8039 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -504,6 +504,10 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, struct net_device *dev, unsigned int *xdp_xmit, struct virtnet_rq_stats *stats); +static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb, + struct sk_buff *curr_skb, + struct page *page, void *buf, + int len, int truesize); static bool is_xdp_frame(void *ptr) { @@ -1128,6 +1132,139 @@ static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct } } +static void xsk_drop_follow_bufs(struct net_device *dev, + struct receive_queue *rq, + u32 num_buf, + struct virtnet_rq_stats *stats) +{ + struct xdp_buff *xdp; + u32 len; + + while (num_buf-- > 1) { + xdp = virtqueue_get_buf(rq->vq, &len); + if (unlikely(!xdp)) { + pr_debug("%s: rx error: %d buffers missing\n", + dev->name, num_buf); + DEV_STATS_INC(dev, rx_length_errors); + break; + } + u64_stats_add(&stats->bytes, len); + xsk_buff_free(xdp); + } +} + +static int xsk_append_merge_buffer(struct virtnet_info *vi, + struct receive_queue *rq, + struct sk_buff *head_skb, + u32 num_buf, + struct virtio_net_hdr_mrg_rxbuf *hdr, + struct virtnet_rq_stats *stats) +{ + struct sk_buff *curr_skb; + struct xdp_buff *xdp; + u32 len, truesize; + struct page *page; + void *buf; + + curr_skb = head_skb; + + while (--num_buf) { + buf = virtqueue_get_buf(rq->vq, &len); + if (unlikely(!buf)) { + pr_debug("%s: rx error: %d buffers out of %d missing\n", + vi->dev->name, num_buf, + virtio16_to_cpu(vi->vdev, + hdr->num_buffers)); + DEV_STATS_INC(vi->dev, rx_length_errors); + return -EINVAL; + } + + u64_stats_add(&stats->bytes, len); + + xdp = buf_to_xdp(vi, rq, buf, len); + if (!xdp) + goto err; + + buf = napi_alloc_frag(len); + if (!buf) { + xsk_buff_free(xdp); + goto err; + } + + memcpy(buf, xdp->data - vi->hdr_len, len); + + xsk_buff_free(xdp); + + page = virt_to_page(buf); + + truesize = len; + + curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page, + buf, len, truesize); + if (!curr_skb) { + put_page(page); + goto err; + } + } + + return 0; + +err: + xsk_drop_follow_bufs(vi->dev, rq, num_buf, stats); + return -EINVAL; +} + +static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct virtnet_info *vi, + struct receive_queue *rq, struct xdp_buff *xdp, + unsigned int *xdp_xmit, + struct virtnet_rq_stats *stats) +{ + struct virtio_net_hdr_mrg_rxbuf *hdr; + struct bpf_prog *prog; + struct sk_buff *skb; + u32 ret, num_buf; + + hdr = xdp->data - vi->hdr_len; + num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers); + + ret = XDP_PASS; + rcu_read_lock(); + prog = rcu_dereference(rq->xdp_prog); + /* TODO: support multi buffer. */ + if (prog && num_buf == 1) + ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats); + rcu_read_unlock(); + + switch (ret) { + case XDP_PASS: + skb = xdp_construct_skb(rq, xdp); + if (!skb) + goto drop_bufs; + + if (xsk_append_merge_buffer(vi, rq, skb, num_buf, hdr, stats)) { + dev_kfree_skb(skb); + goto drop; + } + + return skb; + + case XDP_TX: + case XDP_REDIRECT: + return NULL; + + default: + /* drop packet */ + xsk_buff_free(xdp); + } + +drop_bufs: + xsk_drop_follow_bufs(dev, rq, num_buf, stats); + +drop: + u64_stats_inc(&stats->drops); + return NULL; +} + static struct sk_buff *virtnet_receive_xsk_buf(struct virtnet_info *vi, struct receive_queue *rq, void *buf, u32 len, unsigned int *xdp_xmit, @@ -1154,6 +1291,8 @@ static struct sk_buff *virtnet_receive_xsk_buf(struct virtnet_info *vi, struct r if (!vi->mergeable_rx_bufs) skb = virtnet_receive_xsk_small(dev, vi, rq, xdp, xdp_xmit, stats); + else + skb = virtnet_receive_xsk_merge(dev, vi, rq, xdp, xdp_xmit, stats); return skb; }
Support AF-XDP for merge mode. Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 139 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+)