qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration
@ 2013-04-07  6:42 Dmitry Fleytman
  2013-04-07  6:42 ` [Qemu-devel] [PATCH 1/1 " Dmitry Fleytman
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Fleytman @ 2013-04-07  6:42 UTC (permalink / raw)
  To: qemu-devel

From: Dmitry Fleytman <dfleytma@redhat.com>

This patch implements recently accepted by virtio-spec
dynamic offloads configuration feature.
See commit message for details.

V4 changes:
  1. Feature definitions re-used for command bitmask
  2. Command data made uint64
  3. Commit messsages fixed

Reported-by: Rusty Russell rusty@rustcorp.com.au

V3 changes:
  1. Compat macro added
  2. Feature name beautification

V2 changes:
  1. _GUEST_ added to command and feature names
  2. Live migration logic fixed

Reported-by: Michael S. Tsirkin <mst@redhat.com>

One of recently introduced Windows features (RSC)
requires network driver to be able to enable and disable
HW LRO offload on the fly without device reinitialization.

Current Virtio specification doesn't support this requirement.
The solution proposed by following spec patch is to add
a new control command for this purpose.

The same solution may be used in Linux driver for ethtool interface
implementation.

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-04-07  6:42 [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration Dmitry Fleytman
@ 2013-04-07  6:42 ` Dmitry Fleytman
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Fleytman @ 2013-04-07  6:42 UTC (permalink / raw)
  To: qemu-devel

From: Dmitry Fleytman <dfleytma@redhat.com>

Virtio-net driver currently negotiates network offloads
on startup via features mechanism and have no ability to
disable and re-enable offloads later.
This patch introduced a new control command that allows
to configure device network offloads state dynamically.
The patch also introduces a new feature flag
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.

Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
---
 hw/pc.h         |  4 +++
 hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 hw/virtio-net.h | 13 ++++++++
 3 files changed, 99 insertions(+), 15 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index 8e1dd4c..7ca4698 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
             .property = "vectors",\
             /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
             .value    = stringify(0xFFFFFFFF),\
+        },{ \
+            .driver   = "virtio-net-pci", \
+            .property = "ctrl_guest_offloads", \
+            .value    = "off", \
         },{\
             .driver   = "e1000",\
             .property = "romfile",\
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5917740..a92d061 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     return features;
 }
 
+static void virtio_net_apply_guest_offloads(VirtIONet *n)
+{
+    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
+            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
+            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
+            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
+            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
+            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
+}
+
+static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
+{
+    static const uint64_t guest_offloads_mask =
+        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
+        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
+        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
+        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
+        (1ULL << VIRTIO_NET_F_GUEST_UFO);
+
+    return guest_offloads_mask & features;
+}
+
+static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
+{
+    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
+}
+
 static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
 {
     VirtIONet *n = to_virtio_net(vdev);
@@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
     virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
 
     if (n->has_vnet_hdr) {
-        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
-                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
-                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
-                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
-                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
-                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
+        n->curr_guest_offloads =
+            virtio_net_guest_offloads_by_features(features);
+        virtio_net_apply_guest_offloads(n);
     }
 
     for (i = 0;  i < n->max_queues; i++) {
@@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
     return VIRTIO_NET_OK;
 }
 
+static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
+                                     struct iovec *iov, unsigned int iov_cnt)
+{
+    uint64_t offloads;
+    size_t s;
+
+    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
+        return VIRTIO_NET_ERR;
+    }
+
+    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
+    if (s != sizeof(offloads)) {
+        return VIRTIO_NET_ERR;
+    }
+
+    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
+        uint64_t supported_offloads;
+
+        if (!n->has_vnet_hdr) {
+            return VIRTIO_NET_ERR;
+        }
+
+        supported_offloads = virtio_net_supported_guest_offloads(n);
+        if (offloads & ~supported_offloads) {
+            return VIRTIO_NET_ERR;
+        }
+
+        n->curr_guest_offloads = offloads;
+        virtio_net_apply_guest_offloads(n);
+
+        return VIRTIO_NET_OK;
+    } else {
+        return VIRTIO_NET_ERR;
+    }
+}
+
 static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
                                  struct iovec *iov, unsigned int iov_cnt)
 {
@@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
         } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
             status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
+        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
+            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
         }
 
         s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
@@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
             qemu_put_be32(f, n->vqs[i].tx_waiting);
         }
     }
+
+    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
+        qemu_put_be64(f, n->curr_guest_offloads);
+    }
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
@@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
             error_report("virtio-net: saved image requires vnet_hdr=on");
             return -1;
         }
-
-        if (n->has_vnet_hdr) {
-            tap_set_offload(qemu_get_queue(n->nic)->peer,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
-        }
     }
 
     if (version_id >= 9) {
@@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
         }
     }
 
+    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
+        n->curr_guest_offloads = qemu_get_be64(f);
+    } else {
+        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
+    }
+
+    if (peer_has_vnet_hdr(n)) {
+        virtio_net_apply_guest_offloads(n);
+    }
+
     virtio_net_set_queues(n);
 
     /* Find the first multicast entry in the saved MAC filter */
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 4d1a8cd..70e362d 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -27,6 +27,8 @@
 /* The feature bitmap for virtio net */
 #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
 #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
+#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
+                                         * configuration support */
 #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
 #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
 #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
@@ -182,6 +184,7 @@ typedef struct VirtIONet {
     uint16_t max_queues;
     uint16_t curr_queues;
     size_t config_size;
+    uint64_t curr_guest_offloads;
 } VirtIONet;
 
 #define VIRTIO_NET_CTRL_MAC    1
@@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
 
+/*
+ * Control network offloads
+ *
+ * Dynamic offloads are available with the
+ * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
+ */
+#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
+ #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
+
 #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
         DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
@@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
         DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
         DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
         DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
