diff mbox series

[net-next,v3,23/27] virtio_net: xsk: rx: support recv merge mode

Message ID 20231229073108.57778-24-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 Clearly marked for net-next, async
netdev/apply fail Patch does not apply to net-next

Commit Message

Xuan Zhuo Dec. 29, 2023, 7:31 a.m. UTC
The virtnet_xdp_handler() is re-used. But

1. We need to copy data to create skb for XDP_PASS.
2. We need to call xsk_buff_free() to release the buffer.
3. The handle for xdp_buff is difference.

If we pushed this logic into existing receive handle(merge and small),
we would have to maintain code scattered inside merge and small (and big).
So I think it is a good choice for us to put the xsk code into an
independent function.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio/main.c       |  40 ++++--
 drivers/net/virtio/virtio_net.h |   8 ++
 drivers/net/virtio/xsk.c        | 217 ++++++++++++++++++++++++++++++++
 drivers/net/virtio/xsk.h        |   4 +
 4 files changed, 256 insertions(+), 13 deletions(-)

Comments

kernel test robot Dec. 29, 2023, 9:03 p.m. UTC | #1
Hi Xuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mst-vhost/linux-next]
[cannot apply to net-next/main linus/master horms-ipvs/master v6.7-rc7 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio_net-rename-free_old_xmit_skbs-to-free_old_xmit/20231229-155253
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20231229073108.57778-24-xuanzhuo%40linux.alibaba.com
patch subject: [PATCH net-next v3 23/27] virtio_net: xsk: rx: support recv merge mode
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231230/202312300404.1R72Ssbh-lkp@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231230/202312300404.1R72Ssbh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312300404.1R72Ssbh-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/virtio/xsk.c:216:17: warning: variable 'xdp' is uninitialized when used here [-Wuninitialized]
     216 |                 xsk_buff_free(xdp);
         |                               ^~~
   drivers/net/virtio/xsk.c:210:22: note: initialize the variable 'xdp' to silence this warning
     210 |         struct xdp_buff *xdp;
         |                             ^
         |                              = NULL
   1 warning generated.


vim +/xdp +216 drivers/net/virtio/xsk.c

   202	
   203	struct sk_buff *virtnet_receive_xsk_buf(struct virtnet_info *vi, struct virtnet_rq *rq,
   204						void *buf, u32 len,
   205						unsigned int *xdp_xmit,
   206						struct virtnet_rq_stats *stats)
   207	{
   208		struct net_device *dev = vi->dev;
   209		struct sk_buff *skb = NULL;
   210		struct xdp_buff *xdp;
   211	
   212		if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
   213			pr_debug("%s: short packet %i\n", dev->name, len);
   214			DEV_STATS_INC(dev, rx_length_errors);
   215	
 > 216			xsk_buff_free(xdp);
   217			return NULL;
   218		}
   219	
   220		len -= vi->hdr_len;
   221	
   222		u64_stats_add(&stats->bytes, len);
   223	
   224		xdp = buf_to_xdp(vi, rq, buf, len);
   225		if (!xdp)
   226			return NULL;
   227	
   228		if (vi->mergeable_rx_bufs)
   229			skb = virtnet_receive_xsk_merge(dev, vi, rq, xdp, xdp_xmit, stats);
   230	
   231		return skb;
   232	}
   233
