On Wed, Jan 16, 2013 at 04:24:47PM +0800, Amos Kong wrote:

On Wed, Jan 16, 2013 at 02:20:39PM +0800, Jason Wang wrote:

quoted

On Wednesday, January 16, 2013 01:57:01 PM akong@redhat.com wrote:

quoted

From: Amos Kong <redacted>

Currently we write MAC address to pci config space byte by byte,
this means that we have an intermediate step where mac is wrong.
This patch introduced a new control command to set MAC address
in one time.

VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.

Signed-off-by: Amos Kong <redacted>
---
 drivers/net/virtio_net.c        | 24 +++++++++++++++++-------
 include/uapi/linux/virtio_net.h |  8 +++++++-
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 395ab4f..c8901b6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -802,16 +802,25 @@ static int virtnet_set_mac_address(struct net_device

*dev, void *p) struct virtnet_info *vi = netdev_priv(dev);
 	struct virtio_device *vdev = vi->vdev;
 	int ret;
+	struct sockaddr *addr = p;
+	struct scatterlist sg;

-	ret = eth_mac_addr(dev, p);
-	if (ret)
-		return ret;
-
-	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+		sg_init_one(&sg, addr->sa_data, dev->addr_len);
+		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
+					  VIRTIO_NET_CTRL_MAC_ADDR_SET,
+					  &sg, 1, 0)) {
+			dev_warn(&vdev->dev,
+				 "Failed to set mac address by vq command.\n");
+			return -EINVAL;
+		}
+	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
 		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-		                  dev->dev_addr, dev->addr_len);
+				  addr->sa_data, dev->addr_len);
+	}
+	ret = eth_mac_addr(dev, p);

The you will the validity check in eth_mac_addr which may result a wrong mac 
address to be set in the hardware (or is there any check in qemu) and a 
inconsistency bettween what kernel assumes and qemu has.

You can take a look at netvsc driver that calls eth_mac_addr() first and 
restore the software mac address when fail to enforce it to hardware.

Thanks for the catching, I will move eth_mac_addr() back to above,
just restore addr if fail to send command.

I will also use DEFINE_PROP_BIT to fix migration issue, thanks.

And clear it if running with a compat machine type.

quoted

Thanks

quoted

-	return 0;
+	return ret;
 }