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

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 <mst@redhat.com>
Cc: Aaron Conole <aconole@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost-user.c    | 11 +++++++++++
 include/hw/virtio/vhost.h |  1 +
 2 files changed, 12 insertions(+)

Maxime Coquelin <maxime.coquelin@redhat.com> 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 <mst@redhat.com>
> Cc: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  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?

>      bool started;
>      bool log_enabled;
>      uint64_t log_size;

On 11/18/2016 03:26 PM, Aaron Conole wrote:
> Maxime Coquelin <maxime.coquelin@redhat.com> 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 <mst@redhat.com>
>> Cc: Aaron Conole <aconole@redhat.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  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