Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 9/9] net: phy: mdio-mux-bcm-iproc: simplify getting .driver_data
From: Florian Fainelli @ 2018-10-22 17:02 UTC (permalink / raw)
  To: Wolfram Sang, linux-kernel
  Cc: linux-renesas-soc, Andrew Lunn, David S. Miller, Ray Jui,
	Scott Branden, Jon Mason, bcm-kernel-feedback-list, netdev,
	linux-arm-kernel
In-Reply-To: <20181021200021.1693-10-wsa+renesas@sang-engineering.com>

On 10/21/18 1:00 PM, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH 2/9] net: dsa: qca8k: simplify getting .driver_data
From: Florian Fainelli @ 2018-10-22 17:01 UTC (permalink / raw)
  To: Wolfram Sang, linux-kernel
  Cc: linux-renesas-soc, Andrew Lunn, Vivien Didelot, David S. Miller,
	netdev
In-Reply-To: <20181021200021.1693-3-wsa+renesas@sang-engineering.com>

On 10/21/18 1:00 PM, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH 1/1] net: dsa: legacy: simplify getting .driver_data
From: Florian Fainelli @ 2018-10-22 17:01 UTC (permalink / raw)
  To: Wolfram Sang, linux-kernel
  Cc: linux-renesas-soc, Andrew Lunn, Vivien Didelot, David S. Miller,
	netdev
In-Reply-To: <20181021200105.2414-2-wsa+renesas@sang-engineering.com>

On 10/21/18 1:01 PM, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: CRC errors between mvneta and macb
From: Willy Tarreau @ 2018-10-22 16:34 UTC (permalink / raw)
  To: Richard Genoud
  Cc: linux-kernel, Thomas Petazzoni, Antoine Tenart, Gregory CLEMENT,
	Yelena Krivosheev, Maxime Chevallier, Nicolas Ferre, netdev
In-Reply-To: <31385a3b-2196-94d0-7f4f-52e37ee07a28@sorico.fr>

On Mon, Oct 22, 2018 at 05:15:21PM +0200, Richard Genoud wrote:
> After analyzing the ethernet frame on the Davicom PHY's output (pin
> TX+), I find out that the FCS errors occurs when the ethernet preamble
> is longer than 56bits. (something like 58 or 60 bits)
> 
> To say this in another way, instead of having 28 times 1-0 followed by
> the SFD (10101011), I see 29 or 30 times 1-0 followed by the SFD.
> (sometimes 29, sometimes 30)
> 
> 
> Should a longer preamble be considered as an FCS error ? It seems a
> little harsh since the point of the preamble is to synchronize the frame.

That indeed seems a bit strange considering that you're not supposed to
know what is before the preamble so it would very well contain random
noise looking a lot like alteranted bits.

> I don't know what the 802.3 standard says about that.

Just found it :-)

     https://www.trincoll.edu/Academics/MajorsAndMinors/Engineering/Documents/IEEE%20Standard%20for%20Ethernet.pdf

Page 132, #7.2.3.2 :

    The DTE is required to supply at least 56 bits of preamble in
    order to satisfy system requirements. System components consume
    preamble bits in order to perform their functions. The number
    of preamble bits sourced ensures an adequate number of bits are
    provided to each system component to correctly implement its
    function.

So that totally makes sense since the purpose is to enable signal
detection at the hardware leve, hence the problem definitely is on
the receiver in your case.

Willy

^ permalink raw reply

* [PATCHv3 iproute2-next] ip/geneve: fix ttl inherit behavior
From: Hangbin Liu @ 2018-10-22  7:46 UTC (permalink / raw)
  To: netdev
  Cc: Stephen Hemminger, David Ahern, Phil Sutter, Michal Kubecek,
	Hangbin Liu
In-Reply-To: <1539846110-11607-1-git-send-email-liuhangbin@gmail.com>

Currently when we add geneve with "ttl inherit", we only set ttl to 0, which
is actually use whatever default value instead of inherit the inner protocol's
ttl value.

To make a difference with ttl inherit and ttl == 0, we add an attribute
IFLA_GENEVE_TTL_INHERIT in kernel commit 52d0d404d39dd ("geneve: add ttl
inherit support"). Now let's use "ttl inherit" to inherit the inner
protocol's ttl, and use "ttl auto" to means "use whatever default value",
the same behavior with ttl == 0.

v2:
1) remove IFLA_GENEVE_TTL_INHERIT defination in if_link.h as it's already
   updated.
2) Still use addattr8() so we can enable/disable ttl inherit, as Michal
   suggested.

v3: Update man page

Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 ip/iplink_geneve.c    | 20 +++++++++++++-------
 man/man8/ip-link.8.in |  4 +++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index 26e70ff..c417842 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -34,7 +34,7 @@ static void print_explain(FILE *f)
 		"Where: VNI   := 0-16777215\n"
 		"       ADDR  := IP_ADDRESS\n"
 		"       TOS   := { NUMBER | inherit }\n"
-		"       TTL   := { 1..255 | inherit }\n"
+		"       TTL   := { 1..255 | auto | inherit }\n"
 		"       LABEL := 0-1048575\n"
 	);
 }
@@ -94,7 +94,9 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_GENEVE_TTL, "ttl", *argv);
-			if (strcmp(*argv, "inherit") != 0) {
+			if (strcmp(*argv, "inherit") == 0) {
+				addattr8(n, 1024, IFLA_GENEVE_TTL_INHERIT, 1);
+			} else if (strcmp(*argv, "auto") != 0) {
 				if (get_unsigned(&uval, *argv, 0))
 					invarg("invalid TTL", *argv);
 				if (uval > 255)
@@ -265,12 +267,16 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 	}
 
-	if (tb[IFLA_GENEVE_TTL])
-		ttl = rta_getattr_u8(tb[IFLA_GENEVE_TTL]);
-	if (is_json_context() || ttl)
-		print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
-	else
+	if (tb[IFLA_GENEVE_TTL_INHERIT] &&
+	    rta_getattr_u8(tb[IFLA_GENEVE_TTL_INHERIT])) {
 		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
+	} else if (tb[IFLA_GENEVE_TTL]) {
+		ttl = rta_getattr_u8(tb[IFLA_GENEVE_TTL]);
+		if (is_json_context() || ttl)
+			print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
+		else
+			print_string(PRINT_FP, NULL, "ttl %s ", "auto");
+	}
 
 	if (tb[IFLA_GENEVE_TOS])
 		tos = rta_getattr_u8(tb[IFLA_GENEVE_TOS]);
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index ecbbd4f..4489162 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1190,7 +1190,9 @@ the following additional arguments are supported:
 
 .sp
 .BI ttl " TTL"
-- specifies the TTL value to use in outgoing packets.
+- specifies the TTL value to use in outgoing packets. "0" or "auto" means
+use whatever default value, "inherit" means inherit the inner protocol's
+ttl. Default option is "0".
 
 .sp
 .BI tos " TOS"
-- 
2.5.5

^ permalink raw reply related

* [PATCH] rtlwifi: btcoex: remove set but not used variable 'ppsc'
From: YueHaibing @ 2018-10-22  7:51 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo, Larry Finger, Colin Ian King,
	Nathan Chancellor
  Cc: YueHaibing, linux-wireless, netdev, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c: In function 'halbtc_leave_lps':
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:295:21: warning:
 variable 'ppsc' set but not used [-Wunused-but-set-variable]

drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c: In function 'halbtc_enter_lps':
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:318:21: warning:
 variable 'ppsc' set but not used [-Wunused-but-set-variable]

It never used since introduction in
commit aa45a673b291 ("rtlwifi: btcoexist: Add new mini driver")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 6fbf884..d748aab 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -292,11 +292,9 @@ bool halbtc_send_bt_mp_operation(struct btc_coexist *btcoexist, u8 op_code,
 static void halbtc_leave_lps(struct btc_coexist *btcoexist)
 {
 	struct rtl_priv *rtlpriv;
-	struct rtl_ps_ctl *ppsc;
 	bool ap_enable = false;
 
 	rtlpriv = btcoexist->adapter;
-	ppsc = rtl_psc(rtlpriv);
 
 	btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_AP_MODE_ENABLE,
 			   &ap_enable);
@@ -315,11 +313,9 @@ static void halbtc_leave_lps(struct btc_coexist *btcoexist)
 static void halbtc_enter_lps(struct btc_coexist *btcoexist)
 {
 	struct rtl_priv *rtlpriv;
-	struct rtl_ps_ctl *ppsc;
 	bool ap_enable = false;
 
 	rtlpriv = btcoexist->adapter;
-	ppsc = rtl_psc(rtlpriv);
 
 	btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_AP_MODE_ENABLE,
 			   &ap_enable);

^ permalink raw reply related

* Re: [PATCH] bpf: btf: Fix a missing-check bug
From: Y Song @ 2018-10-22 15:57 UTC (permalink / raw)
  To: wang6495; +Cc: kjlu, Alexei Starovoitov, Daniel Borkmann, netdev, LKML
In-Reply-To: <1539988191-13973-1-git-send-email-wang6495@umn.edu>

On Fri, Oct 19, 2018 at 3:30 PM Wenwen Wang <wang6495@umn.edu> wrote:
>
> In btf_parse(), the header of the user-space btf data 'btf_data' is firstly
> parsed and verified through btf_parse_hdr(). In btf_parse_hdr(), the header
> is copied from user-space 'btf_data' to kernel-space 'btf->hdr' and then
> verified. If no error happens during the verification process, the whole
> data of 'btf_data', including the header, is then copied to 'data' in
> btf_parse(). It is obvious that the header is copied twice here. More
> importantly, no check is enforced after the second copy to make sure the
> headers obtained in these two copies are same. Given that 'btf_data'
> resides in the user space, a malicious user can race to modify the header
> between these two copies. By doing so, the user can inject inconsistent
> data, which can cause undefined behavior of the kernel and introduce
> potential security risk.
>
> To avoid the above issue, this patch rewrites the header after the second
> copy, using 'btf->hdr', which is obtained in the first copy.
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
> ---
>  kernel/bpf/btf.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 138f030..2a85f91 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2202,6 +2202,9 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>                 goto errout;
>         }
>
> +       memcpy(data, &btf->hdr,
> +               min_t(u32, btf->hdr.hdr_len, sizeof(btf->hdr)));

Could you restructure the code to memcpy the header followed by copying
the rest of btf_data with copy_from_user? This way, each byte is only
copied once.
Could you add some comments right before memcpy so later people will know
why we implement this way?

> +
>         err = btf_parse_str_sec(env);
>         if (err)
>                 goto errout;
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH] bpf: btf: Fix a missing-check bug
From: Martin Lau @ 2018-10-22 15:40 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Kangjie Lu, Alexei Starovoitov, Daniel Borkmann,
	open list:BPF (Safe dynamic programs and tools),
	open list:BPF (Safe dynamic programs and tools)
