Netdev List
 help / color / mirror / Atom feed
* [PATCH] net:netfilter alloc xt_byteslimit_htable with wrong size
From: zhizhou.tian @ 2017-09-08  3:00 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel
  Cc: Zhizhou Tian

From: Zhizhou Tian <zhizhou.tian@gmail.com>

struct xt_byteslimit_htable used hlist_head,
but alloc memory with sizeof(struct list_head)

Change-Id: I75bc60e47e0823700d4303c9d763b7995e3b7bb3
Signed-off-by: Zhizhou Tian <zhizhou.tian@gmail.com>
---
 net/netfilter/xt_hashlimit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10d4823..962ea4a 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -279,7 +279,7 @@ static int htable_create(struct net *net, struct hashlimit_cfg3 *cfg,
 		size = cfg->size;
 	} else {
 		size = (totalram_pages << PAGE_SHIFT) / 16384 /
-		       sizeof(struct list_head);
+		       sizeof(struct hlist_head);
 		if (totalram_pages > 1024 * 1024 * 1024 / PAGE_SIZE)
 			size = 8192;
 		if (size < 16)
@@ -287,7 +287,7 @@ static int htable_create(struct net *net, struct hashlimit_cfg3 *cfg,
 	}
 	/* FIXME: don't use vmalloc() here or anywhere else -HW */
 	hinfo = vmalloc(sizeof(struct xt_hashlimit_htable) +
-	                sizeof(struct list_head) * size);
+	                sizeof(struct hlist_head) * size);
 	if (hinfo == NULL)
 		return -ENOMEM;
 	*out_hinfo = hinfo;
-- 
2.7.4


^ permalink raw reply related

* [patch net-next] team: fall back to hash if table entry is empty
From: Jim Hanko @ 2017-09-08  2:25 UTC (permalink / raw)
  To: jiri, netdev, linux-kernel; +Cc: Jim Hanko

If the hash to port mapping table does not have a valid port (i.e. when
a port goes down), fall back to the simple hashing mechanism to avoid
dropping packets.

Signed-off-by: Jim Hanko <hanko@drivescale.com>
---
 drivers/net/team/team_mode_loadbalance.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 1468ddf..a5ef970 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -137,7 +137,13 @@ static struct team_port *lb_htpm_select_tx_port(struct team *team,
 						struct sk_buff *skb,
 						unsigned char hash)
 {
-	return rcu_dereference_bh(LB_HTPM_PORT_BY_HASH(lb_priv, hash));
+	struct team_port *port;
+
+	port = rcu_dereference_bh(LB_HTPM_PORT_BY_HASH(lb_priv, hash));
+	if (likely(port))
+		return port;
+	/* If no valid port in the table, fall back to simple hash */
+	return lb_hash_select_tx_port(team, lb_priv, skb, hash);
 }
 
 struct lb_select_tx_port {
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] net: ipv6: fix regression of no RTM_DELADDR sent after DAD failure
From: David Miller @ 2017-09-08  2:18 UTC (permalink / raw)
  To: mmanning; +Cc: netdev, maheshb
In-Reply-To: <1504536775-23533-1-git-send-email-mmanning@brocade.com>

From: Mike Manning <mmanning@brocade.com>
Date: Mon,  4 Sep 2017 15:52:55 +0100

> Commit f784ad3d79e5 ("ipv6: do not send RTM_DELADDR for tentative
> addresses") incorrectly assumes that no RTM_NEWADDR are sent for
> addresses in tentative state, as this does happen for the standard
> IPv6 use-case of DAD failure, see the call to ipv6_ifa_notify() in
> addconf_dad_stop(). So as a result of this change, no RTM_DELADDR is
> sent after DAD failure for a link-local when strict DAD (accept_dad=2)
> is configured, or on the next admin down in other cases. The absence
> of this notification breaks backwards compatibility and causes problems
> after DAD failure if this notification was being relied on. The
> solution is to allow RTM_DELADDR to still be sent after DAD failure.
> 
> Fixes: f784ad3d79e5("ipv6: do not send RTM_DELADDR for tentative addresses")
> Signed-off-by: Mike Manning <mmanning@brocade.com>
> Cc: Mahesh Bandewar <maheshb@google.com>

Mahesh, please review this patch.

^ permalink raw reply

* Re: [patch net] net: sched: fix memleak for chain zero
From: David Miller @ 2017-09-08  2:18 UTC (permalink / raw)
  To: jiri; +Cc: netdev, jhs, xiyou.wangcong, kubakici, mlxsw
In-Reply-To: <20170906111419.5115-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed,  6 Sep 2017 13:14:19 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> There's a memleak happening for chain 0. The thing is, chain 0 needs to
> be always present, not created on demand. Therefore tcf_block_get upon
> creation of block calls the tcf_chain_create function directly. The
> chain is created with refcnt == 1, which is not correct in this case and
> causes the memleak. So move the refcnt increment into tcf_chain_get
> function even for the case when chain needs to be created.
> 
> Reported-by: Jakub Kicinski <kubakici@wp.pl>
> Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

This doesn't apply cleanly any more, please respin.

Thanks Jiri.

^ permalink raw reply

* [PATCH] netfilter: xt_hashlimit: fix build error caused by 64bit division
From: Vishwanath Pai @ 2017-09-08  2:14 UTC (permalink / raw)
  To: pablo, netfilter-devel, torvalds, davem
  Cc: kadlec, johunt, fw, netdev, pai.vishwain, mingo, ilubashe, bp,
	luto, x86, linux-kernel, brgerst, andrew.cooper3, jgross,
	boris.ostrovsky, keescook, akpm, arnd

64bit division causes build/link errors on 32bit architectures. It
prints out error messages like:

ERROR: "__aeabi_uldivmod" [net/netfilter/xt_hashlimit.ko] undefined!

The value of avg passed through by userspace in BYTE mode cannot exceed
U32_MAX. Which means 64bit division in user2rate_bytes is unnecessary.

This fix changes the size of both the param as well as return type on
user2rate_bytes to u32.

Since anything greater than U32_MAX is an invalid input we error out in
hashlimit_mt_check_common() when this is the case.

Also fixed warning about const pointer conversion in cfg_copy().

Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
Signed-off-by: Vishwanath Pai <vpai@akamai.com>
---
 net/netfilter/xt_hashlimit.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10d4823..1d818f1 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -35,6 +35,7 @@
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <linux/netfilter/xt_hashlimit.h>
 #include <linux/mutex.h>
+#include <linux/kernel.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
@@ -527,12 +528,12 @@ static u64 user2rate(u64 user)
 	}
 }
 
-static u64 user2rate_bytes(u64 user)
+static u32 user2rate_bytes(u32 user)
 {
-	u64 r;
+	u32 r;
 
-	r = user ? 0xFFFFFFFFULL / user : 0xFFFFFFFFULL;
-	r = (r - 1) << 4;
+	r = user ? U32_MAX / user : U32_MAX;
+	r = (r - 1) << XT_HASHLIMIT_BYTE_SHIFT;
 	return r;
 }
 
@@ -588,7 +589,8 @@ static void rateinfo_init(struct dsthash_ent *dh,
 		dh->rateinfo.prev_window = 0;
 		dh->rateinfo.current_rate = 0;
 		if (hinfo->cfg.mode & XT_HASHLIMIT_BYTES) {
-			dh->rateinfo.rate = user2rate_bytes(hinfo->cfg.avg);
+			dh->rateinfo.rate =
+				user2rate_bytes((u32)hinfo->cfg.avg);
 			if (hinfo->cfg.burst)
 				dh->rateinfo.burst =
 					hinfo->cfg.burst * dh->rateinfo.rate;
@@ -870,7 +872,7 @@ static int hashlimit_mt_check_common(const struct xt_mtchk_param *par,
 
 	/* Check for overflow. */
 	if (revision >= 3 && cfg->mode & XT_HASHLIMIT_RATE_MATCH) {
-		if (cfg->avg == 0) {
+		if (cfg->avg == 0 || cfg->avg > U32_MAX) {
 			pr_info("hashlimit invalid rate\n");
 			return -ERANGE;
 		}
-- 
1.9.1


^ permalink raw reply related

* Re: Use after free in __dst_destroy_metrics_generic
From: David Miller @ 2017-09-08  2:13 UTC (permalink / raw)
  To: subashab; +Cc: netdev, eric.dumazet, lorenzo
In-Reply-To: <1234e09b75197d43ed84bdb1b154b4b6@codeaurora.org>

From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Thu, 07 Sep 2017 18:52:02 -0600

> [ 3489.194392]  __ion_alloc+0x180/0x988

I do not see the __ion_alloc function in my tree.

^ permalink raw reply

* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Kosuke Tatsukawa @ 2017-09-08  2:06 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Reinis Rozitis
In-Reply-To: <c9b6ae59-741b-3ecd-47ff-1d5b41710607@cumulusnetworks.com>

Hi,

> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
>> 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+
>> ---
>>  drivers/net/bonding/bond_options.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>> 
> 
> I don't believe this to be the right solution, hardcoding it like this
> changes user-visible behaviour. The issue is that if someone configured
> it to be 0 in tlb mode, suddenly it will become 1 and will silently
> override their config if they switch the mode to alb. Also it robs users
> from their choice.
> 
> If you think this should be settable in ALB mode, then IMO you should
> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
> That would also be consistent with how it's handled in TLB mode.

No, I don't think tlb_dynamic_lb should be settable in balance-alb at
this point.  All the current commits regarding tlb_dynamic_lb are for
balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
to 0 is an intended usage.


> Going back and looking at your previous fix I'd argue that it is also
> wrong, you should've removed the mode check altogether to return the
> original behaviour where the dynamic_lb is set to 1 (enabled) by
> default and then ALB mode would've had it, of course that would've left
> the case of setting it to 0 in TLB mode and switching to ALB, but that
> is a different issue.

Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
tlb_dynamic_lb is referenced in the following functions.

 + bond_do_alb_xmit()  -- Used by both balance-tlb and balance-alb
 + bond_tlb_xmit()  -- Only used by balance-tlb
 + bond_open()  -- Used by both balance-tlb and balance-alb
 + bond_check_params()  -- Used during module initialization
 + bond_fill_info()  -- Used to get/set value
 + bond_option_tlb_dynamic_lb_set()  -- Used to get/set value
 + bonding_show_tlb_dynamic_lb()  -- Used to get/set value
 + bond_is_nondyn_tlb()  -- Only referenced if balance-tlb mode

The following untested patch adds code to make balance-alb work as if
tlb_dynamic_lb==1 for the functions which affect balance-alb mode.  It
also reverts my previous patch.

What do you think about this approach?
---
Kosuke TATSUKAWA  | 1st Platform Software Division
                  | NEC Solution Innovators
                  | tatsu@ab.jp.nec.com

------------------------------------------------------------------------
 drivers/net/bonding/bond_alb.c  |    6 ++++--
 drivers/net/bonding/bond_main.c |    5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index c02cc81..a9229b5 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1325,7 +1325,8 @@ static int bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
 	if (!tx_slave) {
 		/* unbalanced or unassigned, send through primary */
 		tx_slave = rcu_dereference(bond->curr_active_slave);
-		if (bond->params.tlb_dynamic_lb)
+		if (bond->params.tlb_dynamic_lb ||
+		    (BOND_MODE(bond) == BOND_MODE_ALB))
 			bond_info->unbalanced_load += skb->len;
 	}
 
@@ -1339,7 +1340,8 @@ static int bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
 		goto out;
 	}
 
-	if (tx_slave && bond->params.tlb_dynamic_lb) {
+	if (tx_slave && (bond->params.tlb_dynamic_lb ||
+			 BOND_MODE(bond) == BOND_MODE_ALB)) {
 		spin_lock(&bond->mode_lock);
 		__tlb_clear_slave(bond, tx_slave, 0);
 		spin_unlock(&bond->mode_lock);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fc63992..bcb71e7 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3305,7 +3305,8 @@ static int bond_open(struct net_device *bond_dev)
 		 */
 		if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB)))
 			return -ENOMEM;
-		if (bond->params.tlb_dynamic_lb)
+		if (bond->params.tlb_dynamic_lb ||
+		    (BOND_MODE(bond) == BOND_MODE_ALB))
 			queue_delayed_work(bond->wq, &bond->alb_work, 0);
 	}
 
@@ -4601,7 +4602,7 @@ static int bond_check_params(struct bond_params *params)
 	}
 	ad_user_port_key = valptr->value;
 
-	if ((bond_mode == BOND_MODE_TLB) || (bond_mode == BOND_MODE_ALB)) {
+	if (bond_mode == BOND_MODE_TLB) {
 		bond_opt_initstr(&newval, "default");
 		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB),
 					&newval);

^ permalink raw reply related

* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-09-08  1:54 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Reinis Rozitis
In-Reply-To: <CAF2d9jh-724+D3smkYD-HvEZZfnAezBt5D18ZPPOGEjtdvrq3g@mail.gmail.com>

On Thu, Sep 7, 2017 at 5:47 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Thu, Sep 7, 2017 at 5:39 PM, Mahesh Bandewar (महेश बंडेवार)
> <maheshb@google.com> wrote:
>> On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov
>> <nikolay@cumulusnetworks.com> wrote:
>>> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>>> 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")
>>
>> This is wrong! I think you are confusing various things here. ALB is
>> different mode from TLB and TLB-dynamic-lb is *only* a special case of
>> TLB. Your earlier patch is also wrong for the same reasons. However,
>> since the default value of tlb_dynamic_lb is set to 0  it's not
>> causing issues for ALB mode otherwise it would break ALB mode.
> I take this back. The default value is 1 so ALB is broken because of
> the referenced change.
>
>> tlb_dynamic_lb has absolutely nothing to do with ALB mode. Please
>> revert the earlier change (cbf5ecb30560).
>>
>> It's not clear to me what you saw as broken, so can't really suggest
>> what really need to be done.
Please scratch all I said.

^ permalink raw reply

* [PATCH net] perf/bpf: fix a clang compilation issue
From: Yonghong Song @ 2017-09-08  1:36 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, ndesaulniers, netdev; +Cc: kernel-team

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>
---
 include/linux/syscalls.h      | 2 ++
 kernel/trace/trace_syscalls.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 88951b7..95606a2 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -200,6 +200,8 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
 #define SYSCALL_DEFINE5(name, ...) SYSCALL_DEFINEx(5, _##name, __VA_ARGS__)
 #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
 
+#define SYSCALL_DEFINE_MAXARGS	6
+
 #define SYSCALL_DEFINEx(x, sname, ...)				\
 	SYSCALL_METADATA(sname, x, __VA_ARGS__)			\
 	__SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 9c4eef2..696afe7 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -565,7 +565,7 @@ static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
 	struct syscall_tp_t {
 		unsigned long long regs;
 		unsigned long syscall_nr;
-		unsigned long args[sys_data->nb_args];
+		unsigned long args[SYSCALL_DEFINE_MAXARGS];
 	} param;
 	int i;
 
-- 
2.9.5

^ permalink raw reply related

* Re: Use after free in __dst_destroy_metrics_generic
From: Subash Abhinov Kasiviswanathan @ 2017-09-08  1:33 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: netdev, eric.dumazet, lorenzo
In-Reply-To: <20170908025620.47d0ad2e@elisabeth>

> Should be fixed by:
> 
> commit ad65a2f05695aced349e308193c6e2a6b1d87112
> Author: Wei Wang <weiwan@google.com>
> Date:   Sat Jun 17 10:42:35 2017 -0700
> 
>     ipv6: call dst_hold_safe() properly
> 

Thanks for the info Stefano.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Vinicius Costa Gomes @ 2017-09-08  1:29 UTC (permalink / raw)
  To: Henrik Austad
  Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
	ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
	richardcochran
In-Reply-To: <20170907053411.GA6580@sisyphus.home.austad.us>

Henrik Austad <henrik@austad.us> writes:

> On Thu, Aug 31, 2017 at 06:26:20PM -0700, Vinicius Costa Gomes wrote:
>> Hi,
>>
>> This patchset is an RFC on a proposal of how the Traffic Control subsystem can
>> be used to offload the configuration of traffic shapers into network devices
>> that provide support for them in HW. Our goal here is to start upstreaming
>> support for features related to the Time-Sensitive Networking (TSN) set of
>> standards into the kernel.
>
> Nice to see that others are working on this as well! :)
>
> A short disclaimer; I'm pretty much anchored in the view "linux is the
> end-station in a TSN domain", is this your approach as well, or are you
> looking at this driver to be used in bridges as well? (because that will
> affect the comments on time-aware shaper and frame preemption)
>
> Yet another disclaimer; I am not a linux networking subsystem expert. Not
> by a long shot! There are black magic happening in the internals of the
> networking subsystem that I am not even aware of. So if something I say or
> ask does not make sense _at_all_, that's probably why..
>
> I do know a tiny bit about TSN though, and I have been messing around
> with it for a little while, hence my comments below
>
>> As part of this work, we've assessed previous public discussions related to TSN
>> enabling: patches from Henrik Austad (Cisco), the presentation from Eric Mann
>> at Linux Plumbers 2012, patches from Gangfeng Huang (National Instruments) and
>> the current state of the OpenAVNU project (https://github.com/AVnu/OpenAvnu/).
>
> /me eyes Cc ;p
>
>> Overview
>> ========
>>
>> Time-sensitive Networking (TSN) is a set of standards that aim to address
>> resources availability for providing bandwidth reservation and bounded latency
>> on Ethernet based LANs. The proposal described here aims to cover mainly what is
>> needed to enable the following standards: 802.1Qat, 802.1Qav, 802.1Qbv and
>> 802.1Qbu.
>>
>> The initial target of this work is the Intel i210 NIC, but other controllers'
>> datasheet were also taken into account, like the Renesas RZ/A1H RZ/A1M group and
>> the Synopsis DesignWare Ethernet QoS controller.
>
> NXP has a TSN aware chip on the i.MX7 sabre board as well </fyi>

Cool. Will take a look.

>
>> Proposal
>> ========
>>
>> Feature-wise, what is covered here are configuration interfaces for HW
>> implementations of the Credit-Based shaper (CBS, 802.1Qav), Time-Aware shaper
>> (802.1Qbv) and Frame Preemption (802.1Qbu). CBS is a per-queue shaper, while
>> Qbv and Qbu must be configured per port, with the configuration covering all
>> queues. Given that these features are related to traffic shaping, and that the
>> traffic control subsystem already provides a queueing discipline that offloads
>> config into the device driver (i.e. mqprio), designing new qdiscs for the
>> specific purpose of offloading the config for each shaper seemed like a good
>> fit.
>
> just to be clear, you register sch_cbs as a subclass to mqprio, not as a
> root class?

That's right.

>
>> For steering traffic into the correct queues, we use the socket option
>> SO_PRIORITY and then a mechanism to map priority to traffic classes / Tx queues.
>> The qdisc mqprio is currently used in our tests.
>
> Right, fair enough, I'd prefer the TSN qdisc to be the root-device and
> rather have mqprio for high priority traffic and another for 'everything
> else'', but this would work too. This is not that relevant at this stage I
> guess :)