kernel test robot Dec. 30, 2023, 1:01 a.m. UTC | #2
Hi Xuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mst-vhost/linux-next]
[cannot apply to net-next/main linus/master horms-ipvs/master v6.7-rc7 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio_net-rename-free_old_xmit_skbs-to-free_old_xmit/20231229-155253
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20231229073108.57778-24-xuanzhuo%40linux.alibaba.com
patch subject: [PATCH net-next v3 23/27] virtio_net: xsk: rx: support recv merge mode
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20231230/202312300808.efv5HGcd-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231230/202312300808.efv5HGcd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312300808.efv5HGcd-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/virtio/main.c: In function 'virtnet_receive':
>> drivers/net/virtio/main.c:1895:27: warning: variable 'pctx' set but not used [-Wunused-but-set-variable]
    1895 |         void *buf, *ctx, *pctx = NULL;
         |                           ^~~~


vim +/pctx +1895 drivers/net/virtio/main.c

  1889	
  1890	static int virtnet_receive(struct virtnet_rq *rq, int budget,
  1891				   unsigned int *xdp_xmit)
  1892	{
  1893		struct virtnet_info *vi = rq->vq->vdev->priv;
  1894		struct virtnet_rq_stats stats = {};
> 1895		void *buf, *ctx, *pctx = NULL;
  1896		unsigned int len;
  1897		int packets = 0;
  1898		int i;
  1899	
  1900		if (rq->xsk.pool) {
  1901			struct sk_buff *skb;
  1902	
  1903			while (packets < budget) {
  1904				buf = virtqueue_get_buf(rq->vq, &len);
  1905				if (!buf)
  1906					break;
  1907	
  1908				skb = virtnet_receive_xsk_buf(vi, rq, buf, len, xdp_xmit, &stats);
  1909				if (skb)
  1910					virtnet_receive_done(vi, rq, skb);
  1911	
  1912				packets++;
  1913			}
  1914		} else {
  1915			if (!vi->big_packets || vi->mergeable_rx_bufs)
  1916				pctx = &ctx;
  1917			else
  1918				ctx = NULL;
  1919	
  1920			while (packets < budget) {
  1921				buf = virtnet_rq_get_buf(rq, &len, &ctx);
  1922				if (!buf)
  1923					break;
  1924	
  1925				receive_buf(vi, rq, buf, len, ctx, xdp_xmit, &stats);
  1926				packets++;
  1927			}
  1928		}
  1929	
  1930		if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
  1931			if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
  1932				spin_lock(&vi->refill_lock);
  1933				if (vi->refill_enabled)
  1934					schedule_delayed_work(&vi->refill, 0);
  1935				spin_unlock(&vi->refill_lock);
  1936			}
  1937		}
  1938	
  1939		u64_stats_set(&stats.packets, packets);
  1940		u64_stats_update_begin(&rq->stats.syncp);
  1941		for (i = 0; i < VIRTNET_RQ_STATS_LEN; i++) {
  1942			size_t offset = virtnet_rq_stats_desc[i].offset;
  1943			u64_stats_t *item, *src;
  1944	
  1945			item = (u64_stats_t *)((u8 *)&rq->stats + offset);
  1946			src = (u64_stats_t *)((u8 *)&stats + offset);
  1947			u64_stats_add(item, u64_stats_read(src));
  1948		}
  1949		u64_stats_update_end(&rq->stats.syncp);
  1950	
  1951		return packets;
  1952	}
  1953
diff mbox series

Patch

diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
index 264ab8aa5da5..b1567f0746e8 100644
--- a/drivers/net/virtio/main.c
+++ b/drivers/net/virtio/main.c
@@ -797,10 +797,10 @@  static void put_xdp_frags(struct xdp_buff *xdp)
 	}
 }
 
