| Message ID | 20100827223708.2696.47389.stgit@s20.home |
|---|---|
| State | New |
| Headers | show |
* Alex Williamson (alex.williamson@redhat.com) wrote: > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index 075f72d..9ef29f0 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -36,6 +36,7 @@ typedef struct VirtIONet > VirtQueue *ctrl_vq; > NICState *nic; > QEMUTimer *tx_timer; > + uint32_t tx_timeout; > int tx_timer_active; > uint32_t has_vnet_hdr; > uint8_t has_ufo; > @@ -702,7 +703,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq) > virtio_net_flush_tx(n, vq); > } else { > qemu_mod_timer(n->tx_timer, > - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); > + qemu_get_clock(vm_clock) + n->tx_timeout); > n->tx_timer_active = 1; > virtio_queue_set_notification(vq, 0); > } > @@ -842,7 +843,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) > > if (n->tx_timer_active) { > qemu_mod_timer(n->tx_timer, > - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); > + qemu_get_clock(vm_clock) + n->tx_timeout); I think I'm missing where this is stored? Looks like migration would revert a changed tx_timeout back to 150us. thanks, -chris
On Tue, 2010-08-31 at 11:00 -0700, Chris Wright wrote: > * Alex Williamson (alex.williamson@redhat.com) wrote: > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > index 075f72d..9ef29f0 100644 > > --- a/hw/virtio-net.c > > +++ b/hw/virtio-net.c > > @@ -36,6 +36,7 @@ typedef struct VirtIONet > > VirtQueue *ctrl_vq; > > NICState *nic; > > QEMUTimer *tx_timer; > > + uint32_t tx_timeout; > > int tx_timer_active; > > uint32_t has_vnet_hdr; > > uint8_t has_ufo; > > @@ -702,7 +703,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq) > > virtio_net_flush_tx(n, vq); > > } else { > > qemu_mod_timer(n->tx_timer, > > - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); > > + qemu_get_clock(vm_clock) + n->tx_timeout); > > n->tx_timer_active = 1; > > virtio_queue_set_notification(vq, 0); > > } > > @@ -842,7 +843,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) > > > > if (n->tx_timer_active) { > > qemu_mod_timer(n->tx_timer, > > - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); > > + qemu_get_clock(vm_clock) + n->tx_timeout); > > I think I'm missing where this is stored? Looks like migration > would revert a changed tx_timeout back to 150us. It's not stored, it can be instantiated on the migration target any way you please and we can migrate between different values or even different TX mitigation strategies. If a non-default value is used on the source and you want to maintain the same behavior, the target needs to be started the same way. Alex
* Alex Williamson (alex.williamson@redhat.com) wrote: > On Tue, 2010-08-31 at 11:00 -0700, Chris Wright wrote: > > * Alex Williamson (alex.williamson@redhat.com) wrote: > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > > index 075f72d..9ef29f0 100644 > > > --- a/hw/virtio-net.c > > > +++ b/hw/virtio-net.c > > > @@ -36,6 +36,7 @@ typedef struct VirtIONet > > > VirtQueue *ctrl_vq; > > > NICState *nic; > > > QEMUTimer *tx_timer; > > > + uint32_t tx_timeout; > > > int tx_timer_active; > > > uint32_t has_vnet_hdr; > > > uint8_t has_ufo; > > > @@ -702,7 +703,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq) > > > virtio_net_flush_tx(n, vq); > > > } else { > > > qemu_mod_timer(n->tx_timer, > > > - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); > > > + qemu_get_clock(vm_clock) + n->tx_timeout); > > > n->tx_timer_active = 1; > > > virtio_queue_set_notification(vq, 0); > > > } > > > @@ -842,7 +843,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) > > > > > > if (n->tx_timer_active) { > > > qemu_mod_timer(n->tx_timer, > > > - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); > > > + qemu_get_clock(vm_clock) + n->tx_timeout); > > > > I think I'm missing where this is stored? Looks like migration > > would revert a changed tx_timeout back to 150us. > > It's not stored, it can be instantiated on the migration target any way > you please and we can migrate between different values or even different > TX mitigation strategies. If a non-default value is used on the source > and you want to maintain the same behavior, the target needs to be > started the same way. heh, IOW, I did miss how it's stored...on cmdline ;) thanks, -chris
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index fe6884d..4da0b40 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -110,7 +110,7 @@ static int s390_virtio_net_init(VirtIOS390Device *dev) { VirtIODevice *vdev; - vdev = virtio_net_init((DeviceState *)dev, &dev->nic); + vdev = virtio_net_init((DeviceState *)dev, &dev->nic, dev->txtimer); if (!vdev) { return -1; } @@ -327,6 +327,7 @@ static VirtIOS390DeviceInfo s390_virtio_net = { .qdev.size = sizeof(VirtIOS390Device), .qdev.props = (Property[]) { DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic), + DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, 1), DEFINE_PROP_END_OF_LIST(), }, }; diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h index 333fea8..922daf2 100644 --- a/hw/s390-virtio-bus.h +++ b/hw/s390-virtio-bus.h @@ -43,6 +43,8 @@ typedef struct VirtIOS390Device { uint32_t host_features; /* Max. number of ports we can have for a the virtio-serial device */ uint32_t max_virtserial_ports; + /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */ + uint32_t txtimer; } VirtIOS390Device; typedef struct VirtIOS390Bus { diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index abf0370..c8d731a 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -68,6 +68,8 @@ typedef struct { uint32_t id; NICConf nic; uint32_t host_features; + /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */ + uint32_t txtimer; } SyborgVirtIOProxy; static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset) @@ -284,7 +286,7 @@ static int syborg_virtio_net_init(SysBusDevice *dev) VirtIODevice *vdev; SyborgVirtIOProxy *proxy = FROM_SYSBUS(SyborgVirtIOProxy, dev); - vdev = virtio_net_init(&dev->qdev, &proxy->nic); + vdev = virtio_net_init(&dev->qdev, &proxy->nic, proxy->txtimer); return syborg_virtio_init(proxy, vdev); } @@ -295,6 +297,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = { .qdev.props = (Property[]) { DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic), DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features), + DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, 1), DEFINE_PROP_END_OF_LIST(), } }; diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 075f72d..9ef29f0 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -36,6 +36,7 @@ typedef struct VirtIONet VirtQueue *ctrl_vq; NICState *nic; QEMUTimer *tx_timer; + uint32_t tx_timeout; int tx_timer_active; uint32_t has_vnet_hdr; uint8_t has_ufo; @@ -702,7 +703,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq) virtio_net_flush_tx(n, vq); } else { qemu_mod_timer(n->tx_timer, - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); + qemu_get_clock(vm_clock) + n->tx_timeout); n->tx_timer_active = 1; virtio_queue_set_notification(vq, 0); } @@ -842,7 +843,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) if (n->tx_timer_active) { qemu_mod_timer(n->tx_timer, - qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); + qemu_get_clock(vm_clock) + n->tx_timeout); } return 0; } @@ -903,7 +904,8 @@ static void virtio_net_vmstate_change(void *opaque, int running, int reason) virtio_net_set_status(&n->vdev, n->vdev.status & status); } -VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) +VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, + uint32_t txtimer) { VirtIONet *n; @@ -931,6 +933,15 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n); n->tx_timer_active = 0; + if (txtimer) { + if (txtimer == 1) { + /* For convenience, 1 = "on" = predefined default, anything else + * specifies and actual timeout value */ + n->tx_timeout = 150000; /* 150 us */ + } else { + n->tx_timeout = txtimer; + } + } n->mergeable_rx_bufs = 0; n->promisc = 1; /* for compatibility */ diff --git a/hw/virtio-net.h b/hw/virtio-net.h index 235f1a9..ae21c35 100644 --- a/hw/virtio-net.h +++ b/hw/virtio-net.h @@ -47,8 +47,6 @@ #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */ -#define TX_TIMER_INTERVAL 150000 /* 150 us */ - /* Maximum packet size we can receive from tap device: header + 64k */ #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10)) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 6e8f88a..e3b9897 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -107,6 +107,8 @@ typedef struct { #endif /* Max. number of ports we can have for a the virtio-serial device */ uint32_t max_virtserial_ports; + /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */ + uint32_t txtimer; } VirtIOPCIProxy; /* virtio device */ @@ -613,7 +615,7 @@ static int virtio_net_init_pci(PCIDevice *pci_dev) VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); VirtIODevice *vdev; - vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic); + vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, proxy->txtimer); vdev->nvectors = proxy->nvectors; virtio_init_pci(proxy, vdev, @@ -690,6 +692,7 @@ static PCIDeviceInfo virtio_info[] = { DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3), DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features), DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic), + DEFINE_PROP_UINT32("txtimer", VirtIOPCIProxy, txtimer, 1), DEFINE_PROP_END_OF_LIST(), }, .qdev.reset = virtio_pci_reset, diff --git a/hw/virtio.h b/hw/virtio.h index 5836ab6..77d05e0 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -185,7 +185,8 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding, /* Base devices. */ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf); -VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf); +VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, + uint32_t txtimer); VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports); VirtIODevice *virtio_balloon_init(DeviceState *dev); #ifdef CONFIG_LINUX
The tx_timer used for TX mitigation in virtio-net has a hard coded timeout. Make an option for this to be configurable using a txtimer=<ns> device config option. Note that we reserve a value of "1" to simply mean use the default, we'll later use the value "0" to disable the timer. Everything else is the timeout in ns. We limit the timeout to a uint32_t, because anything over a 4s timeout probably doens't make any kind of practical sense. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/s390-virtio-bus.c | 3 ++- hw/s390-virtio-bus.h | 2 ++ hw/syborg_virtio.c | 5 ++++- hw/virtio-net.c | 17 ++++++++++++++--- hw/virtio-net.h | 2 -- hw/virtio-pci.c | 5 ++++- hw/virtio.h | 3 ++- 7 files changed, 28 insertions(+), 9 deletions(-)