[RFC] Make predictable/persistent network interface names more handy

23 messages, 6 authors, 2015-01-12 · open the first message on its own page

[RFC] Make predictable/persistent network interface names more handy

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 20:53:06

Since the Linux distribution of my choice makes use of
predictable network interface names[0] my USB gadgets
are no longer usb0 but enp0s29u1u2. Same for all other network devices.
While I can fully understand that this new naming scheme makes sense
for a lot of people and makes their work easier it does not really work for me.
My brain is not able to remember that my Beaglebone's USB-Ethernet
is now enp0s29u1u2. Even after looking at the output of ifconfig
I have to copy&paste the interface name.
Instead of just disabling the feature I thought about
a generic solution which satisfies both needs.

For block devices we also have predictable device names,
udev creates a symlink to the kernel device.
This works very good and reliable.
My idea is to use the network device alias as symlink.
Such that we can have both the easy to use kernel name and
the predictable/persistent name from udev.
systemd/udev could store the original kernel interface name
as alias after renaming the interface.
Existing users would not notice but one can still use the kernel name.
So I can use tcpdump -i usb0 _and_ tcpdump -i enp0s29u1u2.

This patch series implements my idea.
I'd love to get some feedback!

Patch 1/3 exposes the interfaces alias for general userspace usage, i.e. that ifconfig <alias> works.
Of course you can only use the first 15 chars of an alias.

In-kernel users of interface names need also an update, patch 2/3 updates x_tables.
I'm sure there is more todo, i.e. nftables.
We could also define that netfilter will never use the alias but this needs documented cleary.

Patch 3/3 is a cleanup in continuation of 2/3.

[0] http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/

Thanks,
//richard

git://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git netalias

