Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change

2014-04-02 Thread Michael S. Tsirkin
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=X,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?



 -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

2014-04-02 Thread Yan Vugenfirer

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=X,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.

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

2014-04-02 Thread Michael S. Tsirkin
On Wed, Apr 02, 2014 at 01:46:14PM +0300, 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=X,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.
 
 Yan. 

So why not toggle the link of the guest device?

  
  
  
  -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

2014-04-02 Thread Vlad Yasevich
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=X,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?

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





Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change

2014-04-02 Thread Vlad Yasevich
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=X,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.

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

2014-04-02 Thread Michael S. Tsirkin
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=X,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.

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.

 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.




  
  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

2014-04-02 Thread Vlad Yasevich
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=X,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







Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change

2014-04-02 Thread Michael S. Tsirkin
On Wed, Apr 02, 2014 at 11:25:32AM -0400, Vlad Yasevich wrote:
 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=X,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.

It doesn't as long as the switch it's connected to is up.


  
  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.

So why is it a good idea to make -netdev inconsistent
with this?
It doesn't seem to add anything useful.
It might fix things for users who turned off link
at the wrong place by mistake but I doubt it's
a common scenario.

Where end users getting it wrong and complaining?


  
  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++) {
  +   

Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change

2014-04-02 Thread Vlad Yasevich
On 04/02/2014 11:29 AM, Michael S. Tsirkin wrote:
 On Wed, Apr 02, 2014 at 11:25:32AM -0400, Vlad Yasevich wrote:
 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=X,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.
 
 It doesn't as long as the switch it's connected to is up.
 
 

 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.
 
 So why is it a good idea to make -netdev inconsistent
 with this?

Because -netdev has a 1-1 relationship with -device.   You
could think of it a physical wire that connects the nic
to the network.  If something happens to the wire, the
nic should notice it.

 It doesn't seem to add anything useful.
 It might fix things for users who turned off link
 at the wrong place by mistake but I doubt it's
 a common scenario.

I don't think link state management in general is a common scenario, but
we support it.  I think this makes the behavior better and improves
the vm experience.
 
 Where end users getting it wrong and complaining?

Yes, I can provide you with a list reported bugs if you wish.

-vlad

 
 

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

Re: [Qemu-devel] [PATCH] net: Update netdev peer on link change

2013-11-22 Thread Stefan Hajnoczi
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=X,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(-)

Merged for QEMU 1.8.  Link state changes can lead to weird bugs so I
don't want to rush this into QEMU 1.7.

Thanks, applied to my net-next tree:
https://github.com/stefanha/qemu/commits/net-next

Stefan



[Qemu-devel] [PATCH] net: Update netdev peer on link change

2013-11-21 Thread Vlad Yasevich
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=X,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. */
-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