diff mbox series

[20/33] virtio_net: xsk: introduce virtnet_rq_bind_xsk_pool()

Message ID 20230202110058.130695-21-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

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1 this patch: 3
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang fail Errors and warnings before: 0 this patch: 3
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 1 this patch: 3
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 96 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Xuan Zhuo Feb. 2, 2023, 11 a.m. UTC 
This function is used to bind or unbind xsk pool to virtnet rq.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio/Makefile     |  2 +-
 drivers/net/virtio/main.c       |  8 ++---
 drivers/net/virtio/virtio_net.h | 16 ++++++++++
 drivers/net/virtio/xsk.c        | 56 +++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100644 drivers/net/virtio/xsk.c

Comments

Michael S. Tsirkin Feb. 3, 2023, 8:48 a.m. UTC | #1 
On Thu, Feb 02, 2023 at 07:00:45PM +0800, Xuan Zhuo wrote:
> This function is used to bind or unbind xsk pool to virtnet rq.
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  drivers/net/virtio/Makefile     |  2 +-
>  drivers/net/virtio/main.c       |  8 ++---
>  drivers/net/virtio/virtio_net.h | 16 ++++++++++
>  drivers/net/virtio/xsk.c        | 56 +++++++++++++++++++++++++++++++++
>  4 files changed, 76 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/net/virtio/xsk.c
> 
> diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> index 15ed7c97fd4f..8c2a884d2dba 100644
> --- a/drivers/net/virtio/Makefile
> +++ b/drivers/net/virtio/Makefile
> @@ -5,4 +5,4 @@
>  
>  obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
>  
> -virtio_net-y := main.o
> +virtio_net-y := main.o xsk.o
> diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> index 049a3bb9d88d..0ee23468b795 100644
> --- a/drivers/net/virtio/main.c
> +++ b/drivers/net/virtio/main.c
> @@ -110,7 +110,6 @@ struct padded_vnet_hdr {
>  	char padding[12];
>  };
>  
> -static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
>  static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
>  
>  static void *xdp_to_ptr(struct xdp_frame *ptr)
> @@ -1351,8 +1350,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
>   * before we're receiving packets, or from refill_work which is
>   * careful to disable receiving (using napi_disable).
>   */
> -static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
> -			  gfp_t gfp)
> +bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp)
>  {
>  	int err;
>  	bool oom;
> @@ -1388,7 +1386,7 @@ static void skb_recv_done(struct virtqueue *rvq)
>  	virtqueue_napi_schedule(&rq->napi, rvq);
>  }
>  
> -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> +void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
>  {
>  	napi_enable(napi);
>  
> @@ -3284,7 +3282,7 @@ static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf)
>  		xdp_return_frame(ptr_to_xdp(buf));
>  }
>  
> -static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
> +void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)

If you are making this an API now you better document
what it does. Same applies to other stuff you are
making non-static.


>  {
>  	struct virtnet_info *vi = vq->vdev->priv;
>  	int i = vq2rxq(vq);
> diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> index b46f083a630a..4a7633714802 100644
> --- a/drivers/net/virtio/virtio_net.h
> +++ b/drivers/net/virtio/virtio_net.h
> @@ -168,6 +168,12 @@ struct send_queue {
>  
>  	/* Record whether sq is in reset state. */
>  	bool reset;
> +
> +	struct {
> +		struct xsk_buff_pool __rcu *pool;
> +
> +		dma_addr_t hdr_dma_address;
> +	} xsk;
>  };
>  
>  /* Internal representation of a receive virtqueue */
> @@ -200,6 +206,13 @@ struct receive_queue {
>  	char name[16];
>  
>  	struct xdp_rxq_info xdp_rxq;
> +
> +	struct {
> +		struct xsk_buff_pool __rcu *pool;
> +
> +		/* xdp rxq used by xsk */
> +		struct xdp_rxq_info xdp_rxq;
> +	} xsk;
>  };
>  
>  static inline bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
> @@ -274,4 +287,7 @@ int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
>  			unsigned int *xdp_xmit,
>  			struct virtnet_rq_stats *stats);
>  int virtnet_tx_reset(struct virtnet_info *vi, struct send_queue *sq);
> +bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp);
> +void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi);
> +void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
>  #endif
> diff --git a/drivers/net/virtio/xsk.c b/drivers/net/virtio/xsk.c
> new file mode 100644
> index 000000000000..e01ff2abea11
> --- /dev/null
> +++ b/drivers/net/virtio/xsk.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * virtio-net xsk
> + */
> +
> +#include "virtio_net.h"
> +
> +static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queue *rq,
> +				    struct xsk_buff_pool *pool, struct net_device *dev)

