[Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Maxime Coquelin]

Thanks for the reviews,

This series implements Virtio spec update from Aaron Conole which
defines a way for the host to expose its max MTU to the guest.

"host_mtu" parameter is added to provide QEMU with the MTU value,
and the backend, if supported, gets notified of the MTU value when the
MTU feature neogotiation succeeds.

Only user backend currently supports MTU notification. A new protocol
feature has been implemented for sending MTU value to the backend.

For kernel backend, it is expected the management tool also configures
the tap/macvtap interface with same MTU value.
Daniel, I would be interrested about your feedback on this implementation
from management tool point of view.

Changes since RFC v3:
---------------------
 - No more RFC as consensus seems found on design
 - No more report an error if backend not implemented
 - Instead, check reply from the backend if REPLY_ACK supported
 - Add NET previx to new protocol feature & request names
 - Add return value to vhost_net_set_mtu() when !CONFIG_VHOST_NET

Changes since RFC v2:
---------------------
 - host_mtu propoerty is now used to specify the MTU value
 - update vhost-user spec with MTU additions

Changes since RFC v1:
---------------------
 - Rebased on top of v2.8.0-rc0 (2.7.90)
 - Write MTU unconditionnaly in netcfg to avoid memory leak (Paolo)
 - Add host_mtu property to be able to disable the feature from QEMU

Maxime Coquelin (3):
  vhost-user: Add MTU protocol feature and op
  vhost-net: Notify the backend about the host MTU
  virtio-net: Add MTU feature support

 docs/specs/vhost-user.txt         | 16 ++++++++++++++++
 hw/net/vhost_net.c                | 18 ++++++++++++++++++
 hw/net/virtio-net.c               | 19 +++++++++++++++++++
 hw/virtio/vhost-user.c            | 34 ++++++++++++++++++++++++++++++++++
 include/hw/virtio/vhost-backend.h |  2 ++
 include/hw/virtio/virtio-net.h    |  1 +
 include/net/vhost_net.h           |  2 ++
 7 files changed, 92 insertions(+)

-- 
2.9.3

[Qemu-devel] [PATCH v4 1/3] vhost-user: Add MTU protocol feature and op

[Maxime Coquelin]

This patch implements VHOST_USER_PROTOCOL_F_NET_MTU
protocol feature and VHOST_USER_NET_SET_MTU request so
that the backend gets notified of the user defined host
MTU.

If backend supports VHOST_USER_PROTOCOL_F_REPLY_ACK,
QEMU assumes MTU is valid if success is returned.

Vhost-net driver sends this request through a new
vhost_net_set_mtu vhost_ops entry.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Aaron Conole <aconole@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 docs/specs/vhost-user.txt         | 16 ++++++++++++++++
 hw/virtio/vhost-user.c            | 34 ++++++++++++++++++++++++++++++++++
 include/hw/virtio/vhost-backend.h |  2 ++
 3 files changed, 52 insertions(+)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 7890d71..6f19276 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -259,6 +259,7 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD      1
 #define VHOST_USER_PROTOCOL_F_RARP           2
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK      3
+#define VHOST_USER_PROTOCOL_F_MTU            4
 
 Message types
 -------------
@@ -470,6 +471,21 @@ Message types
       The first 6 bytes of the payload contain the mac address of the guest to
       allow the vhost user backend to construct and broadcast the fake RARP.
 
+ * VHOST_USER_NET_SET_MTU
+
+      Id: 20
+      Equivalent ioctl: N/A
+      Master payload: u64
+
+      Set host MTU value exposed to the guest.
+      This request should be sent only when VIRTIO_NET_F_MTU feature has been
+      successfully negotiated, VHOST_USER_F_PROTOCOL_FEATURES is present in
+      VHOST_USER_GET_FEATURES and protocol feature bit
+      VHOST_USER_PROTOCOL_F_NET_MTU is present in
+      VHOST_USER_GET_PROTOCOL_FEATURES.
+      If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
+      with zero in case the specified MTU is valid, or non-zero otherwise.
+
 VHOST_USER_PROTOCOL_F_REPLY_ACK:
 -------------------------------
 The original vhost-user specification only demands replies for certain
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 7ee92b3..9334a8a 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_NET_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_NET_SET_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_NET_SET_MTU:
         return true;
     default:
         return false;
