Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/1] net sched actions: Add support for user cookies
From: Jamal Hadi Salim @ 2017-01-14 14:59 UTC (permalink / raw)
  To: davem
  Cc: netdev, jiri, paulb, john.fastabend, simon.horman, mrv, hadarh,
	ogerlitz, roid, xiyou.wangcong, daniel, Jamal Hadi Salim,
	Jamal Hadi Salim

From: Jamal Hadi Salim <hadi@mojatatu.com>

Introduce optional 128-bit action cookie.
Like all other cookie schemes in the networking world (eg in protocols
like http or existing kernel fib protocol field, etc) the idea is to save
user state that when retrieved serves as a correlator. The kernel
_should not_ intepret it.  The user can store whatever they wish in the
128 bits.

Sample exercise(using two 64bit values to represent the 128 bits):

.. create an accept action with cookie 0xA:0xa0a0a0a0a0a0a0
sudo $TC actions add action ok index 1 cookie 0xA 0xa0a0a0a0a0a0a0

.. dump all gact actions..
sudo $TC -s actions ls action gact

	action order 0: gact action pass
	 random type none pass val 0
	 index 1 ref 2 bind 1 installed 1221 sec used 27 sec
 	Action statistics:
	Sent 373248 bytes 5056 pkt (dropped 0, overlimits 0 requeues 0)
	backlog 0b 0p requeues 0
	 cookie(0000000a:00000000:a0a0a0a0:00a0a0a0)

.. bind the accept action to a filter..
sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 1

... send some traffic..
$ ping 127.0.0.1 -c 3
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.020 ms
64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.027 ms
64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.038 ms

--- 127.0.0.1 ping statistics ---
3 packets transmitted, 3 received, 0% packet loss, time 2109ms
rtt min/avg/max/mdev = 0.020/0.028/0.038/0.008 ms 1

... show some stats
$ sudo $TC -s actions get  action gact index 1

	action order 1: gact action pass
	 random type none pass val 0
	 index 1 ref 3 bind 1 installed 2182 sec used 1 sec
 	Action statistics:
	Sent 700344 bytes 9486 pkt (dropped 0, overlimits 0 requeues 0)
	backlog 0b 0p requeues 0

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 include/net/act_api.h        |  7 +++++++
 include/uapi/linux/pkt_cls.h |  7 +++++++
 net/sched/act_api.c          | 27 +++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 1d71644..b948db9 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -20,6 +20,12 @@ struct tcf_hashinfo {
 
 struct tc_action_ops;
 
+union act_cookie {
+	u16 ck16[8];
+	u32 ck32[4];
+	u64 ck64[2];
+};
+
 struct tc_action {
 	const struct tc_action_ops	*ops;
 	__u32				type; /* for backward compat(TCA_OLD_COMPAT) */
@@ -41,6 +47,7 @@ struct tc_action {
 	struct rcu_head			tcfa_rcu;
 	struct gnet_stats_basic_cpu __percpu *cpu_bstats;
 	struct gnet_stats_queue __percpu *cpu_qstats;
+	union act_cookie	*ck;
 };
 #define tcf_head	common.tcfa_head
 #define tcf_index	common.tcfa_index
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 1e5e1dd..6379af3 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -4,6 +4,12 @@
 #include <linux/types.h>
 #include <linux/pkt_sched.h>
 
+union u_act_cookie {
+	__u16 ck16[8];
+	__u32 ck32[4];
+	__u64 ck64[2];
+};
+
 /* Action attributes */
 enum {
 	TCA_ACT_UNSPEC,
@@ -12,6 +18,7 @@ enum {
 	TCA_ACT_INDEX,
 	TCA_ACT_STATS,
 	TCA_ACT_PAD,
+	TCA_ACT_COOKIE,
 	__TCA_ACT_MAX
 };
 
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f04715a..85e77181 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -33,6 +33,7 @@ static void free_tcf(struct rcu_head *head)
 
 	free_percpu(p->cpu_bstats);
 	free_percpu(p->cpu_qstats);
+	kfree(p->ck);
 	kfree(p);
 }
 
@@ -464,8 +465,8 @@ int tcf_action_destroy(struct list_head *actions, int bind)
 	return a->ops->dump(skb, a, bind, ref);
 }
 
-int
-tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
+int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind,
+		      int ref)
 {
 	int err = -EINVAL;
 	unsigned char *b = skb_tail_pointer(skb);
@@ -475,6 +476,12 @@ int tcf_action_destroy(struct list_head *actions, int bind)
 		goto nla_put_failure;
 	if (tcf_action_copy_stats(skb, a, 0))
 		goto nla_put_failure;
+	if (a->ck) {
+		if (nla_put(skb, TCA_ACT_COOKIE, sizeof(union act_cookie),
+			    a->ck))
+			goto nla_put_failure;
+	}
+
 	nest = nla_nest_start(skb, TCA_OPTIONS);
 	if (nest == NULL)
 		goto nla_put_failure;
@@ -575,6 +582,22 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
 	if (err < 0)
 		goto err_mod;
 
+	if (tb[TCA_ACT_COOKIE]) {
+		if (nla_len(tb[TCA_ACT_COOKIE]) != sizeof(union act_cookie)) {
+			err = -EINVAL;
+			goto err_mod;
+		}
+
+		a->ck = kzalloc(sizeof(union act_cookie), GFP_KERNEL);
+		if (unlikely(!a->ck)) {
+			err = -ENOMEM;
+			goto err_mod;
+		}
+
+		memcpy((void *)a->ck, nla_data(tb[TCA_ACT_COOKIE]),
+		       sizeof(union act_cookie));
+	}
+
 	/* module count goes up only when brand new policy is created
 	 * if it exists and is only bound to in a_o->init() then
 	 * ACT_P_CREATED is not returned (a zero is).
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next] net/sched: cls_flower: Add user specified data
From: Jiri Pirko @ 2017-01-14 14:48 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: John Fastabend, Paul Blakey, David S. Miller, netdev, Jiri Pirko,
	Hadar Hen Zion, Or Gerlitz, Roi Dayan, Roman Mashak, Simon Horman
In-Reply-To: <68e41fe4-5c4c-e159-fbfc-17d06638707d@mojatatu.com>

Sat, Jan 14, 2017 at 01:56:35PM CET, jhs@mojatatu.com wrote:
>On 17-01-09 01:23 PM, John Fastabend wrote:
>> On 17-01-08 09:19 AM, Jiri Pirko wrote:
>
>[..]
>> > This should never be interpreted by kernel. I think this would be good
>> > to make clear in the comment in the code.
>> > 
>> 
>> Ah OK I had assumed you would be pushing this via tc_cls_flower_offload into
>> the driver in a follow up patch. But if it lives in kernel space as opaque
>> cookie guess its no different then other id fields order/prio/cookie.
>> 
>> Thanks for clarifying.
>
>
>I think the feature makes a lot of sense (as is the action variant).
>But can we make it:
>a) fixed size

Can you elaborate on why is this needed?


>b) apply to all classifiers
>c) please post a usage example via iproute2/tc
>
>I am going to post the action variant in the next while - will do some more
>testing first.

I believe we have to make the cls and ats cookies exactly the same.

>
>cheers,
>jamal

^ permalink raw reply

* Re: [PATCH] can: Fix kernel panic at security_sock_rcv_skb
From: Oliver Hartkopp @ 2017-01-14 13:53 UTC (permalink / raw)
  To: Liu Shuo, Eric Dumazet
  Cc: linux-kernel, yanmin_zhang, shuox.liu, Zhang Yanmin, He, Bo,
	Marc Kleine-Budde, David S. Miller, open list:CAN NETWORK LAYER,
	open list:NETWORKING [GENERAL]
In-Reply-To: <20170114034330.GA30070@shuo-desktop.sh.intel.com>

Hello Eric,

On 01/14/2017 04:43 AM, Liu Shuo wrote:
> On Thu 12.Jan'17 at 17:33:38 +0100, Oliver Hartkopp wrote:
>> On 01/12/2017 02:01 PM, Eric Dumazet wrote:

>>> The main problem seems that the sockets themselves are not RCU
>>> protected.
>>>
>>> If CAN uses RCU for delivery, then sockets should be freed only after
>>> one RCU grace period.
>>>
>>> On recent kernels, following patch could help :
>>>
>>
>> Thanks Eric!
>>
>> @Liu ShuoX: Can you check if Eric's suggestion fixes the issue in your
>> setup?
> Sorry for late reply. I was OOO yesterday.
> With Eric's hint, i just found his patch that "net: add SOCK_RCU_FREE
> socket flag" in the latest kernel. With backporting this one plus Eric's
> following patch, it fixs my failure.

what would be the best approach to fix this issue - even in stable kernels?

E.g. would this change be ok for a stable as a quick fix?

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 1108079d934f..6b974c2b66ef 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -112,6 +112,7 @@ EXPORT_SYMBOL(can_ioctl);

  static void can_sock_destruct(struct sock *sk)
  {
+       synchronize_rcu();
         skb_queue_purge(&sk->sk_receive_queue);
  }

And once this arrived in the mainline tree your suggested patch could be 
applied?

In any case we should not forget to give Reported-by credits to Liu.

Best regards,
Oliver


^ permalink raw reply related

* Re: [PATCH net-next] net/sched: cls_flower: Add user specified data
From: Jamal Hadi Salim @ 2017-01-14 13:06 UTC (permalink / raw)
  To: Paul Blakey, John Fastabend, David S. Miller, netdev
  Cc: Jiri Pirko, Hadar Hen Zion, Or Gerlitz, Roi Dayan, Roman Mashak,
	Simon Horman
In-Reply-To: <606f64cb-1744-d44f-8fd1-a4bdb1ca872f@mellanox.com>

On 17-01-03 07:22 AM, Paul Blakey wrote:
>
> I don't mind having it on TC level but I didn't want to intervene with
> all filters/TC.
>

Please do make it for all classifiers. I described a use case for u32
where the cookie could be used to pretty print the output on a
dump/get.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH net-next] net/sched: cls_flower: Add user specified data
From: Jamal Hadi Salim @ 2017-01-14 12:56 UTC (permalink / raw)
  To: John Fastabend, Jiri Pirko
  Cc: Paul Blakey, David S. Miller, netdev, Jiri Pirko, Hadar Hen Zion,
	Or Gerlitz, Roi Dayan, Roman Mashak, Simon Horman
In-Reply-To: <5873D523.4030301@gmail.com>

On 17-01-09 01:23 PM, John Fastabend wrote:
> On 17-01-08 09:19 AM, Jiri Pirko wrote:

[..]
>> This should never be interpreted by kernel. I think this would be good
>> to make clear in the comment in the code.
>>
>
> Ah OK I had assumed you would be pushing this via tc_cls_flower_offload into
> the driver in a follow up patch. But if it lives in kernel space as opaque
> cookie guess its no different then other id fields order/prio/cookie.
>
> Thanks for clarifying.


I think the feature makes a lot of sense (as is the action variant).
But can we make it:
a) fixed size
b) apply to all classifiers
c) please post a usage example via iproute2/tc

I am going to post the action variant in the next while - will do some 
more testing first.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH] cxgb4:  Remove redundant memset before memcpy
From: Tobias Klauser @ 2017-01-14 12:27 UTC (permalink / raw)
  To: Shyam Saini; +Cc: hariprasad, netdev, linux-kernel
In-Reply-To: <1484356660-19082-1-git-send-email-mayhs11saini@gmail.com>

On 2017-01-14 at 02:17:40 +0100, Shyam Saini <mayhs11saini@gmail.com> wrote:
> The region set by the call to memset, immediately overwritten by
> the subsequent call to memcpy and thus makes the  memset redundant.
> 
> Also remove the memset((&info, 0, sizeof(info)) on line 398 because
> info is memcpy()'ed to before being used in the loop and it isn't
> used outside of the loop.
> 
> Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>

Reviewed-by: Tobias Klauser <tklauser@distanz.ch>

^ permalink raw reply

* [PATCH v3 2/2] bpf: Add tests for the lpm trie map
From: Daniel Mack @ 2017-01-14 12:17 UTC (permalink / raw)
  To: ast; +Cc: dh.herrmann, daniel, netdev, davem, Daniel Mack
In-Reply-To: <20170114121727.14784-1-daniel@zonque.org>

From: David Herrmann <dh.herrmann@gmail.com>

The first part of this program runs randomized tests against the
lpm-bpf-map. It implements a "Trivial Longest Prefix Match" (tlpm)
based on simple, linear, single linked lists. The implementation
should be pretty straightforward.

Based on tlpm, this inserts randomized data into bpf-lpm-maps and
verifies the trie-based bpf-map implementation behaves the same way
as tlpm.

The second part uses 'real world' IPv4 and IPv6 addresses and tests
the trie with those.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 tools/testing/selftests/bpf/.gitignore     |   1 +
 tools/testing/selftests/bpf/Makefile       |   4 +-
 tools/testing/selftests/bpf/test_lpm_map.c | 358 +++++++++++++++++++++++++++++
 3 files changed, 361 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/test_lpm_map.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 071431b..d3b1c9b 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -1,3 +1,4 @@
 test_verifier
 test_maps
 test_lru_map
+test_lpm_map
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 7a5f245..064a3e5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,8 +1,8 @@
 CFLAGS += -Wall -O2 -I../../../../usr/include
 
-test_objs = test_verifier test_maps test_lru_map
+test_objs = test_verifier test_maps test_lru_map test_lpm_map
 
-TEST_PROGS := test_verifier test_maps test_lru_map test_kmod.sh
+TEST_PROGS := test_verifier test_maps test_lru_map test_lpm_map test_kmod.sh
 TEST_FILES := $(test_objs)
 
 all: $(test_objs)
diff --git a/tools/testing/selftests/bpf/test_lpm_map.c b/tools/testing/selftests/bpf/test_lpm_map.c
new file mode 100644
index 0000000..dd83f0b
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_lpm_map.c
@@ -0,0 +1,358 @@
+/*
+ * Randomized tests for eBPF longest-prefix-match maps
+ *
+ * This program runs randomized tests against the lpm-bpf-map. It implements a
+ * "Trivial Longest Prefix Match" (tlpm) based on simple, linear, singly linked
+ * lists. The implementation should be pretty straightforward.
+ *
+ * Based on tlpm, this inserts randomized data into bpf-lpm-maps and verifies
+ * the trie-based bpf-map implementation behaves the same way as tlpm.
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <inttypes.h>
+#include <linux/bpf.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+
+#include "bpf_sys.h"
+#include "bpf_util.h"
+
+struct tlpm_node {
+	struct tlpm_node *next;
+	size_t n_bits;
+	uint8_t key[];
+};
+
+static struct tlpm_node *tlpm_add(struct tlpm_node *list,
+				  const uint8_t *key,
+				  size_t n_bits)
+{
+	struct tlpm_node *node;
+	size_t n;
+
+	/* add new entry with @key/@n_bits to @list and return new head */
+
+	n = (n_bits + 7) / 8;
+	node = malloc(sizeof(*node) + n);
+	assert(node);
+
+	node->next = list;
+	node->n_bits = n_bits;
+	memcpy(node->key, key, n);
+
+	return node;
+}
+
+static void tlpm_clear(struct tlpm_node *list)
+{
+	struct tlpm_node *node;
+
+	/* free all entries in @list */
+
+	while ((node = list)) {
+		list = list->next;
+		free(node);
+	}
+}
+
+static struct tlpm_node *tlpm_match(struct tlpm_node *list,
+				    const uint8_t *key,
+				    size_t n_bits)
+{
+	struct tlpm_node *best = NULL;
+	size_t i;
+
+	/*
+	 * Perform longest prefix-match on @key/@n_bits. That is, iterate all
+	 * entries and match each prefix against @key. Remember the "best"
+	 * entry we find (i.e., the longest prefix that matches) and return it
+	 * to the caller when done.
+	 */
+
+	for ( ; list; list = list->next) {
+		for (i = 0; i < n_bits && i < list->n_bits; ++i) {
+			if ((key[i / 8] & (1 << (7 - i % 8))) !=
+			    (list->key[i / 8] & (1 << (7 - i % 8))))
+				break;
+		}
+
+		if (i >= list->n_bits) {
+			if (!best || i > best->n_bits)
+				best = list;
+		}
+	}
+
+	return best;
+}
+
+static void test_lpm_basic(void)
+{
+	struct tlpm_node *list = NULL, *t1, *t2;
+
+	/* very basic, static tests to verify tlpm works as expected */
+
+	assert(!tlpm_match(list, (uint8_t[]){ 0xff }, 8));
+
+	t1 = list = tlpm_add(list, (uint8_t[]){ 0xff }, 8);
+	assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff }, 8));
+	assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff, 0xff }, 16));
+	assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff, 0x00 }, 16));
+	assert(!tlpm_match(list, (uint8_t[]){ 0x7f }, 8));
+	assert(!tlpm_match(list, (uint8_t[]){ 0xfe }, 8));
+	assert(!tlpm_match(list, (uint8_t[]){ 0xff }, 7));
+
+	t2 = list = tlpm_add(list, (uint8_t[]){ 0xff, 0xff }, 16);
+	assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff }, 8));
+	assert(t2 == tlpm_match(list, (uint8_t[]){ 0xff, 0xff }, 16));
+	assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff, 0xff }, 15));
+	assert(!tlpm_match(list, (uint8_t[]){ 0x7f, 0xff }, 16));
+
+	tlpm_clear(list);
+}
+
+static void test_lpm_order(void)
+{
+	struct tlpm_node *t1, *t2, *l1 = NULL, *l2 = NULL;
+	size_t i, j;
+
+	/*
+	 * Verify the tlpm implementation works correctly regardless of the
+	 * order of entries. Insert a random set of entries into @l1, and copy
+	 * the same data in reverse order into @l2. Then verify a lookup of
+	 * random keys will yield the same result in both sets.
+	 */
+
+	for (i = 0; i < (1 << 12); ++i)
+		l1 = tlpm_add(l1, (uint8_t[]){
+					rand() % 0xff,
+					rand() % 0xff,
+				}, rand() % 16 + 1);
+
+	for (t1 = l1; t1; t1 = t1->next)
+		l2 = tlpm_add(l2, t1->key, t1->n_bits);
+
+	for (i = 0; i < (1 << 8); ++i) {
+		uint8_t key[] = { rand() % 0xff, rand() % 0xff };
+
+		t1 = tlpm_match(l1, key, 16);
+		t2 = tlpm_match(l2, key, 16);
+
+		assert(!t1 == !t2);
+		if (t1) {
+			assert(t1->n_bits == t2->n_bits);
+			for (j = 0; j < t1->n_bits; ++j)
+				assert((t1->key[j / 8] & (1 << (7 - j % 8))) ==
+				       (t2->key[j / 8] & (1 << (7 - j % 8))));
+		}
+	}
+
+	tlpm_clear(l1);
+	tlpm_clear(l2);
+}
+
+static void test_lpm_map(void)
+{
+	size_t i, j, n_matches, n_nodes, n_lookups;
+	struct tlpm_node *t, *list = NULL;
+	struct bpf_lpm_trie_key *key;
+	uint8_t value[8] = {};
+	int r, map;
+
+	/*
+	 * Compare behavior of tlpm vs. bpf-lpm. Create a randomized set of
+	 * prefixes and insert it into both tlpm and bpf-lpm. Then run some
+	 * randomized lookups and verify both maps return the same result.
+	 */
+
+	n_matches = 0;
+	n_nodes = 1 << 8;
+	n_lookups = 1 << 16;
+
+	key = alloca(sizeof(*key) + 4);
+	memset(key, 0, sizeof(*key) + 4);
+
+	map = bpf_map_create(BPF_MAP_TYPE_LPM_TRIE,
+			     sizeof(*key) + 4,
+			     sizeof(value),
+			     4096,
+			     BPF_F_NO_PREALLOC);
+	assert(map >= 0);
+
+	for (i = 0; i < n_nodes; ++i) {
+		value[0] = rand() & 0xff;
+		value[1] = rand() & 0xff;
+		value[2] = rand() & 0xff;
+		value[3] = rand() & 0xff;
+		value[4] = rand() % 33;
+
+		list = tlpm_add(list, value, value[4]);
+
+		key->prefixlen = value[4];
+		memcpy(key->data, value, 4);
+		r = bpf_map_update(map, key, value, 0);
+		assert(!r);
+	}
+
+	for (i = 0; i < n_lookups; ++i) {
+		uint8_t data[] = {
+			rand() % 0xff,
+			rand() % 0xff,
+			rand() % 0xff,
+			rand() % 0xff
+		};
+
+		t = tlpm_match(list, data, 32);
+
+		key->prefixlen = 32;
+		memcpy(key->data, data, 4);
+		r = bpf_map_lookup(map, key, value);
+		assert(!r || errno == ENOENT);
+		assert(!t == !!r);
+
+		if (t) {
+			++n_matches;
+			assert(t->n_bits == value[4]);
+			for (j = 0; j < t->n_bits; ++j)
+				assert((t->key[j / 8] & (1 << (7 - j % 8))) ==
+				       (value[j / 8] & (1 << (7 - j % 8))));
+		}
+	}
+
+	close(map);
+	tlpm_clear(list);
+
+	/*
+	 * With 255 random nodes in the map, we are pretty likely to match
+	 * something on every lookup. For statistics, use this:
+	 *
+	 *     printf("  nodes: %zu\n"
+	 *            "lookups: %zu\n"
+	 *            "matches: %zu\n", n_nodes, n_lookups, n_matches);
+	 */
+}
+
+/* Test the implementation with some 'real world' examples */
+
+static void test_lpm_ipaddr(void)
+{
+	struct bpf_lpm_trie_key *key_ipv4;
+	struct bpf_lpm_trie_key *key_ipv6;
+	size_t key_size_ipv4;
+	size_t key_size_ipv6;
+	int map_fd_ipv4;
+	int map_fd_ipv6;
+	__u64 value;
+
+	key_size_ipv4 = sizeof(*key_ipv4) + sizeof(__u32);
+	key_size_ipv6 = sizeof(*key_ipv6) + sizeof(__u32) * 4;
+	key_ipv4 = alloca(key_size_ipv4);
+	key_ipv6 = alloca(key_size_ipv6);
+
+	map_fd_ipv4 = bpf_map_create(BPF_MAP_TYPE_LPM_TRIE,
+				     key_size_ipv4, sizeof(value),
+				     100, BPF_F_NO_PREALLOC);
+	assert(map_fd_ipv4 >= 0);
+
+	map_fd_ipv6 = bpf_map_create(BPF_MAP_TYPE_LPM_TRIE,
+				     key_size_ipv6, sizeof(value),
+				     100, BPF_F_NO_PREALLOC);
+	assert(map_fd_ipv6 >= 0);
+
+	/* Fill data some IPv4 and IPv6 address ranges */
+	value = 1;
+	key_ipv4->prefixlen = 16;
+	inet_pton(AF_INET, "192.168.0.0", key_ipv4->data);
+	assert(bpf_map_update(map_fd_ipv4, key_ipv4, &value, 0) == 0);
+
+	value = 2;
+	key_ipv4->prefixlen = 24;
+	inet_pton(AF_INET, "192.168.0.0", key_ipv4->data);
+	assert(bpf_map_update(map_fd_ipv4, key_ipv4, &value, 0) == 0);
+
+	value = 3;
+	key_ipv4->prefixlen = 24;
+	inet_pton(AF_INET, "192.168.128.0", key_ipv4->data);
+	assert(bpf_map_update(map_fd_ipv4, key_ipv4, &value, 0) == 0);
+
+	value = 5;
+	key_ipv4->prefixlen = 24;
+	inet_pton(AF_INET, "192.168.1.0", key_ipv4->data);
+	assert(bpf_map_update(map_fd_ipv4, key_ipv4, &value, 0) == 0);
+
+	value = 4;
+	key_ipv4->prefixlen = 23;
+	inet_pton(AF_INET, "192.168.0.0", key_ipv4->data);
+	assert(bpf_map_update(map_fd_ipv4, key_ipv4, &value, 0) == 0);
+
+	value = 0xdeadbeef;
+	key_ipv6->prefixlen = 64;
+	inet_pton(AF_INET6, "2a00:1450:4001:814::200e", key_ipv6->data);
+	assert(bpf_map_update(map_fd_ipv6, key_ipv6, &value, 0) == 0);
+
+	/* Set tprefixlen to maximum for lookups */
+	key_ipv4->prefixlen = 32;
+	key_ipv6->prefixlen = 128;
+
+	/* Test some lookups that should come back with a value */
+	inet_pton(AF_INET, "192.168.128.23", key_ipv4->data);
+	assert(bpf_map_lookup(map_fd_ipv4, key_ipv4, &value) == 0);
+	assert(value == 3);
+
+	inet_pton(AF_INET, "192.168.0.1", key_ipv4->data);
+	assert(bpf_map_lookup(map_fd_ipv4, key_ipv4, &value) == 0);
+	assert(value == 2);
+
+	inet_pton(AF_INET6, "2a00:1450:4001:814::", key_ipv6->data);
+	assert(bpf_map_lookup(map_fd_ipv6, key_ipv6, &value) == 0);
+	assert(value == 0xdeadbeef);
+
+	inet_pton(AF_INET6, "2a00:1450:4001:814::1", key_ipv6->data);
+	assert(bpf_map_lookup(map_fd_ipv6, key_ipv6, &value) == 0);
+	assert(value == 0xdeadbeef);
+
+	/* Test some lookups that should not match any entry */
+	inet_pton(AF_INET, "10.0.0.1", key_ipv4->data);
+	assert(bpf_map_lookup(map_fd_ipv4, key_ipv4, &value) == -1 &&
+	       errno == ENOENT);
+
+	inet_pton(AF_INET, "11.11.11.11", key_ipv4->data);
+	assert(bpf_map_lookup(map_fd_ipv4, key_ipv4, &value) == -1 &&
+	       errno == ENOENT);
+
+	inet_pton(AF_INET6, "2a00:ffff::", key_ipv6->data);
+	assert(bpf_map_lookup(map_fd_ipv6, key_ipv6, &value) == -1 &&
+	       errno == ENOENT);
+
+	close(map_fd_ipv4);
+	close(map_fd_ipv6);
+}
+
+int main(void)
+{
+	struct rlimit limit  = { RLIM_INFINITY, RLIM_INFINITY };
+	int ret;
+
+	/* we want predictable, pseudo random tests */
+	srand(0xf00ba1);
+
+	/* allow unlimited locked memory */
+	ret = setrlimit(RLIMIT_MEMLOCK, &limit);
+	if (ret < 0)
+		perror("Unable to lift memlock rlimit");
+
+	test_lpm_basic();
+	test_lpm_order();
+	test_lpm_map();
+	test_lpm_ipaddr();
+
+	printf("test_lpm: OK\n");
+	return 0;
+}
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 1/2] bpf: add a longest prefix match trie map implementation
From: Daniel Mack @ 2017-01-14 12:17 UTC (permalink / raw)
  To: ast; +Cc: dh.herrmann, daniel, netdev, davem, Daniel Mack
