[v1] vdpa: Allow VDPA to work on big-endian machine

[PATCH] vdpa: Allow VDPA to work on big-endian machine

Posted by Konstantin Shkolnyy 5 months ago

After commit 0caed25cd171 vhost_vdpa_net_load_vlan() started seeing
VIRTIO_NET_F_CTRL_VLAN flag and making 4096 calls to the kernel with
VIRTIO_NET_CTRL_VLAN_ADD command. However, it forgot to convert the
16-bit VLAN IDs to LE format. On BE machine, the kernel calls failed
when they saw "VLAN IDs" greater than 4095, and QEMU then said:
"unable to start vhost net: 5: falling back on userspace virtio", and
VDPA became disabled.

Convert the VLAN ID to LE before putting it into virtio queue.

Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
---
 net/vhost-vdpa.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 58d738945d..99c9eb42b9 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1173,9 +1173,10 @@ static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
                                            struct iovec *in_cursor,
                                            uint16_t vid)
 {
+    __le16 vid_le = cpu_to_le16(vid);
     const struct iovec data = {
-        .iov_base = &vid,
-        .iov_len = sizeof(vid),
+        .iov_base = &vid_le,
+        .iov_len = sizeof(vid_le),
     };
     ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor,
                                         VIRTIO_NET_CTRL_VLAN,
-- 
2.34.1

Re: [PATCH] vdpa: Allow VDPA to work on big-endian machine

Posted by Akihiko Odaki 5 months ago

On 2025/06/15 7:44, Konstantin Shkolnyy wrote:
> After commit 0caed25cd171 vhost_vdpa_net_load_vlan() started seeing
> VIRTIO_NET_F_CTRL_VLAN flag and making 4096 calls to the kernel with
> VIRTIO_NET_CTRL_VLAN_ADD command. However, it forgot to convert the
> 16-bit VLAN IDs to LE format. On BE machine, the kernel calls failed
> when they saw "VLAN IDs" greater than 4095, and QEMU then said:
> "unable to start vhost net: 5: falling back on userspace virtio", and
> VDPA became disabled.

Please add the Fixes: tag to refer the commit; see:
docs/devel/submitting-a-patch.rst

> 
> Convert the VLAN ID to LE before putting it into virtio queue.
> 
> Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
> ---
>   net/vhost-vdpa.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 58d738945d..99c9eb42b9 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1173,9 +1173,10 @@ static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
>                                              struct iovec *in_cursor,
>                                              uint16_t vid)
>   {
> +    __le16 vid_le = cpu_to_le16(vid);

docs/devel/style.rst says:
> Don't use Linux kernel internal types like u32, __u32 or __le32.

It's unfortunate that QEMU lacks endian types and a checker for them; 
such a checker could not have caught this particular case, but can catch 
other similar bugs.

Regards,
Akihiko Odaki

>       const struct iovec data = {
> -        .iov_base = &vid,
> -        .iov_len = sizeof(vid),
> +        .iov_base = &vid_le,
> +        .iov_len = sizeof(vid_le),
>       };
>       ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor,
>                                           VIRTIO_NET_CTRL_VLAN,

Re: [PATCH] vdpa: Allow VDPA to work on big-endian machine

Posted by Eugenio Perez Martin 5 months ago

On Sun, Jun 15, 2025 at 12:46 AM Konstantin Shkolnyy <kshk@linux.ibm.com> wrote:
>
> After commit 0caed25cd171 vhost_vdpa_net_load_vlan() started seeing
> VIRTIO_NET_F_CTRL_VLAN flag and making 4096 calls to the kernel with
> VIRTIO_NET_CTRL_VLAN_ADD command. However, it forgot to convert the
> 16-bit VLAN IDs to LE format. On BE machine, the kernel calls failed
> when they saw "VLAN IDs" greater than 4095, and QEMU then said:
> "unable to start vhost net: 5: falling back on userspace virtio", and
> VDPA became disabled.
>
> Convert the VLAN ID to LE before putting it into virtio queue.
>
> Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>

Good catch! You should add the Fixes: tag though :). With that,

Acked-by: Eugenio Pérez <eperezma@redhat.com>

> ---
>  net/vhost-vdpa.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 58d738945d..99c9eb42b9 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1173,9 +1173,10 @@ static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
>                                             struct iovec *in_cursor,
>                                             uint16_t vid)
>  {
> +    __le16 vid_le = cpu_to_le16(vid);
>      const struct iovec data = {
> -        .iov_base = &vid,
> -        .iov_len = sizeof(vid),
> +        .iov_base = &vid_le,
> +        .iov_len = sizeof(vid_le),
>      };
>      ssize_t r = vhost_vdpa_net_load_cmd(s, out_cursor, in_cursor,
>                                          VIRTIO_NET_CTRL_VLAN,
> --
> 2.34.1
>
>

Re: [PATCH] vdpa: Allow VDPA to work on big-endian machine

Posted by Jason Wang 5 months ago

On Sun, Jun 15, 2025 at 6:44 AM Konstantin Shkolnyy <kshk@linux.ibm.com> wrote:
>
> After commit 0caed25cd171 vhost_vdpa_net_load_vlan() started seeing
> VIRTIO_NET_F_CTRL_VLAN flag and making 4096 calls to the kernel with
> VIRTIO_NET_CTRL_VLAN_ADD command. However, it forgot to convert the
> 16-bit VLAN IDs to LE format. On BE machine, the kernel calls failed
> when they saw "VLAN IDs" greater than 4095, and QEMU then said:
> "unable to start vhost net: 5: falling back on userspace virtio", and
> VDPA became disabled.
>
> Convert the VLAN ID to LE before putting it into virtio queue.
>
> Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks