Per-net and on-demand link indices (and related) v3
Subsystems (4)
cover Per-net and on-demand link indices (and related) v3
Hi!
This set tries to summarize the recent discussion of making ifindices friendly to checkpoint-restore and consists of:
1. Prepare hash function to non-unique ifindices
2. Allow for specifying the desired ifindex on net link creation
3. Make ifindex generation per-net
4. Simplify loopback device ifindex access
Changes since v2:
* The pointer hash mix routine is rewritten
* Put ifindex on struct net in a better place
* Fixed compatibility on veth peer device ifindex configuration
Changes since v1:
* Fixed a stupid mistake with a pointer bits shift
* Turned the netdev_hash_mix routine into the generic ptr_hash_mix one
* Added a comment describing why loopback index is always 1 after the patch
Thanks,
Pavel
PATCH 1/5 net: Dont use ifindices in hash fns
Eric noticed, that when there will be devices with equal indices, some hash functions that use them will become less effective as they could. Fix this in advance by mixing the net_device address into the hash value instead of the device index.
This is true for arp and ndisc hash fns. The netlabel, can and llc ones are also ifindex-based, but that three are init_net-only, thus will not be affected.
Many thanks to David and Eric for the hash32_ptr implementation!
--- include/linux/hash.h | 10 ++++++++++ include/net/arp.h | 3 ++- include/net/ndisc.h | 3 ++- 3 files changed, 14 insertions(+), 2 deletions(-)
include/linux/hash.h +10 -0
@@ -67,4 +67,14 @@ static inline unsigned long hash_ptr(const void *ptr, unsigned int bits) {return hash_long ((unsigned long )ptr, bits); } + +static inline u32 hash32_ptr (const void *ptr) +{ +unsigned long val = (unsigned long )ptr; + +#if BITS_PER_LONG ==64 + val ^= (val >>32 ); +#endif +return (u32 )val; +}#endif /* _LINUX_HASH_H */
include/net/arp.h +2 -1
@@ -3,6 +3,7 @@#define _ARP_H#include <linux/if_arp.h> +#include <linux/hash.h> #include <net/neighbour.h>
@@ -10,7 +11,7 @@ extern struct neigh_table arp_tbl;static inline u32 arp_hashfn (u32 key,const struct net_device *dev,u32 hash_rnd) { -u32 val = key ^ dev->ifindex; +u32 val = key ^hash32_ptr (dev);return val * hash_rnd; }
include/net/ndisc.h +2 -1
@@ -49,6 +49,7 @@ enum {#include <linux/types.h> #include <linux/if_arp.h> #include <linux/netdevice.h> +#include <linux/hash.h> #include <net/neighbour.h>
@@ -134,7 +135,7 @@ static inline u32 ndisc_hashfn(const void *pkey, const struct net_device *dev, _ {const u32 *p32 = pkey; -return (((p32[0 ] ^ dev->ifindex) * hash_rnd[0 ]) + +return (((p32[0 ] ^hash32_ptr (dev)) * hash_rnd[0 ]) + (p32[1 ] * hash_rnd[1 ]) + (p32[2 ] * hash_rnd[2 ]) + (p32[3 ] * hash_rnd[3 ]));
Signature · 1 line
1.7.6.5
PATCH 2/5 net: Allow to create links with given ifindex
In reply to Pavel EmelyanovCurrently the RTM_NEWLINK results in -EOPNOTSUPP if the ifinfomsg->ifi_index is not zero. I propose to allow requesting ifindices on link creation. This is required by the checkpoint-restore to correctly restore a net namespace (i.e. -- a container).
--- net/core/dev.c | 7 ++++++- net/core/rtnetlink.c | 12 +++++++----- 2 files changed, 13 insertions(+), 6 deletions(-)
net/core/dev.c +6 -1
@@ -5579,7 +5579,12 @@ int register_netdevice(struct net_device *dev) } } - dev->ifindex =dev_new_index (net); + ret = -EBUSY ; +if (!dev->ifindex) + dev->ifindex =dev_new_index (net); +else if (__dev_get_by_index (net, dev->ifindex)) + goto err_uninit; +if (dev->iflink ==-1 ) dev->iflink = dev->ifindex;
net/core/rtnetlink.c +7 -5
@@ -1812,8 +1812,6 @@ replay:return -ENODEV ; } -if (ifm->ifi_index) -return -EOPNOTSUPP ;if (tb[IFLA_MAP ] || tb[IFLA_MASTER ] || tb[IFLA_PROTINFO ])return -EOPNOTSUPP ;
@@ -1839,10 +1837,14 @@ replay:return PTR_ERR (dest_net); dev =rtnl_create_link (net, dest_net, ifname, ops, tb); - -if (IS_ERR (dev)) +if (IS_ERR (dev)) { err =PTR_ERR (dev); -else if (ops->newlink) + goto out; + } + + dev->ifindex = ifm->ifi_index; + +if (ops->newlink) err = ops->newlink (net, dev, tb, data);else err =register_netdevice (dev);
Signature · 1 line
1.7.6.5
PATCH 3/5 veth: Allow to create peer link with given ifindex
In reply to Pavel EmelyanovThe ifinfomsg is in there (thanks kaber@ for foreseeing this long time ago), so take the given ifidex and register netdev with it.
Ben noticed, that this code path previously ignored ifmp->ifi_index and userland could be passing in garbage. Thus it may now fail occasionally because the value clashes with an existing interface.
To address this it's assumed that if the caller specifies the ifindex for the veth master device, then it's aware of this possibility and should explicitly specify (or set to 0 for auto-assignment) the peer's ifindex as well. With this the compatibility with old tools not setting ifindex is preserved.
--- drivers/net/veth.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/veth.c +3 -0
@@ -348,6 +348,9 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,if (tbp[IFLA_ADDRESS ] ==NULL )eth_hw_addr_random (peer); +if (ifmp && (dev->ifindex !=0 )) + peer->ifindex = ifmp->ifi_index; + err =register_netdevice (peer);put_net (net); net =NULL ;
Signature · 1 line
1.7.6.5
PATCH 4/5 net: Make ifindex generation per-net namespace
In reply to Pavel EmelyanovStrictly speaking this is only _really_ required for checkpoint-restore to make loopback device always have the same index.
This change appears to be safe wrt "ifindex should be unique per-system" concept, as all the ifindex usage is either already made per net namespace of is explicitly limited with init_net only.
There are two cool side effects of this. The first one -- ifindices of devices in container are always small, regardless of how many containers we've started (and re-started) so far. The second one is -- we can speed up the loopback ifidex access as shown in the next patch.
v2: Place ifindex right after dev_base_seq : avoid two holes and use the
same cache line, dirtied in list_netdevice()/unlist_netdevice()
--- include/net/net_namespace.h | 1 + net/core/dev.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
include/net/net_namespace.h +1 -0
@@ -66,6 +66,7 @@ struct net {struct hlist_head *dev_name_head;struct hlist_head *dev_index_head;unsigned int dev_base_seq;/* protected by rtnl_mutex */ +int ifindex;/* core fib_rules */ struct list_head rules_ops;
net/core/dev.c +2 -2
@@ -5221,12 +5221,12 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)*/ static int dev_new_index (struct net *net) { -static int ifindex; +int ifindex = net->ifindex;for (;;) {if (++ifindex <=0 ) ifindex =1 ;if (!__dev_get_by_index (net, ifindex)) -return ifindex; +return net->ifindex = ifindex; } }
Signature · 1 line
1.7.6.5
PATCH 5/5 net: Loopback ifindex is constant now
In reply to Pavel EmelyanovAs pointed out, there are places, that access net->loopback_dev->ifindex and after ifindex generation is made per-net this value becomes constant equals 1. So go ahead and introduce the LOOPBACK_IFINDEX constant and use it where appropriate.
--- drivers/net/loopback.c | 1 + include/net/net_namespace.h | 7 +++++++ net/decnet/dn_route.c | 6 +++--- net/ipv4/fib_frontend.c | 2 +- net/ipv4/ipmr.c | 2 +- net/ipv4/netfilter/ipt_rpfilter.c | 2 +- net/ipv4/route.c | 6 +++--- net/ipv6/route.c | 2 +- 8 files changed, 18 insertions(+), 10 deletions(-)
drivers/net/loopback.c +1 -0
@@ -197,6 +197,7 @@ static __net_init int loopback_net_init(struct net *net)if (err) goto out_free_netdev; +BUG_ON (dev->ifindex !=LOOPBACK_IFINDEX ); net->loopback_dev = dev;return 0 ;
include/net/net_namespace.h +7 -0
@@ -105,6 +105,13 @@ struct net {struct sock *diag_nlsk; }; +/* +* ifindex generation is per-net namespace, and loopback is +* always the 1st device in ns (see net_dev_init), thus any +* loopback device should get ifindex 1 +*/ + +#define LOOPBACK_IFINDEX 1#include <linux/seq_file_net.h>
net/decnet/dn_route.c +3 -3
@@ -961,7 +961,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowidn *o .saddr = oldflp->saddr, .flowidn_scope = RT_SCOPE_UNIVERSE, .flowidn_mark = oldflp->flowidn_mark, - .flowidn_iif = init_net.loopback_dev->ifindex, + .flowidn_iif = LOOPBACK_IFINDEX, .flowidn_oif = oldflp->flowidn_oif, };struct dn_route *rt =NULL ;
@@ -979,7 +979,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowidn *o"dn_route_output_slow: dst=%04x src=%04x mark=%d" " iif=%d oif=%d\n" ,le16_to_cpu (oldflp->daddr),le16_to_cpu (oldflp->saddr), - oldflp->flowidn_mark, init_net.loopback_dev->ifindex, + oldflp->flowidn_mark,LOOPBACK_IFINDEX , oldflp->flowidn_oif);/* If we have an output interface, verify its a DECnet device */
@@ -1042,7 +1042,7 @@ source_ok:if (!fld.daddr) goto out; } - fld.flowidn_oif = init_net.loopback_dev->ifindex; + fld.flowidn_oif =LOOPBACK_IFINDEX ; res.type =RTN_LOCAL ; goto make_route; }
net/ipv4/fib_frontend.c +1 -1
@@ -218,7 +218,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb) scope =RT_SCOPE_UNIVERSE ;if (!ipv4_is_zeronet (ip_hdr (skb)->saddr)) { fl4.flowi4_oif =0 ; - fl4.flowi4_iif = net->loopback_dev->ifindex; + fl4.flowi4_iif =LOOPBACK_IFINDEX ; fl4.daddr =ip_hdr (skb)->saddr; fl4.saddr =0 ; fl4.flowi4_tos =RT_TOS (ip_hdr (skb)->tos);
net/ipv4/ipmr.c +1 -1
@@ -1798,7 +1798,7 @@ static struct mr_table *ipmr_rt_fib_lookup(struct net *net, struct sk_buff *skb) .flowi4_oif = (rt_is_output_route(rt) ? skb->dev->ifindex : 0), .flowi4_iif = (rt_is_output_route (rt) ? - net->loopback_dev->ifindex : +LOOPBACK_IFINDEX : skb->dev->ifindex), .flowi4_mark = skb->mark, };
net/ipv4/netfilter/ipt_rpfilter.c +1 -1
@@ -85,7 +85,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)return ipv4_is_local_multicast (iph->daddr) ^ invert; flow.flowi4_iif =0 ; }else { - flow.flowi4_iif =dev_net (par->in)->loopback_dev->ifindex; + flow.flowi4_iif =LOOPBACK_IFINDEX ; } flow.daddr = iph->saddr;
net/ipv4/route.c +3 -3
@@ -1619,7 +1619,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,if (res.type ==RTN_LOCAL ) { err =fib_validate_source (skb, saddr, daddr, tos, - net->loopback_dev->ifindex, +LOOPBACK_IFINDEX , dev, in_dev, &itag);if (err <0 ) goto martian_source_keep_err;
@@ -1895,7 +1895,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4) orig_oif = fl4->flowi4_oif; - fl4->flowi4_iif = net->loopback_dev->ifindex; + fl4->flowi4_iif =LOOPBACK_IFINDEX ; fl4->flowi4_tos = tos &IPTOS_RT_MASK ; fl4->flowi4_scope = ((tos &RTO_ONLINK ) ?RT_SCOPE_LINK :RT_SCOPE_UNIVERSE );
@@ -1984,7 +1984,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)if (!fl4->daddr) fl4->daddr = fl4->saddr =htonl (INADDR_LOOPBACK ); dev_out = net->loopback_dev; - fl4->flowi4_oif = net->loopback_dev->ifindex; + fl4->flowi4_oif =LOOPBACK_IFINDEX ; res.type =RTN_LOCAL ; flags |=RTCF_LOCAL ; goto make_route;
net/ipv6/route.c +1 -1
@@ -965,7 +965,7 @@ struct dst_entry * ip6_route_output(struct net *net, const struct sock *sk, {int flags =0 ; - fl6->flowi6_iif = net->loopback_dev->ifindex; + fl6->flowi6_iif =LOOPBACK_IFINDEX ;if ((sk && sk->sk_bound_dev_if) ||rt6_need_strict (&fl6->daddr)) flags |=RT6_LOOKUP_F_IFACE ;
Signature · 1 line
1.7.6.5
David Miller
maintainerFrom: Pavel Emelyanov <[email protected]>
Date: Thu, 09 Aug 2012 11:52:00 +0400
Quoted 7 lines
This set tries to summarize the recent discussion of making ifindices friendly to checkpoint-restore and consists of:
1. Prepare hash function to non-unique ifindices
2. Allow for specifying the desired ifindex on net link creation
3. Make ifindex generation per-net
4. Simplify loopback device ifindex access
Looks good, all applied to net-next, thanks.