[PATCH 0/2 V4] virtio-spec/net: dynamic network offloads configuration

[PATCH 0/2 V4] virtio-spec/net: dynamic network offloads configuration

[Dmitry Fleytman]

From: Dmitry Fleytman <dfleytma@redhat.com>

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.

Patch for QEMU mainline that implements this specification change
attached as well.

-- 
1.8.1.4

[PATCH 2/2 V4] virtio-net: dynamic network offloads configuration

[Dmitry Fleytman]

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

[PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Dmitry Fleytman]

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>
---
 virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 3d2f485..e9dbac4 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -60,6 +60,7 @@
 \author -1930653948 "Amos Kong" 
 \author -608949062 "Rusty Russell,,," 
 \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
+\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
 \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
 \author 1531152142 "Paolo Bonzini,,," 
 \author 1717892615 "Alexey Zaytsev,,," 
@@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
 \end_inset
 
 (1) Guest handles packets with partial checksum
+\change_inserted 460276516 1363712169
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 460276516 1363712334
+VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+\begin_inset space ~
+\end_inset
+
+(2) Control channel offloads reconfiguration support.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Description
@@ -5675,6 +5690,134 @@ virtqueue_pairs = 1
  
 \end_layout
 
+\begin_layout Subsection*
+
+\change_inserted 460276516 1363765850
+Offloads State Configuration
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363765861
+If the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated, the driver can
+ send control commands for dynamic offloads state configuration.
+\end_layout
+
+\begin_layout Subsubsection*
+
+\change_inserted 460276516 1363765928
+Setting offloads state
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363713225
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765996
+
+u64 offloads;
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765997
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766044
+
+#define VIRTIO_NET_F_GUEST_CSUM       1
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766051
+
+#define VIRTIO_NET_F_GUEST_TSO4       7
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766055
+
+#define VIRTIO_NET_F_GUEST_TSO6       8
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766064
+
+#define VIRTIO_NET_F_GUEST_ECN        9
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766035
+
+#define VIRTIO_NET_F_GUEST_UFO        10
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766031
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765865
+
+#define VIRTIO_NET_CTRL_GUEST_OFFLOADS       5
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765867
+
+ #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET          0
+\end_layout
+
+\end_inset
+
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766082
+The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command: VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
+ applies the new offloads configuration.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766435
+u64 value passed as command data is a bitmask, bits set define offloads
+ to be enabled, bits cleared - offloads to be disabled.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766757
+There is a corresponding device feature for each offload.
+ Upon feature negotiation corresponding offload gets enabled to preserve
+ backward compartibility.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766720
+Corresponding feature must be negotiated at startup in order to allow dynamic
+ change of specific offload state.
+\end_layout
+
 \begin_layout Chapter*
 Appendix D: Block Device
 \end_layout
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Paolo Bonzini]

Il 04/04/2013 09:55, Dmitry Fleytman ha scritto:
> 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>
> ---
>  virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 143 insertions(+)
> 
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index 3d2f485..e9dbac4 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -60,6 +60,7 @@
>  \author -1930653948 "Amos Kong" 
>  \author -608949062 "Rusty Russell,,," 
>  \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
> +\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
>  \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
>  \author 1531152142 "Paolo Bonzini,,," 
>  \author 1717892615 "Alexey Zaytsev,,," 
> @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
>  \end_inset
>  
>  (1) Guest handles packets with partial checksum
> +\change_inserted 460276516 1363712169
> +
> +\end_layout
> +
> +\begin_layout Description
> +
> +\change_inserted 460276516 1363712334
> +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> +\begin_inset space ~
> +\end_inset
> +
> +(2) Control channel offloads reconfiguration support.
> +\change_unchanged

Rusty,

you might have missed this patch because your address was misspelled.

Is there a story behind skipping virtio-net feature bits 2..4?

Paolo

>  \end_layout
>  
>  \begin_layout Description
> @@ -5675,6 +5690,134 @@ virtqueue_pairs = 1
>   
>  \end_layout
>  
> +\begin_layout Subsection*
> +
> +\change_inserted 460276516 1363765850
> +Offloads State Configuration
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363765861
> +If the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated, the driver can
> + send control commands for dynamic offloads state configuration.
> +\end_layout
> +
> +\begin_layout Subsubsection*
> +
> +\change_inserted 460276516 1363765928
> +Setting offloads state
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363713225
> +\begin_inset listings
> +inline false
> +status open
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765996
> +
> +u64 offloads;
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765997
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766044
> +
> +#define VIRTIO_NET_F_GUEST_CSUM       1
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766051
> +
> +#define VIRTIO_NET_F_GUEST_TSO4       7
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766055
> +
> +#define VIRTIO_NET_F_GUEST_TSO6       8
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766064
> +
> +#define VIRTIO_NET_F_GUEST_ECN        9
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766035
> +
> +#define VIRTIO_NET_F_GUEST_UFO        10
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766031
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765865
> +
> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS       5
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765867
> +
> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET          0
> +\end_layout
> +
> +\end_inset
> +
> +
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766082
> +The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command: VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
> + applies the new offloads configuration.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766435
> +u64 value passed as command data is a bitmask, bits set define offloads
> + to be enabled, bits cleared - offloads to be disabled.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766757
> +There is a corresponding device feature for each offload.
> + Upon feature negotiation corresponding offload gets enabled to preserve
> + backward compartibility.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766720
> +Corresponding feature must be negotiated at startup in order to allow dynamic
> + change of specific offload state.
> +\end_layout
> +
>  \begin_layout Chapter*
>  Appendix D: Block Device
>  \end_layout
> 

Re: [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Paolo Bonzini]

Il 04/04/2013 09:55, Dmitry Fleytman ha scritto:
> 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>
> ---
>  virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 143 insertions(+)
> 
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index 3d2f485..e9dbac4 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -60,6 +60,7 @@
>  \author -1930653948 "Amos Kong" 
>  \author -608949062 "Rusty Russell,,," 
>  \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
> +\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
>  \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
>  \author 1531152142 "Paolo Bonzini,,," 
>  \author 1717892615 "Alexey Zaytsev,,," 
> @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
>  \end_inset
>  
>  (1) Guest handles packets with partial checksum
> +\change_inserted 460276516 1363712169
> +
> +\end_layout
> +
> +\begin_layout Description
> +
> +\change_inserted 460276516 1363712334
> +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> +\begin_inset space ~
> +\end_inset
> +
> +(2) Control channel offloads reconfiguration support.
> +\change_unchanged

Rusty,

you might have missed this patch because your address was misspelled.

Is there a story behind skipping virtio-net feature bits 2..4?

Paolo

>  \end_layout
>  
>  \begin_layout Description
> @@ -5675,6 +5690,134 @@ virtqueue_pairs = 1
>   
>  \end_layout
>  
> +\begin_layout Subsection*
> +
> +\change_inserted 460276516 1363765850
> +Offloads State Configuration
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363765861
> +If the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated, the driver can
> + send control commands for dynamic offloads state configuration.
> +\end_layout
> +
> +\begin_layout Subsubsection*
> +
> +\change_inserted 460276516 1363765928
> +Setting offloads state
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363713225
> +\begin_inset listings
> +inline false
> +status open
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765996
> +
> +u64 offloads;
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765997
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766044
> +
> +#define VIRTIO_NET_F_GUEST_CSUM       1
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766051
> +
> +#define VIRTIO_NET_F_GUEST_TSO4       7
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766055
> +
> +#define VIRTIO_NET_F_GUEST_TSO6       8
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766064
> +
> +#define VIRTIO_NET_F_GUEST_ECN        9
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766035
> +
> +#define VIRTIO_NET_F_GUEST_UFO        10
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766031
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765865
> +
> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS       5
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765867
> +
> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET          0
> +\end_layout
> +
> +\end_inset
> +
> +
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766082
> +The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command: VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
> + applies the new offloads configuration.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766435
> +u64 value passed as command data is a bitmask, bits set define offloads
> + to be enabled, bits cleared - offloads to be disabled.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766757
> +There is a corresponding device feature for each offload.
> + Upon feature negotiation corresponding offload gets enabled to preserve
> + backward compartibility.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766720
> +Corresponding feature must be negotiated at startup in order to allow dynamic
> + change of specific offload state.
> +\end_layout
> +
>  \begin_layout Chapter*
>  Appendix D: Block Device
>  \end_layout
> 

Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Michael S. Tsirkin]

On Mon, May 20, 2013 at 03:43:51PM +0200, Paolo Bonzini wrote:
> Il 04/04/2013 09:55, Dmitry Fleytman ha scritto:
> > 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>
> > ---
> >  virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> > 
> > diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> > index 3d2f485..e9dbac4 100644
> > --- a/virtio-spec.lyx
> > +++ b/virtio-spec.lyx
> > @@ -60,6 +60,7 @@
> >  \author -1930653948 "Amos Kong" 
> >  \author -608949062 "Rusty Russell,,," 
> >  \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
> > +\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
> >  \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
> >  \author 1531152142 "Paolo Bonzini,,," 
> >  \author 1717892615 "Alexey Zaytsev,,," 
> > @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
> >  \end_inset
> >  
> >  (1) Guest handles packets with partial checksum
> > +\change_inserted 460276516 1363712169
> > +
> > +\end_layout
> > +
> > +\begin_layout Description
> > +
> > +\change_inserted 460276516 1363712334
> > +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> > +\begin_inset space ~
> > +\end_inset
> > +
> > +(2) Control channel offloads reconfiguration support.
> > +\change_unchanged
> 
> Rusty,
> 
> you might have missed this patch because your address was misspelled.

It's commit 721861222802d0ebc48564d727a3684f7b21cadd in
https://github.com/rustyrussell/virtio-spec

> Is there a story behind skipping virtio-net feature bits 2..4?
> 
> Paolo

Bits 3-4 now :)
I'm curious too.

-- 
MST

Re: [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Michael S. Tsirkin]

On Mon, May 20, 2013 at 03:43:51PM +0200, Paolo Bonzini wrote:
> Il 04/04/2013 09:55, Dmitry Fleytman ha scritto:
> > 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>
> > ---
> >  virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> > 
> > diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> > index 3d2f485..e9dbac4 100644
> > --- a/virtio-spec.lyx
> > +++ b/virtio-spec.lyx
> > @@ -60,6 +60,7 @@
> >  \author -1930653948 "Amos Kong" 
> >  \author -608949062 "Rusty Russell,,," 
> >  \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
> > +\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
> >  \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
> >  \author 1531152142 "Paolo Bonzini,,," 
> >  \author 1717892615 "Alexey Zaytsev,,," 
> > @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
> >  \end_inset
> >  
> >  (1) Guest handles packets with partial checksum
> > +\change_inserted 460276516 1363712169
> > +
> > +\end_layout
> > +
> > +\begin_layout Description
> > +
> > +\change_inserted 460276516 1363712334
> > +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> > +\begin_inset space ~
> > +\end_inset
> > +
> > +(2) Control channel offloads reconfiguration support.
> > +\change_unchanged
> 
> Rusty,
> 
> you might have missed this patch because your address was misspelled.

It's commit 721861222802d0ebc48564d727a3684f7b21cadd in
https://github.com/rustyrussell/virtio-spec

> Is there a story behind skipping virtio-net feature bits 2..4?
> 
> Paolo

Bits 3-4 now :)
I'm curious too.

-- 
MST

Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Rusty Russell]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, May 20, 2013 at 03:43:51PM +0200, Paolo Bonzini wrote:
>> Is there a story behind skipping virtio-net feature bits 2..4?
>> 
>> Paolo
>
> Bits 3-4 now :)
> I'm curious too.

Not a good one :)

The year is 2007.  virtio_net was the posterchild of free expression and
crazy experimentation.  We were all so very young...

The original host TSO bits in virtio_net were like so:

 #define VIRTIO_NET_F_TSO4      1
 #define VIRTIO_NET_F_UFO       2
 #define VIRTIO_NET_F_TSO4_ECN  3
 #define VIRTIO_NET_F_TSO6      4

But I decided we might as well offer all or nothing, and if you couldn't
handle one, you could just do it in software:

 #define VIRTIO_NET_F_GSO       6

Reading the git commits, it seems Linux didn't do UFO in software (at
the time, at least), so the bits were split out again, with the explicit
_HOST_ in the names:

 #define VIRTIO_NET_F_HOST_TSO4 11      /* Host can handle TSOv4 in. */
 #define VIRTIO_NET_F_HOST_TSO6 12      /* Host can handle TSOv6 in. */
 #define VIRTIO_NET_F_HOST_ECN  13      /* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO  14      /* Host can handle UFO in. */

This was all before there was a host implementation, so we were playing.

ie. the bits are perfectly fine for reuse.

Cheers,
Rusty.

Re: [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Rusty Russell]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, May 20, 2013 at 03:43:51PM +0200, Paolo Bonzini wrote:
>> Is there a story behind skipping virtio-net feature bits 2..4?
>> 
>> Paolo
>
> Bits 3-4 now :)
> I'm curious too.

Not a good one :)

The year is 2007.  virtio_net was the posterchild of free expression and
crazy experimentation.  We were all so very young...

The original host TSO bits in virtio_net were like so:

 #define VIRTIO_NET_F_TSO4      1
 #define VIRTIO_NET_F_UFO       2
 #define VIRTIO_NET_F_TSO4_ECN  3
 #define VIRTIO_NET_F_TSO6      4

But I decided we might as well offer all or nothing, and if you couldn't
handle one, you could just do it in software:

 #define VIRTIO_NET_F_GSO       6

Reading the git commits, it seems Linux didn't do UFO in software (at
the time, at least), so the bits were split out again, with the explicit
_HOST_ in the names:

 #define VIRTIO_NET_F_HOST_TSO4 11      /* Host can handle TSOv4 in. */
 #define VIRTIO_NET_F_HOST_TSO6 12      /* Host can handle TSOv6 in. */
 #define VIRTIO_NET_F_HOST_ECN  13      /* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO  14      /* Host can handle UFO in. */

This was all before there was a host implementation, so we were playing.

ie. the bits are perfectly fine for reuse.

Cheers,
Rusty.