* [PATCH net-next] vhost/net: align variable names with XDP terminology
@ 2025-05-07 16:02 Jon Kohler
2025-05-07 17:23 ` Willem de Bruijn
2025-05-08 12:00 ` kernel test robot
0 siblings, 2 replies; 6+ messages in thread
From: Jon Kohler @ 2025-05-07 16:02 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend, kvm,
virtualization, netdev, linux-kernel, bpf
Cc: Jon Kohler
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
2025-05-07 16:02 [PATCH net-next] vhost/net: align variable names with XDP terminology Jon Kohler
@ 2025-05-07 17:23 ` Willem de Bruijn
2025-05-07 17:31 ` Jon Kohler
2025-05-08 12:00 ` kernel test robot
1 sibling, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2025-05-07 17:23 UTC (permalink / raw)
To: Jon Kohler, Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend, kvm,
virtualization, netdev, linux-kernel, bpf
Cc: Jon Kohler
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).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
2025-05-07 17:23 ` Willem de Bruijn
@ 2025-05-07 17:31 ` Jon Kohler
2025-05-08 13:42 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Jon Kohler @ 2025-05-07 17:31 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
kvm@vger.kernel.org, virtualization@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
bpf@vger.kernel.org
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
2025-05-07 16:02 [PATCH net-next] vhost/net: align variable names with XDP terminology Jon Kohler
2025-05-07 17:23 ` Willem de Bruijn
@ 2025-05-08 12:00 ` kernel test robot
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-05-08 12:00 UTC (permalink / raw)
To: Jon Kohler, Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend, kvm,
virtualization, linux-kernel, bpf
Cc: llvm, oe-kbuild-all, netdev, Jon Kohler
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
2025-05-07 17:31 ` Jon Kohler
@ 2025-05-08 13:42 ` Willem de Bruijn
2025-05-08 13:45 ` Jon Kohler
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2025-05-08 13:42 UTC (permalink / raw)
To: Jon Kohler, Willem de Bruijn
Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
kvm@vger.kernel.org, virtualization@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
bpf@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vhost/net: align variable names with XDP terminology
2025-05-08 13:42 ` Willem de Bruijn
@ 2025-05-08 13:45 ` Jon Kohler
0 siblings, 0 replies; 6+ messages in thread
From: Jon Kohler @ 2025-05-08 13:45 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
kvm@vger.kernel.org, virtualization@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
bpf@vger.kernel.org
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-05-08 13:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-07 16:02 [PATCH net-next] vhost/net: align variable names with XDP terminology Jon Kohler
2025-05-07 17:23 ` Willem de Bruijn
2025-05-07 17:31 ` Jon Kohler
2025-05-08 13:42 ` Willem de Bruijn
2025-05-08 13:45 ` Jon Kohler
2025-05-08 12:00 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).