* [PATCH 3/3] rhashtable: implement rhashtable_walk_peek() using rhashtable_walk_last_seen()
From: NeilBrown @ 2018-07-06 7:11 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu, Tom Herbert; +Cc: netdev, linux-kernel
In-Reply-To: <153086101070.2825.6850140624411927465.stgit@noble>
rhashtable_walk_last_seen() does most of the work that
rhashtable_walk_peek() needs done, so use it and put
it in a "static inline".
Also update the documentation for rhashtable_walk_peek() to clarify
the expected use case.
Acked-by: Tom Herbert <tom@quantonium.net>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 29 ++++++++++++++++++++++++++++-
lib/rhashtable.c | 34 ----------------------------------
2 files changed, 28 insertions(+), 35 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index d63b472e9d50..96ebc2690027 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -247,10 +247,37 @@ static inline void rhashtable_walk_start(struct rhashtable_iter *iter)
}
void *rhashtable_walk_next(struct rhashtable_iter *iter);
-void *rhashtable_walk_peek(struct rhashtable_iter *iter);
void *rhashtable_walk_last_seen(struct rhashtable_iter *iter);
void rhashtable_walk_stop(struct rhashtable_iter *iter) __releases(RCU);
+/**
+ * rhashtable_walk_peek - Return the next object to use in an interrupted walk
+ * @iter: Hash table iterator
+ *
+ * Returns the "current" object or NULL when the end of the table is reached.
+ * When an rhashtable_walk is interrupted with rhashtable_walk_stop(),
+ * it is often because an object was found that could not be processed
+ * immediately, possible because there is no more space to encode details
+ * of the object (e.g. when producing a seq_file from the table).
+ * When the walk is restarted, the same object needs to be processed again,
+ * if possible. The object might have been removed from the table while
+ * the walk was paused, so it might not be available. In that case, the
+ * normal "next" object should be treated as "current".
+ *
+ * To support this common case, rhashtable_walk_peek() returns the
+ * appropriate object to process after an interrupted walk, either the
+ * one that was most recently returned, or if that doesn't exist - the
+ * next one.
+ *
+ * Returns -EAGAIN if resize event occurred. In that case the iterator
+ * will rewind back to the beginning and you may continue to use it.
+ */
+static inline void *rhashtable_walk_peek(struct rhashtable_iter *iter)
+{
+ return rhashtable_walk_last_seen(iter) ?:
+ rhashtable_walk_next(iter);
+}
+
void rhashtable_free_and_destroy(struct rhashtable *ht,
void (*free_fn)(void *ptr, void *arg),
void *arg);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 2d0227822262..9ddb7134285e 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -913,40 +913,6 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter)
}
EXPORT_SYMBOL_GPL(rhashtable_walk_next);
-/**
- * rhashtable_walk_peek - Return the next object but don't advance the iterator
- * @iter: Hash table iterator
- *
- * Returns the next object or NULL when the end of the table is reached.
- *
- * Returns -EAGAIN if resize event occurred. Note that the iterator
- * will rewind back to the beginning and you may continue to use it.
- */
-void *rhashtable_walk_peek(struct rhashtable_iter *iter)
-{
- struct rhlist_head *list = iter->list;
- struct rhashtable *ht = iter->ht;
- struct rhash_head *p = iter->p;
-
- if (p)
- return rht_obj(ht, ht->rhlist ? &list->rhead : p);
-
- /* No object found in current iter, find next one in the table. */
-
- if (iter->skip) {
- /* A nonzero skip value points to the next entry in the table
- * beyond that last one that was found. Decrement skip so
- * we find the current value. __rhashtable_walk_find_next
- * will restore the original value of skip assuming that
- * the table hasn't changed.
- */
- iter->skip--;
- }
-
- return __rhashtable_walk_find_next(iter);
-}
-EXPORT_SYMBOL_GPL(rhashtable_walk_peek);
-
/**
* rhashtable_walk_last_seen - Return the previously returned object, if available
* @iter: Hash table iterator
^ permalink raw reply related
* [PATCH 2/3] rhashtable: add rhashtable_walk_last_seen()
From: NeilBrown @ 2018-07-06 7:11 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu, Tom Herbert; +Cc: netdev, linux-kernel
In-Reply-To: <153086101070.2825.6850140624411927465.stgit@noble>
rhashtable_walk_last_seen() returns the object returned by
the previous rhashtable_walk_next(), providing it is still in the
table (or was during this grace period).
This works even if rhashtable_walk_stop() and rhashtable_talk_start()
have been called since the last rhashtable_walk_next().
If there have been no calls to rhashtable_walk_next(), or if the
object is gone from the table, then NULL is returned.
This can usefully be used in a seq_file ->start() function.
If the pos is the same as was returned by the last ->next() call,
then rhashtable_walk_last_seen() can be used to re-establish the
current location in the table. If it returns NULL, then
rhashtable_walk_next() should be used.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 1 +
lib/rhashtable.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 657e37ae314c..d63b472e9d50 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -248,6 +248,7 @@ static inline void rhashtable_walk_start(struct rhashtable_iter *iter)
void *rhashtable_walk_next(struct rhashtable_iter *iter);
void *rhashtable_walk_peek(struct rhashtable_iter *iter);
+void *rhashtable_walk_last_seen(struct rhashtable_iter *iter);
void rhashtable_walk_stop(struct rhashtable_iter *iter) __releases(RCU);
void rhashtable_free_and_destroy(struct rhashtable *ht,
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 36f97d0c69ce..2d0227822262 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -947,6 +947,36 @@ void *rhashtable_walk_peek(struct rhashtable_iter *iter)
}
EXPORT_SYMBOL_GPL(rhashtable_walk_peek);
+/**
+ * rhashtable_walk_last_seen - Return the previously returned object, if available
+ * @iter: Hash table iterator
+ *
+ * If rhashtable_walk_next() has previously been called and the object
+ * it returned is still in the hash table, that object is returned again,
+ * otherwise %NULL is returned.
+ *
+ * If the recent rhashtable_walk_next() call was since the most recent
+ * rhashtable_walk_start() call then the returned object may not, strictly
+ * speaking, still be in the table. It will be safe to dereference.
+ *
+ * Note that the iterator is not changed.
+ */
+void *rhashtable_walk_last_seen(struct rhashtable_iter *iter)
+{
+ struct rhashtable *ht = iter->ht;
+ struct rhash_head *p = iter->p;
+
+ if (!p)
+ return NULL;
+ if (!iter->p_is_unsafe || ht->rhlist)
+ return p;
+ rht_for_each_rcu(p, iter->walker.tbl, iter->slot)
+ if (p == iter->p)
+ return p;
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(rhashtable_walk_last_seen);
+
/**
* rhashtable_walk_stop - Finish a hash table walk
* @iter: Hash table iterator
^ permalink raw reply related
* [PATCH 1/3] rhashtable: further improve stability of rhashtable_walk
From: NeilBrown @ 2018-07-06 7:11 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu, Tom Herbert; +Cc: netdev, linux-kernel
In-Reply-To: <153086101070.2825.6850140624411927465.stgit@noble>
If the sequence:
obj = rhashtable_walk_next(iter);
rhashtable_walk_stop(iter);
rhashtable_remove_fast(ht, &obj->head, params);
rhashtable_walk_start(iter);
races with another thread inserting or removing
an object on the same hash chain, a subsequent
rhashtable_walk_next() is not guaranteed to get the "next"
object. It is possible that an object could be
repeated, or missed.
This can be made more reliable by keeping the objects in a hash chain
sorted by memory address. A subsequent rhashtable_walk_next()
call can reliably find the correct position in the list, and thus
find the 'next' object.
It is not possible to take this approach with an rhltable as keeping
the hash chain in order is not so easy. When the first object with a
given key is removed, it is replaced in the chain with the next
object with the same key, and the address of that object may not be
correctly ordered.
I have not yet found any way to achieve the same stability
with rhltables, that doesn't have a major impact on lookup
or insert. No code currently in Linux would benefit from
such extra stability.
With this patch:
- a new object is always inserted after the last object with a
smaller address, or at the start. This preserves the property,
important when allowing objects to be removed and re-added, that
an object is never inserted *after* a position that it previously
held in the list.
- when rhashtable_walk_start() is called, it records that 'p' is not
'safe', meaning that it cannot be dereferenced. The revalidation
that was previously done here is moved to rhashtable_walk_next()
- when rhashtable_walk_next() is called while p is not NULL and not
safe, it walks the chain looking for the first object with an
address greater than p and returns that. If there is none, it moves
to the next hash chain.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable-types.h | 1
include/linux/rhashtable.h | 10 ++++-
lib/rhashtable.c | 82 +++++++++++++++++++++++++-------------
3 files changed, 62 insertions(+), 31 deletions(-)
diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-types.h
index 763d613ce2c2..bc3e84547ba7 100644
--- a/include/linux/rhashtable-types.h
+++ b/include/linux/rhashtable-types.h
@@ -126,6 +126,7 @@ struct rhashtable_iter {
struct rhashtable_walker walker;
unsigned int slot;
unsigned int skip;
+ bool p_is_unsafe;
bool end_of_table;
};
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 10435a77b156..657e37ae314c 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -628,7 +628,12 @@ static inline void *__rhashtable_insert_fast(
(params.obj_cmpfn ?
params.obj_cmpfn(&arg, rht_obj(ht, head)) :
rhashtable_compare(&arg, rht_obj(ht, head)))) {
- pprev = &head->next;
+ if (rhlist) {
+ pprev = &head->next;
+ } else {
+ if (head < obj)
+ headp = &head->next;
+ }
continue;
}
@@ -1124,7 +1129,8 @@ static inline int rhashtable_walk_init(struct rhashtable *ht,
* Note that if you restart a walk after rhashtable_walk_stop you
* may see the same object twice. Also, you may miss objects if
* there are removals in between rhashtable_walk_stop and the next
- * call to rhashtable_walk_start.
+ * call to rhashtable_walk_start. Note that this is different to
+ * rhashtable_walk_enter() which misses objects.
*
* For a completely stable walk you should construct your own data
* structure outside the hash table.
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index f87af707f086..36f97d0c69ce 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -228,6 +228,7 @@ static int rhashtable_rehash_one(struct rhashtable *ht, unsigned int old_hash)
struct bucket_table *old_tbl = rht_dereference(ht->tbl, ht);
struct bucket_table *new_tbl = rhashtable_last_table(ht, old_tbl);
struct rhash_head __rcu **pprev = rht_bucket_var(old_tbl, old_hash);
+ struct rhash_head __rcu **inspos;
int err = -EAGAIN;
struct rhash_head *head, *next, *entry;
spinlock_t *new_bucket_lock;
@@ -256,12 +257,15 @@ static int rhashtable_rehash_one(struct rhashtable *ht, unsigned int old_hash)
new_bucket_lock = rht_bucket_lock(new_tbl, new_hash);
spin_lock_nested(new_bucket_lock, SINGLE_DEPTH_NESTING);
- head = rht_dereference_bucket(new_tbl->buckets[new_hash],
- new_tbl, new_hash);
-
+ inspos = &new_tbl->buckets[new_hash];
+ head = rht_dereference_bucket(*inspos, new_tbl, new_hash);
+ while (!rht_is_a_nulls(head) && head < entry) {
+ inspos = &head->next;
+ head = rht_dereference_bucket(*inspos, new_tbl, new_hash);
+ }
RCU_INIT_POINTER(entry->next, head);
- rcu_assign_pointer(new_tbl->buckets[new_hash], entry);
+ rcu_assign_pointer(*inspos, entry);
spin_unlock(new_bucket_lock);
rcu_assign_pointer(*pprev, next);
@@ -557,6 +561,10 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
return ERR_PTR(-ENOMEM);
head = rht_dereference_bucket(*pprev, tbl, hash);
+ while (!ht->rhlist && !rht_is_a_nulls(head) && head < obj) {
+ pprev = &head->next;
+ head = rht_dereference_bucket(*pprev, tbl, hash);
+ }
RCU_INIT_POINTER(obj->next, head);
if (ht->rhlist) {
@@ -651,10 +659,10 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
*
* This function prepares a hash table walk.
*
- * Note that if you restart a walk after rhashtable_walk_stop you
- * may see the same object twice. Also, you may miss objects if
- * there are removals in between rhashtable_walk_stop and the next
- * call to rhashtable_walk_start.
+ * A walk is guaranteed to return every object that was in
+ * the table before this call, and is still in the table when
+ * rhashtable_walk_next() returns NULL. Duplicates can be
+ * seen, but only if there is a rehash event during the walk.
*
* For a completely stable walk you should construct your own data
* structure outside the hash table.
@@ -738,19 +746,10 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
if (iter->p && !rhlist) {
/*
- * We need to validate that 'p' is still in the table, and
- * if so, update 'skip'
+ * 'p' will be revalidated when rhashtable_walk_next()
+ * is called.
*/
- struct rhash_head *p;
- int skip = 0;
- rht_for_each_rcu(p, iter->walker.tbl, iter->slot) {
- skip++;
- if (p == iter->p) {
- iter->skip = skip;
- goto found;
- }
- }
- iter->p = NULL;
+ iter->p_is_unsafe = true;
} else if (iter->p && rhlist) {
/* Need to validate that 'list' is still in the table, and
* if so, update 'skip' and 'p'.
@@ -867,15 +866,39 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter)
bool rhlist = ht->rhlist;
if (p) {
- if (!rhlist || !(list = rcu_dereference(list->next))) {
- p = rcu_dereference(p->next);
- list = container_of(p, struct rhlist_head, rhead);
- }
- if (!rht_is_a_nulls(p)) {
- iter->skip++;
- iter->p = p;
- iter->list = list;
- return rht_obj(ht, rhlist ? &list->rhead : p);
+ if (!rhlist && iter->p_is_unsafe) {
+ /*
+ * First time next() was called after start().
+ * Need to find location of 'p' in the list.
+ */
+ struct rhash_head *p;
+
+ iter->skip = 0;
+ rht_for_each_rcu(p, iter->walker.tbl, iter->slot) {
+ iter->skip++;
+ if (p <= iter->p)
+ continue;
+
+ /* p is the next object after iter->p */
+ iter->p = p;
+ iter->p_is_unsafe = false;
+ return rht_obj(ht, p);
+ }
+ /* There is no "next" object in the list, move
+ * to next hash chain.
+ */
+ } else {
+ if (!rhlist || !(list = rcu_dereference(list->next))) {
+ p = rcu_dereference(p->next);
+ list = container_of(p, struct rhlist_head,
+ rhead);
+ }
+ if (!rht_is_a_nulls(p)) {
+ iter->skip++;
+ iter->p = p;
+ iter->list = list;
+ return rht_obj(ht, rhlist ? &list->rhead : p);
+ }
}
/* At the end of this slot, switch to next one and then find
@@ -885,6 +908,7 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter)
iter->slot++;
}
+ iter->p_is_unsafe = false;
return __rhashtable_walk_find_next(iter);
}
EXPORT_SYMBOL_GPL(rhashtable_walk_next);
^ permalink raw reply related
* [PATCH 0/3] rhashtable: replace rhashtable_walk_peek implementation
From: NeilBrown @ 2018-07-06 7:11 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu, Tom Herbert; +Cc: netdev, linux-kernel
This is a resend of these three patches with no change
from last time.
The last patch has an Ack from Tom Herbert and Herbert Xu, but the
first two which are necessary pre-requisites don't yet.
Thanks,
NeilBrown
---
NeilBrown (3):
rhashtable: further improve stability of rhashtable_walk
rhashtable: add rhashtable_walk_last_seen()
rhashtable: implement rhashtable_walk_peek() using rhashtable_walk_last_seen()
include/linux/rhashtable-types.h | 1
include/linux/rhashtable.h | 40 +++++++++++-
lib/rhashtable.c | 124 ++++++++++++++++++++++----------------
3 files changed, 110 insertions(+), 55 deletions(-)
--
Signature
^ permalink raw reply
* [PATCH resend] rhashtable: detect when object movement might have invalidated a lookup
From: NeilBrown @ 2018-07-06 7:08 UTC (permalink / raw)
To: Herbert Xu
Cc: Thomas Graf, netdev, linux-kernel, Eric Dumazet, David S. Miller
In-Reply-To: <20180601160613.7ud25g2ux55k3bma@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 5600 bytes --]
Some users of rhashtable might need to change the key
of an object and move it to a different location in the table.
Other users might want to allocate objects using
SLAB_TYPESAFE_BY_RCU which can result in the same memory allocation
being used for a different (type-compatible) purpose and similarly
end up in a different hash-chain.
To support these, we store a unique NULLS_MARKER at the end of
each chain, and when a search fails to find a match, we check
if the NULLS marker found was the expected one. If not,
the search is repeated.
The unique NULLS_MARKER is derived from the address of the
head of the chain.
If an object is removed and re-added to the same hash chain, we won't
notice by looking that the NULLS marker. In this case we must be sure
that it was not re-added *after* its original location, or a lookup may
incorrectly fail. The easiest solution is to ensure it is inserted at
the start of the chain. insert_slow() already does that,
insert_fast() does not. So this patch changes insert_fast to always
insert at the head of the chain.
Note that such a user must do their own double-checking of
the object found by rhashtable_lookup_fast() after ensuring
mutual exclusion which anything that might change the key, such as
successfully taking a new reference.
Signed-off-by: NeilBrown <neilb@suse.com>
---
I'm resending this unchanged. Herbert wasn't sure if we needed all the
functionality provided. I explained that it was useful when
SLAB_TYPESAFE_BY_RCU slabs are used. No further discussion happened.
Thanks,
NeilBrown
include/linux/rhashtable.h | 35 +++++++++++++++++++++++------------
lib/rhashtable.c | 8 +++++---
2 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index eb7111039247..10435a77b156 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -75,8 +75,10 @@ struct bucket_table {
struct rhash_head __rcu *buckets[] ____cacheline_aligned_in_smp;
};
+#define RHT_NULLS_MARKER(ptr) \
+ ((void *)NULLS_MARKER(((unsigned long) (ptr)) >> 1))
#define INIT_RHT_NULLS_HEAD(ptr) \
- ((ptr) = (typeof(ptr)) NULLS_MARKER(0))
+ ((ptr) = RHT_NULLS_MARKER(&(ptr)))
static inline bool rht_is_a_nulls(const struct rhash_head *ptr)
{
@@ -471,6 +473,7 @@ static inline struct rhash_head *__rhashtable_lookup(
.ht = ht,
.key = key,
};
+ struct rhash_head __rcu * const *head;
struct bucket_table *tbl;
struct rhash_head *he;
unsigned int hash;
@@ -478,13 +481,19 @@ static inline struct rhash_head *__rhashtable_lookup(
tbl = rht_dereference_rcu(ht->tbl, ht);
restart:
hash = rht_key_hashfn(ht, tbl, key, params);
- rht_for_each_rcu(he, tbl, hash) {
- if (params.obj_cmpfn ?
- params.obj_cmpfn(&arg, rht_obj(ht, he)) :
- rhashtable_compare(&arg, rht_obj(ht, he)))
- continue;
- return he;
- }
+ head = rht_bucket(tbl, hash);
+ do {
+ rht_for_each_rcu_continue(he, *head, tbl, hash) {
+ if (params.obj_cmpfn ?
+ params.obj_cmpfn(&arg, rht_obj(ht, he)) :
+ rhashtable_compare(&arg, rht_obj(ht, he)))
+ continue;
+ return he;
+ }
+ /* An object might have been moved to a different hash chain,
+ * while we walk along it - better check and retry.
+ */
+ } while (he != RHT_NULLS_MARKER(head));
/* Ensure we see any new tables. */
smp_rmb();
@@ -580,6 +589,7 @@ static inline void *__rhashtable_insert_fast(
.ht = ht,
.key = key,
};
+ struct rhash_head __rcu **headp;
struct rhash_head __rcu **pprev;
struct bucket_table *tbl;
struct rhash_head *head;
@@ -603,12 +613,13 @@ static inline void *__rhashtable_insert_fast(
}
elasticity = RHT_ELASTICITY;
- pprev = rht_bucket_insert(ht, tbl, hash);
+ headp = rht_bucket_insert(ht, tbl, hash);
+ pprev = headp;
data = ERR_PTR(-ENOMEM);
if (!pprev)
goto out;
- rht_for_each_continue(head, *pprev, tbl, hash) {
+ rht_for_each_continue(head, *headp, tbl, hash) {
struct rhlist_head *plist;
struct rhlist_head *list;
@@ -648,7 +659,7 @@ static inline void *__rhashtable_insert_fast(
if (unlikely(rht_grow_above_100(ht, tbl)))
goto slow_path;
- head = rht_dereference_bucket(*pprev, tbl, hash);
+ head = rht_dereference_bucket(*headp, tbl, hash);
RCU_INIT_POINTER(obj->next, head);
if (rhlist) {
@@ -658,7 +669,7 @@ static inline void *__rhashtable_insert_fast(
RCU_INIT_POINTER(list->next, NULL);
}
- rcu_assign_pointer(*pprev, obj);
+ rcu_assign_pointer(*headp, obj);
atomic_inc(&ht->nelems);
if (rht_grow_above_75(ht, tbl))
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 0e04947b7e0c..f87af707f086 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -1164,8 +1164,7 @@ struct rhash_head __rcu **rht_bucket_nested(const struct bucket_table *tbl,
unsigned int hash)
{
const unsigned int shift = PAGE_SHIFT - ilog2(sizeof(void *));
- static struct rhash_head __rcu *rhnull =
- (struct rhash_head __rcu *)NULLS_MARKER(0);
+ static struct rhash_head __rcu *rhnull;
unsigned int index = hash & ((1 << tbl->nest) - 1);
unsigned int size = tbl->size >> tbl->nest;
unsigned int subhash = hash;
@@ -1183,8 +1182,11 @@ struct rhash_head __rcu **rht_bucket_nested(const struct bucket_table *tbl,
subhash >>= shift;
}
- if (!ntbl)
+ if (!ntbl) {
+ if (!rhnull)
+ INIT_RHT_NULLS_HEAD(rhnull, NULL, 0);
return &rhnull;
+ }
return &ntbl[subhash].bucket;
--
2.14.0.rc0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related
* [PATCH v2 2/2] net: macb: Allocate valid memory for TX and RX BD prefetch
From: Harini Katakam @ 2018-07-06 6:48 UTC (permalink / raw)
To: nicolas.ferre, davem, claudiu.beznea
Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux,
harini.katakam
In-Reply-To: <1530859738-11802-1-git-send-email-harini.katakam@xilinx.com>
GEM version in ZynqMP and most versions greater than r1p07 supports
TX and RX BD prefetch. The number of BDs that can be prefetched is a
HW configurable parameter. For ZynqMP, this parameter is 4.
When GEM DMA is accessing the last BD in the ring, even before the
BD is processed and the WRAP bit is noticed, it will have prefetched
BDs outside the BD ring. These will not be processed but it is
necessary to have accessible memory after the last BD. Especially
in cases where SMMU is used, memory locations immediately after the
last BD may not have translation tables triggering HRESP errors. Hence
always allocate extra BDs to accommodate for prefetch.
The value of tx/rx bd prefetch for any given SoC version is:
2 ^ (corresponding field in design config 10 register).
(value of this field >= 1)
Added a capability flag so that older IP versions that do not have
DCFG10 or this prefetch capability are not affected.
Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
v2:
Suggested by Claudiu:
- Renamed CAPS macro and variabled to indicate RD prefetch; renamed buff
- Removed unecessary prefetch variable initialization
Additional change:
- Moved computation of BD prefetch bytes to one place in probe
drivers/net/ethernet/cadence/macb.h | 11 +++++++++++
drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++++++++------
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8665982..3d45f4c 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -166,6 +166,7 @@
#define GEM_DCFG6 0x0294 /* Design Config 6 */
#define GEM_DCFG7 0x0298 /* Design Config 7 */
#define GEM_DCFG8 0x029C /* Design Config 8 */
+#define GEM_DCFG10 0x02A4 /* Design Config 10 */
#define GEM_TXBDCTRL 0x04cc /* TX Buffer Descriptor control register */
#define GEM_RXBDCTRL 0x04d0 /* RX Buffer Descriptor control register */
@@ -490,6 +491,12 @@
#define GEM_SCR2CMP_OFFSET 0
#define GEM_SCR2CMP_SIZE 8
+/* Bitfields in DCFG10 */
+#define GEM_TXBD_RDBUFF_OFFSET 12
+#define GEM_TXBD_RDBUFF_SIZE 4
+#define GEM_RXBD_RDBUFF_OFFSET 8
+#define GEM_RXBD_RDBUFF_SIZE 4
+
/* Bitfields in TISUBN */
#define GEM_SUBNSINCR_OFFSET 0
#define GEM_SUBNSINCR_SIZE 16
@@ -635,6 +642,7 @@
#define MACB_CAPS_USRIO_DISABLED 0x00000010
#define MACB_CAPS_JUMBO 0x00000020
#define MACB_CAPS_GEM_HAS_PTP 0x00000040
+#define MACB_CAPS_BD_RD_PREFETCH 0x00000080
#define MACB_CAPS_FIFO_MODE 0x10000000
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
#define MACB_CAPS_SG_DISABLED 0x40000000
@@ -1203,6 +1211,9 @@ struct macb {
unsigned int max_tuples;
struct tasklet_struct hresp_err_tasklet;
+
+ int rx_bd_rd_prefetch;
+ int tx_bd_rd_prefetch;
};
#ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e56ffa9..c4d08a4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1811,6 +1811,7 @@ static void macb_free_consistent(struct macb *bp)
{
struct macb_queue *queue;
unsigned int q;
+ int size;
bp->macbgem_ops.mog_free_rx_buffers(bp);
@@ -1818,12 +1819,14 @@ static void macb_free_consistent(struct macb *bp)
kfree(queue->tx_skb);
queue->tx_skb = NULL;
if (queue->tx_ring) {
- dma_free_coherent(&bp->pdev->dev, TX_RING_BYTES(bp),
+ size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
+ dma_free_coherent(&bp->pdev->dev, size,
queue->tx_ring, queue->tx_ring_dma);
queue->tx_ring = NULL;
}
if (queue->rx_ring) {
- dma_free_coherent(&bp->pdev->dev, RX_RING_BYTES(bp),
+ size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
+ dma_free_coherent(&bp->pdev->dev, size,
queue->rx_ring, queue->rx_ring_dma);
queue->rx_ring = NULL;
}
@@ -1873,7 +1876,7 @@ static int macb_alloc_consistent(struct macb *bp)
int size;
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
- size = TX_RING_BYTES(bp);
+ size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
&queue->tx_ring_dma,
GFP_KERNEL);
@@ -1889,7 +1892,7 @@ static int macb_alloc_consistent(struct macb *bp)
if (!queue->tx_skb)
goto out_err;
- size = RX_RING_BYTES(bp);
+ size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
&queue->rx_ring_dma, GFP_KERNEL);
if (!queue->rx_ring)
@@ -3794,7 +3797,7 @@ static const struct macb_config np4_config = {
static const struct macb_config zynqmp_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
MACB_CAPS_JUMBO |
- MACB_CAPS_GEM_HAS_PTP,
+ MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -3855,7 +3858,7 @@ static int macb_probe(struct platform_device *pdev)
void __iomem *mem;
const char *mac;
struct macb *bp;
- int err;
+ int err, val;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
mem = devm_ioremap_resource(&pdev->dev, regs);
@@ -3944,6 +3947,18 @@ static int macb_probe(struct platform_device *pdev)
else
dev->max_mtu = ETH_DATA_LEN;
+ if (bp->caps & MACB_CAPS_BD_RD_PREFETCH) {
+ val = GEM_BFEXT(RXBD_RDBUFF, gem_readl(bp, DCFG10));
+ if (val)
+ bp->rx_bd_rd_prefetch = (2 << (val - 1)) *
+ macb_dma_desc_get_size(bp);
+
+ val = GEM_BFEXT(TXBD_RDBUFF, gem_readl(bp, DCFG10));
+ if (val)
+ bp->tx_bd_rd_prefetch = (2 << (val - 1)) *
+ macb_dma_desc_get_size(bp);
+ }
+
mac = of_get_mac_address(np);
if (mac) {
ether_addr_copy(bp->dev->dev_addr, mac);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/2] net: macb: Free RX ring for all queues
From: Harini Katakam @ 2018-07-06 6:48 UTC (permalink / raw)
To: nicolas.ferre, davem, claudiu.beznea
Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux,
harini.katakam
rx ring is allocated for all queues in macb_alloc_consistent.
Free the same for all queues instead of just Q0.
Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
v2:
No changes
drivers/net/ethernet/cadence/macb_main.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 3e93df5..e56ffa9 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1812,13 +1812,7 @@ static void macb_free_consistent(struct macb *bp)
struct macb_queue *queue;
unsigned int q;
- queue = &bp->queues[0];
bp->macbgem_ops.mog_free_rx_buffers(bp);
- if (queue->rx_ring) {
- dma_free_coherent(&bp->pdev->dev, RX_RING_BYTES(bp),
- queue->rx_ring, queue->rx_ring_dma);
- queue->rx_ring = NULL;
- }
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
kfree(queue->tx_skb);
@@ -1828,6 +1822,11 @@ static void macb_free_consistent(struct macb *bp)
queue->tx_ring, queue->tx_ring_dma);
queue->tx_ring = NULL;
}
+ if (queue->rx_ring) {
+ dma_free_coherent(&bp->pdev->dev, RX_RING_BYTES(bp),
+ queue->rx_ring, queue->rx_ring_dma);
+ queue->rx_ring = NULL;
+ }
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH 4.14 55/61] perf bpf: Fix NULL return handling in bpf__prepare_load()
From: Greg Kroah-Hartman @ 2018-07-06 5:47 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, YueHaibing, Daniel Borkmann,
Alexander Shishkin, Namhyung Kim, Peter Zijlstra, netdev,
Arnaldo Carvalho de Melo, Sasha Levin
In-Reply-To: <20180706054712.332416244@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: YueHaibing <yuehaibing@huawei.com>
[ Upstream commit ab4e32ff5aa797eaea551dbb67946e2fcb56cc7e ]
bpf_object__open()/bpf_object__open_buffer can return error pointer or
NULL, check the return values with IS_ERR_OR_NULL() in bpf__prepare_load
and bpf__prepare_load_buffer
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: netdev@vger.kernel.org
Link: https://lkml.kernel.org/n/tip-psf4xwc09n62al2cb9s33v9h@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
tools/perf/util/bpf-loader.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -66,7 +66,7 @@ bpf__prepare_load_buffer(void *obj_buf,
}
obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, name);
- if (IS_ERR(obj)) {
+ if (IS_ERR_OR_NULL(obj)) {
pr_debug("bpf: failed to load buffer\n");
return ERR_PTR(-EINVAL);
}
@@ -102,14 +102,14 @@ struct bpf_object *bpf__prepare_load(con
pr_debug("bpf: successfull builtin compilation\n");
obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, filename);
- if (!IS_ERR(obj) && llvm_param.dump_obj)
+ if (!IS_ERR_OR_NULL(obj) && llvm_param.dump_obj)
llvm__dump_obj(filename, obj_buf, obj_buf_sz);
free(obj_buf);
} else
obj = bpf_object__open(filename);
- if (IS_ERR(obj)) {
+ if (IS_ERR_OR_NULL(obj)) {
pr_debug("bpf: failed to load %s\n", filename);
return obj;
}
^ permalink raw reply
* [PATCH v2 net-next 2/5] net/sched: flower: Add support for matching on vlan ethertype
From: Jianbo Liu @ 2018-07-06 5:38 UTC (permalink / raw)
To: netdev, davem, jiri; +Cc: Jianbo Liu, Jamal Hadi Salim, Cong Wang
In-Reply-To: <20180706053817.17712-1-jianbol@mellanox.com>
As flow dissector stores vlan ethertype, tc flower now can match on that.
It is to make preparation for supporting QinQ.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_flower.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 352876b..da9ec30 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -500,6 +500,7 @@ static int fl_set_key_mpls(struct nlattr **tb,
}
static void fl_set_key_vlan(struct nlattr **tb,
+ __be16 ethertype,
struct flow_dissector_key_vlan *key_val,
struct flow_dissector_key_vlan *key_mask)
{
@@ -516,6 +517,8 @@ static void fl_set_key_vlan(struct nlattr **tb,
VLAN_PRIORITY_MASK;
key_mask->vlan_priority = VLAN_PRIORITY_MASK;
}
+ key_val->vlan_tpid = ethertype;
+ key_mask->vlan_tpid = cpu_to_be16(~0);
}
static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
@@ -592,8 +595,8 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
if (tb[TCA_FLOWER_KEY_ETH_TYPE]) {
ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);
- if (ethertype == htons(ETH_P_8021Q)) {
- fl_set_key_vlan(tb, &key->vlan, &mask->vlan);
+ if (eth_type_vlan(ethertype)) {
+ fl_set_key_vlan(tb, ethertype, &key->vlan, &mask->vlan);
fl_set_key_val(tb, &key->basic.n_proto,
TCA_FLOWER_KEY_VLAN_ETH_TYPE,
&mask->basic.n_proto, TCA_FLOWER_UNSPEC,
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 5/5] net/sched: flower: Add supprt for matching on QinQ vlan headers
From: Jianbo Liu @ 2018-07-06 5:38 UTC (permalink / raw)
To: netdev, davem, jiri; +Cc: Jianbo Liu, Jamal Hadi Salim, Cong Wang
In-Reply-To: <20180706053817.17712-1-jianbol@mellanox.com>
As support dissecting of QinQ inner and outer vlan headers, user can
add rules to match on QinQ vlan headers.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/pkt_cls.h | 4 +++
net/sched/cls_flower.c | 65 ++++++++++++++++++++++++++++++++++----------
2 files changed, 55 insertions(+), 14 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 84e4c1d..c4262d9 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -469,6 +469,10 @@ enum {
TCA_FLOWER_KEY_IP_TTL, /* u8 */
TCA_FLOWER_KEY_IP_TTL_MASK, /* u8 */
+ TCA_FLOWER_KEY_CVLAN_ID, /* be16 */
+ TCA_FLOWER_KEY_CVLAN_PRIO, /* u8 */
+ TCA_FLOWER_KEY_CVLAN_ETH_TYPE, /* be16 */
+
__TCA_FLOWER_MAX,
};
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index e93b13d..487a152 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -35,6 +35,7 @@ struct fl_flow_key {
struct flow_dissector_key_basic basic;
struct flow_dissector_key_eth_addrs eth;
struct flow_dissector_key_vlan vlan;
+ struct flow_dissector_key_vlan cvlan;
union {
struct flow_dissector_key_ipv4_addrs ipv4;
struct flow_dissector_key_ipv6_addrs ipv6;
@@ -449,6 +450,9 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
[TCA_FLOWER_KEY_IP_TOS_MASK] = { .type = NLA_U8 },
[TCA_FLOWER_KEY_IP_TTL] = { .type = NLA_U8 },
[TCA_FLOWER_KEY_IP_TTL_MASK] = { .type = NLA_U8 },
+ [TCA_FLOWER_KEY_CVLAN_ID] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_CVLAN_PRIO] = { .type = NLA_U8 },
+ [TCA_FLOWER_KEY_CVLAN_ETH_TYPE] = { .type = NLA_U16 },
};
static void fl_set_key_val(struct nlattr **tb,
@@ -501,19 +505,20 @@ static int fl_set_key_mpls(struct nlattr **tb,
static void fl_set_key_vlan(struct nlattr **tb,
__be16 ethertype,
+ int vlan_id_key, int vlan_prio_key,
struct flow_dissector_key_vlan *key_val,
struct flow_dissector_key_vlan *key_mask)
{
#define VLAN_PRIORITY_MASK 0x7
- if (tb[TCA_FLOWER_KEY_VLAN_ID]) {
+ if (tb[vlan_id_key]) {
key_val->vlan_id =
- nla_get_u16(tb[TCA_FLOWER_KEY_VLAN_ID]) & VLAN_VID_MASK;
+ nla_get_u16(tb[vlan_id_key]) & VLAN_VID_MASK;
key_mask->vlan_id = VLAN_VID_MASK;
}
- if (tb[TCA_FLOWER_KEY_VLAN_PRIO]) {
+ if (tb[vlan_prio_key]) {
key_val->vlan_priority =
- nla_get_u8(tb[TCA_FLOWER_KEY_VLAN_PRIO]) &
+ nla_get_u8(tb[vlan_prio_key]) &
VLAN_PRIORITY_MASK;
key_mask->vlan_priority = VLAN_PRIORITY_MASK;
}
@@ -596,11 +601,25 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);
if (eth_type_vlan(ethertype)) {
- fl_set_key_vlan(tb, ethertype, &key->vlan, &mask->vlan);
- fl_set_key_val(tb, &key->basic.n_proto,
- TCA_FLOWER_KEY_VLAN_ETH_TYPE,
- &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
- sizeof(key->basic.n_proto));
+ fl_set_key_vlan(tb, ethertype, TCA_FLOWER_KEY_VLAN_ID,
+ TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan,
+ &mask->vlan);
+
+ ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]);
+ if (eth_type_vlan(ethertype)) {
+ fl_set_key_vlan(tb, ethertype,
+ TCA_FLOWER_KEY_CVLAN_ID,
+ TCA_FLOWER_KEY_CVLAN_PRIO,
+ &key->cvlan, &mask->cvlan);
+ fl_set_key_val(tb, &key->basic.n_proto,
+ TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
+ &mask->basic.n_proto,
+ TCA_FLOWER_UNSPEC,
+ sizeof(key->basic.n_proto));
+ } else {
+ key->basic.n_proto = ethertype;
+ mask->basic.n_proto = cpu_to_be16(~0);
+ }
} else {
key->basic.n_proto = ethertype;
mask->basic.n_proto = cpu_to_be16(~0);
@@ -826,6 +845,8 @@ static void fl_init_dissector(struct fl_flow_mask *mask)
FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
FLOW_DISSECTOR_KEY_VLAN, vlan);
FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+ FLOW_DISSECTOR_KEY_CVLAN, cvlan);
+ FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
FLOW_DISSECTOR_KEY_ENC_KEYID, enc_key_id);
FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS, enc_ipv4);
@@ -1201,6 +1222,7 @@ static int fl_dump_key_ip(struct sk_buff *skb,
}
static int fl_dump_key_vlan(struct sk_buff *skb,
+ int vlan_id_key, int vlan_prio_key,
struct flow_dissector_key_vlan *vlan_key,
struct flow_dissector_key_vlan *vlan_mask)
{
@@ -1209,13 +1231,13 @@ static int fl_dump_key_vlan(struct sk_buff *skb,
if (!memchr_inv(vlan_mask, 0, sizeof(*vlan_mask)))
return 0;
if (vlan_mask->vlan_id) {
- err = nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ID,
+ err = nla_put_u16(skb, vlan_id_key,
vlan_key->vlan_id);
if (err)
return err;
}
if (vlan_mask->vlan_priority) {
- err = nla_put_u8(skb, TCA_FLOWER_KEY_VLAN_PRIO,
+ err = nla_put_u8(skb, vlan_prio_key,
vlan_key->vlan_priority);
if (err)
return err;
@@ -1310,13 +1332,28 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
if (fl_dump_key_mpls(skb, &key->mpls, &mask->mpls))
goto nla_put_failure;
- if (fl_dump_key_vlan(skb, &key->vlan, &mask->vlan))
+ if (fl_dump_key_vlan(skb, TCA_FLOWER_KEY_VLAN_ID,
+ TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan, &mask->vlan))
goto nla_put_failure;
- if (mask->vlan.vlan_tpid &&
- nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE, key->basic.n_proto))
+ if (fl_dump_key_vlan(skb, TCA_FLOWER_KEY_CVLAN_ID,
+ TCA_FLOWER_KEY_CVLAN_PRIO,
+ &key->cvlan, &mask->cvlan) ||
+ (mask->cvlan.vlan_tpid &&
+ nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
+ key->cvlan.vlan_tpid)))
goto nla_put_failure;
+ if (mask->cvlan.vlan_tpid) {
+ if (nla_put_be16(skb, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
+ key->basic.n_proto))
+ goto nla_put_failure;
+ } else if (mask->vlan.vlan_tpid) {
+ if (nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
+ key->basic.n_proto))
+ goto nla_put_failure;
+ }
+
if ((key->basic.n_proto == htons(ETH_P_IP) ||
key->basic.n_proto == htons(ETH_P_IPV6)) &&
(fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 3/5] net/flow_dissector: Add support for QinQ dissection
From: Jianbo Liu @ 2018-07-06 5:38 UTC (permalink / raw)
To: netdev, davem, jiri
Cc: Jianbo Liu, Jiri Pirko, Jakub Kicinski, John Crispin, Paolo Abeni,
Michal Kubecek, Tom Herbert, Simon Horman, Sven Eckelmann,
WANG Cong, David Ahern, Jon Maloy
In-Reply-To: <20180706053817.17712-1-jianbol@mellanox.com>
Dissect the QinQ packets to get both outer and inner vlan information,
then store to the extended flow keys.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/flow_dissector.h | 2 ++
net/core/flow_dissector.c | 32 +++++++++++++++++---------------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 8f89968..c644067 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -206,6 +206,7 @@ enum flow_dissector_key_id {
FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
+ FLOW_DISSECTOR_KEY_CVLAN, /* struct flow_dissector_key_flow_vlan */
FLOW_DISSECTOR_KEY_MAX,
};
@@ -237,6 +238,7 @@ struct flow_keys {
struct flow_dissector_key_basic basic;
struct flow_dissector_key_tags tags;
struct flow_dissector_key_vlan vlan;
+ struct flow_dissector_key_vlan cvlan;
struct flow_dissector_key_keyid keyid;
struct flow_dissector_key_ports ports;
struct flow_dissector_key_addrs addrs;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 18cb99b..b555fc2 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -589,7 +589,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
struct flow_dissector_key_tags *key_tags;
struct flow_dissector_key_vlan *key_vlan;
enum flow_dissect_ret fdret;
- bool skip_vlan = false;
+ enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX;
int num_hdrs = 0;
u8 ip_proto = 0;
bool ret;
@@ -748,15 +748,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
}
case htons(ETH_P_8021AD):
case htons(ETH_P_8021Q): {
- const struct vlan_hdr *vlan;
+ const struct vlan_hdr *vlan = NULL;
struct vlan_hdr _vlan;
- bool vlan_tag_present = skb && skb_vlan_tag_present(skb);
__be16 saved_vlan_tpid = proto;
- if (vlan_tag_present)
+ if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX &&
+ skb && skb_vlan_tag_present(skb)) {
proto = skb->protocol;
-
- if (!vlan_tag_present || eth_type_vlan(skb->protocol)) {
+ } else {
vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
data, hlen, &_vlan);
if (!vlan) {
@@ -766,20 +765,23 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
proto = vlan->h_vlan_encapsulated_proto;
nhoff += sizeof(*vlan);
- if (skip_vlan) {
- fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
- break;
- }
}
- skip_vlan = true;
- if (dissector_uses_key(flow_dissector,
- FLOW_DISSECTOR_KEY_VLAN)) {
+ if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX) {
+ dissector_vlan = FLOW_DISSECTOR_KEY_VLAN;
+ } else if (dissector_vlan == FLOW_DISSECTOR_KEY_VLAN) {
+ dissector_vlan = FLOW_DISSECTOR_KEY_CVLAN;
+ } else {
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
+ }
+
+ if (dissector_uses_key(flow_dissector, dissector_vlan)) {
key_vlan = skb_flow_dissector_target(flow_dissector,
- FLOW_DISSECTOR_KEY_VLAN,
+ dissector_vlan,
target_container);
- if (vlan_tag_present) {
+ if (!vlan) {
key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
key_vlan->vlan_priority =
(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 4/5] net/sched: flower: Dump the ethertype encapsulated in vlan
From: Jianbo Liu @ 2018-07-06 5:38 UTC (permalink / raw)
To: netdev, davem, jiri; +Cc: Jianbo Liu, Jamal Hadi Salim, Cong Wang
In-Reply-To: <20180706053817.17712-1-jianbol@mellanox.com>
Currently the encapsulated ethertype is not dumped as it's the same as
TCA_FLOWER_KEY_ETH_TYPE keyvalue. But the dumping result is inconsistent
with input, we add dumping it with TCA_FLOWER_KEY_VLAN_ETH_TYPE.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_flower.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index da9ec30..e93b13d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1313,6 +1313,10 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
if (fl_dump_key_vlan(skb, &key->vlan, &mask->vlan))
goto nla_put_failure;
+ if (mask->vlan.vlan_tpid &&
+ nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE, key->basic.n_proto))
+ goto nla_put_failure;
+
if ((key->basic.n_proto == htons(ETH_P_IP) ||
key->basic.n_proto == htons(ETH_P_IPV6)) &&
(fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 1/5] net/flow_dissector: Save vlan ethertype from headers
From: Jianbo Liu @ 2018-07-06 5:38 UTC (permalink / raw)
To: netdev, davem, jiri
Cc: Jianbo Liu, Jiri Pirko, Simon Horman, Andrew Lunn, Tom Herbert,
John Crispin, Paolo Abeni, Sven Eckelmann, WANG Cong, David Ahern,
Jon Maloy
In-Reply-To: <20180706053817.17712-1-jianbol@mellanox.com>
Change vlan dissector key to save vlan tpid to support both 802.1Q
and 802.1AD ethertype.
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/flow_dissector.h | 2 +-
net/core/flow_dissector.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index adc24df5..8f89968 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -47,7 +47,7 @@ struct flow_dissector_key_tags {
struct flow_dissector_key_vlan {
u16 vlan_id:12,
vlan_priority:3;
- u16 padding;
+ __be16 vlan_tpid;
};
struct flow_dissector_key_mpls {
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 53f96e4..18cb99b 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -751,6 +751,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
const struct vlan_hdr *vlan;
struct vlan_hdr _vlan;
bool vlan_tag_present = skb && skb_vlan_tag_present(skb);
+ __be16 saved_vlan_tpid = proto;
if (vlan_tag_present)
proto = skb->protocol;
@@ -789,6 +790,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
(ntohs(vlan->h_vlan_TCI) &
VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
}
+ key_vlan->vlan_tpid = saved_vlan_tpid;
}
fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 0/5] Introduce matching on double vlan/QinQ headers for TC flower
From: Jianbo Liu @ 2018-07-06 5:38 UTC (permalink / raw)
To: netdev, davem, jiri; +Cc: Jianbo Liu
Currently TC flower supports only one vlan tag, it doesn't match on both outer
and inner vlan headers for QinQ. To do this, we add support to get both outer
and inner vlan headers for flow dissector, and then TC flower do matching on
those information.
We also plan to extend TC command to support this feature. We add new
cvlan_id/cvlan_prio/cvlan_ethtype keywords for inner vlan header. The existing
vlan_id/vlan_prio/vlan_ethtype are for outer vlan header, and vlan_ethtype must
be 802.1q or 802.1ad.
The examples for command and output are as the following.
# tc filter add dev ens1f1 parent ffff: protocol 802.1ad pref 33 \
flower vlan_id 1000 vlan_ethtype 802.1q \
cvlan_id 100 cvlan_ethtype ipv4 \
action vlan pop \
action vlan pop \
action mirred egress redirect dev ens1f1_0
# tc filter show dev ens1f1 ingress
filter protocol 802.1ad pref 33 flower chain 0
filter protocol 802.1ad pref 33 flower chain 0 handle 0x1
vlan_id 1000
vlan_ethtype 802.1Q
cvlan_id 100
cvlan_ethtype ip
eth_type ipv4
in_hw
...
v2:
fix sparse warning.
Jianbo Liu (5):
net/flow_dissector: Save vlan ethertype from headers
net/sched: flower: Add support for matching on vlan ethertype
net/flow_dissector: Add support for QinQ dissection
net/sched: flower: Dump the ethertype encapsulated in vlan
net/sched: flower: Add supprt for matching on QinQ vlan headers
include/net/flow_dissector.h | 4 ++-
include/uapi/linux/pkt_cls.h | 4 +++
net/core/flow_dissector.c | 34 +++++++++++----------
net/sched/cls_flower.c | 70 ++++++++++++++++++++++++++++++++++++--------
4 files changed, 83 insertions(+), 29 deletions(-)
--
2.9.5
^ permalink raw reply
* Re: Crash due to destroying TCP request sockets using SOCK_DESTROY
From: Lorenzo Colitti @ 2018-07-06 4:46 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan; +Cc: netdev, Eric Dumazet, Alistair Strachan
In-Reply-To: <95aaa3d59cb3c9cc11b6b83880d92fec@codeaurora.org>
On Fri, Jul 6, 2018 at 11:37 AM Subash Abhinov Kasiviswanathan
<subashab@codeaurora.org> wrote:
>
> From the call stack, a TCP socket is being destroyed using netlink_diag.
> The memory dump showed that the socket was an inet request socket (in
> state TCP_NEW_SYN_RECV) with refcount of 0.
> [...]
> 13232.479820: <2> refcount_t: underflow; use-after-free.
> 13232.479838: <6> ------------[ cut here ]------------
> 13232.479843: <6> kernel BUG at kernel/msm-4.14/lib/refcount.c:204!
> 13232.479849: <6> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [...]
> 13232.479996: <6> Process netd (pid: 648, stack limit =
> 0xffffff801cf98000)
> 13232.479998: <2> Call trace:
> 13232.480000: <2> refcount_sub_and_test+0x64/0x78
> 13232.480002: <2> refcount_dec_and_test+0x18/0x24
> 13232.480005: <2> sock_gen_put+0x1c/0xb0
> 13232.480009: <2> tcp_diag_destroy+0x54/0x68
> [...]
Looks like for a TCP_NEW_SYN_RECV socket, sock_diag_destroy
essentially ends up doing:
struct request_sock *req = inet_reqsk(sk);
local_bh_disable();
inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
req);
local_bh_enable();
...
sock_gen_put(sk);
It looks like inet_csk_reqsk_queue_drop_and_put calls reqsk_put(req),
which frees the socket, and at that point sock_gen_put is a UAF. Do we
just need:
- inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
- req);
+ inet_csk_reqsk_queue_drop(req->rsk_listener, req);
since sock_gen_put will also end up calling reqsk_put() for a
TCP_SYN_RECV socket?
Alastair - you're able to reproduce this UAF using net_test on qemu,
right? If so, could you try that two-line patch above?
^ permalink raw reply
* [PATCH bpf] xdp: XDP_REDIRECT should check IFF_UP and MTU
From: Toshiaki Makita @ 2018-07-06 2:49 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, John Fastabend
Otherwise we end up with attempting to send packets from down devices
or to send oversized packets, which may cause unexpected driver/device
behaviour. Generic XDP has already done this check, so reuse the logic
in native XDP.
Fixes: 814abfabef3c ("xdp: add bpf_redirect helper function")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
include/linux/filter.h | 6 +++---
kernel/bpf/devmap.c | 7 ++++++-
net/core/filter.c | 9 +++++++--
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 300baad..c73dd73 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -765,8 +765,8 @@ static inline bool bpf_dump_raw_ok(void)
struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
const struct bpf_insn *patch, u32 len);
-static inline int __xdp_generic_ok_fwd_dev(struct sk_buff *skb,
- struct net_device *fwd)
+static inline int xdp_ok_fwd_dev(const struct net_device *fwd,
+ unsigned int pktlen)
{
unsigned int len;
@@ -774,7 +774,7 @@ static inline int __xdp_generic_ok_fwd_dev(struct sk_buff *skb,
return -ENETDOWN;
len = fwd->mtu + fwd->hard_header_len + VLAN_HLEN;
- if (skb->len > len)
+ if (pktlen > len)
return -EMSGSIZE;
return 0;
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 642c97f..d361fc1 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -334,10 +334,15 @@ int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
{
struct net_device *dev = dst->dev;
struct xdp_frame *xdpf;
+ int err;
if (!dev->netdev_ops->ndo_xdp_xmit)
return -EOPNOTSUPP;
+ err = xdp_ok_fwd_dev(dev, xdp->data_end - xdp->data);
+ if (unlikely(err))
+ return err;
+
xdpf = convert_to_xdp_frame(xdp);
if (unlikely(!xdpf))
return -EOVERFLOW;
@@ -350,7 +355,7 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
{
int err;
- err = __xdp_generic_ok_fwd_dev(skb, dst->dev);
+ err = xdp_ok_fwd_dev(dst->dev, skb->len);
if (unlikely(err))
return err;
skb->dev = dst->dev;
diff --git a/net/core/filter.c b/net/core/filter.c
index 0ca6907..2303f73 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3046,12 +3046,16 @@ static int __bpf_tx_xdp(struct net_device *dev,
u32 index)
{
struct xdp_frame *xdpf;
- int sent;
+ int err, sent;
if (!dev->netdev_ops->ndo_xdp_xmit) {
return -EOPNOTSUPP;
}
+ err = xdp_ok_fwd_dev(dev, xdp->data_end - xdp->data);
+ if (unlikely(err))
+ return err;
+
xdpf = convert_to_xdp_frame(xdp);
if (unlikely(!xdpf))
return -EOVERFLOW;
@@ -3285,7 +3289,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
goto err;
}
- if (unlikely((err = __xdp_generic_ok_fwd_dev(skb, fwd))))
+ err = xdp_ok_fwd_dev(fwd, skb->len);
+ if (unlikely(err))
goto err;
skb->dev = fwd;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] ipv4: Return EINVAL when ping_group_range sysctl doesn't map to user ns
From: David Miller @ 2018-07-06 2:52 UTC (permalink / raw)
To: tyhicks
Cc: kuznet, yoshfuji, ebiederm, segoon, netdev, containers,
linux-kernel
In-Reply-To: <1530816563-4478-1-git-send-email-tyhicks@canonical.com>
From: Tyler Hicks <tyhicks@canonical.com>
Date: Thu, 5 Jul 2018 18:49:23 +0000
> The low and high values of the net.ipv4.ping_group_range sysctl were
> being silently forced to the default disabled state when a write to the
> sysctl contained GIDs that didn't map to the associated user namespace.
> Confusingly, the sysctl's write operation would return success and then
> a subsequent read of the sysctl would indicate that the low and high
> values are the overflowgid.
>
> This patch changes the behavior by clearly returning an error when the
> sysctl write operation receives a GID range that doesn't map to the
> associated user namespace. In such a situation, the previous value of
> the sysctl is preserved and that range will be returned in a subsequent
> read of the sysctl.
>
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Looks good to me, applied and queued up for -stable.
Thanks.
^ permalink raw reply
* Crash due to destroying TCP request sockets using SOCK_DESTROY
From: Subash Abhinov Kasiviswanathan @ 2018-07-06 2:37 UTC (permalink / raw)
To: netdev, eric.dumazet, lorenzo
We are seeing a crash on an ARM64 device with Android 4.14 based kernel.
From the call stack, a TCP socket is being destroyed using netlink_diag.
The memory dump showed that the socket was an inet request socket (in
state TCP_NEW_SYN_RECV) with refcount of 0.
The crash seems to have happened during a regression test where wifi
was toggled with some browser activity but it is not very easily
reproducible. I believe netd on Android tries to destroy all sockets in
a system on change of network.
13232.479820: <2> refcount_t: underflow; use-after-free.
13232.479838: <6> ------------[ cut here ]------------
13232.479843: <6> kernel BUG at kernel/msm-4.14/lib/refcount.c:204!
13232.479849: <6> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
13232.479895: <6> CPU: 4 PID: 648 Comm: netd Tainted: G S W O
4.14.49+ #1
13232.479897: <6> task: fffffff5d6e28080 task.stack: ffffff801cf98000
13232.479908: <2> pc : refcount_sub_and_test+0x64/0x78
13232.479910: <2> lr : refcount_sub_and_test+0x64/0x78
13232.479911: <2> sp : ffffff801cf9ba40 pstate : 20400145
13232.479911: <2> x29: ffffff801cf9ba40 x28: fffffff5d6e28080
13232.479914: <2> x27: ffffff801cf9bd10 x26: fffffff4a1428f40
13232.479915: <2> x25: 0000000000000000 x24: ffffffffffffff91
13232.479917: <2> x23: 0000000000000015 x22: fffffff5b837c880
13232.479919: <2> x21: fffffff4a1428f40 x20: 0000000000000000
13232.479920: <2> x19: fffffff4c47c6088 x18: e7b13cd1ecbfea00
13232.479922: <2> x17: 00000008ec3bb553 x16: 011d8776aa792786
13232.479924: <2> x15: e7b13cd1ecbfea00 x14: 000000002bdb7692
13232.479925: <2> x13: 0000000000000000 x12: e7b13cd1ecbfea00
13232.479927: <2> x11: e7b13cd1ecbfea00 x10: 0000000000000000
13232.479928: <2> x9 : e7b13cd1ecbfea00 x8 : 0000000000000000
13232.479929: <2> x7 : 0000000000000001 x6 : 0000000000000001
13232.479931: <2> x5 : 0000000000000000 x4 : 00000c08ed425d69
13232.479932: <2> x3 : 00000066effb6000 x2 : ffffff8f09dc5000
13232.479934: <2> x1 : 0000000000000000 x0 : 0000000000000026
13232.479996: <6> Process netd (pid: 648, stack limit =
0xffffff801cf98000)
13232.479998: <2> Call trace:
13232.480000: <2> refcount_sub_and_test+0x64/0x78
13232.480002: <2> refcount_dec_and_test+0x18/0x24
13232.480005: <2> sock_gen_put+0x1c/0xb0
13232.480009: <2> tcp_diag_destroy+0x54/0x68
13232.480010: <2> inet_diag_cmd_exact+0x78/0xa0
13232.480012: <2> inet_diag_handler_cmd+0xcc/0xf8
13232.480018: <2> sock_diag_rcv_msg+0x130/0x158
13232.480021: <2> netlink_rcv_skb+0xa4/0x11c
13232.480023: <2> sock_diag_rcv+0x34/0x48
13232.480025: <2> netlink_unicast+0x158/0x1f0
13232.480026: <2> netlink_sendmsg+0x334/0x340
13232.480028: <2> sock_sendmsg+0x44/0x60
13232.480031: <2> sock_write_iter+0xac/0xf4
13232.480034: <2> __vfs_write+0x124/0x154
13232.480036: <2> vfs_write+0xcc/0x188
13232.480038: <2> SyS_write+0x60/0xc0
13232.480040: <2> el0_svc_naked+0x34/0x38
13232.480042: <6> Code: 910003fd f0008200 910fd000 97f4158c
(d4210000)
13232.480045: <6> ---[ end trace 994bad5b8077e394 ]---
13232.480061: <6> Kernel panic - not syncing: Fatal exception
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH net-next 0/2] IP listification follow-ups
From: David Miller @ 2018-07-06 2:20 UTC (permalink / raw)
To: ecree; +Cc: netdev
In-Reply-To: <8ab5939a-c34d-1b39-5416-f595bcd86a12@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Thu, 5 Jul 2018 15:45:04 +0100
> While working on IPv6 list processing, I found another bug in the IPv4
> version. So this patch series has that fix, and the IPv6 version with
> both fixes incorporated.
Series applied.
Edward, please put (" ") around the commit header line text in your
Fixes: tags in the future. I fixed it up for you this time.
Thank you.
^ permalink raw reply
* Re: [PATCH net] net: aquantia: vlan unicast address list correct handling
From: David Miller @ 2018-07-06 2:11 UTC (permalink / raw)
To: igor.russkikh; +Cc: netdev, darcari, pavel.belous
In-Reply-To: <6ded5860633d5c93cf5f0e254e61cb17c9a22960.1530714429.git.igor.russkikh@aquantia.com>
From: Igor Russkikh <igor.russkikh@aquantia.com>
Date: Thu, 5 Jul 2018 17:01:09 +0300
> Setting up macvlan/macvtap networks over atlantic NIC results
> in no traffic over these networks because ndo_set_rx_mode did
> not listed UC MACs as registered in unicast filter.
>
> Here we fix that taking into account maximum number of UC
> filters supported by hardware. If more than MAX addresses were
> registered, we just enable promisc and/or allmulti to pass
> the traffic in.
>
> We also remove MULTICAST_ADDRESS_MAX constant from aq_cfg since
> thats not a configurable parameter at all.
>
> Fixes: b21f502 ("net:ethernet:aquantia: Fix for multicast filter handling.")
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Applied and queued up for -stable.
Thanks!
^ permalink raw reply
* Re: [PATCH net] MAINTAINERS: update my email address
From: David Miller @ 2018-07-06 2:09 UTC (permalink / raw)
To: stefan; +Cc: netdev, linux-wpan, aring
In-Reply-To: <20180705115644.24980-1-stefan@datenfreihafen.org>
From: Stefan Schmidt <stefan@datenfreihafen.org>
Date: Thu, 5 Jul 2018 13:56:44 +0200
> The mail server hosting the old address is going to fade out.
> Time to update to an address I control directly.
>
> Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] liquidio: make timeout HZ independent and readable
From: David Miller @ 2018-07-06 2:07 UTC (permalink / raw)
To: hofrat
Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
netdev, linux-kernel
In-Reply-To: <1530555195-28129-1-git-send-email-hofrat@osadl.org>
From: Nicholas Mc Guire <hofrat@osadl.org>
Date: Mon, 2 Jul 2018 20:13:15 +0200
> schedule_timeout_* takes a timeout in jiffies but the code currently is
> passing in a constant which makes this timeout HZ dependent. So define
> a constant with (hopefully) meaningful name and pass it through
> msecs_to_jiffies() to fix the HZ dependency.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> commit f21fb3ed364b ("Add support of Cavium Liquidio ethernet adapters")
Applied, thank you.
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] bpftool: document cgroup tree command
From: Jakub Kicinski @ 2018-07-06 2:01 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev, linux-kernel, kernel-team, Quentin Monnet,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180706010521.23097-2-guro@fb.com>
On Thu, 5 Jul 2018 18:05:21 -0700, Roman Gushchin wrote:
> Describe cgroup tree command in the corresponding bpftool man page.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpftool: introduce cgroup tree command
From: Jakub Kicinski @ 2018-07-06 2:01 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev, linux-kernel, kernel-team, Quentin Monnet,
Daniel Borkmann, Alexei Starovoitov, David Ahern
In-Reply-To: <20180706010521.23097-1-guro@fb.com>
On Thu, 5 Jul 2018 18:05:20 -0700, Roman Gushchin wrote:
> This commit introduces a new bpftool command: cgroup tree.
> The idea is to iterate over the whole cgroup tree and print
> all attached programs.
>
> I was debugging a bpf/systemd issue, and found, that there is
> no simple way to listen all bpf programs attached to cgroups.
> I did master something in bash, but after some time got tired of it,
> and decided, that adding a dedicated bpftool command could be
> a better idea.
>
> So, here it is:
> $ sudo ./bpftool cgroup tree
> CgroupPath
> ID AttachType AttachFlags Name
> /sys/fs/cgroup/system.slice/systemd-machined.service
> 18 ingress
> 17 egress
> /sys/fs/cgroup/system.slice/systemd-logind.service
> 20 ingress
> 19 egress
> /sys/fs/cgroup/system.slice/systemd-udevd.service
> 16 ingress
> 15 egress
> /sys/fs/cgroup/system.slice/systemd-journald.service
> 14 ingress
> 13 egress
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
Looks very useful! Minor nits/questions below. I think the reverse
mapping could also be interesting - similar to how -f flag shows where
program is pinned, we could add a flag which in
# bpftool prog show/list
adds info about cgroups where the program is attached? Obviously as a
future extension.
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 16bee011e16c..125d5b6db568 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -2,7 +2,12 @@
> // Copyright (C) 2017 Facebook
> // Author: Roman Gushchin <guro@fb.com>
>
> +#define _XOPEN_SOURCE 500
> +#include <errno.h>
> #include <fcntl.h>
> +#include <ftw.h>
> +#include <mntent.h>
> +#include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/stat.h>
> @@ -53,7 +58,8 @@ static enum bpf_attach_type parse_attach_type(const char *str)
> }
>
> static int show_bpf_prog(int id, const char *attach_type_str,
> - const char *attach_flags_str)
> + const char *attach_flags_str,
> + int level)
> {
> struct bpf_prog_info info = {};
> __u32 info_len = sizeof(info);
> @@ -78,7 +84,8 @@ static int show_bpf_prog(int id, const char *attach_type_str,
> jsonw_string_field(json_wtr, "name", info.name);
> jsonw_end_object(json_wtr);
> } else {
> - printf("%-8u %-15s %-15s %-15s\n", info.id,
> + printf("%s%-8u %-15s %-15s %-15s\n", level ? " " : "",
> + info.id,
> attach_type_str,
> attach_flags_str,
> info.name);
> @@ -88,7 +95,20 @@ static int show_bpf_prog(int id, const char *attach_type_str,
> return 0;
> }
>
> -static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> +static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> +{
> + __u32 prog_cnt = 0;
> + int ret;
> +
> + ret = bpf_prog_query(cgroup_fd, type, 0, NULL, NULL, &prog_cnt);
> + if (ret)
> + return -1;
> +
> + return prog_cnt;
> +}
> +
> +static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> + int level)
> {
> __u32 prog_ids[1024] = {0};
> char *attach_flags_str;
> @@ -123,7 +143,7 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
>
> for (iter = 0; iter < prog_cnt; iter++)
> show_bpf_prog(prog_ids[iter], attach_type_strings[type],
> - attach_flags_str);
> + attach_flags_str, level);
>
> return 0;
> }
> @@ -161,7 +181,7 @@ static int do_show(int argc, char **argv)
> * If we were able to get the show for at least one
> * attach type, let's return 0.
> */
> - if (show_attached_bpf_progs(cgroup_fd, type) == 0)
> + if (show_attached_bpf_progs(cgroup_fd, type, 0) == 0)
> ret = 0;
> }
>
> @@ -173,6 +193,123 @@ static int do_show(int argc, char **argv)
> return ret;
> }
>
> +static int do_show_tree_fn(const char *fpath, const struct stat *sb,
> + int typeflag, struct FTW *ftw)
> +{
> + enum bpf_attach_type type;
> + bool skip = true;
> + int cgroup_fd;
> +
> + if (typeflag != FTW_D)
> + return 0;
> +
> + cgroup_fd = open(fpath, O_RDONLY);
> + if (cgroup_fd < 0) {
> + p_err("can't open cgroup %s: %s", fpath, strerror(errno));
> + return -1;
> + }
> +
> + for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
> + int count = count_attached_bpf_progs(cgroup_fd, type);
> +
> + if (count < 0 && errno != EINVAL) {
> + p_err("can't query bpf programs attached to %s: %s",
> + fpath, strerror(errno));
> + close(cgroup_fd);
> + return -1;
> + }
> + if (count > 0) {
> + skip = false;
> + break;
> + }
> + }
> +
> + if (skip) {
> + close(cgroup_fd);
> + return 0;
> + }
> +
> + if (json_output) {
> + jsonw_start_object(json_wtr);
> + jsonw_string_field(json_wtr, "cgroup", fpath);
> + jsonw_name(json_wtr, "programs");
> + jsonw_start_array(json_wtr);
> + } else {
> + printf("%s\n", fpath);
> + }
> +
> + for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++)
> + show_attached_bpf_progs(cgroup_fd, type, ftw->level);
> +
> + if (json_output) {
> + jsonw_end_array(json_wtr);
> + jsonw_end_object(json_wtr);
> + }
> +
> + close(cgroup_fd);
> +
> + return 0;
> +}
> +
> +static char *find_cgroup_root(void)
> +{
> + struct mntent *mnt;
> + FILE *f;
> +
> + f = fopen("/proc/mounts", "r");
> + if (f == NULL)
> + return NULL;
> +
> + while ((mnt = getmntent(f))) {
> + if (strcmp(mnt->mnt_type, "cgroup2") == 0) {
> + fclose(f);
> + return strdup(mnt->mnt_dir);
FWIW you don't free this memory.
> + }
> + }
> +
> + fclose(f);
> + return NULL;
> +}
> +
> +static int do_show_tree(int argc, char **argv)
> +{
> + char *cgroup_root;
> + int ret;
> +
> + if (argc > 1) {
> + p_err("too many parameters for cgroup tree");
> + return -1;
> + }
> +
> + if (argc == 1) {
> + cgroup_root = argv[0];
> + } else {
> + cgroup_root = find_cgroup_root();
> +
> + if (!cgroup_root) {
> + p_err("cgroup v2 isn't mounted");
> + return -1;
> + }
> + }
> +
> + if (json_output)
> + jsonw_start_array(json_wtr);
> + else
> + printf("%s\n"
> + "%-8s %-15s %-15s %-15s\n",
> + "CgroupPath",
> + "ID", "AttachType", "AttachFlags", "Name");
> +
> + ret = nftw(cgroup_root, do_show_tree_fn, 1024, FTW_MOUNT);
> + if (ret && errno == ENOENT)
> + p_err("can't iterate over %s: %s", cgroup_root,
> + strerror(errno));
I'm worried this could lead to a duplicated error in JSON output, no?
Is it possible that do_show_tree_fn() would have already printed an
error?
> +
> + if (json_output)
> + jsonw_end_array(json_wtr);
> +
> + return ret;
> +}
> +
> static int do_attach(int argc, char **argv)
> {
> enum bpf_attach_type attach_type;
> @@ -289,6 +426,7 @@ static int do_help(int argc, char **argv)
>
> fprintf(stderr,
> "Usage: %s %s { show | list } CGROUP\n"
> + " %s %s tree [CGROUP_ROOT]\n"
> " %s %s attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]\n"
> " %s %s detach CGROUP ATTACH_TYPE PROG\n"
> " %s %s help\n"
> @@ -298,6 +436,7 @@ static int do_help(int argc, char **argv)
> " " HELP_SPEC_PROGRAM "\n"
> " " HELP_SPEC_OPTIONS "\n"
> "",
> + bin_name, argv[-2],
> bin_name, argv[-2], bin_name, argv[-2],
> bin_name, argv[-2], bin_name, argv[-2]);
>
> @@ -307,6 +446,7 @@ static int do_help(int argc, char **argv)
> static const struct cmd cmds[] = {
> { "show", do_show },
> { "list", do_show },
> + { "tree", do_show_tree },
> { "attach", do_attach },
> { "detach", do_detach },
> { "help", do_help },
Could you please also add this new command to bash completions? It
should be fairly trivial to handle.
^ permalink raw reply
* [PATCH bpf-next 2/2] bpftool: document cgroup tree command
From: Roman Gushchin @ 2018-07-06 1:05 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, Roman Gushchin, Jakub Kicinski,
Quentin Monnet, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180706010521.23097-1-guro@fb.com>
Describe cgroup tree command in the corresponding bpftool man page.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index 7b0e6d453e92..3b3d6c534317 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -21,6 +21,7 @@ MAP COMMANDS
=============
| **bpftool** **cgroup { show | list }** *CGROUP*
+| **bpftool** **cgroup tree** [*CGROUP_ROOT*]
| **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
| **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
| **bpftool** **cgroup help**
@@ -39,6 +40,15 @@ DESCRIPTION
Output will start with program ID followed by attach type,
attach flags and program name.
+ **bpftool cgroup tree** [*CGROUP_ROOT*]
+ Iterate over all cgroups in *CGROUP_ROOT* and list all
+ attached programs. If *CGROUP_ROOT* is not specified,
+ bpftool uses cgroup v2 mountpoint.
+
+ The output is similar to the output of cgroup show/list
+ commands: it starts with absolute cgroup path, followed by
+ program ID, attach type, attach flags and program name.
+
**bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
Attach program *PROG* to the cgroup *CGROUP* with attach type
*ATTACH_TYPE* and optional *ATTACH_FLAGS*.
--
2.14.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