* Re: [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration
[not found] <1365316448-4604-1-git-send-email-dmitry@daynix.com>
@ 2013-04-19 7:10 ` Dmitry Fleytman
2013-04-20 17:04 ` Michael S. Tsirkin
[not found] ` <1365316448-4604-2-git-send-email-dmitry@daynix.com>
1 sibling, 1 reply; 12+ messages in thread
From: Dmitry Fleytman @ 2013-04-19 7:10 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Rusty Russell, Yan Vugenfirer, Ronen Hod,
Anthony Liguori, Dmitry Fleytman, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
Hello All,
Any news regarding this patch?
Thanks,
Dmitry
On Sun, Apr 7, 2013 at 9:34 AM, Dmitry Fleytman <dmitry@daynix.com> wrote:
> 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
>
>
[-- Attachment #2: Type: text/html, Size: 1880 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration
2013-04-19 7:10 ` [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration Dmitry Fleytman
@ 2013-04-20 17:04 ` Michael S. Tsirkin
2013-04-20 20:11 ` Dmitry Fleytman
0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2013-04-20 17:04 UTC (permalink / raw)
To: Dmitry Fleytman
Cc: Rusty Russell, qemu-devel, Ronen Hod, Anthony Liguori,
Yan Vugenfirer, Dmitry Fleytman
On Fri, Apr 19, 2013 at 10:10:01AM +0300, Dmitry Fleytman wrote:
> Hello All,
>
> Any news regarding this patch?
>
> Thanks,
> Dmitry
>
Rusty could you comment on the spec change soon please?
If you pick it up I think we can include the feature in QEMU 1.5.
> On Sun, Apr 7, 2013 at 9:34 AM, Dmitry Fleytman <dmitry@daynix.com> wrote:
>
> 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] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration
2013-04-20 17:04 ` Michael S. Tsirkin
@ 2013-04-20 20:11 ` Dmitry Fleytman
2013-04-22 2:09 ` Rusty Russell
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Fleytman @ 2013-04-20 20:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Rusty Russell, qemu-devel, Ronen Hod, Anthony Liguori,
Yan Vugenfirer, Dmitry Fleytman
Spec patch already inside.
Sent from my iPad
On Apr 20, 2013, at 8:04 PM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Apr 19, 2013 at 10:10:01AM +0300, Dmitry Fleytman wrote:
>> Hello All,
>>
>> Any news regarding this patch?
>>
>> Thanks,
>> Dmitry
>
> Rusty could you comment on the spec change soon please?
> If you pick it up I think we can include the feature in QEMU 1.5.
>
>> On Sun, Apr 7, 2013 at 9:34 AM, Dmitry Fleytman <dmitry@daynix.com> wrote:
>>
>> 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] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration
2013-04-20 20:11 ` Dmitry Fleytman
@ 2013-04-22 2:09 ` Rusty Russell
0 siblings, 0 replies; 12+ messages in thread
From: Rusty Russell @ 2013-04-22 2:09 UTC (permalink / raw)
To: Dmitry Fleytman, Michael S. Tsirkin
Cc: Yan Vugenfirer, Ronen Hod, Dmitry Fleytman, qemu-devel, Anthony Liguori
Dmitry Fleytman <dmitry@daynix.com> writes:
> Spec patch already inside.
>
> Sent from my iPad
>
> On Apr 20, 2013, at 8:04 PM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> On Fri, Apr 19, 2013 at 10:10:01AM +0300, Dmitry Fleytman wrote:
>>> Hello All,
>>>
>>> Any news regarding this patch?
>>>
>>> Thanks,
>>> Dmitry
>>
>> Rusty could you comment on the spec change soon please?
>> If you pick it up I think we can include the feature in QEMU 1.5.
Indeed, spec patch already done. I'm happy...
Thanks,
Rusty.
^ permalink raw reply [flat|nested] 12+ 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; 12+ 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] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration
2013-04-22 17:00 ` [Qemu-devel] [PATCH 1/1 " 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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 " Dmitry Fleytman
@ 2013-04-07 6:42 ` Dmitry Fleytman
0 siblings, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2013-05-19 23:40 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1365316448-4604-1-git-send-email-dmitry@daynix.com>
2013-04-19 7:10 ` [Qemu-devel] [PATCH 0/1 V4] virtio-net: dynamic network offloads configuration Dmitry Fleytman
2013-04-20 17:04 ` Michael S. Tsirkin
2013-04-20 20:11 ` Dmitry Fleytman
2013-04-22 2:09 ` Rusty Russell
[not found] ` <1365316448-4604-2-git-send-email-dmitry@daynix.com>
2013-04-22 17:00 ` [Qemu-devel] [PATCH 1/1 " 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
2013-04-07 6:42 [Qemu-devel] [PATCH 0/1 " Dmitry Fleytman
2013-04-07 6:42 ` [Qemu-devel] [PATCH 1/1 " Dmitry Fleytman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.