+        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
         DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
 
 #endif
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
       [not found] ` <1365316448-4604-2-git-send-email-dmitry@daynix.com>
@ 2013-04-22 17:00   ` Michael S. Tsirkin
  2013-04-22 17:58     ` Anthony Liguori
  0 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2013-04-22 17:00 UTC (permalink / raw)
  To: Dmitry Fleytman
  Cc: Rusty.Russell.rusty, qemu-devel, Ronen Hod, Anthony Liguori,
	Yan Vugenfirer, Dmitry Fleytman

On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
> From: Dmitry Fleytman <dfleytma@redhat.com>
> 
> Virtio-net driver currently negotiates network offloads
> on startup via features mechanism and have no ability to
> disable and re-enable offloads later.
> This patch introduced a new control command that allows
> to configure device network offloads state dynamically.
> The patch also introduces a new feature flag
> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> 
> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Pls pick this up for 1.5.

> ---
>  hw/pc.h         |  4 +++
>  hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>  hw/virtio-net.h | 13 ++++++++
>  3 files changed, 99 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/pc.h b/hw/pc.h
> index 8e1dd4c..7ca4698 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>              .property = "vectors",\
>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
>              .value    = stringify(0xFFFFFFFF),\
> +        },{ \
> +            .driver   = "virtio-net-pci", \
> +            .property = "ctrl_guest_offloads", \
> +            .value    = "off", \
>          },{\
>              .driver   = "e1000",\
>              .property = "romfile",\
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 5917740..a92d061 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>      return features;
>  }
>  
> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
> +{
> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
> +}
> +
> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> +{
> +    static const uint64_t guest_offloads_mask =
> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> +
> +    return guest_offloads_mask & features;
> +}
> +
> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> +{
> +    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
> +}
> +
>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>  {
>      VirtIONet *n = to_virtio_net(vdev);
> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
>  
>      if (n->has_vnet_hdr) {
> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> +        n->curr_guest_offloads =
> +            virtio_net_guest_offloads_by_features(features);
> +        virtio_net_apply_guest_offloads(n);
>      }
>  
>      for (i = 0;  i < n->max_queues; i++) {
> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>      return VIRTIO_NET_OK;
>  }
>  
> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
> +                                     struct iovec *iov, unsigned int iov_cnt)
> +{
> +    uint64_t offloads;
> +    size_t s;
> +
> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
> +        return VIRTIO_NET_ERR;
> +    }
> +
> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
> +    if (s != sizeof(offloads)) {
> +        return VIRTIO_NET_ERR;
> +    }
> +
> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
> +        uint64_t supported_offloads;
> +
> +        if (!n->has_vnet_hdr) {
> +            return VIRTIO_NET_ERR;
> +        }
> +
> +        supported_offloads = virtio_net_supported_guest_offloads(n);
> +        if (offloads & ~supported_offloads) {
> +            return VIRTIO_NET_ERR;
> +        }
> +
> +        n->curr_guest_offloads = offloads;
> +        virtio_net_apply_guest_offloads(n);
> +
> +        return VIRTIO_NET_OK;
> +    } else {
> +        return VIRTIO_NET_ERR;
> +    }
> +}
> +
>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
>                                   struct iovec *iov, unsigned int iov_cnt)
>  {
> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>          }
>  
>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>              qemu_put_be32(f, n->vqs[i].tx_waiting);
>          }
>      }
> +
> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> +        qemu_put_be64(f, n->curr_guest_offloads);
> +    }
>  }
>  
>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>              error_report("virtio-net: saved image requires vnet_hdr=on");
>              return -1;
>          }
> -
> -        if (n->has_vnet_hdr) {
> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> -        }
>      }
>  
>      if (version_id >= 9) {
> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>          }
>      }
>  
> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> +        n->curr_guest_offloads = qemu_get_be64(f);
> +    } else {
> +        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
> +    }
> +
> +    if (peer_has_vnet_hdr(n)) {
> +        virtio_net_apply_guest_offloads(n);
> +    }
> +
>      virtio_net_set_queues(n);
>  
>      /* Find the first multicast entry in the saved MAC filter */
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 4d1a8cd..70e362d 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -27,6 +27,8 @@
>  /* The feature bitmap for virtio net */
>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
> +                                         * configuration support */
>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
>      uint16_t max_queues;
>      uint16_t curr_queues;
>      size_t config_size;
> +    uint64_t curr_guest_offloads;
>  } VirtIONet;
>  
>  #define VIRTIO_NET_CTRL_MAC    1
> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
>  
> +/*
> + * Control network offloads
> + *
> + * Dynamic offloads are available with the
> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
> + */
> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
> +
>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
>  
>  #endif
> -- 
> 1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-04-22 17:00   ` Michael S. Tsirkin
@ 2013-04-22 17:58     ` Anthony Liguori
  2013-04-22 21:02       ` Michael S. Tsirkin
                         ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Anthony Liguori @ 2013-04-22 17:58 UTC (permalink / raw)
  To: Michael S. Tsirkin, Dmitry Fleytman
  Cc: Yan Vugenfirer, Dmitry Fleytman, Ronen Hod, Rusty.Russell.rusty,
	qemu-devel

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
>> From: Dmitry Fleytman <dfleytma@redhat.com>
>> 
>> Virtio-net driver currently negotiates network offloads
>> on startup via features mechanism and have no ability to
>> disable and re-enable offloads later.
>> This patch introduced a new control command that allows
>> to configure device network offloads state dynamically.
>> The patch also introduces a new feature flag
>> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
>> 
>> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

I was looking for a Reviewed-by, not just an Acked-by.

Regards,

Anthony Liguori

