git: 05041794d06f - main - if_vtnet: Defer updating generated MAC address until attached

[Bryan Venteicher]

The branch main has been updated by bryanv:

URL: https://cgit.FreeBSD.org/src/commit/?id=05041794d06f973cf6b4fb3b14a90718656f7f5a

commit 05041794d06f973cf6b4fb3b14a90718656f7f5a
Author:     Bryan Venteicher <bryanv at FreeBSD.org>
AuthorDate: 2021-01-19 04:55:24 +0000
Commit:     Bryan Venteicher <bryanv at FreeBSD.org>
CommitDate: 2021-01-19 04:55:24 +0000

    if_vtnet: Defer updating generated MAC address until attached
    
    This improves spec compliance because the driver is not suppose
    to notify the device prior to setting the DRIVER_OK status, which
    could happen with the VIRTIO_NET_F_CTRL_MAC_ADDR.
    
    The VIRTIO_NET_F_MAC feature should always be negotiated so would
    be a rare situation.
    
    Reviewed by: grehan (mentor)
    Differential Revision: https://reviews.freebsd.org/D27910
---
 sys/dev/virtio/network/if_vtnet.c | 60 ++++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c
index 4a26e403071d..f55bd59ef749 100644
--- a/sys/dev/virtio/network/if_vtnet.c
+++ b/sys/dev/virtio/network/if_vtnet.c
@@ -214,8 +214,9 @@ static int	vtnet_is_link_up(struct vtnet_softc *);
 static void	vtnet_update_link_status(struct vtnet_softc *);
 static int	vtnet_ifmedia_upd(struct ifnet *);
 static void	vtnet_ifmedia_sts(struct ifnet *, struct ifmediareq *);
-static void	vtnet_get_hwaddr(struct vtnet_softc *);
-static void	vtnet_set_hwaddr(struct vtnet_softc *);
+static void	vtnet_get_macaddr(struct vtnet_softc *);
+static void	vtnet_set_macaddr(struct vtnet_softc *);
+static void	vtnet_attached_set_macaddr(struct vtnet_softc *);
 static void	vtnet_vlan_tag_remove(struct mbuf *);
 static void	vtnet_set_rx_process_limit(struct vtnet_softc *);
 static void	vtnet_set_tx_intr_threshold(struct vtnet_softc *);
@@ -568,6 +569,13 @@ vtnet_shutdown(device_t dev)
 static int
 vtnet_attach_completed(device_t dev)
 {
+	struct vtnet_softc *sc;
+
+	sc = device_get_softc(dev);
+
+	VTNET_CORE_LOCK(sc);
+	vtnet_attached_set_macaddr(sc);
+	VTNET_CORE_UNLOCK(sc);
 
 	return (0);
 }
@@ -1016,7 +1024,7 @@ vtnet_setup_interface(struct vtnet_softc *sc)
 	ifmedia_add(&sc->vtnet_media, IFM_ETHER | IFM_AUTO, 0, NULL);
 	ifmedia_set(&sc->vtnet_media, IFM_ETHER | IFM_AUTO);
 
-	vtnet_get_hwaddr(sc);
+	vtnet_get_macaddr(sc);
 	ether_ifattach(ifp, sc->vtnet_hwaddr);
 
 	if (virtio_with_feature(dev, VIRTIO_NET_F_STATUS))
@@ -3108,7 +3116,7 @@ vtnet_reinit(struct vtnet_softc *sc)
 
 	/* Use the current MAC address. */
 	bcopy(IF_LLADDR(ifp), sc->vtnet_hwaddr, ETHER_ADDR_LEN);
-	vtnet_set_hwaddr(sc);
+	vtnet_set_macaddr(sc);
 
 	vtnet_set_active_vq_pairs(sc);
 
@@ -3669,22 +3677,36 @@ vtnet_ifmedia_sts(struct ifnet *ifp, struct ifmediareq *ifmr)
 }
 
 static void
-vtnet_set_hwaddr(struct vtnet_softc *sc)
+vtnet_get_macaddr(struct vtnet_softc *sc)
 {
-	device_t dev;
 
-	dev = sc->vtnet_dev;
+	if (sc->vtnet_flags & VTNET_FLAG_MAC) {
+		virtio_read_device_config_array(sc->vtnet_dev,
+		    offsetof(struct virtio_net_config, mac),
+		    &sc->vtnet_hwaddr[0], sizeof(uint8_t), ETHER_ADDR_LEN);
+	} else {
+		/* Generate a random locally administered unicast address. */
+		sc->vtnet_hwaddr[0] = 0xB2;
+		arc4rand(&sc->vtnet_hwaddr[1], ETHER_ADDR_LEN - 1, 0);
+	}
+}
+
+static void
+vtnet_set_macaddr(struct vtnet_softc *sc)
+{
+	int error;
 
 	if (sc->vtnet_flags & VTNET_FLAG_CTRL_MAC) {
-		if (vtnet_ctrl_mac_cmd(sc, sc->vtnet_hwaddr) != 0)
-			device_printf(dev, "unable to set MAC address\n");
+		error = vtnet_ctrl_mac_cmd(sc, sc->vtnet_hwaddr);
+		if (error)
+			if_printf(sc->vtnet_ifp, "unable to set MAC address\n");
 		return;
 	}
 
-	/* In modern VirtIO the MAC config is read-only. */
+	/* MAC in config is read-only in modern VirtIO. */
 	if (!vtnet_modern(sc) && sc->vtnet_flags & VTNET_FLAG_MAC) {
 		for (int i = 0; i < ETHER_ADDR_LEN; i++) {
-			virtio_write_dev_config_1(dev,
+			virtio_write_dev_config_1(sc->vtnet_dev,
 			    offsetof(struct virtio_net_config, mac) + i,
 			    sc->vtnet_hwaddr[i]);
 		}
@@ -3692,20 +3714,12 @@ vtnet_set_hwaddr(struct vtnet_softc *sc)
 }
 
 static void
-vtnet_get_hwaddr(struct vtnet_softc *sc)
+vtnet_attached_set_macaddr(struct vtnet_softc *sc)
 {
 
-	if (sc->vtnet_flags & VTNET_FLAG_MAC) {
-		virtio_read_device_config_array(sc->vtnet_dev,
-		    offsetof(struct virtio_net_config, mac),
-		    &sc->vtnet_hwaddr[0], sizeof(uint8_t), ETHER_ADDR_LEN);
-	} else {
-		/* Generate a random locally administered unicast address. */
-		sc->vtnet_hwaddr[0] = 0xB2;
-		arc4rand(&sc->vtnet_hwaddr[1], ETHER_ADDR_LEN - 1, 0);
-		/* BMV: FIXME Cannot do before DRIVER_OK! See 3.1.2 */
-		vtnet_set_hwaddr(sc);
-	}
+	/* Assign MAC address if it was generated. */
+	if ((sc->vtnet_flags & VTNET_FLAG_MAC) == 0)
+		vtnet_set_macaddr(sc);
 }
 
 static void