[PATCH 1/3] net: Make interface aliases available for general usage
[PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
[PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

include/linux/netdevice.h | 1 +
include/linux/netfilter/x_tables.h | 41 +++++++++++++++++++++++++++---
include/net/net_namespace.h | 1 +
net/core/dev.c | 52 ++++++++++++++++++++++++++++++++++++++
net/ipv4/netfilter/arp_tables.c | 37 ++++-----------------------
net/ipv4/netfilter/ip_tables.c | 15 ++++-------
net/ipv6/netfilter/ip6_tables.c | 18 +++++--------
net/netfilter/xt_physdev.c | 9 ++-----
8 files changed, 110 insertions(+), 64 deletions(-)

[PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 20:53:16

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/netfilter/x_tables.h | 25 ++++++++++++++++++-------
 net/ipv4/netfilter/arp_tables.c    | 37 ++-----------------------------------
 2 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 15bda23..26dddc1 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h

@@ -331,14 +331,15 @@ static inline void xt_write_recseq_end(unsigned int addend)
 /*
  * This helper is performance critical and must be inlined
  */
-static inline unsigned long ifname_compare_aligned(const char *_a,
-						   const char *_b,
-						   const char *_mask)
+static inline unsigned long ifname_compare(const char *_a,
+					   const char *_b,
+					   const char *_mask)
 {
+	unsigned long ret;
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
 	const unsigned long *a = (const unsigned long *)_a;
 	const unsigned long *b = (const unsigned long *)_b;
 	const unsigned long *mask = (const unsigned long *)_mask;
-	unsigned long ret;
 
 	ret = (a[0] ^ b[0]) & mask[0];
 	if (IFNAMSIZ > sizeof(unsigned long))

@@ -348,11 +349,21 @@ static inline unsigned long ifname_compare_aligned(const char *_a,
 	if (IFNAMSIZ > 3 * sizeof(unsigned long))
 		ret |= (a[3] ^ b[3]) & mask[3];
 	BUILD_BUG_ON(IFNAMSIZ > 4 * sizeof(unsigned long));
+#else
+	const u16 *a = (const u16 *)_a;
+	const u16 *b = (const u16 *)_b;
+	const u16 *mask = (const u16 *)_mask;
+	int i;
+
+	ret = 0;
+	for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
+		ret |= (a[i] ^ b[i]) & mask[i];
+#endif
 	return ret;
 }
 
 /*
- * A wrapper around ifname_compare_aligned() to match against dev->name and
+ * A wrapper around ifname_compare() to match against dev->name and
  * dev->ifalias.
  */
 static inline unsigned long ifname_compare_all(const struct net_device *dev,

@@ -364,9 +375,9 @@ static inline unsigned long ifname_compare_all(const struct net_device *dev,
 	if (!dev)
 		goto out;
 
-	res = ifname_compare_aligned(dev->name, name, mask);
+	res = ifname_compare(dev->name, name, mask);
 	if (unlikely(dev->ifalias && res))
-		res = ifname_compare_aligned(dev->ifalias, name, mask);
+		res = ifname_compare(dev->ifalias, name, mask);
 
 out:
 	return res;

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 457d4ed..c978691 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c

@@ -76,39 +76,6 @@ static inline int arp_devaddr_compare(const struct arpt_devaddr_info *ap,
 	return ret != 0;
 }
 
-/*
- * Unfortunately, _b and _mask are not aligned to an int (or long int)
- * Some arches dont care, unrolling the loop is a win on them.
- * For other arches, we only have a 16bit alignement.
- */
-static unsigned long ifname_compare(const struct net_device *dev,
-				    const char *_b, const char *_mask)
-{
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-	unsigned long ret = ifname_compare_all(dev, _b, _mask);
-#else
-	unsigned long ret = 0;
-	const u16 *a = (const u16 *)dev->name;
-	const u16 *b = (const u16 *)_b;
-	const u16 *mask = (const u16 *)_mask;
-	int i;
-
-	for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
-		ret |= (a[i] ^ b[i]) & mask[i];
-
-	if (likely(!(dev->ifalias && ret)))
-		goto out;
-
-	ret = 0;
-	a = (const u16 *)dev->ifalias;
-	for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
-		ret |= (a[i] ^ b[i]) & mask[i];
-
-out:
-#endif
-	return ret;
-}
-
 /* Returns whether packet matches rule or not. */
 static inline int arp_packet_match(const struct arphdr *arphdr,
 				   struct net_device *dev,

@@ -192,7 +159,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
 	}
 
 	/* Look for ifname matches.  */
-	ret = ifname_compare(indev, arpinfo->iniface, arpinfo->iniface_mask);
+	ret = ifname_compare_all(indev, arpinfo->iniface, arpinfo->iniface_mask);
 
 	if (FWINV(ret != 0, ARPT_INV_VIA_IN)) {
 		dprintf("VIA in mismatch (%s vs %s).%s\n",

@@ -201,7 +168,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
 		return 0;
 	}
 
-	ret = ifname_compare(outdev, arpinfo->outiface, arpinfo->outiface_mask);
+	ret = ifname_compare_all(outdev, arpinfo->outiface, arpinfo->outiface_mask);
 
 	if (FWINV(ret != 0, ARPT_INV_VIA_OUT)) {
 		dprintf("VIA out mismatch (%s vs %s).%s\n",

-- 
1.8.4.5

[PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 20:53:42

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/netfilter/x_tables.h | 22 ++++++++++++++++++++++
 net/ipv4/netfilter/arp_tables.c    | 28 +++++++++++++++++-----------
 net/ipv4/netfilter/ip_tables.c     | 15 +++++----------
 net/ipv6/netfilter/ip6_tables.c    | 18 +++++++-----------
 net/netfilter/xt_physdev.c         |  9 ++-------
 5 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index a3e215b..15bda23 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h

@@ -351,6 +351,28 @@ static inline unsigned long ifname_compare_aligned(const char *_a,
 	return ret;
 }
 
+/*
+ * A wrapper around ifname_compare_aligned() to match against dev->name and
+ * dev->ifalias.
+ */
+static inline unsigned long ifname_compare_all(const struct net_device *dev,
+					       const char *name,
+					       const char *mask)
+{
+	unsigned long res = 0;
+
+	if (!dev)
+		goto out;
+
+	res = ifname_compare_aligned(dev->name, name, mask);
+	if (unlikely(dev->ifalias && res))
+		res = ifname_compare_aligned(dev->ifalias, name, mask);
+
+out:
+	return res;
+}
+
+
 struct nf_hook_ops *xt_hook_link(const struct xt_table *, nf_hookfn *);
 void xt_hook_unlink(const struct xt_table *, struct nf_hook_ops *);

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..457d4ed 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c

@@ -81,19 +81,30 @@ static inline int arp_devaddr_compare(const struct arpt_devaddr_info *ap,
  * Some arches dont care, unrolling the loop is a win on them.
  * For other arches, we only have a 16bit alignement.
  */
-static unsigned long ifname_compare(const char *_a, const char *_b, const char *_mask)
+static unsigned long ifname_compare(const struct net_device *dev,
+				    const char *_b, const char *_mask)
 {
 #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-	unsigned long ret = ifname_compare_aligned(_a, _b, _mask);
+	unsigned long ret = ifname_compare_all(dev, _b, _mask);
 #else
 	unsigned long ret = 0;
-	const u16 *a = (const u16 *)_a;
+	const u16 *a = (const u16 *)dev->name;
 	const u16 *b = (const u16 *)_b;
 	const u16 *mask = (const u16 *)_mask;
 	int i;
 
 	for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
 		ret |= (a[i] ^ b[i]) & mask[i];
+
+	if (likely(!(dev->ifalias && ret)))
+		goto out;
+
+	ret = 0;
+	a = (const u16 *)dev->ifalias;
+	for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
+		ret |= (a[i] ^ b[i]) & mask[i];
+
+out:
 #endif
 	return ret;
 }

@@ -101,8 +112,8 @@ static unsigned long ifname_compare(const char *_a, const char *_b, const char *
 /* Returns whether packet matches rule or not. */
 static inline int arp_packet_match(const struct arphdr *arphdr,
 				   struct net_device *dev,
-				   const char *indev,
-				   const char *outdev,
+				   const struct net_device *indev,
+				   const struct net_device *outdev,
 				   const struct arpt_arp *arpinfo)
 {
 	const char *arpptr = (char *)(arphdr + 1);

@@ -252,11 +263,9 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 			   const struct net_device *out,
 			   struct xt_table *table)
 {
-	static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
 	unsigned int verdict = NF_DROP;
 	const struct arphdr *arp;
 	struct arpt_entry *e, *back;
-	const char *indev, *outdev;
 	void *table_base;
 	const struct xt_table_info *private;
 	struct xt_action_param acpar;

@@ -265,9 +274,6 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 	if (!pskb_may_pull(skb, arp_hdr_len(skb->dev)))
 		return NF_DROP;
 
-	indev = in ? in->name : nulldevname;
-	outdev = out ? out->name : nulldevname;
-
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
 	private = table->private;

@@ -291,7 +297,7 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 	do {
 		const struct xt_entry_target *t;
 
-		if (!arp_packet_match(arp, skb->dev, indev, outdev, &e->arp)) {
+		if (!arp_packet_match(arp, skb->dev, in, out, &e->arp)) {
 			e = arpt_next_entry(e);
 			continue;
 		}

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 99e810f..87df9ef 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c

@@ -73,8 +73,8 @@ EXPORT_SYMBOL_GPL(ipt_alloc_initial_table);
 /* Performance critical - called for every packet */
 static inline bool
 ip_packet_match(const struct iphdr *ip,
-		const char *indev,
-		const char *outdev,
+		const struct net_device *indev,
+		const struct net_device *outdev,
 		const struct ipt_ip *ipinfo,
 		int isfrag)
 {

@@ -97,7 +97,7 @@ ip_packet_match(const struct iphdr *ip,
 		return false;
 	}
 
-	ret = ifname_compare_aligned(indev, ipinfo->iniface, ipinfo->iniface_mask);
+	ret = ifname_compare_all(indev, ipinfo->iniface, ipinfo->iniface_mask);
 
 	if (FWINV(ret != 0, IPT_INV_VIA_IN)) {
 		dprintf("VIA in mismatch (%s vs %s).%s\n",

@@ -106,7 +106,7 @@ ip_packet_match(const struct iphdr *ip,
 		return false;
 	}
 
-	ret = ifname_compare_aligned(outdev, ipinfo->outiface, ipinfo->outiface_mask);
+	ret = ifname_compare_all(outdev, ipinfo->outiface, ipinfo->outiface_mask);
 
 	if (FWINV(ret != 0, IPT_INV_VIA_OUT)) {
 		dprintf("VIA out mismatch (%s vs %s).%s\n",

@@ -292,11 +292,9 @@ ipt_do_table(struct sk_buff *skb,
 	     const struct net_device *out,
 	     struct xt_table *table)
 {
-	static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
 	const struct iphdr *ip;
 	/* Initializing verdict to NF_DROP keeps gcc happy. */
 	unsigned int verdict = NF_DROP;
-	const char *indev, *outdev;
 	const void *table_base;
 	struct ipt_entry *e, **jumpstack;
 	unsigned int *stackptr, origptr, cpu;

@@ -306,8 +304,6 @@ ipt_do_table(struct sk_buff *skb,
 
 	/* Initialization */
 	ip = ip_hdr(skb);
-	indev = in ? in->name : nulldevname;
-	outdev = out ? out->name : nulldevname;
 	/* We handle fragments by dealing with the first fragment as
 	 * if it was a normal packet.  All other fragments are treated
 	 * normally, except that they will NEVER match rules that ask

@@ -348,8 +344,7 @@ ipt_do_table(struct sk_buff *skb,
 		const struct xt_entry_match *ematch;
 
 		IP_NF_ASSERT(e);
-		if (!ip_packet_match(ip, indev, outdev,
-		    &e->ip, acpar.fragoff)) {
+		if (!ip_packet_match(ip, in, out, &e->ip, acpar.fragoff)) {
  no_match:
 			e = ipt_next_entry(e);
 			continue;

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e080fbb..9ed5d70 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c

@@ -83,8 +83,8 @@ EXPORT_SYMBOL_GPL(ip6t_alloc_initial_table);
 /* Performance critical - called for every packet */
 static inline bool
 ip6_packet_match(const struct sk_buff *skb,
-		 const char *indev,
-		 const char *outdev,
+		 const struct net_device *indev,
+		 const struct net_device *outdev,
 		 const struct ip6t_ip6 *ip6info,
 		 unsigned int *protoff,
 		 int *fragoff, bool *hotdrop)

@@ -109,7 +109,7 @@ ip6_packet_match(const struct sk_buff *skb,
 		return false;
 	}
 
-	ret = ifname_compare_aligned(indev, ip6info->iniface, ip6info->iniface_mask);
+	ret = ifname_compare_all(indev, ip6info->iniface, ip6info->iniface_mask);
 
 	if (FWINV(ret != 0, IP6T_INV_VIA_IN)) {
 		dprintf("VIA in mismatch (%s vs %s).%s\n",

@@ -118,7 +118,7 @@ ip6_packet_match(const struct sk_buff *skb,
 		return false;
 	}
 
-	ret = ifname_compare_aligned(outdev, ip6info->outiface, ip6info->outiface_mask);
+	ret = ifname_compare_all(outdev, ip6info->outiface, ip6info->outiface_mask);
 
 	if (FWINV(ret != 0, IP6T_INV_VIA_OUT)) {
 		dprintf("VIA out mismatch (%s vs %s).%s\n",

@@ -318,10 +318,8 @@ ip6t_do_table(struct sk_buff *skb,
 	      const struct net_device *out,
 	      struct xt_table *table)
 {
-	static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
 	/* Initializing verdict to NF_DROP keeps gcc happy. */
 	unsigned int verdict = NF_DROP;
-	const char *indev, *outdev;
 	const void *table_base;
 	struct ip6t_entry *e, **jumpstack;
 	unsigned int *stackptr, origptr, cpu;

@@ -329,10 +327,8 @@ ip6t_do_table(struct sk_buff *skb,
 	struct xt_action_param acpar;
 	unsigned int addend;
 
-	/* Initialization */
-	indev = in ? in->name : nulldevname;
-	outdev = out ? out->name : nulldevname;
-	/* We handle fragments by dealing with the first fragment as
+	/* Initialization:
+	 * We handle fragments by dealing with the first fragment as
 	 * if it was a normal packet.  All other fragments are treated
 	 * normally, except that they will NEVER match rules that ask
 	 * things we don't know, ie. tcp syn flag or ports).  If the

@@ -368,7 +364,7 @@ ip6t_do_table(struct sk_buff *skb,
 
 		IP_NF_ASSERT(e);
 		acpar.thoff = 0;
-		if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
+		if (!ip6_packet_match(skb, in, out, &e->ipv6,
 		    &acpar.thoff, &acpar.fragoff, &acpar.hotdrop)) {
  no_match:
 			e = ip6t_next_entry(e);

diff --git a/net/netfilter/xt_physdev.c b/net/netfilter/xt_physdev.c
index f440f57..8d2ee7d 100644
--- a/net/netfilter/xt_physdev.c
+++ b/net/netfilter/xt_physdev.c

@@ -25,10 +25,8 @@ MODULE_ALIAS("ip6t_physdev");
 static bool
 physdev_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
-	static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
 	const struct xt_physdev_info *info = par->matchinfo;
 	unsigned long ret;
-	const char *indev, *outdev;
 	const struct nf_bridge_info *nf_bridge;
 
 	/* Not a bridged IP packet or no info available yet:

@@ -68,8 +66,7 @@ physdev_mt(const struct sk_buff *skb, struct xt_action_param *par)
 
 	if (!(info->bitmask & XT_PHYSDEV_OP_IN))
 		goto match_outdev;
-	indev = nf_bridge->physindev ? nf_bridge->physindev->name : nulldevname;
-	ret = ifname_compare_aligned(indev, info->physindev, info->in_mask);
+	ret = ifname_compare_all(nf_bridge->physindev, info->physindev, info->in_mask);
 
 	if (!ret ^ !(info->invert & XT_PHYSDEV_OP_IN))
 		return false;

@@ -77,9 +74,7 @@ physdev_mt(const struct sk_buff *skb, struct xt_action_param *par)
 match_outdev:
 	if (!(info->bitmask & XT_PHYSDEV_OP_OUT))
 		return true;
-	outdev = nf_bridge->physoutdev ?
-		 nf_bridge->physoutdev->name : nulldevname;
-	ret = ifname_compare_aligned(outdev, info->physoutdev, info->out_mask);
+	ret = ifname_compare_all(nf_bridge->physoutdev, info->physoutdev, info->out_mask);
 
 	return (!!ret ^ !(info->invert & XT_PHYSDEV_OP_OUT));
 }

-- 
1.8.4.5

[PATCH 1/3] net: Make interface aliases available for general usage

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 20:54:05

Allow interface aliases to be used as regular interfaces.
Such that a command sequence like this one works:
$ ip l set eth0 alias internet
$ ip a s internet
$ tcpdump -n -i internet

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/netdevice.h   |  1 +
 include/net/net_namespace.h |  1 +
 net/core/dev.c              | 52 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 679e6e9..e00b4e2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -1493,6 +1493,7 @@ struct net_device {
 	char			name[IFNAMSIZ];
 	struct hlist_node	name_hlist;
 	char 			*ifalias;
+	struct hlist_node	ifalias_hlist;
 	/*
 	 *	I/O specific fields
 	 *	FIXME: Merge these and struct ifmap into one

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 2e8756b8..9fa0939 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h

@@ -76,6 +76,7 @@ struct net {
 	struct list_head 	dev_base_head;
 	struct hlist_head 	*dev_name_head;
 	struct hlist_head	*dev_index_head;
+	struct hlist_head	*dev_ifalias_head;
 	unsigned int		dev_base_seq;	/* protected by rtnl_mutex */
 	int			ifindex;
 	unsigned int		dev_unreg_count;

diff --git a/net/core/dev.c b/net/core/dev.c
index 683d493..2551b03 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -202,6 +202,14 @@ static inline struct hlist_head *dev_index_hash(struct net *net, int ifindex)
 	return &net->dev_index_head[ifindex & (NETDEV_HASHENTRIES - 1)];
 }
 
+static inline struct hlist_head *dev_ifalias_hash(struct net *net,
+						  const char *ifalias)
+{
+	unsigned int hash = full_name_hash(ifalias, strnlen(ifalias, IFALIASZ));
+
+	return &net->dev_ifalias_head[hash_32(hash, NETDEV_HASHBITS)];
+}
+
 static inline void rps_lock(struct softnet_data *sd)
 {
 #ifdef CONFIG_RPS

@@ -228,6 +236,9 @@ static void list_netdevice(struct net_device *dev)
 	hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
 	hlist_add_head_rcu(&dev->index_hlist,
 			   dev_index_hash(net, dev->ifindex));
+	if (dev->ifalias)
+		hlist_add_head_rcu(&dev->ifalias_hlist,
+			   dev_ifalias_hash(net, dev->ifalias));
 	write_unlock_bh(&dev_base_lock);
 
 	dev_base_seq_inc(net);

@@ -245,6 +256,8 @@ static void unlist_netdevice(struct net_device *dev)
 	list_del_rcu(&dev->dev_list);
 	hlist_del_rcu(&dev->name_hlist);
 	hlist_del_rcu(&dev->index_hlist);
+	if (dev->ifalias)
+		hlist_del_rcu(&dev->ifalias_hlist);
 	write_unlock_bh(&dev_base_lock);
 
 	dev_base_seq_inc(dev_net(dev));

@@ -679,6 +692,11 @@ struct net_device *__dev_get_by_name(struct net *net, const char *name)
 		if (!strncmp(dev->name, name, IFNAMSIZ))
 			return dev;
 
+	head = dev_ifalias_hash(net, name);
+	hlist_for_each_entry(dev, head, ifalias_hlist)
+		if (dev->ifalias && !strncmp(dev->ifalias, name, IFALIASZ))
+			return dev;
+
 	return NULL;
 }
 EXPORT_SYMBOL(__dev_get_by_name);

@@ -704,6 +722,11 @@ struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
 		if (!strncmp(dev->name, name, IFNAMSIZ))
 			return dev;
 
+	head = dev_ifalias_hash(net, name);
+	hlist_for_each_entry_rcu(dev, head, ifalias_hlist)
+		if (dev->ifalias && !strncmp(dev->ifalias, name, IFALIASZ))
+			return dev;
+
 	return NULL;
 }
 EXPORT_SYMBOL(dev_get_by_name_rcu);

@@ -1169,6 +1192,20 @@ rollback:
 	return err;
 }
 
+static void __hlist_del_alias(struct net_device *dev)
+{
+	write_lock_bh(&dev_base_lock);
+	hlist_del_rcu(&dev->ifalias_hlist);
+	write_unlock_bh(&dev_base_lock);
+}
+
+static void __hlist_add_alias(struct net_device *dev)
+{
+	write_lock_bh(&dev_base_lock);
+	hlist_add_head_rcu(&dev->ifalias_hlist, dev_ifalias_hash(dev_net(dev), dev->ifalias));
+	write_unlock_bh(&dev_base_lock);
+}
+
 /**
  *	dev_set_alias - change ifalias of a device
  *	@dev: device

@@ -1189,15 +1226,24 @@ int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
 	if (!len) {
 		kfree(dev->ifalias);
 		dev->ifalias = NULL;
+		__hlist_del_alias(dev);
 		return 0;
 	}
 
 	new_ifalias = krealloc(dev->ifalias, len + 1, GFP_KERNEL);
 	if (!new_ifalias)
 		return -ENOMEM;
+
+	if (dev->ifalias) {
+		__hlist_del_alias(dev);
+		synchronize_rcu();
+	}
+
 	dev->ifalias = new_ifalias;
 
 	strlcpy(dev->ifalias, alias, len+1);
+	__hlist_add_alias(dev);
+
 	return len;
 }

@@ -7150,8 +7196,14 @@ static int __net_init netdev_init(struct net *net)
 	if (net->dev_index_head == NULL)
 		goto err_idx;
 
+	net->dev_ifalias_head = netdev_create_hash();
+	if (net->dev_ifalias_head == NULL)
+		goto err_alias;
+
 	return 0;
 
+err_alias:
+	kfree(net->dev_index_head);
 err_idx:
 	kfree(net->dev_name_head);
 err_name:

-- 
1.8.4.5

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Joe Perches <joe@perches.com>
Date: 2015-01-11 20:59:39

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

[]

quoted hunk

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h

[]

quoted hunk

@@ -331,14 +331,15 @@ static inline void xt_write_recseq_end(unsigned int addend)
 /*
  * This helper is performance critical and must be inlined
  */
-static inline unsigned long ifname_compare_aligned(const char *_a,
-						   const char *_b,
-						   const char *_mask)
+static inline unsigned long ifname_compare(const char *_a,
+					   const char *_b,
+					   const char *_mask)

Perhaps this would be better as bool ifname_compare

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 21:02:28

Am 11.01.2015 um 21:59 schrieb Joe Perches:

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

quoted

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

[]

quoted

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h

[]

quoted

@@ -331,14 +331,15 @@ static inline void xt_write_recseq_end(unsigned int addend)
 /*
  * This helper is performance critical and must be inlined
  */
-static inline unsigned long ifname_compare_aligned(const char *_a,
-						   const char *_b,
-						   const char *_mask)
+static inline unsigned long ifname_compare(const char *_a,
+					   const char *_b,
+					   const char *_mask)

Perhaps this would be better as bool ifname_compare

Let's discuss the whole concept first, then we can go to bikeshedding mode.

Thanks,
//richard

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Joe Perches <joe@perches.com>
Date: 2015-01-11 21:14:24

On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:

Am 11.01.2015 um 21:59 schrieb Joe Perches:

quoted

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

quoted

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

[]

quoted

Perhaps this would be better as bool ifname_compare

Let's discuss the whole concept first

The concept seems obvious enough

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 21:30:49

Am 11.01.2015 um 22:14 schrieb Joe Perches:

On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:

quoted

Am 11.01.2015 um 21:59 schrieb Joe Perches:

quoted

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

quoted

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

[]

quoted

Perhaps this would be better as bool ifname_compare

Let's discuss the whole concept first

The concept seems obvious enough

Anyway, I agree with Linus wrt. bool.
https://lkml.org/lkml/2013/8/31/138

Thanks,
//richard

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Joe Perches <joe@perches.com>
Date: 2015-01-11 21:39:45

On Sun, 2015-01-11 at 22:30 +0100, Richard Weinberger wrote:

Am 11.01.2015 um 22:14 schrieb Joe Perches:

quoted

On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:

quoted

Am 11.01.2015 um 21:59 schrieb Joe Perches:

quoted

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

quoted

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

[]

quoted

Perhaps this would be better as bool ifname_compare

Let's discuss the whole concept first

The concept seems obvious enough

Anyway, I agree with Linus wrt. bool.
https://lkml.org/lkml/2013/8/31/138

I don't.  He was right when he wrote:

https://lkml.org/lkml/2014/3/10/760

Linus Torvalds <>
I guess I haven't gotten over my hatred of people playing games with
them because support wasn't universal enough. But maybe it's
approaching being irrational these days.

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 21:42:44

Am 11.01.2015 um 22:39 schrieb Joe Perches:

On Sun, 2015-01-11 at 22:30 +0100, Richard Weinberger wrote:

quoted

Am 11.01.2015 um 22:14 schrieb Joe Perches:

quoted

On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:

quoted

Am 11.01.2015 um 21:59 schrieb Joe Perches:

quoted

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

quoted

arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.

[]

quoted

Perhaps this would be better as bool ifname_compare

Let's discuss the whole concept first

The concept seems obvious enough

Anyway, I agree with Linus wrt. bool.
https://lkml.org/lkml/2013/8/31/138

I don't.  He was right when he wrote:

Joe, I really don't care. This is the least significant
patch of the series.
I'll no longer waste my time with that.

Thanks,
//richard

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Jan Engelhardt <hidden>
Date: 2015-01-11 22:23:07

On Sunday 2015-01-11 22:30, Richard Weinberger wrote:

quoted

Perhaps this would be better as bool ifname_compare

Anyway, I agree with Linus wrt. bool.
https://lkml.org/lkml/2013/8/31/138

Had the function return "bool", it would have been obvious enough
what to do with its return type. A return type of "int" might have
hinted towards negative-is-error (in general) or strcmpish values
(functions doing string compare work).

Now that it returns "unsigned long", one is pressed to look at the 
function body (not bad per se, but it is a hump) for the return 
value's semantics.

Linus says bool is dangerous to the unsuspecting user — but so is
"volatile", microwave ovens, etc. If the kernel really cared for
entry-level coders, it would be written in something like MISRA C.

Re: [PATCH 1/3] net: Make interface aliases available for general usage

From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2015-01-11 22:40:58

On Sun, 11 Jan 2015 21:52:49 +0100
Richard Weinberger [off-list ref] wrote:

Allow interface aliases to be used as regular interfaces.
Such that a command sequence like this one works:
$ ip l set eth0 alias internet
$ ip a s internet
$ tcpdump -n -i internet

Signed-off-by: Richard Weinberger <richard@nod.at>

There is already a ifalias and it is used by SNMP.
But the common practice is to put longer descriptive names which aren't going
to be usable and there is no requirement that they be unique.

I think you can't do this without breaking some of our users.

Re: [RFC] Make predictable/persistent network interface names more handy

From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2015-01-11 22:42:53

On Sun, 11 Jan 2015 21:52:48 +0100
Richard Weinberger [off-list ref] wrote:

Since the Linux distribution of my choice makes use of
predictable network interface names[0] my USB gadgets
are no longer usb0 but enp0s29u1u2. Same for all other network devices.
While I can fully understand that this new naming scheme makes sense
for a lot of people and makes their work easier it does not really work for me.
My brain is not able to remember that my Beaglebone's USB-Ethernet
is now enp0s29u1u2. Even after looking at the output of ifconfig
I have to copy&paste the interface name.
Instead of just disabling the feature I thought about
a generic solution which satisfies both needs.

Fix your distro. Easier said than done.
Current systemd names are much shorter

P.s:  ifconfig is deprecated.

Re: [PATCH 1/3] net: Make interface aliases available for general usage

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 22:43:58

Stephen,

Am 11.01.2015 um 23:40 schrieb Stephen Hemminger:

On Sun, 11 Jan 2015 21:52:49 +0100
Richard Weinberger [off-list ref] wrote:

quoted

Allow interface aliases to be used as regular interfaces.
Such that a command sequence like this one works:
$ ip l set eth0 alias internet
$ ip a s internet
$ tcpdump -n -i internet

Signed-off-by: Richard Weinberger <richard@nod.at>

There is already a ifalias and it is used by SNMP.
But the common practice is to put longer descriptive names which aren't going
to be usable and there is no requirement that they be unique.

Actually I'm using ifalias. This patch just exposes it.

I think you can't do this without breaking some of our users.

My idea was that udev will not set the alias if already one is used.

Thanks,
//richard

Re: [RFC] Make predictable/persistent network interface names more handy

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 22:47:20

Am 11.01.2015 um 23:42 schrieb Stephen Hemminger:

On Sun, 11 Jan 2015 21:52:48 +0100
Richard Weinberger [off-list ref] wrote:

quoted

Since the Linux distribution of my choice makes use of
predictable network interface names[0] my USB gadgets
are no longer usb0 but enp0s29u1u2. Same for all other network devices.
While I can fully understand that this new naming scheme makes sense
for a lot of people and makes their work easier it does not really work for me.
My brain is not able to remember that my Beaglebone's USB-Ethernet
is now enp0s29u1u2. Even after looking at the output of ifconfig
I have to copy&paste the interface name.
Instead of just disabling the feature I thought about
a generic solution which satisfies both needs.

Fix your distro. Easier said than done.

Sorry for using a mainstream distro. :-)

Current systemd names are much shorter

Hmm, are you sure? When was this changed?
Maybe I misread http://cgit.freedesktop.org/systemd/systemd/tree/src/udev/udev-builtin-net_id.c

Thanks,
//richard

Re: [RFC] Make predictable/persistent network interface names more handy

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-11 22:51:35

Am 11.01.2015 um 23:42 schrieb Stephen Hemminger:

P.s:  ifconfig is deprecated.

ifconfig was just an example. It will work if iproute2 too.

Thanks,
//richard

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: David Miller <davem@davemloft.net>
Date: 2015-01-12 02:50:59

From: Richard Weinberger <richard@nod.at>
Date: Sun, 11 Jan 2015 22:42:37 +0100

Joe, I really don't care. This is the least significant
patch of the series.
I'll no longer waste my time with that.

If you're not willing to fix stylistic issues now, then nobody should
bother wasting their time on the high level issues of your patch.

Just fix these things now rather than being difficult, this is a part
of patch review that everyone has to do, not just you.

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-12 08:18:44

Am 12.01.2015 um 03:50 schrieb David Miller:

From: Richard Weinberger <richard@nod.at>
Date: Sun, 11 Jan 2015 22:42:37 +0100

quoted

Joe, I really don't care. This is the least significant
patch of the series.
I'll no longer waste my time with that.

If you're not willing to fix stylistic issues now, then nobody should
bother wasting their time on the high level issues of your patch.

Just fix these things now rather than being difficult, this is a part
of patch review that everyone has to do, not just you.

I apologize, it was not my intention to be difficult.
Please note that the stylistic issue is not a warning produced
by checkpatch.pl. If you and netfilter folks now prefer bool
for such string compare functions I'll happily address this in
v2 of my series.

Thanks,
//richard

Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()

From: Joe Perches <joe@perches.com>
Date: 2015-01-12 08:41:01

On Mon, 2015-01-12 at 09:18 +0100, Richard Weinberger wrote:

Am 12.01.2015 um 03:50 schrieb David Miller:

quoted

From: Richard Weinberger <richard@nod.at>
Date: Sun, 11 Jan 2015 22:42:37 +0100

quoted

Joe, I really don't care. This is the least significant
patch of the series.
I'll no longer waste my time with that.

If you're not willing to fix stylistic issues now, then nobody should
bother wasting their time on the high level issues of your patch.

Just fix these things now rather than being difficult, this is a part
of patch review that everyone has to do, not just you.

I apologize, it was not my intention to be difficult.

No worries.

The unsigned long return is kind of odd with a
compare_<foo> name as those are generally, as Jan
mentioned, signed comparison style return values.

I'd probably use a different function name too

bool ifname_equal(const char *a, const char *b, const char *mask)
{
}

to try to make the return value more obvious too.

If you and netfilter folks now prefer bool
for such string compare functions I'll happily address this in
v2 of my series.

Thanks

Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

From: Eric Dumazet <hidden>
Date: 2015-01-12 16:04:31

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/netfilter/x_tables.h | 22 ++++++++++++++++++++++
 net/ipv4/netfilter/arp_tables.c    | 28 +++++++++++++++++-----------
 net/ipv4/netfilter/ip_tables.c     | 15 +++++----------
 net/ipv6/netfilter/ip6_tables.c    | 18 +++++++-----------
 net/netfilter/xt_physdev.c         |  9 ++-------
 5 files changed, 53 insertions(+), 39 deletions(-)

Richard, I dislike this, sorry.

iptables is already horribly expensive, you add another expensive step
for every rule.

device aliasing can be done from user space.

iptables should have used ifindex, its sad we allowed the substring
match in first place.

Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

From: Richard Weinberger <richard@nod.at>
Date: 2015-01-12 16:12:43

Am 12.01.2015 um 17:04 schrieb Eric Dumazet:

On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:

quoted

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/netfilter/x_tables.h | 22 ++++++++++++++++++++++
 net/ipv4/netfilter/arp_tables.c    | 28 +++++++++++++++++-----------
 net/ipv4/netfilter/ip_tables.c     | 15 +++++----------
 net/ipv6/netfilter/ip6_tables.c    | 18 +++++++-----------
 net/netfilter/xt_physdev.c         |  9 ++-------
 5 files changed, 53 insertions(+), 39 deletions(-)

Richard, I dislike this, sorry.

That's fine, the series carries the "RFC" burning mark for a reason.

iptables is already horribly expensive, you add another expensive step
for every rule.

Yeah, you mean the extra unlikey() check?

device aliasing can be done from user space.

How?

I did this series because I'm not so happy with the device renaming what udev
does.
The idea was to offer udev a better kernel interface to deal with aliases.
Such that one can use the regular names form the kernel and the predictable
names generated from udev.

For block devices it was easy, we have the good old symlink.
For network interface the kernel does not offer an API.

iptables should have used ifindex, its sad we allowed the substring
match in first place.

Maybe nftables can do better. :-)

Thanks,
//richard

Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

From: Jan Engelhardt <hidden>
Date: 2015-01-12 16:32:59

On Monday 2015-01-12 17:04, Eric Dumazet wrote:

iptables should have used ifindex [for interface matching],
it[']s sad we allowed the substring match in first place.

How would you solve interface name wildcards with ifindices?
(They come in handy if you have something like lots of tun+/veth+
interfaces from openvpn/lxc.)

Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching

From: Eric Dumazet <hidden>
Date: 2015-01-12 16:46:16

On Mon, 2015-01-12 at 17:32 +0100, Jan Engelhardt wrote:

On Monday 2015-01-12 17:04, Eric Dumazet wrote:

quoted

iptables should have used ifindex [for interface matching],
it[']s sad we allowed the substring match in first place.

How would you solve interface name wildcards with ifindices?
(They come in handy if you have something like lots of tun+/veth+
interfaces from openvpn/lxc.)


This is what I said : "it[']s sad we allowed the substring match in
first place."

This obviously referred to wildcards, in the in/out interface match for
every _single_ rule, consuming 64 bytes of memory per rule and per cpu !
Which is absolutely crazy in term of memory usage.

Matching tun+ or whatever could easily be done by a match (-m ...),
because you can factorize this quite easily (called once for a group of
rules)

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help