* Re: [PATCH v11 net-next 00/12] eBPF syscall, verifier, testsuite
From: Alexei Starovoitov @ 2014-09-11 22:29 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Daniel Borkmann, David S. Miller, Ingo Molnar, Linus Torvalds,
Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CALCETrWCEwscbbfX7wAW-A+VQ5Y92igD36BmTXqFra04Qdwk0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Sep 11, 2014 at 2:54 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>
>> the verifier log contains full trace. Last unsafe instruction + error
>> in many cases is useless. What we found empirically from using
>> it over last 2 years is that developers have different learning curve
>> to adjust to 'safe' style of C. Pretty much everyone couldn't
>> figure out why program is rejected based on last error. Therefore
>> verifier emits full log. From the 1st insn all the way till the last
>> 'unsafe' instruction. So the log is multiline output.
>> 'Understanding eBPF verifier messages' section of
>> Documentation/networking/filter.txt provides few trivial
>> examples of these multiline messages.
>> Like for the program:
>> BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>> BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>> BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>> BPF_LD_MAP_FD(BPF_REG_1, 0),
>> BPF_CALL_FUNC(BPF_FUNC_map_lookup_elem),
>> BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
>> BPF_ST_MEM(BPF_DW, BPF_REG_0, 4, 0),
>> BPF_EXIT_INSN(),
>> the verifier log_buf is:
>> 0: (7a) *(u64 *)(r10 -8) = 0
>> 1: (bf) r2 = r10
>> 2: (07) r2 += -8
>> 3: (b7) r1 = 0
>> 4: (85) call 1
>> 5: (15) if r0 == 0x0 goto pc+1
>> R0=map_ptr R10=fp
>> 6: (7a) *(u64 *)(r0 +4) = 0
>> misaligned access off 4 size 8
>>
>> It will surely change over time as verifier becomes smarter,
>> supports new types, optimizations and so on.
>> So this log is not an ABI. It's for humans to read.
>> The log explains _how_ verifier came to conclusion
>> that the program is unsafe.
>
> Given that you've already arranged (I think) for the verifier to be
> compilable in the kernel and in userspace, would it make more sense to
> have the kernel version just say yes or no and to make it easy for
> user code to retry verification in userspace if they want a full
> explanation?
Good memory :) Long ago I had a hack where I compiled
verifier.o for kernel and linked it with userspace wrappers to
have the same verifier for userspace. It was very fragile.
and maps were not separate objects and there were no fds.
It's not feasible anymore, since different subsystems
will configure different bpf_context and helper functions and
verifier output is dynamic based on maps that were created.
For example, if user's samples/bpf/sock_example.c does
bpf_create_map(HASH, sizeof(key) * 2, ...);
instead of
bpf_create_map(HASH, sizeof(key), ...);
the same program will be rejected in first case and will be
accepted in the second, because map sizes and ebpf
program expectations are mismatching.
For the 1st case verifier will complain that program is
trying to pass uninitialized stack into bpf_lookup(key,...)
method or stack may be out of bounds.
Human insight is needed to understand what is going on.
I think more important is that source of truth needs to be
in one place == kernel. If we have two verifiers they will
diverge sooner or later and cause confusion for users.
I think as long as we document that verifier log
messages are not cast in stone and will change, we're fine.
I consider them as continuation of compiler warnings/errors.
They are meant for humans and do change from time to time.
^ permalink raw reply
* [Patch net-next v2 0/8] ipv6: clean up locking code in anycast and mcast
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
This patchset cleans up the locking code in anycast.c and mcast.c
and makes the refcount code more readable.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
v1 -> v2:
* refactor some code and make it in a separated patch
* update comments
Cong Wang (8):
ipv6: drop useless rcu_read_lock() in anycast
ipv6: remove ipv6_sk_ac_lock
ipv6: clean up ipv6_dev_ac_inc()
ipv6: refactor __ipv6_dev_ac_inc()
ipv6: drop ipv6_sk_mc_lock in mcast
ipv6: drop some rcu_read_lock in mcast
ipv6: update the comment in mcast.c
ipv6: refactor ipv6_dev_mc_inc()
include/linux/netdevice.h | 4 +-
include/net/addrconf.h | 2 +-
net/core/dev.c | 14 ++---
net/ipv6/addrconf.c | 2 +-
net/ipv6/anycast.c | 108 ++++++++++++++++++--------------------
net/ipv6/mcast.c | 129 +++++++++++++++++++++-------------------------
6 files changed, 123 insertions(+), 136 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [Patch net-next v2 1/8] ipv6: drop useless rcu_read_lock() in anycast
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
These code is now protected by rtnl lock, rcu read lock
is useless now.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/netdevice.h | 4 ++--
net/core/dev.c | 14 ++++++++------
net/ipv6/anycast.c | 18 ++++++------------
3 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ba72f6b..17e640a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2083,8 +2083,8 @@ void __dev_remove_pack(struct packet_type *pt);
void dev_add_offload(struct packet_offload *po);
void dev_remove_offload(struct packet_offload *po);
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short flags,
- unsigned short mask);
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short flags,
+ unsigned short mask);
struct net_device *dev_get_by_name(struct net *net, const char *name);
struct net_device *dev_get_by_name_rcu(struct net *net, const char *name);
struct net_device *__dev_get_by_name(struct net *net, const char *name);
diff --git a/net/core/dev.c b/net/core/dev.c
index 3c6a967..2595f07 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -897,23 +897,25 @@ struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
EXPORT_SYMBOL(dev_getfirstbyhwtype);
/**
- * dev_get_by_flags_rcu - find any device with given flags
+ * __dev_get_by_flags - find any device with given flags
* @net: the applicable net namespace
* @if_flags: IFF_* values
* @mask: bitmask of bits in if_flags to check
*
* Search for any interface with the given flags. Returns NULL if a device
* is not found or a pointer to the device. Must be called inside
- * rcu_read_lock(), and result refcount is unchanged.
+ * rtnl_lock(), and result refcount is unchanged.
*/
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags,
- unsigned short mask)
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short if_flags,
+ unsigned short mask)
{
struct net_device *dev, *ret;
+ ASSERT_RTNL();
+
ret = NULL;
- for_each_netdev_rcu(net, dev) {
+ for_each_netdev(net, dev) {
if (((dev->flags ^ if_flags) & mask) == 0) {
ret = dev;
break;
@@ -921,7 +923,7 @@ struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags
}
return ret;
}
-EXPORT_SYMBOL(dev_get_by_flags_rcu);
+EXPORT_SYMBOL(__dev_get_by_flags);
/**
* dev_valid_name - check if name is okay for network device
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index ff2de7d..3b0429b 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -78,7 +78,6 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
pac->acl_addr = *addr;
rtnl_lock();
- rcu_read_lock();
if (ifindex == 0) {
struct rt6_info *rt;
@@ -91,11 +90,11 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
goto error;
} else {
/* router, no matching interface: just pick one */
- dev = dev_get_by_flags_rcu(net, IFF_UP,
- IFF_UP | IFF_LOOPBACK);
+ dev = __dev_get_by_flags(net, IFF_UP,
+ IFF_UP | IFF_LOOPBACK);
}
} else
- dev = dev_get_by_index_rcu(net, ifindex);
+ dev = __dev_get_by_index(net, ifindex);
if (dev == NULL) {
err = -ENODEV;
@@ -137,7 +136,6 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
}
error:
- rcu_read_unlock();
rtnl_unlock();
if (pac)
sock_kfree_s(sk, pac, sizeof(*pac));
@@ -174,11 +172,9 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
spin_unlock_bh(&ipv6_sk_ac_lock);
rtnl_lock();
- rcu_read_lock();
- dev = dev_get_by_index_rcu(net, pac->acl_ifindex);
+ dev = __dev_get_by_index(net, pac->acl_ifindex);
if (dev)
ipv6_dev_ac_dec(dev, &pac->acl_addr);
- rcu_read_unlock();
rtnl_unlock();
sock_kfree_s(sk, pac, sizeof(*pac));
@@ -203,12 +199,11 @@ void ipv6_sock_ac_close(struct sock *sk)
prev_index = 0;
rtnl_lock();
- rcu_read_lock();
while (pac) {
struct ipv6_ac_socklist *next = pac->acl_next;
if (pac->acl_ifindex != prev_index) {
- dev = dev_get_by_index_rcu(net, pac->acl_ifindex);
+ dev = __dev_get_by_index(net, pac->acl_ifindex);
prev_index = pac->acl_ifindex;
}
if (dev)
@@ -216,7 +211,6 @@ void ipv6_sock_ac_close(struct sock *sk)
sock_kfree_s(sk, pac, sizeof(*pac));
pac = next;
}
- rcu_read_unlock();
rtnl_unlock();
}
@@ -341,7 +335,7 @@ int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr)
return 0;
}
-/* called with rcu_read_lock() */
+/* called with rtnl_lock() */
static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr)
{
struct inet6_dev *idev = __in6_dev_get(dev);
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 2/8] ipv6: remove ipv6_sk_ac_lock
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Just move rtnl lock up, so that the anycast list can be protected
by rtnl lock now.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/anycast.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 3b0429b..d10f2e2 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -46,10 +46,6 @@
static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr);
-/* Big ac list lock for all the sockets */
-static DEFINE_SPINLOCK(ipv6_sk_ac_lock);
-
-
/*
* socket join an anycast group
*/
@@ -128,10 +124,8 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
err = ipv6_dev_ac_inc(dev, addr);
if (!err) {
- spin_lock_bh(&ipv6_sk_ac_lock);
pac->acl_next = np->ipv6_ac_list;
np->ipv6_ac_list = pac;
- spin_unlock_bh(&ipv6_sk_ac_lock);
pac = NULL;
}
@@ -152,7 +146,7 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
struct ipv6_ac_socklist *pac, *prev_pac;
struct net *net = sock_net(sk);
- spin_lock_bh(&ipv6_sk_ac_lock);
+ rtnl_lock();
prev_pac = NULL;
for (pac = np->ipv6_ac_list; pac; pac = pac->acl_next) {
if ((ifindex == 0 || pac->acl_ifindex == ifindex) &&
@@ -161,7 +155,7 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
prev_pac = pac;
}
if (!pac) {
- spin_unlock_bh(&ipv6_sk_ac_lock);
+ rtnl_unlock();
return -ENOENT;
}
if (prev_pac)
@@ -169,9 +163,6 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
else
np->ipv6_ac_list = pac->acl_next;
- spin_unlock_bh(&ipv6_sk_ac_lock);
-
- rtnl_lock();
dev = __dev_get_by_index(net, pac->acl_ifindex);
if (dev)
ipv6_dev_ac_dec(dev, &pac->acl_addr);
@@ -192,13 +183,11 @@ void ipv6_sock_ac_close(struct sock *sk)
if (!np->ipv6_ac_list)
return;
- spin_lock_bh(&ipv6_sk_ac_lock);
+ rtnl_lock();
pac = np->ipv6_ac_list;
np->ipv6_ac_list = NULL;
- spin_unlock_bh(&ipv6_sk_ac_lock);
prev_index = 0;
- rtnl_lock();
while (pac) {
struct ipv6_ac_socklist *next = pac->acl_next;
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 3/8] ipv6: clean up ipv6_dev_ac_inc()
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Make it accept inet6_dev, and rename it to __ipv6_dev_ac_inc()
to reflect this change.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/addrconf.h | 2 +-
net/ipv6/addrconf.c | 2 +-
net/ipv6/anycast.c | 13 ++++---------
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index f679877..9b1d42e 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -202,7 +202,7 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex,
const struct in6_addr *addr);
void ipv6_sock_ac_close(struct sock *sk);
-int ipv6_dev_ac_inc(struct net_device *dev, const struct in6_addr *addr);
+int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr);
int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr);
bool ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
const struct in6_addr *addr);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ad4598f..6b6a373 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1725,7 +1725,7 @@ static void addrconf_join_anycast(struct inet6_ifaddr *ifp)
ipv6_addr_prefix(&addr, &ifp->addr, ifp->prefix_len);
if (ipv6_addr_any(&addr))
return;
- ipv6_dev_ac_inc(ifp->idev->dev, &addr);
+ __ipv6_dev_ac_inc(ifp->idev, &addr);
}
/* caller must hold RTNL */
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index d10f2e2..66c1932 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -122,7 +122,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
goto error;
}
- err = ipv6_dev_ac_inc(dev, addr);
+ err = __ipv6_dev_ac_inc(idev, addr);
if (!err) {
pac->acl_next = np->ipv6_ac_list;
np->ipv6_ac_list = pac;
@@ -215,20 +215,15 @@ static void aca_put(struct ifacaddr6 *ac)
/*
* device anycast group inc (add if not found)
*/
-int ipv6_dev_ac_inc(struct net_device *dev, const struct in6_addr *addr)
+int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr)
{
struct ifacaddr6 *aca;
- struct inet6_dev *idev;
struct rt6_info *rt;
int err;
ASSERT_RTNL();
- idev = in6_dev_get(dev);
-
- if (idev == NULL)
- return -EINVAL;
-
+ in6_dev_hold(idev);
write_lock_bh(&idev->lock);
if (idev->dead) {
err = -ENODEV;
@@ -276,7 +271,7 @@ int ipv6_dev_ac_inc(struct net_device *dev, const struct in6_addr *addr)
ip6_ins_rt(rt);
- addrconf_join_solict(dev, &aca->aca_addr);
+ addrconf_join_solict(idev->dev, &aca->aca_addr);
aca_put(aca);
return 0;
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 4/8] ipv6: refactor __ipv6_dev_ac_inc()
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Refactor out allocation and initialization and make
the refcount code more readable.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/anycast.c | 62 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 39 insertions(+), 23 deletions(-)
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 66c1932..952c1fd 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -203,6 +203,11 @@ void ipv6_sock_ac_close(struct sock *sk)
rtnl_unlock();
}
+static void aca_get(struct ifacaddr6 *aca)
+{
+ atomic_inc(&aca->aca_refcnt);
+}
+
static void aca_put(struct ifacaddr6 *ac)
{
if (atomic_dec_and_test(&ac->aca_refcnt)) {
@@ -212,6 +217,29 @@ static void aca_put(struct ifacaddr6 *ac)
}
}
+static struct ifacaddr6 *aca_alloc(struct rt6_info *rt,
+ const struct in6_addr *addr)
+{
+ struct inet6_dev *idev = rt->rt6i_idev;
+ struct ifacaddr6 *aca;
+
+ aca = kzalloc(sizeof(*aca), GFP_ATOMIC);
+ if (aca == NULL)
+ return NULL;
+
+ aca->aca_addr = *addr;
+ in6_dev_hold(idev);
+ aca->aca_idev = idev;
+ aca->aca_rt = rt;
+ aca->aca_users = 1;
+ /* aca_tstamp should be updated upon changes */
+ aca->aca_cstamp = aca->aca_tstamp = jiffies;
+ atomic_set(&aca->aca_refcnt, 1);
+ spin_lock_init(&aca->aca_lock);
+
+ return aca;
+}
+
/*
* device anycast group inc (add if not found)
*/
@@ -223,7 +251,6 @@ int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr)
ASSERT_RTNL();
- in6_dev_hold(idev);
write_lock_bh(&idev->lock);
if (idev->dead) {
err = -ENODEV;
@@ -238,35 +265,25 @@ int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr)
}
}
- /*
- * not found: create a new one.
- */
-
- aca = kzalloc(sizeof(struct ifacaddr6), GFP_ATOMIC);
-
- if (aca == NULL) {
- err = -ENOMEM;
- goto out;
- }
-
rt = addrconf_dst_alloc(idev, addr, true);
if (IS_ERR(rt)) {
- kfree(aca);
err = PTR_ERR(rt);
goto out;
}
-
- aca->aca_addr = *addr;
- aca->aca_idev = idev;
- aca->aca_rt = rt;
- aca->aca_users = 1;
- /* aca_tstamp should be updated upon changes */
- aca->aca_cstamp = aca->aca_tstamp = jiffies;
- atomic_set(&aca->aca_refcnt, 2);
- spin_lock_init(&aca->aca_lock);
+ aca = aca_alloc(rt, addr);
+ if (aca == NULL) {
+ ip6_rt_put(rt);
+ err = -ENOMEM;
+ goto out;
+ }
aca->aca_next = idev->ac_list;
idev->ac_list = aca;
+
+ /* Hold this for addrconf_join_solict() below before we unlock,
+ * it is already exposed via idev->ac_list.
+ */
+ aca_get(aca);
write_unlock_bh(&idev->lock);
ip6_ins_rt(rt);
@@ -277,7 +294,6 @@ int __ipv6_dev_ac_inc(struct inet6_dev *idev, const struct in6_addr *addr)
return 0;
out:
write_unlock_bh(&idev->lock);
- in6_dev_put(idev);
return err;
}
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 5/8] ipv6: drop ipv6_sk_mc_lock in mcast
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Similarly the code is already protected by rtnl lock.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/mcast.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 484a942..27ca0b7 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -73,9 +73,6 @@ static void *__mld2_query_bugs[] __attribute__((__unused__)) = {
static struct in6_addr mld2_all_mcr = MLD2_ALL_MCR_INIT;
-/* Big mc list lock for all the sockets */
-static DEFINE_SPINLOCK(ipv6_sk_mc_lock);
-
static void igmp6_join_group(struct ifmcaddr6 *ma);
static void igmp6_leave_group(struct ifmcaddr6 *ma);
static void igmp6_timer_handler(unsigned long data);
@@ -201,10 +198,8 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
return err;
}
- spin_lock(&ipv6_sk_mc_lock);
mc_lst->next = np->ipv6_mc_list;
rcu_assign_pointer(np->ipv6_mc_list, mc_lst);
- spin_unlock(&ipv6_sk_mc_lock);
rcu_read_unlock();
rtnl_unlock();
@@ -226,17 +221,14 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
return -EINVAL;
rtnl_lock();
- spin_lock(&ipv6_sk_mc_lock);
for (lnk = &np->ipv6_mc_list;
- (mc_lst = rcu_dereference_protected(*lnk,
- lockdep_is_held(&ipv6_sk_mc_lock))) != NULL;
+ (mc_lst = rtnl_dereference(*lnk)) != NULL;
lnk = &mc_lst->next) {
if ((ifindex == 0 || mc_lst->ifindex == ifindex) &&
ipv6_addr_equal(&mc_lst->addr, addr)) {
struct net_device *dev;
*lnk = mc_lst->next;
- spin_unlock(&ipv6_sk_mc_lock);
rcu_read_lock();
dev = dev_get_by_index_rcu(net, mc_lst->ifindex);
@@ -256,7 +248,6 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
return 0;
}
}
- spin_unlock(&ipv6_sk_mc_lock);
rtnl_unlock();
return -EADDRNOTAVAIL;
@@ -303,13 +294,10 @@ void ipv6_sock_mc_close(struct sock *sk)
return;
rtnl_lock();
- spin_lock(&ipv6_sk_mc_lock);
- while ((mc_lst = rcu_dereference_protected(np->ipv6_mc_list,
- lockdep_is_held(&ipv6_sk_mc_lock))) != NULL) {
+ while ((mc_lst = rtnl_dereference(np->ipv6_mc_list)) != NULL) {
struct net_device *dev;
np->ipv6_mc_list = mc_lst->next;
- spin_unlock(&ipv6_sk_mc_lock);
rcu_read_lock();
dev = dev_get_by_index_rcu(net, mc_lst->ifindex);
@@ -326,9 +314,7 @@ void ipv6_sock_mc_close(struct sock *sk)
atomic_sub(sizeof(*mc_lst), &sk->sk_omem_alloc);
kfree_rcu(mc_lst, rcu);
- spin_lock(&ipv6_sk_mc_lock);
}
- spin_unlock(&ipv6_sk_mc_lock);
rtnl_unlock();
}
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 6/8] ipv6: drop some rcu_read_lock in mcast
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Similarly the code is already protected by rtnl lock.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/mcast.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 27ca0b7..4fb761d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -162,7 +162,6 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
mc_lst->addr = *addr;
rtnl_lock();
- rcu_read_lock();
if (ifindex == 0) {
struct rt6_info *rt;
rt = rt6_lookup(net, addr, NULL, 0, 0);
@@ -171,10 +170,9 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
ip6_rt_put(rt);
}
} else
- dev = dev_get_by_index_rcu(net, ifindex);
+ dev = __dev_get_by_index(net, ifindex);
if (dev == NULL) {
- rcu_read_unlock();
rtnl_unlock();
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
return -ENODEV;
@@ -192,7 +190,6 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
err = ipv6_dev_mc_inc(dev, addr);
if (err) {
- rcu_read_unlock();
rtnl_unlock();
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
return err;
@@ -201,7 +198,6 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
mc_lst->next = np->ipv6_mc_list;
rcu_assign_pointer(np->ipv6_mc_list, mc_lst);
- rcu_read_unlock();
rtnl_unlock();
return 0;
@@ -230,8 +226,7 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
*lnk = mc_lst->next;
- rcu_read_lock();
- dev = dev_get_by_index_rcu(net, mc_lst->ifindex);
+ dev = __dev_get_by_index(net, mc_lst->ifindex);
if (dev != NULL) {
struct inet6_dev *idev = __in6_dev_get(dev);
@@ -240,7 +235,6 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
__ipv6_dev_mc_dec(idev, &mc_lst->addr);
} else
(void) ip6_mc_leave_src(sk, mc_lst, NULL);
- rcu_read_unlock();
rtnl_unlock();
atomic_sub(sizeof(*mc_lst), &sk->sk_omem_alloc);
@@ -299,8 +293,7 @@ void ipv6_sock_mc_close(struct sock *sk)
np->ipv6_mc_list = mc_lst->next;
- rcu_read_lock();
- dev = dev_get_by_index_rcu(net, mc_lst->ifindex);
+ dev = __dev_get_by_index(net, mc_lst->ifindex);
if (dev) {
struct inet6_dev *idev = __in6_dev_get(dev);
@@ -309,7 +302,6 @@ void ipv6_sock_mc_close(struct sock *sk)
__ipv6_dev_mc_dec(idev, &mc_lst->addr);
} else
(void) ip6_mc_leave_src(sk, mc_lst, NULL);
- rcu_read_unlock();
atomic_sub(sizeof(*mc_lst), &sk->sk_omem_alloc);
kfree_rcu(mc_lst, rcu);
@@ -934,7 +926,7 @@ int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
struct inet6_dev *idev;
int err;
- rcu_read_lock();
+ ASSERT_RTNL();
idev = __in6_dev_get(dev);
if (!idev)
@@ -942,7 +934,6 @@ int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
else
err = __ipv6_dev_mc_dec(idev, addr);
- rcu_read_unlock();
return err;
}
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 7/8] ipv6: update the comment in mcast.c
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/mcast.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 4fb761d..d64e263 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -556,9 +556,8 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
}
err = -EADDRNOTAVAIL;
- /*
- * changes to the ipv6_mc_list require the socket lock and
- * a read lock on ip6_sk_mc_lock. We have the socket lock,
+ /* changes to the ipv6_mc_list require the socket lock and
+ * rtnl lock. We have the socket lock and rcu read lock,
* so reading the list is safe.
*/
@@ -582,9 +581,8 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
copy_to_user(optval, gsf, GROUP_FILTER_SIZE(0))) {
return -EFAULT;
}
- /* changes to psl require the socket lock, a read lock on
- * on ipv6_sk_mc_lock and a write lock on pmc->sflock. We
- * have the socket lock, so reading here is safe.
+ /* changes to psl require the socket lock, and a write lock
+ * on pmc->sflock. We have the socket lock so reading here is safe.
*/
for (i = 0; i < copycount; i++) {
struct sockaddr_in6 *psin6;
@@ -2350,7 +2348,7 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
{
int err;
- /* callers have the socket lock and a write lock on ipv6_sk_mc_lock,
+ /* callers have the socket lock and rtnl lock
* so no other readers or writers of iml or its sflist
*/
if (!iml->sflist) {
--
1.8.3.1
^ permalink raw reply related
* [Patch net-next v2 8/8] ipv6: refactor ipv6_dev_mc_inc()
From: Cong Wang @ 2014-09-11 22:35 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa, Hideaki YOSHIFUJI, David S. Miller,
Cong Wang
In-Reply-To: <1410474916-21873-1-git-send-email-xiyou.wangcong@gmail.com>
Refactor out allocation and initialization and make
the refcount code more readable.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/mcast.c | 82 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 49 insertions(+), 33 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index d64e263..592eba6 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -641,14 +641,6 @@ bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
return rv;
}
-static void ma_put(struct ifmcaddr6 *mc)
-{
- if (atomic_dec_and_test(&mc->mca_refcnt)) {
- in6_dev_put(mc->idev);
- kfree(mc);
- }
-}
-
static void igmp6_group_added(struct ifmcaddr6 *mc)
{
struct net_device *dev = mc->idev->dev;
@@ -814,6 +806,48 @@ static void mld_clear_delrec(struct inet6_dev *idev)
read_unlock_bh(&idev->lock);
}
+static void mca_get(struct ifmcaddr6 *mc)
+{
+ atomic_inc(&mc->mca_refcnt);
+}
+
+static void ma_put(struct ifmcaddr6 *mc)
+{
+ if (atomic_dec_and_test(&mc->mca_refcnt)) {
+ in6_dev_put(mc->idev);
+ kfree(mc);
+ }
+}
+
+static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
+ const struct in6_addr *addr)
+{
+ struct ifmcaddr6 *mc;
+
+ mc = kzalloc(sizeof(*mc), GFP_ATOMIC);
+ if (mc == NULL)
+ return NULL;
+
+ setup_timer(&mc->mca_timer, igmp6_timer_handler, (unsigned long)mc);
+
+ mc->mca_addr = *addr;
+ mc->idev = idev; /* reference taken by caller */
+ mc->mca_users = 1;
+ /* mca_stamp should be updated upon changes */
+ mc->mca_cstamp = mc->mca_tstamp = jiffies;
+ atomic_set(&mc->mca_refcnt, 1);
+ spin_lock_init(&mc->mca_lock);
+
+ /* initial mode is (EX, empty) */
+ mc->mca_sfmode = MCAST_EXCLUDE;
+ mc->mca_sfcount[MCAST_EXCLUDE] = 1;
+
+ if (ipv6_addr_is_ll_all_nodes(&mc->mca_addr) ||
+ IPV6_ADDR_MC_SCOPE(&mc->mca_addr) < IPV6_ADDR_SCOPE_LINKLOCAL)
+ mc->mca_flags |= MAF_NOREPORT;
+
+ return mc;
+}
/*
* device multicast group inc (add if not found)
@@ -849,38 +883,20 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
}
}
- /*
- * not found: create a new one.
- */
-
- mc = kzalloc(sizeof(struct ifmcaddr6), GFP_ATOMIC);
-
- if (mc == NULL) {
+ mc = mca_alloc(idev, addr);
+ if (!mc) {
write_unlock_bh(&idev->lock);
in6_dev_put(idev);
return -ENOMEM;
}
- setup_timer(&mc->mca_timer, igmp6_timer_handler, (unsigned long)mc);
-
- mc->mca_addr = *addr;
- mc->idev = idev; /* (reference taken) */
- mc->mca_users = 1;
- /* mca_stamp should be updated upon changes */
- mc->mca_cstamp = mc->mca_tstamp = jiffies;
- atomic_set(&mc->mca_refcnt, 2);
- spin_lock_init(&mc->mca_lock);
-
- /* initial mode is (EX, empty) */
- mc->mca_sfmode = MCAST_EXCLUDE;
- mc->mca_sfcount[MCAST_EXCLUDE] = 1;
-
- if (ipv6_addr_is_ll_all_nodes(&mc->mca_addr) ||
- IPV6_ADDR_MC_SCOPE(&mc->mca_addr) < IPV6_ADDR_SCOPE_LINKLOCAL)
- mc->mca_flags |= MAF_NOREPORT;
-
mc->next = idev->mc_list;
idev->mc_list = mc;
+
+ /* Hold this for the code below before we unlock,
+ * it is already exposed via idev->mc_list.
+ */
+ mca_get(mc);
write_unlock_bh(&idev->lock);
mld_del_delrec(idev, &mc->mca_addr);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next v3 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-11 22:44 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <CAF2d9jhy=kv0nqQDWngKO7-d=zz7sGE-8YNEtRz=X2Q9VN0iqQ@mail.gmail.com>
On 09/12/2014 12:08 AM, Mahesh Bandewar wrote:
> some how my earlier mail bounced back (formatting issues, I suppose!).
> So it's a resend.
>
> On Thu, Sep 11, 2014 at 2:27 PM, Mahesh Bandewar <maheshb@google.com> wrote:
>>
>> On Thu, Sep 11, 2014 at 2:39 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>>> On 11/09/14 06:16, Mahesh Bandewar wrote:
>>>>
>>>> Earlier change to use usable slave array for TLB mode had an additional
>>>> performance advantage. So extending the same logic to all other modes
>>>> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
>>>> Also consolidating this with the earlier TLB change.
>>>>
>>>> The main idea is to build the usable slaves array in the control path
>>>> and use that array for slave selection during xmit operation.
>>>>
>>>> Measured performance in a setup with a bond of 4x1G NICs with 200
>>>> instances of netperf for the modes involved (3ad, xor, tlb)
>>>> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>>>>
>>>> Mode TPS-Before TPS-After
>>>>
>>>> 802.3ad : 468,694 493,101
>>>> TLB (lb=0): 392,583 392,965
>>>> XOR : 475,696 484,517
>>>>
>>>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>>> ---
>>>> v1:
>>>> (a) If bond_update_slave_arr() fails to allocate memory, it will
>>>> overwrite
>>>> the slave that need to be removed.
>>>> (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>>>> transition gracefully.
>>>> (c) Change from pr_debug() to pr_err() if bond_update_slave_arr()
>>>> returns
>>>> failure.
>>>> (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases
>>>> and
>>>> will populate the array even if these parameters are not used.
>>>> (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>>>> v2:
>>>> (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>>>> (b) Slave link-events now refresh array for all these modes.
>>>> (c) Moved free-array call from bond_close() to bond_uninit().
>>>> v3:
>>>> (a) Fixed null pointer dereference.
>>>> (b) Removed bond->lock lockdep dependency.
>>>>
>>> Hello Mahesh,
>>> You should've given me time to respond, the reason I wrote this:
>>> "First a question, if a bond device in XOR mode is up and we enslave a
>>> single
>>> slave how would it start transmitting ? Same question, if we are enslaving
>>> a
>>> second device then the array will be rebuild with only the first upon
>>> NETDEV_UP
>>> (of course all this is in the case miimon is 0).
>>> The NETDEV_UP upon enslave happens before the slave is linked in."
>>> was not because I wanted you to remove the slave rebuilding from the
>>> NETDEV_UP/DOWN events, but because I didn't see how would a slave start
>>> transmitting in XOR mode after enslaving, and I just tested it - it doesn't
>>> since the slave array never gets rebuilt. The NETDEV_UP event is carried by
>>> the dev_open() done in bond_enslave() earlier so the bond_set_carrier() in
>>> the end isn't of much importance in most cases, simply do the following and
>>> you'll see:
>>> modprobe bonding mode=2
>>> ip set bond0 up
>>> ifenslave bond0 eth0
>>>
>>> Try to transmit anything and watch on the other side, you won't be able to
>>> see anything as there's no slave array. My second question was given all
>>> this, if you enslave any subsequent slaves, will it start transmitting ? But
>>> I just tested this scenario and it still doesn't as the array doesn't get
>>> built.
>>
> OK I tried that and I don't see what you have observed -
>
> Here is the log -
>
> ~# modprobe bonding mode=2
> ~# ip link set bond0 up
> [ 133.537516] New slave count=0
> ~# [add ip addr]
> ~# [add default route]
> ~# ifenslave bond0 eth0
> ~# [ 211.044852] Adding slave=eth0
> [ 211.047826] New slave count=1
> ~# ifenslave bond0 eth1
> [ 723.795877] Adding slave=eth0
> [ 723.798853] Adding slave=eth1
> [ 723.801824] New slave count=2
>
> I have added some instrumentation to see what is happening and
> following are the couple of changes (basically printfs!) -
>
>
> @@ -3730,13 +3736,16 @@ int bond_update_slave_arr(struct bonding
> *bond, struct slave *skipslave)
> continue;
> if (skipslave == slave)
> continue;
> +pr_err("Adding slave=%s\n", slave->dev->name);
> new_arr->arr[new_arr->count++] = slave;
> }
>
> old_arr = rcu_dereference_protected(bond->slave_arr,
> lockdep_rtnl_is_held() ||
>
> lockdep_is_held(&bond->curr_slave_lock));
> rcu_assign_pointer(bond->slave_arr, new_arr);
> +pr_err("New slave count=%d\n", new_arr->count);
> if (old_arr)
> kfree_rcu(old_arr, rcu);
>
>
>
Something is wrong here either you have some modified version or something
else is going on because with your patch + the pr_err()s in
bond_update_slave_arr() I get the following:
# modprobe bonding mode=2
# ip l set bond0 up
[ 47.905891] Slave count: 0
# ip addr add 192.168.160.2/24 dev bond0
# ifenslave bond0 eth1
[ 78.056764] bond0: Enslaving eth1 as an active interface with an up link
[ 78.056789] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
*(no slave array messages)*
# ping ...
nothing.
Doing a down/up cycle on the bond works as it should:
# ip l set bond0 down
# ip l set bond0 up
[ 686.786898] Adding slave: eth1
[ 686.787153] Adding slave: eth2
[ 686.787329] Slave count: 2
And now it begins to work.
Same happens if I enslave a subsequent slave, I really don't see how your
arrays get rebuilt, do you have a different patch which does array
rebuilding in bond_enslave() ? The NETDEV_UP for the slave is generated by
dev_open() and it's too early to be caught as a slave by the bond's
notifier. I'd be curious to see the stack trace on your slave rebuilding
after the first enslave, could you insert a WARN_ON(1) in the
bond_update_slave_arr() for example and see what's triggering the slave
rebuild ?
In contrast the same module without your patches:
# modprobe bonding mode=2
# ip l set bond0 up
# ip addr add 192.168.160.2/24 dev bond0
# ifenslave bond0 eth1
# ping ...
works.
I'm doing these tests in a VM with virtio_net devices.
Also my default miimon is 0, if that matters.
My tree looks like:
9afec9969e3b15a8d52acb07df48423c2f941e50 bonding: Simplify the xmit
function for modes that use xmit_hash
d876c8ca5d46c3b4c201289f48011350efa545ce bonding: display xmit_hash_policy
for non-dynamic-tlb mode
9b5a8a12737ee9ff100e87ab7fdfdec4d0999f4e net: bpf: only build
bpf_jit_binary_{alloc, free}() when jit selected
>>> A few suggestions below, nothing serious though.
>>>
>>>
>>>> drivers/net/bonding/bond_3ad.c | 76 +++-----------------
>>>> drivers/net/bonding/bond_alb.c | 51 ++------------
>>>> drivers/net/bonding/bond_alb.h | 8 ---
>>>> drivers/net/bonding/bond_main.c | 152
>>>> ++++++++++++++++++++++++++++++++++++----
>>>> drivers/net/bonding/bonding.h | 8 +++
>>>> 5 files changed, 164 insertions(+), 131 deletions(-)
>>>>
>>>> diff --git a/drivers/net/bonding/bond_3ad.c
>>>> b/drivers/net/bonding/bond_3ad.c
>>>> index 5d27a6207384..516075f0a740 100644
>>>> --- a/drivers/net/bonding/bond_3ad.c
>>>> +++ b/drivers/net/bonding/bond_3ad.c
>>>> @@ -1579,6 +1579,8 @@ static void ad_agg_selection_logic(struct aggregator
>>>> *agg)
>>>> __disable_port(port);
>>>> }
>>>> }
>>>> + if (bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for 3ad
>>>> mode.\n");
>>>> }
>>>>
>>>> /* if the selected aggregator is of join individuals
>>>> @@ -1717,6 +1719,8 @@ static void ad_enable_collecting_distributing(struct
>>>> port *port)
>>>> port->actor_port_number,
>>>> port->aggregator->aggregator_identifier);
>>>> __enable_port(port);
>>>> + if (bond_update_slave_arr(port->slave->bond, NULL))
>>>> + pr_err("Failed to build slave-array for 3ad
>>>> mode.\n");
>>>> }
>>>> }
>>>>
>>>> @@ -1733,6 +1737,8 @@ static void
>>>> ad_disable_collecting_distributing(struct port *port)
>>>> port->actor_port_number,
>>>> port->aggregator->aggregator_identifier);
>>>> __disable_port(port);
>>>> + if (bond_update_slave_arr(port->slave->bond, NULL))
>>>> + pr_err("Failed to build slave-array for 3ad
>>>> mode.\n");
>>>> }
>>>> }
>>>>
>>>> @@ -2311,6 +2317,9 @@ void bond_3ad_handle_link_change(struct slave
>>>> *slave, char link)
>>>> */
>>>> port->sm_vars |= AD_PORT_BEGIN;
>>>>
>>>> + if (bond_update_slave_arr(slave->bond, NULL))
>>>> + pr_err("Failed to build slave-array for 3ad mode.\n");
>>>> +
>>>> __release_state_machine_lock(port);
>>>> }
>>>>
>>>> @@ -2406,73 +2415,6 @@ int bond_3ad_get_active_agg_info(struct bonding
>>>> *bond, struct ad_info *ad_info)
>>>> return ret;
>>>> }
>>>>
>>>> -int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
>>>> -{
>>>> - struct bonding *bond = netdev_priv(dev);
>>>> - struct slave *slave, *first_ok_slave;
>>>> - struct aggregator *agg;
>>>> - struct ad_info ad_info;
>>>> - struct list_head *iter;
>>>> - int slaves_in_agg;
>>>> - int slave_agg_no;
>>>> - int agg_id;
>>>> -
>>>> - if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>>>> - netdev_dbg(dev, "__bond_3ad_get_active_agg_info
>>>> failed\n");
>>>> - goto err_free;
>>>> - }
>>>> -
>>>> - slaves_in_agg = ad_info.ports;
>>>> - agg_id = ad_info.aggregator_id;
>>>> -
>>>> - if (slaves_in_agg == 0) {
>>>> - netdev_dbg(dev, "active aggregator is empty\n");
>>>> - goto err_free;
>>>> - }
>>>> -
>>>> - slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
>>>> - first_ok_slave = NULL;
>>>> -
>>>> - bond_for_each_slave_rcu(bond, slave, iter) {
>>>> - agg = SLAVE_AD_INFO(slave)->port.aggregator;
>>>> - if (!agg || agg->aggregator_identifier != agg_id)
>>>> - continue;
>>>> -
>>>> - if (slave_agg_no >= 0) {
>>>> - if (!first_ok_slave && bond_slave_can_tx(slave))
>>>> - first_ok_slave = slave;
>>>> - slave_agg_no--;
>>>> - continue;
>>>> - }
>>>> -
>>>> - if (bond_slave_can_tx(slave)) {
>>>> - bond_dev_queue_xmit(bond, skb, slave->dev);
>>>> - goto out;
>>>> - }
>>>> - }
>>>> -
>>>> - if (slave_agg_no >= 0) {
>>>> - netdev_err(dev, "Couldn't find a slave to tx on for
>>>> aggregator ID %d\n",
>>>> - agg_id);
>>>> - goto err_free;
>>>> - }
>>>> -
>>>> - /* we couldn't find any suitable slave after the agg_no, so use
>>>> the
>>>> - * first suitable found, if found.
>>>> - */
>>>> - if (first_ok_slave)
>>>> - bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
>>>> - else
>>>> - goto err_free;
>>>> -
>>>> -out:
>>>> - return NETDEV_TX_OK;
>>>> -err_free:
>>>> - /* no suitable interface, frame not sent */
>>>> - dev_kfree_skb_any(skb);
>>>> - goto out;
>>>> -}
>>>> -
>>>> int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding
>>>> *bond,
>>>> struct slave *slave)
>>>> {
>>>> diff --git a/drivers/net/bonding/bond_alb.c
>>>> b/drivers/net/bonding/bond_alb.c
>>>> index 028496205f39..dbac0ceb17f6 100644
>>>> --- a/drivers/net/bonding/bond_alb.c
>>>> +++ b/drivers/net/bonding/bond_alb.c
>>>> @@ -200,7 +200,6 @@ static int tlb_initialize(struct bonding *bond)
>>>> static void tlb_deinitialize(struct bonding *bond)
>>>> {
>>>> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>>>> - struct tlb_up_slave *arr;
>>>>
>>>> _lock_tx_hashtbl_bh(bond);
>>>>
>>>> @@ -208,10 +207,6 @@ static void tlb_deinitialize(struct bonding *bond)
>>>> bond_info->tx_hashtbl = NULL;
>>>>
>>>> _unlock_tx_hashtbl_bh(bond);
>>>> -
>>>> - arr = rtnl_dereference(bond_info->slave_arr);
>>>> - if (arr)
>>>> - kfree_rcu(arr, rcu);
>>>> }
>>>>
>>>> static long long compute_gap(struct slave *slave)
>>>> @@ -1409,39 +1404,9 @@ out:
>>>> return NETDEV_TX_OK;
>>>> }
>>>>
>>>> -static int bond_tlb_update_slave_arr(struct bonding *bond,
>>>> - struct slave *skipslave)
>>>> -{
>>>> - struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>>>> - struct slave *tx_slave;
>>>> - struct list_head *iter;
>>>> - struct tlb_up_slave *new_arr, *old_arr;
>>>> -
>>>> - new_arr = kzalloc(offsetof(struct tlb_up_slave,
>>>> arr[bond->slave_cnt]),
>>>> - GFP_ATOMIC);
>>>> - if (!new_arr)
>>>> - return -ENOMEM;
>>>> -
>>>> - bond_for_each_slave(bond, tx_slave, iter) {
>>>> - if (!bond_slave_can_tx(tx_slave))
>>>> - continue;
>>>> - if (skipslave == tx_slave)
>>>> - continue;
>>>> - new_arr->arr[new_arr->count++] = tx_slave;
>>>> - }
>>>> -
>>>> - old_arr = rtnl_dereference(bond_info->slave_arr);
>>>> - rcu_assign_pointer(bond_info->slave_arr, new_arr);
>>>> - if (old_arr)
>>>> - kfree_rcu(old_arr, rcu);
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>>>> {
>>>> struct bonding *bond = netdev_priv(bond_dev);
>>>> - struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>>>> struct ethhdr *eth_data;
>>>> struct slave *tx_slave = NULL;
>>>> u32 hash_index;
>>>> @@ -1462,12 +1427,14 @@ int bond_tlb_xmit(struct sk_buff *skb, struct
>>>> net_device *bond_dev)
>>>> hash_index &
>>>> 0xFF,
>>>> skb->len);
>>>> } else {
>>>> - struct tlb_up_slave *slaves;
>>>> + struct bond_up_slave *slaves;
>>>> + unsigned int count;
>>>>
>>>> - slaves =
>>>> rcu_dereference(bond_info->slave_arr);
>>>> - if (slaves && slaves->count)
>>>> + slaves = rcu_dereference(bond->slave_arr);
>>>> + count = slaves ? slaves->count : 0;
>>>> + if (count)
>>>
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> In both of these cases (slaves & slaves->count) you could use likely() as in
>>> the case we have slaves to transmit, it'd be advantageous and will be hit
>>> every time. The cases that we get here and don't have a slave_arr/count are
>>> mostly 2:
>>> 1. The slaves got released between the start of xmit and this part
>>> 2. They were not eligible so the array got emptied
>>> In both of these cases we don't care about the fallout path since
>>> transmission isn't possible anyway.
>>> Also another minor thing is that usually in the cases where we want to fetch
>>> a variable only once ACCESS_ONCE() is used as a weak compiler barrier to
>>> make sure the compiler doesn't optimize out something. The others can
>>> correct me if I'm wrong, but I think in this case it's a good precaution for
>>> slaves->count.
>>> These are merely suggestions, I might be wrong.
>>>
> Will do.
>>>
>>>> tx_slave = slaves->arr[hash_index
>>>> %
>>>> -
>>>> slaves->count];
>>>> + count];
>>>> }
>>>> break;
>>>> }
>>>> @@ -1733,10 +1700,6 @@ void bond_alb_deinit_slave(struct bonding *bond,
>>>> struct slave *slave)
>>>> rlb_clear_slave(bond, slave);
>>>> }
>>>>
>>>> - if (bond_is_nondyn_tlb(bond))
>>>> - if (bond_tlb_update_slave_arr(bond, slave))
>>>> - pr_err("Failed to build slave-array for TLB
>>>> mode.\n");
>>>> -
>>>> }
>>>>
>>>> /* Caller must hold bond lock for read */
>>>> @@ -1762,7 +1725,7 @@ void bond_alb_handle_link_change(struct bonding
>>>> *bond, struct slave *slave, char
>>>> }
>>>>
>>>> if (bond_is_nondyn_tlb(bond)) {
>>>> - if (bond_tlb_update_slave_arr(bond, NULL))
>>>> + if (bond_update_slave_arr(bond, NULL))
>>>> pr_err("Failed to build slave-array for TLB
>>>> mode.\n");
>>>> }
>>>> }
>>>> diff --git a/drivers/net/bonding/bond_alb.h
>>>> b/drivers/net/bonding/bond_alb.h
>>>> index aaeac61d03cf..5fc76c01636c 100644
>>>> --- a/drivers/net/bonding/bond_alb.h
>>>> +++ b/drivers/net/bonding/bond_alb.h
>>>> @@ -139,20 +139,12 @@ struct tlb_slave_info {
>>>> */
>>>> };
>>>>
>>>> -struct tlb_up_slave {
>>>> - unsigned int count;
>>>> - struct rcu_head rcu;
>>>> - struct slave *arr[0];
>>>> -};
>>>> -
>>>> struct alb_bond_info {
>>>> struct tlb_client_info *tx_hashtbl; /* Dynamically allocated */
>>>> spinlock_t tx_hashtbl_lock;
>>>> u32 unbalanced_load;
>>>> int tx_rebalance_counter;
>>>> int lp_counter;
>>>> - /* -------- non-dynamic tlb mode only ---------*/
>>>> - struct tlb_up_slave __rcu *slave_arr; /* Up slaves */
>>>> /* -------- rlb parameters -------- */
>>>> int rlb_enabled;
>>>> struct rlb_client_info *rx_hashtbl; /* Receive hash table */
>>>> diff --git a/drivers/net/bonding/bond_main.c
>>>> b/drivers/net/bonding/bond_main.c
>>>> index b43b2df9e5d1..4412c458939d 100644
>>>> --- a/drivers/net/bonding/bond_main.c
>>>> +++ b/drivers/net/bonding/bond_main.c
>>>> @@ -1700,6 +1700,10 @@ static int __bond_release_one(struct net_device
>>>> *bond_dev,
>>>> write_unlock_bh(&bond->curr_slave_lock);
>>>> }
>>>>
>>>> + if (bond_mode_uses_xmit_hash(bond) &&
>>>> + bond_update_slave_arr(bond, slave))
>>>> + pr_err("Failed to build slave-array.\n");
>>>> +
>>>> netdev_info(bond_dev, "Releasing %s interface %s\n",
>>>> bond_is_active_slave(slave) ? "active" : "backup",
>>>> slave_dev->name);
>>>> @@ -2015,6 +2019,10 @@ static void bond_miimon_commit(struct bonding
>>>> *bond)
>>>> bond_alb_handle_link_change(bond, slave,
>>>> BOND_LINK_UP);
>>>>
>>>> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
>>>> + bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for
>>>> XOR mode.\n");
>>>> +
>>>> if (!bond->curr_active_slave || slave == primary)
>>>> goto do_failover;
>>>>
>>>> @@ -2042,6 +2050,10 @@ static void bond_miimon_commit(struct bonding
>>>> *bond)
>>>> bond_alb_handle_link_change(bond, slave,
>>>>
>>>> BOND_LINK_DOWN);
>>>>
>>>> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
>>>> + bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for
>>>> XOR mode.\n");
>>>> +
>>>> if (slave ==
>>>> rcu_access_pointer(bond->curr_active_slave))
>>>> goto do_failover;
>>>>
>>>> @@ -2505,6 +2517,9 @@ static void bond_loadbalance_arp_mon(struct
>>>> work_struct *work)
>>>>
>>>> if (slave_state_changed) {
>>>> bond_slave_state_change(bond);
>>>> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
>>>> + bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for
>>>> XOR mode.\n");
>>>> } else if (do_failover) {
>>>> /* the bond_select_active_slave must hold RTNL
>>>> * and curr_slave_lock for write.
>>>> @@ -2899,11 +2914,23 @@ static int bond_slave_netdev_event(unsigned long
>>>> event,
>>>> if (old_duplex != slave->duplex)
>>>> bond_3ad_adapter_duplex_changed(slave);
>>>> }
>>>> + /* Refresh slave-array if applicable!
>>>> + * If the setuo does not use miimon or arpmon
>>>> (mode-specific!),
>>>> + * then these event will not cause the slave-array to be
>>>> + * refreshed. This will cause xmit to use a slave that is
>>>> not
>>>> + * usable. Avoid such situation by refeshing the array at
>>>> these
>>>> + * events. If these (miimon/arpmon) parameters are
>>>> configured
>>>> + * then array gets refreshed twice and that should be
>>>> fine!
>>>> + */
>>>> + if (bond_mode_uses_xmit_hash(bond) &&
>>>> + bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for XOR
>>>> mode.\n");
>>>> break;
>>>> case NETDEV_DOWN:
>>>> - /*
>>>> - * ... Or is it this?
>>>> - */
>>>> + /* Refresh slave-array if applicable! */
>>>> + if (bond_mode_uses_xmit_hash(bond) &&
>>>> + bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for XOR
>>>> mode.\n");
>>>> break;
>>>> case NETDEV_CHANGEMTU:
>>>> /*
>>>> @@ -3147,6 +3174,10 @@ static int bond_open(struct net_device *bond_dev)
>>>> bond_3ad_initiate_agg_selection(bond, 1);
>>>> }
>>>>
>>>> + if (bond_mode_uses_xmit_hash(bond) &&
>>>> + bond_update_slave_arr(bond, NULL))
>>>> + pr_err("Failed to build slave-array for XOR mode.\n");
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> @@ -3654,15 +3685,106 @@ static int bond_xmit_activebackup(struct sk_buff
>>>> *skb, struct net_device *bond_d
>>>> return NETDEV_TX_OK;
>>>> }
>>>>
>>>> -/* In bond_xmit_xor() , we determine the output device by using a pre-
>>>> - * determined xmit_hash_policy(), If the selected device is not enabled,
>>>> - * find the next active slave.
>>>> +/* Build the usable slaves array in control path for modes that use
>>>> xmit-hash
>>>> + * to determine the slave interface -
>>>> + * (a) BOND_MODE_8023AD
>>>> + * (b) BOND_MODE_XOR
>>>> + * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
>>>> */
>>>> -static int bond_xmit_xor(struct sk_buff *skb, struct net_device
>>>> *bond_dev)
>>>> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
>>>> {
>>>> - struct bonding *bond = netdev_priv(bond_dev);
>>>> + struct slave *slave;
>>>> + struct list_head *iter;
>>>> + struct bond_up_slave *new_arr, *old_arr;
>>>> + int slaves_in_agg;
>>>> + int agg_id = 0;
>>>> + int ret = 0;
>>>> +
>>>> + new_arr = kzalloc(offsetof(struct bond_up_slave,
>>>> arr[bond->slave_cnt]),
>>>> + GFP_ATOMIC);
>>>> + if (!new_arr) {
>>>> + ret = -ENOMEM;
>>>> + goto out;
>>>> + }
>>>> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>>>> + struct ad_info ad_info;
>>>> +
>>>> + if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
>>>> + pr_debug("bond_3ad_get_active_agg_info failed\n");
>>>> + kfree_rcu(new_arr, rcu);
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> + slaves_in_agg = ad_info.ports;
>>>> + agg_id = ad_info.aggregator_id;
>>>> + }
>>>> + bond_for_each_slave(bond, slave, iter) {
>>>> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>>>> + struct aggregator *agg;
>>>>
>>>> - bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) %
>>>> bond->slave_cnt);
>>>> + agg = SLAVE_AD_INFO(slave)->port.aggregator;
>>>> + if (!agg || agg->aggregator_identifier != agg_id)
>>>> + continue;
>>>> + }
>>>> + if (!bond_slave_can_tx(slave))
>>>> + continue;
>>>> + if (skipslave == slave)
>>>> + continue;
>>>> + new_arr->arr[new_arr->count++] = slave;
>>>> + }
>>>> +
>>>> + old_arr = rcu_dereference_protected(bond->slave_arr,
>>>> + lockdep_rtnl_is_held() ||
>>>> +
>>>> lockdep_is_held(&bond->curr_slave_lock));
>>>> + rcu_assign_pointer(bond->slave_arr, new_arr);
>>>> + if (old_arr)
>>>> + kfree_rcu(old_arr, rcu);
>>>> +
>>>> +out:
>>>> + if (ret != 0 && skipslave) {
>>>> + int idx;
>>>> +
>>>> + /* Rare situation where caller has asked to skip a
>>>> specific
>>>> + * slave but allocation failed (most likely!). BTW this is
>>>> + * only possible when the call is initiated from
>>>> + * __bond_release_one(). In this sitation; overwrite the
>>>> + * skipslave entry in the array with the last entry from
>>>> the
>>>> + * array to avoid a situation where the xmit path may
>>>> choose
>>>> + * this to-be-skipped slave to send a packet out.
>>>> + */
>>>> + old_arr = rtnl_dereference(bond->slave_arr);
>>>> + for (idx = 0; idx < old_arr->count; idx++) {
>>>> + if (skipslave == old_arr->arr[idx]) {
>>>> + old_arr->arr[idx] =
>>>> + old_arr->arr[old_arr->count-1];
>>>> + old_arr->count--;
>>>> + break;
>>>> + }
>>>> + }
>>>> + }
>>>> + return ret;
>>>> +}
>>>> +
>>>> +/* Use this Xmit function for 3AD as well as XOR modes. The current
>>>> + * usable slave array is formed in the control path. The xmit function
>>>> + * just calculates hash and sends the packet out.
>>>> + */
>>>> +int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
>>>> +{
>>>> + struct bonding *bond = netdev_priv(dev);
>>>> + struct slave *slave;
>>>> + struct bond_up_slave *slaves;
>>>> + unsigned int count;
>>>> +
>>>> + slaves = rcu_dereference(bond->slave_arr);
>>>> + count = slaves ? slaves->count : 0;
>>>> + if (count) {
>>>
>>> ^^^^^^^^^^^^^^
>>> The same comment as above applies here, too.
>>>
>>>
>>>> + slave = slaves->arr[bond_xmit_hash(bond, skb) % count];
>>>> + bond_dev_queue_xmit(bond, skb, slave->dev);
>>>> + } else {
>>>> + dev_kfree_skb_any(skb);
>>>> + atomic_long_inc(&dev->tx_dropped);
>>>> + }
>>>>
>>>> return NETDEV_TX_OK;
>>>> }
>>>> @@ -3764,12 +3886,11 @@ static netdev_tx_t __bond_start_xmit(struct
>>>> sk_buff *skb, struct net_device *dev
>>>> return bond_xmit_roundrobin(skb, dev);
>>>> case BOND_MODE_ACTIVEBACKUP:
>>>> return bond_xmit_activebackup(skb, dev);
>>>> + case BOND_MODE_8023AD:
>>>> case BOND_MODE_XOR:
>>>> - return bond_xmit_xor(skb, dev);
>>>> + return bond_3ad_xor_xmit(skb, dev);
>>>> case BOND_MODE_BROADCAST:
>>>> return bond_xmit_broadcast(skb, dev);
>>>> - case BOND_MODE_8023AD:
>>>> - return bond_3ad_xmit_xor(skb, dev);
>>>> case BOND_MODE_ALB:
>>>> return bond_alb_xmit(skb, dev);
>>>> case BOND_MODE_TLB:
>>>> @@ -3947,6 +4068,7 @@ static void bond_uninit(struct net_device *bond_dev)
>>>> struct bonding *bond = netdev_priv(bond_dev);
>>>> struct list_head *iter;
>>>> struct slave *slave;
>>>> + struct bond_up_slave *arr;
>>>>
>>>> bond_netpoll_cleanup(bond_dev);
>>>>
>>>> @@ -3955,6 +4077,12 @@ static void bond_uninit(struct net_device
>>>> *bond_dev)
>>>> __bond_release_one(bond_dev, slave->dev, true);
>>>> netdev_info(bond_dev, "Released all slaves\n");
>>>>
>>>> + arr = rtnl_dereference(bond->slave_arr);
>>>> + if (arr) {
>>>> + kfree_rcu(arr, rcu);
>>>> + RCU_INIT_POINTER(bond->slave_arr, NULL);
>>>> + }
>>>> +
>>>> list_del(&bond->bond_list);
>>>>
>>>> bond_debug_unregister(bond);
>>>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>>> index 8375133dd347..78d6d3b7a780 100644
>>>> --- a/drivers/net/bonding/bonding.h
>>>> +++ b/drivers/net/bonding/bonding.h
>>>> @@ -177,6 +177,12 @@ struct slave {
>>>> struct kobject kobj;
>>>> };
>>>>
>>>> +struct bond_up_slave {
>>>> + unsigned int count;
>>>> + struct rcu_head rcu;
>>>> + struct slave *arr[0];
>>>> +};
>>>> +
>>>> /*
>>>> * Link pseudo-state only used internally by monitors
>>>> */
>>>> @@ -193,6 +199,7 @@ struct bonding {
>>>> struct slave __rcu *curr_active_slave;
>>>> struct slave __rcu *current_arp_slave;
>>>> struct slave __rcu *primary_slave;
>>>> + struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves
>>>> */
>>>> bool force_primary;
>>>> s32 slave_cnt; /* never change this value outside the
>>>> attach/detach wrappers */
>>>> int (*recv_probe)(const struct sk_buff *, struct bonding *,
>>>> @@ -530,6 +537,7 @@ const char *bond_slave_link_status(s8 link);
>>>> struct bond_vlan_tag *bond_verify_device_path(struct net_device
>>>> *start_dev,
>>>> struct net_device *end_dev,
>>>> int level);
>>>> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
>>>>
>>>> #ifdef CONFIG_PROC_FS
>>>> void bond_create_proc_entry(struct bonding *bond);
>>>>
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [GIT net-next] Open vSwitch
From: Pravin B Shelar @ 2014-09-11 22:01 UTC (permalink / raw)
To: davem; +Cc: netdev
Following patches adds recirculation and hash action to OVS.
First three patches does code restructuring which is required
for last patch.
Recirculation implementation is changed, according to comments from
David Miller, to avoid using recursive calls in OVS. It is using
queue to record recirc action and deferred recirc is executed at
the end of current actions execution.
----------------------------------------------------------------
The following changes since commit b954d83421d51d822c42e5ab7b65069b25ad3005:
net: bpf: only build bpf_jit_binary_{alloc, free}() when jit selected (2014-09-10 14:05:07 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pshelar/openvswitch.git net_next_ovs
for you to fetch changes up to 9b8ede54a8bd319789e8bceb19789463bb944701:
openvswitch: Add recirc and hash action. (2014-09-11 13:35:29 -0700)
----------------------------------------------------------------
Andy Zhou (2):
datapath: simplify sample action implementation
openvswitch: Add recirc and hash action.
Pravin B Shelar (2):
datapath: refactor ovs flow extract API.
datapath: Use tun_key only for egress tunnel path.
include/uapi/linux/openvswitch.h | 26 +++++
net/openvswitch/actions.c | 247 ++++++++++++++++++++++++++++++++++-----
net/openvswitch/datapath.c | 52 +++++----
net/openvswitch/datapath.h | 17 ++-
net/openvswitch/flow.c | 54 +++++++--
net/openvswitch/flow.h | 10 +-
net/openvswitch/flow_netlink.c | 63 +++++++---
net/openvswitch/flow_netlink.h | 4 +-
net/openvswitch/vport-gre.c | 22 ++--
net/openvswitch/vport-vxlan.c | 20 ++--
net/openvswitch/vport.c | 13 ++-
11 files changed, 419 insertions(+), 109 deletions(-)
^ permalink raw reply
* [PATCH net-next 1/4] datapath: refactor ovs flow extract API.
From: Pravin B Shelar @ 2014-09-11 22:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Pravin B Shelar
OVS flow extract is called on packet receive or packet
execute code path. Following patch defines separate API
for extracting flow-key in packet execute code path.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
---
net/openvswitch/datapath.c | 26 ++++++++++++++---------
net/openvswitch/datapath.h | 5 ++++-
net/openvswitch/flow.c | 48 +++++++++++++++++++++++++++++++-----------
net/openvswitch/flow.h | 6 +++++-
net/openvswitch/flow_netlink.c | 22 ++++++-------------
net/openvswitch/flow_netlink.h | 4 ++--
net/openvswitch/vport.c | 3 ++-
7 files changed, 72 insertions(+), 42 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 91d66b7..7c3939a 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -237,8 +237,9 @@ void ovs_dp_detach_port(struct vport *p)
}
/* Must be called with rcu_read_lock. */
-void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
+void ovs_dp_process_received_packet(struct sk_buff *skb)
{
+ const struct vport *p = OVS_CB(skb)->input_vport;
struct datapath *dp = p->dp;
struct sw_flow *flow;
struct dp_stats_percpu *stats;
@@ -250,7 +251,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
stats = this_cpu_ptr(dp->stats_percpu);
/* Extract flow from 'skb' into 'key'. */
- error = ovs_flow_extract(skb, p->port_no, &key);
+ error = ovs_flow_key_extract(skb, &key);
if (unlikely(error)) {
kfree_skb(skb);
return;
@@ -273,9 +274,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
stats_counter = &stats->n_missed;
goto out;
}
-
OVS_CB(skb)->flow = flow;
- OVS_CB(skb)->pkt_key = &key;
ovs_flow_stats_update(OVS_CB(skb)->flow, key.tp.flags, skb);
ovs_execute_actions(dp, skb);
@@ -340,7 +339,7 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb,
break;
if (skb == segs && gso_type & SKB_GSO_UDP) {
- /* The initial flow key extracted by ovs_flow_extract()
+ /* The initial flow key extracted by ovs-key-extract()
* in this case is for a first fragment, so we need to
* properly mark later fragments.
*/
@@ -515,6 +514,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
struct sw_flow *flow;
struct datapath *dp;
struct ethhdr *eth;
+ struct vport *input_vport;
int len;
int err;
@@ -549,13 +549,11 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(flow))
goto err_kfree_skb;
- err = ovs_flow_extract(packet, -1, &flow->key);
+ err = ovs_flow_key_extract_userspace(a[OVS_PACKET_ATTR_KEY], packet,
+ &flow->key);
if (err)
goto err_flow_free;
- err = ovs_nla_get_flow_metadata(flow, a[OVS_PACKET_ATTR_KEY]);
- if (err)
- goto err_flow_free;
acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_PACKET_ATTR_ACTIONS]));
err = PTR_ERR(acts);
if (IS_ERR(acts))
@@ -568,7 +566,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
goto err_flow_free;
OVS_CB(packet)->flow = flow;
- OVS_CB(packet)->pkt_key = &flow->key;
packet->priority = flow->key.phy.priority;
packet->mark = flow->key.phy.skb_mark;
@@ -578,6 +575,15 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
if (!dp)
goto err_unlock;
+ input_vport = ovs_vport_rcu(dp, flow->key.phy.in_port);
+ if (!input_vport)
+ input_vport = ovs_vport_rcu(dp, OVSP_LOCAL);
+
+ if (!input_vport)
+ goto err_unlock;
+
+ OVS_CB(packet)->input_vport = input_vport;
+
local_bh_disable();
err = ovs_execute_actions(dp, packet);
local_bh_enable();
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 701b573..9d5b7d1 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -98,11 +98,14 @@ struct datapath {
* @pkt_key: The flow information extracted from the packet. Must be nonnull.
* @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
* packet is not being tunneled.
+ * @input_vport: The original vport packet came in on. This value is cached
+ * when a packet is received by OVS.
*/
struct ovs_skb_cb {
struct sw_flow *flow;
struct sw_flow_key *pkt_key;
struct ovs_key_ipv4_tunnel *tun_key;
+ struct vport *input_vport;
};
#define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
@@ -183,7 +186,7 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
-void ovs_dp_process_received_packet(struct vport *, struct sk_buff *);
+void ovs_dp_process_received_packet(struct sk_buff *);
void ovs_dp_detach_port(struct vport *);
int ovs_dp_upcall(struct datapath *, struct sk_buff *,
const struct dp_upcall_info *);
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 7064da9..8c82376 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -16,8 +16,6 @@
* 02110-1301, USA
*/
-#include "flow.h"
-#include "datapath.h"
#include <linux/uaccess.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
@@ -46,6 +44,10 @@
#include <net/ipv6.h>
#include <net/ndisc.h>
+#include "datapath.h"
+#include "flow.h"
+#include "flow_netlink.h"
+
u64 ovs_flow_used_time(unsigned long flow_jiffies)
{
struct timespec cur_ts;
@@ -420,7 +422,7 @@ invalid:
}
/**
- * ovs_flow_extract - extracts a flow key from an Ethernet frame.
+ * key_extract - extracts a flow key from an Ethernet frame.
* @skb: sk_buff that contains the frame, with skb->data pointing to the
* Ethernet header
* @in_port: port number on which @skb was received.
@@ -442,19 +444,11 @@ invalid:
* of a correct length, otherwise the same as skb->network_header.
* For other key->eth.type values it is left untouched.
*/
-int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
+static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
{
int error;
struct ethhdr *eth;
- memset(key, 0, sizeof(*key));
-
- key->phy.priority = skb->priority;
- if (OVS_CB(skb)->tun_key)
- memcpy(&key->tun_key, OVS_CB(skb)->tun_key, sizeof(key->tun_key));
- key->phy.in_port = in_port;
- key->phy.skb_mark = skb->mark;
-
skb_reset_mac_header(skb);
/* Link layer. We are guaranteed to have at least the 14 byte Ethernet
@@ -611,5 +605,35 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
}
}
+ OVS_CB(skb)->pkt_key = key;
return 0;
}
+
+int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ /* Extract metadata from packet. */
+ memset(key, 0, sizeof(*key));
+ if (OVS_CB(skb)->tun_key)
+ memcpy(&key->tun_key, OVS_CB(skb)->tun_key, sizeof(key->tun_key));
+
+ key->phy.priority = skb->priority;
+ key->phy.in_port = OVS_CB(skb)->input_vport->port_no;
+ key->phy.skb_mark = skb->mark;
+
+ return key_extract(skb, key);
+}
+
+int ovs_flow_key_extract_userspace(const struct nlattr *attr,
+ struct sk_buff *skb,
+ struct sw_flow_key *key)
+{
+ int err;
+
+ memset(key, 0, sizeof(*key));
+ /* Extract metadata from netlink attributes. */
+ err = ovs_nla_get_flow_metadata(attr, key);
+ if (err)
+ return err;
+
+ return key_extract(skb, key);
+}
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 5e5aaed..251789b 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -187,6 +187,10 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
-int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *);
+int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key);
+/* Extract key from packet coming from userspace. */
+int ovs_flow_key_extract_userspace(const struct nlattr *attr,
+ struct sk_buff *skb,
+ struct sw_flow_key *key);
#endif /* flow.h */
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index d757848..630b320 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -836,7 +836,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
/**
* ovs_nla_get_flow_metadata - parses Netlink attributes into a flow key.
- * @flow: Receives extracted in_port, priority, tun_key and skb_mark.
+ * @key: Receives extracted in_port, priority, tun_key and skb_mark.
* @attr: Netlink attribute holding nested %OVS_KEY_ATTR_* Netlink attribute
* sequence.
*
@@ -846,32 +846,24 @@ int ovs_nla_get_match(struct sw_flow_match *match,
* extracted from the packet itself.
*/
-int ovs_nla_get_flow_metadata(struct sw_flow *flow,
- const struct nlattr *attr)
+int ovs_nla_get_flow_metadata(const struct nlattr *attr,
+ struct sw_flow_key *key)
{
- struct ovs_key_ipv4_tunnel *tun_key = &flow->key.tun_key;
const struct nlattr *a[OVS_KEY_ATTR_MAX + 1];
+ struct sw_flow_match match;
u64 attrs = 0;
int err;
- struct sw_flow_match match;
-
- flow->key.phy.in_port = DP_MAX_PORTS;
- flow->key.phy.priority = 0;
- flow->key.phy.skb_mark = 0;
- memset(tun_key, 0, sizeof(flow->key.tun_key));
err = parse_flow_nlattrs(attr, a, &attrs);
if (err)
return -EINVAL;
memset(&match, 0, sizeof(match));
- match.key = &flow->key;
+ match.key = key;
- err = metadata_from_nlattrs(&match, &attrs, a, false);
- if (err)
- return err;
+ key->phy.in_port = DP_MAX_PORTS;
- return 0;
+ return metadata_from_nlattrs(&match, &attrs, a, false);
}
int ovs_nla_put_flow(const struct sw_flow_key *swkey,
diff --git a/net/openvswitch/flow_netlink.h b/net/openvswitch/flow_netlink.h
index 4401510..206e45a 100644
--- a/net/openvswitch/flow_netlink.h
+++ b/net/openvswitch/flow_netlink.h
@@ -42,8 +42,8 @@ void ovs_match_init(struct sw_flow_match *match,
int ovs_nla_put_flow(const struct sw_flow_key *,
const struct sw_flow_key *, struct sk_buff *);
-int ovs_nla_get_flow_metadata(struct sw_flow *flow,
- const struct nlattr *attr);
+int ovs_nla_get_flow_metadata(const struct nlattr *, struct sw_flow_key *);
+
int ovs_nla_get_match(struct sw_flow_match *match,
const struct nlattr *,
const struct nlattr *);
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index f7e63f9..d21251f 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -443,7 +443,8 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
u64_stats_update_end(&stats->syncp);
OVS_CB(skb)->tun_key = tun_key;
- ovs_dp_process_received_packet(vport, skb);
+ OVS_CB(skb)->input_vport = vport;
+ ovs_dp_process_received_packet(skb);
}
/**
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 2/4] datapath: Use tun_key only for egress tunnel path.
From: Pravin B Shelar @ 2014-09-11 22:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Pravin B Shelar
Currently tun_key is used for passing tunnel information
on ingress and egress path, this cause confusion. Following
patch removes its use on ingress path make it egress only parameter.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
---
net/openvswitch/actions.c | 3 +--
net/openvswitch/datapath.c | 19 ++++++-------------
net/openvswitch/datapath.h | 8 ++++----
net/openvswitch/flow.c | 7 ++++---
net/openvswitch/flow.h | 3 ++-
net/openvswitch/vport-gre.c | 22 ++++++++++++----------
net/openvswitch/vport-vxlan.c | 20 +++++++++++---------
net/openvswitch/vport.c | 12 ++++++++++--
8 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 5231652..39c722f 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -511,7 +511,7 @@ static int execute_set_action(struct sk_buff *skb,
break;
case OVS_KEY_ATTR_IPV4_TUNNEL:
- OVS_CB(skb)->tun_key = nla_data(nested_attr);
+ OVS_CB(skb)->egress_tun_key = nla_data(nested_attr);
break;
case OVS_KEY_ATTR_ETHERNET:
@@ -612,6 +612,5 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb)
{
struct sw_flow_actions *acts = rcu_dereference(OVS_CB(skb)->flow->sf_acts);
- OVS_CB(skb)->tun_key = NULL;
return do_execute_actions(dp, skb, acts->actions, acts->actions_len);
}
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 7c3939a..4046bc2 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -237,33 +237,26 @@ void ovs_dp_detach_port(struct vport *p)
}
/* Must be called with rcu_read_lock. */
-void ovs_dp_process_received_packet(struct sk_buff *skb)
+void ovs_dp_process_packet(struct sk_buff *skb)
{
const struct vport *p = OVS_CB(skb)->input_vport;
+ struct sw_flow_key *pkt_key = OVS_CB(skb)->pkt_key;
struct datapath *dp = p->dp;
struct sw_flow *flow;
struct dp_stats_percpu *stats;
- struct sw_flow_key key;
u64 *stats_counter;
u32 n_mask_hit;
- int error;
stats = this_cpu_ptr(dp->stats_percpu);
- /* Extract flow from 'skb' into 'key'. */
- error = ovs_flow_key_extract(skb, &key);
- if (unlikely(error)) {
- kfree_skb(skb);
- return;
- }
-
/* Look up flow. */
- flow = ovs_flow_tbl_lookup_stats(&dp->table, &key, &n_mask_hit);
+ flow = ovs_flow_tbl_lookup_stats(&dp->table, pkt_key, &n_mask_hit);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
+ int error;
upcall.cmd = OVS_PACKET_CMD_MISS;
- upcall.key = &key;
+ upcall.key = pkt_key;
upcall.userdata = NULL;
upcall.portid = ovs_vport_find_upcall_portid(p, skb);
error = ovs_dp_upcall(dp, skb, &upcall);
@@ -276,7 +269,7 @@ void ovs_dp_process_received_packet(struct sk_buff *skb)
}
OVS_CB(skb)->flow = flow;
- ovs_flow_stats_update(OVS_CB(skb)->flow, key.tp.flags, skb);
+ ovs_flow_stats_update(OVS_CB(skb)->flow, pkt_key->tp.flags, skb);
ovs_execute_actions(dp, skb);
stats_counter = &stats->n_hit;
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 9d5b7d1..fdd96c0 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -96,15 +96,15 @@ struct datapath {
* struct ovs_skb_cb - OVS data in skb CB
* @flow: The flow associated with this packet. May be %NULL if no flow.
* @pkt_key: The flow information extracted from the packet. Must be nonnull.
- * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
- * packet is not being tunneled.
+ * @egress_tun_key: Tunnel information about this packet on egress path.
+ * NULL if the packet is not being tunneled.
* @input_vport: The original vport packet came in on. This value is cached
* when a packet is received by OVS.
*/
struct ovs_skb_cb {
struct sw_flow *flow;
struct sw_flow_key *pkt_key;
- struct ovs_key_ipv4_tunnel *tun_key;
+ struct ovs_key_ipv4_tunnel *egress_tun_key;
struct vport *input_vport;
};
#define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
@@ -186,7 +186,7 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
-void ovs_dp_process_received_packet(struct sk_buff *);
+void ovs_dp_process_packet(struct sk_buff *);
void ovs_dp_detach_port(struct vport *);
int ovs_dp_upcall(struct datapath *, struct sk_buff *,
const struct dp_upcall_info *);
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 8c82376..53a19a54 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -609,12 +609,13 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return 0;
}
-int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
+ struct sk_buff *skb, struct sw_flow_key *key)
{
/* Extract metadata from packet. */
memset(key, 0, sizeof(*key));
- if (OVS_CB(skb)->tun_key)
- memcpy(&key->tun_key, OVS_CB(skb)->tun_key, sizeof(key->tun_key));
+ if (tun_key)
+ memcpy(&key->tun_key, tun_key, sizeof(key->tun_key));
key->phy.priority = skb->priority;
key->phy.in_port = OVS_CB(skb)->input_vport->port_no;
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 251789b..3869a54 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -187,7 +187,8 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
-int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key);
+int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
+ struct sk_buff *skb, struct sw_flow_key *key);
/* Extract key from packet coming from userspace. */
int ovs_flow_key_extract_userspace(const struct nlattr *attr,
struct sk_buff *skb,
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index f49148a..e92617b 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -63,7 +63,7 @@ static __be16 filter_tnl_flags(__be16 flags)
static struct sk_buff *__build_header(struct sk_buff *skb,
int tunnel_hlen)
{
- const struct ovs_key_ipv4_tunnel *tun_key = OVS_CB(skb)->tun_key;
+ const struct ovs_key_ipv4_tunnel *tun_key = OVS_CB(skb)->egress_tun_key;
struct tnl_ptk_info tpi;
skb = gre_handle_offloads(skb, !!(tun_key->tun_flags & TUNNEL_CSUM));
@@ -129,6 +129,7 @@ static int gre_err(struct sk_buff *skb, u32 info,
static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
{
struct net *net = ovs_dp_get_net(vport->dp);
+ struct ovs_key_ipv4_tunnel *tun_key;
struct flowi4 fl;
struct rtable *rt;
int min_headroom;
@@ -136,16 +137,17 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
__be16 df;
int err;
- if (unlikely(!OVS_CB(skb)->tun_key)) {
+ if (unlikely(!OVS_CB(skb)->egress_tun_key)) {
err = -EINVAL;
goto error;
}
+ tun_key = OVS_CB(skb)->egress_tun_key;
/* Route lookup */
memset(&fl, 0, sizeof(fl));
- fl.daddr = OVS_CB(skb)->tun_key->ipv4_dst;
- fl.saddr = OVS_CB(skb)->tun_key->ipv4_src;
- fl.flowi4_tos = RT_TOS(OVS_CB(skb)->tun_key->ipv4_tos);
+ fl.daddr = tun_key->ipv4_dst;
+ fl.saddr = tun_key->ipv4_src;
+ fl.flowi4_tos = RT_TOS(tun_key->ipv4_tos);
fl.flowi4_mark = skb->mark;
fl.flowi4_proto = IPPROTO_GRE;
@@ -153,7 +155,7 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
if (IS_ERR(rt))
return PTR_ERR(rt);
- tunnel_hlen = ip_gre_calc_hlen(OVS_CB(skb)->tun_key->tun_flags);
+ tunnel_hlen = ip_gre_calc_hlen(tun_key->tun_flags);
min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
+ tunnel_hlen + sizeof(struct iphdr)
@@ -185,15 +187,15 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
goto err_free_rt;
}
- df = OVS_CB(skb)->tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
+ df = tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
htons(IP_DF) : 0;
skb->ignore_df = 1;
return iptunnel_xmit(skb->sk, rt, skb, fl.saddr,
- OVS_CB(skb)->tun_key->ipv4_dst, IPPROTO_GRE,
- OVS_CB(skb)->tun_key->ipv4_tos,
- OVS_CB(skb)->tun_key->ipv4_ttl, df, false);
+ tun_key->ipv4_dst, IPPROTO_GRE,
+ tun_key->ipv4_tos,
+ tun_key->ipv4_ttl, df, false);
err_free_rt:
ip_rt_put(rt);
error:
diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index d8b7e24..d3f088f 100644
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -140,22 +140,24 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
struct net *net = ovs_dp_get_net(vport->dp);
struct vxlan_port *vxlan_port = vxlan_vport(vport);
__be16 dst_port = inet_sk(vxlan_port->vs->sock->sk)->inet_sport;
+ struct ovs_key_ipv4_tunnel *tun_key;
struct rtable *rt;
struct flowi4 fl;
__be16 src_port;
__be16 df;
int err;
- if (unlikely(!OVS_CB(skb)->tun_key)) {
+ if (unlikely(!OVS_CB(skb)->egress_tun_key)) {
err = -EINVAL;
goto error;
}
+ tun_key = OVS_CB(skb)->egress_tun_key;
/* Route lookup */
memset(&fl, 0, sizeof(fl));
- fl.daddr = OVS_CB(skb)->tun_key->ipv4_dst;
- fl.saddr = OVS_CB(skb)->tun_key->ipv4_src;
- fl.flowi4_tos = RT_TOS(OVS_CB(skb)->tun_key->ipv4_tos);
+ fl.daddr = tun_key->ipv4_dst;
+ fl.saddr = tun_key->ipv4_src;
+ fl.flowi4_tos = RT_TOS(tun_key->ipv4_tos);
fl.flowi4_mark = skb->mark;
fl.flowi4_proto = IPPROTO_UDP;
@@ -165,7 +167,7 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
goto error;
}
- df = OVS_CB(skb)->tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
+ df = tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
htons(IP_DF) : 0;
skb->ignore_df = 1;
@@ -173,11 +175,11 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
src_port = udp_flow_src_port(net, skb, 0, 0, true);
err = vxlan_xmit_skb(vxlan_port->vs, rt, skb,
- fl.saddr, OVS_CB(skb)->tun_key->ipv4_dst,
- OVS_CB(skb)->tun_key->ipv4_tos,
- OVS_CB(skb)->tun_key->ipv4_ttl, df,
+ fl.saddr, tun_key->ipv4_dst,
+ tun_key->ipv4_tos,
+ tun_key->ipv4_ttl, df,
src_port, dst_port,
- htonl(be64_to_cpu(OVS_CB(skb)->tun_key->tun_id) << 8),
+ htonl(be64_to_cpu(tun_key->tun_id) << 8),
false);
if (err < 0)
ip_rt_put(rt);
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index d21251f..d3f76df 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -435,6 +435,8 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
struct ovs_key_ipv4_tunnel *tun_key)
{
struct pcpu_sw_netstats *stats;
+ struct sw_flow_key key;
+ int error;
stats = this_cpu_ptr(vport->percpu_stats);
u64_stats_update_begin(&stats->syncp);
@@ -442,9 +444,15 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
stats->rx_bytes += skb->len;
u64_stats_update_end(&stats->syncp);
- OVS_CB(skb)->tun_key = tun_key;
OVS_CB(skb)->input_vport = vport;
- ovs_dp_process_received_packet(skb);
+ OVS_CB(skb)->egress_tun_key = NULL;
+ /* Extract flow from 'skb' into 'key'. */
+ error = ovs_flow_key_extract(tun_key, skb, &key);
+ if (unlikely(error)) {
+ kfree_skb(skb);
+ return;
+ }
+ ovs_dp_process_packet(skb);
}
/**
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 3/4] datapath: simplify sample action implementation
From: Pravin B Shelar @ 2014-09-11 22:03 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou, Pravin B Shelar
From: Andy Zhou <azhou@nicira.com>
The current sample() function implementation is more complicated
than necessary in handling single user space action optimization
and skb reference counting. There is no functional changes.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
net/openvswitch/actions.c | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 39c722f..fda7ef3 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -449,7 +449,6 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
{
const struct nlattr *acts_list = NULL;
const struct nlattr *a;
- struct sk_buff *sample_skb;
int rem;
for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
@@ -467,33 +466,27 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
}
rem = nla_len(acts_list);
- a = nla_data(acts_list);
+ /* Actions list is empty, do nothing */
+ if (unlikely(!rem))
+ return 0;
- /* Actions list is either empty or only contains a single user-space
- * action, the latter being a special case as it is the only known
- * usage of the sample action.
- * In these special cases don't clone the skb as there are no
- * side-effects in the nested actions.
- * Otherwise, clone in case the nested actions have side effects.
+ a = nla_data(acts_list);
+ /* The only known usage of sample action is having a single user-space
+ * action. Treat this usage as a special case.
+ * The output_userspace() should clone the skb to be sent to the
+ * user space. This skb will be consumed by its caller.
*/
- if (likely(rem == 0 || (nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
- last_action(a, rem)))) {
- sample_skb = skb;
- skb_get(skb);
- } else {
- sample_skb = skb_clone(skb, GFP_ATOMIC);
- if (!sample_skb) /* Skip sample action when out of memory. */
- return 0;
- }
+ if (likely(nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
+ last_action(a, rem)))
+ return output_userspace(dp, skb, a);
- /* Note that do_execute_actions() never consumes skb.
- * In the case where skb has been cloned above it is the clone that
- * is consumed. Otherwise the skb_get(skb) call prevents
- * consumption by do_execute_actions(). Thus, it is safe to simply
- * return the error code and let the caller (also
- * do_execute_actions()) free skb on error.
- */
- return do_execute_actions(dp, sample_skb, a, rem);
+ skb = skb_clone(skb, GFP_ATOMIC);
+ if (!skb)
+ /* Skip the sample action when out of memory. */
+ return 0;
+
+ /* do_execute_actions() will consume the cloned skb. */
+ return do_execute_actions(dp, skb, a, rem);
}
static int execute_set_action(struct sk_buff *skb,
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 4/4] datapath: Add recirc and hash action.
From: Pravin B Shelar @ 2014-09-11 22:04 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou, Pravin B Shelar
From: Andy Zhou <azhou@nicira.com>
Recirc action allows a packet to reenter openvswitch processing.
currently openvswitch lookup flow for packet received and execute
set of actions on that packet, with help of recirc action we can
process/modify the packet and recirculate it back in openvswitch
for another pass.
OVS hash action calculates 5-tupple hash and set hash in flow-key
hash. This can be used along with recirculation for distributing
packets among different ports for bond devices.
For example:
OVS bonding can use following actions:
Match on: bond flow; Action: hash, recirc(id)
Match on: recirc-id == id and hash lower bits == a;
Action: output port_bond_a
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
include/uapi/linux/openvswitch.h | 26 +++++
net/openvswitch/actions.c | 207 ++++++++++++++++++++++++++++++++++++++-
net/openvswitch/datapath.c | 11 ++-
net/openvswitch/datapath.h | 6 +-
net/openvswitch/flow.c | 5 +
net/openvswitch/flow.h | 3 +
net/openvswitch/flow_netlink.c | 41 +++++++-
7 files changed, 290 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index a794d1d..f7fc507 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -289,6 +289,9 @@ enum ovs_key_attr {
OVS_KEY_ATTR_TUNNEL, /* Nested set of ovs_tunnel attributes */
OVS_KEY_ATTR_SCTP, /* struct ovs_key_sctp */
OVS_KEY_ATTR_TCP_FLAGS, /* be16 TCP flags. */
+ OVS_KEY_ATTR_DP_HASH, /* u32 hash value. Value 0 indicates the hash
+ is not computed by the datapath. */
+ OVS_KEY_ATTR_RECIRC_ID, /* u32 recirc id */
#ifdef __KERNEL__
OVS_KEY_ATTR_IPV4_TUNNEL, /* struct ovs_key_ipv4_tunnel */
@@ -493,6 +496,27 @@ struct ovs_action_push_vlan {
__be16 vlan_tci; /* 802.1Q TCI (VLAN ID and priority). */
};
+/* Data path hash algorithm for computing Datapath hash.
+ *
+ * The algorithm type only specifies the fields in a flow
+ * will be used as part of the hash. Each datapath is free
+ * to use its own hash algorithm. The hash value will be
+ * opaque to the user space daemon.
+ */
+enum ovs_hash_alg {
+ OVS_HASH_ALG_L4,
+};
+
+/*
+ * struct ovs_action_hash - %OVS_ACTION_ATTR_HASH action argument.
+ * @hash_alg: Algorithm used to compute hash prior to recirculation.
+ * @hash_basis: basis used for computing hash.
+ */
+struct ovs_action_hash {
+ uint32_t hash_alg; /* One of ovs_hash_alg. */
+ uint32_t hash_basis;
+};
+
/**
* enum ovs_action_attr - Action types.
*
@@ -521,6 +545,8 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_PUSH_VLAN, /* struct ovs_action_push_vlan. */
OVS_ACTION_ATTR_POP_VLAN, /* No argument. */
OVS_ACTION_ATTR_SAMPLE, /* Nested OVS_SAMPLE_ATTR_*. */
+ OVS_ACTION_ATTR_RECIRC, /* u32 recirc_id. */
+ OVS_ACTION_ATTR_HASH, /* struct ovs_action_hash. */
__OVS_ACTION_ATTR_MAX
};
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index fda7ef3..52a6a16 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2007-2013 Nicira, Inc.
+ * Copyright (c) 2007-2014 Nicira, Inc.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General Public
@@ -35,11 +35,87 @@
#include <net/sctp/checksum.h>
#include "datapath.h"
+#include "flow.h"
#include "vport.h"
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *attr, int len);
+struct deferred_action {
+ struct sk_buff *skb;
+ const struct nlattr *actions;
+
+ /* Store pkt_key clone when creating deferred action. */
+ struct sw_flow_key pkt_key;
+};
+
+#define DEFERRED_ACTION_FIFO_SIZE 10
+struct action_fifo {
+ int head;
+ int tail;
+ /* Deferred action fifo queue storage. */
+ struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
+};
+
+static struct action_fifo __percpu *action_fifos;
+static DEFINE_PER_CPU(int, exec_actions_level);
+
+static void action_fifo_init(struct action_fifo *fifo)
+{
+ fifo->head = 0;
+ fifo->tail = 0;
+}
+
+static bool action_fifo_is_empty(struct action_fifo *fifo)
+{
+ return (fifo->head == fifo->tail);
+}
+
+static struct deferred_action *action_fifo_get(struct action_fifo *fifo)
+{
+ if (action_fifo_is_empty(fifo))
+ return NULL;
+
+ return &fifo->fifo[fifo->tail++];
+}
+
+static struct deferred_action *action_fifo_put(struct action_fifo *fifo)
+{
+ if (fifo->head >= DEFERRED_ACTION_FIFO_SIZE - 1)
+ return NULL;
+
+ return &fifo->fifo[fifo->head++];
+}
+
+static void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key)
+{
+ *new_key = *OVS_CB(skb)->pkt_key;
+ OVS_CB(skb)->pkt_key = new_key;
+}
+
+/* Return true if fifo is not full */
+static bool add_deferred_actions(struct sk_buff *skb,
+ const struct nlattr *attr)
+{
+ struct action_fifo *fifo;
+ struct deferred_action *da;
+
+ fifo = this_cpu_ptr(action_fifos);
+ da = action_fifo_put(fifo);
+ if (da) {
+ da->skb = skb;
+ da->actions = attr;
+ flow_key_clone(skb, &da->pkt_key);
+ }
+
+ return (da != NULL);
+}
+
+static void flow_key_set_recirc_id(struct sk_buff *skb, u32 recirc_id)
+{
+ OVS_CB(skb)->pkt_key->recirc_id = recirc_id;
+}
+
static int make_writable(struct sk_buff *skb, int write_len)
{
if (!pskb_may_pull(skb, write_len))
@@ -485,8 +561,29 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
/* Skip the sample action when out of memory. */
return 0;
- /* do_execute_actions() will consume the cloned skb. */
- return do_execute_actions(dp, skb, a, rem);
+ if (!add_deferred_actions(skb, a)) {
+ if (net_ratelimit())
+ pr_warn("%s: deferred actions limit reached, dropping sample action\n",
+ ovs_dp_name(dp));
+
+ kfree_skb(skb);
+ }
+ return 0;
+}
+
+static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
+{
+ struct sw_flow_key *key = OVS_CB(skb)->pkt_key;
+ struct ovs_action_hash *hash_act = nla_data(attr);
+ u32 hash = 0;
+
+ /* OVS_HASH_ALG_L4 is the only possible hash algorithm. */
+ hash = skb_get_hash(skb);
+ hash = jhash_1word(hash, hash_act->hash_basis);
+ if (!hash)
+ hash = 0x1;
+
+ key->ovs_flow_hash = hash;
}
static int execute_set_action(struct sk_buff *skb,
@@ -535,6 +632,41 @@ static int execute_set_action(struct sk_buff *skb,
return err;
}
+static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
+ const struct nlattr *a, int rem)
+{
+ int err;
+
+ err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
+ if (err)
+ return err;
+
+ if (!last_action(a, rem)) {
+ /* Recirc action is the not the last action
+ * of the action list, need to clone the skb.
+ */
+ skb = skb_clone(skb, GFP_ATOMIC);
+
+ /* Skip the recirc action when out of memory, but
+ * continue on with the rest of the action list.
+ */
+ if (!skb)
+ return 0;
+ }
+
+ if (add_deferred_actions(skb, NULL)) {
+ flow_key_set_recirc_id(skb, nla_get_u32(a));
+ } else {
+ kfree_skb(skb);
+
+ if (net_ratelimit())
+ pr_warn("%s: deferred action limit reached, drop recirc action\n",
+ ovs_dp_name(dp));
+ }
+
+ return 0;
+}
+
/* Execute a list of actions against 'skb'. */
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *attr, int len)
@@ -565,6 +697,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
output_userspace(dp, skb, a);
break;
+ case OVS_ACTION_ATTR_HASH:
+ execute_hash(skb, a);
+ break;
+
case OVS_ACTION_ATTR_PUSH_VLAN:
err = push_vlan(skb, nla_data(a));
if (unlikely(err)) /* skb already freed. */
@@ -575,6 +711,17 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
err = pop_vlan(skb);
break;
+ case OVS_ACTION_ATTR_RECIRC:
+ err = execute_recirc(dp, skb, a, rem);
+ if (last_action(a, rem)) {
+ /* If this is the last action, the skb has
+ * been consumed or freed.
+ * Return immediately.
+ */
+ return err;
+ }
+ break;
+
case OVS_ACTION_ATTR_SET:
err = execute_set_action(skb, nla_data(a));
break;
@@ -600,10 +747,60 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
return 0;
}
+static void process_deferred_actions(struct datapath *dp)
+{
+ struct action_fifo *fifo = this_cpu_ptr(action_fifos);
+
+ /* Do not touch the FIFO in case there is no deferred actions. */
+ if (action_fifo_is_empty(fifo))
+ return;
+
+ /* Finishing executing all deferred actions. */
+ do {
+ struct deferred_action *da = action_fifo_get(fifo);
+ struct sk_buff *skb = da->skb;
+ const struct nlattr *actions = da->actions;
+
+ if (actions)
+ do_execute_actions(dp, skb, actions,
+ nla_len(actions));
+ else
+ ovs_dp_process_packet(skb);
+ } while (!action_fifo_is_empty(fifo));
+
+ /* Reset FIFO for the next packet. */
+ action_fifo_init(fifo);
+}
+
/* Execute a list of actions against 'skb'. */
int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb)
{
- struct sw_flow_actions *acts = rcu_dereference(OVS_CB(skb)->flow->sf_acts);
+ int level = this_cpu_read(exec_actions_level);
+ struct sw_flow_actions *acts;
+ int err;
+
+ acts = rcu_dereference(OVS_CB(skb)->flow->sf_acts);
+
+ this_cpu_inc(exec_actions_level);
+ err = do_execute_actions(dp, skb, acts->actions, acts->actions_len);
- return do_execute_actions(dp, skb, acts->actions, acts->actions_len);
+ if (!level)
+ process_deferred_actions(dp);
+
+ this_cpu_dec(exec_actions_level);
+ return err;
+}
+
+int action_fifos_init(void)
+{
+ action_fifos = alloc_percpu(struct action_fifo);
+ if (!action_fifos)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void action_fifos_exit(void)
+{
+ free_percpu(action_fifos);
}
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 4046bc2..2409ac0 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -156,7 +156,7 @@ static struct datapath *get_dp(struct net *net, int dp_ifindex)
}
/* Must be called with rcu_read_lock or ovs_mutex. */
-static const char *ovs_dp_name(const struct datapath *dp)
+const char *ovs_dp_name(const struct datapath *dp)
{
struct vport *vport = ovs_vport_ovsl_rcu(dp, OVSP_LOCAL);
return vport->ops->get_name(vport);
@@ -2065,10 +2065,14 @@ static int __init dp_init(void)
pr_info("Open vSwitch switching datapath\n");
- err = ovs_internal_dev_rtnl_link_register();
+ err = action_fifos_init();
if (err)
goto error;
+ err = ovs_internal_dev_rtnl_link_register();
+ if (err)
+ goto error_action_fifos_exit;
+
err = ovs_flow_init();
if (err)
goto error_unreg_rtnl_link;
@@ -2101,6 +2105,8 @@ error_flow_exit:
ovs_flow_exit();
error_unreg_rtnl_link:
ovs_internal_dev_rtnl_link_unregister();
+error_action_fifos_exit:
+ action_fifos_exit();
error:
return err;
}
@@ -2114,6 +2120,7 @@ static void dp_cleanup(void)
ovs_vport_exit();
ovs_flow_exit();
ovs_internal_dev_rtnl_link_unregister();
+ action_fifos_exit();
}
module_init(dp_init);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index fdd96c0..ca162c0 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2007-2012 Nicira, Inc.
+ * Copyright (c) 2007-2014 Nicira, Inc.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General Public
@@ -191,12 +191,16 @@ void ovs_dp_detach_port(struct vport *);
int ovs_dp_upcall(struct datapath *, struct sk_buff *,
const struct dp_upcall_info *);
+const char *ovs_dp_name(const struct datapath *dp);
struct sk_buff *ovs_vport_cmd_build_info(struct vport *, u32 pid, u32 seq,
u8 cmd);
int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb);
void ovs_dp_notify_wq(struct work_struct *work);
+int action_fifos_init(void);
+void action_fifos_exit(void);
+
#define OVS_NLERR(fmt, ...) \
do { \
if (net_ratelimit()) \
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 53a19a54..e85e9ae 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -609,6 +609,11 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return 0;
}
+int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ return key_extract(skb, key);
+}
+
int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
struct sk_buff *skb, struct sw_flow_key *key)
{
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 3869a54..0f5db4e 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -72,6 +72,8 @@ struct sw_flow_key {
u32 skb_mark; /* SKB mark. */
u16 in_port; /* Input switch port (or DP_MAX_PORTS). */
} __packed phy; /* Safe when right after 'tun_key'. */
+ u32 ovs_flow_hash; /* Datapath computed hash value. */
+ u32 recirc_id; /* Recirculation ID. */
struct {
u8 src[ETH_ALEN]; /* Ethernet source address. */
u8 dst[ETH_ALEN]; /* Ethernet destination address. */
@@ -187,6 +189,7 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
+int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key);
int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
struct sk_buff *skb, struct sw_flow_key *key);
/* Extract key from packet coming from userspace. */
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 630b320..61c8add 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -251,6 +251,8 @@ static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_KEY_ATTR_ICMPV6] = sizeof(struct ovs_key_icmpv6),
[OVS_KEY_ATTR_ARP] = sizeof(struct ovs_key_arp),
[OVS_KEY_ATTR_ND] = sizeof(struct ovs_key_nd),
+ [OVS_KEY_ATTR_RECIRC_ID] = sizeof(u32),
+ [OVS_KEY_ATTR_DP_HASH] = sizeof(u32),
[OVS_KEY_ATTR_TUNNEL] = -1,
};
@@ -454,6 +456,20 @@ static int ipv4_tun_to_nlattr(struct sk_buff *skb,
static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs,
const struct nlattr **a, bool is_mask)
{
+ if (*attrs & (1 << OVS_KEY_ATTR_DP_HASH)) {
+ u32 hash_val = nla_get_u32(a[OVS_KEY_ATTR_DP_HASH]);
+
+ SW_FLOW_KEY_PUT(match, ovs_flow_hash, hash_val, is_mask);
+ *attrs &= ~(1 << OVS_KEY_ATTR_DP_HASH);
+ }
+
+ if (*attrs & (1 << OVS_KEY_ATTR_RECIRC_ID)) {
+ u32 recirc_id = nla_get_u32(a[OVS_KEY_ATTR_RECIRC_ID]);
+
+ SW_FLOW_KEY_PUT(match, recirc_id, recirc_id, is_mask);
+ *attrs &= ~(1 << OVS_KEY_ATTR_RECIRC_ID);
+ }
+
if (*attrs & (1 << OVS_KEY_ATTR_PRIORITY)) {
SW_FLOW_KEY_PUT(match, phy.priority,
nla_get_u32(a[OVS_KEY_ATTR_PRIORITY]), is_mask);
@@ -873,6 +889,12 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
struct nlattr *nla, *encap;
bool is_mask = (swkey != output);
+ if (nla_put_u32(skb, OVS_KEY_ATTR_RECIRC_ID, output->recirc_id))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, OVS_KEY_ATTR_DP_HASH, output->ovs_flow_hash))
+ goto nla_put_failure;
+
if (nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, output->phy.priority))
goto nla_put_failure;
@@ -1401,11 +1423,13 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
/* Expected argument lengths, (u32)-1 for variable length. */
static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
[OVS_ACTION_ATTR_OUTPUT] = sizeof(u32),
+ [OVS_ACTION_ATTR_RECIRC] = sizeof(u32),
[OVS_ACTION_ATTR_USERSPACE] = (u32)-1,
[OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct ovs_action_push_vlan),
[OVS_ACTION_ATTR_POP_VLAN] = 0,
[OVS_ACTION_ATTR_SET] = (u32)-1,
- [OVS_ACTION_ATTR_SAMPLE] = (u32)-1
+ [OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
+ [OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
};
const struct ovs_action_push_vlan *vlan;
int type = nla_type(a);
@@ -1432,6 +1456,18 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
return -EINVAL;
break;
+ case OVS_ACTION_ATTR_HASH: {
+ const struct ovs_action_hash *act_hash = nla_data(a);
+
+ switch (act_hash->hash_alg) {
+ case OVS_HASH_ALG_L4:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ break;
+ }
case OVS_ACTION_ATTR_POP_VLAN:
break;
@@ -1444,6 +1480,9 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
return -EINVAL;
break;
+ case OVS_ACTION_ATTR_RECIRC:
+ break;
+
case OVS_ACTION_ATTR_SET:
err = validate_set(a, key, sfa, &skip_copy);
if (err)
--
1.9.3
^ permalink raw reply related
* Re: [GIT net-next] Open vSwitch
From: Pravin Shelar @ 2014-09-11 23:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1410472889-1417-1-git-send-email-pshelar@nicira.com>
Please ignore this series, I have used datapath as subsystem name
rather than openvswitch. I will respin it shortly.
On Thu, Sep 11, 2014 at 3:01 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
> Following patches adds recirculation and hash action to OVS.
> First three patches does code restructuring which is required
> for last patch.
> Recirculation implementation is changed, according to comments from
> David Miller, to avoid using recursive calls in OVS. It is using
> queue to record recirc action and deferred recirc is executed at
> the end of current actions execution.
>
> ----------------------------------------------------------------
> The following changes since commit b954d83421d51d822c42e5ab7b65069b25ad3005:
>
> net: bpf: only build bpf_jit_binary_{alloc, free}() when jit selected (2014-09-10 14:05:07 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pshelar/openvswitch.git net_next_ovs
>
> for you to fetch changes up to 9b8ede54a8bd319789e8bceb19789463bb944701:
>
> openvswitch: Add recirc and hash action. (2014-09-11 13:35:29 -0700)
>
> ----------------------------------------------------------------
> Andy Zhou (2):
> datapath: simplify sample action implementation
> openvswitch: Add recirc and hash action.
>
> Pravin B Shelar (2):
> datapath: refactor ovs flow extract API.
> datapath: Use tun_key only for egress tunnel path.
>
> include/uapi/linux/openvswitch.h | 26 +++++
> net/openvswitch/actions.c | 247 ++++++++++++++++++++++++++++++++++-----
> net/openvswitch/datapath.c | 52 +++++----
> net/openvswitch/datapath.h | 17 ++-
> net/openvswitch/flow.c | 54 +++++++--
> net/openvswitch/flow.h | 10 +-
> net/openvswitch/flow_netlink.c | 63 +++++++---
> net/openvswitch/flow_netlink.h | 4 +-
> net/openvswitch/vport-gre.c | 22 ++--
> net/openvswitch/vport-vxlan.c | 20 ++--
> net/openvswitch/vport.c | 13 ++-
> 11 files changed, 419 insertions(+), 109 deletions(-)
^ permalink raw reply
* [PATCH -next] net: stmmac: fix return value check in socfpga_dwmac_parse_data()
From: weiyj_lk @ 2014-09-11 23:12 UTC (permalink / raw)
To: Giuseppe Cavallaro; +Cc: Wei Yongjun, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
In case of error, the function devm_ioremap_resource() returns
ERR_PTR() and never returns NULL. The NULL test in the return
value check should be replaced with IS_ERR().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index ddc6115..3aad413 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -120,9 +120,9 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
}
dwmac->splitter_base = devm_ioremap_resource(dev, &res_splitter);
- if (!dwmac->splitter_base) {
+ if (IS_ERR(dwmac->splitter_base)) {
dev_info(dev, "Failed to mapping emac splitter\n");
- return -EINVAL;
+ return PTR_ERR(dwmac->splitter_base);
}
}
^ permalink raw reply related
* [PATCH RFC 0/6] ipv6: dst_entry socket caching improvments
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev
Eric Dumazet noticed that rt6_nodes wich are neither RTF_NONEXTHOP nor
RTF_GATEWAY but DST_HOST ones cause major routing lookup churn because
their rt6_genid is never renewed, thus ip6_dst_check always considers
them outdated. This is a major problem, because these kind of routes
are normally used to in input handling.
This patchset tries to improve the situation by also updating the
fn_sernum in the routing tables during address deletion. The only
expensive operation left, which needs a walk over all routing tables,
are xfrm policy modifications.
I didn't annotate the patches with fixes-tags as it only solves a
performance issue. Please review carefully, thanks! I'll do some more
tests and will do a propoer submission if the xfrm slow paths looks ok
to everyone.
Hannes Frederic Sowa (6):
ipv6: also increase fib6_node sernum on deletion events
ipv6: no need to bump rt_genid_ipv6 on address change anymore
ipv6: if no function for cleaner is specified only visit nodes
ipv6: new function fib6_flush_trees and use it instead of bumping
removed rt6_genid
ipv6: keep rt_sernum per namespace to reduce number of flushes
ipv6: switch rt_sernum to atomic_t and clean up types
include/net/ip6_fib.h | 2 +-
include/net/net_namespace.h | 14 ++------
include/net/netns/ipv6.h | 2 +-
net/ipv6/addrconf.c | 1 -
net/ipv6/addrconf_core.c | 6 ++++
net/ipv6/af_inet6.c | 2 +-
net/ipv6/ip6_fib.c | 81 ++++++++++++++++++++++++++++++++-------------
net/ipv6/route.c | 4 ---
8 files changed, 70 insertions(+), 42 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH RFC 1/6] ipv6: also increase fib6_node sernum on deletion events
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
fib6_add increases the fn_sernum of fib6_nodes while it traverses the
tree. This serial number is used by ip6_dst_check to judge whether a
relookup for the socket cache should be done (e.g. a better route is
available).
We didn't do so for fib6_del, so we missed relookups on ipv6 address
deletion events. Because this caused trouble in the SCTP stack, instead
the genid for ipv6 was bumped. Also TCP connections used old source
addresses, which were not available anymore.
Because we have static rt6_nodes in the tree (no RTF_GATEWAY,
RTF_NONEXTHOP nor RTF_CACHE nodes but still DST_HOST) flag, we ended up
in a situation where the genid of the routing node was always smaller
than the published genid in the namespace. That caused ip6_dst_check to
always discard the current dst_entry and a relookup happend.
This patch prepares for the removal of the ipv6 genid by also modifying
the fn_sernum on route deletion.
Thanks to Eric Dumazet who noticed this problem!
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/ip6_fib.c | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 76b7f5e..101efab 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -60,6 +60,7 @@ struct fib6_cleaner_t {
struct fib6_walker_t w;
struct net *net;
int (*func)(struct rt6_info *, void *arg);
+ u32 sernum;
void *arg;
};
@@ -71,7 +72,8 @@ static DEFINE_RWLOCK(fib6_walker_lock);
#define FWS_INIT FWS_L
#endif
-static void fib6_prune_clones(struct net *net, struct fib6_node *fn);
+static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
+ u32 sernum);
static struct rt6_info *fib6_find_prefix(struct net *net, struct fib6_node *fn);
static struct fib6_node *fib6_repair_tree(struct net *net, struct fib6_node *fn);
static int fib6_walk(struct fib6_walker_t *w);
@@ -84,7 +86,7 @@ static int fib6_walk_continue(struct fib6_walker_t *w);
* result of redirects, path MTU changes, etc.
*/
-static __u32 rt_sernum;
+static u32 rt_sernum;
static void fib6_gc_timer_cb(unsigned long arg);
@@ -423,14 +425,14 @@ out:
static struct fib6_node *fib6_add_1(struct fib6_node *root,
struct in6_addr *addr, int plen,
int offset, int allow_create,
- int replace_required)
+ int replace_required,
+ u32 sernum)
{
struct fib6_node *fn, *in, *ln;
struct fib6_node *pn = NULL;
struct rt6key *key;
int bit;
__be32 dir = 0;
- __u32 sernum = fib6_new_sernum();
RT6_TRACE("fib6_add_1\n");
@@ -848,6 +850,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
int err = -ENOMEM;
int allow_create = 1;
int replace_required = 0;
+ u32 sernum = fib6_new_sernum();
if (info->nlh) {
if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
@@ -860,7 +863,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
fn = fib6_add_1(root, &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
offsetof(struct rt6_info, rt6i_dst), allow_create,
- replace_required);
+ replace_required, sernum);
if (IS_ERR(fn)) {
err = PTR_ERR(fn);
fn = NULL;
@@ -894,14 +897,14 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
sfn->leaf = info->nl_net->ipv6.ip6_null_entry;
atomic_inc(&info->nl_net->ipv6.ip6_null_entry->rt6i_ref);
sfn->fn_flags = RTN_ROOT;
- sfn->fn_sernum = fib6_new_sernum();
+ sfn->fn_sernum = sernum;
/* Now add the first leaf node to new subtree */
sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
rt->rt6i_src.plen,
offsetof(struct rt6_info, rt6i_src),
- allow_create, replace_required);
+ allow_create, replace_required, sernum);
if (IS_ERR(sn)) {
/* If it is failed, discard just allocated
@@ -920,7 +923,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
sn = fib6_add_1(fn->subtree, &rt->rt6i_src.addr,
rt->rt6i_src.plen,
offsetof(struct rt6_info, rt6i_src),
- allow_create, replace_required);
+ allow_create, replace_required, sernum);
if (IS_ERR(sn)) {
err = PTR_ERR(sn);
@@ -940,7 +943,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
if (!err) {
fib6_start_gc(info->nl_net, rt);
if (!(rt->rt6i_flags & RTF_CACHE))
- fib6_prune_clones(info->nl_net, pn);
+ fib6_prune_clones(info->nl_net, pn, sernum);
}
out:
@@ -1352,6 +1355,7 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
struct net *net = info->nl_net;
struct fib6_node *fn = rt->rt6i_node;
struct rt6_info **rtp;
+ u32 sernum = fib6_new_sernum();
#if RT6_DEBUG >= 2
if (rt->dst.obsolete > 0) {
@@ -1364,6 +1368,7 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
WARN_ON(!(fn->fn_flags & RTN_RTINFO));
+ fn->fn_sernum = sernum;
if (!(rt->rt6i_flags & RTF_CACHE)) {
struct fib6_node *pn = fn;
#ifdef CONFIG_IPV6_SUBTREES
@@ -1374,7 +1379,7 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
pn = pn->parent;
}
#endif
- fib6_prune_clones(info->nl_net, pn);
+ fib6_prune_clones(info->nl_net, pn, sernum);
}
/*
@@ -1521,6 +1526,9 @@ static int fib6_clean_node(struct fib6_walker_t *w)
.nl_net = c->net,
};
+ if (c->sernum)
+ c->w.node->fn_sernum = c->sernum;
+
for (rt = w->leaf; rt; rt = rt->dst.rt6_next) {
res = c->func(rt, c->arg);
if (res < 0) {
@@ -1554,7 +1562,7 @@ static int fib6_clean_node(struct fib6_walker_t *w)
static void fib6_clean_tree(struct net *net, struct fib6_node *root,
int (*func)(struct rt6_info *, void *arg),
- int prune, void *arg)
+ int prune, u32 sernum, void *arg)
{
struct fib6_cleaner_t c;
@@ -1563,6 +1571,7 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
c.w.prune = prune;
c.w.count = 0;
c.w.skip = 0;
+ c.sernum = sernum;
c.func = func;
c.arg = arg;
c.net = net;
@@ -1583,7 +1592,7 @@ void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
hlist_for_each_entry_rcu(table, head, tb6_hlist) {
write_lock_bh(&table->tb6_lock);
fib6_clean_tree(net, &table->tb6_root,
- func, 0, arg);
+ func, 0, 0, arg);
write_unlock_bh(&table->tb6_lock);
}
}
@@ -1600,9 +1609,10 @@ static int fib6_prune_clone(struct rt6_info *rt, void *arg)
return 0;
}
-static void fib6_prune_clones(struct net *net, struct fib6_node *fn)
+static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
+ u32 sernum)
{
- fib6_clean_tree(net, fn, fib6_prune_clone, 1, NULL);
+ fib6_clean_tree(net, fn, fib6_prune_clone, 1, sernum, NULL);
}
/*
@@ -1811,7 +1821,7 @@ struct ipv6_route_iter {
struct fib6_walker_t w;
loff_t skip;
struct fib6_table *tbl;
- __u32 sernum;
+ u32 sernum;
};
static int ipv6_route_seq_show(struct seq_file *seq, void *v)
--
1.9.3
^ permalink raw reply related
* [PATCH RFC 2/6] ipv6: no need to bump rt_genid_ipv6 on address change anymore
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
The fn_sernum is now modified on fib6_del events so we don't need to
bump the ipv6 genid anymore.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/addrconf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ad4598f..16ce390 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4781,7 +4781,6 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
break;
}
atomic_inc(&net->ipv6.dev_addr_genid);
- rt_genid_bump_ipv6(net);
}
static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
--
1.9.3
^ permalink raw reply related
* [PATCH RFC 3/6] ipv6: if no function for cleaner is specified only visit nodes
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
We now allow NULL rt6_info walker functions to we only visit nodes.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/ip6_fib.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 101efab..590c5d2 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1529,6 +1529,13 @@ static int fib6_clean_node(struct fib6_walker_t *w)
if (c->sernum)
c->w.node->fn_sernum = c->sernum;
+ /* if func is NULL only visit node to update sernum */
+ if (!c->func) {
+ WARN_ON_ONCE(!c->sernum);
+ w->leaf = NULL;
+ return 0;
+ }
+
for (rt = w->leaf; rt; rt = rt->dst.rt6_next) {
res = c->func(rt, c->arg);
if (res < 0) {
--
1.9.3
^ permalink raw reply related
* [PATCH RFC 4/6] ipv6: new function fib6_flush_trees and use it instead of bumping removed rt6_genid
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
fib6_flush_trees is still a very costly operation but now is only called
by xfrm code when a policy changes.
fib6_flush_tree must walk all ipv6 routing tables and modify fn_sernum,
so all sockets relookup their dst_entries. Use a NULL callback, so we
only walk the nodes without looking at the rt6_infos.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/net_namespace.h | 14 +++-----------
include/net/netns/ipv6.h | 1 -
net/ipv6/addrconf_core.c | 6 ++++++
net/ipv6/af_inet6.c | 1 -
net/ipv6/ip6_fib.c | 23 ++++++++++++++++++++---
net/ipv6/route.c | 4 ----
6 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 361d260..61aad36 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -353,21 +353,13 @@ static inline void rt_genid_bump_ipv4(struct net *net)
}
#if IS_ENABLED(CONFIG_IPV6)
-static inline int rt_genid_ipv6(struct net *net)
-{
- return atomic_read(&net->ipv6.rt_genid);
-}
-
+extern void (*__fib6_flush_trees)(struct net *);
static inline void rt_genid_bump_ipv6(struct net *net)
{
- atomic_inc(&net->ipv6.rt_genid);
+ if (__fib6_flush_trees)
+ __fib6_flush_trees(net);
}
#else
-static inline int rt_genid_ipv6(struct net *net)
-{
- return 0;
-}
-
static inline void rt_genid_bump_ipv6(struct net *net)
{
}
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index eade27a..3291ba6 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -76,7 +76,6 @@ struct netns_ipv6 {
#endif
#endif
atomic_t dev_addr_genid;
- atomic_t rt_genid;
};
#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index e696045..8b2d99a 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -10,6 +10,12 @@
#define IPV6_ADDR_SCOPE_TYPE(scope) ((scope) << 16)
+/* if ipv6 module registers this function is used by xfrm to force
+ * all sockets to relookup their nodes - this is fairly expensive
+ */
+void (*__fib6_flush_trees)(struct net *);
+EXPORT_SYMBOL(__fib6_flush_trees);
+
static inline unsigned int ipv6_addr_scope2type(unsigned int scope)
{
switch (scope) {
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index e4865a3..2189d2d 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -766,7 +766,6 @@ static int __net_init inet6_net_init(struct net *net)
net->ipv6.sysctl.icmpv6_time = 1*HZ;
net->ipv6.sysctl.flowlabel_consistency = 1;
net->ipv6.sysctl.auto_flowlabels = 0;
- atomic_set(&net->ipv6.rt_genid, 0);
err = ipv6_init_mibs(net);
if (err)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 590c5d2..cffee60 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1586,26 +1586,36 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
fib6_walk(&c.w);
}
-void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
- void *arg)
+static void __fib6_clean_all(struct net *net,
+ int (*func)(struct rt6_info *, void *arg),
+ bool update_sernum, void *arg)
{
struct fib6_table *table;
struct hlist_head *head;
unsigned int h;
+ u32 sernum = 0;
rcu_read_lock();
for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
head = &net->ipv6.fib_table_hash[h];
hlist_for_each_entry_rcu(table, head, tb6_hlist) {
write_lock_bh(&table->tb6_lock);
+ sernum = update_sernum && !sernum ?
+ fib6_new_sernum() : 0;
fib6_clean_tree(net, &table->tb6_root,
- func, 0, 0, arg);
+ func, 0, sernum, arg);
write_unlock_bh(&table->tb6_lock);
}
}
rcu_read_unlock();
}
+void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
+ void *arg)
+{
+ __fib6_clean_all(net, func, false, arg);
+}
+
static int fib6_prune_clone(struct rt6_info *rt, void *arg)
{
if (rt->rt6i_flags & RTF_CACHE) {
@@ -1622,6 +1632,11 @@ static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
fib6_clean_tree(net, fn, fib6_prune_clone, 1, sernum, NULL);
}
+static void fib6_flush_trees(struct net *net)
+{
+ __fib6_clean_all(net, NULL, true, NULL);
+}
+
/*
* Garbage collection
*/
@@ -1805,6 +1820,8 @@ int __init fib6_init(void)
NULL);
if (ret)
goto out_unregister_subsys;
+
+ __fib6_flush_trees = fib6_flush_trees;
out:
return ret;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f74b041..a318dd89 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -314,7 +314,6 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
- rt->rt6i_genid = rt_genid_ipv6(net);
INIT_LIST_HEAD(&rt->rt6i_siblings);
}
return rt;
@@ -1096,9 +1095,6 @@ static struct dst_entry *ip6_dst_check(struct dst_entry *dst, u32 cookie)
* DST_OBSOLETE_FORCE_CHK which forces validation calls down
* into this function always.
*/
- if (rt->rt6i_genid != rt_genid_ipv6(dev_net(rt->dst.dev)))
- return NULL;
-
if (!rt->rt6i_node || (rt->rt6i_node->fn_sernum != cookie))
return NULL;
--
1.9.3
^ permalink raw reply related
* [PATCH RFC 5/6] ipv6: keep rt_sernum per namespace to reduce number of flushes
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
This patch moves rt_sernum into per-namespace storage. This reduces number
of flushes needed when a xfrm policy gets modified in one namespace.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/netns/ipv6.h | 1 +
net/ipv6/af_inet6.c | 1 +
net/ipv6/ip6_fib.c | 21 +++++++++++----------
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 3291ba6..2319949 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -76,6 +76,7 @@ struct netns_ipv6 {
#endif
#endif
atomic_t dev_addr_genid;
+ u32 rt_sernum;
};
#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 2189d2d..7ff8996 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -766,6 +766,7 @@ static int __net_init inet6_net_init(struct net *net)
net->ipv6.sysctl.icmpv6_time = 1*HZ;
net->ipv6.sysctl.flowlabel_consistency = 1;
net->ipv6.sysctl.auto_flowlabels = 0;
+ net->ipv6.rt_sernum = 1;
err = ipv6_init_mibs(net);
if (err)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index cffee60..0ffabe2 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -86,8 +86,6 @@ static int fib6_walk_continue(struct fib6_walker_t *w);
* result of redirects, path MTU changes, etc.
*/
-static u32 rt_sernum;
-
static void fib6_gc_timer_cb(unsigned long arg);
static LIST_HEAD(fib6_walkers);
@@ -106,12 +104,15 @@ static inline void fib6_walker_unlink(struct fib6_walker_t *w)
list_del(&w->lh);
write_unlock_bh(&fib6_walker_lock);
}
-static __inline__ u32 fib6_new_sernum(void)
+
+static u32 fib6_new_sernum(struct net *net)
{
- u32 n = ++rt_sernum;
- if ((__s32)n <= 0)
- rt_sernum = n = 1;
- return n;
+ int *n = &net->ipv6.rt_sernum;
+
+ (*n)++;
+ if ((s32)*n <= 0)
+ *n = 1;
+ return *n;
}
/*
@@ -850,7 +851,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
int err = -ENOMEM;
int allow_create = 1;
int replace_required = 0;
- u32 sernum = fib6_new_sernum();
+ u32 sernum = fib6_new_sernum(dev_net(rt->dst.dev));
if (info->nlh) {
if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
@@ -1355,7 +1356,7 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
struct net *net = info->nl_net;
struct fib6_node *fn = rt->rt6i_node;
struct rt6_info **rtp;
- u32 sernum = fib6_new_sernum();
+ u32 sernum = fib6_new_sernum(dev_net(rt->dst.dev));
#if RT6_DEBUG >= 2
if (rt->dst.obsolete > 0) {
@@ -1601,7 +1602,7 @@ static void __fib6_clean_all(struct net *net,
hlist_for_each_entry_rcu(table, head, tb6_hlist) {
write_lock_bh(&table->tb6_lock);
sernum = update_sernum && !sernum ?
- fib6_new_sernum() : 0;
+ fib6_new_sernum(net) : 0;
fib6_clean_tree(net, &table->tb6_root,
func, 0, sernum, arg);
write_unlock_bh(&table->tb6_lock);
--
1.9.3
^ permalink raw reply related
* [PATCH RFC 6/6] ipv6: switch rt_sernum to atomic_t and clean up types
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
Switch rt_sernum to atomic_t, make it concurrency safe (the old scheme
looked broken to me) and switch from u32 to int types for the fn_sernum.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/ip6_fib.h | 2 +-
include/net/netns/ipv6.h | 2 +-
net/ipv6/af_inet6.c | 2 +-
net/ipv6/ip6_fib.c | 40 ++++++++++++++++++++--------------------
4 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 9bcb220..59c391f 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -64,7 +64,7 @@ struct fib6_node {
__u16 fn_bit; /* bit key */
__u16 fn_flags;
- __u32 fn_sernum;
+ int fn_sernum;
struct rt6_info *rr_ptr;
};
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 2319949..7dee21b 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -76,7 +76,7 @@ struct netns_ipv6 {
#endif
#endif
atomic_t dev_addr_genid;
- u32 rt_sernum;
+ atomic_t rt_sernum;
};
#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 7ff8996..6cde9b4 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -766,7 +766,7 @@ static int __net_init inet6_net_init(struct net *net)
net->ipv6.sysctl.icmpv6_time = 1*HZ;
net->ipv6.sysctl.flowlabel_consistency = 1;
net->ipv6.sysctl.auto_flowlabels = 0;
- net->ipv6.rt_sernum = 1;
+ atomic_set(&net->ipv6.rt_sernum, 1);
err = ipv6_init_mibs(net);
if (err)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 0ffabe2..03234bb 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -60,7 +60,7 @@ struct fib6_cleaner_t {
struct fib6_walker_t w;
struct net *net;
int (*func)(struct rt6_info *, void *arg);
- u32 sernum;
+ int sernum;
void *arg;
};
@@ -73,7 +73,7 @@ static DEFINE_RWLOCK(fib6_walker_lock);
#endif
static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
- u32 sernum);
+ int sernum);
static struct rt6_info *fib6_find_prefix(struct net *net, struct fib6_node *fn);
static struct fib6_node *fib6_repair_tree(struct net *net, struct fib6_node *fn);
static int fib6_walk(struct fib6_walker_t *w);
@@ -105,14 +105,17 @@ static inline void fib6_walker_unlink(struct fib6_walker_t *w)
write_unlock_bh(&fib6_walker_lock);
}
-static u32 fib6_new_sernum(struct net *net)
+static int fib6_new_sernum(struct net *net)
{
- int *n = &net->ipv6.rt_sernum;
+ int old, new;
- (*n)++;
- if ((s32)*n <= 0)
- *n = 1;
- return *n;
+ do {
+ old = atomic_read(&net->ipv6.rt_sernum);
+ new = old + 1;
+ if (new <= 0)
+ new = 1;
+ } while (atomic_cmpxchg(&net->ipv6.rt_sernum, old, new) != old);
+ return new;
}
/*
@@ -427,7 +430,7 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
struct in6_addr *addr, int plen,
int offset, int allow_create,
int replace_required,
- u32 sernum)
+ int sernum)
{
struct fib6_node *fn, *in, *ln;
struct fib6_node *pn = NULL;
@@ -851,7 +854,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
int err = -ENOMEM;
int allow_create = 1;
int replace_required = 0;
- u32 sernum = fib6_new_sernum(dev_net(rt->dst.dev));
+ int sernum = fib6_new_sernum(dev_net(rt->dst.dev));
if (info->nlh) {
if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
@@ -1356,7 +1359,7 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
struct net *net = info->nl_net;
struct fib6_node *fn = rt->rt6i_node;
struct rt6_info **rtp;
- u32 sernum = fib6_new_sernum(dev_net(rt->dst.dev));
+ int sernum = fib6_new_sernum(dev_net(rt->dst.dev));
#if RT6_DEBUG >= 2
if (rt->dst.obsolete > 0) {
@@ -1570,7 +1573,7 @@ static int fib6_clean_node(struct fib6_walker_t *w)
static void fib6_clean_tree(struct net *net, struct fib6_node *root,
int (*func)(struct rt6_info *, void *arg),
- int prune, u32 sernum, void *arg)
+ int prune, int sernum, void *arg)
{
struct fib6_cleaner_t c;
@@ -1589,20 +1592,17 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
static void __fib6_clean_all(struct net *net,
int (*func)(struct rt6_info *, void *arg),
- bool update_sernum, void *arg)
+ int sernum, void *arg)
{
struct fib6_table *table;
struct hlist_head *head;
unsigned int h;
- u32 sernum = 0;
rcu_read_lock();
for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
head = &net->ipv6.fib_table_hash[h];
hlist_for_each_entry_rcu(table, head, tb6_hlist) {
write_lock_bh(&table->tb6_lock);
- sernum = update_sernum && !sernum ?
- fib6_new_sernum(net) : 0;
fib6_clean_tree(net, &table->tb6_root,
func, 0, sernum, arg);
write_unlock_bh(&table->tb6_lock);
@@ -1614,7 +1614,7 @@ static void __fib6_clean_all(struct net *net,
void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
void *arg)
{
- __fib6_clean_all(net, func, false, arg);
+ __fib6_clean_all(net, func, fib6_new_sernum(net), arg);
}
static int fib6_prune_clone(struct rt6_info *rt, void *arg)
@@ -1628,14 +1628,14 @@ static int fib6_prune_clone(struct rt6_info *rt, void *arg)
}
static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
- u32 sernum)
+ int sernum)
{
fib6_clean_tree(net, fn, fib6_prune_clone, 1, sernum, NULL);
}
static void fib6_flush_trees(struct net *net)
{
- __fib6_clean_all(net, NULL, true, NULL);
+ __fib6_clean_all(net, NULL, fib6_new_sernum(net), NULL);
}
/*
@@ -1846,7 +1846,7 @@ struct ipv6_route_iter {
struct fib6_walker_t w;
loff_t skip;
struct fib6_table *tbl;
- u32 sernum;
+ int sernum;
};
static int ipv6_route_seq_show(struct seq_file *seq, void *v)
--
1.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox