Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
Il 16/04/2012 23:43, Anthony Liguori ha scritto: For a 10 line test that enumerates the PCI device given the command line argument? Here's the thing, I just looked through the code and spotted what I think is a buffer overflow. It's hard to tell purely from code inspection. With just a basic qtest harness, it makes it possible to attempt to test whether or not you can overflow. Something like what you attached is indeed a reasonable request, but I don't see why it could not be written by whoever spots the bug. You need to know the spec of the hardware anyway to set up descriptors etc. Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/15/2012 04:16 AM, Yan Vugenfirer wrote: On Wed, Apr 11, 2012 at 10:10 PM, Anthony Liguorianth...@codemonkey.ws wrote: On 04/11/2012 02:08 PM, Paolo Bonzini wrote: Il 11/04/2012 19:25, Anthony Liguori ha scritto: Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. I think as things stand it is a bit too much to request this. You're basically asking to write a libos. The only functionality you need is PCI device enumeration which is pretty much dead simple. What other functions would you need a libos for? Regards, Anthony Liguori Paolo Regarding the testing - we ran WHQL networking tests on the device. If we provide the logs will it be sufficient? I believe the test coverage is much more comprehensive than anything that we will do with qtest. I'm not sure I'd agree about comprehensive, but the problem with WHQL is that it's not reproducible. As you've seen from this thread, there's no a tremendous amount of interest in supporting this device. Since it's likely you'll be the only ones using it, having an in-tree test case will help reduce the maintenance burden for everyone else. But VMXNET3 isn't really special here. From this point forward, I would expect all new devices to come with a qtest-based test case. Regards, Anthony Liguori Best regards, Yan.
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
Regarding the testing - we ran WHQL networking tests on the device. If we provide the logs will it be sufficient? I believe the test coverage is much more comprehensive than anything that we will do with qtest. I'm not sure I'd agree about comprehensive Let's just say that passing WHQL can easily be months of work. As you've seen from this thread, there's no a tremendous amount of interest in supporting this device. That's your opinion. Personally I would be very glad to help getting the vmw_pvscsi device in QEMU via the SCSI tree, and I don't see why VMXNET3 should be different. But VMXNET3 isn't really special here. From this point forward, I would expect all new devices to come with a qtest-based test case. I find this to be hard to justify. With a grand total of 1 device tested, and with a coverage of almost zero even for that device, I think it's only sane to consider qtest a proof of concept. We can talk again when QEMU has: * libos bindings for at least PCI and the APICs, perhaps the 8259 too, and examples of how to use those; * mocks for network devices (block device mocks are needed too, but not for this device of course). I helped moving qtest forward hoping that other people (like Stefan is doing, and like Andreas did for QOM) would contribute other pieces of the infrastructure. I certainly would have spent my time otherwise, had I known that the immediate outcome was making QEMU development slow and unwelcoming due to unreasonable prerequisites for contributing new devices. Certainly it is not reasonable to expect infrequent contributors to do our homework. Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/16/2012 02:49 PM, Paolo Bonzini wrote: Regarding the testing - we ran WHQL networking tests on the device. If we provide the logs will it be sufficient? I believe the test coverage is much more comprehensive than anything that we will do with qtest. I'm not sure I'd agree about comprehensive Let's just say that passing WHQL can easily be months of work. I'm very well aware of that. But WHQL is designed to test the drivers as much as it's a hardware certification. The bits I'm more interested about is edge case testing (things that could pose a security concern). Since WHQL interfaces at the expected paths for the driver, it's unlikely that it can test any of this. As you've seen from this thread, there's no a tremendous amount of interest in supporting this device. That's your opinion. Personally I would be very glad to help getting the vmw_pvscsi device in QEMU via the SCSI tree, and I don't see why VMXNET3 should be different. But VMXNET3 isn't really special here. From this point forward, I would expect all new devices to come with a qtest-based test case. I find this to be hard to justify. With a grand total of 1 device tested, and with a coverage of almost zero even for that device, I think it's only sane to consider qtest a proof of concept. How else are we going to get there other than asking people to use it? Look, it's pretty darn simple to add a basic test for vmxnet3 to qtest that initializes the device. I don't see what the big deal is asking for that. We can talk again when QEMU has: * libos bindings for at least PCI and the APICs, perhaps the 8259 too, and examples of how to use those; Stefan's posted patches for the PCI bits. * mocks for network devices (block device mocks are needed too, but not for this device of course). I helped moving qtest forward hoping that other people (like Stefan is doing, and like Andreas did for QOM) would contribute other pieces of the infrastructure. I certainly would have spent my time otherwise, had I known that the immediate outcome was making QEMU development slow and unwelcoming due to unreasonable prerequisites for contributing new devices. Certainly it is not reasonable to expect infrequent contributors to do our homework. Perhaps the issue here is about what is expected from a test case? All I expect is something that does basic device initialization and begins interacting with the device. It doesn't need to start as an exhaustive test but I think there's tremendous value in at least having something to start with. Otherwise, we'll continue to exist in the same chicken and the egg state. Regards, Anthony Liguroi Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
The bits I'm more interested about is edge case testing (things that could pose a security concern). Since WHQL interfaces at the expected paths for the driver, it's unlikely that it can test any of this. It does include fuzz tests. But VMXNET3 isn't really special here. From this point forward, I would expect all new devices to come with a qtest-based test case. I find this to be hard to justify. With a grand total of 1 device tested, and with a coverage of almost zero even for that device, I think it's only sane to consider qtest a proof of concept. How else are we going to get there other than asking people to use it? I agree. But I'm saying it's too early even for that. Look, it's pretty darn simple to add a basic test for vmxnet3 to qtest that initializes the device. I don't see what the big deal is asking for that. For that, qemu-test is enough. Just boot into a Linux system that has the driver. It doesn't need to start as an exhaustive test but I think there's tremendous value in at least having something to start with. Otherwise, we'll continue to exist in the same chicken and the egg state. Yes, that's a risk. I guess you were aware of that though. I've long planned to contact again my academic friends, ask for a bachelor student or two and have them work on QEMU. qtest would be perfect for that (libos and a decent block layer mock would be two nice projects). However, mentoring can be time consuming, and right now I'm not really able to set aside time for that. Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/16/2012 03:14 PM, Paolo Bonzini wrote: The bits I'm more interested about is edge case testing (things that could pose a security concern). Since WHQL interfaces at the expected paths for the driver, it's unlikely that it can test any of this. It does include fuzz tests. But VMXNET3 isn't really special here. From this point forward, I would expect all new devices to come with a qtest-based test case. I find this to be hard to justify. With a grand total of 1 device tested, and with a coverage of almost zero even for that device, I think it's only sane to consider qtest a proof of concept. How else are we going to get there other than asking people to use it? I agree. But I'm saying it's too early even for that. For a 10 line test that enumerates the PCI device given the command line argument? Here's the thing, I just looked through the code and spotted what I think is a buffer overflow. It's hard to tell purely from code inspection. With just a basic qtest harness, it makes it possible to attempt to test whether or not you can overflow. Look, it's pretty darn simple to add a basic test for vmxnet3 to qtest that initializes the device. I don't see what the big deal is asking for that. For that, qemu-test is enough. Just boot into a Linux system that has the driver. I'm basically looking for It doesn't need to start as an exhaustive test but I think there's tremendous value in at least having something to start with. Otherwise, we'll continue to exist in the same chicken and the egg state. Yes, that's a risk. I guess you were aware of that though. I've long planned to contact again my academic friends, ask for a bachelor student or two and have them work on QEMU. qtest would be perfect for that (libos and a decent block layer mock would be two nice projects). However, mentoring can be time consuming, and right now I'm not really able to set aside time for that. Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/16/2012 03:14 PM, Paolo Bonzini wrote:
The bits I'm more interested about is edge case testing (things that
could pose a security concern). Since WHQL interfaces at the expected
paths for the driver, it's unlikely that it can test any of this.
It does include fuzz tests.
But VMXNET3 isn't really special here. From this point forward, I
would expect all new devices to come with a qtest-based test case.
I find this to be hard to justify.
With a grand total of 1 device tested, and with a coverage of almost
zero even for that device, I think it's only sane to consider qtest
a proof of concept.
How else are we going to get there other than asking people to use it?
I agree. But I'm saying it's too early even for that.
This is basically what I'm looking for. I haven't submitted this yet simply to
allow folks time to update.
I don't think it's too early for this level of testing.
Regards,
Anthony Liguori
Look, it's pretty darn simple to add a basic test for vmxnet3 to qtest
that initializes the device. I don't see what the big deal is asking for
that.
For that, qemu-test is enough. Just boot into a Linux system that has
the driver.
It doesn't need to start as an exhaustive test but I think there's
tremendous value in at least having something to start with. Otherwise,
we'll continue to exist in the same chicken and the egg state.
Yes, that's a risk. I guess you were aware of that though.
I've long planned to contact again my academic friends, ask for a
bachelor student or two and have them work on QEMU. qtest would be
perfect for that (libos and a decent block layer mock would be two
nice projects). However, mentoring can be time consuming, and right
now I'm not really able to set aside time for that.
Paolo
From 30d8ef0c6b17af4d88e8788f5e680a4c4355397f Mon Sep 17 00:00:00 2001
From: Anthony Liguori aligu...@us.ibm.com
Date: Wed, 4 Jan 2012 14:46:53 -0600
Subject: qtest: add test to demonstrate e1000 legacy mode overflow
There isn't proper checking in legacy mode packets such that if two large
packets arrive back to back without the EOP flag set in the first packet, you
can easily overrun your buffer.
Because data is written to the packets after the packet is processed, this
could allow a heap overflow which is exploitable.
Reported-by: Nicolae Mogoreanu m...@google.com
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
Makefile |1 +
e1000-overflow-test.c | 104 +
2 files changed, 105 insertions(+), 0 deletions(-)
create mode 100644 e1000-overflow-test.c
diff --git a/Makefile b/Makefile
index 838cb01..99dc3eb 100644
--- a/Makefile
+++ b/Makefile
@@ -218,6 +218,7 @@ qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qobject-
libqtest.o: libqtest.c
rtc-test$(EXESUF): rtc-test.o libqtest.o
+e1000-overflow-test$(EXESUF): e1000-overflow-test.o libqtest.o
QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
diff --git a/e1000-overflow-test.c b/e1000-overflow-test.c
new file mode 100644
index 000..83bb3f9
--- /dev/null
+++ b/e1000-overflow-test.c
@@ -0,0 +1,104 @@
+#include libqtest.h
+#include glib.h
+#include stdio.h
+#include string.h
+#include hw/e1000_hw.h
+#include hw/pci_regs.h
+
+static uint32_t pci_config_read(uint8_t bus, uint8_t devfn,
+uint8_t addr, int size)
+{
+outl(0xcf8, (bus 16) | (devfn 8) | addr | (1u 31));
+if (size == 1) {
+return inb(0xcfc);
+} else if (size == 2) {
+return inw(0xcfc);
+}
+return inl(0xcfc);
+}
+
+static void pci_config_write(uint8_t bus, uint8_t devfn,
+ uint32_t addr, int size, uint32_t value)
+{
+outl(0xcf8, (bus 16) | (devfn 8) | addr | (1u 31));
+if (size == 1) {
+outb(0xcfc, value);
+} else if (size == 2) {
+outw(0xcfc, value);
+} else {
+outl(0xcfc, value);
+}
+}
+
+
+static void stw(uint64_t addr, uint16_t value)
+{
+memwrite(addr, value, sizeof(value));
+}
+
+static void stl(uint64_t addr, uint32_t value)
+{
+memwrite(addr, value, sizeof(value));
+}
+
+static void e1000_probe(uint8_t bus, uint8_t devfn)
+{
+uint32_t value = 0;
+uint32_t bar0 = 0xe000;
+uint32_t tx_addr = 4 20;
+struct e1000_tx_desc desc[2] = {};
+
+pci_config_write(bus, devfn, PCI_COMMAND, 2,
+ (PCI_COMMAND_IO | PCI_COMMAND_MEMORY));
+
+pci_config_write(bus, devfn, PCI_BASE_ADDRESS_0, 4, bar0);
+bar0 = pci_config_read(bus, devfn, PCI_BASE_ADDRESS_0, 4);
+
+desc[0].buffer_addr = tx_addr;
+desc[0].lower.data = 0x;
+desc[1].buffer_addr = tx_addr;
+desc[1].lower.data = 0x;
+
+memwrite(tx_addr, desc, sizeof(desc));
+
+stl(bar0 + E1000_TDBAH, 0);
+stl(bar0 + E1000_TDBAL, tx_addr);
+
+stw(bar0 + E1000_TDLEN, 0x80);
+stw(bar0 + E1000_TDH, 0);
+stw(bar0 + E1000_TDT, 2);
+
+value = E1000_TCTL_EN;
+
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/16/2012 03:34 PM, Anthony Liguori wrote: On 04/16/2012 03:14 PM, Paolo Bonzini wrote: The bits I'm more interested about is edge case testing (things that could pose a security concern). Since WHQL interfaces at the expected paths for the driver, it's unlikely that it can test any of this. It does include fuzz tests. But VMXNET3 isn't really special here. From this point forward, I would expect all new devices to come with a qtest-based test case. I find this to be hard to justify. With a grand total of 1 device tested, and with a coverage of almost zero even for that device, I think it's only sane to consider qtest a proof of concept. How else are we going to get there other than asking people to use it? I agree. But I'm saying it's too early even for that. For a 10 line test that enumerates the PCI device given the command line argument? Here's the thing, I just looked through the code and spotted what I think is a buffer overflow. It's hard to tell purely from code inspection. With just a basic qtest harness, it makes it possible to attempt to test whether or not you can overflow. Look, it's pretty darn simple to add a basic test for vmxnet3 to qtest that initializes the device. I don't see what the big deal is asking for that. For that, qemu-test is enough. Just boot into a Linux system that has the driver. I'm basically looking for A better email client apparently... :-/ I'm just looking for something simple. I send an example in another note. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 11, 2012 at 10:10 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 04/11/2012 02:08 PM, Paolo Bonzini wrote: Il 11/04/2012 19:25, Anthony Liguori ha scritto: Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. I think as things stand it is a bit too much to request this. You're basically asking to write a libos. The only functionality you need is PCI device enumeration which is pretty much dead simple. What other functions would you need a libos for? Regards, Anthony Liguori Paolo Regarding the testing - we ran WHQL networking tests on the device. If we provide the logs will it be sufficient? I believe the test coverage is much more comprehensive than anything that we will do with qtest. Best regards, Yan.
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 11, 2012 at 9:45 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 11/04/2012 21:10, Anthony Liguori ha scritto: On 04/11/2012 02:08 PM, Paolo Bonzini wrote: Il 11/04/2012 19:25, Anthony Liguori ha scritto: Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. I think as things stand it is a bit too much to request this. You're basically asking to write a libos. The only functionality you need is PCI device enumeration which is pretty much dead simple. What other functions would you need a libos for? You need mocks for a network device. Starting to get off-topic but net/socket.c already provides an easy packet injection/capture interface that can be used for testing. Stefan
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Tue, Apr 10, 2012 at 6:47 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. I know it makes migration of existing VMware guests easy, but now we have the burden of maintaining these device models, whose feature set and performance will never be equivalent to virtio. The reason is because we cannot extend the device spec without breaking guests (we don't control the device specification and therefore cannot add new features) and we now have twice as much performance optimization work if we want the same level of performance as virtio devices. For these reasons, I think that VMware device models can only ever be 2nd class device models in QEMU. And I wonder if the effort isn't better invested in good v2v migration tooling instead of adding this to QEMU. I'm not strongly against VMware device models in QEMU, I do see the benefit too, but please explain what the plan here is. Stefan In my opinion there is a great opportunity to create painless migration method from VMWare to QEMU\KVM. You just copy image and run, no image conversions and no issues with V2V which are painful to debug to regular person. After VM is already running on top of QEMU - changing the devices is much easier. Considering that VMWare rule the world more or less - enabling more people to switch easily or at least to get a taste of QEMU\KVM is a huge advantage. Regarding optimization - adding vhost support to VMXNET3 doesn't look like a huge effort anyway and if you look at the patch you will see that we are using virtio-net mechanisms in VMXNET3 device (for example using virtio headers for offloading). Best regards, Yan Vugenfirer.
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Tue, Apr 10, 2012 at 04:47:19PM +0100, Stefan Hajnoczi wrote: On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. I know it makes migration of existing VMware guests easy, but now we have the burden of maintaining these device models, whose feature set and performance will never be equivalent to virtio. The reason is because we cannot extend the device spec without breaking guests (we don't control the device specification and therefore cannot add new features) and we now have twice as much performance optimization work if we want the same level of performance as virtio devices. For these reasons, I think that VMware device models can only ever be 2nd class device models in QEMU. And I wonder if the effort isn't better invested in good v2v migration tooling instead of adding this to QEMU. I'm not strongly against VMware device models in QEMU, I do see the benefit too, but please explain what the plan here is. While I can sort of understand where you're coming from, this does seem to be inventing new patch acceptance criteria (which VMXNET3 authors have to satisfy) that haven't generally existed / been applied to any other device impl submission in the past. AFAICT, QEMU has welcomed / accepted patches implementing pretty much any hardware device, provided the code quality was acceptable. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 11, 2012 at 2:53 PM, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Apr 10, 2012 at 04:47:19PM +0100, Stefan Hajnoczi wrote: On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. I know it makes migration of existing VMware guests easy, but now we have the burden of maintaining these device models, whose feature set and performance will never be equivalent to virtio. The reason is because we cannot extend the device spec without breaking guests (we don't control the device specification and therefore cannot add new features) and we now have twice as much performance optimization work if we want the same level of performance as virtio devices. For these reasons, I think that VMware device models can only ever be 2nd class device models in QEMU. And I wonder if the effort isn't better invested in good v2v migration tooling instead of adding this to QEMU. I'm not strongly against VMware device models in QEMU, I do see the benefit too, but please explain what the plan here is. While I can sort of understand where you're coming from, this does seem to be inventing new patch acceptance criteria (which VMXNET3 authors have to satisfy) that haven't generally existed / been applied to any other device impl submission in the past. AFAICT, QEMU has welcomed / accepted patches implementing pretty much any hardware device, provided the code quality was acceptable. I am not trying to change patch acceptance criteria. I'm trying to understand what problem the submitter is trying to solve with these patches and how they intend to support them in the future. And I'm hoping to explain the risk of adding this feature to QEMU. But as I said, I'm not strongly against them. I'd just like some discussion before merge. Stefan
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 11, 2012 at 2:38 PM, Yan Vugenfirer y...@daynix.com wrote: On Tue, Apr 10, 2012 at 6:47 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. I know it makes migration of existing VMware guests easy, but now we have the burden of maintaining these device models, whose feature set and performance will never be equivalent to virtio. The reason is because we cannot extend the device spec without breaking guests (we don't control the device specification and therefore cannot add new features) and we now have twice as much performance optimization work if we want the same level of performance as virtio devices. For these reasons, I think that VMware device models can only ever be 2nd class device models in QEMU. And I wonder if the effort isn't better invested in good v2v migration tooling instead of adding this to QEMU. I'm not strongly against VMware device models in QEMU, I do see the benefit too, but please explain what the plan here is. Stefan In my opinion there is a great opportunity to create painless migration method from VMWare to QEMU\KVM. You just copy image and run, no image conversions and no issues with V2V which are painful to debug to regular person. After VM is already running on top of QEMU - changing the devices is much easier. Considering that VMWare rule the world more or less - enabling more people to switch easily or at least to get a taste of QEMU\KVM is a huge advantage. The problem with supporting VMware devices in order to ease v2v migration is that it actually creates a worse user experience in the long run. Users will be running devices that are not integrated or supported to the level that the virtio devices are. The danger is that we end up with a bad user experience because users never do the full migration from VMware to KVM. We need to get inside the guest to perform full v2v migration (e.g. install guest agent). Because of this it seems we might as well install virtio drivers and migrate the guest at that point. Letting the user run the guest before this has been done only results in the poor experience I've mentioned above. These are the reasons why I'm not sure this effort will pay off. What are the steps in the v2v process that you have in mind? Regarding optimization - adding vhost support to VMXNET3 doesn't look like a huge effort anyway and if you look at the patch you will see that we are using virtio-net mechanisms in VMXNET3 device (for example using virtio headers for offloading). When you say vhost support do you mean using the vhost_net.ko userspace interface from hw/vmxnet3.c? All I/O emulation would still go through the QEMU process and that defeats the biggest advantage of vhost. Stefan
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/04/2012 02:39 PM, Michael S. Tsirkin wrote: On Wed, Apr 04, 2012 at 02:44:01PM +0300, Izik Eidus wrote: Hi, What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Thanks. Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. Regards, Anthony Liguori On 18/03/2012 11:27, Dmitry Fleytman wrote: Signed-off-by: Dmitry Fleytmandmi...@daynix.com Signed-off-by: Yan Vugenfirery...@daynix.com --- Makefile.objs |1 + default-configs/pci.mak |1 + hw/pci.c|2 + hw/pci.h|1 + hw/vmxnet3.c| 2454 +++ hw/vmxnet3.h| 757 +++ net.c |2 +- 7 files changed, 3217 insertions(+), 1 deletions(-) create mode 100644 hw/vmxnet3.c create mode 100644 hw/vmxnet3.h
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/10/2012 10:47 AM, Stefan Hajnoczi wrote: On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. As long as the code is high quality and there's a test suite to go along with it, I think adding additional device emulation is perfectly fine. I don't think *inventing* new paravirtual devices is a good idea but this is just like someone submitting BNX emulation. Emulating a wide variety of devices is what we do. Regards, Anthony Liguori I know it makes migration of existing VMware guests easy, but now we have the burden of maintaining these device models, whose feature set and performance will never be equivalent to virtio. The reason is because we cannot extend the device spec without breaking guests (we don't control the device specification and therefore cannot add new features) and we now have twice as much performance optimization work if we want the same level of performance as virtio devices. For these reasons, I think that VMware device models can only ever be 2nd class device models in QEMU. And I wonder if the effort isn't better invested in good v2v migration tooling instead of adding this to QEMU. I'm not strongly against VMware device models in QEMU, I do see the benefit too, but please explain what the plan here is. Stefan
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 11, 2012 at 6:27 PM, Anthony Liguori aligu...@us.ibm.com wrote: On 04/10/2012 10:47 AM, Stefan Hajnoczi wrote: On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. As long as the code is high quality and there's a test suite to go along with it, I think adding additional device emulation is perfectly fine. I don't think *inventing* new paravirtual devices is a good idea but this is just like someone submitting BNX emulation. Emulating a wide variety of devices is what we do. The discussion I'm trying to get is: how does adding VMware emulated devices get us closer to solving v2v? I think the answer is that it doesn't but I'm curious to find out more about how exactly this fits in to a v2v process. Stefan
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
Il 11/04/2012 19:25, Anthony Liguori ha scritto: Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. I think as things stand it is a bit too much to request this. You're basically asking to write a libos. Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On 04/11/2012 02:08 PM, Paolo Bonzini wrote: Il 11/04/2012 19:25, Anthony Liguori ha scritto: Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. I think as things stand it is a bit too much to request this. You're basically asking to write a libos. The only functionality you need is PCI device enumeration which is pretty much dead simple. What other functions would you need a libos for? Regards, Anthony Liguori Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
Il 11/04/2012 21:10, Anthony Liguori ha scritto: On 04/11/2012 02:08 PM, Paolo Bonzini wrote: Il 11/04/2012 19:25, Anthony Liguori ha scritto: Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. I'd like to see qtest test cases for this too. I think as things stand it is a bit too much to request this. You're basically asking to write a libos. The only functionality you need is PCI device enumeration which is pretty much dead simple. What other functions would you need a libos for? You need mocks for a network device. Paolo
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 4, 2012 at 12:44 PM, Izik Eidus izik.ei...@ravellosystems.com wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Hang on, I asked what the point of the VMware paravirt device models is. I don't think that was ever answered fully. I know it makes migration of existing VMware guests easy, but now we have the burden of maintaining these device models, whose feature set and performance will never be equivalent to virtio. The reason is because we cannot extend the device spec without breaking guests (we don't control the device specification and therefore cannot add new features) and we now have twice as much performance optimization work if we want the same level of performance as virtio devices. For these reasons, I think that VMware device models can only ever be 2nd class device models in QEMU. And I wonder if the effort isn't better invested in good v2v migration tooling instead of adding this to QEMU. I'm not strongly against VMware device models in QEMU, I do see the benefit too, but please explain what the plan here is. Stefan
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Thu, 5 Apr 2012, Yan Vugenfirer wrote: On Thu, Apr 5, 2012 at 8:25 AM, Gerhard Wiesinger li...@wiesinger.com wrote: On Wed, 4 Apr 2012, Izik Eidus wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? As already discussed on the list patch v5 doesn't work at least for me. Previous patches worked better but were not stable under load. I'm also appreciating a working version and integrating the patch fast. Ciao, Gerhard -- http://www.wiesinger.com/ Hello Gerhard, 1. I understand that you send your exact command line to Dmitry and your test scripts, but we cannot reproduce the issues. Can you send us host network configuration? 2. Is it possible for you to run tcpdump on the guest and on the host and send us the files for review? Since there was no difference for v5 patch, I already sent this for v4 patch: See: https://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04062.html https://lists.gnu.org/archive/html/qemu-devel/2012-03/msg04634.html I also sniffed between working NIC (rtl8139) and non working NIC (vmxnet3) and as already thought in //lists.gnu.org/archive/html/qemu-devel/2012-03/msg04062.html TCP offloading checksum is NOT correct. From wireshark sniff: Checksum SYN1: 0xe937 [incorrect, should be 0xe936 (maybe caused by TCP checksum offload?)] Checksum SYN2: [incorrect, should be 0xe5af (maybe caused by TCP checksum offload?)] = checksum is always too high by 1. Please fix it. Ciao, Gerhard -- http://www.wiesinger.com/
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Thu, Apr 5, 2012 at 8:25 AM, Gerhard Wiesinger li...@wiesinger.com wrote: On Wed, 4 Apr 2012, Izik Eidus wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? As already discussed on the list patch v5 doesn't work at least for me. Previous patches worked better but were not stable under load. I'm also appreciating a working version and integrating the patch fast. Ciao, Gerhard -- http://www.wiesinger.com/ Hello Gerhard, 1. I understand that you send your exact command line to Dmitry and your test scripts, but we cannot reproduce the issues. Can you send us host network configuration? 2. Is it possible for you to run tcpdump on the guest and on the host and send us the files for review? Best regards, Yan.
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, Apr 04, 2012 at 02:44:01PM +0300, Izik Eidus wrote: Hi, What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Thanks. Off the top of my head: issues with v5: polluting global namespace, must scope names appropriately with vmxnet_ VMXNET_ unless they have file scope. Don't use names with _ followed by an upper case letter or that star with two underscores. Don't mix underscores and mixed case. Don't stick any new types in net.c/pci.c - new devices should use -device not -net. Global stuff like ethernet header size should move to central place instead of copy paste. On 18/03/2012 11:27, Dmitry Fleytman wrote: Signed-off-by: Dmitry Fleytmandmi...@daynix.com Signed-off-by: Yan Vugenfirery...@daynix.com --- Makefile.objs |1 + default-configs/pci.mak |1 + hw/pci.c|2 + hw/pci.h|1 + hw/vmxnet3.c| 2454 +++ hw/vmxnet3.h| 757 +++ net.c |2 +- 7 files changed, 3217 insertions(+), 1 deletions(-) create mode 100644 hw/vmxnet3.c create mode 100644 hw/vmxnet3.h
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
Hi, What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? Thanks. On 18/03/2012 11:27, Dmitry Fleytman wrote: Signed-off-by: Dmitry Fleytmandmi...@daynix.com Signed-off-by: Yan Vugenfirery...@daynix.com --- Makefile.objs |1 + default-configs/pci.mak |1 + hw/pci.c|2 + hw/pci.h|1 + hw/vmxnet3.c| 2454 +++ hw/vmxnet3.h| 757 +++ net.c |2 +- 7 files changed, 3217 insertions(+), 1 deletions(-) create mode 100644 hw/vmxnet3.c create mode 100644 hw/vmxnet3.h
Re: [Qemu-devel] [PATCH 7/7 v5] VMXNET3 paravirtualized device implementation Interface type vmxnet3 added.
On Wed, 4 Apr 2012, Izik Eidus wrote: What about this patch?, everything that was asked from Dmitry was accomplished... What prevent us from progressing with merging this patch? As already discussed on the list patch v5 doesn't work at least for me. Previous patches worked better but were not stable under load. I'm also appreciating a working version and integrating the patch fast. Ciao, Gerhard -- http://www.wiesinger.com/
