* Re: [PATCH 1/3] ath10k: snoc: skip regulator operations
From: Kalle Valo @ 2019-09-17 14:04 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Govind Singh, David S. Miller, ath10k, linux-wireless, netdev,
linux-kernel, linux-arm-msm
In-Reply-To: <20190725174755.23432-2-bjorn.andersson@linaro.org>
Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> The regulator operations is trying to set a voltage to a fixed value, by
> giving some wiggle room. But some board designs specifies regulator
> voltages outside this limited range. One such example is the Lenovo Yoga
> C630, with vdd-3.3-ch0 in particular specified at 3.1V.
>
> But consumers with fixed voltage requirements should just rely on the
> board configuration to provide the power at the required level, so this
> code should be removed.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
3 patches applied to ath-next branch of ath.git, thanks.
b003e7f1974e ath10k: snoc: skip regulator operations
c56c7f24d7f8 ath10k: Use standard regulator bulk API in snoc
f93bcf0ce6a1 ath10k: Use standard bulk clock API in snoc
--
https://patchwork.kernel.org/patch/11059507/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: divide error in cdc_ncm_update_rxtx_max
From: syzbot @ 2019-09-17 14:09 UTC (permalink / raw)
To: andreyknvl, bjorn, davem, linux-kernel, linux-usb, netdev, oliver,
syzkaller-bugs
In-Reply-To: <87h85bm3nh.fsf@miraculix.mork.no>
Hello,
syzbot has tested the proposed patch but the reproducer still triggered
crash:
divide error in usbnet_update_max_qlen
cdc_ncm 5-1:1.0: setting tx_max = 16384
divide error: 0000 [#1] SMP KASAN
CPU: 1 PID: 1737 Comm: kworker/1:2 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
RIP: 0010:usbnet_update_max_qlen drivers/net/usb/usbnet.c:344 [inline]
RIP: 0010:usbnet_update_max_qlen+0x231/0x370 drivers/net/usb/usbnet.c:338
Code: 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 46 01
00 00 48 8d bb f4 00 00 00 31 d2 b8 c8 63 01 00 48 89 f9 <48> f7 b3 a8 01
00 00 48 ba 00 00 00 00 00 fc ff df 48 c1 e9 03 0f
RSP: 0018:ffff8881c05c7010 EFLAGS: 00010246
RAX: 00000000000163c8 RBX: ffff8881c7334ec0 RCX: ffff8881c7334fb4
RDX: 0000000000000000 RSI: ffffffff8344ecde RDI: ffff8881c7334fb4
RBP: 0000000000000003 R08: ffff8881d4a9b000 R09: ffffed1038e6688a
R10: ffffed1038e66889 R11: ffff8881c733444f R12: 0000000000004000
R13: ffff8881c7334ec0 R14: 0000000000000000 R15: ffff8881d62e77ac
FS: 0000000000000000(0000) GS:ffff8881db300000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f5a513e8000 CR3: 00000001d6436000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
cdc_ncm_update_rxtx_max+0xa61/0xf30 drivers/net/usb/cdc_ncm.c:437
cdc_ncm_setup drivers/net/usb/cdc_ncm.c:664 [inline]
cdc_ncm_bind_common+0x1005/0x2570 drivers/net/usb/cdc_ncm.c:921
cdc_ncm_bind+0x7c/0x1c0 drivers/net/usb/cdc_ncm.c:1035
usbnet_probe+0xb43/0x23cf drivers/net/usb/usbnet.c:1722
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace 472dbe98145a6d5e ]---
RIP: 0010:usbnet_update_max_qlen drivers/net/usb/usbnet.c:344 [inline]
RIP: 0010:usbnet_update_max_qlen+0x231/0x370 drivers/net/usb/usbnet.c:338
Code: 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 46 01
00 00 48 8d bb f4 00 00 00 31 d2 b8 c8 63 01 00 48 89 f9 <48> f7 b3 a8 01
00 00 48 ba 00 00 00 00 00 fc ff df 48 c1 e9 03 0f
RSP: 0018:ffff8881c05c7010 EFLAGS: 00010246
RAX: 00000000000163c8 RBX: ffff8881c7334ec0 RCX: ffff8881c7334fb4
RDX: 0000000000000000 RSI: ffffffff8344ecde RDI: ffff8881c7334fb4
RBP: 0000000000000003 R08: ffff8881d4a9b000 R09: ffffed1038e6688a
R10: ffffed1038e66889 R11: ffff8881c733444f R12: 0000000000004000
R13: ffff8881c7334ec0 R14: 0000000000000000 R15: ffff8881d62e77ac
FS: 0000000000000000(0000) GS:ffff8881db300000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f5a513e8000 CR3: 00000001d6436000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Tested on:
commit: f0df5c1b usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=173ea979600000
kernel config: https://syzkaller.appspot.com/x/.config?x=5c6633fa4ed00be5
dashboard link: https://syzkaller.appspot.com/bug?extid=ce366e2b8296e25d84f5
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1538611d600000
^ permalink raw reply
* Re: [PATCH] ath10k: Use ARRAY_SIZE
From: Kalle Valo @ 2019-09-17 14:09 UTC (permalink / raw)
To: Vasyl Gomonovych
Cc: davem, ath10k, linux-wireless, Vasyl Gomonovych, netdev,
linux-kernel
In-Reply-To: <20190718203032.15528-1-gomonovych@gmail.com>
Vasyl Gomonovych <gomonovych@gmail.com> wrote:
> fix coccinelle warning, use ARRAY_SIZE
>
> Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
7921ae091907 ath10k: Use ARRAY_SIZE
--
https://patchwork.kernel.org/patch/11049553/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] selftests/net: replace AF_MAX with INT_MAX in socket.c
From: shuah @ 2019-09-17 14:09 UTC (permalink / raw)
To: Marcelo Henrique Cerri
Cc: David S. Miller, netdev, linux-kselftest, linux-kernel, shuah
In-Reply-To: <20190917071222.6nfzmcxt4kxzgpki@gallifrey>
On 9/17/19 1:12 AM, Marcelo Henrique Cerri wrote:
> So the problem arises because the headers we have in userspace might
> be older and not match what we have in the kernel. In that case, the
> actual value of AF_MAX in the userspace headers might be a valid
> protocol family in the new kernel.
>
> That happens relatively often for us because we support different
> kernel versions at the same time in a given Ubuntu series.
>
Right. This is an evolving use-case for kselftest to make it easier to
run on distribution kernels.
> An alternative is to use the headers we have in the kernel tree, but I
> believe that might cause other issues.
>
Kselftest is tied to the kernel in such as way that you do need to use
the kernel headers to compile.
Do you run newer tests on older kernels? Where do you build them? What
I would like to see is fixing the test to run on older kernels and not
changing the tests to suit older kernel needs.
This definitely isn't a change that is good to make. We have to come
with a better way to solve this. Could you please send me the errors
you are seeing so I can help you find a better solution.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH] ath10k: add cleanup in ath10k_sta_state()
From: Kalle Valo @ 2019-09-17 14:11 UTC (permalink / raw)
To: Wenwen Wang
Cc: Wenwen Wang, David S. Miller,
open list:QUALCOMM ATHEROS ATH10K WIRELESS DRIVER,
open list:NETWORKING DRIVERS (WIRELESS),
open list:NETWORKING DRIVERS, open list
In-Reply-To: <1565903072-3948-1-git-send-email-wenwen@cs.uga.edu>
Wenwen Wang <wenwen@cs.uga.edu> wrote:
> If 'sta->tdls' is false, no cleanup is executed, leading to memory/resource
> leaks, e.g., 'arsta->tx_stats'. To fix this issue, perform cleanup before
> go to the 'exit' label.
>
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
334f5b61a6f2 ath10k: add cleanup in ath10k_sta_state()
--
https://patchwork.kernel.org/patch/11096481/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath10k: fix spelling mistake "eanble" -> "enable"
From: Kalle Valo @ 2019-09-17 14:13 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, ath10k, linux-wireless, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20190913074339.27280-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> There is a spelling mistake in a ath10k_warn warning message. Fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
09764659003d ath10k: fix spelling mistake "eanble" -> "enable"
--
https://patchwork.kernel.org/patch/11144035/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH bpf-next 1/5] perf/core: Add PERF_FORMAT_LOST read_format
From: kbuild test robot @ 2019-09-17 14:32 UTC (permalink / raw)
To: Daniel Xu
Cc: kbuild-all, bpf, songliubraving, yhs, andriin, peterz, mingo,
acme, Daniel Xu, ast, alexander.shishkin, jolsa, namhyung,
linux-kernel, netdev, kernel-team
In-Reply-To: <20190917133056.5545-2-dxu@dxuuu.xyz>
[-- Attachment #1: Type: text/plain, Size: 1672 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Daniel-Xu/perf-core-Add-PERF_FORMAT_LOST-read_format/20190917-213515
base: https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/events/core.c: In function 'perf_event_lost':
>> kernel/events/core.c:4753:11: error: implicit declaration of function 'perf_kprobe_missed'; did you mean 'perf_release'? [-Werror=implicit-function-declaration]
lost += perf_kprobe_missed(event);
^~~~~~~~~~~~~~~~~~
perf_release
cc1: some warnings being treated as errors
vim +4753 kernel/events/core.c
4739
4740 static struct pmu perf_kprobe;
4741 static u64 perf_event_lost(struct perf_event *event)
4742 {
4743 struct ring_buffer *rb;
4744 u64 lost = 0;
4745
4746 rcu_read_lock();
4747 rb = rcu_dereference(event->rb);
4748 if (likely(!!rb))
4749 lost += local_read(&rb->lost);
4750 rcu_read_unlock();
4751
4752 if (event->attr.type == perf_kprobe.type)
> 4753 lost += perf_kprobe_missed(event);
4754
4755 return lost;
4756 }
4757
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7185 bytes --]
^ permalink raw reply
* [RESENT PATCH v2] ixgbe: Use memzero_explicit directly in crypto cases
From: zhong jiang @ 2019-09-17 14:44 UTC (permalink / raw)
To: jakub.kicinski, davem
Cc: anna.schumaker, trond.myklebust, netdev, linux-kernel, zhongjiang
It's better to use memzero_explicit() to replace memset() in crypto cases.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 31629fc..7e4f32f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -960,10 +960,10 @@ int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
return 0;
err_aead:
- memset(xs->aead, 0, sizeof(*xs->aead));
+ memzero_explicit(xs->aead, sizeof(*xs->aead));
kfree(xs->aead);
err_xs:
- memset(xs, 0, sizeof(*xs));
+ memzero_explicit(xs, sizeof(*xs));
kfree(xs);
err_out:
msgbuf[1] = err;
@@ -1049,7 +1049,7 @@ int ixgbe_ipsec_vf_del_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
ixgbe_ipsec_del_sa(xs);
/* remove the xs that was made-up in the add request */
- memset(xs, 0, sizeof(*xs));
+ memzero_explicit(xs, sizeof(*xs));
kfree(xs);
return 0;
--
1.7.12.4
^ permalink raw reply related
* Re: [PATCH] netfilter: bridge: drop a broken include
From: Jeremy Sowden @ 2019-09-17 14:59 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Adam Borowski, Jozsef Kadlecsik, Florian Westphal, Roopa Prabhu,
Nikolay Aleksandrov, netfilter-devel, coreteam, netdev
In-Reply-To: <20190917050946.kmzajvqh3kjr4ch5@salvia>
[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]
On 2019-09-17, at 07:09:46 +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 16, 2019 at 02:08:12PM +0100, Jeremy Sowden wrote:
> > On 2019-09-16, at 02:05:16 +0200, Adam Borowski wrote:
> > > This caused a build failure if CONFIG_NF_CONNTRACK_BRIDGE is set
> > > but CONFIG_NF_TABLES=n -- and appears to be unused anyway.
> [...]
> > There are already changes in the net-next tree that will fix it.
>
> If the fix needs to go to -stable 5.3 kernel release, then you have to
> point to the particular commit ID of this patch to fix this one.
> net-next contains 5.4-rc material. I'd appreciate also if you can help
> identify the patch with a Fixes: tag.
The commit in the mainline that needs fixing is:
3c171f496ef5 ("netfilter: bridge: add connection tracking system")
The commit in net-next that fixes it is:
47e640af2e49 ("netfilter: add missing IS_ENABLED(CONFIG_NF_TABLES) check to header-file.")
I applied it to the mainline and compile-tested it to verify that it
does indeed fix the build failure.
From my reading of stable-kernel-rules.rst and netdev-FAQ.rst, it
appears that the fix should come from the mainline, so I will wait for
it to get there.
J.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 0/2] net/ibmvnic: serialization fixes
From: Juliet Kim @ 2019-09-17 14:52 UTC (permalink / raw)
To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
This series includes two fixes. The first improves reset code to allow
linkwatch_event to proceed during reset. The second ensures that no more
than one thread runs in reset at a time.
v2:
- Separate change param reset from do_reset()
- Return IBMVNIC_OPEN_FAILED if __ibmvnic_open fails
- Remove setting wait_for_reset to false from __ibmvnic_reset(), this
is done in wait_for_reset()
- Move the check for force_reset_recovery from patch 1 to patch 2
Juliet Kim (2):
net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
net/ibmvnic: prevent more than one thread from running in reset
drivers/net/ethernet/ibm/ibmvnic.c | 244 ++++++++++++++++++++++++++-----------
drivers/net/ethernet/ibm/ibmvnic.h | 4 +
2 files changed, 180 insertions(+), 68 deletions(-)
--
2.16.4
^ permalink raw reply
* [PATCH v2 1/2] net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
From: Juliet Kim @ 2019-09-17 14:52 UTC (permalink / raw)
To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
In-Reply-To: <20190917145249.15334-1-julietk@linux.vnet.ibm.com>
Commit a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
made the change to hold the RTNL lock during a reset to avoid deadlock
but linkwatch_event is fired during the reset and needs the RTNL lock.
That keeps linkwatch_event process from proceeding until the reset
is complete. The reset process cannot tolerate the linkwatch_event
processing after reset completes, so release the RTNL lock during the
process to allow a chance for linkwatch_event to run during reset.
This does not guarantee that the linkwatch_event will be processed as
soon as link state changes, but is an improvement over the current code
where linkwatch_event processing is always delayed, which prevents
transmissions on the device from being deactivated leading transmit
watchdog timer to time-out.
Release the RTNL lock before link state change and re-acquire after
the link state change to allow linkwatch_event to grab the RTNL lock
and run during the reset.
Fixes: a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 223 ++++++++++++++++++++++++++-----------
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 156 insertions(+), 68 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5cb55ea671e3..fc760c1eb0b0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1723,6 +1723,86 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
return rc;
}
+/**
+ * do_change_param_reset returns zero if we are able to keep processing reset
+ * events, or non-zero if we hit a fatal error and must halt.
+ */
+static int do_change_param_reset(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_rwi *rwi,
+ u32 reset_state)
+{
+ struct net_device *netdev = adapter->netdev;
+ int i, rc;
+
+ netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
+ rwi->reset_reason);
+
+ netif_carrier_off(netdev);
+ adapter->reset_reason = rwi->reset_reason;
+
+ ibmvnic_cleanup(netdev);
+
+ if (reset_state == VNIC_OPEN) {
+ rc = __ibmvnic_close(netdev);
+ if (rc)
+ return rc;
+ }
+
+ release_resources(adapter);
+ release_sub_crqs(adapter, 1);
+ release_crq_queue(adapter);
+
+ adapter->state = VNIC_PROBED;
+
+ rc = init_crq_queue(adapter);
+
+ if (rc) {
+ netdev_err(adapter->netdev,
+ "Couldn't initialize crq. rc=%d\n", rc);
+ return rc;
+ }
+
+ rc = ibmvnic_reset_init(adapter);
+ if (rc)
+ return IBMVNIC_INIT_FAILED;
+
+ /* If the adapter was in PROBE state prior to the reset,
+ * exit here.
+ */
+ if (reset_state == VNIC_PROBED)
+ return 0;
+
+ rc = ibmvnic_login(netdev);
+ if (rc) {
+ adapter->state = reset_state;
+ return rc;
+ }
+
+ rc = init_resources(adapter);
+ if (rc)
+ return rc;
+
+ ibmvnic_disable_irqs(adapter);
+
+ adapter->state = VNIC_CLOSED;
+
+ if (reset_state == VNIC_CLOSED)
+ return 0;
+
+ rc = __ibmvnic_open(netdev);
+ if (rc)
+ return IBMVNIC_OPEN_FAILED;
+
+ /* refresh device's multicast list */
+ ibmvnic_set_multi(netdev);
+
+ /* kick napi */
+ for (i = 0; i < adapter->req_rx_queues; i++)
+ napi_schedule(&adapter->napi[i]);
+
+ return 0;
+}
+
/**
* do_reset returns zero if we are able to keep processing reset events, or
* non-zero if we hit a fatal error and must halt.
@@ -1738,6 +1818,8 @@ static int do_reset(struct ibmvnic_adapter *adapter,
netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
rwi->reset_reason);
+ rtnl_lock();
+
netif_carrier_off(netdev);
adapter->reset_reason = rwi->reset_reason;
@@ -1751,16 +1833,25 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (reset_state == VNIC_OPEN &&
adapter->reset_reason != VNIC_RESET_MOBILITY &&
adapter->reset_reason != VNIC_RESET_FAILOVER) {
- rc = __ibmvnic_close(netdev);
+ adapter->state = VNIC_CLOSING;
+
+ /* Release the RTNL lock before link state change and
+ * re-acquire after the link state change to allow
+ * linkwatch_event to grab the RTNL lock and run during
+ * a reset.
+ */
+ rtnl_unlock();
+ rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+ rtnl_lock();
if (rc)
- return rc;
- }
+ goto out;
- if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
- adapter->wait_for_reset) {
- release_resources(adapter);
- release_sub_crqs(adapter, 1);
- release_crq_queue(adapter);
+ if (adapter->state != VNIC_CLOSING) {
+ rc = -1;
+ goto out;
+ }
+
+ adapter->state = VNIC_CLOSED;
}
if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -1769,9 +1860,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
- if (adapter->wait_for_reset) {
- rc = init_crq_queue(adapter);
- } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+ if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
rc = ibmvnic_reenable_crq_queue(adapter);
release_sub_crqs(adapter, 1);
} else {
@@ -1783,36 +1872,35 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (rc) {
netdev_err(adapter->netdev,
"Couldn't initialize crq. rc=%d\n", rc);
- return rc;
+ goto out;
}
rc = ibmvnic_reset_init(adapter);
- if (rc)
- return IBMVNIC_INIT_FAILED;
+ if (rc) {
+ rc = IBMVNIC_INIT_FAILED;
+ goto out;
+ }
/* If the adapter was in PROBE state prior to the reset,
* exit here.
*/
- if (reset_state == VNIC_PROBED)
- return 0;
+ if (reset_state == VNIC_PROBED) {
+ rc = 0;
+ goto out;
+ }
rc = ibmvnic_login(netdev);
if (rc) {
adapter->state = reset_state;
- return rc;
+ goto out;
}
- if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
- adapter->wait_for_reset) {
- rc = init_resources(adapter);
- if (rc)
- return rc;
- } else if (adapter->req_rx_queues != old_num_rx_queues ||
- adapter->req_tx_queues != old_num_tx_queues ||
- adapter->req_rx_add_entries_per_subcrq !=
- old_num_rx_slots ||
- adapter->req_tx_entries_per_subcrq !=
- old_num_tx_slots) {
+ if (adapter->req_rx_queues != old_num_rx_queues ||
+ adapter->req_tx_queues != old_num_tx_queues ||
+ adapter->req_rx_add_entries_per_subcrq !=
+ old_num_rx_slots ||
+ adapter->req_tx_entries_per_subcrq !=
+ old_num_tx_slots) {
release_rx_pools(adapter);
release_tx_pools(adapter);
release_napi(adapter);
@@ -1820,32 +1908,30 @@ static int do_reset(struct ibmvnic_adapter *adapter,
rc = init_resources(adapter);
if (rc)
- return rc;
+ goto out;
} else {
rc = reset_tx_pools(adapter);
if (rc)
- return rc;
+ goto out;
rc = reset_rx_pools(adapter);
if (rc)
- return rc;
+ goto out;
}
ibmvnic_disable_irqs(adapter);
}
adapter->state = VNIC_CLOSED;
- if (reset_state == VNIC_CLOSED)
- return 0;
+ if (reset_state == VNIC_CLOSED) {
+ rc = 0;
+ goto out;
+ }
rc = __ibmvnic_open(netdev);
if (rc) {
- if (list_empty(&adapter->rwi_list))
- adapter->state = VNIC_CLOSED;
- else
- adapter->state = reset_state;
-
- return 0;
+ rc = IBMVNIC_OPEN_FAILED;
+ goto out;
}
/* refresh device's multicast list */
@@ -1855,11 +1941,15 @@ static int do_reset(struct ibmvnic_adapter *adapter,
for (i = 0; i < adapter->req_rx_queues; i++)
napi_schedule(&adapter->napi[i]);
- if (adapter->reset_reason != VNIC_RESET_FAILOVER &&
- adapter->reset_reason != VNIC_RESET_CHANGE_PARAM)
+ if (adapter->reset_reason != VNIC_RESET_FAILOVER)
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, netdev);
- return 0;
+ rc = 0;
+
+out:
+ rtnl_unlock();
+
+ return rc;
}
static int do_hard_reset(struct ibmvnic_adapter *adapter,
@@ -1919,14 +2009,8 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter,
return 0;
rc = __ibmvnic_open(netdev);
- if (rc) {
- if (list_empty(&adapter->rwi_list))
- adapter->state = VNIC_CLOSED;
- else
- adapter->state = reset_state;
-
- return 0;
- }
+ if (rc)
+ return IBMVNIC_OPEN_FAILED;
return 0;
}
@@ -1965,20 +2049,11 @@ static void __ibmvnic_reset(struct work_struct *work)
{
struct ibmvnic_rwi *rwi;
struct ibmvnic_adapter *adapter;
- bool we_lock_rtnl = false;
u32 reset_state;
int rc = 0;
adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
- /* netif_set_real_num_xx_queues needs to take rtnl lock here
- * unless wait_for_reset is set, in which case the rtnl lock
- * has already been taken before initializing the reset
- */
- if (!adapter->wait_for_reset) {
- rtnl_lock();
- we_lock_rtnl = true;
- }
reset_state = adapter->state;
rwi = get_next_rwi(adapter);
@@ -1990,14 +2065,31 @@ static void __ibmvnic_reset(struct work_struct *work)
break;
}
- if (adapter->force_reset_recovery) {
- adapter->force_reset_recovery = false;
- rc = do_hard_reset(adapter, rwi, reset_state);
+ if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+ /* CHANGE_PARAM requestor holds rtnl_lock */
+ rc = do_change_param_reset(adapter, rwi, reset_state);
+ } else if (adapter->force_reset_recovery) {
+ /* Transport event occurred during previous reset */
+ if (adapter->wait_for_reset) {
+ /* Previous was CHANGE_PARAM; caller locked */
+ adapter->force_reset_recovery = false;
+ rc = do_hard_reset(adapter, rwi, reset_state);
+ } else {
+ rtnl_lock();
+ adapter->force_reset_recovery = false;
+ rc = do_hard_reset(adapter, rwi, reset_state);
+ rtnl_unlock();
+ }
} else {
rc = do_reset(adapter, rwi, reset_state);
}
kfree(rwi);
- if (rc && rc != IBMVNIC_INIT_FAILED &&
+ if (rc == IBMVNIC_OPEN_FAILED) {
+ if (list_empty(&adapter->rwi_list))
+ adapter->state = VNIC_CLOSED;
+ else
+ adapter->state = reset_state;
+ } else if (rc && rc != IBMVNIC_INIT_FAILED &&
!adapter->force_reset_recovery)
break;
@@ -2005,7 +2097,6 @@ static void __ibmvnic_reset(struct work_struct *work)
}
if (adapter->wait_for_reset) {
- adapter->wait_for_reset = false;
adapter->reset_done_rc = rc;
complete(&adapter->reset_done);
}
@@ -2016,8 +2107,6 @@ static void __ibmvnic_reset(struct work_struct *work)
}
adapter->resetting = false;
- if (we_lock_rtnl)
- rtnl_unlock();
}
static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
@@ -2078,8 +2167,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
return 0;
err:
- if (adapter->wait_for_reset)
- adapter->wait_for_reset = false;
return -ret;
}
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 70bd286f8932..9d3d35cc91d6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -20,6 +20,7 @@
#define IBMVNIC_INVALID_MAP -1
#define IBMVNIC_STATS_TIMEOUT 1
#define IBMVNIC_INIT_FAILED 2
+#define IBMVNIC_OPEN_FAILED 3
/* basic structures plus 100 2k buffers */
#define IBMVNIC_IO_ENTITLEMENT_DEFAULT 610305
--
2.16.4
^ permalink raw reply related
* [PATCH v2 2/2] net/ibmvnic: prevent more than one thread from running in reset
From: Juliet Kim @ 2019-09-17 14:52 UTC (permalink / raw)
To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
In-Reply-To: <20190917145249.15334-1-julietk@linux.vnet.ibm.com>
The current code allows more than one thread to run in reset. This can
corrupt struct adapter data. Check adapter->resetting before performing
a reset, if there is another reset running delay (100 msec) before trying
again.
Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 23 ++++++++++++++++++++++-
drivers/net/ethernet/ibm/ibmvnic.h | 3 +++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index fc760c1eb0b0..d3ebf4caa09c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2054,6 +2054,13 @@ static void __ibmvnic_reset(struct work_struct *work)
adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
+ if (adapter->resetting) {
+ schedule_delayed_work(&adapter->ibmvnic_delayed_reset,
+ IBMVNIC_RESET_DELAY);
+ return;
+ }
+
+ adapter->resetting = true;
reset_state = adapter->state;
rwi = get_next_rwi(adapter);
@@ -2094,6 +2101,10 @@ static void __ibmvnic_reset(struct work_struct *work)
break;
rwi = get_next_rwi(adapter);
+
+ if (rwi && (rwi->reset_reason == VNIC_RESET_FAILOVER ||
+ rwi->reset_reason == VNIC_RESET_MOBILITY))
+ adapter->force_reset_recovery = true;
}
if (adapter->wait_for_reset) {
@@ -2109,6 +2120,15 @@ static void __ibmvnic_reset(struct work_struct *work)
adapter->resetting = false;
}
+static void __ibmvnic_delayed_reset(struct work_struct *work)
+{
+ struct ibmvnic_adapter *adapter;
+
+ adapter = container_of(work, struct ibmvnic_adapter,
+ ibmvnic_delayed_reset.work);
+ __ibmvnic_reset(&adapter->ibmvnic_reset);
+}
+
static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
enum ibmvnic_reset_reason reason)
{
@@ -2161,7 +2181,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
rwi->reset_reason = reason;
list_add_tail(&rwi->list, &adapter->rwi_list);
spin_unlock_irqrestore(&adapter->rwi_lock, flags);
- adapter->resetting = true;
netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
schedule_work(&adapter->ibmvnic_reset);
@@ -4932,6 +4951,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
spin_lock_init(&adapter->stats_lock);
INIT_WORK(&adapter->ibmvnic_reset, __ibmvnic_reset);
+ INIT_DELAYED_WORK(&adapter->ibmvnic_delayed_reset,
+ __ibmvnic_delayed_reset);
INIT_LIST_HEAD(&adapter->rwi_list);
spin_lock_init(&adapter->rwi_lock);
init_completion(&adapter->init_done);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 9d3d35cc91d6..4f4651d92cc1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -39,6 +39,8 @@
#define IBMVNIC_MAX_LTB_SIZE ((1 << (MAX_ORDER - 1)) * PAGE_SIZE)
#define IBMVNIC_BUFFER_HLEN 500
+#define IBMVNIC_RESET_DELAY 100
+
static const char ibmvnic_priv_flags[][ETH_GSTRING_LEN] = {
#define IBMVNIC_USE_SERVER_MAXES 0x1
"use-server-maxes"
@@ -1077,6 +1079,7 @@ struct ibmvnic_adapter {
spinlock_t rwi_lock;
struct list_head rwi_list;
struct work_struct ibmvnic_reset;
+ struct delayed_work ibmvnic_delayed_reset;
bool resetting;
bool napi_enabled, from_passive_init;
--
2.16.4
^ permalink raw reply related
* Re: [PATCH bpf-next 1/5] perf/core: Add PERF_FORMAT_LOST read_format
From: Daniel Xu @ 2019-09-17 15:14 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, bpf, songliubraving, yhs, andriin, peterz, mingo,
acme, Daniel Xu, ast, alexander.shishkin, jolsa, namhyung,
linux-kernel, netdev, kernel-team
On Tue Sep 17, 2019 at 10:32 PM kbuild test robot wrote:
> All errors (new ones prefixed by >>):
>
> kernel/events/core.c: In function 'perf_event_lost':
> >> kernel/events/core.c:4753:11: error: implicit declaration of function 'perf_kprobe_missed'; did you mean 'perf_release'? [-Werror=implicit-function-declaration]
> lost += perf_kprobe_missed(event);
> ^~~~~~~~~~~~~~~~~~
> perf_release
> cc1: some warnings being treated as errors
>
Ah forgot the #ifdef for CONFIG_KPROBE_EVENTS. I've applied the fix and
will send it in the next version.
^ permalink raw reply
* Re: [PATCH bpf-next 1/5] perf/core: Add PERF_FORMAT_LOST read_format
From: kbuild test robot @ 2019-09-17 15:22 UTC (permalink / raw)
To: Daniel Xu
Cc: kbuild-all, bpf, songliubraving, yhs, andriin, peterz, mingo,
acme, Daniel Xu, ast, alexander.shishkin, jolsa, namhyung,
linux-kernel, netdev, kernel-team
In-Reply-To: <20190917133056.5545-2-dxu@dxuuu.xyz>
[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Daniel-Xu/perf-core-Add-PERF_FORMAT_LOST-read_format/20190917-213515
base: https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-randconfig-e003-201937 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/events/core.c: In function 'perf_event_lost':
>> kernel/events/core.c:4753:11: error: implicit declaration of function 'perf_kprobe_missed'; did you mean 'perf_uprobe_init'? [-Werror=implicit-function-declaration]
lost += perf_kprobe_missed(event);
^~~~~~~~~~~~~~~~~~
perf_uprobe_init
cc1: some warnings being treated as errors
vim +4753 kernel/events/core.c
4739
4740 static struct pmu perf_kprobe;
4741 static u64 perf_event_lost(struct perf_event *event)
4742 {
4743 struct ring_buffer *rb;
4744 u64 lost = 0;
4745
4746 rcu_read_lock();
4747 rb = rcu_dereference(event->rb);
4748 if (likely(!!rb))
4749 lost += local_read(&rb->lost);
4750 rcu_read_unlock();
4751
4752 if (event->attr.type == perf_kprobe.type)
> 4753 lost += perf_kprobe_missed(event);
4754
4755 return lost;
4756 }
4757
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36877 bytes --]
^ permalink raw reply
* Re: [PATCH iproute2 4/4] devlink: Fix devlink health set command
From: Stephen Hemminger @ 2019-09-17 15:23 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Ahern, netdev, Moshe Shemesh, Aya Levin, Jiri Pirko
In-Reply-To: <1567687387-12993-5-git-send-email-tariqt@mellanox.com>
On Thu, 5 Sep 2019 15:43:07 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> From: Aya Levin <ayal@mellanox.com>
>
> Prior to this patch both the reporter's name and the grace period
> attributes shared the same bit. This caused zeroing grace period when
> setting auto recovery. Let each parameter has its own bit.
>
> Fixes: b18d89195b16 ("devlink: Add devlink health set command")
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Does not apply to current iproute2.
^ permalink raw reply
* [PATCH net] net/rds: Check laddr_check before calling it
From: Ka-Cheong Poon @ 2019-09-17 15:29 UTC (permalink / raw)
To: netdev; +Cc: santosh.shilimkar, davem, rds-devel
In rds_bind(), laddr_check is called without checking if it is NULL or
not. And rs_transport should be reset if rds_add_bound() fails.
Reported-by: syzbot+fae39afd2101a17ec624@syzkaller.appspotmail.com
Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
---
net/rds/bind.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/rds/bind.c b/net/rds/bind.c
index 20c156a..5b5fb4c 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -244,7 +244,8 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
*/
if (rs->rs_transport) {
trans = rs->rs_transport;
- if (trans->laddr_check(sock_net(sock->sk),
+ if (!trans->laddr_check ||
+ trans->laddr_check(sock_net(sock->sk),
binding_addr, scope_id) != 0) {
ret = -ENOPROTOOPT;
goto out;
@@ -263,6 +264,8 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
sock_set_flag(sk, SOCK_RCU_FREE);
ret = rds_add_bound(rs, binding_addr, &port, scope_id);
+ if (ret)
+ rs->rs_transport = NULL;
out:
release_sock(sk);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] net/rds: Check laddr_check before calling it
From: santosh.shilimkar @ 2019-09-17 15:31 UTC (permalink / raw)
To: Ka-Cheong Poon; +Cc: netdev, davem, rds-devel
In-Reply-To: <1568734158-18021-1-git-send-email-ka-cheong.poon@oracle.com>
On 9/17/19 8:29 AM, Ka-Cheong Poon wrote:
> In rds_bind(), laddr_check is called without checking if it is NULL or
> not. And rs_transport should be reset if rds_add_bound() fails.
>
> Reported-by: syzbot+fae39afd2101a17ec624@syzkaller.appspotmail.com
> Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
> ---
Thanks Ka-Cheong for getting this out quickly on list.
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ permalink raw reply
* Re: divide error in cdc_ncm_update_rxtx_max
From: Bjørn Mork @ 2019-09-17 15:46 UTC (permalink / raw)
To: syzbot
Cc: andreyknvl, davem, linux-kernel, linux-usb, netdev, oliver,
syzkaller-bugs
In-Reply-To: <00000000000018e4250592c043c3@google.com>
syzbot <syzbot+ce366e2b8296e25d84f5@syzkaller.appspotmail.com> writes:
> syzbot has tested the proposed patch but the reproducer still
> triggered crash:
> divide error in usbnet_update_max_qlen
>
> cdc_ncm 5-1:1.0: setting tx_max = 16384
> divide error: 0000 [#1] SMP KASAN
> CPU: 1 PID: 1737 Comm: kworker/1:2 Not tainted 5.3.0-rc7+ #0
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 01/01/2011
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:usbnet_update_max_qlen drivers/net/usb/usbnet.c:344 [inline]
> RIP: 0010:usbnet_update_max_qlen+0x231/0x370 drivers/net/usb/usbnet.c:338
Sure, but that's another error already fixed by Oliver..
I guess this fix worked. But I believe we should see if this is a more
generic issue than just this single driver/bug. I fear it is...
Bjørn
^ permalink raw reply
* Re: [PATCH] net: sysctl: cleanup net_sysctl_init error exit paths
From: George G. Davis @ 2019-09-17 15:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20190708224732.GA8009@mam-gdavis-lt>
Hello David,
On Mon, Jul 08, 2019 at 06:47:32PM -0400, George G. Davis wrote:
> Hello David,
>
> On Fri, May 17, 2019 at 10:43:45AM -0400, George G. Davis wrote:
> > Hello David,
> >
> > On Thu, May 16, 2019 at 02:27:44PM -0700, David Miller wrote:
> > > From: "George G. Davis" <george_davis@mentor.com>
> > > Date: Thu, 16 May 2019 11:23:08 -0400
> > >
> > > > Unwind net_sysctl_init error exit goto spaghetti code
> > > >
> > > > Suggested-by: Joshua Frkuska <joshua_frkuska@mentor.com>
> > > > Signed-off-by: George G. Davis <george_davis@mentor.com>
> > >
> > > Cleanups are not appropriate until the net-next tree opens back up.
> > >
> > > So please resubmit at that time.
> >
> > I fear that I may be distracted by other shiny objects by then but
> > I'll make a reminder and try to resubmit during the next merge window.
>
> Since the "Linux 5.2" kernel has been released [1], I'm guessing that the
> net-next merge window is open now? If yes, the patch remains unchanged
> since my initial post. Please consider applying or let me know when to
> resubmit when the net-next merge window is again open.
Ping, "Linux 5.3" kernel has been released [1] and it appears that the
5.4 merge window is open. The patch [2] remains unchanged since my initial
post. Please consider applying it.
Thanks!
--
Regards,
George
[1] https://lwn.net/Articles/799250/
[2] https://lore.kernel.org/patchwork/patch/1074969/
^ permalink raw reply
* Re: -Wsizeof-array-div warnings in ethernet drivers
From: Nick Desaulniers @ 2019-09-17 16:00 UTC (permalink / raw)
To: Jose Abreu
Cc: David Laight, Nathan Chancellor, Tom Lendacky, Giuseppe Cavallaro,
Alexandre Torgue, David S. Miller, Ilie Halip, David Bolvansky,
netdev@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <BN8PR12MB32662378E844E6ECBA3FE8D7D38F0@BN8PR12MB3266.namprd12.prod.outlook.com>
On Tue, Sep 17, 2019 at 6:27 AM Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Sep/17/2019, 11:36:21 (UTC+00:00)
>
> > From: Jose Abreu
> > > Sent: 17 September 2019 08:59
> > > From: Nathan Chancellor <natechancellor@gmail.com>
> > > Date: Sep/17/2019, 08:32:32 (UTC+00:00)
> > >
> > > > Hi all,
> > > >
> > > > Clang recently added a new diagnostic in r371605, -Wsizeof-array-div,
> > > > that tries to warn when sizeof(X) / sizeof(Y) does not compute the
> > > > number of elements in an array X (i.e., sizeof(Y) is wrong). See that
> > > > commit for more details:
> > ...
> > > > ../drivers/net/ethernet/amd/xgbe/xgbe-dev.c:361:49: warning: expression
> > > > does not compute the number of elements in this array; element type is
> > > > 'u8' (aka 'unsigned char'), not 'u32' (aka 'unsigned int')
> > > > [-Wsizeof-array-div]
> > > > unsigned int key_regs = sizeof(pdata->rss_key) / sizeof(u32);
> > > > ~~~~~~~~~~~~~~ ^
> > ...
> > > > What is the reasoning behind having the key being an array of u8s but
> > > > seemlingly converting it into an array of u32s? It's not immediately
> > > > apparent from reading over the code but I am not familiar with it so I
> > > > might be making a mistake. I assume this is intentional? If so, the
> > > > warning can be silenced and we'll send patches to do so but we want to
> > > > make sure we aren't actually papering over a mistake.
> > >
> > > This is because we write 32 bits at a time to the reg but internally the
> > > driver uses 8 bits to store the array. If you look at
> > > dwxgmac2_rss_configure() you'll see that cfg->key is casted to u32 which
> > > is the value we use in HW writes. Then the for loop just does the math
> > > to check how many u32's has to write.
> >
> > That stinks of a possible misaligned data access.....
>
> It's possible to happen only if structure field is not aligned. I guess
> I can either change all to u32 or just __align the field of the struct
Would __aligning the struct still produce the warning? It's good to
know that this case is intentional, but I would like to consider other
instances of it before we seriously consider turning it off. If the
driver can be rewritten to just make use of u32, I would find that
preferrable.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [patch iproute2-next v2] devlink: add reload failed indication
From: David Ahern @ 2019-09-17 16:46 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: stephen, idosch, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <20190916094448.26072-1-jiri@resnulli.us>
On 9/16/19 3:44 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Add indication about previous failed devlink reload.
>
> Example outputs:
>
> $ devlink dev
> netdevsim/netdevsim10: reload_failed true
odd output to user. Why not just "reload failed"?
^ permalink raw reply
* Re: BUG: sleeping function called from invalid context in tcf_chain0_head_change_cb_del
From: Cong Wang @ 2019-09-17 17:03 UTC (permalink / raw)
To: Vlad Buslov
Cc: syzbot, Alexei Starovoitov, Daniel Borkmann, David Miller,
David Ahern, Florian Fainelli, hawk@kernel.org, Ido Schimmel,
Jakub Kicinski, Jamal Hadi Salim, Jiri Pirko, Jiri Pirko,
John Fastabend, Martin KaFai Lau, LKML,
Linux Kernel Network Developers, Nikolay Aleksandrov,
Petr Machata, Roopa Prabhu, Song Liu, syzkaller-bugs,
xdp-newbies@vger.kernel.org, yhs@fb.com
In-Reply-To: <vbfk1a7cooq.fsf@mellanox.com>
On Tue, Sep 17, 2019 at 1:27 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> Hi Cong,
>
> Don't see why we would need qdisc tree lock while releasing the
> reference to (or destroying) previous Qdisc. I've skimmed through other
> scheds and it looks like sch_multiq, sch_htb and sch_tbf are also
> affected. Do you want me to send patches?
Yes, please do.
^ permalink raw reply
* Re: [PATCH] selftests: Add test cases for `ip nexthop flush proto XX`
From: David Ahern @ 2019-09-17 17:04 UTC (permalink / raw)
To: Donald Sharp, netdev, dsahern
In-Reply-To: <20190916122650.24124-1-sharpd@cumulusnetworks.com>
On 9/16/19 6:26 AM, Donald Sharp wrote:
> Add some test cases to allow the fib_nexthops.sh test code
> to test the flushing of nexthops based upon the proto passed
> in upon creation of the nexthop group.
>
> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
> ---
> tools/testing/selftests/net/fib_nexthops.sh | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH 3/6] Timer: expose monotonic clock and counter value
From: Marc Zyngier @ 2019-09-17 17:10 UTC (permalink / raw)
To: Jianyong Wu, netdev, pbonzini, sean.j.christopherson,
richardcochran, Mark.Rutland, Will.Deacon, suzuki.poulose
Cc: linux-kernel, Steve.Capper, Kaly.Xin, justin.he, nd,
linux-arm-kernel
In-Reply-To: <20190917112430.45680-4-jianyong.wu@arm.com>
On 17/09/2019 12:24, Jianyong Wu wrote:
> A number of PTP drivers (such as ptp-kvm) are assuming what the
> current clock source is, which could lead to interesting effects on
> systems where the clocksource can change depending on external events.
>
> For this purpose, add a new API that retrives both the current
> monotonic clock as well as its counter value.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
There must be something wrong with the way you've taken this patch in
your tree. My authorship is gone (not that I deeply care about it, but
it is good practice to keep attributions), and the subject line has been
rewritten.
I'd appreciate it if you could fix this in a future revision of this
series. For reference, the original patch is here[1].
> ---
> include/linux/timekeeping.h | 3 +++
> kernel/time/timekeeping.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> index a8ab0f143ac4..a5389adaa8bc 100644
> --- a/include/linux/timekeeping.h
> +++ b/include/linux/timekeeping.h
> @@ -247,6 +247,9 @@ extern int get_device_system_crosststamp(
> struct system_time_snapshot *history,
> struct system_device_crosststamp *xtstamp);
>
> +/* Obtain current monotonic clock and its counter value */
> +extern void get_current_counterval(struct system_counterval_t *sc);
> +
> /*
> * Simultaneously snapshot realtime and monotonic raw clocks
> */
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 44b726bab4bd..07a0969625b1 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1098,6 +1098,19 @@ static bool cycle_between(u64 before, u64 test, u64 after)
> return false;
> }
>
> +/**
> + * get_current_counterval - Snapshot the current clocksource and counter value
> + * @sc: Pointer to a struct containing the current clocksource and its value
> + */
> +void get_current_counterval(struct system_counterval_t *sc)
> +{
> + struct timekeeper *tk = &tk_core.timekeeper;
> +
> + sc->cs = READ_ONCE(tk->tkr_mono.clock);
> + sc->cycles = sc->cs->read(sc->cs);
> +}
> +EXPORT_SYMBOL_GPL(get_current_counterval);
This export wasn't in my original patch. I guess you need it because
your ptp driver builds as a module? It'd be good to mention it in the
commit log.
> +
> /**
> * get_device_system_crosststamp - Synchronously capture system/device timestamp
> * @get_time_fn: Callback to get simultaneous device time and
>
Thanks,
M.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=timer/counterval&id=a6e8abce025691b6a55e1c195878d7f76bfeb9d1
--
Jazz is not dead, it just smells funny...
^ permalink raw reply
* Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received out-of-order
From: Jason Baron @ 2019-09-17 17:12 UTC (permalink / raw)
To: Eric Dumazet, Thomas Higdon; +Cc: netdev, Jonathan Lemon, Dave Jones
In-Reply-To: <CANn89iKCSae880bS3MTwrm=MeTyPsntyXfkhJS7CfgtpiEpOsQ@mail.gmail.com>
On 9/10/19 4:38 PM, Eric Dumazet wrote:
> On Tue, Sep 10, 2019 at 10:11 PM Thomas Higdon <tph@fb.com> wrote:
>>
>>
> ...
>> Because an additional 32-bit member in struct tcp_info would cause
>> a hole on 64-bit systems, we reserve a struct member '_reserved'.
> ...
>> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
>> index b3564f85a762..990a5bae3ac1 100644
>> --- a/include/uapi/linux/tcp.h
>> +++ b/include/uapi/linux/tcp.h
>> @@ -270,6 +270,9 @@ struct tcp_info {
>> __u64 tcpi_bytes_retrans; /* RFC4898 tcpEStatsPerfOctetsRetrans */
>> __u32 tcpi_dsack_dups; /* RFC4898 tcpEStatsStackDSACKDups */
>> __u32 tcpi_reord_seen; /* reordering events seen */
>> +
>> + __u32 _reserved; /* Reserved for future 32-bit member. */
>> + __u32 tcpi_rcv_ooopack; /* Out-of-order packets received */
>> };
>>
>
> Unfortunately we won't be able to use this hole, because the way the
> TCP_INFO works,
>
> The kernel will report the same size after the reserved field is
> renamed to something else.
>
> User space code is able to detect which fields are there or not based
> on what the kernel
> returns for the size of the structure.
>
Hi,
I was interested in adding a field to tcp_info around the TFO state of a
socket. So for the server side it would indicate if TFO was used to
create the socket and on the client side it would report whether TFO
worked and if not that it failed with maybe some additional states
around why it failed. I'm thinking it would be maybe 3 bits.
My question is whether its reasonable to use the unused bits of
__u8 tcpi_delivery_rate_app_limited:1;. Or is this not good because
the size hasn't changed? What if I avoided using 0 for the new field to
avoid the possibility of not knowing if 0 because its the old kernel or
0 because that's now its a TFO state? IE the new field could always be >
0 for the new kernel.
Thanks,
-Jason
^ 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