* [PATCH ipsec 2/2] xfrm interface: ifname may be wrong in logs
From: Nicolas Dichtel @ 2019-07-10 7:45 UTC (permalink / raw)
To: steffen.klassert, davem; +Cc: netdev, Nicolas Dichtel
In-Reply-To: <20190710074536.7505-1-nicolas.dichtel@6wind.com>
The ifname is copied when the interface is created, but is never updated
later. In fact, this property is used only in one error message, where the
netdevice pointer is available, thus let's use it.
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/net/xfrm.h | 1 -
net/xfrm/xfrm_interface.c | 10 +---------
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index a2907873ed56..287e39753d94 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -988,7 +988,6 @@ static inline void xfrm_dst_destroy(struct xfrm_dst *xdst)
void xfrm_dst_ifdown(struct dst_entry *dst, struct net_device *dev);
struct xfrm_if_parms {
- char name[IFNAMSIZ]; /* name of XFRM device */
int link; /* ifindex of underlying L2 interface */
u32 if_id; /* interface identifyer */
};
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index dfa5aebdec57..a60d391f7ebe 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -145,8 +145,6 @@ static int xfrmi_create(struct net_device *dev)
if (err < 0)
goto out;
- strcpy(xi->p.name, dev->name);
-
dev_hold(dev);
xfrmi_link(xfrmn, xi);
@@ -294,7 +292,7 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
if (tdev == dev) {
stats->collisions++;
net_warn_ratelimited("%s: Local routing loop detected!\n",
- xi->p.name);
+ dev->name);
goto tx_err_dst_release;
}
@@ -638,12 +636,6 @@ static int xfrmi_newlink(struct net *src_net, struct net_device *dev,
int err;
xfrmi_netlink_parms(data, &p);
-
- if (!tb[IFLA_IFNAME])
- return -EINVAL;
-
- nla_strlcpy(p.name, tb[IFLA_IFNAME], IFNAMSIZ);
-
xi = xfrmi_locate(net, &p);
if (xi)
return -EEXIST;
--
2.21.0
^ permalink raw reply related
* [PATCH] ipvs: remove unnecessary space
From: yangxingwu @ 2019-07-10 7:45 UTC (permalink / raw)
To: wensong
Cc: horms, ja, pablo, kadlec, fw, davem, netdev, lvs-devel,
netfilter-devel, coreteam, linux-kernel, yangxingwu
this patch removes the extra space.
Signed-off-by: yangxingwu <xingwu.yang@gmail.com>
---
net/netfilter/ipvs/ip_vs_mh.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
index 94d9d34..98e358e 100644
--- a/net/netfilter/ipvs/ip_vs_mh.c
+++ b/net/netfilter/ipvs/ip_vs_mh.c
@@ -174,8 +174,8 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s,
return 0;
}
- table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
- sizeof(unsigned long), GFP_KERNEL);
+ table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
+ sizeof(unsigned long), GFP_KERNEL);
if (!table)
return -ENOMEM;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next,v4 12/12] netfilter: nf_tables: add hardware offload support
From: Jiri Pirko @ 2019-07-10 7:52 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
maxime.chevallier, cphealy, phil, netfilter-devel
In-Reply-To: <20190709205550.3160-13-pablo@netfilter.org>
Tue, Jul 09, 2019 at 10:55:50PM CEST, pablo@netfilter.org wrote:
[...]
>+ if (!dev || !dev->netdev_ops->ndo_setup_tc)
Why didn't you rename ndo_setup_tc? I put a comment about it in the
previous version thread. I expect that you can at least write why it is
a wrong idea.
[...]
^ permalink raw reply
* Re: [PATCH] vhost: fix null pointer dereference in vhost_del_umem_range
From: Denis Kirjanov @ 2019-07-10 7:56 UTC (permalink / raw)
To: David Miller; +Cc: mst, jasowang, kvm, netdev
In-Reply-To: <20190709.125850.2133620086434576103.davem@davemloft.net>
On 7/9/19, David Miller <davem@davemloft.net> wrote:
> From: Denis Kirjanov <kda@linux-powerpc.org>
> Date: Tue, 9 Jul 2019 13:42:51 +0200
>
>> @@ -962,7 +962,8 @@ static void vhost_del_umem_range(struct vhost_umem
>> *umem,
>>
>> while ((node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
>> start, end)))
>> - vhost_umem_free(umem, node);
>> + if (node)
>> + vhost_umem_free(umem, node);
>
> If 'node' is NULL we will not be in the body of the loop as per
> the while() condition.
The patch is incorrect, please ignore
>
> How did you test this?
>
^ permalink raw reply
* general protection fault in rcu_core
From: syzbot @ 2019-07-10 7:57 UTC (permalink / raw)
To: ast, bp, daniel, drake, hpa, jacob.jun.pan, john.fastabend,
linux-kernel, mingo, netdev, puwen, rppt, syzkaller-bugs, tglx,
x86
Hello,
syzbot found the following crash on:
HEAD commit: 4608a726 Add linux-next specific files for 20190709
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=14458387a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=7a02e36d356a9a17
dashboard link: https://syzkaller.appspot.com/bug?extid=73ac69a8f7a5e5c126f1
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=128e3217a00000
The bug was bisected to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1155a96fa00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=1355a96fa00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1555a96fa00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+73ac69a8f7a5e5c126f1@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 9817 Comm: blkid Not tainted 5.2.0-next-20190709 #34
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:lookup_object lib/debugobjects.c:193 [inline]
RIP: 0010:debug_object_active_state lib/debugobjects.c:900 [inline]
RIP: 0010:debug_object_active_state+0x16e/0x350 lib/debugobjects.c:885
Code: 1a 4c 89 e0 48 c1 e8 03 80 3c 08 00 0f 85 6c 01 00 00 4d 8b 24 24 4d
85 e4 74 6a 4d 8d 44 24 18 83 c3 01 4c 89 c7 48 c1 ef 03 <80> 3c 0f 00 0f
85 17 01 00 00 4d 3b 7c 24 18 75 c6 49 8d 7c 24 10
RSP: 0018:ffff8880ae809d00 EFLAGS: 00010802
RAX: 1ffffffff0c4eadc RBX: 0000000000000005 RCX: dffffc0000000000
RDX: 0000000000000001 RSI: 0000000000000286 RDI: 1dc43d1fffffc920
RBP: ffff8880ae809de8 R08: ee21e8fffffe4901 R09: ffffed1015d0138d
R10: ffffffff8a9b1768 R11: 0000000000000003 R12: ee21e8fffffe48e9
R13: 1ffff11015d013a4 R14: ffffffff88dab220 R15: ffff8880a21f7f58
FS: 00007f3fd76fc740(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3fd72daa20 CR3: 000000008fe3c000 CR4: 00000000001406f0
Call Trace:
<IRQ>
debug_rcu_head_unqueue kernel/rcu/rcu.h:185 [inline]
rcu_do_batch kernel/rcu/tree.c:2113 [inline]
rcu_core+0x745/0x1580 kernel/rcu/tree.c:2314
rcu_core_si+0x9/0x10 kernel/rcu/tree.c:2323
__do_softirq+0x262/0x98c kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x19b/0x1e0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:537 [inline]
smp_apic_timer_interrupt+0x1a3/0x610 arch/x86/kernel/apic/apic.c:1095
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:828
</IRQ>
RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:767
[inline]
RIP: 0010:console_unlock+0xdab/0xf10 kernel/printk/printk.c:2467
Code: 88 48 ba 00 00 00 00 00 fc ff df 48 c1 e8 03 80 3c 10 00 75 30 48 83
3d 42 4a 77 07 00 74 1f e8 7b ab 16 00 48 8b 7d 98 57 9d <0f> 1f 44 00 00
e9 64 fa ff ff e8 c6 ea 50 00 e9 0e f5 ff ff e8 5c
RSP: 0018:ffff8880991bf2b8 EFLAGS: 00000293 ORIG_RAX: ffffffffffffff13
RAX: ffff8880a7e1e000 RBX: 0000000000000200 RCX: 1ffffffff134a59e
RDX: 0000000000000000 RSI: ffffffff815b9995 RDI: 0000000000000293
RBP: ffff8880991bf340 R08: ffff8880a7e1e000 R09: fffffbfff1349f60
R10: fffffbfff1349f5f R11: ffffffff89a4faff R12: 0000000000000001
R13: ffffffff843434b0 R14: dffffc0000000000 R15: ffffffff893cb710
vprintk_emit+0x2a0/0x700 kernel/printk/printk.c:1986
vprintk_default+0x28/0x30 kernel/printk/printk.c:2013
vprintk_func+0x7e/0x189 kernel/printk/printk_safe.c:386
printk+0xba/0xed kernel/printk/printk.c:2046
__warn_printk+0x9b/0xf3 kernel/panic.c:630
debug_mutex_wake_waiter+0x1d7/0x330 kernel/locking/mutex-debug.c:41
__mutex_unlock_slowpath+0x3d5/0x6b0 kernel/locking/mutex.c:1241
mutex_unlock+0xd/0x10 kernel/locking/mutex.c:714
kobj_lookup+0x250/0x460 drivers/base/map.c:123
get_gendisk+0x4d/0x390 block/genhd.c:869
bdev_get_gendisk fs/block_dev.c:1100 [inline]
__blkdev_get+0x457/0x1660 fs/block_dev.c:1493
blkdev_get+0xc4/0x990 fs/block_dev.c:1652
blkdev_open+0x205/0x290 fs/block_dev.c:1810
do_dentry_open+0x4df/0x1250 fs/open.c:778
vfs_open+0xa0/0xd0 fs/open.c:887
do_last fs/namei.c:3416 [inline]
path_openat+0x10e9/0x4630 fs/namei.c:3533
do_filp_open+0x1a1/0x280 fs/namei.c:3563
do_sys_open+0x3fe/0x5d0 fs/open.c:1070
__do_sys_open fs/open.c:1088 [inline]
__se_sys_open fs/open.c:1083 [inline]
__x64_sys_open+0x7e/0xc0 fs/open.c:1083
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f3fd7004120
Code: 48 8b 15 1b 4d 2b 00 f7 d8 64 89 02 83 c8 ff c3 90 90 90 90 90 90 90
90 90 90 83 3d d5 a4 2b 00 00 75 10 b8 02 00 00 00 0f 05 <48> 3d 01 f0 ff
ff 73 31 c3 48 83 ec 08 e8 5e 8c 01 00 48 89 04 24
RSP: 002b:00007ffc2fe70dc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000002
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3fd7004120
RDX: 00007ffc2fe72f33 RSI: 0000000000000000 RDI: 00007ffc2fe72f33
RBP: 0000000000000000 R08: 0000000000000078 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000001882030
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000005
Modules linked in:
---[ end trace d75a73838ffe8a54 ]---
RIP: 0010:lookup_object lib/debugobjects.c:193 [inline]
RIP: 0010:debug_object_active_state lib/debugobjects.c:900 [inline]
RIP: 0010:debug_object_active_state+0x16e/0x350 lib/debugobjects.c:885
Code: 1a 4c 89 e0 48 c1 e8 03 80 3c 08 00 0f 85 6c 01 00 00 4d 8b 24 24 4d
85 e4 74 6a 4d 8d 44 24 18 83 c3 01 4c 89 c7 48 c1 ef 03 <80> 3c 0f 00 0f
85 17 01 00 00 4d 3b 7c 24 18 75 c6 49 8d 7c 24 10
RSP: 0018:ffff8880ae809d00 EFLAGS: 00010802
RAX: 1ffffffff0c4eadc RBX: 0000000000000005 RCX: dffffc0000000000
RDX: 0000000000000001 RSI: 0000000000000286 RDI: 1dc43d1fffffc920
RBP: ffff8880ae809de8 R08: ee21e8fffffe4901 R09: ffffed1015d0138d
R10: ffffffff8a9b1768 R11: 0000000000000003 R12: ee21e8fffffe48e9
R13: 1ffff11015d013a4 R14: ffffffff88dab220 R15: ffff8880a21f7f58
FS: 00007f3fd76fc740(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3fd72daa20 CR3: 000000008fe3c000 CR4: 00000000001406f0
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] tipc: ensure skb->lock is initialised
From: Eric Dumazet @ 2019-07-10 8:00 UTC (permalink / raw)
To: Jon Maloy, Eric Dumazet, Chris Packham, ying.xue@windriver.com,
davem@davemloft.net
Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <MN2PR15MB35813EA3ADE7E5E83A657D3F9AF10@MN2PR15MB3581.namprd15.prod.outlook.com>
On 7/9/19 10:15 PM, Jon Maloy wrote:
>
> It is not only for lockdep purposes, -it is essential. But please provide details about where you see that more fixes are needed.
>
Simple fact that you detect a problem only when skb_queue_purge() is called should talk by itself.
As I stated, there are many places where the list is manipulated _without_ its spinlock being held.
You want consistency, then
- grab the spinlock all the time.
- Or do not ever use it.
Do not initialize the spinlock just in case a path will use skb_queue_purge() (instead of using __skb_queue_purge())
^ permalink raw reply
* [PATCH] [net-next] davinci_cpdma: don't cast dma_addr_t to pointer
From: Arnd Bergmann @ 2019-07-10 8:00 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Ivan Khoronzhuk, Grygorii Strashko, Andrew Lunn,
Ilias Apalodimas, linux-omap, netdev, linux-kernel
dma_addr_t may be 64-bit wide on 32-bit architectures, so it is not
valid to cast between it and a pointer:
drivers/net/ethernet/ti/davinci_cpdma.c: In function 'cpdma_chan_submit_si':
drivers/net/ethernet/ti/davinci_cpdma.c:1047:12: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
drivers/net/ethernet/ti/davinci_cpdma.c: In function 'cpdma_chan_idle_submit_mapped':
drivers/net/ethernet/ti/davinci_cpdma.c:1114:12: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
drivers/net/ethernet/ti/davinci_cpdma.c: In function 'cpdma_chan_submit_mapped':
drivers/net/ethernet/ti/davinci_cpdma.c:1164:12: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
Solve this by using two separate members in 'struct submit_info'.
Since this avoids the use of the 'flag' member, the structure does
not even grow in typical configurations.
Fixes: 6670acacd59e ("net: ethernet: ti: davinci_cpdma: add dma mapped submit")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/ti/davinci_cpdma.c | 26 ++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 0ca2a1a254de..a65edd2770e6 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -138,8 +138,8 @@ struct submit_info {
struct cpdma_chan *chan;
int directed;
void *token;
- void *data;
- int flags;
+ void *data_virt;
+ dma_addr_t data_dma;
int len;
};
@@ -1043,12 +1043,12 @@ static int cpdma_chan_submit_si(struct submit_info *si)
mode = CPDMA_DESC_OWNER | CPDMA_DESC_SOP | CPDMA_DESC_EOP;
cpdma_desc_to_port(chan, mode, si->directed);
- if (si->flags & CPDMA_DMA_EXT_MAP) {
- buffer = (dma_addr_t)si->data;
+ if (si->data_dma) {
+ buffer = si->data_dma;
dma_sync_single_for_device(ctlr->dev, buffer, len, chan->dir);
swlen |= CPDMA_DMA_EXT_MAP;
} else {
- buffer = dma_map_single(ctlr->dev, si->data, len, chan->dir);
+ buffer = dma_map_single(ctlr->dev, si->data_virt, len, chan->dir);
ret = dma_mapping_error(ctlr->dev, buffer);
if (ret) {
cpdma_desc_free(ctlr->pool, desc, 1);
@@ -1086,10 +1086,10 @@ int cpdma_chan_idle_submit(struct cpdma_chan *chan, void *token, void *data,
si.chan = chan;
si.token = token;
- si.data = data;
+ si.data_virt = data;
+ si.data_dma = 0;
si.len = len;
si.directed = directed;
- si.flags = 0;
spin_lock_irqsave(&chan->lock, flags);
if (chan->state == CPDMA_STATE_TEARDOWN) {
@@ -1111,10 +1111,10 @@ int cpdma_chan_idle_submit_mapped(struct cpdma_chan *chan, void *token,
si.chan = chan;
si.token = token;
- si.data = (void *)data;
+ si.data_virt = NULL;
+ si.data_dma = data;
si.len = len;
si.directed = directed;
- si.flags = CPDMA_DMA_EXT_MAP;
spin_lock_irqsave(&chan->lock, flags);
if (chan->state == CPDMA_STATE_TEARDOWN) {
@@ -1136,10 +1136,10 @@ int cpdma_chan_submit(struct cpdma_chan *chan, void *token, void *data,
si.chan = chan;
si.token = token;
- si.data = data;
+ si.data_virt = data;
+ si.data_dma = 0;
si.len = len;
si.directed = directed;
- si.flags = 0;
spin_lock_irqsave(&chan->lock, flags);
if (chan->state != CPDMA_STATE_ACTIVE) {
@@ -1161,10 +1161,10 @@ int cpdma_chan_submit_mapped(struct cpdma_chan *chan, void *token,
si.chan = chan;
si.token = token;
- si.data = (void *)data;
+ si.data_virt = NULL;
+ si.data_dma = data;
si.len = len;
si.directed = directed;
- si.flags = CPDMA_DMA_EXT_MAP;
spin_lock_irqsave(&chan->lock, flags);
if (chan->state != CPDMA_STATE_ACTIVE) {
--
2.20.0
^ permalink raw reply related
* Re: [PATCH net-next,v4 08/12] drivers: net: use flow block API
From: Jiri Pirko @ 2019-07-10 8:01 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
maxime.chevallier, cphealy, phil, netfilter-devel
In-Reply-To: <20190709205550.3160-9-pablo@netfilter.org>
Tue, Jul 09, 2019 at 10:55:46PM CEST, pablo@netfilter.org wrote:
[...]
> static int
> mlxsw_sp_setup_tc_block_flower_bind(struct mlxsw_sp_port *mlxsw_sp_port,
>- struct tcf_block *block, bool ingress,
>- struct netlink_ext_ack *extack)
>+ struct flow_block_offload *f, bool ingress)
> {
> struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
> struct mlxsw_sp_acl_block *acl_block;
>- struct tcf_block_cb *block_cb;
>+ struct flow_block_cb *block_cb;
>+ bool register_block = false;
> int err;
>
>- block_cb = tcf_block_cb_lookup(block, mlxsw_sp_setup_tc_block_cb_flower,
>- mlxsw_sp);
>+ block_cb = flow_block_cb_lookup(f, mlxsw_sp_setup_tc_block_cb_flower,
>+ mlxsw_sp);
> if (!block_cb) {
>- acl_block = mlxsw_sp_acl_block_create(mlxsw_sp, block->net);
>+ acl_block = mlxsw_sp_acl_block_create(mlxsw_sp, f->net);
> if (!acl_block)
> return -ENOMEM;
>- block_cb = __tcf_block_cb_register(block,
>- mlxsw_sp_setup_tc_block_cb_flower,
>- mlxsw_sp, acl_block, extack);
>+ block_cb = flow_block_cb_alloc(f->net,
>+ mlxsw_sp_setup_tc_block_cb_flower,
>+ mlxsw_sp, acl_block,
>+ mlxsw_sp_tc_block_flower_release);
> if (IS_ERR(block_cb)) {
>+ mlxsw_sp_acl_block_destroy(acl_block);
> err = PTR_ERR(block_cb);
> goto err_cb_register;
> }
>+ register_block = true;
> } else {
>- acl_block = tcf_block_cb_priv(block_cb);
>+ acl_block = flow_block_cb_priv(block_cb);
> }
>- tcf_block_cb_incref(block_cb);
>+ flow_block_cb_incref(block_cb);
> err = mlxsw_sp_acl_block_bind(mlxsw_sp, acl_block,
> mlxsw_sp_port, ingress);
> if (err)
>@@ -1622,28 +1634,31 @@ mlxsw_sp_setup_tc_block_flower_bind(struct mlxsw_sp_port *mlxsw_sp_port,
> else
> mlxsw_sp_port->eg_acl_block = acl_block;
>
>+ if (register_block) {
>+ flow_block_cb_add(block_cb, f);
>+ list_add_tail(&block_cb->driver_list, &mlxsw_sp_block_cb_list);
>+ }
What prevents you from doing these 2 above right after
flow_block_cb_alloc?
More than that, what prevents you do maintain the same flow as was there
originally? You just need struct flow_block as a replacement of
struct tcf_block and have it contained in both struct nft_base_chain
and struct tcf_block.
And you would push pointer to struct flow_block down to drivers
in struct flow_block_offload.
[...]
^ permalink raw reply
* Re: [PATCH] ipvs: remove unnecessary space
From: Simon Horman @ 2019-07-10 8:06 UTC (permalink / raw)
To: yangxingwu, Pablo Neira Ayuso
Cc: wensong, ja, pablo, kadlec, fw, davem, netdev, lvs-devel,
netfilter-devel, coreteam, linux-kernel
In-Reply-To: <20190710074552.74394-1-xingwu.yang@gmail.com>
On Wed, Jul 10, 2019 at 03:45:52PM +0800, yangxingwu wrote:
> this patch removes the extra space.
>
> Signed-off-by: yangxingwu <xingwu.yang@gmail.com>
Thanks, this looks good to me.
Acked-by: Simon Horman <horms@verge.net.au>
Pablo, please consider including this in nf-next.
> ---
> net/netfilter/ipvs/ip_vs_mh.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
> index 94d9d34..98e358e 100644
> --- a/net/netfilter/ipvs/ip_vs_mh.c
> +++ b/net/netfilter/ipvs/ip_vs_mh.c
> @@ -174,8 +174,8 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s,
> return 0;
> }
>
> - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> - sizeof(unsigned long), GFP_KERNEL);
> + table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> + sizeof(unsigned long), GFP_KERNEL);
> if (!table)
> return -ENOMEM;
>
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: Question about nf_conntrack_proto for IPsec
From: Florian Westphal @ 2019-07-10 8:07 UTC (permalink / raw)
To: Naruto Nguyen; +Cc: Florian Westphal, netfilter-devel, netdev, netfilter
In-Reply-To: <CANpxKHGa6DpV-9n8La7wh6r7MbEZpzGTWOO1AhmhWv072b4LAg@mail.gmail.com>
Naruto Nguyen <narutonguyen2018@gmail.com> wrote:
> Could you please elaborate more on how generic tracker tracks ESP connection?
All protocols that do not have a more specific l4 tracker are tracked
based on l3 protocol + l4 proto number.
IOW, any ESP packet sent between the same endpoint addresses is seen
as matching a single esp flow.
We could easily add the ESP SPI as additional distinction marker if needed.
^ permalink raw reply
* [PATCH bpf] bpf: fix BTF verifier size resolution logic
From: Andrii Nakryiko @ 2019-07-10 8:08 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team
Cc: Andrii Nakryiko, Martin KaFai Lau
BTF verifier has Different logic depending on whether we are following
a PTR or STRUCT/ARRAY (or something else). This is an optimization to
stop early in DFS traversal while resolving BTF types. But it also
results in a size resolution bug, when there is a chain, e.g., of PTR ->
TYPEDEF -> ARRAY, in which case due to being in pointer context ARRAY
size won't be resolved, as it is considered to be a sink for pointer,
leading to TYPEDEF being in RESOLVED state with zero size, which is
completely wrong.
Optimization is doubtful, though, as btf_check_all_types() will iterate
over all BTF types anyways, so the only saving is a potentially slightly
shorter stack. But correctness is more important that tiny savings.
This bug manifests itself in rejecting BTF-defined maps that use array
typedef as a value type:
typedef int array_t[16];
struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__type(value, array_t); /* i.e., array_t *value; */
} test_map SEC(".maps");
Fixes: eb3f595dab40 ("bpf: btf: Validate type reference")
Cc: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
kernel/bpf/btf.c | 42 +++---------------------------------------
1 file changed, 3 insertions(+), 39 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index cad09858a5f2..c68c7e73b0d1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -231,14 +231,6 @@ enum visit_state {
RESOLVED,
};
-enum resolve_mode {
- RESOLVE_TBD, /* To Be Determined */
- RESOLVE_PTR, /* Resolving for Pointer */
- RESOLVE_STRUCT_OR_ARRAY, /* Resolving for struct/union
- * or array
- */
-};
-
#define MAX_RESOLVE_DEPTH 32
struct btf_sec_info {
@@ -254,7 +246,6 @@ struct btf_verifier_env {
u32 log_type_id;
u32 top_stack;
enum verifier_phase phase;
- enum resolve_mode resolve_mode;
};
static const char * const btf_kind_str[NR_BTF_KINDS] = {
@@ -964,26 +955,7 @@ static void btf_verifier_env_free(struct btf_verifier_env *env)
static bool env_type_is_resolve_sink(const struct btf_verifier_env *env,
const struct btf_type *next_type)
{
- switch (env->resolve_mode) {
- case RESOLVE_TBD:
- /* int, enum or void is a sink */
- return !btf_type_needs_resolve(next_type);
- case RESOLVE_PTR:
- /* int, enum, void, struct, array, func or func_proto is a sink
- * for ptr
- */
- return !btf_type_is_modifier(next_type) &&
- !btf_type_is_ptr(next_type);
- case RESOLVE_STRUCT_OR_ARRAY:
- /* int, enum, void, ptr, func or func_proto is a sink
- * for struct and array
- */
- return !btf_type_is_modifier(next_type) &&
- !btf_type_is_array(next_type) &&
- !btf_type_is_struct(next_type);
- default:
- BUG();
- }
+ return !btf_type_needs_resolve(next_type);
}
static bool env_type_is_resolved(const struct btf_verifier_env *env,
@@ -1010,13 +982,6 @@ static int env_stack_push(struct btf_verifier_env *env,
v->type_id = type_id;
v->next_member = 0;
- if (env->resolve_mode == RESOLVE_TBD) {
- if (btf_type_is_ptr(t))
- env->resolve_mode = RESOLVE_PTR;
- else if (btf_type_is_struct(t) || btf_type_is_array(t))
- env->resolve_mode = RESOLVE_STRUCT_OR_ARRAY;
- }
-
return 0;
}
@@ -1038,7 +1003,7 @@ static void env_stack_pop_resolved(struct btf_verifier_env *env,
env->visit_states[type_id] = RESOLVED;
}
-static const struct resolve_vertex *env_stack_peak(struct btf_verifier_env *env)
+static const struct resolve_vertex *env_stack_peek(struct btf_verifier_env *env)
{
return env->top_stack ? &env->stack[env->top_stack - 1] : NULL;
}
@@ -3030,9 +2995,8 @@ static int btf_resolve(struct btf_verifier_env *env,
const struct resolve_vertex *v;
int err = 0;
- env->resolve_mode = RESOLVE_TBD;
env_stack_push(env, t, type_id);
- while (!err && (v = env_stack_peak(env))) {
+ while (!err && (v = env_stack_peek(env))) {
env->log_type_id = v->type_id;
err = btf_type_ops(v->t)->resolve(env, v);
}
--
2.17.1
^ permalink raw reply related
* [PATCH] [net-next] netfilter: bridge: make NF_TABLES_BRIDGE tristate
From: Arnd Bergmann @ 2019-07-10 8:08 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
Roopa Prabhu, Nikolay Aleksandrov, David S. Miller
Cc: Arnd Bergmann, wenxu, netfilter-devel, coreteam, bridge, netdev,
linux-kernel
The new nft_meta_bridge code fails to link as built-in when NF_TABLES
is a loadable module.
net/bridge/netfilter/nft_meta_bridge.o: In function `nft_meta_bridge_get_eval':
nft_meta_bridge.c:(.text+0x1e8): undefined reference to `nft_meta_get_eval'
net/bridge/netfilter/nft_meta_bridge.o: In function `nft_meta_bridge_get_init':
nft_meta_bridge.c:(.text+0x468): undefined reference to `nft_meta_get_init'
nft_meta_bridge.c:(.text+0x49c): undefined reference to `nft_parse_register'
nft_meta_bridge.c:(.text+0x4cc): undefined reference to `nft_validate_register_store'
net/bridge/netfilter/nft_meta_bridge.o: In function `nft_meta_bridge_module_exit':
nft_meta_bridge.c:(.exit.text+0x14): undefined reference to `nft_unregister_expr'
net/bridge/netfilter/nft_meta_bridge.o: In function `nft_meta_bridge_module_init':
nft_meta_bridge.c:(.init.text+0x14): undefined reference to `nft_register_expr'
net/bridge/netfilter/nft_meta_bridge.o:(.rodata+0x60): undefined reference to `nft_meta_get_dump'
net/bridge/netfilter/nft_meta_bridge.o:(.rodata+0x88): undefined reference to `nft_meta_set_eval'
This can happen because the NF_TABLES_BRIDGE dependency itself is just a
'bool'. Make the symbol a 'tristate' instead so Kconfig can propagate the
dependencies correctly.
Fixes: 30e103fe24de ("netfilter: nft_meta: move bridge meta keys into nft_meta_bridge")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/bridge/netfilter/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index 154fa558bb90..d0c75d7ec074 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -6,7 +6,7 @@
menuconfig NF_TABLES_BRIDGE
depends on BRIDGE && NETFILTER && NF_TABLES
select NETFILTER_FAMILY_BRIDGE
- bool "Ethernet Bridge nf_tables support"
+ tristate "Ethernet Bridge nf_tables support"
if NF_TABLES_BRIDGE
--
2.20.0
^ permalink raw reply related
* Re: [PATCH iproute2 2/2] ip tunnel: warn when changing IPv6 tunnel without tunnel name
From: Andrea Claudi @ 2019-07-10 8:33 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: linux-netdev, Stephen Hemminger, David Ahern
In-Reply-To: <CAF2d9jhiUk0Jpz54EbA+3Fyf-cMniRHZrpktu57yZ+tX+QsuEQ@mail.gmail.com>
On Wed, Jul 10, 2019 at 12:15 AM Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
>
> On Tue, Jul 9, 2019 at 6:16 AM Andrea Claudi <aclaudi@redhat.com> wrote:
> >
> > Tunnel change fails if a tunnel name is not specified while using
> > 'ip -6 tunnel change'. However, no warning message is printed and
> > no error code is returned.
> >
> > $ ip -6 tunnel add ip6tnl1 mode ip6gre local fd::1 remote fd::2 tos inherit ttl 127 encaplimit none dev dummy0
> > $ ip -6 tunnel change dev dummy0 local 2001:1234::1 remote 2001:1234::2
> > $ ip -6 tunnel show ip6tnl1
> > ip6tnl1: gre/ipv6 remote fd::2 local fd::1 dev dummy0 encaplimit none hoplimit 127 tclass inherit flowlabel 0x00000 (flowinfo 0x00000000)
> >
> > This commit checks if tunnel interface name is equal to an empty
> > string: in this case, it prints a warning message to the user.
> > It intentionally avoids to return an error to not break existing
> > script setup.
> >
> > This is the output after this commit:
> > $ ip -6 tunnel add ip6tnl1 mode ip6gre local fd::1 remote fd::2 tos inherit ttl 127 encaplimit none dev dummy0
> > $ ip -6 tunnel change dev dummy0 local 2001:1234::1 remote 2001:1234::2
> > Tunnel interface name not specified
> > $ ip -6 tunnel show ip6tnl1
> > ip6tnl1: gre/ipv6 remote fd::2 local fd::1 dev dummy0 encaplimit none hoplimit 127 tclass inherit flowlabel 0x00000 (flowinfo 0x00000000)
> >
> > Reviewed-by: Matteo Croce <mcroce@redhat.com>
> > Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
>
> I tried your patch and the commands that I posted in my (previous) patch.
>
Hi Mahesh,
Thank you for taking the time to review my patch.
> Here is the output after reverting my patch and applying your patch
>
> <show command>
> ------------------------
> vm0:/tmp# ./ip -6 tunnel add ip6tnl1 mode ip6gre local fd::1 remote
> fd::2 tos inherit ttl 127 encaplimit none
> vm0:/tmp# ./ip -6 tunnel show dev ip6tnl1
> vm0:/tmp# echo $?
> 0
>
> here the output is NULL and return code is 0. This is wrong and I
> would expect to see the tunnel info (as displayed in 'ip -6 tunnel
> show ip6tnl1')
It seems to me there is a bit of misunderstanding here. Looking at man
page for ip tunnel:
dev NAME
bind the tunnel to the device NAME so that tunneled packets
will only be routed via this device and will not be able to escape to
another device when the route to endpoint changes.
From what I read, dev parameter should not be used as an alias to the
tunnel device, but to indicate the device to which the tunnel should
be binded.
As such, ip -6 tunnel show dev <name> is a legitimate query that must
show the tunnel device(s) binded to <name> interface.
With the query ip -6 tunnel show <name> you instead obtain the tunnel itself.
By the way, the proper selector for the tunnel device name is the
default "name" parameter. From the man page:
name NAME (default)
select the tunnel device name.
For example:
$ ip -6 tunnel show name ip6tnl0
ip6tnl0: ipv6/ipv6 remote :: local :: encaplimit 0 hoplimit inherit
tclass 0x00 flowlabel 0x00000 (flowinfo 0x00000000)
> <change command>
> lpaa10:/tmp# ip -6 tunnel change dev ip6tnl1 local 2001:1234::1 remote
> 2001:1234::2 encaplimit none ttl 127 tos inherit allow-localremote
> lpaa10:/tmp# echo $?
> 0
> lpaa10:/tmp# ip -6 tunnel show dev ip6tnl1
> lpaa10:/tmp# ip -6 tunnel show ip6tnl1
> ip6tnl1: gre/ipv6 remote fd::2 local fd::1 encaplimit none hoplimit
> 127 tclass inherit flowlabel 0x00000 (flowinfo 0x00000000)
>
> the change command appeared to be successful but change wasn't applied
> (expecting the allow-localremote to be present on the tunnel).
As you can read from the commit message, here I am not changing the
return code intentionally to not break existing script setup.
However, I can easily change this to return an error code in a v2.
> ---
> > ip/ip6tunnel.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
> > index 999408ed801b1..e3da11eb4518e 100644
> > --- a/ip/ip6tunnel.c
> > +++ b/ip/ip6tunnel.c
> > @@ -386,6 +386,9 @@ static int do_add(int cmd, int argc, char **argv)
> > if (parse_args(argc, argv, cmd, &p) < 0)
> > return -1;
> >
> > + if (!*p.name)
> > + fprintf(stderr, "Tunnel interface name not specified\n");
> > +
> > if (p.proto == IPPROTO_GRE)
> > basedev = "ip6gre0";
> > else if (p.i_flags & VTI_ISVTI)
> > --
> > 2.20.1
> >
Regards,
Andrea
^ permalink raw reply
* RE: [PATCH v2 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Tony Chuang @ 2019-07-10 8:36 UTC (permalink / raw)
To: Jian-Hong Pan, Kalle Valo, David S . Miller, Larry Finger,
David Laight
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux@endlessm.com, Daniel Drake,
stable@vger.kernel.org
In-Reply-To: <20190709102059.7036-1-jian-hong@endlessm.com>
> Subject: [PATCH v2 1/2] rtw88: pci: Rearrange the memory usage for skb in
> RX ISR
>
> Testing with RTL8822BE hardware, when available memory is low, we
> frequently see a kernel panic and system freeze.
>
> First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
>
> rx routine starvation
> WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822
> rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
>
> Then we see a variety of different error conditions and kernel panics,
> such as this one (trimmed):
>
> rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415
> head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0
> dev:<NULL>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:105!
> invalid opcode: 0000 [#1] SMP NOPTI
> RIP: 0010:skb_panic+0x43/0x45
>
> When skb allocation fails and the "rx routine starvation" is hit, the
> function returns immediately without updating the RX ring. At this
> point, the RX ring may continue referencing an old skb which was already
> handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> bad things happen.
>
> This patch allocates a new, data-sized skb first in RX ISR. After
> copying the data in, we pass it to the upper layers. However, if skb
> allocation fails, we effectively drop the frame. In both cases, the
> original, full size ring skb is reused.
>
> In addition, to fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
> 1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..e9fe3ad896c8 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> struct rtw_pci *rtwpci,
> u32 pkt_offset;
> u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> u32 buf_desc_sz = chip->rx_buf_desc_sz;
> + u32 new_len;
> u8 *rx_desc;
> dma_addr_t dma;
>
> @@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> struct rtw_pci *rtwpci,
> pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> pkt_stat.shift;
>
> - if (pkt_stat.is_c2h) {
> - /* keep rx_desc, halmac needs it */
> - skb_put(skb, pkt_stat.pkt_len + pkt_offset);
> + /* discard current skb if the new skb cannot be allocated as a
> + * new one in rx ring later
> + */
> + new_len = pkt_stat.pkt_len + pkt_offset;
> + new = dev_alloc_skb(new_len);
> + if (WARN_ONCE(!new, "rx routine starvation\n"))
> + goto next_rp;
> +
> + /* put the DMA data including rx_desc from phy to new skb */
> + skb_put_data(new, skb->data, new_len);
>
> - /* pass offset for further operation */
> - *((u32 *)skb->cb) = pkt_offset;
> - skb_queue_tail(&rtwdev->c2h_queue, skb);
> + if (pkt_stat.is_c2h) {
> + /* pass rx_desc & offset for further operation */
> + *((u32 *)new->cb) = pkt_offset;
> + skb_queue_tail(&rtwdev->c2h_queue, new);
> ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
> } else {
> - /* remove rx_desc, maybe use skb_pull? */
> - skb_put(skb, pkt_stat.pkt_len);
> - skb_reserve(skb, pkt_offset);
> -
> - /* alloc a smaller skb to mac80211 */
> - new = dev_alloc_skb(pkt_stat.pkt_len);
> - if (!new) {
> - new = skb;
> - } else {
> - skb_put_data(new, skb->data, skb->len);
> - dev_kfree_skb_any(skb);
> - }
> - /* TODO: merge into rx.c */
> - rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> + /* remove rx_desc */
> + skb_pull(new, pkt_offset);
> +
> + rtw_rx_stats(rtwdev, pkt_stat.vif, new);
> memcpy(new->cb, &rx_status, sizeof(rx_status));
> ieee80211_rx_irqsafe(rtwdev->hw, new);
> }
>
> - /* skb delivered to mac80211, alloc a new one in rx ring */
> - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> - if (WARN(!new, "rx routine starvation\n"))
> - return;
> -
> - ring->buf[cur_rp] = new;
> - rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> +next_rp:
> + /* new skb delivered to mac80211, re-enable original skb DMA */
> + rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
>
> /* host read next element in ring */
> if (++cur_rp >= ring->r.len)
> --
> 2.22.0
Now it looks good to me. Thanks.
Acked-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
Yan-Hsuan
^ permalink raw reply
* [PATCH v3 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-10 8:38 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight, Christoph Hellwig
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake,
Jian-Hong Pan, stable
In-Reply-To: <20190709161550.GA8703@infradead.org>
Testing with RTL8822BE hardware, when available memory is low, we
frequently see a kernel panic and system freeze.
First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
rx routine starvation
WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822 rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
[ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
Then we see a variety of different error conditions and kernel panics,
such as this one (trimmed):
rtw_pci 0000:02:00.0: pci bus timeout, check dma status
skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:105!
invalid opcode: 0000 [#1] SMP NOPTI
RIP: 0010:skb_panic+0x43/0x45
When skb allocation fails and the "rx routine starvation" is hit, the
function returns immediately without updating the RX ring. At this
point, the RX ring may continue referencing an old skb which was already
handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
bad things happen.
This patch allocates a new, data-sized skb first in RX ISR. After
copying the data in, we pass it to the upper layers. However, if skb
allocation fails, we effectively drop the frame. In both cases, the
original, full size ring skb is reused.
In addition, by fixing the kernel crash, the RX routine should now
generally behave better under low memory conditions.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Cc: <stable@vger.kernel.org>
---
v2:
- Allocate new data-sized skb and put data into it, then pass it to
mac80211. Reuse the original skb in RX ring by DMA sync.
- Modify the commit message.
- Introduce following [PATCH v3 2/2] rtw88: pci: Use DMA sync instead
of remapping in RX ISR.
v3:
- Same as v2.
drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index cfe05ba7280d..e9fe3ad896c8 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
u32 pkt_offset;
u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
u32 buf_desc_sz = chip->rx_buf_desc_sz;
+ u32 new_len;
u8 *rx_desc;
dma_addr_t dma;
@@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
pkt_stat.shift;
- if (pkt_stat.is_c2h) {
- /* keep rx_desc, halmac needs it */
- skb_put(skb, pkt_stat.pkt_len + pkt_offset);
+ /* discard current skb if the new skb cannot be allocated as a
+ * new one in rx ring later
+ */
+ new_len = pkt_stat.pkt_len + pkt_offset;
+ new = dev_alloc_skb(new_len);
+ if (WARN_ONCE(!new, "rx routine starvation\n"))
+ goto next_rp;
+
+ /* put the DMA data including rx_desc from phy to new skb */
+ skb_put_data(new, skb->data, new_len);
- /* pass offset for further operation */
- *((u32 *)skb->cb) = pkt_offset;
- skb_queue_tail(&rtwdev->c2h_queue, skb);
+ if (pkt_stat.is_c2h) {
+ /* pass rx_desc & offset for further operation */
+ *((u32 *)new->cb) = pkt_offset;
+ skb_queue_tail(&rtwdev->c2h_queue, new);
ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
} else {
- /* remove rx_desc, maybe use skb_pull? */
- skb_put(skb, pkt_stat.pkt_len);
- skb_reserve(skb, pkt_offset);
-
- /* alloc a smaller skb to mac80211 */
- new = dev_alloc_skb(pkt_stat.pkt_len);
- if (!new) {
- new = skb;
- } else {
- skb_put_data(new, skb->data, skb->len);
- dev_kfree_skb_any(skb);
- }
- /* TODO: merge into rx.c */
- rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
+ /* remove rx_desc */
+ skb_pull(new, pkt_offset);
+
+ rtw_rx_stats(rtwdev, pkt_stat.vif, new);
memcpy(new->cb, &rx_status, sizeof(rx_status));
ieee80211_rx_irqsafe(rtwdev->hw, new);
}
- /* skb delivered to mac80211, alloc a new one in rx ring */
- new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
- if (WARN(!new, "rx routine starvation\n"))
- return;
-
- ring->buf[cur_rp] = new;
- rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
+next_rp:
+ /* new skb delivered to mac80211, re-enable original skb DMA */
+ rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
/* host read next element in ring */
if (++cur_rp >= ring->r.len)
--
2.22.0
^ permalink raw reply related
* [PATCH v3 2/2] rtw88: pci: Use DMA sync instead of remapping in RX ISR
From: Jian-Hong Pan @ 2019-07-10 8:38 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight, Christoph Hellwig
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake,
Jian-Hong Pan, stable
In-Reply-To: <20190709161550.GA8703@infradead.org>
Since each skb in RX ring is reused instead of new allocation, we can
treat the DMA in a more efficient way by DMA synchronization.
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Cc: <stable@vger.kernel.org>
---
v2:
- New patch by following [PATCH v3 1/2] rtw88: pci: Rearrange the
memory usage for skb in RX ISR.
v3:
- Remove rtw_pci_sync_rx_desc_cpu and call dma_sync_single_for_cpu in
rtw_pci_rx_isr directly.
- Remove the return value of rtw_pci_sync_rx_desc_device.
- Use DMA_FROM_DEVICE instead of PCI_DMA_FROMDEVICE.
drivers/net/wireless/realtek/rtw88/pci.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index e9fe3ad896c8..485d30c06935 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -206,6 +206,23 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
return 0;
}
+static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
+ struct rtw_pci_rx_ring *rx_ring,
+ u32 idx, u32 desc_sz)
+{
+ struct device *dev = rtwdev->dev;
+ struct rtw_pci_rx_buffer_desc *buf_desc;
+ int buf_sz = RTK_PCI_RX_BUF_SIZE;
+
+ dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+
+ buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
+ idx * desc_sz);
+ memset(buf_desc, 0, sizeof(*buf_desc));
+ buf_desc->buf_size = cpu_to_le16(RTK_PCI_RX_BUF_SIZE);
+ buf_desc->dma = cpu_to_le32(dma);
+}
+
static int rtw_pci_init_rx_ring(struct rtw_dev *rtwdev,
struct rtw_pci_rx_ring *rx_ring,
u8 desc_size, u32 len)
@@ -782,8 +799,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rtw_pci_dma_check(rtwdev, ring, cur_rp);
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
- pci_unmap_single(rtwpci->pdev, dma, RTK_PCI_RX_BUF_SIZE,
- PCI_DMA_FROMDEVICE);
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
rx_desc = skb->data;
chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
@@ -818,7 +835,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
next_rp:
/* new skb delivered to mac80211, re-enable original skb DMA */
- rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
+ rtw_pci_sync_rx_desc_device(rtwdev, dma, ring, cur_rp,
+ buf_desc_sz);
/* host read next element in ring */
if (++cur_rp >= ring->r.len)
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v2 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-10 8:54 UTC (permalink / raw)
To: Tony Chuang
Cc: Kalle Valo, David S . Miller, Larry Finger, David Laight,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux@endlessm.com, Daniel Drake,
stable@vger.kernel.org
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D1864779@RTITMBSVM04.realtek.com.tw>
Tony Chuang <yhchuang@realtek.com> 於 2019年7月10日 週三 下午4:37寫道：
>
> > Subject: [PATCH v2 1/2] rtw88: pci: Rearrange the memory usage for skb in
> > RX ISR
> >
> > Testing with RTL8822BE hardware, when available memory is low, we
> > frequently see a kernel panic and system freeze.
> >
> > First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
> >
> > rx routine starvation
> > WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822
> > rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> > [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> >
> > Then we see a variety of different error conditions and kernel panics,
> > such as this one (trimmed):
> >
> > rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> > skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415
> > head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0
> > dev:<NULL>
> > ------------[ cut here ]------------
> > kernel BUG at net/core/skbuff.c:105!
> > invalid opcode: 0000 [#1] SMP NOPTI
> > RIP: 0010:skb_panic+0x43/0x45
> >
> > When skb allocation fails and the "rx routine starvation" is hit, the
> > function returns immediately without updating the RX ring. At this
> > point, the RX ring may continue referencing an old skb which was already
> > handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> > bad things happen.
> >
> > This patch allocates a new, data-sized skb first in RX ISR. After
> > copying the data in, we pass it to the upper layers. However, if skb
> > allocation fails, we effectively drop the frame. In both cases, the
> > original, full size ring skb is reused.
> >
> > In addition, to fixing the kernel crash, the RX routine should now
> > generally behave better under low memory conditions.
> >
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
> > 1 file changed, 22 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> > b/drivers/net/wireless/realtek/rtw88/pci.c
> > index cfe05ba7280d..e9fe3ad896c8 100644
> > --- a/drivers/net/wireless/realtek/rtw88/pci.c
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> > struct rtw_pci *rtwpci,
> > u32 pkt_offset;
> > u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> > u32 buf_desc_sz = chip->rx_buf_desc_sz;
> > + u32 new_len;
> > u8 *rx_desc;
> > dma_addr_t dma;
> >
> > @@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev,
> > struct rtw_pci *rtwpci,
> > pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> > pkt_stat.shift;
> >
> > - if (pkt_stat.is_c2h) {
> > - /* keep rx_desc, halmac needs it */
> > - skb_put(skb, pkt_stat.pkt_len + pkt_offset);
> > + /* discard current skb if the new skb cannot be allocated as a
> > + * new one in rx ring later
> > + */
> > + new_len = pkt_stat.pkt_len + pkt_offset;
> > + new = dev_alloc_skb(new_len);
> > + if (WARN_ONCE(!new, "rx routine starvation\n"))
> > + goto next_rp;
> > +
> > + /* put the DMA data including rx_desc from phy to new skb */
> > + skb_put_data(new, skb->data, new_len);
> >
> > - /* pass offset for further operation */
> > - *((u32 *)skb->cb) = pkt_offset;
> > - skb_queue_tail(&rtwdev->c2h_queue, skb);
> > + if (pkt_stat.is_c2h) {
> > + /* pass rx_desc & offset for further operation */
> > + *((u32 *)new->cb) = pkt_offset;
> > + skb_queue_tail(&rtwdev->c2h_queue, new);
> > ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
> > } else {
> > - /* remove rx_desc, maybe use skb_pull? */
> > - skb_put(skb, pkt_stat.pkt_len);
> > - skb_reserve(skb, pkt_offset);
> > -
> > - /* alloc a smaller skb to mac80211 */
> > - new = dev_alloc_skb(pkt_stat.pkt_len);
> > - if (!new) {
> > - new = skb;
> > - } else {
> > - skb_put_data(new, skb->data, skb->len);
> > - dev_kfree_skb_any(skb);
> > - }
> > - /* TODO: merge into rx.c */
> > - rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> > + /* remove rx_desc */
> > + skb_pull(new, pkt_offset);
> > +
> > + rtw_rx_stats(rtwdev, pkt_stat.vif, new);
> > memcpy(new->cb, &rx_status, sizeof(rx_status));
> > ieee80211_rx_irqsafe(rtwdev->hw, new);
> > }
> >
> > - /* skb delivered to mac80211, alloc a new one in rx ring */
> > - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> > - if (WARN(!new, "rx routine starvation\n"))
> > - return;
> > -
> > - ring->buf[cur_rp] = new;
> > - rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> > +next_rp:
> > + /* new skb delivered to mac80211, re-enable original skb DMA */
> > + rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
> >
> > /* host read next element in ring */
> > if (++cur_rp >= ring->r.len)
> > --
> > 2.22.0
>
> Now it looks good to me. Thanks.
>
> Acked-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> Yan-Hsuan
Uh! Thanks for your ack.
But I just sent version 3 patches (including [PATCH v3 2/2] rtw88:
pci: Use DMA sync instead of remapping in RX ISR) by following
Christoph's comment. [1]
Could you please also review the 2 patches of version 3? Thank you.
[1]: https://lkml.org/lkml/2019/7/9/507
Jian-Hong Pan
^ permalink raw reply
* RE: [PATCH v3 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: David Laight @ 2019-07-10 8:57 UTC (permalink / raw)
To: 'Jian-Hong Pan', Yan-Hsuan Chuang, Kalle Valo,
David S . Miller, Larry Finger, Christoph Hellwig
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux@endlessm.com, Daniel Drake,
stable@vger.kernel.org
In-Reply-To: <20190710083825.7115-1-jian-hong@endlessm.com>
From: Jian-Hong Pan
> Sent: 10 July 2019 09:38
>
> Testing with RTL8822BE hardware, when available memory is low, we
> frequently see a kernel panic and system freeze.
>
> First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
>
> rx routine starvation
> WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822
> rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
>
> Then we see a variety of different error conditions and kernel panics,
> such as this one (trimmed):
>
> rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f
> data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:105!
> invalid opcode: 0000 [#1] SMP NOPTI
> RIP: 0010:skb_panic+0x43/0x45
>
> When skb allocation fails and the "rx routine starvation" is hit, the
> function returns immediately without updating the RX ring. At this
> point, the RX ring may continue referencing an old skb which was already
> handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> bad things happen.
>
> This patch allocates a new, data-sized skb first in RX ISR. After
> copying the data in, we pass it to the upper layers. However, if skb
> allocation fails, we effectively drop the frame. In both cases, the
> original, full size ring skb is reused.
>
> In addition, by fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
A couple of minor nits (see below).
You may want to do a followup patch that changes the rx buffers
(used by the hardware) to by just memory buffers.
Nothing (probably) relies on them being skb with all the accociated
baggage.
David
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
> v2:
> - Allocate new data-sized skb and put data into it, then pass it to
> mac80211. Reuse the original skb in RX ring by DMA sync.
> - Modify the commit message.
> - Introduce following [PATCH v3 2/2] rtw88: pci: Use DMA sync instead
> of remapping in RX ISR.
>
> v3:
> - Same as v2.
>
> drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
> 1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..e9fe3ad896c8 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> u32 pkt_offset;
> u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> u32 buf_desc_sz = chip->rx_buf_desc_sz;
> + u32 new_len;
> u8 *rx_desc;
> dma_addr_t dma;
>
> @@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> pkt_stat.shift;
>
> - if (pkt_stat.is_c2h) {
> - /* keep rx_desc, halmac needs it */
> - skb_put(skb, pkt_stat.pkt_len + pkt_offset);
> + /* discard current skb if the new skb cannot be allocated as a
> + * new one in rx ring later
> + */
That comment isn't quite right.
maybe: "Allocate a new skb for this frame, discard if none available"
> + new_len = pkt_stat.pkt_len + pkt_offset;
> + new = dev_alloc_skb(new_len);
> + if (WARN_ONCE(!new, "rx routine starvation\n"))
I think you should count these??
> + goto next_rp;
> +
> + /* put the DMA data including rx_desc from phy to new skb */
> + skb_put_data(new, skb->data, new_len);
>
> - /* pass offset for further operation */
> - *((u32 *)skb->cb) = pkt_offset;
> - skb_queue_tail(&rtwdev->c2h_queue, skb);
> + if (pkt_stat.is_c2h) {
> + /* pass rx_desc & offset for further operation */
> + *((u32 *)new->cb) = pkt_offset;
> + skb_queue_tail(&rtwdev->c2h_queue, new);
> ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
> } else {
> - /* remove rx_desc, maybe use skb_pull? */
> - skb_put(skb, pkt_stat.pkt_len);
> - skb_reserve(skb, pkt_offset);
> -
> - /* alloc a smaller skb to mac80211 */
> - new = dev_alloc_skb(pkt_stat.pkt_len);
> - if (!new) {
> - new = skb;
> - } else {
> - skb_put_data(new, skb->data, skb->len);
> - dev_kfree_skb_any(skb);
> - }
> - /* TODO: merge into rx.c */
> - rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> + /* remove rx_desc */
> + skb_pull(new, pkt_offset);
> +
> + rtw_rx_stats(rtwdev, pkt_stat.vif, new);
> memcpy(new->cb, &rx_status, sizeof(rx_status));
> ieee80211_rx_irqsafe(rtwdev->hw, new);
> }
>
> - /* skb delivered to mac80211, alloc a new one in rx ring */
> - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> - if (WARN(!new, "rx routine starvation\n"))
> - return;
> -
> - ring->buf[cur_rp] = new;
> - rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> +next_rp:
> + /* new skb delivered to mac80211, re-enable original skb DMA */
> + rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
>
> /* host read next element in ring */
> if (++cur_rp >= ring->r.len)
> --
> 2.22.0
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH bpf] selftests/bpf: fix bpf_target_sparc check
From: Ilya Leoshkevich @ 2019-07-10 8:59 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, Networking
In-Reply-To: <CAEf4BzZswDkvPbhNnovLjWWmmhR2VBWtrCJkpMXM8M_5Ztn4-w@mail.gmail.com>
> Am 09.07.2019 um 19:56 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Tue, Jul 9, 2019 at 8:22 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> bpf_helpers.h fails to compile on sparc: the code should be checking
>> for defined(bpf_target_sparc), but checks simply for bpf_target_sparc.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>> tools/testing/selftests/bpf/bpf_helpers.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
>> index 5f6f9e7aba2a..a8fea087aa90 100644
>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
>> @@ -443,7 +443,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
>> #ifdef bpf_target_powerpc
>
> While at it, can you please also fix this one?
Do you mean #ifdef bpf_target_powerpc? I think it’s correct, because it’s #ifdef, not #if.
But I could change it to #if defined() for consistency.
Best regards,
Ilya
^ permalink raw reply
* Re: [PATCH net-next,v4 05/12] net: flow_offload: add list handling functions
From: Jiri Pirko @ 2019-07-10 9:06 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
maxime.chevallier, cphealy, phil, netfilter-devel
In-Reply-To: <20190710073652.GD2282@nanopsycho>
Wed, Jul 10, 2019 at 09:36:52AM CEST, jiri@resnulli.us wrote:
>Tue, Jul 09, 2019 at 10:55:43PM CEST, pablo@netfilter.org wrote:
>
>[...]
>
>
>>@@ -176,6 +176,7 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
>> if (!block_cb)
>> return ERR_PTR(-ENOMEM);
>>
>>+ block_cb->net = net;
>> block_cb->cb = cb;
>> block_cb->cb_ident = cb_ident;
>> block_cb->cb_priv = cb_priv;
>>@@ -194,6 +195,22 @@ void flow_block_cb_free(struct flow_block_cb *block_cb)
>> }
>> EXPORT_SYMBOL(flow_block_cb_free);
>>
>>+struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
>>+ tc_setup_cb_t *cb, void *cb_ident)
>>+{
>>+ struct flow_block_cb *block_cb;
>>+
>>+ list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
>>+ if (block_cb->net == f->net &&
Looking at this a bit more, this is wrong. This breaks block sharing
concept. The original lookup look up the block_cb in certain block - the
block to be shared. With this, you broke the block sharing feature for
mlxsw.
We need to maintain the existing block concept (changed to flow_block).
>
>I don't understand why you need net for this. You should have a list of
>cbs per subsystem (tc/nft) go over it here.
>
>The clash of 2 suybsytems is prevented later on by
>flow_block_cb_is_busy().
>
>Am I missing something?
>If not, could you please remove use of net from flow_block_cb_alloc()
>and from here and replace it by some shared flow structure holding the
>cb list that would be used by both tc and nft?
>
>
>
>>+ block_cb->cb == cb &&
>>+ block_cb->cb_ident == cb_ident)
>>+ return block_cb;
>>+ }
>>+
>>+ return NULL;
>>+}
>>+EXPORT_SYMBOL(flow_block_cb_lookup);
>>+
>
>[...]
^ permalink raw reply
* Re: [PATCH 00/12] treewide: Fix GENMASK misuses
From: Johannes Berg @ 2019-07-10 9:17 UTC (permalink / raw)
To: Joe Perches, Andrew Morton, Patrick Venture, Nancy Yuen,
Benjamin Fair, Andrew Jeffery, openbmc, linux-kernel,
linux-aspeed, linux-arm-kernel, linux-amlogic, netdev,
linux-mediatek, linux-stm32, linux-wireless, linux-media
Cc: dri-devel, linux-iio, linux-mmc, devel, alsa-devel
In-Reply-To: <cover.1562734889.git.joe@perches.com>
On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
> These GENMASK uses are inverted argument order and the
> actual masks produced are incorrect. Fix them.
>
> Add checkpatch tests to help avoid more misuses too.
>
> Joe Perches (12):
> checkpatch: Add GENMASK tests
IMHO this doesn't make a lot of sense as a checkpatch test - just throw
in a BUILD_BUG_ON()?
johannes
^ permalink raw reply
* Re: [PATCH net-next v3] net/mlx5e: Convert single case statement switch statements into if statements
From: Leon Romanovsky @ 2019-07-10 9:31 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Saeed Mahameed, David S. Miller, Boris Pismenny, netdev,
linux-rdma, linux-kernel, clang-built-linux, Nick Desaulniers
In-Reply-To: <20190710060614.6155-1-natechancellor@gmail.com>
On Tue, Jul 09, 2019 at 11:06:15PM -0700, Nathan Chancellor wrote:
> During the review of commit 1ff2f0fa450e ("net/mlx5e: Return in default
> case statement in tx_post_resync_params"), Leon and Nick pointed out
> that the switch statements can be converted to single if statements
> that return early so that the code is easier to follow.
>
> Suggested-by: Leon Romanovsky <leon@kernel.org>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
Thanks again,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
^ permalink raw reply
* [PATCH net-next] net: mlx5: Fix compiling error in tls.c
From: Mao Wenan @ 2019-07-10 9:38 UTC (permalink / raw)
To: davem, saeedm; +Cc: netdev, linux-kernel, Mao Wenan
There are some errors while compiling tls.c if
CONFIG_MLX5_FPGA_TLS is not obvious on.
drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls.c: In function mlx5e_tls_set_ipv4_flow:
./include/linux/mlx5/device.h:61:39: error: invalid application of sizeof to incomplete type struct mlx5_ifc_tls_flow_bits
#define __mlx5_st_sz_bits(typ) sizeof(struct mlx5_ifc_##typ##_bits)
^
./include/linux/compiler.h:330:9: note: in definition of macro __compiletime_assert
if (!(condition)) \
^~~~~~~~~
...
drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls.c: In function mlx5e_tls_build_netdev:
drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls.c:202:13: error: MLX5_ACCEL_TLS_TX undeclared (first use in this function); did you mean __MLX5_ACCEL_TLS_H__?
if (caps & MLX5_ACCEL_TLS_TX) {
^~~~~~~~~~~~~~~~~
__MLX5_ACCEL_TLS_H__
drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls.c:207:13: error: MLX5_ACCEL_TLS_RX undeclared (first use in this function); did you mean MLX5_ACCEL_TLS_TX?
if (caps & MLX5_ACCEL_TLS_RX) {
^~~~~~~~~~~~~~~~~
MLX5_ACCEL_TLS_TX
drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls.c:212:15: error: MLX5_ACCEL_TLS_LRO undeclared (first use in this function); did you mean MLX5_ACCEL_TLS_RX?
if (!(caps & MLX5_ACCEL_TLS_LRO)) {
^~~~~~~~~~~~~~~~~~
MLX5_ACCEL_TLS_RX
make[5]: *** [drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [drivers/net/ethernet/mellanox/mlx5/core] Error 2
make[3]: *** [drivers/net/ethernet/mellanox] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [drivers/net/ethernet] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [drivers/net] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2
make: *** Waiting for unfinished jobs....
this patch is to fix this error using 'depends on MLX5_FPGA_TLS' when MLX5_TLS is set.
Fixes: e2869fb2068b ("net/mlx5: Kconfig, Better organize compilation flags")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 37fef8c..1da2770 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -139,6 +139,7 @@ config MLX5_TLS
depends on MLX5_CORE_EN
depends on TLS_DEVICE
depends on TLS=y || MLX5_CORE=m
+ depends on MLX5_FPGA_TLS
select MLX5_ACCEL
default n
help
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net v2] Validate required parameters in inet6_validate_link_af
From: Maxim Mikityanskiy @ 2019-07-10 9:36 UTC (permalink / raw)
To: Stefan Lippers-Hollmann
Cc: David Miller, jakub.kicinski@netronome.com, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org, netdev@vger.kernel.org, Leon Romanovsky
In-Reply-To: <20190709031135.123a5b19@mir>
On 2019-07-09 04:11, Stefan Lippers-Hollmann wrote:
> Hi
>
> On 2019-05-22, David Miller wrote:
>> From: Maxim Mikityanskiy <maximmi@mellanox.com>
>> Date: Tue, 21 May 2019 06:40:04 +0000
>>
>>> inet6_set_link_af requires that at least one of IFLA_INET6_TOKEN or
>>> IFLA_INET6_ADDR_GET_MODE is passed. If none of them is passed, it
>>> returns -EINVAL, which may cause do_setlink() to fail in the middle of
>>> processing other commands and give the following warning message:
>>>
>>> A link change request failed with some changes committed already.
>>> Interface eth0 may have been left with an inconsistent configuration,
>>> please check.
>>>
>>> Check the presence of at least one of them in inet6_validate_link_af to
>>> detect invalid parameters at an early stage, before do_setlink does
>>> anything. Also validate the address generation mode at an early stage.
>>>
>>> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
>>
>> Applied, thank you.
>
> After updating from kernel 5.1.16 to 5.2, I noticed that my
> systemd-networkd (241-5, Debian/unstable) managed bridges didn't
> come up and needed a manual "ip link set dev br-lan up" to get
> configured. Bisecting between v5.1 and v5.2 pointed to this
> patch and reverting just this change from v5.2 fixes the issue
> for me again.
This patch changes behavior only in case of invalid input. If the
userspace sends a valid message over netlink, nothing changes after my
patch. However, for some subset of invalid inputs, it used to be
undefined behavior, and the kernel used to apply partial configuration
before it noticed that the input was invalid and failed. After my patch,
this subset of invalid inputs is handled properly, resulting in an
immediate error returned to the userspace, and no configuration is
affected. So, my patch is actually a bug fix.
Unfortunately, commit [1] introduced a regression in systemd, and it
started sending invalid input to the kernel, apparently didn't pay
attention to the error returned and relied on the undefined behavior
(the partial configuration update that took place before my patch).
Later on, commit [2] was introduced, and it should fix that regression
in systemd.
What you experience may be explained by this bug in systemd:
1. systemd broke, but the issue remained unnoticed, because some part of
the configuration was still applied.
2. The bug in systemd was eventually fixed, but apparently you haven't
updated to the version that has this fix yet.
3. My fix for the kernel was merged.
4. As you are using a systemd without fix, the issue led to more severe
consequences, because now no configuration is applied after an invalid
request from systemd.
I haven't tried to reproduce your configuration though, but I guess the
things above are what has happened. I suggest you to update systemd to a
version that has commit [2] (or to build it from master if no newer
version has been released since then) - I hope it solves your issue.
Otherwise, let me know.
[1]:
https://github.com/systemd/systemd/commit/0e2fdb83bb5e22047e0c7cc058b415d0e93f02cf
[2]:
https://github.com/systemd/systemd/commit/4d48747c43922250a62cf6e0ad9ee364665ef82d
> $ git bisect start
> # good: [e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd] Linux 5.1
> git bisect good e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
> # bad: [46713c3d2f8da5e3d8ddd2249bcb1d9974fb5d28] Merge tag 'for-linus-20190706' of git://git.kernel.dk/linux-block
> git bisect bad 46713c3d2f8da5e3d8ddd2249bcb1d9974fb5d28
> # good: [a2d635decbfa9c1e4ae15cb05b68b2559f7f827c] Merge tag 'drm-next-2019-05-09' of git://anongit.freedesktop.org/drm/drm
> git bisect good a2d635decbfa9c1e4ae15cb05b68b2559f7f827c
> # good: [22c58fd70ca48a29505922b1563826593b08cc00] Merge tag 'armsoc-soc' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
> git bisect good 22c58fd70ca48a29505922b1563826593b08cc00
> # good: [61939b12dc24d0ac958020f261046c35a16e0c48] block: print offending values when cloned rq limits are exceeded
> git bisect good 61939b12dc24d0ac958020f261046c35a16e0c48
> # bad: [3510955b327176fd4cbab5baa75b449f077722a2] mm/list_lru.c: fix memory leak in __memcg_init_list_lru_node
> git bisect bad 3510955b327176fd4cbab5baa75b449f077722a2
> # bad: [30d1d92a888d03681b927c76a35181b4eed7071f] Merge tag 'nds32-for-linux-5.2-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/greentime/linux
> git bisect bad 30d1d92a888d03681b927c76a35181b4eed7071f
> # bad: [dbde71df810c62e72e2aa6d88a0686a6092956cd] Merge tag 'tty-5.2-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty
> git bisect bad dbde71df810c62e72e2aa6d88a0686a6092956cd
> # bad: [100f6d8e09905c59be45b6316f8f369c0be1b2d8] net: correct zerocopy refcnt with udp MSG_MORE
> git bisect bad 100f6d8e09905c59be45b6316f8f369c0be1b2d8
> # bad: [4ca6dee5220fe2377bf12b354ef85978425c9ec7] dpaa2-eth: Make constant 64-bit long
> git bisect bad 4ca6dee5220fe2377bf12b354ef85978425c9ec7
> # bad: [b5730061d1056abf317caea823b94d6e12b5b4f6] cxgb4: offload VLAN flows regardless of VLAN ethtype
> git bisect bad b5730061d1056abf317caea823b94d6e12b5b4f6
> # bad: [c1e85c6ce57ef1eb73966152993a341c8123a8ea] net: macb: save/restore the remaining registers and features
> git bisect bad c1e85c6ce57ef1eb73966152993a341c8123a8ea
> # bad: [f42c104f2ec94a9255a835cd4cd1bd76279d4d06] Documentation: add TLS offload documentation
> git bisect bad f42c104f2ec94a9255a835cd4cd1bd76279d4d06
> # bad: [d008b3d2be4b00267e7af5c21269e7af4f65c6e2] mISDN: Fix indenting in dsp_cmx.c
> git bisect bad d008b3d2be4b00267e7af5c21269e7af4f65c6e2
> # bad: [40a1578d631a8ac1cf0ef797c435114107747859] ocelot: Dont allocate another multicast list, use __dev_mc_sync
> git bisect bad 40a1578d631a8ac1cf0ef797c435114107747859
> # bad: [7dc2bccab0ee37ac28096b8fcdc390a679a15841] Validate required parameters in inet6_validate_link_af
> git bisect bad 7dc2bccab0ee37ac28096b8fcdc390a679a15841
> # first bad commit: [7dc2bccab0ee37ac28096b8fcdc390a679a15841] Validate required parameters in inet6_validate_link_af
>
> While I originally noticed this issue on real hardware (r8169, e1000,
> e1000e, e100, alx) and multiple systems with a slightly complex bridge
> setup, I can reproduce it with a very basic configuration under kvm
> (upon which all the tests below are based):
>
> $ cat /etc/systemd/network/20-wired.network
> [Match]
> Name=ens4
>
> [Network]
> DHCP=yes
>
> (same results with just DHCP=ipv4)
>
> With the above systemd-networkd configuration, the system comes up
> without network access:
>
> # ip a
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> inet 127.0.0.1/8 scope host lo
> valid_lft forever preferred_lft forever
> inet6 ::1/128 scope host
> valid_lft forever preferred_lft forever
> 2: ens4: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
> link/ether 00:16:3e:00:00:00 brd ff:ff:ff:ff:ff:ff
>
> # networkctl | cat -
> IDX LINK TYPE OPERATIONAL SETUP
> 1 lo loopback carrier unmanaged
> 2 ens4 ether off configuring
>
> 2 links listed.
>
> Manually enabling the interface does help:
>
> # ip link set dev ens4 up
>
> # ip a
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> inet 127.0.0.1/8 scope host lo
> valid_lft forever preferred_lft forever
> inet6 ::1/128 scope host
> valid_lft forever preferred_lft forever
> 2: ens4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
> link/ether 00:16:3e:00:00:00 brd ff:ff:ff:ff:ff:ff
> inet 172.23.6.0/14 brd 172.23.255.255 scope global dynamic ens4
> valid_lft 43199sec preferred_lft 43199sec
> inet6 2003:xxxx:xxxx:xxxx::197/128 scope global tentative dynamic noprefixroute
> valid_lft 13809sec preferred_lft 1209sec
> inet6 fdxx:xxxx:xxxx::197/128 scope global tentative noprefixroute
> valid_lft forever preferred_lft forever
> inet6 fdxx:xxxx:xxxx:0:216:3eff:fe00:0/64 scope global tentative mngtmpaddr noprefixroute
> valid_lft forever preferred_lft forever
> inet6 2003:xxxx:xxxx:xxxx:216:3eff:fe00:0/64 scope global tentative dynamic mngtmpaddr noprefixroute
> valid_lft 13809sec preferred_lft 1209sec
> inet6 fe80::216:3eff:fe00:0/64 scope link
> valid_lft forever preferred_lft forever
>
> # networkctl | cat -
> IDX LINK TYPE OPERATIONAL SETUP
> 1 lo loopback carrier unmanaged
> 2 ens4 ether routable configured
>
> 2 links listed.
>
> A quick test of upgrading all systemd packages to 242-2 from
> Debian/experimental shows the same issue; Debian 10/ buster (stable)
> is shipping with systemd 241-5.
>
> DHCPv4 is served by a recent OpenWrt/ master snapshot on ipq8065/ nbg6817
> (ARMv7), using dnsmasq 2.80-13 and odhcpd-ipv6only 2019-05-17-41a74cba-3
> covering DHCPv6 and prefix delegation.
>
> Attached are xz compressed versions of the kernel configuration (amd64),
> dmesg and journalctl output.
>
> The Debian/unstable VM was started with qemu-kvm 1:3.1+dfsg-8 on a
> Debian/unstable host running kernel 5.2 with this patch reverted:
>
> $ QEMU_AUDIO_DRV=pa qemu-system-x86_64 \
> -machine accel=kvm:tcg \
> -monitor stdio \
> -rtc base=localtime \
> -cpu qemu64,+vmx \
> -smp 3 \
> -m 4096 \
> -device virtio-gpu-pci \
> -device virtio-net-pci,mac=00:16:3E:00:00:00,netdev=tap-br-lan0 \
> -netdev tap,ifname=tap-br-lan0,script=no,id=tap-br-lan0 \
> -device AC97 \
> -drive file=/srv/storage/vm/linux.qcow2.img,if=none,discard=unmap,index=0,media=disk,id=hd0 \
> -device virtio-scsi-pci,id=scsi -device scsi-hd,drive=hd0 \
> -usb \
> -device usb-tablet \
> -device usb-ehci,id=ehci \
> -device nec-usb-xhci,id=xhci \
> -device virtio-rng-pci \
> -boot menu=on
>
> Regards
> Stefan Lippers-Hollmann
>
^ permalink raw reply
* Re: [PATCH 00/12] treewide: Fix GENMASK misuses
From: Russell King - ARM Linux admin @ 2019-07-10 9:43 UTC (permalink / raw)
To: Johannes Berg
Cc: Joe Perches, Andrew Morton, Patrick Venture, Nancy Yuen,
Benjamin Fair, Andrew Jeffery, openbmc, linux-kernel,
linux-aspeed, linux-arm-kernel, linux-amlogic, netdev,
linux-mediatek, linux-stm32, linux-wireless, linux-media,
linux-iio, devel, alsa-devel, linux-mmc, dri-devel
In-Reply-To: <5fa1fa6998332642c49e2d5209193ffe2713f333.camel@sipsolutions.net>
On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote:
> On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
> > These GENMASK uses are inverted argument order and the
> > actual masks produced are incorrect. Fix them.
> >
> > Add checkpatch tests to help avoid more misuses too.
> >
> > Joe Perches (12):
> > checkpatch: Add GENMASK tests
>
> IMHO this doesn't make a lot of sense as a checkpatch test - just throw
> in a BUILD_BUG_ON()?
My personal take on this is that GENMASK() is really not useful, it's
just pure obfuscation and leads to exactly these kinds of mistakes.
Yes, I fully understand the argument that you can just specify the
start and end bits, and it _in theory_ makes the code more readable.
However, the problem is when writing code. GENMASK(a, b). Is a the
starting bit or ending bit? Is b the number of bits? It's confusing
and causes mistakes resulting in incorrect code. A BUILD_BUG_ON()
can catch some of the cases, but not all of them.
For example:
GENMASK(6, 2)
would satisify the requirement that a > b, so a BUILD_BUG_ON() will
not trigger, but was the author meaning 0x3c or 0xc0?
Personally, I've decided I am _not_ going to use GENMASK() in my code
because I struggle to get the macro arguments correct - I'm _much_
happier, and it is way more reliable for me to write the mask in hex
notation.
I think this is where use of a ternary operator would come in use. The
normal way of writing a number of bits tends to be "a:b", so if GENMASK
took something like GENMASK(6:2), then I'd have less issue with it,
because it's argument is then in a familiar notation.
Yes, I'm sure that someone will point out that the GENMASK arguments
are just in the same order, but that doesn't prevent _me_ frequently
getting it wrong - and that's the point. The macro seems to me to
cause more problems than it solves.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ 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