* Re: [PATCH net] Revert "mdio_bus: Remove unneeded gpiod NULL check"
From: Linus Walleij @ 2017-09-08 23:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, Andrew Lunn,
Fabio Estevam, Sergei Shtylyov, Fabio Estevam, Bryan.Whitehead
In-Reply-To: <fe6846be-afc3-1025-d78a-96674e1b2a35@gmail.com>
On Sat, Sep 9, 2017 at 1:18 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 09/08/2017 04:13 PM, Linus Walleij wrote:
>> On Sat, Sep 9, 2017 at 12:38 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>> This reverts commit 95b80bf3db03c2bf572a357cf74b9a6aefef0a4a ("mdio_bus:
>>> Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB
>>> checks for NULL descriptors, so it's safe to drop them, but it is not
>>> when CONFIG_GPIOLIB is disabled in the kernel. If we do call
>>> gpiod_set_value_cansleep() on a GPIO descriptor we will issue warnings
>>> coming from the inline stubs declared in include/linux/gpio/consumer.h.
>>>
>>> Fixes: 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")
>>> Reported-by: Woojung Huh <Woojung.Huh@microchip.com>
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> Yeah I guess you don't wanna have these messages spewing
>> in the console. :/
>>
>> But what about simply doing this:
>
> That would create another dependency which really does not need to be
> there as it really prevents you from testing more configurations,
> including but not limited to having CONFIG_GPIOLIB=n w/
> CONFIG_MDIO_DEVICE=[ym].
>
> The GPIOLIB=n inline stubs have a "contract" with the code calling them
> that is fairly clear, which is what this revert is leveraging.
Ayeah the contract is something like "you can call us if compiled out,
but then you get warnings".
Yeah NULL checks does the trick as well as #ifdef:in in that sense.
It's no big deal so if you prefer this:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH net] Revert "mdio_bus: Remove unneeded gpiod NULL check"
From: Florian Fainelli @ 2017-09-08 23:18 UTC (permalink / raw)
To: Linus Walleij
Cc: netdev@vger.kernel.org, David S. Miller, Andrew Lunn,
Fabio Estevam, Sergei Shtylyov, Fabio Estevam, Bryan.Whitehead
In-Reply-To: <CACRpkdZD-nRXXusX36Gaypf5A601CMEO3vVgaOV5tKufvy1M3Q@mail.gmail.com>
On 09/08/2017 04:13 PM, Linus Walleij wrote:
> On Sat, Sep 9, 2017 at 12:38 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>> This reverts commit 95b80bf3db03c2bf572a357cf74b9a6aefef0a4a ("mdio_bus:
>> Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB
>> checks for NULL descriptors, so it's safe to drop them, but it is not
>> when CONFIG_GPIOLIB is disabled in the kernel. If we do call
>> gpiod_set_value_cansleep() on a GPIO descriptor we will issue warnings
>> coming from the inline stubs declared in include/linux/gpio/consumer.h.
>>
>> Fixes: 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")
>> Reported-by: Woojung Huh <Woojung.Huh@microchip.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Yeah I guess you don't wanna have these messages spewing
> in the console. :/
>
> But what about simply doing this:
That would create another dependency which really does not need to be
there as it really prevents you from testing more configurations,
including but not limited to having CONFIG_GPIOLIB=n w/
CONFIG_MDIO_DEVICE=[ym].
The GPIOLIB=n inline stubs have a "contract" with the code calling them
that is fairly clear, which is what this revert is leveraging.
Instead of doing what you suggest, we could also encapsulate the
offending code within an #IS_ENABLED(CONFIG_GPIOLIB) block, which would
be virtually the same thing in terms of object code generated, but would
make it clear there is a functional dependency, I would personally be
keener on that approach than having a select or depends.
Thanks
>
>
> From 150e4f3c1f227c9a4c31bbb48d44220e25b792ec Mon Sep 17 00:00:00 2001
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Sat, 9 Sep 2017 01:07:22 +0200
> Subject: [PATCH] net: phy: mdio_bus: mandate gpiolib
>
> The core mdio bus unconditionally uses gpiolib APIs to handle
> reset lines which results in runtime errors when it is compiled
> out. It is not supposed to be possible to stub out gpiolib
> like this, the error messages from the stubs are a sign that
> something is wrong.
>
> So just select it. Fix some unneeded #includes while we're
> at it.>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/net/phy/Kconfig | 1 +
> drivers/net/phy/mdio_bus.c | 2 --
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index a9d16a3af514..b6c5bb9941c3 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -4,6 +4,7 @@
>
> menuconfig MDIO_DEVICE
> tristate "MDIO bus device drivers"
> + select GPIOLIB
> help
> MDIO devices and driver infrastructure code.
>
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index b6f9fa670168..9c60f87b7209 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -22,11 +22,9 @@
> #include <linux/init.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> -#include <linux/gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/of_device.h>
> #include <linux/of_mdio.h>
> -#include <linux/of_gpio.h>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> #include <linux/skbuff.h>
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] mdio_bus: Remove unneeded gpiod NULL check
From: Linus Walleij @ 2017-09-08 23:17 UTC (permalink / raw)
To: Florian Fainelli
Cc: Woojung.Huh, Andrew Lunn, Fabio Estevam, Sergei Shtylyov,
David S. Miller, netdev@vger.kernel.org, Fabio Estevam
In-Reply-To: <81338659-4f46-0f2f-13c3-135b3ac876d7@gmail.com>
On Thu, Sep 7, 2017 at 11:39 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> I think it means CONFIG_GPIOLIB=n in the kernel because it's not needed,
> yet you run code (like drivers/net/phy/mdio_bus.c) that unconditionally
> calls into GPIOLIB and attempts to configure a given GPIO if available.
> This thread is actually what prompted me to write this email:
Yeah I think GPIOLIB should simply be selected in KConfig then.
Sent an inline patch to do that in the other reply.
Maybe I should simply delete the stubs if they confuse
people.
It is possible to select GPIOLIB on any platform these days
and get the right APIs, this used to not be the case but I fixed
it a while back.
Alternatively we can depend on GPIOLIB but it's simpler to just
select it I think, it's like a library this code uses and so that's
it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH net] Revert "mdio_bus: Remove unneeded gpiod NULL check"
From: Linus Walleij @ 2017-09-08 23:13 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, Andrew Lunn,
Fabio Estevam, Sergei Shtylyov, Fabio Estevam, Bryan.Whitehead
In-Reply-To: <20170908223807.6015-1-f.fainelli@gmail.com>
On Sat, Sep 9, 2017 at 12:38 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> This reverts commit 95b80bf3db03c2bf572a357cf74b9a6aefef0a4a ("mdio_bus:
> Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB
> checks for NULL descriptors, so it's safe to drop them, but it is not
> when CONFIG_GPIOLIB is disabled in the kernel. If we do call
> gpiod_set_value_cansleep() on a GPIO descriptor we will issue warnings
> coming from the inline stubs declared in include/linux/gpio/consumer.h.
>
> Fixes: 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")
> Reported-by: Woojung Huh <Woojung.Huh@microchip.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Yeah I guess you don't wanna have these messages spewing
in the console. :/
But what about simply doing this:
>From 150e4f3c1f227c9a4c31bbb48d44220e25b792ec Mon Sep 17 00:00:00 2001
From: Linus Walleij <linus.walleij@linaro.org>
Date: Sat, 9 Sep 2017 01:07:22 +0200
Subject: [PATCH] net: phy: mdio_bus: mandate gpiolib
The core mdio bus unconditionally uses gpiolib APIs to handle
reset lines which results in runtime errors when it is compiled
out. It is not supposed to be possible to stub out gpiolib
like this, the error messages from the stubs are a sign that
something is wrong.
So just select it. Fix some unneeded #includes while we're
at it.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/phy/Kconfig | 1 +
drivers/net/phy/mdio_bus.c | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index a9d16a3af514..b6c5bb9941c3 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -4,6 +4,7 @@
menuconfig MDIO_DEVICE
tristate "MDIO bus device drivers"
+ select GPIOLIB
help
MDIO devices and driver infrastructure code.
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index b6f9fa670168..9c60f87b7209 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -22,11 +22,9 @@
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/device.h>
-#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/of_device.h>
#include <linux/of_mdio.h>
-#include <linux/of_gpio.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/skbuff.h>
--
2.13.5
Yours,
Linus Walleij
^ permalink raw reply related
* Re: [PATCH net] ipv6: fix typo in fib6_net_exit()
From: Sabrina Dubroca @ 2017-09-08 23:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1504910927.15310.105.camel@edumazet-glaptop3.roam.corp.google.com>
2017-09-08, 15:48:47 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> IPv6 FIB should use FIB6_TABLE_HASHSZ, not FIB_TABLE_HASHSZ.
>
> Fixes: ba1cc08d9488 ("ipv6: fix memory leak with multiple tables during netns destruction")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Ouch, yes :(
Thanks for the fix.
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
--
Sabrina
^ permalink raw reply
* Re: [PATCH net] ipv6: fix typo in fib6_net_exit()
From: David Miller @ 2017-09-08 23:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: sd, netdev
In-Reply-To: <1504910927.15310.105.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 08 Sep 2017 15:48:47 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> IPv6 FIB should use FIB6_TABLE_HASHSZ, not FIB_TABLE_HASHSZ.
>
> Fixes: ba1cc08d9488 ("ipv6: fix memory leak with multiple tables during netns destruction")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] tcp: fix a request socket leak
From: David Miller @ 2017-09-08 23:08 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: eric.dumazet, netdev
In-Reply-To: <20170908225921.bxbgaldzt5fslpda@ast-mbp>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Fri, 8 Sep 2017 15:59:23 -0700
> On Fri, Sep 08, 2017 at 12:44:47PM -0700, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> While the cited commit fixed a possible deadlock, it added a leak
>> of the request socket, since reqsk_put() must be called if the BPF
>> filter decided the ACK packet must be dropped.
>>
>> Fixes: d624d276d1dd ("tcp: fix possible deadlock in TCP stack vs BPF filter")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> ---
>> David, this is a stable candidate (linux-4.13 has this bug)
>
> Thanks for the fix
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> .. since the fix makes sense to me and as a reminder not to forget
> to backport it as well :)
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] tcp: fix a request socket leak
From: Alexei Starovoitov @ 2017-09-08 22:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1504899887.15310.95.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, Sep 08, 2017 at 12:44:47PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> While the cited commit fixed a possible deadlock, it added a leak
> of the request socket, since reqsk_put() must be called if the BPF
> filter decided the ACK packet must be dropped.
>
> Fixes: d624d276d1dd ("tcp: fix possible deadlock in TCP stack vs BPF filter")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> David, this is a stable candidate (linux-4.13 has this bug)
Thanks for the fix
Acked-by: Alexei Starovoitov <ast@kernel.org>
.. since the fix makes sense to me and as a reminder not to forget
to backport it as well :)
^ permalink raw reply
* [PATCH net] ipv6: fix typo in fib6_net_exit()
From: Eric Dumazet @ 2017-09-08 22:48 UTC (permalink / raw)
To: Sabrina Dubroca, David Miller; +Cc: netdev
In-Reply-To: <1504893592.15310.80.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
IPv6 FIB should use FIB6_TABLE_HASHSZ, not FIB_TABLE_HASHSZ.
Fixes: ba1cc08d9488 ("ipv6: fix memory leak with multiple tables during netns destruction")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/ip6_fib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 8280172c806ca47c5ca4aef723d405a0a8d7df2a..e5308d7cbd75c4fb67861082382122d445cf5b74 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -2033,7 +2033,7 @@ static void fib6_net_exit(struct net *net)
rt6_ifdown(net, NULL);
del_timer_sync(&net->ipv6.ip6_fib_timer);
- for (i = 0; i < FIB_TABLE_HASHSZ; i++) {
+ for (i = 0; i < FIB6_TABLE_HASHSZ; i++) {
struct hlist_head *head = &net->ipv6.fib_table_hash[i];
struct hlist_node *tmp;
struct fib6_table *tb;
^ permalink raw reply related
* Re: [net PATCH 3/3] bpf: devmap, use cond_resched instead of cpu_relax
From: Alexei Starovoitov @ 2017-09-08 22:40 UTC (permalink / raw)
To: John Fastabend, davem; +Cc: netdev, daniel
In-Reply-To: <150490447017.11590.17727368565733920857.stgit@john-XPS-13-9360>
On 9/8/17 2:01 PM, John Fastabend wrote:
> Be a bit more friendly about waiting for flush bits to complete.
> Replace the cpu_relax() with a cond_resched().
>
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
unlike patch 1 and 2, this one could have waited till net-next opens,
but I don't mind now. lgtm
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [net PATCH 2/3] bpf: add support for sockmap detach programs
From: Alexei Starovoitov @ 2017-09-08 22:38 UTC (permalink / raw)
To: John Fastabend, davem; +Cc: netdev, daniel
In-Reply-To: <150490444956.11590.4733752227675213913.stgit@john-XPS-13-9360>
On 9/8/17 2:00 PM, John Fastabend wrote:
> The bpf map sockmap supports adding programs via attach commands. This
> patch adds the detach command to keep the API symmetric and allow
> users to remove previously added programs. Otherwise the user would
> have to delete the map and re-add it to get in this state.
>
> This also adds a series of additional tests to capture detach operation
> and also attaching/detaching invalid prog types.
>
> API note: socks will run (or not run) programs depending on the state
> of the map at the time the sock is added. We do not for example walk
> the map and remove programs from previously attached socks.
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Nice clean patch. Thx
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [net PATCH 1/3] net: rcu lock and preempt disable missing around generic xdp
From: Alexei Starovoitov @ 2017-09-08 22:38 UTC (permalink / raw)
To: John Fastabend, davem; +Cc: netdev, daniel
In-Reply-To: <150490443011.11590.2847947557652786219.stgit@john-XPS-13-9360>
On 9/8/17 2:00 PM, John Fastabend wrote:
> do_xdp_generic must be called inside rcu critical section with preempt
> disabled to ensure BPF programs are valid and per-cpu variables used
> for redirect operations are consistent. This patch ensures this is true
> and fixes the splat below.
>
> The netif_receive_skb_internal() code path is now broken into two rcu
> critical sections. I decided it was better to limit the preempt_enable/disable
> block to just the xdp static key portion and the fallout is more
> rcu_read_lock/unlock calls. Seems like the best option to me.
>
> [ 607.596901] =============================
> [ 607.596906] WARNING: suspicious RCU usage
> [ 607.596912] 4.13.0-rc4+ #570 Not tainted
> [ 607.596917] -----------------------------
> [ 607.596923] net/core/dev.c:3948 suspicious rcu_dereference_check() usage!
> [ 607.596927]
> [ 607.596927] other info that might help us debug this:
> [ 607.596927]
> [ 607.596933]
> [ 607.596933] rcu_scheduler_active = 2, debug_locks = 1
> [ 607.596938] 2 locks held by pool/14624:
> [ 607.596943] #0: (rcu_read_lock_bh){......}, at: [<ffffffff95445ffd>] ip_finish_output2+0x14d/0x890
> [ 607.596973] #1: (rcu_read_lock_bh){......}, at: [<ffffffff953c8e3a>] __dev_queue_xmit+0x14a/0xfd0
> [ 607.597000]
> [ 607.597000] stack backtrace:
> [ 607.597006] CPU: 5 PID: 14624 Comm: pool Not tainted 4.13.0-rc4+ #570
> [ 607.597011] Hardware name: Dell Inc. Precision Tower 5810/0HHV7N, BIOS A17 03/01/2017
> [ 607.597016] Call Trace:
> [ 607.597027] dump_stack+0x67/0x92
> [ 607.597040] lockdep_rcu_suspicious+0xdd/0x110
> [ 607.597054] do_xdp_generic+0x313/0xa50
> [ 607.597068] ? time_hardirqs_on+0x5b/0x150
> [ 607.597076] ? mark_held_locks+0x6b/0xc0
> [ 607.597088] ? netdev_pick_tx+0x150/0x150
> [ 607.597117] netif_rx_internal+0x205/0x3f0
> [ 607.597127] ? do_xdp_generic+0xa50/0xa50
> [ 607.597144] ? lock_downgrade+0x2b0/0x2b0
> [ 607.597158] ? __lock_is_held+0x93/0x100
> [ 607.597187] netif_rx+0x119/0x190
> [ 607.597202] loopback_xmit+0xfd/0x1b0
> [ 607.597214] dev_hard_start_xmit+0x127/0x4e0
>
> Fixes: d445516966dc ("net: xdp: support xdp generic on virtual devices")
> Fixes: b5cdae3291f7 ("net: Generic XDP")
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
argh, so it's due to virtual devices and loopback.
Not pretty, but have to agree I don't see another way of fixing it.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* [PATCH net] Revert "mdio_bus: Remove unneeded gpiod NULL check"
From: Florian Fainelli @ 2017-09-08 22:38 UTC (permalink / raw)
To: netdev
Cc: davem, linus.walleij, andrew, festevam, sergei.shtylyov,
fabio.estevam, Bryan.Whitehead, Florian Fainelli
This reverts commit 95b80bf3db03c2bf572a357cf74b9a6aefef0a4a ("mdio_bus:
Remove unneeded gpiod NULL check"), this commit assumed that GPIOLIB
checks for NULL descriptors, so it's safe to drop them, but it is not
when CONFIG_GPIOLIB is disabled in the kernel. If we do call
gpiod_set_value_cansleep() on a GPIO descriptor we will issue warnings
coming from the inline stubs declared in include/linux/gpio/consumer.h.
Fixes: 95b80bf3db03 ("mdio_bus: Remove unneeded gpiod NULL check")
Reported-by: Woojung Huh <Woojung.Huh@microchip.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/mdio_bus.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index b6f9fa670168..2df7b62c1a36 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -399,7 +399,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
}
/* Put PHYs in RESET to save power */
- gpiod_set_value_cansleep(bus->reset_gpiod, 1);
+ if (bus->reset_gpiod)
+ gpiod_set_value_cansleep(bus->reset_gpiod, 1);
device_del(&bus->dev);
return err;
@@ -424,7 +425,8 @@ void mdiobus_unregister(struct mii_bus *bus)
}
/* Put PHYs in RESET to save power */
- gpiod_set_value_cansleep(bus->reset_gpiod, 1);
+ if (bus->reset_gpiod)
+ gpiod_set_value_cansleep(bus->reset_gpiod, 1);
device_del(&bus->dev);
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] ipv4: Namespaceify tcp_max_orphans knob
From: Cong Wang @ 2017-09-08 22:13 UTC (permalink / raw)
To: Haishuang Yan
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Eric Dumazet, Linux Kernel Network Developers, LKML
In-Reply-To: <1504753808-13266-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
On Wed, Sep 6, 2017 at 8:10 PM, Haishuang Yan
<yanhaishuang@cmss.chinamobile.com> wrote:
> Different namespace application might require different maximal number
> of TCP sockets independently of the host.
So after your patch we could have N * net->ipv4.sysctl_tcp_max_orphans
in a whole system, right? This just makes OOM easier to trigger.
^ permalink raw reply
* Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Pavel Machek @ 2017-09-08 21:57 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121E3D@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 531 bytes --]
Hi!
> > > + default:
> > > + processed = false;
> > > + break;
> > > + }
> > > + if (processed)
> > > + *val = data;
> > > +}
> >
> > Similar code will be needed by other drivers, right?
>
> Although KSZ8795 and KSZ8895 may use the same code, the other
> chips will have different code.
Ok, please make sure code is shared between these two.
Thansk,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH net] netlink: fix an use-after-free issue for nlk groups
From: Cong Wang @ 2017-09-08 21:56 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, David Miller, Florian Westphal, chunwang, syzkaller
In-Reply-To: <1217baf9faf9d034ba8224cb6362b822c51a0eae.1504669632.git.lucien.xin@gmail.com>
On Tue, Sep 5, 2017 at 8:47 PM, Xin Long <lucien.xin@gmail.com> wrote:
> ChunYu found a netlink use-after-free issue by syzkaller:
>
> [28448.842981] BUG: KASAN: use-after-free in __nla_put+0x37/0x40 at addr ffff8807185e2378
> [28448.969918] Call Trace:
> [...]
> [28449.117207] __nla_put+0x37/0x40
> [28449.132027] nla_put+0xf5/0x130
> [28449.146261] sk_diag_fill.isra.4.constprop.5+0x5a0/0x750 [netlink_diag]
> [28449.176608] __netlink_diag_dump+0x25a/0x700 [netlink_diag]
> [28449.202215] netlink_diag_dump+0x176/0x240 [netlink_diag]
> [28449.226834] netlink_dump+0x488/0xbb0
> [28449.298014] __netlink_dump_start+0x4e8/0x760
> [28449.317924] netlink_diag_handler_dump+0x261/0x340 [netlink_diag]
> [28449.413414] sock_diag_rcv_msg+0x207/0x390
> [28449.432409] netlink_rcv_skb+0x149/0x380
> [28449.467647] sock_diag_rcv+0x2d/0x40
> [28449.484362] netlink_unicast+0x562/0x7b0
> [28449.564790] netlink_sendmsg+0xaa8/0xe60
> [28449.661510] sock_sendmsg+0xcf/0x110
> [28449.865631] __sys_sendmsg+0xf3/0x240
> [28450.000964] SyS_sendmsg+0x32/0x50
> [28450.016969] do_syscall_64+0x25c/0x6c0
> [28450.154439] entry_SYSCALL64_slow_path+0x25/0x25
>
> It was caused by no protection between nlk groups' free in netlink_release
> and nlk groups' accessing in sk_diag_dump_groups. The similar issue also
> exists in netlink_seq_show().
>
> This patch is to defer nlk groups' free in deferred_put_nlk_sk.
This looks odd too, at least not complete.
The netlink sock itself is protected by RCU to speed up
the lookup path, but not necessarily nlk->groups, at least
I don't see rcu_dereference() in sk_diag_dump_groups().
And netlink_realloc_groups() needs fix too, right? Otherwise
krealloc() could reallocate a brand new memory and
existing readers will crash too?
I am afraid you need more work to make nlk->groups
RCU friendly. RCU is not just about call_rcu(), both
readers and writers need to use proper RCU API.
^ permalink raw reply
* Re: [PATCH RFC 5/5] Add KSZ8795 SPI driver
From: Pavel Machek @ 2017-09-08 21:55 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121E1C@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 931 bytes --]
Hi!
> > Please format according to CodingStyle. (Not only this.)
> >
> > And this will be common for more drivers. Can it go to a header file
> > and be included...?
> >
>
> Sorry about the formatting. It seems my e-mail system needs to be checked
> to make sure it does not auto-format the contents again.
>
> About the SPI access functions they are the same for each driver except the
> low level ksz_spi_read_reg and ksz_spi_write_reg. The dev_io_ops structure
> should contain only those 2 and ksz_spi_get and ksz_spi_set.
>
> But that requires changing ksz_spi.c. The idea was to keep the code of
> KSZ9477 driver with little change as possible while introducing another driver.
So we change ksz_spi.c. Goal is to have clean code.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
From: Pavel Machek @ 2017-09-08 21:53 UTC (permalink / raw)
To: Tristram.Ha
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121E68@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]
Hi!
> There will be 5 drivers to support these devices:
>
> ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477
> ksz8795.c - KSZ8795/KSZ8795/KSZ8765
> ksz8895.c - KSZ8895/KSZ8864
Could we see the 8895 driver, please?
> Out of topic I have a question to ask the community regarding the DSA
> port creation:
>
> Port 1 can be specified using the reg parameter specifying 0; port 2, 1;
> and so on. The KSZ8794 is a variant of KSZ8795 with port 4 disabled.
> So
> lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4.
> But our company Marketing does not want to promote that fact but treat
> KSZ8794 as a distinct product.
> So
> lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3.
> Is this okay to hide this information inside the driver? This is manageable
> for KSZ8794 but for KSZ8864 the first port is disabled:
> lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4.
>
> I am not sure whether DSA has or will have a way to display the port
> mapping to regular users.
Kernel is not a place to play marketing games, and people reading the
kernel sources are not targets of your marketing department.
Please let us just see the underlying hardware, as is.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH iproute2 3/3] bridge: request vlans along with link information
From: Roman Mashak @ 2017-09-08 21:52 UTC (permalink / raw)
To: stephen; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1504907543-14394-1-git-send-email-mrv@mojatatu.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
bridge/link.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/bridge/link.c b/bridge/link.c
index 60200f1..9e4206f 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -461,9 +461,19 @@ static int brlink_show(int argc, char **argv)
}
}
- if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETLINK) < 0) {
- perror("Cannon send dump request");
- exit(1);
+ if (show_details) {
+ if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE, RTM_GETLINK,
+ (compress_vlans ?
+ RTEXT_FILTER_BRVLAN_COMPRESSED :
+ RTEXT_FILTER_BRVLAN)) < 0) {
+ perror("Cannon send dump request");
+ exit(1);
+ }
+ } else {
+ if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETLINK) < 0) {
+ perror("Cannon send dump request");
+ exit(1);
+ }
}
if (rtnl_dump_filter(&rth, print_linkinfo, stdout) < 0) {
--
1.9.1
^ permalink raw reply related
* [PATCH iproute2 2/3] bridge: dump vlan table information for link
From: Roman Mashak @ 2017-09-08 21:52 UTC (permalink / raw)
To: stephen; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1504907543-14394-1-git-send-email-mrv@mojatatu.com>
Kernel also reports vlans a port is member of, so print it. Since vlan
table can be quite large, dump it only when detailed information is
requested.
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
bridge/link.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/bridge/link.c b/bridge/link.c
index 93472ad..60200f1 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -213,6 +213,13 @@ int print_linkinfo(const struct sockaddr_nl *who,
if (aftb[IFLA_BRIDGE_MODE])
print_hwmode(fp, rta_getattr_u16(aftb[IFLA_BRIDGE_MODE]));
+ if (show_details) {
+ if (aftb[IFLA_BRIDGE_VLAN_INFO]) {
+ fprintf(fp, "\n");
+ print_vlan_info(fp, tb[IFLA_AF_SPEC],
+ ifi->ifi_index);
+ }
+ }
}
fprintf(fp, "\n");
--
1.9.1
^ permalink raw reply related
* [PATCH iproute2 1/3] bridge: isolate vlans parsing code in a separate API
From: Roman Mashak @ 2017-09-08 21:52 UTC (permalink / raw)
To: stephen; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1504907543-14394-1-git-send-email-mrv@mojatatu.com>
IFLA_BRIDGE_VLAN_INFO parsing logic will be used in link and vlan
processing code, so it makes sense to move it in the separate function.
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
bridge/br_common.h | 1 +
bridge/vlan.c | 145 +++++++++++++++++++++++++++--------------------------
2 files changed, 76 insertions(+), 70 deletions(-)
diff --git a/bridge/br_common.h b/bridge/br_common.h
index c649e7d..01447dd 100644
--- a/bridge/br_common.h
+++ b/bridge/br_common.h
@@ -4,6 +4,7 @@
#define MDB_RTR_RTA(r) \
((struct rtattr *)(((char *)(r)) + RTA_ALIGN(sizeof(__u32))))
+extern void print_vlan_info(FILE *fp, struct rtattr *tb, int ifindex);
extern int print_linkinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n,
void *arg);
diff --git a/bridge/vlan.c b/bridge/vlan.c
index ebcdace..fc361ae 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -189,7 +189,6 @@ static int print_vlan(const struct sockaddr_nl *who,
struct ifinfomsg *ifm = NLMSG_DATA(n);
int len = n->nlmsg_len;
struct rtattr *tb[IFLA_MAX+1];
- bool vlan_flags = false;
if (n->nlmsg_type != RTM_NEWLINK) {
fprintf(stderr, "Not RTM_NEWLINK: %08x %08x %08x\n",
@@ -218,75 +217,7 @@ static int print_vlan(const struct sockaddr_nl *who,
ll_index_to_name(ifm->ifi_index));
return 0;
} else {
- struct rtattr *i, *list = tb[IFLA_AF_SPEC];
- int rem = RTA_PAYLOAD(list);
- __u16 last_vid_start = 0;
-
- if (!filter_vlan)
- print_vlan_port(fp, ifm->ifi_index);
-
- for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
- struct bridge_vlan_info *vinfo;
- int vcheck_ret;
-
- if (i->rta_type != IFLA_BRIDGE_VLAN_INFO)
- continue;
-
- vinfo = RTA_DATA(i);
-
- if (!(vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END))
- last_vid_start = vinfo->vid;
- vcheck_ret = filter_vlan_check(vinfo);
- if (vcheck_ret == -1)
- break;
- else if (vcheck_ret == 0)
- continue;
-
- if (filter_vlan)
- print_vlan_port(fp, ifm->ifi_index);
- if (jw_global) {
- jsonw_start_object(jw_global);
- jsonw_uint_field(jw_global, "vlan",
- last_vid_start);
- if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN)
- continue;
- } else {
- fprintf(fp, "\t %hu", last_vid_start);
- }
- if (last_vid_start != vinfo->vid) {
- if (jw_global)
- jsonw_uint_field(jw_global, "vlanEnd",
- vinfo->vid);
- else
- fprintf(fp, "-%hu", vinfo->vid);
- }
- if (vinfo->flags & BRIDGE_VLAN_INFO_PVID) {
- if (jw_global) {
- start_json_vlan_flags_array(&vlan_flags);
- jsonw_string(jw_global, "PVID");
- } else {
- fprintf(fp, " PVID");
- }
- }
- if (vinfo->flags & BRIDGE_VLAN_INFO_UNTAGGED) {
- if (jw_global) {
- start_json_vlan_flags_array(&vlan_flags);
- jsonw_string(jw_global,
- "Egress Untagged");
- } else {
- fprintf(fp, " Egress Untagged");
- }
- }
- if (jw_global && vlan_flags) {
- jsonw_end_array(jw_global);
- vlan_flags = false;
- }
-
- if (jw_global)
- jsonw_end_object(jw_global);
- else
- fprintf(fp, "\n");
- }
+ print_vlan_info(fp, tb[IFLA_AF_SPEC], ifm->ifi_index);
}
if (!filter_vlan) {
if (jw_global)
@@ -470,6 +401,80 @@ static int vlan_show(int argc, char **argv)
return 0;
}
+void print_vlan_info(FILE *fp, struct rtattr *tb, int ifindex)
+{
+ struct rtattr *i, *list = tb;
+ int rem = RTA_PAYLOAD(list);
+ __u16 last_vid_start = 0;
+ bool vlan_flags = false;
+
+ if (!filter_vlan)
+ print_vlan_port(fp, ifindex);
+
+ for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
+ struct bridge_vlan_info *vinfo;
+ int vcheck_ret;
+
+ if (i->rta_type != IFLA_BRIDGE_VLAN_INFO)
+ continue;
+
+ vinfo = RTA_DATA(i);
+
+ if (!(vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END))
+ last_vid_start = vinfo->vid;
+ vcheck_ret = filter_vlan_check(vinfo);
+ if (vcheck_ret == -1)
+ break;
+ else if (vcheck_ret == 0)
+ continue;
+
+ if (filter_vlan)
+ print_vlan_port(fp, ifindex);
+ if (jw_global) {
+ jsonw_start_object(jw_global);
+ jsonw_uint_field(jw_global, "vlan",
+ last_vid_start);
+ if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN)
+ continue;
+ } else {
+ fprintf(fp, "\t %hu", last_vid_start);
+ }
+ if (last_vid_start != vinfo->vid) {
+ if (jw_global)
+ jsonw_uint_field(jw_global, "vlanEnd",
+ vinfo->vid);
+ else
+ fprintf(fp, "-%hu", vinfo->vid);
+ }
+ if (vinfo->flags & BRIDGE_VLAN_INFO_PVID) {
+ if (jw_global) {
+ start_json_vlan_flags_array(&vlan_flags);
+ jsonw_string(jw_global, "PVID");
+ } else {
+ fprintf(fp, " PVID");
+ }
+ }
+ if (vinfo->flags & BRIDGE_VLAN_INFO_UNTAGGED) {
+ if (jw_global) {
+ start_json_vlan_flags_array(&vlan_flags);
+ jsonw_string(jw_global,
+ "Egress Untagged");
+ } else {
+ fprintf(fp, " Egress Untagged");
+ }
+ }
+ if (jw_global && vlan_flags) {
+ jsonw_end_array(jw_global);
+ vlan_flags = false;
+ }
+
+ if (jw_global)
+ jsonw_end_object(jw_global);
+ else
+ fprintf(fp, "\n");
+ }
+}
+
int do_vlan(int argc, char **argv)
{
ll_init_map(&rth);
--
1.9.1
^ permalink raw reply related
* [PATCH iproute2 0/3] Process IFLA_BRIDGE_VLAN_INFO tlv
From: Roman Mashak @ 2017-09-08 21:52 UTC (permalink / raw)
To: stephen; +Cc: netdev, jhs, Roman Mashak
Process IFLA_BRIDGE_VLAN_INFO attribute nested in IFLA_AF_SPEC, which
contains bridge vlan table per port passed in link events.
Roman Mashak (3):
bridge: isolate vlans parsing code in a separate API
bridge: dump vlan table information for link
bridge: request vlans along with link information
bridge/br_common.h | 1 +
bridge/link.c | 23 +++++++--
bridge/vlan.c | 145 +++++++++++++++++++++++++++--------------------------
3 files changed, 96 insertions(+), 73 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
From: Pavel Machek @ 2017-09-08 21:50 UTC (permalink / raw)
To: Andrew Lunn
Cc: Tristram.Ha, muvarov, nathan.leigh.conrad, vivien.didelot,
f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170908190122.GM25219@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]
On Fri 2017-09-08 21:01:22, Andrew Lunn wrote:
> > > So i would suggest one driver supporting all the different devices.
> >
> > There will be 5 drivers to support these devices:
> >
> > ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477
> > ksz8795.c - KSZ8795/KSZ8795/KSZ8765
> > ksz8895.c - KSZ8895/KSZ8864
> > ksz8863.c - KSZ8863/KSZ8873
> > ksz8463.c - KSZ8463
> >
> > These chips have different SPI access mechanisms, MIB counter reading,
> > and register set. These can be combined into one single driver using
> > function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463. My
> > only concern is the memory footprint. The customer may not want a
> > big driver to cover all the switches while only one is used.
>
> If memory footprint is your problem, make it a compile time choice
> which devices are supported within the one driver. In practice, you
> will find most distributions just enable them all.
I have to side with Tristram here. The register layouts are so
different that single driver does not make sense.
What could make sense is single function, compiled 5 times, based on
different includes; same source code but 5 different binaries.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket
From: David Miller @ 2017-09-08 21:44 UTC (permalink / raw)
To: eduval
Cc: dwmw2, vallish, shuah, richardcochran, xiyou.wangcong, netdev,
linux-kernel, anchalag, dwmw
In-Reply-To: <20170908185521.GA12340@u40b0340c692b58f6553c.ant.amazon.com>
From: Eduardo Valentin <eduval@amazon.com>
Date: Fri, 8 Sep 2017 11:55:21 -0700
> I agree. I would prefer to understand here what is the technical
> reason not to accept these patches other than "use other system
> calls".
I explained this, let me reiterate:
====================
And most importantly, letting the kernel have flexibility in this area
is absolutely essential for various forms of optimizations and power
savings.
====================
^ permalink raw reply
* Re: [PATCH iproute2] tc: fq: support low_rate_threshold attribute
From: Soheil Hassas Yeganeh @ 2017-09-08 21:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: Stephen Hemminger, netdev, Neal Cardwell, Soheil Hassas Yeganeh
In-Reply-To: <1504905179.15310.101.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, Sep 8, 2017 at 5:12 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> TCA_FQ_LOW_RATE_THRESHOLD sch_fq attribute was added in linux-4.9
>
> Tested:
>
> lpaa5:/tmp# tc -qd add dev eth1 root fq
> lpaa5:/tmp# tc -s qd sh dev eth1
> qdisc fq 8003: root refcnt 5 limit 10000p flow_limit 1000p buckets 4096 \
> orphan_mask 4095 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3648 \
> initial_quantum 18240 low_rate_threshold 550Kbit refill_delay 40.0ms
> Sent 62139 bytes 395 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> 116 flows (114 inactive, 0 throttled)
> 1 gc, 0 highprio, 0 throttled
>
> lpaa5:/tmp# ./netperf -H lpaa6 -t TCP_RR -l10 -- -q 500000 -r 300,300 -o P99_LATENCY
> 99th Percentile Latency Microseconds
> 7081
>
> lpaa5:/tmp# tc qd replace dev eth1 root fq low_rate_threshold 10Mbit
> lpaa5:/tmp# ./netperf -H lpaa6 -t TCP_RR -l10 -- -q 500000 -r 300,300 -o P99_LATENCY
> 99th Percentile Latency Microseconds
> 858
>
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Thank you, Eric!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox