* Re: [RFC] longer netdev names proposal
From: Michal Kubecek @ 2019-06-27 18:39 UTC (permalink / raw)
To: netdev
Cc: Andrew Lunn, Stephen Hemminger, David Ahern, Jiri Pirko, davem,
jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <20190627183538.GI31189@lunn.ch>
On Thu, Jun 27, 2019 at 08:35:38PM +0200, Andrew Lunn wrote:
> On Thu, Jun 27, 2019 at 11:23:05AM -0700, Stephen Hemminger wrote:
> > On Thu, 27 Jun 2019 20:08:03 +0200 Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > > It often feels as a deficiency that unlike block devices where we can
> > > keep one name and create multiple symlinks based on different naming
> > > schemes, network devices can have only one name. There are aliases but
> > > AFAIK they are only used (and can be only used) for SNMP. IMHO this
> > > limitation is part of the mess that left us with so-called "predictable
> > > names" which are in practice neither persistent nor predictable.
> > >
> > > So perhaps we could introduce actual aliases (or altnames or whatever we
> > > would call them) for network devices that could be used to identify
> > > a network device whenever both kernel and userspace tool supports them.
> > > Old (and ancient) tools would have to use the one canonical name limited
> > > to current IFNAMSIZ, new tools would allow using any alias which could
> > > be longer.
> >
> > That is already there in current network model.
> > # ip li set dev eno1 alias 'Onboard Ethernet'
> > # ip li show dev eno1
> > 2: eno1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
> > link/ether ac:1f:6b:74:38:c0 brd ff:ff:ff:ff:ff:ff
> > alias Onboard Ethernet
>
> $ ip li set dev enp3s0 alias "Onboard Ethernet"
> # ip link show "Onboard Ethernet"
> Device "Onboard Ethernet" does not exist.
>
> So it does not really appear to be an alias, it is a label. To be
> truly useful, it needs to be more than a label, it needs to be a real
> alias which you can use.
That's exactly what I meant: to be really useful, one should be able to
use the alias(es) for setting device options, for adding routes, in
netfilter rules etc.
Michal
^ permalink raw reply
* [PATCH iproute2 v2 1/2] devlink: fix format string warning for 32bit targets
From: Baruch Siach @ 2019-06-27 18:37 UTC (permalink / raw)
To: Stephen Hemminger, Jiri Pirko
Cc: netdev, Baruch Siach, Aya Levin, Moshe Shemesh
32bit targets define uint64_t as long long unsigned. This leads to the
following build warning:
devlink.c: In function ‘pr_out_u64’:
devlink.c:1729:11: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t {aka long long unsigned int}’ [-Wformat=]
pr_out("%s %lu", name, val);
^
devlink.c:59:21: note: in definition of macro ‘pr_out’
fprintf(stdout, ##args); \
^~~~
Use uint64_t specific conversion specifiers in the format string to fix
that.
Cc: Aya Levin <ayal@mellanox.com>
Cc: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Use inttypes.h conversion specifiers
---
devlink/devlink.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 436935f88bda..b6e68f9a4d65 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1726,9 +1726,9 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
jsonw_u64_field(dl->jw, name, val);
} else {
if (g_indent_newline)
- pr_out("%s %lu", name, val);
+ pr_out("%s %"PRIu64, name, val);
else
- pr_out(" %s %lu", name, val);
+ pr_out(" %s %"PRIu64, name, val);
}
}
@@ -1753,7 +1753,7 @@ 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);
+ pr_out(" %"PRIu64, value);
}
static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
--
2.20.1
^ permalink raw reply related
* [PATCH iproute2 v2 2/2] devlink: fix libc and kernel headers collision
From: Baruch Siach @ 2019-06-27 18:37 UTC (permalink / raw)
To: Stephen Hemminger, Jiri Pirko
Cc: netdev, Baruch Siach, Aya Levin, Moshe Shemesh
In-Reply-To: <7a72ae0f9519e6a445d9712399d989fed648e6eb.1561660639.git.baruch@tkos.co.il>
Since commit 2f1242efe9d ("devlink: Add devlink health show command") we
use the sys/sysinfo.h header for the sysinfo(2) system call. But since
iproute2 carries a local version of the kernel struct sysinfo, this
causes a collision with libc that do not rely on kernel defined sysinfo
like musl libc:
In file included from devlink.c:25:0:
.../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo'
struct sysinfo {
^~~~~~~
In file included from ../include/uapi/linux/kernel.h:5:0,
from ../include/uapi/linux/netlink.h:5,
from ../include/uapi/linux/genetlink.h:6,
from devlink.c:21:
../include/uapi/linux/sysinfo.h:8:8: note: originally defined here
struct sysinfo {
^~~~~~~
Move the sys/sysinfo.h userspace header before kernel headers, and
suppress the indirect include of linux/sysinfo.h.
Cc: Aya Levin <ayal@mellanox.com>
Cc: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Shorten comment
---
devlink/devlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index b6e68f9a4d65..039225df7cbf 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -18,11 +18,12 @@
#include <limits.h>
#include <errno.h>
#include <inttypes.h>
+#include <sys/sysinfo.h>
+#define _LINUX_SYSINFO_H /* avoid collision with musl header */
#include <linux/genetlink.h>
#include <linux/devlink.h>
#include <libmnl/libmnl.h>
#include <netinet/ether.h>
-#include <sys/sysinfo.h>
#include <sys/queue.h>
#include "SNAPSHOT.h"
--
2.20.1
^ permalink raw reply related
* Re: [RFC] longer netdev names proposal
From: Andrew Lunn @ 2019-06-27 18:35 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michal Kubecek, netdev, David Ahern, Jiri Pirko, davem,
jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <20190627112305.7e05e210@hermes.lan>
On Thu, Jun 27, 2019 at 11:23:05AM -0700, Stephen Hemminger wrote:
> On Thu, 27 Jun 2019 20:08:03 +0200
> Michal Kubecek <mkubecek@suse.cz> wrote:
>
> > It often feels as a deficiency that unlike block devices where we can
> > keep one name and create multiple symlinks based on different naming
> > schemes, network devices can have only one name. There are aliases but
> > AFAIK they are only used (and can be only used) for SNMP. IMHO this
> > limitation is part of the mess that left us with so-called "predictable
> > names" which are in practice neither persistent nor predictable.
> >
> > So perhaps we could introduce actual aliases (or altnames or whatever we
> > would call them) for network devices that could be used to identify
> > a network device whenever both kernel and userspace tool supports them.
> > Old (and ancient) tools would have to use the one canonical name limited
> > to current IFNAMSIZ, new tools would allow using any alias which could
> > be longer.
> >
> > Michal
>
>
> That is already there in current network model.
> # ip li set dev eno1 alias 'Onboard Ethernet'
> # ip li show dev eno1
> 2: eno1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:74:38:c0 brd ff:ff:ff:ff:ff:ff
> alias Onboard Ethernet
Hi Stephen
$ ip li set dev enp3s0 alias "Onboard Ethernet"
# ip link show "Onboard Ethernet"
Device "Onboard Ethernet" does not exist.
So it does not really appear to be an alias, it is a label. To be
truly useful, it needs to be more than a label, it needs to be a real
alias which you can use.
Andrew
^ permalink raw reply
* Re: [PATCH net v2] vxlan: do not destroy fdb if register_netdevice() is failed
From: Roopa Prabhu @ 2019-06-27 18:33 UTC (permalink / raw)
To: Taehee Yoo; +Cc: David Miller, netdev, Petr Machata
In-Reply-To: <20190627145010.21073-1-ap420073@gmail.com>
On Thu, Jun 27, 2019 at 7:50 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> __vxlan_dev_create() destroys FDB using specific pointer which indicates
> a fdb when error occurs.
> But that pointer should not be used when register_netdevice() fails because
> register_netdevice() internally destroys fdb when error occurs.
>
> This patch makes vxlan_fdb_create() to do not link fdb entry to vxlan dev
> internally.
> Instead, a new function vxlan_fdb_link() is added to link fdb to vxlan dev.
>
> vxlan_fdb_link() is called after calling register_netdevice().
> This routine can avoid situation that ->ndo_uninit() destroys fdb entry
> in error path of register_netdevice().
> Hence, error path of __vxlan_dev_create() routine can have an opportunity
> to destroy default fdb entry by hand.
>
> Test command
> ip link add bonding_masters type vxlan id 0 group 239.1.1.1 \
> dev enp0s9 dstport 4789
>
> Splat looks like:
> [ 213.392816] kasan: GPF could be caused by NULL-ptr deref or user memory access
> [ 213.401257] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
> [ 213.402178] CPU: 0 PID: 1414 Comm: ip Not tainted 5.2.0-rc5+ #256
> [ 213.402178] RIP: 0010:vxlan_fdb_destroy+0x120/0x220 [vxlan]
> [ 213.402178] Code: df 48 8b 2b 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 06 01 00 00 4c 8b 63 08 48 b8 00 00 00 00 00 fc d
> [ 213.402178] RSP: 0018:ffff88810cb9f0a0 EFLAGS: 00010202
> [ 213.402178] RAX: dffffc0000000000 RBX: ffff888101d4a8c8 RCX: 0000000000000000
> [ 213.402178] RDX: 1bd5a00000000040 RSI: ffff888101d4a8c8 RDI: ffff888101d4a8d0
> [ 213.402178] RBP: 0000000000000000 R08: fffffbfff22b72d9 R09: 0000000000000000
> [ 213.402178] R10: 00000000ffffffef R11: 0000000000000000 R12: dead000000000200
> [ 213.402178] R13: ffff88810cb9f1f8 R14: ffff88810efccda0 R15: ffff88810efccda0
> [ 213.402178] FS: 00007f7f6621a0c0(0000) GS:ffff88811b000000(0000) knlGS:0000000000000000
> [ 213.402178] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 213.402178] CR2: 000055746f0807d0 CR3: 00000001123e0000 CR4: 00000000001006f0
> [ 213.402178] Call Trace:
> [ 213.402178] __vxlan_dev_create+0x3a9/0x7d0 [vxlan]
> [ 213.402178] ? vxlan_changelink+0x740/0x740 [vxlan]
> [ 213.402178] ? rcu_read_unlock+0x60/0x60 [vxlan]
> [ 213.402178] ? __kasan_kmalloc.constprop.3+0xa0/0xd0
> [ 213.402178] vxlan_newlink+0x8d/0xc0 [vxlan]
> [ 213.402178] ? __vxlan_dev_create+0x7d0/0x7d0 [vxlan]
> [ 213.554119] ? __netlink_ns_capable+0xc3/0xf0
> [ 213.554119] __rtnl_newlink+0xb75/0x1180
> [ 213.554119] ? rtnl_link_unregister+0x230/0x230
> [ ... ]
>
> Fixes: 0241b836732f ("vxlan: fix default fdb entry netlink notify ordering during netdev create")
> Suggested-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>
> v1 -> v2 :
> - Add a new function vxlan_fdb_link().
> - Fix fdb entry leak.
> - Update description.
>
thanks for v2!. a few comments inline below ...
> drivers/net/vxlan.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 083f3f0bf37f..4066346d6f41 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -804,6 +804,14 @@ static struct vxlan_fdb *vxlan_fdb_alloc(struct vxlan_dev *vxlan,
> return f;
> }
>
> +static void vxlan_fdb_link(struct vxlan_dev *vxlan, const u8 *mac,
> + __be32 src_vni, struct vxlan_fdb *f)
I would prefer vxlan_fdb_insert or something along those lines.
> +{
> + ++vxlan->addrcnt;
> + hlist_add_head_rcu(&f->hlist,
> + vxlan_fdb_head(vxlan, mac, src_vni));
> +}
> +
> static int vxlan_fdb_create(struct vxlan_dev *vxlan,
> const u8 *mac, union vxlan_addr *ip,
> __u16 state, __be16 port, __be32 src_vni,
> @@ -829,10 +837,6 @@ static int vxlan_fdb_create(struct vxlan_dev *vxlan,
> return rc;
> }
>
> - ++vxlan->addrcnt;
> - hlist_add_head_rcu(&f->hlist,
> - vxlan_fdb_head(vxlan, mac, src_vni));
> -
> *fdb = f;
>
> return 0;
> @@ -977,6 +981,7 @@ static int vxlan_fdb_update_create(struct vxlan_dev *vxlan,
> if (rc < 0)
> return rc;
>
> + vxlan_fdb_link(vxlan, mac, src_vni, f);
> rc = vxlan_fdb_notify(vxlan, f, first_remote_rtnl(f), RTM_NEWNEIGH,
> swdev_notify, extack);
> if (rc)
> @@ -3571,12 +3576,17 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
> if (err)
> goto errout;
>
> - /* notify default fdb entry */
> if (f) {
> + vxlan_fdb_link(vxlan, all_zeros_mac,
> + vxlan->default_dst.remote_vni, f);
> +
> + /* notify default fdb entry */
> err = vxlan_fdb_notify(vxlan, f, first_remote_rtnl(f),
> RTM_NEWNEIGH, true, extack);
> - if (err)
> - goto errout;
> + if (err) {
> + vxlan_fdb_destroy(vxlan, f, false, false);
> + goto unregister;
> + }
> }
>
> list_add(&vxlan->next, &vn->vxlan_list);
> @@ -3588,7 +3598,8 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
> * destroy the entry by hand here.
> */
> if (f)
> - vxlan_fdb_destroy(vxlan, f, false, false);
> + call_rcu(&f->rcu, vxlan_fdb_free);
f is local to this function and not inserted at this point, so maybe
we dont need to call_rcu here ?
> +unregister:
> if (unregister)
> unregister_netdevice(dev);
> return err;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v2] net: dsa: mv88e6xxx: wait after reset deactivation
From: Andrew Lunn @ 2019-06-27 18:27 UTC (permalink / raw)
To: Baruch Siach; +Cc: Vivien Didelot, netdev
In-Reply-To: <2e272a4e588ae44137864237d0cd73e2208f2c60.1561659459.git.baruch@tkos.co.il>
On Thu, Jun 27, 2019 at 09:17:39PM +0300, Baruch Siach wrote:
> Add a 1ms delay after reset deactivation. Otherwise the chip returns
> bogus ID value. This is observed with 88E6390 (Peridot) chip.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Stephen Hemminger @ 2019-06-27 18:23 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Ahern, Jiri Pirko, davem, jakub.kicinski, sthemmin,
mlxsw
In-Reply-To: <20190627180803.GJ27240@unicorn.suse.cz>
On Thu, 27 Jun 2019 20:08:03 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:
> It often feels as a deficiency that unlike block devices where we can
> keep one name and create multiple symlinks based on different naming
> schemes, network devices can have only one name. There are aliases but
> AFAIK they are only used (and can be only used) for SNMP. IMHO this
> limitation is part of the mess that left us with so-called "predictable
> names" which are in practice neither persistent nor predictable.
>
> So perhaps we could introduce actual aliases (or altnames or whatever we
> would call them) for network devices that could be used to identify
> a network device whenever both kernel and userspace tool supports them.
> Old (and ancient) tools would have to use the one canonical name limited
> to current IFNAMSIZ, new tools would allow using any alias which could
> be longer.
>
> Michal
That is already there in current network model.
# ip li set dev eno1 alias 'Onboard Ethernet'
# ip li show dev eno1
2: eno1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
link/ether ac:1f:6b:74:38:c0 brd ff:ff:ff:ff:ff:ff
alias Onboard Ethernet
^ permalink raw reply
* RE: [net/tls] Re: KMSAN: uninit-value in aesti_encrypt
From: John Fastabend @ 2019-06-27 18:19 UTC (permalink / raw)
To: Eric Biggers, Boris Pismenny, Aviad Yehezkel, Dave Watson,
John Fastabend, Daniel Borkmann, netdev
Cc: davem, glider, herbert, linux-crypto, linux-kernel,
syzkaller-bugs, syzbot
In-Reply-To: <20190627164627.GF686@sol.localdomain>
Eric Biggers wrote:
> [+TLS maintainers]
>
> Very likely a net/tls bug, not a crypto bug.
>
> Possibly a duplicate of other reports such as "KMSAN: uninit-value in gf128mul_4k_lle (3)"
>
> See https://lore.kernel.org/netdev/20190625055019.GD17703@sol.localdomain/ for
> the list of 17 other open syzbot bugs I've assigned to the TLS subsystem. TLS
> maintainers, when are you planning to look into these?
>
> On Thu, Jun 27, 2019 at 09:37:05AM -0700, syzbot wrote:
I'm looking at this issue now. There is a series on bpf list now to address
many of those 17 open issues but this is a separate issue. I can reproduce
it locally so should have a fix soon.
Thanks,
John
^ permalink raw reply
* Re: [PATCH next 3/3] blackhole_dev: add a selftest
From: Mahesh Bandewar (महेश बंडेवार) @ 2019-06-27 18:19 UTC (permalink / raw)
To: David Miller; +Cc: linux-netdev, Eric Dumazet, michael.chan, dja, mahesh
In-Reply-To: <20190627.110852.372215308913618999.davem@davemloft.net>
On Thu, Jun 27, 2019 at 11:08 AM David Miller <davem@davemloft.net> wrote:
>
> From: Mahesh Bandewar <maheshb@google.com>
> Date: Fri, 21 Jun 2019 17:45:39 -0700
>
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -4,8 +4,9 @@
> > CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
> > CFLAGS += -I../../../../usr/include/
> >
> > +<<<<<<< HEAD
> > TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
>
> Ummm... yeah... might want to resolve this conflict...
>
oops, my bad! Let me send v2
> :-)
^ permalink raw reply
* [PATCH v2] net: dsa: mv88e6xxx: wait after reset deactivation
From: Baruch Siach @ 2019-06-27 18:17 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot; +Cc: netdev, Baruch Siach
Add a 1ms delay after reset deactivation. Otherwise the chip returns
bogus ID value. This is observed with 88E6390 (Peridot) chip.
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Address Andrew Lunn's comments:
Use usleep_range.
Delay only when reset line is valid.
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ae750ab9a4d7..5f81d9a3a2a6 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4910,6 +4910,8 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
err = PTR_ERR(chip->reset);
goto out;
}
+ if (chip->reset)
+ usleep_range(1000, 2000);
err = mv88e6xxx_detect(chip);
if (err)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v1 2/2] net: dsa: qca8k: introduce reset via gpio feature
From: David Miller @ 2019-06-27 18:17 UTC (permalink / raw)
To: chunkeey
Cc: netdev, mark.rutland, robh+dt, f.fainelli, vivien.didelot, andrew
In-Reply-To: <36b1e912b47bc079a78e06e05a33213833715314.1561452044.git.chunkeey@gmail.com>
From: Christian Lamparter <chunkeey@gmail.com>
Date: Tue, 25 Jun 2019 10:41:51 +0200
> The QCA8337(N) has a RESETn signal on Pin B42 that
> triggers a chip reset if the line is pulled low.
> The datasheet says that: "The active low duration
> must be greater than 10 ms".
>
> This can hopefully fix some of the issues related
> to pin strapping in OpenWrt for the EA8500 which
> suffers from detection issues after a SoC reset.
>
> Please note that the qca8k_probe() function does
> currently require to read the chip's revision
> register for identification purposes.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: net: dsa: qca8k: document reset-gpios property
From: David Miller @ 2019-06-27 18:17 UTC (permalink / raw)
To: chunkeey
Cc: netdev, mark.rutland, robh+dt, f.fainelli, vivien.didelot, andrew
In-Reply-To: <08e0fd513620f03a2207b9f32637cdb434ed8def.1561452044.git.chunkeey@gmail.com>
From: Christian Lamparter <chunkeey@gmail.com>
Date: Tue, 25 Jun 2019 10:41:50 +0200
> This patch documents the qca8k's reset-gpios property that
> can be used if the QCA8337N ends up in a bad state during
> reset.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: link_watch: prevent starvation when processing linkwatch wq
From: David Miller @ 2019-06-27 18:17 UTC (permalink / raw)
To: linyunsheng
Cc: hkallweit1, f.fainelli, netdev, linux-kernel, linuxarm, pbonzini,
rkrcmar, kvm, xuwei5
In-Reply-To: <5c06e5dd-cfb1-870c-a0a3-42397b59c734@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Tue, 25 Jun 2019 10:28:04 +0800
> So It is ok for me to fall back to reschedule the link watch wq
> every 100 items, or is there a better way to fix it properly?
Yes, that is fine for now.
^ permalink raw reply
* RE: [PATCH 0/2] tls, add unhash callback
From: John Fastabend @ 2019-06-27 18:16 UTC (permalink / raw)
To: John Fastabend, daniel, jakub.kicinski, ast
Cc: netdev, edumazet, john.fastabend, bpf
In-Reply-To: <156165697019.32598.7171757081688035707.stgit@john-XPS-13-9370>
John Fastabend wrote:
> Resolve a series of splats discovered by syzbot and noted by
> Eric Dumazet. The primary problem here is we resolved an issue on
> the BPF sockmap side by adding an unhash callback. This is
> required to ensure sockmap sockets do not transition out of
> ESTABLISHED state into a LISTEN state. When we did this it
> created a case where the interaction between callbacks in TLS
> and sockmap when used together could break. This resulted in
> leaking TLS memory and potential to build loops of callbacks
> where sockmap called into TLS and TLS called back into BPF.
>
> Additionally, TLS was releasing the sock lock and then
> reaquiring it during the tear down process which could hang
> if another sock operation happened while the lock was not
> held.
>
> To fix this first refactor TLS code so lock is held for the
> entire teardown operation. Then add an unhash callback to ensure
> TLS can not transition from ESTABLISHED to LISTEN state. This
> transition is a similar bug to the one found and fixed previously
> in sockmap. And cleans up the callbacks to fix the syzbot
> errors.
>
> ---
>
Jakub,
If you could test this for the offload case that would
be helpful. I don't have any hardware here. We will still need
a few fixes in the unhash/hardware case but would be good to
know we don't cause any regressions here.
Thanks,
John
^ permalink raw reply
* Re: [PATCH v2 net-next] ipv6: Convert gateway validation to use fib6_info
From: David Miller @ 2019-06-27 18:11 UTC (permalink / raw)
To: dsahern; +Cc: netdev, kafai, weiwan, dsahern
In-Reply-To: <20190624204451.10929-1-dsahern@kernel.org>
From: David Ahern <dsahern@kernel.org>
Date: Mon, 24 Jun 2019 13:44:51 -0700
> From: David Ahern <dsahern@gmail.com>
>
> Gateway validation does not need a dst_entry, it only needs the fib
> entry to validate the gateway resolution and egress device. So,
> convert ip6_nh_lookup_table from ip6_pol_route to fib6_table_lookup
> and ip6_route_check_nh to use fib6_lookup over rt6_lookup.
>
> ip6_pol_route is a call to fib6_table_lookup and if successful a call
> to fib6_select_path. From there the exception cache is searched for an
> entry or a dst_entry is created to return to the caller. The exception
> entry is not relevant for gateway validation, so what matters are the
> calls to fib6_table_lookup and then fib6_select_path.
>
> Similarly, rt6_lookup can be replaced with a call to fib6_lookup with
> RT6_LOOKUP_F_IFACE set in flags. Again, the exception cache search is
> not relevant, only the lookup with path selection. The primary difference
> in the lookup paths is the use of rt6_select with fib6_lookup versus
> rt6_device_match with rt6_lookup. When you remove complexities in the
> rt6_select path, e.g.,
> 1. saddr is not set for gateway validation, so RT6_LOOKUP_F_HAS_SADDR
> is not relevant
> 2. rt6_check_neigh is not called so that removes the RT6_NUD_FAIL_DO_RR
> return and round-robin logic.
>
> the code paths are believed to be equivalent for the given use case -
> validate the gateway and optionally given the device. Furthermore, it
> aligns the validation with onlink code path and the lookup path actually
> used for rx and tx.
>
> Adjust the users, ip6_route_check_nh_onlink and ip6_route_check_nh to
> handle a fib6_info vs a rt6_info when performing validation checks.
>
> Existing selftests fib-onlink-tests.sh and fib_tests.sh are used to
> verify the changes.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> v2
> - use in6_dev_get versus __in6_dev_get + in6_dev_hold (comment from Wei)
> - updated commit message
Applied, thanks.
^ permalink raw reply
* Re: [PATCH next 3/3] blackhole_dev: add a selftest
From: David Miller @ 2019-06-27 18:08 UTC (permalink / raw)
To: maheshb; +Cc: netdev, edumazet, michael.chan, dja, mahesh
In-Reply-To: <20190622004539.92199-1-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Fri, 21 Jun 2019 17:45:39 -0700
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -4,8 +4,9 @@
> CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
> CFLAGS += -I../../../../usr/include/
>
> +<<<<<<< HEAD
> TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
Ummm... yeah... might want to resolve this conflict...
:-)
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: James Morris @ 2019-06-27 18:06 UTC (permalink / raw)
To: Stephen Smalley
Cc: Andy Lutomirski, Andy Lutomirski, Matthew Garrett, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Matthew Garrett, Network Development, Chun-Yi Lee,
Daniel Borkmann, linux-security-module
In-Reply-To: <bce70c8b-9efd-6362-d536-cfbbcf70b0b7@tycho.nsa.gov>
On Thu, 27 Jun 2019, Stephen Smalley wrote:
> There are two scenarios where finer-grained distinctions make sense:
>
> - Users may need to enable specific functionality that falls under the
> umbrella of "confidentiality" or "integrity" lockdown. Finer-grained lockdown
> reasons free them from having to make an all-or-nothing choice between lost
> functionality or no lockdown at all.
Agreed. This will be used for more than just UEFI secure boot on desktops,
e.g. embedded systems using verified boot, where finer grained policy will
be needed for what are sometimes very specific use-cases (which may be
also covered by other mitigations).
> This can be supported directly by the
> lockdown module without any help from SELinux or other security modules; we
> just need the ability to specify these finer-grained lockdown levels via the
> boot parameters and securityfs nodes.
If the lockdown LSM implements fine grained policy (rather than the simple
coarse grained policy), I'd suggest adding a new lockdown level of
'custom' which by default enables all hooks but allows selective
disablement via params/sysfs.
This would be simpler than telling users to use a different lockdown LSM
for this.
> - Different processes/programs may need to use different sets of functionality
> restricted via lockdown confidentiality or integrity categories. If we have
> to allow all-or-none for the set of interfaces/functionality covered by the
> generic confidentiality or integrity categories, then we'll end up having to
> choose between lost functionality or overprivileged processes, neither of
> which is optimal.
>
> Is it truly the case that everything under the "confidentiality" category
> poses the same level of risk to kernel confidentiality, and similarly for
> everything under the "integrity" category? If not, then being able to
> distinguish them definitely has benefit.
Good question. We can't know the answer to this unless we know how an
attacker might leverage access.
The value here IMHO is more in allowing tradeoffs to be made by system
designers vs. disabling lockdown entirely.
> I'm still not clear though on how/if this will compose with or be overridden
> by other security modules. We would need some means for another security
> module to take over lockdown decisions once it has initialized (including
> policy load), and to be able to access state that is currently private to the
> lockdown module, like the level.
Why not utilize stacking (restrictively), similarly to capabilities?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Michal Kubecek @ 2019-06-27 18:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Jiri Pirko, davem, jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <26b73332-9ea0-9d2c-9185-9de522c72bb9@gmail.com>
On Thu, Jun 27, 2019 at 11:14:31AM -0600, David Ahern wrote:
> > 4) There are two cases that can happen during rename:
> > A) The name is shorter than IFNAMSIZ
> > -> both IFLA_NAME and IFLA_NAME_EXT would contain the same string:
> > original IFLA_NAME = eth0
> > original IFLA_NAME_EXT = eth0
> > renamed IFLA_NAME = enp5s0f1npf0vf1
> > renamed IFLA_NAME_EXT = enp5s0f1npf0vf1
> > B) The name is longer tha IFNAMSIZ
> > -> IFLA_NAME would contain the original one, IFLA_NAME_EXT would
> > contain the new one:
> > original IFLA_NAME = eth0
> > original IFLA_NAME_EXT = eth0
> > renamed IFLA_NAME = eth0
> > renamed IFLA_NAME_EXT = enp131s0f1npf0vf22
>
> so kernel side there will be 2 names for the same net_device?
It often feels as a deficiency that unlike block devices where we can
keep one name and create multiple symlinks based on different naming
schemes, network devices can have only one name. There are aliases but
AFAIK they are only used (and can be only used) for SNMP. IMHO this
limitation is part of the mess that left us with so-called "predictable
names" which are in practice neither persistent nor predictable.
So perhaps we could introduce actual aliases (or altnames or whatever we
would call them) for network devices that could be used to identify
a network device whenever both kernel and userspace tool supports them.
Old (and ancient) tools would have to use the one canonical name limited
to current IFNAMSIZ, new tools would allow using any alias which could
be longer.
Michal
^ permalink raw reply
* Re: [PATCH net-next 00/10] FDB, VLAN and PTP fixes for SJA1105 DSA
From: David Miller @ 2019-06-27 18:04 UTC (permalink / raw)
To: olteanv; +Cc: f.fainelli, vivien.didelot, andrew, netdev
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
From: Vladimir Oltean <olteanv@gmail.com>
Date: Wed, 26 Jun 2019 02:39:32 +0300
> This patchset is an assortment of fixes for the net-next version of the
> sja1105 DSA driver:
> - Avoid a kernel panic when the driver fails to probe or unregisters
> - Finish Arnd Bermann's idea of compiling PTP support as part of the
> main DSA driver and not separately
> - Better handling of initial port-based VLAN as well as VLANs for
> dsa_8021q FDB entries
> - Fix address learning for the SJA1105 P/Q/R/S family
> - Make static FDB entries persistent across switch resets
> - Fix reporting of statically-added FDB entries in 'bridge fdb show'
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH V4 00/10] net: dsa: microchip: Convert to regmap
From: David Miller @ 2019-06-27 18:00 UTC (permalink / raw)
To: marex; +Cc: netdev, andrew, f.fainelli, Tristram.Ha, Woojung.Huh
In-Reply-To: <20190625234348.16246-1-marex@denx.de>
From: Marek Vasut <marex@denx.de>
Date: Wed, 26 Jun 2019 01:43:38 +0200
> This patchset converts KSZ9477 switch driver to regmap.
>
> This was tested with extra patches on KSZ8795. This was also tested
> on KSZ9477 on Microchip KSZ9477EVB board, which I now have.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v3 0/8] net: aquantia: implement vlan offloads
From: David Miller @ 2019-06-27 17:58 UTC (permalink / raw)
To: Igor.Russkikh; +Cc: netdev
In-Reply-To: <cover.1561552290.git.igor.russkikh@aquantia.com>
From: Igor Russkikh <Igor.Russkikh@aquantia.com>
Date: Wed, 26 Jun 2019 12:35:30 +0000
> This patchset introduces hardware VLAN offload support and also does some
> maintenance: we replace driver version with uts version string, add
> documentation file for atlantic driver, and update maintainers
> adding Igor as a maintainer.
>
> v3: shuffle doc sections, per Andrew's comments
>
> v2: updates in doc, gpl spdx tag cleanup
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net] net/ibmvnic: Report last valid speed and duplex values to ethtool
From: Andrew Lunn @ 2019-06-27 17:57 UTC (permalink / raw)
To: Thomas Falcon; +Cc: netdev, linuxppc-dev, bjking1, pradeep, dnbanerg
In-Reply-To: <1561655353-17114-1-git-send-email-tlfalcon@linux.ibm.com>
On Thu, Jun 27, 2019 at 12:09:13PM -0500, Thomas Falcon wrote:
> This patch resolves an issue with sensitive bonding modes
> that require valid speed and duplex settings to function
> properly. Currently, the adapter will report that device
> speed and duplex is unknown if the communication link
> with firmware is unavailable.
Dumb question. If you cannot communicate with the firmware, isn't the
device FUBAR? So setting the LACP port to disabled is the correct
things to do.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 12/16] mlxsw: spectrum: PTP: Support timestamping on Spectrum-1
From: Petr Machata @ 2019-06-27 17:57 UTC (permalink / raw)
To: David Miller
Cc: idosch@idosch.org, netdev@vger.kernel.org, Jiri Pirko, mlxsw,
Ido Schimmel
In-Reply-To: <20190627.100427.16208207750306183.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Ido Schimmel <idosch@idosch.org>
> Date: Thu, 27 Jun 2019 16:52:55 +0300
>
>> + for (i = 0; i < num_rec; ++i) {
>
> Please use the more canonical "i++" here, thank you.
OK.
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] libbpf: capture value in BTF type info for BTF-defined map defs
From: Song Liu @ 2019-06-27 17:55 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, bpf@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <CAEf4Bza1p4ozVV-Vn8ibV6JRtGc_voh-Mkx51eWvuVi1P8ogSA@mail.gmail.com>
> On Jun 27, 2019, at 10:47 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jun 27, 2019 at 10:27 AM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Jun 26, 2019, at 4:21 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>>>
>>> Change BTF-defined map definitions to capture compile-time integer
>>> values as part of BTF type definition, to avoid split of key/value type
>>> information and actual type/size/flags initialization for maps.
>>
>> If I have an old bpf program and compiled it with new llvm, will it
>> work with new libbpf?
>
> You mean BPF programs that used previous incarnation of BTF-defined
> maps? No, they won't work. But we never released them, so I think it's
> ok to change them. Nothing should be using that except for selftests,
> which I fixed.
I see. This makes sense.
>
>>
>>
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> ---
>
> <snip>
>
>>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
>>> index 1a5b1accf091..aa5ddf58c088 100644
>>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
>>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
>>> @@ -8,6 +8,9 @@
>>> */
>>> #define SEC(NAME) __attribute__((section(NAME), used))
>>>
>>> +#define __int(name, val) int (*name)[val]
>>> +#define __type(name, val) val *name
>>> +
>>
>> I think we need these two in libbpf.
>
> Yes, but it's another story for another set of patches. We'll need to
> provide bpf_helpers as part of libbpf for inclusion into BPF programs,
> but there are a bunch of problems right now with existing
> bpf_heplers.h that prevents us from just copying it into libbpf. We'll
> need to resolve those first.
>
> But then again, there is no use of __int and __type for user-space
> programs, so for now it's ok.
OK. How about we put these two lines in an separate patch?
Thanks,
Song
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Stephen Hemminger @ 2019-06-27 17:56 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Jiri Pirko, netdev, davem, sthemmin, dsahern, mlxsw
In-Reply-To: <20190627104808.1404049a@cakuba.netronome.com>
On Thu, 27 Jun 2019 10:48:08 -0700
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Thu, 27 Jun 2019 11:43:27 +0200, Jiri Pirko wrote:
> > Hi all.
> >
> > In the past, there was repeatedly discussed the IFNAMSIZ (16) limit for
> > netdevice name length. Now when we have PF and VF representors
> > with port names like "pfXvfY", it became quite common to hit this limit:
> > 0123456789012345
> > enp131s0f1npf0vf6
> > enp131s0f1npf0vf22
> >
> > Since IFLA_NAME is just a string, I though it might be possible to use
> > it to carry longer names as it is. However, the userspace tools, like
> > iproute2, are doing checks before print out. So for example in output of
> > "ip addr" when IFLA_NAME is longer than IFNAMSIZE, the netdevice is
> > completely avoided.
> >
> > So here is a proposal that might work:
> > 1) Add a new attribute IFLA_NAME_EXT that could carry names longer than
> > IFNAMSIZE, say 64 bytes. The max size should be only defined in kernel,
> > user should be prepared for any string size.
> > 2) Add a file in sysfs that would indicate that NAME_EXT is supported by
> > the kernel.
> > 3) Udev is going to look for the sysfs indication file. In case when
> > kernel supports long names, it will do rename to longer name, setting
> > IFLA_NAME_EXT. If not, it does what it does now - fail.
> > 4) There are two cases that can happen during rename:
> > A) The name is shorter than IFNAMSIZ
> > -> both IFLA_NAME and IFLA_NAME_EXT would contain the same string:
> > original IFLA_NAME = eth0
> > original IFLA_NAME_EXT = eth0
> > renamed IFLA_NAME = enp5s0f1npf0vf1
> > renamed IFLA_NAME_EXT = enp5s0f1npf0vf1
> > B) The name is longer tha IFNAMSIZ
> > -> IFLA_NAME would contain the original one, IFLA_NAME_EXT would
> > contain the new one:
> > original IFLA_NAME = eth0
> > original IFLA_NAME_EXT = eth0
> > renamed IFLA_NAME = eth0
> > renamed IFLA_NAME_EXT = enp131s0f1npf0vf22
>
> I think B is the only way, A risks duplicate IFLA_NAMEs over ioctl,
> right? And maybe there is some crazy application out there which
> mixes netlink and ioctl.
>
> I guess it's not worse than status quo, given that today renames
> will fail and we will either get truncated names or eth0s..
>
> > This would allow the old tools to work with "eth0" and the new
> > tools would work with "enp131s0f1npf0vf22". In sysfs, there would
> > be symlink from one name to another.
> >
> > Also, there might be a warning added to kernel if someone works
> > with IFLA_NAME that the userspace tool should be upgraded.
> >
> > Eventually, only IFLA_NAME_EXT is going to be used by everyone.
> >
> > I'm aware there are other places where similar new attribute
> > would have to be introduced too (ip rule for example).
> > I'm not saying this is a simple work.
> >
> > Question is what to do with the ioctl api (get ifindex etc). I would
> > probably leave it as is and push tools to use rtnetlink instead.
> >
> > Any ideas why this would not work? Any ideas how to solve this
> > differently?
>
> Since we'd have to update all user space to make use of the new names
> I'd be tempted to move to a more structured device identification.
>
> 5: enp131s0f1npf0vf6: <BROADCAST,MULTICAST> ...
>
> vs:
>
> 5: eth5 (parent enp131s0f1 pf 0 vf 6 peer X*): <BROADCAST,MULTICAST> ...
>
> * ;)
>
> And allow filtering/selection of device based on more attributes than
> just name and ifindex. In practice in container workloads, for example,
> the names are already very much insufficient to identify the device.
> Refocusing on attributes is probably a big effort and not that practical
> for traditional CLI users? IDK
>
> Anyway, IMHO your scheme is strictly better than status quo.
Or Cisco style naming ;-) Ethernet0/0
There is a better solution for human use already.
the field ifalias allows arbitrary values and hooked into SNMP.
Why not have userspace fill in this field with something by default?
^ 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