In-Reply-To: <1539988191-13973-1-git-send-email-wang6495@umn.edu>

On Fri, Oct 19, 2018 at 05:29:51PM -0500, Wenwen Wang wrote:
> In btf_parse(), the header of the user-space btf data 'btf_data' is firstly
> parsed and verified through btf_parse_hdr(). In btf_parse_hdr(), the header
> is copied from user-space 'btf_data' to kernel-space 'btf->hdr' and then
> verified. If no error happens during the verification process, the whole
> data of 'btf_data', including the header, is then copied to 'data' in
> btf_parse(). It is obvious that the header is copied twice here. More
> importantly, no check is enforced after the second copy to make sure the
> headers obtained in these two copies are same. Given that 'btf_data'
> resides in the user space, a malicious user can race to modify the header
> between these two copies. By doing so, the user can inject inconsistent
> data, which can cause undefined behavior of the kernel and introduce
> potential security risk.
> 
> To avoid the above issue, this patch rewrites the header after the second
> copy, using 'btf->hdr', which is obtained in the first copy.
Acked-by: Martin KaFai Lau <kafai@fb.com>

^ permalink raw reply

* Re: [PATCH] net/mlx5: allocate enough space in
From: Dan Carpenter @ 2018-10-22  6:41 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Saeed Mahameed, Paul Blakey, kernel-janitors, Linux Netdev List,
	Leon Romanovsky
In-Reply-To: <CAJ3xEMj5NPo-BJ2t_xno+R5O6R0_i3CU0Dig3_Wakop0xLa8CQ@mail.gmail.com>

On Mon, Oct 22, 2018 at 09:18:43AM +0300, Or Gerlitz wrote:
> On Mon, Oct 22, 2018 at 8:23 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > On Sun, Oct 21, 2018 at 01:56:26PM +0300, Or Gerlitz wrote:
> > > I will re-post your patch, this time to netdev since the original
> > > commit is there
> > > and so should be the fix, thanks for reporting/fixing!
> >
> > I didn't realize it had been posted to netdev already so I deliberately
> > left that off the CC.  If Dave hasn't applied the original (he probably
> 
> you didn't post it to netdev, I don't see how he could have got it
> 

I meant commit 328edb499f99 ("net/mlx5: Split FDB fast path prio to
multiple namespaces").  It turns out that Dave has applied that, but I
was expecting to see his S-o-B on it.

It's hard for me to know which tree patches are applied to.

Also I see that I screwed up the subject.  Thanks for fixing that.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] net/mlx5: Allocate enough space for the FDB sub-namespaces
From: Or Gerlitz @ 2018-10-22  6:19 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: David Miller, Saeed Mahameed, Linux Netdev List, Dan Carpenter
In-Reply-To: <1540119949-1748-1-git-send-email-ogerlitz@mellanox.com>

On Sun, Oct 21, 2018 at 2:47 PM Or Gerlitz <ogerlitz@mellanox.com> wrote:
>
> From: Dan Carpenter <dan.carpenter@oracle.com>
>
> FDB_MAX_CHAIN is three.  We wanted to allocate enough memory to hold four
> structs but there are missing parentheses so we only allocate enough
> memory for three structs and the first byte of the fourth one.
>
> Fixes: 328edb499f99 ("net/mlx5: Split FDB fast path prio to multiple namespaces")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>

Dave, I forgot to mark it as net-next, so here I am saying that..


> ---
>  drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index 67ba4c9..9d73eb9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@ -2470,7 +2470,7 @@ static int init_fdb_root_ns(struct mlx5_flow_steering *steering)
>                 return -ENOMEM;
>
>         steering->fdb_sub_ns = kzalloc(sizeof(steering->fdb_sub_ns) *
> -                                      FDB_MAX_CHAIN + 1, GFP_KERNEL);
> +                                      (FDB_MAX_CHAIN + 1), GFP_KERNEL);
>         if (!steering->fdb_sub_ns)
>                 return -ENOMEM;
>
> --
> 2.3.7
>

^ permalink raw reply

* Re: [PATCH 0/2] Mark expected switch fall-throughs and fix missing break
From: Jes Sorensen @ 2018-10-22 14:36 UTC (permalink / raw)
  To: Gustavo A. R. Silva, linux-kernel
  Cc: Kalle Valo, linux-wireless, David S. Miller, netdev
In-Reply-To: <cover.1540208577.git.gustavo@embeddedor.com>

On 10/22/18 7:49 AM, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, this patchset aims
> to mark multiple switch cases where we are expecting to fall through.
> 
> Also, the second patch in this series fixes a missing break in switch.

Enabling that sounds like a great way to inflict pain and suffering.

Not a big deal to put that in the code, the second patch looks fine though.

Jes

^ permalink raw reply

* Re: [PATCH] net/mlx5: allocate enough space in
From: Or Gerlitz @ 2018-10-22  6:18 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Saeed Mahameed, Paul Blakey, kernel-janitors, Linux Netdev List,
	Leon Romanovsky
In-Reply-To: <20181022052336.fr4kvd7idba5d2om@mwanda>

On Mon, Oct 22, 2018 at 8:23 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Sun, Oct 21, 2018 at 01:56:26PM +0300, Or Gerlitz wrote:
> > I will re-post your patch, this time to netdev since the original
> > commit is there
> > and so should be the fix, thanks for reporting/fixing!
>
> I didn't realize it had been posted to netdev already so I deliberately
> left that off the CC.  If Dave hasn't applied the original (he probably

you didn't post it to netdev, I don't see how he could have got it


> has now because he is so quick) then it's fine by me if you fold them
> together.
>
> regards,
> dan carpenter
>

^ permalink raw reply

* Re: [PATCH] net/mlx5: allocate enough space in
From: Dan Carpenter @ 2018-10-22  5:23 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Saeed Mahameed, Paul Blakey, kernel-janitors, Linux Netdev List,
	Leon Romanovsky
In-Reply-To: <CAJ3xEMjZ8WQu2cj_QgU9RoZvUZu7X2Szh6-Si6KZU-qMBFqwHQ@mail.gmail.com>

On Sun, Oct 21, 2018 at 01:56:26PM +0300, Or Gerlitz wrote:
> I will re-post your patch, this time to netdev since the original
> commit is there
> and so should be the fix, thanks for reporting/fixing!

I didn't realize it had been posted to netdev already so I deliberately
left that off the CC.  If Dave hasn't applied the original (he probably
has now because he is so quick) then it's fine by me if you fold them
together.

regards,
dan carpenter

^ permalink raw reply

* Re: [rtnetlink] Potential bug in Linux (rt)netlink code
From: Henning Rogge @ 2018-10-22  5:22 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <cae45e94-72f5-3aac-8c91-8e71facc3482@fkie.fraunhofer.de>

Does anyone else have an idea how to debug this problem?

Henning Rogge

Am 15.10.2018 um 07:25 schrieb Henning Rogge:
> Am 12.10.2018 um 20:51 schrieb Stephen Hemminger:
>> On Fri, 12 Oct 2018 09:30:40 +0200
>> Henning Rogge <henning.rogge@fkie.fraunhofer.de> wrote:
>>
>>> Hi,
>>>
>>> I am working on a self-written routing agent
>>> (https://github.com/OLSR/OONF) and am stuck on a problem with netlink
>>> that I cannot explain with an userspace error.
>>>
>>> I am using a netlink socket for setting routes
>>> (RTM_NEWROUTE/RTM_DELROUTE), querying the kernel for the current routes
>>> in the database (via a RTM_GETROUTE dump) and for getting multicast
>>> messages for ongoing routing changes.
>>>
>>> After a few netlink messages I get to the point where the kernel just
>>> does not responst to a RTM_NEWROUTE. No error, no answer, despite the
>>> NLM_F_ACK flag set)... but sometime when (during shutdown of the routing
>>> agent) the program sends another route command (most times a
>>> RTM_DELROUTE) I get a single netlink packet with a "successful" response
>>> for both the "missing" RTM_NEWROUTE and one for the new RTM DELROUTE
>>> sequence number.
>>>
>>> I am testing two routing agents, each of them in a systemd-nspawn based
>>> container connected over a bridge on the host system on a current Debian
>>> Testing (kernel 4.18.0-1-amd64).
>>>
>>> I am directly using the netlink sockets, without any other userspace
>>> library in between.
>>>
>>> I have checked the hexdumps of a couple of netlink messages (including
>>> the ones just before the bug happens) by hand and they seem to be okay.
>>>
>>> When I tried to add a "netlink listener" socket for futher debugging (ip
>>> link add nlmon0 type nlmon) the problem vanished until I removed the
>>> listener socket again.
>>>
>>> Any ideas how to debug this problem? Unfortunately I have no short
>>> example program to trigger the bug... I have rarely seen the problem for
>>> years (once every couple of months), but until a few days ago I never
>>> managed to reproduce it.
>>>
>>> Henning Rogge
>>
>> Are you reading the responses to your requests?  If you don't read
>> the response, the socket will get flow blocked.
> 
> Yes, I do...
> 
> all netlink sockets the program uses are constantly watched for traffic 
> coming from the kernel (with an epoll()-based event loop, no edge-trigger).
> 
> I even have a rate limitation towards the kernel, only sending a 
> "pagesize" full of netlink data towards the kernel, then waiting for the 
> reply before sending more (I had the blocking problem a few years ago 
> when experimenting with LOTS of routes).
> 
> Henning Rogge

Henning Rogge
-- 
Diplom-Informatiker Henning Rogge , Fraunhofer-Institut für
Kommunikation, Informationsverarbeitung und Ergonomie FKIE
Kommunikationssysteme (KOM)
Zanderstrasse 5, 53177 Bonn, Germany
Telefon +49 228 50212-469
mailto:henning.rogge@fkie.fraunhofer.de http://www.fkie.fraunhofer.de

^ permalink raw reply