This static function is unused after this patch, so compiler will
complain. Yes it's just a warning but still not nice.


> +{
> +	bool running = netif_running(vi->dev);
> +	int err, qindex;
> +
> +	qindex = rq - vi->rq;
> +
> +	if (pool) {
> +		err = xdp_rxq_info_reg(&rq->xsk.xdp_rxq, dev, qindex, rq->napi.napi_id);
> +		if (err < 0)
> +			return err;
> +
> +		err = xdp_rxq_info_reg_mem_model(&rq->xsk.xdp_rxq,
> +						 MEM_TYPE_XSK_BUFF_POOL, NULL);
> +		if (err < 0) {
> +			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> +			return err;
> +		}
> +
> +		xsk_pool_set_rxq_info(pool, &rq->xsk.xdp_rxq);
> +	} else {
> +		xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> +	}
> +
> +	if (running)
> +		napi_disable(&rq->napi);
> +
> +	err = virtqueue_reset(rq->vq, virtnet_rq_free_unused_buf);
> +	if (err)
> +		netdev_err(vi->dev, "reset rx fail: rx queue index: %d err: %d\n", qindex, err);
> +
> +	if (pool) {
> +		if (err)
> +			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> +		else
> +			rcu_assign_pointer(rq->xsk.pool, pool);
> +	} else {
> +		rcu_assign_pointer(rq->xsk.pool, NULL);
> +	}
> +
> +	if (!try_fill_recv(vi, rq, GFP_KERNEL))
> +		schedule_delayed_work(&vi->refill, 0);
> +
> +	if (running)
> +		virtnet_napi_enable(rq->vq, &rq->napi);
> +
> +	return err;
> +}
> -- 
> 2.32.0.3.g01195cf9f
Xuan Zhuo Feb. 3, 2023, 8:52 a.m. UTC | #2 
On Fri, 3 Feb 2023 03:48:33 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Feb 02, 2023 at 07:00:45PM +0800, Xuan Zhuo wrote:
> > This function is used to bind or unbind xsk pool to virtnet rq.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  drivers/net/virtio/Makefile     |  2 +-
> >  drivers/net/virtio/main.c       |  8 ++---
> >  drivers/net/virtio/virtio_net.h | 16 ++++++++++
> >  drivers/net/virtio/xsk.c        | 56 +++++++++++++++++++++++++++++++++
> >  4 files changed, 76 insertions(+), 6 deletions(-)
> >  create mode 100644 drivers/net/virtio/xsk.c
> >
> > diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> > index 15ed7c97fd4f..8c2a884d2dba 100644
> > --- a/drivers/net/virtio/Makefile
> > +++ b/drivers/net/virtio/Makefile
> > @@ -5,4 +5,4 @@
> >
> >  obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
> >
> > -virtio_net-y := main.o
> > +virtio_net-y := main.o xsk.o
> > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > index 049a3bb9d88d..0ee23468b795 100644
> > --- a/drivers/net/virtio/main.c
> > +++ b/drivers/net/virtio/main.c
> > @@ -110,7 +110,6 @@ struct padded_vnet_hdr {
> >  	char padding[12];
> >  };
> >
> > -static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
> >  static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
> >
> >  static void *xdp_to_ptr(struct xdp_frame *ptr)
> > @@ -1351,8 +1350,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> >   * before we're receiving packets, or from refill_work which is
> >   * careful to disable receiving (using napi_disable).
> >   */
> > -static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
> > -			  gfp_t gfp)
> > +bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp)
> >  {
> >  	int err;
> >  	bool oom;
> > @@ -1388,7 +1386,7 @@ static void skb_recv_done(struct virtqueue *rvq)
> >  	virtqueue_napi_schedule(&rq->napi, rvq);
> >  }
> >
> > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > +void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> >  {
> >  	napi_enable(napi);
> >
> > @@ -3284,7 +3282,7 @@ static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf)
> >  		xdp_return_frame(ptr_to_xdp(buf));
> >  }
> >
> > -static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
> > +void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
>
> If you are making this an API now you better document
> what it does. Same applies to other stuff you are
> making non-static.

