* 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
* 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 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 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] 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
* [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: [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
* 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 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
* [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: [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
* 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] 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] 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] 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-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: 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] 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
* [PATCH net] bpf: make error reporting in bpf_warn_invalid_xdp_action more clear
From: Daniel Borkmann @ 2017-09-08 23:40 UTC (permalink / raw)
To: davem; +Cc: ast, john.fastabend, netdev, Daniel Borkmann
Differ between illegal XDP action code and just driver
unsupported one to provide better feedback when we throw
a one-time warning here. Reason is that with 814abfabef3c
("xdp: add bpf_redirect helper function") not all drivers
support the new XDP return code yet and thus they will
fall into their 'default' case when checking for return
codes after program return, which then triggers a
bpf_warn_invalid_xdp_action() stating that the return
code is illegal, but from XDP perspective it's not.
I decided not to place something like a XDP_ACT_MAX define
into uapi i) given we don't have this either for all other
program types, ii) future action codes could have further
encoding there, which would render such define unsuitable
and we wouldn't be able to rip it out again, and iii) we
rarely add new action codes.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/uapi/linux/bpf.h | 4 ++--
net/core/filter.c | 6 +++++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ba848b7..43ab5c4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -766,8 +766,8 @@ struct bpf_sock {
/* User return codes for XDP prog type.
* A valid XDP program must return one of these defined values. All other
- * return codes are reserved for future use. Unknown return codes will result
- * in packet drop.
+ * return codes are reserved for future use. Unknown return codes will
+ * result in packet drops and a warning via bpf_warn_invalid_xdp_action().
*/
enum xdp_action {
XDP_ABORTED = 0,
diff --git a/net/core/filter.c b/net/core/filter.c
index 0848df2..adac4eb 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3609,7 +3609,11 @@ static bool xdp_is_valid_access(int off, int size,
void bpf_warn_invalid_xdp_action(u32 act)
{
- WARN_ONCE(1, "Illegal XDP return value %u, expect packet loss\n", act);
+ const u32 act_max = XDP_REDIRECT;
+
+ WARN_ONCE(1, "%s XDP return value %u, expect packet loss!\n",
+ act > act_max ? "Illegal" : "Driver unsupported",
+ act);
}
EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action);
--
1.9.3
^ 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 23: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: <03b61877-053b-2f0e-dc35-8fe31cc90c08@cumulusnetworks.com>
On Fri, Sep 8, 2017 at 7:30 AM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 08/09/17 17:17, Kosuke Tatsukawa wrote:
>> Hi,
>>
>>> On 08/09/17 13:10, Nikolay Aleksandrov wrote:
>>>> On 08/09/17 05:06, Kosuke Tatsukawa wrote:
>>>>> 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
>>>>>
>>>>
>>>> Logically the approach looks good, that being said it adds unnecessary tests in
>>>> the fast path, why not just something like the patch below ? That only leaves the
>>>> problem if it is zeroed in TLB and switched to ALB mode, and that is a one line
>>>> fix to unsuppmodes just allow it to be set for that specific case. The below
>>>> returns the default behaviour before the commit in your Fixes tag.
>>>>
>>>>
>>>
>>> Actually I'm fine with your approach, too. It will fix this regardless of the
>>> value of tlb_dynamic_lb which sounds good to me for the price of a test in
>>> the fast path.
>>
>> If you're concerned about the additional test in the fast path, how
>> about the patch below. I've added an arguemnt to bond_do_alb_xmit()
>> to handle both balance-tlb and balance-alb similary.
>>
>
> Even better, looks great! 1 question below though.
>
>> I'm not sure if this causes any problem if tlb_dynamic_lb is changed
>> while calling bond_do_alb_xmit() in balance-tlb mode.
>
> The option has the ifdown flag, you shouldn't be able to change it while
> the bond dev is up, but even if you could I don't think it will be an issue
> for the xmit.
>
>> ---
>> Kosuke TATSUKAWA | 1st Platform Software Division
>> | NEC Solution Innovators
>> | tatsu@ab.jp.nec.com
>>
>> ------------------------------------------------------------------------
>> drivers/net/bonding/bond_alb.c | 11 ++++++-----
>> drivers/net/bonding/bond_main.c | 5 +++--
>> 2 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index c02cc81..7710f20 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -1317,7 +1317,7 @@ void bond_alb_deinitialize(struct bonding *bond)
>> }
>>
>> static int bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>> - struct slave *tx_slave)
>> + struct slave *tx_slave, int tlb_dynamic_lb)
>> {
>> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>> struct ethhdr *eth_data = eth_hdr(skb);
>> @@ -1325,7 +1325,7 @@ 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 (tlb_dynamic_lb)
>> bond_info->unbalanced_load += skb->len;
>> }
>>
>> @@ -1339,7 +1339,7 @@ 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 && tlb_dynamic_lb) {
>> spin_lock(&bond->mode_lock);
>> __tlb_clear_slave(bond, tx_slave, 0);
>> spin_unlock(&bond->mode_lock);
>> @@ -1386,7 +1386,8 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>> break;
>> }
>> }
>> - return bond_do_alb_xmit(skb, bond, tx_slave);
>> + return bond_do_alb_xmit(skb, bond, tx_slave,
>> + bond->params.tlb_dynamic_lb);
>> }
>>
>> int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>> @@ -1483,7 +1484,7 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>> tx_slave = tlb_choose_channel(bond, hash_index, skb->len);
>> }
>>
>> - return bond_do_alb_xmit(skb, bond, tx_slave);
>> + return bond_do_alb_xmit(skb, bond, tx_slave, 1);
>> }
>>
>> void bond_alb_monitor(struct work_struct *work)
>> 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_TLB))
>
> mode == tlb ? shouldn't this check be for alb ?
>
Actually this is not needed since this is already inside if
(bond_is_lb()) condition.
>> 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);
>>
>
I think the underlying issue is that tlb_dynamic_lb should be set to 1
for all modes which was not the case when it was getting initialized
only forTLB mode. So from that perspective I prefer Nik's patch with a
small variation that guards the case when mode transitions from TLB to
ALB. The reason why I like that patch is because it's simple and
avoids complications.
Here is what I meant -
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fc63992ab0e0..c99dc59d729b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4289,7 +4289,7 @@ static int bond_check_params(struct bond_params *params)
int bond_mode = BOND_MODE_ROUNDROBIN;
int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
int lacp_fast = 0;
- int tlb_dynamic_lb = 0;
+ int tlb_dynamic_lb;
/* Convert string parameters. */
if (mode) {
@@ -4601,16 +4601,13 @@ 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)) {
- bond_opt_initstr(&newval, "default");
- valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB),
- &newval);
- if (!valptr) {
- pr_err("Error: No tlb_dynamic_lb default value");
- return -EINVAL;
- }
- tlb_dynamic_lb = valptr->value;
+ bond_opt_initstr(&newval, "default");
+ valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB), &newval);
+ if (!valptr) {
+ pr_err("Error: No tlb_dynamic_lb default value");
+ return -EINVAL;
}
+ tlb_dynamic_lb = valptr->value;
if (lp_interval == 0) {
pr_warn("Warning: ip_interval must be between 1 and
%d, so it was reset to %d\n",
diff --git a/drivers/net/bonding/bond_options.c
b/drivers/net/bonding/bond_options.c
index a12d603d41c6..7feacd262182 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -754,6 +754,12 @@ static int bond_option_mode_set(struct bonding *bond,
bond->params.miimon);
}
+ /* Guard against transition TLB/tlb_dynamic_lb=0 -> ALB mode.
+ * In ALB mode, tlb_dynamic_lb must be set to 1.
+ */
+ if (newval->value == BOND_MODE_ALB && bond->params.tlb_dynamic_lb != 1)
+ bond->params.tlb_dynamic_lb = 1;
+
/* don't cache arp_validate between modes */
bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
bond->params.mode = newval->value;
^ permalink raw reply related
* RE: [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.
From: Tristram.Ha @ 2017-09-09 1:10 UTC (permalink / raw)
To: pavel
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170908085405.GB16834@amd>
> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Friday, September 08, 2017 1:54 AM
> To: Tristram Ha - C24268
> Cc: andrew@lunn.ch; muvarov@gmail.com; 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 0/6] Modify KSZ9477 DSA driver in preparation to
> add other KSZ switch drivers.
>
> Hi!
>
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > This series of patches is to modify the original KSZ9477 DSA driver so that
> other KSZ switch drivers can be added and use the common code.
> >
>
> Please wrap the lines from time to time...
>
>
> > This patch set is against net-next.
> >
> > drivers/net/dsa/microchip/Makefile | 2 +-
> > drivers/net/dsa/microchip/ksz9477.c | 1317
> ++++++++++++++++++++++++++++++++
>
> We already have ksz_9477_reg.h. So should this be ksz_9477.c for
> consistency?
The product name is KSZ9477 and other switches are also like KSZ####,
so I would prefer to have no separation between KSZ and the product
number. I think the file ksz_9477_reg.h was named that way because
the other files were named ksz_common.c and ksz_spi.c. If need to
we can change the file name.
^ permalink raw reply
* Re: [PATCH] ipv4: Namespaceify tcp_max_orphans knob
From: 严海双 @ 2017-09-09 1:25 UTC (permalink / raw)
To: Cong Wang
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Eric Dumazet, Linux Kernel Network Developers, LKML
In-Reply-To: <CAM_iQpV4gRwhT4VXPtJft38ixG6xE3PF9z4gB7HHJEUsqbCtOw@mail.gmail.com>
> On 2017年9月9日, at 上午6:13, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> 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.
>
>From my understanding, before the patch, we had N * net->ipv4.sysctl_tcp_max_orphans,
and after the patch, we could have ns1.sysctl_tcp_max_orphans + ns2.sysctl_tcp_max_orphans
+ ns3.sysctl_tcp_max_orphans, is that right? Thanks for your reviewing.
^ permalink raw reply
* RE: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Tristram.Ha @ 2017-09-09 1:44 UTC (permalink / raw)
To: pavel
Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170908215735.GF27428@amd>
> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Friday, September 08, 2017 2:58 PM
> To: Tristram Ha - C24268
> Cc: andrew@lunn.ch; muvarov@gmail.com; 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 3/5] Add KSZ8795 switch driver
>
> 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.
The exact function probably cannot be shared between KSZ8795
and KSZ8895 drivers because although the register constants look
the same but their exact locations are different in the 2 chips.
^ permalink raw reply
* Re: [PATCH 1/2] ip_tunnel: fix setting ttl and tos value in collect_md mode
From: Alexei Starovoitov @ 2017-09-09 1:54 UTC (permalink / raw)
To: Haishuang Yan, David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1504764515-13536-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
On 9/6/17 11:08 PM, Haishuang Yan wrote:
> ttl and tos variables are declared and assigned, but are not used in
> iptunnel_xmit() function.
>
> Fixes: cfc7381b3002 ("ip_tunnel: add collect_md mode to IPIP tunnel")
> Cc: Alexei Starovoitov <ast@fb.com>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> ---
> net/ipv4/ip_tunnel.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 129d1a3..e1856bf 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -618,8 +618,8 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, u8 proto)
> ip_rt_put(rt);
> goto tx_dropped;
> }
> - iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, proto, key->tos,
> - key->ttl, df, !net_eq(tunnel->net, dev_net(dev)));
> + iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, proto, tos, ttl,
> + df, !net_eq(tunnel->net, dev_net(dev)));
indeed. good catch. thanks
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions
From: Jassi Brar @ 2017-09-09 3:33 UTC (permalink / raw)
To: Florian Fainelli
Cc: Kunihiko Hayashi, netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
Andrew Lunn, Rob Herring, Mark Rutland,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
lkml, Devicetree List, Masahiro Yamada, Masami Hiramatsu,
Jongsung Kim
In-Reply-To: <4173a876-3cff-205a-ce87-ce049f419aa0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 9 September 2017 at 00:21, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
>> From: Jassi Brar <jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> Add RTL8201F phy-id and the related functions to the driver.
>>
>> The original patch is as follows:
>> https://patchwork.kernel.org/patch/2538341/
>>
>> Signed-off-by: Jongsung Kim <neidhard.kim-Hm3cg6mZ9cc@public.gmane.org>
>> Signed-off-by: Jassi Brar <jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
>> ---
>> drivers/net/phy/realtek.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 45 insertions(+)
>>
>> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
>> index 9cbe645..d9974ce 100644
>> --- a/drivers/net/phy/realtek.c
>> +++ b/drivers/net/phy/realtek.c
>> @@ -29,10 +29,23 @@
>> #define RTL8211F_PAGE_SELECT 0x1f
>> #define RTL8211F_TX_DELAY 0x100
>>
>> +#define RTL8201F_ISR 0x1e
>> +#define RTL8201F_PAGE_SELECT 0x1f
>
> We have a page select register define for the RTL8211F right above, so
> surely we can make that a common definition?
>
That is just for the sake of consistency.
I mean RTL8211 wouldn't look neat among everything else RTL8201.
Also the page-select offsets just _happen_ to be same value...
RTL8211E_INER_LINK_STATUS and RTL8211F_INER_LINK_STATUS are very
different.
>> +#define RTL8201F_IER 0x13
>> +
>> MODULE_DESCRIPTION("Realtek PHY driver");
>> MODULE_AUTHOR("Johnson Leung");
>> MODULE_LICENSE("GPL");
>>
>> +static int rtl8201_ack_interrupt(struct phy_device *phydev)
>> +{
>> + int err;
>> +
>> + err = phy_read(phydev, RTL8201F_ISR);
>> +
>> + return (err < 0) ? err : 0;
>> +}
>> +
>> static int rtl821x_ack_interrupt(struct phy_device *phydev)
>> {
>> int err;
>> @@ -54,6 +67,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
>> return (err < 0) ? err : 0;
>> }
>>
>> +static int rtl8201_config_intr(struct phy_device *phydev)
>> +{
>> + int err;
>> +
>> + /* switch to page 7 */
>> + phy_write(phydev, RTL8201F_PAGE_SELECT, 0x7);
>> +
>> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
>> + err = phy_write(phydev, RTL8201F_IER,
>> + BIT(13) | BIT(12) | BIT(11));
>
> Can you detail what bits 11, 12 and 13 do? Do they correspond to link,
> duplex and pause changes by any chance?
>
Sorry no idea. The datasheet would say, and other functions too use
such magic values.
>> + else
>> + err = phy_write(phydev, RTL8201F_IER, 0);
>> +
>> + /* restore to default page 0 */
>> + phy_write(phydev, RTL8201F_PAGE_SELECT, 0x0);
>> +
>> + return err;
>> +}
>> +
>
> Other than that, LGTM:
>
> Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
Thank you.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
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