In-Reply-To: <20170114121727.14784-1-daniel@zonque.org>

This trie implements a longest prefix match algorithm that can be used
to match IP addresses to a stored set of ranges.

Internally, data is stored in an unbalanced trie of nodes that has a
maximum height of n, where n is the prefixlen the trie was created
with.

Tries may be created with prefix lengths that are multiples of 8, in
the range from 8 to 2048. The key used for lookup and update operations
is a struct bpf_lpm_trie_key, and the value is a uint64_t.

The code carries more information about the internal implementation.

Signed-off-by: Daniel Mack <daniel@zonque.org>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/uapi/linux/bpf.h |   7 +
 kernel/bpf/Makefile      |   2 +-
 kernel/bpf/lpm_trie.c    | 493 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 501 insertions(+), 1 deletion(-)
 create mode 100644 kernel/bpf/lpm_trie.c

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0eb0e87..d564277 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -63,6 +63,12 @@ struct bpf_insn {
 	__s32	imm;		/* signed immediate constant */
 };
 
+/* Key of an a BPF_MAP_TYPE_LPM_TRIE entry */
+struct bpf_lpm_trie_key {
+	__u32	prefixlen;	/* up to 32 for AF_INET, 128 for AF_INET6 */
+	__u8	data[0];	/* Arbitrary size */
+};
+
 /* BPF syscall commands, see bpf(2) man-page for details. */
 enum bpf_cmd {
 	BPF_MAP_CREATE,
@@ -89,6 +95,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_CGROUP_ARRAY,
 	BPF_MAP_TYPE_LRU_HASH,
 	BPF_MAP_TYPE_LRU_PERCPU_HASH,
+	BPF_MAP_TYPE_LPM_TRIE,
 };
 
 enum bpf_prog_type {
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 1276474..e1ce4f4 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1,7 +1,7 @@
 obj-y := core.o
 
 obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o
-obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o
+obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o
 ifeq ($(CONFIG_PERF_EVENTS),y)
 obj-$(CONFIG_BPF_SYSCALL) += stackmap.o
 endif
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
new file mode 100644
index 0000000..1c1ad27
--- /dev/null
+++ b/kernel/bpf/lpm_trie.c
@@ -0,0 +1,493 @@
+/*
+ * Longest prefix match list implementation
+ *
+ * Copyright (c) 2016,2017 Daniel Mack
+ * Copyright (c) 2016 David Herrmann
+ *
+ * This file is subject to the terms and conditions of version 2 of the GNU
+ * General Public License.  See the file COPYING in the main directory of the
+ * Linux distribution for more details.
+ */
+
+#include <linux/bpf.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/vmalloc.h>
+#include <net/ipv6.h>
+
+/* Intermediate node */
+#define LPM_TREE_NODE_FLAG_IM BIT(0)
+
+struct lpm_trie_node;
+
+struct lpm_trie_node {
+	struct rcu_head rcu;
+	struct lpm_trie_node __rcu	*child[2];
+	u32				prefixlen;
+	u32				flags;
+	u8				data[0];
+};
+
+struct lpm_trie {
+	struct bpf_map			map;
+	struct lpm_trie_node __rcu	*root;
+	size_t				n_entries;
+	size_t				max_prefixlen;
+	size_t				data_size;
+	raw_spinlock_t			lock;
+};
+
+/* This trie implements a longest prefix match algorithm that can be used to
+ * match IP addresses to a stored set of ranges.
+ *
+ * Data stored in @data of struct bpf_lpm_key and struct lpm_trie_node is
+ * interpreted as big endian, so data[0] stores the most significant byte.
+ *
+ * Match ranges are internally stored in instances of struct lpm_trie_node
+ * which each contain their prefix length as well as two pointers that may
+ * lead to more nodes containing more specific matches. Each node also stores
+ * a value that is defined by and returned to userspace via the update_elem
+ * and lookup functions.
+ *
+ * For instance, let's start with a trie that was created with a prefix length
+ * of 32, so it can be used for IPv4 addresses, and one single element that
+ * matches 192.168.0.0/16. The data array would hence contain
+ * [0xc0, 0xa8, 0x00, 0x00] in big-endian notation. This documentation will
+ * stick to IP-address notation for readability though.
+ *
+ * As the trie is empty initially, the new node (1) will be places as root
+ * node, denoted as (R) in the example below. As there are no other node, both
+ * child pointers are %NULL.
+ *
+ *              +----------------+
+ *              |       (1)  (R) |
+ *              | 192.168.0.0/16 |
+ *              |    value: 1    |
+ *              |   [0]    [1]   |
+ *              +----------------+
+ *
+ * Next, let's add a new node (2) matching 192.168.0.0/24. As there is already
+ * a node with the same data and a smaller prefix (ie, a less specific one),
+ * node (2) will become a child of (1). In child index depends on the next bit
+ * that is outside of what (1) matches, and that bit is 0, so (2) will be
+ * child[0] of (1):
+ *
+ *              +----------------+
+ *              |       (1)  (R) |
+ *              | 192.168.0.0/16 |
+ *              |    value: 1    |
+ *              |   [0]    [1]   |
+ *              +----------------+
+ *                   |
+ *    +----------------+
+ *    |       (2)      |
+ *    | 192.168.0.0/24 |
+ *    |    value: 2    |
+ *    |   [0]    [1]   |
+ *    +----------------+
+ *
+ * The child[1] slot of (1) could be filled with another node which has bit #17
+ * (the next bit after the ones that (1) matches on) set to 1. For instance,
+ * 192.168.128.0/24:
+ *
+ *              +----------------+
+ *              |       (1)  (R) |
+ *              | 192.168.0.0/16 |
+ *              |    value: 1    |
+ *              |   [0]    [1]   |
+ *              +----------------+
+ *                   |      |
+ *    +----------------+  +------------------+
+ *    |       (2)      |  |        (3)       |
+ *    | 192.168.0.0/24 |  | 192.168.128.0/24 |
+ *    |    value: 2    |  |     value: 3     |
+ *    |   [0]    [1]   |  |    [0]    [1]    |
+ *    +----------------+  +------------------+
+ *
+ * Let's add another node (4) to the game for 192.168.1.0/24. In order to place
+ * it, node (1) is looked at first, and because (4) of the semantics laid out
+ * above (bit #17 is 0), it would normally be attached to (1) as child[0].
+ * However, that slot is already allocated, so a new node is needed in between.
+ * That node does not have a value attached to it and it will never be
+ * returned to users as result of a lookup. It is only there to differentiate
+ * the traversal further. It will get a prefix as wide as necessary to
+ * distinguish its two children:
+ *
+ *                      +----------------+
+ *                      |       (1)  (R) |
+ *                      | 192.168.0.0/16 |
+ *                      |    value: 1    |
+ *                      |   [0]    [1]   |
+ *                      +----------------+
+ *                           |      |
+ *            +----------------+  +------------------+
+ *            |       (4)  (I) |  |        (3)       |
+ *            | 192.168.0.0/23 |  | 192.168.128.0/24 |
+ *            |    value: ---  |  |     value: 3     |
+ *            |   [0]    [1]   |  |    [0]    [1]    |
+ *            +----------------+  +------------------+
+ *                 |      |
+ *  +----------------+  +----------------+
+ *  |       (2)      |  |       (5)      |
+ *  | 192.168.0.0/24 |  | 192.168.1.0/24 |
+ *  |    value: 2    |  |     value: 5   |
+ *  |   [0]    [1]   |  |   [0]    [1]   |
+ *  +----------------+  +----------------+
+ *
+ * 192.168.1.1/32 would be a child of (5) etc.
+ *
+ * An intermediate node will be turned into a 'real' node on demand. In the
+ * example above, (4) would be re-used if 192.168.0.0/23 is added to the trie.
+ *
+ * A fully populated trie would have a height of 32 nodes, as the trie was
+ * created with a prefix length of 32.
+ *
+ * The lookup starts at the root node. If the current node matches and if there
+ * is a child that can be used to become more specific, the trie is traversed
+ * downwards. The last node in the traversal that is a non-intermediate one is
+ * returned.
+ */
+
+static inline int extract_bit(const u8 *data, size_t index)
+{
+	return !!(data[index / 8] & (1 << (7 - (index % 8))));
+}
+
+/**
+ * longest_prefix_match() - determine the longest prefix
+ * @trie:	The trie to get internal sizes from
+ * @node:	The node to operate on
+ * @key:	The key to compare to @node
+ *
+ * Determine the longest prefix of @node that matches the bits in @key.
+ */
+static size_t longest_prefix_match(const struct lpm_trie *trie,
+				   const struct lpm_trie_node *node,
+				   const struct bpf_lpm_trie_key *key)
+{
+	size_t prefixlen = 0;
+	size_t i;
+
+	for (i = 0; i < trie->data_size; i++) {
+		size_t b;
+
+		b = 8 - fls(node->data[i] ^ key->data[i]);
+		prefixlen += b;
+
+		if (prefixlen >= node->prefixlen || prefixlen >= key->prefixlen)
+			return min(node->prefixlen, key->prefixlen);
+
+		if (b < 8)
+			break;
+	}
+
+	return prefixlen;
+}
+
+/* Called from syscall or from eBPF program */
+static void *trie_lookup_elem(struct bpf_map *map, void *_key)
+{
+	struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
+	struct lpm_trie_node *node, *found = NULL;
+	struct bpf_lpm_trie_key *key = _key;
+
+	/* Start walking the trie from the root node ... */
+
+	for (node = rcu_dereference(trie->root); node;) {
+		unsigned int next_bit;
+		size_t matchlen;
+
+		/* Determine the longest prefix of @node that matches @key.
+		 * If it's the maximum possible prefix for this trie, we have
+		 * an exact match and can return it directly.
+		 */
+		matchlen = longest_prefix_match(trie, node, key);
+		if (matchlen == trie->max_prefixlen) {
+			found = node;
+			break;
+		}
+
+		/* If the number of bits that match is smaller than the prefix
+		 * length of @node, bail out and return the node we have seen
+		 * last in the traversal (ie, the parent).
+		 */
+		if (matchlen < node->prefixlen)
+			break;
+
+		/* Consider this node as return candidate unless it is an
+		 * artificially added intermediate one.
+		 */
+		if (!(node->flags & LPM_TREE_NODE_FLAG_IM))
+			found = node;
+
+		/* If the node match is fully satisfied, let's see if we can
+		 * become more specific. Determine the next bit in the key and
+		 * traverse down.
+		 */
+		next_bit = extract_bit(key->data, node->prefixlen);
+		node = rcu_dereference(node->child[next_bit]);
+	}
+
+	if (!found)
+		return NULL;
+
+	return found->data + trie->data_size;
+}
+
+static struct lpm_trie_node *lpm_trie_node_alloc(const struct lpm_trie *trie,
+						 const void *value)
+{
+	struct lpm_trie_node *node;
+	size_t size = sizeof(struct lpm_trie_node) + trie->data_size;
+
+	if (value)
+		size += trie->map.value_size;
+
+	node = kmalloc(size, GFP_ATOMIC | __GFP_NOWARN);
+	if (!node)
+		return NULL;
+
+	node->flags = 0;
+
+	if (value)
+		memcpy(node->data + trie->data_size, value,
+		       trie->map.value_size);
+
+	return node;
+}
+
+/* Called from syscall or from eBPF program */
+static int trie_update_elem(struct bpf_map *map,
+			    void *_key, void *value, u64 flags)
+{
+	struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
+	struct lpm_trie_node *node, *im_node, *new_node = NULL;
+	struct lpm_trie_node __rcu **slot;
+	struct bpf_lpm_trie_key *key = _key;
+	unsigned long irq_flags;
+	unsigned int next_bit;
+	size_t matchlen = 0;
+	int ret = 0;
+
+	if (unlikely(flags > BPF_EXIST))
+		return -EINVAL;
+
+	if (key->prefixlen > trie->max_prefixlen)
+		return -EINVAL;
+
+	raw_spin_lock_irqsave(&trie->lock, irq_flags);
+
+	/* Allocate and fill a new node */
+
+	if (trie->n_entries == trie->map.max_entries) {
+		ret = -ENOSPC;
+		goto out;
+	}
+
+	new_node = lpm_trie_node_alloc(trie, value);
+	if (!new_node) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	trie->n_entries++;
+
+	new_node->prefixlen = key->prefixlen;
+	RCU_INIT_POINTER(new_node->child[0], NULL);
+	RCU_INIT_POINTER(new_node->child[1], NULL);
+	memcpy(new_node->data, key->data, trie->data_size);
+
+	/* Now find a slot to attach the new node. To do that, walk the tree
+	 * from the root and match as many bits as possible for each node until
+	 * we either find an empty slot or a slot that needs to be replaced by
+	 * an intermediate node.
+	 */
+	slot = &trie->root;
+
+	while ((node = rcu_dereference_protected(*slot,
+					lockdep_is_held(&trie->lock)))) {
+		matchlen = longest_prefix_match(trie, node, key);
+
+		if (node->prefixlen != matchlen ||
+		    node->prefixlen == key->prefixlen ||
+		    node->prefixlen == trie->max_prefixlen)
+			break;
+
+		next_bit = extract_bit(key->data, node->prefixlen);
+		slot = &node->child[next_bit];
+	}
+
+	/* If the slot is empty (a free child pointer or an empty root),
+	 * simply assign the @new_node to that slot and be done.
+	 */
+	if (!node) {
+		rcu_assign_pointer(*slot, new_node);
+		goto out;
+	}
+
+	/* If the slot we picked already exists, replace it with @new_node
+	 * which already has the correct data array set.
+	 */
+	if (node->prefixlen == matchlen) {
+		new_node->child[0] = node->child[0];
+		new_node->child[1] = node->child[1];
+
+		if (!(node->flags & LPM_TREE_NODE_FLAG_IM))
+			trie->n_entries--;
+
+		rcu_assign_pointer(*slot, new_node);
+		kfree_rcu(node, rcu);
+
+		goto out;
+	}
+
+	/* If the new node matches the prefix completely, it must be inserted
+	 * as an ancestor. Simply insert it between @node and *@slot.
+	 */
+	if (matchlen == key->prefixlen) {
+		next_bit = extract_bit(node->data, matchlen);
+		rcu_assign_pointer(new_node->child[next_bit], node);
+		rcu_assign_pointer(*slot, new_node);
+		goto out;
+	}
+
+	im_node = lpm_trie_node_alloc(trie, NULL);
+	if (!im_node) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	im_node->prefixlen = matchlen;
+	im_node->flags |= LPM_TREE_NODE_FLAG_IM;
+	memcpy(im_node->data, node->data, trie->data_size);
+
+	/* Now determine which child to install in which slot */
+	if (extract_bit(key->data, matchlen)) {
+		rcu_assign_pointer(im_node->child[0], node);
+		rcu_assign_pointer(im_node->child[1], new_node);
+	} else {
+		rcu_assign_pointer(im_node->child[0], new_node);
+		rcu_assign_pointer(im_node->child[1], node);
+	}
+
+	/* Finally, assign the intermediate node to the determined spot */
+	rcu_assign_pointer(*slot, im_node);
+
+out:
+	if (ret) {
+		if (new_node)
+			trie->n_entries--;
+
+		kfree(new_node);
+		kfree(im_node);
+	}
+
+	raw_spin_unlock_irqrestore(&trie->lock, irq_flags);
+
+	return ret;
+}
+
+static struct bpf_map *trie_alloc(union bpf_attr *attr)
+{
+	size_t cost, cost_per_node;
+	struct lpm_trie *trie;
+	int ret;
+
+	/* check sanity of attributes */
+	if (attr->max_entries == 0 ||
+	    attr->map_flags != BPF_F_NO_PREALLOC ||
+	    attr->key_size < sizeof(struct bpf_lpm_trie_key) + 1   ||
+	    attr->key_size > sizeof(struct bpf_lpm_trie_key) + 256 ||
+	    attr->value_size == 0)
+		return ERR_PTR(-EINVAL);
+
+	trie = kzalloc(sizeof(*trie), GFP_USER | __GFP_NOWARN);
+	if (!trie)
+		return ERR_PTR(-ENOMEM);
+
+	/* copy mandatory map attributes */
+	trie->map.map_type = attr->map_type;
+	trie->map.key_size = attr->key_size;
+	trie->map.value_size = attr->value_size;
+	trie->map.max_entries = attr->max_entries;
+	trie->data_size = attr->key_size -
+			  offsetof(struct bpf_lpm_trie_key, data);
+	trie->max_prefixlen = trie->data_size * 8;
+
+	cost_per_node = sizeof(struct lpm_trie_node) +
+			attr->value_size + trie->data_size;
+	cost = sizeof(*trie) + attr->max_entries * cost_per_node;
+	trie->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
+
+	ret = bpf_map_precharge_memlock(trie->map.pages);
+	if (ret) {
+		kfree(trie);
+		return ERR_PTR(ret);
+	}
+
+	raw_spin_lock_init(&trie->lock);
+
+	return &trie->map;
+}
+
+static void trie_free(struct bpf_map *map)
+{
+	struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
+	struct lpm_trie_node __rcu **slot;
+	struct lpm_trie_node *node;
+
+	raw_spin_lock(&trie->lock);
+
+	/* Always start at the root and walk down to a node that has no
+	 * children. Then free that node, nullify its reference in the parent
+	 * and start over.
+	 */
+
+	for (;;) {
+		slot = &trie->root;
+
+		for (;;) {
+			node = rcu_dereference_protected(*slot,
+					lockdep_is_held(&trie->lock));
+			if (!node)
+				goto unlock;
+
+			if (rcu_access_pointer(node->child[0])) {
+				slot = &node->child[0];
+				continue;
+			}
+
+			if (rcu_access_pointer(node->child[1])) {
+				slot = &node->child[1];
+				continue;
+			}
+
+			kfree(node);
+			RCU_INIT_POINTER(*slot, NULL);
+			break;
+		}
+	}
+
+unlock:
+	raw_spin_unlock(&trie->lock);
+}
+
+static const struct bpf_map_ops trie_ops = {
+	.map_alloc = trie_alloc,
+	.map_free = trie_free,
+	.map_lookup_elem = trie_lookup_elem,
+	.map_update_elem = trie_update_elem,
+};
+
+static struct bpf_map_type_list trie_type __read_mostly = {
+	.ops = &trie_ops,
+	.type = BPF_MAP_TYPE_LPM_TRIE,
+};
+
+static int __init register_trie_map(void)
+{
+	bpf_register_map_type(&trie_type);
+	return 0;
+}
+late_initcall(register_trie_map);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 0/2] bpf: add longest prefix match map
From: Daniel Mack @ 2017-01-14 12:17 UTC (permalink / raw)
  To: ast; +Cc: dh.herrmann, daniel, netdev, davem, Daniel Mack