>
> Pls pick this up for 1.5.
>
>> ---
>>  hw/pc.h         |  4 +++
>>  hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>>  hw/virtio-net.h | 13 ++++++++
>>  3 files changed, 99 insertions(+), 15 deletions(-)
>> 
>> diff --git a/hw/pc.h b/hw/pc.h
>> index 8e1dd4c..7ca4698 100644
>> --- a/hw/pc.h
>> +++ b/hw/pc.h
>> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>>              .property = "vectors",\
>>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
>>              .value    = stringify(0xFFFFFFFF),\
>> +        },{ \
>> +            .driver   = "virtio-net-pci", \
>> +            .property = "ctrl_guest_offloads", \
>> +            .value    = "off", \
>>          },{\
>>              .driver   = "e1000",\
>>              .property = "romfile",\
>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
>> index 5917740..a92d061 100644
>> --- a/hw/virtio-net.c
>> +++ b/hw/virtio-net.c
>> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>>      return features;
>>  }
>>  
>> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
>> +{
>> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
>> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
>> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
>> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
>> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
>> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
>> +}
>> +
>> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
>> +{
>> +    static const uint64_t guest_offloads_mask =
>> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
>> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
>> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
>> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
>> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
>> +
>> +    return guest_offloads_mask & features;
>> +}
>> +
>> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
>> +{
>> +    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
>> +}
>> +
>>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>>  {
>>      VirtIONet *n = to_virtio_net(vdev);
>> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
>>  
>>      if (n->has_vnet_hdr) {
>> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
>> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>> +        n->curr_guest_offloads =
>> +            virtio_net_guest_offloads_by_features(features);
>> +        virtio_net_apply_guest_offloads(n);
>>      }
>>  
>>      for (i = 0;  i < n->max_queues; i++) {
>> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>>      return VIRTIO_NET_OK;
>>  }
>>  
>> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
>> +                                     struct iovec *iov, unsigned int iov_cnt)
>> +{
>> +    uint64_t offloads;
>> +    size_t s;
>> +
>> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
>> +        return VIRTIO_NET_ERR;
>> +    }
>> +
>> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
>> +    if (s != sizeof(offloads)) {
>> +        return VIRTIO_NET_ERR;
>> +    }
>> +
>> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
>> +        uint64_t supported_offloads;
>> +
>> +        if (!n->has_vnet_hdr) {
>> +            return VIRTIO_NET_ERR;
>> +        }
>> +
>> +        supported_offloads = virtio_net_supported_guest_offloads(n);
>> +        if (offloads & ~supported_offloads) {
>> +            return VIRTIO_NET_ERR;
>> +        }
>> +
>> +        n->curr_guest_offloads = offloads;
>> +        virtio_net_apply_guest_offloads(n);
>> +
>> +        return VIRTIO_NET_OK;
>> +    } else {
>> +        return VIRTIO_NET_ERR;
>> +    }
>> +}
>> +
>>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
>>                                   struct iovec *iov, unsigned int iov_cnt)
>>  {
>> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
>>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
>>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
>> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>>          }
>>  
>>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
>> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>>              qemu_put_be32(f, n->vqs[i].tx_waiting);
>>          }
>>      }
>> +
>> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
>> +        qemu_put_be64(f, n->curr_guest_offloads);
>> +    }
>>  }
>>  
>>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>>              error_report("virtio-net: saved image requires vnet_hdr=on");
>>              return -1;
>>          }
>> -
>> -        if (n->has_vnet_hdr) {
>> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>> -        }
>>      }
>>  
>>      if (version_id >= 9) {
>> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>>          }
>>      }
>>  
>> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
>> +        n->curr_guest_offloads = qemu_get_be64(f);
>> +    } else {
>> +        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
>> +    }
>> +
>> +    if (peer_has_vnet_hdr(n)) {
>> +        virtio_net_apply_guest_offloads(n);
>> +    }
>> +
>>      virtio_net_set_queues(n);
>>  
>>      /* Find the first multicast entry in the saved MAC filter */
>> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
>> index 4d1a8cd..70e362d 100644
>> --- a/hw/virtio-net.h
>> +++ b/hw/virtio-net.h
>> @@ -27,6 +27,8 @@
>>  /* The feature bitmap for virtio net */
>>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
>>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
>> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
>> +                                         * configuration support */
>>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
>>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
>>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
>> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
>>      uint16_t max_queues;
>>      uint16_t curr_queues;
>>      size_t config_size;
>> +    uint64_t curr_guest_offloads;
>>  } VirtIONet;
>>  
>>  #define VIRTIO_NET_CTRL_MAC    1
>> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
>>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
>>  
>> +/*
>> + * Control network offloads
>> + *
>> + * Dynamic offloads are available with the
>> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
>> + */
>> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
>> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
>> +
>>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
>>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
>>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
>> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
>>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
>>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
>>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
>> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
>>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
>>  
>>  #endif
>> -- 
>> 1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-04-22 17:58     ` Anthony Liguori
@ 2013-04-22 21:02       ` Michael S. Tsirkin
  2013-05-02  5:20       ` Michael S. Tsirkin
  2013-05-19 11:56       ` Michael S. Tsirkin
  2 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2013-04-22 21:02 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Rusty.Russell.rusty, qemu-devel, Yan Vugenfirer, Ronen Hod,
	Dmitry Fleytman, Dmitry Fleytman

On Mon, Apr 22, 2013 at 12:58:26PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
> >> From: Dmitry Fleytman <dfleytma@redhat.com>
> >> 
> >> Virtio-net driver currently negotiates network offloads
> >> on startup via features mechanism and have no ability to
> >> disable and re-enable offloads later.
> >> This patch introduced a new control command that allows
> >> to configure device network offloads state dynamically.
> >> The patch also introduces a new feature flag
> >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> >> 
> >> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I was looking for a Reviewed-by, not just an Acked-by.
> 
> Regards,
> 
> Anthony Liguori
> 


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> >
> > Pls pick this up for 1.5.
> >
> >> ---
> >>  hw/pc.h         |  4 +++
> >>  hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> >>  hw/virtio-net.h | 13 ++++++++
> >>  3 files changed, 99 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/hw/pc.h b/hw/pc.h
> >> index 8e1dd4c..7ca4698 100644
> >> --- a/hw/pc.h
> >> +++ b/hw/pc.h
> >> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>              .property = "vectors",\
> >>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> >>              .value    = stringify(0xFFFFFFFF),\
> >> +        },{ \
> >> +            .driver   = "virtio-net-pci", \
> >> +            .property = "ctrl_guest_offloads", \
> >> +            .value    = "off", \
> >>          },{\
> >>              .driver   = "e1000",\
> >>              .property = "romfile",\
> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> >> index 5917740..a92d061 100644
> >> --- a/hw/virtio-net.c
> >> +++ b/hw/virtio-net.c
> >> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
> >>      return features;
> >>  }
> >>  
> >> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
> >> +{
> >> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
> >> +}
> >> +
> >> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> >> +{
> >> +    static const uint64_t guest_offloads_mask =
> >> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> >> +
> >> +    return guest_offloads_mask & features;
> >> +}
> >> +
> >> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> >> +{
> >> +    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
> >> +}
> >> +
> >>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>  {
> >>      VirtIONet *n = to_virtio_net(vdev);
> >> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
> >>  
> >>      if (n->has_vnet_hdr) {
> >> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> +        n->curr_guest_offloads =
> >> +            virtio_net_guest_offloads_by_features(features);
> >> +        virtio_net_apply_guest_offloads(n);
> >>      }
> >>  
> >>      for (i = 0;  i < n->max_queues; i++) {
> >> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> >>      return VIRTIO_NET_OK;
> >>  }
> >>  
> >> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
> >> +                                     struct iovec *iov, unsigned int iov_cnt)
> >> +{
> >> +    uint64_t offloads;
> >> +    size_t s;
> >> +
> >> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +
> >> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
> >> +    if (s != sizeof(offloads)) {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +
> >> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
> >> +        uint64_t supported_offloads;
> >> +
> >> +        if (!n->has_vnet_hdr) {
> >> +            return VIRTIO_NET_ERR;
> >> +        }
> >> +
> >> +        supported_offloads = virtio_net_supported_guest_offloads(n);
> >> +        if (offloads & ~supported_offloads) {
> >> +            return VIRTIO_NET_ERR;
> >> +        }
> >> +
> >> +        n->curr_guest_offloads = offloads;
> >> +        virtio_net_apply_guest_offloads(n);
> >> +
> >> +        return VIRTIO_NET_OK;
> >> +    } else {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +}
> >> +
> >>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> >>                                   struct iovec *iov, unsigned int iov_cnt)
> >>  {
> >> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
> >>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> >>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> >> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> >> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> >>          }
> >>  
> >>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
> >> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
> >>              qemu_put_be32(f, n->vqs[i].tx_waiting);
> >>          }
> >>      }
> >> +
> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> >> +        qemu_put_be64(f, n->curr_guest_offloads);
> >> +    }
> >>  }
> >>  
> >>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >>              error_report("virtio-net: saved image requires vnet_hdr=on");
> >>              return -1;
> >>          }
> >> -
> >> -        if (n->has_vnet_hdr) {
> >> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> -        }
> >>      }
> >>  
> >>      if (version_id >= 9) {
> >> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >>          }
> >>      }
> >>  
> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> >> +        n->curr_guest_offloads = qemu_get_be64(f);
> >> +    } else {
> >> +        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
> >> +    }
> >> +
> >> +    if (peer_has_vnet_hdr(n)) {
> >> +        virtio_net_apply_guest_offloads(n);
> >> +    }
> >> +
> >>      virtio_net_set_queues(n);
> >>  
> >>      /* Find the first multicast entry in the saved MAC filter */
> >> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> >> index 4d1a8cd..70e362d 100644
> >> --- a/hw/virtio-net.h
> >> +++ b/hw/virtio-net.h
> >> @@ -27,6 +27,8 @@
> >>  /* The feature bitmap for virtio net */
> >>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
> >>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
> >> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
> >> +                                         * configuration support */
> >>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
> >>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
> >>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
> >> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
> >>      uint16_t max_queues;
> >>      uint16_t curr_queues;
> >>      size_t config_size;
> >> +    uint64_t curr_guest_offloads;
> >>  } VirtIONet;
> >>  
> >>  #define VIRTIO_NET_CTRL_MAC    1
> >> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> >>  
> >> +/*
> >> + * Control network offloads
> >> + *
> >> + * Dynamic offloads are available with the
> >> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
> >> + */
> >> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
> >> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
> >> +
> >>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
> >>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
> >>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
> >> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
> >>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
> >>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
> >>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
> >> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
> >>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
> >>  
> >>  #endif
> >> -- 
> >> 1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-04-22 17:58     ` Anthony Liguori
  2013-04-22 21:02       ` Michael S. Tsirkin
