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

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

[Richard Weinberger]

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 1/3] net: Make interface aliases available for general usage

[Richard Weinberger]

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

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

[Richard Weinberger]

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 3/3] x_tables: Factor out 16bit aligment ifname_compare()

[Richard Weinberger]

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

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

[Joe Perches]

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.
[]
> diff --git 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)

Perhaps this would be better as bool ifname_compare

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

[Richard Weinberger]

Am 11.01.2015 um 21:59 schrieb Joe Perches:
> 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.
> []
>> diff --git 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)
> 
> 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()

[Joe Perches]

On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
> Am 11.01.2015 um 21:59 schrieb Joe Perches:
> > 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.
[]
> > 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()

[Richard Weinberger]

Am 11.01.2015 um 22:14 schrieb Joe Perches:
> On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
>> Am 11.01.2015 um 21:59 schrieb Joe Perches:
>>> 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.
> []
>>> 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()

[Joe Perches]

On Sun, 2015-01-11 at 22:30 +0100, Richard Weinberger wrote:
> Am 11.01.2015 um 22:14 schrieb Joe Perches:
> > On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
> >> Am 11.01.2015 um 21:59 schrieb Joe Perches:
> >>> 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.
> > []
> >>> 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()

[Richard Weinberger]

Am 11.01.2015 um 22:39 schrieb Joe Perches:
> On Sun, 2015-01-11 at 22:30 +0100, Richard Weinberger wrote:
>> Am 11.01.2015 um 22:14 schrieb Joe Perches:
>>> On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
>>>> Am 11.01.2015 um 21:59 schrieb Joe Perches:
>>>>> 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.
>>> []
>>>>> 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()

[Jan Engelhardt]

On Sunday 2015-01-11 22:30, Richard Weinberger wrote:
>>>> 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 3/3] x_tables: Factor out 16bit aligment ifname_compare()

[Jan Engelhardt]

On Sunday 2015-01-11 22:30, Richard Weinberger wrote:
>>>> 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

[Stephen Hemminger]

On Sun, 11 Jan 2015 21:52:49 +0100
Richard Weinberger <richard@nod.at> 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

[Stephen Hemminger]

On Sun, 11 Jan 2015 21:52:48 +0100
Richard Weinberger <richard@nod.at> 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

[Richard Weinberger]

Stephen,

Am 11.01.2015 um 23:40 schrieb Stephen Hemminger:
> On Sun, 11 Jan 2015 21:52:49 +0100
> Richard Weinberger <richard@nod.at> 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.

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

[Richard Weinberger]

Am 11.01.2015 um 23:42 schrieb Stephen Hemminger:
> On Sun, 11 Jan 2015 21:52:48 +0100
> Richard Weinberger <richard@nod.at> 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.

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

[Richard Weinberger]

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()

[David Miller]

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()

[Richard Weinberger]

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
> 
>> 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()

[Joe Perches]

On Mon, 2015-01-12 at 09:18 +0100, Richard Weinberger wrote:
> 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
> > 
> >> 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

[Eric Dumazet]

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

[Richard Weinberger]

Am 12.01.2015 um 17:04 schrieb Eric Dumazet:
> 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.

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

[Jan Engelhardt]

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

[Eric Dumazet]

On Mon, 2015-01-12 at 17:32 +0100, Jan Engelhardt wrote:
> 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.)


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)

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

[Eric Dumazet]

On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
> > iptables should have used ifindex, its sad we allowed the substring
> 
> > match in first place.
> 
>  
> 
> Not to comment on the ifalias thing, which I think is unneccessary,
> too, but matching on interface names instead of only ifindex, is
> definitely needed, so that one can establish a full ruleset before
> interfaces even exist. That's good practise at boottime, but also
> needed for dynamic interface creation during runtime.
> 
>  
> 
> A pure ifindex-during-packet-inspection approach might still work, but
> the ruleset must IMO keep the interface names. Maybe register them in
> a hash, keyed by name, with values an ifindex or ifindex set (for
> wildcard names), plus a reverse mapping from active ifindices to all
> places in these hash values where an ifindex has been remembered. On
> interface creation / destruction that structure could then be updated,
> and active packet filtering rules would refer to (and keep a refcount
> on) specific hash elements.
> 
Please do not send html messages : Your reply did not reach the lists.

Then, all you mention could have been solved by proper userspace
support.

Every time you add an interface or change device name, you could change
firewalls rules if needed. Nothing shocking here.

The ruleset can indeed mention interface names, but the kernel part
really should not care about names, which are a 'human' convenient way
to represent things.

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

[Patrick Schaaf]

On Monday 12 January 2015 08:51:54 Eric Dumazet wrote:
> On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
> > 
> > Not to comment on the ifalias thing, which I think is unneccessary,
> > too, but matching on interface names instead of only ifindex, is
> > definitely needed, so that one can establish a full ruleset before
> > interfaces even exist. That's good practise at boottime, but also
> > needed for dynamic interface creation during runtime.
> 
> Please do not send html messages : Your reply did not reach the lists.

Sigh. Sorry...

