* Re: [PATCH 1/3] ipv4: icmp: use icmp_sk_exit()
From: Eric Dumazet @ 2019-02-22 17:11 UTC (permalink / raw)
To: Kefeng Wang, davem, netdev
In-Reply-To: <20190222015800.192135-1-wangkefeng.wang@huawei.com>
On 02/21/2019 05:57 PM, Kefeng Wang wrote:
> Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> net/ipv4/icmp.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 065997f414e6..364cfe5e414b 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
> return 0;
>
> fail:
> - for_each_possible_cpu(i)
> - inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
> - free_percpu(net->ipv4.icmp_sk);
> + icmp_sk_exit(net);
> return err;
> }
>
>
I do not like this. Future changes in icmp_sk_exit() might trigger a bug in this seldom tested path.
^ permalink raw reply
* Re: [PATCH 1/3] ipv4: icmp: use icmp_sk_exit()
From: Eric Dumazet @ 2019-02-22 17:16 UTC (permalink / raw)
To: Kefeng Wang, davem, netdev
In-Reply-To: <bb408779-5ddb-f9ef-e16b-1d0d7a5c3719@gmail.com>
On 02/22/2019 09:11 AM, Eric Dumazet wrote:
>
>
> On 02/21/2019 05:57 PM, Kefeng Wang wrote:
>> Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> net/ipv4/icmp.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> index 065997f414e6..364cfe5e414b 100644
>> --- a/net/ipv4/icmp.c
>> +++ b/net/ipv4/icmp.c
>> @@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
>> return 0;
>>
>> fail:
>> - for_each_possible_cpu(i)
>> - inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
>> - free_percpu(net->ipv4.icmp_sk);
>> + icmp_sk_exit(net);
>> return err;
>> }
>>
>>
>
>
> I do not like this. Future changes in icmp_sk_exit() might trigger a bug in this seldom tested path.
>
Oh I see the next patches, now it makes sense.
Please send a proper patch series, with a cover letter, so that it is obvious.
Also you need to include "net-next" instead of plain [PATCH] to express this targets net-next tree.
Carefully read Documentation/networking/netdev-FAQ.rst
Thank you.
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Anand Moon @ 2019-02-22 17:21 UTC (permalink / raw)
To: Simon Huelck
Cc: Jerome Brunet, Jose Abreu, Martin Blumenstingl, linux-amlogic,
netdev, Alexandre TORGUE, Emiliano Ingrassia, Gpeppe.cavallaro
In-Reply-To: <e009cbbb-e31d-2438-e454-2fa10ebec542@gmx.de>
Hi All,
On Fri, 22 Feb 2019 at 01:05, Simon Huelck <simonmail@gmx.de> wrote:
>
> Am 21.02.2019 um 18:46 schrieb Jerome Brunet:
> > On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
> >> Hi,
> >>
> >>
> >>
> >> this was changed recently, with a patch for the EEE stuff , see here:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
> > Hu, I was not aware this finally went through. Good !
> > As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
> >
> > The realtek PHY has EEE enabled by default. Having this enabled generates a
> > lot of (Low Power) Interrupts.
> >
> > Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
> > eventually miss an IRQ and the interface would just die. Unfortunately, it was
> > not that easy find out.
> >
> > 2 years ago, we just noticed that disabling EEE would make the failure go
> > away. Forcing this EEE feature off through DT was merely a work around.
> >
> > Now that the real cause of the problem is known, there is no reason to keep
> > this hack around.
> >
> > Whether EEE adds a performance penality and why, is another topic.
> > As Jose pointed out, you can disable EEE at runtime, using ethtool.
> >
> > Jerome
> >
> Hi,
>
>
>
> i disabled EEE via ethtool and via the .dtb , but the performance
> penalty stays. Kernel 4.14 still gives me the former "good" performance.
>
>
>
> regards,
>
> Simon
>
>
Sorry this is off the topic.
I am using Archlinux on Odroid C1+ and the latest kernel loads with no issue.
only issue is I have is the each time their is random MAC address so I
get new IP from dhcp server.
How can I avoid this. I have tried to enable eFuse driver but with no success.
On Odroid C2 I dont have any issue, it always retained the unique MAC address.
Best Regards
-Anand
^ permalink raw reply
* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Michal Kubecek @ 2019-02-22 17:27 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222090204.20c410e3@cakuba.netronome.com>
On Fri, Feb 22, 2019 at 09:02:04AM -0800, Jakub Kicinski wrote:
> On Fri, 22 Feb 2019 11:04:50 +0100, Michal Kubecek wrote:
> > On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
> > > Support getting devlink instance from a new NDO.
> > >
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > ---
> > > drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
> > > drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
> > > drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
> > > drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
> > > 4 files changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > index d578d856a009..f8d422713705 100644
> > > --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > @@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
> > > int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
> > > struct nfp_net *nn, unsigned int id);
> > >
> > > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
> > > +
> > > #endif
> > > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > index db2da99f6aa7..e9eca99cf493 100644
> > > --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > @@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
> > > {
> > > devlink_port_unregister(&port->dl_port);
> > > }
> > > +
> > > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
> > > +{
> > > + struct nfp_app *app;
> > > +
> > > + app = nfp_app_from_netdev(netdev);
> > > + if (!app)
> > > + return NULL;
> > > +
> > > + return priv_to_devlink(app->pf);
> > > +}
> >
> > AFAICS this would return a pointer to zero initialized struct devlink
> > when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
> > would execute
> >
> > if (!dev->netdev_ops->ndo_get_devlink)
> > return;
> >
> > devlink = dev->netdev_ops->ndo_get_devlink(dev);
> > if (!devlink || !devlink->ops->info_get)
> > return;
> >
> > with non-null devlink but null devlink->ops so that it dereferences null
> > pointer (and so does devlink_compat_flash_update()).
>
> devlink_compat_flash_update() is this if CONFIG_DEVLINK=n:
>
> static inline int
> devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> {
> return -EOPNOTSUPP;
> }
>
> No?
You are right, I missed that devlink_compat_running_version() and
devlink_compat_flash_update() also have alternative versions when
devlink is disabled. So there is no problem.
> > Maybe it would be safer not to call ndo_get_devlink directly and have
> > an inline wrapper like
> >
> > #if IS_ENABLED(CONFIG_NET_DEVLINK)
> > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > {
> > if (dev->netdev_ops->ndo_get_devlink)
> > return dev->netdev_ops->ndo_get_devlink();
> > else
> > retrurn NULL;
> > }
> > #else
> > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > {
> > return NULL;
> > }
> > #endif
> >
> > so that one can simply call the wrapper and check return value for NULL.
>
> Only devlink code can call this ndo, and it doesn't exist with
> DEVLINK=n. I don't dislike wrappers for NDOs, but I'll defer to Jiri
> to decide if we want a wrapper here (without the #if/#else, just the
> first part for code clarity) :)
If the NDO is only supposed to be called from devlink code (or, more
precisely, code built only with CONFIG_DEVLINK=y), it should be IMHO
mentioned in its description. Another option would be enforcing it by
adding #ifdef around the ndo_get_devlink entry in struct net_device_ops
but that would require using ifdefs also in each driver providing the
NDO which seems inconvenient.
Michal
^ permalink raw reply
* Re: [PATCH net-next 0/8] Add tests for unlocked flower classifier implementation
From: Cong Wang @ 2019-02-22 17:35 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, David Miller,
shuah, batuhanosmantaskaya, Lucas Bates, Davide Caratti,
Marcelo Ricardo Leitner, Chris Mi
In-Reply-To: <20190222140047.13215-1-vladbu@mellanox.com>
On Fri, Feb 22, 2019 at 6:00 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Implement tests for tdc testsuite to verify concurrent rules update with
> rtnl-unlocked flower classifier implementation. The goal of these tests
> is to verify general flower classifier correctness by updating filters
> on same classifier instance in parallel and to verify its atomicity by
> concurrently updating filters in same handle range. All three filter
> update operations (add, replace, delete) are tested.
>
> Existing script tdc_batch.py is re-used for batch file generation. It is
> extended with several optional CLI arguments that are needed for
> concurrency tests. Thin wrapper tdc_multibatch.py is implemented on top
> of tdc_batch.py to simplify its usage when generating multiple batch
> files for several test configurations.
>
> Parallelism in tests is implemented by running multiple instances of tc
> in batch mode with xargs tool. Xargs is chosen for its ease of use and
> because it is available by default on most modern Linux distributions.
I don't look into the details, but I love to see we have tests for parallel
filter updates! So for the whole patchset:
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks!
^ permalink raw reply
* [PATCH v2 0/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:39 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>
The original discussion is here:
https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
Changes in v2:
- original patch divided on 2 parts: add __icmp_send function, fix cipso_v4_error
- add __ip_option_compile not using icmp_send in case nof error
---
include/net/icmp.h | 9 ++++++++-
include/net/ip.h | 2 ++
net/ipv4/cipso_ipv4.c | 17 +++++++++++++++--
net/ipv4/icmp.c | 7 ++++---
net/ipv4/ip_options.c | 22 +++++++++++++++++-----
5 files changed, 46 insertions(+), 11 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:43 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>
Add __icmp_send function having ip_options struct parameter
---
include/net/icmp.h | 9 ++++++++-
net/ipv4/icmp.c | 7 ++++---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/net/icmp.h b/include/net/icmp.h
index 6ac3a5b..e0f709d 100644
--- a/include/net/icmp.h
+++ b/include/net/icmp.h
@@ -22,6 +22,7 @@
#include <net/inet_sock.h>
#include <net/snmp.h>
+#include <net/ip.h>
struct icmp_err {
int errno;
@@ -39,7 +40,13 @@ struct icmp_err {
struct sk_buff;
struct net;
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info);
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+ const struct ip_options *opt);
+static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+{
+ __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
+}
+
int icmp_rcv(struct sk_buff *skb);
int icmp_err(struct sk_buff *skb, u32 info);
int icmp_init(void);
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 065997f..3f24414 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
* MUST reply to only the first fragment.
*/
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+ const struct ip_options *opt)
{
struct iphdr *iph;
int room;
@@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
iph->tos;
mark = IP4_REPLY_MARK(net, skb_in->mark);
- if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
+ if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
goto out_unlock;
@@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
local_bh_enable();
out:;
}
-EXPORT_SYMBOL(icmp_send);
+EXPORT_SYMBOL(__icmp_send);
static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
---
^ permalink raw reply related
* Re: [PATCH net-next v3 3/4] enetc: Add ENETC PF level external MDIO support
From: Andrew Lunn @ 2019-02-22 17:44 UTC (permalink / raw)
To: Claudiu Manoil
Cc: Shawn Guo, Li Yang, David S . Miller, devicetree,
alexandru.marginean, linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1550847859-17346-4-git-send-email-claudiu.manoil@nxp.com>
On Fri, Feb 22, 2019 at 05:04:18PM +0200, Claudiu Manoil wrote:
> Each ENETC PF has its own MDIO interface, the corresponding
> MDIO registers are mapped in the ENETC's Port register block.
> The current patch adds a driver for these PF level MDIO buses,
> so that each PF can manage directly its own external link.
>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Dumazet @ 2019-02-22 17:45 UTC (permalink / raw)
To: Eric Biggers, netdev, David S . Miller
Cc: linux-kernel, Mao Wenan, Cong Wang, Lorenzo Colitti, Tetsuo Handa,
Al Viro
In-Reply-To: <20190221221356.173485-1-ebiggers@kernel.org>
On 02/21/2019 02:13 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> closed concurrently with fchownat(). However, it ignored that many
> other proto_ops::release() methods don't set sock->sk to NULL and
> therefore allow the same use-after-free:
>
I fail to see how setting a pointer to NULL can avoid races.
We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
while sockfs_setattr() is used.
sockfs_setattr()
...
if (sock->sk)
// even if sock->sk was not NULL for the if (...).
// it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.
sock->sk->sk_uid = iattr->ia_uid;
> - base_sock_release
> - bnep_sock_release
> - cmtp_sock_release
> - data_sock_release
> - dn_release
> - hci_sock_release
> - hidp_sock_release
> - iucv_sock_release
> - l2cap_sock_release
> - llcp_sock_release
> - llc_ui_release
> - rawsock_release
> - rfcomm_sock_release
> - sco_sock_release
> - svc_release
> - vcc_release
> - x25_release
>
> Rather than fixing all these and relying on every socket type to get
> this right forever, just make __sock_release() set sock->sk to NULL
> itself after calling proto_ops::release().
>
> Reproducer that produces the KASAN splat when any of these socket types
> are configured into the kernel:
>
> #include <pthread.h>
> #include <stdlib.h>
> #include <sys/socket.h>
> #include <unistd.h>
>
> pthread_t t;
> volatile int fd;
>
> void *close_thread(void *arg)
> {
> for (;;) {
> usleep(rand() % 100);
> close(fd);
> }
> }
>
> int main()
> {
> pthread_create(&t, NULL, close_thread, NULL);
> for (;;) {
> fd = socket(rand() % 50, rand() % 11, 0);
> fchownat(fd, "", 1000, 1000, 0x1000);
> close(fd);
> }
> }
>
> Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>
> NOTE: I am not an expert in the networking code, so please carefully
> check that I haven't missed some reason why this simple fix won't do.
>
> net/socket.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/socket.c b/net/socket.c
> index d80d87a395ea..320f51b22b19 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -577,6 +577,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
> if (inode)
> inode_lock(inode);
> sock->ops->release(sock);
> + sock->sk = NULL;
> if (inode)
> inode_unlock(inode);
> sock->ops = NULL;
>
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Russell King - ARM Linux admin @ 2019-02-22 17:48 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Vivien Didelot
In-Reply-To: <20190217195524.rhc4rsvzsalptyua@shell.armlinux.org.uk>
On Sun, Feb 17, 2019 at 07:55:24PM +0000, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 07:11:43PM +0100, Andrew Lunn wrote:
> > The following false positive lockdep splat has been observed.
> >
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 4.20.0+ #302 Not tainted
> > ------------------------------------------------------
> > systemd-udevd/160 is trying to acquire lock:
> > edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> >
> > but task is already holding lock:
> > edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> >
> > which lock already depends on the new lock.
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #1 (&desc->request_mutex){+.+.}:
> > mutex_lock_nested+0x1c/0x24
> > __setup_irq+0xa0/0x704
> > request_threaded_irq+0xd0/0x150
> > mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
> > mdio_probe+0x2c/0x54
> > really_probe+0x200/0x2c4
> > driver_probe_device+0x5c/0x174
> > __driver_attach+0xd8/0xdc
> > bus_for_each_dev+0x58/0x7c
> > bus_add_driver+0xe4/0x1f0
> > driver_register+0x7c/0x110
> > mdio_driver_register+0x24/0x58
> > do_one_initcall+0x74/0x2e8
> > do_init_module+0x60/0x1d0
> > load_module+0x1968/0x1ff4
> > sys_finit_module+0x8c/0x98
> > ret_fast_syscall+0x0/0x28
> > 0xbedf2ae8
> >
> > -> #0 (&chip->reg_lock){+.+.}:
> > __mutex_lock+0x50/0x8b8
> > mutex_lock_nested+0x1c/0x24
> > __setup_irq+0x640/0x704
> > request_threaded_irq+0xd0/0x150
> > mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
> > mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
> > mdio_probe+0x2c/0x54
> > really_probe+0x200/0x2c4
> > driver_probe_device+0x5c/0x174
> > __driver_attach+0xd8/0xdc
> > bus_for_each_dev+0x58/0x7c
> > bus_add_driver+0xe4/0x1f0
> > driver_register+0x7c/0x110
> > mdio_driver_register+0x24/0x58
> > do_one_initcall+0x74/0x2e8
> > do_init_module+0x60/0x1d0
> > load_module+0x1968/0x1ff4
> > sys_finit_module+0x8c/0x98
> > ret_fast_syscall+0x0/0x28
> > 0xbedf2ae8
> >
> > other info that might help us debug this:
> >
> > Possible unsafe locking scenario:
> >
> > CPU0 CPU1
> > ---- ----
> > lock(&desc->request_mutex);
> > lock(&chip->reg_lock);
> > lock(&desc->request_mutex);
> > lock(&chip->reg_lock);
> >
> > &desc->request_mutex refer to two different mutex. #1 is the GPIO for
> > the chip interrupt. #2 is the chained interrupt between global 1 and
> > global 2.
> >
> > Add lockdep classes to the GPIO interrupt to avoid this.
> >
> > Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >
> > Hi Russell
> >
> > Does this fix it for you on Clearfog?
>
> Yes, that also fixes the problem, but I do think this is just papering
> over mv88e6xxx needlessly holding locks when it doesn't need to do so.
Hi Andrew,
Do we have a way forward for this issue?
Thanks.
>
> >
> > drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> > index 32e7af5caa69..936d53a92144 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -442,12 +442,20 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
> >
> > static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
> > {
> > + static struct lock_class_key lock_key;
> > + static struct lock_class_key request_key;
> > int err;
> >
> > err = mv88e6xxx_g1_irq_setup_common(chip);
> > if (err)
> > return err;
> >
> > + /* These lock classes tells lockdep that global 1 irqs are in
> > + * a different category than their parent GPIO, so it won't
> > + * report false recursion.
> > + */
> > + irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
> > +
> > err = request_threaded_irq(chip->irq, NULL,
> > mv88e6xxx_g1_irq_thread_fn,
> > IRQF_ONESHOT | IRQF_SHARED,
> > --
> > 2.20.1
> >
> >
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces
From: Stefano Brivio @ 2019-02-22 17:49 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <1d298b6839828c96cd8a0f50bdfac893cc645895.1550851038.git.pabeni@redhat.com>
On Fri, 22 Feb 2019 17:06:32 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> Otherwise, the configured IPv6 address could be still "tentative"
> at test time, possibly causing tests failures.
> We can also drop some sleep along the code and decrease the
> timeout for most commands so that the test runtime decreases.
Thanks for fixing this!
I tried this back then and it didn't work, perhaps some DAD changes
intervened meanwhile (or I simply tried it wrong).
Just one comment:
> Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index e2c94e47707c..634e91e8fe25 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -263,8 +263,6 @@ setup_fou_or_gue() {
>
> ${ns_a} ip link set ${encap}_a up
> ${ns_b} ip link set ${encap}_b up
> -
> - sleep 1
> }
>
> setup_fou44() {
> @@ -302,6 +300,10 @@ setup_gue66() {
> setup_namespaces() {
> for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
> ip netns add ${n} || return 1
> +
> + # disable dad, so that we don't have to wait to use the
> + # configured IPv6 addresses
For consistency: "Disable DAD ..."
--
Stefano
^ permalink raw reply
* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup
From: Stefano Brivio @ 2019-02-22 17:50 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <06a88ed3b2efab312f6301e9fe8eabd4383a9d3a.1550851038.git.pabeni@redhat.com>
On Fri, 22 Feb 2019 17:06:33 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> Add a couple of new tests, explicitly checking that the kernel
> timely releases pmtu exceptions on related device removal.
> This is mostly a regression test vs the issue fixed by:
> https://patchwork.ozlabs.org/patch/1045488/
>
> Only 2 new test cases have been added, instead of extending all
> the existing ones, because the reproducer requires executing
> several commands and would slow down too much the tests otherwise.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/pmtu.sh | 52 ++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 634e91e8fe25..4bc72bc26899 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -135,7 +135,9 @@ tests="
> pmtu_vti6_default_mtu vti6: default MTU assignment
> pmtu_vti4_link_add_mtu vti4: MTU setting on link creation
> pmtu_vti6_link_add_mtu vti6: MTU setting on link creation
> - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes"
> + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes
> + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions
> + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions"
>
> NS_A="ns-$(mktemp -u XXXXXX)"
> NS_B="ns-$(mktemp -u XXXXXX)"
> @@ -1006,6 +1008,54 @@ test_pmtu_vti6_link_change_mtu() {
> return ${fail}
> }
>
> +test_cleanup_vxlanY_or_geneveY_exception() {
> + outer="${1}"
> + encap="${2}"
As you don't implement GENEVE tests, this function can be kept simpler.
> + ll_mtu=4000
> +
> + which taskset 2>/dev/null | return 2
> + which timeout 2>/dev/null | return 2
This test will not be skipped if taskset or timeout are not available,
I guess you meant: || return 2.
Besides, 'which' can output to both stdout and stderr. See how it's
used anywhere else in this script, e.g.:
which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
You might also want to skip this test on non-SMP.
You should also indicate why the test is skipped, see how it's done in
other places, e.g.:
${ns_a} ip link add dummy0 mtu 1500 type dummy
[ $? -ne 0 ] && err " dummy not supported" && return 2
> + cpu_list=`grep processor /proc/cpuinfo | cut -d ' ' -f 2`
For consistency, please use $( ... )
If you grep -m2, then you can skip the ncpus thing below.
> +
> + setup namespaces routing ${encap}${outer} || return 2
> + trace "${ns_a}" ${encap}_a "${ns_b}" ${encap}_b \
> + "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \
> + "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B
> +
> + # Create route exception by exceeding link layer MTU
> + mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000))
> + mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
> + mtu "${ns_b}" veth_B-R1 ${ll_mtu}
> + mtu "${ns_r1}" veth_R1-B ${ll_mtu}
> +
> + mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
> + mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
> +
> + # fill exception cache for multiple cpus [2]
> + # we can always use inner ipv4 for that
Please keep comments consistent with the rest, "Fill ... CPUs (2) ... IPv4".
> + ncpus=0
> + for cpu in $cpu_list; do
Nit: ${cpu_list}
> + taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
> + ncpus=$((ncpus + 1))
> + [ ${ncpus} -gt 1 ] && break
> + done
> +
> + fail=0
No need for fail=0...
> + if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then
> + err " can't delete veth device in a timely manner, pmtu dst likely leaked"
(For consistency: "PMTU")
> + fail=1
...just return 1 here and
> + fi
> + return ${fail}
...skip the explicit return. The return code comes from the last
command executed.
> +}
> +
> +test_pmtu_ipv6_exception_cleanup() {
> + test_cleanup_vxlanY_or_geneveY_exception 6 vxlan
> +}
> +
> +test_pmtu_ipv4_exception_cleanup() {
> + test_cleanup_vxlanY_or_geneveY_exception 4 vxlan
> +}
> +
> usage() {
> echo
> echo "$0 [OPTIONS] [TEST]..."
--
Stefano
^ permalink raw reply
* Re: [PATCH v2 2/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:50 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>
Extract IP options in cipso_v4_error and use __icmp_send.
---
include/net/ip.h | 2 ++
net/ipv4/cipso_ipv4.c | 17 +++++++++++++++--
net/ipv4/ip_options.c | 22 +++++++++++++++++-----
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 8866bfc..f0e8d06 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -667,6 +667,8 @@ static inline int ip_options_echo(struct net *net, struct ip_options *dopt,
}
void ip_options_fragment(struct sk_buff *skb);
+int __ip_options_compile(struct net *net, struct ip_options *opt,
+ struct sk_buff *skb, __be32 *info);
int ip_options_compile(struct net *net, struct ip_options *opt,
struct sk_buff *skb);
int ip_options_get(struct net *net, struct ip_options_rcu **optp,
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b..eff86a7 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1735,13 +1735,26 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
*/
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
{
+ unsigned char optbuf[sizeof(struct ip_options) + 40];
+ struct ip_options *opt = (struct ip_options *)optbuf;
+
if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
return;
+ /*
+ * We might be called above the IP layer,
+ * so we can not use icmp_send and IPCB here.
+ */
+
+ memset(opt, 0, sizeof(struct ip_options));
+ opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
+ if (__ip_options_compile(dev_net(skb->dev), opt, skb, NULL))
+ return;
+
if (gateway)
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
else
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
}
/**
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ed194d4..32a3504 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -251,8 +251,9 @@ static void spec_dst_fill(__be32 *spec_dst, struct sk_buff *skb)
* If opt == NULL, then skb->data should point to IP header.
*/
-int ip_options_compile(struct net *net,
- struct ip_options *opt, struct sk_buff *skb)
+int __ip_options_compile(struct net *net,
+ struct ip_options *opt, struct sk_buff *skb,
+ __be32 *info)
{
__be32 spec_dst = htonl(INADDR_ANY);
unsigned char *pp_ptr = NULL;
@@ -468,11 +469,22 @@ int ip_options_compile(struct net *net,
return 0;
error:
- if (skb) {
- icmp_send(skb, ICMP_PARAMETERPROB, 0, htonl((pp_ptr-iph)<<24));
- }
+ if (info)
+ *info = htonl((pp_ptr-iph)<<24);
return -EINVAL;
}
+
+int ip_options_compile(struct net *net,
+ struct ip_options *opt, struct sk_buff *skb)
+{
+ int ret;
+ __be32 info;
+
+ ret = __ip_options_compile(net, opt, skb, &info);
+ if (ret != 0 && skb)
+ icmp_send(skb, ICMP_PARAMETERPROB, 0, info);
+ return ret;
+}
EXPORT_SYMBOL(ip_options_compile);
/*
---
^ permalink raw reply related
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Biggers @ 2019-02-22 17:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S . Miller, linux-kernel, Mao Wenan, Cong Wang,
Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <e4d0f1e8-68ff-512e-ad69-480523ff300f@gmail.com>
Hi Eric,
On Fri, Feb 22, 2019 at 09:45:35AM -0800, Eric Dumazet wrote:
>
>
> On 02/21/2019 02:13 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> > closed concurrently with fchownat(). However, it ignored that many
> > other proto_ops::release() methods don't set sock->sk to NULL and
> > therefore allow the same use-after-free:
> >
>
> I fail to see how setting a pointer to NULL can avoid races.
>
>
> We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
> while sockfs_setattr() is used.
>
> sockfs_setattr()
> ...
> if (sock->sk)
>
> // even if sock->sk was not NULL for the if (...).
>
> // it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.
>
> sock->sk->sk_uid = iattr->ia_uid;
>
>
->setattr() is called under inode_lock(), which __sock_release() also takes. So
the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close
race condition between sock_close() and sockfs_setattr()").
The issue now is that if ->setattr() happens *after* __sock_release() (which is
possible if fchownat() gets the reference to the file's 'struct path', then the
file is close()d by another thread, then fchownat() continues), it will see
stale sock->sk because for many socket types it wasn't set to NULL earlier.
- Eric
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Andrew Lunn @ 2019-02-22 17:59 UTC (permalink / raw)
To: Russell King - ARM Linux admin; +Cc: netdev, Vivien Didelot
In-Reply-To: <20190222174856.tmisqn7ih3ea3xvz@shell.armlinux.org.uk>
> Hi Andrew,
>
> Do we have a way forward for this issue?
Hi Russell
Yes. I tested releasing the mutex around the request for the
interrupt. That works. So i will submit a patchset adding both the
lockdep class, and the unlock/lock pair.
Andrew
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Stephen Hemminger @ 2019-02-22 18:01 UTC (permalink / raw)
To: Jiri Pirko
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190222085832.GA2254@nanopsycho>
On Fri, 22 Feb 2019 09:58:32 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
> >On Thu, 21 Feb 2019 15:42:40 +0200
> >Aya Levin <ayal@mellanox.com> wrote:
> >
> >> Add a new helper functions which outputs only values (without name
> >> label) for different types: boolean, uint, uint64, string and binary.
> >> In addition add a helper function which prints only the name label.
> >>
> >> Signed-off-by: Aya Levin <ayal@mellanox.com>
> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> >> ---
> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 65 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/devlink/devlink.c b/devlink/devlink.c
> >> index b073ae020d52..5d69c4f24f29 100644
> >> --- a/devlink/devlink.c
> >> +++ b/devlink/devlink.c
> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
> >> pr_out("\n");
> >> }
> >>
> >> -
> >> static void pr_out_str(struct dl *dl, const char *name, const char *val)
> >> {
> >> if (dl->json_output) {
> >> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
> >> }
> >> }
> >>
> >> +static void pr_out_bool_value(struct dl *dl, bool value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_bool(dl->jw, value);
> >> + else
> >> + pr_out(" %s", value ? "true" : "false");
> >> +}
> >> +
> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_uint(dl->jw, value);
> >> + else
> >> + pr_out(" %u", value);
> >> +}
> >> +
> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_u64(dl->jw, value);
> >> + else
> >> + pr_out(" %lu", value);
> >> +}
> >> +
> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
> >> +{
> >> + int i = 1;
> >> +
> >> + if (dl->json_output)
> >> + jsonw_start_array(dl->jw);
> >> + else
> >> + pr_out("\n");
> >> +
> >> + while (i < len) {
> >> + if (dl->json_output) {
> >> + jsonw_printf(dl->jw, "%d", data[i]);
> >> + } else {
> >> + pr_out(" %02x", data[i]);
> >> + if (!(i % 16))
> >> + pr_out("\n");
> >> + }
> >> + i++;
> >> + }
> >> + if (dl->json_output)
> >> + jsonw_end_array(dl->jw);
> >> + else if ((i - 1) % 16)
> >> + pr_out("\n");
> >> +}
> >> +
> >> +static void pr_out_str_value(struct dl *dl, const char *value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_string(dl->jw, value);
> >> + else
> >> + pr_out(" %s", value);
> >> +}
> >> +
> >> +static void pr_out_name(struct dl *dl, const char *name)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_name(dl->jw, name);
> >> + else
> >> + pr_out(" %s:", name);
> >> +}
> >> +
> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
> >> {
> >> if (dl->json_output) {
> >
> >NAK
> >
> >Please convert devlink to use existing iproute2 json_print infrasructure
> >rather than reinventing it.
>
> That requires whole devlink/devlink.c code change. Could this be done as
> a follow-up?
There is a tradeoff, if more code creeps in it is harder to do the code change.
Can you commit to a timeframe to convert the JSON support code?
^ permalink raw reply
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Cong Wang @ 2019-02-22 18:05 UTC (permalink / raw)
To: Eric Biggers
Cc: Linux Kernel Network Developers, David S . Miller, LKML,
Mao Wenan, Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <20190221221356.173485-1-ebiggers@kernel.org>
On Thu, Feb 21, 2019 at 2:14 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> closed concurrently with fchownat(). However, it ignored that many
> other proto_ops::release() methods don't set sock->sk to NULL and
> therefore allow the same use-after-free:
>
> - base_sock_release
> - bnep_sock_release
> - cmtp_sock_release
> - data_sock_release
> - dn_release
> - hci_sock_release
> - hidp_sock_release
> - iucv_sock_release
> - l2cap_sock_release
> - llcp_sock_release
> - llc_ui_release
> - rawsock_release
> - rfcomm_sock_release
> - sco_sock_release
> - svc_release
> - vcc_release
> - x25_release
>
> Rather than fixing all these and relying on every socket type to get
> this right forever, just make __sock_release() set sock->sk to NULL
> itself after calling proto_ops::release().
>
> Reproducer that produces the KASAN splat when any of these socket types
> are configured into the kernel:
>
> #include <pthread.h>
> #include <stdlib.h>
> #include <sys/socket.h>
> #include <unistd.h>
>
> pthread_t t;
> volatile int fd;
>
> void *close_thread(void *arg)
> {
> for (;;) {
> usleep(rand() % 100);
> close(fd);
> }
> }
>
> int main()
> {
> pthread_create(&t, NULL, close_thread, NULL);
> for (;;) {
> fd = socket(rand() % 50, rand() % 11, 0);
> fchownat(fd, "", 1000, 1000, 0x1000);
> close(fd);
> }
> }
>
> Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Jiri Pirko @ 2019-02-22 17:59 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190222100125.20d8dd54@shemminger-XPS-13-9360>
Fri, Feb 22, 2019 at 07:01:25PM CET, stephen@networkplumber.org wrote:
>On Fri, 22 Feb 2019 09:58:32 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
>> >On Thu, 21 Feb 2019 15:42:40 +0200
>> >Aya Levin <ayal@mellanox.com> wrote:
>> >
>> >> Add a new helper functions which outputs only values (without name
>> >> label) for different types: boolean, uint, uint64, string and binary.
>> >> In addition add a helper function which prints only the name label.
>> >>
>> >> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> >> ---
>> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> >> 1 file changed, 65 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/devlink/devlink.c b/devlink/devlink.c
>> >> index b073ae020d52..5d69c4f24f29 100644
>> >> --- a/devlink/devlink.c
>> >> +++ b/devlink/devlink.c
>> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
>> >> pr_out("\n");
>> >> }
>> >>
>> >> -
>> >> static void pr_out_str(struct dl *dl, const char *name, const char *val)
>> >> {
>> >> if (dl->json_output) {
>> >> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
>> >> }
>> >> }
>> >>
>> >> +static void pr_out_bool_value(struct dl *dl, bool value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_bool(dl->jw, value);
>> >> + else
>> >> + pr_out(" %s", value ? "true" : "false");
>> >> +}
>> >> +
>> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_uint(dl->jw, value);
>> >> + else
>> >> + pr_out(" %u", value);
>> >> +}
>> >> +
>> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_u64(dl->jw, value);
>> >> + else
>> >> + pr_out(" %lu", value);
>> >> +}
>> >> +
>> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
>> >> +{
>> >> + int i = 1;
>> >> +
>> >> + if (dl->json_output)
>> >> + jsonw_start_array(dl->jw);
>> >> + else
>> >> + pr_out("\n");
>> >> +
>> >> + while (i < len) {
>> >> + if (dl->json_output) {
>> >> + jsonw_printf(dl->jw, "%d", data[i]);
>> >> + } else {
>> >> + pr_out(" %02x", data[i]);
>> >> + if (!(i % 16))
>> >> + pr_out("\n");
>> >> + }
>> >> + i++;
>> >> + }
>> >> + if (dl->json_output)
>> >> + jsonw_end_array(dl->jw);
>> >> + else if ((i - 1) % 16)
>> >> + pr_out("\n");
>> >> +}
>> >> +
>> >> +static void pr_out_str_value(struct dl *dl, const char *value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_string(dl->jw, value);
>> >> + else
>> >> + pr_out(" %s", value);
>> >> +}
>> >> +
>> >> +static void pr_out_name(struct dl *dl, const char *name)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_name(dl->jw, name);
>> >> + else
>> >> + pr_out(" %s:", name);
>> >> +}
>> >> +
>> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
>> >> {
>> >> if (dl->json_output) {
>> >
>> >NAK
>> >
>> >Please convert devlink to use existing iproute2 json_print infrasructure
>> >rather than reinventing it.
>>
>> That requires whole devlink/devlink.c code change. Could this be done as
>> a follow-up?
>
>There is a tradeoff, if more code creeps in it is harder to do the code change.
>Can you commit to a timeframe to convert the JSON support code?
Okay, I'll take care of it in March. Promise.
^ permalink raw reply
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
From: Stephen Hemminger @ 2019-02-22 18:09 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <24d8a96d-4bac-fe8d-1308-c2caee2a9d2c@mellanox.com>
On Fri, 22 Feb 2019 04:58:06 +0000
Moshe Shemesh <moshe@mellanox.com> wrote:
> On 2/21/2019 7:54 PM, Stephen Hemminger wrote:
> > The netlink API for IFLA_VF_VLAN_LIST allows multiple VLAN tags to
> > be passed (and the message was validated) but only the first VLAN
> > tag was being passed to the device. Change to iterate over each tag received.
> >
> > Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > net/core/rtnetlink.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index a51cab95ba64..3a9ec988ae21 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -2207,11 +2207,10 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> > if (tb[IFLA_VF_VLAN_LIST]) {
> > struct ifla_vf_vlan_info *ivvl[MAX_VLAN_LIST_LEN];
> > struct nlattr *attr;
> > - int rem, len = 0;
> > + int i, rem, len = 0;
> >
> > - err = -EOPNOTSUPP;
> > if (!ops->ndo_set_vf_vlan)
> > - return err;
> > + return -EOPNOTSUPP;
> >
> > nla_for_each_nested(attr, tb[IFLA_VF_VLAN_LIST], rem) {
> > if (nla_type(attr) != IFLA_VF_VLAN_INFO ||
> > @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> >
> > len++;
> > }
> > - if (len == 0)
> > - return -EINVAL;
> >
> > - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> > - ivvl[0]->qos, ivvl[0]->vlan_proto);
> > - if (err < 0)
> > - return err;
> > + err = -EINVAL; /* empty list error */
> > + for (i = 0; i < len; i++) {
> > + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> > + ivvl[i]->vlan, ivvl[i]->qos,
> > + ivvl[i]->vlan_proto);
>
> Doing that each vlan will just overwrite the vf vlan configuration set
> by its preceding one.
> Note #define MAX_VLAN_LIST_LEN 1
> The point here was that I had to add the rtnl interface to set vf vlan
> with option to set vf vlan protocol. While doing that I was asked to add
> option to get a list of vlans from user to support QinQ, so once it will
> be needed there won't be a need to add another rtnl interface.
>
> The driver which will support setting double vlan or more per vf will
> change MAX_VLAN_LIST_LEN and change the ndo function.
>
OK, then the original code kind of makes sense now. Maybe the whole loop
could be eliminated.
I expected that the vlan list was for allowing multiple vlan's to go to
a single VF but it is really a future (unlikely to ever be implemented)
support for QinQ.
^ permalink raw reply
* [PATCH net-next 0/3] net: protodown support for macvlan and vxlan
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
This patch series adds dev_change_proto_down_generic, a generic
implementation of ndo_change_proto_down, which sets the netdev carrier
state according to the new proto_down value.
This handler adds the ability to set protodown on macvlan and vxlan
interfaces in a generic way for use by control protocols like VRRPD.
Patch (1) introduces the handler in net/code/dev.c. Patch (2) and (3) add
support for change_proto_down in macvlan and vxlan drivers, respectively,
using the new function.
Andy Roulin (3):
net: dev: add generic protodown handler
macvlan: add ndo_change_proto_down support
vxlan: add ndo_change_proto_down support
drivers/net/macvlan.c | 1 +
drivers/net/vxlan.c | 1 +
include/linux/netdevice.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
4 files changed, 22 insertions(+)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 3/3] vxlan: add ndo_change_proto_down support
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
In-Reply-To: <20190222180638.10904-1-aroulin@cumulusnetworks.com>
Add ndo_change_proto_down support through dev_change_proto_down_generic
for use by control protocols like VRRPD.
Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/vxlan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 33edc78e818d..577201cd880c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2923,6 +2923,7 @@ static const struct net_device_ops vxlan_netdev_ether_ops = {
.ndo_fdb_dump = vxlan_fdb_dump,
.ndo_fdb_get = vxlan_fdb_get,
.ndo_fill_metadata_dst = vxlan_fill_metadata_dst,
+ .ndo_change_proto_down = dev_change_proto_down_generic,
};
static const struct net_device_ops vxlan_netdev_raw_ops = {
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/3] net: dev: add generic protodown handler
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
In-Reply-To: <20190222180638.10904-1-aroulin@cumulusnetworks.com>
Introduce dev_change_proto_down_generic, a generic ndo_change_proto_down
implementation, which sets the netdev carrier state according to proto_down.
This adds the ability to set protodown on vxlan and macvlan devices in a
generic way for use by control protocols like VRRPD.
Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
include/linux/netdevice.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index aab4d9f6613d..9a093fde030d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3663,6 +3663,7 @@ int dev_get_port_parent_id(struct net_device *dev,
struct netdev_phys_item_id *ppid, bool recurse);
bool netdev_port_same_parent_id(struct net_device *a, struct net_device *b);
int dev_change_proto_down(struct net_device *dev, bool proto_down);
+int dev_change_proto_down_generic(struct net_device *dev, bool proto_down);
struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev, bool *again);
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
diff --git a/net/core/dev.c b/net/core/dev.c
index a3d13f5e2bfc..e99a870772c1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7954,6 +7954,25 @@ int dev_change_proto_down(struct net_device *dev, bool proto_down)
}
EXPORT_SYMBOL(dev_change_proto_down);
+/**
+ * dev_change_proto_down_generic - generic implementation for
+ * ndo_change_proto_down that sets carrier according to
+ * proto_down.
+ *
+ * @dev: device
+ * @proto_down: new value
+ */
+int dev_change_proto_down_generic(struct net_device *dev, bool proto_down)
+{
+ if (proto_down)
+ netif_carrier_off(dev);
+ else
+ netif_carrier_on(dev);
+ dev->proto_down = proto_down;
+ return 0;
+}
+EXPORT_SYMBOL(dev_change_proto_down_generic);
+
u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
enum bpf_netdev_command cmd)
{
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/3] macvlan: add ndo_change_proto_down support
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
In-Reply-To: <20190222180638.10904-1-aroulin@cumulusnetworks.com>
Add ndo_change_proto_down support through dev_change_proto_down_generic
for use by control protocols like VRRPD.
Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/macvlan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 26e53832b095..0c0f105657d3 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1122,6 +1122,7 @@ static const struct net_device_ops macvlan_netdev_ops = {
#endif
.ndo_get_iflink = macvlan_dev_get_iflink,
.ndo_features_check = passthru_features_check,
+ .ndo_change_proto_down = dev_change_proto_down_generic,
};
void macvlan_common_setup(struct net_device *dev)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Dumazet @ 2019-02-22 18:25 UTC (permalink / raw)
To: Eric Biggers, Eric Dumazet
Cc: netdev, David S . Miller, linux-kernel, Mao Wenan, Cong Wang,
Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <20190222175743.GA163909@gmail.com>
On 02/22/2019 09:57 AM, Eric Biggers wrote:
> ->setattr() is called under inode_lock(), which __sock_release() also takes. So
> the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close
> race condition between sock_close() and sockfs_setattr()").
Oh right, we added another inode_lock()/inode_unlock() for sock_close()
>
> The issue now is that if ->setattr() happens *after* __sock_release() (which is
> possible if fchownat() gets the reference to the file's 'struct path', then the
> file is close()d by another thread, then fchownat() continues), it will see
> stale sock->sk because for many socket types it wasn't set to NULL earlier.
>
> - Eric
>
^ permalink raw reply
* Re: general protection fault in rose_send_frame
From: syzbot @ 2019-02-22 18:26 UTC (permalink / raw)
To: davem, linux-hams, linux-kernel, netdev, ralf, syzkaller-bugs
In-Reply-To: <00000000000089904d057f1e0ae0@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 7a25c6c0aac8 rocker: Add missing break for PRE_BRIDGE_FLAGS
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=104002d8c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=58cb9d752ba5f3e0
dashboard link: https://syzkaller.appspot.com/bug?extid=7078ae989d857fe17988
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1224c304c00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=151824f8c00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7078ae989d857fe17988@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_CHANGE): hsr_slave_1: link becomes ready
8021q: adding VLAN 0 to HW filter on device batadv0
IPv6: ADDRCONF(NETDEV_CHANGE): rose0: link becomes ready
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc7+ #76
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:rose_send_frame+0x1a8/0x280 net/rose/rose_link.c:104
Code: c1 ea 03 80 3c 02 00 0f 85 8d 00 00 00 48 b8 00 00 00 00 00 fc ff df
4c 8b 63 20 49 8d bc 24 58 03 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75
7e 49 8b 94 24 58 03 00 00 e9 b8 fe ff ff e8 f0 53
RSP: 0018:ffff8880ae807ae8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff88809aa1fa00 RCX: ffffffff86378efb
RDX: 000000000000006b RSI: ffffffff8637902c RDI: 0000000000000358
RBP: ffff8880ae807b18 R08: ffffffff8887dec0 R09: ffffed101263146d
R10: ffffed101263146c R11: ffff88809318a363 R12: 0000000000000000
R13: 0000000000000078 R14: 0000000000000005 R15: ffff8880a4f6fbc0
FS: 0000000000000000(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f23b3e4ee78 CR3: 000000009eef3000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
rose_transmit_clear_request+0x1de/0x2a0 net/rose/rose_link.c:258
rose_rx_call_request+0x4ea/0x1990 net/rose/af_rose.c:1001
rose_loopback_timer+0x26a/0x3f0 net/rose/rose_loopback.c:100
call_timer_fn+0x190/0x720 kernel/time/timer.c:1325
expire_timers kernel/time/timer.c:1362 [inline]
__run_timers kernel/time/timer.c:1681 [inline]
__run_timers kernel/time/timer.c:1649 [inline]
run_timer_softirq+0x652/0x1700 kernel/time/timer.c:1694
__do_softirq+0x266/0x95a kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x180/0x1d0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x14a/0x570 arch/x86/kernel/apic/apic.c:1062
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
Code: ff ff ff 48 89 c7 48 89 45 d8 e8 59 6c a1 fa 48 8b 45 d8 e9 ce fe ff
ff 48 89 df e8 48 6c a1 fa eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66
2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffffff88807d08 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: 1ffffffff1125061 RBX: ffffffff8887dec0 RCX: 0000000000000000
RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffffffff8887e73c
RBP: ffffffff88807d38 R08: ffffffff8887dec0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffffffff889282f8 R14: 0000000000000000 R15: 0000000000000000
arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
default_idle_call+0x36/0x90 kernel/sched/idle.c:93
cpuidle_idle_call kernel/sched/idle.c:153 [inline]
do_idle+0x386/0x570 kernel/sched/idle.c:262
cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
rest_init+0x245/0x37b init/main.c:442
arch_call_rest_init+0xe/0x1b
start_kernel+0x803/0x83c init/main.c:739
x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:470
x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:451
secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
Modules linked in:
---[ end trace 8f29030702ddb052 ]---
RIP: 0010:rose_send_frame+0x1a8/0x280 net/rose/rose_link.c:104
Code: c1 ea 03 80 3c 02 00 0f 85 8d 00 00 00 48 b8 00 00 00 00 00 fc ff df
4c 8b 63 20 49 8d bc 24 58 03 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75
7e 49 8b 94 24 58 03 00 00 e9 b8 fe ff ff e8 f0 53
RSP: 0018:ffff8880ae807ae8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff88809aa1fa00 RCX: ffffffff86378efb
RDX: 000000000000006b RSI: ffffffff8637902c RDI: 0000000000000358
RBP: ffff8880ae807b18 R08: ffffffff8887dec0 R09: ffffed101263146d
R10: ffffed101263146c R11: ffff88809318a363 R12: 0000000000000000
R13: 0000000000000078 R14: 0000000000000005 R15: ffff8880a4f6fbc0
FS: 0000000000000000(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f23b3e4ee78 CR3: 000000009eef3000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
^ 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