Date: Wed, 27 Aug 2014 06:35:23 +0000 From: "Pokala, Ravi" <rpokala@panasas.com> To: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>, Ryan Stone <rysto32@gmail.com>, Brooks Davis <brooks@freebsd.org> Subject: Re: Common storage of original MAC address Message-ID: <D022CA39.11D8E5%rpokala@panasas.com>
next in thread | raw e-mail | index | archive | help
-----Original Message-----
From: <Pokala>, Ravi Pokala <rpokala@panasas.com>
Date: Monday, August 18, 2014 at 10:19 PM
To: Ryan Stone <rysto32@gmail.com>
Cc: Brooks Davis <brooks@freebsd.org>, "freebsd-hackers@freebsd.org"
<freebsd-hackers@freebsd.org>
Subject: Re: Common storage of original MAC address
> -----Original Message-----
> From: Ryan Stone <rysto32@gmail.com>
> Date: Monday, August 18, 2014 at 11:14 AM
> To: Ravi Pokala <rpokala@panasas.com>
> Cc: Brooks Davis <brooks@freebsd.org>, "freebsd-hackers@freebsd.org"
> <freebsd-hackers@freebsd.org>
> Subject: Re: Common storage of original MAC address
>=20
>> On Mon, Aug 18, 2014 at 11:59 AM, Pokala, Ravi <rpokala@panasas.com>
>> wrote:
>>=20
>>> ...
>>=20
>> Personally I think that it would be better to save it in binary format
>> and convert it to a string as needed.
>=20
> I'm fine with that too; the reason I suggested a string is because that's
> what's already getting passed to ether_ifattach(). I suppose it's easy
> enough to run the string through ether_aton() to get the binary version.
Actually, it turns out ether_aton() is defined in <net/ethernet.h>... but
only for userspace. But that's okay - the 'lla' passed to ether_ifattach()
isn't a string, it's a byte array; I mis-read it. So, a simple bcopy()
DTRT.
> But again, not everything is Ethernet, so we might need to have different
> binary representations; if we're doing that, we might as well just use
> the string version, and let the caller decide how to parse the string.
> (I'm specifically thinking about IP-over-Infiniband - while I'm sure IB
> cards must have some type of hardware address, it's probably not the same
> format as an Ethernet MAC address.)
I was right and wrong - the IB hardware address format is different from
the Ethernet MAC address format:
sys/ofed/drivers/net/mlx4/mlx4_en.h
485 struct mlx4_en_priv {
...
524 u64 mac;
But the OFED driver maps the IB hardware address to an Ethernet MAC
address:
sys/ofed/drivers/net/mlx4/en_netdev.c: mlx4_en_init_netdev()
...
1530 struct mlx4_en_priv *priv;
1531 uint8_t dev_addr[ETHER_ADDR_LEN];
...
1642 /* Set default MAC */
1643 for (i =3D 0; i < ETHER_ADDR_LEN; i++)
1644 dev_addr[ETHER_ADDR_LEN - 1 - i] =3D (u8) (priv->mac >> (8 * i)=
);
1645
1646 ether_ifattach(dev, dev_addr);
So just using a 'struct ether_addr' seems to be fine.
>> It would be useful to have the MAC address saved aside somewhere so that
>> a "Restore MAC to HW default" ioctl could be implemented; this would be
>> useful in the if_lagg driver to restore the MAC on a port after it has
>> been removed from a lagg.
Doesn't this restore the original MAC on LAGG teardown?
sys/net/if_lagg.c: lagg_port_destroy()
729 /*
730 * Remove multicast addresses and interface flags from this port and
731 * reset the MAC address, skip if the interface is being detached.
732 */
733 if (!lp->lp_detaching) {
734 lagg_ether_cmdmulti(lp, 0);
735 lagg_setflags(lp, 0);
736 lagg_port_lladdr(lp, lp->lp_lladdr);
737 }
If someone can enumerate the places where we want to restore the original
LLADDR but currently don't, then I'll add that.
> Or how about an ioctl to get the original MAC (rather than a sysctl).
> Then the "restore to default" code would be a two-step process - get the
> original MAC with the new ioctl (say "SIOCGHWLLADDR" for "Get Hardware
> LLADDR"?), and then set the working MAC to that value w/ the existing
> ioctl (SIOCSIFLLADDR).
>=20
> I actually like this idea more than the sysctl, because it could be done
> in one place (probably in net/if.c, next to if_setlladdr() (which is what
> implements the guts of SIOCSIFLLADDR)).
I've done the easy stuff - added the field to 'struct arpcom', populated
it, added the ioctl and a function to implement it, and created a simple
test program. See the patch and test code (at end of message); the test
output:
root@fbsd-X:~ # ifconfig em1 ether
em1: flags=3D8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500
options=3D9b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM>
ether 08:00:27:f0:8f:8a
root@fbsd-X:~ # ./ghwlladdr-test em1
em1 current lladdr: 08:00:27:f0:8f:8a
em1 hardware lladdr: 08:00:27:f0:8f:8a
root@fbsd-X:~ # ifconfig em1 ether 12:34:56:78:9a:bc
root@fbsd-X:~ # ifconfig em1 ether
em1: flags=3D8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500
options=3D9b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM>
ether 12:34:56:78:9a:bc
root@fbsd-X:~ # ./ghwlladdr-test em1
em1 current lladdr: 12:34:56:78:9a:bc
em1 hardware lladdr: 08:00:27:f0:8f:8a
> Does that sound like a plan?
If what I've done so far looks good, I'll continue to the next steps:
changing the places where we want to auto-restore the HW LLADDR (see my
question above), and updating `ifconfig' to report and restore the HW
LLADDR.
Thanks,
Ravi
--------------------------- ghwlladdr.patch ----------------------------
Index: sys/net/if.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/net/if.c (revision 270646)
+++ sys/net/if.c (working copy)
@@ -2480,6 +2480,10 @@
EVENTHANDLER_INVOKE(iflladdr_event, ifp);
break;
+ case SIOCGHWLLADDR:
+ error =3D if_gethwlladdr(ifp, ifr);
+ break;
+
case SIOCAIFGROUP:
{
struct ifgroupreq *ifgr =3D (struct ifgroupreq *)ifr;
@@ -3353,6 +3357,33 @@
}
/*
+ * Get the link layer address that was read from the hardware at probe.
+ *
+ * At this time we only support certain types of interfaces.
+ */
+int
+if_gethwlladdr(struct ifnet *ifp, struct ifreq *ifr)
+{
+ switch (ifp->if_type) {
+ case IFT_ETHER:
+ case IFT_FDDI:
+ case IFT_XETHER:
+ case IFT_ISO88025:
+ case IFT_L2VLAN:
+ case IFT_BRIDGE:
+ case IFT_ARCNET:
+ case IFT_IEEE8023ADLAG:
+ case IFT_IEEE80211:
+ bcopy(&IFP2AC(ifp)->hw_lladdr, ifr->ifr_addr.sa_data, ETHER_ADDR_LEN);
+ ifr->ifr_addr.sa_len =3D ETHER_ADDR_LEN;
+ break;
+ default:
+ return (ENODEV);
+ }
+ return (0);
+}
+
+/*
* The name argument must be a pointer to storage which will last as
* long as the interface does. For physical devices, the result of
* device_get_name(dev) is a good choice and for pseudo-devices a
Index: sys/net/if_arp.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/net/if_arp.h (revision 270646)
+++ sys/net/if_arp.h (working copy)
@@ -102,9 +102,11 @@
* Structure shared between the ethernet driver modules and
* the address resolution code.
*/
+#include <net/ethernet.h>
struct arpcom {
struct ifnet *ac_ifp; /* network-visible interface */
void *ac_netgraph; /* ng_ether(4) netgraph node info */
+ struct ether_addr hw_lladdr; /* original lladdr from hardware */
};
#define IFP2AC(ifp) ((struct arpcom *)(ifp->if_l2com))
#define AC2IFP(ac) ((ac)->ac_ifp)
Index: sys/net/if_ethersubr.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/net/if_ethersubr.c (revision 270646)
+++ sys/net/if_ethersubr.c (working copy)
@@ -841,6 +841,7 @@
sdl->sdl_type =3D IFT_ETHER;
sdl->sdl_alen =3D ifp->if_addrlen;
bcopy(lla, LLADDR(sdl), ifp->if_addrlen);
+ bcopy(lla, &IFP2AC(ifp)->hw_lladdr, ifp->if_addrlen);
bpfattach(ifp, DLT_EN10MB, ETHER_HDR_LEN);
if (ng_ether_attach_p !=3D NULL)
Index: sys/net/if_var.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/net/if_var.h (revision 270646)
+++ sys/net/if_var.h (working copy)
@@ -485,6 +485,7 @@
void if_ref(struct ifnet *);
void if_rele(struct ifnet *);
int if_setlladdr(struct ifnet *, const u_char *, int);
+int if_gethwlladdr(struct ifnet *, struct ifreq *);
void if_up(struct ifnet *);
int ifioctl(struct socket *, u_long, caddr_t, struct thread *);
int ifpromisc(struct ifnet *, int);
Index: sys/sys/sockio.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/sys/sockio.h (revision 270646)
+++ sys/sys/sockio.h (working copy)
@@ -96,6 +96,7 @@
#define SIOCGIFSTATUS _IOWR('i', 59, struct ifstat) /* get IF status */
#define SIOCSIFLLADDR _IOW('i', 60, struct ifreq) /* set linklevel addr
*/
+#define SIOCGHWLLADDR _IOWR('i', 61, struct ifreq) /* get hw lladdr */
#define SIOCSIFPHYADDR _IOW('i', 70, struct ifaliasreq) /* set gif
addres */
#define SIOCGIFPSRCADDR _IOWR('i', 71, struct ifreq) /* get gif psrc addr
*/
--------------------------- ghwlladdr-test.c ---------------------------
#include <errno.h>
#include <fcntl.h>
#include <ifaddrs.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <net/ethernet.h>
#include <net/if.h>
#include <net/if_dl.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
int
main(
int argc,
char **argv)
{
char *ifname =3D NULL;
struct ifaddrs *ifap =3D NULL;
struct ifaddrs *ifa =3D NULL;
struct sockaddr_dl *sdl =3D NULL;
struct ifreq ifr;
int sock =3D -1;
int error =3D 0;
if (argc !=3D 2) {
printf("usage: %s <ifname>\n", argv[0]);
error =3D 1;
goto cleanup;
}
ifname =3D argv[1];
error =3D getifaddrs(&ifap);
if (error !=3D 0) {
error =3D errno;
printf("getifaddrs(): %d (%s)\n", error, strerror(error));
goto cleanup;
}
for (ifa =3D ifap; ifa; ifa =3D ifa->ifa_next) {
if (strcmp(ifa->ifa_name, ifname) =3D=3D 0) {
sdl =3D (struct sockaddr_dl *) ifa->ifa_addr;
printf("%s current lladdr: %s\n", ifname,
ether_ntoa((struct ether_addr *) LLADDR(sdl)));
break;
}
}
strncpy(ifr.ifr_name, ifa->ifa_name, sizeof(ifr.ifr_name));
memcpy(&ifr.ifr_addr, ifa->ifa_addr, sizeof(ifa->ifa_addr->sa_len));
ifr.ifr_addr.sa_family =3D AF_LOCAL;
sock =3D socket(ifr.ifr_addr.sa_family, SOCK_DGRAM, 0);
if (sock =3D=3D -1) {
error =3D errno;
printf("socket(): %d (%s)\n", error, strerror(error));
goto cleanup;
}
error =3D ioctl(sock, SIOCGHWLLADDR, &ifr);
if (error =3D=3D -1) {
printf("ioctl(): %d (%s)\n", error, strerror(error));
goto cleanup;
}
printf("%s hardware lladdr: %s\n", ifname,
ether_ntoa((const struct ether_addr *) &ifr.ifr_addr.sa_data));
cleanup:
if (sock !=3D -1) {
close(sock);
}
return error;
}
------------------------------------------------------------------------
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D022CA39.11D8E5%rpokala>