* Re: [PATCH v3] Add BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES bpf(2) command
From: Alexei Starovoitov @ 2018-08-10 22:29 UTC (permalink / raw)
To: Daniel Colascione
Cc: Joel Fernandes, linux-kernel, Tim Murray, netdev, Lorenzo Colitti,
Chenbo Feng, Mathieu Desnoyers, Alexei Starovoitov,
Daniel Borkmann
In-Reply-To: <CAKOZues6SE_c=ix7ap6QaJHqd1TmYpWWMJiu3=TtuqgKuqOUCA@mail.gmail.com>
On Thu, Aug 09, 2018 at 10:17:50AM -0700, Daniel Colascione wrote:
> Ping.
sorry for delay. have been off grid. let's continue in the other thread for full context
^ permalink raw reply
* pull request: bluetooth-next 2018-08-10
From: Johan Hedberg @ 2018-08-10 19:57 UTC (permalink / raw)
To: davem; +Cc: linux-bluetooth, netdev
[-- Attachment #1: Type: text/plain, Size: 2231 bytes --]
Hi Dave,
Here's one more (most likely last) bluetooth-next pull request for the
4.19 kernel.
- Added support for MediaTek serial Bluetooth devices
- Initial skeleton for controller-side address resolution support
- Fix BT_HCIUART_RTL related Kconfig dependencies
- A few other minor fixes/cleanups
Please let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 981467033a37d916649647fa3afe1fe99bba1817:
tc-testing: remove duplicate spaces in skbedit match patterns (2018-08-05 17:39:24 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream
for you to fetch changes up to aa12af77aae05008b3e637b85944dcd512f75eba:
Bluetooth: Add definitions for LE set address resolution (2018-08-10 16:57:57 +0200)
----------------------------------------------------------------
Ankit Navik (1):
Bluetooth: Add definitions for LE set address resolution
Marcel Holtmann (2):
Bluetooth: btqca: Introduce HCI_EV_VENDOR and use it
Bluetooth: Introduce BT_HCIUART_RTL configuration option
Sean Wang (3):
dt-bindings: net: bluetooth: Add mediatek-bluetooth
Bluetooth: mediatek: Add protocol support for MediaTek serial devices
MAINTAINERS: add an entry for MediaTek Bluetooth driver
YueHaibing (1):
Bluetooth: remove redundant variables 'adv_set' and 'cp'
.../devicetree/bindings/net/mediatek-bluetooth.txt | 35 ++
MAINTAINERS | 8 +
drivers/bluetooth/Kconfig | 24 +
drivers/bluetooth/Makefile | 1 +
drivers/bluetooth/btmtkuart.c | 629 +++++++++++++++++++++
drivers/bluetooth/btqca.c | 6 +-
drivers/bluetooth/hci_h5.c | 4 +
include/net/bluetooth/hci.h | 5 +
net/bluetooth/hci_event.c | 31 +-
net/bluetooth/mgmt.c | 3 -
10 files changed, 737 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
create mode 100644 drivers/bluetooth/btmtkuart.c
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] net: mvneta: fix mvneta_config_rss on armada 3700
From: Andrew Lunn @ 2018-08-10 22:23 UTC (permalink / raw)
To: David Miller
Cc: thomas.petazzoni, netdev, Jisheng.Zhang, linux-kernel,
linux-arm-kernel
In-Reply-To: <20180810.144031.861564620036768413.davem@davemloft.net>
On Fri, Aug 10, 2018 at 02:40:31PM -0700, David Miller wrote:
> From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> Date: Fri, 10 Aug 2018 11:36:27 +0800
>
> > The mvneta Ethernet driver is used on a few different Marvell SoCs.
> > Some SoCs have per cpu interrupts for Ethernet events, the driver uses
> > a per CPU napi structure for this case. Some SoCs such as armada 3700
> > have a single interrupt for Ethernet events, the driver uses a global
> > napi structure for this case.
> >
> > Current mvneta_config_rss() always operates the per cpu napi structure.
> > Fix it by operating a global napi for "single interrupt" case, and per
> > cpu napi structure for remaining cases.
> >
> > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > Fixes: 2636ac3cc2b4 ("net: mvneta: Add network support for Armada 3700 SoC")
>
> Applied, thanks.
Hi David
Please can you queue up:
Fixes: 7a86f05faf11 ("net: ethernet: mvneta: Fix napi structure mixup on armada 3700")
and this patch for stable.
Thanks
Andrew
^ permalink raw reply
* [PATCH net-next] r8169: remove version info
From: Heiner Kallweit @ 2018-08-10 19:27 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
The version number hasn't changed for ages and in general I doubt it
provides any benefit. The message in rtl_init_one() may even be
misleading because it's printed also if something fails in probe.
Therefore let's remove the version information.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 8ea1fa36..77802a9d 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -32,7 +32,6 @@
#include <asm/io.h>
#include <asm/irq.h>
-#define RTL8169_VERSION "2.3LK-NAPI"
#define MODULENAME "r8169"
#define FIRMWARE_8168D_1 "rtl_nic/rtl8168d-1.fw"
@@ -784,7 +783,6 @@ MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
module_param_named(debug, debug.msg_enable, int, 0);
MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)");
MODULE_LICENSE("GPL");
-MODULE_VERSION(RTL8169_VERSION);
MODULE_FIRMWARE(FIRMWARE_8168D_1);
MODULE_FIRMWARE(FIRMWARE_8168D_2);
MODULE_FIRMWARE(FIRMWARE_8168E_1);
@@ -1635,7 +1633,6 @@ static void rtl8169_get_drvinfo(struct net_device *dev,
struct rtl_fw *rtl_fw = tp->rtl_fw;
strlcpy(info->driver, MODULENAME, sizeof(info->driver));
- strlcpy(info->version, RTL8169_VERSION, sizeof(info->version));
strlcpy(info->bus_info, pci_name(tp->pci_dev), sizeof(info->bus_info));
BUILD_BUG_ON(sizeof(info->fw_version) < sizeof(rtl_fw->version));
if (!IS_ERR_OR_NULL(rtl_fw))
@@ -7292,11 +7289,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
int chipset, region, i;
int rc;
- if (netif_msg_drv(&debug)) {
- printk(KERN_INFO "%s Gigabit Ethernet driver %s loaded\n",
- MODULENAME, RTL8169_VERSION);
- }
-
dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
if (!dev)
return -ENOMEM;
--
2.18.0
^ permalink raw reply related
* [PATCH next-queue] ixgbe: don't clear ipsec sa counters on hw clearing
From: Shannon Nelson @ 2018-08-10 19:24 UTC (permalink / raw)
To: intel-wired-lan, jeffrey.t.kirsher; +Cc: steffen.klassert, netdev
The software SA record counters should not be cleared when clearing
the hardware tables. This causes the counters to be out of sync
after a driver reset.
Fixes: 63a67fe229ea ("ixgbe: add ipsec offload add and remove SA")
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index c116f45..df2f997 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -113,7 +113,6 @@ static void ixgbe_ipsec_set_rx_ip(struct ixgbe_hw *hw, u16 idx, __be32 addr[])
**/
static void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
{
- struct ixgbe_ipsec *ipsec = adapter->ipsec;
struct ixgbe_hw *hw = &adapter->hw;
u32 buf[4] = {0, 0, 0, 0};
u16 idx;
@@ -132,9 +131,6 @@ static void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
ixgbe_ipsec_set_tx_sa(hw, idx, buf, 0);
ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
}
-
- ipsec->num_rx_sa = 0;
- ipsec->num_tx_sa = 0;
}
/**
--
2.7.4
^ permalink raw reply related
* Re: [PATCH bpf-next 0/4] Convert filter.txt to RST
From: Tobin C. Harding @ 2018-08-10 21:54 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Daniel Borkmann, Alexei Starovoitov, Alexei Starovoitov,
David S. Miller, Kees Cook, Andy Lutomirski, Will Drewry,
linux-doc, netdev, linux-kernel
In-Reply-To: <20180810065752.2ab5473d@lwn.net>
On Fri, Aug 10, 2018 at 06:57:52AM -0600, Jonathan Corbet wrote:
> On Fri, 10 Aug 2018 11:46:36 +1000
> "Tobin C. Harding" <me@tobin.cc> wrote:
>
> > Thanks for clarifying. My understanding is now; this is a case where
> > checkpatch is too verbose and we do not actually need to add a specific
> > license identifier to the documentation files (new or otherwise). They
> > get an implicit GPLv2.
>
> The objective actually is to have SPDX tags in all files in the kernel.
> That includes documentation, even though people, as always, care less
> about the docs than they do the code.
>
> As I understood it, the complaint with the tags you put in wasn't their
> existence, it was your putting GPLv2+ rather than straight GPLv2. In the
> absence of information to the contrary, you really have to assume the
> latter, since that's the overall license for the kernel.
Righto, thanks Jon. GPLv0 tags going in for v3
Tobin
^ permalink raw reply
* Re: [RFC PATCH net-next] qed/qede: qede_setup_tc() can be static
From: David Miller @ 2018-08-10 21:42 UTC (permalink / raw)
To: fengguang.wu
Cc: manish.chopra, kbuild-all, netdev, ariel.elior, everest-linux-l2,
linux-kernel
In-Reply-To: <20180810012959.GA22458@athens>
From: kbuild test robot <fengguang.wu@intel.com>
Date: Fri, 10 Aug 2018 09:29:59 +0800
> Fixes: 5e7baf0fcb2a ("qed/qede: Multi CoS support.")
> Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: core: remove unnecessary function mlxsw_core_driver_put
From: David Miller @ 2018-08-10 21:42 UTC (permalink / raw)
To: yuehaibing; +Cc: jiri, idosch, linux-kernel, netdev
In-Reply-To: <20180810023730.13140-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 10 Aug 2018 10:37:30 +0800
> The function mlxsw_core_driver_put only traverse mlxsw_core_driver_list
> to find the matched mlxsw_driver,but never used it.
> So it can be removed safely.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: mvneta: fix mvneta_config_rss on armada 3700
From: David Miller @ 2018-08-10 21:40 UTC (permalink / raw)
To: Jisheng.Zhang
Cc: thomas.petazzoni, andrew, linux-arm-kernel, netdev, linux-kernel
In-Reply-To: <20180810113627.5dec5d40@xhacker.debian>
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Date: Fri, 10 Aug 2018 11:36:27 +0800
> The mvneta Ethernet driver is used on a few different Marvell SoCs.
> Some SoCs have per cpu interrupts for Ethernet events, the driver uses
> a per CPU napi structure for this case. Some SoCs such as armada 3700
> have a single interrupt for Ethernet events, the driver uses a global
> napi structure for this case.
>
> Current mvneta_config_rss() always operates the per cpu napi structure.
> Fix it by operating a global napi for "single interrupt" case, and per
> cpu napi structure for remaining cases.
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> Fixes: 2636ac3cc2b4 ("net: mvneta: Add network support for Armada 3700 SoC")
Applied, thanks.
^ permalink raw reply
* Re: [PATCH bpf-next] BPF: helpers: New helper to obtain namespace data from current task
From: Carlos Neira @ 2018-08-10 19:08 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Carlos Neira, netdev, ebiederm, quentin.monnet, ys114321,
Daniel Borkmann
In-Reply-To: <20180810124029.4d2c6c1f@redhat.com>
Jesper,
I'm sorry, thanks a lot for the pointers to the current process.
I just submitted v2 of for this patch. Thanks again for checking this out.
Bests
On Fri, Aug 10, 2018 at 12:40:29PM +0200, Jesper Dangaard Brouer wrote:
> On Thu, 9 Aug 2018 12:07:00 -0400
> Carlos Neira <cneirabustos@gmail.com> wrote:
>
> > Jesper,
> > Here is the updated patch.
> >
> > From 92633f6819423093932e8d04aa3dc99a5913f6fd Mon Sep 17 00:00:00 2001
> > From: Carlos Neira <cneirabustos@gmail.com>
> > Date: Thu, 9 Aug 2018 09:55:32 -0400
> > Subject: [PATCH bpf-next] BPF: helpers: New helper to obtain namespace
> > data from current task
> >
> [...]
>
> Hi Carlos,
>
> This is not how you resubmit a patch, it is both documented in [1] and
> [2], that: "In case the patch or patch series has to be reworked and
> sent out again in a second or later revision, it is also required to
> add a version number (v2, v3, ...) into the subject prefix"
>
> [1] https://github.com/torvalds/linux/blob/master/Documentation/bpf/bpf_devel_QA.rst#q-how-do-i-indicate-which-tree-bpf-vs-bpf-next-my-patch-should-be-applied-to
>
> [2] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
>
> Take a look at [1], which toplevel doc is about "HOWTO interact with
> BPF subsystem".
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next 1/1] net/smc: send response to test link signal
From: David Miller @ 2018-08-10 21:39 UTC (permalink / raw)
To: ubraun; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180810154511.71106-1-ubraun@linux.ibm.com>
From: Ursula Braun <ubraun@linux.ibm.com>
Date: Fri, 10 Aug 2018 17:45:11 +0200
> With SMC-D z/OS sends a test link signal every 10 seconds. Linux is
> supposed to answer, otherwise the SMC-D connection breaks.
>
> Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
Applied.
^ permalink raw reply
* Re: KMSAN: uninit-value in sit_tunnel_xmit
From: syzbot @ 2018-08-10 21:30 UTC (permalink / raw)
To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji
In-Reply-To: <00000000000082e3a90571fadb4c@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 0cc51dc9a291 kmsan: instrument get_user() and put_user() p..
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=14973622400000
kernel config: https://syzkaller.appspot.com/x/.config?x=3618361797c180a3
dashboard link: https://syzkaller.appspot.com/bug?extid=782ee96f9147673d8822
compiler: clang version 8.0.0 (trunk 339414)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=10fd6e30400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17a2aee2400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+782ee96f9147673d8822@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KMSAN: uninit-value in __ipv6_prefix_equal64_half
include/net/ipv6.h:515 [inline]
BUG: KMSAN: uninit-value in ipv6_prefix_equal include/net/ipv6.h:532
[inline]
BUG: KMSAN: uninit-value in check_6rd net/ipv6/sit.c:771 [inline]
BUG: KMSAN: uninit-value in try_6rd net/ipv6/sit.c:805 [inline]
BUG: KMSAN: uninit-value in ipip6_tunnel_xmit net/ipv6/sit.c:865 [inline]
BUG: KMSAN: uninit-value in sit_tunnel_xmit+0x18c0/0x3640
net/ipv6/sit.c:1029
CPU: 0 PID: 4467 Comm: syz-executor896 Not tainted 4.18.0-rc8+ #33
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x17c/0x1c0 lib/dump_stack.c:113
kmsan_report+0x188/0x2a0 mm/kmsan/kmsan.c:982
__msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:645
__ipv6_prefix_equal64_half include/net/ipv6.h:515 [inline]
ipv6_prefix_equal include/net/ipv6.h:532 [inline]
check_6rd net/ipv6/sit.c:771 [inline]
try_6rd net/ipv6/sit.c:805 [inline]
ipip6_tunnel_xmit net/ipv6/sit.c:865 [inline]
sit_tunnel_xmit+0x18c0/0x3640 net/ipv6/sit.c:1029
__netdev_start_xmit include/linux/netdevice.h:4148 [inline]
netdev_start_xmit include/linux/netdevice.h:4157 [inline]
xmit_one net/core/dev.c:3034 [inline]
dev_hard_start_xmit+0x5df/0xc20 net/core/dev.c:3050
__dev_queue_xmit+0x2eea/0x3a70 net/core/dev.c:3569
dev_queue_xmit+0x4b/0x60 net/core/dev.c:3602
packet_snd net/packet/af_packet.c:2921 [inline]
packet_sendmsg+0x7fb5/0x8ae0 net/packet/af_packet.c:2946
sock_sendmsg_nosec net/socket.c:642 [inline]
sock_sendmsg net/socket.c:652 [inline]
___sys_sendmsg+0xe32/0x1250 net/socket.c:2126
__sys_sendmsg net/socket.c:2164 [inline]
__do_sys_sendmsg net/socket.c:2173 [inline]
__se_sys_sendmsg net/socket.c:2171 [inline]
__x64_sys_sendmsg+0x32d/0x460 net/socket.c:2171
do_syscall_64+0x15b/0x220 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x441149
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 db 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffdb58f66a8 EFLAGS: 00000286 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000441149
RDX: 0000000000000000 RSI: 0000000020000000 RDI: 0000000000000003
RBP: 00000000006cc018 R08: 0000000000000100 R09: 0000000000000100
R10: 0000000000000100 R11: 0000000000000286 R12: 00000000004020b0
R13: 0000000000402140 R14: 0000000000000000 R15: 0000000000000000
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:181
kmsan_kmalloc+0x98/0x100 mm/kmsan/kmsan_hooks.c:91
kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan_hooks.c:100
slab_post_alloc_hook mm/slab.h:446 [inline]
slab_alloc_node mm/slub.c:2720 [inline]
__kmalloc_node_track_caller+0xb4c/0x11d0 mm/slub.c:4353
__kmalloc_reserve net/core/skbuff.c:138 [inline]
__alloc_skb+0x2ce/0x9b0 net/core/skbuff.c:206
alloc_skb include/linux/skbuff.h:988 [inline]
alloc_skb_with_frags+0x1d0/0xac0 net/core/skbuff.c:5272
sock_alloc_send_pskb+0xb47/0x1120 net/core/sock.c:2095
packet_alloc_skb net/packet/af_packet.c:2776 [inline]
packet_snd net/packet/af_packet.c:2867 [inline]
packet_sendmsg+0x6480/0x8ae0 net/packet/af_packet.c:2946
sock_sendmsg_nosec net/socket.c:642 [inline]
sock_sendmsg net/socket.c:652 [inline]
___sys_sendmsg+0xe32/0x1250 net/socket.c:2126
__sys_sendmsg net/socket.c:2164 [inline]
__do_sys_sendmsg net/socket.c:2173 [inline]
__se_sys_sendmsg net/socket.c:2171 [inline]
__x64_sys_sendmsg+0x32d/0x460 net/socket.c:2171
do_syscall_64+0x15b/0x220 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x63/0xe7
==================================================================
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Daniel Borkmann @ 2018-08-10 18:45 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: ast, netdev, yhs
In-Reply-To: <20180810164039.f4vtlmnu36wno7ph@ast-mbp.dhcp.thefacebook.com>
On 08/10/2018 06:40 PM, Alexei Starovoitov wrote:
> On Fri, Aug 10, 2018 at 09:55:35AM +0200, Daniel Borkmann wrote:
>> On 08/10/2018 04:13 AM, Alexei Starovoitov wrote:
>>> On Fri, Aug 10, 2018 at 12:43:20AM +0200, Daniel Borkmann wrote:
>>>> On 08/09/2018 11:44 PM, Alexei Starovoitov wrote:
>>>>> On Thu, Aug 09, 2018 at 11:30:52PM +0200, Daniel Borkmann wrote:
>>>>>> On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
>>>>>>> On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
>>>>>>>> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
>>>>>>>> the basic arraymap") enabled support for BTF and dumping via
>>>>>>>> BPF fs for arraymap. However, both can be decoupled from each
>>>>>>>> other such that all BPF maps can be supported for attaching
>>>>>>>> BTF key/value information, while not all maps necessarily
>>>>>>>> need to dump via map_seq_show_elem() callback.
>>>>>>>>
>>>>>>>> The check in array_map_check_btf() can be generalized as
>>>>>>>> ultimatively the key and value size is the only contraint
>>>>>>>> that needs to match for the map. The fact that the key needs
>>>>>>>> to be of type int is optional; it could be any data type as
>>>>>>>> long as it matches the 4 byte key size, just like hash table
>>>>>>>> key or others could be of any data type as well.
>>>>>>>>
>>>>>>>> Minimal example of a hash table dump which then works out
>>>>>>>> of the box for bpftool:
>>>>>>>>
>>>>>>>> # bpftool map dump id 19
>>>>>>>> [{
>>>>>>>> "key": {
>>>>>>>> "": {
>>>>>>>> "vip": 0,
>>>>>>>> "vipv6": []
>>>>>>>> },
>>>>>>>> "port": 0,
>>>>>>>> "family": 0,
>>>>>>>> "proto": 0
>>>>>>>> },
>>>>>>>> "value": {
>>>>>>>> "flags": 0,
>>>>>>>> "vip_num": 0
>>>>>>>> }
>>>>>>>> }
>>>>>>>> ]
>>>>>>>>
>>>>>>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>>>>>>>> Cc: Yonghong Song <yhs@fb.com>
>>>>>>>> ---
>>>>>>>> include/linux/bpf.h | 4 +---
>>>>>>>> kernel/bpf/arraymap.c | 27 ---------------------------
>>>>>>>> kernel/bpf/inode.c | 3 ++-
>>>>>>>> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
>>>>>>>> 4 files changed, 23 insertions(+), 35 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>>>>>>> index cd8790d..eb76e8e 100644
>>>>>>>> --- a/include/linux/bpf.h
>>>>>>>> +++ b/include/linux/bpf.h
>>>>>>>> @@ -48,8 +48,6 @@ struct bpf_map_ops {
>>>>>>>> u32 (*map_fd_sys_lookup_elem)(void *ptr);
>>>>>>>> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
>>>>>>>> struct seq_file *m);
>>>>>>>> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
>>>>>>>> - u32 key_type_id, u32 value_type_id);
>>>>>>>> };
>>>>>>>>
>>>>>>>> struct bpf_map {
>>>>>>>> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
>>>>>>>>
>>>>>>>> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
>>>>>>>> {
>>>>>>>> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
>>>>>>>> + return map->btf && map->ops->map_seq_show_elem;
>>>>>>>> }
>>>>>>>>
>>>>>>>> extern const struct bpf_map_ops bpf_map_offload_ops;
>>>>>>>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>>>>>>>> index 2aa55d030..67f0bdf 100644
>>>>>>>> --- a/kernel/bpf/arraymap.c
>>>>>>>> +++ b/kernel/bpf/arraymap.c
>>>>>>>> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
>>>>>>>> rcu_read_unlock();
>>>>>>>> }
>>>>>>>>
>>>>>>>> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
>>>>>>>> - u32 btf_key_id, u32 btf_value_id)
>>>>>>>> -{
>>>>>>>> - const struct btf_type *key_type, *value_type;
>>>>>>>> - u32 key_size, value_size;
>>>>>>>> - u32 int_data;
>>>>>>>> -
>>>>>>>> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
>>>>>>>> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
>>>>>>>> - return -EINVAL;
>>>>>>>> -
>>>>>>>> - int_data = *(u32 *)(key_type + 1);
>>>>>>>> - /* bpf array can only take a u32 key. This check makes
>>>>>>>> - * sure that the btf matches the attr used during map_create.
>>>>>>>> - */
>>>>>>>> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
>>>>>>>> - BTF_INT_OFFSET(int_data))
>>>>>>>> - return -EINVAL;
>>>>>>>
>>>>>>> I think most of these checks are still necessary for array type.
>>>>>>> Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
>>>>>>> is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
>>>>>>
>>>>>> Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
>>>>>> on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
>>>>>> could be array of u8 foo[4], for example, or u16 foo[2]. But how would
>>>>>> it ultimately be different from e.g. having 'struct a' versus 'struct b'
>>>>>> where both are of same size and while actual key has 'struct a', the one
>>>>>> who writes the prog resp. loads the BTF into the kernel would lie about
>>>>>> it stating it's of type 'struct b' instead? It's basically trusting the
>>>>>> app that it advertised sane key types which kernel is propagating back.
>>>>>
>>>>> for hash map - yes. the kernel cannot yet catch the lie that
>>>>> key == 'struct a' that user said in BTF is not what program used
>>>>> (which used 'struct b' of the same size).
>>>>> Eventually we will annotate all load/store in the program and will
>>>>> make sure that memory access match what BTF said.
>>>>
>>>> But in that case, would you reject the program? E.g. from prog point of
>>>> view, it's just a buffer of x bytes, so key could be casted to different
>>>> struct/types potentially and used for lookup; similar with value if you
>>>> would go the route to annotate all access into it. I don't think this
>>>> serves as a security feature (since you might as well just load the prog
>>>> without BTF just fine), but it can be used to help verifier to perform
>>>> rewrites like in tracing for implicit bpf_probe_read() based on member
>>>> access. But also in that case, if you might have e.g. stale or wrong BTF
>>>> data e.g. of the whole kernel or some application it would follow/walk
>>>> that one instead. But such user error would be "acceptable" since it serves
>>>> as a hint, roughly similar to (explicitly) walking the data structures
>>>> based on the headers today, you do have better control in terms of header
>>>> mismatches in that you can ship the BTF directly from the build, but there's
>>>> still no guarantee in that sense that you "verified" that these bytes
>>>> originally were mapped to struct foo somewhere in a C program.
>>>
>>> I wouldn't view such key checks as safety feature, but rather
>>> as honesty check. Meaning that user space shouldn't be able to cheat
>>> by passing completely bogus BTF into the kernel that only
>>> statisfies single size check.
>>
>> Ok, meaning, we agree that BTF is passed as a hint to the kernel. It is
>> a 'should not cheat' requirement but not 'cannot cheat', meaning if it
>> was the latter, BTF is core part of the verifier's safety analysis /
>> checks, but as it's optional on top of it (well, due to compatibility it
>> kind of needs to be anyway), it doesn't affect kernel's safety in
>> relation to what the program is doing internally.
>
> Eventually BTF will not be optional, since it's not another dwarf-like debug info.
> Today its type information serves the purpose of debug info,
> but it's more than that. It's a type description.
> Meaning that the kernel verifies it and the verifier will eventually use
> it to make safety decisions.
> Right now BTF is disjoined from the code, but later we'll tie
> them together and the verifer will start rejecting programs
> where memory accesses don't match the description.
> At that point if the code has annotated load/store they gotta be correct.
> We won't be able to do gradual tightening due to compatibility
> reasons, so when it happens it's gotta be fully strict from the start.
> So I really see it as "cannot cheat".
Okay.
>>> Say, BTF has a key that is a 4 byte struct { char a,b,c,d; };
>>> but program operates with it as u32. I very much would like
>>> the verifier to notice and reject such program, since if C code
>>> has struct { char a,b,c,d; }; the compiler won't generate u32 access
>>> to it unless C code type casts, but then llvm will warn. So for u32
>>> to legitimately appear in the generated code the struct should be:
>>> union {
>>> struct { char a,b,c,d;}
>>> u32 e;
>>> };
>>> Narrow access (like u8 load/store in the bpf prog form u32 BTF field)
>>> is ok, since that's normal compiler optimization, but any other
>>> field/size mismatch the verifier should reject to prevent cheating.
>>>
>>> In other words even proprietary bpf programs should not be able to cheat.
>>> If they provide BTF to the kernel, it should correspond exactly to the program.
>>> That's the key to _trusted_ introspection.
>>
>> But then wouldn't this artificially force users into programming /
>> thinking style that verifier dictates upon them similar as we have
>> today as opposed to further moving away from it to allow more C-style
>> programs to be accepted?
>
> I think it will be the opposite. Strict code<->BTF check will force
> users to use llvm with C source (or other language with proper
> compiler support). Hacking in asm will get harder for folks
> who want to take advantage of BTF.
>
>> But even if that is the goal, it is still used as a hint today, e.g.
>> even for an array I could tell the compiler that the key is '__u32'
>> for BTF sake while using the underlying key differently (e.g. as struct,
>> enum, etc) in order to pass the array_map_check_btf() check in the
>> kernel.
>
> correct that's possible today.
>
>> Those type of programs would still need to be accepted due
>> to compatibility reasons.
>
> I don't think there will be a compatibility issue.
> The programs need to be tied with BTF first. The verifier cannot
> just get smarter and understand the field accesses.
> For load/store we only check alignment and range.
> We don't really know the field within a struct. In very simple cases
> of fixed offset the verifier can figure that out, but it's too limited
> to start acting on it.
Right, agree, for verifier w/o additional help via annotations, it's
likely not possible.
> I think all load/stores have to be annotated with BTF.
> Including packet, stack, and even bpf_probe_read.
> Then the verifier will see which ethernet or IP header is being loaded
> and can act on it. The life of HW JITs will become easier too.
That makes more sense, so really _all_ load / stores would have to
be annotated in the program with BTF info and verified, and at that
point we could be able to start relying on it in future.
>> Same if we only test on size match in other
>> maps, it's nothing different. I don't see how verifier would start
>> enforcing programs to be rejected based on access patterns on the
>> types;
>
> right. In the current shape of progs and BTF I don't see how
> the verifier can do that.
>
>> while it might work for newly added program types that this
>> could be enforced, it cannot for all the many existing program types,
>
> even for existing program types it's ok, since recompiling the code
> with new annotations is an explicit act of the user.
> Future code annotations+BTF is not '-g' flag. It's not debug info.
> It's a new feature that is using ISA extensions.
> It's very similar to alu32 llvm flag.
> A bunch of existing programs will fail to load if they're simply
> recompiled with alu32.
To make them an ISA extension would be fine, and they could also
help with other features potentially (like loops). For annotations
and BTF to opt into, I think there would need to be additional
incentives from developer point of view, but they could certainly
be in the case of abstracting details away to make writing programs
more user friendly (as in mentioned implicit bpf_probe_read() case,
or perhaps to hide away pt_regs entirely).
>>> Admin that ssh-es into the box and operates with bpftool should be
>>> certain that the map introspection represent the real situation of
>>> the program. If proprietary prog is paranoid about layout of map
>>> it shouldn't be using BTF, but if it does, BTF should match.
>>> In the future I'd like to enfore availability of BTF for
>>> new program types.
>>
>> Sure, and in vast majority / normal cases it will represent the real
>> situation. Do you mistrust DWARF debugging data that gets shipped with
>> your distro, for example? pahole and other tools on top of it? It is
>
> dwarf is not used to make safety checks or optimizations.
> well, unless we consider eh_frame to be part of dwarf, then yes
> dwarf has to be correct otherwise programs will crash
> and user space has to trust it for safety.
>
>> pretty much the same situation. Given gdb example below, does gdb do
>> any verification of the binary in order to analyze load/store patterns
>> and whether it matches with what sits in DWARF as types?
>
> gdb doesn't do it, since it treats dwarf as debug info only.
>
>> as well map the types into memory and dump it as pretty print instead.
>> And that's okay, the one thing that needs to be verified for correctness
>> resp. validity is the debugging format itself so it can be used for
>> that.
>
> I don't think BTF belongs in kernel if it's used for pretty print only.
^ permalink raw reply
* [PATCH net v2] l2tp: use sk_dst_check() to avoid race on sk->sk_dst_cache
From: Wei Wang @ 2018-08-10 18:14 UTC (permalink / raw)
To: David Miller, netdev
Cc: Martin KaFai Lau, Wei Wang, Guillaume Nault, David Ahern,
Cong Wang
From: Wei Wang <weiwan@google.com>
In l2tp code, if it is a L2TP_UDP_ENCAP tunnel, tunnel->sk points to a
UDP socket. User could call sendmsg() on both this tunnel and the UDP
socket itself concurrently. As l2tp_xmit_skb() holds socket lock and call
__sk_dst_check() to refresh sk->sk_dst_cache, while udpv6_sendmsg() is
lockless and call sk_dst_check() to refresh sk->sk_dst_cache, there
could be a race and cause the dst cache to be freed multiple times.
So we fix l2tp side code to always call sk_dst_check() to garantee
xchg() is called when refreshing sk->sk_dst_cache to avoid race
conditions.
Syzkaller reported stack trace:
BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
BUG: KASAN: use-after-free in atomic_fetch_add_unless include/linux/atomic.h:575 [inline]
BUG: KASAN: use-after-free in atomic_add_unless include/linux/atomic.h:597 [inline]
BUG: KASAN: use-after-free in dst_hold_safe include/net/dst.h:308 [inline]
BUG: KASAN: use-after-free in ip6_hold_safe+0xe6/0x670 net/ipv6/route.c:1029
Read of size 4 at addr ffff8801aea9a880 by task syz-executor129/4829
CPU: 0 PID: 4829 Comm: syz-executor129 Not tainted 4.18.0-rc7-next-20180802+ #30
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
check_memory_region_inline mm/kasan/kasan.c:260 [inline]
check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272
atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
atomic_fetch_add_unless include/linux/atomic.h:575 [inline]
atomic_add_unless include/linux/atomic.h:597 [inline]
dst_hold_safe include/net/dst.h:308 [inline]
ip6_hold_safe+0xe6/0x670 net/ipv6/route.c:1029
rt6_get_pcpu_route net/ipv6/route.c:1249 [inline]
ip6_pol_route+0x354/0xd20 net/ipv6/route.c:1922
ip6_pol_route_output+0x54/0x70 net/ipv6/route.c:2098
fib6_rule_lookup+0x283/0x890 net/ipv6/fib6_rules.c:122
ip6_route_output_flags+0x2c5/0x350 net/ipv6/route.c:2126
ip6_dst_lookup_tail+0x1278/0x1da0 net/ipv6/ip6_output.c:978
ip6_dst_lookup_flow+0xc8/0x270 net/ipv6/ip6_output.c:1079
ip6_sk_dst_lookup_flow+0x5ed/0xc50 net/ipv6/ip6_output.c:1117
udpv6_sendmsg+0x2163/0x36b0 net/ipv6/udp.c:1354
inet_sendmsg+0x1a1/0x690 net/ipv4/af_inet.c:798
sock_sendmsg_nosec net/socket.c:622 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:632
___sys_sendmsg+0x51d/0x930 net/socket.c:2115
__sys_sendmmsg+0x240/0x6f0 net/socket.c:2210
__do_sys_sendmmsg net/socket.c:2239 [inline]
__se_sys_sendmmsg net/socket.c:2236 [inline]
__x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2236
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x446a29
Code: e8 ac b8 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f4de5532db8 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
RAX: ffffffffffffffda RBX: 00000000006dcc38 RCX: 0000000000446a29
RDX: 00000000000000b8 RSI: 0000000020001b00 RDI: 0000000000000003
RBP: 00000000006dcc30 R08: 00007f4de5533700 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dcc3c
R13: 00007ffe2b830fdf R14: 00007f4de55339c0 R15: 0000000000000001
Fixes: 71b1391a4128 ("l2tp: ensure sk->dst is still valid")
Reported-by: syzbot+05f840f3b04f211bad55@syzkaller.appspotmail.com
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Guillaume Nault <g.nault@alphalink.fr>
Cc: David Ahern <dsahern@gmail.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
---
v1->v2: Removed dst_clone() as Guillaume Nault suggested
net/l2tp/l2tp_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 40261cb68e83..8aaf8157da2b 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1110,7 +1110,7 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
/* Get routing info from the tunnel socket */
skb_dst_drop(skb);
- skb_dst_set(skb, dst_clone(__sk_dst_check(sk, 0)));
+ skb_dst_set(skb, sk_dst_check(sk, 0));
inet = inet_sk(sk);
fl = &inet->cork.fl;
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply related
* Re: [PATCH net] l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()
From: David Miller @ 2018-08-10 18:04 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
In-Reply-To: <20180810175837.GB1430@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Fri, 10 Aug 2018 19:58:38 +0200
> As far as I can see, f664e37dcc52 ("l2tp: fix missing refcount drop in
> pppol2tp_tunnel_ioctl()") is still in your -stable queue, but the two
> patches it depends on haven't made their way to -stable. I'd suggest to
> either drop this patch from your -stable queue, or to also queue up
> ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
> and
> f664e37dcc52 ("l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()").
Thanks Guillaume, I'll sort this out the next time I work on stable
submissions.
^ permalink raw reply
* Re: [PATCH net] l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()
From: Guillaume Nault @ 2018-08-10 17:58 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jchapman
In-Reply-To: <20180805112157.64rx4btyuwvxlzwb@kdev>
On Sun, Aug 05, 2018 at 01:24:13PM +0200, Guillaume Nault wrote:
> On Fri, Aug 03, 2018 at 12:42:22PM -0700, David Miller wrote:
> > From: Guillaume Nault <g.nault@alphalink.fr>
> > Date: Fri, 3 Aug 2018 17:00:11 +0200
> >
> > > If 'session' is not NULL and is not a PPP pseudo-wire, then we fail to
> > > drop the reference taken by l2tp_session_get().
> > >
> > > Fixes: ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
> > > Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> > > ---
> > > Sorry for the stupid mistake. I guess I got blinded by the apparent
> > > simplicity of the bug when I wrote the original patch.
> >
> > Applied, thanks.
> >
> > I'm pretty sure I backported the commit this fixes, so I'm queueing
> > this up for -stable as well.
> >
> Well, I think it wasn't. I didn't receive any notification from the
> stable team about it and I don't see it in Greg's stable queue nor
> in any -stable tree.
>
> Also, we'd have to queue 90904ff5f958 ("l2tp: fix pseudo-wire type for
> sessions created by pppol2tp_connect()") first, which is necessary for
> properly identifying PPP sessions.
>
> To recapitulate, three patches are needed to fix the original bug:
>
> * 90904ff5f958 ("l2tp: fix pseudo-wire type for sessions created by
> pppol2tp_connect()"): allows later patches to check if a session is
> PPP.
>
> * ecd012e45ab5 ("l2tp: filter out non-PPP sessions in
> pppol2tp_tunnel_ioctl()"): refuses calling pppol2tp_session_ioctl()
> on non-PPP sessions. This fixes an invalid pointer dereference when
> the session is Ethernet. Unfortunately it fails to drop the
> reference it takes on the session.
>
> * f664e37dcc52 ("l2tp: fix missing refcount drop in
> pppol2tp_tunnel_ioctl()"): fixes the memory leak introduced by the
> previous patch.
>
Hi Dave,
As far as I can see, f664e37dcc52 ("l2tp: fix missing refcount drop in
pppol2tp_tunnel_ioctl()") is still in your -stable queue, but the two
patches it depends on haven't made their way to -stable. I'd suggest to
either drop this patch from your -stable queue, or to also queue up
ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
and
f664e37dcc52 ("l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()").
^ permalink raw reply
* [PATCH net-next v2 15/15] net: sched: act_police: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect police action private data from concurrent
modification during dump. (init already uses tcf spinlock when changing
police action state)
Pass tcf spinlock as estimator lock argument to gen_replace_estimator()
during action init.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_police.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 1f3192ea8df7..88c16d80c1cf 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -274,14 +274,15 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_police *police = to_police(a);
struct tc_police opt = {
.index = police->tcf_index,
- .action = police->tcf_action,
- .mtu = police->tcfp_mtu,
- .burst = PSCHED_NS2TICKS(police->tcfp_burst),
.refcnt = refcount_read(&police->tcf_refcnt) - ref,
.bindcnt = atomic_read(&police->tcf_bindcnt) - bind,
};
struct tcf_t t;
+ spin_lock_bh(&police->tcf_lock);
+ opt.action = police->tcf_action;
+ opt.mtu = police->tcfp_mtu;
+ opt.burst = PSCHED_NS2TICKS(police->tcfp_burst);
if (police->rate_present)
psched_ratecfg_getrate(&opt.rate, &police->rate);
if (police->peak_present)
@@ -301,10 +302,12 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
t.expires = jiffies_to_clock_t(police->tcf_tm.expires);
if (nla_put_64bit(skb, TCA_POLICE_TM, sizeof(t), &t, TCA_POLICE_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&police->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&police->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 14/15] net: core: protect rate estimator statistics pointer with lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Extend gen_new_estimator() to also take stats_lock when re-assigning rate
estimator statistics pointer. (to be used by unlocked actions)
Rename 'stats_lock' to 'lock' and change argument description to explain
that it is now also used for control path.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/gen_stats.h | 4 ++--
net/core/gen_estimator.c | 21 +++++++++++++--------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index 0304ba2ae353..883bb9085f15 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -59,13 +59,13 @@ int gnet_stats_finish_copy(struct gnet_dump *d);
int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **rate_est,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running, struct nlattr *opt);
void gen_kill_estimator(struct net_rate_estimator __rcu **ptr);
int gen_replace_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **ptr,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running, struct nlattr *opt);
bool gen_estimator_active(struct net_rate_estimator __rcu **ptr);
bool gen_estimator_read(struct net_rate_estimator __rcu **ptr,
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 98fd12721221..e4e442d70c2d 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -112,7 +112,7 @@ static void est_timer(struct timer_list *t)
* @bstats: basic statistics
* @cpu_bstats: bstats per cpu
* @rate_est: rate estimator statistics
- * @stats_lock: statistics lock
+ * @lock: lock for statistics and control path
* @running: qdisc running seqcount
* @opt: rate estimator configuration TLV
*
@@ -128,7 +128,7 @@ static void est_timer(struct timer_list *t)
int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **rate_est,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running,
struct nlattr *opt)
{
@@ -154,19 +154,22 @@ int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
seqcount_init(&est->seq);
intvl_log = parm->interval + 2;
est->bstats = bstats;
- est->stats_lock = stats_lock;
+ est->stats_lock = lock;
est->running = running;
est->ewma_log = parm->ewma_log;
est->intvl_log = intvl_log;
est->cpu_bstats = cpu_bstats;
- if (stats_lock)
+ if (lock)
local_bh_disable();
est_fetch_counters(est, &b);
- if (stats_lock)
+ if (lock)
local_bh_enable();
est->last_bytes = b.bytes;
est->last_packets = b.packets;
+
+ if (lock)
+ spin_lock_bh(lock);
old = rcu_dereference_protected(*rate_est, 1);
if (old) {
del_timer_sync(&old->timer);
@@ -179,6 +182,8 @@ int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
mod_timer(&est->timer, est->next_jiffies);
rcu_assign_pointer(*rate_est, est);
+ if (lock)
+ spin_unlock_bh(lock);
if (old)
kfree_rcu(old, rcu);
return 0;
@@ -209,7 +214,7 @@ EXPORT_SYMBOL(gen_kill_estimator);
* @bstats: basic statistics
* @cpu_bstats: bstats per cpu
* @rate_est: rate estimator statistics
- * @stats_lock: statistics lock
+ * @lock: lock for statistics and control path
* @running: qdisc running seqcount (might be NULL)
* @opt: rate estimator configuration TLV
*
@@ -221,11 +226,11 @@ EXPORT_SYMBOL(gen_kill_estimator);
int gen_replace_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **rate_est,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running, struct nlattr *opt)
{
return gen_new_estimator(bstats, cpu_bstats, rate_est,
- stats_lock, running, opt);
+ lock, running, opt);
}
EXPORT_SYMBOL(gen_replace_estimator);
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 06/15] net: sched: act_pedit: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Rearrange pedit init code to only access pedit action data while holding
tcf spinlock. Change keys allocation type to atomic to allow it to execute
while holding tcf spinlock. Take tcf spinlock in dump function when
accessing pedit action data.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_pedit.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 43ba999b2d23..3f62da72ab6a 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -187,44 +187,38 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
tcf_idr_cleanup(tn, parm->index);
goto out_free;
}
- p = to_pedit(*a);
- keys = kmalloc(ksize, GFP_KERNEL);
- if (!keys) {
- tcf_idr_release(*a, bind);
- ret = -ENOMEM;
- goto out_free;
- }
ret = ACT_P_CREATED;
} else if (err > 0) {
if (bind)
goto out_free;
if (!ovr) {
- tcf_idr_release(*a, bind);
ret = -EEXIST;
- goto out_free;
- }
- p = to_pedit(*a);
- if (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys) {
- keys = kmalloc(ksize, GFP_KERNEL);
- if (!keys) {
- ret = -ENOMEM;
- goto out_free;
- }
+ goto out_release;
}
} else {
return err;
}
+ p = to_pedit(*a);
spin_lock_bh(&p->tcf_lock);
- p->tcfp_flags = parm->flags;
- p->tcf_action = parm->action;
- if (keys) {
+
+ if (ret == ACT_P_CREATED ||
+ (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys)) {
+ keys = kmalloc(ksize, GFP_ATOMIC);
+ if (!keys) {
+ spin_unlock_bh(&p->tcf_lock);
+ ret = -ENOMEM;
+ goto out_release;
+ }
kfree(p->tcfp_keys);
p->tcfp_keys = keys;
p->tcfp_nkeys = parm->nkeys;
}
memcpy(p->tcfp_keys, parm->keys, ksize);
+ p->tcfp_flags = parm->flags;
+ p->tcf_action = parm->action;
+
kfree(p->tcfp_keys_ex);
p->tcfp_keys_ex = keys_ex;
@@ -232,6 +226,9 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
+
+out_release:
+ tcf_idr_release(*a, bind);
out_free:
kfree(keys_ex);
return ret;
@@ -410,6 +407,7 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
if (unlikely(!opt))
return -ENOBUFS;
+ spin_lock_bh(&p->tcf_lock);
memcpy(opt->keys, p->tcfp_keys,
p->tcfp_nkeys * sizeof(struct tc_pedit_key));
opt->index = p->tcf_index;
@@ -432,11 +430,13 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &p->tcf_tm);
if (nla_put_64bit(skb, TCA_PEDIT_TM, sizeof(t), &t, TCA_PEDIT_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&p->tcf_lock);
kfree(opt);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&p->tcf_lock);
nlmsg_trim(skb, b);
kfree(opt);
return -1;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 13/15] net: sched: act_mirred: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Re-introduce mirred list spinlock, that was removed some time ago, in order
to protect it from concurrent modifications, instead of relying on rtnl
lock.
Use tcf spinlock to protect mirred action private data from concurrent
modification in init and dump. Rearrange access to mirred data in order to
be performed only while holding the lock.
Rearrange net dev access to always hold reference while working with it,
instead of relying on rntl lock.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_mirred.c | 78 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 51 insertions(+), 27 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 7a045cc7fe3b..327be257033d 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -30,6 +30,7 @@
#include <net/tc_act/tc_mirred.h>
static LIST_HEAD(mirred_list);
+static DEFINE_SPINLOCK(mirred_list_lock);
static bool tcf_mirred_is_act_redirect(int action)
{
@@ -62,13 +63,23 @@ static bool tcf_mirred_can_reinsert(int action)
return false;
}
+static struct net_device *tcf_mirred_dev_dereference(struct tcf_mirred *m)
+{
+ return rcu_dereference_protected(m->tcfm_dev,
+ lockdep_is_held(&m->tcf_lock));
+}
+
static void tcf_mirred_release(struct tc_action *a)
{
struct tcf_mirred *m = to_mirred(a);
struct net_device *dev;
+ spin_lock(&mirred_list_lock);
list_del(&m->tcfm_list);
- dev = rtnl_dereference(m->tcfm_dev);
+ spin_unlock(&mirred_list_lock);
+
+ /* last reference to action, no need to lock */
+ dev = rcu_dereference_protected(m->tcfm_dev, 1);
if (dev)
dev_put(dev);
}
@@ -128,22 +139,9 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
return -EINVAL;
}
- if (parm->ifindex) {
- dev = __dev_get_by_index(net, parm->ifindex);
- if (dev == NULL) {
- if (exists)
- tcf_idr_release(*a, bind);
- else
- tcf_idr_cleanup(tn, parm->index);
- return -ENODEV;
- }
- mac_header_xmit = dev_is_mac_header_xmit(dev);
- } else {
- dev = NULL;
- }
if (!exists) {
- if (!dev) {
+ if (!parm->ifindex) {
tcf_idr_cleanup(tn, parm->index);
NL_SET_ERR_MSG_MOD(extack, "Specified device does not exist");
return -EINVAL;
@@ -161,19 +159,31 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
m = to_mirred(*a);
- ASSERT_RTNL();
+ spin_lock(&m->tcf_lock);
m->tcf_action = parm->action;
m->tcfm_eaction = parm->eaction;
- if (dev != NULL) {
- if (ret != ACT_P_CREATED)
- dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
- dev_hold(dev);
- rcu_assign_pointer(m->tcfm_dev, dev);
+
+ if (parm->ifindex) {
+ dev = dev_get_by_index(net, parm->ifindex);
+ if (!dev) {
+ spin_unlock(&m->tcf_lock);
+ tcf_idr_release(*a, bind);
+ return -ENODEV;
+ }
+ mac_header_xmit = dev_is_mac_header_xmit(dev);
+ rcu_swap_protected(m->tcfm_dev, dev,
+ lockdep_is_held(&m->tcf_lock));
+ if (dev)
+ dev_put(dev);
m->tcfm_mac_header_xmit = mac_header_xmit;
}
+ spin_unlock(&m->tcf_lock);
if (ret == ACT_P_CREATED) {
+ spin_lock(&mirred_list_lock);
list_add(&m->tcfm_list, &mirred_list);
+ spin_unlock(&mirred_list_lock);
+
tcf_idr_insert(tn, *a);
}
@@ -287,26 +297,33 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
{
unsigned char *b = skb_tail_pointer(skb);
struct tcf_mirred *m = to_mirred(a);
- struct net_device *dev = rtnl_dereference(m->tcfm_dev);
struct tc_mirred opt = {
.index = m->tcf_index,
- .action = m->tcf_action,
.refcnt = refcount_read(&m->tcf_refcnt) - ref,
.bindcnt = atomic_read(&m->tcf_bindcnt) - bind,
- .eaction = m->tcfm_eaction,
- .ifindex = dev ? dev->ifindex : 0,
};
+ struct net_device *dev;
struct tcf_t t;
+ spin_lock(&m->tcf_lock);
+ opt.action = m->tcf_action;
+ opt.eaction = m->tcfm_eaction;
+ dev = tcf_mirred_dev_dereference(m);
+ if (dev)
+ opt.ifindex = dev->ifindex;
+
if (nla_put(skb, TCA_MIRRED_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
tcf_tm_dump(&t, &m->tcf_tm);
if (nla_put_64bit(skb, TCA_MIRRED_TM, sizeof(t), &t, TCA_MIRRED_PAD))
goto nla_put_failure;
+ spin_unlock(&m->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock(&m->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
@@ -337,15 +354,19 @@ static int mirred_device_event(struct notifier_block *unused,
ASSERT_RTNL();
if (event == NETDEV_UNREGISTER) {
+ spin_lock(&mirred_list_lock);
list_for_each_entry(m, &mirred_list, tcfm_list) {
- if (rcu_access_pointer(m->tcfm_dev) == dev) {
+ spin_lock(&m->tcf_lock);
+ if (tcf_mirred_dev_dereference(m) == dev) {
dev_put(dev);
/* Note : no rcu grace period necessary, as
* net_device are already rcu protected.
*/
RCU_INIT_POINTER(m->tcfm_dev, NULL);
}
+ spin_unlock(&m->tcf_lock);
}
+ spin_unlock(&mirred_list_lock);
}
return NOTIFY_DONE;
@@ -358,10 +379,13 @@ static struct notifier_block mirred_device_notifier = {
static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
{
struct tcf_mirred *m = to_mirred(a);
- struct net_device *dev = rtnl_dereference(m->tcfm_dev);
+ struct net_device *dev;
+ rcu_read_lock();
+ dev = rcu_dereference(m->tcfm_dev);
if (dev)
dev_hold(dev);
+ rcu_read_unlock();
return dev;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 12/15] net: sched: extend action ops with put_dev callback
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
As a preparation for removing dependency on rtnl lock from rules update
path, all users of shared objects must take reference while working with
them.
Extend action ops with put_dev() API to be used on net device returned by
get_dev().
Modify mirred action (only action that implements get_dev callback):
- Take reference to net device in get_dev.
- Implement put_dev API that releases reference to net device.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/act_api.h | 1 +
net/sched/act_mirred.c | 12 +++++++++++-
net/sched/cls_api.c | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 8c9bc02d05e1..1ad5b19e83a9 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -101,6 +101,7 @@ struct tc_action_ops {
void (*stats_update)(struct tc_action *, u64, u32, u64);
size_t (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a);
+ void (*put_dev)(struct net_device *dev);
int (*delete)(struct net *net, u32 index);
};
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index b26d060da08e..7a045cc7fe3b 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -358,8 +358,17 @@ static struct notifier_block mirred_device_notifier = {
static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
{
struct tcf_mirred *m = to_mirred(a);
+ struct net_device *dev = rtnl_dereference(m->tcfm_dev);
+
+ if (dev)
+ dev_hold(dev);
- return rtnl_dereference(m->tcfm_dev);
+ return dev;
+}
+
+static void tcf_mirred_put_dev(struct net_device *dev)
+{
+ dev_put(dev);
}
static int tcf_mirred_delete(struct net *net, u32 index)
@@ -382,6 +391,7 @@ static struct tc_action_ops act_mirred_ops = {
.lookup = tcf_mirred_search,
.size = sizeof(struct tcf_mirred),
.get_dev = tcf_mirred_get_dev,
+ .put_dev = tcf_mirred_put_dev,
.delete = tcf_mirred_delete,
};
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e8b0bbd0883f..bdabf8144e21 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2167,6 +2167,7 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
if (!dev)
continue;
ret = tc_setup_cb_egdev_call(dev, type, type_data, err_stop);
+ a->ops->put_dev(dev);
if (ret < 0)
return ret;
ok_count += ret;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 05/15] net: sched: act_ipt: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect ipt action private data from concurrent
modification during dump. Ipt init already takes tcf spinlock when
modifying ipt state.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_ipt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 0dc787a57798..e149f0e66cb6 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -288,6 +288,7 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
* for foolproof you need to not assume this
*/
+ spin_lock_bh(&ipt->tcf_lock);
t = kmemdup(ipt->tcfi_t, ipt->tcfi_t->u.user.target_size, GFP_ATOMIC);
if (unlikely(!t))
goto nla_put_failure;
@@ -307,10 +308,12 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
if (nla_put_64bit(skb, TCA_IPT_TM, sizeof(tm), &tm, TCA_IPT_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&ipt->tcf_lock);
kfree(t);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&ipt->tcf_lock);
nlmsg_trim(skb, b);
kfree(t);
return -1;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 11/15] net: sched: act_vlan: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect vlan action private data from concurrent
modification during dump and init. Use rcu swap operation to reassign
params pointer under protection of tcf lock. (old params value is not used
by init, so there is no need of standalone rcu dereference step)
Remove rtnl assertion that is no longer necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_vlan.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 15a0ee214c9c..5bde17fe3608 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -109,7 +109,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
struct nlattr *tb[TCA_VLAN_MAX + 1];
- struct tcf_vlan_params *p, *p_old;
+ struct tcf_vlan_params *p;
struct tc_vlan *parm;
struct tcf_vlan *v;
int action;
@@ -202,26 +202,24 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
v = to_vlan(*a);
- ASSERT_RTNL();
p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p) {
tcf_idr_release(*a, bind);
return -ENOMEM;
}
- v->tcf_action = parm->action;
-
- p_old = rtnl_dereference(v->vlan_p);
-
p->tcfv_action = action;
p->tcfv_push_vid = push_vid;
p->tcfv_push_prio = push_prio;
p->tcfv_push_proto = push_proto;
- rcu_assign_pointer(v->vlan_p, p);
+ spin_lock(&v->tcf_lock);
+ v->tcf_action = parm->action;
+ rcu_swap_protected(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
+ spin_unlock(&v->tcf_lock);
- if (p_old)
- kfree_rcu(p_old, rcu);
+ if (p)
+ kfree_rcu(p, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -243,16 +241,18 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
{
unsigned char *b = skb_tail_pointer(skb);
struct tcf_vlan *v = to_vlan(a);
- struct tcf_vlan_params *p = rtnl_dereference(v->vlan_p);
+ struct tcf_vlan_params *p;
struct tc_vlan opt = {
.index = v->tcf_index,
.refcnt = refcount_read(&v->tcf_refcnt) - ref,
.bindcnt = atomic_read(&v->tcf_bindcnt) - bind,
- .action = v->tcf_action,
- .v_action = p->tcfv_action,
};
struct tcf_t t;
+ spin_lock(&v->tcf_lock);
+ opt.action = v->tcf_action;
+ p = rcu_dereference_protected(v->vlan_p, lockdep_is_held(&v->tcf_lock));
+ opt.v_action = p->tcfv_action;
if (nla_put(skb, TCA_VLAN_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -268,9 +268,12 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &v->tcf_tm);
if (nla_put_64bit(skb, TCA_VLAN_TM, sizeof(t), &t, TCA_VLAN_PAD))
goto nla_put_failure;
+ spin_unlock(&v->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock(&v->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 09/15] net: sched: act_skbmod: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Move read of skbmod_p rcu pointer to be protected by tcf spinlock. Use tcf
spinlock to protect private skbmod data from concurrent modification during
dump.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_skbmod.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index c437c6d51a71..e9c86ade3b40 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -156,7 +156,6 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
d = to_skbmod(*a);
- ASSERT_RTNL();
p = kzalloc(sizeof(struct tcf_skbmod_params), GFP_KERNEL);
if (unlikely(!p)) {
tcf_idr_release(*a, bind);
@@ -166,10 +165,10 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
p->flags = lflags;
d->tcf_action = parm->action;
- p_old = rtnl_dereference(d->skbmod_p);
-
if (ovr)
spin_lock_bh(&d->tcf_lock);
+ /* Protected by tcf_lock if overwriting existing action. */
+ p_old = rcu_dereference_protected(d->skbmod_p, 1);
if (lflags & SKBMOD_F_DMAC)
ether_addr_copy(p->eth_dst, daddr);
@@ -205,15 +204,18 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
{
struct tcf_skbmod *d = to_skbmod(a);
unsigned char *b = skb_tail_pointer(skb);
- struct tcf_skbmod_params *p = rtnl_dereference(d->skbmod_p);
+ struct tcf_skbmod_params *p;
struct tc_skbmod opt = {
.index = d->tcf_index,
.refcnt = refcount_read(&d->tcf_refcnt) - ref,
.bindcnt = atomic_read(&d->tcf_bindcnt) - bind,
- .action = d->tcf_action,
};
struct tcf_t t;
+ spin_lock_bh(&d->tcf_lock);
+ opt.action = d->tcf_action;
+ p = rcu_dereference_protected(d->skbmod_p,
+ lockdep_is_held(&d->tcf_lock));
opt.flags = p->flags;
if (nla_put(skb, TCA_SKBMOD_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -231,8 +233,10 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_64bit(skb, TCA_SKBMOD_TM, sizeof(t), &t, TCA_SKBMOD_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&d->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&d->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 10/15] net: sched: act_tunnel_key: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf lock to protect tunnel key action struct private data from
concurrent modification in init and dump. Use rcu swap operation to
reassign params pointer under protection of tcf lock. (old params value is
not used by init, so there is no need of standalone rcu dereference step)
Remove rtnl lock assertion that is no longer required.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_tunnel_key.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index d42d9e112789..ba2ae9f75ef5 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -204,7 +204,6 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
struct nlattr *tb[TCA_TUNNEL_KEY_MAX + 1];
- struct tcf_tunnel_key_params *params_old;
struct tcf_tunnel_key_params *params_new;
struct metadata_dst *metadata = NULL;
struct tc_tunnel_key *parm;
@@ -346,24 +345,22 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
t = to_tunnel_key(*a);
- ASSERT_RTNL();
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
tcf_idr_release(*a, bind);
NL_SET_ERR_MSG(extack, "Cannot allocate tunnel key parameters");
return -ENOMEM;
}
-
- params_old = rtnl_dereference(t->params);
-
- t->tcf_action = parm->action;
params_new->tcft_action = parm->t_action;
params_new->tcft_enc_metadata = metadata;
- rcu_assign_pointer(t->params, params_new);
-
- if (params_old)
- kfree_rcu(params_old, rcu);
+ spin_lock(&t->tcf_lock);
+ t->tcf_action = parm->action;
+ rcu_swap_protected(t->params, params_new,
+ lockdep_is_held(&t->tcf_lock));
+ spin_unlock(&t->tcf_lock);
+ if (params_new)
+ kfree_rcu(params_new, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -485,12 +482,13 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
.index = t->tcf_index,
.refcnt = refcount_read(&t->tcf_refcnt) - ref,
.bindcnt = atomic_read(&t->tcf_bindcnt) - bind,
- .action = t->tcf_action,
};
struct tcf_t tm;
- params = rtnl_dereference(t->params);
-
+ spin_lock(&t->tcf_lock);
+ params = rcu_dereference_protected(t->params,
+ lockdep_is_held(&t->tcf_lock));
+ opt.action = t->tcf_action;
opt.t_action = params->tcft_action;
if (nla_put(skb, TCA_TUNNEL_KEY_PARMS, sizeof(opt), &opt))
@@ -522,10 +520,12 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_64bit(skb, TCA_TUNNEL_KEY_TM, sizeof(tm),
&tm, TCA_TUNNEL_KEY_PAD))
goto nla_put_failure;
+ spin_unlock(&t->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock(&t->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
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