I agree.

>
>
> >  {
> >  	struct virtnet_info *vi = vq->vdev->priv;
> >  	int i = vq2rxq(vq);
> > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > index b46f083a630a..4a7633714802 100644
> > --- a/drivers/net/virtio/virtio_net.h
> > +++ b/drivers/net/virtio/virtio_net.h
> > @@ -168,6 +168,12 @@ struct send_queue {
> >
> >  	/* Record whether sq is in reset state. */
> >  	bool reset;
> > +
> > +	struct {
> > +		struct xsk_buff_pool __rcu *pool;
> > +
> > +		dma_addr_t hdr_dma_address;
> > +	} xsk;
> >  };
> >
> >  /* Internal representation of a receive virtqueue */
> > @@ -200,6 +206,13 @@ struct receive_queue {
> >  	char name[16];
> >
> >  	struct xdp_rxq_info xdp_rxq;
> > +
> > +	struct {
> > +		struct xsk_buff_pool __rcu *pool;
> > +
> > +		/* xdp rxq used by xsk */
> > +		struct xdp_rxq_info xdp_rxq;
> > +	} xsk;
> >  };
> >
> >  static inline bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
> > @@ -274,4 +287,7 @@ int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> >  			unsigned int *xdp_xmit,
> >  			struct virtnet_rq_stats *stats);
> >  int virtnet_tx_reset(struct virtnet_info *vi, struct send_queue *sq);
> > +bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp);
> > +void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi);
> > +void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
> >  #endif
> > diff --git a/drivers/net/virtio/xsk.c b/drivers/net/virtio/xsk.c
> > new file mode 100644
> > index 000000000000..e01ff2abea11
> > --- /dev/null
> > +++ b/drivers/net/virtio/xsk.c
> > @@ -0,0 +1,56 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * virtio-net xsk
> > + */
> > +
> > +#include "virtio_net.h"
> > +
> > +static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queue *rq,
> > +				    struct xsk_buff_pool *pool, struct net_device *dev)
>
> This static function is unused after this patch, so compiler will
> complain. Yes it's just a warning but still not nice.

Otherwise, we need merge some patches, which will increase the difficulty of
review.

Is there a better way to deal with? Remove Static?

Thanks.


