Re: [PATCH 1/2 V2] virtio-net: dynamic network offloads configuration

[Dmitry Fleytman <dmitry@xxxxxxxxxx>]

On Sun, Mar 31, 2013 at 10:53 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:

On Sat, Mar 30, 2013 at 09:47:58PM +0300, Dmitry Fleytman wrote:
> Michael, thanks for review,
> I'll rename command as you suggested, indeed it will be better.
>
> Not sure I understand a concept of compat macros.
> Is there documentation available on this topic?
>
> Dmitry

No, but it's simple. It's just lots of arrays of options we apply only
one of them depending on -M. See e.g. pc_machine_init.

Do you mean something like this in PC_COMPAT_1_4:

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",\

 

>
> On Thu, Mar 28, 2013 at 6:39 PM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
>
>     On Thu, Mar 28, 2013 at 12:00:59PM +0200, Dmitry Fleytman wrote:
>     > From: Dmitry Fleytman <dfleytma@xxxxxxxxxx>
>     >
>     > Virtio-net driver currently negotiates network offloads
>     > on startup via features mechanism and have no ability to
>     > change offloads state 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_OFFLD.
>     >
>     > Signed-off-by: Dmitry Fleytman <dfleytma@xxxxxxxxxx>
>     > ---

>     > áhw/virtio-net.c | 111
>     ++++++++++++++++++++++++++++++++++++++++++++++++--------
>     > áhw/virtio-net.h | á19 ++++++++++
>     > á2 files changed, 115 insertions(+), 15 deletions(-)

>     >
>     > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
>     > index 5917740..a016823 100644
>     > --- a/hw/virtio-net.c
>     > +++ b/hw/virtio-net.c
>     > @@ -360,6 +360,48 @@ static uint32_t virtio_net_bad_features(VirtIODevice
>     *vdev)

>     > á á áreturn features;
>     > á}

>     >
>     > +static void virtio_net_apply_offloads(VirtIONet *n)
>     > +{

>     > + á átap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
>     > + á á á á á á á á á á!!(n->curr_offloads &
>     VIRTIO_NET_OFFLOAD_GUEST_CSUM),
>     > + á á á á á á á á á á!!(n->curr_offloads &
>     VIRTIO_NET_OFFLOAD_GUEST_TSO4),
>     > + á á á á á á á á á á!!(n->curr_offloads &
>     VIRTIO_NET_OFFLOAD_GUEST_TSO6),
>     > + á á á á á á á á á á!!(n->curr_offloads & VIRTIO_NET_OFFLOAD_GUEST_ECN),
>     > + á á á á á á á á á á!!(n->curr_offloads &

>     VIRTIO_NET_OFFLOAD_GUEST_UFO));
>     > +}
>     > +
>     > +static uint32_t virtio_net_offloads_by_features(uint32_t features)
>     > +{

>     > + á áuint32_t offloads = 0;
>     > +
>     > + á áif ((1 << VIRTIO_NET_F_GUEST_CSUM) & features) {
>     > + á á á áoffloads |= VIRTIO_NET_OFFLOAD_GUEST_CSUM;
>     > + á á}
>     > +
>     > + á áif ((1 << VIRTIO_NET_F_GUEST_TSO4) & features) {
>     > + á á á áoffloads |= VIRTIO_NET_OFFLOAD_GUEST_TSO4;
>     > + á á}
>     > +
>     > + á áif ((1 << VIRTIO_NET_F_GUEST_TSO6) & features) {
>     > + á á á áoffloads |= VIRTIO_NET_OFFLOAD_GUEST_TSO6;
>     > + á á}
>     > +
>     > + á áif ((1 << VIRTIO_NET_F_GUEST_ECN) & features) {
>     > + á á á áoffloads |= VIRTIO_NET_OFFLOAD_GUEST_ECN;
>     > + á á}
>     > +
>     > + á áif ((1 << VIRTIO_NET_F_GUEST_UFO) & features) {
>     > + á á á áoffloads |= VIRTIO_NET_OFFLOAD_GUEST_UFO;
>     > + á á}
>     > +
>     > + á áreturn offloads;

>     > +}
>     > +
>     > +static inline uint32_t virtio_net_supported_offloads(VirtIONet *n)
>     > +{

>     > + á áreturn virtio_net_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 +413,8 @@ 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_offloads = virtio_net_offloads_by_features(features);
>     > + á á á ávirtio_net_apply_offloads(n);
>     > á á á}
>     >
>     > á á áfor (i = 0; ái < n->max_queues; i++) {

>     > @@ -422,6 +460,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)
>     > +{
>     > + á áuint32_t offloads;
>     > + á ásize_t s;
>     > +
>     > + á áif (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLD) & 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) {
>     > + á á á áuint32_t supported_offloads;
>     > +
>     > + á á á áif (!n->has_vnet_hdr) {
>     > + á á á á á áreturn VIRTIO_NET_ERR;
>     > + á á á á}
>     > +
>     > + á á á ásupported_offloads = virtio_net_supported_offloads(n);
>     > + á á á áif (offloads & ~supported_offloads) {
>     > + á á á á á áreturn VIRTIO_NET_ERR;
>     > + á á á á}
>     > +
>     > + á á á án->curr_offloads = offloads;
>     > + á á á ávirtio_net_apply_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 +665,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 +1176,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_OFFLD) & n->vdev.guest_features) {
>     > + á á á áqemu_put_be32(f, n->curr_offloads);
>     > + á á}
>     > á}
>     >
>     > ástatic int virtio_net_load(QEMUFile *f, void *opaque, int version_id)

