netdev - Re: [Qemu-devel] [RFC v2 PATCH 5/4 PATCH] virtio-net: send gratuitous packet when needed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 25 Oct 2011 10:50:41 +0800
From:	Jason Wang <jasowang@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
CC:	Rusty Russell <rusty@...tcorp.com.au>, aliguori@...ibm.com,
	quintela@...hat.com, jan.kiszka@...mens.com, qemu-devel@...gnu.org,
	blauwirbel@...il.com, pbonzini@...hat.com, kvm@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [Qemu-devel] [RFC v2 PATCH 5/4 PATCH] virtio-net: send gratuitous
 packet when needed

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@...hat.com> 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@...hat.com>
>>
>> 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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html