This patch set adds a longest prefix match algorithm that can be used
to match IP addresses to a stored set of ranges. It is exposed as a
bpf map type.
   
Internally, data is stored in an unbalanced tree of nodes that has a
maximum height of n, where n is the prefixlen the trie was created
with.
 
Note that this has nothing to do with fib or fib6 and is in no way meant
to replace or share code with it. It's rather a much simpler
implementation that is specifically written with bpf maps in mind.
 
Patch 1/2 adds the implementation, and 2/2 an extensive test suite.

Feedback is much appreciated.
 
 
Thanks,
Daniel

Changelog:

v2 -> v3:
	* Store both the key match data and the caller provided
	  value in the same byte array attached to a node. This
	  avoids double allocations
	* Bring back node->flags to distinguish between 'real'
	  and intermediate nodes
	* Fix comment style and some typos

v1 -> v2:
	* Turn spin lock into raw spinlock
	* Lock with irqsave options during trie_update_elem()
	* Return -ENOMEM properly from trie_alloc()
	* Force attr->flags == BPF_F_NO_PREALLOC during creation
	* Set trie->map.pages after creation to account for map memory
	* Allow arbitrary value sizes
	* Removed node->flags and denode intermediate nodes through
	  node->value == NULL instead

rfc -> v1:
	* Add __rcu pointer annotations to make sparse happy
	* Fold _lpm_trie_find_target_node() into its only caller
	* Fix some minor documentation issues


Daniel Mack (1):
  bpf: add a longest prefix match trie map implementation

David Herrmann (1):
  bpf: Add tests for the lpm trie map

 include/uapi/linux/bpf.h                   |   7 +
 kernel/bpf/Makefile                        |   2 +-
 kernel/bpf/lpm_trie.c                      | 493 +++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/.gitignore     |   1 +
 tools/testing/selftests/bpf/Makefile       |   4 +-
 tools/testing/selftests/bpf/test_lpm_map.c | 358 +++++++++++++++++++++
 6 files changed, 862 insertions(+), 3 deletions(-)
 create mode 100644 kernel/bpf/lpm_trie.c
 create mode 100644 tools/testing/selftests/bpf/test_lpm_map.c

-- 
2.9.3

^ permalink raw reply

* Re: [PATCH net-next] net/mlx5e: Support bpf_xdp_adjust_head()
From: Saeed Mahameed @ 2017-01-14 12:15 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Linux Netdev List, Saeed Mahameed, Tariq Toukan, Kernel Team
In-Reply-To: <20170113223141.GA87089@kafai-mba.local>