@ 2013-05-02  5:20       ` Michael S. Tsirkin
  2013-05-07  7:23         ` Dmitry Fleytman
  2013-05-19 11:56       ` Michael S. Tsirkin
  2 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2013-05-02  5:20 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Rusty.Russell.rusty, qemu-devel, Yan Vugenfirer, Ronen Hod,
	Dmitry Fleytman, Dmitry Fleytman

On Mon, Apr 22, 2013 at 12:58:26PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
> >> From: Dmitry Fleytman <dfleytma@redhat.com>
> >> 
> >> Virtio-net driver currently negotiates network offloads
> >> on startup via features mechanism and have no ability to
> >> disable and re-enable offloads later.
> >> This patch introduced a new control command that allows
> >> to configure device network offloads state dynamically.
> >> The patch also introduces a new feature flag
> >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> >> 
> >> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I was looking for a Reviewed-by, not just an Acked-by.
> 
> Regards,
> 
> Anthony Liguori

Ping. We can't pass WHQL without this support -
could this be merged for 1.5 pretty please?

> >
> > Pls pick this up for 1.5.
> >
> >> ---
> >>  hw/pc.h         |  4 +++
> >>  hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> >>  hw/virtio-net.h | 13 ++++++++
> >>  3 files changed, 99 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/hw/pc.h b/hw/pc.h
> >> index 8e1dd4c..7ca4698 100644
> >> --- a/hw/pc.h
> >> +++ b/hw/pc.h
> >> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>              .property = "vectors",\
> >>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> >>              .value    = stringify(0xFFFFFFFF),\
> >> +        },{ \
> >> +            .driver   = "virtio-net-pci", \
> >> +            .property = "ctrl_guest_offloads", \
> >> +            .value    = "off", \
> >>          },{\
> >>              .driver   = "e1000",\
> >>              .property = "romfile",\
> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> >> index 5917740..a92d061 100644
> >> --- a/hw/virtio-net.c
> >> +++ b/hw/virtio-net.c
> >> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
> >>      return features;
> >>  }
> >>  
> >> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
> >> +{
> >> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
> >> +}
> >> +
> >> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> >> +{
> >> +    static const uint64_t guest_offloads_mask =
> >> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> >> +
> >> +    return guest_offloads_mask & features;
> >> +}
> >> +
> >> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> >> +{
> >> +    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
> >> +}
> >> +
> >>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>  {
> >>      VirtIONet *n = to_virtio_net(vdev);
> >> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
> >>  
> >>      if (n->has_vnet_hdr) {
> >> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> +        n->curr_guest_offloads =
> >> +            virtio_net_guest_offloads_by_features(features);
> >> +        virtio_net_apply_guest_offloads(n);
> >>      }
> >>  
> >>      for (i = 0;  i < n->max_queues; i++) {
> >> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> >>      return VIRTIO_NET_OK;
> >>  }
> >>  
> >> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
> >> +                                     struct iovec *iov, unsigned int iov_cnt)
> >> +{
> >> +    uint64_t offloads;
> >> +    size_t s;
> >> +
> >> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +
> >> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
> >> +    if (s != sizeof(offloads)) {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +
> >> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
> >> +        uint64_t supported_offloads;
> >> +
> >> +        if (!n->has_vnet_hdr) {
> >> +            return VIRTIO_NET_ERR;
> >> +        }
> >> +
> >> +        supported_offloads = virtio_net_supported_guest_offloads(n);
> >> +        if (offloads & ~supported_offloads) {
> >> +            return VIRTIO_NET_ERR;
> >> +        }
> >> +
> >> +        n->curr_guest_offloads = offloads;
> >> +        virtio_net_apply_guest_offloads(n);
> >> +
> >> +        return VIRTIO_NET_OK;
> >> +    } else {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +}
> >> +
> >>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> >>                                   struct iovec *iov, unsigned int iov_cnt)
> >>  {
> >> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
> >>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> >>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> >> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> >> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> >>          }
> >>  
> >>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
> >> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
> >>              qemu_put_be32(f, n->vqs[i].tx_waiting);
> >>          }
> >>      }
> >> +
> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> >> +        qemu_put_be64(f, n->curr_guest_offloads);
> >> +    }
> >>  }
> >>  
> >>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >>              error_report("virtio-net: saved image requires vnet_hdr=on");
> >>              return -1;
> >>          }
> >> -
> >> -        if (n->has_vnet_hdr) {
> >> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> -        }
> >>      }
> >>  
> >>      if (version_id >= 9) {
> >> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >>          }
> >>      }
> >>  
> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> >> +        n->curr_guest_offloads = qemu_get_be64(f);
> >> +    } else {
> >> +        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
> >> +    }
> >> +
> >> +    if (peer_has_vnet_hdr(n)) {
> >> +        virtio_net_apply_guest_offloads(n);
> >> +    }
> >> +
> >>      virtio_net_set_queues(n);
> >>  
> >>      /* Find the first multicast entry in the saved MAC filter */
> >> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> >> index 4d1a8cd..70e362d 100644
> >> --- a/hw/virtio-net.h
> >> +++ b/hw/virtio-net.h
> >> @@ -27,6 +27,8 @@
> >>  /* The feature bitmap for virtio net */
> >>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
> >>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
> >> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
> >> +                                         * configuration support */
> >>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
> >>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
> >>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
> >> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
> >>      uint16_t max_queues;
> >>      uint16_t curr_queues;
> >>      size_t config_size;
> >> +    uint64_t curr_guest_offloads;
> >>  } VirtIONet;
> >>  
> >>  #define VIRTIO_NET_CTRL_MAC    1
> >> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> >>  
> >> +/*
> >> + * Control network offloads
> >> + *
> >> + * Dynamic offloads are available with the
> >> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
> >> + */
> >> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
> >> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
> >> +
> >>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
> >>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
> >>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
> >> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
> >>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
> >>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
> >>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
> >> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
> >>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
> >>  
> >>  #endif
> >> -- 
> >> 1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-05-02  5:20       ` Michael S. Tsirkin
@ 2013-05-07  7:23         ` Dmitry Fleytman
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Fleytman @ 2013-05-07  7:23 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty.Russell.rusty, qemu-devel, Ronen Hod, Anthony Liguori,
	Yan Vugenfirer, Dmitry Fleytman

[-- Attachment #1: Type: text/plain, Size: 11118 bytes --]

Hello All,

Is there any new regarding this patch?

Thanks,
Dmitry


On Thu, May 2, 2013 at 8:20 AM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Mon, Apr 22, 2013 at 12:58:26PM -0500, Anthony Liguori wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> >
> > > On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
> > >> From: Dmitry Fleytman <dfleytma@redhat.com>
> > >>
> > >> Virtio-net driver currently negotiates network offloads
> > >> on startup via features mechanism and have no ability to
> > >> disable and re-enable offloads later.
> > >> This patch introduced a new control command that allows
> > >> to configure device network offloads state dynamically.
> > >> The patch also introduces a new feature flag
> > >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> > >>
> > >> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> > >
> > > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > I was looking for a Reviewed-by, not just an Acked-by.
> >
> > Regards,
> >
> > Anthony Liguori
>
> Ping. We can't pass WHQL without this support -
> could this be merged for 1.5 pretty please?
>
> > >
> > > Pls pick this up for 1.5.
> > >
> > >> ---
> > >>  hw/pc.h         |  4 +++
> > >>  hw/virtio-net.c | 97
> ++++++++++++++++++++++++++++++++++++++++++++++++---------
> > >>  hw/virtio-net.h | 13 ++++++++
> > >>  3 files changed, 99 insertions(+), 15 deletions(-)
> > >>
> > >> diff --git a/hw/pc.h b/hw/pc.h
> > >> index 8e1dd4c..7ca4698 100644
> > >> --- a/hw/pc.h
> > >> +++ b/hw/pc.h
> > >> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> > >>              .property = "vectors",\
> > >>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> > >>              .value    = stringify(0xFFFFFFFF),\
> > >> +        },{ \
> > >> +            .driver   = "virtio-net-pci", \
> > >> +            .property = "ctrl_guest_offloads", \
> > >> +            .value    = "off", \
> > >>          },{\
> > >>              .driver   = "e1000",\
> > >>              .property = "romfile",\
> > >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > >> index 5917740..a92d061 100644
> > >> --- a/hw/virtio-net.c
> > >> +++ b/hw/virtio-net.c
> > >> @@ -360,6 +360,33 @@ static uint32_t
> virtio_net_bad_features(VirtIODevice *vdev)
> > >>      return features;
> > >>  }
> > >>
> > >> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
> > >> +{
> > >> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> > >> +            !!(n->curr_guest_offloads & (1ULL <<
> VIRTIO_NET_F_GUEST_CSUM)),
> > >> +            !!(n->curr_guest_offloads & (1ULL <<
> VIRTIO_NET_F_GUEST_TSO4)),
> > >> +            !!(n->curr_guest_offloads & (1ULL <<
> VIRTIO_NET_F_GUEST_TSO6)),
> > >> +            !!(n->curr_guest_offloads & (1ULL <<
> VIRTIO_NET_F_GUEST_ECN)),
> > >> +            !!(n->curr_guest_offloads & (1ULL <<
> VIRTIO_NET_F_GUEST_UFO)));
> > >> +}
> > >> +
> > >> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t
> features)
> > >> +{
> > >> +    static const uint64_t guest_offloads_mask =
> > >> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> > >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> > >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> > >> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> > >> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> > >> +
> > >> +    return guest_offloads_mask & features;
> > >> +}
> > >> +
> > >> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet
> *n)
> > >> +{
> > >> +    return
> virtio_net_guest_offloads_by_features(n->vdev.guest_features);
> > >> +}
> > >> +
> > >>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t
> features)
> > >>  {
> > >>      VirtIONet *n = to_virtio_net(vdev);
> > >> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice
> *vdev, uint32_t features)
> > >>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 <<
> VIRTIO_NET_F_MRG_RXBUF)));
> > >>
> > >>      if (n->has_vnet_hdr) {
> > >> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> > >> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> > >> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> > >> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> > >> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> > >> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> > >> +        n->curr_guest_offloads =
> > >> +            virtio_net_guest_offloads_by_features(features);
> > >> +        virtio_net_apply_guest_offloads(n);
> > >>      }
> > >>
> > >>      for (i = 0;  i < n->max_queues; i++) {
> > >> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet
> *n, uint8_t cmd,
> > >>      return VIRTIO_NET_OK;
> > >>  }
> > >>
> > >> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
> > >> +                                     struct iovec *iov, unsigned int
> iov_cnt)
> > >> +{
> > >> +    uint64_t offloads;
> > >> +    size_t s;
> > >> +
> > >> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) &
> n->vdev.guest_features)) {
> > >> +        return VIRTIO_NET_ERR;
> > >> +    }
> > >> +
> > >> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
> > >> +    if (s != sizeof(offloads)) {
> > >> +        return VIRTIO_NET_ERR;
> > >> +    }
> > >> +
> > >> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
> > >> +        uint64_t supported_offloads;
> > >> +
> > >> +        if (!n->has_vnet_hdr) {
> > >> +            return VIRTIO_NET_ERR;
> > >> +        }
> > >> +
> > >> +        supported_offloads = virtio_net_supported_guest_offloads(n);
> > >> +        if (offloads & ~supported_offloads) {
> > >> +            return VIRTIO_NET_ERR;
> > >> +        }
> > >> +
> > >> +        n->curr_guest_offloads = offloads;
> > >> +        virtio_net_apply_guest_offloads(n);
> > >> +
> > >> +        return VIRTIO_NET_OK;
> > >> +    } else {
> > >> +        return VIRTIO_NET_ERR;
> > >> +    }
> > >> +}
> > >> +
> > >>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> > >>                                   struct iovec *iov, unsigned int
> iov_cnt)
> > >>  {
> > >> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice
> *vdev, VirtQueue *vq)
> > >>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov,
> iov_cnt);
> > >>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> > >>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> > >> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> > >> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov,
> iov_cnt);
> > >>          }
> > >>
> > >>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status,
> sizeof(status));
> > >> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void
> *opaque)
> > >>              qemu_put_be32(f, n->vqs[i].tx_waiting);
> > >>          }
> > >>      }
> > >> +
> > >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) &
> n->vdev.guest_features) {
> > >> +        qemu_put_be64(f, n->curr_guest_offloads);
> > >> +    }
> > >>  }
> > >>
> > >>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> > >> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void
> *opaque, int version_id)
> > >>              error_report("virtio-net: saved image requires
> vnet_hdr=on");
> > >>              return -1;
> > >>          }
> > >> -
> > >> -        if (n->has_vnet_hdr) {
> > >> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
> > >> -                    (n->vdev.guest_features >>
> VIRTIO_NET_F_GUEST_CSUM) & 1,
> > >> -                    (n->vdev.guest_features >>
> VIRTIO_NET_F_GUEST_TSO4) & 1,
> > >> -                    (n->vdev.guest_features >>
> VIRTIO_NET_F_GUEST_TSO6) & 1,
> > >> -                    (n->vdev.guest_features >>
> VIRTIO_NET_F_GUEST_ECN)  & 1,
> > >> -                    (n->vdev.guest_features >>
> VIRTIO_NET_F_GUEST_UFO)  & 1);
> > >> -        }
> > >>      }
> > >>
> > >>      if (version_id >= 9) {
> > >> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void
> *opaque, int version_id)
> > >>          }
> > >>      }
> > >>
> > >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) &
> n->vdev.guest_features) {
> > >> +        n->curr_guest_offloads = qemu_get_be64(f);
> > >> +    } else {
> > >> +        n->curr_guest_offloads =
> virtio_net_supported_guest_offloads(n);
> > >> +    }
> > >> +
> > >> +    if (peer_has_vnet_hdr(n)) {
> > >> +        virtio_net_apply_guest_offloads(n);
> > >> +    }
> > >> +
> > >>      virtio_net_set_queues(n);
> > >>
> > >>      /* Find the first multicast entry in the saved MAC filter */
> > >> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> > >> index 4d1a8cd..70e362d 100644
> > >> --- a/hw/virtio-net.h
> > >> +++ b/hw/virtio-net.h
> > >> @@ -27,6 +27,8 @@
> > >>  /* The feature bitmap for virtio net */
> > >>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/
> partial csum */
> > >>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/
> partial csum */
> > >> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
> > >> +                                         * configuration support */
> > >>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC
> address. */
> > >>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any
> GSO type */
> > >>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4
> in. */
> > >> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
> > >>      uint16_t max_queues;
> > >>      uint16_t curr_queues;
> > >>      size_t config_size;
> > >> +    uint64_t curr_guest_offloads;
> > >>  } VirtIONet;
> > >>
> > >>  #define VIRTIO_NET_CTRL_MAC    1
> > >> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
> > >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
> > >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> > >>
> > >> +/*
> > >> + * Control network offloads
> > >> + *
> > >> + * Dynamic offloads are available with the
> > >> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
> > >> + */
> > >> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
> > >> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
> > >> +
> > >>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
> > >>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
> > >>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM,
> true), \
> > >> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
> > >>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field,
> VIRTIO_NET_F_CTRL_VLAN, true), \
> > >>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field,
> VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
> > >>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field,
> VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
> > >> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field,
> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
> > >>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
> > >>
> > >>  #endif
> > >> --
> > >> 1.8.1.4
>

[-- Attachment #2: Type: text/html, Size: 15025 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-04-22 17:58     ` Anthony Liguori
  2013-04-22 21:02       ` Michael S. Tsirkin
  2013-05-02  5:20       ` Michael S. Tsirkin