@@ -685,6 +688,36 @@ static bool vhost_user_can_merge(struct vhost_dev *dev,
     return mfd == rfd;
 }
 
+static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
+{
+    VhostUserMsg msg;
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                              VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU))) {
+        return 0;
+    }
+
+    msg.request = VHOST_USER_NET_SET_MTU;
+    msg.payload.u64 = mtu;
+    msg.size = sizeof(msg.payload.u64);
+    msg.flags = VHOST_USER_VERSION;
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_REPLY_MASK;
+    }
+
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        return -1;
+    }
+
+    /* If reply_ack supported, slave has to ack specified MTU is valid */
+    if (reply_supported) {
+        return process_message_reply(dev, msg.request);
+    }
+
+    return 0;
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_init,
@@ -708,4 +741,5 @@ const VhostOps user_ops = {
         .vhost_requires_shm_log = vhost_user_requires_shm_log,
         .vhost_migration_done = vhost_user_migration_done,
         .vhost_backend_can_merge = vhost_user_can_merge,
+        .vhost_net_set_mtu = vhost_user_net_set_mtu,
 };
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 6e90703..30abc11 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -32,6 +32,7 @@ typedef int (*vhost_backend_memslots_limit)(struct vhost_dev *dev);
 
 typedef int (*vhost_net_set_backend_op)(struct vhost_dev *dev,
                                 struct vhost_vring_file *file);
+typedef int (*vhost_net_set_mtu_op)(struct vhost_dev *dev, uint16_t mtu);
 typedef int (*vhost_scsi_set_endpoint_op)(struct vhost_dev *dev,
                                   struct vhost_scsi_target *target);
 typedef int (*vhost_scsi_clear_endpoint_op)(struct vhost_dev *dev,
@@ -83,6 +84,7 @@ typedef struct VhostOps {
     vhost_backend_cleanup vhost_backend_cleanup;
     vhost_backend_memslots_limit vhost_backend_memslots_limit;
     vhost_net_set_backend_op vhost_net_set_backend;
+    vhost_net_set_mtu_op vhost_net_set_mtu;
     vhost_scsi_set_endpoint_op vhost_scsi_set_endpoint;
     vhost_scsi_clear_endpoint_op vhost_scsi_clear_endpoint;
     vhost_scsi_get_abi_version_op vhost_scsi_get_abi_version;
-- 
2.9.3

[Qemu-devel] [PATCH v4 2/3] vhost-net: Notify the backend about the host MTU

[Maxime Coquelin]

This patch provides a way for virtio-net to notify the
backend about the host MTU set by the user.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Aaron Conole <aconole@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/net/vhost_net.c      | 18 ++++++++++++++++++
 include/net/vhost_net.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f2d49ad..6280422 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -51,6 +51,7 @@ static const int kernel_feature_bits[] = {
     VIRTIO_RING_F_EVENT_IDX,
     VIRTIO_NET_F_MRG_RXBUF,
     VIRTIO_F_VERSION_1,
+    VIRTIO_NET_F_MTU,
     VHOST_INVALID_FEATURE_BIT
 };
 
@@ -74,6 +75,7 @@ static const int user_feature_bits[] = {
     VIRTIO_NET_F_HOST_ECN,
     VIRTIO_NET_F_HOST_UFO,
     VIRTIO_NET_F_MRG_RXBUF,
+    VIRTIO_NET_F_MTU,
 
     /* This bit implies RARP isn't sent by QEMU out of band */
     VIRTIO_NET_F_GUEST_ANNOUNCE,
@@ -435,6 +437,17 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
     return 0;
 }
 
+int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
+{
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+
+    if (!vhost_ops->vhost_net_set_mtu) {
+        return 0;
+    }
+
+    return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
+}
+
 #else
 uint64_t vhost_net_get_max_queues(VHostNetState *net)
 {
@@ -501,4 +514,9 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
 {
     return 0;
 }
+
+int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
+{
+    return 0;
+}
 #endif
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 5a08eff..afc1499 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -35,4 +35,6 @@ int vhost_set_vring_enable(NetClientState * nc, int enable);
 
 uint64_t vhost_net_get_acked_features(VHostNetState *net);
 
+int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
+
 #endif
-- 
2.9.3

[Qemu-devel] [PATCH v4 3/3] virtio-net: Add MTU feature support

[Maxime Coquelin]

This patch allows advising guest with host MTU's by setting
host_mtu parameter.

If VIRTIO_NET_F_MTU has been successfully negotiated, MTU
value is passed to the backend.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Aaron Conole <aconole@redhat.com
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/net/virtio-net.c            | 19 +++++++++++++++++++
 include/hw/virtio/virtio-net.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 5009533..b6da529 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -55,6 +55,8 @@ static VirtIOFeature feature_sizes[] = {
      .end = endof(struct virtio_net_config, status)},
     {.flags = 1 << VIRTIO_NET_F_MQ,
      .end = endof(struct virtio_net_config, max_virtqueue_pairs)},
+    {.flags = 1 << VIRTIO_NET_F_MTU,
+     .end = endof(struct virtio_net_config, mtu)},
     {}
 };
 
@@ -81,6 +83,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 
     virtio_stw_p(vdev, &netcfg.status, n->status);
     virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
+    virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
     memcpy(netcfg.mac, n->mac, ETH_ALEN);
     memcpy(config, &netcfg, n->config_size);
 }
@@ -152,6 +155,16 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
             qemu_net_queue_purge(qnc->incoming_queue, qnc->peer);
         }
 
