* [RFC] Make predictable/persistent network interface names more handy
@ 2015-01-11 20:52 Richard Weinberger
2015-01-11 20:52 ` [PATCH 1/3] net: Make interface aliases available for general usage Richard Weinberger
` (3 more replies)
0 siblings, 4 replies; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen
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(-)
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/3] net: Make interface aliases available for general usage
2015-01-11 20:52 [RFC] Make predictable/persistent network interface names more handy Richard Weinberger
@ 2015-01-11 20:52 ` Richard Weinberger
2015-01-11 22:40 ` Stephen Hemminger
2015-01-11 20:52 ` [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching Richard Weinberger
` (2 subsequent siblings)
3 siblings, 1 reply; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-11 20:52 [RFC] Make predictable/persistent network interface names more handy Richard Weinberger
2015-01-11 20:52 ` [PATCH 1/3] net: Make interface aliases available for general usage Richard Weinberger
@ 2015-01-11 20:52 ` Richard Weinberger
2015-01-12 16:04 ` Eric Dumazet
2015-01-11 20:52 ` [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare() Richard Weinberger
2015-01-11 22:42 ` [RFC] Make predictable/persistent network interface names more handy Stephen Hemminger
3 siblings, 1 reply; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 20:52 [RFC] Make predictable/persistent network interface names more handy Richard Weinberger
2015-01-11 20:52 ` [PATCH 1/3] net: Make interface aliases available for general usage Richard Weinberger
2015-01-11 20:52 ` [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching Richard Weinberger
@ 2015-01-11 20:52 ` Richard Weinberger
2015-01-11 20:59 ` Joe Perches
2015-01-11 22:42 ` [RFC] Make predictable/persistent network interface names more handy Stephen Hemminger
3 siblings, 1 reply; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 20:52 ` [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare() Richard Weinberger
@ 2015-01-11 20:59 ` Joe Perches
2015-01-11 21:02 ` Richard Weinberger
0 siblings, 1 reply; 28+ messages in thread
From: Joe Perches @ 2015-01-11 20:59 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 20:59 ` Joe Perches
@ 2015-01-11 21:02 ` Richard Weinberger
2015-01-11 21:14 ` Joe Perches
0 siblings, 1 reply; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 21:02 UTC (permalink / raw)
To: Joe Perches
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 21:02 ` Richard Weinberger
@ 2015-01-11 21:14 ` Joe Perches
2015-01-11 21:30 ` Richard Weinberger
0 siblings, 1 reply; 28+ messages in thread
From: Joe Perches @ 2015-01-11 21:14 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 21:14 ` Joe Perches
@ 2015-01-11 21:30 ` Richard Weinberger
2015-01-11 21:39 ` Joe Perches
2015-01-11 22:23 ` Jan Engelhardt
0 siblings, 2 replies; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 21:30 UTC (permalink / raw)
To: Joe Perches
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 21:30 ` Richard Weinberger
@ 2015-01-11 21:39 ` Joe Perches
2015-01-11 21:42 ` Richard Weinberger
2015-01-11 22:23 ` Jan Engelhardt
1 sibling, 1 reply; 28+ messages in thread
From: Joe Perches @ 2015-01-11 21:39 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 21:39 ` Joe Perches
@ 2015-01-11 21:42 ` Richard Weinberger
2015-01-12 2:50 ` David Miller
0 siblings, 1 reply; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 21:42 UTC (permalink / raw)
To: Joe Perches
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 21:30 ` Richard Weinberger
@ 2015-01-11 22:23 ` Jan Engelhardt
2015-01-11 22:23 ` Jan Engelhardt
1 sibling, 0 replies; 28+ messages in thread
From: Jan Engelhardt @ 2015-01-11 22:23 UTC (permalink / raw)
To: Richard Weinberger
Cc: Joe Perches, Netfilter Developer Mailing List,
Linux Kernel Mailing List,
Linux Networking Developer Mailing List
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
@ 2015-01-11 22:23 ` Jan Engelhardt
0 siblings, 0 replies; 28+ messages in thread
From: Jan Engelhardt @ 2015-01-11 22:23 UTC (permalink / raw)
To: Richard Weinberger
Cc: Joe Perches, Netfilter Developer Mailing List,
Linux Kernel Mailing List,
Linux Networking Developer Mailing List
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/3] net: Make interface aliases available for general usage
2015-01-11 20:52 ` [PATCH 1/3] net: Make interface aliases available for general usage Richard Weinberger
@ 2015-01-11 22:40 ` Stephen Hemminger
2015-01-11 22:43 ` Richard Weinberger
0 siblings, 1 reply; 28+ messages in thread
From: Stephen Hemminger @ 2015-01-11 22:40 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Make predictable/persistent network interface names more handy
2015-01-11 20:52 [RFC] Make predictable/persistent network interface names more handy Richard Weinberger
` (2 preceding siblings ...)
2015-01-11 20:52 ` [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare() Richard Weinberger
@ 2015-01-11 22:42 ` Stephen Hemminger
2015-01-11 22:47 ` Richard Weinberger
2015-01-11 22:51 ` Richard Weinberger
3 siblings, 2 replies; 28+ messages in thread
From: Stephen Hemminger @ 2015-01-11 22:42 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/3] net: Make interface aliases available for general usage
2015-01-11 22:40 ` Stephen Hemminger
@ 2015-01-11 22:43 ` Richard Weinberger
0 siblings, 0 replies; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 22:43 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Make predictable/persistent network interface names more handy
2015-01-11 22:42 ` [RFC] Make predictable/persistent network interface names more handy Stephen Hemminger
@ 2015-01-11 22:47 ` Richard Weinberger
2015-01-11 22:51 ` Richard Weinberger
1 sibling, 0 replies; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 22:47 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Make predictable/persistent network interface names more handy
2015-01-11 22:42 ` [RFC] Make predictable/persistent network interface names more handy Stephen Hemminger
2015-01-11 22:47 ` Richard Weinberger
@ 2015-01-11 22:51 ` Richard Weinberger
1 sibling, 0 replies; 28+ messages in thread
From: Richard Weinberger @ 2015-01-11 22:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-11 21:42 ` Richard Weinberger
@ 2015-01-12 2:50 ` David Miller
2015-01-12 8:18 ` Richard Weinberger
0 siblings, 1 reply; 28+ messages in thread
From: David Miller @ 2015-01-12 2:50 UTC (permalink / raw)
To: richard
Cc: joe, coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-12 2:50 ` David Miller
@ 2015-01-12 8:18 ` Richard Weinberger
2015-01-12 8:40 ` Joe Perches
0 siblings, 1 reply; 28+ messages in thread
From: Richard Weinberger @ 2015-01-12 8:18 UTC (permalink / raw)
To: David Miller
Cc: joe, coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
2015-01-12 8:18 ` Richard Weinberger
@ 2015-01-12 8:40 ` Joe Perches
0 siblings, 0 replies; 28+ messages in thread
From: Joe Perches @ 2015-01-12 8:40 UTC (permalink / raw)
To: Richard Weinberger
Cc: David Miller, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-11 20:52 ` [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching Richard Weinberger
@ 2015-01-12 16:04 ` Eric Dumazet
2015-01-12 16:12 ` Richard Weinberger
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Eric Dumazet @ 2015-01-12 16:04 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-12 16:04 ` Eric Dumazet
@ 2015-01-12 16:12 ` Richard Weinberger
2015-01-12 16:32 ` Jan Engelhardt
[not found] ` <1425960.ovH4s7sjue@rofl>
2 siblings, 0 replies; 28+ messages in thread
From: Richard Weinberger @ 2015-01-12 16:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-12 16:04 ` Eric Dumazet
2015-01-12 16:12 ` Richard Weinberger
@ 2015-01-12 16:32 ` Jan Engelhardt
2015-01-12 16:46 ` Eric Dumazet
[not found] ` <1425960.ovH4s7sjue@rofl>
2 siblings, 1 reply; 28+ messages in thread
From: Jan Engelhardt @ 2015-01-12 16:32 UTC (permalink / raw)
To: Eric Dumazet
Cc: Richard Weinberger, davem, coreteam, netfilter-devel,
linux-kernel, netdev, bhutchings, john.fastabend, herbert,
vyasevic, jiri, vfalico, therbert, edumazet, yoshfuji, jmorris,
kuznet, kadlec, kaber, pablo, kay, stephen
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.)
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-12 16:32 ` Jan Engelhardt
@ 2015-01-12 16:46 ` Eric Dumazet
0 siblings, 0 replies; 28+ messages in thread
From: Eric Dumazet @ 2015-01-12 16:46 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Richard Weinberger, davem, coreteam, netfilter-devel,
linux-kernel, netdev, bhutchings, john.fastabend, herbert,
vyasevic, jiri, vfalico, therbert, edumazet, yoshfuji, jmorris,
kuznet, kadlec, kaber, pablo, kay, stephen
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)
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
[not found] ` <1425960.ovH4s7sjue@rofl>
@ 2015-01-12 16:51 ` Eric Dumazet
2015-01-12 17:19 ` Patrick Schaaf
0 siblings, 1 reply; 28+ messages in thread
From: Eric Dumazet @ 2015-01-12 16:51 UTC (permalink / raw)
To: Patrick Schaaf
Cc: Richard Weinberger, davem, coreteam, netfilter-devel,
linux-kernel, netdev, bhutchings, john.fastabend, herbert,
vyasevic, jiri, vfalico, therbert, edumazet, yoshfuji, jmorris,
kuznet, kadlec, kaber, pablo, kay, stephen
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-12 16:51 ` Eric Dumazet
@ 2015-01-12 17:19 ` Patrick Schaaf
2015-01-12 17:22 ` Patrick McHardy
0 siblings, 1 reply; 28+ messages in thread
From: Patrick Schaaf @ 2015-01-12 17:19 UTC (permalink / raw)
To: Eric Dumazet
Cc: Richard Weinberger, davem, coreteam, netfilter-devel,
linux-kernel, netdev, bhutchings, john.fastabend, herbert,
vyasevic, jiri, vfalico, therbert, edumazet, yoshfuji, jmorris,
kuznet, kadlec, kaber, pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-12 17:19 ` Patrick Schaaf
@ 2015-01-12 17:22 ` Patrick McHardy
2015-01-12 17:41 ` Patrick Schaaf
0 siblings, 1 reply; 28+ messages in thread
From: Patrick McHardy @ 2015-01-12 17:22 UTC (permalink / raw)
To: Patrick Schaaf
Cc: Eric Dumazet, Richard Weinberger, davem, coreteam,
netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, pablo, kay, stephen
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
2015-01-12 17:22 ` Patrick McHardy
@ 2015-01-12 17:41 ` Patrick Schaaf
0 siblings, 0 replies; 28+ messages in thread
From: Patrick Schaaf @ 2015-01-12 17:41 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Dumazet, Richard Weinberger, davem, coreteam,
netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, pablo, kay, stephen
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
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2015-01-12 17:41 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-11 20:52 [RFC] Make predictable/persistent network interface names more handy Richard Weinberger
2015-01-11 20:52 ` [PATCH 1/3] net: Make interface aliases available for general usage Richard Weinberger
2015-01-11 22:40 ` Stephen Hemminger
2015-01-11 22:43 ` Richard Weinberger
2015-01-11 20:52 ` [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching Richard Weinberger
2015-01-12 16:04 ` Eric Dumazet
2015-01-12 16:12 ` Richard Weinberger
2015-01-12 16:32 ` Jan Engelhardt
2015-01-12 16:46 ` Eric Dumazet
[not found] ` <1425960.ovH4s7sjue@rofl>
2015-01-12 16:51 ` Eric Dumazet
2015-01-12 17:19 ` Patrick Schaaf
2015-01-12 17:22 ` Patrick McHardy
2015-01-12 17:41 ` Patrick Schaaf
2015-01-11 20:52 ` [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare() Richard Weinberger
2015-01-11 20:59 ` Joe Perches
2015-01-11 21:02 ` Richard Weinberger
2015-01-11 21:14 ` Joe Perches
2015-01-11 21:30 ` Richard Weinberger
2015-01-11 21:39 ` Joe Perches
2015-01-11 21:42 ` Richard Weinberger
2015-01-12 2:50 ` David Miller
2015-01-12 8:18 ` Richard Weinberger
2015-01-12 8:40 ` Joe Perches
2015-01-11 22:23 ` Jan Engelhardt
2015-01-11 22:23 ` Jan Engelhardt
2015-01-11 22:42 ` [RFC] Make predictable/persistent network interface names more handy Stephen Hemminger
2015-01-11 22:47 ` Richard Weinberger
2015-01-11 22:51 ` Richard Weinberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.