Re: git: 3878bbf1bb9e - main - Teach if_smsc to get MAC from bootargs.

On Thu, 7 Dec 2023 11:33:05 GMT
Ronald Klop <ronald@FreeBSD.org> wrote:

> The branch main has been updated by ronald:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=3878bbf1bb9e68f8579b57cde7d4e5c77de93320
> 
> commit 3878bbf1bb9e68f8579b57cde7d4e5c77de93320
> Author:     Ronald Klop <ronald@FreeBSD.org>
> AuthorDate: 2023-11-04 14:14:00 +0000
> Commit:     Ronald Klop <ronald@FreeBSD.org>
> CommitDate: 2023-12-07 11:32:01 +0000
> 
>     Teach if_smsc to get MAC from bootargs.
>     
>     Some Raspberry Pi pass smsc95xx.macaddr=XX:XX:XX:XX:XX:XX as bootargs.
>     Use this if no ethernet address is found in an EEPROM.
>     As last resort fall back to ether_gen_addr() instead of random MAC.
>     
>     PR:     274092
>     Reported by:    Patrick M. Hausen (via ML)
>     Reviewed by:    imp, karels, zlei
>     Tested by:      Patrick M. Hausen
>     Approved by:    karels
>     MFC after:      1 month
>     Relnotes:       yes
>     Differential Revision: https://reviews.freebsd.org/D42463
> ---
>  sys/dev/usb/net/if_smsc.c | 86 +++++++++++++++++++++++++++++++++++++++++++++--
>  sys/net/ethernet.h        |  1 +
>  sys/net/if_ethersubr.c    | 10 ++++--
>  3 files changed, 92 insertions(+), 5 deletions(-)
> 
> diff --git a/sys/dev/usb/net/if_smsc.c b/sys/dev/usb/net/if_smsc.c
> index ec8197229f17..54b18e0a67c9 100644
> --- a/sys/dev/usb/net/if_smsc.c
> +++ b/sys/dev/usb/net/if_smsc.c
> @@ -179,6 +179,8 @@ static const struct usb_device_id smsc_devs[] = {
>  #define ETHER_IS_VALID(addr) \
>  	(!ETHER_IS_MULTICAST(addr) && !ETHER_IS_ZERO(addr))
>  
> +#define BOOTARGS_SMSC95XX	"smsc95xx.macaddr"
> +
>  static device_probe_t smsc_probe;
>  static device_attach_t smsc_attach;
>  static device_detach_t smsc_detach;
> @@ -1538,6 +1540,76 @@ smsc_ioctl(if_t ifp, u_long cmd, caddr_t data)
>  	return (rc);
>  }
>  
> +#ifdef FDT
> +static bool
> +smsc_get_smsc95xx_macaddr(char* bootargs, size_t len, struct usb_ether *ue)
> +{
> +	int values[6];
> +	int i;
> +	char* p;
> +
> +	p = strnstr(bootargs, BOOTARGS_SMSC95XX, len);
> +	if (p == NULL)
> +		return (false);
> +
> +	if (sscanf(p, BOOTARGS_SMSC95XX "=%x:%x:%x:%x:%x:%x%*c",
> +			&values[0], &values[1], &values[2],
> +			&values[3], &values[4], &values[5]) != 6) {
> +		smsc_warn_printf((struct smsc_softc *)ue->ue_sc,
> +				"invalid mac from bootargs '%s'.\n", p);
> +		return (false);
> +	}
> +
> +	for (i = 0; i < ETHER_ADDR_LEN; ++i)
> +		ue->ue_eaddr[i] = values[i];
> +
> +	smsc_dbg_printf((struct smsc_softc *)ue->ue_sc,
> +			"bootargs mac=%6D.\n", ue->ue_eaddr, ":");
> +	return (true);
> +}
> +
> +/**
> + * Raspberry Pi is known to pass smsc95xx.macaddr=XX:XX:XX:XX:XX:XX via
> + * bootargs.
> + */
> +static bool
> +smsc_bootargs_get_mac_addr(device_t dev, struct usb_ether *ue)
> +{
> +	char *bootargs;
> +	ssize_t len;
> +	phandle_t node;
> +
> +	/* only use bootargs for the first device
> +	 * to prevent duplicate mac addresses */
> +	if (device_get_unit(dev) != 0)
> +		return (false);
> +	node = OF_finddevice("/chosen");
> +	if (node == -1)
> +		return (false);
> +	if (OF_hasprop(node, "bootargs") == 0) {
> +		smsc_dbg_printf((struct smsc_softc *)ue->ue_sc,
> +				"bootargs not found");
> +		return (false);
> +	}
> +	len = OF_getprop_alloc(node, "bootargs", (void **)&bootargs);
> +	if (len == -1 || bootargs == NULL) {
> +		smsc_warn_printf((struct smsc_softc *)ue->ue_sc,
> +				"failed alloc for bootargs (%lu)", len);
> +		return (false);
> +	}

 All this can be replaced with a call to fdt_get_chosen_bootargs.
 Also we already do this for arm and arm64 in machdep_boot.c and store
the linux boot args in a static var called linux_command_line, maybe
make it not static and get it from this driver.
 While you're at it make bcm2835_fbd.c and bcm2835_fb.c use this as
they also get and parse the bootargs.

 Cheers,

> +	smsc_dbg_printf((struct smsc_softc *)ue->ue_sc, "bootargs: %s.\n",
> +			bootargs);
> +	if (!smsc_get_smsc95xx_macaddr(bootargs, len, ue)) {
> +		OF_prop_free(bootargs);
> +		return (false);
> +	}
> +	OF_prop_free(bootargs);
> +	device_printf(dev, "MAC address found in bootargs %6D.\n",
> +			ue->ue_eaddr, ":");
> +	return (true);
> +}
> +#endif
> +
>  /**
>   *	smsc_attach_post - Called after the driver attached to the USB interface
>   *	@ue: the USB ethernet device
> @@ -1552,8 +1624,10 @@ static void
>  smsc_attach_post(struct usb_ether *ue)
>  {
>  	struct smsc_softc *sc = uether_getsc(ue);
> +	struct ether_addr eaddr;
>  	uint32_t mac_h, mac_l;
>  	int err;
> +	int i;
>  
>  	smsc_dbg_printf(sc, "smsc_attach_post\n");
>  
> @@ -1585,11 +1659,17 @@ smsc_attach_post(struct usb_ether *ue)
>  #ifdef FDT
>  		if ((err != 0) || (!ETHER_IS_VALID(sc->sc_ue.ue_eaddr)))
>  			err = usb_fdt_get_mac_addr(sc->sc_ue.ue_dev, &sc->sc_ue);
> +		if ((err != 0) || (!ETHER_IS_VALID(sc->sc_ue.ue_eaddr)))
> +			err = smsc_bootargs_get_mac_addr(sc->sc_ue.ue_dev,
> +					&sc->sc_ue) ? (0) : (1);
>  #endif
>  		if ((err != 0) || (!ETHER_IS_VALID(sc->sc_ue.ue_eaddr))) {
> -			read_random(sc->sc_ue.ue_eaddr, ETHER_ADDR_LEN);
> -			sc->sc_ue.ue_eaddr[0] &= ~0x01;     /* unicast */
> -			sc->sc_ue.ue_eaddr[0] |=  0x02;     /* locally administered */
> +			smsc_dbg_printf(sc, "No MAC address found."
> +					" Using ether_gen_addr().\n");
> +			ether_gen_addr_byname(device_get_nameunit(ue->ue_dev),
> +					&eaddr);
> +			for (i = 0; i < ETHER_ADDR_LEN; i++)
> +				sc->sc_ue.ue_eaddr[i] = eaddr.octet[i];
>  		}
>  	}
>  
> diff --git a/sys/net/ethernet.h b/sys/net/ethernet.h
> index fca6748a0da7..e7313e78c5bb 100644
> --- a/sys/net/ethernet.h
> +++ b/sys/net/ethernet.h
> @@ -448,6 +448,7 @@ struct mbuf  *ether_vlanencap_proto(struct mbuf *, uint16_t, uint16_t);
>  bool	ether_8021q_frame(struct mbuf **mp, struct ifnet *ife,
>  		struct ifnet *p, const struct ether_8021q_tag *);
>  void	ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr);
> +void	ether_gen_addr_byname(const char *nameunit, struct ether_addr *hwaddr);
>  
>  static __inline struct mbuf *ether_vlanencap(struct mbuf *m, uint16_t tag)
>  {
> diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
> index 7a32d0091260..ef0b1f705260 100644
> --- a/sys/net/if_ethersubr.c
> +++ b/sys/net/if_ethersubr.c
> @@ -1487,7 +1487,7 @@ ether_8021q_frame(struct mbuf **mp, struct ifnet *ife, struct ifnet *p,
>   * allocate non-locally-administered addresses.
>   */
>  void
> -ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)
> +ether_gen_addr_byname(const char *nameunit, struct ether_addr *hwaddr)
>  {
>  	SHA1_CTX ctx;
>  	char *buf;
> @@ -1506,7 +1506,7 @@ ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)
>  	/* If each (vnet) jail would also have a unique hostuuid this would not
>  	 * be necessary. */
>  	getjailname(curthread->td_ucred, jailname, sizeof(jailname));
> -	sz = asprintf(&buf, M_TEMP, "%s-%s-%s", uuid, if_name(ifp),
> +	sz = asprintf(&buf, M_TEMP, "%s-%s-%s", uuid, nameunit,
>  	    jailname);
>  	if (sz < 0) {
>  		/* Fall back to a random mac address. */
> @@ -1535,5 +1535,11 @@ rando:
>  	hwaddr->octet[0] |= 0x02;
>  }
>  
> +void
> +ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)
> +{
> +	ether_gen_addr_byname(if_name(ifp), hwaddr);
> +}
> +
>  DECLARE_MODULE(ether, ether_mod, SI_SUB_INIT_IF, SI_ORDER_ANY);
>  MODULE_VERSION(ether, 1);


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>