* Re: [PATCH net-next 4/4] test_rhashtable: add test case for rhl_table interface
From: kbuild test robot @ 2017-09-19 14:59 UTC (permalink / raw)
To: Florian Westphal; +Cc: kbuild-all, netdev, Florian Westphal
In-Reply-To: <20170918210711.10202-5-fw@strlen.de>
[-- Attachment #1: Type: text/plain, Size: 6339 bytes --]
Hi Florian,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Westphal/test_rhashtable-add-test-case-for-rhl-table/20170919-135550
config: x86_64-randconfig-a0-09192105 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
lib/test_rhashtable.c: In function 'test_rhltable':
>> lib/test_rhashtable.c:433: warning: the frame size of 2144 bytes is larger than 2048 bytes
vim +433 lib/test_rhashtable.c
254
255 static int __init test_rhltable(unsigned int entries)
256 {
257 struct test_obj_rhl *rhl_test_objects;
258 unsigned long *obj_in_table;
259 struct rhltable rhlt;
260 unsigned int i, j, k;
261 int ret, err;
262
263 if (entries == 0)
264 entries = 1;
265
266 rhl_test_objects = vzalloc(sizeof(*rhl_test_objects) * entries);
267 if (!rhl_test_objects)
268 return -ENOMEM;
269
270 ret = -ENOMEM;
271 obj_in_table = vzalloc(BITS_TO_LONGS(entries) * sizeof(unsigned long));
272 if (!obj_in_table)
273 goto out_free;
274
275 /* nulls_base not supported in rhlist interface */
276 test_rht_params.nulls_base = 0;
277 err = rhltable_init(&rhlt, &test_rht_params);
278 if (WARN_ON(err))
279 goto out_free;
280
281 k = prandom_u32();
282 ret = 0;
283 for (i = 0; i < entries; i++) {
284 rhl_test_objects[i].value.id = k;
285 err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node,
286 test_rht_params);
287 if (WARN(err, "error %d on element %d\n", err, i))
288 break;
289 if (err == 0)
290 set_bit(i, obj_in_table);
291 }
292
293 if (err)
294 ret = err;
295
296 pr_info("test %d add/delete pairs into rhlist\n", entries);
297 for (i = 0; i < entries; i++) {
298 struct rhlist_head *h, *pos;
299 struct test_obj_rhl *obj;
300 struct test_obj_val key = {
301 .id = k,
302 };
303 bool found;
304
305 rcu_read_lock();
306 h = rhltable_lookup(&rhlt, &key, test_rht_params);
307 if (WARN(!h, "key not found during iteration %d of %d", i, entries)) {
308 rcu_read_unlock();
309 break;
310 }
311
312 if (i) {
313 j = i - 1;
314 rhl_for_each_entry_rcu(obj, pos, h, list_node) {
315 if (WARN(pos == &rhl_test_objects[j].list_node, "old element found, should be gone"))
316 break;
317 }
318 }
319
320 cond_resched_rcu();
321
322 found = false;
323
324 rhl_for_each_entry_rcu(obj, pos, h, list_node) {
325 if (pos == &rhl_test_objects[i].list_node) {
326 found = true;
327 break;
328 }
329 }
330
331 rcu_read_unlock();
332
333 if (WARN(!found, "element %d not found", i))
334 break;
335
336 err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
337 WARN(err, "rhltable_remove: err %d for iteration %d\n", err, i);
338 if (err == 0)
339 clear_bit(i, obj_in_table);
340 }
341
342 if (ret == 0 && err)
343 ret = err;
344
345 for (i = 0; i < entries; i++) {
346 WARN(test_bit(i, obj_in_table), "elem %d allegedly still present", i);
347
348 err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node,
349 test_rht_params);
350 if (WARN(err, "error %d on element %d\n", err, i))
351 break;
352 if (err == 0)
353 set_bit(i, obj_in_table);
354 }
355
356 pr_info("test %d random rhlist add/delete operations\n", entries);
357 for (j = 0; j < entries; j++) {
358 u32 i = prandom_u32_max(entries);
359 u32 prand = prandom_u32();
360
361 cond_resched();
362
363 if (prand == 0)
364 prand = prandom_u32();
365
366 if (prand & 1) {
367 prand >>= 1;
368 continue;
369 }
370
371 err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
372 if (test_bit(i, obj_in_table)) {
373 clear_bit(i, obj_in_table);
374 if (WARN(err, "cannot remove element at slot %d", i))
375 continue;
376 } else {
377 if (WARN(err != -ENOENT, "removed non-existant element %d, error %d not %d",
378 i, err, -ENOENT))
379 continue;
380 }
381
382 if (prand & 1) {
383 prand >>= 1;
384 continue;
385 }
386
387 err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
388 if (err == 0) {
389 if (WARN(test_and_set_bit(i, obj_in_table), "succeeded to insert same object %d", i))
390 continue;
391 } else {
392 if (WARN(!test_bit(i, obj_in_table), "failed to insert object %d", i))
393 continue;
394 }
395
396 if (prand & 1) {
397 prand >>= 1;
398 continue;
399 }
400
401 i = prandom_u32_max(entries);
402 if (test_bit(i, obj_in_table)) {
403 err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
404 WARN(err, "cannot remove element at slot %d", i);
405 if (err == 0)
406 clear_bit(i, obj_in_table);
407 } else {
408 err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
409 WARN(err, "failed to insert object %d", i);
410 if (err == 0)
411 set_bit(i, obj_in_table);
412 }
413 }
414
415 for (i = 0; i < entries; i++) {
416 cond_resched();
417 err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
418 if (test_bit(i, obj_in_table)) {
419 if (WARN(err, "cannot remove element at slot %d", i))
420 continue;
421 } else {
422 if (WARN(err != -ENOENT, "removed non-existant element, error %d not %d",
423 err, -ENOENT))
424 continue;
425 }
426 }
427
428 rhltable_destroy(&rhlt);
429 out_free:
430 vfree(rhl_test_objects);
431 vfree(obj_in_table);
432 return ret;
> 433 }
434
---
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: 25659 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Craig Gallek @ 2017-09-19 15:08 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Daniel Mack, Daniel Borkmann, David S . Miller, netdev
In-Reply-To: <3e537b56-d0f2-de9a-5bb1-f60fbfc11ca5@fb.com>
On Mon, Sep 18, 2017 at 6:53 PM, Alexei Starovoitov <ast@fb.com> wrote:
Thanks for the review! Please correct me if I'm wrong...
> On 9/18/17 12:30 PM, Craig Gallek wrote:
>>
>> From: Craig Gallek <kraig@google.com>
>>
>> This is a simple non-recursive delete operation. It prunes paths
>> of empty nodes in the tree, but it does not try to further compress
>> the tree as nodes are removed.
>>
>> Signed-off-by: Craig Gallek <kraig@google.com>
>> ---
>> kernel/bpf/lpm_trie.c | 80
>> +++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 77 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
>> index 1b767844a76f..9d58a576b2ae 100644
>> --- a/kernel/bpf/lpm_trie.c
>> +++ b/kernel/bpf/lpm_trie.c
>> @@ -389,10 +389,84 @@ static int trie_update_elem(struct bpf_map *map,
>> return ret;
>> }
>>
>> -static int trie_delete_elem(struct bpf_map *map, void *key)
>> +/* Called from syscall or from eBPF program */
>> +static int trie_delete_elem(struct bpf_map *map, void *_key)
>> {
>> - /* TODO */
>> - return -ENOSYS;
>> + struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
>> + struct bpf_lpm_trie_key *key = _key;
>> + struct lpm_trie_node __rcu **trim;
>> + struct lpm_trie_node *node;
>> + unsigned long irq_flags;
>> + unsigned int next_bit;
>> + size_t matchlen = 0;
>> + int ret = 0;
>> +
>> + if (key->prefixlen > trie->max_prefixlen)
>> + return -EINVAL;
>> +
>> + raw_spin_lock_irqsave(&trie->lock, irq_flags);
>> +
>> + /* Walk the tree looking for an exact key/length match and keeping
>> + * track of where we could begin trimming the tree. The
>> trim-point
>> + * is the sub-tree along the walk consisting of only single-child
>> + * intermediate nodes and ending at a leaf node that we want to
>> + * remove.
>> + */
>> + trim = &trie->root;
>> + node = rcu_dereference_protected(
>> + trie->root, lockdep_is_held(&trie->lock));
>> + while (node) {
>> + matchlen = longest_prefix_match(trie, node, key);
>> +
>> + if (node->prefixlen != matchlen ||
>> + node->prefixlen == key->prefixlen)
>> + break;
>
>
> curious why there is no need to do
> 'node->prefixlen == trie->max_prefixlen' in the above
> like update/lookup do?
I don't believe the node->prefixlen == trie->max_prefixlen check in
trie_update_elem is necessary. In order to get to this third clause,
it implies that the first two clauses evaluated false. Which happens
when we find an exact prefix match for the current node, but the
to-be-inserted key prefix is different. If the node we are comparing
against had a prefix of max_prefixlen, it would not be possible to
have both a full prefix match but different prefix lengths. This
assumes that there are no nodes in the tree with > max_prefixlen
prefixes, but that is handled earlier in the update function.
There's a similar (I believe) unnecessary max_prefixlen check in
trie_lookup_elem. The function should behave the same way without
that check, but at least in this case it's used as an early-out and
saves a few lines of execution.
>
>> +
>> + next_bit = extract_bit(key->data, node->prefixlen);
>> + /* If we hit a node that has more than one child or is a
>> valid
>> + * prefix itself, do not remove it. Reset the root of the
>> trim
>> + * path to its descendant on our path.
>> + */
>> + if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
>> + (node->child[0] && node->child[1]))
>> + trim = &node->child[next_bit];
>> + node = rcu_dereference_protected(
>> + node->child[next_bit],
>> lockdep_is_held(&trie->lock));
>> + }
>> +
>> + if (!node || node->prefixlen != key->prefixlen ||
>> + (node->flags & LPM_TREE_NODE_FLAG_IM)) {
>> + ret = -ENOENT;
>> + goto out;
>> + }
>> +
>> + trie->n_entries--;
>> +
>> + /* If the node we are removing is not a leaf node, simply mark it
>> + * as intermediate and we are done.
>> + */
>> + if (rcu_access_pointer(node->child[0]) ||
>> + rcu_access_pointer(node->child[1])) {
>> + node->flags |= LPM_TREE_NODE_FLAG_IM;
>> + goto out;
>> + }
>> +
>> + /* trim should now point to the slot holding the start of a path
>> from
>> + * zero or more intermediate nodes to our leaf node for deletion.
>> + */
>> + while ((node = rcu_dereference_protected(
>> + *trim, lockdep_is_held(&trie->lock)))) {
>> + RCU_INIT_POINTER(*trim, NULL);
>> + trim = rcu_access_pointer(node->child[0]) ?
>> + &node->child[0] :
>> + &node->child[1];
>> + kfree_rcu(node, rcu);
>
>
> can it be that some of the nodes this loop walks have
> both child[0] and [1] ?
No, the loop above will push trim down the walk every time it
encounters a node with two children. The only other trim assignment
is the initial trim = &trie->root. But the only time we would skip
the assignment in the loop is if the node being removed is the root.
If the root had multiple children and is being removed, it would be
handled by the case that turns the node into an intermediate node
rather than walking the trim path freeing things.
^ permalink raw reply
* [PATCH net-next 0/4] net: dsa: move master ethtool code
From: Vivien Didelot @ 2017-09-19 15:56 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
The DSA core overrides the master device's ethtool_ops structure so that
it can inject statistics and such of its dedicated switch CPU port.
This ethtool code is currently called on unnecessary conditions or
before the master interface and its switch CPU port get wired up.
This patchset fixes this.
Similarly to slave.c where the DSA slave net_device is the entry point
of the dsa_slave_* functions, this patchset also isolates the master's
ethtool code in a new master.c file, where the DSA master net_device is
the entry point of the dsa_master_* functions.
This is a first step towards better control of the master device and
support for multiple CPU ports.
Vivien Didelot (4):
net: dsa: remove copy of master ethtool_ops
net: dsa: setup master ethtool unconditionally
net: dsa: setup master ethtool after dsa_ptr
net: dsa: move master ethtool code
include/net/dsa.h | 1 -
net/dsa/Makefile | 2 +-
net/dsa/dsa.c | 28 -------------
net/dsa/dsa2.c | 18 ++++----
net/dsa/dsa_priv.h | 7 ++--
net/dsa/legacy.c | 10 ++---
net/dsa/master.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
net/dsa/slave.c | 80 -----------------------------------
8 files changed, 136 insertions(+), 130 deletions(-)
create mode 100644 net/dsa/master.c
--
2.14.1
^ permalink raw reply
* [PATCH net-next 1/4] net: dsa: remove copy of master ethtool_ops
From: Vivien Didelot @ 2017-09-19 15:56 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170919155700.14474-1-vivien.didelot@savoirfairelinux.com>
There is no need to store a copy of the master ethtool ops, storing the
original pointer in DSA and the new one in the master netdev itself is
enough.
In the meantime, set orig_ethtool_ops to NULL when restoring the master
ethtool ops and check the presence of the master original ethtool ops as
well as its needed functions before calling them.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/dsa.h | 1 -
net/dsa/dsa.c | 8 ++++----
net/dsa/slave.c | 19 +++++++++++--------
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index dd44d6ce1097..8dee216a5a9b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -188,7 +188,6 @@ struct dsa_port {
/*
* Original copy of the master netdev ethtool_ops
*/
- struct ethtool_ops ethtool_ops;
const struct ethtool_ops *orig_ethtool_ops;
};
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 03c58b0eb082..abadf7b49236 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -124,11 +124,10 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
if (!cpu_ops)
return -ENOMEM;
- memcpy(&cpu_dp->ethtool_ops, master->ethtool_ops,
- sizeof(struct ethtool_ops));
cpu_dp->orig_ethtool_ops = master->ethtool_ops;
- memcpy(cpu_ops, &cpu_dp->ethtool_ops,
- sizeof(struct ethtool_ops));
+ if (cpu_dp->orig_ethtool_ops)
+ memcpy(cpu_ops, cpu_dp->orig_ethtool_ops, sizeof(*cpu_ops));
+
dsa_cpu_port_ethtool_init(cpu_ops);
master->ethtool_ops = cpu_ops;
@@ -138,6 +137,7 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
{
cpu_dp->netdev->ethtool_ops = cpu_dp->orig_ethtool_ops;
+ cpu_dp->orig_ethtool_ops = NULL;
}
void dsa_cpu_dsa_destroy(struct dsa_port *port)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2afa99506f8b..2ff4f907d137 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -574,12 +574,13 @@ static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
struct dsa_switch *ds = cpu_dp->ds;
+ const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
s8 cpu_port = cpu_dp->index;
int count = 0;
- if (cpu_dp->ethtool_ops.get_sset_count) {
- count = cpu_dp->ethtool_ops.get_sset_count(dev, ETH_SS_STATS);
- cpu_dp->ethtool_ops.get_ethtool_stats(dev, stats, data);
+ if (ops && ops->get_sset_count && ops->get_ethtool_stats) {
+ count = ops->get_sset_count(dev, ETH_SS_STATS);
+ ops->get_ethtool_stats(dev, stats, data);
}
if (ds->ops->get_ethtool_stats)
@@ -591,10 +592,11 @@ static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
struct dsa_switch *ds = cpu_dp->ds;
+ const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
int count = 0;
- if (cpu_dp->ethtool_ops.get_sset_count)
- count += cpu_dp->ethtool_ops.get_sset_count(dev, sset);
+ if (ops && ops->get_sset_count)
+ count += ops->get_sset_count(dev, sset);
if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
count += ds->ops->get_sset_count(ds);
@@ -608,6 +610,7 @@ static void dsa_cpu_port_get_strings(struct net_device *dev,
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
struct dsa_switch *ds = cpu_dp->ds;
+ const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
s8 cpu_port = cpu_dp->index;
int len = ETH_GSTRING_LEN;
int mcount = 0, count;
@@ -619,9 +622,9 @@ static void dsa_cpu_port_get_strings(struct net_device *dev,
/* We do not want to be NULL-terminated, since this is a prefix */
pfx[sizeof(pfx) - 1] = '_';
- if (cpu_dp->ethtool_ops.get_sset_count) {
- mcount = cpu_dp->ethtool_ops.get_sset_count(dev, ETH_SS_STATS);
- cpu_dp->ethtool_ops.get_strings(dev, stringset, data);
+ if (ops && ops->get_sset_count && ops->get_strings) {
+ mcount = ops->get_sset_count(dev, ETH_SS_STATS);
+ ops->get_strings(dev, stringset, data);
}
if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: dsa: setup master ethtool unconditionally
From: Vivien Didelot @ 2017-09-19 15:56 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170919155700.14474-1-vivien.didelot@savoirfairelinux.com>
When a DSA switch tree is meant to be applied, it already has a CPU
port. Thus remove the condition of dst->cpu_dp.
Moreover, the next lines access dst->cpu_dp unconditionally.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa2.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 873af0108e24..bd19304f862f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -433,11 +433,9 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
return err;
}
- if (dst->cpu_dp) {
- err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
- if (err)
- return err;
- }
+ err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+ if (err)
+ return err;
/* If we use a tagging format that doesn't have an ethertype
* field, make sure that all packets from this point on get
@@ -474,10 +472,8 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
dsa_ds_unapply(dst, ds);
}
- if (dst->cpu_dp) {
- dsa_cpu_port_ethtool_restore(dst->cpu_dp);
- dst->cpu_dp = NULL;
- }
+ dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+ dst->cpu_dp = NULL;
pr_info("DSA: tree %d unapplied\n", dst->tree);
dst->applied = false;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: dsa: move master ethtool code
From: Vivien Didelot @ 2017-09-19 15:57 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170919155700.14474-1-vivien.didelot@savoirfairelinux.com>
DSA overrides the master device ethtool ops, so that it can inject stats
from its dedicated switch CPU port as well.
The related code is currently split in dsa.c and slave.c, but it only
scopes the master net device. Move it to a new master.c DSA core file.
This file will be later extented with master net device specific code.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/Makefile | 2 +-
net/dsa/dsa.c | 28 -------------
net/dsa/dsa2.c | 4 +-
net/dsa/dsa_priv.h | 7 ++--
net/dsa/legacy.c | 4 +-
net/dsa/master.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
net/dsa/slave.c | 83 ------------------------------------
7 files changed, 129 insertions(+), 119 deletions(-)
create mode 100644 net/dsa/master.c
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index fcce25da937c..2e7ac8bab19d 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,6 +1,6 @@
# the core
obj-$(CONFIG_NET_DSA) += dsa_core.o
-dsa_core-y += dsa.o dsa2.o legacy.o port.o slave.o switch.o
+dsa_core-y += dsa.o dsa2.o legacy.o master.o port.o slave.o switch.o
# tagging formats
dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index abadf7b49236..81c852e32821 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -112,34 +112,6 @@ const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
return ops;
}
-int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
-{
- struct dsa_switch *ds = cpu_dp->ds;
- struct net_device *master;
- struct ethtool_ops *cpu_ops;
-
- master = cpu_dp->netdev;
-
- cpu_ops = devm_kzalloc(ds->dev, sizeof(*cpu_ops), GFP_KERNEL);
- if (!cpu_ops)
- return -ENOMEM;
-
- cpu_dp->orig_ethtool_ops = master->ethtool_ops;
- if (cpu_dp->orig_ethtool_ops)
- memcpy(cpu_ops, cpu_dp->orig_ethtool_ops, sizeof(*cpu_ops));
-
- dsa_cpu_port_ethtool_init(cpu_ops);
- master->ethtool_ops = cpu_ops;
-
- return 0;
-}
-
-void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
-{
- cpu_dp->netdev->ethtool_ops = cpu_dp->orig_ethtool_ops;
- cpu_dp->orig_ethtool_ops = NULL;
-}
-
void dsa_cpu_dsa_destroy(struct dsa_port *port)
{
struct device_node *port_dn = port->dn;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 032f8bc3e788..dcccaebde708 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -440,7 +440,7 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
wmb();
dst->cpu_dp->netdev->dsa_ptr = dst;
- err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+ err = dsa_master_ethtool_setup(dst->cpu_dp->netdev);
if (err)
return err;
@@ -457,7 +457,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
if (!dst->applied)
return;
- dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+ dsa_master_ethtool_restore(dst->cpu_dp->netdev);
dst->cpu_dp->netdev->dsa_ptr = NULL;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9c3eeb72462d..f616b3444418 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -97,8 +97,6 @@ struct dsa_slave_priv {
int dsa_cpu_dsa_setup(struct dsa_port *port);
void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
-int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp);
-void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp);
bool dsa_schedule_work(struct work_struct *work);
/* legacy.c */
@@ -112,6 +110,10 @@ int dsa_legacy_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr, u16 vid);
+/* master.c */
+int dsa_master_ethtool_setup(struct net_device *dev);
+void dsa_master_ethtool_restore(struct net_device *dev);
+
/* port.c */
int dsa_port_set_state(struct dsa_port *dp, u8 state,
struct switchdev_trans *trans);
@@ -139,7 +141,6 @@ int dsa_port_vlan_del(struct dsa_port *dp,
/* slave.c */
extern const struct dsa_device_ops notag_netdev_ops;
void dsa_slave_mii_bus_init(struct dsa_switch *ds);
-void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops);
int dsa_slave_create(struct dsa_port *port, const char *name);
void dsa_slave_destroy(struct net_device *slave_dev);
int dsa_slave_suspend(struct net_device *slave_dev);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 163910699db7..ae505d8e4417 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -602,7 +602,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
wmb();
dev->dsa_ptr = dst;
- return dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+ return dsa_master_ethtool_setup(dst->cpu_dp->netdev);
}
static int dsa_probe(struct platform_device *pdev)
@@ -667,7 +667,7 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
{
int i;
- dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+ dsa_master_ethtool_restore(dst->cpu_dp->netdev);
dst->cpu_dp->netdev->dsa_ptr = NULL;
diff --git a/net/dsa/master.c b/net/dsa/master.c
new file mode 100644
index 000000000000..5e5147ec5a44
--- /dev/null
+++ b/net/dsa/master.c
@@ -0,0 +1,120 @@
+/*
+ * Handling of a master device, switching frames via its switch fabric CPU port
+ *
+ * Copyright (c) 2017 Savoir-faire Linux Inc.
+ * Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include "dsa_priv.h"
+
+static void dsa_master_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats,
+ uint64_t *data)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_port *port = dst->cpu_dp;
+ struct dsa_switch *ds = port->ds;
+ const struct ethtool_ops *ops = port->orig_ethtool_ops;
+ int count = 0;
+
+ if (ops && ops->get_sset_count && ops->get_ethtool_stats) {
+ count = ops->get_sset_count(dev, ETH_SS_STATS);
+ ops->get_ethtool_stats(dev, stats, data);
+ }
+
+ if (ds->ops->get_ethtool_stats)
+ ds->ops->get_ethtool_stats(ds, port->index, data + count);
+}
+
+static int dsa_master_get_sset_count(struct net_device *dev, int sset)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_port *port = dst->cpu_dp;
+ struct dsa_switch *ds = port->ds;
+ const struct ethtool_ops *ops = port->orig_ethtool_ops;
+ int count = 0;
+
+ if (ops && ops->get_sset_count)
+ count += ops->get_sset_count(dev, sset);
+
+ if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
+ count += ds->ops->get_sset_count(ds);
+
+ return count;
+}
+
+static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
+ uint8_t *data)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_port *port = dst->cpu_dp;
+ struct dsa_switch *ds = port->ds;
+ const struct ethtool_ops *ops = port->orig_ethtool_ops;
+ int len = ETH_GSTRING_LEN;
+ int mcount = 0, count;
+ unsigned int i;
+ uint8_t pfx[4];
+ uint8_t *ndata;
+
+ snprintf(pfx, sizeof(pfx), "p%.2d", port->index);
+ /* We do not want to be NULL-terminated, since this is a prefix */
+ pfx[sizeof(pfx) - 1] = '_';
+
+ if (ops && ops->get_sset_count && ops->get_strings) {
+ mcount = ops->get_sset_count(dev, ETH_SS_STATS);
+ ops->get_strings(dev, stringset, data);
+ }
+
+ if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
+ ndata = data + mcount * len;
+ /* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
+ * the output after to prepend our CPU port prefix we
+ * constructed earlier
+ */
+ ds->ops->get_strings(ds, port->index, ndata);
+ count = ds->ops->get_sset_count(ds);
+ for (i = 0; i < count; i++) {
+ memmove(ndata + (i * len + sizeof(pfx)),
+ ndata + i * len, len - sizeof(pfx));
+ memcpy(ndata + i * len, pfx, sizeof(pfx));
+ }
+ }
+}
+
+int dsa_master_ethtool_setup(struct net_device *dev)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_port *port = dst->cpu_dp;
+ struct dsa_switch *ds = port->ds;
+ struct ethtool_ops *ops;
+
+ ops = devm_kzalloc(ds->dev, sizeof(*ops), GFP_KERNEL);
+ if (!ops)
+ return -ENOMEM;
+
+ port->orig_ethtool_ops = dev->ethtool_ops;
+ if (port->orig_ethtool_ops)
+ memcpy(ops, port->orig_ethtool_ops, sizeof(*ops));
+
+ ops->get_sset_count = dsa_master_get_sset_count;
+ ops->get_ethtool_stats = dsa_master_get_ethtool_stats;
+ ops->get_strings = dsa_master_get_strings;
+
+ dev->ethtool_ops = ops;
+
+ return 0;
+}
+
+void dsa_master_ethtool_restore(struct net_device *dev)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_port *port = dst->cpu_dp;
+
+ dev->ethtool_ops = port->orig_ethtool_ops;
+ port->orig_ethtool_ops = NULL;
+}
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2ff4f907d137..d51b10450e1b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -567,82 +567,6 @@ static void dsa_slave_get_strings(struct net_device *dev,
}
}
-static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
- struct ethtool_stats *stats,
- uint64_t *data)
-{
- struct dsa_switch_tree *dst = dev->dsa_ptr;
- struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
- struct dsa_switch *ds = cpu_dp->ds;
- const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
- s8 cpu_port = cpu_dp->index;
- int count = 0;
-
- if (ops && ops->get_sset_count && ops->get_ethtool_stats) {
- count = ops->get_sset_count(dev, ETH_SS_STATS);
- ops->get_ethtool_stats(dev, stats, data);
- }
-
- if (ds->ops->get_ethtool_stats)
- ds->ops->get_ethtool_stats(ds, cpu_port, data + count);
-}
-
-static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
-{
- struct dsa_switch_tree *dst = dev->dsa_ptr;
- struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
- struct dsa_switch *ds = cpu_dp->ds;
- const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
- int count = 0;
-
- if (ops && ops->get_sset_count)
- count += ops->get_sset_count(dev, sset);
-
- if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
- count += ds->ops->get_sset_count(ds);
-
- return count;
-}
-
-static void dsa_cpu_port_get_strings(struct net_device *dev,
- uint32_t stringset, uint8_t *data)
-{
- struct dsa_switch_tree *dst = dev->dsa_ptr;
- struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
- struct dsa_switch *ds = cpu_dp->ds;
- const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
- s8 cpu_port = cpu_dp->index;
- int len = ETH_GSTRING_LEN;
- int mcount = 0, count;
- unsigned int i;
- uint8_t pfx[4];
- uint8_t *ndata;
-
- snprintf(pfx, sizeof(pfx), "p%.2d", cpu_port);
- /* We do not want to be NULL-terminated, since this is a prefix */
- pfx[sizeof(pfx) - 1] = '_';
-
- if (ops && ops->get_sset_count && ops->get_strings) {
- mcount = ops->get_sset_count(dev, ETH_SS_STATS);
- ops->get_strings(dev, stringset, data);
- }
-
- if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
- ndata = data + mcount * len;
- /* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
- * the output after to prepend our CPU port prefix we
- * constructed earlier
- */
- ds->ops->get_strings(ds, cpu_port, ndata);
- count = ds->ops->get_sset_count(ds);
- for (i = 0; i < count; i++) {
- memmove(ndata + (i * len + sizeof(pfx)),
- ndata + i * len, len - sizeof(pfx));
- memcpy(ndata + i * len, pfx, sizeof(pfx));
- }
- }
-}
-
static void dsa_slave_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats,
uint64_t *data)
@@ -979,13 +903,6 @@ static void dsa_slave_get_stats64(struct net_device *dev,
}
}
-void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
-{
- ops->get_sset_count = dsa_cpu_port_get_sset_count;
- ops->get_ethtool_stats = dsa_cpu_port_get_ethtool_stats;
- ops->get_strings = dsa_cpu_port_get_strings;
-}
-
static int dsa_slave_get_rxnfc(struct net_device *dev,
struct ethtool_rxnfc *nfc, u32 *rule_locs)
{
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next 00/14] gtp: Additional feature support
From: Tom Herbert @ 2017-09-19 15:59 UTC (permalink / raw)
To: Harald Welte
Cc: Tom Herbert, David S. Miller, Linux Kernel Network Developers,
Pablo Neira Ayuso, Rohit Seth
In-Reply-To: <20170919124352.l4k67ygb6xt4eloq@nataraja>
On Tue, Sep 19, 2017 at 5:43 AM, Harald Welte <laforge@gnumonks.org> wrote:
> Hi Tom,
>
> first of all, thanks a lot for your patch series. It makes me happy to
> see contributions on the GTP code :)
>
> On Mon, Sep 18, 2017 at 05:38:50PM -0700, Tom Herbert wrote:
>> - IPv6 support
>
> see my detailed comments in other mails. It's unfortunately only
> support for the already "deprecated" IPv6-only PDP contexts, not the
> more modern v4v6 type. In order to interoperate with old and new
> approach, all three cases (v4, v6 and v4v6) should be supported from one
> code base.
>
It sounds like something that can be subsequently added. Do you have a
reference to the spec?
>> - Configurable networking interfaces so that GTP kernel can be used
>> and tested without needing GSN network emulation (i.e. no user space
>> daemon needed).
>
> We have some pretty decent userspace utilities for configuring the GTP
> interfaces and tunnels in the libgtpnl repository, but if it helps
> people to have another way of configuration, I won't be against it.
>
AFAIK those userspace utilities don't support IPv6. Being able to
configure GTP like any other encapsulation will facilitate development
of IPv6 and other features.
> What we have to keep in mind is that the current model of 1:1 mapping of
> a "UDP socket' to a GTP netdevice is conceptually broken and needs to be
> refactored soon (without breaking backwards compatibility). See related
> earlier discussions with patches submitted by Andreas Schultz.
>
I don't think I changed the model, so this can evolve.
> Summary:
>
> In real-world GGSNs you often want to host multiple virtual GGSNs on a
> single GGSN (= UDP socket). Each virtual GGSN terminates into one
> external PDN (packet data network), which can be a private corporate vpn
> or any other IP network, with no routing between those networks.
>
Sounds like network virtualization and VNIs.
> Naively one would assume you "simply" run another virtual GGSN
> instance on another IP address, and then differentiate like that.
>
> However, the problem is that adding a new GGSN IP address will require
> manual configuration changes at each of your roaming partners (easily
> hundreds of operators!) and hence it is avoided at all cost due to the
> related long schedule, requirement for interop testing with each of them,
> etc.
>
> So what you do in reality at operators is that you operate many of those
> virtual GGSNs on the same IP:Port combination (and hence UDP socket),
> which means you have PDP contexts for vGGSN A which terminate on e.g.
> gtp0 and PDP contexts for vGGSN B on gtp1, and so on. The decision
> which gtp-device a given PDP context is a member is made by the GTP-C
> instance. In the kenel we'll have to decouple net-devices from sockets.
>
> So whatever new configuration mechanism or architectural changes we
> introduce, we need to make sure that those will accomodate the "new
> model" rather than introducing further dependencies for which we will
> have to maintain backwards compatibility workaronds later on.
>
>> - Port numbers are configurable
>
> I'm not sure if this is a useful feature. GTP is used only in
> operator-controlled networks and only on standard ports. It's not
> possible to negotiate any non-standard ports on the signaling plane
> either.
>
Bear in mind that we're not required to do everything the GTP spec
says. Adding port configuration is another one of those things that
gives us flexibility and and better capability to test without needing
a full blown GSN network. One feature I didn't implement was UDP
source for flow entropy-- as we've seen with other encapsulation
protocols this helps significantly to get good ECMP in the network. My
impression is GTP designers probably didn't think in terms of getting
best performance. But we can ;-)
>> - Addition of a dst_cache in the GTP structure and other cleanup
>
> looks fine to me.
>
>> - GSO,GRO
>> - Control of zero UDP checksums
>
> [...]
>
>> Additionally, this patch set also includes a couple of general support
>> capabilities:
>>
>> - A facility that allows application specific GSO callbacks
>> - Common functions to get a route fo for an IP tunnel
>
> This is where the "core netdev" folks will have to comment. I'm too
> remote from mainline kernel development these days and will focus on
> reviewing the GTP specific bits of your patch series.
>
Thanks. Obviously, I and many on this list have more expertise on the
core networking side than GTP, so your review is quite welcome.
>> For IPv6 support, the mobile subscriber needs to allow IPv6 addresses,
>> and the remote enpoint can be IPv6.
>
> Minor correction: The mobile subscriber specifically requests a PDP Type
> when establishing the PDP context via Session Management related
> signaling from MS/UE to SGSN. The SGSN simply translates this to GTP
> and then forwards it to the GGSN. So it's acutally not "allow" but
> "specifically request".
>
Okay.
>> Configured the matrix of IPv4/IPv6 mobile subscriber, IPv4/IPv6 remote
>> peer, and GTP version 0 and 1 (eight combinations). Observed
>> connectivity and proper GSO/GRO. Also, tested VXLAN for
>> regression.
>
> I presume those tests were done with manually configured GTP-devices and
> PDP contexts to the (patched) kernel GTP module? If so, I would like to
> strongly suggest interop testing with a different implementation, such
> as real phones on the MS/UE side and e.g. OsmoSGSN. That would,
> however, of course mean that the netlink related bits would have to be
> added to libgtpnl and OsmoGGSN (or ergw) so that you have a daemon for
> the control plane.
>
I also brought up open_ggsn. ggsn to sgsn.
> For IPv6 (and v4v6) PDP contexts there is quite a bit of extra headache
> related to the way how router solicitation/advertisements are modified
> in the 3GPP world.
>
> The address allocation in v4 is simple:
> * MS/UE requests dynamic or fixed IPv4 address via EUA IE of PDP context
> activation
> * GGSN responds with IPv4 address in EUA of Activate PDP context
> response (and then uses netlink to tell the kernel about that
> IPv4 address)
>
> In v6 or the v6 portion of v4v6 it works differently:
> * MS/UE requests dynamic or fixed IPv4 address in EUA IE of PDP context
> activation
> * GGSN responds with an IPv6 address, but that address is *not* used
> for communication, but simply used as an "interface identifier" to
> build a link-local address.
> * MS then uses router solicitation using that link-local address
> * GGSN responds with router advertisement, allocating a single /64
> prefix, from which the MS then generates a fully-qualified IPv6
> source address for communication.
>
> How did you envision this to be done with the v6 support you just added?
> At the very least, the /64 prefix matching would have to be implemented
> so that in fact all addresses within that /64 prefix are matched +
> encapsulated for a given PDP context in the downlink (to phone)
> direction.
>
> Also, I think the responsibility for the router advertisements would be
> in the kernel, too. Otherwise, a GTP-C userspace implementation would
> have to inject packets into the user plane (which is otherwise handled
> completely inside the kernel). Injecting packets would mean that in caes
> GTP sequence numbers are used, that userspace implementation would have
> to alter the sequence numbers of the kernel gtp.ko code using netlink,
> but therre would be race conditions, ...
>
> The router advertisements and neighbor advertisements basically have the
> semantics of one link per PDP context. Each of them is a point-to-point
> link, and it's not one router advertisement that's sent to all of the
> PDP contexts on that gtp-device.
>
> I know it all sucks. I'm still happy to see somebody tackling v6
> support in gtp.c :)
>
I would hope all the above you're describing is mostly control plane
matters. At least a good design decouples data palne and control
plane. I know that GTP is a bit convoluted in this regard.
Tom
^ permalink raw reply
* [PATCH net-next 3/4] net: dsa: setup master ethtool after dsa_ptr
From: Vivien Didelot @ 2017-09-19 15:56 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170919155700.14474-1-vivien.didelot@savoirfairelinux.com>
DSA overrides the master's ethtool ops so that we can inject its CPU
port's statistics. Because of that, we need to setup the ethtool ops
after the master's dsa_ptr pointer has been assigned, not before.
This patch setups the ethtool ops after dsa_ptr is assigned, and
restores them before it gets cleared.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa2.c | 12 +++++++-----
net/dsa/legacy.c | 10 +++-------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index bd19304f862f..032f8bc3e788 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -433,16 +433,17 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
return err;
}
- err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
- if (err)
- return err;
-
/* If we use a tagging format that doesn't have an ethertype
* field, make sure that all packets from this point on get
* sent to the tag format's receive function.
*/
wmb();
dst->cpu_dp->netdev->dsa_ptr = dst;
+
+ err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+ if (err)
+ return err;
+
dst->applied = true;
return 0;
@@ -456,6 +457,8 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
if (!dst->applied)
return;
+ dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+
dst->cpu_dp->netdev->dsa_ptr = NULL;
/* If we used a tagging format that doesn't have an ethertype
@@ -472,7 +475,6 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
dsa_ds_unapply(dst, ds);
}
- dsa_cpu_port_ethtool_restore(dst->cpu_dp);
dst->cpu_dp = NULL;
pr_info("DSA: tree %d unapplied\n", dst->tree);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 91e6f7981d39..163910699db7 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -206,10 +206,6 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
netdev_err(master, "[%d] : can't configure CPU and DSA ports\n",
index);
- ret = dsa_cpu_port_ethtool_setup(ds->dst->cpu_dp);
- if (ret)
- return ret;
-
return 0;
}
@@ -606,7 +602,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
wmb();
dev->dsa_ptr = dst;
- return 0;
+ return dsa_cpu_port_ethtool_setup(dst->cpu_dp);
}
static int dsa_probe(struct platform_device *pdev)
@@ -671,6 +667,8 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
{
int i;
+ dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+
dst->cpu_dp->netdev->dsa_ptr = NULL;
/* If we used a tagging format that doesn't have an ethertype
@@ -686,8 +684,6 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
dsa_switch_destroy(ds);
}
- dsa_cpu_port_ethtool_restore(dst->cpu_dp);
-
dev_put(dst->cpu_dp->netdev);
}
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next 03/14] gtp: Call common functions to get tunnel routes and add dst_cache
From: Tom Herbert @ 2017-09-19 16:05 UTC (permalink / raw)
To: David Miller
Cc: Tom Herbert, Linux Kernel Network Developers, Pablo Neira Ayuso,
Harald Welte, Rohit Seth
In-Reply-To: <20170918.211751.1871429944584121281.davem@davemloft.net>
On Mon, Sep 18, 2017 at 9:17 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <tom@quantonium.net>
> Date: Mon, 18 Sep 2017 17:38:53 -0700
>
>> Call ip_tunnel_get_route and dst_cache to pdp context which should
>> improve performance by obviating the need to perform a route lookup
>> on every packet.
>>
>> Signed-off-by: Tom Herbert <tom@quantonium.net>
>
> Not caused by your changes, but something to think about:
>
>> -static struct rtable *ip4_route_output_gtp(struct flowi4 *fl4,
>> - const struct sock *sk,
>> - __be32 daddr)
>> -{
>> - memset(fl4, 0, sizeof(*fl4));
>> - fl4->flowi4_oif = sk->sk_bound_dev_if;
>> - fl4->daddr = daddr;
>> - fl4->saddr = inet_sk(sk)->inet_saddr;
>> - fl4->flowi4_tos = RT_CONN_FLAGS(sk);
>> - fl4->flowi4_proto = sk->sk_protocol;
>> -
>> - return ip_route_output_key(sock_net(sk), fl4);
>> -}
>
> This and the new dst caching code ignores any source address selection
> done by ip_route_output_key() or the new tunnel route lookup helpers.
>
> Either source address selection should be respected, or if saddr will
> never be modified by a route lookup for some specific reason here,
> that should be documented.
Yes, I noticed that. In this case the source address is intended to be
taken bound on the socket which would imply we aren't interested in
source address selection.
Tom
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-19 16:05 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=e7SE-M4r2nJ8K_FoDMVcN2xYdtw=MetvD5MvXMm9iA1Q@mail.gmail.com>
And 2 more events:
===
$ dmesg --time-format iso | grep RIP
…
2017-09-19T16:52:21,623328+0200 RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
2017-09-19T16:52:40,455296+0200 RIP: 0010:tcp_fastretrans_alert+0x7c8/0x990
2017-09-19T16:52:41,047378+0200 RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
…
2017-09-19T16:54:59,930726+0200 RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
2017-09-19T16:55:07,985767+0200 RIP: 0010:tcp_fastretrans_alert+0x7c8/0x990
2017-09-19T16:55:41,911527+0200 RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
…
===
On pondělí 18. září 2017 23:40:08 CEST Yuchung Cheng wrote:
> On Mon, Sep 18, 2017 at 1:46 PM, Oleksandr Natalenko
>
> <oleksandr@natalenko.name> wrote:
> > Actually, same warning was just triggered with RACK enabled. But main
> > warning was not triggered in this case.
>
> Thanks.
>
> I assume this kernel does not have the patch that Neal proposed in his
> first reply?
>
> The main warning needs to be triggered by another peculiar SACK that
> kicks the sender into recovery again (after undo). Please let it run
> longer if possible to see if we can get both. But the new data does
> indicate the we can (validly) be in CA_Open with retrans_out > 0.
>
> > ===
> > Sep 18 22:44:32 defiant kernel: ------------[ cut here ]------------
> > Sep 18 22:44:32 defiant kernel: WARNING: CPU: 1 PID: 702 at net/ipv4/
> > tcp_input.c:2392 tcp_undo_cwnd_reduction+0xbd/0xd0
> > Sep 18 22:44:32 defiant kernel: Modules linked in: netconsole ctr ccm
> > cls_bpf sch_htb act_mirred cls_u32 sch_ingress sit tunnel4 ip_tunnel
> > 8021q mrp nf_conntrack_ipv6 nf_defrag_ipv6 nft_ct nft_set_bitmap
> > nft_set_hash nft_set_rbtree nf_tables_inet nf_tables_ipv6 nft_masq_ipv4
> > nf_nat_masquerade_ipv4 nft_masq nft_nat nft_counter nft_meta
> > nft_chain_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat
> > nf_conntrack libcrc32c crc32c_generic nf_tables_ipv4 nf_tables tun nct6775
> > nfnetlink hwmon_vid nls_iso8859_1 nls_cp437 vfat fat ext4
> > snd_hda_codec_hdmi mbcache jbd2 snd_hda_codec_realtek
> > snd_hda_codec_generic f2fs arc4 fscrypto intel_rapl iTCO_wdt ath9k
> > iTCO_vendor_support intel_powerclamp ath9k_common ath9k_hw coretemp
> > kvm_intel ath mac80211 kvm irqbypass intel_cstate cfg80211 pcspkr
> > snd_hda_intel snd_hda_codec r8169
> > Sep 18 22:44:32 defiant kernel: joydev evdev mii snd_hda_core mousedev
> > mei_txe input_leds i2c_i801 mac_hid i915 lpc_ich mei shpchp snd_hwdep
> > snd_intel_sst_acpi snd_intel_sst_core snd_soc_rt5670
> > snd_soc_sst_atom_hifi2_platform battery snd_soc_sst_match snd_soc_rl6231
> > drm_kms_helper hci_uart ov5693(C) ov2722(C) lm3554(C) btbcm btqca
> > v4l2_common snd_soc_core btintel snd_compress videodev snd_pcm_dmaengine
> > snd_pcm video bluetooth snd_timer drm media tpm_tis snd i2c_hid soundcore
> > tpm_tis_core rfkill_gpio ac97_bus soc_button_array ecdh_generic rfkill
> > crc16 tpm 8250_dw intel_gtt syscopyarea sysfillrect acpi_pad sysimgblt
> > intel_int0002_vgpio fb_sys_fops pinctrl_cherryview i2c_algo_bit button
> > sch_fq_codel tcp_bbr ifb ip_tables x_tables btrfs xor raid6_pq
> > algif_skcipher af_alg hid_logitech_hidpp hid_logitech_dj usbhid hid uas
> > Sep 18 22:44:32 defiant kernel: usb_storage dm_crypt dm_mod dax raid10
> > md_mod sd_mod crct10dif_pclmul crc32_pclmul crc32c_intel
> > ghash_clmulni_intel pcbc ahci aesni_intel xhci_pci libahci aes_x86_64
> > crypto_simd glue_helper xhci_hcd cryptd libata usbcore scsi_mod
> > usb_common serio sdhci_acpi sdhci led_class mmc_core
> > Sep 18 22:44:32 defiant kernel: CPU: 1 PID: 702 Comm: irq/123-enp3s0
> > Tainted: G WC 4.13.0-pf4 #1
> > Sep 18 22:44:32 defiant kernel: Hardware name: To Be Filled By O.E.M. To
> > Be
> > Filled By O.E.M./J3710-ITX, BIOS P1.30 03/30/2016
> > Sep 18 22:44:32 defiant kernel: task: ffff88923a738000 task.stack:
> > ffff958001500000
> > Sep 18 22:44:32 defiant kernel: RIP:
> > 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> > Sep 18 22:44:32 defiant kernel: RSP: 0018:ffff88927fc83a48 EFLAGS:
> > 00010202
> > Sep 18 22:44:32 defiant kernel: RAX: 0000000000000001 RBX:
> > ffff8892412d9800
> > RCX: ffff88927fc83b0c
> > Sep 18 22:44:32 defiant kernel: RDX: 000000007fffffff RSI:
> > 0000000000000001
> > RDI: ffff8892412d9800
> > Sep 18 22:44:32 defiant kernel: RBP: ffff88927fc83a50 R08:
> > 0000000000000000
> > R09: 0000000018dfb063
> > Sep 18 22:44:32 defiant kernel: R10: 0000000018dfd223 R11:
> > 0000000018dfb063
> > R12: 0000000000005320
> > Sep 18 22:44:32 defiant kernel: R13: ffff88927fc83b10 R14:
> > 0000000000000001
> > R15: ffff88927fc83b0c
> > Sep 18 22:44:32 defiant kernel: FS: 0000000000000000(0000)
> > GS:ffff88927fc80000(0000) knlGS:0000000000000000
> > Sep 18 22:44:32 defiant kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
> > 0000000080050033
> > Sep 18 22:44:32 defiant kernel: CR2: 00007f1cd1a43620 CR3:
> > 0000000114a09000
> > CR4: 00000000001006e0
> > Sep 18 22:44:32 defiant kernel: Call Trace:
> > Sep 18 22:44:32 defiant kernel: <IRQ>
> > Sep 18 22:44:32 defiant kernel: tcp_try_undo_loss+0xb3/0xf0
> > Sep 18 22:44:32 defiant kernel: tcp_fastretrans_alert+0x746/0x990
> > Sep 18 22:44:32 defiant kernel: tcp_ack+0x741/0x1110
> > Sep 18 22:44:32 defiant kernel: tcp_rcv_established+0x325/0x770
> > Sep 18 22:44:32 defiant kernel: ? sk_filter_trim_cap+0xd4/0x1a0
> > Sep 18 22:44:32 defiant kernel: tcp_v4_do_rcv+0x90/0x1e0
> > Sep 18 22:44:32 defiant kernel: tcp_v4_rcv+0x950/0xa10
> > Sep 18 22:44:32 defiant kernel: ? nf_ct_deliver_cached_events+0xb8/0x110
> > [nf_conntrack]
> > Sep 18 22:44:32 defiant kernel: ip_local_deliver_finish+0x68/0x210
> > Sep 18 22:44:32 defiant kernel: ip_local_deliver+0xfa/0x110
> > Sep 18 22:44:32 defiant kernel: ? ip_rcv_finish+0x410/0x410
> > Sep 18 22:44:32 defiant kernel: ip_rcv_finish+0x120/0x410
> > Sep 18 22:44:32 defiant kernel: ip_rcv+0x28e/0x3b0
> > Sep 18 22:44:32 defiant kernel: ? inet_del_offload+0x40/0x40
> > Sep 18 22:44:32 defiant kernel: __netif_receive_skb_core+0x39b/0xb00
> > Sep 18 22:44:32 defiant kernel: ? netif_receive_skb_internal+0xa0/0x480
> > Sep 18 22:44:32 defiant kernel: ? dev_gro_receive+0x2eb/0x4a0
> > Sep 18 22:44:32 defiant kernel: __netif_receive_skb+0x18/0x60
> > Sep 18 22:44:32 defiant kernel: netif_receive_skb_internal+0x98/0x480
> > Sep 18 22:44:32 defiant kernel: netif_receive_skb+0x1c/0x80
> > Sep 18 22:44:32 defiant kernel: ifb_ri_tasklet+0x109/0x26a [ifb]
> > Sep 18 22:44:32 defiant kernel: tasklet_action+0x63/0x120
> > Sep 18 22:44:32 defiant kernel: __do_softirq+0xdf/0x2e5
> > Sep 18 22:44:32 defiant kernel: ? irq_finalize_oneshot.part.39+0xe0/0xe0
> > Sep 18 22:44:32 defiant kernel: do_softirq_own_stack+0x1c/0x30
> > Sep 18 22:44:32 defiant kernel: </IRQ>
> > Sep 18 22:44:32 defiant kernel: do_softirq.part.17+0x4e/0x60
> > Sep 18 22:44:32 defiant kernel: __local_bh_enable_ip+0x77/0x80
> > Sep 18 22:44:32 defiant kernel: irq_forced_thread_fn+0x5c/0x70
> > Sep 18 22:44:32 defiant kernel: irq_thread+0x131/0x1a0
> > Sep 18 22:44:32 defiant kernel: ? wake_threads_waitq+0x30/0x30
> > Sep 18 22:44:32 defiant kernel: kthread+0x126/0x140
> > Sep 18 22:44:32 defiant kernel: ? irq_thread_check_affinity+0x90/0x90
> > Sep 18 22:44:32 defiant kernel: ? kthread_create_on_node+0x70/0x70
> > Sep 18 22:44:32 defiant kernel: ret_from_fork+0x25/0x30
> > Sep 18 22:44:32 defiant kernel: Code: 5d c3 80 60 35 fb 48 8b 00 48 39 c2
> > 74 85 48 3b 83 50 01 00 00 75 eb e9 77 ff ff ff 89 83 48 06 00 00 80 a3
> > 1e 06 00 00 fb eb b3 <0f> ff 5b 5d c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00
> > 00 00 0f 1f Sep 18 22:44:32 defiant kernel: ---[ end trace
> > 1aea180efeedb474 ]--- ===
> >
> > On pondělí 18. září 2017 20:01:42 CEST Yuchung Cheng wrote:
> >> On Mon, Sep 18, 2017 at 10:59 AM, Oleksandr Natalenko
> >>
> >> <oleksandr@natalenko.name> wrote:
> >> > OK. Should I keep FACK disabled?
> >>
> >> Yes since it is disabled in the upstream by default. Although you can
> >> experiment FACK enabled additionally.
> >>
> >> Do we know the crash you first experienced is tied to this issue?
> >>
> >> > On pondělí 18. září 2017 19:51:21 CEST Yuchung Cheng wrote:
> >> >> Can you try this patch to verify my theory with tcp_recovery=0 and 1?
> >> >> thanks
> >> >>
> >> >> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> >> >> index 5af2f04f8859..9253d9ee7d0e 100644
> >> >> --- a/net/ipv4/tcp_input.c
> >> >> +++ b/net/ipv4/tcp_input.c
> >> >> @@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
> >> >> *sk, bool unmark_loss)
> >> >>
> >> >> }
> >> >> tp->snd_cwnd_stamp = tcp_time_stamp;
> >> >> tp->undo_marker = 0;
> >> >>
> >> >> + WARN_ON(tp->retrans_out);
> >> >>
> >> >> }
^ permalink raw reply
* Re: Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-19 16:09 UTC (permalink / raw)
To: Nixiaoming
Cc: xiyou.wangcong@gmail.com, davem@davemloft.net,
edumazet@google.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <E490CD805F7529488761C40FD9D26EF1299D0CAD@DGGEMA505-MBX.china.huawei.com>
On Tue, Sep 19, 2017 at 3:21 AM, Nixiaoming <nixiaoming@huawei.com> wrote:
> On Fri, Sep 15, 2017 at 10:46 AM, Willem de Bruijn
>
> <willemdebruijn.kernel@gmail.com> wrote:
>
>>
>
>> In case of failure we also need to unlink and free match. I
>
>> sent the following:
>
>>
>
>> http://patchwork.ozlabs.org/patch/813945/
>
>
>
> + spin_lock(&po->bind_lock);
>
> + if (po->running &&
>
> + match->type == type &&
>
> match->prot_hook.type == po->prot_hook.type &&
>
> match->prot_hook.dev == po->prot_hook.dev) {
>
> err = -ENOSPC;
>
> @@ -1761,6 +1760,13 @@ static int fanout_add(struct sock *sk, u16 id, u16
> type_flags)
>
> err = 0;
>
> }
>
> }
>
> + spin_unlock(&po->bind_lock);
>
> +
>
> + if (err && !refcount_read(&match->sk_ref)) {
>
> + list_del(&match->list);
>
> + kfree(match);
>
> + }
>
>
>
>
>
> In the function fanout_add add spin_lock to protect po-> running and po->
> fanout,
>
> then whether it should be in the function fanout_release also add spin_lock
> protection ?
po->bind_lock is held when registering and unregistering the
protocol hook. fanout_release does access po->running or
prot_hook.
It is called from packet_release, which does hold the bind_lock
when unregistering the protocol hook.
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Daniel Borkmann @ 2017-09-19 16:12 UTC (permalink / raw)
To: Craig Gallek, Alexei Starovoitov; +Cc: Daniel Mack, David S . Miller, netdev
In-Reply-To: <CAEfhGixLCqEmR=bwBqC6bT=-j22X5+rSScPb=1qWX3F-mi6H3g@mail.gmail.com>
On 09/19/2017 05:08 PM, Craig Gallek wrote:
> On Mon, Sep 18, 2017 at 6:53 PM, Alexei Starovoitov <ast@fb.com> wrote:
>> On 9/18/17 12:30 PM, Craig Gallek wrote:
[...]
>>> +
>>> + next_bit = extract_bit(key->data, node->prefixlen);
>>> + /* If we hit a node that has more than one child or is a
>>> valid
>>> + * prefix itself, do not remove it. Reset the root of the
>>> trim
>>> + * path to its descendant on our path.
>>> + */
>>> + if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
>>> + (node->child[0] && node->child[1]))
>>> + trim = &node->child[next_bit];
>>> + node = rcu_dereference_protected(
>>> + node->child[next_bit],
>>> lockdep_is_held(&trie->lock));
>>> + }
>>> +
>>> + if (!node || node->prefixlen != key->prefixlen ||
>>> + (node->flags & LPM_TREE_NODE_FLAG_IM)) {
>>> + ret = -ENOENT;
>>> + goto out;
>>> + }
>>> +
>>> + trie->n_entries--;
>>> +
>>> + /* If the node we are removing is not a leaf node, simply mark it
>>> + * as intermediate and we are done.
>>> + */
>>> + if (rcu_access_pointer(node->child[0]) ||
>>> + rcu_access_pointer(node->child[1])) {
>>> + node->flags |= LPM_TREE_NODE_FLAG_IM;
>>> + goto out;
>>> + }
>>> +
>>> + /* trim should now point to the slot holding the start of a path
>>> from
>>> + * zero or more intermediate nodes to our leaf node for deletion.
>>> + */
>>> + while ((node = rcu_dereference_protected(
>>> + *trim, lockdep_is_held(&trie->lock)))) {
>>> + RCU_INIT_POINTER(*trim, NULL);
>>> + trim = rcu_access_pointer(node->child[0]) ?
>>> + &node->child[0] :
>>> + &node->child[1];
>>> + kfree_rcu(node, rcu);
>>
>> can it be that some of the nodes this loop walks have
>> both child[0] and [1] ?
> No, the loop above will push trim down the walk every time it
> encounters a node with two children. The only other trim assignment
> is the initial trim = &trie->root. But the only time we would skip
> the assignment in the loop is if the node being removed is the root.
> If the root had multiple children and is being removed, it would be
> handled by the case that turns the node into an intermediate node
> rather than walking the trim path freeing things.
Looks good to me. We should probably still merge nodes once we turn
a real node into an im which just has a single child attached to it;
parent can be im or real node. Thus, we don't need to traverse this
extra one on lookup.
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next 2/3] bpf: Add uniqueness invariant to trivial lpm test implementation
From: Daniel Borkmann @ 2017-09-19 16:12 UTC (permalink / raw)
To: Craig Gallek, Daniel Mack, Alexei Starovoitov, David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-3-kraigatgoog@gmail.com>
On 09/18/2017 09:30 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> The 'trivial' lpm implementation in this test allows equivalent nodes
> to be added (that is, nodes consisting of the same prefix and prefix
> length). For lookup operations, this is fine because insertion happens
> at the head of the (singly linked) list and the first, best match is
> returned. In order to support deletion, the tlpm data structue must
> first enforce uniqueness. This change modifies the insertion algorithm
> to search for equivalent nodes and remove them. Note: the
> BPF_MAP_TYPE_LPM_TRIE already has a uniqueness invariant that is
> implemented as node replacement.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-19 16:12 UTC (permalink / raw)
To: Nixiaoming
Cc: xiyou.wangcong@gmail.com, davem@davemloft.net,
edumazet@google.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAF=yD-+kG8HHwtL55dRh+mayA+1W0iy7WRRK0uyHfkiX=woH_A@mail.gmail.com>
On Tue, Sep 19, 2017 at 12:09 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Tue, Sep 19, 2017 at 3:21 AM, Nixiaoming <nixiaoming@huawei.com> wrote:
>> On Fri, Sep 15, 2017 at 10:46 AM, Willem de Bruijn
>>
>> <willemdebruijn.kernel@gmail.com> wrote:
>>
>>>
>>
>>> In case of failure we also need to unlink and free match. I
>>
>>> sent the following:
>>
>>>
>>
>>> http://patchwork.ozlabs.org/patch/813945/
>>
>>
>>
>> + spin_lock(&po->bind_lock);
>>
>> + if (po->running &&
>>
>> + match->type == type &&
>>
>> match->prot_hook.type == po->prot_hook.type &&
>>
>> match->prot_hook.dev == po->prot_hook.dev) {
>>
>> err = -ENOSPC;
>>
>> @@ -1761,6 +1760,13 @@ static int fanout_add(struct sock *sk, u16 id, u16
>> type_flags)
>>
>> err = 0;
>>
>> }
>>
>> }
>>
>> + spin_unlock(&po->bind_lock);
>>
>> +
>>
>> + if (err && !refcount_read(&match->sk_ref)) {
>>
>> + list_del(&match->list);
>>
>> + kfree(match);
>>
>> + }
>>
>>
>>
>>
>>
>> In the function fanout_add add spin_lock to protect po-> running and po->
>> fanout,
>>
>> then whether it should be in the function fanout_release also add spin_lock
>> protection ?
>
> po->bind_lock is held when registering and unregistering the
> protocol hook. fanout_release does access po->running or
> prot_hook.
whoops. does *not* access.
^ permalink raw reply
* Re: [PATCH net-next 3/3] bpf: Test deletion in BPF_MAP_TYPE_LPM_TRIE
From: Daniel Borkmann @ 2017-09-19 16:12 UTC (permalink / raw)
To: Craig Gallek, Daniel Mack, Alexei Starovoitov, David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-4-kraigatgoog@gmail.com>
On 09/18/2017 09:30 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> Extend the 'random' operation tests to include a delete operation
> (delete half of the nodes from both lpm implementions and ensure
> that lookups are still equivalent).
>
> Also, add a simple IPv4 test which verifies lookup behavior as nodes
> are deleted from the tree.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* RE: [PATCH net-next 05/12] net: dsa: b53: Use a macro to define I/O operations
From: David Laight @ 2017-09-19 16:14 UTC (permalink / raw)
To: 'Florian Fainelli', Vivien Didelot,
netdev@vger.kernel.org
Cc: davem@davemloft.net, andrew@lunn.ch
In-Reply-To: <12153E6A-CB96-4EED-91CE-05604B640927@gmail.com>
> >>> +#define b53_build_op(type, op_size, val_type) \
> >>> +static inline int b53_##type##op_size(struct b53_device *dev, u8
> >page, \
> >>> + u8 reg, val_type val) \
> >>> +{ \
> >>> + int ret; \
> >>> + \
> >>> + mutex_lock(&dev->reg_mutex); \
> >>> + ret = dev->ops->type##op_size(dev, page, reg, val); \
> >>> + mutex_unlock(&dev->reg_mutex); \
> >>> + \
> >>> + return ret; \
> >>> }
> >>
> >> Why separate the 'type' and 'op_size' arguments since they
> >> are always pasted together?
> >
> >For read/write48, the value type is u64.
>
> The way I read David's comment is that instead of calling the macro with read, 48, just combine that
> in a single argument: read48. I don't have a preference about that and can respin eventually.
Indeed, factoring in the type is harder because reads want 'u64 *' not 'u64'.
While that could be factored, it would take more source lines and make
things very obfuscated.
David
^ permalink raw reply
* [RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
From: Douglas Anderson @ 2017-09-19 16:15 UTC (permalink / raw)
To: Oliver Neukum
Cc: groeck, grundler, Douglas Anderson, netdev, linux-usb,
linux-kernel
Every once in a while when my system is under a bit of stress I see
some spammy messages show up in my logs that say:
kevent X may have been dropped
As far as I can tell these messages aren't terribly useful. The
comments around the messages make me think that either workqueues used
to work differently or that the original author of the code missed a
sublety related to them. The error message appears to predate the git
conversion of the kernel so it's somewhat hard to tell.
Specifically, workqueues should work like this:
A) If a workqueue hasn't been scheduled then schedule_work() schedules
it and returns true.
B) If a workqueue has been scheduled (but hasn't started) then
schedule_work() will do nothing and return false.
C) If a workqueue has been scheduled (and has started) then
schedule_work() will put it on the queue to run again and return
true.
Said another way: if you call schedule_work() you can guarantee that
at least one full runthrough of the work will happen again. That
should mean that the work will get processed and I don't see any
reason to think something should be dropped.
Reading the comments in in usbnet_defer_kevent() made me think that B)
and C) would be treated the same. That is: even if we've started the
work and are 99% of the way through then schedule_work() would return
false and the work wouldn't be queued again. If schedule_work()
really did behave that way then, truly, some amount of work would be
lost. ...but it doesn't.
NOTE: if somehow these warnings are useful to mean something then
perhaps we should change them to make it more obvious. If it's
interesting to know when the work is backlogged then we should change
the spam to say "warning: usbnet is backlogged".
ALSO NOTE: If somehow some of the types of work need to be repeated if
usbnet_defer_kevent() is called multiple times then that should be
quite easy to accomplish without dropping any work on the floor. We
can just keep an atomic count for that type of work and add a loop
into usbnet_deferred_kevent().
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/net/usb/usbnet.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 6510e5cc1817..a3e8dbaadcf9 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -450,19 +450,17 @@ static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
}
/* some work can't be done in tasklets, so we use keventd
- *
- * NOTE: annoying asymmetry: if it's active, schedule_work() fails,
- * but tasklet_schedule() doesn't. hope the failure is rare.
*/
void usbnet_defer_kevent (struct usbnet *dev, int work)
{
set_bit (work, &dev->flags);
- if (!schedule_work (&dev->kevent)) {
- if (net_ratelimit())
- netdev_err(dev->net, "kevent %d may have been dropped\n", work);
- } else {
- netdev_dbg(dev->net, "kevent %d scheduled\n", work);
- }
+
+ /* If work is already started this will mark it to run again when it
+ * finishes; if we already had work pending and it hadn't started
+ * yet then that's fine too.
+ */
+ schedule_work (&dev->kevent);
+ netdev_dbg(dev->net, "kevent %d scheduled\n", work);
}
EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
From: Douglas Anderson @ 2017-09-19 16:15 UTC (permalink / raw)
To: Oliver Neukum
Cc: groeck, grundler, Douglas Anderson, netdev, linux-usb,
linux-kernel
In-Reply-To: <20170919161522.995-1-dianders@chromium.org>
In general when you've got a flag communicating that "something needs
to be done" you want to clear that flag _before_ doing the task. If
you clear the flag _after_ doing the task you end up with the risk
that this will happen:
1. Requester sets flag saying task A needs to be done.
2. Worker comes and stars doing task A.
3. Worker finishes task A but hasn't yet cleared the flag.
4. Requester wants to set flag saying task A needs to be done again.
5. Worker clears the flag without doing anything.
Let's make the usbnet codebase consistently clear the flag _before_ it
does the requested work. That way if there's another request to do
the work while the work is already in progress it won't be lost.
NOTES:
- No known bugs are fixed by this; it's just found by code inspection.
- This changes the semantics in some of the error conditions.
-> If we fail to clear the "tx halt" or "rx halt" we still clear the
flag and thus won't retry the clear next time we happen to be in
the work function. Had the old code really wanted to retry these
events it should have re-scheduled the worker anyway.
-> If we fail to allocate memory in usb_alloc_urb() we will still
clear the EVENT_RX_MEMORY flag. This makes it consistent with
how we would deal with other failures, including failure to
allocate a memory chunk in rx_submit(). It can also be noted
that usb_alloc_urb() in this case is allocating much less than 4K
worth of data and probably never fails.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/net/usb/usbnet.c | 50 +++++++++++++++++++++---------------------------
1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index a3e8dbaadcf9..e72547d8d0e6 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1103,8 +1103,6 @@ static void __handle_link_change(struct usbnet *dev)
/* hard_mtu or rx_urb_size may change during link change */
usbnet_update_max_qlen(dev);
-
- clear_bit(EVENT_LINK_CHANGE, &dev->flags);
}
static void usbnet_set_rx_mode(struct net_device *net)
@@ -1118,8 +1116,6 @@ static void __handle_set_rx_mode(struct usbnet *dev)
{
if (dev->driver_info->set_rx_mode)
(dev->driver_info->set_rx_mode)(dev);
-
- clear_bit(EVENT_SET_RX_MODE, &dev->flags);
}
/* work that cannot be done in interrupt context uses keventd.
@@ -1135,7 +1131,7 @@ usbnet_deferred_kevent (struct work_struct *work)
int status;
/* usb_clear_halt() needs a thread context */
- if (test_bit (EVENT_TX_HALT, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_TX_HALT, &dev->flags)) {
unlink_urbs (dev, &dev->txq);
status = usb_autopm_get_interface(dev->intf);
if (status < 0)
@@ -1150,12 +1146,11 @@ usbnet_deferred_kevent (struct work_struct *work)
netdev_err(dev->net, "can't clear tx halt, status %d\n",
status);
} else {
- clear_bit (EVENT_TX_HALT, &dev->flags);
if (status != -ESHUTDOWN)
netif_wake_queue (dev->net);
}
}
- if (test_bit (EVENT_RX_HALT, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_RX_HALT, &dev->flags)) {
unlink_urbs (dev, &dev->rxq);
status = usb_autopm_get_interface(dev->intf);
if (status < 0)
@@ -1170,41 +1165,39 @@ usbnet_deferred_kevent (struct work_struct *work)
netdev_err(dev->net, "can't clear rx halt, status %d\n",
status);
} else {
- clear_bit (EVENT_RX_HALT, &dev->flags);
tasklet_schedule (&dev->bh);
}
}
/* tasklet could resubmit itself forever if memory is tight */
- if (test_bit (EVENT_RX_MEMORY, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_RX_MEMORY, &dev->flags)) {
struct urb *urb = NULL;
int resched = 1;
- if (netif_running (dev->net))
+ if (netif_running (dev->net)) {
urb = usb_alloc_urb (0, GFP_KERNEL);
- else
- clear_bit (EVENT_RX_MEMORY, &dev->flags);
- if (urb != NULL) {
- clear_bit (EVENT_RX_MEMORY, &dev->flags);
- status = usb_autopm_get_interface(dev->intf);
- if (status < 0) {
- usb_free_urb(urb);
- goto fail_lowmem;
- }
- if (rx_submit (dev, urb, GFP_KERNEL) == -ENOLINK)
- resched = 0;
- usb_autopm_put_interface(dev->intf);
+ if (urb != NULL) {
+ status = usb_autopm_get_interface(dev->intf);
+ if (status < 0) {
+ usb_free_urb(urb);
+ goto fail_lowmem;
+ }
+ if (rx_submit (dev, urb, GFP_KERNEL) ==
+ -ENOLINK)
+ resched = 0;
+ usb_autopm_put_interface(dev->intf);
fail_lowmem:
- if (resched)
- tasklet_schedule (&dev->bh);
+ if (resched)
+ tasklet_schedule (&dev->bh);
+ }
}
+
}
- if (test_bit (EVENT_LINK_RESET, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_LINK_RESET, &dev->flags)) {
struct driver_info *info = dev->driver_info;
int retval = 0;
- clear_bit (EVENT_LINK_RESET, &dev->flags);
status = usb_autopm_get_interface(dev->intf);
if (status < 0)
goto skip_reset;
@@ -1221,13 +1214,14 @@ usbnet_deferred_kevent (struct work_struct *work)
}
/* handle link change from link resetting */
+ clear_bit(EVENT_LINK_CHANGE, &dev->flags);
__handle_link_change(dev);
}
- if (test_bit (EVENT_LINK_CHANGE, &dev->flags))
+ if (test_and_clear_bit (EVENT_LINK_CHANGE, &dev->flags))
__handle_link_change(dev);
- if (test_bit (EVENT_SET_RX_MODE, &dev->flags))
+ if (test_and_clear_bit (EVENT_SET_RX_MODE, &dev->flags))
__handle_set_rx_mode(dev);
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* [RFC PATCH 3/3] usbnet: Fix memory leak when rx_submit() fails
From: Douglas Anderson @ 2017-09-19 16:15 UTC (permalink / raw)
To: Oliver Neukum
Cc: groeck, grundler, Douglas Anderson, netdev, linux-usb,
linux-kernel
In-Reply-To: <20170919161522.995-1-dianders@chromium.org>
If rx_submit() returns an error code then nobody calls usb_free_urb().
That means it's leaked.
NOTE: This problem was found solely by code inspection and not due to
any failing test cases.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/net/usb/usbnet.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e72547d8d0e6..4c067aaeea5a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1182,9 +1182,12 @@ usbnet_deferred_kevent (struct work_struct *work)
usb_free_urb(urb);
goto fail_lowmem;
}
- if (rx_submit (dev, urb, GFP_KERNEL) ==
- -ENOLINK)
- resched = 0;
+ status = rx_submit (dev, urb, GFP_KERNEL);
+ if (status) {
+ usb_free_urb(urb);
+ if (status == -ENOLINK)
+ resched = 0;
+ }
usb_autopm_put_interface(dev->intf);
fail_lowmem:
if (resched)
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* [PATCH net] bpf: do not disable/enable BH in bpf_map_free_id()
From: Eric Dumazet @ 2017-09-19 16:15 UTC (permalink / raw)
To: David Miller; +Cc: Martin KaFai Lau, Alexei Starovoitov, netdev
From: Eric Dumazet <edumazet@google.com>
syzkaller reported following splat [1]
Since hard irq are disabled by the caller, bpf_map_free_id()
should not try to enable/disable BH.
Another solution would be to change htab_map_delete_elem() to
defer the free_htab_elem() call after
raw_spin_unlock_irqrestore(&b->lock, flags), but this might be not
enough to cover other code paths.
[1]
WARNING: CPU: 1 PID: 8052 at kernel/softirq.c:161 __local_bh_enable_ip
+0x1e/0x160 kernel/softirq.c:161
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 8052 Comm: syz-executor1 Not tainted 4.13.0-next-20170915+
#23
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
panic+0x1e4/0x417 kernel/panic.c:181
__warn+0x1c4/0x1d9 kernel/panic.c:542
report_bug+0x211/0x2d0 lib/bug.c:183
fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:178
do_trap_no_signal arch/x86/kernel/traps.c:212 [inline]
do_trap+0x260/0x390 arch/x86/kernel/traps.c:261
do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:298
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:311
invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
RIP: 0010:__local_bh_enable_ip+0x1e/0x160 kernel/softirq.c:161
RSP: 0018:ffff8801cdcd7748 EFLAGS: 00010046
RAX: 0000000000000082 RBX: 0000000000000201 RCX: 0000000000000000
RDX: 1ffffffff0b5933c RSI: 0000000000000201 RDI: ffffffff85ac99e0
RBP: ffff8801cdcd7758 R08: ffffffff85b87158 R09: 1ffff10039b9aec6
R10: ffff8801c99f24c0 R11: 0000000000000002 R12: ffffffff817b0b47
R13: dffffc0000000000 R14: ffff8801cdcd77e8 R15: 0000000000000001
__raw_spin_unlock_bh include/linux/spinlock_api_smp.h:176 [inline]
_raw_spin_unlock_bh+0x30/0x40 kernel/locking/spinlock.c:207
spin_unlock_bh include/linux/spinlock.h:361 [inline]
bpf_map_free_id kernel/bpf/syscall.c:197 [inline]
__bpf_map_put+0x267/0x320 kernel/bpf/syscall.c:227
bpf_map_put+0x1a/0x20 kernel/bpf/syscall.c:235
bpf_map_fd_put_ptr+0x15/0x20 kernel/bpf/map_in_map.c:96
free_htab_elem+0xc3/0x1b0 kernel/bpf/hashtab.c:658
htab_map_delete_elem+0x74d/0x970 kernel/bpf/hashtab.c:1063
map_delete_elem kernel/bpf/syscall.c:633 [inline]
SYSC_bpf kernel/bpf/syscall.c:1479 [inline]
SyS_bpf+0x2188/0x46a0 kernel/bpf/syscall.c:1451
entry_SYSCALL_64_fastpath+0x1f/0xbe
Fixes: f3f1c054c288 ("bpf: Introduce bpf_map ID")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/syscall.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cb17e1cd1d434dc2e052a2a9fb0aea967fcf4417..25d074920a009ff682d97bf88e68f466c79bd564 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -186,15 +186,17 @@ static int bpf_map_alloc_id(struct bpf_map *map)
static void bpf_map_free_id(struct bpf_map *map, bool do_idr_lock)
{
+ unsigned long flags;
+
if (do_idr_lock)
- spin_lock_bh(&map_idr_lock);
+ spin_lock_irqsave(&map_idr_lock, flags);
else
__acquire(&map_idr_lock);
idr_remove(&map_idr, map->id);
if (do_idr_lock)
- spin_unlock_bh(&map_idr_lock);
+ spin_unlock_irqrestore(&map_idr_lock, flags);
else
__release(&map_idr_lock);
}
^ permalink raw reply related
* [PATCH V2 net 0/7] Bug fixes for the HNS3 Ethernet Driver for Hip08 SoC
From: Salil Mehta @ 2017-09-19 16:17 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linux-rdma, linuxarm
This patch set presents some bug fixes for the HNS3 Ethernet driver identified
during internal testing & stabilization efforts.
Change Log:
Patch V2: Resolved comments from Leon Romanovsky
Patch V1: Initial Submit
Lipeng (6):
net: hns3: Fixes initialization of phy address from firmware
net: hns3: Fixes the command used to unmap ring from vector
net: hns3: Fixes ring-to-vector map-and-unmap command
net: hns3: Fixes the initialization of MAC address in hardware
net: hns3: Fixes the default VLAN-id of PF
net: hns3: Fixes the premature exit of loop when matching clients
Salil Mehta (1):
net: hns3: Fixes the ether address copy with appropriate API
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 43 +++++-----------------
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h | 8 +++-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 20 ++++++++--
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 7 ++--
4 files changed, 35 insertions(+), 43 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH V2 net 1/7] net: hns3: Fixes initialization of phy address from firmware
From: Salil Mehta @ 2017-09-19 16:17 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20170919161716.92680-1-salil.mehta@huawei.com>
From: Lipeng <lipeng321@huawei.com>
Default phy address of every port is 0. Therefore, phy address for
each port need to be fetched from firmware and device initialized
with fetched non-default phy address.
Fixes: 6427264ef330 ("net: hns3: Add HNS3 Acceleration Engine &
Compatibility Layer Support")
Signed-off-by: Lipeng <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index bb45365..db4e07d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -1066,6 +1066,7 @@ static int hclge_configure(struct hclge_dev *hdev)
for (i = 0; i < ETH_ALEN; i++)
hdev->hw.mac.mac_addr[i] = cfg.mac_addr[i];
hdev->hw.mac.media_type = cfg.media_type;
+ hdev->hw.mac.phy_addr = cfg.phy_addr;
hdev->num_desc = cfg.tqp_desc_num;
hdev->tm_info.num_pg = 1;
hdev->tm_info.num_tc = cfg.tc_num;
--
2.7.4
^ permalink raw reply related
* [PATCH V2 net 2/7] net: hns3: Fixes the command used to unmap ring from vector
From: Salil Mehta @ 2017-09-19 16:17 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20170919161716.92680-1-salil.mehta@huawei.com>
From: Lipeng <lipeng321@huawei.com>
This patch fixes the IMP command being used to unmap the vector
from the corresponding ring.
Fixes: 6427264ef330 ("net: hns3: Add HNS3 Acceleration Engine &
Compatibility Layer Support")
Signed-off-by: Lipeng <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index db4e07d..e324bc6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2779,7 +2779,7 @@ static int hclge_unmap_ring_from_vector(
}
i = 0;
hclge_cmd_setup_basic_desc(&desc,
- HCLGE_OPC_ADD_RING_TO_VECTOR,
+ HCLGE_OPC_DEL_RING_TO_VECTOR,
false);
req->int_vector_id = vector_id;
}
--
2.7.4
^ permalink raw reply related
* [PATCH V2 net 3/7] net: hns3: Fixes ring-to-vector map-and-unmap command
From: Salil Mehta @ 2017-09-19 16:17 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linux-rdma, linuxarm, Mingguang Qu
In-Reply-To: <20170919161716.92680-1-salil.mehta@huawei.com>
From: Lipeng <lipeng321@huawei.com>
This patch fixes the vector-to-ring map and unmap command and adds
INT_GL(for, Gap Limiting Interrupts) and VF id to it as required
by the hardware interface.
Fixes: 6427264ef330 ("net: hns3: Add HNS3 Acceleration Engine &
Compatibility Layer Support")
Signed-off-by: Lipeng <lipeng321@huawei.com>
Signed-off-by: Mingguang Qu <qumingguang@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h | 8 ++++++--
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 8 ++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 91ae013..c2b613b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -238,7 +238,7 @@ struct hclge_tqp_map {
u8 rsv[18];
};
-#define HCLGE_VECTOR_ELEMENTS_PER_CMD 11
+#define HCLGE_VECTOR_ELEMENTS_PER_CMD 10
enum hclge_int_type {
HCLGE_INT_TX,
@@ -252,8 +252,12 @@ struct hclge_ctrl_vector_chain {
#define HCLGE_INT_TYPE_S 0
#define HCLGE_INT_TYPE_M 0x3
#define HCLGE_TQP_ID_S 2
-#define HCLGE_TQP_ID_M (0x3fff << HCLGE_TQP_ID_S)
+#define HCLGE_TQP_ID_M (0x7ff << HCLGE_TQP_ID_S)
+#define HCLGE_INT_GL_IDX_S 13
+#define HCLGE_INT_GL_IDX_M (0x3 << HCLGE_INT_GL_IDX_S)
__le16 tqp_type_and_id[HCLGE_VECTOR_ELEMENTS_PER_CMD];
+ u8 vfid;
+ u8 rsv;
};
#define HCLGE_TC_NUM 8
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index e324bc6..eafd9c6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2680,7 +2680,11 @@ int hclge_map_vport_ring_to_vector(struct hclge_vport *vport, int vector_id,
hnae_get_bit(node->flag, HNAE3_RING_TYPE_B));
hnae_set_field(req->tqp_type_and_id[i], HCLGE_TQP_ID_M,
HCLGE_TQP_ID_S, node->tqp_index);
+ hnae_set_field(req->tqp_type_and_id[i], HCLGE_INT_GL_IDX_M,
+ HCLGE_INT_GL_IDX_S,
+ hnae_get_bit(node->flag, HNAE3_RING_TYPE_B));
req->tqp_type_and_id[i] = cpu_to_le16(req->tqp_type_and_id[i]);
+ req->vfid = vport->vport_id;
if (++i >= HCLGE_VECTOR_ELEMENTS_PER_CMD) {
req->int_cause_num = HCLGE_VECTOR_ELEMENTS_PER_CMD;
@@ -2764,8 +2768,12 @@ static int hclge_unmap_ring_from_vector(
hnae_get_bit(node->flag, HNAE3_RING_TYPE_B));
hnae_set_field(req->tqp_type_and_id[i], HCLGE_TQP_ID_M,
HCLGE_TQP_ID_S, node->tqp_index);
+ hnae_set_field(req->tqp_type_and_id[i], HCLGE_INT_GL_IDX_M,
+ HCLGE_INT_GL_IDX_S,
+ hnae_get_bit(node->flag, HNAE3_RING_TYPE_B));
req->tqp_type_and_id[i] = cpu_to_le16(req->tqp_type_and_id[i]);
+ req->vfid = vport->vport_id;
if (++i >= HCLGE_VECTOR_ELEMENTS_PER_CMD) {
req->int_cause_num = HCLGE_VECTOR_ELEMENTS_PER_CMD;
--
2.7.4
^ permalink raw reply related
* [PATCH V2 net 4/7] net: hns3: Fixes the initialization of MAC address in hardware
From: Salil Mehta @ 2017-09-19 16:17 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20170919161716.92680-1-salil.mehta@huawei.com>
From: Lipeng <lipeng321@huawei.com>
This patch fixes the initialization of MAC address, fetched from HNS3
firmware i.e. when it is not randomly generated, to the HNS3 hardware.
Fixes: ca60906d2795 ("net: hns3: Add support of HNS3 Ethernet Driver for
hip08 SoC")
Signed-off-by: Lipeng <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index 1c3e294..4d68d6e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -2705,10 +2705,11 @@ static void hns3_init_mac_addr(struct net_device *netdev)
eth_hw_addr_random(netdev);
dev_warn(priv->dev, "using random MAC address %pM\n",
netdev->dev_addr);
- /* Also copy this new MAC address into hdev */
- if (h->ae_algo->ops->set_mac_addr)
- h->ae_algo->ops->set_mac_addr(h, netdev->dev_addr);
}
+
+ if (h->ae_algo->ops->set_mac_addr)
+ h->ae_algo->ops->set_mac_addr(h, netdev->dev_addr);
+
}
static void hns3_nic_set_priv_ops(struct net_device *netdev)
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox