* [PATCH 0/6] rhashtable: assorted fixes and enhancements
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
Hi,
I'm hoping to use rhashtable in lustre, to replace the resizeable
hashtable implementation in libcfs.
While working through the conversion I found some minor bugs in the
rhashtable code and documentation, and some areas where enhancements
could make rhashtable a better fit for lustre.
Following 6 patches are the result. Please review.
It would help me if I could get an Ack for these patches, and could
then submit them through the drivers/staging tree together with the
lustre changes that make use to rhashtable. The first 2 are mostly
just fixes to comments and can go in through the netdev tree if you
prefer - the last 4 are needed for lustre to work
correctly/optimally.
Thanks,
NeilBrown
---
NeilBrown (6):
rhashtable: improve documentation for rhashtable_walk_peek()
rhashtable: remove outdated comments about grow_decision etc
rhashtable: reset iter when rhashtable_walk_start sees new table
rhashtable: allow a walk of the hash table without missing objects.
rhashtable: support guaranteed successful insertion.
rhashtable: allow element counting to be disabled.
include/linux/rhashtable.h | 89 ++++++++++++++++++++++++++++----------
lib/rhashtable.c | 102 +++++++++++++++++++++++++++++++-------------
2 files changed, 136 insertions(+), 55 deletions(-)
--
Signature
^ permalink raw reply
* [PATCH 1/6] rhashtable: improve documentation for rhashtable_walk_peek()
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
The documentation for rhashtable_walk_peek() wrong. It claims to
return the *next* entry, whereas it in fact returns the *previous*
entry.
However if no entries have yet been returned - or if the iterator
was reset due to a resize event, then rhashtable_walk_peek()
*does* return the next entry, but also advances the iterator.
I suspect that this interface should be discarded and the one user
should be changed to not require it. Possibly this patch should be
seen as a first step in that conversation.
This patch mostly corrects the documentation, but does make a
small code change so that the documentation can be correct without
listing too many special cases. I don't think the one user will
be affected by the code change.
Signed-off-by: NeilBrown <neilb@suse.com>
---
lib/rhashtable.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 3825c30aaa36..24a57ca494cb 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -853,13 +853,17 @@ 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
+ * rhashtable_walk_peek - Return the previously returned object without advancing the iterator
* @iter: Hash table iterator
*
- * Returns the next object or NULL when the end of the table is reached.
+ * Returns the last object returned, or NULL if no object has yet been returned.
+ * If the previously returned object has since been removed, then some other arbitrary
+ * object maybe returned, or possibly NULL will be returned. In that case, the
+ * iterator might be advanced.
*
* Returns -EAGAIN if resize event occurred. Note that the iterator
- * will rewind back to the beginning and you may continue to use it.
+ * will rewind back to the beginning and rhashtable_walk_next() should be
+ * used to get the next object.
*/
void *rhashtable_walk_peek(struct rhashtable_iter *iter)
{
@@ -880,7 +884,12 @@ void *rhashtable_walk_peek(struct rhashtable_iter *iter)
* the table hasn't changed.
*/
iter->skip--;
- }
+ } else
+ /* ->skip is only zero after rhashtable_walk_start()
+ * or when the iterator is reset. In this case there
+ * is no previous object to return.
+ */
+ return NULL;
return __rhashtable_walk_find_next(iter);
}
^ permalink raw reply related
* [PATCH 2/6] rhashtable: remove outdated comments about grow_decision etc
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
grow_decision and shink_decision no longer exist, so remove
the remaining references to them.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index c9df2527e0cd..3bd19d29f46b 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -834,9 +834,8 @@ static inline void *__rhashtable_insert_fast(
*
* It is safe to call this function from atomic context.
*
- * Will trigger an automatic deferred table resizing if the size grows
- * beyond the watermark indicated by grow_decision() which can be passed
- * to rhashtable_init().
+ * Will trigger an automatic deferred table resizing if residency in the
+ * table grows beyond 70%.
*/
static inline int rhashtable_insert_fast(
struct rhashtable *ht, struct rhash_head *obj,
@@ -864,9 +863,8 @@ static inline int rhashtable_insert_fast(
*
* It is safe to call this function from atomic context.
*
- * Will trigger an automatic deferred table resizing if the size grows
- * beyond the watermark indicated by grow_decision() which can be passed
- * to rhashtable_init().
+ * Will trigger an automatic deferred table resizing if residency in the
+ * table grows beyond 70%.
*/
static inline int rhltable_insert_key(
struct rhltable *hlt, const void *key, struct rhlist_head *list,
@@ -888,9 +886,8 @@ static inline int rhltable_insert_key(
*
* It is safe to call this function from atomic context.
*
- * Will trigger an automatic deferred table resizing if the size grows
- * beyond the watermark indicated by grow_decision() which can be passed
- * to rhashtable_init().
+ * Will trigger an automatic deferred table resizing if residency in the
+ * table grows beyond 70%.
*/
static inline int rhltable_insert(
struct rhltable *hlt, struct rhlist_head *list,
@@ -920,9 +917,8 @@ static inline int rhltable_insert(
*
* It is safe to call this function from atomic context.
*
- * Will trigger an automatic deferred table resizing if the size grows
- * beyond the watermark indicated by grow_decision() which can be passed
- * to rhashtable_init().
+ * Will trigger an automatic deferred table resizing if residency in the
+ * table grows beyond 70%.
*/
static inline int rhashtable_lookup_insert_fast(
struct rhashtable *ht, struct rhash_head *obj,
@@ -979,9 +975,8 @@ static inline void *rhashtable_lookup_get_insert_fast(
*
* Lookups may occur in parallel with hashtable mutations and resizing.
*
- * Will trigger an automatic deferred table resizing if the size grows
- * beyond the watermark indicated by grow_decision() which can be passed
- * to rhashtable_init().
+ * Will trigger an automatic deferred table resizing if residency in the
+ * table grows beyond 70%.
*
* Returns zero on success.
*/
@@ -1132,8 +1127,8 @@ static inline int __rhashtable_remove_fast(
* walk the bucket chain upon removal. The removal operation is thus
* considerable slow if the hash table is not correctly sized.
*
- * Will automatically shrink the table via rhashtable_expand() if the
- * shrink_decision function specified at rhashtable_init() returns true.
+ * Will automatically shrink the table if permitted when residency drops
+ * below 30%.
*
* Returns zero on success, -ENOENT if the entry could not be found.
*/
@@ -1154,8 +1149,8 @@ static inline int rhashtable_remove_fast(
* walk the bucket chain upon removal. The removal operation is thus
* considerable slow if the hash table is not correctly sized.
*
- * Will automatically shrink the table via rhashtable_expand() if the
- * shrink_decision function specified at rhashtable_init() returns true.
+ * Will automatically shrink the table if permitted when residency drops
+ * below 30%
*
* Returns zero on success, -ENOENT if the entry could not be found.
*/
^ permalink raw reply related
* [PATCH 3/6] rhashtable: reset intr when rhashtable_walk_start sees new table
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
The documentation claims that when rhashtable_walk_start_check()
detects a resize event, it will rewind back to the beginning
of the table. This is not true. We need to set ->slot and
->skip to be zero for it to be true.
Signed-off-by: NeilBrown <neilb@suse.com>
---
lib/rhashtable.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 24a57ca494cb..08018198f045 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -733,6 +733,8 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
if (!iter->walker.tbl && !iter->end_of_table) {
iter->walker.tbl = rht_dereference_rcu(ht->tbl, ht);
+ iter->slot = 0;
+ iter->skip = 0;
return -EAGAIN;
}
^ permalink raw reply related
* [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
When a walk of the hashtable can be done entirely under RCU,
no objects will be missed - though seeing duplicates is possible.
This is because a cursor is kept in iter->p.
Without the cursor we depend on the ->skip counter. If an object
before the current location in hash chain is removed, the ->skip
counter will be too large and would could miss a later object.
In many cases where the walker needs to drop out of RCU protection,
it will take a reference to the object and this can prevent it from
being removed from the hash table. In those cases, the last-returned
object can still be used as a cursor. rhashtable cannot detect
these cases itself.
This patch adds a new rhashtable_walk_start_continue() interface which
is passed the last object returned. This can be used if the caller
knows that the object is still in the hash table. When it is used,
a walk of the hash table will return every object that was in the
hastable for the duration of the walk, at least once. This can be
used, for example, to selectively delete objects from the table.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 28 ++++++++++++++++++++++++++--
lib/rhashtable.c | 42 ++++++++++++++++++++++++++----------------
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 3bd19d29f46b..4ffd96949d4f 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -387,11 +387,35 @@ void *rhashtable_insert_slow(struct rhashtable *ht, const void *key,
void rhashtable_walk_enter(struct rhashtable *ht,
struct rhashtable_iter *iter);
void rhashtable_walk_exit(struct rhashtable_iter *iter);
-int rhashtable_walk_start_check(struct rhashtable_iter *iter) __acquires(RCU);
+int rhashtable_walk_start_continue(struct rhashtable_iter *iter,
+ struct rhash_head *obj) __acquires(RCU);
+
+/**
+ * rhashtable_walk_start_check - Start a hash table walk
+ * @iter: Hash table iterator
+ *
+ * Start a hash table walk at the current iterator position. Note that we take
+ * the RCU lock in all cases including when we return an error. So you must
+ * always call rhashtable_walk_stop to clean up.
+ *
+ * Returns zero if successful.
+ *
+ * Returns -EAGAIN if resize event occured. Note that the iterator
+ * will rewind back to the beginning and you may use it immediately
+ * by calling rhashtable_walk_next.
+ *
+ * rhashtable_walk_start is defined as an inline variant that returns
+ * void. This is preferred in cases where the caller would ignore
+ * resize events and always continue.
+ */
+static inline int rhashtable_walk_start_check(struct rhashtable_iter *iter)
+{
+ return rhashtable_walk_start_continue(iter, NULL);
+}
static inline void rhashtable_walk_start(struct rhashtable_iter *iter)
{
- (void)rhashtable_walk_start_check(iter);
+ (void)rhashtable_walk_start_continue(iter, NULL);
}
void *rhashtable_walk_next(struct rhashtable_iter *iter);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 08018198f045..fd6f320b9704 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -702,30 +702,41 @@ void rhashtable_walk_exit(struct rhashtable_iter *iter)
EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
/**
- * rhashtable_walk_start_check - Start a hash table walk
- * @iter: Hash table iterator
+ * rhashtable_walk_start_continue - Restart a hash table walk from last object
+ * @iter: Hask table iterator
+ * @obj: pointer to rhash_head in last object returned.
+ *
+ * Restart a hash table walk, ensuring not to miss any objects. The
+ * previously returned object must still be in the hash table, and must be
+ * provided as an argument.
*
- * Start a hash table walk at the current iterator position. Note that we take
- * the RCU lock in all cases including when we return an error. So you must
- * always call rhashtable_walk_stop to clean up.
+ * When rhashtable_walk_start() or rhashtable_walk_start_check() is used,
+ * a deletion since the previous walk_start can result in objects being missed
+ * as a hash chain might be shorter than expected. This can be avoided by
+ * using the last returned object as a cursor.
*
- * Returns zero if successful.
+ * If the @obj passed is NULL, or not the most recently returned object,
+ * rhashtable_walk_start_continue() will act like rhashtable_walk_start_check();
*
- * Returns -EAGAIN if resize event occured. Note that the iterator
- * will rewind back to the beginning and you may use it immediately
- * by calling rhashtable_walk_next.
+ * Returns -EAGAIN if a resize event was detected. The iterator will
+ * rewind back to the beginning and can be used immediately. Seeing duplicates
+ * is possible but missing objects isn't.
+ * Returns zero if no resize event was detected. This does not guarantee
+ * that no duplicates will be seen.
*
- * rhashtable_walk_start is defined as an inline variant that returns
- * void. This is preferred in cases where the caller would ignore
- * resize events and always continue.
+ * Always takes the RCU read lock, so rhashtable_walk_stop() must always be called
+ * to clean up.
*/
-int rhashtable_walk_start_check(struct rhashtable_iter *iter)
+int rhashtable_walk_start_continue(struct rhashtable_iter *iter, struct rhash_head *obj)
__acquires(RCU)
{
struct rhashtable *ht = iter->ht;
rcu_read_lock();
+ if (!obj || iter->p != obj)
+ iter->p = NULL;
+
spin_lock(&ht->lock);
if (iter->walker.tbl)
list_del(&iter->walker.list);
@@ -733,6 +744,7 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
if (!iter->walker.tbl && !iter->end_of_table) {
iter->walker.tbl = rht_dereference_rcu(ht->tbl, ht);
+ iter->p = NULL;
iter->slot = 0;
iter->skip = 0;
return -EAGAIN;
@@ -740,7 +752,7 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
return 0;
}
-EXPORT_SYMBOL_GPL(rhashtable_walk_start_check);
+EXPORT_SYMBOL_GPL(rhashtable_walk_start_continue);
/**
* __rhashtable_walk_find_next - Find the next element in a table (or the first
@@ -922,8 +934,6 @@ void rhashtable_walk_stop(struct rhashtable_iter *iter)
iter->walker.tbl = NULL;
spin_unlock(&ht->lock);
- iter->p = NULL;
-
out:
rcu_read_unlock();
}
^ permalink raw reply related
* [PATCH 5/6] rhashtable: support guaranteed successful insertion.
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
The current rhashtable will fail an insertion if the hashtable
it "too full", one of:
- table already has 2^31 elements (-E2BIG)
- a max_size was specified and table already has that
many elements (rounded up to power of 2) (-E2BIG)
- a single chain has more than 16 elements (-EBUSY)
- table has more elements than the current table size,
and allocating a new table fails (-ENOMEM)
- a new page needed to be allocated for a nested table,
and the memory allocation failed (-ENOMEM).
A traditional hash table does not have a concept of "too full", and
insertion only fails if the key already exists. Many users of hash
tables have separate means of limiting the total number of entries,
and are not susceptible to an attack which could cause unusually large
hash chains. For those users, the need to check for errors when
inserting objects to an rhashtable is an unnecessary burden and hence
a potential source of bugs (as these failures are likely to be rare).
This patch adds a "never_fail_insert" configuration parameter which
ensures that insertion will only fail if the key already exists.
When this option is in effect:
- nelems is capped at INT_MAX and will never decrease once it reaches
that value
- max_size is largely ignored
- elements will be added to a table that is nominally "full", though
a rehash will be scheduled
- a new table will never be allocated directly by the insert
function, that is always left for the worker.
For this to trigger a rehash when long chains are detected (possibly
still useful) an extra field in the table records if a long chain
has been seen. This shares a word with the 'nest' value. As
'nest' is never changed once the table is created, updating the
new ->long_chain without locking cannot cause any corruption.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 18 +++++++++++++++---
lib/rhashtable.c | 27 +++++++++++++++++++--------
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 4ffd96949d4f..abdeb1f3f378 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -77,6 +77,7 @@ struct rhlist_head {
* struct bucket_table - Table of hash buckets
* @size: Number of hash buckets
* @nest: Number of bits of first-level nested table.
+ * @long_chain: %true when a chain longer than RHT_ELASTICITY seen.
* @rehash: Current bucket being rehashed
* @hash_rnd: Random seed to fold into hash
* @locks_mask: Mask to apply before accessing locks[]
@@ -89,7 +90,8 @@ struct rhlist_head {
*/
struct bucket_table {
unsigned int size;
- unsigned int nest;
+ unsigned short nest;
+ bool long_chain;
unsigned int rehash;
u32 hash_rnd;
unsigned int locks_mask;
@@ -129,6 +131,9 @@ struct rhashtable;
* @min_size: Minimum size while shrinking
* @locks_mul: Number of bucket locks to allocate per cpu (default: 32)
* @automatic_shrinking: Enable automatic shrinking of tables
+ * @never_fail_insert: Insert will always succeed, even if table will become
+ * unbalanced. Without this, -E2BIG, -EBUSY, and -ENOMEM are possible
+ * errors from rhashtable_*insert*()
* @nulls_base: Base value to generate nulls marker
* @hashfn: Hash function (default: jhash2 if !(key_len % 4), or jhash)
* @obj_hashfn: Function to hash object
@@ -142,6 +147,7 @@ struct rhashtable_params {
unsigned int max_size;
u16 min_size;
bool automatic_shrinking;
+ bool never_fail_insert;
u8 locks_mul;
u32 nulls_base;
rht_hashfn_t hashfn;
@@ -832,7 +838,10 @@ static inline void *__rhashtable_insert_fast(
rcu_assign_pointer(*pprev, obj);
- atomic_inc(&ht->nelems);
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
@@ -1104,7 +1113,10 @@ static inline int __rhashtable_remove_fast_one(
spin_unlock_bh(lock);
if (err > 0) {
- atomic_dec(&ht->nelems);
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, -1, INT_MAX);
+ else
+ atomic_dec(&ht->nelems);
if (unlikely(ht->p.automatic_shrinking &&
rht_shrink_below_30(ht, tbl)))
schedule_work(&ht->run_work);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index fd6f320b9704..427836aace60 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -424,7 +424,7 @@ static void rht_deferred_worker(struct work_struct *work)
err = rhashtable_rehash_alloc(ht, tbl, tbl->size * 2);
else if (ht->p.automatic_shrinking && rht_shrink_below_30(ht, tbl))
err = rhashtable_shrink(ht);
- else if (tbl->nest)
+ else if (tbl->nest || tbl->long_chain)
err = rhashtable_rehash_alloc(ht, tbl, tbl->size);
if (!err)
@@ -549,14 +549,22 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
if (new_tbl)
return new_tbl;
- if (PTR_ERR(data) != -ENOENT)
- return ERR_CAST(data);
+ if (ht->p.never_fail_insert) {
+ if (PTR_ERR(data) == -EAGAIN &&
+ atomic_read(&ht->nelems) != INT_MAX) {
+ tbl->long_chain = true;
+ schedule_work(&ht->run_work);
+ }
+ } else {
+ if (PTR_ERR(data) != -ENOENT)
+ return ERR_CAST(data);
- if (unlikely(rht_grow_above_max(ht, tbl)))
- return ERR_PTR(-E2BIG);
+ if (unlikely(rht_grow_above_max(ht, tbl)))
+ return ERR_PTR(-E2BIG);
- if (unlikely(rht_grow_above_100(ht, tbl)))
- return ERR_PTR(-EAGAIN);
+ if (unlikely(rht_grow_above_100(ht, tbl)))
+ return ERR_PTR(-EAGAIN);
+ }
pprev = rht_bucket_insert(ht, tbl, hash);
if (!pprev)
@@ -574,7 +582,10 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
rcu_assign_pointer(*pprev, obj);
- atomic_inc(&ht->nelems);
+ if (ht->p.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
^ permalink raw reply related
* [PATCH 6/6] rhashtable: allow element counting to be disabled.
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
If multiple CPUs are performing concurrent updates, they can
contend on accessing the element counter even when they
don't often content on hash chains or spin locks. This can
hurt performance.
The nelems counter is only used to trigger a resize at the
70% and 30% marks, so it does not need to be precise.
It is easy to calculate an approximate value when the table
is being rehashed, and this happens when a chain is found to
be 16 elements long. So just moving the counting from
"every update" to "every rehash" removes lots of contention,
but has the down-side is that it allows the max bucket size
to grow to 16 (so average is probably under 8). The normal
average is close to 1.
As a rehash can sometimes not see all (or any) elements, such as when
multiple tables are in the table chain, it is only safe to increase
nelems to match the number rehashed, never to decrease it.
If a client wants minimal contention while still maintaining
a shorter chain length, it can run a periodic task which
counts the number of elements and updates ->nelems directly.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 26 ++++++++++++++++++--------
lib/rhashtable.c | 22 +++++++++++++++-------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index abdeb1f3f378..d0ce5635540f 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -134,6 +134,11 @@ struct rhashtable;
* @never_fail_insert: Insert will always succeed, even if table will become
* unbalanced. Without this, -E2BIG, -EBUSY, and -ENOMEM are possible
* errors from rhashtable_*insert*()
+ * @disable_count: Disable precise counting of number of entries. It is only
+ * updated approximately when the hash table is resized.
+ * This reduces contention in parallel updates, but means we only
+ * grow the table when a hash chain length reaches 16 or when owner
+ * directly updates ->nelems.
* @nulls_base: Base value to generate nulls marker
* @hashfn: Hash function (default: jhash2 if !(key_len % 4), or jhash)
* @obj_hashfn: Function to hash object
@@ -148,6 +153,7 @@ struct rhashtable_params {
u16 min_size;
bool automatic_shrinking;
bool never_fail_insert;
+ bool disable_count;
u8 locks_mul;
u32 nulls_base;
rht_hashfn_t hashfn;
@@ -838,10 +844,12 @@ static inline void *__rhashtable_insert_fast(
rcu_assign_pointer(*pprev, obj);
- if (params.never_fail_insert)
- atomic_add_unless(&ht->nelems, 1, INT_MAX);
- else
- atomic_inc(&ht->nelems);
+ if (!params.disable_count) {
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
+ }
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
@@ -1113,10 +1121,12 @@ static inline int __rhashtable_remove_fast_one(
spin_unlock_bh(lock);
if (err > 0) {
- if (params.never_fail_insert)
- atomic_add_unless(&ht->nelems, -1, INT_MAX);
- else
- atomic_dec(&ht->nelems);
+ if (!params.disable_count) {
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, -1, INT_MAX);
+ else
+ atomic_dec(&ht->nelems);
+ }
if (unlikely(ht->p.automatic_shrinking &&
rht_shrink_below_30(ht, tbl)))
schedule_work(&ht->run_work);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 427836aace60..686193faf271 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -278,12 +278,13 @@ static int rhashtable_rehash_chain(struct rhashtable *ht,
struct bucket_table *old_tbl = rht_dereference(ht->tbl, ht);
spinlock_t *old_bucket_lock;
int err;
+ int cnt = 0;
old_bucket_lock = rht_bucket_lock(old_tbl, old_hash);
spin_lock_bh(old_bucket_lock);
while (!(err = rhashtable_rehash_one(ht, old_hash)))
- ;
+ cnt++;
if (err == -ENOENT) {
old_tbl->rehash++;
@@ -291,7 +292,7 @@ static int rhashtable_rehash_chain(struct rhashtable *ht,
}
spin_unlock_bh(old_bucket_lock);
- return err;
+ return err ?: cnt;
}
static int rhashtable_rehash_attach(struct rhashtable *ht,
@@ -324,6 +325,7 @@ static int rhashtable_rehash_table(struct rhashtable *ht)
struct rhashtable_walker *walker;
unsigned int old_hash;
int err;
+ unsigned int cnt = 0;
new_tbl = rht_dereference(old_tbl->future_tbl, ht);
if (!new_tbl)
@@ -331,12 +333,16 @@ static int rhashtable_rehash_table(struct rhashtable *ht)
for (old_hash = 0; old_hash < old_tbl->size; old_hash++) {
err = rhashtable_rehash_chain(ht, old_hash);
- if (err)
+ if (err < 0)
return err;
+ if (INT_MAX - cnt > err)
+ cnt += err;
}
/* Publish the new table pointer. */
rcu_assign_pointer(ht->tbl, new_tbl);
+ if (ht->p.disable_count && cnt > atomic_read(&ht->nelems))
+ atomic_set(&ht->nelems, cnt);
spin_lock(&ht->lock);
list_for_each_entry(walker, &old_tbl->walkers, list)
@@ -582,10 +588,12 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
rcu_assign_pointer(*pprev, obj);
- if (ht->p.never_fail_insert)
- atomic_add_unless(&ht->nelems, 1, INT_MAX);
- else
- atomic_inc(&ht->nelems);
+ if (!ht->p.disable_count) {
+ if (ht->p.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
+ }
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
^ permalink raw reply related
* Re: [PATCH net-next 0/2] net: broadcom: Adaptive interrupt coalescing
From: Florian Fainelli @ 2018-03-26 23:40 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michael.chan, gospo,
saeedm
In-Reply-To: <67bfc05b-01cf-401f-d802-b3b5cf400928@mellanox.com>
On 03/26/2018 04:21 PM, Tal Gilboa wrote:
> On 3/27/2018 1:29 AM, Florian Fainelli wrote:
>> On 03/26/2018 03:04 PM, Florian Fainelli wrote:
>>> On 03/26/2018 02:16 PM, Tal Gilboa wrote:
>>>> On 3/23/2018 4:19 AM, Florian Fainelli wrote:
>>>>> Hi all,
>>>>>
>>>>> This patch series adds adaptive interrupt coalescing for the Gigabit
>>>>> Ethernet
>>>>> drivers SYSTEMPORT and GENET.
>>>>>
>>>>> This really helps lower the interrupt count and system load, as
>>>>> measured by
>>>>> vmstat for a Gigabit TCP RX session:
>>>>
>>>> I don't see an improvement in system load, the opposite - 42% vs. 100%
>>>> for SYSTEMPORT and 85% vs. 100% for GENET. Both with the same
>>>> bandwidth.
>>>
>>> Looks like I did not extract the correct data the load could spike in
>>> both cases (with and without net_dim) up to 100, but averaged over the
>>> transmission I see the following:
>>>
>>> GENET without:
>>> 1 0 0 1169568 0 25556 0 0 0 0 130079
>>> 62795 2
>>> 86 13 0 0
>>>
>>> GENET with:
>>> 1 0 0 1169536 0 25556 0 0 0 0 10566 10869 1
>>> 21 78 0 0
>>>
>>>> Am I missing something? Talking about bandwidth, I would expect 941Mb/s
>>>> (assuming this is TCP over IPv4). Do you know why the reduced interrupt
>>>> rate doesn't improve bandwidth?
>>>
>>> I am assuming that this comes down to a latency, still capturing some
>>> pcap files to analyze the TCP session with wireshark and see if that is
>>> indeed what is going on. The test machine is actually not that great
>
> I would expect 1GbE full wire speed on almost any setup. I'll try
> applying your code on my setup and see what I get.
The test machine that I am using appears to be loaded by other non
networking workload which perturbs the tests I am running, other than
than I agree, wire speed should be expected.
>
>>>
>>>> Also, any effect on the client side (you
>>>> mentioned enabling TX moderation for SYSTEMPORT)?
>>>
>>> Yes, on SYSTEMPORT, being the TCP IPv4 client, I have the following:
>>>
>>> SYSTEMPORT without:
>>> 2 0 0 191428 0 25748 0 0 0 0 86254 264
>>> 0 41
>>> 59 0 0
>>>
>>> SYSTEMPORT with:
>>> 3 0 0 190176 0 25748 0 0 0 0 45485 31332 0
>>> 100 0 0 0
>>>
>>> I don't get top to agree with these load results though but it looks
>>> like we just have the CPU spinning more, does not look like a win.
>>
>> The problem appears to be the timeout selection on TX, ignoring it
>> completely allows us to keep the load average down while maintaining the
>> bandwidth. Looks like NAPI on TX already does a good job, so interrupt
>> mitigation on TX is not such a great idea actually...
>
> I saw a similar behavior for TX. For me the issue was too many
> outstanding bytes without a completion (defined to be 256KB by sysctl
> net.ipv4.tcp_limit_output_bytes). I tested on a 100GbE connection so
> with reasonable timeout values I already waited too long (4 TSO
> sessions). For the 1GbE case this might have no effect since you need a
> very long timeout. I'm currently working on adding TX support for dim.
> If you don't see a good benefit currently you might want to wait a
> little with TX adaptive interrupt moderation. Maybe only adjust static
> moderation for now?
Yes static moderation appears to be doing just fine.
>
>>
>> Also, doing UDP TX tests shows that we can lower the interrupt count by
>> setting an appropriate tx-frames (as expected), but we won't be lowering
>> the CPU load since that is inherently a CPU intensive work. Past
>
> Do you see higher TX UDP bandwidth? If you are bounded by CPU on both
> cases I would at least expect higher bandwidth with less interrupts
> since you reduce work from the CPU.
UDP bandwidth was intentionally limited the UDP bandwidth to
800Mbits/sec, we are definitively not CPU bound (18% CPU load), but we
can still lower the interrupt count.
--
Florian
^ permalink raw reply
* bpf stable request
From: Chenbo Feng @ 2018-03-26 23:52 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, Lorenzo Colitti, Joel Fernandes
0fa4fe85f4724fff89b09741c437cbee9cf8b008 bpf: skip unnecessary
capability check
This patch fixes the false alarms from security system such as selinux
when doing the capability check. The problem exists since the
sysctl_unprivileged_bpf_disabled is added in linux 4.4. So I suggest to
backport this patch to all LTS stable branches starting from linux-4.4-y.
^ permalink raw reply
* RE: [Intel-wired-lan] [next-queue PATCH v5 7/9] igb: Add MAC address support for ethtool nftuple filters
From: Vinicius Costa Gomes @ 2018-03-26 23:55 UTC (permalink / raw)
To: Brown, Aaron F, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Sanchez-Palencia, Jesus
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C81BBB3@ORSMSX101.amr.corp.intel.com>
Hi Aaron,
"Brown, Aaron F" <aaron.f.brown@intel.com> writes:
>
> Maybe not "this" patch, but this is the one that enables the ethtool commands, so replying here.
> The filters do not seem to take effect with this version (v5) of the
> series. The commands are accepted for i210 and rejected with
> unsupported messages for the other adapters (as desired) and an
> ethtool -n shows the filter, however, with either the src or dst
> filter set I can run traffic (netperf) that should be caught by the
> filter and rather than being directed to the single queue it is spread
> across queues as would be expected without the filter set.
>
> The test system still has a kernel / driver with the v4 series of this
> patch set and the exact same filter commands / system setup does
> filter the traffic to the specified rx queue with the v4 series.
That's interesting. The only difference is that now non steering filters
(filters added by 'ip (m)addr', PACKET_ADD_MEMBERSHIP and the local MAC
address, for example) do not have the QSEL bit set.
On my tests I cannot detect any change in behaviour between those two
versions of the series, for example. trying to add a filter for the
local MAC address has no visible effect in both versions. (This raises a
question: should this be an error, or should this override the default
entry configuration, or this behaviour is fine?)
Can you share more information about your tests? so I can reproduce it here.
Thank you,
^ permalink raw reply
* Re: bpf stable request
From: Daniel Borkmann @ 2018-03-27 0:00 UTC (permalink / raw)
To: Chenbo Feng; +Cc: netdev, Lorenzo Colitti, Joel Fernandes
In-Reply-To: <360670e8-e80c-eb56-8892-cfa8ff54235a@gmail.com>
On 03/27/2018 01:52 AM, Chenbo Feng wrote:
> 0fa4fe85f4724fff89b09741c437cbee9cf8b008 bpf: skip unnecessary capability check
>
> This patch fixes the false alarms from security system such as selinux when doing the capability check. The problem exists since the sysctl_unprivileged_bpf_disabled is added in linux 4.4. So I suggest to backport this patch to all LTS stable branches starting from linux-4.4-y.
I will include this into the next batch, thanks guys!
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 0:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem, daniel, torvalds, peterz,
mathieu.desnoyers, netdev, kernel-team, linux-api
In-Reply-To: <24d0ff40-c6fd-6349-4a89-dffda22cb596@fb.com>
On Mon, 26 Mar 2018 15:25:32 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> On 3/26/18 3:15 PM, Steven Rostedt wrote:
> > On Mon, 26 Mar 2018 15:08:45 -0700
> > Alexei Starovoitov <ast@kernel.org> wrote:
> >
> >> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> >> and therefore cannot be changed.
> >> Instead introduce kernel_tracepoint_find_by_name() to find
> >> tracepoint by name.
> >>
> >> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
> >> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> >
> > I'm curious, why can't you rebase? The first patch was never acked.
>
> because I think it makes sense to keep such things in the commit log
> and in the separate diff, so next developer is aware of what kind of
> minefield the tracpoints are.
This is a bunch of BS. It's not a minefield, and you can change that
function. Mathieu is perfectly fine in modifying his code to deal with
it. He has several times in the past. But I did not agree with the
approach you were taking, that is why I'm against it. You are playing
the straw man with this.
> No wonder some maintainers refuse to add them.
Good grief. No! The reason maintainers refuse to add them is that
userspace can depend on them, and if that happens, it becomes an ABI.
Stop with this nonsense.
-- Steve
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 0:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180326230220.1069180-1-ast@kernel.org>
On Mon, 26 Mar 2018 16:02:20 -0700
Alexei Starovoitov <ast@kernel.org> wrote:
> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> and therefore cannot be changed.
This is false and misleading. NACK.
-- Steve
> Instead introduce kernel_tracepoint_find_by_name() to find
> tracepoint by name.
>
> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> v1->v2: fix 'undef CONFIG_TRACEPOINTS' build as spotted by Mathieu
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 0:08 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mathieu Desnoyers, Alexei Starovoitov, David S. Miller,
Daniel Borkmann, Linus Torvalds, Peter Zijlstra, netdev,
kernel-team, linux-api
In-Reply-To: <761ce9e6-aea4-01d8-8ff0-a17ad8a92526@fb.com>
On Mon, 26 Mar 2018 15:35:56 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> > This patch is not reverting to the old code properly. It introduces a
> > static inline void function that returns NULL. Please compile-test
> > with CONFIG_TRACEPOINTS=n before submitting a patch involving tracepoints.
>
> right. good catch. v2 is coming.
Either fold the patch into the original patch, or I'm pulling in
Mathieu's patch and pushing it to Linus come the merge window.
-- Steve
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Rob Herring @ 2018-03-27 0:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Alexandre Belloni, David S . Miller,
Allan Nielsen, razvan.stefanescu, Po Liu, Thomas Petazzoni,
netdev, devicetree, linux-kernel@vger.kernel.org, Linux-MIPS
In-Reply-To: <20180326225020.GF5862@lunn.ch>
On Mon, Mar 26, 2018 at 5:50 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> ports and port collide with the OF graph binding. It would be good if
>> this moved to ethernet-port(s) or similar.
>
> Hi Rob
>
> Well, we have been using port in DSA since March 2013. ports is a bit
> newer, June 2016.
Yes, understood.
>
> Changing DSA is not going to happen. But new switch bindings could use
> ethernet-port(s). It just makes them inconsistent with existing switch
> drivers.
I'm not saying to change existing bindings, but evolve to something
that doesn't collide on new bindings if you don't have dependencies on
what the node names are. It's mainly so we can have something to key
off of to validate bindings better.
Rob
^ permalink raw reply
* Re: [PATCH net-next] XDP router for veth
From: Md. Islam @ 2018-03-27 1:01 UTC (permalink / raw)
To: David Miller
Cc: ebiederm, Pavel Emelyanov, netdev, shemminger, Eric Dumazet,
David Ahern, Roopa Prabhu, Tom Herbert, alexei.starovoitov,
Florian Fainelli, brouer, bjorn.topel, magnus.karlsson, u9012063
In-Reply-To: <20180326.102110.1068883293248258544.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On Mon, Mar 26, 2018 at 10:21 AM, David Miller <davem@davemloft.net> wrote:
> From: "Md. Islam" <mislam4@kent.edu>
> Date: Fri, 23 Mar 2018 02:43:16 -0400
>
>> +#ifdef CONFIG_XDP_ROUTER
>> + //if IP forwarding is enabled on the receiver, create xdp_buff
>> + //from skb and call xdp_router_forward()
>
> Never use C++ comments, only use C style.
>
>> + if(is_forwarding_enabled(rcv)){
>
> There must be a space between 'if' and the openning parenthesis. You need
> to also have a space before the openning curly braces.
>
> In fact this entire patch is full of coding style issues, please run your
> changes through checkpatch.pl before resubmitting.
Thanks David for the suggestions! I fixed all the formating errors. I
also modified the forwarding logic. Now the iperf throughput improved
from
53.8Gb/s to around 60Gb/s and the median RTT improved from .055 ms to
.35 ms. The patch is attached.
[-- Attachment #2: xdp-fastpath.patch --]
[-- Type: text/x-patch, Size: 8409 bytes --]
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 944ec3c..8474eef 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -328,6 +328,18 @@ config VETH
When one end receives the packet it appears on its pair and vice
versa.
+config XDP_ROUTER
+ bool "XDP router for veth"
+ depends on IP_ADVANCED_ROUTER
+ depends on VETH
+ default y
+ ---help---
+ This option will enable IP forwarding on incoming xdp_buff.
+ Currently it is only supported by veth. Say y or n.
+
+ Currently veth uses slow path for packet forwarding. This option
+ forwards packets as soon as it is received (as XDP generic).
+
config VIRTIO_NET
tristate "Virtio network driver"
depends on VIRTIO
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index a69ad39..4ce10c9 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -111,6 +111,28 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
goto drop;
}
+#ifdef CONFIG_XDP_ROUTER
+
+ /* if IP forwarding is enabled on the receiver, create xdp_buff
+ * from skb and call xdp_router_forward()
+ */
+ if (is_forwarding_enabled(rcv)) {
+ struct xdp_buff *xdp = kmalloc(sizeof(*xdp), GFP_KERNEL);
+
+ xdp->data = skb->data;
+ xdp->data_end = skb->data + (skb->len - skb->data_len);
+ xdp->data_meta = skb;
+ if (likely(xdp_router_forward(rcv, xdp) == NET_RX_SUCCESS)) {
+ struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
+
+ u64_stats_update_begin(&stats->syncp);
+ stats->bytes += length;
+ stats->packets++;
+ u64_stats_update_end(&stats->syncp);
+ goto success;
+ }
+ }
+#endif
if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
@@ -122,6 +144,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
drop:
atomic64_inc(&priv->dropped);
}
+success:
rcu_read_unlock();
return NETDEV_TX_OK;
}
@@ -276,6 +299,61 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
rcu_read_unlock();
}
+#ifdef CONFIG_XDP_ROUTER
+int veth_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ struct net_device *rcv;
+ struct ethhdr *ethh;
+ struct sk_buff *skb;
+ int length = xdp->data_end - xdp->data;
+
+ rcu_read_lock();
+ rcv = rcu_dereference(priv->peer);
+ if (unlikely(!rcv)) {
+ kfree(xdp);
+ goto drop;
+ }
+
+ /* Update MAC address and checksum */
+ ethh = eth_hdr_xdp(xdp);
+ ether_addr_copy(ethh->h_source, dev->dev_addr);
+ ether_addr_copy(ethh->h_dest, rcv->dev_addr);
+
+ /* if IP forwarding is enabled on the receiver,
+ * call xdp_router_forward()
+ */
+ if (is_forwarding_enabled(rcv)) {
+ if (likely(xdp_router_forward(rcv, xdp) == NET_RX_SUCCESS)) {
+ struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
+
+ u64_stats_update_begin(&stats->syncp);
+ stats->bytes += length;
+ stats->packets++;
+ u64_stats_update_end(&stats->syncp);
+ goto success;
+ }
+ }
+
+ /* Local deliver */
+ skb = (struct sk_buff *)xdp->data_meta;
+ if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
+ struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
+
+ u64_stats_update_begin(&stats->syncp);
+ stats->bytes += length;
+ stats->packets++;
+ u64_stats_update_end(&stats->syncp);
+ } else {
+drop:
+ atomic64_inc(&priv->dropped);
+ }
+success:
+ rcu_read_unlock();
+ return NETDEV_TX_OK;
+}
+#endif
+
static const struct net_device_ops veth_netdev_ops = {
.ndo_init = veth_dev_init,
.ndo_open = veth_open,
@@ -290,6 +368,9 @@ static const struct net_device_ops veth_netdev_ops = {
.ndo_get_iflink = veth_get_iflink,
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
+#ifdef CONFIG_XDP_ROUTER
+ .ndo_xdp_xmit = veth_xdp_xmit,
+#endif
};
#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
diff --git a/include/linux/ip.h b/include/linux/ip.h
index 492bc65..025a3ec 100644
--- a/include/linux/ip.h
+++ b/include/linux/ip.h
@@ -19,6 +19,29 @@
#include <linux/skbuff.h>
#include <uapi/linux/ip.h>
+#include <linux/filter.h>
+
+#ifdef CONFIG_XDP_ROUTER
+
+#define MIN_PACKET_SIZE 55
+
+static inline struct iphdr *ip_hdr_xdp(const struct xdp_buff *xdp)
+{
+ return (struct iphdr *)(xdp->data+ETH_HLEN);
+}
+
+static inline struct ethhdr *eth_hdr_xdp(const struct xdp_buff *xdp)
+{
+ return (struct ethhdr *)(xdp->data);
+}
+
+static inline bool is_xdp_forwardable(const struct xdp_buff *xdp)
+{
+ return xdp->data_end - xdp->data >= MIN_PACKET_SIZE;
+}
+
+#endif
+
static inline struct iphdr *ip_hdr(const struct sk_buff *skb)
{
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4c77f39..8369e5e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3290,6 +3290,11 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
__dev_kfree_skb_any(skb, SKB_REASON_CONSUMED);
}
+#ifdef CONFIG_XDP_ROUTER
+bool is_forwarding_enabled(struct net_device *dev);
+int xdp_router_forward(struct net_device *dev, struct xdp_buff *xdp);
+#endif
+
void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb);
int netif_rx(struct sk_buff *skb);
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f805243..623b2de 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -369,6 +369,12 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event, bool force);
int fib_sync_down_addr(struct net_device *dev, __be32 local);
int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
+#ifdef CONFIG_XDP_ROUTER
+int ip_route_lookup(__be32 daddr, __be32 saddr,
+ u8 tos, struct net_device *dev,
+ struct fib_result *res);
+#endif
+
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
const struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index dda9d7b..18c18f3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4090,6 +4090,56 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(do_xdp_generic);
+#ifdef CONFIG_XDP_ROUTER
+
+bool is_forwarding_enabled(struct net_device *dev)
+{
+ struct in_device *in_dev;
+
+ /* verify forwarding is enabled on this interface */
+ in_dev = __in_dev_get_rcu(dev);
+ if (unlikely(!in_dev || !IN_DEV_FORWARD(in_dev)))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(is_forwarding_enabled);
+
+int xdp_router_forward(struct net_device *dev, struct xdp_buff *xdp)
+{
+ int err;
+ struct fib_result res;
+ struct iphdr *iph;
+ struct net_device *rcv;
+
+ if (unlikely(xdp->data_end - xdp->data < MIN_PACKET_SIZE))
+ return NET_RX_DROP;
+
+ iph = (struct iphdr *)(xdp->data + ETH_HLEN);
+
+ /*currently only supports IPv4
+ */
+ if (unlikely(iph->version != 4))
+ return NET_RX_DROP;
+
+ err = ip_route_lookup(iph->daddr, iph->saddr,
+ iph->tos, dev, &res);
+ if (unlikely(err))
+ return NET_RX_DROP;
+
+ rcv = FIB_RES_DEV(res);
+ if (likely(rcv)) {
+ if (likely(rcv->netdev_ops->ndo_xdp_xmit(rcv, xdp) ==
+ NETDEV_TX_OK))
+ return NET_RX_SUCCESS;
+ }
+
+ return NET_RX_DROP;
+}
+EXPORT_SYMBOL_GPL(xdp_router_forward);
+
+#endif
+
static int netif_rx_internal(struct sk_buff *skb)
{
int ret;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 49cc1c1..2333205 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1866,6 +1866,35 @@ static int ip_mkroute_input(struct sk_buff *skb,
return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
}
+#ifdef CONFIG_XDP_ROUTER
+
+int ip_route_lookup(__be32 daddr, __be32 saddr,
+ u8 tos, struct net_device *dev,
+ struct fib_result *res)
+{
+ struct flowi4 fl4;
+ int err;
+ struct net *net = dev_net(dev);
+
+ fl4.flowi4_oif = 0;
+ fl4.flowi4_iif = dev->ifindex;
+ fl4.flowi4_mark = 0;
+ fl4.flowi4_tos = tos & IPTOS_RT_MASK;
+ fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+ fl4.flowi4_flags = 0;
+ fl4.daddr = daddr;
+ fl4.saddr = saddr;
+
+ err = fib_lookup(net, &fl4, res, 0);
+
+ if (unlikely(err != 0 || res->type != RTN_UNICAST))
+ return -EINVAL;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ip_route_lookup);
+#endif
+
/*
* NOTE. We drop all the packets that has local source
* addresses, because every properly looped back packet
^ permalink raw reply related
* Re: [PATCH net-next] XDP router for veth
From: Md. Islam @ 2018-03-27 1:03 UTC (permalink / raw)
To: David Miller
Cc: ebiederm, Pavel Emelyanov, netdev, shemminger, Eric Dumazet,
David Ahern, Roopa Prabhu, Tom Herbert, alexei.starovoitov,
Florian Fainelli, brouer, bjorn.topel, magnus.karlsson, u9012063
In-Reply-To: <CAFgPn1CZ4E=FaHvywCoTpWGo9y5bzF7nmvwUmUQDnzE+wx=yOg@mail.gmail.com>
On Mon, Mar 26, 2018 at 9:01 PM, Md. Islam <mislam4@kent.edu> wrote:
> On Mon, Mar 26, 2018 at 10:21 AM, David Miller <davem@davemloft.net> wrote:
>> From: "Md. Islam" <mislam4@kent.edu>
>> Date: Fri, 23 Mar 2018 02:43:16 -0400
>>
>>> +#ifdef CONFIG_XDP_ROUTER
>>> + //if IP forwarding is enabled on the receiver, create xdp_buff
>>> + //from skb and call xdp_router_forward()
>>
>> Never use C++ comments, only use C style.
>>
>>> + if(is_forwarding_enabled(rcv)){
>>
>> There must be a space between 'if' and the openning parenthesis. You need
>> to also have a space before the openning curly braces.
>>
>> In fact this entire patch is full of coding style issues, please run your
>> changes through checkpatch.pl before resubmitting.
>
>
> Thanks David for the suggestions! I fixed all the formating errors. I
> also modified the forwarding logic. Now the iperf throughput improved
> from
> 53.8Gb/s to around 60Gb/s and the median RTT improved from .055 ms to
> .35 ms. The patch is attached.
Sorry, the RTT improved from .055 ms to .035 ms.
^ permalink raw reply
* ip6_forward / NF_HOOK and counters
From: Jeff Barnhill @ 2018-03-27 1:35 UTC (permalink / raw)
To: netdev
At the end of ip6_forward(), is there a good reason why
IPSTATS_MIB_OUTFORWDATAGRAMS and IPSTATS_MIB_OUTOCTETS are incremented
before the NF_HOOK? If the hook steals or drops the packet, this
counts still go up, which seems incorrect.
v4/ip_forward() increments these counters in ip_forward_finish(). It
seems that v6 should do it in ip6_forward_finish() ?? Thoughts?
Jeff
^ permalink raw reply
* RE: [Intel-wired-lan] [next-queue PATCH v5 7/9] igb: Add MAC address support for ethtool nftuple filters
From: Brown, Aaron F @ 2018-03-27 1:40 UTC (permalink / raw)
To: Gomes, Vinicius, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Sanchez-Palencia, Jesus
In-Reply-To: <87lgeecryc.fsf@intel.com>
> From: Gomes, Vinicius
> Sent: Monday, March 26, 2018 4:56 PM
> To: Brown, Aaron F <aaron.f.brown@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia@intel.com>
> Subject: RE: [Intel-wired-lan] [next-queue PATCH v5 7/9] igb: Add MAC
> address support for ethtool nftuple filters
>
> Hi Aaron,
>
> "Brown, Aaron F" <aaron.f.brown@intel.com> writes:
>
> >
> > Maybe not "this" patch, but this is the one that enables the ethtool
> commands, so replying here.
> > The filters do not seem to take effect with this version (v5) of the
> > series. The commands are accepted for i210 and rejected with
> > unsupported messages for the other adapters (as desired) and an
> > ethtool -n shows the filter, however, with either the src or dst
> > filter set I can run traffic (netperf) that should be caught by the
> > filter and rather than being directed to the single queue it is spread
> > across queues as would be expected without the filter set.
> >
> > The test system still has a kernel / driver with the v4 series of this
> > patch set and the exact same filter commands / system setup does
> > filter the traffic to the specified rx queue with the v4 series.
>
> That's interesting. The only difference is that now non steering filters
> (filters added by 'ip (m)addr', PACKET_ADD_MEMBERSHIP and the local MAC
> address, for example) do not have the QSEL bit set.
>
> On my tests I cannot detect any change in behaviour between those two
> versions of the series, for example. trying to add a filter for the
> local MAC address has no visible effect in both versions. (This raises a
> question: should this be an error, or should this override the default
> entry configuration, or this behaviour is fine?)
>
> Can you share more information about your tests? so I can reproduce it here.
Sure, on I'm running a system the i210 is eth3 and it is linked back to back (crossover cable) to a link partner. The i210 has a mac address of a0:36:9f:10:cc:d7 and the MAC address of the port it is connected to is 00:1b:21:60:29:ea. It should not matter, but eth0 is an older e1000 based adapter (82540EM) I use for connecting ssh sessions, eth1 and eth2 are other igb ports (a dual port 82575EB adapter.)
I tried both an ethtool src and ethtool dst filter with these addresses and with the older version they both seemed to work and with the latest patch in place neither seemed to work. My steps were to set the filter, check that ethtool shows the filter in place then run some netperf traffic (using a script we have called netperf_stress that cycles through a number of sessions to provide constant traffic) and from another login session I watched the rx_queue counters.
For the src filter I used the MAC address of the link partner eth3 interface, 00:1b:21:60:29:ea :
-----------------------------------------------------------------------------
u1485:[0]/root> ethtool -N eth3 flow-type ether src 00:1b:21:60:29:ea action 0
Added rule with ID 15
u1485:[0]/root> ethtool -n eth3
4 RX rings available
Total 1 rules
Filter: 15
Flow Type: Raw Ethernet
Src MAC addr: 00:1B:21:60:29:EA mask: 00:00:00:00:00:00
Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF
Ethertype: 0x0 mask: 0xFFFF
Action: Direct to VF 0 queue 0
u1485:[0]/root>netperf_stress u0485-3
Test limit is NONE
Found netperf support
u0485:[0]/root
Contacted u0485-3
---------> Testing with u0485-3 - Testing since Mon Mar 26 17:35:32 PDT 2018
...
...
-----------------------------------------------------------------------------
Then on another login I watch on an ethtool stat dump grepping for rx_queue. I threw a -d to get a better visual of the differences.
-----------------------------------------------------------------------------
1485:[1]/root> watch -d 'ethtool -S eth3|grep rx_queue'
Every 2.0s: ethtool -S eth3|grep rx_queue Mon Mar 26 18:19:21 2018
rx_queue_0_packets: 13
rx_queue_0_bytes: 2204
rx_queue_0_drops: 0
rx_queue_0_csum_err: 0
rx_queue_0_alloc_failed: 0
rx_queue_1_packets: 330677
rx_queue_1_bytes: 500632203
rx_queue_1_drops: 0
rx_queue_1_csum_err: 0
rx_queue_1_alloc_failed: 0
rx_queue_2_packets: 141902
rx_queue_2_bytes: 214787953
rx_queue_2_drops: 0
rx_queue_2_csum_err: 0
rx_queue_2_alloc_failed: 0
rx_queue_3_packets: 234377
rx_queue_3_bytes: 347172517
rx_queue_3_drops: 0
rx_queue_3_csum_err: 0
rx_queue_3_alloc_failed: 0
-----------------------------------------------------------------------------
After a few moments I can tell the counters are all over the place. I do pretty much the exact same thing for the dst filter, except I use the i210's MAC address for the destination instead of the one on the link partner:
-----------------------------------------------------------------------------
u1485:[1]/root> ethtool -N eth3 delete 15
u1485:[1]/root> ethtool -N eth3 flow-type ether dst a0:36:9f:10:cc:d7 action 0
Added rule with ID 15
u1485:[1]/root> ethtool -n eth3
4 RX rings available
Total 1 rules
Filter: 15
Flow Type: Raw Ethernet
Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF
Dest MAC addr: A0:36:9F:10:CC:D7 mask: 00:00:00:00:00:00
Ethertype: 0x0 mask: 0xFFFF
Action: Direct to VF 0 queue 0
u1485:[1]/root> netperf_stress u0485-3
Test limit is NONE
Found netperf support
u0485:[0]/root
Contacted u0485-3
---------> Testing with u0485-3 - Testing since Mon Mar 26 17:35:32 PDT 2018
...
...
-----------------------------------------------------------------------------
And watching the rx_queue counters continues to be spread across the different queues. This is with Jeff Kirsher's next queue, kernel 4.16.0-rc4_next-queue_dev-queue_e31d20a, which has the series of 8 igb patches applied.
When I go back and run the an older build (with an earlier version of the series) of the same tree, 4.16.0-rc4_next-queue_dev-queue_84a3942, with the same procedure and same systems all the rx traffic is relegated to queue 0 (or whichever queue I assign it to) for either the src or dst filter. Here is a sample of my counters after it had been running netperf_stress over the weekend:
-----------------------------------------------------------------------------1485:[1]/root> watch -d 'ethtool -S eth3|grep rx_queue'
Every 2.0s: ethtool -S eth3|grep rx_queue Mon Mar 26 17:37:44 2018
rx_queue_0_packets: 16453268766
rx_queue_0_bytes: 23420384893393
rx_queue_0_drops: 0
rx_queue_0_csum_err: 0
rx_queue_0_alloc_failed: 0
rx_queue_1_packets: 0
rx_queue_1_bytes: 0
rx_queue_1_drops: 0
rx_queue_1_csum_err: 0
rx_queue_1_alloc_failed: 0
rx_queue_2_packets: 0
rx_queue_2_bytes: 0
rx_queue_2_drops: 0
rx_queue_2_csum_err: 0
rx_queue_2_alloc_failed: 0
rx_queue_3_packets: 0
rx_queue_3_bytes: 0
rx_queue_3_drops: 0
rx_queue_3_csum_err: 0
rx_queue_3_alloc_failed: 0
>
>
> Thank you,
> --
> Vinicius
^ permalink raw reply
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Yuchung Cheng @ 2018-03-27 2:07 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <1520936711-16784-2-git-send-email-ilpo.jarvinen@helsinki.fi>
On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
>
> A miscalculation for the number of acknowledged packets occurs during
> RTO recovery whenever SACK is not enabled and a cumulative ACK covers
> any non-retransmitted skbs. The reason is that pkts_acked value
> calculated in tcp_clean_rtx_queue is not correct for slow start after
> RTO as it may include segments that were not lost and therefore did
> not need retransmissions in the slow start following the RTO. Then
> tcp_slow_start will add the excess into cwnd bloating it and
> triggering a burst.
>
> Instead, we want to pass only the number of retransmitted segments
> that were covered by the cumulative ACK (and potentially newly sent
> data segments too if the cumulative ACK covers that far).
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
> net/ipv4/tcp_input.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 9a1b3c1..4a26c09 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3027,6 +3027,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> long seq_rtt_us = -1L;
> long ca_rtt_us = -1L;
> u32 pkts_acked = 0;
> + u32 rexmit_acked = 0;
> + u32 newdata_acked = 0;
> u32 last_in_flight = 0;
> bool rtt_update;
> int flag = 0;
> @@ -3056,8 +3058,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (unlikely(sacked & TCPCB_RETRANS)) {
> - if (sacked & TCPCB_SACKED_RETRANS)
> + if (sacked & TCPCB_SACKED_RETRANS) {
> tp->retrans_out -= acked_pcount;
> + rexmit_acked += acked_pcount;
> + }
> flag |= FLAG_RETRANS_DATA_ACKED;
> } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> last_ackt = skb->skb_mstamp;
> @@ -3070,6 +3074,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> reord = start_seq;
> if (!after(scb->end_seq, tp->high_seq))
> flag |= FLAG_ORIG_SACK_ACKED;
> + else
> + newdata_acked += acked_pcount;
> }
>
> if (sacked & TCPCB_SACKED_ACKED) {
> @@ -3151,6 +3157,14 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (tcp_is_reno(tp)) {
> + /* Due to discontinuity on RTO in the artificial
> + * sacked_out calculations, TCP must restrict
> + * pkts_acked without SACK to rexmits and new data
> + * segments
> + */
> + if (icsk->icsk_ca_state == TCP_CA_Loss)
> + pkts_acked = rexmit_acked + newdata_acked;
> +
My understanding is there are two problems
1) your fix: the reordering logic in tcp-remove_reno_sacks requires
precise cumulatively acked count, not newly acked count?
2) current code: pkts_acked can substantially over-estimate the newly
delivered pkts in both SACK and non-SACK cases. For example, let's say
99/100 packets are already sacked, and the next ACK acks 100 pkts.
pkts_acked == 100 but really only one packet is delivered. It's wrong
to inform congestion control that 100 packets have just delivered.
AFAICT, the CCs that have pkts_acked callbacks all treat pkts_acked as
the newly delivered packets.
A better fix for both SACK and non-SACK, seems to be moving
ca_ops->pkts_acked into tcp_cong_control, where the "acked_sacked" is
calibrated? this is what BBR is currently doing to avoid these pitfalls.
> tcp_remove_reno_sacks(sk, pkts_acked);
> } else {
> int delta;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Alexei Starovoitov @ 2018-03-27 2:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: Mathieu Desnoyers, Alexei Starovoitov, David S. Miller,
Daniel Borkmann, Linus Torvalds, Peter Zijlstra, netdev,
kernel-team, linux-api
In-Reply-To: <20180326200840.2b9eda2c@gandalf.local.home>
On 3/26/18 5:08 PM, Steven Rostedt wrote:
> On Mon, 26 Mar 2018 15:35:56 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>
>>> This patch is not reverting to the old code properly. It introduces a
>>> static inline void function that returns NULL. Please compile-test
>>> with CONFIG_TRACEPOINTS=n before submitting a patch involving tracepoints.
>>
>> right. good catch. v2 is coming.
>
> Either fold the patch into the original patch, or I'm pulling in
> Mathieu's patch and pushing it to Linus come the merge window.
Ok. I will fold this patch into previous set and rebase the tree.
^ permalink raw reply
* Re: [PATCH 4/4] selftests/bpf: fix compiling errors
From: Du, Changbin @ 2018-03-27 2:20 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: changbin.du, shuah, linux-kselftest, linux-kernel,
Daniel Borkmann, netdev
In-Reply-To: <20180326145511.lzoi6wja6iht5lvq@ast-mbp.dhcp.thefacebook.com>
On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin.du@intel.com wrote:
> > Signed-off-by: Changbin Du <changbin.du@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> > GENFLAGS := -DHAVE_GENHDR
> > endif
> >
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> > LDLIBS += -lcap -lelf -lrt -lpthread
> >
> > # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> > CPU ?= generic
> > endif
> >
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include \
> > -Wno-compare-distinct-pointer-types
>
> Nack.
> I suspect that will break the build for everyone else who's doing it in the directory
> itself instead of the outer one.
>
This one? But I didn't see any problem.
changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make
make -C ../../../lib/bpf OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
HOSTCC /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep.o
HOSTLD /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep-in.o
LINK /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep
CC /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.o
CC /home/changbin/work/linux/tools/testing/selftests/bpf/bpf.o
CC /home/changbin/work/linux/tools/testing/selftests/bpf/nlattr.o
LD /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf-in.o
LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a
LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.so
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
make -C ../../../lib/bpf OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_verifier.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_verifier
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_tag.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tag
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_maps.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_maps
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_lru_map.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_lru_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_lpm_map.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_lpm_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_progs.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_progs
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_align.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_align
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_verifier_log.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_verifier_log
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_dev_cgroup.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_dev_cgroup
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_tcpbpf_user.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tcpbpf_user
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_libbpf_open.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_libbpf_open
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_pkt_access.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_xdp.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_l4lb.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_l4lb.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_tcp_estats.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tcp_estats.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_obj_id.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_obj_id.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_pkt_md_access.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_md_access.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_xdp_redirect.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp_redirect.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_xdp_meta.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp_meta.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sockmap_parse_prog.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sockmap_parse_prog.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sockmap_verdict_prog.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sockmap_verdict_prog.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c dev_cgroup.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/dev_cgroup.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sample_ret0.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sample_ret0.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_tracepoint.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tracepoint.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types -fno-inline \
-O2 -target bpf -emit-llvm -c test_l4lb_noinline.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_l4lb_noinline.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types -fno-inline \
-O2 -target bpf -emit-llvm -c test_xdp_noinline.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp_noinline.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_stacktrace_map.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_stacktrace_map.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sample_map_ret0.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sample_map_ret0.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_tcpbpf_kern.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tcpbpf_kern.o
changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$
--
Thanks,
Changbin Du
^ permalink raw reply
* Re: [PATCH 4/4] selftests/bpf: fix compiling errors
From: Du, Changbin @ 2018-03-27 2:33 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: changbin.du, shuah, linux-kselftest, linux-kernel,
Daniel Borkmann, netdev
In-Reply-To: <20180326145511.lzoi6wja6iht5lvq@ast-mbp.dhcp.thefacebook.com>
Hi Starovoitov,
This one does have the issue you mentioned.
[PATCH 2/4] selftests/gpio: fix paths in Makefile
And can be fixed by:
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
+OUTPUT ?= $(shell pwd)
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
BINARIES := gpio-mockup-chardev
@@ -24,7 +25,7 @@ LDLIBS += -lmount -I/usr/include/libmount
$(BINARIES): gpio-utils.o ../../../../usr/include/linux/gpio.h
gpio-utils.o:
- make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+ make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) OUTPUT=$(OUTPUT)/ -C ../../../gpio
../../../../usr/include/linux/gpio.h:
I will update it later.
On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin.du@intel.com wrote:
> > From: Changbin Du <changbin.du@intel.com>
> >
> > This patch fixed below errors of missing head files.
> >
> > tools/testing/selftests$ make
> > ...
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_pkt_access.o
> > In file included from test_pkt_access.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_xdp.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_xdp.o
> > In file included from test_xdp.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_l4lb.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_l4lb.o
> > In file included from test_l4lb.c:10:
> > In file included from /usr/include/linux/pkt_cls.h:4:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_tcp_estats.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_tcp_estats.o
> > In file included from test_tcp_estats.c:35:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ...
> >
> > Signed-off-by: Changbin Du <changbin.du@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> > GENFLAGS := -DHAVE_GENHDR
> > endif
> >
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> > LDLIBS += -lcap -lelf -lrt -lpthread
> >
> > # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> > CPU ?= generic
> > endif
> >
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include \
> > -Wno-compare-distinct-pointer-types
>
> Nack.
> I suspect that will break the build for everyone else who's doing it in the directory
> itself instead of the outer one.
>
--
Thanks,
Changbin Du
^ permalink raw reply
* [PATCH v6 bpf-next 11/11] selftests/bpf: test for bpf_get_stackid() from raw tracepoints
From: Alexei Starovoitov @ 2018-03-27 2:47 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180327024706.2064725-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
similar to traditional traceopint test add bpf_get_stackid() test
from raw tracepoints
and reduce verbosity of existing stackmap test
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/testing/selftests/bpf/test_progs.c | 91 ++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index e9df48b306df..faadbe233966 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -877,7 +877,7 @@ static void test_stacktrace_map()
err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
- goto out;
+ return;
/* Get the ID for the sched/sched_switch tracepoint */
snprintf(buf, sizeof(buf),
@@ -888,8 +888,7 @@ static void test_stacktrace_map()
bytes = read(efd, buf, sizeof(buf));
close(efd);
- if (CHECK(bytes <= 0 || bytes >= sizeof(buf),
- "read", "bytes %d errno %d\n", bytes, errno))
+ if (bytes <= 0 || bytes >= sizeof(buf))
goto close_prog;
/* Open the perf event and attach bpf progrram */
@@ -906,29 +905,24 @@ static void test_stacktrace_map()
goto close_prog;
err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
- if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
- err, errno))
- goto close_pmu;
+ if (err)
+ goto disable_pmu;
err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
- if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
- err, errno))
+ if (err)
goto disable_pmu;
/* find map fds */
control_map_fd = bpf_find_map(__func__, obj, "control_map");
- if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
- "err %d errno %d\n", err, errno))
+ if (control_map_fd < 0)
goto disable_pmu;
stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
- if (CHECK(stackid_hmap_fd < 0, "bpf_find_map stackid_hmap",
- "err %d errno %d\n", err, errno))
+ if (stackid_hmap_fd < 0)
goto disable_pmu;
stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
- if (CHECK(stackmap_fd < 0, "bpf_find_map stackmap", "err %d errno %d\n",
- err, errno))
+ if (stackmap_fd < 0)
goto disable_pmu;
/* give some time for bpf program run */
@@ -945,24 +939,78 @@ static void test_stacktrace_map()
err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
"err %d errno %d\n", err, errno))
- goto disable_pmu;
+ goto disable_pmu_noerr;
err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
"err %d errno %d\n", err, errno))
- ; /* fall through */
+ goto disable_pmu_noerr;
+ goto disable_pmu_noerr;
disable_pmu:
+ error_cnt++;
+disable_pmu_noerr:
ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
close(pmu_fd);
-
close_prog:
bpf_object__close(obj);
+}
-out:
- return;
+static void test_stacktrace_map_raw_tp()
+{
+ int control_map_fd, stackid_hmap_fd, stackmap_fd;
+ const char *file = "./test_stacktrace_map.o";
+ int efd, err, prog_fd;
+ __u32 key, val, duration = 0;
+ struct bpf_object *obj;
+
+ err = bpf_prog_load(file, BPF_PROG_TYPE_RAW_TRACEPOINT, &obj, &prog_fd);
+ if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
+ return;
+
+ efd = bpf_raw_tracepoint_open("sched_switch", prog_fd);
+ if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+ goto close_prog;
+
+ /* find map fds */
+ control_map_fd = bpf_find_map(__func__, obj, "control_map");
+ if (control_map_fd < 0)
+ goto close_prog;
+
+ stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
+ if (stackid_hmap_fd < 0)
+ goto close_prog;
+
+ stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
+ if (stackmap_fd < 0)
+ goto close_prog;
+
+ /* give some time for bpf program run */
+ sleep(1);
+
+ /* disable stack trace collection */
+ key = 0;
+ val = 1;
+ bpf_map_update_elem(control_map_fd, &key, &val, 0);
+
+ /* for every element in stackid_hmap, we can find a corresponding one
+ * in stackmap, and vise versa.
+ */
+ err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
+ if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
+ "err %d errno %d\n", err, errno))
+ goto close_prog;
+
+ err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
+ if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
+ "err %d errno %d\n", err, errno))
+ goto close_prog;
+
+ goto close_prog_noerr;
+close_prog:
+ error_cnt++;
+close_prog_noerr:
+ bpf_object__close(obj);
}
static int extract_build_id(char *build_id, size_t size)
@@ -1138,6 +1186,7 @@ int main(void)
test_tp_attach_query();
test_stacktrace_map();
test_stacktrace_build_id();
+ test_stacktrace_map_raw_tp();
printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.9.5
^ permalink raw reply related
* [PATCH v6 bpf-next 04/11] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
From: Alexei Starovoitov @ 2018-03-27 2:46 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180327024706.2064725-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
fix iwlwifi_dev_ucode_error tracepoint to pass pointer to a table
instead of all 17 arguments by value.
dvm/main.c and mvm/utils.c have 'struct iwl_error_event_table'
defined with very similar yet subtly different fields and offsets.
tracepoint is still common and using definition of 'struct iwl_error_event_table'
from dvm/commands.h while copying fields.
Long term this tracepoint probably should be split into two.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/dvm/main.c | 7 +---
.../wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h | 39 ++++++++++------------
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 7 +---
4 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/main.c b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
index d11d72615de2..e68254e12764 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/main.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
@@ -1651,12 +1651,7 @@ static void iwl_dump_nic_error_log(struct iwl_priv *priv)
priv->status, table.valid);
}
- trace_iwlwifi_dev_ucode_error(trans->dev, table.error_id, table.tsf_low,
- table.data1, table.data2, table.line,
- table.blink2, table.ilink1, table.ilink2,
- table.bcon_time, table.gp1, table.gp2,
- table.gp3, table.ucode_ver, table.hw_ver,
- 0, table.brd_ver);
+ trace_iwlwifi_dev_ucode_error(trans->dev, &table, 0, table.brd_ver);
IWL_ERR(priv, "0x%08X | %-28s\n", table.error_id,
desc_lookup(table.error_id));
IWL_ERR(priv, "0x%08X | uPc\n", table.pc);
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h
index 9518a82f44c2..27e3e4e96aa2 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h
@@ -126,14 +126,11 @@ TRACE_EVENT(iwlwifi_dev_tx,
__entry->framelen, __entry->skbaddr)
);
+struct iwl_error_event_table;
TRACE_EVENT(iwlwifi_dev_ucode_error,
- TP_PROTO(const struct device *dev, u32 desc, u32 tsf_low,
- u32 data1, u32 data2, u32 line, u32 blink2, u32 ilink1,
- u32 ilink2, u32 bcon_time, u32 gp1, u32 gp2, u32 rev_type,
- u32 major, u32 minor, u32 hw_ver, u32 brd_ver),
- TP_ARGS(dev, desc, tsf_low, data1, data2, line,
- blink2, ilink1, ilink2, bcon_time, gp1, gp2,
- rev_type, major, minor, hw_ver, brd_ver),
+ TP_PROTO(const struct device *dev, const struct iwl_error_event_table *table,
+ u32 hw_ver, u32 brd_ver),
+ TP_ARGS(dev, table, hw_ver, brd_ver),
TP_STRUCT__entry(
DEV_ENTRY
__field(u32, desc)
@@ -155,20 +152,20 @@ TRACE_EVENT(iwlwifi_dev_ucode_error,
),
TP_fast_assign(
DEV_ASSIGN;
- __entry->desc = desc;
- __entry->tsf_low = tsf_low;
- __entry->data1 = data1;
- __entry->data2 = data2;
- __entry->line = line;
- __entry->blink2 = blink2;
- __entry->ilink1 = ilink1;
- __entry->ilink2 = ilink2;
- __entry->bcon_time = bcon_time;
- __entry->gp1 = gp1;
- __entry->gp2 = gp2;
- __entry->rev_type = rev_type;
- __entry->major = major;
- __entry->minor = minor;
+ __entry->desc = table->error_id;
+ __entry->tsf_low = table->tsf_low;
+ __entry->data1 = table->data1;
+ __entry->data2 = table->data2;
+ __entry->line = table->line;
+ __entry->blink2 = table->blink2;
+ __entry->ilink1 = table->ilink1;
+ __entry->ilink2 = table->ilink2;
+ __entry->bcon_time = table->bcon_time;
+ __entry->gp1 = table->gp1;
+ __entry->gp2 = table->gp2;
+ __entry->rev_type = table->gp3;
+ __entry->major = table->ucode_ver;
+ __entry->minor = table->hw_ver;
__entry->hw_ver = hw_ver;
__entry->brd_ver = brd_ver;
),
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
index 50510fb6ab8c..6aa719865a58 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
@@ -30,6 +30,7 @@
#ifndef __CHECKER__
#include "iwl-trans.h"
+#include "dvm/commands.h"
#define CREATE_TRACE_POINTS
#include "iwl-devtrace.h"
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
index d65e1db7c097..5442ead876eb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
@@ -549,12 +549,7 @@ static void iwl_mvm_dump_lmac_error_log(struct iwl_mvm *mvm, u32 base)
IWL_ERR(mvm, "Loaded firmware version: %s\n", mvm->fw->fw_version);
- trace_iwlwifi_dev_ucode_error(trans->dev, table.error_id, table.tsf_low,
- table.data1, table.data2, table.data3,
- table.blink2, table.ilink1,
- table.ilink2, table.bcon_time, table.gp1,
- table.gp2, table.fw_rev_type, table.major,
- table.minor, table.hw_ver, table.brd_ver);
+ trace_iwlwifi_dev_ucode_error(trans->dev, &table, table.hw_ver, table.brd_ver);
IWL_ERR(mvm, "0x%08X | %-28s\n", table.error_id,
desc_lookup(table.error_id));
IWL_ERR(mvm, "0x%08X | trm_hw_status0\n", table.trm_hw_status0);
--
2.9.5
^ 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