* Re: [PATCH v4 00/13] net: Add generic and Allwinner YAML bindings
From: Rob Herring @ 2019-06-27 19:17 UTC (permalink / raw)
To: David Miller
Cc: Maxime Ripard, Mark Rutland, Frank Rowand, Chen-Yu Tsai,
Maxime Coquelin, Alexandre Torgue, netdev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
devicetree, linux-stm32, Maxime Chevallier, Antoine Tenart,
Andrew Lunn, Florian Fainelli, Heiner Kallweit
In-Reply-To: <20190627.102256.1839462093915893704.davem@davemloft.net>
On Thu, Jun 27, 2019 at 11:22 AM David Miller <davem@davemloft.net> wrote:
>
> From: Maxime Ripard <maxime.ripard@bootlin.com>
> Date: Thu, 27 Jun 2019 17:31:42 +0200
>
> > This is an attempt at getting the main generic DT bindings for the ethernet
> > (and related) devices, and convert some DT bindings for the Allwinner DTs
> > to YAML as well.
> >
> > This should provide some DT validation coverage.
>
> I don't think this should go via my tree as it's all DT stuff.
That's fine. I can take it. There's one conflict with commit
79b647a0c0d5 ("dt-bindings: net: document new usxgmii phy mode"), but
that's easy enough to handle. Any other changes to the binding docs
will need to go thru me this cycle.
Rob
^ permalink raw reply
* Re: [PATCH 2/2 nf-next] netfilter:nft_meta: add NFT_META_VLAN support
From: Pablo Neira Ayuso @ 2019-06-27 19:16 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <c9ce6a77-a5db-b3a1-ab48-eb6bc97337e1@ucloud.cn>
On Thu, Jun 27, 2019 at 09:37:53PM +0800, wenxu wrote:
>
> 在 2019/6/27 20:35, Pablo Neira Ayuso 写道:
> > On Thu, Jun 27, 2019 at 10:09:17AM +0800, wenxu@ucloud.cn wrote:
> >> From: wenxu <wenxu@ucloud.cn>
> >>
> >> This patch provide a meta vlan to set the vlan tag of the packet.
> >>
> >> for q-in-q vlan id 20:
> >> meta vlan set 0x88a8:20
> > Actually, I think this is not very useful for stacked vlan since this
> > just sets/mangles the existing meta vlan data.
> >
> > We'll need infrastructure that uses skb_vlan_push() and _pop().
> >
> > Patch looks good anyway, such infrastructure to push/pop can be added
> > later on.
> >
> > Thanks.
>
> yes, It's just ste/mangle the meta vlan data. I just wonder if we set for stacked vlan.
> vlan meta 0x88a8:20. The packet should contain a 0x8100 vlan tag, we just push the
> inner vlan and the the vlan meta with the outer 0x88a8:20. Or the packet don't contain
> only vlan tag, we add a inner 0x8100:20 tag and outer 0x88a8:20 tag?
You got me thinking here.
I wonder if we can just make this fit into nft_payload.
Or just add a new nft_vlan extension for this specifically, to push,
to mangle and to pop vlan headers. This would be a simple solution for
this.
I need to explore this by the weekend, will get back to you beginning
next week.
Feedback is welcome in any case :-), thanks.
^ permalink raw reply
* Re: [PATCH 1/2 nf-next v2] netfilter: nft_meta: add NFT_META_BRI_O/IIFVPROTO support
From: Pablo Neira Ayuso @ 2019-06-27 19:12 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1561640835-4507-1-git-send-email-wenxu@ucloud.cn>
On Thu, Jun 27, 2019 at 09:07:14PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> This patch provide a meta to get the bridge vlan proto
>
> nft add rule bridge firewall zones counter meta br_iifvproto 0x8100
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> include/uapi/linux/netfilter/nf_tables.h | 4 ++++
> net/netfilter/nft_meta.c | 18 ++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 8859535..0f75a6d 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -796,6 +796,8 @@ enum nft_exthdr_attributes {
> * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
> * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
> * @NFT_META_BRI_PVID: packet input bridge port pvid
An initial patch to re-name NFT_META_BRI_PVID to NFT_META_BRI_IIFVID
would be good, and to add NFT_META_BRI_OIFVID... if you have a usecase
for this, of course.
Thanks.
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Daniel Santos @ 2019-06-27 19:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King - ARM Linux admin, René van Dorst, sean.wang,
f.fainelli, davem, matthias.bgg, vivien.didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <20190625204148.GB27733@lunn.ch>
On 6/25/19 3:41 PM, Andrew Lunn wrote:
> On Tue, Jun 25, 2019 at 02:27:55PM -0500, Daniel Santos wrote:
>> On 6/25/19 2:02 PM, Andrew Lunn wrote:
>>>> But will there still be a mechanism to ignore link partner's advertising
>>>> and force these parameters?
>>> >From man 1 ethtool:
>>>
>>> -a --show-pause
>>> Queries the specified Ethernet device for pause parameter information.
>>>
>>> -A --pause
>>> Changes the pause parameters of the specified Ethernet device.
>>>
>>> autoneg on|off
>>> Specifies whether pause autonegotiation should be enabled.
>>>
>>> rx on|off
>>> Specifies whether RX pause should be enabled.
>>>
>>> tx on|off
>>> Specifies whether TX pause should be enabled.
>>>
>>> You need to check the driver to see if it actually implements this
>>> ethtool call, but that is how it should be configured.
>>>
>>> Andrew
>>>
>> Thank you Andrew,
>>
>> So in this context, my question is the difference between "enabling" and
>> "forcing". Here's that register for the mt7620 (which has an mt7530 on
>> its die): https://imgur.com/a/pTk0668 I believe this is also what René
>> is seeking clarity on?
> Lets start with normal operation. If the MAC supports pause or asym
> pause, it calls phy_support_sym_pause() or phy_support_asym_pause().
> phylib will then configure the PHY to advertise pause as appropriate.
> Once auto-neg has completed, the results of the negotiation are set in
> phydev. So phdev->pause and phydev->asym_pause. The MAC callback is
> then used to tell the MAC about the autoneg results. The MAC should be
> programmed using the values in phdev->pause and phydev->asym_pause.
>
> For ethtool, the MAC driver needs to implement .get_pauseparam and
> .set_pauseparam. The set_pauseparam needs to validate the settings,
> using phy_validate_pause(). If valid, phy_set_asym_pause() is used to
> tell the PHY about the new configuration. This will trigger a new
> auto-neg if auto-neg is enabled, and the results will be passed back
> in the usual way. If auto-neg is disabled, or pause auto-neg is
> disabled, the MAC should configure pause directly based on the
> settings passed.
>
> Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
> want the MAC directly talking to the PHY. Bad things will happen.
So what exactly do you mean by the MAC directly talking to the PHY? Do
you mean setting speed, duplex, etc. via the MAC registers instead of
via MDIO to the MII registers of the PHY?
> Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
> phydev->asym_pause.
>
> The same idea applies when using phylink.
>
> Andrew
You're help is greatly appreciated here. Admittedly, I'm also trying to
get this working in the now deprecated swconfig for a 3.18 kernel that's
in production. In my code, I had just set the appropriate bits in both
the MAC and mii registers -- did I just shoot myself in the foot or only
toe or two? :) I should probably start a separate thread for this.
(And probably attempt to wrestle an mt7530 programmer's guide out of
MediaTek!)
Thanks,
Daniel
PS: I found a rather humorous quote from the mt7621 datasheet regarding
the MAC registers (at 0x3000 for port 0, 0x3100 for port 1, etc.):
2.4 Link Status
You can find MAC control register put at 0x3500 for MAC 5, and
0x3600 for MAC 6. You can change
MAC ability at this register. We would suggest don’t use the
register 0x3000 to 0x3400. It may not
work.
I'm not sure if this only applies to something in between the mt7621 and
it's internal mt7530 or not.
^ permalink raw reply
* Re: [net/tls] Re: KMSAN: uninit-value in aesti_encrypt
From: Eric Biggers @ 2019-06-27 19:01 UTC (permalink / raw)
To: John Fastabend
Cc: Boris Pismenny, Aviad Yehezkel, Dave Watson, Daniel Borkmann,
netdev, davem, glider, herbert, linux-crypto, linux-kernel,
syzkaller-bugs, syzbot
In-Reply-To: <5d1508c79587a_e392b1ee39f65b45b@john-XPS-13-9370.notmuch>
On Thu, Jun 27, 2019 at 11:19:51AM -0700, John Fastabend wrote:
> 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.
>
Okay, great! However, just to clarify, the 17 syzbot bugs I assigned to TLS are
in addition to the 30 I assigned to BPF
(https://lore.kernel.org/lkml/20190624050114.GA30702@sol.localdomain/).
(Well, since I sent that it's actually up to 35 now.)
I do expect most of these are duplicates, so when you are fixing the bugs, it
would be really helpful (for everyone, including you in the future :-) ) if you
would include the corresponding Reported-by syzbot line for *every* syzbot
report you think is addressed, so they get closed.
- Eric
^ permalink raw reply
* Re: [PATCH] netfilter: Fix remainder of pseudo-header protocol 0
From: Pablo Neira Ayuso @ 2019-06-27 18:49 UTC (permalink / raw)
To: zhe.he; +Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <1561346258-272481-1-git-send-email-zhe.he@windriver.com>
On Mon, Jun 24, 2019 at 11:17:38AM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> Since v5.1-rc1, some types of packets do not get unreachable reply with the
> following iptables setting. Fox example,
>
> $ iptables -A INPUT -p icmp --icmp-type 8 -j REJECT
> $ ping 127.0.0.1 -c 1
> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
> — 127.0.0.1 ping statistics —
> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
>
> We should have got the following reply from command line, but we did not.
> From 127.0.0.1 icmp_seq=1 Destination Port Unreachable
>
> Yi Zhao reported it and narrowed it down to:
> 7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it"),
>
> This is because nf_ip_checksum still expects pseudo-header protocol type 0 for
> packets that are of neither TCP or UDP, and thus ICMP packets are mistakenly
> treated as TCP/UDP.
>
> This patch corrects the conditions in nf_ip_checksum and all other places that
> still call it with protocol 0.
Looking at 7fc38225363dd8f19e667ad7c77b63bc4a5c065d, I wonder this can
be fixed while simplifying it...
I think nf_reject_verify_csum() is useless?
In your patch, now you explicitly check for IPPROTO_TCP and
IPPROTO_UDP to validate the checksum.
^ permalink raw reply
* 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
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