From nobody Thu Dec 7 15:18:21 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SmHwQ0QHjz537vQ; Thu, 7 Dec 2023 15:18:30 +0000 (UTC) (envelope-from SRS0=aNgP=HS=klop.ws=ronald-lists@realworks.nl) Received: from smtp-relay-int.realworks.nl (smtp-relay-int.realworks.nl [194.109.157.24]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4SmHwP3mxcz4LRn; Thu, 7 Dec 2023 15:18:29 +0000 (UTC) (envelope-from SRS0=aNgP=HS=klop.ws=ronald-lists@realworks.nl) Authentication-Results: mx1.freebsd.org; none Date: Thu, 7 Dec 2023 16:18:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=klop.ws; s=rw2; t=1701962301; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=esk4jL0wRV+Ps6u6KwjpcGqafssBlw1ln9Q/jfMWdWk=; b=tW+QC7zA0x1lHIYTTgKMOOBhylJ0CLJlhgRpmKEKSLjEQhyo77pu6Zwjp2cBFBJkPuPFJE pLmpjd5PoTSn8fuAoTatisD4CSUPBVoc9c+6KvJkBW9QtSKUpABD84OVoC6mJ68i0Rg9Q9 oh010YcqhHJQu55/IVDmbfSe1+BbQlqy/bH9ExTecTyV2ukGuvGM8soVrtW/JBFxMk63rJ wzXlp6HG0JgFYnXp1+/+Vd8I/Ym8sp4VwYml6cpL/X+BTjpRXrigmk1UBfhuXtxxK9PJJ6 477Wdo7B3PYQYLLgwygHHeSsVpkOZYlFY2HHMMs38nKu5qFc/3UnDfNJ8z4STQ== From: Ronald Klop To: Ronald Klop Cc: dev-commits-src-all@FreeBSD.org, src-committers@FreeBSD.org, dev-commits-src-main@FreeBSD.org Message-ID: <271104654.206755.1701962301276@localhost> In-Reply-To: <202312071133.3B7BX5iQ047655@gitrepo.freebsd.org> References: <202312071133.3B7BX5iQ047655@gitrepo.freebsd.org> Subject: Re: git: 3878bbf1bb9e - main - Teach if_smsc to get MAC from bootargs. List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_206754_566537090.1701962301262" X-Mailer: Realworks (681.40) Importance: Normal X-Priority: 3 (Normal) X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:3265, ipnet:194.109.0.0/16, country:NL] X-Spamd-Bar: ---- X-Rspamd-Queue-Id: 4SmHwP3mxcz4LRn ------=_Part_206754_566537090.1701962301262 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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? Regards, Ronald. Van: Ronald Klop 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 > AuthorDate: 2023-11-04 14:14:00 +0000 > Commit: Ronald Klop > 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); > > > > ------=_Part_206754_566537090.1701962301262 Content-Type: text/html; charset=us-ascii Content-Transfer-Encoding: 7bit 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?


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);
 


  ------=_Part_206754_566537090.1701962301262--