Re: [PATCH net-next] net: virtio: use eth_hw_addr_set()

On Wed, Oct 27, 2021 at 06:26:40AM -0700, Jakub Kicinski wrote:

On Wed, 27 Oct 2021 03:23:17 -0400 Michael S. Tsirkin wrote:

quoted

quoted

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c501b5974aee..b7f35aff8e82 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev)
 	dev->max_mtu = MAX_MTU;
 
 	/* Configuration may specify what MAC to use.  Otherwise random. */
-	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
+		u8 addr[MAX_ADDR_LEN];
+
 		virtio_cread_bytes(vdev,
 				   offsetof(struct virtio_net_config, mac),
-				   dev->dev_addr, dev->addr_len);
-	else
+				   addr, dev->addr_len);  

Maybe BUG_ON(dev->addr_len > sizeof addr);

here just to make sure we don't overflow addr silently?

Since I need to post a v2 and we're talking... can I just use
eth_hw_addr_set() instead? AFAICT netdev is always allocated with 
alloc_etherdev_mq() and ->addr_len never changed. Plus there is 
a number of Ethernet address helpers used so ETH_ALEN is kind of 
already assumed.

Right. Sure, just document this in the commit log pls.

quoted

quoted

+		dev_addr_set(dev, addr);
+	} else {
 		eth_hw_addr_random(dev);
+	}
 
 	/* Set up our device-specific information */
 	vi = netdev_priv(dev);