@ 2013-05-19 11:56       ` Michael S. Tsirkin
  2013-05-19 23:40         ` Anthony Liguori
  2 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2013-05-19 11:56 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Rusty.Russell.rusty, qemu-devel, Yan Vugenfirer, Ronen Hod,
	Dmitry Fleytman, Dmitry Fleytman

On Mon, Apr 22, 2013 at 12:58:26PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
> >> From: Dmitry Fleytman <dfleytma@redhat.com>
> >> 
> >> Virtio-net driver currently negotiates network offloads
> >> on startup via features mechanism and have no ability to
> >> disable and re-enable offloads later.
> >> This patch introduced a new control command that allows
> >> to configure device network offloads state dynamically.
> >> The patch also introduces a new feature flag
> >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> >> 
> >> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I was looking for a Reviewed-by, not just an Acked-by.
> 
> Regards,
> 
> Anthony Liguori

Is there anything else we are looking for?
Can we add this patch in 1.5?



> >
> > Pls pick this up for 1.5.
> >
> >> ---
> >>  hw/pc.h         |  4 +++
> >>  hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> >>  hw/virtio-net.h | 13 ++++++++
> >>  3 files changed, 99 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/hw/pc.h b/hw/pc.h
> >> index 8e1dd4c..7ca4698 100644
> >> --- a/hw/pc.h
> >> +++ b/hw/pc.h
> >> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>              .property = "vectors",\
> >>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> >>              .value    = stringify(0xFFFFFFFF),\
> >> +        },{ \
> >> +            .driver   = "virtio-net-pci", \
> >> +            .property = "ctrl_guest_offloads", \
> >> +            .value    = "off", \
> >>          },{\
> >>              .driver   = "e1000",\
> >>              .property = "romfile",\
> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> >> index 5917740..a92d061 100644
> >> --- a/hw/virtio-net.c
> >> +++ b/hw/virtio-net.c
> >> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
> >>      return features;
> >>  }
> >>  
> >> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
> >> +{
> >> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
> >> +}
> >> +
> >> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> >> +{
> >> +    static const uint64_t guest_offloads_mask =
> >> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> >> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
> >> +
> >> +    return guest_offloads_mask & features;
> >> +}
> >> +
> >> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> >> +{
> >> +    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
> >> +}
> >> +
> >>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>  {
> >>      VirtIONet *n = to_virtio_net(vdev);
> >> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
> >>  
> >>      if (n->has_vnet_hdr) {
> >> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> +        n->curr_guest_offloads =
> >> +            virtio_net_guest_offloads_by_features(features);
> >> +        virtio_net_apply_guest_offloads(n);
> >>      }
> >>  
> >>      for (i = 0;  i < n->max_queues; i++) {
> >> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> >>      return VIRTIO_NET_OK;
> >>  }
> >>  
> >> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
> >> +                                     struct iovec *iov, unsigned int iov_cnt)
> >> +{
> >> +    uint64_t offloads;
> >> +    size_t s;
> >> +
> >> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +
> >> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
> >> +    if (s != sizeof(offloads)) {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +
> >> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
> >> +        uint64_t supported_offloads;
> >> +
> >> +        if (!n->has_vnet_hdr) {
> >> +            return VIRTIO_NET_ERR;
> >> +        }
> >> +
> >> +        supported_offloads = virtio_net_supported_guest_offloads(n);
> >> +        if (offloads & ~supported_offloads) {
> >> +            return VIRTIO_NET_ERR;
> >> +        }
> >> +
> >> +        n->curr_guest_offloads = offloads;
> >> +        virtio_net_apply_guest_offloads(n);
> >> +
> >> +        return VIRTIO_NET_OK;
> >> +    } else {
> >> +        return VIRTIO_NET_ERR;
> >> +    }
> >> +}
> >> +
> >>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> >>                                   struct iovec *iov, unsigned int iov_cnt)
> >>  {
> >> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
> >>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> >>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> >> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> >> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> >>          }
> >>  
> >>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
> >> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
> >>              qemu_put_be32(f, n->vqs[i].tx_waiting);
> >>          }
> >>      }
> >> +
> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> >> +        qemu_put_be64(f, n->curr_guest_offloads);
> >> +    }
> >>  }
> >>  
> >>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >>              error_report("virtio-net: saved image requires vnet_hdr=on");
> >>              return -1;
> >>          }
> >> -
> >> -        if (n->has_vnet_hdr) {
> >> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> -        }
> >>      }
> >>  
> >>      if (version_id >= 9) {
> >> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> >>          }
> >>      }
> >>  
> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
> >> +        n->curr_guest_offloads = qemu_get_be64(f);
> >> +    } else {
> >> +        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
> >> +    }
> >> +
> >> +    if (peer_has_vnet_hdr(n)) {
> >> +        virtio_net_apply_guest_offloads(n);
> >> +    }
> >> +
> >>      virtio_net_set_queues(n);
> >>  
> >>      /* Find the first multicast entry in the saved MAC filter */
> >> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> >> index 4d1a8cd..70e362d 100644
> >> --- a/hw/virtio-net.h
> >> +++ b/hw/virtio-net.h
> >> @@ -27,6 +27,8 @@
> >>  /* The feature bitmap for virtio net */
> >>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
> >>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
> >> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
> >> +                                         * configuration support */
> >>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
> >>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
> >>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
> >> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
> >>      uint16_t max_queues;
> >>      uint16_t curr_queues;
> >>      size_t config_size;
> >> +    uint64_t curr_guest_offloads;
> >>  } VirtIONet;
> >>  
> >>  #define VIRTIO_NET_CTRL_MAC    1
> >> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> >>  
> >> +/*
> >> + * Control network offloads
> >> + *
> >> + * Dynamic offloads are available with the
> >> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
> >> + */
> >> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
> >> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
> >> +
> >>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
> >>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
> >>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
> >> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
> >>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
> >>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
> >>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
> >> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
> >>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
> >>  
> >>  #endif
> >> -- 
> >> 1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
  2013-05-19 11:56       ` Michael S. Tsirkin
@ 2013-05-19 23:40         ` Anthony Liguori
  0 siblings, 0 replies; 9+ messages in thread
