Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-04-08 Thread Amos Kong
On Mon, Mar 25, 2013 at 08:58:49AM +0200, Michael S. Tsirkin wrote:
 On Mon, Mar 25, 2013 at 10:23:57AM +0800, Amos Kong wrote:
  On Fri, Mar 22, 2013 at 10:45:09AM +1030, Rusty Russell wrote:
   Michael S. Tsirkin m...@redhat.com writes:
On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
The lengcy guests don't have mac programming command, we don't know 
when
it's safe to use MAC. This patch changed qemu to makes MAC change 
effect
when the last byte of MAC is written to config space.

MAC address takes first 6 bytes of config space of virtio-net, the addr
is 5 when the last byte is written in virtio_config_writeb().

MAC change will effect when n-mac is updated in 
virtio_net_set_config().

Signed-off-by: Amos Kong ak...@redhat.com
   
Let's see what Rusty says about the spec change.
   
   Implementation notes like this belong as a footnote, eg:
   
   For older systems, it is recommended and typical that the device
   write byte 5 of the mac address last, so devices can use that as
   a trigger to commit the mac address change.
   
   Now, is this a real, or theoretical issue?  Have we seen this problem in
   practice, or should we continue to ignore it?
  
  Hi Rusty, Michael
  
  I didn't touch any problem. MST, and you?
  
  In Linux guest, we should disable the interface before changing mac address.
   ifconfig eth0 down
   ifconfig eth0 hw ether 10:12:13:14:15:16
   ifconfig eth0 up
   
  In Windows 7 guest, after changing mac address in register table,
  re-enabling interface to make it effect.
   reg add HKLM 
  SYSTEM\CurrentControlSet\Control\Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0001]
   /v NetworkAddress /d 0123456789AB
   netsh interface set interface Local Area Connection DISABLED
   netsh interface set interface Local Area Connection ENABLED
  
  
  So when we change the mac address, guest os always disable interface
  to receive all packages. It seems the theoretical issue doesn't exist?
  
   Cheers,
   Rusty.
   
  
  -- 
  Amos.
 
 Nope. Looks like no spec change is necessary. We already say it's
 not atomic and it looks like guests expect exactly that
 and disable link to prevent strange issues.

I just found Jiri has a commit to allow changing mac when iface
is running. It seems only the virt nics are allowned, right?

But the leagcy guests don't allow to change mac when interface is
running, so no spec change is necessary.

[upstream kernel]
commit d4fc6918f4b3cc844185f59fc518351525950449
Author: Jiri Pirko jpi...@redhat.com
Date:   Wed Jun 27 05:27:46 2012 +

virtio_net: allow to change mac when iface is running

Signed-off-by: Jiri Pirko jpi...@redhat.com
Signed-off-by: David S. Miller da...@davemloft.net

RH-BZ: https://bugzilla.redhat.com/show_bug.cgi?id=882868

-- 
Amos.



Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-03-25 Thread Michael S. Tsirkin
On Mon, Mar 25, 2013 at 10:23:57AM +0800, Amos Kong wrote:
 On Fri, Mar 22, 2013 at 10:45:09AM +1030, Rusty Russell wrote:
  Michael S. Tsirkin m...@redhat.com writes:
   On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
   The lengcy guests don't have mac programming command, we don't know when
   it's safe to use MAC. This patch changed qemu to makes MAC change effect
   when the last byte of MAC is written to config space.
   
   MAC address takes first 6 bytes of config space of virtio-net, the addr
   is 5 when the last byte is written in virtio_config_writeb().
   
   MAC change will effect when n-mac is updated in virtio_net_set_config().
   
   Signed-off-by: Amos Kong ak...@redhat.com
  
   Let's see what Rusty says about the spec change.
  
  Implementation notes like this belong as a footnote, eg:
  
  For older systems, it is recommended and typical that the device
  write byte 5 of the mac address last, so devices can use that as
  a trigger to commit the mac address change.
  
  Now, is this a real, or theoretical issue?  Have we seen this problem in
  practice, or should we continue to ignore it?
 
 Hi Rusty, Michael
 
 I didn't touch any problem. MST, and you?
 
 In Linux guest, we should disable the interface before changing mac address.
  ifconfig eth0 down
  ifconfig eth0 hw ether 10:12:13:14:15:16
  ifconfig eth0 up
  
 In Windows 7 guest, after changing mac address in register table,
 re-enabling interface to make it effect.
  reg add HKLM 
 SYSTEM\CurrentControlSet\Control\Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0001]
  /v NetworkAddress /d 0123456789AB
  netsh interface set interface Local Area Connection DISABLED
  netsh interface set interface Local Area Connection ENABLED
 
 
 So when we change the mac address, guest os always disable interface
 to receive all packages. It seems the theoretical issue doesn't exist?
 
  Cheers,
  Rusty.
  
 
 -- 
   Amos.

