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

From: Mike Karels <mike_at_karels.net>
Date: Thu, 07 Dec 2023 15:36:17 UTC
On 7 Dec 2023, at 9:18, Ronald Klop wrote:

> Hi,
>
> My commit broke the build on armv6 and armv7 according to Jenkins.
>
> Printf(3) says %zd will be a better format specifier for ssize_t than %lu.
>
> diff --git a/sys/dev/usb/net/if_smsc.c b/sys/dev/usb/net/if_smsc.c
> index 54b18e0a67c..a59501b6bbf 100644
> --- a/sys/dev/usb/net/if_smsc.c
> +++ b/sys/dev/usb/net/if_smsc.c
> @@ -1594,7 +1594,7 @@ smsc_bootargs_get_mac_addr(device_t dev, struct usb_ether *ue)
>        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);
> +                               "failed alloc for bootargs (%zd)", len);
>                return (false);
>        }
>        smsc_dbg_printf((struct smsc_softc *)ue->ue_sc, "bootargs: %s.\n",
>
>
> Ok to commit this?

Yes, please do.

		Mike

> Regards,
> Ronald.
>
>
> Van: Ronald Klop <ronald@FreeBSD.org>
> Datum: donderdag, 7 december 2023 12:33
> Aan: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
> Onderwerp: git: 3878bbf1bb9e - main - Teach if_smsc to get MAC from bootargs.
>>
>> 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);
>> +   }
>> +   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);
>>
>>
>>