+        if (virtio_has_feature(vdev->guest_features, VIRTIO_NET_F_MTU)) {
+            r = vhost_net_set_mtu(get_vhost_net(nc->peer), n->net_conf.mtu);
+            if (r < 0) {
+                error_report("%uBytes MTU not supported by the backend",
+                             n->net_conf.mtu);
+
+                return;
+            }
+        }
+
         n->vhost_started = 1;
         r = vhost_net_start(vdev, n->nic->ncs, queues);
         if (r < 0) {
@@ -1695,6 +1708,7 @@ static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
 {
     int i, config_size = 0;
     virtio_add_feature(&host_features, VIRTIO_NET_F_MAC);
+
     for (i = 0; feature_sizes[i].flags != 0; i++) {
         if (host_features & feature_sizes[i].flags) {
             config_size = MAX(feature_sizes[i].end, config_size);
@@ -1724,6 +1738,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     NetClientState *nc;
     int i;
 
+    if (n->net_conf.mtu) {
+        n->host_features |= (0x1 << VIRTIO_NET_F_MTU);
+    }
+
     virtio_net_set_config_size(n, n->host_features);
     virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
@@ -1922,6 +1940,7 @@ static Property virtio_net_properties[] = {
     DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
     DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size,
                        VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE),
+    DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 0ced975..8ea56a8 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -36,6 +36,7 @@ typedef struct virtio_net_conf
     int32_t txburst;
     char *tx;
     uint16_t rx_queue_size;
+    uint16_t mtu;
 } virtio_net_conf;
 
 /* Maximum packet size we can receive from tap device: header + 64k */
-- 
2.9.3

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Daniel P. Berrange]

On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
> Thanks for the reviews,
> 
> This series implements Virtio spec update from Aaron Conole which
> defines a way for the host to expose its max MTU to the guest.
> 
> "host_mtu" parameter is added to provide QEMU with the MTU value,
> and the backend, if supported, gets notified of the MTU value when the
> MTU feature neogotiation succeeds.
> 
> Only user backend currently supports MTU notification. A new protocol
> feature has been implemented for sending MTU value to the backend.
> 
> For kernel backend, it is expected the management tool also configures
> the tap/macvtap interface with same MTU value.
> Daniel, I would be interrested about your feedback on this implementation
> from management tool point of view.

I can't give real feedback yet, as I'm not seeing clear information on
what problem this series is designed to solve....

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Maxime Coquelin]

Hi Daniel,

On 12/12/2016 11:02 AM, Daniel P. Berrange wrote:
> On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
>> Thanks for the reviews,
>>
>> This series implements Virtio spec update from Aaron Conole which
>> defines a way for the host to expose its max MTU to the guest.
>>
>> "host_mtu" parameter is added to provide QEMU with the MTU value,
>> and the backend, if supported, gets notified of the MTU value when the
>> MTU feature neogotiation succeeds.
>>
>> Only user backend currently supports MTU notification. A new protocol
>> feature has been implemented for sending MTU value to the backend.
>>
>> For kernel backend, it is expected the management tool also configures
>> the tap/macvtap interface with same MTU value.
>> Daniel, I would be interrested about your feedback on this implementation
>> from management tool point of view.
>
> I can't give real feedback yet, as I'm not seeing clear information on
> what problem this series is designed to solve....

Right, I agree it is missing a bit of context here, I'll add more about
the background in next revision.

The goal of this series is to address two things:
1. Providing a way for the guests to use the same MTU as the host,
    in order to have a consistent MTU value across the infrastructure.

2. Improve Rx performance for small packets, as if all parties agrees
    on a MTU value for which packets fit into a single descriptor, then
    Rx mergeable buffers feature can be disabled (saving one
    cache-miss on guest side by not accessing the virtio header, when
    offloading has not been negotiated).

Thanks,
Maxime

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Daniel P. Berrange]

