qemu-devel
[Top][All Lists]
Advanced

[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 09:11:25 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0

On 04/02/2014 04:51 AM, Michael S. Tsirkin 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?

This was causing some interesting issues in the non-hub
configurations.  In these configs, the loss of link on the backend
did not propagate to the guest.  The guest would continue to send
traffic and it would just queue up and get dropped.  Eventually the
guest could lose the DHCP lease and there wouldn't be any indication
at all as to why it happened.  From the guest perspective, the link
is working, but it is completely disconnected.
This patch essentially equates the backend to a carrier on the guest
and the loss of a backend is loss of carrier.

This is only done for configurations where you have a 1-to-1
relationship between a hw nic and netdev.  For the old vlan/hubport,
there is no change, since in those case, the guest nic can talk to
other hubports.  In the case of netdev, it can't.

As an example, try booting a VM in which the netdev link is down.
First, it'll take a while to boot if it uses DHCP.  When you
re-enable, the link, the VM will not issue DHCP requests and you'd
have to manually restart the nic in the VM.

With this patch, link detection works properly.

-vlad


> 
> 
> 
>> -    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
>>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]