On Sat, Jan 14, 2017 at 12:31 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Fri, Jan 13, 2017 at 03:58:46PM +0200, Saeed Mahameed wrote:
>> >> > @@ -680,7 +687,7 @@ static inline void mlx5e_xmit_xdp_frame(struct mlx5e_rq *rq,
>> >> >         memset(wqe, 0, sizeof(*wqe));
>> >> >
>> >> >         /* copy the inline part */
>> >> > -       memcpy(eseg->inline_hdr_start, data, MLX5E_XDP_MIN_INLINE);
>> >> > +       memcpy(eseg->inline_hdr_start, xdp->data, MLX5E_XDP_MIN_INLINE);
>> >> >         eseg->inline_hdr_sz = cpu_to_be16(MLX5E_XDP_MIN_INLINE);
>> >> >
>> >> >         dseg = (struct mlx5_wqe_data_seg *)cseg + (MLX5E_XDP_TX_DS_COUNT - 1);
>> >> > @@ -706,22 +713,16 @@ static inline void mlx5e_xmit_xdp_frame(struct mlx5e_rq *rq,
>> >> >  static inline bool mlx5e_xdp_handle(struct mlx5e_rq *rq,
>> >> >                                     const struct bpf_prog *prog,
>> >> >                                     struct mlx5e_dma_info *di,
>> >> > -                                   void *data, u16 len)
>> >> > +                                   struct xdp_buff *xdp)
>> >> >  {
>> >> > -       struct xdp_buff xdp;
>> >> >         u32 act;
>> >> >
>> >> > -       if (!prog)
>> >> > -               return false;
>> >> > -
>> >> > -       xdp.data = data;
>> >> > -       xdp.data_end = xdp.data + len;
>> >> > -       act = bpf_prog_run_xdp(prog, &xdp);
>> >> > +       act = bpf_prog_run_xdp(prog, xdp);
>> >> >         switch (act) {
>> >> >         case XDP_PASS:
>> >> >                 return false;
>> >> >         case XDP_TX:
>> >> > -               mlx5e_xmit_xdp_frame(rq, di, MLX5_RX_HEADROOM, len);
>> >> > +               mlx5e_xmit_xdp_frame(rq, di, xdp);
>> >> >                 return true;
>> >> >         default:
>> >> >                 bpf_warn_invalid_xdp_action(act);
>> >> > @@ -737,18 +738,19 @@ static inline
>> >> >  struct sk_buff *skb_from_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
>> >> >                              u16 wqe_counter, u32 cqe_bcnt)
>> >> >  {
>> >> > +       const struct bpf_prog *xdp_prog;
>> >> >         struct mlx5e_dma_info *di;
>> >> >         struct sk_buff *skb;
>> >> >         void *va, *data;
>> >> > -       bool consumed;
>> >> > +       u16 rx_headroom = rq->rx_headroom;
>> >> >
>> >> >         di             = &rq->dma_info[wqe_counter];
>> >> >         va             = page_address(di->page);
>> >> > -       data           = va + MLX5_RX_HEADROOM;
>> >> > +       data           = va + rx_headroom;
>> >> >
>> >> >         dma_sync_single_range_for_cpu(rq->pdev,
>> >> >                                       di->addr,
>> >> > -                                     MLX5_RX_HEADROOM,
>> >> > +                                     rx_headroom,
>> >> >                                       rq->buff.wqe_sz,
>> >> >                                       DMA_FROM_DEVICE);
>> >> >         prefetch(data);
>> >> > @@ -760,11 +762,26 @@ struct sk_buff *skb_from_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
>> >> >         }
>> >> >
>> >> >         rcu_read_lock();
>> >> > -       consumed = mlx5e_xdp_handle(rq, READ_ONCE(rq->xdp_prog), di, data,
>> >> > -                                   cqe_bcnt);
>> >> > +       xdp_prog = READ_ONCE(rq->xdp_prog);
>> >> > +       if (xdp_prog) {
>> >> > +               struct xdp_buff xdp;
>> >> > +               bool consumed;
>> >> > +
>> >> > +               xdp.data = data;
>> >> > +               xdp.data_end = xdp.data + cqe_bcnt;
>> >> > +               xdp.data_hard_start = va;
>> >> > +
>> >> > +               consumed = mlx5e_xdp_handle(rq, xdp_prog, di, &xdp);
>> >> > +
>> >> > +               if (consumed) {
>> >> > +                       rcu_read_unlock();
>> >> > +                       return NULL; /* page/packet was consumed by XDP */
>> >> > +               }
>> >> > +
>> >> > +               rx_headroom = xdp.data - xdp.data_hard_start;
>> >> > +               cqe_bcnt = xdp.data_end - xdp.data;
>> >> > +       }
>> >>
>> >> This whole new logic belongs to mlx5e_xdp_handle, I would like to keep
>> >> xdp related code in one place.
>> >>
>> >> move the xdp_buff initialization back to there and keep the xdp_prog
>> >> check in mlx5e_xdp_handle;
>> >> +      xdp_prog = READ_ONCE(rq->xdp_prog);
>> >> +       if (!xdp_prog)
>> >> +                    return false
>> >>
>> >> you can remove "const struct bpf_prog *prog" parameter from
>> >> mlx5e_xdp_handle and take it directly from rq.
>> >>
>> >> if you need va for xdp_buff you can pass it as a paramter to
>> >> mlx5e_xdp_handle  as well:
>> >> mlx5e_xdp_handle(rq, di, va, data, cqe_bcnt);
>> >> Make sense ?
>> > I moved them because xdp.data could be adjusted which then
>> > rx_headroom and cqe_bcnt have to be adjusted accordingly
>> > in skb_from_cqe() also.
>> >
>> > I understand your point.  After another quick thought,
>> > the adjusted xdp.data is the only one that we want in skb_from_cqe().
>> > I will try to make mlx5e_xdp_handle() to return the adjusted xdp.data
>> > instead of bool.
>> >
>>
>> hmm, You also need the adjusted cqe_bcnt! this will make
>> mlx5e_xdp_handle stuffed with parameters,
>>
>> what if, in skb_from_cqe we warp data, rx_headroom and cqe_bcnt in one struct.
>>
>> struct mlx5e_rx_buff {
>> void *data;
>> u6 headroom;
>> u32 bcnt;
>> }
>>
>> initialize it at the start of skb_from_cqe:
>>
>> struct mlx5e_rx_buff rxb;
>>
>> rxb.headroom = rq->headroom;
>> rxb.data = va + rxb.headroom;
>> rxb.bcnt = cqe_bcnt;
>>
>> pass it to mlx5e_xdp_handle(rq, di, va, &rxb) in case xdp_prog is ON
>> and rxb needs adjustment.
>>
>> At the end use it to build the SKB:
>> skb = build_skb(va, RQ_PAGE_SIZE(rq));
>> skb_reserve(skb, rxb.headroom);
>> skb_put(skb, rxb.bcnt);
> How about something like this without introducing a new struct?
>
> -static inline bool mlx5e_xdp_handle(struct mlx5e_rq *rq,
> -                                   const struct bpf_prog *prog,
> -                                   struct mlx5e_dma_info *di,
> -                                   void *data, u16 len)
> +static inline int mlx5e_xdp_handle(struct mlx5e_rq *rq,
> +                                  struct mlx5e_dma_info *di,
> +                                  void *va, u16 *rx_headroom, u32 *len)

Also good.

>  {
> +       const struct bpf_prog *prog = READ_ONCE(rq->xdp_prog);
>         struct xdp_buff xdp;
>         u32 act;
>
>         if (!prog)
>                 return false;
>
> -       xdp.data = data;
> -       xdp.data_end = xdp.data + len;
> +       xdp.data = va + *rx_headroom;
> +       xdp.data_end = xdp.data + *len;
> +       xdp.data_hard_start = va;
> +
>         act = bpf_prog_run_xdp(prog, &xdp);
>         switch (act) {
>         case XDP_PASS:
> +               *rx_headroom = xdp.data - xdp.data_hard_start;
> +               *len = xdp.data_end - xdp.data;
>                 return false;
>         case XDP_TX:
> -               mlx5e_xmit_xdp_frame(rq, di, MLX5_RX_HEADROOM, len);
> +               mlx5e_xmit_xdp_frame(rq, di, &xdp);
>                 return true;
>         default:
>                 bpf_warn_invalid_xdp_action(act);
> @@ -740,15 +751,16 @@ struct sk_buff *skb_from_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
>         struct mlx5e_dma_info *di;
>         struct sk_buff *skb;
>         void *va, *data;
> +       u16 rx_headroom = rq->rx_headroom;
>         bool consumed;
>
>         di             = &rq->dma_info[wqe_counter];
>         va             = page_address(di->page);
> -       data           = va + MLX5_RX_HEADROOM;
> +       data           = va + rx_headroom;
>
>         dma_sync_single_range_for_cpu(rq->pdev,
>                                       di->addr,
> -                                     MLX5_RX_HEADROOM,
> +                                     rx_headroom,
>                                       rq->buff.wqe_sz,
>                                       DMA_FROM_DEVICE);
>         prefetch(data);
> @@ -760,8 +772,7 @@ struct sk_buff *skb_from_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
>         }
>
>         rcu_read_lock();
> -       consumed = mlx5e_xdp_handle(rq, READ_ONCE(rq->xdp_prog), di, data,
> -                                   cqe_bcnt);
> +       consumed = mlx5e_xdp_handle(rq, di, va, &rx_headroom, &cqe_bcnt);
>         rcu_read_unlock();
>         if (consumed)
>                 return NULL; /* page/packet was consumed by XDP */
> @@ -777,7 +788,7 @@ struct sk_buff *skb_from_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
>         page_ref_inc(di->page);
>         mlx5e_page_release(rq, di, true);
>
> -       skb_reserve(skb, MLX5_RX_HEADROOM);
> +       skb_reserve(skb, rx_headroom);
>         skb_put(skb, cqe_bcnt);
>
>         return skb;

^ permalink raw reply

* [PATCH] net: marvell: skge: use new api ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2017-01-14 12:08 UTC (permalink / raw)
  To: mlindner, stephen, davem; +Cc: netdev, linux-kernel, Philippe Reynes

The ethtool api {get|set}_settings is deprecated.
We move this driver to new api {get|set}_link_ksettings.

The callback set_link_ksettings no longer update the value
of advertising, as the struct ethtool_link_ksettings is
defined as const.

Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
 drivers/net/ethernet/marvell/skge.c |   63 ++++++++++++++++++++--------------
 1 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 9146a51..81106b7 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -300,65 +300,76 @@ static u32 skge_supported_modes(const struct skge_hw *hw)
 	return supported;
 }
 
-static int skge_get_settings(struct net_device *dev,
-			     struct ethtool_cmd *ecmd)
+static int skge_get_link_ksettings(struct net_device *dev,
+				   struct ethtool_link_ksettings *cmd)
 {
 	struct skge_port *skge = netdev_priv(dev);
 	struct skge_hw *hw = skge->hw;
+	u32 supported, advertising;
 
-	ecmd->transceiver = XCVR_INTERNAL;
-	ecmd->supported = skge_supported_modes(hw);
+	supported = skge_supported_modes(hw);
 
 	if (hw->copper) {
-		ecmd->port = PORT_TP;
-		ecmd->phy_address = hw->phy_addr;
+		cmd->base.port = PORT_TP;
+		cmd->base.phy_address = hw->phy_addr;
 	} else
-		ecmd->port = PORT_FIBRE;
+		cmd->base.port = PORT_FIBRE;
+
+	advertising = skge->advertising;
+	cmd->base.autoneg = skge->autoneg;
+	cmd->base.speed = skge->speed;
+	cmd->base.duplex = skge->duplex;
+
+	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
+						supported);
+	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
+						advertising);
 
-	ecmd->advertising = skge->advertising;
-	ecmd->autoneg = skge->autoneg;
-	ethtool_cmd_speed_set(ecmd, skge->speed);
-	ecmd->duplex = skge->duplex;
 	return 0;
 }
 
