* [PATCH] net/core/filter: fix unused-variable warning
From: Anders Roxell @ 2018-09-07 12:50 UTC (permalink / raw)
To: davem, ast, daniel, tehnerd; +Cc: netdev, linux-kernel, Anders Roxell
Building with CONFIG_INET=n will show the warning below:
net/core/filter.c: In function ‘____bpf_getsockopt’:
net/core/filter.c:4048:19: warning: unused variable ‘tp’ [-Wunused-variable]
struct tcp_sock *tp;
^~
net/core/filter.c:4046:31: warning: unused variable ‘icsk’ [-Wunused-variable]
struct inet_connection_sock *icsk;
^~~~
Move the variable declarations inside the {} block where they are used.
Fixes: 1e215300f138 ("bpf: add TCP_SAVE_SYN/TCP_SAVED_SYN options for bpf_(set|get)sockopt")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
net/core/filter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index d301134bca3a..0ae7185b2207 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4043,14 +4043,14 @@ static const struct bpf_func_proto bpf_setsockopt_proto = {
BPF_CALL_5(bpf_getsockopt, struct bpf_sock_ops_kern *, bpf_sock,
int, level, int, optname, char *, optval, int, optlen)
{
- struct inet_connection_sock *icsk;
struct sock *sk = bpf_sock->sk;
- struct tcp_sock *tp;
if (!sk_fullsock(sk))
goto err_clear;
#ifdef CONFIG_INET
if (level == SOL_TCP && sk->sk_prot->getsockopt == tcp_getsockopt) {
+ struct inet_connection_sock *icsk;
+ struct tcp_sock *tp;
switch (optname) {
case TCP_CONGESTION:
icsk = inet_csk(sk);
--
2.18.0
^ permalink raw reply related
* [PATCH] ethernet: hnae: add unlikely() to assert()
From: Igor Stoppa @ 2018-09-07 17:26 UTC (permalink / raw)
To: huangdaode
Cc: igor.stoppa, Igor Stoppa, Yisen Zhuang, Salil Mehta,
David S. Miller, netdev, linux-kernel
The assert() condition is likely to be true.
Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
Cc: huangdaode <huangdaode@hisilicon.com>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/hisilicon/hns/hnae.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.h b/drivers/net/ethernet/hisilicon/hns/hnae.h
index 08a750fb60c4..bd3c180a3fe9 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.h
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.h
@@ -47,7 +47,7 @@
#ifndef assert
#define assert(expr) \
do { \
- if (!(expr)) { \
+ if (unlikely(!(expr))) { \
pr_err("Assertion failed! %s, %s, %s, line %d\n", \
#expr, __FILE__, __func__, __LINE__); \
} \
--
2.17.1
^ permalink raw reply related
* [PATCH] freescale: ethernet: remove unnecessary unlikely()
From: Igor Stoppa @ 2018-09-07 17:23 UTC (permalink / raw)
To: Madalin Bucur
Cc: igor.stoppa, Igor Stoppa, David S. Miller, netdev, linux-kernel
Both WARN_ON() and WARN_ONCE() already contain an unlikely(), so it's not
necessary to wrap it into another.
Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 65a22cd9aef2..783134f1b779 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -1280,7 +1280,7 @@ static int dpaa_bman_release(const struct dpaa_bp *dpaa_bp,
err = bman_release(dpaa_bp->pool, bmb, cnt);
/* Should never occur, address anyway to avoid leaking the buffers */
- if (unlikely(WARN_ON(err)) && dpaa_bp->free_buf_cb)
+ if (WARN_ON(err) && dpaa_bp->free_buf_cb)
while (cnt-- > 0)
dpaa_bp->free_buf_cb(dpaa_bp, &bmb[cnt]);
@@ -1704,10 +1704,8 @@ static struct sk_buff *contig_fd_to_skb(const struct dpaa_priv *priv,
skb = build_skb(vaddr, dpaa_bp->size +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
- if (unlikely(!skb)) {
- WARN_ONCE(1, "Build skb failure on Rx\n");
+ if (WARN_ONCE(!skb, "Build skb failure on Rx\n"))
goto free_buffer;
- }
WARN_ON(fd_off != priv->rx_headroom);
skb_reserve(skb, fd_off);
skb_put(skb, qm_fd_get_length(fd));
@@ -1770,7 +1768,7 @@ static struct sk_buff *sg_fd_to_skb(const struct dpaa_priv *priv,
sz = dpaa_bp->size +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
skb = build_skb(sg_vaddr, sz);
- if (WARN_ON(unlikely(!skb)))
+ if (WARN_ON(!skb))
goto free_buffers;
skb->ip_summed = rx_csum_offload(priv, fd);
--
2.17.1
^ permalink raw reply related
* [PATCH] wimax: i2400m: remove unnecessary unlikely()
From: Igor Stoppa @ 2018-09-07 17:22 UTC (permalink / raw)
To: Inaky Perez-Gonzalez
Cc: igor.stoppa, Igor Stoppa, linux-wimax, David S. Miller, netdev,
linux-kernel
WARN_ON() already contains an unlikely(), so it's not necessary to
wrap it into another.
Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
Cc: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
Cc: linux-wimax@intel.com
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/wimax/i2400m/tx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index f20886ade1cc..59c70d928249 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -655,8 +655,7 @@ void i2400m_tx_close(struct i2400m *i2400m)
padding = aligned_size - tx_msg_moved->size;
if (padding > 0) {
pad_buf = i2400m_tx_fifo_push(i2400m, padding, 0, 0);
- if (unlikely(WARN_ON(pad_buf == NULL
- || pad_buf == TAIL_FULL))) {
+ if (WARN_ON(!pad_buf || pad_buf == TAIL_FULL)) {
/* This should not happen -- append should verify
* there is always space left at least to append
* tx_block_size */
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 2/2] net: ethernet: i40evf: fix underlying build error
From: Wang, Dongsheng @ 2018-09-07 17:14 UTC (permalink / raw)
To: Sergei Shtylyov, jeffrey.t.kirsher@intel.com
Cc: jacob.e.keller@intel.com, davem@davemloft.net,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <59fd149c-bacc-c39c-b0d8-a4fb9366f26a@cogentembedded.com>
On 9/7/2018 11:33 PM, Sergei Shtylyov wrote:
> On 09/07/2018 02:19 PM, Wang Dongsheng wrote:
>
>> Can't have non-inline function in a header file.
>> There is a risk of "Multiple definition" from cross-including.
>>
>> Tested on: x86_64, make ARCH=i386
>>
>> Modules section .text:
>> i40e: 00019380 <__i40e_add_stat_strings>:
>> i40evf: 00006b00 <__i40e_add_stat_strings>:
>>
>> Buildin section .text:
>> i40e: c351ca60 <__i40e_add_stat_strings>:
>> i40evf: c354f2c0 <__i40e_add_stat_strings>:
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>> ---
>> V3: add static
>> ---
>> .../intel/i40evf/i40e_ethtool_stats.h | 23 +-----------------
>> .../ethernet/intel/i40evf/i40evf_ethtool.c | 24 +++++++++++++++++++
>> 2 files changed, 25 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
>> index 60b595dd8c39..62ab67a77753 100644
>> --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
>> +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
>> @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring *ring)
>> *data += size;
>> }
>>
>> -/**
>> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
>> - * @p: ethtool supplied buffer
>> - * @stats: stat definitions array
>> - * @size: size of the stats array
>> - *
>> - * Format and copy the strings described by stats into the buffer pointed at
>> - * by p.
>> - **/
>> static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> There's no point to keeping *static* function in the header file (unless it's
> also *inline*).
Yes， we need it for now. Because there is a copy file at i40e dir, and
a "Multiple definition" will show up when we buildin i40e&i40evf and
remove this *static* .
Cause the header file is only used in ethtool.c so we can keep this
static, and another option is not touch this header.
As I replied to Jacob's email earlier, we can do without touch i40evf at
all. Because this header is only for one and not included in another.
Cheers,
Dongsheng
>> - const unsigned int size, ...)
>> -{
>> - unsigned int i;
>> -
>> - for (i = 0; i < size; i++) {
>> - va_list args;
>> -
>> - va_start(args, size);
>> - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
>> - *p += ETH_GSTRING_LEN;
>> - va_end(args);
>> - }
>> -}
>> + const unsigned int size, ...);
>>
>> /**
>> * 40e_add_stat_strings - copy stat strings into ethtool buffer
> [...]
>
> MBR, Sergei
>
^ permalink raw reply
* [PATCH] nfp: replace spin_lock_bh with spin_lock in tasklet callback
From: jun qian @ 2018-09-07 17:01 UTC (permalink / raw)
To: Jakub Kicinski, Dirk van der Merwe, Daniel Borkmann,
Quentin Monnet
Cc: oss-drivers, netdev, linux-kernel, jun qian
As you are already in a tasklet, it is unnecessary to call spin_lock_bh.
Signed-off-by: jun qian <hangdianqj@163.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index a8b9fbab5f73..084c983ec3c2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2075,10 +2075,10 @@ static void nfp_ctrl_poll(unsigned long arg)
{
struct nfp_net_r_vector *r_vec = (void *)arg;
- spin_lock_bh(&r_vec->lock);
+ spin_lock(&r_vec->lock);
nfp_net_tx_complete(r_vec->tx_ring, 0);
__nfp_ctrl_tx_queued(r_vec);
- spin_unlock_bh(&r_vec->lock);
+ spin_unlock(&r_vec->lock);
nfp_ctrl_rx(r_vec);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/7] fix hnode refcounting
From: Jamal Hadi Salim @ 2018-09-07 12:13 UTC (permalink / raw)
To: Al Viro; +Cc: netdev, Cong Wang, Jiri Pirko, stable
In-Reply-To: <20180907023529.GV19965@ZenIV.linux.org.uk>
On 2018-09-06 10:35 p.m., Al Viro wrote:
> On Thu, Sep 06, 2018 at 06:21:09AM -0400, Jamal Hadi Salim wrote:
[..]
>
> Argh... Unfortunately, there's this: in u32_delete() we have
> if (root_ht) {
> if (root_ht->refcnt > 1) {
> *last = false;
> goto ret;
> }
> if (root_ht->refcnt == 1) {
> if (!ht_empty(root_ht)) {
> *last = false;
> goto ret;
> }
> }
> }
> and that would need to be updated.
It is not detrimental as you have it right now but
you are right an adjustment is needed...
Deleting of a root directly should not be allowed. But
you can flush a whole tp. Consider this:
--
sudo tc qdisc add dev $P ingress
sudo tc filter add dev $P parent ffff: protocol ip prio 10 \
u32 match ip protocol 1 0xff
Which creates root ht 800
You shouldnt be allowed to do this:
--
tc filter delete dev $P parent ffff: protocol ip prio 10 handle 800: u32
---
But you can delete the tp entirely as such:
---
tc filter delete dev $P parent ffff: protocol ip prio 10 u32
--
The later will go via the destroy() path and flush all filters.
You should also be able to delete individual filters. ex:
$tc filter del dev $P parent ffff: prio 10 handle 800:0:800 u32
Where that code you are referring to is important is when
the last filter deleted - we need the caller to know
and it destroys root.
i.e you should return last=true when the last filter is
deleted so root gets auto deleted (just like it was autocreated)
> However, that logics is bloody odd
> to start with. First of all, root_ht has come from
> struct tc_u_hnode *root_ht = rtnl_dereference(tp->root);
> and the only place where it's ever modified is
> rcu_assign_pointer(tp->root, root_ht);
> in u32_init(), where we'd bloody well checked that root_ht is non-NULL
> (see
> if (root_ht == NULL)
> return -ENOBUFS;
> upstream of that place) and where that assignment is inevitable on the
> way to returning 0. No matter what, if tp has passed u32_init() it
> will have non-NULL ->root, forever. And there is no way for tcf_proto
> to be seen outside of tcf_proto_create() without ->init() having returned
> 0 - it gets freed before anyone sees it.
>
Yes, the check for root_ht is not necessary - but the check for the
last filter (and testing for last) is needed.
> So this 'if (root_ht)' can't be false. What's more, what the hell is the
> whole thing checking? We are in u32_delete(). It's called (as ->delete())
> from tfilter_del_notify(), which is called from tc_del_tfilter(). If we
> return 0 with *last true, we follow up calling tcf_proto_destroy().
> OK, let's look at the logics in there:
> * if there are links to root hnode => false
> * if there's no links to root hnode and it has knodes => false
> (BTW, if we ever get there with root_ht->refcnt < 1, we are obviously screwed)
> * if there is a tcf_proto sharing tp->data => false (i.e. any filters
> with different prio - don't bother)
> * if tp is the only one with reference to tp->data and there are *any*
> knodes => false.
>
> Any extra links can come only from knodes in a non-empty hnode. And it's not
> a common case. Shouldn't thIe whole thing be
> * shared tp->data => false
> * any non-empty hnode => false
> instead? Perhaps even with the knode counter in tp->data, avoiding any loops
> in there, as well as the entire ht_empty()...
>
> Now, in the very beginning of u32_delete() we have this:
> struct tc_u_hnode *ht = arg;
>
> if (ht == NULL)
> goto out;
> OK, but the call of ->delete() is
> err = tp->ops->delete(tp, fh, last, extack);
> and arg == NULL seen in u32_delete() means fh == NULL in tfilter_del_notify().
> Which is called in
> if (!fh) {
> ...
> } else {
> bool last;
>
> err = tfilter_del_notify(net, skb, n, tp, block,
> q, parent, fh, false, &last,
> extack);
> How can we ever get there with NULL fh?
>
Try:
tc filter delete dev $P parent ffff: protocol ip prio 10 u32
tcm handle is 0, so will hit that code path.
> The whole thing makes very little sense; looks like it used to live in
> u32_destroy() prior to commit 763dbf6328e41 ("net_sched: move the empty tp
> check from ->destroy() to ->delete()"), but looking at the rationale in
> that commit... I don't see how it fixes anything - sure, now we remove
> tcf_proto from the list before calling ->destroy(). Without any RCU delays
> in between. How could it possibly solve any issues with ->classify()
> called in parallel with ->destroy()? cls_u32 (at least these days)
> does try to survive u32_destroy() in parallel with u32_classify();
> if any other classifiers do not, they are still broken and that commit
> has not done anything for them.
>
> Anyway, adjusting 1/7 for that is trivial, but I would really like to
> understand what that code is doing... Comments?
>
Refer to above.
cheers,
jamal
^ permalink raw reply
* network device suspend/resume
From: Lakshmi @ 2018-09-07 11:57 UTC (permalink / raw)
To: Oliver Neukum, netdev
Hi,
I am bringing kernel bugzilla bug here 196399
https://bugzilla.kernel.org/show_bug.cgi?id=196399
This issue occurred last time two months ago and I wonder if it appears
again. Can you confirm if there is any issue related to network device
suspend/resume.
last instance is here
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4376/fi-skl-6600u/igt@gem_exec_suspend@basic-s4-devices.html
==================================================================
0b95:7720 ASIX Electronics Corp. AX88772
USB network card
driver seems to be usbnet
==================================================================
Bug Report: 196399
We have found out that since at least 4.11-rc1, some machines in the
Intel GFX CI lab have been generating the following warning when
suspending to s4 (suspend to disk):
[ 287.212825] ------------[ cut here ]------------
[ 287.212829] WARNING: CPU: 0 PID: 3165 at net/sched/sch_generic.c:316
dev_watchdog+0x218/0x220
[ 287.212830] Modules linked in: mcs7830 usbnet mii snd_hda_codec_hdmi
snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal
intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel
snd_hda_codec snd_hwdep ghash_clmulni_intel snd_hda_core snd_pcm
i2c_designware_platform i2c_designware_core mei_me mei prime_numbers
i2c_hid pinctrl_sunrisepoint pinctrl_intel
[ 287.212864] CPU: 0 PID: 3165 Comm: gem_exec_suspen Tainted: G U
4.12.0-CI-CI_DRM_2829+ #1
[ 287.212865] Hardware name: Dell Inc. XPS 13 9360/093TW6, BIOS 1.3.2
01/18/2017
[ 287.212867] task: ffff8801b4084f40 task.stack: ffffc900001d8000
[ 287.212869] RIP: 0010:dev_watchdog+0x218/0x220
[ 287.212870] RSP: 0018:ffff88027e403e38 EFLAGS: 00010292
[ 287.212872] RAX: 000000000000005a RBX: 0000000000000000 RCX:
0000000000000000
[ 287.212874] RDX: 0000000000000002 RSI: ffffffff81cbcf89 RDI:
ffffffff81c9c627
[ 287.212875] RBP: ffff88027e403e68 R08: 0000000000000000 R09:
0000000000000001
[ 287.212876] R10: 0000000028e9c215 R11: 0000000000000000 R12:
ffff88026e08a848
[ 287.212877] R13: 0000000000000000 R14: ffff88026e050020 R15:
0000000000000001
[ 287.212878] FS: 00007f345056a8c0(0000) GS:ffff88027e400000(0000)
knlGS:0000000000000000
[ 287.212880] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 287.212881] CR2: 00000000008d7008 CR3: 00000001b4314000 CR4:
00000000003406f0
[ 287.212882] Call Trace:
[ 287.212883] <IRQ>
[ 287.212886] ? qdisc_rcu_free+0x40/0x40
[ 287.212888] ? qdisc_rcu_free+0x40/0x40
[ 287.212891] call_timer_fn+0x8e/0x370
[ 287.212894] ? qdisc_rcu_free+0x40/0x40
[ 287.212896] expire_timers+0x150/0x1f0
[ 287.212899] run_timer_softirq+0x7c/0x160
[ 287.212903] __do_softirq+0x116/0x4a0
[ 287.212906] irq_exit+0xa9/0xc0
[ 287.212909] smp_apic_timer_interrupt+0x38/0x50
[ 287.212912] apic_timer_interrupt+0x90/0xa0
[ 287.212914] RIP: 0010:delay_tsc+0x33/0xc0
[ 287.212916] RSP: 0018:ffffc900001dbcd8 EFLAGS: 00000286 ORIG_RAX:
ffffffffffffff10
[ 287.212918] RAX: 0000000080000000 RBX: 00000005964f23a0 RCX:
0000000000000001
[ 287.212919] RDX: 0000000080000001 RSI: ffffffff81c8e23a RDI:
00000000ffffffff
[ 287.212920] RBP: ffffc900001dbcf8 R08: 0000000000000000 R09:
0000000000000001
[ 287.212921] R10: 0000000000000000 R11: 0000000000000000 R12:
000000059633478e
[ 287.212922] R13: 0000000000249f13 R14: 0000000000000000 R15:
ffff880272eac008
[ 287.212924] </IRQ>
[ 287.212929] ? delay_tsc+0x6b/0xc0
[ 287.212932] __delay+0xa/0x10
[ 287.212934] __const_udelay+0x31/0x40
[ 287.212936] hibernation_debug_sleep+0x20/0x30
[ 287.212938] hibernation_snapshot+0x2bc/0x5f0
[ 287.212940] hibernate+0x159/0x2f0
[ 287.212943] state_store+0xe0/0xf0
[ 287.212947] kobj_attr_store+0xf/0x20
[ 287.212949] sysfs_kf_write+0x40/0x50
[ 287.212951] kernfs_fop_write+0x130/0x1b0
[ 287.212955] __vfs_write+0x23/0x120
[ 287.212957] ? rcu_read_lock_sched_held+0x75/0x80
[ 287.212959] ? rcu_sync_lockdep_assert+0x2a/0x50
[ 287.212961] ? __sb_start_write+0xfa/0x1f0
[ 287.212964] vfs_write+0xc5/0x1d0
[ 287.212966] ? trace_hardirqs_on_caller+0xe7/0x1c0
[ 287.212969] SyS_write+0x44/0xb0
[ 287.212972] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 287.212973] RIP: 0033:0x7f344ed4a4a0
[ 287.212974] RSP: 002b:00007ffef50dfaa8 EFLAGS: 00000246 ORIG_RAX:
0000000000000001
[ 287.212977] RAX: ffffffffffffffda RBX: ffffffff81470683 RCX:
00007f344ed4a4a0
[ 287.212978] RDX: 0000000000000004 RSI: 000000000041d211 RDI:
0000000000000006
[ 287.212979] RBP: ffffc900001dbf88 R08: 00000000008d6a50 R09:
0000000000000000
[ 287.212980] R10: 0000000000000000 R11: 0000000000000246 R12:
000000000041d211
[ 287.212981] R13: 0000000000000006 R14: 0000000000000000 R15:
0000000000000000
[ 287.212984] ? __this_cpu_preempt_check+0x13/0x20
[ 287.212988] Code: 63 8e 18 04 00 00 eb 93 4c 89 f7 c6 05 77 5c 77 00
01 e8 dc 7f fd ff 89 d9 48 89 c2 4c 89 f6 48 c7 c7 18 f4 cf 81 e8 f1 c4
9d ff <0f> ff eb c3 0f 1f 40 00 48 c7 47 08 00 00 00 00 55 48 c7 07 00
[ 287.213051] ---[ end trace b6016dcc7544a681 ]---
This is caught while running the intel-gpu-tools test named
'igt@gem_exec_suspend@basic-s4-devices' on the following machines:
- Intel Kaby Lake-R RVP: Failure rate 123/135 run(s) (91%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2828/fi-kbl-r/igt@gem_exec_suspend@basic-s4-devices.html
- Intel Kaby Lake i7-7560u: Failure rate 196/305 run(s) (64%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2827/fi-kbl-7560u/igt@gem_exec_suspend@basic-s4-devices.html
- Intel Skylake i7-6600u: Failure rate 23/75 run(s) (30%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2824/fi-skl-6600u/igt@gem_exec_suspend@basic-s4-devices.html
- Intel Sandy Bridge i7-2600: Failure rate 10/293 run(s) (3%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2816/fi-snb-2600/igt@gem_exec_suspend@basic-s4-devices.html
We have plenty of other machines that do not trigger this warning at all.
The bug used to live in fd.o's bugzilla, but it had no business being
there: https://bugs.freedesktop.org/show_bug.cgi?id=100125
Let me know if I can help in some ways.
=====================================================================
Lakshmi.
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH] ethernet:netronome:nfp:move spin_lock_bh to spin_lock in tasklet
From: Jakub Kicinski @ 2018-09-07 16:36 UTC (permalink / raw)
To: jun qian
Cc: Dirk van der Merwe, Daniel Borkmann, Quentin Monnet, oss-drivers,
netdev, linux-kernel
In-Reply-To: <20180907162117.47361-1-hangdianqj@163.com>
On Fri, 7 Sep 2018 09:21:17 -0700, jun qian wrote:
> As you are already in a tasklet, it is unnecessary to call spin_lock_bh.
>
> Signed-off-by: jun qian <hangdianqj@163.com>
FWIW you should put spaces after the colons. It's generally a good
practice to look at the prefix previous authors used for a given piece
of code with
git log -- $file_path
This would be a better subject:
nfp: replace spin_lock_bh with spin_lock in tasklet callback
^ permalink raw reply
* Re: [PATCH] ethernet:netronome:nfp:move spin_lock_bh to spin_lock in tasklet
From: Jakub Kicinski @ 2018-09-07 16:33 UTC (permalink / raw)
To: jun qian
Cc: Dirk van der Merwe, Daniel Borkmann, Quentin Monnet, oss-drivers,
netdev, linux-kernel
In-Reply-To: <20180907162117.47361-1-hangdianqj@163.com>
On Fri, 7 Sep 2018 09:21:17 -0700, jun qian wrote:
> As you are already in a tasklet, it is unnecessary to call spin_lock_bh.
>
> Signed-off-by: jun qian <hangdianqj@163.com>
We had more of those at some point, I wonder if you(r tool) can find
them :)
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Network device suspend/resume
From: Lakshmi @ 2018-09-07 11:49 UTC (permalink / raw)
To: Oliver Neukum, netdev
Hi,
I am bringing kernel bugzilla bug here
https://bugzilla.kernel.org/show_bug.cgi?id=196399
This issue occured 2 months ago and we didn't see this again. Wondering
if that appears again. Can you confirm if there is any bug in network
suspend/resume in the case.
One more instance here
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4376/fi-skl-6600u/igt@gem_exec_suspend@basic-s4-devices.html
================================================================================
Network device is
0b95:7720 ASIX Electronics Corp. AX88772
USB network card
driver seems to be usbnet
=================================================================================
Bug 196399:
We have found out that since at least 4.11-rc1, some machines in the
Intel GFX CI lab have been generating the following warning when
suspending to s4 (suspend to disk):
[ 287.212825] ------------[ cut here ]------------
[ 287.212829] WARNING: CPU: 0 PID: 3165 at net/sched/sch_generic.c:316
dev_watchdog+0x218/0x220
[ 287.212830] Modules linked in: mcs7830 usbnet mii snd_hda_codec_hdmi
snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal
intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel
snd_hda_codec snd_hwdep ghash_clmulni_intel snd_hda_core snd_pcm
i2c_designware_platform i2c_designware_core mei_me mei prime_numbers
i2c_hid pinctrl_sunrisepoint pinctrl_intel
[ 287.212864] CPU: 0 PID: 3165 Comm: gem_exec_suspen Tainted: G U
4.12.0-CI-CI_DRM_2829+ #1
[ 287.212865] Hardware name: Dell Inc. XPS 13 9360/093TW6, BIOS 1.3.2
01/18/2017
[ 287.212867] task: ffff8801b4084f40 task.stack: ffffc900001d8000
[ 287.212869] RIP: 0010:dev_watchdog+0x218/0x220
[ 287.212870] RSP: 0018:ffff88027e403e38 EFLAGS: 00010292
[ 287.212872] RAX: 000000000000005a RBX: 0000000000000000 RCX:
0000000000000000
[ 287.212874] RDX: 0000000000000002 RSI: ffffffff81cbcf89 RDI:
ffffffff81c9c627
[ 287.212875] RBP: ffff88027e403e68 R08: 0000000000000000 R09:
0000000000000001
[ 287.212876] R10: 0000000028e9c215 R11: 0000000000000000 R12:
ffff88026e08a848
[ 287.212877] R13: 0000000000000000 R14: ffff88026e050020 R15:
0000000000000001
[ 287.212878] FS: 00007f345056a8c0(0000) GS:ffff88027e400000(0000)
knlGS:0000000000000000
[ 287.212880] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 287.212881] CR2: 00000000008d7008 CR3: 00000001b4314000 CR4:
00000000003406f0
[ 287.212882] Call Trace:
[ 287.212883] <IRQ>
[ 287.212886] ? qdisc_rcu_free+0x40/0x40
[ 287.212888] ? qdisc_rcu_free+0x40/0x40
[ 287.212891] call_timer_fn+0x8e/0x370
[ 287.212894] ? qdisc_rcu_free+0x40/0x40
[ 287.212896] expire_timers+0x150/0x1f0
[ 287.212899] run_timer_softirq+0x7c/0x160
[ 287.212903] __do_softirq+0x116/0x4a0
[ 287.212906] irq_exit+0xa9/0xc0
[ 287.212909] smp_apic_timer_interrupt+0x38/0x50
[ 287.212912] apic_timer_interrupt+0x90/0xa0
[ 287.212914] RIP: 0010:delay_tsc+0x33/0xc0
[ 287.212916] RSP: 0018:ffffc900001dbcd8 EFLAGS: 00000286 ORIG_RAX:
ffffffffffffff10
[ 287.212918] RAX: 0000000080000000 RBX: 00000005964f23a0 RCX:
0000000000000001
[ 287.212919] RDX: 0000000080000001 RSI: ffffffff81c8e23a RDI:
00000000ffffffff
[ 287.212920] RBP: ffffc900001dbcf8 R08: 0000000000000000 R09:
0000000000000001
[ 287.212921] R10: 0000000000000000 R11: 0000000000000000 R12:
000000059633478e
[ 287.212922] R13: 0000000000249f13 R14: 0000000000000000 R15:
ffff880272eac008
[ 287.212924] </IRQ>
[ 287.212929] ? delay_tsc+0x6b/0xc0
[ 287.212932] __delay+0xa/0x10
[ 287.212934] __const_udelay+0x31/0x40
[ 287.212936] hibernation_debug_sleep+0x20/0x30
[ 287.212938] hibernation_snapshot+0x2bc/0x5f0
[ 287.212940] hibernate+0x159/0x2f0
[ 287.212943] state_store+0xe0/0xf0
[ 287.212947] kobj_attr_store+0xf/0x20
[ 287.212949] sysfs_kf_write+0x40/0x50
[ 287.212951] kernfs_fop_write+0x130/0x1b0
[ 287.212955] __vfs_write+0x23/0x120
[ 287.212957] ? rcu_read_lock_sched_held+0x75/0x80
[ 287.212959] ? rcu_sync_lockdep_assert+0x2a/0x50
[ 287.212961] ? __sb_start_write+0xfa/0x1f0
[ 287.212964] vfs_write+0xc5/0x1d0
[ 287.212966] ? trace_hardirqs_on_caller+0xe7/0x1c0
[ 287.212969] SyS_write+0x44/0xb0
[ 287.212972] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 287.212973] RIP: 0033:0x7f344ed4a4a0
[ 287.212974] RSP: 002b:00007ffef50dfaa8 EFLAGS: 00000246 ORIG_RAX:
0000000000000001
[ 287.212977] RAX: ffffffffffffffda RBX: ffffffff81470683 RCX:
00007f344ed4a4a0
[ 287.212978] RDX: 0000000000000004 RSI: 000000000041d211 RDI:
0000000000000006
[ 287.212979] RBP: ffffc900001dbf88 R08: 00000000008d6a50 R09:
0000000000000000
[ 287.212980] R10: 0000000000000000 R11: 0000000000000246 R12:
000000000041d211
[ 287.212981] R13: 0000000000000006 R14: 0000000000000000 R15:
0000000000000000
[ 287.212984] ? __this_cpu_preempt_check+0x13/0x20
[ 287.212988] Code: 63 8e 18 04 00 00 eb 93 4c 89 f7 c6 05 77 5c 77 00
01 e8 dc 7f fd ff 89 d9 48 89 c2 4c 89 f6 48 c7 c7 18 f4 cf 81 e8 f1 c4
9d ff <0f> ff eb c3 0f 1f 40 00 48 c7 47 08 00 00 00 00 55 48 c7 07 00
[ 287.213051] ---[ end trace b6016dcc7544a681 ]---
This is caught while running the intel-gpu-tools test named
'igt@gem_exec_suspend@basic-s4-devices' on the following machines:
- Intel Kaby Lake-R RVP: Failure rate 123/135 run(s) (91%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2828/fi-kbl-r/igt@gem_exec_suspend@basic-s4-devices.html
- Intel Kaby Lake i7-7560u: Failure rate 196/305 run(s) (64%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2827/fi-kbl-7560u/igt@gem_exec_suspend@basic-s4-devices.html
- Intel Skylake i7-6600u: Failure rate 23/75 run(s) (30%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2824/fi-skl-6600u/igt@gem_exec_suspend@basic-s4-devices.html
- Intel Sandy Bridge i7-2600: Failure rate 10/293 run(s) (3%), last
occurence:
https://intel-gfx-ci.01.org/CI/CI_DRM_2816/fi-snb-2600/igt@gem_exec_suspend@basic-s4-devices.html
We have plenty of other machines that do not trigger this warning at all.
The bug used to live in fd.o's bugzilla, but it had no business being
there: https://bugs.freedesktop.org/show_bug.cgi?id=100125
Let me know if I can help in some ways.
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH v2 net-next 3/4] net: make listified RX functions return number of good packets
From: Eric Dumazet @ 2018-09-07 11:43 UTC (permalink / raw)
To: Edward Cree, Eric Dumazet, davem; +Cc: linux-net-drivers, netdev
In-Reply-To: <e184f2c1-fe28-ad6e-460d-950d8a363852@solarflare.com>
On 09/07/2018 03:44 AM, Edward Cree wrote:
>
> Any suggestions on how to construct a test that will?
>
Say 50 concurrent netperf -t TCP_RR -- -r 8000,8000
This way you have a mix of GRO-candidates, and non GRO ones (pure acks)
GRO sizes would be reasonable (not full size GRO packets).
^ permalink raw reply
* [PATCH] ethernet:netronome:nfp:move spin_lock_bh to spin_lock in tasklet
From: jun qian @ 2018-09-07 16:21 UTC (permalink / raw)
To: Jakub Kicinski, Dirk van der Merwe, Daniel Borkmann,
Quentin Monnet
Cc: oss-drivers, netdev, linux-kernel, jun qian
As you are already in a tasklet, it is unnecessary to call spin_lock_bh.
Signed-off-by: jun qian <hangdianqj@163.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index a8b9fbab5f73..084c983ec3c2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2075,10 +2075,10 @@ static void nfp_ctrl_poll(unsigned long arg)
{
struct nfp_net_r_vector *r_vec = (void *)arg;
- spin_lock_bh(&r_vec->lock);
+ spin_lock(&r_vec->lock);
nfp_net_tx_complete(r_vec->tx_ring, 0);
__nfp_ctrl_tx_queued(r_vec);
- spin_unlock_bh(&r_vec->lock);
+ spin_unlock(&r_vec->lock);
nfp_ctrl_rx(r_vec);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 11/11] vhost_net: batch submitting XDP buffers to underlayer sockets
From: Michael S. Tsirkin @ 2018-09-07 16:13 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <ffb802e6-1505-e01e-f6d5-11cde8dace9b@redhat.com>
On Fri, Sep 07, 2018 at 03:41:52PM +0800, Jason Wang wrote:
> > > @@ -556,10 +667,14 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock)
> > > size_t len, total_len = 0;
> > > int err;
> > > int sent_pkts = 0;
> > > + bool bulking = (sock->sk->sk_sndbuf == INT_MAX);
> > What does bulking mean?
>
> The name is misleading, it means whether we can do batching. For simplicity,
> I disable batching is sndbuf is not INT_MAX.
But what does batching have to do with sndbuf?
> > > for (;;) {
> > > bool busyloop_intr = false;
> > > + if (nvq->done_idx == VHOST_NET_BATCH)
> > > + vhost_tx_batch(net, nvq, sock, &msg);
> > > +
> > > head = get_tx_bufs(net, nvq, &msg, &out, &in, &len,
> > > &busyloop_intr);
> > > /* On error, stop handling until the next kick. */
> > > @@ -577,14 +692,34 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock)
> > > break;
> > > }
> > > - vq->heads[nvq->done_idx].id = cpu_to_vhost32(vq, head);
> > > - vq->heads[nvq->done_idx].len = 0;
> > > -
> > > total_len += len;
> > > - if (tx_can_batch(vq, total_len))
> > > - msg.msg_flags |= MSG_MORE;
> > > - else
> > > - msg.msg_flags &= ~MSG_MORE;
> > > +
> > > + /* For simplicity, TX batching is only enabled if
> > > + * sndbuf is unlimited.
> > What if sndbuf changes while this processing is going on?
>
> We will get the correct sndbuf in the next run of handle_tx(). I think this
> is safe.
If it's safe why bother with special-casing INT_MAX?
--
MST
^ permalink raw reply
* [PATCH v3 1/2] net: ethernet: i40e: fix build error
From: Wang Dongsheng @ 2018-09-07 11:19 UTC (permalink / raw)
To: jeffrey.t.kirsher, sergei.shtylyov
Cc: jacob.e.keller, davem, intel-wired-lan, netdev, linux-kernel,
Wang Dongsheng
In-Reply-To: <1536319175-3660-1-git-send-email-dongsheng.wang@hxt-semitech.com>
Remove "inline" from __i40e_add_stat_strings and move the function.
In file included from
drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
__i40e_add_stat_string:
drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
function __i40e_add_stat_strings can never be inlined because it uses
variable argument lists
static inline void __i40e_add_stat_strings(u8 **p, const struct
i40e_stats stats[],
Tested on: x86_64, make ARCH=i386
Modules section .text:
i40e: 00019380 <__i40e_add_stat_strings>:
i40evf: 00006b00 <__i40e_add_stat_strings>:
Buildin section .text:
i40e: c351ca60 <__i40e_add_stat_strings>:
i40evf: c354f2c0 <__i40e_add_stat_strings>:
Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
---
V3: add static
V2: Move function
---
.../net/ethernet/intel/i40e/i40e_ethtool.c | 24 ++++++++++++++++++
.../ethernet/intel/i40e/i40e_ethtool_stats.h | 25 ++-----------------
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index d7d3974beca2..34121a72f2e7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
"ethtool stats count mismatch!");
}
+/**
+ * __i40e_add_stat_strings - copy stat strings into ethtool buffer
+ * @p: ethtool supplied buffer
+ * @stats: stat definitions array
+ * @size: size of the stats array
+ *
+ * Format and copy the strings described by stats into the buffer pointed at
+ * by p.
+ **/
+static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+ const unsigned int size, ...)
+{
+ unsigned int i;
+
+ for (i = 0; i < size; i++) {
+ va_list args;
+
+ va_start(args, size);
+ vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
+ *p += ETH_GSTRING_LEN;
+ va_end(args);
+ }
+}
+
/**
* i40e_get_stat_strings - copy stat strings into supplied buffer
* @netdev: the netdev to collect strings for
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
index bba1cb0b658f..553b0d720839 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
@@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring *ring)
*data += size;
}
-/**
- * __i40e_add_stat_strings - copy stat strings into ethtool buffer
- * @p: ethtool supplied buffer
- * @stats: stat definitions array
- * @size: size of the stats array
- *
- * Format and copy the strings described by stats into the buffer pointed at
- * by p.
- **/
-static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
- const unsigned int size, ...)
-{
- unsigned int i;
-
- for (i = 0; i < size; i++) {
- va_list args;
-
- va_start(args, size);
- vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
- *p += ETH_GSTRING_LEN;
- va_end(args);
- }
-}
+static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+ const unsigned int size, ...);
/**
* 40e_add_stat_strings - copy stat strings into ethtool buffer
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] net: ethernet: i40evf: fix underlying build error
From: Sergei Shtylyov @ 2018-09-07 15:33 UTC (permalink / raw)
To: Wang Dongsheng, jeffrey.t.kirsher
Cc: jacob.e.keller, davem, intel-wired-lan, netdev, linux-kernel
In-Reply-To: <1536319175-3660-3-git-send-email-dongsheng.wang@hxt-semitech.com>
On 09/07/2018 02:19 PM, Wang Dongsheng wrote:
> Can't have non-inline function in a header file.
> There is a risk of "Multiple definition" from cross-including.
>
> Tested on: x86_64, make ARCH=i386
>
> Modules section .text:
> i40e: 00019380 <__i40e_add_stat_strings>:
> i40evf: 00006b00 <__i40e_add_stat_strings>:
>
> Buildin section .text:
> i40e: c351ca60 <__i40e_add_stat_strings>:
> i40evf: c354f2c0 <__i40e_add_stat_strings>:
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
> ---
> V3: add static
> ---
> .../intel/i40evf/i40e_ethtool_stats.h | 23 +-----------------
> .../ethernet/intel/i40evf/i40evf_ethtool.c | 24 +++++++++++++++++++
> 2 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> index 60b595dd8c39..62ab67a77753 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring *ring)
> *data += size;
> }
>
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
There's no point to keeping *static* function in the header file (unless it's
also *inline*).
> - const unsigned int size, ...)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < size; i++) {
> - va_list args;
> -
> - va_start(args, size);
> - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> - *p += ETH_GSTRING_LEN;
> - va_end(args);
> - }
> -}
> + const unsigned int size, ...);
>
> /**
> * 40e_add_stat_strings - copy stat strings into ethtool buffer
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 net-next 0/4] net: batched receive in GRO path
From: Edward Cree @ 2018-09-07 10:51 UTC (permalink / raw)
To: Eric Dumazet, davem; +Cc: linux-net-drivers, netdev
In-Reply-To: <01bc9bf5-1780-2650-958f-961bd24b8c26@gmail.com>
On 07/09/18 03:32, Eric Dumazet wrote:
> Your performance numbers are not convincing, since TCP stream test should
> get nominal GRO gains.
I'm not quite sure what you mean here, could you explain a bit more?
> Adding this complexity and icache pressure needs more experimental results.
>
> What about RPC workloads (eg 100 concurrent netperf -t TCP_RR -- -r 8000,8000 )
I'll try that. Any other tests that would be worthwhile?
-Ed
^ permalink raw reply
* RE: linux-next: build failure after merge of the net-next tree
From: Keller, Jacob E @ 2018-09-07 15:30 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Kirsher, Jeffrey T, Bowers, AndrewX
In-Reply-To: <20180907102055.001777b8@canb.auug.org.au>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Stephen Rothwell
> Sent: Thursday, September 06, 2018 5:21 PM
> To: David Miller <davem@davemloft.net>; Networking
> <netdev@vger.kernel.org>
> Cc: Linux-Next Mailing List <linux-next@vger.kernel.org>; Linux Kernel Mailing
> List <linux-kernel@vger.kernel.org>; Keller, Jacob E <jacob.e.keller@intel.com>;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; Bowers, AndrewX
> <andrewx.bowers@intel.com>
> Subject: Re: linux-next: build failure after merge of the net-next tree
>
> Hi all,
>
> On Mon, 3 Sep 2018 09:47:02 +1000 Stephen Rothwell <sfr@canb.auug.org.au>
> wrote:
> >
> > After merging the net-next tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> >
> > In file included from drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:
> > drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
> '__i40e_add_stat_strings':
> > drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error: function
> '__i40e_add_stat_strings' can never be inlined because it uses variable argument
> lists
> > static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats
> stats[],
> > ^~~~~~~~~~~~~~~~~~~~~~~
> >
> > Caused by commit
> >
> > 8fd75c58a09a ("i40e: move ethtool stats boiler plate code to
> i40e_ethtool_stats.h")
> >
> > It is not clear this patch has any value anyway as the moved functions
> > are only used in the file they were moved from.
> >
> > I reverted that commit for today.
> >
> > The same problem would exist in drivers/net/ethernet/intel/i40evf (where
> > a lot of code is duplicated from drivers/net/ethernet/intel/i40e) except
> > that this function is not declared inline there.
> > Luckily, i40e_ethtool_stats.h is only included my one file
> > drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c, otherwise there
> > would be multiple copies of __i40e_add_stat_strings().
> >
> > Surely there is some scope for factoring out some common code between
> > these two drivers?
>
> Ping?
>
> --
> Cheers,
> Stephen Rothwell
There's some discussion about this going on in the intel-wired-lan mailing list.
Thanks,
Jake
^ permalink raw reply
* RE: [PATCH v3 1/2] net: ethernet: i40e: fix build error
From: Keller, Jacob E @ 2018-09-07 15:27 UTC (permalink / raw)
To: Wang Dongsheng, Kirsher, Jeffrey T,
sergei.shtylyov@cogentembedded.com
Cc: davem@davemloft.net, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1536319175-3660-2-git-send-email-dongsheng.wang@hxt-semitech.com>
> -----Original Message-----
> From: Wang Dongsheng [mailto:dongsheng.wang@hxt-semitech.com]
> Sent: Friday, September 07, 2018 4:20 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sergei.shtylyov@cogentembedded.com
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wang Dongsheng <dongsheng.wang@hxt-
> semitech.com>
> Subject: [PATCH v3 1/2] net: ethernet: i40e: fix build error
>
> Remove "inline" from __i40e_add_stat_strings and move the function.
>
> In file included from
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
> __i40e_add_stat_string:
> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
> function __i40e_add_stat_strings can never be inlined because it uses
> variable argument lists
> static inline void __i40e_add_stat_strings(u8 **p, const struct
> i40e_stats stats[],
>
> Tested on: x86_64, make ARCH=i386
>
> Modules section .text:
> i40e: 00019380 <__i40e_add_stat_strings>:
> i40evf: 00006b00 <__i40e_add_stat_strings>:
>
> Buildin section .text:
> i40e: c351ca60 <__i40e_add_stat_strings>:
> i40evf: c354f2c0 <__i40e_add_stat_strings>:
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
> ---
> V3: add static
> V2: Move function
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
This patch fixes the build issue, and is fine with me.
Thanks,
Jake
> ---
> .../net/ethernet/intel/i40e/i40e_ethtool.c | 24 ++++++++++++++++++
> .../ethernet/intel/i40e/i40e_ethtool_stats.h | 25 ++-----------------
> 2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index d7d3974beca2..34121a72f2e7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device
> *netdev,
> "ethtool stats count mismatch!");
> }
>
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> + const unsigned int size, ...)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + va_list args;
> +
> + va_start(args, size);
> + vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> + *p += ETH_GSTRING_LEN;
> + va_end(args);
> + }
> +}
> +
> /**
> * i40e_get_stat_strings - copy stat strings into supplied buffer
> * @netdev: the netdev to collect strings for
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> index bba1cb0b658f..553b0d720839 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring
> *ring)
> *data += size;
> }
>
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> - const unsigned int size, ...)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < size; i++) {
> - va_list args;
> -
> - va_start(args, size);
> - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> - *p += ETH_GSTRING_LEN;
> - va_end(args);
> - }
> -}
> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> + const unsigned int size, ...);
>
> /**
> * 40e_add_stat_strings - copy stat strings into ethtool buffer
> --
> 2.18.0
^ permalink raw reply
* RE: [PATCH v3 2/2] net: ethernet: i40evf: fix underlying build error
From: Wyborny, Carolyn @ 2018-09-07 15:26 UTC (permalink / raw)
To: Wang, Dongsheng, Kirsher, Jeffrey T,
sergei.shtylyov@cogentembedded.com
Cc: Keller, Jacob E, davem@davemloft.net,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <77898840b49847a7a835b20b383e21a2@HXTBJIDCEMVIW02.hxtcorp.net>
> -----Original Message-----
> From: Wang, Dongsheng [mailto:dongsheng.wang@hxt-semitech.com]
> Sent: Friday, September 07, 2018 5:34 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sergei.shtylyov@cogentembedded.com
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net;
> intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wyborny, Carolyn <carolyn.wyborny@intel.com>
> Subject: Re: [PATCH v3 2/2] net: ethernet: i40evf: fix underlying build error
>
> Hello Jacob,
>
> Since Carolyn' team is working this, I think we don't need this patch
> anymore because this header file is only for ethtool.c.
> [..]
Hello Dongsheng,
The commonization effort we're working on is prioritizing our newest drivers. The i40e work is still being scoped, so we should fix this problem as needed now and not wait.
I apologize for any miscommunication. Was trying to let people know that we aware of the issue and are trying to make progress in that direction.
Thanks,
Carolyn
^ permalink raw reply
* Re: [PATCH v2 net-next 3/4] net: make listified RX functions return number of good packets
From: Edward Cree @ 2018-09-07 10:44 UTC (permalink / raw)
To: Eric Dumazet, davem; +Cc: linux-net-drivers, netdev
In-Reply-To: <2ed84271-06da-b5c2-3f23-940357880002@gmail.com>
On 07/09/18 03:27, Eric Dumazet wrote:
> On 09/06/2018 07:26 AM, Edward Cree wrote:
>> Signed-off-by: Edward Cree <ecree@solarflare.com>
> Lack of changelog here ?
>
> I do not know what is a good packet.
The comment on netif_receive_skb_list() defines this as "skbs for which
netif_receive_skb() would have returned %NET_RX_SUCCESS". But I shall put
that into the changelog as well.
> You are adding a lot of conditional expressions, that cpu
> will mispredict quite often.
I don't see an alternative, since this is needed by patch #4 and I daresay
there are other drivers that will also want to get NET_RX_SUCCESS-like
information (possibly from regular receives as well as GRO; I'm not quite
sure why sfc only cares about the latter).
> Typical micro benchmarks wont really notice.
Any suggestions on how to construct a test that will?
^ permalink raw reply
* [PATCH net-next] cxgb4: impose mandatory VLAN usage when non-zero TAG ID
From: Ganesh Goudar @ 2018-09-07 10:29 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, dt, Casey Leedom, Ganesh Goudar
From: Casey Leedom <leedom@chelsio.com>
When a non-zero VLAN Tag ID is passed to t4_set_vlan_acl()
then impose mandatory VLAN Usage with that VLAN ID.
I.e any other VLAN ID should result in packets getting
dropped.
Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 3 +++
drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 5fe5d16..6f1bd7b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -10210,6 +10210,9 @@ int t4_set_vlan_acl(struct adapter *adap, unsigned int mbox, unsigned int vf,
vlan_cmd.en_to_len16 = cpu_to_be32(enable | FW_LEN16(vlan_cmd));
/* Drop all packets that donot match vlan id */
vlan_cmd.dropnovlan_fm = FW_ACL_VLAN_CMD_FM_F;
+ vlan_cmd.dropnovlan_fm = (enable
+ ? (FW_ACL_VLAN_CMD_DROPNOVLAN_F |
+ FW_ACL_VLAN_CMD_FM_F) : 0);
if (enable != 0) {
vlan_cmd.nvlan = 1;
vlan_cmd.vlanid[0] = cpu_to_be16(vlan);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
index 5dc6c41..6d2bc87 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
@@ -2464,6 +2464,7 @@ struct fw_acl_vlan_cmd {
#define FW_ACL_VLAN_CMD_DROPNOVLAN_S 7
#define FW_ACL_VLAN_CMD_DROPNOVLAN_V(x) ((x) << FW_ACL_VLAN_CMD_DROPNOVLAN_S)
+#define FW_ACL_VLAN_CMD_DROPNOVLAN_F FW_ACL_VLAN_CMD_DROPNOVLAN_V(1U)
#define FW_ACL_VLAN_CMD_FM_S 6
#define FW_ACL_VLAN_CMD_FM_M 0x1
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] PCI: Reprogram bridge prefetch registers on resume
From: Peter Wu @ 2018-09-07 15:05 UTC (permalink / raw)
To: Daniel Drake
Cc: mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w,
nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
keith.busch-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
linux-6IF/jdPJHihWk0Htik3J/w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20180907053614.6540-1-drake-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
On Fri, Sep 07, 2018 at 01:36:14PM +0800, Daniel Drake wrote:
<..>
> Thomas Martitz reports that this workaround also solves an issue where
> the AMD Radeon Polaris 10 GPU on the HP Zbook 14u G5 is unresponsive
> after S3 suspend/resume.
Where was this claimed? It is not stated in the linked bug:
(https://bugs.freedesktop.org/show_bug.cgi?id=105760
> On resume, reprogram the PCI bridge prefetch registers, including the
> magic register mentioned above.
>
> This matches Win10 behaviour, which also rewrites these registers
> during S3 resume (checked with qemu tracing).
Windows 10 unconditionally rewrites these registers (BAR, I/O Base +
Limit, Memory Base + Limit, etc. from top to bottom), see annotations:
https://www.spinics.net/lists/linux-pci/msg75856.html
Linux has a generic "restore" operation that works backwards from the
end of the PCI config space to the beginning, see
pci_restore_config_space. Do you have a dmesg where you see the
"restoring config space at offset" messages?
Would it be reasonable to unconditionally write these registers in
pci_restore_config_dword, like Windows does?
Kind regards,
Peter
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply
* Re: [PATCH 3/4] of: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-09-07 14:58 UTC (permalink / raw)
To: Thierry Reding
Cc: Frank Rowand, devicetree, linux-kernel@vger.kernel.org,
Andrew Lunn, Florian Fainelli, netdev
In-Reply-To: <20180907122928.GA5821@ulmo>
On Fri, Sep 7, 2018 at 7:29 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Tue, Aug 28, 2018 at 10:52:53AM -0500, Rob Herring wrote:
> > In preparation to remove the node name pointer from struct device_node,
> > convert printf users to use the %pOFn format specifier.
> >
> > Cc: Frank Rowand <frowand.list@gmail.com>
> > Cc: Andrew Lunn <andrew@lunn.ch>
> > Cc: Florian Fainelli <f.fainelli@gmail.com>
> > Cc: devicetree@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > drivers/of/device.c | 4 ++--
> > drivers/of/of_mdio.c | 12 ++++++------
> > drivers/of/of_numa.c | 4 ++--
> > drivers/of/overlay.c | 4 ++--
> > drivers/of/platform.c | 8 ++++----
> > drivers/of/unittest.c | 12 ++++++------
> > 6 files changed, 22 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index 5957cd4fa262..daa075d87317 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > @@ -219,7 +219,7 @@ static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len
> > return -ENODEV;
> >
> > /* Name & Type */
> > - csize = snprintf(str, len, "of:N%sT%s", dev->of_node->name,
> > + csize = snprintf(str, len, "of:N%pOFnT%s", dev->of_node,
> > dev->of_node->type);
> > tsize = csize;
> > len -= csize;
>
> This seems to cause the modalias to be improperly constructed. As a
> consequence, automatic module loading at boot time is now broken. I
> think the reason why this fails is because vsnprintf() will skip all
> alpha-numeric characters after a call to pointer(). Presumably this
> is meant to be a generic way of skipping whatever specifiers we throw
> at it.
>
> Unfortunately for the case of OF modaliases, this means that the 'T'
> character gets eaten, so we end up with something like this:
>
> # udevadm info /sys/bus/platform/devices/54200000.dc
> [...]
> E: MODALIAS=of:Ndc<NULL>Cnvidia,tegra124-dc
> [...]
>
> instead of this:
>
> # udevadm info /sys/bus/platform/devices/54200000.dc
> [...]
> E: MODALIAS=of:NdcT<NULL>Cnvidia,tegra124-dc
> [...]
Oops. Thanks for finding this.
> Everything is back to normal if I revert this patch. However, since
> that's obviously not what we want, I think perhaps what we need is a
> way for pointer() (and its implementations) to report back how many
> characters in the format string it consumed so that we can support
> these kinds of back-to-back strings.
I don't think we can change it because if I have something like
%pOFMoreWords and then later on want to add 'M' as a new modifier we'd
break any existing cases with 'M'. Of course, I could search the tree
for that case and find unused characters, but that seems fragile
(though silently throwing away the characters does too).
> If nobody else has the time I can look into coding up a fix, but in the
> meantime it might be best to back this one out until we can handle the
> OF modalias format string.
There's an easy fix though. Just replace the 'T' with a '%c'.
I found one other case in the clock code and one in soundbus (which I
haven't posted yet).
Rob
^ permalink raw reply
* Re: kernels > v4.12 oops/crash with ipsec-traffic: bisected to b838d5e1c5b6e57b10ec8af2268824041e3ea911: ipv4: mark DST_NOGC and remove the operation of dst_free()
From: Wolfgang Walter @ 2018-09-07 9:53 UTC (permalink / raw)
To: Steffen Klassert; +Cc: netdev, Wei Wang
In-Reply-To: <20180831065024.GG23674@gauss3.secunet.de>
Hello,
didn't respond as I've been on vacation.
Am Freitag, 31. August 2018, 08:50:24 schrieb Steffen Klassert:
> On Thu, Aug 30, 2018 at 08:53:50PM +0200, Wolfgang Walter wrote:
> > Hello,
> >
> > kernels > 4.12 do not work on one of our main routers. They crash as soon
> > as ipsec-tunnels are configured and ipsec-traffic actually flows.
>
> Can you please send the backtrace of this crash?
>
I'll try today. The oops quickly disappears because other problems arising
from it pop up. The machine crashes and no logs are logged. I try to make foto
or try to log to the serial console.
At the moment I only see that there is xfrm_???? stuff in the call trace as
xfrm_lookup, xfrm_route_????, and it is while routing a packet.
With later kernels (4.18.5) the machine seems to crash without a call trace on
console.
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ 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