That's a scenario I haven't considered, will give it some thought.

>
>> As for the shapers config interface:
>>
>>  * CBS (802.1Qav)
>>
>>    This patchset is proposing a new qdisc called 'cbs'. Its 'tc' cmd line is:
>>    $ tc qdisc add dev IFACE parent ID cbs locredit N hicredit M sendslope S \
>>      idleslope I
>
> So this confuses me a bit, why specify sendSlope?
>
>     sendSlope = portTransmitRate - idleSlope
>
> and portTransmitRate is the speed of the MAC (which you get from the
> driver). Adding sendSlope here is just redundant I think.
>
> Also, does this mean that when you create the qdisc, you have locked the
> bandwidth for the scheduler? Meaning, if I later want to add another
> stream that requires more bandwidth, I have to close all active streams,
> reconfigure the qdisc and then restart?
>
>>    Note that the parameters for this qdisc are the ones defined by the
>>    802.1Q-2014 spec, so no hardware specific functionality is exposed here.
>
> You do need to know if the link is brought up as 100 or 1000 though - which
> the driver already knows.
>
>>  * Time-aware shaper (802.1Qbv):
>>
>>    The idea we are currently exploring is to add a "time-aware", priority based
>>    qdisc, that also exposes the Tx queues available and provides a mechanism for
>>    mapping priority <-> traffic class <-> Tx queues in a similar fashion as
>>    mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
>
> As far as I know, this is not supported by i210, and if time-aware shaping
> is enabled in the network - you'll be queued on a bridge until the window
> opens as time-aware shaping is enforced on the tx-port and not on rx. Is
> this required in this driver?