From: Anthony Liguori @ 2013-05-19 23:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty.Russell.rusty, qemu-devel, Yan Vugenfirer, Ronen Hod,
	Dmitry Fleytman, Dmitry Fleytman

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Apr 22, 2013 at 12:58:26PM -0500, Anthony Liguori wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
>> >> From: Dmitry Fleytman <dfleytma@redhat.com>
>> >> 
>> >> Virtio-net driver currently negotiates network offloads
>> >> on startup via features mechanism and have no ability to
>> >> disable and re-enable offloads later.
>> >> This patch introduced a new control command that allows
>> >> to configure device network offloads state dynamically.
>> >> The patch also introduces a new feature flag
>> >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
>> >> 
>> >> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
>> >
>> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> 
>> I was looking for a Reviewed-by, not just an Acked-by.
>> 
>> Regards,
>> 
>> Anthony Liguori
>
> Is there anything else we are looking for?
> Can we add this patch in 1.5?

1.5 is done.  This patch hasn't applied for quite a long time so it
needs to be resubmitted.

Regards,

Anthony Liguori

>
>
>
>> >
>> > Pls pick this up for 1.5.
>> >
>> >> ---
>> >>  hw/pc.h         |  4 +++
>> >>  hw/virtio-net.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>> >>  hw/virtio-net.h | 13 ++++++++
>> >>  3 files changed, 99 insertions(+), 15 deletions(-)
>> >> 
>> >> diff --git a/hw/pc.h b/hw/pc.h
>> >> index 8e1dd4c..7ca4698 100644
>> >> --- a/hw/pc.h
>> >> +++ b/hw/pc.h
>> >> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>> >>              .property = "vectors",\
>> >>              /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
>> >>              .value    = stringify(0xFFFFFFFF),\
>> >> +        },{ \
>> >> +            .driver   = "virtio-net-pci", \
>> >> +            .property = "ctrl_guest_offloads", \
>> >> +            .value    = "off", \
>> >>          },{\
>> >>              .driver   = "e1000",\
>> >>              .property = "romfile",\
>> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
>> >> index 5917740..a92d061 100644
>> >> --- a/hw/virtio-net.c
>> >> +++ b/hw/virtio-net.c
>> >> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>> >>      return features;
>> >>  }
>> >>  
>> >> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
>> >> +{
>> >> +    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
>> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
>> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
>> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
>> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
>> >> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
>> >> +}
>> >> +
>> >> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
>> >> +{
>> >> +    static const uint64_t guest_offloads_mask =
>> >> +        (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
>> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
>> >> +        (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
>> >> +        (1ULL << VIRTIO_NET_F_GUEST_ECN)  |
>> >> +        (1ULL << VIRTIO_NET_F_GUEST_UFO);
>> >> +
>> >> +    return guest_offloads_mask & features;
>> >> +}
>> >> +
>> >> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
>> >> +{
>> >> +    return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
>> >> +}
>> >> +
>> >>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>> >>  {
>> >>      VirtIONet *n = to_virtio_net(vdev);
>> >> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>> >>      virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF)));
>> >>  
>> >>      if (n->has_vnet_hdr) {
>> >> -        tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
>> >> -                        (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>> >> -                        (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>> >> -                        (features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>> >> -                        (features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>> >> +        n->curr_guest_offloads =
>> >> +            virtio_net_guest_offloads_by_features(features);
>> >> +        virtio_net_apply_guest_offloads(n);
>> >>      }
>> >>  
>> >>      for (i = 0;  i < n->max_queues; i++) {
>> >> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>> >>      return VIRTIO_NET_OK;
>> >>  }
>> >>  
>> >> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
>> >> +                                     struct iovec *iov, unsigned int iov_cnt)
>> >> +{
>> >> +    uint64_t offloads;
>> >> +    size_t s;
>> >> +
>> >> +    if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features)) {
>> >> +        return VIRTIO_NET_ERR;
>> >> +    }
>> >> +
>> >> +    s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
>> >> +    if (s != sizeof(offloads)) {
>> >> +        return VIRTIO_NET_ERR;
>> >> +    }
>> >> +
>> >> +    if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
>> >> +        uint64_t supported_offloads;
>> >> +
>> >> +        if (!n->has_vnet_hdr) {
>> >> +            return VIRTIO_NET_ERR;
>> >> +        }
>> >> +
>> >> +        supported_offloads = virtio_net_supported_guest_offloads(n);
>> >> +        if (offloads & ~supported_offloads) {
>> >> +            return VIRTIO_NET_ERR;
>> >> +        }
>> >> +
>> >> +        n->curr_guest_offloads = offloads;
>> >> +        virtio_net_apply_guest_offloads(n);
>> >> +
>> >> +        return VIRTIO_NET_OK;
>> >> +    } else {
>> >> +        return VIRTIO_NET_ERR;
>> >> +    }
>> >> +}
>> >> +
>> >>  static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
>> >>                                   struct iovec *iov, unsigned int iov_cnt)
>> >>  {
>> >> @@ -591,6 +651,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>> >>              status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
>> >>          } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
>> >>              status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
>> >> +        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>> >> +            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>> >>          }
>> >>  
>> >>          s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
>> >> @@ -1100,6 +1162,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>> >>              qemu_put_be32(f, n->vqs[i].tx_waiting);
>> >>          }
>> >>      }
>> >> +
>> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
>> >> +        qemu_put_be64(f, n->curr_guest_offloads);
>> >> +    }
>> >>  }
>> >>  
>> >>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>> >> @@ -1156,15 +1222,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>> >>              error_report("virtio-net: saved image requires vnet_hdr=on");
>> >>              return -1;
>> >>          }
>> >> -
>> >> -        if (n->has_vnet_hdr) {
>> >> -            tap_set_offload(qemu_get_queue(n->nic)->peer,
>> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>> >> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>> >> -        }
>> >>      }
>> >>  
>> >>      if (version_id >= 9) {
>> >> @@ -1198,6 +1255,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>> >>          }
>> >>      }
>> >>  
>> >> +    if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & n->vdev.guest_features) {
>> >> +        n->curr_guest_offloads = qemu_get_be64(f);
>> >> +    } else {
>> >> +        n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
>> >> +    }
>> >> +
>> >> +    if (peer_has_vnet_hdr(n)) {
>> >> +        virtio_net_apply_guest_offloads(n);
>> >> +    }
>> >> +
>> >>      virtio_net_set_queues(n);
>> >>  
>> >>      /* Find the first multicast entry in the saved MAC filter */
>> >> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
>> >> index 4d1a8cd..70e362d 100644
>> >> --- a/hw/virtio-net.h
>> >> +++ b/hw/virtio-net.h
>> >> @@ -27,6 +27,8 @@
>> >>  /* The feature bitmap for virtio net */
>> >>  #define VIRTIO_NET_F_CSUM       0       /* Host handles pkts w/ partial csum */
>> >>  #define VIRTIO_NET_F_GUEST_CSUM 1       /* Guest handles pkts w/ partial csum */
>> >> +#define VIRTIO_NET_F_CTRL_GUEST_OFFLOADS 2 /* Control channel offload
>> >> +                                         * configuration support */
>> >>  #define VIRTIO_NET_F_MAC        5       /* Host has given MAC address. */
>> >>  #define VIRTIO_NET_F_GSO        6       /* Host handles pkts w/ any GSO type */
>> >>  #define VIRTIO_NET_F_GUEST_TSO4 7       /* Guest can handle TSOv4 in. */
>> >> @@ -182,6 +184,7 @@ typedef struct VirtIONet {
>> >>      uint16_t max_queues;
>> >>      uint16_t curr_queues;
>> >>      size_t config_size;
>> >> +    uint64_t curr_guest_offloads;
>> >>  } VirtIONet;
>> >>  
>> >>  #define VIRTIO_NET_CTRL_MAC    1
>> >> @@ -221,6 +224,15 @@ struct virtio_net_ctrl_mq {
>> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>> >>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
>> >>  
>> >> +/*
>> >> + * Control network offloads
>> >> + *
>> >> + * Dynamic offloads are available with the
>> >> + * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature bit.
>> >> + */
>> >> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
>> >> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
>> >> +
>> >>  #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
>> >>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
>> >>          DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
>> >> @@ -241,6 +253,7 @@ struct virtio_net_ctrl_mq {
>> >>          DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
>> >>          DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
>> >>          DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
>> >> +        DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \
>> >>          DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
>> >>  
>> >>  #endif
>> >> -- 
>> >> 1.8.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-05-19 23:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-07  6:42 [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration Dmitry Fleytman
2013-04-07  6:42 ` [Qemu-devel] [PATCH 1/1 " Dmitry Fleytman
     [not found] <1365316448-4604-1-git-send-email-dmitry@daynix.com>
     [not found] ` <1365316448-4604-2-git-send-email-dmitry@daynix.com>
2013-04-22 17:00   ` Michael S. Tsirkin
2013-04-22 17:58     ` Anthony Liguori
2013-04-22 21:02       ` Michael S. Tsirkin
2013-05-02  5:20       ` Michael S. Tsirkin
2013-05-07  7:23         ` Dmitry Fleytman
2013-05-19 11:56       ` Michael S. Tsirkin
2013-05-19 23:40         ` Anthony Liguori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).