* 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
* [PATCH v2 net-next] net: phy: improve definition of __ETHTOOL_LINK_MODE_MASK_NBITS
From: Heiner Kallweit @ 2019-02-22 18:25 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
The way to define __ETHTOOL_LINK_MODE_MASK_NBITS seems to be overly
complicated, go with a standard approach instead.
Whilst we're at it, move the comment to the right place.
v2:
- rebased
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
include/linux/ethtool.h | 4 ----
include/uapi/linux/ethtool.h | 17 +++++++++--------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 19a8de532..e6ebc9761 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -98,10 +98,6 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
return index % n_rx_rings;
}
-/* number of link mode bits/ulongs handled internally by kernel */
-#define __ETHTOOL_LINK_MODE_MASK_NBITS \
- (__ETHTOOL_LINK_MODE_LAST + 1)
-
/* declare a link mode bitmap */
#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 378c52308..3652b239d 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1432,6 +1432,13 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT = 29,
ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT = 30,
ETHTOOL_LINK_MODE_25000baseCR_Full_BIT = 31,
+
+ /* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
+ * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
+ * macro for bits > 31. The only way to use indices > 31 is to
+ * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
+ */
+
ETHTOOL_LINK_MODE_25000baseKR_Full_BIT = 32,
ETHTOOL_LINK_MODE_25000baseSR_Full_BIT = 33,
ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT = 34,
@@ -1469,14 +1476,8 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT = 65,
ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66,
- /* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
- * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
- * macro for bits > 31. The only way to use indices > 31 is to
- * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
- */
-
- __ETHTOOL_LINK_MODE_LAST
- = ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
+ /* must be last entry */
+ __ETHTOOL_LINK_MODE_MASK_NBITS
};
#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name) \
--
2.20.1
^ permalink raw reply related
* [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets
From: Haiyang Zhang @ 2019-02-22 18:25 UTC (permalink / raw)
To: sashal, linux-hyperv
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, netdev,
linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
Incoming packets may have IP header checksum verified by the host.
They may not have IP header checksum computed after coalescing.
This patch re-compute the checksum when necessary, otherwise the
packets may be dropped, because Linux network stack always checks it.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 256adbd044f5..cf4897043e83 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
schedule_delayed_work(&ndev_ctx->dwork, 0);
}
+static void netvsc_comp_ipcsum(struct sk_buff *skb)
+{
+ struct iphdr *iph = (struct iphdr *)skb->data;
+
+ iph->check = 0;
+ iph->check = ip_fast_csum(iph, iph->ihl);
+}
+
static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
struct netvsc_channel *nvchan)
{
@@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
/* skb is already created with CHECKSUM_NONE */
skb_checksum_none_assert(skb);
- /*
- * In Linux, the IP checksum is always checked.
- * Do L4 checksum offload if enabled and present.
+ /* Incoming packets may have IP header checksum verified by the host.
+ * They may not have IP header checksum computed after coalescing.
+ * We compute it here if the flags are set, because on Linux, the IP
+ * checksum is always checked.
+ */
+ if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
+ csum_info->receive.ip_checksum_succeeded &&
+ skb->protocol == htons(ETH_P_IP))
+ netvsc_comp_ipcsum(skb);
+
+ /* Do L4 checksum offload if enabled and present.
*/
if (csum_info && (net->features & NETIF_F_RXCSUM)) {
if (csum_info->receive.tcp_checksum_succeeded ||
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next 1/7] net: phy: marvell10g: Use get_features to get the PHY abilities
From: Heiner Kallweit @ 2019-02-22 18:42 UTC (permalink / raw)
To: Maxime Chevallier, davem
Cc: netdev, linux-kernel, Andrew Lunn, Florian Fainelli, Russell King,
linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190221095128.28188-2-maxime.chevallier@bootlin.com>
On 21.02.2019 10:51, Maxime Chevallier wrote:
> The Alaska family of 10G PHYs has more abilities than the ones listed in
> PHY_10GBIT_FULL_FEATURES, the exact list depending on the model.
>
> Make use of the newly introduced .get_features call to build this list,
> using genphy_c45_pma_read_abilities to build the list of supported
> linkmodes, and adding autoneg ability based on what's reported by the AN
> MMD.
>
> .config_init is still used to validate the interface_mode.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> drivers/net/phy/marvell10g.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 9ea27acf05ad..65ef469adf58 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev)
>
> static int mv3310_config_init(struct phy_device *phydev)
> {
> - int ret, val;
> -
> /* Check that the PHY interface type is compatible */
> if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
> phydev->interface != PHY_INTERFACE_MODE_XAUI &&
> @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev)
> phydev->interface != PHY_INTERFACE_MODE_10GKR)
> return -ENODEV;
>
> + return 0;
> +}
> +
> +static int mv3310_get_features(struct phy_device *phydev)
> +{
After my just submitted patch to include the aneg capability checking in
genphy_c45_pma_read_abilities() function mv3310_get_features() isn't
needed any longer and can be replaced with the generic one.
But we can make this change afterwards, then you don't have to
rework your series.
Also I'm not sure whether there will be a 5.0-rc8 or whether beginning
of next week we'll see 5.0. In the latter case we're a little bit in a
hurry because the merge window will start very soon.
> + int ret, val;
> if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
> val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> if (val < 0)
> @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = {
> .phy_id = 0x002b09aa,
> .phy_id_mask = MARVELL_PHY_ID_MASK,
> .name = "mv88x3310",
> - .features = PHY_10GBIT_FEATURES,
> + .get_features = mv3310_get_features,
> .soft_reset = gen10g_no_soft_reset,
> .config_init = mv3310_config_init,
> .probe = mv3310_probe,
>
^ permalink raw reply
* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-22 18:53 UTC (permalink / raw)
To: Michal Kubecek; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222172715.GT23151@unicorn.suse.cz>
On Fri, 22 Feb 2019 18:27:15 +0100, Michal Kubecek wrote:
> > > 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.
Yes, let's just go with your first proposal of a static inline helper.
I think it's the cleanest solution.
^ permalink raw reply
* [PATCH] net: dsa: Remove documentation for port_fdb_prepare
From: Hauke Mehrtens @ 2019-02-22 19:07 UTC (permalink / raw)
To: davem; +Cc: arkadis, netdev, linux-doc, Hauke Mehrtens
This callback was removed some time ago, also remove the documentation.
Fixes: 1b6dd556c304 ("net: dsa: Remove prepare phase for FDB")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
Documentation/networking/dsa/dsa.txt | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/Documentation/networking/dsa/dsa.txt b/Documentation/networking/dsa/dsa.txt
index 25170ad7d25b..101f2b2c69ad 100644
--- a/Documentation/networking/dsa/dsa.txt
+++ b/Documentation/networking/dsa/dsa.txt
@@ -533,16 +533,12 @@ Bridge VLAN filtering
function that the driver has to call for each VLAN the given port is a member
of. A switchdev object is used to carry the VID and bridge flags.
-- port_fdb_prepare: bridge layer function invoked when the bridge prepares the
- installation of a Forwarding Database entry. If the operation is not
- supported, this function should return -EOPNOTSUPP to inform the bridge code
- to fallback to a software implementation. No hardware setup must be done in
- this function. See port_fdb_add for this and details.
-
- port_fdb_add: bridge layer function invoked when the bridge wants to install a
Forwarding Database entry, the switch hardware should be programmed with the
specified address in the specified VLAN Id in the forwarding database
- associated with this VLAN ID
+ associated with this VLAN ID. If the operation is not supported, this
+ function should return -EOPNOTSUPP to inform the bridge code to fallback to
+ a software implementation.
Note: VLAN ID 0 corresponds to the port private database, which, in the context
of DSA, would be the its port-based VLAN, used by the associated bridge device.
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Al Viro @ 2019-02-22 19:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Biggers, netdev, David S . Miller, linux-kernel, Mao Wenan,
Cong Wang, Lorenzo Colitti, Tetsuo Handa
In-Reply-To: <0de89a50-6d5c-1d7b-19f5-9a13465bcebd@gmail.com>
On Fri, Feb 22, 2019 at 10:25:09AM -0800, Eric Dumazet wrote:
>
>
> 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()
An interesting question is whether anything else will be confused by
sock->sk && sock->sk->sk_socket != sock
I'd still like to figure out if we could simply make sock_orphan()
do something like
if (likely(sk->sk_socket))
sk->sk_socket->sk = NULL;
just before sk_set_socket(sk, NULL);
That would make for much easier rules; the question is whether anything
relies upon the windows when linkage between socket and sock is not
symmetrical...
^ permalink raw reply
* [PATCH] net: dsa: lantiq: Add GPHY firmware files
From: Hauke Mehrtens @ 2019-02-22 19:11 UTC (permalink / raw)
To: davem; +Cc: netdev, Hauke Mehrtens
This adds the file names of the FW files which this driver handles into
the module description.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
drivers/net/dsa/lantiq_gswip.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 693a67f45bef..ddc1f9ca8ebc 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1162,6 +1162,12 @@ static struct platform_driver gswip_driver = {
module_platform_driver(gswip_driver);
+MODULE_FIRMWARE("lantiq/xrx300_phy11g_a21.bin");
+MODULE_FIRMWARE("lantiq/xrx300_phy22f_a21.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy11g_a14.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy11g_a22.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy22f_a14.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy22f_a22.bin");
MODULE_AUTHOR("Hauke Mehrtens <hauke@hauke-m.de>");
MODULE_DESCRIPTION("Lantiq / Intel GSWIP driver");
MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related
* [PATCH] net: lantiq: Do not use eth_change_mtu()
From: Hauke Mehrtens @ 2019-02-22 19:12 UTC (permalink / raw)
To: davem; +Cc: netdev, Hauke Mehrtens
eth_change_mtu() is not needed any more, the networking subsystem will
call it automatically when this callback is not implemented.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
drivers/net/ethernet/lantiq_xrx200.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 2d4d10a017e5..d29104de0d53 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -335,7 +335,6 @@ static const struct net_device_ops xrx200_netdev_ops = {
.ndo_start_xmit = xrx200_start_xmit,
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
- .ndo_change_mtu = eth_change_mtu,
};
static irqreturn_t xrx200_dma_irq(int irq, void *ptr)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
From: David Summers @ 2019-02-22 19:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Stefan Wahren
Cc: Rob Herring, Mark Rutland, devicetree, Johan Hedberg,
Maxime Ripard, netdev, Marcel Holtmann, linux-bluetooth,
Chen-Yu Tsai, David S. Miller, arm-linux
In-Reply-To: <CA+E=qVdMNmVkytCzXqYgJtV196-G=c_bWx+aLvs=Y0iuRpjdRg@mail.gmail.com>
Hi Vasily,
Just catching up with this series - good that you are doing it. We need
it for users on ArchLinux ARM!
On question though, what is "firmware-postfix" used for? I see in the
documentation, that it points to the board name. But can't see where
else it is used.
Is there a need to have the board name?
Anyway good work.
David.
On 18/02/2019 22:28, Vasily Khoruzhick wrote:
> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Hi Vasily,
> Hi Stefan,
>
>>> Vasily Khoruzhick <anarsoul@gmail.com> hat am 18. Februar 2019 um 22:24 geschrieben:
>>>
>>>
>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <robh@kernel.org> wrote:
>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>>>
>>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>>>> ---
>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>>>> 1 file changed, 35 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>> new file mode 100644
>>>>> index 000000000000..8357f242ae4c
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>> @@ -0,0 +1,35 @@
>>>>> +RTL8723BS/RTL8723CS Bluetooth
>>>>> +---------------------
>>>>> +
>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>>>> +to upload firmware and change module speed.
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible: should be one of the following:
>>>>> + * "realtek,rtl8723bs-bt"
>>>>> + * "realtek,rtl8723cs-bt"
>>>>> +Optional properties:
>>>>> +
>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>>
>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> Looks good to me, but you may need to add firmware-postfix.
>
>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
>
>> Stefan
^ permalink raw reply
* Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
From: Vasily Khoruzhick @ 2019-02-22 19:21 UTC (permalink / raw)
To: David Summers
Cc: Stefan Wahren, Rob Herring, Mark Rutland, devicetree,
Johan Hedberg, Maxime Ripard, netdev, Marcel Holtmann,
open list:BLUETOOTH DRIVERS, Chen-Yu Tsai, David S. Miller,
arm-linux
In-Reply-To: <8b97f349-4416-058c-0d01-3be592b89a7d@davidjohnsummers.uk>
On Fri, Feb 22, 2019 at 11:14 AM David Summers
<beagleboard@davidjohnsummers.uk> wrote:
>
> Hi Vasily,
>
> Just catching up with this series - good that you are doing it. We need
> it for users on ArchLinux ARM!
>
> On question though, what is "firmware-postfix" used for? I see in the
> documentation, that it points to the board name. But can't see where
> else it is used.
See h5_serdev_probe() in drivers/bluetooth/hci_h5.c. Basically it
specifies what firmware config to use.
I agree with Rob that we should probably use firmware-name here instead.
> Is there a need to have the board name?
As far as I understand firmware config depends on board, so I think
it's a good idea to use board name here.
> Anyway good work.
Thanks!
>
> David.
>
> On 18/02/2019 22:28, Vasily Khoruzhick wrote:
> > On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >> Hi Vasily,
> > Hi Stefan,
> >
> >>> Vasily Khoruzhick <anarsoul@gmail.com> hat am 18. Februar 2019 um 22:24 geschrieben:
> >>>
> >>>
> >>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <robh@kernel.org> wrote:
> >>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> >>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> >>>>>
> >>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> >>>>> ---
> >>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> >>>>> 1 file changed, 35 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..8357f242ae4c
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>> @@ -0,0 +1,35 @@
> >>>>> +RTL8723BS/RTL8723CS Bluetooth
> >>>>> +---------------------
> >>>>> +
> >>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> >>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
> >>>>> +to upload firmware and change module speed.
> >>>>> +
> >>>>> +Required properties:
> >>>>> +
> >>>>> + - compatible: should be one of the following:
> >>>>> + * "realtek,rtl8723bs-bt"
> >>>>> + * "realtek,rtl8723cs-bt"
> >>>>> +Optional properties:
> >>>>> +
> >>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> >>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> >>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> >>>>> + - firmware-postfix: firmware postfix to be used for firmware config
> >> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
> >>
> >> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> > Looks good to me, but you may need to add firmware-postfix.
> >
> >> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> > Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
> >
> >> Stefan
>
>
^ permalink raw reply
* Re: [PATCH] sctp: don't compare hb_timer expire date before starting it
From: David Miller @ 2019-02-22 19:22 UTC (permalink / raw)
To: kwiecienmaciek; +Cc: linux-sctp, netdev, alexander.sverdlin, maciej.kwiecien
In-Reply-To: <20190222084526.8214-1-maciej.kwiecien@nokia.com>
From: kwiecienmaciek@gmail.com
Date: Fri, 22 Feb 2019 09:45:26 +0100
> From: Maciej Kwiecien <maciej.kwiecien@nokia.com>
>
> hb_timer might not start at all for a particular transport because its
> start is conditional. In a result a node is not sending heartbeats.
>
> Function sctp_transport_reset_hb_timer has two roles:
> - initial start of hb_timer for a given transport,
> - update expire date of hb_timer for a given transport.
> The function is optimized to update timer's expire only if it is before
> a new calculated one but this comparison is invalid for a timer which
> has not yet started. Such a timer has expire == 0 and if a new expire
> value is bigger than (MAX_JIFFIES / 2 + 2) then "time_before" macro will
> fail and timer will not start resulting in no heartbeat packets send by
> the node.
>
> This was found when association was initialized within first 5 mins
> after system boot due to jiffies init value which is near to MAX_JIFFIES.
>
> Test kernel version: 4.9.154 (ARCH=arm)
> hb_timer.expire = 0; //initialized, not started timer
> new_expire = MAX_JIFFIES / 2 + 2; //or more
> time_before(hb_timer.expire, new_expire) == false
>
> Fixes: ba6f5e33bdbb ("sctp: avoid refreshing heartbeat timer too often")
> Reported-by: Marcin Stojek <marcin.stojek@nokia.com>
> Tested-by: Marcin Stojek <marcin.stojek@nokia.com>
> Signed-off-by: Maciej Kwiecien <maciej.kwiecien@nokia.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: Tom Herbert @ 2019-02-22 19:24 UTC (permalink / raw)
To: Dominique Martinet
Cc: Tom Herbert, David Miller, Doron Roberts-Kedes, Dave Watson,
Linux Kernel Network Developers, LKML
In-Reply-To: <20190221082209.GA32719@nautica>
On Thu, Feb 21, 2019 at 12:22 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Tom Herbert wrote on Wed, Feb 20, 2019:
> > > When the client closes the socket, some messages are obviously still "in
> > > flight", and the server will recv a POLLERR notification on the csock at
> > > some point with many messages left.
> > > The documentation says to unattach the csock when you get POLLER. If I
> > > do that, the kcm socket will no longer give me any message, so all the
> > > messages still in flight at the time are lost.
> >
> > So basically it sounds like you're interested in supporting TCP
> > connections that are half closed. I believe that the error in half
> > closed is EPIPE, so if the TCP socket returns that it can be ignored
> > and the socket can continue being attached and used to send data.
>
> Did you mean 'can continue being attached and used to receive data'?
>
No, I meant shutdown on receive side when FIN is receved. TX is still
allowed to drain an queued bytes. To support shutdown on the TX side
would require additional logic since we need to effectively detach the
transmit path but retain the receive path. I'm not sure this is a
compelling use case to support.
> I can confirm getsockopt with SO_ERROR gets me EPIPE, but I don't see
> how to efficiently ignore EPIPE until POLLIN gets unset -- polling on
> both the csock and kcm socket will do many needless wakeups on only the
> csock from what I can see, so I'd need some holdoff timer or something.
> I guess it's possible though.
We might need to clear the error somehow. May a read of zero bytes?
>
> > Another possibility is to add some linger semantics to an attached
> > socket. For instance, a large message might be sent so that part of
> > the messge is queued in TCP and part is queued in the KCM socket.
> > Unattach would probably break that message. We probably want to linger
> > option similar to SO_LINGER (or maybe just use the option on the TCP
> > socket) that means don't complete the detach until any message being
> > transmitted on the lower socket has been queued.
>
> That would certainly work, even if non-obvious from a user standpoint.
>
>
> > > > I'd like to see some retry on ENOMEM before this is merged though, so
> > > > while I'm there I'll resend this with a second patch doing that
> > > > retry,.. I think just not setting strp->interrupted and not reporting
> > > > the error up might be enough? Will have to try either way.
> > >
> > > I also tried playing with that without much success.
> > > I had assumed just not calling strp_parser_err() (which calls the
> > > abort_parser cb) would be enough, eventually calling strp_start_timer()
> > > like the !len case, but no can do.
> >
> > I think you need to ignore the ENOMEM and have a timer or other
> > callback to retry the operation in the future.
>
> Yes, that's what I had intended to try; basically just break out and
> schedule timer as said above.
You might want to look at some other systems, I don't recall if
there's a hook that can be used for when memory pressure is relieved.
>
> After a bit more debugging, this part works (__strp_recv() is called
> again); but the next packet that is treated properly is rejected because
> by the time __strp_recv() was called again a new skb was read and the
> length isn't large enough to go all the way into the new packet, so this
> test fails:
> } else if (len <= (ssize_t)head->len -
> skb->len - stm->strp.offset) {
> /* Length must be into new skb (and also
> * greater than zero)
> */
> STRP_STATS_INCR(strp->stats.bad_hdr_len);
> strp_parser_err(strp, -EPROTO, desc);
>
> So I need to figure a way to say "call this function again without
> reading more data" somehow, or make this check more lax e.g. accept any
> len > 0 after a retry maybe...
> Removing that branch altogether seems to work at least but I'm not sure
> we'd want to?
I like the check since it's conservative and covers the normal case.
Maybe just need some more logic?
> (grmbl at this slow VM and strparser not being possible to enable as a
> module, it takes ages to test)
>
>
> > > With that said, returning 0 from the parse function also raises POLLERR
> > > on the csock and hangs netparser, so things aren't that simple...
> >
> > Can you point to where this is happening. If the parse_msg callback
> > returns 0 that is suppose to indicate that more bytes are needed.
>
> I just blindly returned 0 "from time to time" in the kcm parser
> function, but looking at above it was failing on the same check.
> This somewhat makes sense for this one to fail here if a new packet was
> read, no sane parser function should give a length smaller than what
> they require to determine the length.
>
>
> Thanks,
> --
> Dominique
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Alexei Starovoitov @ 2019-02-22 19:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Masami Hiramatsu, Steven Rostedt, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski, daniel,
netdev, bpf
In-Reply-To: <CAHk-=whNf_n1WXWW+ugAVeL5ZK0GcEP3cTYocju1nS85VtMjjQ@mail.gmail.com>
On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>
> Then we should still probably fix up "__probe_kernel_read()" to not
> allow user accesses. The easiest way to do that is actually likely to
> use the "unsafe_get_user()" functions *without* doing a
> uaccess_begin(), which will mean that modern CPU's will simply fault
> on a kernel access to user space.
On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
and users pass both user and kernel addresses into it and expect
that the helper will actually try to read from that address.
If __probe_kernel_read will suddenly start failing on all user addresses
it will break the expectations.
How do we solve it in bpf_probe_read?
Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
in the loop?
That's doable, but people already complain that bpf_probe_read() is slow
and shows up in their perf report.
^ permalink raw reply
* Re: [PATCH net] ipvlan: disallow userns cap_net_admin to change global mode/flags
From: David Miller @ 2019-02-22 19:28 UTC (permalink / raw)
To: daniel; +Cc: maheshb, m, netdev
In-Reply-To: <20190219231530.11306-1-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 20 Feb 2019 00:15:30 +0100
> When running Docker with userns isolation e.g. --userns-remap="default"
> and spawning up some containers with CAP_NET_ADMIN under this realm, I
> noticed that link changes on ipvlan slave device inside that container
> can affect all devices from this ipvlan group which are in other net
> namespaces where the container should have no permission to make changes
> to, such as the init netns, for example.
>
> This effectively allows to undo ipvlan private mode and switch globally to
> bridge mode where slaves can communicate directly without going through
> hostns, or it allows to switch between global operation mode (l2/l3/l3s)
> for everyone bound to the given ipvlan master device. libnetwork plugin
> here is creating an ipvlan master and ipvlan slave in hostns and a slave
> each that is moved into the container's netns upon creation event.
...
> One way to mitigate it is to check CAP_NET_ADMIN permissions of
> the ipvlan master device's ns, and only then allow to change
> mode or flags for all devices bound to it. Above two cases are
> then disallowed after the patch.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied and queued up for -stable.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: dsa: Remove documentation for port_fdb_prepare
From: Florian Fainelli @ 2019-02-22 19:28 UTC (permalink / raw)
To: Hauke Mehrtens, davem; +Cc: arkadis, netdev, linux-doc
In-Reply-To: <20190222190745.28020-1-hauke@hauke-m.de>
On 2/22/19 11:07 AM, Hauke Mehrtens wrote:
> This callback was removed some time ago, also remove the documentation.
>
> Fixes: 1b6dd556c304 ("net: dsa: Remove prepare phase for FDB")
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Not sure if this is worth a Fixes: tag since this is not quite a
functional change, but other than:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Steven Rostedt @ 2019-02-22 19:30 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linus Torvalds, Masami Hiramatsu, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski, daniel,
netdev, bpf
In-Reply-To: <20190222192703.epvgxghwybte7gxs@ast-mbp.dhcp.thefacebook.com>
On Fri, 22 Feb 2019 11:27:05 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> >
> > Then we should still probably fix up "__probe_kernel_read()" to not
> > allow user accesses. The easiest way to do that is actually likely to
> > use the "unsafe_get_user()" functions *without* doing a
> > uaccess_begin(), which will mean that modern CPU's will simply fault
> > on a kernel access to user space.
>
> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> and users pass both user and kernel addresses into it and expect
> that the helper will actually try to read from that address.
>
> If __probe_kernel_read will suddenly start failing on all user addresses
> it will break the expectations.
> How do we solve it in bpf_probe_read?
> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> in the loop?
> That's doable, but people already complain that bpf_probe_read() is slow
> and shows up in their perf report.
We're changing kprobes to add a specific flag to say that we want to
differentiate between kernel or user reads. Can this be done with
bpf_probe_read()? If it's showing up in perf report, I doubt a single
check is going to cause an issue. In fact, it may actually help speed
things up as the read will be optimized for either user or kernel
address reading.
-- Steve
^ permalink raw reply
* Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Cong Wang @ 2019-02-22 19:32 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <vbfzhqpxcj9.fsf@mellanox.com>
On Thu, Feb 21, 2019 at 9:45 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Wed 20 Feb 2019 at 22:33, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Tue, Feb 19, 2019 at 1:46 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >>
> >> On Mon 18 Feb 2019 at 19:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >> > On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
> >> >>
> >> >> Flower classifier only changes root pointer during init and destroy. Cls
> >> >> API implements reference counting for tcf_proto, so there is no danger of
> >> >> concurrent access to tp when it is being destroyed, even without protection
> >> >> provided by rtnl lock.
> >> >
> >> > How about atomicity? Refcnt doesn't guarantee atomicity, how do
> >> > you make sure two concurrent modifications are atomic?
> >>
> >> In order to guarantee atomicity I lock shared flower classifier data
> >> structures with tp->lock in following patches.
> >
> > Sure, I meant the atomicity of the _whole_ change, as you know
> > the TC filters are stored in hierarchical structures: a block, a chain,
> > a tp struct, some type-specific data structure like a hash table.
> >
> > Locking tp only solves a partial of the atomicity here. Are you
> > going to restart the whole change from top down to the bottom?
>
> You mean in cases where there is a conflict? Yes, processing EAGAIN in
> tc_new_tfilter() involves releasing and re-obtaining all locks and
> references.
Sure, restart only happens when a conflict is detected, this is
why I called it worst scenario.
>
> >
> >
> >>
> >> >
> >> >
> >> >>
> >> >> Implement new function fl_head_dereference() to dereference tp->root
> >> >> without checking for rtnl lock. Use it in all flower function that obtain
> >> >> head pointer instead of rtnl_dereference().
> >> >>
> >> >
> >> > So what lock protects RCU writers after this patch?
> >>
> >> I explained it in comment for fl_head_dereference(), but should have
> >> copied this information to changelog as well:
> >> Flower classifier only changes root pointer during init and destroy.
> >> Cls API implements reference counting for tcf_proto, so there is no
> >> danger of concurrent access to tp when it is being destroyed, even
> >> without protection provided by rtnl lock.
> >
> > So you are saying an RCU pointer is okay to deference without
> > any lock eve without RCU read lock, right?
> >
> >
> >>
> >> In initial version of this change I used tp->lock to protect tp->root
> >> access and verified it with lockdep, but during internal review Jiri
> >> noted that this is not needed in current flower implementation.
> >
> > Let's see what you have on top of your own branch
> > unlocked_flower_cong_1:
> >
> > 1458 static int fl_change(struct net *net, struct sk_buff *in_skb,
> > 1459 struct tcf_proto *tp, unsigned long base,
> > 1460 u32 handle, struct nlattr **tca,
> > 1461 void **arg, bool ovr, bool rtnl_held,
> > 1462 struct netlink_ext_ack *extack)
> > 1463 {
> > 1464 struct cls_fl_head *head = fl_head_dereference(tp);
> >
> > At the point of line 1464, there is no lock taken, tp->lock is taken
> > after it, block->lock or chain lock is already unlocked before ->change().
> >
> > So, what protects this RCU structure? According to RCU, it must be
> > either RCU read lock or some writer lock. I see none here. :(
> >
> > What am I missing?
>
> Initially I had flower implementation that protected tp->root access
> with tp->lock, re-obtaining lock and head reference each time it was
> used, sometimes multiple times per function (head reference is usually
> obtained in beginning of every flower API function and used multiple
> times afterwards). This complicated the code and reduced parallelism.
> However, in case of flower classifier tp->root is never reassigned after
> init, so essentially there is no "CU" part in this "RCU" pointer. This
> realization allowed me to refactor unlocked flower code to look much
> nicer and perform better. Am I missing something in flower classifier
> implementation?
So if it is no longer RCU any more, why do you still use
rcu_dereference_protected()? That is, why not just deref it as a raw
pointer?
And, I don't think I can buy your argument here. The RCU infrastructure
should not be changed even after your patches, the fast path is still
protocted by RCU read lock, while the slow path now is protected by
some smaller-scope locks. What makes cls_flower so unique that
it doesn't even need RCU here? tp->root is not reassigned but it is still
freed via RCU infra, that is in fl_destroy_sleepable().
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Alexei Starovoitov @ 2019-02-22 19:34 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linus Torvalds, Masami Hiramatsu, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski, daniel,
netdev, bpf
In-Reply-To: <20190222143026.17d6f0f6@gandalf.local.home>
On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
> On Fri, 22 Feb 2019 11:27:05 -0800
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> > On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> > >
> > > Then we should still probably fix up "__probe_kernel_read()" to not
> > > allow user accesses. The easiest way to do that is actually likely to
> > > use the "unsafe_get_user()" functions *without* doing a
> > > uaccess_begin(), which will mean that modern CPU's will simply fault
> > > on a kernel access to user space.
> >
> > On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> > and users pass both user and kernel addresses into it and expect
> > that the helper will actually try to read from that address.
> >
> > If __probe_kernel_read will suddenly start failing on all user addresses
> > it will break the expectations.
> > How do we solve it in bpf_probe_read?
> > Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> > in the loop?
> > That's doable, but people already complain that bpf_probe_read() is slow
> > and shows up in their perf report.
>
> We're changing kprobes to add a specific flag to say that we want to
> differentiate between kernel or user reads. Can this be done with
> bpf_probe_read()? If it's showing up in perf report, I doubt a single
so you're saying you will break existing kprobe scripts?
I don't think it's a good idea.
It's not acceptable to break bpf_probe_read uapi.
^ permalink raw reply
* Re: [PATCH] r8152: Fix an error on RTL8153-BD MAC Address Passthrough support
From: David Miller @ 2019-02-22 19:37 UTC (permalink / raw)
To: david0813
Cc: linux-usb, netdev, linux-kernel, hayeswang, mario.limonciello,
bigeasy, edumazet, jslaby, f.fainelli, david.chen7, kai.heng.feng,
zhongjiang
In-Reply-To: <20190220054719.7280-1-david0813@gmail.com>
From: David Chen <david0813@gmail.com>
Date: Wed, 20 Feb 2019 13:47:19 +0800
> From: David Chen <david.chen7@dell.com>
>
> RTL8153-BD is used in Dell DA300 type-C dongle.
> Added RTL8153-BD support to activate MAC address pass through on DA300.
> Apply correction on previously submitted patch in net.git tree.
>
> Signed-off-by: David Chen <david.chen7@dell.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] tls: Return type of non-data records retrieved using MSG_PEEK in recvmsg
From: David Miller @ 2019-02-22 19:39 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
In-Reply-To: <20190220061314.28582-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Wed, 20 Feb 2019 06:15:42 +0000
> Fixes: 692d7b5d1f912 ("tls: Fix recvmsg() to be able to peek across...)
Please do not partially quote the commit header line string, always
provide the entire, full, string on a single line.
Thank you.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Steven Rostedt @ 2019-02-22 19:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linus Torvalds, Masami Hiramatsu, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski, daniel,
netdev, bpf
In-Reply-To: <20190222193456.5vqppubzrcx5wsul@ast-mbp.dhcp.thefacebook.com>
On Fri, 22 Feb 2019 11:34:58 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> so you're saying you will break existing kprobe scripts?
Yes we may.
> I don't think it's a good idea.
> It's not acceptable to break bpf_probe_read uapi.
Then you may need to add more code to determine if the address is user
space or not in the kernel, and then go the appropriate route, before
calling probe_kernel_read().
-- Steve
^ permalink raw reply
* Re: [PATCH net-next] net: phy: aquantia: Use get_features for the PHYs abilities
From: David Miller @ 2019-02-22 19:40 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <eb1c6f5e-2962-9151-db2d-fe9e301ecc0b@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 20 Feb 2019 07:46:22 +0100
> From: Andrew Lunn <andrew@lunn.ch>
> Use the new PHY driver call to get the PHYs supported features.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> [hkallweit1@gmail.com: removed new config_init callback from patch]
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net v2] team: use operstate consistently for linkup
From: David Miller @ 2019-02-22 19:41 UTC (permalink / raw)
To: gwilkie; +Cc: jiri, netdev
In-Reply-To: <20190220081911.803-1-gwilkie@vyatta.att-mail.com>
From: George Wilkie <gwilkie@vyatta.att-mail.com>
Date: Wed, 20 Feb 2019 08:19:11 +0000
> When a port is added to a team, its initial state is derived
> from netif_carrier_ok rather than netif_oper_up.
> If it is carrier up but operationally down at the time of being
> added, the port state.linkup will be set prematurely.
> port state.linkup should be set consistently using
> netif_oper_up rather than netif_carrier_ok.
>
> Fixes: f1d22a1e0595 ("team: account for oper state")
> Signed-off-by: George Wilkie <gwilkie@vyatta.att-mail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net: ip6_gre: fix possible NULL pointer dereference in ip6erspan_set_version
From: David Miller @ 2019-02-22 19:42 UTC (permalink / raw)
To: lorenzo.bianconi; +Cc: netdev, syzbot+30191cf1057abd3064af
In-Reply-To: <367cdc7c2c859abccfe67eee36cd97f9bf0b2544.1550620650.git.lorenzo.bianconi@redhat.com>
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Wed, 20 Feb 2019 09:23:03 +0100
> Fix a possible NULL pointer dereference in ip6erspan_set_version checking
> nlattr data pointer
...
> Fixes: 4974d5f678ab ("net: ip6_gre: initialize erspan_ver just for erspan tunnels")
> Reported-and-tested-by: syzbot+30191cf1057abd3064af@syzkaller.appspotmail.com
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
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