-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)
+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)
 {
 	struct xdp_frame *xdpf;
 	int err;
@@ -1892,23 +1892,37 @@  static int virtnet_receive(struct virtnet_rq *rq, int budget,
 {
 	struct virtnet_info *vi = rq->vq->vdev->priv;
 	struct virtnet_rq_stats stats = {};
+	void *buf, *ctx, *pctx = NULL;
 	unsigned int len;
 	int packets = 0;
-	void *buf;
 	int i;
 
-	if (!vi->big_packets || vi->mergeable_rx_bufs) {
-		void *ctx;
+	if (rq->xsk.pool) {
+		struct sk_buff *skb;
+
+		while (packets < budget) {
+			buf = virtqueue_get_buf(rq->vq, &len);
+			if (!buf)
+				break;
+
+			skb = virtnet_receive_xsk_buf(vi, rq, buf, len, xdp_xmit, &stats);
+			if (skb)
+				virtnet_receive_done(vi, rq, skb);
 
-		while (packets < budget &&
-		       (buf = virtnet_rq_get_buf(rq, &len, &ctx))) {
-			receive_buf(vi, rq, buf, len, ctx, xdp_xmit, &stats);
 			packets++;
 		}
 	} else {
-		while (packets < budget &&
-		       (buf = virtnet_rq_get_buf(rq, &len, NULL)) != NULL) {
-			receive_buf(vi, rq, buf, len, NULL, xdp_xmit, &stats);
+		if (!vi->big_packets || vi->mergeable_rx_bufs)
+			pctx = &ctx;
+		else
+			ctx = NULL;
+
+		while (packets < budget) {
+			buf = virtnet_rq_get_buf(rq, &len, &ctx);
+			if (!buf)
+				break;
+
+			receive_buf(vi, rq, buf, len, ctx, xdp_xmit, &stats);
 			packets++;
 		}
 	}
diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
index eaa5da0b0b3c..ac1adb2015db 100644
--- a/drivers/net/virtio/virtio_net.h
+++ b/drivers/net/virtio/virtio_net.h
@@ -326,4 +326,12 @@  void virtnet_rq_free_unused_bufs(struct virtqueue *vq);
 void virtnet_check_sq_full_and_disable(struct virtnet_info *vi,
 				       struct net_device *dev,
 				       struct virtnet_sq *sq);
+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);
+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);
 #endif
diff --git a/drivers/net/virtio/xsk.c b/drivers/net/virtio/xsk.c
index c54cd08e9c77..005bb5f66271 100644
--- a/drivers/net/virtio/xsk.c
+++ b/drivers/net/virtio/xsk.c
@@ -14,6 +14,223 @@  static void sg_fill_dma(struct scatterlist *sg, dma_addr_t addr, u32 len)
 	sg->length = len;
 }
 
+static void xsk_drop_follow_bufs(struct net_device *dev,
+				 struct virtnet_rq *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 struct xdp_buff *buf_to_xdp(struct virtnet_info *vi,
+				   struct virtnet_rq *rq, void *buf, u32 len)
+{
+	struct xdp_buff *xdp;
+	u32 bufsize;
+
+	xdp = (struct xdp_buff *)buf;
+
+	bufsize = xsk_pool_get_rx_frame_size(rq->xsk.pool) + vi->hdr_len;
+
+	if (unlikely(len > bufsize)) {
+		pr_debug("%s: rx error: len %u exceeds truesize %u\n",
+			 vi->dev->name, len, bufsize);
+		DEV_STATS_INC(vi->dev, rx_length_errors);
+		xsk_buff_free(xdp);
+		return NULL;
+	}
+
+	xsk_buff_set_size(xdp, len);
+	xsk_buff_dma_sync_for_cpu(xdp, rq->xsk.pool);
+
+	return xdp;
+}
+
+static int xsk_append_merge_buffer(struct virtnet_info *vi,
+				   struct virtnet_rq *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 *xdp_construct_skb(struct virtnet_rq *rq,
+					 struct xdp_buff *xdp)
+{
+	unsigned int metasize = xdp->data - xdp->data_meta;
+	struct sk_buff *skb;
+	unsigned int size;
+
+	size = xdp->data_end - xdp->data_hard_start;
+	skb = napi_alloc_skb(&rq->napi, size);
+	if (unlikely(!skb)) {
+		xsk_buff_free(xdp);
+		return NULL;
+	}
+
+	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
+
+	size = xdp->data_end - xdp->data_meta;
+	memcpy(__skb_put(skb, size), xdp->data_meta, size);
+
+	if (metasize) {
+		__skb_pull(skb, metasize);
+		skb_metadata_set(skb, metasize);
+	}
+
+	xsk_buff_free(xdp);
+
+	return skb;
+}
+
+static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct virtnet_info *vi,
+						 struct virtnet_rq *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;
+}
+
+struct sk_buff *virtnet_receive_xsk_buf(struct virtnet_info *vi, struct virtnet_rq *rq,
+					void *buf, u32 len,
+					unsigned int *xdp_xmit,
+					struct virtnet_rq_stats *stats)
+{
+	struct net_device *dev = vi->dev;
+	struct sk_buff *skb = NULL;
+	struct xdp_buff *xdp;
+
+	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
+		pr_debug("%s: short packet %i\n", dev->name, len);
+		DEV_STATS_INC(dev, rx_length_errors);
+
+		xsk_buff_free(xdp);
+		return NULL;
+	}
+
+	len -= vi->hdr_len;
+
+	u64_stats_add(&stats->bytes, len);
+
+	xdp = buf_to_xdp(vi, rq, buf, len);
+	if (!xdp)
+		return NULL;
+
+	if (vi->mergeable_rx_bufs)
+		skb = virtnet_receive_xsk_merge(dev, vi, rq, xdp, xdp_xmit, stats);
+
+	return skb;
+}
+
 int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct virtnet_rq *rq,
 			    struct xsk_buff_pool *pool, gfp_t gfp)
 {
diff --git a/drivers/net/virtio/xsk.h b/drivers/net/virtio/xsk.h
index bef41a3f954e..e78fcd0a4946 100644
--- a/drivers/net/virtio/xsk.h
+++ b/drivers/net/virtio/xsk.h
@@ -25,4 +25,8 @@  bool virtnet_xsk_xmit(struct virtnet_sq *sq, struct xsk_buff_pool *pool,
 int virtnet_xsk_wakeup(struct net_device *dev, u32 qid, u32 flag);
 int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct virtnet_rq *rq,
 			    struct xsk_buff_pool *pool, gfp_t gfp);
+struct sk_buff *virtnet_receive_xsk_buf(struct virtnet_info *vi, struct virtnet_rq *rq,
+					void *buf, u32 len,
+					unsigned int *xdp_xmit,
+					struct virtnet_rq_stats *stats);
 #endif