>     > @@ -1156,15 +1236,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 +1269,16 @@ static int virtio_net_load(QEMUFile *f, void
>     *opaque, int version_id)

>     > á á á á á}
>     > á á á}
>     >
>     > + á áif ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLD) & n->vdev.guest_features) {
>     > + á á á án->curr_offloads = qemu_get_be32(f);
>     > + á á} else {
>     > + á á á án->curr_offloads = virtio_net_supported_offloads(n);
>     > + á á}
>     > +
>     > + á áif (peer_has_vnet_hdr(n)) {
>     > + á á á ávirtio_net_apply_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..002d526 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_OFFLD 2 /* Control channel offload
>
>     It's very long anyway, so let's make it the full
>     VIRTIO_NET_F_CTRL_GUEST_OFFLOADS to match the command name.
>

>     > + á á á á á á á á á á á á á á á á á á á á * 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;
>     > + á áuint32_t curr_offloads;
>     > á} VirtIONet;
>     >
>     > á#define VIRTIO_NET_CTRL_MAC á á1

>     > @@ -221,6 +224,21 @@ 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_OFFLD feature bit.
>     > + */

>     > +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS á 5
>     > + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET á á á á0
>     > +
>     > +#define VIRTIO_NET_OFFLOAD_GUEST_CSUM á á á 1
>     > +#define VIRTIO_NET_OFFLOAD_GUEST_TSO4 á á á 2
>     > +#define VIRTIO_NET_OFFLOAD_GUEST_TSO6 á á á 4
>     > +#define VIRTIO_NET_OFFLOAD_GUEST_ECN á á á á8
>     > +#define VIRTIO_NET_OFFLOAD_GUEST_UFO á á á 16
>     > +
>     > á#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 +259,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_gst_offld", _state, _field,
>     VIRTIO_NET_F_CTRL_GUEST_OFFLD, true), \
>     > á á á á áDEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
>     >
>     > á#endif

>
>
>     Looks good. Better to replace ctrl_gst_offld by ctrl_guest_offloads.
>     Also need to patch hw/pc.h with compat macros so that
>     the feature is disabled for old machine types.
>
>
>     > --
>     > 1.8.1.4
>
>

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization