On Wed, Nov 6, 2024 at 4:54 PM Michael S. Tsirkin [off-list ref] wrote:

On Sun, Sep 15, 2024 at 10:35:53AM +0900, Akihiko Odaki wrote:

quoted

The specification says the device MUST set num_buffers to 1 if
VIRTIO_NET_F_MRG_RXBUF has not been negotiated.

Fixes: 41e3e42108bc ("vhost/net: enable virtio 1.0")
Signed-off-by: Akihiko Odaki <redacted>

True, this is out of spec. But, qemu is also out of spec :(

Given how many years this was out there, I wonder whether
we should just fix the spec, instead of changing now.

Jason, what's your take?

Fixing the spec (if you mean release the requirement) seems to be less risky.

Thanks

quoted

---
 drivers/vhost/net.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f16279351db5..d4d97fa9cc8f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c

@@ -1107,6 +1107,7 @@ static void handle_rx(struct vhost_net *net)
      size_t vhost_hlen, sock_hlen;
      size_t vhost_len, sock_len;
      bool busyloop_intr = false;
+     bool set_num_buffers;
      struct socket *sock;
      struct iov_iter fixup;
      __virtio16 num_buffers;

@@ -1129,6 +1130,8 @@ static void handle_rx(struct vhost_net *net)
      vq_log = unlikely(vhost_has_feature(vq, VHOST_F_LOG_ALL)) ?
              vq->log : NULL;
      mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
+     set_num_buffers = mergeable ||
+                       vhost_has_feature(vq, VIRTIO_F_VERSION_1);

      do {
              sock_len = vhost_net_rx_peek_head_len(net, sock->sk,

@@ -1205,7 +1208,7 @@ static void handle_rx(struct vhost_net *net)
              /* TODO: Should check and handle checksum. */

              num_buffers = cpu_to_vhost16(vq, headcount);
-             if (likely(mergeable) &&
+             if (likely(set_num_buffers) &&
                  copy_to_iter(&num_buffers, sizeof num_buffers,
                               &fixup) != sizeof num_buffers) {
                      vq_err(vq, "Failed num_buffers write");

---
base-commit: 46a0057a5853cbdb58211c19e89ba7777dc6fd50
change-id: 20240908-v1-90fc83ff8b09

Best regards,
--
Akihiko Odaki [off-list ref]