On Mon, Dec 12, 2016 at 11:12:56AM +0100, Maxime Coquelin wrote:
> Hi Daniel,
> 
> On 12/12/2016 11:02 AM, Daniel P. Berrange wrote:
> > On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
> > > Thanks for the reviews,
> > > 
> > > This series implements Virtio spec update from Aaron Conole which
> > > defines a way for the host to expose its max MTU to the guest.
> > > 
> > > "host_mtu" parameter is added to provide QEMU with the MTU value,
> > > and the backend, if supported, gets notified of the MTU value when the
> > > MTU feature neogotiation succeeds.
> > > 
> > > Only user backend currently supports MTU notification. A new protocol
> > > feature has been implemented for sending MTU value to the backend.
> > > 
> > > For kernel backend, it is expected the management tool also configures
> > > the tap/macvtap interface with same MTU value.
> > > Daniel, I would be interrested about your feedback on this implementation
> > > from management tool point of view.
> > 
> > I can't give real feedback yet, as I'm not seeing clear information on
> > what problem this series is designed to solve....
> 
> Right, I agree it is missing a bit of context here, I'll add more about
> the background in next revision.
> 
> The goal of this series is to address two things:
> 1. Providing a way for the guests to use the same MTU as the host,
>    in order to have a consistent MTU value across the infrastructure.

Ok, currently libvirt sets the MTU of the tap device based on the MTU
of the device it will be attached to. This change means we need to pass
that MTU value into QEMU via the -netdev command line so it can inform
the guest what the MTU is.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Maxime Coquelin]

On 12/12/2016 11:34 AM, Daniel P. Berrange wrote:
> On Mon, Dec 12, 2016 at 11:12:56AM +0100, Maxime Coquelin wrote:
>> Hi Daniel,
>>
>> On 12/12/2016 11:02 AM, Daniel P. Berrange wrote:
>>> On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
>>>> Thanks for the reviews,
>>>>
>>>> This series implements Virtio spec update from Aaron Conole which
>>>> defines a way for the host to expose its max MTU to the guest.
>>>>
>>>> "host_mtu" parameter is added to provide QEMU with the MTU value,
>>>> and the backend, if supported, gets notified of the MTU value when the
>>>> MTU feature neogotiation succeeds.
>>>>
>>>> Only user backend currently supports MTU notification. A new protocol
>>>> feature has been implemented for sending MTU value to the backend.
>>>>
>>>> For kernel backend, it is expected the management tool also configures
>>>> the tap/macvtap interface with same MTU value.
>>>> Daniel, I would be interrested about your feedback on this implementation
>>>> from management tool point of view.
>>>
>>> I can't give real feedback yet, as I'm not seeing clear information on
>>> what problem this series is designed to solve....
>>
>> Right, I agree it is missing a bit of context here, I'll add more about
>> the background in next revision.
>>
>> The goal of this series is to address two things:
>> 1. Providing a way for the guests to use the same MTU as the host,
>>    in order to have a consistent MTU value across the infrastructure.
>
> Ok, currently libvirt sets the MTU of the tap device based on the MTU
> of the device it will be attached to. This change means we need to pass
> that MTU value into QEMU via the -netdev command line so it can inform
> the guest what the MTU is.
Nice, do you have a pointer on where this is done in QEMU?