-static int skge_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
+static int skge_set_link_ksettings(struct net_device *dev,
+				   const struct ethtool_link_ksettings *cmd)
 {
 	struct skge_port *skge = netdev_priv(dev);
 	const struct skge_hw *hw = skge->hw;
 	u32 supported = skge_supported_modes(hw);
 	int err = 0;
+	u32 advertising;
+
+	ethtool_convert_link_mode_to_legacy_u32(&advertising,
+						cmd->link_modes.advertising);
 
-	if (ecmd->autoneg == AUTONEG_ENABLE) {
-		ecmd->advertising = supported;
+	if (cmd->base.autoneg == AUTONEG_ENABLE) {
+		advertising = supported;
 		skge->duplex = -1;
 		skge->speed = -1;
 	} else {
 		u32 setting;
-		u32 speed = ethtool_cmd_speed(ecmd);
+		u32 speed = cmd->base.speed;
 
 		switch (speed) {
 		case SPEED_1000:
-			if (ecmd->duplex == DUPLEX_FULL)
+			if (cmd->base.duplex == DUPLEX_FULL)
 				setting = SUPPORTED_1000baseT_Full;
-			else if (ecmd->duplex == DUPLEX_HALF)
+			else if (cmd->base.duplex == DUPLEX_HALF)
 				setting = SUPPORTED_1000baseT_Half;
 			else
 				return -EINVAL;
 			break;
 		case SPEED_100:
-			if (ecmd->duplex == DUPLEX_FULL)
+			if (cmd->base.duplex == DUPLEX_FULL)
 				setting = SUPPORTED_100baseT_Full;
-			else if (ecmd->duplex == DUPLEX_HALF)
+			else if (cmd->base.duplex == DUPLEX_HALF)
 				setting = SUPPORTED_100baseT_Half;
 			else
 				return -EINVAL;
 			break;
 
 		case SPEED_10:
-			if (ecmd->duplex == DUPLEX_FULL)
+			if (cmd->base.duplex == DUPLEX_FULL)
 				setting = SUPPORTED_10baseT_Full;
-			else if (ecmd->duplex == DUPLEX_HALF)
+			else if (cmd->base.duplex == DUPLEX_HALF)
 				setting = SUPPORTED_10baseT_Half;
 			else
 				return -EINVAL;
@@ -371,11 +382,11 @@ static int skge_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
 			return -EINVAL;
 
 		skge->speed = speed;
-		skge->duplex = ecmd->duplex;
+		skge->duplex = cmd->base.duplex;
 	}
 
-	skge->autoneg = ecmd->autoneg;
-	skge->advertising = ecmd->advertising;
+	skge->autoneg = cmd->base.autoneg;
+	skge->advertising = advertising;
 
 	if (netif_running(dev)) {
 		skge_down(dev);
@@ -875,8 +886,6 @@ static int skge_set_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom
 }
 
 static const struct ethtool_ops skge_ethtool_ops = {
-	.get_settings	= skge_get_settings,
-	.set_settings	= skge_set_settings,
 	.get_drvinfo	= skge_get_drvinfo,
 	.get_regs_len	= skge_get_regs_len,
 	.get_regs	= skge_get_regs,
@@ -899,6 +908,8 @@ static int skge_set_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom
 	.set_phys_id	= skge_set_phys_id,
 	.get_sset_count = skge_get_sset_count,
 	.get_ethtool_stats = skge_get_ethtool_stats,
+	.get_link_ksettings = skge_get_link_ksettings,
+	.set_link_ksettings = skge_set_link_ksettings,
 };
 
 /*
-- 
1.7.4.4

^ permalink raw reply related

* Re: [Patch net] atm: remove an unnecessary loop
From: Chas Williams @ 2017-01-14 11:34 UTC (permalink / raw)
  To: Cong Wang
  Cc: David Miller, Francois Romieu, Linux Kernel Network Developers,
	Michal Hocko, Andrey Konovalov
In-Reply-To: <CAM_iQpW+gg5OmBA+xFDVBh7uWcMmPkyQ6D5NCO1Om=YtMiAsNw@mail.gmail.com>

On Fri, 2017-01-13 at 16:30 -0800, Cong Wang wrote:
> On Fri, Jan 13, 2017 at 3:54 PM, Chas Williams <3chas3@gmail.com> wrote:
> > On Fri, 2017-01-13 at 10:20 -0800, Cong Wang wrote:
> >> On Fri, Jan 13, 2017 at 9:10 AM, David Miller <davem@davemloft.net> wrote:
> >> > From: Francois Romieu <romieu@fr.zoreil.com>
> >> > Date: Fri, 13 Jan 2017 01:07:00 +0100
> >> >
> >> >> Were alloc_skb moved one level up in the call stack, there would be
> >> >> no need to use the new wait api in the subsequent page, thus easing
> >> >> pre 3.19 longterm kernel maintenance (at least those on korg page).
> >> >>
> >> >> But it tastes a tad bit too masochistic.
> >> >
> >> > Lack of error handling of allocation failure is always a huge red
> >> > flag.  We even long ago tried to do something like this for TCP FIN
> >> > handling.
> >> >
> >> > It's dumb, it doesn't work.
> >> >
> >> > Therefore I agree that the correct fix is to move the SKB allocation
> >> > up one level to vcc_sendmsg() and make it handle errors properly.
> >>
> >> If you can justify API is not broken by doing that, I am more than happy
> >> to do it, as I already stated in the latter patch:
> >
> > The man page for sendmsg() allows for ENOMEM.  See below.
> >
> 
> Errno is just one part, you miss the behavior behind the logic.
> 
> >>
> >> "Of course, the logic itself is suspicious, other sendmsg()
> >> could handle skb allocation failure very well, not sure
> >> why ATM has to wait for a successful one here. But probably
> >> it is too late to change since the errno and behavior is
> >> visible to user-space. So just leave the logic as it is."
> >>
> >> For some reason, no one reads that patch. :-/
> >
> > I read it and I agree.  I think it should be moved up/conflated with
> > vcc_sendmsg().  vcc_sendmsg() can already return an errno for other
> > conditions so if so has written something where they are explicitly
> > not expecting a ENOMEM, we really can't help them.
> 
> Nope, the reason is never ENOMEM is expected or not. The current
> _behavior_ behind this logic might be relied on by user-space.
> The behavior here is, when allocation fails, kernel will retry under
> certain circumstances, for example, if any fatal signal pending,
> returns ERESTARTSYS, etc.. This is what I worry, not just ENOMEM
> or not, which is too obvious.

Yes, and that behavior is certainly wrong.  Proving that nothing relies
on it would be very difficult since this is a negative supposition.

It's not clear to me that it is a good idea to ignore the pending signal
and just send the data.  At best, this seems like the signal is getting
ignored when the program might actaully want to do something about it.
The way the vcc sockets work, you are almost always waiting for "space
available to send".  Since vcc_sendmsg() has always been able to return
ERESTARTSYS for this condition, this isn't exactly new behavior, it
could just happen (very) slightly more often.

The loop in alloc_tx() also ignores the MSG_DONTWAIT flag.  The user
might end up waiting after all.  So that seems broken as well.  If
someone is expecting to wait with MSG_DONTWAIT when memory pressure
is present, I can't help them.  They are insane.

> Of course, I could be too conservative, I'd rather not to break things
> for -stable at least.
> 
> Thanks.

^ permalink raw reply

* [PATCH] net: korina: use new api ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2017-01-14 11:33 UTC (permalink / raw)
  To: davem, mugunthanvnm, a, fw, jarod, f.fainelli
  Cc: netdev, linux-kernel, Philippe Reynes

The ethtool api {get|set}_settings is deprecated.
We move this driver to new api {get|set}_link_ksettings.

Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
 drivers/net/ethernet/korina.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
index 8037426..3e415b8 100644
--- a/drivers/net/ethernet/korina.c
+++ b/drivers/net/ethernet/korina.c
@@ -695,25 +695,27 @@ static void netdev_get_drvinfo(struct net_device *dev,
 	strlcpy(info->bus_info, lp->dev->name, sizeof(info->bus_info));
 }
 
-static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int netdev_get_link_ksettings(struct net_device *dev,
+				     struct ethtool_link_ksettings *cmd)
 {
 	struct korina_private *lp = netdev_priv(dev);
 	int rc;
 
 	spin_lock_irq(&lp->lock);
-	rc = mii_ethtool_gset(&lp->mii_if, cmd);
+	rc = mii_ethtool_get_link_ksettings(&lp->mii_if, cmd);
 	spin_unlock_irq(&lp->lock);
 
 	return rc;
 }
 
-static int netdev_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int netdev_set_link_ksettings(struct net_device *dev,
+				     const struct ethtool_link_ksettings *cmd)
 {
 	struct korina_private *lp = netdev_priv(dev);
 	int rc;
 
 	spin_lock_irq(&lp->lock);
-	rc = mii_ethtool_sset(&lp->mii_if, cmd);
+	rc = mii_ethtool_set_link_ksettings(&lp->mii_if, cmd);
 	spin_unlock_irq(&lp->lock);
 	korina_set_carrier(&lp->mii_if);
 
@@ -729,9 +731,9 @@ static u32 netdev_get_link(struct net_device *dev)
 
 static const struct ethtool_ops netdev_ethtool_ops = {
 	.get_drvinfo            = netdev_get_drvinfo,
-	.get_settings           = netdev_get_settings,
-	.set_settings           = netdev_set_settings,
 	.get_link               = netdev_get_link,
+	.get_link_ksettings     = netdev_get_link_ksettings,
+	.set_link_ksettings     = netdev_set_link_ksettings,
 };
 
 static int korina_alloc_ring(struct net_device *dev)
-- 
1.7.4.4

^ permalink raw reply related

* Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants
From: Leon Romanovsky @ 2017-01-14 10:56 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, Vlastimil Babka, David Rientjes, Mel Gorman,
	Johannes Weiner, Al Viro, linux-mm, LKML, Michal Hocko,
	Martin Schwidefsky, Heiko Carstens, Herbert Xu, Anton Vorontsov,
	Colin Cross, Kees Cook, Tony Luck, Rafael J. Wysocki, Ben Skeggs,
	Kent Overstreet, Santosh Raspatur, Hariprasad S, Tariq Toukan,
	Yishai Hadas, Dan Williams
In-Reply-To: <20170112153717.28943-6-mhocko@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 4757 bytes --]

On Thu, Jan 12, 2017 at 04:37:16PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Kent Overstreet <kent.overstreet@gmail.com>
> Cc: Santosh Raspatur <santosh@chelsio.com>
> Cc: Hariprasad S <hariprasad@chelsio.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>
> Cc: Yishai Hadas <yishaih@mellanox.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Oleg Drokin <oleg.drokin@intel.com>
> Cc: Andreas Dilger <andreas.dilger@intel.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Sterba <dsterba@suse.com>
> Cc: "Yan, Zheng" <zyan@redhat.com>
> Cc: Ilya Dryomov <idryomov@gmail.com>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  arch/s390/kvm/kvm-s390.c                           | 10 ++-----
>  crypto/lzo.c                                       |  4 +--
>  drivers/acpi/apei/erst.c                           |  8 ++---
>  drivers/char/agp/generic.c                         |  8 +----
>  drivers/gpu/drm/nouveau/nouveau_gem.c              |  4 +--
>  drivers/md/bcache/util.h                           | 12 ++------
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h    |  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--------------
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c           |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    | 31 ++++----------------
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c         |  9 ++----
>  drivers/net/ethernet/mellanox/mlx4/mr.c            |  9 ++----
>  drivers/nvdimm/dimm_devs.c                         |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +------
>  drivers/xen/evtchn.c                               | 14 +--------
>  fs/btrfs/ctree.c                                   |  9 ++----
>  fs/btrfs/ioctl.c                                   |  9 ++----
>  fs/btrfs/send.c                                    | 27 ++++++-----------
>  fs/ceph/file.c                                     |  9 ++----
>  fs/select.c                                        |  5 +---
>  fs/xattr.c                                         | 27 ++++++-----------
>  kernel/bpf/hashtab.c                               | 11 ++-----
>  lib/iov_iter.c                                     |  5 +---
>  mm/frame_vector.c                                  |  5 +---
>  net/ipv4/inet_hashtables.c                         |  6 +---
>  net/ipv4/tcp_metrics.c                             |  5 +---
>  net/mpls/af_mpls.c                                 |  5 +---
>  net/netfilter/x_tables.c                           | 34 ++++++----------------
>  net/netfilter/xt_recent.c                          |  5 +---
>  net/sched/sch_choke.c                              |  5 +---
>  net/sched/sch_fq_codel.c                           | 26 ++++-------------
>  net/sched/sch_hhf.c                                | 33 ++++++---------------
>  net/sched/sch_netem.c                              |  6 +---
>  net/sched/sch_sfq.c                                |  6 +---
>  security/keys/keyctl.c                             | 22 ++++----------
>  35 files changed, 96 insertions(+), 319 deletions(-)

Hi Michal,

I don't see mlx5_vzalloc in the changed list. Any reason why did you skip it?

 881 static inline void *mlx5_vzalloc(unsigned long size)
 882 {
 883         void *rtn;
 884
 885         rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
 886         if (!rtn)
 887                 rtn = vzalloc(size);
 888         return rtn;
 889 }

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/2] bpf: add a longest prefix match trie map implementation
From: Daniel Mack @ 2017-01-14 10:37 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: ast, dh.herrmann, daniel, netdev, davem
In-Reply-To: <20170113180139.GB49247@ast-mbp.thefacebook.com>

On 01/13/2017 07:01 PM, Alexei Starovoitov wrote:
> On Thu, Jan 12, 2017 at 06:29:21PM +0100, Daniel Mack wrote:
>> This trie implements a longest prefix match algorithm that can be used
>> to match IP addresses to a stored set of ranges.
>>
>> Internally, data is stored in an unbalanced trie of nodes that has a
>> maximum height of n, where n is the prefixlen the trie was created
>> with.
>>
>> Tries may be created with prefix lengths that are multiples of 8, in
>> the range from 8 to 2048. The key used for lookup and update operations
>> is a struct bpf_lpm_trie_key, and the value is a uint64_t.
>>
>> The code carries more information about the internal implementation.
>>
>> Signed-off-by: Daniel Mack <daniel@zonque.org>
>> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>>  include/uapi/linux/bpf.h |   7 +
>>  kernel/bpf/Makefile      |   2 +-
>>  kernel/bpf/lpm_trie.c    | 499 +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 507 insertions(+), 1 deletion(-)
>>  create mode 100644 kernel/bpf/lpm_trie.c

...

Thanks for spotting my typos! :)

>> +static struct lpm_trie_node *lpm_trie_node_alloc(const struct lpm_trie *trie,
>> +						 const void *value)
>> +{
>> +	struct lpm_trie_node *node;
>> +	gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
>> +
>> +	node = kmalloc(sizeof(struct lpm_trie_node) + trie->data_size, gfp);
>> +	if (!node)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	if (value) {
>> +		node->value = kmemdup(value, trie->map.value_size, gfp);
> 
> can you make value to be part of the node? similar to how hash map is done?
> that will help avoid 2nd allocation, will speedup insertion and will
> help converting this code to user pre-allocated elements.
> I suspect the concern was that for many inner nodes that value is null ?
> But in your use case the value_size will be == 0 eventually,
> so by embedding it when can save memory too, since 'value' pointer will
> be replaced with boolean present flag ?
> So potentially less memory and less cache misses?

Yes, that's a good idea. Implemented that now.

> Overall algorithm is indeed straightforward and simple which is great,
> but I would still like to see some performance numbers.

I'm not sure yet how to implement such a test in a meaningful way, tbh.
Given that the lookups have to be done one by one, I expect the syscall
overhead to be quite significant.

> Looks like
> the best case for single 32-bit element it needs 4 xors and compares
> which is fine. For mostly populate trie it's 4xors * 32 depth
> which is pretty good too, but cache misses on pointer walks may
> kill performance unless we're hitting the same path all the time.
> I think it's all acceptable due to simplicity of the implementation
> which we may improve later if it turns out to be a bottle neck for
> some use cases. We just need a baseline to have realistic expectations.

Yes, the maximum height of the trie is the number of bits in the prefix,
so for n bits, the iteration would at most take n steps to finish. For
each step, an xor and compare for n/8 bytes are needed.

As you say, the implementation could be improved under the hood if
someone spots a bottleneck somewhere.

I'll post a v3 with your comments addressed for further discussion.


Thanks,
Daniel

^ permalink raw reply

* [net-next 3/3] tipc: reduce risk of user starvation during link congestion
From: Jon Maloy @ 2017-01-03 15:26 UTC (permalink / raw)
  To: davem
  Cc: netdev, Al Viro, parthasarathy.bhuvaragan, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1483457208-29033-1-git-send-email-jon.maloy@ericsson.com>

The socket code currently handles link congestion by either blocking
and trying to send again when the congestion has abated, or just
returning to the user with -EAGAIN and let him re-try later.

This mechanism is prone to starvation, because the wakeup algorithm is
non-atomic. During the time the link issues a wakeup signal, until the
socket wakes up and re-attempts sending, other senders may have come
in between and occupied the free buffer space in the link. This in turn
may lead to a socket having to make many send attempts before it is
successful. In extremely loaded systems we have observed latency times
of several seconds before a low-priority socket is able to send out a
message.

In this commit, we simplify this mechanism and reduce the risk of the
described scenario happening. When a message is attempted sent via a
congested link, we now let it be added to the link's backlog queue
anyway, thus permitting an oversubscription of one message per source
socket. We still create a wakeup item and return an error code, hence
instructing the sender to block or stop sending. Only when enough space
has been freed up in the link's backlog queue do we issue a wakeup event
that allows the sender to continue with the next message, if any.

The fact that a socket now can consider a message sent even when the
link returns a congestion code means that the sending socket code can
be simplified. Also, since this is a good opportunity to get rid of the
obsolete 'mtu change' condition in the three socket send functions, we
now choose to refactor those functions completely.

Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bcast.c  |   6 +-
 net/tipc/link.c   |  75 +++++-------
 net/tipc/msg.h    |   2 -
 net/tipc/node.c   |  15 +--
 net/tipc/socket.c | 347 ++++++++++++++++++++++++------------------------------
 5 files changed, 194 insertions(+), 251 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index aa1babb..c35fad3 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -174,7 +174,7 @@ static void tipc_bcbase_xmit(struct net *net, struct sk_buff_head *xmitq)
  *                    and to identified node local sockets
  * @net: the applicable net namespace
  * @list: chain of buffers containing message
- * Consumes the buffer chain, except when returning -ELINKCONG
+ * Consumes the buffer chain.
  * Returns 0 if success, otherwise errno: -ELINKCONG,-EHOSTUNREACH,-EMSGSIZE
  */
 int tipc_bcast_xmit(struct net *net, struct sk_buff_head *list)
@@ -197,7 +197,7 @@ int tipc_bcast_xmit(struct net *net, struct sk_buff_head *list)
 	tipc_bcast_unlock(net);
 
 	/* Don't send to local node if adding to link failed */
-	if (unlikely(rc)) {
+	if (unlikely(rc && (rc != -ELINKCONG))) {
 		__skb_queue_purge(&rcvq);
 		return rc;
 	}
@@ -206,7 +206,7 @@ int tipc_bcast_xmit(struct net *net, struct sk_buff_head *list)
 	tipc_bcbase_xmit(net, &xmitq);
 	tipc_sk_mcast_rcv(net, &rcvq, &inputq);
 	__skb_queue_purge(list);
-	return 0;
+	return rc;
 }
 
 /* tipc_bcast_rcv - receive a broadcast packet, and deliver to rcv link
diff --git a/net/tipc/link.c b/net/tipc/link.c
index bda89bf..b758ca8 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -776,60 +776,47 @@ int tipc_link_timeout(struct tipc_link *l, struct sk_buff_head *xmitq)
 
 /**
  * link_schedule_user - schedule a message sender for wakeup after congestion
- * @link: congested link
- * @list: message that was attempted sent
+ * @l: congested link
+ * @hdr: header of message that is being sent
  * Create pseudo msg to send back to user when congestion abates
- * Does not consume buffer list
  */
-static int link_schedule_user(struct tipc_link *link, struct sk_buff_head *list)
+static int link_schedule_user(struct tipc_link *l, struct tipc_msg *hdr)
 {
-	struct tipc_msg *msg = buf_msg(skb_peek(list));
-	int imp = msg_importance(msg);
-	u32 oport = msg_origport(msg);
-	u32 addr = tipc_own_addr(link->net);
+	u32 dnode = tipc_own_addr(l->net);
+	u32 dport = msg_origport(hdr);
 	struct sk_buff *skb;
 
-	/* This really cannot happen...  */
-	if (unlikely(imp > TIPC_CRITICAL_IMPORTANCE)) {
-		pr_warn("%s<%s>, send queue full", link_rst_msg, link->name);
-		return -ENOBUFS;
-	}
-	/* Non-blocking sender: */
-	if (TIPC_SKB_CB(skb_peek(list))->wakeup_pending)
-		return -ELINKCONG;
-
 	/* Create and schedule wakeup pseudo message */
 	skb = tipc_msg_create(SOCK_WAKEUP, 0, INT_H_SIZE, 0,
-			      addr, addr, oport, 0, 0);
+			      dnode, l->addr, dport, 0, 0);
 	if (!skb)
 		return -ENOBUFS;
-	TIPC_SKB_CB(skb)->chain_sz = skb_queue_len(list);
-	TIPC_SKB_CB(skb)->chain_imp = imp;
-	skb_queue_tail(&link->wakeupq, skb);
-	link->stats.link_congs++;
+	msg_set_dest_droppable(buf_msg(skb), true);
+	TIPC_SKB_CB(skb)->chain_imp = msg_importance(hdr);
+	skb_queue_tail(&l->wakeupq, skb);
+	l->stats.link_congs++;
 	return -ELINKCONG;
 }
 
 /**
  * link_prepare_wakeup - prepare users for wakeup after congestion
- * @link: congested link
- * Move a number of waiting users, as permitted by available space in
- * the send queue, from link wait queue to node wait queue for wakeup
+ * @l: congested link
+ * Wake up a number of waiting users, as permitted by available space
+ * in the send queue
  */
 void link_prepare_wakeup(struct tipc_link *l)
 {
-	int pnd[TIPC_SYSTEM_IMPORTANCE + 1] = {0,};
-	int imp, lim;
 	struct sk_buff *skb, *tmp;
+	int imp, i = 0;
 
 	skb_queue_walk_safe(&l->wakeupq, skb, tmp) {
 		imp = TIPC_SKB_CB(skb)->chain_imp;
-		lim = l->backlog[imp].limit;
-		pnd[imp] += TIPC_SKB_CB(skb)->chain_sz;
-		if ((pnd[imp] + l->backlog[imp].len) >= lim)
+		if (l->backlog[imp].len < l->backlog[imp].limit) {
+			skb_unlink(skb, &l->wakeupq);
+			skb_queue_tail(l->inputq, skb);
+		} else if (i++ > 10) {
 			break;
-		skb_unlink(skb, &l->wakeupq);
-		skb_queue_tail(l->inputq, skb);
+		}
 	}
 }
 
@@ -869,8 +856,7 @@ void tipc_link_reset(struct tipc_link *l)
  * @list: chain of buffers containing message
  * @xmitq: returned list of packets to be sent by caller
  *
- * Consumes the buffer chain, except when returning -ELINKCONG,
- * since the caller then may want to make more send attempts.
+ * Consumes the buffer chain.
  * Returns 0 if success, or errno: -ELINKCONG, -EMSGSIZE or -ENOBUFS
  * Messages at TIPC_SYSTEM_IMPORTANCE are always accepted
  */
@@ -879,7 +865,7 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
 {
 	struct tipc_msg *hdr = buf_msg(skb_peek(list));
 	unsigned int maxwin = l->window;
-	unsigned int i, imp = msg_importance(hdr);
+	int imp = msg_importance(hdr);
 	unsigned int mtu = l->mtu;
 	u16 ack = l->rcv_nxt - 1;
 	u16 seqno = l->snd_nxt;
@@ -888,19 +874,22 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
 	struct sk_buff_head *backlogq = &l->backlogq;
 	struct sk_buff *skb, *_skb, *bskb;
 	int pkt_cnt = skb_queue_len(list);
+	int rc = 0;
 
-	/* Match msg importance against this and all higher backlog limits: */
-	if (!skb_queue_empty(backlogq)) {
-		for (i = imp; i <= TIPC_SYSTEM_IMPORTANCE; i++) {
-			if (unlikely(l->backlog[i].len >= l->backlog[i].limit))
-				return link_schedule_user(l, list);
-		}
-	}
 	if (unlikely(msg_size(hdr) > mtu)) {
 		skb_queue_purge(list);
 		return -EMSGSIZE;
 	}
 
+	/* Allow oversubscription of one data msg per source at congestion */
+	if (unlikely(l->backlog[imp].len >= l->backlog[imp].limit)) {
+		if (imp == TIPC_SYSTEM_IMPORTANCE) {
+			pr_warn("%s<%s>, link overflow", link_rst_msg, l->name);
+			return -ENOBUFS;
+		}
+		rc = link_schedule_user(l, hdr);
+	}
+
 	if (pkt_cnt > 1) {
 		l->stats.sent_fragmented++;
 		l->stats.sent_fragments += pkt_cnt;
@@ -946,7 +935,7 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
 		skb_queue_splice_tail_init(list, backlogq);
 	}
 	l->snd_nxt = seqno;
-	return 0;
+	return rc;
 }
 
 void tipc_link_advance_backlog(struct tipc_link *l, struct sk_buff_head *xmitq)
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 8d40861..850ae0e 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -98,8 +98,6 @@ struct tipc_skb_cb {
 	u32 bytes_read;
 	struct sk_buff *tail;
 	bool validated;
-	bool wakeup_pending;
-	u16 chain_sz;
 	u16 chain_imp;
 	u16 ackers;
 };
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 9d2f4c2..2883f6a 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1167,7 +1167,7 @@ static int __tipc_nl_add_node(struct tipc_nl_msg *msg, struct tipc_node *node)
  * @list: chain of buffers containing message
  * @dnode: address of destination node
  * @selector: a number used for deterministic link selection
- * Consumes the buffer chain, except when returning -ELINKCONG
+ * Consumes the buffer chain.
  * Returns 0 if success, otherwise: -ELINKCONG,-EHOSTUNREACH,-EMSGSIZE,-ENOBUF
  */
 int tipc_node_xmit(struct net *net, struct sk_buff_head *list,
@@ -1206,10 +1206,10 @@ int tipc_node_xmit(struct net *net, struct sk_buff_head *list,
 	spin_unlock_bh(&le->lock);
 	tipc_node_read_unlock(n);
 
-	if (likely(rc == 0))
-		tipc_bearer_xmit(net, bearer_id, &xmitq, &le->maddr);
-	else if (rc == -ENOBUFS)
+	if (unlikely(rc == -ENOBUFS))
 		tipc_node_link_down(n, bearer_id, false);
+	else
+		tipc_bearer_xmit(net, bearer_id, &xmitq, &le->maddr);
 
 	tipc_node_put(n);
 
@@ -1221,20 +1221,15 @@ int tipc_node_xmit(struct net *net, struct sk_buff_head *list,
  * messages, which will not be rejected
  * The only exception is datagram messages rerouted after secondary
  * lookup, which are rare and safe to dispose of anyway.
- * TODO: Return real return value, and let callers use
- * tipc_wait_for_sendpkt() where applicable
  */
 int tipc_node_xmit_skb(struct net *net, struct sk_buff *skb, u32 dnode,
 		       u32 selector)
 {
 	struct sk_buff_head head;
-	int rc;
 
 	skb_queue_head_init(&head);
 	__skb_queue_tail(&head, skb);
-	rc = tipc_node_xmit(net, &head, dnode, selector);
-	if (rc == -ELINKCONG)
-		kfree_skb(skb);
+	tipc_node_xmit(net, &head, dnode, selector);
 	return 0;
 }
 
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index fae6a55..d2f3539 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -67,12 +67,14 @@ enum {
  * @max_pkt: maximum packet size "hint" used when building messages sent by port
  * @portid: unique port identity in TIPC socket hash table
  * @phdr: preformatted message header used when sending messages
+ * #cong_links: list of congested links
  * @publications: list of publications for port
+ * @blocking_link: address of the congested link we are currently sleeping on
  * @pub_count: total # of publications port has made during its lifetime
  * @probing_state:
  * @conn_timeout: the time we can wait for an unresponded setup request
  * @dupl_rcvcnt: number of bytes counted twice, in both backlog and rcv queue
- * @link_cong: non-zero if owner must sleep because of link congestion
+ * @cong_link_cnt: number of congested links
  * @sent_unacked: # messages sent by socket, and not yet acked by peer
  * @rcv_unacked: # messages read by user, but not yet acked back to peer
  * @peer: 'connected' peer for dgram/rdm
@@ -87,13 +89,13 @@ struct tipc_sock {
 	u32 max_pkt;
 	u32 portid;
 	struct tipc_msg phdr;
-	struct list_head sock_list;
+	struct list_head cong_links;
 	struct list_head publications;
 	u32 pub_count;
 	uint conn_timeout;
 	atomic_t dupl_rcvcnt;
 	bool probe_unacked;
-	bool link_cong;
+	u16 cong_link_cnt;
 	u16 snt_unacked;
 	u16 snd_win;
 	u16 peer_caps;
@@ -118,8 +120,7 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
 static struct tipc_sock *tipc_sk_lookup(struct net *net, u32 portid);
 static int tipc_sk_insert(struct tipc_sock *tsk);
 static void tipc_sk_remove(struct tipc_sock *tsk);
-static int __tipc_send_stream(struct socket *sock, struct msghdr *m,
-			      size_t dsz);
+static int __tipc_sendstream(struct socket *sock, struct msghdr *m, size_t dsz);
 static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dsz);
 
 static const struct proto_ops packet_ops;
@@ -424,6 +425,7 @@ static int tipc_sk_create(struct net *net, struct socket *sock,
 	tsk = tipc_sk(sk);
 	tsk->max_pkt = MAX_PKT_DEFAULT;
 	INIT_LIST_HEAD(&tsk->publications);
+	INIT_LIST_HEAD(&tsk->cong_links);
 	msg = &tsk->phdr;
 	tn = net_generic(sock_net(sk), tipc_net_id);
 	tipc_msg_init(tn->own_addr, msg, TIPC_LOW_IMPORTANCE, TIPC_NAMED_MSG,
@@ -474,9 +476,14 @@ static void __tipc_shutdown(struct socket *sock, int error)
 	struct sock *sk = sock->sk;
 	struct tipc_sock *tsk = tipc_sk(sk);
 	struct net *net = sock_net(sk);
+	long timeout = CONN_TIMEOUT_DEFAULT;
 	u32 dnode = tsk_peer_node(tsk);
 	struct sk_buff *skb;
 
+	/* Avoid that hi-prio shutdown msgs bypass msgs in link wakeup queue */
+	tipc_wait_for_cond(sock, &timeout, (!tsk->cong_link_cnt &&
+					    !tsk_conn_cong(tsk)));
+
 	/* Reject all unreceived messages, except on an active connection
 	 * (which disconnects locally & sends a 'FIN+' to peer).
 	 */
@@ -547,7 +554,8 @@ static int tipc_release(struct socket *sock)
 
 	/* Reject any messages that accumulated in backlog queue */
 	release_sock(sk);
-
+	u32_list_purge(&tsk->cong_links);
+	tsk->cong_link_cnt = 0;
 	call_rcu(&tsk->rcu, tipc_sk_callback);
 	sock->sk = NULL;
 
@@ -690,7 +698,7 @@ static unsigned int tipc_poll(struct file *file, struct socket *sock,
 
 	switch (sk->sk_state) {
 	case TIPC_ESTABLISHED:
-		if (!tsk->link_cong && !tsk_conn_cong(tsk))
+		if (!tsk->cong_link_cnt && !tsk_conn_cong(tsk))
 			mask |= POLLOUT;
 		/* fall thru' */
 	case TIPC_LISTEN:
@@ -699,7 +707,7 @@ static unsigned int tipc_poll(struct file *file, struct socket *sock,
 			mask |= (POLLIN | POLLRDNORM);
 		break;
 	case TIPC_OPEN:
-		if (!tsk->link_cong)
+		if (!tsk->cong_link_cnt)
 			mask |= POLLOUT;
 		if (tipc_sk_type_connectionless(sk) &&
 		    (!skb_queue_empty(&sk->sk_receive_queue)))
@@ -718,63 +726,48 @@ static unsigned int tipc_poll(struct file *file, struct socket *sock,
  * @sock: socket structure
  * @seq: destination address
  * @msg: message to send
- * @dsz: total length of message data
- * @timeo: timeout to wait for wakeup
+ * @dlen: length of data to send
+ * @timeout: timeout to wait for wakeup
  *
  * Called from function tipc_sendmsg(), which has done all sanity checks
  * Returns the number of bytes sent on success, or errno
  */
 static int tipc_sendmcast(struct  socket *sock, struct tipc_name_seq *seq,
-			  struct msghdr *msg, size_t dsz, long timeo)
+			  struct msghdr *msg, size_t dlen, long timeout)
 {
 	struct sock *sk = sock->sk;
 	struct tipc_sock *tsk = tipc_sk(sk);
+	struct tipc_msg *hdr = &tsk->phdr;
 	struct net *net = sock_net(sk);
-	struct tipc_msg *mhdr = &tsk->phdr;
-	struct sk_buff_head pktchain;
-	struct iov_iter save = msg->msg_iter;
-	uint mtu;
+	int mtu = tipc_bcast_get_mtu(net);
+	struct sk_buff_head pkts;
 	int rc;
 
-	if (!timeo && tsk->link_cong)
-		return -ELINKCONG;
-
-	msg_set_type(mhdr, TIPC_MCAST_MSG);
-	msg_set_lookup_scope(mhdr, TIPC_CLUSTER_SCOPE);
-	msg_set_destport(mhdr, 0);
-	msg_set_destnode(mhdr, 0);
-	msg_set_nametype(mhdr, seq->type);
-	msg_set_namelower(mhdr, seq->lower);
-	msg_set_nameupper(mhdr, seq->upper);
-	msg_set_hdr_sz(mhdr, MCAST_H_SIZE);
-
-	skb_queue_head_init(&pktchain);
+	rc = tipc_wait_for_cond(sock, &timeout, !tsk->cong_link_cnt);
+	if (unlikely(rc))
+		return rc;
 
-new_mtu:
-	mtu = tipc_bcast_get_mtu(net);
-	rc = tipc_msg_build(mhdr, msg, 0, dsz, mtu, &pktchain);
-	if (unlikely(rc < 0))
+	msg_set_type(hdr, TIPC_MCAST_MSG);
+	msg_set_lookup_scope(hdr, TIPC_CLUSTER_SCOPE);
+	msg_set_destport(hdr, 0);
+	msg_set_destnode(hdr, 0);
+	msg_set_nametype(hdr, seq->type);
+	msg_set_namelower(hdr, seq->lower);
+	msg_set_nameupper(hdr, seq->upper);
+	msg_set_hdr_sz(hdr, MCAST_H_SIZE);
+
+	skb_queue_head_init(&pkts);
+	rc = tipc_msg_build(hdr, msg, 0, dlen, mtu, &pkts);
+	if (unlikely(rc != dlen))
 		return rc;
 
-	do {
-		rc = tipc_bcast_xmit(net, &pktchain);
-		if (likely(!rc))
-			return dsz;
-
-		if (rc == -ELINKCONG) {
-			tsk->link_cong = 1;
-			rc = tipc_wait_for_cond(sock, &timeo, !tsk->link_cong);
-			if (!rc)
-				continue;
-		}
-		__skb_queue_purge(&pktchain);
-		if (rc == -EMSGSIZE) {
-			msg->msg_iter = save;
-			goto new_mtu;
-		}
-		break;
-	} while (1);
-	return rc;
+	rc = tipc_bcast_xmit(net, &pkts);
+	if (unlikely(rc == -ELINKCONG)) {
+		tsk->cong_link_cnt = 1;
+		rc = 0;
+	}
+
+	return rc ? rc : dlen;
 }
 
 /**
@@ -898,35 +891,38 @@ static int tipc_sendmsg(struct socket *sock,
 	return ret;
 }
 
-static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dsz)
+static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 {
-	DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
 	struct sock *sk = sock->sk;
-	struct tipc_sock *tsk = tipc_sk(sk);
 	struct net *net = sock_net(sk);
-	struct tipc_msg *mhdr = &tsk->phdr;
-	u32 dnode, dport;
-	struct sk_buff_head pktchain;
-	bool is_connectionless = tipc_sk_type_connectionless(sk);
-	struct sk_buff *skb;
+	struct tipc_sock *tsk = tipc_sk(sk);
+	DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
+	long timeout = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
+	struct list_head *clinks = &tsk->cong_links;
+	bool syn = !tipc_sk_type_connectionless(sk);
+	struct tipc_msg *hdr = &tsk->phdr;
 	struct tipc_name_seq *seq;
-	struct iov_iter save;
-	u32 mtu;
-	long timeo;
-	int rc;
+	struct sk_buff_head pkts;
+	u32 type, inst, domain;
+	u32 dnode, dport;
+	int mtu, rc;
 
-	if (dsz > TIPC_MAX_USER_MSG_SIZE)
+	if (unlikely(dlen > TIPC_MAX_USER_MSG_SIZE))
 		return -EMSGSIZE;
+
 	if (unlikely(!dest)) {
-		if (is_connectionless && tsk->peer.family == AF_TIPC)
-			dest = &tsk->peer;
-		else
+		dest = &tsk->peer;
+		if (!syn || dest->family != AF_TIPC)
 			return -EDESTADDRREQ;
-	} else if (unlikely(m->msg_namelen < sizeof(*dest)) ||
-		   dest->family != AF_TIPC) {
-		return -EINVAL;
 	}
-	if (!is_connectionless) {
+
+	if (unlikely(m->msg_namelen < sizeof(*dest)))
+		return -EINVAL;
+
+	if (unlikely(dest->family != AF_TIPC))
+		return -EINVAL;
+
+	if (unlikely(syn)) {
 		if (sk->sk_state == TIPC_LISTEN)
 			return -EPIPE;
 		if (sk->sk_state != TIPC_OPEN)
@@ -938,72 +934,62 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dsz)
 			tsk->conn_instance = dest->addr.name.name.instance;
 		}
 	}
-	seq = &dest->addr.nameseq;
-	timeo = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
 
-	if (dest->addrtype == TIPC_ADDR_MCAST) {
-		return tipc_sendmcast(sock, seq, m, dsz, timeo);
-	} else if (dest->addrtype == TIPC_ADDR_NAME) {
-		u32 type = dest->addr.name.name.type;
-		u32 inst = dest->addr.name.name.instance;
-		u32 domain = dest->addr.name.domain;
+	seq = &dest->addr.nameseq;
+	if (dest->addrtype == TIPC_ADDR_MCAST)
+		return tipc_sendmcast(sock, seq, m, dlen, timeout);
 
+	if (dest->addrtype == TIPC_ADDR_NAME) {
+		type = dest->addr.name.name.type;
+		inst = dest->addr.name.name.instance;
+		domain = dest->addr.name.domain;
 		dnode = domain;
-		msg_set_type(mhdr, TIPC_NAMED_MSG);
-		msg_set_hdr_sz(mhdr, NAMED_H_SIZE);
-		msg_set_nametype(mhdr, type);
-		msg_set_nameinst(mhdr, inst);
-		msg_set_lookup_scope(mhdr, tipc_addr_scope(domain));
+		msg_set_type(hdr, TIPC_NAMED_MSG);
+		msg_set_hdr_sz(hdr, NAMED_H_SIZE);
+		msg_set_nametype(hdr, type);
+		msg_set_nameinst(hdr, inst);
+		msg_set_lookup_scope(hdr, tipc_addr_scope(domain));
 		dport = tipc_nametbl_translate(net, type, inst, &dnode);
-		msg_set_destnode(mhdr, dnode);
-		msg_set_destport(mhdr, dport);
+		msg_set_destnode(hdr, dnode);
+		msg_set_destport(hdr, dport);
 		if (unlikely(!dport && !dnode))
 			return -EHOSTUNREACH;
+
 	} else if (dest->addrtype == TIPC_ADDR_ID) {
 		dnode = dest->addr.id.node;
-		msg_set_type(mhdr, TIPC_DIRECT_MSG);
-		msg_set_lookup_scope(mhdr, 0);
-		msg_set_destnode(mhdr, dnode);
-		msg_set_destport(mhdr, dest->addr.id.ref);
-		msg_set_hdr_sz(mhdr, BASIC_H_SIZE);
+		msg_set_type(hdr, TIPC_DIRECT_MSG);
+		msg_set_lookup_scope(hdr, 0);
+		msg_set_destnode(hdr, dnode);
+		msg_set_destport(hdr, dest->addr.id.ref);
+		msg_set_hdr_sz(hdr, BASIC_H_SIZE);
 	}
 
-	skb_queue_head_init(&pktchain);
-	save = m->msg_iter;
-new_mtu:
+	/* Block or return if destination link is congested */
+	rc = tipc_wait_for_cond(sock, &timeout, !u32_find(clinks, dnode));
+	if (unlikely(rc))
+		return rc;
+
+	skb_queue_head_init(&pkts);
 	mtu = tipc_node_get_mtu(net, dnode, tsk->portid);
-	rc = tipc_msg_build(mhdr, m, 0, dsz, mtu, &pktchain);
-	if (rc < 0)
+	rc = tipc_msg_build(hdr, m, 0, dlen, mtu, &pkts);
+	if (unlikely(rc != dlen))
 		return rc;
 
-	do {
-		skb = skb_peek(&pktchain);
-		TIPC_SKB_CB(skb)->wakeup_pending = tsk->link_cong;
-		rc = tipc_node_xmit(net, &pktchain, dnode, tsk->portid);
-		if (likely(!rc)) {
-			if (!is_connectionless)
-				tipc_set_sk_state(sk, TIPC_CONNECTING);
-			return dsz;
-		}
-		if (rc == -ELINKCONG) {
-			tsk->link_cong = 1;
-			rc = tipc_wait_for_cond(sock, &timeo, !tsk->link_cong);
-			if (!rc)
-				continue;
-		}
-		__skb_queue_purge(&pktchain);
-		if (rc == -EMSGSIZE) {
-			m->msg_iter = save;
-			goto new_mtu;
-		}
-		break;
-	} while (1);
+	rc = tipc_node_xmit(net, &pkts, dnode, tsk->portid);
+	if (unlikely(rc == -ELINKCONG)) {
+		u32_push(clinks, dnode);
+		tsk->cong_link_cnt++;
+		rc = 0;
+	}
 
-	return rc;
+	if (unlikely(syn && !rc))
+		tipc_set_sk_state(sk, TIPC_CONNECTING);
+
+	return rc ? rc : dlen;
 }
 
 /**
- * tipc_send_stream - send stream-oriented data
+ * tipc_sendstream - send stream-oriented data
  * @sock: socket structure
  * @m: data to send
  * @dsz: total length of data to be transmitted
@@ -1013,97 +999,69 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dsz)
  * Returns the number of bytes sent on success (or partial success),
  * or errno if no data sent
  */
-static int tipc_send_stream(struct socket *sock, struct msghdr *m, size_t dsz)
+static int tipc_sendstream(struct socket *sock, struct msghdr *m, size_t dsz)
 {
 	struct sock *sk = sock->sk;
 	int ret;
 
 	lock_sock(sk);
-	ret = __tipc_send_stream(sock, m, dsz);
+	ret = __tipc_sendstream(sock, m, dsz);
 	release_sock(sk);
 
 	return ret;
 }
 
-static int __tipc_send_stream(struct socket *sock, struct msghdr *m, size_t dsz)
+static int __tipc_sendstream(struct socket *sock, struct msghdr *m, size_t dlen)
 {
 	struct sock *sk = sock->sk;
-	struct net *net = sock_net(sk);
-	struct tipc_sock *tsk = tipc_sk(sk);
-	struct tipc_msg *mhdr = &tsk->phdr;
-	struct sk_buff_head pktchain;
 	DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
-	u32 portid = tsk->portid;
-	int rc = -EINVAL;
-	long timeo;
-	u32 dnode;
-	uint mtu, send, sent = 0;
-	struct iov_iter save;
-	int hlen = MIN_H_SIZE;
-
-	/* Handle implied connection establishment */
-	if (unlikely(dest)) {
-		rc = __tipc_sendmsg(sock, m, dsz);
-		hlen = msg_hdr_sz(mhdr);
-		if (dsz && (dsz == rc))
-			tsk->snt_unacked = tsk_inc(tsk, dsz + hlen);
-		return rc;
-	}
-	if (dsz > (uint)INT_MAX)
-		return -EMSGSIZE;
-
-	if (unlikely(!tipc_sk_connected(sk))) {
-		if (sk->sk_state == TIPC_DISCONNECTING)
-			return -EPIPE;
-		else
-			return -ENOTCONN;
-	}
+	long timeout = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
+	struct tipc_sock *tsk = tipc_sk(sk);
+	struct tipc_msg *hdr = &tsk->phdr;
+	struct net *net = sock_net(sk);
+	struct sk_buff_head pkts;
+	u32 dnode = tsk_peer_node(tsk);
+	int send, sent = 0;
+	int rc = 0;
 
-	timeo = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
-	if (!timeo && tsk->link_cong)
-		return -ELINKCONG;
+	skb_queue_head_init(&pkts);
 
-	dnode = tsk_peer_node(tsk);
-	skb_queue_head_init(&pktchain);
+	if (unlikely(dlen > INT_MAX))
+		return -EMSGSIZE;
 
-next:
-	save = m->msg_iter;
-	mtu = tsk->max_pkt;
-	send = min_t(uint, dsz - sent, TIPC_MAX_USER_MSG_SIZE);
-	rc = tipc_msg_build(mhdr, m, sent, send, mtu, &pktchain);
-	if (unlikely(rc < 0))
+	/* Handle implicit connection setup */
+	if (unlikely(dest)) {
+		rc = __tipc_sendmsg(sock, m, dlen);
+		if (dlen && (dlen == rc))
+			tsk->snt_unacked = tsk_inc(tsk, dlen + msg_hdr_sz(hdr));
 		return rc;
+	}
 
 	do {
-		if (likely(!tsk_conn_cong(tsk))) {
-			rc = tipc_node_xmit(net, &pktchain, dnode, portid);
-			if (likely(!rc)) {
-				tsk->snt_unacked += tsk_inc(tsk, send + hlen);
-				sent += send;
-				if (sent == dsz)
-					return dsz;
-				goto next;
-			}
-			if (rc == -EMSGSIZE) {
-				__skb_queue_purge(&pktchain);
-				tsk->max_pkt = tipc_node_get_mtu(net, dnode,
-								 portid);
-				m->msg_iter = save;
-				goto next;
-			}
-			if (rc != -ELINKCONG)
-				break;
-
-			tsk->link_cong = 1;
-		}
-		rc = tipc_wait_for_cond(sock, &timeo,
-					(!tsk->link_cong &&
+		rc = tipc_wait_for_cond(sock, &timeout,
+					(!tsk->cong_link_cnt &&
 					 !tsk_conn_cong(tsk) &&
 					 tipc_sk_connected(sk)));
-	} while (!rc);
+		if (unlikely(rc))
+			break;
+
+		send = min_t(size_t, dlen - sent, TIPC_MAX_USER_MSG_SIZE);
+		rc = tipc_msg_build(hdr, m, sent, send, tsk->max_pkt, &pkts);
+		if (unlikely(rc != send))
+			break;
+
+		rc = tipc_node_xmit(net, &pkts, dnode, tsk->portid);
+		if (unlikely(rc == -ELINKCONG)) {
+			tsk->cong_link_cnt = 1;
+			rc = 0;
+		}
+		if (likely(!rc)) {
+			tsk->snt_unacked += tsk_inc(tsk, send + MIN_H_SIZE);
+			sent += send;
+		}
+	} while (sent < dlen && !rc);
 
-	__skb_queue_purge(&pktchain);
-	return sent ? sent : rc;
+	return rc ? rc : sent;
 }
 
 /**
@@ -1121,7 +1079,7 @@ static int tipc_send_packet(struct socket *sock, struct msghdr *m, size_t dsz)
 	if (dsz > TIPC_MAX_USER_MSG_SIZE)
 		return -EMSGSIZE;
 
-	return tipc_send_stream(sock, m, dsz);
+	return tipc_sendstream(sock, m, dsz);
 }
 
 /* tipc_sk_finish_conn - complete the setup of a connection
@@ -1688,6 +1646,7 @@ static bool filter_rcv(struct sock *sk, struct sk_buff *skb,
 	unsigned int limit = rcvbuf_limit(sk, skb);
 	int err = TIPC_OK;
 	int usr = msg_user(hdr);
+	u32 onode;
 
 	if (unlikely(msg_user(hdr) == CONN_MANAGER)) {
 		tipc_sk_proto_rcv(tsk, skb, xmitq);
@@ -1695,8 +1654,10 @@ static bool filter_rcv(struct sock *sk, struct sk_buff *skb,
 	}
 
 	if (unlikely(usr == SOCK_WAKEUP)) {
+		onode = msg_orignode(hdr);
 		kfree_skb(skb);
-		tsk->link_cong = 0;
+		u32_del(&tsk->cong_links, onode);
+		tsk->cong_link_cnt--;
 		sk->sk_write_space(sk);
 		return false;
 	}
@@ -2104,7 +2065,7 @@ static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags)
 		struct msghdr m = {NULL,};
 
 		tsk_advance_rx_queue(sk);
-		__tipc_send_stream(new_sock, &m, 0);
+		__tipc_sendstream(new_sock, &m, 0);
 	} else {
 		__skb_dequeue(&sk->sk_receive_queue);
 		__skb_queue_head(&new_sk->sk_receive_queue, buf);
@@ -2565,7 +2526,7 @@ static const struct proto_ops stream_ops = {
 	.shutdown	= tipc_shutdown,
 	.setsockopt	= tipc_setsockopt,
 	.getsockopt	= tipc_getsockopt,
-	.sendmsg	= tipc_send_stream,
+	.sendmsg	= tipc_sendstream,
 	.recvmsg	= tipc_recv_stream,
 	.mmap		= sock_no_mmap,
 	.sendpage	= sock_no_sendpage
-- 
2.7.4

^ permalink raw reply related

* [net-next 2/3] tipc: modify struct tipc_plist to be more versatile
From: Jon Maloy @ 2017-01-03 15:26 UTC (permalink / raw)
  To: davem
  Cc: netdev, Al Viro, parthasarathy.bhuvaragan, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1483457208-29033-1-git-send-email-jon.maloy@ericsson.com>

During multicast reception we currently use a simple linked list with
push/pop semantics to store port numbers.

We now see a need for a more generic list for storing values of type
u32. We therefore make some modifications to this list, while replacing
the prefix 'tipc_plist_' with 'u32_'. We also add a couple of new
functions which will come to use in the next commits.

Acked-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/name_table.c | 100 ++++++++++++++++++++++++++++++++++++--------------
 net/tipc/name_table.h |  21 ++++-------
 net/tipc/socket.c     |   8 ++--
 3 files changed, 83 insertions(+), 46 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index e190460..5a86df1 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -608,7 +608,7 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
  * Returns non-zero if any off-node ports overlap
  */
 int tipc_nametbl_mc_translate(struct net *net, u32 type, u32 lower, u32 upper,
-			      u32 limit, struct tipc_plist *dports)
+			      u32 limit, struct list_head *dports)
 {
 	struct name_seq *seq;
 	struct sub_seq *sseq;
@@ -633,7 +633,7 @@ int tipc_nametbl_mc_translate(struct net *net, u32 type, u32 lower, u32 upper,
 		info = sseq->info;
 		list_for_each_entry(publ, &info->node_list, node_list) {
 			if (publ->scope <= limit)
-				tipc_plist_push(dports, publ->ref);
+				u32_push(dports, publ->ref);
 		}
 
 		if (info->cluster_list_size != info->node_list_size)
@@ -1022,40 +1022,84 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
-void tipc_plist_push(struct tipc_plist *pl, u32 port)
+struct u32_item {
+	struct list_head list;
+	u32 value;
+};
+
+bool u32_find(struct list_head *l, u32 value)
 {
-	struct tipc_plist *nl;
+	struct u32_item *item;
 
-	if (likely(!pl->port)) {
-		pl->port = port;
-		return;
+	list_for_each_entry(item, l, list) {
+		if (item->value == value)
+			return true;
 	}
-	if (pl->port == port)
-		return;
-	list_for_each_entry(nl, &pl->list, list) {
-		if (nl->port == port)
-			return;
+	return false;
+}
+
+bool u32_push(struct list_head *l, u32 value)
+{
+	struct u32_item *item;
+
+	list_for_each_entry(item, l, list) {
+		if (item->value == value)
+			return false;
+	}
+	item = kmalloc(sizeof(*item), GFP_ATOMIC);
+	if (unlikely(!item))
+		return false;
+
+	item->value = value;
+	list_add(&item->list, l);
+	return true;
+}
+
+u32 u32_pop(struct list_head *l)
+{
+	struct u32_item *item;
+	u32 value = 0;
+
+	if (list_empty(l))
+		return 0;
+	item = list_first_entry(l, typeof(*item), list);
+	value = item->value;
+	list_del(&item->list);
+	kfree(item);
+	return value;
+}
+
+bool u32_del(struct list_head *l, u32 value)
+{
+	struct u32_item *item, *tmp;
+
+	list_for_each_entry_safe(item, tmp, l, list) {
+		if (item->value != value)
+			continue;
+		list_del(&item->list);
+		kfree(item);
+		return true;
 	}
-	nl = kmalloc(sizeof(*nl), GFP_ATOMIC);
-	if (nl) {
-		nl->port = port;
-		list_add(&nl->list, &pl->list);
+	return false;
+}
+
+void u32_list_purge(struct list_head *l)
+{
+	struct u32_item *item, *tmp;
+
+	list_for_each_entry_safe(item, tmp, l, list) {
+		list_del(&item->list);
+		kfree(item);
 	}
 }
 
-u32 tipc_plist_pop(struct tipc_plist *pl)
+int u32_list_len(struct list_head *l)
 {
-	struct tipc_plist *nl;
-	u32 port = 0;
+	struct u32_item *item;
+	int i = 0;
 
-	if (likely(list_empty(&pl->list))) {
-		port = pl->port;
-		pl->port = 0;
-		return port;
+	list_for_each_entry(item, l, list) {
+		i++;
 	}
-	nl = list_first_entry(&pl->list, typeof(*nl), list);
-	port = nl->port;
-	list_del(&nl->list);
-	kfree(nl);
-	return port;
+	return i;
 }
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 1524a73..c89bb3f 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -99,7 +99,7 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb);
 
 u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *node);
 int tipc_nametbl_mc_translate(struct net *net, u32 type, u32 lower, u32 upper,
-			      u32 limit, struct tipc_plist *dports);
+			      u32 limit, struct list_head *dports);
 struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
 					 u32 upper, u32 scope, u32 port_ref,
 					 u32 key);
@@ -116,18 +116,11 @@ void tipc_nametbl_unsubscribe(struct tipc_subscription *s);
 int tipc_nametbl_init(struct net *net);
 void tipc_nametbl_stop(struct net *net);
 
-struct tipc_plist {
-	struct list_head list;
-	u32 port;
-};
-
-static inline void tipc_plist_init(struct tipc_plist *pl)
-{
-	INIT_LIST_HEAD(&pl->list);
-	pl->port = 0;
-}
-
-void tipc_plist_push(struct tipc_plist *pl, u32 port);
-u32 tipc_plist_pop(struct tipc_plist *pl);
+bool u32_push(struct list_head *l, u32 value);
+u32 u32_pop(struct list_head *l);
+bool u32_find(struct list_head *l, u32 value);
+bool u32_del(struct list_head *l, u32 value);
+void u32_list_purge(struct list_head *l);
+int u32_list_len(struct list_head *l);
 
 #endif
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index f27462e..fae6a55 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -788,7 +788,7 @@ void tipc_sk_mcast_rcv(struct net *net, struct sk_buff_head *arrvq,
 		       struct sk_buff_head *inputq)
 {
 	struct tipc_msg *msg;
-	struct tipc_plist dports;
+	struct list_head dports;
 	u32 portid;
 	u32 scope = TIPC_CLUSTER_SCOPE;
 	struct sk_buff_head tmpq;
@@ -796,7 +796,7 @@ void tipc_sk_mcast_rcv(struct net *net, struct sk_buff_head *arrvq,
 	struct sk_buff *skb, *_skb;
 
 	__skb_queue_head_init(&tmpq);
-	tipc_plist_init(&dports);
+	INIT_LIST_HEAD(&dports);
 
 	skb = tipc_skb_peek(arrvq, &inputq->lock);
 	for (; skb; skb = tipc_skb_peek(arrvq, &inputq->lock)) {
@@ -810,8 +810,8 @@ void tipc_sk_mcast_rcv(struct net *net, struct sk_buff_head *arrvq,
 		tipc_nametbl_mc_translate(net,
 					  msg_nametype(msg), msg_namelower(msg),
 					  msg_nameupper(msg), scope, &dports);
-		portid = tipc_plist_pop(&dports);
-		for (; portid; portid = tipc_plist_pop(&dports)) {
+		portid = u32_pop(&dports);
+		for (; portid; portid = u32_pop(&dports)) {
 			_skb = __pskb_copy(skb, hsz, GFP_ATOMIC);
 			if (_skb) {
 				msg_set_destport(buf_msg(_skb), portid);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next 2/2] mpls: Packet stats
From: kbuild test robot @ 2017-01-14  6:58 UTC (permalink / raw)
  To: Robert Shearman
  Cc: kbuild-all, davem, netdev, roopa, David Ahern, ebiederm,
	Robert Shearman
In-Reply-To: <1484331253-5908-3-git-send-email-rshearma@brocade.com>

[-- Attachment #1: Type: text/plain, Size: 7847 bytes --]

Hi Robert,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Robert-Shearman/net-AF-specific-RTM_GETSTATS-attributes/20170114-095819
config: i386-randconfig-sb0-01141243 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/net/netns/mib.h:4:0,
                    from include/net/net_namespace.h:14,
                    from include/linux/netdevice.h:43,
                    from include/uapi/linux/if_arp.h:26,
                    from include/linux/if_arp.h:27,
                    from net/mpls/af_mpls.c:7:
   net/mpls/af_mpls.c: In function 'mpls_stats_inc_outucastpkts':
>> include/net/ipv6.h:163:35: error: 'struct netns_mib' has no member named 'ipv6_statistics'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
                                      ^
   include/net/snmp.h:145:15: note: in definition of macro 'SNMP_UPD_PO_STATS'
      __typeof__((mib->mibs) + 0) ptr = mib->mibs; \
                  ^
   include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^
   net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^
>> include/net/ipv6.h:163:35: error: 'struct netns_mib' has no member named 'ipv6_statistics'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
                                      ^
   include/net/snmp.h:145:37: note: in definition of macro 'SNMP_UPD_PO_STATS'
      __typeof__((mib->mibs) + 0) ptr = mib->mibs; \
                                        ^
   include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^
   net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^
   In file included from include/asm-generic/percpu.h:6:0,
                    from arch/x86/include/asm/percpu.h:542,
                    from arch/x86/include/asm/preempt.h:5,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/mm_types.h:8,
                    from include/linux/kmemcheck.h:4,
                    from include/linux/skbuff.h:18,
                    from net/mpls/af_mpls.c:2:
   include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   include/linux/percpu-defs.h:206:47: note: in definition of macro '__verify_pcpu_ptr'
     const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL; \
                                                  ^
   include/linux/percpu-defs.h:496:33: note: in expansion of macro '__pcpu_size_call'
    #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                    ^
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^
   include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^
   include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^
   net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^
   include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   include/linux/percpu-defs.h:363:16: note: in definition of macro '__pcpu_size_call'
     switch(sizeof(variable)) {     \
                   ^
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^
   include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^
   include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^
   net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^
   In file included from arch/x86/include/asm/preempt.h:5:0,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/mm_types.h:8,
                    from include/linux/kmemcheck.h:4,
                    from include/linux/skbuff.h:18,
                    from net/mpls/af_mpls.c:2:
   include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   arch/x86/include/asm/percpu.h:128:17: note: in definition of macro 'percpu_add_op'
     typedef typeof(var) pao_T__;     \
                    ^
   include/linux/percpu-defs.h:364:11: note: in expansion of macro 'this_cpu_add_1'
      case 1: stem##1(variable, __VA_ARGS__);break;  \
              ^
   include/linux/percpu-defs.h:496:33: note: in expansion of macro '__pcpu_size_call'
    #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                    ^
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^
   include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^
   include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^
   net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^
   include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector

vim +163 include/net/ipv6.h

8e7999c4 Pavel Emelyanov 2007-10-15  157  
13415e46 Eric Dumazet    2016-04-27  158  #define _DEVUPD(net, statname, mod, idev, field, val)			\
edf391ff Neil Horman     2009-04-27  159  ({									\
edf391ff Neil Horman     2009-04-27  160  	struct inet6_dev *_idev = (idev);				\
edf391ff Neil Horman     2009-04-27  161  	if (likely(_idev != NULL))					\
13415e46 Eric Dumazet    2016-04-27  162  		mod##SNMP_UPD_PO_STATS((_idev)->stats.statname, field, (val)); \
13415e46 Eric Dumazet    2016-04-27 @163  	mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
edf391ff Neil Horman     2009-04-27  164  })
edf391ff Neil Horman     2009-04-27  165  
14878f75 David L Stevens 2007-09-16  166  /* MIBs */

:::::: The code at line 163 was first introduced by commit
:::::: 13415e46c5915e2dac089de516369005fbc045f9 net: snmp: kill STATS_BH macros

:::::: TO: Eric Dumazet <edumazet@google.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24141 bytes --]

^ permalink raw reply

* Re: Commit 1fe8e0... (include more headers in if_tunnel.h) breaks my user-space build.
From: Mikko Rapeli @ 2017-01-14  6:43 UTC (permalink / raw)
  To: Ben Greear; +Cc: Stephen Hemminger, netdev
In-Reply-To: <7f212e93-e9e7-d2aa-2e2d-b36bca8d2f6e@candelatech.com>

On Fri, Jan 13, 2017 at 02:11:41PM -0800, Ben Greear wrote:
> On 01/13/2017 02:08 PM, Stephen Hemminger wrote:
> >On Fri, 13 Jan 2017 11:50:32 -0800
> >Ben Greear <greearb@candelatech.com> wrote:
> >
> >>On 01/13/2017 11:41 AM, Stephen Hemminger wrote:
> >>>On Fri, 13 Jan 2017 11:12:32 -0800
> >>>Ben Greear <greearb@candelatech.com> wrote:
> >>>
> >>>>I am including netinet/ip.h, and also linux/if_tunnel.h, and the linux/ip.h conflicts with
> >>>>netinet/ip.h.
> >>>>
> >>>>Maybe my build environment is screwed up, but maybe also it would be better to
> >>>>just let the user include appropriate headers before including if_tunnel.h
> >>>>and revert this patch?
> >>>>
> >>>>
> >>>>include/uapi/linux/if_tunnel.h: include linux/if.h, linux/ip.h and linux/in6.h
> >>>>
> >>>>     Fixes userspace compilation errors like:
> >>>>
> >>>>     error: field ‘iph’ has incomplete type
> >>>>     error: field ‘prefix’ has incomplete type
> >>>>
> >>>>     Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
> >>>>     Signed-off-by: David S. Miller <davem@davemloft.net>
> >>>>
> >>>>Thanks,
> >>>>Ben
> >>>>
> >>>
> >>>What I ended up doing for iproute2 was including all headers used by the source
> >>>based on sanitized kernel headers.  Basically
> >>>  $ git grep '^#include <linux/' | \
> >>>	awk -F: '{print $2}' | \
> >>>	sed -e 's/^#include <//' -e 's/>.*$//' | \
> >>>	sort -u >linux.headers
> >>>   $ for f in $(cat linux.headers)
> >>>     do cp ~/kernel/net-next/usr/include/$f include/$f
> >>>     done
> >>>
> >>>You can't take only some of the headers, once you decide to diverge from glibc provided
> >>>headers, you got to take them all.
> >>>
> >>
> >>I do grab a copy of the linux kernel headers and compile against that, but netinet/ip.h is
> >>coming from the OS.  Do you mean I should not include netinet/ip.h and instead use linux/ip.h?
> >
> >I don't think you can mix netinet/ip.h and linux/ip.h, yes that is a mess.
> >
> 
> Well, I still like the idea of reverting this patch..that way user-space does not have to use
> linux/ip.h, and that lets them use netinet/ip.h and if_tunnel.h.

I might have patches for glibc compat for your case in
https://github.com/mcfrisk/linux/commits/headers_test_v06
if you include glibc headers first and then kernel uapi ones.

> Anyway, I'll let Dave and/or the original committer decide....I've reverted it in my local tree
> so I am able to build again...

My changes make uapi headers compile as they are in userspace. That exposes
problems like this for which user space has had workarounds for decades.
Sorry for that. The glibc compat fixes should help.

-Mikko

> Thanks,
> Ben
> 
> -- 
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
> 

^ permalink raw reply

* Re: [PATCH net-next 2/2] mpls: Packet stats
From: kbuild test robot @ 2017-01-14  6:41 UTC (permalink / raw)
  To: Robert Shearman
  Cc: kbuild-all, davem, netdev, roopa, David Ahern, ebiederm,
	Robert Shearman
In-Reply-To: <1484331253-5908-3-git-send-email-rshearma@brocade.com>

[-- Attachment #1: Type: text/plain, Size: 10427 bytes --]

Hi Robert,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Robert-Shearman/net-AF-specific-RTM_GETSTATS-attributes/20170114-095819
config: x86_64-randconfig-u0-01141334 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/net/netns/mib.h:4:0,
                    from include/net/net_namespace.h:14,
                    from include/linux/netdevice.h:43,
                    from include/uapi/linux/if_arp.h:26,
                    from include/linux/if_arp.h:27,
                    from net/mpls/af_mpls.c:7:
   net/mpls/af_mpls.c: In function 'mpls_stats_inc_outucastpkts':
>> include/net/ipv6.h:163:35: error: 'struct netns_mib' has no member named 'ipv6_statistics'; did you mean 'ip_statistics'?
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
                                      ^
   include/net/snmp.h:145:15: note: in definition of macro 'SNMP_UPD_PO_STATS'
      __typeof__((mib->mibs) + 0) ptr = mib->mibs; \
                  ^~~
>> include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^~~~~~~
>> net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^~~~~~~~~~~~~~~~
>> include/net/ipv6.h:163:35: error: 'struct netns_mib' has no member named 'ipv6_statistics'; did you mean 'ip_statistics'?
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
                                      ^
   include/net/snmp.h:145:37: note: in definition of macro 'SNMP_UPD_PO_STATS'
      __typeof__((mib->mibs) + 0) ptr = mib->mibs; \
                                        ^~~
>> include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^~~~~~~
>> net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^~~~~~~~~~~~~~~~
   In file included from include/asm-generic/percpu.h:6:0,
                    from arch/x86/include/asm/percpu.h:542,
                    from arch/x86/include/asm/preempt.h:5,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/mm_types.h:8,
                    from include/linux/kmemcheck.h:4,
                    from include/linux/skbuff.h:18,
                    from net/mpls/af_mpls.c:2:
>> include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   include/linux/percpu-defs.h:206:47: note: in definition of macro '__verify_pcpu_ptr'
     const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL; \
                                                  ^~~
   include/linux/percpu-defs.h:496:33: note: in expansion of macro '__pcpu_size_call'
    #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                    ^~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^~~~~~~~~~~~
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^~~~~~~~~~~~
>> include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^~~~~~~~~~~~~~~~~
>> include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^~~~~~~
>> net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^~~~~~~~~~~~~~~~
>> include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   include/linux/percpu-defs.h:363:16: note: in definition of macro '__pcpu_size_call'
     switch(sizeof(variable)) {     \
                   ^~~~~~~~
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^~~~~~~~~~~~
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^~~~~~~~~~~~
>> include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^~~~~~~~~~~~~~~~~
>> include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^~~~~~~
>> net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^~~~~~~~~~~~~~~~
   In file included from arch/x86/include/asm/preempt.h:5:0,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/mm_types.h:8,
                    from include/linux/kmemcheck.h:4,
                    from include/linux/skbuff.h:18,
                    from net/mpls/af_mpls.c:2:
>> include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   arch/x86/include/asm/percpu.h:128:17: note: in definition of macro 'percpu_add_op'
     typedef typeof(var) pao_T__;     \
                    ^~~
   include/linux/percpu-defs.h:364:11: note: in expansion of macro 'this_cpu_add_1'
      case 1: stem##1(variable, __VA_ARGS__);break;  \
              ^~~~
   include/linux/percpu-defs.h:496:33: note: in expansion of macro '__pcpu_size_call'
    #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                    ^~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^~~~~~~~~~~~
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^~~~~~~~~~~~
>> include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^~~~~~~~~~~~~~~~~
>> include/net/ipv6.h:177:3: note: in expansion of macro '_DEVUPD'
      _DEVUPD(net, ipv6, , idev, field, val)
      ^~~~~~~
>> net/mpls/af_mpls.c:114:4: note: in expansion of macro 'IP6_UPD_PO_STATS'
       IP6_UPD_PO_STATS(dev_net(dev), in6dev,
       ^~~~~~~~~~~~~~~~
>> include/net/snmp.h:146:19: error: subscripted value is neither array nor pointer nor vector
      this_cpu_inc(ptr[basefield##PKTS]);  \
                      ^
   arch/x86/include/asm/percpu.h:137:17: note: in definition of macro 'percpu_add_op'
     switch (sizeof(var)) {      \
                    ^~~
   include/linux/percpu-defs.h:364:11: note: in expansion of macro 'this_cpu_add_1'
      case 1: stem##1(variable, __VA_ARGS__);break;  \
              ^~~~
   include/linux/percpu-defs.h:496:33: note: in expansion of macro '__pcpu_size_call'
    #define this_cpu_add(pcp, val)  __pcpu_size_call(this_cpu_add_, pcp, val)
                                    ^~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:507:28: note: in expansion of macro 'this_cpu_add'
    #define this_cpu_inc(pcp)  this_cpu_add(pcp, 1)
                               ^~~~~~~~~~~~
   include/net/snmp.h:146:3: note: in expansion of macro 'this_cpu_inc'
      this_cpu_inc(ptr[basefield##PKTS]);  \
      ^~~~~~~~~~~~
>> include/net/ipv6.h:163:7: note: in expansion of macro 'SNMP_UPD_PO_STATS'
     mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
          ^~~~~~~~~~~~~~~~~

vim +163 include/net/ipv6.h

8e7999c4 Pavel Emelyanov 2007-10-15  157  
13415e46 Eric Dumazet    2016-04-27  158  #define _DEVUPD(net, statname, mod, idev, field, val)			\
edf391ff Neil Horman     2009-04-27  159  ({									\
edf391ff Neil Horman     2009-04-27  160  	struct inet6_dev *_idev = (idev);				\
edf391ff Neil Horman     2009-04-27  161  	if (likely(_idev != NULL))					\
13415e46 Eric Dumazet    2016-04-27  162  		mod##SNMP_UPD_PO_STATS((_idev)->stats.statname, field, (val)); \
13415e46 Eric Dumazet    2016-04-27 @163  	mod##SNMP_UPD_PO_STATS((net)->mib.statname##_statistics, field, (val));\
edf391ff Neil Horman     2009-04-27  164  })
edf391ff Neil Horman     2009-04-27  165  
14878f75 David L Stevens 2007-09-16  166  /* MIBs */
14878f75 David L Stevens 2007-09-16  167  
087fe240 Denis V. Lunev  2008-10-08  168  #define IP6_INC_STATS(net, idev,field)		\
13415e46 Eric Dumazet    2016-04-27  169  		_DEVINC(net, ipv6, , idev, field)
1d015503 Eric Dumazet    2016-04-27  170  #define __IP6_INC_STATS(net, idev,field)	\
13415e46 Eric Dumazet    2016-04-27  171  		_DEVINC(net, ipv6, __, idev, field)
edf391ff Neil Horman     2009-04-27  172  #define IP6_ADD_STATS(net, idev,field,val)	\
13415e46 Eric Dumazet    2016-04-27  173  		_DEVADD(net, ipv6, , idev, field, val)
1d015503 Eric Dumazet    2016-04-27  174  #define __IP6_ADD_STATS(net, idev,field,val)	\
13415e46 Eric Dumazet    2016-04-27  175  		_DEVADD(net, ipv6, __, idev, field, val)
edf391ff Neil Horman     2009-04-27  176  #define IP6_UPD_PO_STATS(net, idev,field,val)   \
13415e46 Eric Dumazet    2016-04-27 @177  		_DEVUPD(net, ipv6, , idev, field, val)
c2005eb0 Eric Dumazet    2016-04-27  178  #define __IP6_UPD_PO_STATS(net, idev,field,val)   \
13415e46 Eric Dumazet    2016-04-27  179  		_DEVUPD(net, ipv6, __, idev, field, val)
087fe240 Denis V. Lunev  2008-10-08  180  #define ICMP6_INC_STATS(net, idev, field)	\

:::::: The code at line 163 was first introduced by commit
:::::: 13415e46c5915e2dac089de516369005fbc045f9 net: snmp: kill STATS_BH macros

:::::: TO: Eric Dumazet <edumazet@google.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28286 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: Fix misleading packet/frame count stats.
From: David Miller @ 2017-01-14  4:35 UTC (permalink / raw)
  To: ganeshgr; +Cc: netdev, nirranjan, hariprasad, leedom
In-Reply-To: <1484299000-7586-1-git-send-email-ganeshgr@chelsio.com>

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri, 13 Jan 2017 14:46:40 +0530

> Do not count pause frames as part of general TX/RX frame
> counters.
> 
> Based on the original work of Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next v2 0/5] bnxt_en: Misc. updates for net-next.
From: David Miller @ 2017-01-14  4:22 UTC (permalink / raw)
  To: michael.chan; +Cc: netdev
In-Reply-To: <1484289124-28853-1-git-send-email-michael.chan@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>
Date: Fri, 13 Jan 2017 01:31:59 -0500

> Miscellaneous updates including firmware spec update, ethtool -p blinking
> LED support, RDMA SRIOV config callback, and minor fixes.
> 
> v2: Dropped the DCBX RoCE app TLV patch until the ETH_P_IBOE RDMA patch
> is merged.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v2 00/13] tcp: RACK fast recovery
From: David Miller @ 2017-01-14  4:19 UTC (permalink / raw)
  To: ycheng; +Cc: netdev, edumazet, ncardwell, nanditad
In-Reply-To: <20170113061142.127344-1-ycheng@google.com>

From: Yuchung Cheng <ycheng@google.com>
Date: Thu, 12 Jan 2017 22:11:29 -0800

> The patch set enables RACK loss detection (draft-ietf-tcpm-rack-01)
> to trigger fast recovery with a reordering timer.
> 
> Previously RACK has been running in auxiliary mode where it is
> used to detect packet losses once the recovery has triggered by
> other algorithms (e.g., FACK). By inspecting packet timestamps,
> RACK can start ACK-driven repairs timely. A few similar heuristics
> are no longer needed and are either removed or disabled to reduce
> the complexity of the Linux TCP loss recovery engine:
> 
>   1. FACK (Forward Acknowledgement)
>   2. Early Retransmit (RFC5827)
>   3. thin_dupack (fast recovery on single DUPACK for thin-streams)
>   4. NCR (Non-Congestion Robustness RFC4653) (RFC4653)
>   5. Forward Retransmit
> 
> After this change, Linux's loss recovery algorithms consist of
>   1. Conventional DUPACK threshold approach (RFC6675)
>   2. RACK and Tail Loss Probe (draft-ietf-tcpm-rack-01)
>   3. RTO plus F-RTO extension (RFC5682)
> 
> The patch set has been tested on Google servers extensively and
> presented in several IETF meetings. The data suggests that RACK
> successfully improves recovery performance:
> https://www.ietf.org/proceedings/97/slides/slides-97-tcpm-draft-ietf-tcpm-rack-01.pdf
> https://www.ietf.org/proceedings/96/slides/slides-96-tcpm-3.pdf

Series applied, thanks for all of your hard work.

^ permalink raw reply

* Re: [PATCH net-next] ipv6: sr: add missing Kbuild export for header files
From: kbuild test robot @ 2017-01-14  4:07 UTC (permalink / raw)
  To: David Lebrun; +Cc: kbuild-all, netdev, David Lebrun
In-Reply-To: <1484299564-6165-1-git-send-email-david.lebrun@uclouvain.be>

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

Hi David,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/David-Lebrun/ipv6-sr-add-missing-Kbuild-export-for-header-files/20170114-095347
config: x86_64-randconfig-n0-01141038 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> ./usr/include/linux/seg6.h:21: found __[us]{8,16,32,64} type without #include <linux/types.h>
>> ./usr/include/linux/seg6_hmac.h:11: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26743 bytes --]

^ permalink raw reply

* Re: [PATCH v5 00/13] net: ethernet: aquantia: Add AQtion 2.5/5 GB NIC driver
From: David VomLehn @ 2017-01-14  3:41 UTC (permalink / raw)
  To: Florian Fainelli, Alexander Loktionov, netdev
  Cc: David S . Miller, Simon Edelhaus, Dmitrii Tarakanov, Pavel Belous
In-Reply-To: <9a4e95cc-9daa-f883-46c0-f477e28b9f0f@gmail.com>

On 01/13/2017 05:38 PM, Florian Fainelli wrote:
> On 01/12/2017 09:02 PM, Alexander Loktionov wrote:
>> From: David VomLehn <vomlehn@texas.net>
>>
>> v1: Initial version
>> v2: o Make necessary drivers/net/ethernet changes to integrate software
>>      o Drop intermediate atlantic directory
>>      o Remove Makefile things only appropriate to out of tree module
>>        building
>> v3: o Move changes to drivers/net/ethernet/{Kconfig,Makefile} to the last
>>        patch to ensure clean bisection.
>>      o Removed inline attribute aq_hw_write_req() as it was defined in
>>        only one .c file.
>>      o #included pci.h in aq_common.h to get struct pci definition.
>>      o Modified code to unlock based execution flow rather than using a
>>        flag.
>>      o Made a number of functions that were only used in a single file
>>        static.
>>      o Cleaned up error and return code handling in various places.
>>      o Remove AQ_CFG_IP_ALIGN definition.
>>      o Other minor code clean up.
>> v4: o Using do_div for 64 bit division.
>>      o Modified NIC statistics code.
>>      o Using build_skb instead netdev_alloc_skb for single fragment
>>        packets.
>>      o Removed extra aq_nic.o from Makefile
>> v5: o Removed extra newline at the end of the files.
>>      o Wrapped cover letter lines.
> Have not looked at the driver yet, but the threading of your emails is
> weird, each patch is in reply to the previous one. It would be more
> natural to have all numbered patches be in reply to the cover letter,
> which according to the version of git you seem to have used (2.7.4)
> should already be the default. In graphical terms what we see right now is:
>
> [PATCH 00/13]
> 	[PATCH 01/13]
> 		[PATCH 02/13]
> 			....
>
> While we should see:
>
>
> [PATCH 00/13]
> 	[PATCH 01/13]
> 	[PATCH 02/13]
> 	....
>
> Can you fix that for future submissions, this may sound like a cosmetic
> thing, but it really helps with threading/reading etc.
>
> Thanks!
This looks like it is a consequence of setting the git configuration 
item format.thread to deep. I've switched it to shallow, which should 
give the threading you suggest.

-- 
David VL

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox