Prev: wm8960: fix compile error
Next: sysfs: add entry to indicate network interfaces with random MAC address
From: Ben Hutchings on 20 Jul 2010 07:30
On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> From: Stefan Assmann <sassmann(a)redhat.com>
>
> Reserve a bit in struct net_device to indicate whether an interface
> generates its MAC address randomly, and expose the information via
> sysfs.
> May look like this:
> /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/net/eth0/ifrndmac
>
> By default the value of ifrndmac is 0. Any driver that generates the MAC
> address randomly should return a value to 1.

The name should incorporate 'address', not 'mac', for consistency with
the generic 'address' attribute.

What about devices that 'steal' MAC addresses from slave devices?
Currently I believe udev has special cases for them but ideally these
drivers would indicate explicitly that their addresses are not stable
identifiers (even though they aren't random).

[...]
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index b626289..2ea0298 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -845,6 +845,7 @@ struct net_device {
> #define NETIF_F_FCOE_MTU (1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
> #define NETIF_F_NTUPLE (1 << 27) /* N-tuple filters supported */
> #define NETIF_F_RXHASH (1 << 28) /* Receive hashing offload */
> +#define NETIF_F_RNDMAC (1 << 29) /* Interface with random MAC address */
[...]

This is not really a feature, and we are running out of real feature
bits. Can you find somewhere else to put this flag?

Ben.

--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Ben Hutchings on 20 Jul 2010 08:10
On Tue, 2010-07-20 at 13:47 +0200, Stefan Assmann wrote:
> On 20.07.2010 13:20, Ben Hutchings wrote:
> > On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> >> From: Stefan Assmann <sassmann(a)redhat.com>
> >>
> >> Reserve a bit in struct net_device to indicate whether an interface
> >> generates its MAC address randomly, and expose the information via
> >> sysfs.
> >> May look like this:
> >> /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/net/eth0/ifrndmac
> >>
> >> By default the value of ifrndmac is 0. Any driver that generates the MAC
> >> address randomly should return a value to 1.
> >
> > The name should incorporate 'address', not 'mac', for consistency with
> > the generic 'address' attribute.
>
> We can call it "ifrndhwaddr" if that's more consistent.
>
> >
> > What about devices that 'steal' MAC addresses from slave devices?
> > Currently I believe udev has special cases for them but ideally these
> > drivers would indicate explicitly that their addresses are not stable
> > identifiers (even though they aren't random).
>
> It's really up to the driver to decide whether it makes sense to set the
> flag or not. The question is what should udev do with these MAC address
> stealing devices apart from ignoring them? Sorry I have no higher
> insight into it.
> This flag has the purpose to allow udev to explicitly handle devices
> that generate their MAC address randomly and generate a persistent
> rule based on the device path instead of the MAC address.
> I'm open for suggestions but I'm not sure we can handle both cases with
> a single flag.

OK, then call it something like 'address_temporary'.

> JFYI this is an alternative approach to changing the kernel name of VFs
> to vfeth. The advantage of this way should be that we don't break any
> user-space applications that rely on network interfaces being names
> "ethX". Actually this goes in the direction of "fixing udev" which was
> what you asked for in your comment on my first patch concering vfeth. :)
>
> >
> > [...]
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index b626289..2ea0298 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -845,6 +845,7 @@ struct net_device {
> >> #define NETIF_F_FCOE_MTU (1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
> >> #define NETIF_F_NTUPLE (1 << 27) /* N-tuple filters supported */
> >> #define NETIF_F_RXHASH (1 << 28) /* Receive hashing offload */
> >> +#define NETIF_F_RNDMAC (1 << 29) /* Interface with random MAC address */
> > [...]
> >
> > This is not really a feature, and we are running out of real feature
> > bits. Can you find somewhere else to put this flag?
>
> Actually Dave Miller suggested to put it there. What other place is
> there to put it?

If Dave said that then I'm sure it's OK.

However, if you define this as an interface flag (net_device::flags;
<linux/if.h>) and add it to the set of changeable flags in
__dev_change_flags(), user-space will be able to clear the flag if it
later sets a stable address.

Ben.

--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Ben Hutchings on 20 Jul 2010 10:40
On Tue, 2010-07-20 at 14:41 +0200, Stefan Assmann wrote:
> On 20.07.2010 14:07, Ben Hutchings wrote:
> > On Tue, 2010-07-20 at 13:47 +0200, Stefan Assmann wrote:
> >> On 20.07.2010 13:20, Ben Hutchings wrote:
> >>> On Tue, 2010-07-20 at 12:50 +0200, Stefan Assmann wrote:
> >>>> From: Stefan Assmann <sassmann(a)redhat.com>
>
> [snip]
>
> >> Actually Dave Miller suggested to put it there. What other place is
> >> there to put it?
> >
> > If Dave said that then I'm sure it's OK.
> >
> > However, if you define this as an interface flag (net_device::flags;
> > <linux/if.h>) and add it to the set of changeable flags in
> > __dev_change_flags(), user-space will be able to clear the flag if it
> > later sets a stable address.
>
> As I said I'm not that knowledgeable about this MAC address stealing
> thing and I'm assuming that's what you're aiming at. Would you really
> want/need it to be user-space writable? Currently all I can think of is
> the scenario where you set a "stable" address that outlasts a reboot so
> udev might be able to assign it a permanent name after it gets stable.
>
> So it might make sense to have it writable, but I'd like to avoid to add
> unnecessary complexity that may cause errors if it's not necessary.
> Read-only is simple, just read the flag and deal with it.

Once this flag has been added, it may be used by any tool and not just
udev, so it ought to indicate the status of the currently assigned
address. That requires that it be writable.

> Btw, the driver itself could also alter the flag. Then we'd have a well
> defined way of setting a stable address.

The driver can't know whether an address assigned by the user is stable.

Ben.

--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 20 Jul 2010 16:20
From: Ben Hutchings <bhutchings(a)solarflare.com>
Date: Tue, 20 Jul 2010 15:29:54 +0100

> On Tue, 2010-07-20 at 14:41 +0200, Stefan Assmann wrote:
>> Btw, the driver itself could also alter the flag. Then we'd have a well
>> defined way of setting a stable address.
>
> The driver can't know whether an address assigned by the user is stable.

If userspace can somehow obtain a persistent address, it can kick
udev too.

I really don't see any real value provided by letting userspace mess
with this. Because the permanence communicated in this value is from
the perspective of the kernel driver, it's really therefore about the
thing that's in ->perm_addr[] not what happens to be in ->addr[] right
now.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Hemminger on 20 Jul 2010 17:20
On Tue, 20 Jul 2010 13:17:48 -0700 (PDT)
David Miller <davem(a)davemloft.net> wrote:

> From: Ben Hutchings <bhutchings(a)solarflare.com>
> Date: Tue, 20 Jul 2010 15:29:54 +0100
>
> > On Tue, 2010-07-20 at 14:41 +0200, Stefan Assmann wrote:
> >> Btw, the driver itself could also alter the flag. Then we'd have a well
> >> defined way of setting a stable address.
> >
> > The driver can't know whether an address assigned by the user is stable.
>
> If userspace can somehow obtain a persistent address, it can kick
> udev too.
>
> I really don't see any real value provided by letting userspace mess
> with this. Because the permanence communicated in this value is from
> the perspective of the kernel driver, it's really therefore about the
> thing that's in ->perm_addr[] not what happens to be in ->addr[] right
> now.

No one mentioned that the first octet of an Ethernet address already
indicates "software generated" Ethernet address. Per the standard,
if bit 1 is set it means address is locally assigned.

static inline bool is_locally_assigned_ether(const u8 *addr)
{
return (addr[0] & 0x2) != 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2 3 4
Prev: wm8960: fix compile error
Next: sysfs: add entry to indicate network interfaces with random MAC address