Date: Thu, 7 Dec 2023 16:18:21 +0100 (CET) From: Ronald Klop <ronald-lists@klop.ws> To: Ronald Klop <ronald@FreeBSD.org> Cc: dev-commits-src-all@FreeBSD.org, src-committers@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 3878bbf1bb9e - main - Teach if_smsc to get MAC from bootargs. Message-ID: <271104654.206755.1701962301276@localhost> In-Reply-To: <202312071133.3B7BX5iQ047655@gitrepo.freebsd.org> References: <202312071133.3B7BX5iQ047655@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
------=_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 <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
Content-Type: text/html; charset=us-ascii
Content-Transfer-Encoding: 7bit
<html><head></head><body>Hi,<br>
<br>
My commit broke the build on armv6 and armv7 according to Jenkins.<br>
<br>
Printf(3) says %zd will be a better format specifier for ssize_t than %lu.<br>
<br>
diff --git a/sys/dev/usb/net/if_smsc.c b/sys/dev/usb/net/if_smsc.c<br>
index 54b18e0a67c..a59501b6bbf 100644<br>
--- a/sys/dev/usb/net/if_smsc.c<br>
+++ b/sys/dev/usb/net/if_smsc.c<br>
@@ -1594,7 +1594,7 @@ smsc_bootargs_get_mac_addr(device_t dev, struct usb_ether *ue)<br>
len = OF_getprop_alloc(node, "bootargs", (void **)&bootargs);<br>
if (len == -1 || bootargs == NULL) {<br>
smsc_warn_printf((struct smsc_softc *)ue->ue_sc,<br>
- "failed alloc for bootargs (%lu)", len);<br>
+ "failed alloc for bootargs (%zd)", len);<br>
return (false);<br>
}<br>
smsc_dbg_printf((struct smsc_softc *)ue->ue_sc, "bootargs: %s.\n",<br>
<br>
<br>
Ok to commit this?<br>
<br>
<br>
Regards,<br>
Ronald.<br>
<br>
<p><strong>Van:</strong> Ronald Klop <ronald@FreeBSD.org><br>
<strong>Datum:</strong> donderdag, 7 december 2023 12:33<br>
<strong>Aan:</strong> src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org<br>
<strong>Onderwerp:</strong> git: 3878bbf1bb9e - main - Teach if_smsc to get MAC from bootargs.</p>
<blockquote style="padding-right: 0px; padding-left: 5px; margin-left: 5px; border-left: #000000 2px solid; margin-right: 0px">
<div class="MessageRFC822Viewer" id="P">
<div class="TextPlainViewer" id="P.P">The branch main has been updated by ronald:<br>
<br>
URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=3878bbf1bb9e68f8579b57cde7d4e5c77de93320">https://cgit.FreeBSD.org/src/commit/?id=3878bbf1bb9e68f8579b57cde7d4e5c77de93320</a><br>;
<br>
commit 3878bbf1bb9e68f8579b57cde7d4e5c77de93320<br>
Author: Ronald Klop <ronald@FreeBSD.org><br>
AuthorDate: 2023-11-04 14:14:00 +0000<br>
Commit: Ronald Klop <ronald@FreeBSD.org><br>
CommitDate: 2023-12-07 11:32:01 +0000<br>
<br>
Teach if_smsc to get MAC from bootargs.<br>
<br>
Some Raspberry Pi pass smsc95xx.macaddr=XX:XX:XX:XX:XX:XX as bootargs.<br>
Use this if no ethernet address is found in an EEPROM.<br>
As last resort fall back to ether_gen_addr() instead of random MAC.<br>
<br>
PR: 274092<br>
Reported by: Patrick M. Hausen (via ML)<br>
Reviewed by: imp, karels, zlei<br>
Tested by: Patrick M. Hausen<br>
Approved by: karels<br>
MFC after: 1 month<br>
Relnotes: yes<br>
Differential Revision: <a href="https://reviews.freebsd.org/D42463">https://reviews.freebsd.org/D42463</a><br>;
---<br>
sys/dev/usb/net/if_smsc.c | 86 +++++++++++++++++++++++++++++++++++++++++++++--<br>
sys/net/ethernet.h | 1 +<br>
sys/net/if_ethersubr.c | 10 ++++--<br>
3 files changed, 92 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/sys/dev/usb/net/if_smsc.c b/sys/dev/usb/net/if_smsc.c<br>
index ec8197229f17..54b18e0a67c9 100644<br>
--- a/sys/dev/usb/net/if_smsc.c<br>
+++ b/sys/dev/usb/net/if_smsc.c<br>
@@ -179,6 +179,8 @@ static const struct usb_device_id smsc_devs[] = {<br>
#define ETHER_IS_VALID(addr) \<br>
(!ETHER_IS_MULTICAST(addr) && !ETHER_IS_ZERO(addr))<br>
<br>
+#define BOOTARGS_SMSC95XX "smsc95xx.macaddr"<br>
+<br>
static device_probe_t smsc_probe;<br>
static device_attach_t smsc_attach;<br>
static device_detach_t smsc_detach;<br>
@@ -1538,6 +1540,76 @@ smsc_ioctl(if_t ifp, u_long cmd, caddr_t data)<br>
return (rc);<br>
}<br>
<br>
+#ifdef FDT<br>
+static bool<br>
+smsc_get_smsc95xx_macaddr(char* bootargs, size_t len, struct usb_ether *ue)<br>
+{<br>
+ int values[6];<br>
+ int i;<br>
+ char* p;<br>
+<br>
+ p = strnstr(bootargs, BOOTARGS_SMSC95XX, len);<br>
+ if (p == NULL)<br>
+ return (false);<br>
+<br>
+ if (sscanf(p, BOOTARGS_SMSC95XX "=%x:%x:%x:%x:%x:%x%*c",<br>
+ &values[0], &values[1], &values[2],<br>
+ &values[3], &values[4], &values[5]) != 6) {<br>
+ smsc_warn_printf((struct smsc_softc *)ue->ue_sc,<br>
+ "invalid mac from bootargs '%s'.\n", p);<br>
+ return (false);<br>
+ }<br>
+<br>
+ for (i = 0; i < ETHER_ADDR_LEN; ++i)<br>
+ ue->ue_eaddr[i] = values[i];<br>
+<br>
+ smsc_dbg_printf((struct smsc_softc *)ue->ue_sc,<br>
+ "bootargs mac=%6D.\n", ue->ue_eaddr, ":");<br>
+ return (true);<br>
+}<br>
+<br>
+/**<br>
+ * Raspberry Pi is known to pass smsc95xx.macaddr=XX:XX:XX:XX:XX:XX via<br>
+ * bootargs.<br>
+ */<br>
+static bool<br>
+smsc_bootargs_get_mac_addr(device_t dev, struct usb_ether *ue)<br>
+{<br>
+ char *bootargs;<br>
+ ssize_t len;<br>
+ phandle_t node;<br>
+<br>
+ /* only use bootargs for the first device<br>
+ * to prevent duplicate mac addresses */<br>
+ if (device_get_unit(dev) != 0)<br>
+ return (false);<br>
+ node = OF_finddevice("/chosen");<br>
+ if (node == -1)<br>
+ return (false);<br>
+ if (OF_hasprop(node, "bootargs") == 0) {<br>
+ smsc_dbg_printf((struct smsc_softc *)ue->ue_sc,<br>
+ "bootargs not found");<br>
+ return (false);<br>
+ }<br>
+ len = OF_getprop_alloc(node, "bootargs", (void **)&bootargs);<br>
+ if (len == -1 || bootargs == NULL) {<br>
+ smsc_warn_printf((struct smsc_softc *)ue->ue_sc,<br>
+ "failed alloc for bootargs (%lu)", len);<br>
+ return (false);<br>
+ }<br>
+ smsc_dbg_printf((struct smsc_softc *)ue->ue_sc, "bootargs: %s.\n",<br>
+ bootargs);<br>
+ if (!smsc_get_smsc95xx_macaddr(bootargs, len, ue)) {<br>
+ OF_prop_free(bootargs);<br>
+ return (false);<br>
+ }<br>
+ OF_prop_free(bootargs);<br>
+ device_printf(dev, "MAC address found in bootargs %6D.\n",<br>
+ ue->ue_eaddr, ":");<br>
+ return (true);<br>
+}<br>
+#endif<br>
+<br>
/**<br>
* smsc_attach_post - Called after the driver attached to the USB interface<br>
* @ue: the USB ethernet device<br>
@@ -1552,8 +1624,10 @@ static void<br>
smsc_attach_post(struct usb_ether *ue)<br>
{<br>
struct smsc_softc *sc = uether_getsc(ue);<br>
+ struct ether_addr eaddr;<br>
uint32_t mac_h, mac_l;<br>
int err;<br>
+ int i;<br>
<br>
smsc_dbg_printf(sc, "smsc_attach_post\n");<br>
<br>
@@ -1585,11 +1659,17 @@ smsc_attach_post(struct usb_ether *ue)<br>
#ifdef FDT<br>
if ((err != 0) || (!ETHER_IS_VALID(sc->sc_ue.ue_eaddr)))<br>
err = usb_fdt_get_mac_addr(sc->sc_ue.ue_dev, &sc->sc_ue);<br>
+ if ((err != 0) || (!ETHER_IS_VALID(sc->sc_ue.ue_eaddr)))<br>
+ err = smsc_bootargs_get_mac_addr(sc->sc_ue.ue_dev,<br>
+ &sc->sc_ue) ? (0) : (1);<br>
#endif<br>
if ((err != 0) || (!ETHER_IS_VALID(sc->sc_ue.ue_eaddr))) {<br>
- read_random(sc->sc_ue.ue_eaddr, ETHER_ADDR_LEN);<br>
- sc->sc_ue.ue_eaddr[0] &= ~0x01; /* unicast */<br>
- sc->sc_ue.ue_eaddr[0] |= 0x02; /* locally administered */<br>
+ smsc_dbg_printf(sc, "No MAC address found."<br>
+ " Using ether_gen_addr().\n");<br>
+ ether_gen_addr_byname(device_get_nameunit(ue->ue_dev),<br>
+ &eaddr);<br>
+ for (i = 0; i < ETHER_ADDR_LEN; i++)<br>
+ sc->sc_ue.ue_eaddr[i] = eaddr.octet[i];<br>
}<br>
}<br>
<br>
diff --git a/sys/net/ethernet.h b/sys/net/ethernet.h<br>
index fca6748a0da7..e7313e78c5bb 100644<br>
--- a/sys/net/ethernet.h<br>
+++ b/sys/net/ethernet.h<br>
@@ -448,6 +448,7 @@ struct mbuf *ether_vlanencap_proto(struct mbuf *, uint16_t, uint16_t);<br>
bool ether_8021q_frame(struct mbuf **mp, struct ifnet *ife,<br>
struct ifnet *p, const struct ether_8021q_tag *);<br>
void ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr);<br>
+void ether_gen_addr_byname(const char *nameunit, struct ether_addr *hwaddr);<br>
<br>
static __inline struct mbuf *ether_vlanencap(struct mbuf *m, uint16_t tag)<br>
{<br>
diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c<br>
index 7a32d0091260..ef0b1f705260 100644<br>
--- a/sys/net/if_ethersubr.c<br>
+++ b/sys/net/if_ethersubr.c<br>
@@ -1487,7 +1487,7 @@ ether_8021q_frame(struct mbuf **mp, struct ifnet *ife, struct ifnet *p,<br>
* allocate non-locally-administered addresses.<br>
*/<br>
void<br>
-ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)<br>
+ether_gen_addr_byname(const char *nameunit, struct ether_addr *hwaddr)<br>
{<br>
SHA1_CTX ctx;<br>
char *buf;<br>
@@ -1506,7 +1506,7 @@ ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)<br>
/* If each (vnet) jail would also have a unique hostuuid this would not<br>
* be necessary. */<br>
getjailname(curthread->td_ucred, jailname, sizeof(jailname));<br>
- sz = asprintf(&buf, M_TEMP, "%s-%s-%s", uuid, if_name(ifp),<br>
+ sz = asprintf(&buf, M_TEMP, "%s-%s-%s", uuid, nameunit,<br>
jailname);<br>
if (sz < 0) {<br>
/* Fall back to a random mac address. */<br>
@@ -1535,5 +1535,11 @@ rando:<br>
hwaddr->octet[0] |= 0x02;<br>
}<br>
<br>
+void<br>
+ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)<br>
+{<br>
+ ether_gen_addr_byname(if_name(ifp), hwaddr);<br>
+}<br>
+<br>
DECLARE_MODULE(ether, ether_mod, SI_SUB_INIT_IF, SI_ORDER_ANY);<br>
MODULE_VERSION(ether, 1);<br>
</div>
<hr></div>
</blockquote>
<br>
</body></html>
------=_Part_206754_566537090.1701962301262--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?271104654.206755.1701962301276>