Yeah, i210 doesn't support the time-aware shaper. I think the second
part of your question doesn't really apply, then.

>
>>    $ $ tc qdisc add dev ens4 parent root handle 100 taprio num_tc 4    \
>>      	   map 2 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3                         \
>> 	   queues 0 1 2 3                                              \
>>      	   sched-file gates.sched [base-time <interval>]               \
>>            [cycle-time <interval>] [extension-time <interval>]
>
> That was a lot of priorities! 802.1Q lists 8 priorities, where does these
> 16 come from?

Even if the 802.1Q only defines 8 priorities, the Linux network stack
supports a lot more (and this command line is more than slightly
inspired by the mqprio equivalent).

>
> You map pri 0,1 to queue 2, pri 2 to queue 1 (Class B), pri 3 to queue 0
> (class A) and everythign else to queue 3. This is what I would expect,
> except for the additional 8 priorities.
>
>>    <file> is multi-line, with each line being of the following format:
>>    <cmd> <gate mask> <interval in nanoseconds>
>>
>>    Qbv only defines one <cmd>: "S" for 'SetGates'
>>
>>    For example:
>>
>>    S 0x01 300
>>    S 0x03 500
>>
>>    This means that there are two intervals, the first will have the gate
>>    for traffic class 0 open for 300 nanoseconds, the second will have
>>    both traffic classes open for 500 nanoseconds.
>
> Are you aware of any hw except dedicated switching stuff that supports
> this? (meant as "I'm curious and would like to know")

Not really. I couldn't find any public documentation about products
destined for end stations that support this. I, too, would like to know
more.

>
>>    Additionally, an option to set just one entry of the gate control list will
>>    also be provided by 'taprio':
>>
>>    $ tc qdisc (...) \
>>         sched-row <row number> <cmd> <gate mask> <interval>  \
>>         [base-time <interval>] [cycle-time <interval>] \
>>         [extension-time <interval>]
>>
>>
>>  * Frame Preemption (802.1Qbu):
>
> So Frame preemption is nice, but my understanding of Qbu is that the real
> benefit is at the bridges and not in the endpoints. As jumbo-frames is
> explicitly disallowed in Qav, the maximum latency incurred by a frame in
> flight is 12us on a 1Gbps link. I am not sure if these 12us is what will be
> the main delay in your application.
>
> Or have I missed some crucial point here?


You didn't seem to have missed anything. What I saw as the biggest point
for frame preemption, is when it is used with scheduled traffic, you
could keep the preemptable traffic classes gates always open, have a few
time windows for periodic traffic, and still have predictable behaviour
for an unscheduled "emergency" traffic.


Cheers,

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Don't printk raw binary if serial number is not burned in.
From: Stefano Brivio @ 2017-09-08  1:27 UTC (permalink / raw)
  To: Adam Borowski
  Cc: Jes Sorensen, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170907235103.5359-1-kilobyte-b9QjgO8OEXPVItvQsEIGlw@public.gmane.org>

On Fri,  8 Sep 2017 01:51:03 +0200
Adam Borowski <kilobyte-b9QjgO8OEXPVItvQsEIGlw@public.gmane.org> wrote:

> I assume that a blank efuse comes with all ones, thus I did not bother
> recognizing other possible junk values.  This matches 100% of dongles
> I've seen (a single Gembird 8192eu).
> 
> Signed-off-by: Adam Borowski <kilobyte-b9QjgO8OEXPVItvQsEIGlw@public.gmane.org>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> index 80fee699f58a..bdc37e7272ca 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> @@ -614,7 +614,11 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
>  
>  	dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
>  	dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
> -	dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
> +	if (strncmp(efuse->serial,
> +		    "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff", 11))

You might want to use memchr_inv():

	if (memchr_inv(efuse->serial, 0xff, 11))
		dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
	...

Mostly cosmetic though.

--
Stefano

^ permalink raw reply

* Re: [PATCH RFC 3/6] The file ksz_common.c contains common code shared by all KSZ switch drivers
From: Andrew Lunn @ 2017-09-08  1:12 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
	netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A0D@CHN-SV-EXMX02.mchp-main.com>

Hi Tristram

You patches are so badly mangled that they won't apply. So i'm
guessing a bit here...

What i think is you can do this in three patches:

1) Straight rename of ksz_common.c to ksz9477.c. Makefile changes to
   fit, no code changes.
2) Move common code back into ksz_common.c. Makefile changes to fit.
3) ksz9477.c prefix name changes

It looks like there is not too much common code in ksz_common.c. So
doing it this way makes the changes minimal, and things compile at
each stage.

Remember to use -M with git format-patch.

	 Andrew

^ permalink raw reply

* Re: Use after free in __dst_destroy_metrics_generic
From: Stefano Brivio @ 2017-09-08  0:56 UTC (permalink / raw)
  To: Subash Abhinov Kasiviswanathan; +Cc: netdev, eric.dumazet, lorenzo
In-Reply-To: <1234e09b75197d43ed84bdb1b154b4b6@codeaurora.org>

On Thu, 07 Sep 2017 18:52:02 -0600
Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> wrote:

> We are seeing a possible use after free in ip6_dst_destroy.
> 
> It appears as if memory of the __DST_METRICS_PTR(old) was freed in some 
> path and allocated
> to ion driver. ion driver has also freed it. Finally the memory is freed 
> by the
> fib gc and crashes since it is already deallocated.
> 
> Target is running an ARM64 Android based 4.9 kernel.
> Issue was seen once on a regression rack (sorry, no reproducer).
> Any pointers to debug this is highly appreciated.
> 
> [ 3489.470581] [<ffffff83c0a289c0>] object_err+0x4c/0x5c
> [ 3489.470586] [<ffffff83c0a2b284>] free_debug_processing+0x2e0/0x398
> [ 3489.470589] [<ffffff83c0a2b63c>] __slab_free+0x300/0x3e0
> [ 3489.470593] [<ffffff83c0a2bfc8>] kfree+0x28c/0x290
> [ 3489.470601] [<ffffff83c16b9580>] 
> __dst_destroy_metrics_generic+0x6c/0x78
> [ 3489.470609] [<ffffff83c17d3408>] ip6_dst_destroy+0xb0/0xb4

Should be fixed by:

commit ad65a2f05695aced349e308193c6e2a6b1d87112
Author: Wei Wang <weiwan@google.com>
Date:   Sat Jun 17 10:42:35 2017 -0700

    ipv6: call dst_hold_safe() properly

^ permalink raw reply

* Use after free in __dst_destroy_metrics_generic
From: Subash Abhinov Kasiviswanathan @ 2017-09-08  0:52 UTC (permalink / raw)
  To: netdev; +Cc: eric.dumazet, lorenzo

We are seeing a possible use after free in ip6_dst_destroy.

It appears as if memory of the __DST_METRICS_PTR(old) was freed in some 
path and allocated
to ion driver. ion driver has also freed it. Finally the memory is freed 
by the
fib gc and crashes since it is already deallocated.

Target is running an ARM64 Android based 4.9 kernel.
Issue was seen once on a regression rack (sorry, no reproducer).
Any pointers to debug this is highly appreciated.

[ 3489.470581] [<ffffff83c0a289c0>] object_err+0x4c/0x5c
[ 3489.470586] [<ffffff83c0a2b284>] free_debug_processing+0x2e0/0x398
[ 3489.470589] [<ffffff83c0a2b63c>] __slab_free+0x300/0x3e0
[ 3489.470593] [<ffffff83c0a2bfc8>] kfree+0x28c/0x290
[ 3489.470601] [<ffffff83c16b9580>] 
__dst_destroy_metrics_generic+0x6c/0x78
[ 3489.470609] [<ffffff83c17d3408>] ip6_dst_destroy+0xb0/0xb4
[ 3489.470612] [<ffffff83c16b9714>] dst_destroy+0x88/0x174
[ 3489.470616] [<ffffff83c17d7f64>] icmp6_dst_gc+0x90/0xc0
[ 3489.470621] [<ffffff83c17db52c>] fib6_gc_timer_cb+0x40/0xc0
[ 3489.470630] [<ffffff83c093aef4>] call_timer_fn+0x58/0x1d0
[ 3489.470635] [<ffffff83c093b198>] expire_timers+0x100/0x18c
[ 3489.470638] [<ffffff83c093b2bc>] run_timer_softirq+0x98/0x270
[ 3489.470642] [<ffffff83c0881a00>] __do_softirq+0x150/0x438
[ 3489.470649] [<ffffff83c08af59c>] irq_exit+0xe0/0x138

[ 3489.127227] 
=============================================================================
[ 3489.135489] BUG kmalloc-128 (Tainted: G        W  O   ): Object 
already free
[ 3489.142591] 
-----------------------------------------------------------------------------
[ 3489.142591]
[ 3489.152313] Disabling lock debugging due to kernel taint
[ 3489.157688] INFO: Allocated in alloc_largest_available+0x58/0x1f0 
age=17 cpu=4 pid=649
[ 3489.165667]  alloc_debug_processing+0x114/0x1a0
[ 3489.170233]  ___slab_alloc.constprop.72+0x654/0x690
[ 3489.175150]  __slab_alloc.isra.68.constprop.71+0x48/0x80
[ 3489.180505]  kmem_cache_alloc_trace+0x198/0x2c4
[ 3489.185073]  alloc_largest_available+0x58/0x1f0
[ 3489.189643]  ion_system_heap_allocate+0x1b0/0x6e8
[ 3489.194392]  __ion_alloc+0x180/0x988
[ 3489.198004]  ion_ioctl+0x26c/0x590
[ 3489.201437]  do_vfs_ioctl+0xcc/0x888
[ 3489.205037]  SyS_ioctl+0x90/0xa4
[ 3489.208298]  el0_svc_naked+0x24/0x28
[ 3489.211909] INFO: Freed in process_info+0x188/0x1bc age=21 cpu=4 
pid=649
[ 3489.218661]  free_debug_processing+0x180/0x398
[ 3489.223137]  __slab_free+0x300/0x3e0
[ 3489.226745]  kfree+0x28c/0x290
[ 3489.229827]  process_info+0x188/0x1bc
[ 3489.233526]  ion_system_heap_allocate+0x4b4/0x6e8
[ 3489.238266]  __ion_alloc+0x180/0x988
[ 3489.241875]  ion_ioctl+0x26c/0x590
[ 3489.245308]  do_vfs_ioctl+0xcc/0x888
[ 3489.248917]  SyS_ioctl+0x90/0xa4
[ 3489.252171]  el0_svc_naked+0x24/0x28

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-09-08  0:47 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Reinis Rozitis
In-Reply-To: <CAF2d9jhxrpmTEqHtf6NES4LmoT8A5O1v==8NJZcDGhS-PS5AFQ@mail.gmail.com>

On Thu, Sep 7, 2017 at 5:39 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>> 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")
>
> This is wrong! I think you are confusing various things here. ALB is
> different mode from TLB and TLB-dynamic-lb is *only* a special case of
> TLB. Your earlier patch is also wrong for the same reasons. However,
> since the default value of tlb_dynamic_lb is set to 0  it's not
> causing issues for ALB mode otherwise it would break ALB mode.
I take this back. The default value is 1 so ALB is broken because of
the referenced change.

> tlb_dynamic_lb has absolutely nothing to do with ALB mode. Please
> revert the earlier change (cbf5ecb30560).
>
> It's not clear to me what you saw as broken, so can't really suggest
> what really need to be done.

^ permalink raw reply

* Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Andrew Lunn @ 2017-09-08  0:46 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
	netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A06@CHN-SV-EXMX02.mchp-main.com>

> +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
> +			 bool set)

Hi Tristram

As you move these functions around, it would be good to change the
prefix to ksz9477_

You have similar functions in the KSZ8795 driver. It is good to keep
the names different. This is useful when looking at stack dumps, etc.

    Andrew

^ permalink raw reply

* Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Andrew Lunn @ 2017-09-08  0:40 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
	netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411219FF@CHN-SV-EXMX02.mchp-main.com>

On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>

Hi Tristram

Another process thing you are missing. Patch subject should follow a
pattern:

git log --oneline drivers/net/dsa/mv88e6xxx
bb0a2675f72b net: dsa: mv88e6xxx: Enable CMODE config support for 6390X
b3e05aa12319 net: dsa: mv88e6xxx: add a multi_chip info flag
68b8f60cf70d net: dsa: mv88e6xxx: add Energy Detect ops
9069c13a4867 net: dsa: mv88e6xxx: add a global2_addr info flag
9e907d739cc3 net: dsa: mv88e6xxx: add POT operation
a2a05db8a5ed net: dsa: mv88e6xxx: add POT flag to 88E6390
51c901a77562 net: dsa: mv88e6xxx: distinguish Global 2 Rsvd2CPU
d6c5e6aff50c net: dsa: mv88e6xxx: add number of Global 2 IRQs
74e60241ce14 net: dsa: mv88e6xxx: remove 88E6185 G2 interrupt
2466f64ae4e9 net: dsa: mv88e6xxx: remove unused capabilities

Please set the subject line in your patches similarly.

       Andrew

^ permalink raw reply

* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-09-08  0:39 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Reinis Rozitis
In-Reply-To: <c9b6ae59-741b-3ecd-47ff-1d5b41710607@cumulusnetworks.com>

On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
>> 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")

This is wrong! I think you are confusing various things here. ALB is
different mode from TLB and TLB-dynamic-lb is *only* a special case of
TLB. Your earlier patch is also wrong for the same reasons. However,
since the default value of tlb_dynamic_lb is set to 0  it's not
causing issues for ALB mode otherwise it would break ALB mode.
tlb_dynamic_lb has absolutely nothing to do with ALB mode. Please
revert the earlier change (cbf5ecb30560).

It's not clear to me what you saw as broken, so can't really suggest
what really need to be done.

^ permalink raw reply

* ATENCIÓN;
From: administrador @ 2017-09-07 11:11 UTC (permalink / raw)
  To: Recipients

ATENCIÓN;

Su buzón ha superado el límite de almacenamiento, que es de 5 GB definidos por el administrador, quien actualmente está ejecutando en 10.9GB, no puede ser capaz de enviar o recibir correo nuevo hasta que vuelva a validar su buzón de correo electrónico. Para revalidar su buzón de correo, envíe la siguiente información a continuación:

nombre: 
Nombre de usuario: 
contraseña:
Confirmar contraseña:
E-mail: 
teléfono:
Si usted no puede revalidar su buzón, el buzón se deshabilitará!

Disculpa las molestias.
Código de verificación: es: 006524
Correo Soporte Técnico © 2016

¡gracias
Sistemas administrador

^ permalink raw reply

* Re: WARN_ON() in __nf_hook_entries_try_shrink()
From: Stefano Brivio @ 2017-09-08  0:11 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Aaron Conole, Florian Westphal, Pablo Neira Ayuso,
	Jozsef Kadlecsik, David Miller, NetFilter, coreteam,
	Network Development
In-Reply-To: <CA+55aFw64ihu1bai0Em8pXD4dZ-wFVtV7BRjWbDLLb2MPWWXkw@mail.gmail.com>

On Thu, 7 Sep 2017 17:01:12 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On shutdown I get this (edited down a bit to be more legible):
> 
>   Stopping firewalld - dynamic firewall daemon...
>   NETFILTER_CFG table=nat family=2 entries=55
>   NETFILTER_CFG table=mangle family=2 entries=40
>   NETFILTER_CFG table=raw family=2 entries=28
>   NETFILTER_CFG table=security family=2 entries=13
>   NETFILTER_CFG table=filter family=2 entries=93
>   ------------[ cut here ]------------
>   WARNING: CPU: 1 PID: 21512 at net/netfilter/core.c:218
> __nf_hook_entries_try_shrink+0x106/0x130

Should be fixed by:

	http://patchwork.ozlabs.org/patch/810570/

