[PATCH net-next] vhost/net: align variable names with XDP terminology

Jon Kohler posted 1 patch 9 months ago
drivers/vhost/net.c | 53 ++++++++++++++++++++++-----------------------
1 file changed, 26 insertions(+), 27 deletions(-)
[PATCH net-next] vhost/net: align variable names with XDP terminology
Posted by Jon Kohler 9 months ago
Refactor variable names in vhost_net_build_xdp to align with XDP
terminology, enhancing code clarity and consistency. Additionally,
reorder variables to follow a reverse Christmas tree structure,
improving code organization and readability.

This change introduces no functional modifications.

Signed-off-by: Jon Kohler <jon@nutanix.com>
---
 drivers/vhost/net.c | 53 ++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 7cbfc7d718b3..86db8add92eb 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -665,44 +665,43 @@ static int vhost_net_build_xdp(struct vhost_net_virtqueue *nvq,
 	struct vhost_virtqueue *vq = &nvq->vq;
 	struct vhost_net *net = container_of(vq->dev, struct vhost_net,
 					     dev);
+	int copied, headroom, ret, sock_hlen = nvq->sock_hlen;
+	struct xdp_buff *xdp = &nvq->xdp[nvq->batched_xdp];
 	struct socket *sock = vhost_vq_get_backend(vq);
+	size_t data_len = iov_iter_count(from);
 	struct virtio_net_hdr *gso;
-	struct xdp_buff *xdp = &nvq->xdp[nvq->batched_xdp];
 	struct tun_xdp_hdr *hdr;
-	size_t len = iov_iter_count(from);
-	int headroom = vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0;
-	int buflen = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
-	int pad = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + headroom + nvq->sock_hlen);
-	int sock_hlen = nvq->sock_hlen;
-	void *buf;
-	int copied;
-	int ret;
+	void *hard_start;
+	u32 frame_sz;
 
-	if (unlikely(len < nvq->sock_hlen))
+	if (unlikely(data_len < sock_hlen))
 		return -EFAULT;
 
-	if (SKB_DATA_ALIGN(len + pad) +
-	    SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE)
+	headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
+				  vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
+
+	frame_sz = SKB_HEAD_ALIGN(headroom + data_len);
+
+	if (frame_sz > PAGE_SIZE)
 		return -ENOSPC;
 
-	buflen += SKB_DATA_ALIGN(len + pad);
-	buf = page_frag_alloc_align(&net->pf_cache, buflen, GFP_KERNEL,
-				    SMP_CACHE_BYTES);
-	if (unlikely(!buf))
+	hard_start = page_frag_alloc_align(&net->pf_cache, frame_sz,
+					   GFP_KERNEL, SMP_CACHE_BYTES);
+	if (unlikely(!hard_start))
 		return -ENOMEM;
 
