* Re: Extended IDR API
From: David Miller @ 2017-09-11 21:31 UTC (permalink / raw)
To: willy; +Cc: chrism, jiri, tj, linux-kernel, netdev, aquannie
In-Reply-To: <20170911211408.GA26747@bombadil.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Mon, 11 Sep 2017 14:14:08 -0700
> I really don't like your new API. I wish you'd discussed it before
> merging it.
If I recall correctly, linux-kernel was CC:'d and nobody engaged.
I could be wrong, and if I am then my bad, I should have enforced that.
Thanks.
^ permalink raw reply
* Re: [PATCH] tipc: Use bsearch library function
From: David Miller @ 2017-09-11 21:30 UTC (permalink / raw)
To: thomas; +Cc: jon.maloy, ying.xue, netdev, tipc-discussion, linux-kernel
In-Reply-To: <20170909031819.13622-1-thomas@m3y3r.de>
From: Thomas Meyer <thomas@m3y3r.de>
Date: Sat, 9 Sep 2017 05:18:19 +0200
> @@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
> return nseq;
> }
>
> +static int nameseq_find_subseq_cmp(const void *key, const void *elt)
> +{
> + u32 instance = *(u32 *)key;
> + struct sub_seq *sseq = (struct sub_seq *)elt;
Please order local variables from longest to shortest (ie. reverse
christmas tree).
Thank you.
^ permalink raw reply
* Re: [PATCH net] perf/bpf: fix a clang compilation issue
From: David Miller @ 2017-09-11 21:28 UTC (permalink / raw)
To: yhs; +Cc: peterz, rostedt, ast, daniel, ndesaulniers, netdev, kernel-team
In-Reply-To: <20170908013615.973478-1-yhs@fb.com>
From: Yonghong Song <yhs@fb.com>
Date: Thu, 7 Sep 2017 18:36:15 -0700
> clang does not support variable length array for structure member.
> It has the following error during compilation:
>
> kernel/trace/trace_syscalls.c:568:17: error: fields must have a constant size:
> 'variable length array in structure' extension will never be supported
> unsigned long args[sys_data->nb_args];
> ^
>
> The fix is to use a fixed array length instead.
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: David Miller @ 2017-09-11 21:26 UTC (permalink / raw)
To: tatsu; +Cc: j.vosburgh, vfalico, andy, netdev, linux-kernel, r
In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A02985CB5@BPXM09GP.gisp.nec.co.jp>
From: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
Date: Wed, 6 Sep 2017 22:47:59 +0000
> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
> balance-alb mode") tried to fix transmit dynamic load balancing in
> balance-alb mode, which wasn't working after commit 8b426dc54cf4
> ("bonding: remove hardcoded value").
>
> It turned out that my previous patch only fixed the case when
> balance-alb was specified as bonding module parameter, and not when
> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
> common usage). In the latter case, tlb_dynamic_lb was set up according
> to the default mode of the bonding interface, which happens to be
> balance-rr.
>
> This additional patch addresses this issue by setting up tlb_dynamic_lb
> to 1 if "mode" is set to balance-alb through the sysfs interface.
>
> I didn't add code to change tlb_balance_lb back to the default value for
> other modes, because "mode" is usually set up only once during
> initialization, and it's not worthwhile to change the static variable
> bonding_defaults in bond_main.c to a global variable just for this
> purpose.
>
> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
> balance-tlb mode if it is set up using the sysfs interface. I didn't
> change that behavior, because the value of tlb_balance_lb can be changed
> using the sysfs interface for balance-tlb, and I didn't like changing
> the default value back and forth for balance-tlb.
>
> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
> is not an intended usage, so there is little use making it writable at
> this moment.
>
> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
> Reported-by: Reinis Rozitis <r@roze.lv>
> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
> Cc: stable@vger.kernel.org # v4.12+
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] hv_netvsc: avoid unnecessary wakeups on subchannel creation
From: David Miller @ 2017-09-11 21:21 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170906205306.10541-3-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 6 Sep 2017 13:53:06 -0700
> Only need to wakeup the initiator after all sub-channels
> are opened.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] hv_netvsc: fix deadlock on hotplug
From: David Miller @ 2017-09-11 21:21 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170906205306.10541-2-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 6 Sep 2017 13:53:05 -0700
> When a virtual device is added dynamically (via host console), then
> the vmbus sends an offer message for the primary channel. The processing
> of this message for networking causes the network device to then
> initialize the sub channels.
>
> The problem is that setting up the sub channels needs to wait until
> the subsequent subchannel offers have been processed. These offers
> come in on the same ring buffer and work queue as where the primary
> offer is being processed; leading to a deadlock.
>
> This did not happen in older kernels, because the sub channel waiting
> logic was broken (it wasn't really waiting).
>
> The solution is to do the sub channel setup in its own work queue
> context that is scheduled by the primary channel setup; and then
> happens later.
>
> Fixes: 732e49850c5e ("netvsc: fix race on sub channel creation")
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> v2 - fix module removal race with new work queue
Applied.
^ permalink raw reply
* Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter chain
From: Cong Wang @ 2017-09-11 21:14 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jakub Kicinski, Jiri Pirko
In-Reply-To: <20170910143300.GB1860@nanopsycho>
On Sun, Sep 10, 2017 at 7:33 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Sep 08, 2017 at 06:37:55PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Thu, Sep 7, 2017 at 1:52 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:
>>>>Yes it is for chain 0, because block holds a reference to chain 0 during
>>>>creation. Non-0 chains are created with refcnt==1 too but paired with
>>>>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0
>>>>not special w.r.t. refcnt.
>>>
>>> So you need to tcf_chain_put only chain 0 here, right? The rest of the
>>> chains get destroyed by the previous list_for_each_entry iteration when
>>> flush happens and actions destroy happens what decrements recnt to 0
>>> there.
>>
>>
>>This is correct. And it should be only chain 0 after flush.
>>
>>>
>>> What do I miss, who would still hold reference for non-0 chains when all
>>> tps and all goto_chain actions are gone?
>>
>>No one. This is totally correct and is exactly what this patch intends to do.
>>
>>Look, this is why we never need an object with refcnt==0 to exist. ;)
>
> So, I understand that correctly, good. But this is a problem.
>
> When you do:
> list_for_each_entry(chain, &block->chain_list, list)
> tcf_chain_flush(chain);
>
> The reference may get dropped for chains to 0 (for those that does not
> have a goto_chain action holding a ref), and therefore they get freed
> within the loop. That is problematic when you do the traversing of the
> list. You may use list_for_each_entry_safe, but there is another issue:
>
> As a part of tcf_chain_flush destruction, act goto_chain destruction may
> get scheduled by call_rcu. That may be the last reference held for the
> chain. So you race between this loop and rcu callback.
>
> Consider following example:
>
> chain0 - has only one rule with goto_chain 22 action
> chain22 - no rule (refcnt 1 because of the action mentioned above)
>
> CPU0 CPU1
>
> tcf_chain_flush(0)
> -> call_rcu(free_tcf)
> free_tcf
> ->tcf_chain_put(22)
> ->tcf_chain_destroy(22)
> ->kfree(22)
Looks like all due to the lack of locking on block->chain_list.
I thought the rcu_barrier() could properly handle this,
but seems still not, probably I need to move it in the loop,
I am still not 100% sure if it is totally safe with
list_for_each_safe():
- list_for_each_entry(chain, &block->chain_list, list)
+ list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
tcf_chain_flush(chain);
- rcu_barrier();
+ rcu_barrier(); // are we safe now???
+ }
> tcf_chain_flush(22)...use-after-free
>
Same race could happen with your code too, right?
chain 22 still has refcnt==1 so chain_put() will destroy
it in flush() too. So this is not a regression.
I know you have list_for_each_safe() but you lack of
rcu_barrier(). This is why I said the lack of locking is
the cause, not your code or my code.
>
> So what I suggest in order to prevent this is to change your code to
> something like:
>
> /* To avoid race between getting reference in the next loop and
> * rcu callbacks from deleleted actions freeing the chain.
> */
> rcu_barrier();
>
> list_for_each_entry(chain, &block->chain_list, list)
> if (chain->index) /* we already hold ref to chain 0 */
> tcf_chain_hold(chain);
>
> list_for_each_entry(chain, &block->chain_list, list)
> tcf_chain_flush(chain);
>
> /* Wait for rcu callbacks from deleleted actions that were
> * sheduled as a result of tcf_chain_flush in the previous loop.
> * This is not absolutelly necessary, as the chain may live after
> * the tcf_chain_put is called in the next iteration and would
> * get freed on tcf_chain_put call from rcu callback later on.
> */
> rcu_barrier();
>
> /* Now we are sure that we are the only one holding a reference
> * to all chains, drop it and let them go.
> */
> list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
> tcf_chain_put(chain);
> kfree(block);
>
> Does this make sense?
Yeah, but again this is all due to lack of locking. Do we really
have to to be such complex or just use either a) proper sync
with rcu_barrier() or b) proper locking on chain_list (with RCU
of course)?
Thanks.
^ permalink raw reply
* Extended IDR API
From: Matthew Wilcox @ 2017-09-11 21:14 UTC (permalink / raw)
To: Chris Mi
Cc: Jiri Pirko, David S. Miller, Tejun Heo, linux-kernel, netdev,
Rehas Sachdeva
I really don't like your new API. I wish you'd discussed it before
merging it. Here's my redesign. Does anybody have a suggestion for
improvement?
We have a lovely new test-suite for the IDR (in tools/testing/radix-tree)
... when adding a new API, it's polite to update the test-suite too.
Do you have any plans to add test cases?
(Compile tested only; I'm at a conference. Also, I didn't check the
kerneldoc because I don't have Sphinx installed on my laptop.)
>From ff45b2a6806cd0e4177c5a10f26c97999164c10c Mon Sep 17 00:00:00 2001
From: Matthew Wilcox <mawilcox@microsoft.com>
Date: Mon, 11 Sep 2017 16:16:29 -0400
Subject: [PATCH] idr: Rewrite extended IDR API
- Rename the API to be 'ul' for unsigned long instead of 'ext'. This
fits better with other usage in the Linux kernel.
- idr_alloc() moves back to being a function instead of inline
- idr_alloc_ul() takes 'nextid' as an in-out parameter like idr_get_next(),
instead of having 'index' as an out-only parameter.
- idr_alloc_ul() needs a __must_check to ensure that users don't look at
the result without checking whether the function succeeded.
- idr_alloc_ul() takes 'max' rather than 'end', or it is impossible to
allocate the ULONG_MAX id.
- idr_replace() can simply take an unsigned long parameter instead of
an int.
- idr_remove() and idr_find() are the same as idr_replace().
- We don't need separate idr_get_free() and idr_get_free_ext().
- Add kerneldoc for idr_alloc_ul().
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 75 +++++----------------------------------
include/linux/radix-tree.h | 17 +--------
lib/idr.c | 88 +++++++++++++++++++++++++++++++++-------------
lib/radix-tree.c | 2 +-
net/sched/act_api.c | 22 ++++++------
net/sched/cls_flower.c | 16 +++++----
6 files changed, 95 insertions(+), 125 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 7c3a365f7e12..90faf8279559 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -81,74 +81,22 @@ static inline void idr_set_cursor(struct idr *idr, unsigned int val)
void idr_preload(gfp_t gfp_mask);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext);
-
-/**
- * idr_alloc - allocate an id
- * @idr: idr handle
- * @ptr: pointer to be associated with the new id
- * @start: the minimum id (inclusive)
- * @end: the maximum id (exclusive)
- * @gfp: memory allocation flags
- *
- * Allocates an unused ID in the range [start, end). Returns -ENOSPC
- * if there are no unused IDs in that range.
- *
- * Note that @end is treated as max when <= 0. This is to always allow
- * using @start + N as @end as long as N is inside integer range.
- *
- * Simultaneous modifications to the @idr are not allowed and should be
- * prevented by the user, usually with a lock. idr_alloc() may be called
- * concurrently with read-only accesses to the @idr, such as idr_find() and
- * idr_for_each_entry().
- */
-static inline int idr_alloc(struct idr *idr, void *ptr,
- int start, int end, gfp_t gfp)
-{
- unsigned long id;
- int ret;
-
- if (WARN_ON_ONCE(start < 0))
- return -EINVAL;
-
- ret = idr_alloc_cmn(idr, ptr, &id, start, end, gfp, false);
-
- if (ret)
- return ret;
-
- return id;
-}
-
-static inline int idr_alloc_ext(struct idr *idr, void *ptr,
- unsigned long *index,
- unsigned long start,
- unsigned long end,
- gfp_t gfp)
-{
- return idr_alloc_cmn(idr, ptr, index, start, end, gfp, true);
-}
-
+int idr_alloc(struct idr *, void *, int start, int end, gfp_t);
+int __must_check idr_alloc_ul(struct idr *, void *, unsigned long *nextid,
+ unsigned long max, gfp_t);
int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t);
int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
void *idr_get_next(struct idr *, int *nextid);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
-void *idr_replace(struct idr *, void *, int id);
-void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
+void *idr_get_next_ul(struct idr *, unsigned long *nextid);
+void *idr_replace(struct idr *, void *, unsigned long id);
void idr_destroy(struct idr *);
-static inline void *idr_remove_ext(struct idr *idr, unsigned long id)
+static inline void *idr_remove(struct idr *idr, unsigned long id)
{
return radix_tree_delete_item(&idr->idr_rt, id, NULL);
}
-static inline void *idr_remove(struct idr *idr, int id)
-{
- return idr_remove_ext(idr, id);
-}
-
static inline void idr_init(struct idr *idr)
{
INIT_RADIX_TREE(&idr->idr_rt, IDR_RT_MARKER);
@@ -184,16 +132,11 @@ static inline void idr_preload_end(void)
* This function can be called under rcu_read_lock(), given that the leaf
* pointers lifetimes are correctly managed.
*/
-static inline void *idr_find_ext(const struct idr *idr, unsigned long id)
+static inline void *idr_find(const struct idr *idr, unsigned long id)
{
return radix_tree_lookup(&idr->idr_rt, id);
}
-static inline void *idr_find(const struct idr *idr, int id)
-{
- return idr_find_ext(idr, id);
-}
-
/**
* idr_for_each_entry - iterate over an idr's elements of a given type
* @idr: idr handle
@@ -206,8 +149,8 @@ static inline void *idr_find(const struct idr *idr, int id)
*/
#define idr_for_each_entry(idr, entry, id) \
for (id = 0; ((entry) = idr_get_next(idr, &(id))) != NULL; ++id)
-#define idr_for_each_entry_ext(idr, entry, id) \
- for (id = 0; ((entry) = idr_get_next_ext(idr, &(id))) != NULL; ++id)
+#define idr_for_each_entry_ul(idr, entry, id) \
+ for (id = 0; ((entry) = idr_get_next_ul(idr, &(id))) != NULL; ++id)
/**
* idr_for_each_entry_continue - continue iteration over an idr's elements of a given type
diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 567ebb5eaab0..8275fc2ed0f4 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -358,24 +358,9 @@ int radix_tree_split(struct radix_tree_root *, unsigned long index,
int radix_tree_join(struct radix_tree_root *, unsigned long index,
unsigned new_order, void *);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max);
-static inline void __rcu **idr_get_free(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- int end)
-{
- return idr_get_free_cmn(root, iter, gfp, end > 0 ? end - 1 : INT_MAX);
-}
-
-static inline void __rcu **idr_get_free_ext(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- unsigned long end)
-{
- return idr_get_free_cmn(root, iter, gfp, end - 1);
-}
enum {
RADIX_TREE_ITER_TAG_MASK = 0x0f, /* tag index in lower nybble */
diff --git a/lib/idr.c b/lib/idr.c
index 082778cf883e..230879a65d99 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -7,9 +7,26 @@
DEFINE_PER_CPU(struct ida_bitmap *, ida_bitmap);
static DEFINE_SPINLOCK(simple_ida_lock);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext)
+/**
+ * idr_alloc_ul - allocate a large ID
+ * @idr: idr handle
+ * @ptr: pointer to be associated with the new ID
+ * @nextid: Pointer to minimum ID to allocate
+ * @max: the maximum ID (inclusive)
+ * @gfp: memory allocation flags
+ *
+ * Allocates an unused ID in the range [*nextid, end] and stores it in
+ * @nextid. Note that @max differs from the @end parameter to idr_alloc().
+ *
+ * Simultaneous modifications to the @idr are not allowed and should be
+ * prevented by the user, usually with a lock. idr_alloc_ul() may be called
+ * concurrently with read-only accesses to the @idr, such as idr_find() and
+ * idr_for_each_entry().
+ *
+ * Return: 0 on success or a negative errno on failure (ENOMEM or ENOSPC)
+ */
+int idr_alloc_ul(struct idr *idr, void *ptr, unsigned long *nextid,
+ unsigned long max, gfp_t gfp)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -17,22 +34,54 @@ int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
return -EINVAL;
- radix_tree_iter_init(&iter, start);
- if (ext)
- slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
- else
- slot = idr_get_free(&idr->idr_rt, &iter, gfp, end);
+ radix_tree_iter_init(&iter, *nextid);
+ slot = idr_get_free(&idr->idr_rt, &iter, gfp, max);
if (IS_ERR(slot))
return PTR_ERR(slot);
radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
- if (index)
- *index = iter.index;
+ *nextid = iter.index;
return 0;
}
-EXPORT_SYMBOL_GPL(idr_alloc_cmn);
+EXPORT_SYMBOL_GPL(idr_alloc_ul);
+
+/**
+ * idr_alloc - allocate an id
+ * @idr: idr handle
+ * @ptr: pointer to be associated with the new id
+ * @start: the minimum id (inclusive)
+ * @end: the maximum id (exclusive)
+ * @gfp: memory allocation flags
+ *
+ * Allocates an unused ID in the range [start, end). Returns -ENOSPC
+ * if there are no unused IDs in that range.
+ *
+ * Note that @end is treated as max when <= 0. This is to always allow
+ * using @start + N as @end as long as N is inside integer range.
+ *
+ * Simultaneous modifications to the @idr are not allowed and should be
+ * prevented by the user, usually with a lock. idr_alloc() may be called
+ * concurrently with read-only accesses to the @idr, such as idr_find() and
+ * idr_for_each_entry().
+ */
+int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
+{
+ unsigned long id = start;
+ int ret;
+
+ if (WARN_ON_ONCE(start < 0))
+ return -EINVAL;
+
+ ret = idr_alloc_ul(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
+
+ if (ret)
+ return ret;
+
+ return id;
+}
+EXPORT_SYMBOL_GPL(idr_alloc);
/**
* idr_alloc_cyclic - allocate new idr entry in a cyclical fashion
@@ -121,7 +170,7 @@ void *idr_get_next(struct idr *idr, int *nextid)
}
EXPORT_SYMBOL(idr_get_next);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
+void *idr_get_next_ul(struct idr *idr, unsigned long *nextid)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -133,7 +182,7 @@ void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
*nextid = iter.index;
return rcu_dereference_raw(*slot);
}
-EXPORT_SYMBOL(idr_get_next_ext);
+EXPORT_SYMBOL(idr_get_next_ul);
/**
* idr_replace - replace pointer for given id
@@ -149,16 +198,7 @@ EXPORT_SYMBOL(idr_get_next_ext);
* Returns: 0 on success. %-ENOENT indicates that @id was not found.
* %-EINVAL indicates that @id or @ptr were not valid.
*/
-void *idr_replace(struct idr *idr, void *ptr, int id)
-{
- if (WARN_ON_ONCE(id < 0))
- return ERR_PTR(-EINVAL);
-
- return idr_replace_ext(idr, ptr, id);
-}
-EXPORT_SYMBOL(idr_replace);
-
-void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
+void *idr_replace(struct idr *idr, void *ptr, unsigned long id)
{
struct radix_tree_node *node;
void __rcu **slot = NULL;
@@ -175,7 +215,7 @@ void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
return entry;
}
-EXPORT_SYMBOL(idr_replace_ext);
+EXPORT_SYMBOL(idr_replace);
/**
* DOC: IDA description
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 8b1feca1230a..9fcd4e5c5237 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -2139,7 +2139,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
}
EXPORT_SYMBOL(ida_pre_get);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max)
{
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a306974e2fb4..131817ab3ad3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -73,7 +73,7 @@ static void free_tcf(struct rcu_head *head)
static void tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p)
{
spin_lock_bh(&idrinfo->lock);
- idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);
+ idr_remove(&idrinfo->action_idr, p->tcfa_index);
spin_unlock_bh(&idrinfo->lock);
gen_kill_estimator(&p->tcfa_rate_est);
/*
@@ -121,7 +121,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
s_i = cb->args[0];
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
index++;
if (index < s_i)
continue;
@@ -178,7 +178,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (nla_put_string(skb, TCA_KIND, ops->kind))
goto nla_put_failure;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED) {
module_put(p->ops->owner);
@@ -216,10 +216,10 @@ EXPORT_SYMBOL(tcf_generic_walker);
static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
{
- struct tc_action *p = NULL;
+ struct tc_action *p;
spin_lock_bh(&idrinfo->lock);
- p = idr_find_ext(&idrinfo->action_idr, index);
+ p = idr_find(&idrinfo->action_idr, index);
spin_unlock_bh(&idrinfo->lock);
return p;
@@ -296,10 +296,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
spin_lock_init(&p->tcfa_lock);
/* user doesn't specify an index */
if (!index) {
+ idr_index = 1;
idr_preload(GFP_KERNEL);
spin_lock_bh(&idrinfo->lock);
- err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
- GFP_ATOMIC);
+ err = idr_alloc_ul(idr, NULL, &idr_index, UINT_MAX, GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
idr_preload_end();
if (err) {
@@ -309,10 +309,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
}
p->tcfa_index = idr_index;
} else {
+ idr_index = index;
idr_preload(GFP_KERNEL);
spin_lock_bh(&idrinfo->lock);
- err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
- GFP_ATOMIC);
+ err = idr_alloc_ul(idr, NULL, &idr_index, index, GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
idr_preload_end();
if (err)
@@ -345,7 +345,7 @@ void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
struct tcf_idrinfo *idrinfo = tn->idrinfo;
spin_lock_bh(&idrinfo->lock);
- idr_replace_ext(&idrinfo->action_idr, a, a->tcfa_index);
+ idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
spin_unlock_bh(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);
@@ -358,7 +358,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
int ret;
unsigned long id = 1;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED)
module_put(ops->owner);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 1a267e77c6de..ad30968f587d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -298,7 +298,7 @@ static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f)
{
struct cls_fl_head *head = rtnl_dereference(tp->root);
- idr_remove_ext(&head->handle_idr, f->handle);
+ idr_remove(&head->handle_idr, f->handle);
list_del_rcu(&f->list);
if (!tc_skip_hw(f->flags))
fl_hw_destroy_filter(tp, f);
@@ -341,7 +341,7 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
{
struct cls_fl_head *head = rtnl_dereference(tp->root);
- return idr_find_ext(&head->handle_idr, handle);
+ return idr_find(&head->handle_idr, handle);
}
static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
@@ -901,8 +901,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout;
if (!handle) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- 1, 0x80000000, GFP_KERNEL);
+ idr_index = 1;
+ err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
+ INT_MAX, GFP_KERNEL);
if (err)
goto errout;
fnew->handle = idr_index;
@@ -910,8 +911,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
/* user specifies a handle and it doesn't exist */
if (handle && !fold) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- handle, handle + 1, GFP_KERNEL);
+ idr_index = handle;
+ err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
+ handle, GFP_KERNEL);
if (err)
goto errout;
fnew->handle = idr_index;
@@ -970,7 +972,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
if (fold) {
fnew->handle = handle;
- idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
+ idr_replace(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->list, &fnew->list);
tcf_unbind_filter(tp, &fold->res);
call_rcu(&fold->rcu, fl_destroy_filter);
--
2.14.1
^ permalink raw reply related
* Re: [net-next PATCHv6 1/2] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Rob Herring @ 2017-09-11 20:37 UTC (permalink / raw)
To: Jassi Brar
Cc: netdev, devicetree, linux-arm-kernel, davem, patches, arnd,
mark.rutland, andy, Jassi Brar
In-Reply-To: <1504088752-6204-1-git-send-email-jaswinder.singh@linaro.org>
On Wed, Aug 30, 2017 at 03:55:52PM +0530, Jassi Brar wrote:
> This patch adds documentation for Device-Tree bindings for the
> Socionext NetSec Controller driver.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
> .../devicetree/bindings/net/socionext-netsec.txt | 46 ++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/socionext-netsec.txt
>
> diff --git a/Documentation/devicetree/bindings/net/socionext-netsec.txt b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> new file mode 100644
> index 0000000..12d596c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> @@ -0,0 +1,46 @@
> +* Socionext NetSec Ethernet Controller IP
> +
> +Required properties:
> +- compatible: Should be "socionext,netsecv5"
This should be SoC specific, or also have an SoC specific string.
> +- reg: Address and length of the register sets, the first is the main
> + registers, then the rdlar and tdlar regions for the SoC
> +- interrupts: Should contain ethernet controller interrupt
> +- clocks: phandle to any clocks to be switched by runtime_pm
> +- phy-mode: See ethernet.txt file in the same directory
> +- max-speed: See ethernet.txt file in the same directory
> +- max-frame-size: See ethernet.txt file in the same directory, if 9000 or
> + above jumbo frames are enabled
> +- local-mac-address: See ethernet.txt file in the same directory
> +- phy-handle: phandle to select child phy
> +
> +Optional properties:
> +- use-jumbo: Boolean property to suggest if jumbo packets should be used or not
Is this common or vendor specific. The former should refer to a common
doc. The latter should have a vendor prefix. I'd think the former makes
sense here.
But then why do you need this and max-frame-size?
> +
> +For the child phy
> +
> +- compatible "ethernet-phy-ieee802.3-c22" is needed
> +- device_type "ethernet-phy"
> +- reg: phy address
> +
> +
> +Example:
> + eth0: ethernet {
> + compatible = "socionext,netsecv5";
> + reg = <0 0x31600000 0x10000>, <0 0x31618000 0x4000>, <0 0x3161c000 0x4000>;
> + interrupts = <0 163 0x4>;
> + clocks = <&clk_alw_0_8>;
> + phy-mode = "rgmii";
> + max-speed = <1000>;
> + max-frame-size = <9000>;
> + local-mac-address = [ a4 17 31 00 00 ed ];
> + phy-handle = <ðphy0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy0: ethernet-phy@1 {
> + device_type = "ethernet-phy";
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + };
> + };
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 net 1/3] lan78xx: Fix for eeprom read/write when device auto suspend
From: Andrew Lunn @ 2017-09-11 20:24 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA335C84@CHN-SV-EXMX03.mchp-main.com>
On Mon, Sep 11, 2017 at 06:51:49PM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Fix for eeprom read/write when device auto suspend
>
> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index b99a7fb..baf91c7 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1265,30 +1265,44 @@ static int lan78xx_ethtool_get_eeprom(struct net_device *netdev,
> struct ethtool_eeprom *ee, u8 *data)
> {
> struct lan78xx_net *dev = netdev_priv(netdev);
> + int ret = -EINVAL;
> +
> + if (usb_autopm_get_interface(dev->intf) < 0)
> + return ret;
Hi Nisar
It is better to do
ret = usb_autopm_get_interface(dev->intf;
if (ret)
return ret;
i.e. use the error code usb_autopm_get_interface() gives you.
> ee->magic = LAN78XX_EEPROM_MAGIC;
>
> - return lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
> + ret = lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
> +
> + usb_autopm_put_interface(dev->intf);
> +
> + return ret;
> }
>
> static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
> struct ethtool_eeprom *ee, u8 *data)
> {
> struct lan78xx_net *dev = netdev_priv(netdev);
> + int ret = -EINVAL;
> +
> + if (usb_autopm_get_interface(dev->intf) < 0)
> + return ret;
Same here.
Andrew
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Andrew Lunn @ 2017-09-11 20:19 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170911190850.GA2291@Red>
> Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
> So no the CLK/RST are really for the PHY.
Thanks for trying that.
You said it was probably during scanning of the bus it times out. What
address is causing the timeout? 0 or 1? If the internal bus can only
have one PHY on it, maybe we need to set bus->phy_mask to 0x1?
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH v2 net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Woojung.Huh @ 2017-09-11 20:13 UTC (permalink / raw)
To: Nisar.Sayed, davem; +Cc: UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA335C94@CHN-SV-EXMX03.mchp-main.com>
Nisar,
>@@ -1290,8 +1290,8 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
>
> /* Allow entire eeprom update only */
> if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
>- (ee->offset == 0) &&
>- (ee->len == 512) &&
>+ (ee->offset >= 0 && ee->offset < MAX_EEPROM_SIZE) &&
>+ (ee->len > 0 && (ee->offset + ee->len) <= MAX_EEPROM_SIZE) &&
> (data[0] == EEPROM_INDICATOR))
This patch is for writing any len at any offset.
However, "(data[0] == EEPROM_INDICATOR)" prevents it.
> ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
> else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
- Woojung
^ permalink raw reply
* Re: [PATCH] netfilter: ipset: ipset list may return wrong member count for set with timeout
From: Vishwanath Pai @ 2017-09-11 19:57 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Pablo Neira Ayuso, johunt, pai.vishwain, netfilter-devel,
coreteam, netdev
In-Reply-To: <alpine.DEB.2.11.1709112134060.12411@blackhole.kfki.hu>
Hi Jozsef,
Thank you. Yes, that is true, the count can still be incorrect in the
case of a huge set.
Thanks,
Vishwanath
On 09/11/2017 03:36 PM, Jozsef Kadlecsik wrote:
> Hi,
>
> Your patch is applied in the ipset git tree and I'm going to push it for
> kernel inclusion.
>
> I modified the comment part: the elements counter can still be incorrect
> in the case of a huge set, because elements might time out during the
> listing.
>
> Thanks for your patience!
>
> Best regards,
> Jozsef
>
> On Thu, 17 Aug 2017, Vishwanath Pai wrote:
>
>> Simple testcase:
>>
>> $ ipset create test hash:ip timeout 5
>> $ ipset add test 1.2.3.4
>> $ ipset add test 1.2.2.2
>> $ sleep 5
>>
>> $ ipset l
>> Name: test
>> Type: hash:ip
>> Revision: 5
>> Header: family inet hashsize 1024 maxelem 65536 timeout 5
>> Size in memory: 296
>> References: 0
>> Number of entries: 2
>> Members:
>>
>> We return "Number of entries: 2" but no members are listed. That is
>> because mtype_list runs "ip_set_timeout_expired" and does not list the
>> expired entries, but set->elements is never upated (until mtype_gc
>> cleans it up later).
>>
>> Reviewed-by: Joshua Hunt <johunt@akamai.com>
>> Signed-off-by: Vishwanath Pai <vpai@akamai.com>
>> ---
>> net/netfilter/ipset/ip_set_hash_gen.h | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
>> index f236c0b..ff3d31c 100644
>> --- a/net/netfilter/ipset/ip_set_hash_gen.h
>> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
>> @@ -1041,12 +1041,22 @@ struct htype {
>> static int
>> mtype_head(struct ip_set *set, struct sk_buff *skb)
>> {
>> - const struct htype *h = set->data;
>> + struct htype *h = set->data;
>> const struct htable *t;
>> struct nlattr *nested;
>> size_t memsize;
>> u8 htable_bits;
>>
>> + /* If any members have expired, set->elements will be wrong
>> + * mytype_expire function will update it with the right count.
>> + * we do not hold set->lock here, so grab it first.
>> + */
>> + if (SET_WITH_TIMEOUT(set)) {
>> + spin_lock_bh(&set->lock);
>> + mtype_expire(set, h);
>> + spin_unlock_bh(&set->lock);
>> + }
>> +
>> rcu_read_lock_bh();
>> t = rcu_dereference_bh_nfnl(h->table);
>> memsize = mtype_ahash_memsize(h, t) + set->ext_size;
>> --
>> 1.9.1
>>
>>
>
> -
> E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
> PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
> Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
> H-1525 Budapest 114, POB. 49, Hungary
>
^ permalink raw reply
* [PATCH v2] openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Christophe JAILLET @ 2017-09-11 19:56 UTC (permalink / raw)
To: pshelar, davem, xiangxia.m.yue
Cc: netdev, dev, linux-kernel, kernel-janitors, Christophe JAILLET
All other error handling paths in this function go through the 'error'
label. This one should do the same.
Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.
I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.
So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.
Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.
V2: update Subject line
---
net/openvswitch/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
"Flow key attribute not present in set flow.");
- return -EINVAL;
+ error = -EINVAL;
+ goto error;
}
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] netfilter: ipset: ipset list may return wrong member count for set with timeout
From: Jozsef Kadlecsik @ 2017-09-11 19:36 UTC (permalink / raw)
To: Vishwanath Pai
Cc: Pablo Neira Ayuso, johunt, pai.vishwain, netfilter-devel,
coreteam, netdev
In-Reply-To: <1502947435-18549-1-git-send-email-vpai@akamai.com>
Hi,
Your patch is applied in the ipset git tree and I'm going to push it for
kernel inclusion.
I modified the comment part: the elements counter can still be incorrect
in the case of a huge set, because elements might time out during the
listing.
Thanks for your patience!
Best regards,
Jozsef
On Thu, 17 Aug 2017, Vishwanath Pai wrote:
> Simple testcase:
>
> $ ipset create test hash:ip timeout 5
> $ ipset add test 1.2.3.4
> $ ipset add test 1.2.2.2
> $ sleep 5
>
> $ ipset l
> Name: test
> Type: hash:ip
> Revision: 5
> Header: family inet hashsize 1024 maxelem 65536 timeout 5
> Size in memory: 296
> References: 0
> Number of entries: 2
> Members:
>
> We return "Number of entries: 2" but no members are listed. That is
> because mtype_list runs "ip_set_timeout_expired" and does not list the
> expired entries, but set->elements is never upated (until mtype_gc
> cleans it up later).
>
> Reviewed-by: Joshua Hunt <johunt@akamai.com>
> Signed-off-by: Vishwanath Pai <vpai@akamai.com>
> ---
> net/netfilter/ipset/ip_set_hash_gen.h | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> index f236c0b..ff3d31c 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -1041,12 +1041,22 @@ struct htype {
> static int
> mtype_head(struct ip_set *set, struct sk_buff *skb)
> {
> - const struct htype *h = set->data;
> + struct htype *h = set->data;
> const struct htable *t;
> struct nlattr *nested;
> size_t memsize;
> u8 htable_bits;
>
> + /* If any members have expired, set->elements will be wrong
> + * mytype_expire function will update it with the right count.
> + * we do not hold set->lock here, so grab it first.
> + */
> + if (SET_WITH_TIMEOUT(set)) {
> + spin_lock_bh(&set->lock);
> + mtype_expire(set, h);
> + spin_unlock_bh(&set->lock);
> + }
> +
> rcu_read_lock_bh();
> t = rcu_dereference_bh_nfnl(h->table);
> memsize = mtype_ahash_memsize(h, t) + set->ext_size;
> --
> 1.9.1
>
>
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* Re: [PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: David Miller @ 2017-09-11 19:34 UTC (permalink / raw)
To: christophe.jaillet-39ZsbGIQGT5GWvitb5QawA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170911192015.17553-1-christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org>
Please use a proper Subject subsystem prefix for openvswitch patches.
"datapath" isn't very specific in the global kernel namespace at all.
The entire networking stack packet processing path is a "datapath"
"openvswitch: " would have been much better.
^ permalink raw reply
* [PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Christophe JAILLET @ 2017-09-11 19:20 UTC (permalink / raw)
To: pshelar, davem, xiangxia.m.yue
Cc: netdev, dev, linux-kernel, kernel-janitors, Christophe JAILLET
All other error handling paths in this function go through the 'error'
label. This one should do the same.
Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.
I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.
So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.
Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.
---
net/openvswitch/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
"Flow key attribute not present in set flow.");
- return -EINVAL;
+ error = -EINVAL;
+ goto error;
}
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Corentin Labbe @ 2017-09-11 19:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170911161124.GD27599@lunn.ch>
On Mon, Sep 11, 2017 at 06:11:24PM +0200, Andrew Lunn wrote:
> On Fri, Sep 08, 2017 at 04:28:25PM +0200, Corentin Labbe wrote:
> > On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > > > Do you know why the reset times out/fails?
> > > > >
> > > >
> > > > Because there are nothing connected to it.
> > >
> > > That should not be an issue. A read should just return 0xffff. And it
> > > should return 0xffff fast. The timing of the MDIO protocol is fixed. A
> > > read or a write takes a fixed number of cycles, independent of if
> > > there is a device there or not. The bus data line has a pullup, so if
> > > you try to access a missing device, you automatically read 0xffff.
> > >
> >
> > Perhaps, but the reality is that with nothing connected to it, the reset of the MAC timeout.
> > Certainly, the MAC does not support finding no PHY.
>
> Are you sure this is not because of the clock and reset?
>
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
>
> The way you describe it here, the clock and reset are for the PHY. But
> maybe it is actually for the bus? I can understand a bus timing out if
> it has no clock, or it is held in reset. Try enabling the clock and
> reset when the internal bus is selected, not when the PHY on the bus
> is selected.
>
Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
So no the CLK/RST are really for the PHY.
Regards
PS: patch and result with "integrated CLK/RST always on"
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -659,7 +659,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
u32 reg, val;
int ret = 0;
- bool need_reset = false;
+ bool need_reset = true;
if (current_child ^ desired_child) {
regmap_read(gmac->regmap, SYSCON_EMAC_REG, ®);
@@ -824,7 +824,7 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
int ret;
if (!gmac->use_internal_phy)
- return 0;
+ dev_info(priv->device, "IPHY BYPASS\n");
ret = clk_prepare_enable(gmac->ephy_clk);
if (ret) {
[ 18.057162] dwmac-sun8i 1c30000.ethernet: Will use external PHY
[ 18.183789] dwmac-sun8i 1c30000.ethernet: IPHY BYPASS
[ 18.184136] dwmac-sun8i 1c30000.ethernet: Chain mode enabled
[ 18.184158] dwmac-sun8i 1c30000.ethernet: No HW DMA feature register supported
[ 18.184175] dwmac-sun8i 1c30000.ethernet: Normal descriptors
[ 18.184192] dwmac-sun8i 1c30000.ethernet: RX Checksum Offload Engine supported
[ 18.184214] dwmac-sun8i 1c30000.ethernet: COE Type 2
[ 18.184231] dwmac-sun8i 1c30000.ethernet: TX Checksum insertion supported
[ 18.185491] libphy: stmmac: probed
[ 18.188481] libphy: mdio_mux: probed
[ 18.188831] dwmac-sun8i 1c30000.ethernet: Switch mux to internal PHY
[ 18.288981] dwmac-sun8i 1c30000.ethernet: EMAC reset timeout
[ 18.289559] libphy: mdio_mux: probed
[ 18.289629] dwmac-sun8i 1c30000.ethernet: Switch mux to external PHY
[ 20.578316] EXT4-fs (mmcblk0p1): re-mounted. Opts: (null)
[ 31.240650] RTL8211E Gigabit Ethernet 0.1:00: attached PHY driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=0.1:00, irq=POLL)
^ permalink raw reply
* [PATCH v2 net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Nisar.Sayed @ 2017-09-11 18:52 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Allow EEPROM write for less than MAX_EEPROM_SIZE
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index baf91c7..02d64f75 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1290,8 +1290,8 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
- (ee->offset == 0) &&
- (ee->len == 512) &&
+ (ee->offset >= 0 && ee->offset < MAX_EEPROM_SIZE) &&
+ (ee->len > 0 && (ee->offset + ee->len) <= MAX_EEPROM_SIZE) &&
(data[0] == EEPROM_INDICATOR))
ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 3/3] lan78xx: Use default value loaded from EEPROM/OTP when resetting
From: Nisar.Sayed @ 2017-09-11 18:52 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Use default value loaded from EEPROM/OTP when resetting
The LAN78xx allows platform configurations to be loaded from EEPROM/OTP.
Ex: When external phy is connected, the MAC can be configured to
have speed, duplex, polarity configurations loadded from the EEPROM/OTP.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 02d64f75..ffe302a 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2448,7 +2448,6 @@ static int lan78xx_reset(struct lan78xx_net *dev)
/* LAN7801 only has RGMII mode */
if (dev->chipid == ID_REV_CHIP_ID_7801_)
buf &= ~MAC_CR_GMII_EN_;
- buf |= MAC_CR_AUTO_DUPLEX_ | MAC_CR_AUTO_SPEED_;
ret = lan78xx_write_reg(dev, MAC_CR, buf);
ret = lan78xx_read_reg(dev, MAC_TX, &buf);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 1/3] lan78xx: Fix for eeprom read/write when device auto suspend
From: Nisar.Sayed @ 2017-09-11 18:51 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Fix for eeprom read/write when device auto suspend
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index b99a7fb..baf91c7 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1265,30 +1265,44 @@ static int lan78xx_ethtool_get_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
+ int ret = -EINVAL;
+
+ if (usb_autopm_get_interface(dev->intf) < 0)
+ return ret;
ee->magic = LAN78XX_EEPROM_MAGIC;
- return lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
+ ret = lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
+
+ usb_autopm_put_interface(dev->intf);
+
+ return ret;
}
static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
+ int ret = -EINVAL;
+
+ if (usb_autopm_get_interface(dev->intf) < 0)
+ return ret;
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == EEPROM_INDICATOR))
- return lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
+ ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == OTP_INDICATOR_1))
- return lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
+ ret = lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
- return -EINVAL;
+ usb_autopm_put_interface(dev->intf);
+
+ return ret;
}
static void lan78xx_get_strings(struct net_device *netdev, u32 stringset,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 0/3] lan78xx: Fixes to lan78xx driver
From: Nisar.Sayed @ 2017-09-11 18:51 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
This series of patches are for lan78xx driver.
These patches fixes potential issues associated with lan78xx driver
v2
- Added patch version information
- Added fixes tag
- Updated patch description
- Updated changes as per comments
v1
- Spitted patches as per comments
- Dropped "fixed_phy device support" and "Fix for system suspend" changes
Nisar Sayed (3):
Fix for eeprom read/write when device auto suspend
Allow EEPROM write for less than MAX_EEPROM_SIZE
Use default value loaded from EEPROM/OTP when resetting
drivers/net/usb/lan78xx.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH] net/sched: fix pointer check in gen_handle
From: Cong Wang @ 2017-09-11 18:05 UTC (permalink / raw)
To: Josh Hunt
Cc: David Miller, Jamal Hadi Salim, Jiri Pirko,
Linux Kernel Network Developers
In-Reply-To: <1505072930-13049-1-git-send-email-johunt@akamai.com>
On Sun, Sep 10, 2017 at 12:48 PM, Josh Hunt <johunt@akamai.com> wrote:
> Fixes sparse warning about pointer in gen_handle:
> net/sched/cls_rsvp.h:392:40: warning: Using plain integer as NULL pointer
>
> Fixes: 8113c095672f6 ("net_sched: use void pointer for filter handle")
> Signed-off-by: Josh Hunt <johunt@akamai.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Alexander Duyck @ 2017-09-11 17:48 UTC (permalink / raw)
To: Samudrala, Sridhar; +Cc: Duyck, Alexander H, Netdev
In-Reply-To: <91a48bc5-57d9-db09-78c0-98a49b414a28@intel.com>
On Mon, Sep 11, 2017 at 9:48 AM, Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
>
> On 9/9/2017 6:28 PM, Alexander Duyck wrote:
>>
>> On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
>> <sridhar.samudrala@intel.com> wrote:
>>>
>>> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be
>>> used
>>> to enable symmetric tx and rx queues on a socket.
>>>
>>> This option is specifically useful for epoll based multi threaded
>>> workloads
>>> where each thread handles packets received on a single RX queue . In this
>>> model,
>>> we have noticed that it helps to send the packets on the same TX queue
>>> corresponding to the queue-pair associated with the RX queue specifically
>>> when
>>> busy poll is enabled with epoll().
>>>
>>> Two new fields are added to struct sock_common to cache the last rx
>>> ifindex and
>>> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns
>>> the cached
>>> rx queue when this option is enabled and the TX is happening on the same
>>> device.
>>>
>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>> ---
>>> include/net/request_sock.h | 1 +
>>> include/net/sock.h | 17 +++++++++++++++++
>>> include/uapi/asm-generic/socket.h | 2 ++
>>> net/core/dev.c | 8 +++++++-
>>> net/core/sock.c | 10 ++++++++++
>>> net/ipv4/tcp_input.c | 1 +
>>> net/ipv4/tcp_ipv4.c | 1 +
>>> net/ipv4/tcp_minisocks.c | 1 +
>>> 8 files changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>>> index 23e2205..c3bc12e 100644
>>> --- a/include/net/request_sock.h
>>> +++ b/include/net/request_sock.h
>>> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct
>>> request_sock *req)
>>> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
>>> sk_node_init(&req_to_sk(req)->sk_node);
>>> sk_tx_queue_clear(req_to_sk(req));
>>> + req_to_sk(req)->sk_symmetric_queues =
>>> sk_listener->sk_symmetric_queues;
>>> req->saved_syn = NULL;
>>> refcount_set(&req->rsk_refcnt, 0);
>>>
>>> diff --git a/include/net/sock.h b/include/net/sock.h
>>> index 03a3625..3421809 100644
>>> --- a/include/net/sock.h
>>> +++ b/include/net/sock.h
>>> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char
>>> *msg, ...)
>>> * @skc_node: main hash linkage for various protocol lookup tables
>>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>>> * @skc_tx_queue_mapping: tx queue number for this connection
>>> + * @skc_rx_queue_mapping: rx queue number for this connection
>>> + * @skc_rx_ifindex: rx ifindex for this connection
>>> * @skc_flags: place holder for sk_flags
>>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>>> * @skc_incoming_cpu: record/match cpu processing incoming packets
>>> * @skc_refcnt: reference count
>>> + * @skc_symmetric_queues: symmetric tx/rx queues
>>> *
>>> * This is the minimal network layer representation of sockets, the
>>> header
>>> * for struct sock and struct inet_timewait_sock.
>>> @@ -177,6 +180,7 @@ struct sock_common {
>>> unsigned char skc_reuseport:1;
>>> unsigned char skc_ipv6only:1;
>>> unsigned char skc_net_refcnt:1;
>>> + unsigned char skc_symmetric_queues:1;
>>> int skc_bound_dev_if;
>>> union {
>>> struct hlist_node skc_bind_node;
>>> @@ -214,6 +218,8 @@ struct sock_common {
>>> struct hlist_nulls_node skc_nulls_node;
>>> };
>>> int skc_tx_queue_mapping;
>>> + int skc_rx_queue_mapping;
>>> + int skc_rx_ifindex;
>>> union {
>>> int skc_incoming_cpu;
>>> u32 skc_rcv_wnd;
>>> @@ -324,6 +330,8 @@ struct sock {
>>> #define sk_nulls_node __sk_common.skc_nulls_node
>>> #define sk_refcnt __sk_common.skc_refcnt
>>> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
>>> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
>>> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>>>
>>> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
>>> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
>>> @@ -340,6 +348,7 @@ struct sock {
>>> #define sk_reuseport __sk_common.skc_reuseport
>>> #define sk_ipv6only __sk_common.skc_ipv6only
>>> #define sk_net_refcnt __sk_common.skc_net_refcnt
>>> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
>>> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
>>> #define sk_bind_node __sk_common.skc_bind_node
>>> #define sk_prot __sk_common.skc_prot
>>> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct
>>> sock *sk)
>>> return sk ? sk->sk_tx_queue_mapping : -1;
>>> }
>>>
>>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff
>>> *skb)
>>> +{
>>> + if (sk->sk_symmetric_queues) {
>>> + sk->sk_rx_ifindex = skb->skb_iif;
>>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>>> + }
>>> +}
>>> +
>>> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
>>> {
>>> sk_tx_queue_clear(sk);
>>> diff --git a/include/uapi/asm-generic/socket.h
>>> b/include/uapi/asm-generic/socket.h
>>> index e47c9e4..f6b416e 100644
>>> --- a/include/uapi/asm-generic/socket.h
>>> +++ b/include/uapi/asm-generic/socket.h
>>> @@ -106,4 +106,6 @@
>>>
>>> #define SO_ZEROCOPY 60
>>>
>>> +#define SO_SYMMETRIC_QUEUES 61
>>> +
>>> #endif /* __ASM_GENERIC_SOCKET_H */
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 270b547..d96cda8 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device
>>> *dev, struct sk_buff *skb)
>>>
>>> if (queue_index < 0 || skb->ooo_okay ||
>>> queue_index >= dev->real_num_tx_queues) {
>>> - int new_index = get_xps_queue(dev, skb);
>>> + int new_index = -1;
>>> +
>>> + if (sk && sk->sk_symmetric_queues && dev->ifindex ==
>>> sk->sk_rx_ifindex)
>>> + new_index = sk->sk_rx_queue_mapping;
>>> +
>>> + if (new_index < 0 || new_index >=
>>> dev->real_num_tx_queues)
>>> + new_index = get_xps_queue(dev, skb);
>>>
>>> if (new_index < 0)
>>> new_index = skb_tx_hash(dev, skb);
>>
>> So one thing I am not sure about is if we should be overriding XPS. It
>> might make sense to instead place this after XPS so that if the root
>> user configures it then it applies, otherwise if the socket is
>> requesting symmetric queues you could fall back to that, and then
>> finally just use hashing as the final solution for distributing the
>> workload.
>
> Isn't XPS on by default and all the devices that support XPS setup XPS maps
> as part of
> the initialization?
> Are you suggesting that the root user needs to disable XPS on the specific
> queues before an
> application can use this option to enable symmetric queues?
>
XPS and this symmetric queue logic won't really play well together
since they are both attempting to do the same thing but from different
ends. Some sort of priority needs to be defined, and I would place the
request of the kernel/root user above the request of a socket and/or
application.
I guess it comes down to if we let the Tx CPU pick the queue or Rx
queue, and I would argue with an XPS configuration in place the kernel
is requesting that the Tx CPU sets the Tx queue, not the Rx/Tx hash or
Rx queue. This is similar to how we handle this for routing today as
XPS will also override the Rx to Tx mapping used there.
>>
>> That way if somebody decides to reserve queues for some sort of
>> specific traffic like AF_PACKET then they can configure the Tx via
>> XPS, configure the Rx via RSS redirection table reprogramming, and
>> then setup a filters on the hardware to direct the traffic they want
>> to the queues that are running AF_PACKET.
>>
>>> diff --git a/net/core/sock.c b/net/core/sock.c
>>> index 9b7b6bb..3876cce 100644
>>> --- a/net/core/sock.c
>>> +++ b/net/core/sock.c
>>> @@ -1059,6 +1059,10 @@ int sock_setsockopt(struct socket *sock, int
>>> level, int optname,
>>> sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
>>> break;
>>>
>>> + case SO_SYMMETRIC_QUEUES:
>>> + sk->sk_symmetric_queues = valbool;
>>> + break;
>>> +
>>> default:
>>> ret = -ENOPROTOOPT;
>>> break;
>>> @@ -1391,6 +1395,10 @@ int sock_getsockopt(struct socket *sock, int
>>> level, int optname,
>>> v.val = sock_flag(sk, SOCK_ZEROCOPY);
>>> break;
>>>
>>> + case SO_SYMMETRIC_QUEUES:
>>> + v.val = sk->sk_symmetric_queues;
>>> + break;
>>> +
>>> default:
>>> /* We implement the SO_SNDLOWAT etc to not be settable
>>> * (1003.1g 7).
>>> @@ -2738,6 +2746,8 @@ void sock_init_data(struct socket *sock, struct
>>> sock *sk)
>>> sk->sk_max_pacing_rate = ~0U;
>>> sk->sk_pacing_rate = ~0U;
>>> sk->sk_incoming_cpu = -1;
>>> + sk->sk_rx_ifindex = -1;
>>> + sk->sk_rx_queue_mapping = -1;
>>> /*
>>> * Before updating sk_refcnt, we must commit prior changes to
>>> memory
>>> * (Documentation/RCU/rculist_nulls.txt for details)
>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>> index c5d7656..12381e0 100644
>>> --- a/net/ipv4/tcp_input.c
>>> +++ b/net/ipv4/tcp_input.c
>>> @@ -6356,6 +6356,7 @@ int tcp_conn_request(struct request_sock_ops
>>> *rsk_ops,
>>> tcp_rsk(req)->snt_isn = isn;
>>> tcp_rsk(req)->txhash = net_tx_rndhash();
>>> tcp_openreq_init_rwin(req, sk, dst);
>>> + sk_mark_rx_queue(req_to_sk(req), skb);
>>> if (!want_cookie) {
>>> tcp_reqsk_record_syn(sk, req, skb);
>>> fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc);
>>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>>> index a63486a..82f9af4 100644
>>> --- a/net/ipv4/tcp_ipv4.c
>>> +++ b/net/ipv4/tcp_ipv4.c
>>> @@ -1450,6 +1450,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff
>>> *skb)
>>>
>>> sock_rps_save_rxhash(sk, skb);
>>> sk_mark_napi_id(sk, skb);
>>> + sk_mark_rx_queue(sk, skb);
>>> if (dst) {
>>> if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif
>>> ||
>>> !dst->ops->check(dst, 0)) {
>>> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
>>> index 188a6f3..2b5efd5 100644
>>> --- a/net/ipv4/tcp_minisocks.c
>>> +++ b/net/ipv4/tcp_minisocks.c
>>> @@ -809,6 +809,7 @@ int tcp_child_process(struct sock *parent, struct
>>> sock *child,
>>>
>>> /* record NAPI ID of child */
>>> sk_mark_napi_id(child, skb);
>>> + sk_mark_rx_queue(child, skb);
>>>
>>> tcp_segs_in(tcp_sk(child), skb);
>>> if (!sock_owned_by_user(child)) {
>>> --
>>> 1.8.3.1
>>>
>
^ permalink raw reply
* [PATCH v2 net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled
From: Nisar.Sayed @ 2017-09-11 17:43 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev, steve.glendinning
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Configure pause time to 0xffff when tx flow control enabled
Set pause time to 0xffff in the pause frame to indicate the
partner to stop sending the packets. When RX buffer frees up,
the device sends pause frame with pause time zero for partner to
resume transmission.
Fixes: 2f7ca802bdae ("Add SMSC LAN9500 USB2.0 10/100 ethernet adapter driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
v0 -> v1:
* Added patch description in detail.
v1 -> v2:
* Added fixes tag
---
drivers/net/usb/smsc95xx.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 340c134..309b88a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -526,7 +526,7 @@ static void smsc95xx_set_multicast(struct net_device *netdev)
static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
u16 lcladv, u16 rmtadv)
{
- u32 flow, afc_cfg = 0;
+ u32 flow = 0, afc_cfg;
int ret = smsc95xx_read_reg(dev, AFC_CFG, &afc_cfg);
if (ret < 0)
@@ -537,20 +537,19 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
if (cap & FLOW_CTRL_RX)
flow = 0xFFFF0002;
- else
- flow = 0;
- if (cap & FLOW_CTRL_TX)
+ if (cap & FLOW_CTRL_TX) {
afc_cfg |= 0xF;
- else
+ flow |= 0xFFFF0000;
+ } else {
afc_cfg &= ~0xF;
+ }
netif_dbg(dev, link, dev->net, "rx pause %s, tx pause %s\n",
cap & FLOW_CTRL_RX ? "enabled" : "disabled",
cap & FLOW_CTRL_TX ? "enabled" : "disabled");
} else {
netif_dbg(dev, link, dev->net, "half duplex\n");
- flow = 0;
afc_cfg |= 0xF;
}
--
1.9.1
^ 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