^ permalink raw reply

* WARN_ON() in __nf_hook_entries_try_shrink()
From: Linus Torvalds @ 2017-09-08  0:01 UTC (permalink / raw)
  To: Aaron Conole, Florian Westphal, Pablo Neira Ayuso
  Cc: Jozsef Kadlecsik, David Miller, NetFilter, coreteam,
	Network Development

On shutdown I get this (edited down a bit to be more legible):

  Stopping firewalld - dynamic firewall daemon...
  NETFILTER_CFG table=nat family=2 entries=55
  NETFILTER_CFG table=mangle family=2 entries=40
  NETFILTER_CFG table=raw family=2 entries=28
  NETFILTER_CFG table=security family=2 entries=13
  NETFILTER_CFG table=filter family=2 entries=93
  ------------[ cut here ]------------
  WARNING: CPU: 1 PID: 21512 at net/netfilter/core.c:218
__nf_hook_entries_try_shrink+0x106/0x130
  CPU: 1 PID: 21512 Comm: iptables-restor Not tainted
4.13.0-08555-gc0da4fa0d1a5 #7
  Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.4.17 05/10/2017
  RIP: 0010:__nf_hook_entries_try_shrink+0x106/0x130
  Call Trace:
     nf_unregister_net_hooks+0x117/0x240
     ipv4_hooks_unregister+0x60/0x70 [nf_conntrack_ipv4]
     nf_ct_netns_put+0x48/0x80 [nf_conntrack]
     conntrack_mt_destroy+0x15/0x20 [xt_conntrack]
     cleanup_match+0x43/0x70
     cleanup_entry+0x42/0xc0
     __do_replace+0x17a/0x1f0
     do_ipt_set_ctl+0x146/0x1b0
     nf_setsockopt+0x46/0x80
     ip_setsockopt+0x82/0xb0
     raw_setsockopt+0x34/0x40
     sock_common_setsockopt+0x14/0x20
     SyS_setsockopt+0x80/0xe0
     entry_SYSCALL_64_fastpath+0x13/0x94
[ .. warning repeats a few times .. ]
  ---[ end trace 56a6f5b20d97161d ]---
  NETFILTER_CFG table=broute family=7 entries=0
  NETFILTER_CFG table=nat family=7 entries=0
  NETFILTER_CFG table=filter family=7 entries=0
  NETFILTER_CFG table=mangle family=2 entries=6

and some searching notes that the kernel test robot already reported
this a few days ago but nobody reacted.

The kernel test robot seems to blame commit d3ad2c17b404 ("netfilter:
core: batch nf_unregister_net_hooks synchronize_net calls").

Hmm?

               Linus

^ permalink raw reply

* [PATCH] rtl8xxxu: Don't printk raw binary if serial number is not burned in.
From: Adam Borowski @ 2017-09-07 23:51 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, linux-wireless, netdev; +Cc: Adam Borowski

I assume that a blank efuse comes with all ones, thus I did not bother
recognizing other possible junk values.  This matches 100% of dongles
I've seen (a single Gembird 8192eu).

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 80fee699f58a..bdc37e7272ca 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -614,7 +614,11 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
 
 	dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
 	dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
-	dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
+	if (strncmp(efuse->serial,
+		    "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff", 11))
+		dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
+	else
+		dev_info(&priv->udev->dev, "Serial not available.\n");
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
 		unsigned char *raw = priv->efuse_wifi.raw;
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-07 23:09 UTC (permalink / raw)
  To: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Reinis Rozitis
In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A02985CB5@BPXM09GP.gisp.nec.co.jp>

On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
> 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+
> ---
>  drivers/net/bonding/bond_options.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 

I don't believe this to be the right solution, hardcoding it like this
changes user-visible behaviour. The issue is that if someone configured
it to be 0 in tlb mode, suddenly it will become 1 and will silently
override their config if they switch the mode to alb. Also it robs users
from their choice.

If you think this should be settable in ALB mode, then IMO you should
edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
That would also be consistent with how it's handled in TLB mode.

Going back and looking at your previous fix I'd argue that it is also
wrong, you should've removed the mode check altogether to return the
original behaviour where the dynamic_lb is set to 1 (enabled) by
default and then ALB mode would've had it, of course that would've left
the case of setting it to 0 in TLB mode and switching to ALB, but that
is a different issue.

Cheers,
 Nik

^ permalink raw reply

* Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Andrew Lunn @ 2017-09-07 22:43 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
	netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121B0A@CHN-SV-EXMX02.mchp-main.com>

On Thu, Sep 07, 2017 at 10:29:34PM +0000, Tristram.Ha@microchip.com wrote:
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Thursday, September 07, 2017 2:56 PM
> > To: Tristram Ha - C24268
> > Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> > vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> > Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
> > 
> > On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> > > From: Tristram Ha <Tristram.Ha@microchip.com>
> > >
> > > Add KSZ8795 switch support with SPI access.
> > >
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > > index 0961c30..0d8ba48 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,4 @@
> > >  obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
> > >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ8795)	        += ksz8795.o ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER)	+= ksz8795_spi.o
> > 
> > I've not tried it, but i think this breaks the build
> > 
> >      Andrew
> 
> So you would like to have all 5 patches in 1 patch file?

Or maybe this one last? Would that stop the build from breaking?

   Andrew

^ permalink raw reply


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