-	copied = copy_from_iter(buf + offsetof(struct tun_xdp_hdr, gso),
+	copied = copy_from_iter(hard_start + offsetof(struct tun_xdp_hdr, gso),
 				sock_hlen, from);
 	if (copied != sock_hlen) {
 		ret = -EFAULT;
 		goto err;
 	}
 
-	hdr = buf;
+	hdr = hard_start;
 	gso = &hdr->gso;
 
 	if (!sock_hlen)
-		memset(buf, 0, pad);
+		memset(hard_start, 0, headroom);
 
 	if ((gso->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
 	    vhost16_to_cpu(vq, gso->csum_start) +
@@ -712,29 +711,29 @@ static int vhost_net_build_xdp(struct vhost_net_virtqueue *nvq,
 			       vhost16_to_cpu(vq, gso->csum_start) +
 			       vhost16_to_cpu(vq, gso->csum_offset) + 2);
 
-		if (vhost16_to_cpu(vq, gso->hdr_len) > len) {
+		if (vhost16_to_cpu(vq, gso->hdr_len) > data_len) {
 			ret = -EINVAL;
 			goto err;
 		}
 	}
 
-	len -= sock_hlen;
-	copied = copy_from_iter(buf + pad, len, from);
-	if (copied != len) {
+	data_len -= sock_hlen;
+	copied = copy_from_iter(hard_start + headroom, data_len, from);
+	if (copied != data_len) {
 		ret = -EFAULT;
 		goto err;
 	}
 
-	xdp_init_buff(xdp, buflen, NULL);
-	xdp_prepare_buff(xdp, buf, pad, len, true);
-	hdr->buflen = buflen;
+	xdp_init_buff(xdp, frame_sz, NULL);
+	xdp_prepare_buff(xdp, hard_start, headroom, data_len, true);
+	hdr->buflen = frame_sz;
 
 	++nvq->batched_xdp;
 
 	return 0;
 
 err:
-	page_frag_free(buf);
+	page_frag_free(hard_start);
 	return ret;
 }
 
-- 
2.43.0
Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
Posted by kernel test robot 9 months ago
Hi Jon,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jon-Kohler/vhost-net-align-variable-names-with-XDP-terminology/20250507-233429
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250507160206.3267692-1-jon%40nutanix.com
patch subject: [PATCH net-next] vhost/net: align variable names with XDP terminology
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20250508/202505081920.FOOj1Z0e-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505081920.FOOj1Z0e-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/202505081920.FOOj1Z0e-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/vhost/net.c:681:28: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first [-Wparentheses]
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:51: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/uapi/linux/const.h:49:41: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                            ^
   drivers/vhost/net.c:681:28: note: place parentheses around the '+' expression to silence this warning
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:51: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/uapi/linux/const.h:49:41: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                            ^
   drivers/vhost/net.c:681:28: note: place parentheses around the '?:' expression to evaluate it first
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:51: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/uapi/linux/const.h:49:41: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                            ^
>> drivers/vhost/net.c:681:28: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first [-Wparentheses]
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:66: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/uapi/linux/const.h:49:47: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                  ^~~~
   drivers/vhost/net.c:681:28: note: place parentheses around the '+' expression to silence this warning
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:66: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/uapi/linux/const.h:49:47: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                  ^~~~
   drivers/vhost/net.c:681:28: note: place parentheses around the '?:' expression to evaluate it first
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:66: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/uapi/linux/const.h:49:47: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                  ^~~~
>> drivers/vhost/net.c:681:28: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first [-Wparentheses]
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:66: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/uapi/linux/const.h:49:58: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                             ^~~~
   drivers/vhost/net.c:681:28: note: place parentheses around the '+' expression to silence this warning
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:66: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/uapi/linux/const.h:49:58: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                             ^~~~
   drivers/vhost/net.c:681:28: note: place parentheses around the '?:' expression to evaluate it first
     680 |         headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
         |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     681 |                                   vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
         |                                   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/skbuff.h:256:33: note: expanded from macro 'SKB_DATA_ALIGN'
     256 | #define SKB_DATA_ALIGN(X)       ALIGN(X, SMP_CACHE_BYTES)
         |                                 ~~~~~~^~~~~~~~~~~~~~~~~~~
   include/vdso/align.h:8:38: note: expanded from macro 'ALIGN'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ~~~~~~~~~~~~~~~~^~~~~~~~
   include/uapi/linux/const.h:48:66: note: expanded from macro '__ALIGN_KERNEL'
      48 | #define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
         |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/uapi/linux/const.h:49:58: note: expanded from macro '__ALIGN_KERNEL_MASK'
      49 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                             ^~~~
   3 warnings generated.


vim +681 drivers/vhost/net.c

   661	
   662	static int vhost_net_build_xdp(struct vhost_net_virtqueue *nvq,
   663				       struct iov_iter *from)
   664	{
   665		struct vhost_virtqueue *vq = &nvq->vq;
   666		struct vhost_net *net = container_of(vq->dev, struct vhost_net,
   667						     dev);
   668		int copied, headroom, ret, sock_hlen = nvq->sock_hlen;
   669		struct xdp_buff *xdp = &nvq->xdp[nvq->batched_xdp];
   670		struct socket *sock = vhost_vq_get_backend(vq);
   671		size_t data_len = iov_iter_count(from);
   672		struct virtio_net_hdr *gso;
   673		struct tun_xdp_hdr *hdr;
   674		void *hard_start;
   675		u32 frame_sz;
   676	
   677		if (unlikely(data_len < sock_hlen))
   678			return -EFAULT;
   679	
   680		headroom = SKB_DATA_ALIGN(VHOST_NET_RX_PAD + sock_hlen +
 > 681					  vhost_sock_xdp(sock) ? XDP_PACKET_HEADROOM : 0);
   682	
   683		frame_sz = SKB_HEAD_ALIGN(headroom + data_len);
   684	
   685		if (frame_sz > PAGE_SIZE)
   686			return -ENOSPC;
   687	
   688		hard_start = page_frag_alloc_align(&net->pf_cache, frame_sz,
   689						   GFP_KERNEL, SMP_CACHE_BYTES);
   690		if (unlikely(!hard_start))
   691			return -ENOMEM;
   692	
   693		copied = copy_from_iter(hard_start + offsetof(struct tun_xdp_hdr, gso),
   694					sock_hlen, from);
   695		if (copied != sock_hlen) {
   696			ret = -EFAULT;
   697			goto err;
   698		}
   699	
   700		hdr = hard_start;
   701		gso = &hdr->gso;
   702	
   703		if (!sock_hlen)
   704			memset(hard_start, 0, headroom);
   705	
   706		if ((gso->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
   707		    vhost16_to_cpu(vq, gso->csum_start) +
   708		    vhost16_to_cpu(vq, gso->csum_offset) + 2 >
   709		    vhost16_to_cpu(vq, gso->hdr_len)) {
   710			gso->hdr_len = cpu_to_vhost16(vq,
   711				       vhost16_to_cpu(vq, gso->csum_start) +
   712				       vhost16_to_cpu(vq, gso->csum_offset) + 2);
   713	
   714			if (vhost16_to_cpu(vq, gso->hdr_len) > data_len) {
   715				ret = -EINVAL;
   716				goto err;
   717			}
   718		}
   719	
   720		data_len -= sock_hlen;
   721		copied = copy_from_iter(hard_start + headroom, data_len, from);
   722		if (copied != data_len) {
   723			ret = -EFAULT;
   724			goto err;
   725		}
   726	
   727		xdp_init_buff(xdp, frame_sz, NULL);
   728		xdp_prepare_buff(xdp, hard_start, headroom, data_len, true);
   729		hdr->buflen = frame_sz;
   730	
   731		++nvq->batched_xdp;
   732	
   733		return 0;
   734	
   735	err:
   736		page_frag_free(hard_start);
   737		return ret;
   738	}
   739	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
Posted by Willem de Bruijn 9 months ago
Jon Kohler wrote:
> Refactor variable names in vhost_net_build_xdp to align with XDP
> terminology, enhancing code clarity and consistency. Additionally,
> reorder variables to follow a reverse Christmas tree structure,
> improving code organization and readability.
> 
> This change introduces no functional modifications.
> 
> Signed-off-by: Jon Kohler <jon@nutanix.com>

We generally don't do pure refactoring patches.

They add churn to code history for little gain (and some
overhead and risk).
Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
Posted by Jon Kohler 9 months ago

> On May 7, 2025, at 1:23 PM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> Jon Kohler wrote:
>> Refactor variable names in vhost_net_build_xdp to align with XDP
>> terminology, enhancing code clarity and consistency. Additionally,
>> reorder variables to follow a reverse Christmas tree structure,
>> improving code organization and readability.
>> 
>> This change introduces no functional modifications.
>> 
>> Signed-off-by: Jon Kohler <jon@nutanix.com>
> 
> We generally don't do pure refactoring patches.
> 
> They add churn to code history for little gain (and some
> overhead and risk).
> 

Ok, I’ll club this together with the larger change I’m working on
for multi-buffer support in vhost/net, ill send that as a series
when it is ready for eyes
Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
Posted by Willem de Bruijn 9 months ago
Jon Kohler wrote:
> 
> 
> > On May 7, 2025, at 1:23 PM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> > 
> > !-------------------------------------------------------------------|
> >  CAUTION: External Email
> > 
> > |-------------------------------------------------------------------!
> > 

Minor: can you fix email to avoid the above?

> > Jon Kohler wrote:
> >> Refactor variable names in vhost_net_build_xdp to align with XDP
> >> terminology, enhancing code clarity and consistency. Additionally,
> >> reorder variables to follow a reverse Christmas tree structure,
> >> improving code organization and readability.
> >> 
> >> This change introduces no functional modifications.
> >> 
> >> Signed-off-by: Jon Kohler <jon@nutanix.com>
> > 
> > We generally don't do pure refactoring patches.
> > 
> > They add churn to code history for little gain (and some
> > overhead and risk).
> > 
> 
> Ok, I’ll club this together with the larger change I’m working on
> for multi-buffer support in vhost/net, ill send that as a series
> when it is ready for eyes

I forgot to add that it makes stable fixes harder to apply across
LTS, distro and other derived kernels.

So resist the urge the just make stylistic changes. Functional
improvements warrants the risk, churn and extra work.
Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
Posted by Jon Kohler 9 months ago

> On May 8, 2025, at 9:42 AM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> 
> 
> Jon Kohler wrote:
>> 
>> 
>>> On May 7, 2025, at 1:23 PM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
>>> 
> 
> Minor: can you fix email to avoid the above?

I think its a corporate email thing, but good reminder, ill clip it out in future
responses to not pollute the list

> 
>>> Jon Kohler wrote:
>>>> Refactor variable names in vhost_net_build_xdp to align with XDP
>>>> terminology, enhancing code clarity and consistency. Additionally,
>>>> reorder variables to follow a reverse Christmas tree structure,
>>>> improving code organization and readability.
>>>> 
>>>> This change introduces no functional modifications.
>>>> 
>>>> Signed-off-by: Jon Kohler <jon@nutanix.com>
>>> 
>>> We generally don't do pure refactoring patches.
>>> 
>>> They add churn to code history for little gain (and some
>>> overhead and risk).
>>> 
>> 
>> Ok, I’ll club this together with the larger change I’m working on
>> for multi-buffer support in vhost/net, ill send that as a series
>> when it is ready for eyes
> 
> I forgot to add that it makes stable fixes harder to apply across
> LTS, distro and other derived kernels.
> 
> So resist the urge the just make stylistic changes. Functional
> improvements warrants the risk, churn and extra work.
> 

Fair enough, I think this will make more sense in the context of the
broader series which will end up re-writing the majority of this func.
Was trying to separate some of the prep patches, but I see what
you're saying.