>
>
> > +{
> > +	bool running = netif_running(vi->dev);
> > +	int err, qindex;
> > +
> > +	qindex = rq - vi->rq;
> > +
> > +	if (pool) {
> > +		err = xdp_rxq_info_reg(&rq->xsk.xdp_rxq, dev, qindex, rq->napi.napi_id);
> > +		if (err < 0)
> > +			return err;
> > +
> > +		err = xdp_rxq_info_reg_mem_model(&rq->xsk.xdp_rxq,
> > +						 MEM_TYPE_XSK_BUFF_POOL, NULL);
> > +		if (err < 0) {
> > +			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> > +			return err;
> > +		}
> > +
> > +		xsk_pool_set_rxq_info(pool, &rq->xsk.xdp_rxq);
> > +	} else {
> > +		xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> > +	}
> > +
> > +	if (running)
> > +		napi_disable(&rq->napi);
> > +
> > +	err = virtqueue_reset(rq->vq, virtnet_rq_free_unused_buf);
> > +	if (err)
> > +		netdev_err(vi->dev, "reset rx fail: rx queue index: %d err: %d\n", qindex, err);
> > +
> > +	if (pool) {
> > +		if (err)
> > +			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> > +		else
> > +			rcu_assign_pointer(rq->xsk.pool, pool);
> > +	} else {
> > +		rcu_assign_pointer(rq->xsk.pool, NULL);
> > +	}
> > +
> > +	if (!try_fill_recv(vi, rq, GFP_KERNEL))
> > +		schedule_delayed_work(&vi->refill, 0);
> > +
> > +	if (running)
> > +		virtnet_napi_enable(rq->vq, &rq->napi);
> > +
> > +	return err;
> > +}
> > --
> > 2.32.0.3.g01195cf9f
>
Michael S. Tsirkin Feb. 3, 2023, 9:28 a.m. UTC | #3 
On Fri, Feb 03, 2023 at 04:52:35PM +0800, Xuan Zhuo wrote:
> On Fri, 3 Feb 2023 03:48:33 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Thu, Feb 02, 2023 at 07:00:45PM +0800, Xuan Zhuo wrote:
> > > This function is used to bind or unbind xsk pool to virtnet rq.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > >  drivers/net/virtio/Makefile     |  2 +-
> > >  drivers/net/virtio/main.c       |  8 ++---
> > >  drivers/net/virtio/virtio_net.h | 16 ++++++++++
> > >  drivers/net/virtio/xsk.c        | 56 +++++++++++++++++++++++++++++++++
> > >  4 files changed, 76 insertions(+), 6 deletions(-)
> > >  create mode 100644 drivers/net/virtio/xsk.c
> > >
> > > diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> > > index 15ed7c97fd4f..8c2a884d2dba 100644
> > > --- a/drivers/net/virtio/Makefile
> > > +++ b/drivers/net/virtio/Makefile
> > > @@ -5,4 +5,4 @@
> > >
> > >  obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
> > >
> > > -virtio_net-y := main.o
> > > +virtio_net-y := main.o xsk.o
> > > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > > index 049a3bb9d88d..0ee23468b795 100644
> > > --- a/drivers/net/virtio/main.c
> > > +++ b/drivers/net/virtio/main.c
> > > @@ -110,7 +110,6 @@ struct padded_vnet_hdr {
> > >  	char padding[12];
> > >  };
> > >
> > > -static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
> > >  static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
> > >
> > >  static void *xdp_to_ptr(struct xdp_frame *ptr)
> > > @@ -1351,8 +1350,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> > >   * before we're receiving packets, or from refill_work which is
> > >   * careful to disable receiving (using napi_disable).
> > >   */
> > > -static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
> > > -			  gfp_t gfp)
> > > +bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp)
> > >  {
> > >  	int err;
> > >  	bool oom;
> > > @@ -1388,7 +1386,7 @@ static void skb_recv_done(struct virtqueue *rvq)
> > >  	virtqueue_napi_schedule(&rq->napi, rvq);
> > >  }
> > >
> > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > +void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > >  {
> > >  	napi_enable(napi);
> > >
> > > @@ -3284,7 +3282,7 @@ static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf)
> > >  		xdp_return_frame(ptr_to_xdp(buf));
> > >  }
> > >
> > > -static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
> > > +void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
> >
> > If you are making this an API now you better document
> > what it does. Same applies to other stuff you are
> > making non-static.
> 
> I agree.
> 
> >
> >
> > >  {
> > >  	struct virtnet_info *vi = vq->vdev->priv;
> > >  	int i = vq2rxq(vq);
> > > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > > index b46f083a630a..4a7633714802 100644
> > > --- a/drivers/net/virtio/virtio_net.h
> > > +++ b/drivers/net/virtio/virtio_net.h
> > > @@ -168,6 +168,12 @@ struct send_queue {
> > >
> > >  	/* Record whether sq is in reset state. */
> > >  	bool reset;
> > > +
> > > +	struct {
> > > +		struct xsk_buff_pool __rcu *pool;
> > > +
> > > +		dma_addr_t hdr_dma_address;
> > > +	} xsk;
> > >  };
> > >
> > >  /* Internal representation of a receive virtqueue */
> > > @@ -200,6 +206,13 @@ struct receive_queue {
> > >  	char name[16];
> > >
> > >  	struct xdp_rxq_info xdp_rxq;
> > > +
> > > +	struct {
> > > +		struct xsk_buff_pool __rcu *pool;
> > > +
> > > +		/* xdp rxq used by xsk */
> > > +		struct xdp_rxq_info xdp_rxq;
> > > +	} xsk;
> > >  };
> > >
> > >  static inline bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
> > > @@ -274,4 +287,7 @@ int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> > >  			unsigned int *xdp_xmit,
> > >  			struct virtnet_rq_stats *stats);
> > >  int virtnet_tx_reset(struct virtnet_info *vi, struct send_queue *sq);
> > > +bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp);
> > > +void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi);
> > > +void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
> > >  #endif
> > > diff --git a/drivers/net/virtio/xsk.c b/drivers/net/virtio/xsk.c
> > > new file mode 100644
> > > index 000000000000..e01ff2abea11
> > > --- /dev/null
> > > +++ b/drivers/net/virtio/xsk.c
> > > @@ -0,0 +1,56 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * virtio-net xsk
> > > + */
> > > +
> > > +#include "virtio_net.h"
> > > +
> > > +static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queue *rq,
> > > +				    struct xsk_buff_pool *pool, struct net_device *dev)
> >
> > This static function is unused after this patch, so compiler will
> > complain. Yes it's just a warning but still not nice.
> 
> Otherwise, we need merge some patches, which will increase the difficulty of
> review.
> 
> Is there a better way to deal with? Remove Static?
> 
> Thanks.