Thanks,
Maxime

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Daniel P. Berrange]

On Tue, Dec 13, 2016 at 02:04:52PM +0100, Maxime Coquelin wrote:
> 
> 
> On 12/12/2016 11:34 AM, Daniel P. Berrange wrote:
> > On Mon, Dec 12, 2016 at 11:12:56AM +0100, Maxime Coquelin wrote:
> > > Hi Daniel,
> > > 
> > > On 12/12/2016 11:02 AM, Daniel P. Berrange wrote:
> > > > On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
> > > > > Thanks for the reviews,
> > > > > 
> > > > > This series implements Virtio spec update from Aaron Conole which
> > > > > defines a way for the host to expose its max MTU to the guest.
> > > > > 
> > > > > "host_mtu" parameter is added to provide QEMU with the MTU value,
> > > > > and the backend, if supported, gets notified of the MTU value when the
> > > > > MTU feature neogotiation succeeds.
> > > > > 
> > > > > Only user backend currently supports MTU notification. A new protocol
> > > > > feature has been implemented for sending MTU value to the backend.
> > > > > 
> > > > > For kernel backend, it is expected the management tool also configures
> > > > > the tap/macvtap interface with same MTU value.
> > > > > Daniel, I would be interrested about your feedback on this implementation
> > > > > from management tool point of view.
> > > > 
> > > > I can't give real feedback yet, as I'm not seeing clear information on
> > > > what problem this series is designed to solve....
> > > 
> > > Right, I agree it is missing a bit of context here, I'll add more about
> > > the background in next revision.
> > > 
> > > The goal of this series is to address two things:
> > > 1. Providing a way for the guests to use the same MTU as the host,
> > >    in order to have a consistent MTU value across the infrastructure.
> > 
> > Ok, currently libvirt sets the MTU of the tap device based on the MTU
> > of the device it will be attached to. This change means we need to pass
> > that MTU value into QEMU via the -netdev command line so it can inform
> > the guest what the MTU is.
> Nice, do you have a pointer on where this is done in QEMU?

Its in libvirt code via virNetDevSetMTUFromDevice()


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Maxime Coquelin]

On 12/13/2016 02:07 PM, Daniel P. Berrange wrote:
> On Tue, Dec 13, 2016 at 02:04:52PM +0100, Maxime Coquelin wrote:
>>
>>
>> On 12/12/2016 11:34 AM, Daniel P. Berrange wrote:
>>> On Mon, Dec 12, 2016 at 11:12:56AM +0100, Maxime Coquelin wrote:
>>>> Hi Daniel,
>>>>
>>>> On 12/12/2016 11:02 AM, Daniel P. Berrange wrote:
>>>>> On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
>>>>>> Thanks for the reviews,
>>>>>>
>>>>>> This series implements Virtio spec update from Aaron Conole which
>>>>>> defines a way for the host to expose its max MTU to the guest.
>>>>>>
>>>>>> "host_mtu" parameter is added to provide QEMU with the MTU value,
>>>>>> and the backend, if supported, gets notified of the MTU value when the
>>>>>> MTU feature neogotiation succeeds.
>>>>>>
>>>>>> Only user backend currently supports MTU notification. A new protocol
>>>>>> feature has been implemented for sending MTU value to the backend.
>>>>>>
>>>>>> For kernel backend, it is expected the management tool also configures
>>>>>> the tap/macvtap interface with same MTU value.
>>>>>> Daniel, I would be interrested about your feedback on this implementation
>>>>>> from management tool point of view.
>>>>>
>>>>> I can't give real feedback yet, as I'm not seeing clear information on
>>>>> what problem this series is designed to solve....
>>>>
>>>> Right, I agree it is missing a bit of context here, I'll add more about
>>>> the background in next revision.
>>>>
>>>> The goal of this series is to address two things:
>>>> 1. Providing a way for the guests to use the same MTU as the host,
>>>>    in order to have a consistent MTU value across the infrastructure.
>>>
>>> Ok, currently libvirt sets the MTU of the tap device based on the MTU
>>> of the device it will be attached to. This change means we need to pass
>>> that MTU value into QEMU via the -netdev command line so it can inform
>>> the guest what the MTU is.
>> Nice, do you have a pointer on where this is done in QEMU?
>
> Its in libvirt code via virNetDevSetMTUFromDevice()

