On 10/24/2011 01:25 PM, Michael S. Tsirkin wrote: > On Mon, Oct 24, 2011 at 02:54:59PM +1030, Rusty Russell wrote: >> On Sat, 22 Oct 2011 13:43:11 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote: >>> This make let virtio-net driver can send gratituous packet by a new >>> config bit - VIRTIO_NET_S_ANNOUNCE in each config update >>> interrupt. When this bit is set by backend, the driver would schedule >>> a workqueue to send gratituous packet through NETDEV_NOTIFY_PEERS. >>> >>> This feature is negotiated through bit VIRTIO_NET_F_GUEST_ANNOUNCE. >>> >>> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx> >> >> This seems like a huge layering violation. Imagine this in real >> hardware, for example. > > commits 06c4648d46d1b757d6b9591a86810be79818b60c > and 99606477a5888b0ead0284fecb13417b1da8e3af > document the need for this: > > NETDEV_NOTIFY_PEERS notifier indicates that a device moved to a > different physical link. > and > In real hardware such notifications are only > generated when the device comes up or the address changes. > > So hypervisor could get the same behaviour by sending link up/down > events, this is just an optimization so guest won't do > unecessary stuff like try to reconfigure an IP address. > > > Maybe LOCATION_CHANGE would be a better name? > ANNOUNCE_SELF? > >> There may be a good reason why virtual devices might want this kind of >> reconfiguration cheat, which is unnecessary for normal machines, > > I think yes, the difference with real hardware is guest can change > location without link getting dropped. > FWIW, Xen seems to use this capability too. So does ms netvsc. > >> but >> it'd have to be spelled out clearly in the spec to justify it... >> >> Cheers, >> Rusty. > > Agree, and I'd like to see the spec too. The interface seems > to involve the guest clearing the status bit when it detects > an event? I would describe this in spec. The interface need guest to clear the status bit, this would let the back-end know it has finished the work as we may need to send the gratuitous packets many times. > > Also - how does it interact with the link up event? > We probably don't want to schedule this when we detect > a link status change or during initialization, as > this patch seems to do? What if link goes down > while the work is running? Is that OK? > Looks like there's are duplications if guest enable arp_notify vm is started, but we need to handle the situation that resuming a stopped virtual machine. For the link down race, I don't see any real issue, either dropping or queued. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
