On Thu, Mar 13, 2014 at 02:56:41PM +0800, Jason Wang wrote:
> It's hard to track all mac addresses and their configurations (e.g
> vlan or ipv6)in qemu. Without those information, it's impossible to
> build proper garp packet after migration. The only possible solution
> to this is let guest ( who knew all configurations) to do this.
>
> So, this patch introduces a new rw config status bit of virtio-net,
> VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
> presence of its link through config update interrupt.When gust have
^^^^^^
typo: guest has
> done the announcement, it should ack the notification through
> VIRTIO_NET_CTRL_ANNOUNCE_ACK cmd. This feature is negotiated by a new
> feature bit VIRTIO_NET_F_ANNOUNCE (which has already been supported by
> Linux guest).
>
> During load, a counter of announcing rounds were set so that the after
> the vm is running it can trigger rounds of config interrupts to notify
> the guest to build and send the correct garps.
>
> Reference:
> Last version of discussion is here:
> https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg01127.html
>
> Changes from last version:
> - Instead of introducing a global method for each kind of nic, this
> version limit the changes into virtio-net itself.
>
> Slightly tested by myself.
>
> Cc: Liuyongan <liuyon...@huawei.com>
> Signed-off-by: Jason Wang <jasow...@redhat.com>
> ---
> ---
> hw/net/virtio-net.c | 47
> ++++++++++++++++++++++++++++++++++++++++++
> include/hw/virtio/virtio-net.h | 19 ++++++++++++++++-
> 2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 3626608..a0e66fa 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -25,6 +25,7 @@
> #include "monitor/monitor.h"
>
> #define VIRTIO_NET_VM_VERSION 11
> +#define VIRTIO_NET_ANNOUNCE_ROUNDS 3
>
> #define MAC_TABLE_ENTRIES 64
> #define MAX_VLAN (1 << 12) /* Per 802.1Q definition */
> @@ -99,6 +100,22 @@ static bool virtio_net_started(VirtIONet *n, uint8_t
> status)
> (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
> }
>
> +static void virtio_net_announce_timer(void *opaque)
> +{
> + VirtIONet *n = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(n);
> +
> + if (n->announce &&
> + virtio_net_started(n, vdev->status) &&
> + vdev->guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE) &&
> + vdev->guest_features & (0x1 << VIRTIO_NET_F_CTRL_VQ)) {
> +
> + n->announce--;
if (n->status & VIRTIO_NET_S_ANNOUNCE) {
error_report("announce bit in virtio_net status wasn't
cleaned");
}
If guest driver doesn't ACK announce, VIRTIO_NET_S_ANNOUNCE
bit of n->status won't be cleaned. We should output error
in this case.
> + n->status |= VIRTIO_NET_S_ANNOUNCE;
> + virtio_notify_config(vdev);
> + }
> +}
> +
> static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(n);
> @@ -136,6 +153,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t
> status)
> vhost_net_stop(vdev, n->nic->ncs, queues);
> n->vhost_started = 0;
> }
> +
> + virtio_net_announce_timer(n);
>
> }
>
> static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> @@ -147,6 +166,11 @@ static void virtio_net_set_status(struct VirtIODevice
> *vdev, uint8_t status)
>
> virtio_net_vhost_status(n, status);
virtio_net_announce_timer(n);
we don't only announce when vhost is used, why do you start first
announce in the end of "virtio_net_VHOST_status()?
How about move it after virtio_net_vhost_status(n, status); ?
> + if (!virtio_net_started(n, status)) {
> + n->announce = 0;
> + timer_del(n->announce_timer);
> + }
> +
> for (i = 0; i < n->max_queues; i++) {
> q = &n->vqs[i];
>
> @@ -306,6 +330,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
> n->nobcast = 0;
> /* multiqueue is disabled by default */
> n->curr_queues = 1;
> + n->announce = 0;
>
> /* Flush any MAC and VLAN filter table state */
> n->mac_table.in_use = 0;
> @@ -710,6 +735,22 @@ static int virtio_net_handle_vlan_table(VirtIONet *n,
> uint8_t cmd,
> return VIRTIO_NET_OK;
> }
>
> +static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
> + struct iovec *iov, unsigned int
> iov_cnt)
> +{
> + if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK) {
> + n->status &= ~VIRTIO_NET_S_ANNOUNCE;
> + if (n->announce) {
> + timer_mod(n->announce_timer,
> + qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 50 +
> + (VIRTIO_NET_ANNOUNCE_ROUNDS - n->announce - 1) * 100);
What's the reason for the interval 50 .. 150 .. 250 ?
> + }
> + return VIRTIO_NET_OK;
> + } else {
> + return VIRTIO_NET_ERR;
> + }
> +}
> +
> static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
> struct iovec *iov, unsigned int iov_cnt)
> {
> @@ -773,6 +814,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev,
> VirtQueue *vq)
> status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
> } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
> status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
> + } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
> + status = virtio_net_handle_announce(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) {
> @@ -1418,6 +1461,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque,
> int version_id)
> qemu_get_subqueue(n->nic, i)->link_down = link_down;
> }
>
> + n->announce = VIRTIO_NET_ANNOUNCE_ROUNDS;
> return 0;
> }
>
> @@ -1529,6 +1573,8 @@ static void virtio_net_device_realize(DeviceState *dev,
> Error **errp)
> qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
> memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
> n->status = VIRTIO_NET_S_LINK_UP;
> + n->announce_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
> + virtio_net_announce_timer, n);
>
> if (n->netclient_type) {
> /*
> @@ -1609,6 +1655,7 @@ static void virtio_net_device_unrealize(DeviceState
> *dev, Error **errp)
> }
> }
>
> + timer_del(n->announce_timer);
> g_free(n->vqs);
> qemu_del_nic(n->nic);
> virtio_cleanup(vdev);
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index df60f16..9f83059 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -48,13 +48,16 @@
> #define VIRTIO_NET_F_CTRL_VQ 17 /* Control channel available */
> #define VIRTIO_NET_F_CTRL_RX 18 /* Control channel RX mode support */
> #define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering */
> -#define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support */
> +#define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control
> + support */
^^ your emacs breaks the comment?
> +#define VIRTIO_NET_F_GUEST_ANNOUNCE 21 /* Guest can announce itself */
> #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
> * Steering */
>
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>
> #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
> +#define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
>
> #define TX_TIMER_INTERVAL 150000 /* 150 us */
>
> @@ -193,6 +196,8 @@ typedef struct VirtIONet {
> char *netclient_name;
> char *netclient_type;
> uint64_t curr_guest_offloads;
> + QEMUTimer *announce_timer;
> + int announce;
> } VirtIONet;
>
> #define VIRTIO_NET_CTRL_MAC 1
> @@ -213,6 +218,17 @@ typedef struct VirtIONet {
> #define VIRTIO_NET_CTRL_VLAN_DEL 1
>
> /*
> + * Control link announce acknowledgement
> + *
> + * The command VIRTIO_NET_CTRL_ANNOUNCE_ACK is used to indicate that
> + * driver has recevied the notification and device would clear the
> + * VIRTIO_NET_S_ANNOUNCE bit in the status filed after it received
> + * this command.
> + */
It's a general method for device to notify guest(set status bit, notify)
and get feedback(by vq command)
We can extend more vq commands (eg: VIRTIO_NET_CTRL_ACK_SETUP), and
add a new bit of status 'VIRTIO_NET_S_SETUP'
So we should rename 'VIRTIO_NET_CTRL_ANNOUNCE' to 'VIRTIO_NET_CTRL_ACK',
and rename 'VIRTIO_NET_CTRL_ANNOUNCE_ACK' to 'VIRTIO_NET_CTRL_ACK_ANNOUNCE'
ACK is the action of vq command, ANNOUNCE is the type.
> +#define VIRTIO_NET_CTRL_ANNOUNCE 3
> + #define VIRTIO_NET_CTRL_ANNOUNCE_ACK 0
> +
> +/*
> * Control Multiqueue
> *
> * The command VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
> @@ -251,6 +267,7 @@ struct virtio_net_ctrl_mq {
> DEFINE_PROP_BIT("guest_tso6", _state, _field,
> VIRTIO_NET_F_GUEST_TSO6, true), \
> DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN,
> true), \
> DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO,
> true), \
> + DEFINE_PROP_BIT("guest_announce", _state, _field,
> VIRTIO_NET_F_GUEST_ANNOUNCE, true), \
> DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4,
> true), \
> DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6,
> true), \
> DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN,
> true), \
> --
> 1.8.3.2
>
--
Amos.
