From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: virtio-dev-return-4354-cohuck=redhat.com@lists.oasis-open.org Sender: List-Post: List-Help: List-Unsubscribe: List-Subscribe: Received: from lists.oasis-open.org (oasis-open.org [66.179.20.138]) by lists.oasis-open.org (Postfix) with ESMTP id 6641458191DE for ; Tue, 12 Jun 2018 04:54:48 -0700 (PDT) Date: Tue, 12 Jun 2018 14:54:36 +0300 From: "Michael S. Tsirkin" Message-ID: <20180612144743-mutt-send-email-mst@kernel.org> References: <1525734594-11134-1-git-send-email-sridhar.samudrala@intel.com> <20180605152344-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net To: Jason Wang Cc: "Samudrala, Sridhar" , virtualization@lists.linux-foundation.org, virtio-dev@lists.oasis-open.org, jesse.brandeburg@intel.com, alexander.h.duyck@intel.com, qemu-devel@nongnu.org List-ID: On Wed, Jun 06, 2018 at 10:29:03AM +0800, Jason Wang wrote: > > > On 2018年06月05日 20:33, Michael S. Tsirkin wrote: > > I don't think this is sufficient. > > > > If both primary and standby devices are present, a legacy guest without > > support for the feature might see two devices with same mac and get > > confused. > > > > I think that we should only make primary visible after guest acked the > > backup feature bit. > > I think we want exactly the reverse? E.g fail the negotiation when guest > does not ack backup feature. > > Otherwise legacy guest won't even have the chance to see primary device in > the guest. That's by design. > > > > And on reset or when backup is cleared in some other way, unplug the > > primary. > > What if guest does not support hotplug? It shouldn't ack the backup feature then and it's a good point. We should both document this and check kernel config has hotplug support. Sridhar could you take a look pls? > > > > Something like the below will do the job: > > > > Primary device is added with a special "primary-failover" flag. > > A virtual machine is then initialized with just a standby virtio > > device. Primary is not yet added. > > A question is how to do the matching? Qemu knows nothing about e.g mac > address of a pass-through device I believe? Supply a flag to the VFIO when it's added, this way QEMU will know. > > > > Later QEMU detects that guest driver device set DRIVER_OK. > > It then exposes the primary device to the guest, and triggers > > a device addition event (hot-plug event) for it. > > Do you mean we won't have primary device in the initial qemu cli? No, that's not what I mean. I mean management will supply a flag to VFIO and then - VFIO defers exposing primary to guest until guest acks the feature bit. - When we see guest ack, initiate hotplug. - On reboot, hide it again. - On reset without reboot, request hot-unplug and on eject hide it again. > > > > If QEMU detects guest driver removal, it initiates a hot-unplug sequence > > to remove the primary driver. In particular, if QEMU detects guest > > re-initialization (e.g. by detecting guest reset) it immediately removes > > the primary device. > > I believe guest failover module should handle this gracefully? It can't control enumeration order, if primary is enumerated before standby then guest will load its driver and it's too late when failover driver is loaded. > Thanks > > > > > We can move some of this code to management as well, architecturally it > > does not make too much sense but it might be easier implementation-wise. > > > > HTH > > > > On Mon, Jun 04, 2018 at 06:41:48PM -0700, Samudrala, Sridhar wrote: > > > Ping on this patch now that the kernel patches are accepted into davem's net-next tree. > > > https://patchwork.ozlabs.org/cover/920005/ > > > > > > > > > On 5/7/2018 4:09 PM, Sridhar Samudrala wrote: > > > > This feature bit can be used by hypervisor to indicate virtio_net device to > > > > act as a standby for another device with the same MAC address. > > > > > > > > I tested this with a small change to the patch to mark the STANDBY feature 'true' > > > > by default as i am using libvirt to start the VMs. > > > > Is there a way to pass the newly added feature bit 'standby' to qemu via libvirt > > > > XML file? > > > > > > > > Signed-off-by: Sridhar Samudrala > > > > --- > > > > hw/net/virtio-net.c | 2 ++ > > > > include/standard-headers/linux/virtio_net.h | 3 +++ > > > > 2 files changed, 5 insertions(+) > > > > > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > > > index 90502fca7c..38b3140670 100644 > > > > --- a/hw/net/virtio-net.c > > > > +++ b/hw/net/virtio-net.c > > > > @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] = { > > > > true), > > > > DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN), > > > > DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str), > > > > + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTIO_NET_F_STANDBY, > > > > + false), > > > > DEFINE_PROP_END_OF_LIST(), > > > > }; > > > > diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h > > > > index e9f255ea3f..01ec09684c 100644 > > > > --- a/include/standard-headers/linux/virtio_net.h > > > > +++ b/include/standard-headers/linux/virtio_net.h > > > > @@ -57,6 +57,9 @@ > > > > * Steering */ > > > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > > > +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another device > > > > + * with the same MAC. > > > > + */ > > > > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */ > > > > #ifndef VIRTIO_NET_NO_LEGACY --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net Date: Tue, 12 Jun 2018 14:54:36 +0300 Message-ID: <20180612144743-mutt-send-email-mst@kernel.org> References: <1525734594-11134-1-git-send-email-sridhar.samudrala@intel.com> <20180605152344-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Jason Wang Cc: alexander.h.duyck@intel.com, virtio-dev@lists.oasis-open.org, "Samudrala, Sridhar" , qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org List-Id: virtualization@lists.linuxfoundation.org T24gV2VkLCBKdW4gMDYsIDIwMTggYXQgMTA6Mjk6MDNBTSArMDgwMCwgSmFzb24gV2FuZyB3cm90 ZToKPiAKPiAKPiBPbiAyMDE45bm0MDbmnIgwNeaXpSAyMDozMywgTWljaGFlbCBTLiBUc2lya2lu IHdyb3RlOgo+ID4gSSBkb24ndCB0aGluayB0aGlzIGlzIHN1ZmZpY2llbnQuCj4gPiAKPiA+IElm IGJvdGggcHJpbWFyeSBhbmQgc3RhbmRieSBkZXZpY2VzIGFyZSBwcmVzZW50LCBhIGxlZ2FjeSBn dWVzdCB3aXRob3V0Cj4gPiBzdXBwb3J0IGZvciB0aGUgZmVhdHVyZSBtaWdodCBzZWUgdHdvIGRl dmljZXMgd2l0aCBzYW1lIG1hYyBhbmQgZ2V0Cj4gPiBjb25mdXNlZC4KPiA+IAo+ID4gSSB0aGlu ayB0aGF0IHdlIHNob3VsZCBvbmx5IG1ha2UgcHJpbWFyeSB2aXNpYmxlIGFmdGVyIGd1ZXN0IGFj a2VkIHRoZQo+ID4gYmFja3VwIGZlYXR1cmUgYml0Lgo+IAo+IEkgdGhpbmsgd2Ugd2FudCBleGFj dGx5IHRoZSByZXZlcnNlPyBFLmcgZmFpbCB0aGUgbmVnb3RpYXRpb24gd2hlbiBndWVzdAo+IGRv ZXMgbm90IGFjayBiYWNrdXAgZmVhdHVyZS4KPiAKPiBPdGhlcndpc2UgbGVnYWN5IGd1ZXN0IHdv bid0IGV2ZW4gaGF2ZSB0aGUgY2hhbmNlIHRvIHNlZSBwcmltYXJ5IGRldmljZSBpbgo+IHRoZSBn dWVzdC4KClRoYXQncyBieSBkZXNpZ24uCgo+ID4gCj4gPiBBbmQgb24gcmVzZXQgb3Igd2hlbiBi YWNrdXAgaXMgY2xlYXJlZCBpbiBzb21lIG90aGVyIHdheSwgdW5wbHVnIHRoZQo+ID4gcHJpbWFy eS4KPiAKPiBXaGF0IGlmIGd1ZXN0IGRvZXMgbm90IHN1cHBvcnQgaG90cGx1Zz8KCkl0IHNob3Vs ZG4ndCBhY2sgdGhlIGJhY2t1cCBmZWF0dXJlIHRoZW4gYW5kIGl0J3MgYSBnb29kIHBvaW50LgpX ZSBzaG91bGQgYm90aCBkb2N1bWVudCB0aGlzIGFuZCBjaGVjayBrZXJuZWwgY29uZmlnIGhhcwpo b3RwbHVnIHN1cHBvcnQuIFNyaWRoYXIgY291bGQgeW91IHRha2UgYSBsb29rIHBscz8KCj4gPiAK PiA+IFNvbWV0aGluZyBsaWtlIHRoZSBiZWxvdyB3aWxsIGRvIHRoZSBqb2I6Cj4gPiAKPiA+IFBy aW1hcnkgZGV2aWNlIGlzIGFkZGVkIHdpdGggYSBzcGVjaWFsICJwcmltYXJ5LWZhaWxvdmVyIiBm bGFnLgo+ID4gQSB2aXJ0dWFsIG1hY2hpbmUgaXMgdGhlbiBpbml0aWFsaXplZCB3aXRoIGp1c3Qg YSBzdGFuZGJ5IHZpcnRpbwo+ID4gZGV2aWNlLiBQcmltYXJ5IGlzIG5vdCB5ZXQgYWRkZWQuCj4g Cj4gQSBxdWVzdGlvbiBpcyBob3cgdG8gZG8gdGhlIG1hdGNoaW5nPyBRZW11IGtub3dzIG5vdGhp bmcgYWJvdXQgZS5nIG1hYwo+IGFkZHJlc3Mgb2YgYSBwYXNzLXRocm91Z2ggZGV2aWNlIEkgYmVs aWV2ZT8KClN1cHBseSBhIGZsYWcgdG8gdGhlIFZGSU8gd2hlbiBpdCdzIGFkZGVkLCB0aGlzIHdh eSBRRU1VIHdpbGwga25vdy4KCj4gPiAKPiA+IExhdGVyIFFFTVUgZGV0ZWN0cyB0aGF0IGd1ZXN0 IGRyaXZlciBkZXZpY2Ugc2V0IERSSVZFUl9PSy4KPiA+IEl0IHRoZW4gZXhwb3NlcyB0aGUgcHJp bWFyeSBkZXZpY2UgdG8gdGhlIGd1ZXN0LCBhbmQgdHJpZ2dlcnMKPiA+IGEgZGV2aWNlIGFkZGl0 aW9uIGV2ZW50IChob3QtcGx1ZyBldmVudCkgZm9yIGl0Lgo+IAo+IERvIHlvdSBtZWFuIHdlIHdv bid0IGhhdmUgcHJpbWFyeSBkZXZpY2UgaW4gdGhlIGluaXRpYWwgcWVtdSBjbGk/CgpObywgdGhh dCdzIG5vdCB3aGF0IEkgbWVhbi4KCkkgbWVhbiBtYW5hZ2VtZW50IHdpbGwgc3VwcGx5IGEgZmxh ZyB0byBWRklPIGFuZCB0aGVuCgoKLSBWRklPIGRlZmVycyBleHBvc2luZwpwcmltYXJ5IHRvIGd1 ZXN0IHVudGlsIGd1ZXN0IGFja3MgdGhlIGZlYXR1cmUgYml0LgotIFdoZW4gd2Ugc2VlIGd1ZXN0 IGFjaywgaW5pdGlhdGUgaG90cGx1Zy4KLSBPbiByZWJvb3QsIGhpZGUgaXQgYWdhaW4uCi0gT24g cmVzZXQgd2l0aG91dCByZWJvb3QsIHJlcXVlc3QgaG90LXVucGx1ZyBhbmQgb24gZWplY3QgaGlk ZSBpdCBhZ2Fpbi4KCgo+ID4gCj4gPiBJZiBRRU1VIGRldGVjdHMgZ3Vlc3QgZHJpdmVyIHJlbW92 YWwsIGl0IGluaXRpYXRlcyBhIGhvdC11bnBsdWcgc2VxdWVuY2UKPiA+IHRvIHJlbW92ZSB0aGUg cHJpbWFyeSBkcml2ZXIuICBJbiBwYXJ0aWN1bGFyLCBpZiBRRU1VIGRldGVjdHMgZ3Vlc3QKPiA+ IHJlLWluaXRpYWxpemF0aW9uIChlLmcuIGJ5IGRldGVjdGluZyBndWVzdCByZXNldCkgaXQgaW1t ZWRpYXRlbHkgcmVtb3Zlcwo+ID4gdGhlIHByaW1hcnkgZGV2aWNlLgo+IAo+IEkgYmVsaWV2ZSBn dWVzdCBmYWlsb3ZlciBtb2R1bGUgc2hvdWxkIGhhbmRsZSB0aGlzIGdyYWNlZnVsbHk/CgpJdCBj YW4ndCBjb250cm9sIGVudW1lcmF0aW9uIG9yZGVyLCBpZiBwcmltYXJ5IGlzIGVudW1lcmF0ZWQg YmVmb3JlCnN0YW5kYnkgdGhlbiBndWVzdCB3aWxsIGxvYWQgaXRzIGRyaXZlciBhbmQgaXQncyB0 b28gbGF0ZQp3aGVuIGZhaWxvdmVyIGRyaXZlciBpcyBsb2FkZWQuCgo+IFRoYW5rcwo+IAo+ID4g Cj4gPiBXZSBjYW4gbW92ZSBzb21lIG9mIHRoaXMgY29kZSB0byBtYW5hZ2VtZW50IGFzIHdlbGws IGFyY2hpdGVjdHVyYWxseSBpdAo+ID4gZG9lcyBub3QgbWFrZSB0b28gbXVjaCBzZW5zZSBidXQg aXQgbWlnaHQgYmUgZWFzaWVyIGltcGxlbWVudGF0aW9uLXdpc2UuCj4gPiAKPiA+IEhUSAo+ID4g Cj4gPiBPbiBNb24sIEp1biAwNCwgMjAxOCBhdCAwNjo0MTo0OFBNIC0wNzAwLCBTYW11ZHJhbGEs IFNyaWRoYXIgd3JvdGU6Cj4gPiA+IFBpbmcgb24gdGhpcyBwYXRjaCBub3cgdGhhdCB0aGUga2Vy bmVsIHBhdGNoZXMgYXJlIGFjY2VwdGVkIGludG8gZGF2ZW0ncyBuZXQtbmV4dCB0cmVlLgo+ID4g PiBodHRwczovL3BhdGNod29yay5vemxhYnMub3JnL2NvdmVyLzkyMDAwNS8KPiA+ID4gCj4gPiA+ IAo+ID4gPiBPbiA1LzcvMjAxOCA0OjA5IFBNLCBTcmlkaGFyIFNhbXVkcmFsYSB3cm90ZToKPiA+ ID4gPiBUaGlzIGZlYXR1cmUgYml0IGNhbiBiZSB1c2VkIGJ5IGh5cGVydmlzb3IgdG8gaW5kaWNh dGUgdmlydGlvX25ldCBkZXZpY2UgdG8KPiA+ID4gPiBhY3QgYXMgYSBzdGFuZGJ5IGZvciBhbm90 aGVyIGRldmljZSB3aXRoIHRoZSBzYW1lIE1BQyBhZGRyZXNzLgo+ID4gPiA+IAo+ID4gPiA+IEkg dGVzdGVkIHRoaXMgd2l0aCBhIHNtYWxsIGNoYW5nZSB0byB0aGUgcGF0Y2ggdG8gbWFyayB0aGUg U1RBTkRCWSBmZWF0dXJlICd0cnVlJwo+ID4gPiA+IGJ5IGRlZmF1bHQgYXMgaSBhbSB1c2luZyBs aWJ2aXJ0IHRvIHN0YXJ0IHRoZSBWTXMuCj4gPiA+ID4gSXMgdGhlcmUgYSB3YXkgdG8gcGFzcyB0 aGUgbmV3bHkgYWRkZWQgZmVhdHVyZSBiaXQgJ3N0YW5kYnknIHRvIHFlbXUgdmlhIGxpYnZpcnQK PiA+ID4gPiBYTUwgZmlsZT8KPiA+ID4gPiAKPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBTcmlkaGFy IFNhbXVkcmFsYSA8c3JpZGhhci5zYW11ZHJhbGFAaW50ZWwuY29tPgo+ID4gPiA+IC0tLQo+ID4g PiA+ICAgIGh3L25ldC92aXJ0aW8tbmV0LmMgICAgICAgICAgICAgICAgICAgICAgICAgfCAyICsr Cj4gPiA+ID4gICAgaW5jbHVkZS9zdGFuZGFyZC1oZWFkZXJzL2xpbnV4L3ZpcnRpb19uZXQuaCB8 IDMgKysrCj4gPiA+ID4gICAgMiBmaWxlcyBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKykKPiA+ID4g PiAKPiA+ID4gPiBkaWZmIC0tZ2l0IGEvaHcvbmV0L3ZpcnRpby1uZXQuYyBiL2h3L25ldC92aXJ0 aW8tbmV0LmMKPiA+ID4gPiBpbmRleCA5MDUwMmZjYTdjLi4zOGIzMTQwNjcwIDEwMDY0NAo+ID4g PiA+IC0tLSBhL2h3L25ldC92aXJ0aW8tbmV0LmMKPiA+ID4gPiArKysgYi9ody9uZXQvdmlydGlv LW5ldC5jCj4gPiA+ID4gQEAgLTIxOTgsNiArMjE5OCw4IEBAIHN0YXRpYyBQcm9wZXJ0eSB2aXJ0 aW9fbmV0X3Byb3BlcnRpZXNbXSA9IHsKPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICB0 cnVlKSwKPiA+ID4gPiAgICAgICAgREVGSU5FX1BST1BfSU5UMzIoInNwZWVkIiwgVmlydElPTmV0 LCBuZXRfY29uZi5zcGVlZCwgU1BFRURfVU5LTk9XTiksCj4gPiA+ID4gICAgICAgIERFRklORV9Q Uk9QX1NUUklORygiZHVwbGV4IiwgVmlydElPTmV0LCBuZXRfY29uZi5kdXBsZXhfc3RyKSwKPiA+ ID4gPiArICAgIERFRklORV9QUk9QX0JJVDY0KCJzdGFuZGJ5IiwgVmlydElPTmV0LCBob3N0X2Zl YXR1cmVzLCBWSVJUSU9fTkVUX0ZfU1RBTkRCWSwKPiA+ID4gPiArICAgICAgICAgICAgICAgICAg ICAgIGZhbHNlKSwKPiA+ID4gPiAgICAgICAgREVGSU5FX1BST1BfRU5EX09GX0xJU1QoKSwKPiA+ ID4gPiAgICB9Owo+ID4gPiA+IGRpZmYgLS1naXQgYS9pbmNsdWRlL3N0YW5kYXJkLWhlYWRlcnMv bGludXgvdmlydGlvX25ldC5oIGIvaW5jbHVkZS9zdGFuZGFyZC1oZWFkZXJzL2xpbnV4L3ZpcnRp b19uZXQuaAo+ID4gPiA+IGluZGV4IGU5ZjI1NWVhM2YuLjAxZWMwOTY4NGMgMTAwNjQ0Cj4gPiA+ ID4gLS0tIGEvaW5jbHVkZS9zdGFuZGFyZC1oZWFkZXJzL2xpbnV4L3ZpcnRpb19uZXQuaAo+ID4g PiA+ICsrKyBiL2luY2x1ZGUvc3RhbmRhcmQtaGVhZGVycy9saW51eC92aXJ0aW9fbmV0LmgKPiA+ ID4gPiBAQCAtNTcsNiArNTcsOSBAQAo+ID4gPiA+ICAgIAkJCQkJICogU3RlZXJpbmcgKi8KPiA+ ID4gPiAgICAjZGVmaW5lIFZJUlRJT19ORVRfRl9DVFJMX01BQ19BRERSIDIzCS8qIFNldCBNQUMg YWRkcmVzcyAqLwo+ID4gPiA+ICsjZGVmaW5lIFZJUlRJT19ORVRfRl9TVEFOREJZICAgICAgNjIg ICAgLyogQWN0IGFzIHN0YW5kYnkgZm9yIGFub3RoZXIgZGV2aWNlCj4gPiA+ID4gKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKiB3aXRoIHRoZSBzYW1lIE1BQy4KPiA+ ID4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAqLwo+ID4gPiA+ ICAgICNkZWZpbmUgVklSVElPX05FVF9GX1NQRUVEX0RVUExFWCA2MwkvKiBEZXZpY2Ugc2V0IGxp bmtzcGVlZCBhbmQgZHVwbGV4ICovCj4gPiA+ID4gICAgI2lmbmRlZiBWSVJUSU9fTkVUX05PX0xF R0FDWQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpWaXJ0 dWFsaXphdGlvbiBtYWlsaW5nIGxpc3QKVmlydHVhbGl6YXRpb25AbGlzdHMubGludXgtZm91bmRh dGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRpb24ub3JnL21haWxtYW4vbGlzdGlu Zm8vdmlydHVhbGl6YXRpb24= From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35326) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fShso-0001jy-1Z for qemu-devel@nongnu.org; Tue, 12 Jun 2018 07:54:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fShsj-0003KB-Oy for qemu-devel@nongnu.org; Tue, 12 Jun 2018 07:54:42 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:59226 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fShsj-0003Ja-IR for qemu-devel@nongnu.org; Tue, 12 Jun 2018 07:54:37 -0400 Date: Tue, 12 Jun 2018 14:54:36 +0300 From: "Michael S. Tsirkin" Message-ID: <20180612144743-mutt-send-email-mst@kernel.org> References: <1525734594-11134-1-git-send-email-sridhar.samudrala@intel.com> <20180605152344-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: "Samudrala, Sridhar" , virtualization@lists.linux-foundation.org, virtio-dev@lists.oasis-open.org, jesse.brandeburg@intel.com, alexander.h.duyck@intel.com, qemu-devel@nongnu.org On Wed, Jun 06, 2018 at 10:29:03AM +0800, Jason Wang wrote: >=20 >=20 > On 2018=E5=B9=B406=E6=9C=8805=E6=97=A5 20:33, Michael S. Tsirkin wrote: > > I don't think this is sufficient. > >=20 > > If both primary and standby devices are present, a legacy guest witho= ut > > support for the feature might see two devices with same mac and get > > confused. > >=20 > > I think that we should only make primary visible after guest acked th= e > > backup feature bit. >=20 > I think we want exactly the reverse? E.g fail the negotiation when gues= t > does not ack backup feature. >=20 > Otherwise legacy guest won't even have the chance to see primary device= in > the guest. That's by design. > >=20 > > And on reset or when backup is cleared in some other way, unplug the > > primary. >=20 > What if guest does not support hotplug? It shouldn't ack the backup feature then and it's a good point. We should both document this and check kernel config has hotplug support. Sridhar could you take a look pls? > >=20 > > Something like the below will do the job: > >=20 > > Primary device is added with a special "primary-failover" flag. > > A virtual machine is then initialized with just a standby virtio > > device. Primary is not yet added. >=20 > A question is how to do the matching? Qemu knows nothing about e.g mac > address of a pass-through device I believe? Supply a flag to the VFIO when it's added, this way QEMU will know. > >=20 > > Later QEMU detects that guest driver device set DRIVER_OK. > > It then exposes the primary device to the guest, and triggers > > a device addition event (hot-plug event) for it. >=20 > Do you mean we won't have primary device in the initial qemu cli? No, that's not what I mean. I mean management will supply a flag to VFIO and then - VFIO defers exposing primary to guest until guest acks the feature bit. - When we see guest ack, initiate hotplug. - On reboot, hide it again. - On reset without reboot, request hot-unplug and on eject hide it again. > >=20 > > If QEMU detects guest driver removal, it initiates a hot-unplug seque= nce > > to remove the primary driver. In particular, if QEMU detects guest > > re-initialization (e.g. by detecting guest reset) it immediately remo= ves > > the primary device. >=20 > I believe guest failover module should handle this gracefully? It can't control enumeration order, if primary is enumerated before standby then guest will load its driver and it's too late when failover driver is loaded. > Thanks >=20 > >=20 > > We can move some of this code to management as well, architecturally = it > > does not make too much sense but it might be easier implementation-wi= se. > >=20 > > HTH > >=20 > > On Mon, Jun 04, 2018 at 06:41:48PM -0700, Samudrala, Sridhar wrote: > > > Ping on this patch now that the kernel patches are accepted into da= vem's net-next tree. > > > https://patchwork.ozlabs.org/cover/920005/ > > >=20 > > >=20 > > > On 5/7/2018 4:09 PM, Sridhar Samudrala wrote: > > > > This feature bit can be used by hypervisor to indicate virtio_net= device to > > > > act as a standby for another device with the same MAC address. > > > >=20 > > > > I tested this with a small change to the patch to mark the STANDB= Y feature 'true' > > > > by default as i am using libvirt to start the VMs. > > > > Is there a way to pass the newly added feature bit 'standby' to q= emu via libvirt > > > > XML file? > > > >=20 > > > > Signed-off-by: Sridhar Samudrala > > > > --- > > > > hw/net/virtio-net.c | 2 ++ > > > > include/standard-headers/linux/virtio_net.h | 3 +++ > > > > 2 files changed, 5 insertions(+) > > > >=20 > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > > > index 90502fca7c..38b3140670 100644 > > > > --- a/hw/net/virtio-net.c > > > > +++ b/hw/net/virtio-net.c > > > > @@ -2198,6 +2198,8 @@ static Property virtio_net_properties[] =3D= { > > > > true), > > > > DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEE= D_UNKNOWN), > > > > DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_st= r), > > > > + DEFINE_PROP_BIT64("standby", VirtIONet, host_features, VIRTI= O_NET_F_STANDBY, > > > > + false), > > > > DEFINE_PROP_END_OF_LIST(), > > > > }; > > > > diff --git a/include/standard-headers/linux/virtio_net.h b/includ= e/standard-headers/linux/virtio_net.h > > > > index e9f255ea3f..01ec09684c 100644 > > > > --- a/include/standard-headers/linux/virtio_net.h > > > > +++ b/include/standard-headers/linux/virtio_net.h > > > > @@ -57,6 +57,9 @@ > > > > * Steering */ > > > > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > > > +#define VIRTIO_NET_F_STANDBY 62 /* Act as standby for an= other device > > > > + * with the same MAC. > > > > + */ > > > > #define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed a= nd duplex */ > > > > #ifndef VIRTIO_NET_NO_LEGACY