Thanks.
For QEMU part, this series adds host_mtu parameter to virtio-net device,
not to vhost netdev. Should it be reworked?

Maxime

Re: [Qemu-devel] [PATCH v4 0/3] virtio-net: Add support to MTU feature

[Michael S. Tsirkin]

On Tue, Dec 13, 2016 at 02:17:11PM +0100, Maxime Coquelin wrote:
> 
> 
> On 12/13/2016 02:07 PM, Daniel P. Berrange wrote:
> > On Tue, Dec 13, 2016 at 02:04:52PM +0100, Maxime Coquelin wrote:
> > > 
> > > 
> > > On 12/12/2016 11:34 AM, Daniel P. Berrange wrote:
> > > > On Mon, Dec 12, 2016 at 11:12:56AM +0100, Maxime Coquelin wrote:
> > > > > Hi Daniel,
> > > > > 
> > > > > On 12/12/2016 11:02 AM, Daniel P. Berrange wrote:
> > > > > > On Sat, Dec 10, 2016 at 04:30:35PM +0100, Maxime Coquelin wrote:
> > > > > > > Thanks for the reviews,
> > > > > > > 
> > > > > > > This series implements Virtio spec update from Aaron Conole which
> > > > > > > defines a way for the host to expose its max MTU to the guest.
> > > > > > > 
> > > > > > > "host_mtu" parameter is added to provide QEMU with the MTU value,
> > > > > > > and the backend, if supported, gets notified of the MTU value when the
> > > > > > > MTU feature neogotiation succeeds.
> > > > > > > 
> > > > > > > Only user backend currently supports MTU notification. A new protocol
> > > > > > > feature has been implemented for sending MTU value to the backend.
> > > > > > > 
> > > > > > > For kernel backend, it is expected the management tool also configures
> > > > > > > the tap/macvtap interface with same MTU value.
> > > > > > > Daniel, I would be interrested about your feedback on this implementation
> > > > > > > from management tool point of view.
> > > > > > 
> > > > > > I can't give real feedback yet, as I'm not seeing clear information on
> > > > > > what problem this series is designed to solve....
> > > > > 
> > > > > Right, I agree it is missing a bit of context here, I'll add more about
> > > > > the background in next revision.
> > > > > 
> > > > > The goal of this series is to address two things:
> > > > > 1. Providing a way for the guests to use the same MTU as the host,
> > > > >    in order to have a consistent MTU value across the infrastructure.
> > > > 
> > > > Ok, currently libvirt sets the MTU of the tap device based on the MTU
> > > > of the device it will be attached to. This change means we need to pass
> > > > that MTU value into QEMU via the -netdev command line so it can inform
> > > > the guest what the MTU is.
> > > Nice, do you have a pointer on where this is done in QEMU?
> > 
> > Its in libvirt code via virNetDevSetMTUFromDevice()
> 
> Thanks.
> For QEMU part, this series adds host_mtu parameter to virtio-net device,
> not to vhost netdev. Should it be reworked?
> 
> Maxime

I applied as is for now, we can rework as necessary but people want to
be able to test this.