* RE: [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer driven by a regmap device
From: Pankaj Bansal @ 2018-10-22  4:22 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn; +Cc: netdev@vger.kernel.org
In-Reply-To: <HE1PR0402MB33233F0E876E2091DFD9C9D0F1F80@HE1PR0402MB3323.eurprd04.prod.outlook.com>


> -----Original Message-----
> From: Pankaj Bansal
> Sent: Thursday, October 18, 2018 10:00 AM
> To: Florian Fainelli <f.fainelli@gmail.com>; Andrew Lunn <andrew@lunn.ch>
> Cc: netdev@vger.kernel.org
> Subject: RE: [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer driven by
> a regmap device
> 
> Hi Florian
> 
> > -----Original Message-----
> > From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> > Sent: Sunday, October 7, 2018 11:32 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>; Andrew Lunn
> > <andrew@lunn.ch>
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer
> > driven by a regmap device
> >
> >
> >
> > On 10/07/18 11:24, Pankaj Bansal wrote:
> > > Add support for an MDIO bus multiplexer controlled by a regmap
> > > device, like an FPGA.
> > >
> > > Tested on a NXP LX2160AQDS board which uses the "QIXIS" FPGA
> > > attached to the i2c bus.
> > >
> > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > ---
> > >
> > > Notes:
> > >     V2:
> > >      - Fixed formatting error caused by using space instead of tab
> > >      - Add newline between property list and subnode
> > >      - Add newline between two subnodes
> > >
> > >  .../bindings/net/mdio-mux-regmap.txt         | 95 ++++++++++++++++++
> > >  1 file changed, 95 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
> > > b/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
> > > new file mode 100644
> > > index 000000000000..88ebac26c1c5
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
> > > @@ -0,0 +1,95 @@
> > > +Properties for an MDIO bus multiplexer controlled by a regmap
> > > +
> > > +This is a special case of a MDIO bus multiplexer.  A regmap device,
> > > +like an FPGA, is used to control which child bus is connected.  The
> > > +mdio-mux node must be a child of the device that is controlled by a
> regmap.
> > > +The driver currently only supports devices with upto 32-bit registers.
> >
> > I would omit any sort of details about Linux constructs designed to
> > support specific needs (e.g: regmap) as well as putting driver
> > limitations into a binding document because it really ought to be restricted to
> describing hardware.
> >
> 
> Actually the platform driver mdio-mux-regmap.c, is generalization of mdio-mux-
> mmioreg.c As such, it doesn't describe any new hardware, so no new properties
> are described by it.
> The only new property is compatible field.
> I don't know how to describe this driver otherwise.  Can you please help?

I further thought about it. mdio-mux-regmap.c is not meant to control a specific device.
It is meant to control some registers of parent device. Therefore, IMO this should not be a
Platform driver and there should not be any "compatible" property to which this driver is associated.

Rather this driver should expose the APIs, which should be called by parent devices' driver.

What is your thought on this ?

> 
> > > +
> > > +Required properties in addition to the generic multiplexer properties:
> > > +
> > > +- compatible : string, must contain "mdio-mux-regmap"
> > > +
> > > +- reg : integer, contains the offset of the register that controls the bus
> > > +	multiplexer. it can be 32 bit number.
> >
> > Technically it could be any "reg" property size, the only requirement
> > is that it must be of the appropriate size with respect to what the
> > parent node of this "mdio-mux-regmap" node has, determined by #address-
> cells/#size-cells width.
> 
> We are reading only single cell of this property using "of_propert_read_u32".
> That is why I put the size in this.
> 
> >
> > > +
> > > +- mux-mask : integer, contains an 32 bit mask that specifies which
> > > +	bits in the register control the actual bus multiplexer.  The
> > > +	'reg' property of each child mdio-mux node must be constrained by
> > > +	this mask.
> >
> > Same thing here.
> 
> We are reading only single cell of this property using "of_propert_read_u32".
> That is why I put the size in this.
> 
> >
> > Since this is a MDIO mux, I would invite you to specify what the
> > correct #address-cells/#size-cells values should be (1, and 0
> > respectively as your example correctly shows).
> >
> 
> I will mention #address-cells/#size-cells values
> 
> > > +
> > > +Example:
> > > +
> > > +The FPGA node defines a i2c connected FPGA with a register space of
> > > +0x30
> > bytes.
> > > +For the "EMI2" MDIO bus, register 0x54 (BRDCFG4) controls the mux
> > > +on that
> > bus.
> > > +A bitmask of 0x07 means that bits 0, 1 and 2 (bit 0 is lsb) are the
> > > +bits on
> > > +BRDCFG4 that control the actual mux.
> > > +
> > > +i2c@2000000 {
> > > +	compatible = "fsl,vf610-i2c";
> > > +	#address-cells = <1>;
> > > +	#size-cells = <0>;
> > > +	reg = <0x0 0x2000000 0x0 0x10000>;
> > > +	interrupts = <0 34 0x4>; // Level high type
> > > +	clock-names = "i2c";
> > > +	clocks = <&clockgen 4 7>;
> > > +	fsl-scl-gpio = <&gpio2 15 0>;
> > > +	status = "okay";
> > > +
> > > +	/* The FPGA node */
> > > +	fpga@66 {
> > > +		compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > > +		reg = <0x66>;
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		mdio1_mux@54 {
> > > +			compatible = "mdio-mux-regmap", "mdio-mux";
> > > +			mdio-parent-bus = <&emdio2>; /* MDIO bus */
> > > +			reg = <0x54>;		 /* BRDCFG4 */
> > > +			mux-mask = <0x07>;      /* EMI2_MDIO */
> > > +			#address-cells=<1>;
> > > +			#size-cells = <0>;
> > > +
> > > +			mdio1_ioslot1@0 {   // Slot 1
> > > +				reg = <0x00>;
> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +
> > > +				phy1@1 {
> > > +					reg = <1>;
> > > +					compatible = "ethernet-phy-
> > id0210.7441";
> > > +				};
> > > +
> > > +				phy1@0 {
> > > +					reg = <0>;
> > > +					compatible = "ethernet-phy-
> > id0210.7441";
> > > +				};
> > > +			};
> > > +
> > > +			mdio1_ioslot2@1 {   // Slot 2
> > > +				reg = <0x01>;
> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +
> > > +			};
> > > +
> > > +			mdio1_ioslot3@2 {   // Slot 3
> > > +				reg = <0x02>;
> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +	/* The parent MDIO bus. */
> > > +	emdio2: mdio@0x8B97000 {
> > > +		compatible = "fsl,fman-memac-mdio";
> > > +		reg = <0x0 0x8B97000 0x0 0x1000>;
> > > +		device_type = "mdio";
> > > +		little-endian;
> > > +
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +	};
> > >
> >
> > --
> > Florian

^ permalink raw reply

* Re: pull-request: bpf-next 2018-10-21
From: David Miller @ 2018-10-22  4:12 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev
In-Reply-To: <20181021192426.22776-1-daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Sun, 21 Oct 2018 21:24:26 +0200

> The following pull-request contains BPF updates for your *net-next* tree.
> 
> The main changes are:
 ...
> Please consider pulling these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git

Pulled, thanks Daniel.

^ permalink raw reply

* [PATCH 1/2] rtl8xxxu: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2018-10-22 11:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jes Sorensen, Kalle Valo, linux-wireless, David S. Miller, netdev,
	Gustavo A. R. Silva
In-Reply-To: <cover.1540208577.git.gustavo@embeddedor.com>

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1357355 ("Missing break in switch")
Addresses-Coverity-ID: 1357378 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 56040b1..c6b0686 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw)
 	switch (hw->conf.chandef.width) {
 	case NL80211_CHAN_WIDTH_20_NOHT:
 		ht = false;
+		/* fall through */
 	case NL80211_CHAN_WIDTH_20:
 		opmode |= BW_OPMODE_20MHZ;
 		rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
@@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
 	switch (hw->conf.chandef.width) {
 	case NL80211_CHAN_WIDTH_20_NOHT:
 		ht = false;
+		/* fall through */
 	case NL80211_CHAN_WIDTH_20:
 		rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
 		subchannel = 0;
@@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv)
 		case 3:
 			priv->ep_tx_low_queue = 1;
 			priv->ep_tx_count++;
+			/* fall through */
 		case 2:
 			priv->ep_tx_normal_queue = 1;
 			priv->ep_tx_count++;
+			/* fall through */
 		case 1:
 			priv->ep_tx_high_queue = 1;
 			priv->ep_tx_count++;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] Mark expected switch fall-throughs and fix missing break
From: Gustavo A. R. Silva @ 2018-10-22 11:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jes Sorensen, Kalle Valo, linux-wireless, David S. Miller, netdev,
	Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, this patchset aims
to mark multiple switch cases where we are expecting to fall through.

Also, the second patch in this series fixes a missing break in switch.

Thanks

Gustavo A. R. Silva (2):
  rtl8xxxu: Mark expected switch fall-throughs
  rtl8xxxu: Fix missing break in switch

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.7.4

^ permalink raw reply

* RE: [PATCH] net: ethernet:fec: Consistently use SPEED_ prefix
From: Andy Duan @ 2018-10-22  2:41 UTC (permalink / raw)
  To: Andrew Lunn, David Miller
  Cc: clabbe@baylibre.com, netdev, Florian Fainelli,
	hkallweit1@gmail.com
In-Reply-To: <1540068491-14760-1-git-send-email-andrew@lunn.ch>

From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org>
> All other calls to phy_set_max_speed() use the SPEED_ prefix. Make the
> FEC driver follow this common pattern. This makes no different to
> generated code since SPEED_1000 is 1000, and SPEED_100 is 100.
> 

Please also add more information that was introduced by commit: 58056c1e1b0e ("net: ethernet: Use phy_set_max_speed() to limit advertised speed ")

Andy
> Reported-by: Corentin Labbe <clabbe@baylibre.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 6db69ba30dcd..b067eaf8b792 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1948,7 +1948,7 @@ static int fec_enet_mii_probe(struct
> net_device *ndev)
> 
>  	/* mask with MAC supported features */
>  	if (fep->quirks & FEC_QUIRK_HAS_GBIT) {
> -		phy_set_max_speed(phy_dev, 1000);
> +		phy_set_max_speed(phy_dev, SPEED_1000);
>  		phy_remove_link_mode(phy_dev,
>  				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
>  #if !defined(CONFIG_M5272)
> @@ -1956,7 +1956,7 @@ static int fec_enet_mii_probe(struct
> net_device *ndev)  #endif
>  	}
>  	else
> -		phy_set_max_speed(phy_dev, 100);
> +		phy_set_max_speed(phy_dev, SPEED_100);
> 
>  	fep->link = 0;
>  	fep->full_duplex = 0;
> --
> 2.19.0

^ permalink raw reply

* Re: r8169 regression: UDP packets dropped intermittantly
From: Jonathan Woithe @ 2018-10-22  0:01 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel
In-Reply-To: <20180115065658.GI16419@marvin.atrad.com.au>

On Mon, Jan 15, 2018 at 05:26:59PM +1030, Jonathan Woithe wrote:
> Is there any more information that can be provided (or tests done) to assist
> in tracking this problem down?  Based on the tests done in December it seems
> that the problem only affects specific RTL-8169 variants, with most being
> ok.  Is it a case that we simply need to accept that for the greater good
> commit da78dbff2e05630921c551dbbc70a4b7981a8fff has permanently broken
> Netgear GA311 [1] network cards with respect to these UDP packets and that
> nothing can be done?

For future reference, commit 6b839b6cf9eada30b086effb51e5d6076bafc761
("r8169: fix NAPI handling under high load") appears to have fixed the
regression documented by this thread.  Thanks to Heiner Kallweit for the
work which lead to this solution.

Regards
  jonathan

> [1] Or perhaps any using the RTL8110s variant.

^ permalink raw reply

* Re: [PATCH 2/9] net: dsa: qca8k: simplify getting .driver_data
From: Andrew Lunn @ 2018-10-21 23:15 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Vivien Didelot, Florian Fainelli,
	David S. Miller, netdev
In-Reply-To: <20181021200021.1693-3-wsa+renesas@sang-engineering.com>

On Sun, Oct 21, 2018 at 10:00:13PM +0200, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 9/9] net: phy: mdio-mux-bcm-iproc: simplify getting .driver_data
From: Andrew Lunn @ 2018-10-21 23:14 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Florian Fainelli,
	David S. Miller, Ray Jui, Scott Branden, Jon Mason,
	bcm-kernel-feedback-list, netdev, linux-arm-kernel
In-Reply-To: <20181021200021.1693-10-wsa+renesas@sang-engineering.com>

On Sun, Oct 21, 2018 at 10:00:20PM +0200, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 1/1] net: dsa: legacy: simplify getting .driver_data
From: Andrew Lunn @ 2018-10-21 23:13 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Vivien Didelot, Florian Fainelli,
	David S. Miller, netdev
In-Reply-To: <20181021200105.2414-2-wsa+renesas@sang-engineering.com>

On Sun, Oct 21, 2018 at 10:01:05PM +0200, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* RE: [PATCH] bonding: avoid repeated display of same link status change
From: Manish Kumar Singh @ 2018-10-22  7:29 UTC (permalink / raw)
  To: Eric Dumazet, netdev
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	linux-kernel
In-Reply-To: <dd819430-a67e-4507-b03f-d9d40528e74d@default>



> -----Original Message-----
> From: Manish Kumar Singh
> Sent: 24 सितम्बर 2018 12:36
> To: Eric Dumazet; netdev@vger.kernel.org
> Cc: Jay Vosburgh; Veaceslav Falico; Andy Gospodarek; David S. Miller; linux-
> kernel@vger.kernel.org
> Subject: RE: [PATCH] bonding: avoid repeated display of same link status
> change
> 
> 
> 
> > -----Original Message-----
> > From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> > Sent: 18 सितम्बर 2018 19:30
> > To: Manish Kumar Singh; Eric Dumazet; netdev@vger.kernel.org
> > Cc: Jay Vosburgh; Veaceslav Falico; Andy Gospodarek; David S. Miller; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH] bonding: avoid repeated display of same link status
> > change
> >
> >
> >
> > On 09/17/2018 10:05 PM, Manish Kumar Singh wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> > >> Sent: 17 सितम्बर 2018 20:08
> > >> To: Manish Kumar Singh; netdev@vger.kernel.org
> > >> Cc: Jay Vosburgh; Veaceslav Falico; Andy Gospodarek; David S. Miller;
> > linux-
> > >> kernel@vger.kernel.org
> > >> Subject: Re: [PATCH] bonding: avoid repeated display of same link status
> > >> change
> > >>
> > >>
> > >>
> > >> On 09/17/2018 12:20 AM, mk.singh@oracle.com wrote:
> > >>> From: Manish Kumar Singh <mk.singh@oracle.com>
> > >>>
> > >>> When link status change needs to be committed and rtnl lock couldn't
> be
> > >>> taken, avoid redisplay of same link status change message.
> > >>>
> > >>> Signed-off-by: Manish Kumar Singh <mk.singh@oracle.com>
> > >>> ---
> > >>>  drivers/net/bonding/bond_main.c | 6 ++++--
> > >>>  include/net/bonding.h           | 1 +
> > >>>  2 files changed, 5 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/net/bonding/bond_main.c
> > >> b/drivers/net/bonding/bond_main.c
> > >>> index 217b790d22ed..fb4e3aff1677 100644
> > >>> --- a/drivers/net/bonding/bond_main.c
> > >>> +++ b/drivers/net/bonding/bond_main.c
> > >>> @@ -2087,7 +2087,7 @@ static int bond_miimon_inspect(struct
> bonding
> > >> *bond)
> > >>>  			bond_propose_link_state(slave, BOND_LINK_FAIL);
> > >>>  			commit++;
> > >>>  			slave->delay = bond->params.downdelay;
> > >>> -			if (slave->delay) {
> > >>> +			if (slave->delay && !bond->rtnl_needed) {
> > >>>  				netdev_info(bond->dev, "link status down
> > for
> > >> %sinterface %s, disabling it in %d ms\n",
> > >>>  					    (BOND_MODE(bond) ==
> > >>>  					     BOND_MODE_ACTIVEBACKUP) ?
> > >>> @@ -2127,7 +2127,7 @@ static int bond_miimon_inspect(struct
> bonding
> > >> *bond)
> > >>>  			commit++;
> > >>>  			slave->delay = bond->params.updelay;
> > >>>
> > >>> -			if (slave->delay) {
> > >>> +			if (slave->delay && !bond->rtnl_needed) {
> > >>>  				netdev_info(bond->dev, "link status up for
> > >> interface %s, enabling it in %d ms\n",
> > >>>  					    slave->dev->name,
> > >>>  					    ignore_updelay ? 0 :
> > >>> @@ -2301,9 +2301,11 @@ static void bond_mii_monitor(struct
> > >> work_struct *work)
> > >>>  		if (!rtnl_trylock()) {
> > >>>  			delay = 1;
> > >>>  			should_notify_peers = false;
> > >>> +			bond->rtnl_needed = true;
> > >>
> > >> How can you set a shared variable with no synchronization ?
> > > Thanks Eric for reviewing the patch. rtnl_needed is not a shared variable,
> it
> > is part of bonding structure, that is one per bonding driver instance. There
> > can't be two parallel instances of bond_miimon_inspect for a
> single  bonding
> > driver instance at any given point of time. and only bond_miimon_inspect
> > updates it. That’s why I think there is no need of any synchronization here.
> > >
> > >
> >
> > If rtnl_trylock() can not grab RTNL,
> > there is no way the current thread can set the  variable without a race, if
> the
> > word including rtnl_needed is shared by other fields in the structure.
> >
> > Your patch adds a subtle possibility of future bugs, even if it runs fine
> today.
> >
> > Do not pave the way for future bugs, make your code robust, please.
> 
> Thankyou Eric, we are making the changes and will repost the patch after
> testing it.
> -Manish

Please review the updated patch, I have changed the rtnl_needed to an atomic variable, to avoid any race condition:

From: Manish Kumar Singh <mk.singh@oracle.com>

When link status change needs to be committed and rtnl lock couldn't be taken, avoid redisplay of same link status change message.

Signed-off-by: Manish Kumar Singh <mk.singh@oracle.com>
---
 drivers/net/bonding/bond_main.c | 6 ++++--
 include/net/bonding.h           | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 217b790d22ed..fac5350bf19c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2087,7 +2087,7 @@ static int bond_miimon_inspect(struct bonding *bond)
 			bond_propose_link_state(slave, BOND_LINK_FAIL);
 			commit++;
 			slave->delay = bond->params.downdelay;
-			if (slave->delay) {
+			if (slave->delay && !atomic_read(&bond->rtnl_needed)) {
 				netdev_info(bond->dev, "link status down for %sinterface %s, disabling it in %d ms\n",
 					    (BOND_MODE(bond) ==
 					     BOND_MODE_ACTIVEBACKUP) ?
@@ -2127,7 +2127,7 @@ static int bond_miimon_inspect(struct bonding *bond)
 			commit++;
 			slave->delay = bond->params.updelay;
 
-			if (slave->delay) {
+			if (slave->delay && !atomic_read(&bond->rtnl_needed)) {
 				netdev_info(bond->dev, "link status up for interface %s, enabling it in %d ms\n",
 					    slave->dev->name,
 					    ignore_updelay ? 0 :
@@ -2301,9 +2301,11 @@ static void bond_mii_monitor(struct work_struct *work)
 		if (!rtnl_trylock()) {
 			delay = 1;
 			should_notify_peers = false;
+			atomic_set(&bond->rtnl_needed, 1);
 			goto re_arm;
 		}
 
+		atomic_set(&bond->rtnl_needed, 0);
 		bond_for_each_slave(bond, slave, iter) {
 			bond_commit_link_state(slave, BOND_SLAVE_NOTIFY_LATER);
 		}
diff --git a/include/net/bonding.h b/include/net/bonding.h index 808f1d167349..ffc1219f7a07 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -234,6 +234,7 @@ struct bonding {
 	struct	 dentry *debug_dir;
 #endif /* CONFIG_DEBUG_FS */
 	struct rtnl_link_stats64 bond_stats;
+	atomic_t rtnl_needed;
 };
 
 #define bond_slave_get_rcu(dev) \
--
2.14.1



Thanks,
Manish

> >
> >
> >
> >
> >
> >
> >

^ permalink raw reply

* Re: [PATCH net] r8169: fix NAPI handling under high load
From: Jonathan Woithe @ 2018-10-21 23:07 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Francois Romieu, Holger Hoffstätte, David Miller,
	Realtek linux nic maintainers, netdev@vger.kernel.org
In-Reply-To: <20181018061554.GF2487@marvin.atrad.com.au>

On Fri, 19 Oct 2018 17:59:21 +1030, Jonathan Woithe wrote:
> On 10/18/18 08:15, Jonathan Woithe wrote:
> > On Thu, Oct 18, 2018 at 08:03:32AM +0200, Heiner Kallweit wrote:
> > > Proposed fix is here:
> > > https://patchwork.ozlabs.org/patch/985014/
> > > Would be good if you could test it. Thanks!
> > 
> > I should be able to do so tomorrow.  Which kernel would you like me to apply
> > the patch to?
>
> It turns out I couldn't compile 4.18.15 conveniently ...
> I could compile 4.14 though - I had previously confirmed that the problem
> was still seen with that kernel.  I therefore applied the fix to 4.14 (which
> was trivial) and started a test with that.  The system has been running
> without exhibiting the effects of the bug for over five hours.  With an
> unpatched 4.14, symptoms were typically seen within 30 minutes.
> 
> I will leave the system to operate over the weekend to be sure, but at this
> stage it seems likely that the patch will resolve this long-standing
> difficulty that we've experienced.  I will report back on Monday.

Our test system has now been running for just under 3 days (69 hours) and
there has not been any incidence of the problem with this patch applied to
the 4.14 r8169 driver.  Without the patch, multiple problems are logged
within 30 minutes.

Given this, I would conclude that this patch fixes the problem for us. 
Although belated (since the patch has already been accepted):

  Tested-by: Jonathan Woithe <jwoithe@atrad.com.au>

Thank you very much for your work which lead to the patch!  It means we can
finally provide an upgrade path for systems in the field which are equipped
with the r8169 hardware.

For reference, the problem being tested on our systems is the one discussed
in the "r8169 regression: UDP packets dropped intermittantly" thread on this
mailing list.

Regards
  jonathan

^ 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