Virtualization - Nov 2014 - [PATCH v6 34/46] virtio_net: disable mac write for virtio 1.0

The spec states that mac in config space is only driver-writable in the
legacy case.  Fence writing it in virtnet_set_mac_address() in the
virtio 1.0 case.

Suggested-by: Cornelia Huck <cornelia.huck at de.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
 drivers/net/virtio_net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c6a72d3..9ab3c50 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1030,7 +1030,8 @@ static int virtnet_set_mac_address(struct net_device *dev,
void *p)
 				 "Failed to set mac address by vq command.\n");
 			return -EINVAL;
 		}
-	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
+	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC) &&
+		   !virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
 		unsigned int i;
 
 		/* Naturally, this has an atomicity problem. */
-- 
MST

On Fri, Nov 28, 2014 at 4:10 AM, Michael S. Tsirkin <mst at redhat.com> 
wrote:
> The spec states that mac in config space is only driver-writable in 
> the
> legacy case.  Fence writing it in virtnet_set_mac_address() in the
> virtio 1.0 case.
> 
> Suggested-by: Cornelia Huck <cornelia.huck at de.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  drivers/net/virtio_net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index c6a72d3..9ab3c50 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1030,7 +1030,8 @@ static int virtnet_set_mac_address(struct 
> net_device *dev, void *p)
>  				 "Failed to set mac address by vq command.\n");
>  			return -EINVAL;
>  		}
> -	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
> +	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC) &&
> +		   !virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>  		unsigned int i;
>  
>  		/* Naturally, this has an atomicity problem. */
> -- 
> MST
Does this mean there's no way to setting mac addres
if ctrl mac addr is disabled in virtio-1?

If yes, we'd better keep this compatibility somehow.

On Fri, Nov 28, 2014 at 08:48:24AM +0008, Jason Wang
wrote:
> 
> 
> On Fri, Nov 28, 2014 at 4:10 AM, Michael S. Tsirkin <mst at
redhat.com> wrote:
> >The spec states that mac in config space is only driver-writable in the
> >legacy case.  Fence writing it in virtnet_set_mac_address() in the
> >virtio 1.0 case.
> >
> >Suggested-by: Cornelia Huck <cornelia.huck at de.ibm.com>
> >Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> >---
> > drivers/net/virtio_net.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >index c6a72d3..9ab3c50 100644
> >--- a/drivers/net/virtio_net.c
> >+++ b/drivers/net/virtio_net.c
> >@@ -1030,7 +1030,8 @@ static int virtnet_set_mac_address(struct
net_device
> >*dev, void *p)
> > 				 "Failed to set mac address by vq command.\n");
> > 			return -EINVAL;
> > 		}
> >-	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
> >+	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC) &&
> >+		   !virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> > 		unsigned int i;
> > 		/* Naturally, this has an atomicity problem. */
> >-- 
> >MST
> 
> Does this mean there's no way to setting mac addres
> if ctrl mac addr is disabled in virtio-1?
More exactly: no way for host to know the guest mac address.
> If yes, we'd better keep this compatibility somehow.
If host wants to be notified about guest mac updates,
it must enable the appropriate ctrl command.
Or if it's an old host, it just doesn't enable virtio-1.

There's no way around this - it's in the spec.


-- 
MST