On 04/02/2014 11:03 AM, Michael S. Tsirkin wrote:
> On Wed, Apr 02, 2014 at 10:57:08AM -0400, Vlad Yasevich wrote:
>> On 04/02/2014 06:46 AM, Yan Vugenfirer wrote:
>>>
>>> On Apr 2, 2014, at 11:51 AM, Michael S. Tsirkin <m...@redhat.com> 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 <vyase...@redhat.com>
>>>>> ---
>>>>> 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.
>
> It's how networking works.
> If I turn off my router at home I can't get dhcp either.
And you device link will show that it has not carrier.
>
> We had a way to disable networking at link or at
> the router, then removed the ability to turn it
> off at the router and I'm asking why.
We didn't remove the ability to turn it off at router/switch.
We just now propagate carrier loss correctly.
In fact, if you have a configuration where the VM is connected
to a hub in qemu, turning off the link on the 'tap' connected
to the same hub doesn't not change guest state.
>
>> This patch chose to propagate the event under certain configuration.
>>
>> -vlad
>
> So don't do this then.
> If you want guest to know that link is down,
> put it down on guest side.
>
> It's that simple.
>
Sorry, but that's not always possible. The host administrator
may not always be vm administrator and without this patch vm
administrator has no idea what happened to his network.
-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
>>>>>
>>>>
>>>
