| Message ID | 20120705024446.26317.49693.stgit@build.warmcat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2012-07-05 at 10:44 +0800, Andy Green wrote: > From: Andy Green <andy@warmcat.com> > > This introduces a small helper in net/ethernet, which registers a network > notifier at core_initcall time, and accepts registrations mapping expected > asynchronously-probed network device paths (like, "usb1/1-1/1-1.1/1-1.1:1.0") > and the MAC that is needed to be assigned to the device when it appears. The mac prefix is poor. I think eth_mac is better. [] > diff --git a/net/ethernet/mac-platform.c b/net/ethernet/mac-platform.c [] > +static int mac_platform_netdev_event(struct notifier_block *this, > + unsigned long event, void *ptr) alignment to parenthesis please. [] > +int mac_platform_register_device_macs(const struct mac_platform *macs) > +{ [] > + next = kmalloc(sizeof(struct mac_platform), GFP_KERNEL); > + if (!next) { > + ret = -ENOMEM; > + goto bail; > + } > + > + next->device_path = kmalloc(strlen(macs->device_path) + 1, > + GFP_KERNEL); > + if (!next->device_path) { > + ret = -ENOMEM; > + goto bail; > + } > + > + strcpy(next->device_path, macs->device_path); > + memcpy(next->mac, macs->mac, sizeof macs->mac); kmemdup and kstrdup() > + list_add(&next->list, &mac_platform_list); > + > + macs++; > + } > + > +bail: > + mutex_unlock(&mac_platform_mutex); > + > + return ret; > +} leaking memory on failures. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/12 11:12, the mail apparently from Joe Perches included: Thanks for the comments. >> This introduces a small helper in net/ethernet, which registers a network >> notifier at core_initcall time, and accepts registrations mapping expected >> asynchronously-probed network device paths (like, "usb1/1-1/1-1.1/1-1.1:1.0") >> and the MAC that is needed to be assigned to the device when it appears. > > The mac prefix is poor. I think eth_mac is better. OK. >> diff --git a/net/ethernet/mac-platform.c b/net/ethernet/mac-platform.c > [] >> +static int mac_platform_netdev_event(struct notifier_block *this, >> + unsigned long event, void *ptr) > > alignment to parenthesis please. OK. Although different places in the kernel seem to have different expectations about that. >> +int mac_platform_register_device_macs(const struct mac_platform *macs) >> +{ > [] >> + next = kmalloc(sizeof(struct mac_platform), GFP_KERNEL); >> + if (!next) { >> + ret = -ENOMEM; >> + goto bail; >> + } >> + >> + next->device_path = kmalloc(strlen(macs->device_path) + 1, >> + GFP_KERNEL); >> + if (!next->device_path) { >> + ret = -ENOMEM; >> + goto bail; >> + } >> + >> + strcpy(next->device_path, macs->device_path); >> + memcpy(next->mac, macs->mac, sizeof macs->mac); > > kmemdup and kstrdup() OK >> + list_add(&next->list, &mac_platform_list); >> + >> + macs++; >> + } >> + >> +bail: >> + mutex_unlock(&mac_platform_mutex); >> + >> + return ret; >> +} > > leaking memory on failures. Right... I'll fix these and wait for more comments. Thanks again for the review. -Andy
On Thu, 2012-07-05 at 11:20 +0800, Andy Green wrote: > On 05/07/12 11:12, the mail apparently from Joe Perches included: [] > >> diff --git a/net/ethernet/mac-platform.c b/net/ethernet/mac-platform.c > > [] > >> +static int mac_platform_netdev_event(struct notifier_block *this, > >> + unsigned long event, void *ptr) > > > > alignment to parenthesis please. > > OK. Although different places in the kernel seem to have different > expectations about that. net and drivers/net is pretty consistent. Most of the exceptions are old code. Some of those exceptions are being slowly updated too. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-07-05 at 10:44 +0800, Andy Green wrote: [...] > To make use of this safely you also need to make sure that any drivers that > may compete for the bus ordinal you are using (eg, mUSB and ehci in Panda > case) are loaded in a deterministic order. [...] This seems very restrictive... would it be practical to also allow a driver name as a path component? [...] > --- /dev/null > +++ b/include/net/mac-platform.h > @@ -0,0 +1,39 @@ > +/* > + * mac-platform.h: Enforces platform-defined MAC for Async probed devices > + */ > + > +#ifndef __NET_MAC_PLATFORM_H__ > +#define __NET_MAC_PLATFORM_H__ > + > +#include <linux/if_ether.h> > + > +/** > + * struct mac_platform - associates asynchronously probed device path with > + * MAC address to be assigned to the device when it > + * is created A kernel-doc summary is strictly limited to one line. The longer explanation can go in a paragraph under the field descriptions. > + * @device_path: device path name of network device > + * @mac: MAC address to assign to network device matching device path > + * @list: can be left uninitialized when passing from platform > + */ > + > +struct mac_platform { > + char *device_path; > + u8 mac[ETH_ALEN]; > + struct list_head list; /* unused in platform data usage */ > +}; [...] > --- /dev/null > +++ b/net/ethernet/mac-platform.c [...] > +static struct mac_platform *__mac_platform_check(struct device *dev) > +{ > + const char *path; > + const char *p; > + const char *try; > + int len; > + struct device *devn; > + struct mac_platform *tmp; > + struct list_head *pos; > + > + list_for_each(pos, &mac_platform_list) { > + > + tmp = list_entry(pos, struct mac_platform, list); > + > + try = tmp->device_path; > + > + p = try + strlen(try); > + devn = dev; > + > + while (devn) { > + > + path = dev_name(devn); > + len = strlen(path); > + > + if ((p - try) < len) { > + devn = NULL; > + continue; > + } > + > + p -= len; [...] There are so many blank lines here, it's hard to see much code at once. Ben.
diff --git a/include/net/mac-platform.h b/include/net/mac-platform.h new file mode 100644 index 0000000..3a59098 --- /dev/null +++ b/include/net/mac-platform.h @@ -0,0 +1,39 @@ +/* + * mac-platform.h: Enforces platform-defined MAC for Async probed devices + */ + +#ifndef __NET_MAC_PLATFORM_H__ +#define __NET_MAC_PLATFORM_H__ + +#include <linux/if_ether.h> + +/** + * struct mac_platform - associates asynchronously probed device path with + * MAC address to be assigned to the device when it + * is created + * + * @device_path: device path name of network device + * @mac: MAC address to assign to network device matching device path + * @list: can be left uninitialized when passing from platform + */ + +struct mac_platform { + char *device_path; + u8 mac[ETH_ALEN]; + struct list_head list; /* unused in platform data usage */ +}; + +#ifdef CONFIG_NET +/** + * mac_platform_register_device_macs - add an array of device path to monitor + * and MAC to apply when the network device + * at the device path appears + * @macs: array of struct mac_platform terminated by entry with + * NULL device_path + */ +int mac_platform_register_device_macs(const struct mac_platform *macs); +#else +static inline int mac_platform_register_device_macs(macs) { return 0; } +#endif /* !CONFIG_NET */ + +#endif /* __NET_MAC_PLATFORM_H__ */ diff --git a/net/Kconfig b/net/Kconfig index 245831b..487c9e6 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -335,9 +335,12 @@ source "net/caif/Kconfig" source "net/ceph/Kconfig" source "net/nfc/Kconfig" - endif # if NET +# used by board / dt platform to enforce MACs for Async-probed devices +config MAC_PLATFORM + bool + # Used by archs to tell that they support BPF_JIT config HAVE_BPF_JIT bool diff --git a/net/ethernet/Makefile b/net/ethernet/Makefile index 7cef1d8..475db2b 100644 --- a/net/ethernet/Makefile +++ b/net/ethernet/Makefile @@ -5,3 +5,6 @@ obj-y += eth.o obj-$(subst m,y,$(CONFIG_IPX)) += pe2.o obj-$(subst m,y,$(CONFIG_ATALK)) += pe2.o +ifneq ($(CONFIG_NET),) +obj-$(CONFIG_MAC_PLATFORM) += mac-platform.o +endif diff --git a/net/ethernet/mac-platform.c b/net/ethernet/mac-platform.c new file mode 100644 index 0000000..88e50ce --- /dev/null +++ b/net/ethernet/mac-platform.c @@ -0,0 +1,151 @@ +/* + * Helper to allow platform code to enforce association of a locally- + * administered MAC address automatically on asynchronously probed devices, + * such as SDIO and USB based devices. + * + * Because the "device path" is used for matching, this is only useful for + * network assets physcally wired on the board, and also requires any + * different drivers that can compete for bus ordinals (eg mUSB vs ehci) to + * have fixed initialization ordering, eg, by having ehci in monolithic + * kernel + * + * Neither a driver nor a module as needs to be callable from machine file + * before the network devices are registered. + * + * (c) 2012 Andy Green <andy.green@linaro.org> + */ + +#include <linux/netdevice.h> +#include <net/mac-platform.h> + +static LIST_HEAD(mac_platform_list); +static DEFINE_MUTEX(mac_platform_mutex); + +static struct mac_platform *__mac_platform_check(struct device *dev) +{ + const char *path; + const char *p; + const char *try; + int len; + struct device *devn; + struct mac_platform *tmp; + struct list_head *pos; + + list_for_each(pos, &mac_platform_list) { + + tmp = list_entry(pos, struct mac_platform, list); + + try = tmp->device_path; + + p = try + strlen(try); + devn = dev; + + while (devn) { + + path = dev_name(devn); + len = strlen(path); + + if ((p - try) < len) { + devn = NULL; + continue; + } + + p -= len; + + if (strncmp(path, p, len)) { + devn = NULL; + continue; + } + + devn = devn->parent; + if (p == try) + return tmp; + + if (devn != NULL && (p - try) < 2) + devn = NULL; + + p--; + if (devn != NULL && *p != '/') + devn = NULL; + } + } + + return NULL; +} + +static int mac_platform_netdev_event(struct notifier_block *this, + unsigned long event, void *ptr) +{ + struct net_device *dev = ptr; + struct sockaddr sa; + struct mac_platform *match; + + if (event != NETDEV_REGISTER) + return NOTIFY_DONE; + + mutex_lock(&mac_platform_mutex); + + match = __mac_platform_check(dev->dev.parent); + if (match == NULL) + goto bail; + + sa.sa_family = dev->type; + memcpy(sa.sa_data, match->mac, sizeof match->mac); + dev->netdev_ops->ndo_set_mac_address(dev, &sa); + +bail: + mutex_unlock(&mac_platform_mutex); + + return NOTIFY_DONE; +} + +int mac_platform_register_device_macs(const struct mac_platform *macs) +{ + struct mac_platform *next; + int ret = 0; + + mutex_lock(&mac_platform_mutex); + + while (macs->device_path) { + + next = kmalloc(sizeof(struct mac_platform), GFP_KERNEL); + if (!next) { + ret = -ENOMEM; + goto bail; + } + + next->device_path = kmalloc(strlen(macs->device_path) + 1, + GFP_KERNEL); + if (!next->device_path) { + ret = -ENOMEM; + goto bail; + } + + strcpy(next->device_path, macs->device_path); + memcpy(next->mac, macs->mac, sizeof macs->mac); + list_add(&next->list, &mac_platform_list); + + macs++; + } + +bail: + mutex_unlock(&mac_platform_mutex); + + return ret; +} + +static struct notifier_block mac_platform_netdev_notifier = { + .notifier_call = mac_platform_netdev_event, + .priority = 1, +}; + +static int __init mac_platform_init(void) +{ + int ret = register_netdevice_notifier(&mac_platform_netdev_notifier); + if (ret) + pr_err("mac_platform_init: Notifier registration failed\n"); + + return ret; +} + +core_initcall(mac_platform_init);