> Then, all you mention could have been solved by proper userspace
> support.
> 
> Every time you add an interface or change device name, you could change
> firewalls rules if needed. Nothing shocking here.

That is totally impractical, IMO.

Interfaces come and go through many different actions. There's the admin 
downing and upping stuff like bridges or bonds. There's stuff like libvirt / 
KVM / qemu creating and destroying interfaces. In all these cases, in my 
practise, I give the interfaces useful names to that I can prefix-match them 
in iptables rules.

Dynamically modifying the ruleset for each such creation and destruction, 
would be a huge burden. The base ruleset would need suitable "hooks" where 
these rules were inserted (ordering matters!). The addition would hardly be 
atomic (with traditional iptables, unless done by generating a whole new 
ruleset and restoring). The programs (e.g. libvirt) would need to be able to 
call out to these specially crafted rule generator scripts. The admin would 
need to add them as pre/post actions to their static (manual) interface 
configuration. Loading and looking at the ruleset before bringing up the 
interface would be impossible.

Note that I do fully agree that it's sad that iptables rules waste all that 
memory for each and every rule! I remember musing about improving that in 
talks with Harald Welte back in the 90ies. A simple match would be perfectly 
fine for me. Only having ifindex support, isn't.

best regards
  Patrick

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

[Patrick McHardy]

On 12.01, Patrick Schaaf wrote:
> On Monday 12 January 2015 08:51:54 Eric Dumazet wrote:
> > On Mon, 2015-01-12 at 17:39 +0100, Patrick Schaaf wrote:
> > > 
> > > Not to comment on the ifalias thing, which I think is unneccessary,
> > > too, but matching on interface names instead of only ifindex, is
> > > definitely needed, so that one can establish a full ruleset before
> > > interfaces even exist. That's good practise at boottime, but also
> > > needed for dynamic interface creation during runtime.
> > 
> > Please do not send html messages : Your reply did not reach the lists.
> 
> Sigh. Sorry...
> 
> > Then, all you mention could have been solved by proper userspace
> > support.
> > 
> > Every time you add an interface or change device name, you could change
> > firewalls rules if needed. Nothing shocking here.
> 
> That is totally impractical, IMO.
> 
> Interfaces come and go through many different actions. There's the admin 
> downing and upping stuff like bridges or bonds. There's stuff like libvirt / 
> KVM / qemu creating and destroying interfaces. In all these cases, in my 
> practise, I give the interfaces useful names to that I can prefix-match them 
> in iptables rules.
> 
> Dynamically modifying the ruleset for each such creation and destruction, 
> would be a huge burden. The base ruleset would need suitable "hooks" where 
> these rules were inserted (ordering matters!). The addition would hardly be 
> atomic (with traditional iptables, unless done by generating a whole new 
> ruleset and restoring). The programs (e.g. libvirt) would need to be able to 
> call out to these specially crafted rule generator scripts. The admin would 
> need to add them as pre/post actions to their static (manual) interface 
> configuration. Loading and looking at the ruleset before bringing up the 
> interface would be impossible.

devgroups seem like the best solution for this.

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

[Patrick Schaaf]

On Monday 12 January 2015 17:22:57 Patrick McHardy wrote:
> On 12.01, Patrick Schaaf wrote:
> >
> > Interfaces come and go through many different actions. There's the admin
> > downing and upping stuff like bridges or bonds. There's stuff like libvirt
> > / KVM / qemu creating and destroying interfaces. In all these cases, in
> > my practise, I give the interfaces useful names to that I can
> > prefix-match them in iptables rules.
> > 
> > Dynamically modifying the ruleset for each such creation and destruction,
> > would be a huge burden. The base ruleset would need suitable "hooks" where
> > these rules were inserted (ordering matters!). The addition would hardly
> > be
> > atomic (with traditional iptables, unless done by generating a whole new
> > ruleset and restoring). The programs (e.g. libvirt) would need to be able
> > to call out to these specially crafted rule generator scripts. The admin
> > would need to add them as pre/post actions to their static (manual)
> > interface configuration. Loading and looking at the ruleset before
> > bringing up the interface would be impossible.
> 
> devgroups seem like the best solution for this.

Could be, technically.

Is there devgroup support in libvirt, ifcfg, whatever other distros use for 
their static interface configuration? Or, do I again have to write pre/post 
scripts to set devgroups? Wouldn't bother me too much nowadays, I've automated 
that for ifcfg style stuff in my production environment a year ago, but it's 
something an admin must actively manage...

There is other stuff, apart from libvirt, that creates and destroys interfaces 
on the fly. From my production environment, there's at least keepalived, which 
creates macvlan interfaces on the fly for VRRP VMAC support. I can configure 
the name for that, but nothing else, nor can I call a script pre/post for 
that. And my iptables rules on that boxen _do_ match specially on these 
interfaces.

Gooling a bit around does not immediately turn up any good documentation on it 
at all (four year old iproute2 commits, once I give that as a search term 
too?). Looks very sketchy (although the fundamental idea is clear to me. I'm 
looking through the normal admin practise lens....)

best regards
  Patrick