[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change
From: |
Vlad Yasevich |
Subject: |
Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change |
Date: |
Wed, 02 Apr 2014 10:57:08 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 04/02/2014 06:46 AM, Yan Vugenfirer wrote:
>
> On Apr 2, 2014, at 11:51 AM, Michael S. Tsirkin <address@hidden> wrote:
>
>> On Thu, Nov 21, 2013 at 09:05:51PM -0500, Vlad Yasevich wrote:
>>> When a link change occurs on a backend (like tap), we currently do
>>> not propage such change to the nic. As a result, when someone turns
>>> off a link on a tap device, for instance, then a guest doesn't see
>>> that change and continues to try to send traffic or run DHCP even
>>> though the lower-layer is disconnected. This is OK when the network
>>> is set up as a HUB since the the guest may be connected to other HUB
>>> ports too, but when it's set up as a netdev, it makes thinkgs worse.
>>>
>>> The patch addresses this by setting the peers link down only when the
>>> peer is not a HUBPORT device. With this patch, in the following config
>>> -netdev tap,id=net0 -device e1000,mac=XXXXX,netdev=net0
>>> when net0 link is turned off, the guest e1000 shows lower-layer link
>>> down. This allows guests to boot much faster in such configurations.
>>> With windows guest, it also allows the network to recover properly
>>> since windows will not configure the link-local IPv4 address, and
>>> when the link is turned on, the proper address address is configured.
>>>
>>> Signed-off-by: Vlad Yasevich <address@hidden>
>>> ---
>>> net/net.c | 26 +++++++++++++++++---------
>>> 1 file changed, 17 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 0a88e68..8a084bf 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1065,15 +1065,23 @@ void qmp_set_link(const char *name, bool up, Error
>>> **errp)
>>> nc->info->link_status_changed(nc);
>>> }
>>>
>>> - /* Notify peer. Don't update peer link status: this makes it possible
>>> to
>>> - * disconnect from host network without notifying the guest.
>>> - * FIXME: is disconnected link status change operation useful?
>>> - *
>>> - * Current behaviour is compatible with qemu vlans where there could be
>>> - * multiple clients that can still communicate with each other in
>>> - * disconnected mode. For now maintain this compatibility. */
>>
>> Hmm this went in without much discussion.
>>
>> But before this change went in, it was possible to control link state on NIC
>> and on link separately, without guest noticing.
>>
>> Why did we decide to drop this functionality?
>
> Not sure there was a real need for the patch.
>
> On other hand Windows guest will not receive IP address from DHCP server if
> you start VM with tap link down and then set it to up without toggling link
> state of the guest device as well.
Same for a linux guest. It a fault scenario. So we either handle
it by propagating the link event, or we forbid it. Doing neither
allows for a bad state in common configuration.
This patch chose to propagate the event under certain configuration.
-vlad
>
> Yan.
>
>>
>>
>>
>>> - if (nc->peer && nc->peer->info->link_status_changed) {
>>> - nc->peer->info->link_status_changed(nc->peer);
>>> + if (nc->peer) {
>>> + /* Change peer link only if the peer is NIC and then notify peer.
>>> + * If the peer is a HUBPORT or a backend, we do not change the
>>> + * link status.
>>> + *
>>> + * This behavior is compatible with qemu vlans where there could be
>>> + * multiple clients that can still communicate with each other in
>>> + * disconnected mode. For now maintain this compatibility.
>>> + */
>>> + if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
>>> + for (i = 0; i < queues; i++) {
>>> + ncs[i]->peer->link_down = !up;
>>> + }
>>> + }
>>> + if (nc->peer->info->link_status_changed) {
>>> + nc->peer->info->link_status_changed(nc->peer);
>>> + }
>>> }
>>> }
>>>
>>> --
>>> 1.8.4.2
>>>
>>
>
Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change, Vlad Yasevich, 2014/04/02