Nope. Looks like no spec change is necessary. We already say it's
not atomic and it looks like guests expect exactly that
and disable link to prevent strange issues.

-- 
MST



Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-03-24 Thread Amos Kong
On Fri, Mar 22, 2013 at 10:45:09AM +1030, Rusty Russell wrote:
 Michael S. Tsirkin m...@redhat.com writes:
  On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
  The lengcy guests don't have mac programming command, we don't know when
  it's safe to use MAC. This patch changed qemu to makes MAC change effect
  when the last byte of MAC is written to config space.
  
  MAC address takes first 6 bytes of config space of virtio-net, the addr
  is 5 when the last byte is written in virtio_config_writeb().
  
  MAC change will effect when n-mac is updated in virtio_net_set_config().
  
  Signed-off-by: Amos Kong ak...@redhat.com
 
  Let's see what Rusty says about the spec change.
 
 Implementation notes like this belong as a footnote, eg:
 
 For older systems, it is recommended and typical that the device
 write byte 5 of the mac address last, so devices can use that as
 a trigger to commit the mac address change.
 
 Now, is this a real, or theoretical issue?  Have we seen this problem in
 practice, or should we continue to ignore it?

Hi Rusty, Michael

I didn't touch any problem. MST, and you?

In Linux guest, we should disable the interface before changing mac address.
 ifconfig eth0 down
 ifconfig eth0 hw ether 10:12:13:14:15:16
 ifconfig eth0 up
 
In Windows 7 guest, after changing mac address in register table,
re-enabling interface to make it effect.
 reg add HKLM 
SYSTEM\CurrentControlSet\Control\Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0001]
 /v NetworkAddress /d 0123456789AB
 netsh interface set interface Local Area Connection DISABLED
 netsh interface set interface Local Area Connection ENABLED


So when we change the mac address, guest os always disable interface
to receive all packages. It seems the theoretical issue doesn't exist?

 Cheers,
 Rusty.
 

-- 
Amos.



Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-03-21 Thread Michael S. Tsirkin
On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
 The lengcy guests don't have mac programming command, we don't know when
 it's safe to use MAC. This patch changed qemu to makes MAC change effect
 when the last byte of MAC is written to config space.
 
 MAC address takes first 6 bytes of config space of virtio-net, the addr
 is 5 when the last byte is written in virtio_config_writeb().
 
 MAC change will effect when n-mac is updated in virtio_net_set_config().
 
 Signed-off-by: Amos Kong ak...@redhat.com

Let's see what Rusty says about the spec change.

 ---
  hw/virtio.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/hw/virtio.c b/hw/virtio.c
 index 26fbc79..2e08302 100644
 --- a/hw/virtio.c
 +++ b/hw/virtio.c
 @@ -605,8 +605,9 @@ void virtio_config_writeb(VirtIODevice *vdev, uint32_t 
 addr, uint32_t data)
  
  stb_p(vdev-config + addr, val);
  
 -if (vdev-set_config)
 +if (vdev-set_config  (addr == 5 || strcmp(vdev-name, virtio-net))) 
 {
  vdev-set_config(vdev, vdev-config);
 +}
  }
  
  void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data)
 -- 
 1.7.1



Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-03-21 Thread Rusty Russell
Michael S. Tsirkin m...@redhat.com writes:
 On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
 The lengcy guests don't have mac programming command, we don't know when
 it's safe to use MAC. This patch changed qemu to makes MAC change effect
 when the last byte of MAC is written to config space.
 
 MAC address takes first 6 bytes of config space of virtio-net, the addr
 is 5 when the last byte is written in virtio_config_writeb().
 
 MAC change will effect when n-mac is updated in virtio_net_set_config().
 
 Signed-off-by: Amos Kong ak...@redhat.com

 Let's see what Rusty says about the spec change.

Implementation notes like this belong as a footnote, eg:

For older systems, it is recommended and typical that the device
write byte 5 of the mac address last, so devices can use that as
a trigger to commit the mac address change.

Now, is this a real, or theoretical issue?  Have we seen this problem in
practice, or should we continue to ignore it?

Cheers,
Rusty.