In this case review is not made easier because the API does not make
much sense by its own and is undocumented anyway. To review one has to
jump back and forth between multiple patches - that is harder not easier
than a single bigger patch. Others in this thread already commented that
the patches are too small.


> 
> >
> >
> > > +{
> > > +	bool running = netif_running(vi->dev);
> > > +	int err, qindex;
> > > +
> > > +	qindex = rq - vi->rq;
> > > +
> > > +	if (pool) {
> > > +		err = xdp_rxq_info_reg(&rq->xsk.xdp_rxq, dev, qindex, rq->napi.napi_id);
> > > +		if (err < 0)
> > > +			return err;
> > > +
> > > +		err = xdp_rxq_info_reg_mem_model(&rq->xsk.xdp_rxq,
> > > +						 MEM_TYPE_XSK_BUFF_POOL, NULL);
> > > +		if (err < 0) {
> > > +			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> > > +			return err;
> > > +		}
> > > +
> > > +		xsk_pool_set_rxq_info(pool, &rq->xsk.xdp_rxq);
> > > +	} else {
> > > +		xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> > > +	}
> > > +
> > > +	if (running)
> > > +		napi_disable(&rq->napi);
> > > +
> > > +	err = virtqueue_reset(rq->vq, virtnet_rq_free_unused_buf);
> > > +	if (err)
> > > +		netdev_err(vi->dev, "reset rx fail: rx queue index: %d err: %d\n", qindex, err);
> > > +
> > > +	if (pool) {
> > > +		if (err)
> > > +			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
> > > +		else
> > > +			rcu_assign_pointer(rq->xsk.pool, pool);
> > > +	} else {
> > > +		rcu_assign_pointer(rq->xsk.pool, NULL);
> > > +	}
> > > +
> > > +	if (!try_fill_recv(vi, rq, GFP_KERNEL))
> > > +		schedule_delayed_work(&vi->refill, 0);
> > > +
> > > +	if (running)
> > > +		virtnet_napi_enable(rq->vq, &rq->napi);
> > > +
> > > +	return err;
> > > +}
> > > --
> > > 2.32.0.3.g01195cf9f
> >
diff mbox series

Patch

diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 15ed7c97fd4f..8c2a884d2dba 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -5,4 +5,4 @@ 
 
 obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
 
-virtio_net-y := main.o
+virtio_net-y := main.o xsk.o
diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
index 049a3bb9d88d..0ee23468b795 100644
--- a/drivers/net/virtio/main.c
+++ b/drivers/net/virtio/main.c
@@ -110,7 +110,6 @@  struct padded_vnet_hdr {
 	char padding[12];
 };
 
