Re: [Qemu-devel] [RFC v2 1/3] vhost-user: Add new protocol feature MTU

[Maxime Coquelin]

On 11/18/2016 03:26 PM, Aaron Conole wrote:
> Maxime Coquelin <address@hidden> writes:
>
> > This patch adds VHOST_USER_PROTOCOL_F_MTU protocol feature.
> >
> > If supported, QEMU sends VHOST_USER_GET_MTU request to the client,
> > and expects a u64 reply containing the MTU advised for the guest.
> >
> > Cc: Michael S. Tsirkin <address@hidden>
> > Cc: Aaron Conole <address@hidden>
> > Signed-off-by: Maxime Coquelin <address@hidden>
> > ---
> >  hw/virtio/vhost-user.c    | 11 +++++++++++
> >  include/hw/virtio/vhost.h |  1 +
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 7ee92b3..eaf007d 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -32,6 +32,7 @@ enum VhostUserProtocolFeature {
> >      VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
> >      VHOST_USER_PROTOCOL_F_RARP = 2,
> >      VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
> > +    VHOST_USER_PROTOCOL_F_MTU = 4,
> >
> >      VHOST_USER_PROTOCOL_F_MAX
> >  };
> > @@ -59,6 +60,7 @@ typedef enum VhostUserRequest {
> >      VHOST_USER_GET_QUEUE_NUM = 17,
> >      VHOST_USER_SET_VRING_ENABLE = 18,
> >      VHOST_USER_SEND_RARP = 19,
> > +    VHOST_USER_GET_MTU = 20,
> >      VHOST_USER_MAX
> >  } VhostUserRequest;
> >
> > @@ -186,6 +188,7 @@ static bool vhost_user_one_time_request(VhostUserRequest 
> > request)
> >      case VHOST_USER_RESET_OWNER:
> >      case VHOST_USER_SET_MEM_TABLE:
> >      case VHOST_USER_GET_QUEUE_NUM:
> > +    case VHOST_USER_GET_MTU:
> >          return true;
> >      default:
> >          return false;
> > @@ -602,6 +605,14 @@ static int vhost_user_init(struct vhost_dev *dev, void 
> > *opaque)
> >                  return err;
> >              }
> >          }
> > +
> > +        /* query the MTU we support if backend supports MTU feature */
> > +        if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MTU)) {
> > +            err = vhost_user_get_u64(dev, VHOST_USER_GET_MTU, &dev->mtu);
> > +            if (err < 0) {
> > +                return err;
> > +            }
> > +        }
> >      }
> >
> >      if (dev->migration_blocker == NULL &&
> > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > index 1fe5aad..c674a05 100644
> > --- a/include/hw/virtio/vhost.h
> > +++ b/include/hw/virtio/vhost.h
> > @@ -51,6 +51,7 @@ struct vhost_dev {
> >      uint64_t backend_features;
> >      uint64_t protocol_features;
> >      uint64_t max_queues;
> > +    uint64_t mtu;
>
> Just a question why the MTU is stored as a u64?  would uint16_t make
> more sense - then we can be sure we never have an excessively large mtu
> value.
>
> What do you think?
This is because the vihst-user message payload is 64 bits.
Note that the max number of queues is also 64 bits.

However, a check could be added to ensure the value is coherent.

Maxime