-static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
 static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
 
 static void *xdp_to_ptr(struct xdp_frame *ptr)
@@ -1351,8 +1350,7 @@  static int add_recvbuf_mergeable(struct virtnet_info *vi,
  * before we're receiving packets, or from refill_work which is
  * careful to disable receiving (using napi_disable).
  */
-static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
-			  gfp_t gfp)
+bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp)
 {
 	int err;
 	bool oom;
@@ -1388,7 +1386,7 @@  static void skb_recv_done(struct virtqueue *rvq)
 	virtqueue_napi_schedule(&rq->napi, rvq);
 }
 
-static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
+void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
 {
 	napi_enable(napi);
 
@@ -3284,7 +3282,7 @@  static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf)
 		xdp_return_frame(ptr_to_xdp(buf));
 }
 
-static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
+void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf)
 {
 	struct virtnet_info *vi = vq->vdev->priv;
 	int i = vq2rxq(vq);
diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
index b46f083a630a..4a7633714802 100644
--- a/drivers/net/virtio/virtio_net.h
+++ b/drivers/net/virtio/virtio_net.h
@@ -168,6 +168,12 @@  struct send_queue {
 
 	/* Record whether sq is in reset state. */
 	bool reset;
+
+	struct {
+		struct xsk_buff_pool __rcu *pool;
+
+		dma_addr_t hdr_dma_address;
+	} xsk;
 };
 
 /* Internal representation of a receive virtqueue */
@@ -200,6 +206,13 @@  struct receive_queue {
 	char name[16];
 
 	struct xdp_rxq_info xdp_rxq;
+
+	struct {
+		struct xsk_buff_pool __rcu *pool;
+
+		/* xdp rxq used by xsk */
+		struct xdp_rxq_info xdp_rxq;
+	} xsk;
 };
 
 static inline bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
@@ -274,4 +287,7 @@  int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
 			unsigned int *xdp_xmit,
 			struct virtnet_rq_stats *stats);
 int virtnet_tx_reset(struct virtnet_info *vi, struct send_queue *sq);
+bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, gfp_t gfp);
+void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi);
+void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
 #endif
diff --git a/drivers/net/virtio/xsk.c b/drivers/net/virtio/xsk.c
new file mode 100644
index 000000000000..e01ff2abea11
--- /dev/null
+++ b/drivers/net/virtio/xsk.c
@@ -0,0 +1,56 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * virtio-net xsk
+ */
+
+#include "virtio_net.h"
+
+static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queue *rq,
+				    struct xsk_buff_pool *pool, struct net_device *dev)
+{
+	bool running = netif_running(vi->dev);
+	int err, qindex;
+
+	qindex = rq - vi->rq;
+
+	if (pool) {
+		err = xdp_rxq_info_reg(&rq->xsk.xdp_rxq, dev, qindex, rq->napi.napi_id);
+		if (err < 0)
+			return err;
+
+		err = xdp_rxq_info_reg_mem_model(&rq->xsk.xdp_rxq,
+						 MEM_TYPE_XSK_BUFF_POOL, NULL);
+		if (err < 0) {
+			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
+			return err;
+		}
+
+		xsk_pool_set_rxq_info(pool, &rq->xsk.xdp_rxq);
+	} else {
+		xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
+	}
+
+	if (running)
+		napi_disable(&rq->napi);
+
+	err = virtqueue_reset(rq->vq, virtnet_rq_free_unused_buf);
+	if (err)
+		netdev_err(vi->dev, "reset rx fail: rx queue index: %d err: %d\n", qindex, err);
+
+	if (pool) {
+		if (err)
+			xdp_rxq_info_unreg(&rq->xsk.xdp_rxq);
+		else
+			rcu_assign_pointer(rq->xsk.pool, pool);
+	} else {
+		rcu_assign_pointer(rq->xsk.pool, NULL);
+	}
+
+	if (!try_fill_recv(vi, rq, GFP_KERNEL))
+		schedule_delayed_work(&vi->refill, 0);
+
+	if (running)
+		virtnet_napi_enable(rq->vq, &rq->napi);
+
+	return err;
+}