* [PATCH net-next 3/3] netfilter: nft_fib: Convert nft_fib4_eval to new dev helper
From: dsahern @ 2018-09-20 20:50 UTC (permalink / raw)
To: netdev, netfilter-devel; +Cc: pablo, fw, David Ahern
In-Reply-To: <20180920205049.15143-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Convert nft_fib4_eval to the new device checking helper and
remove the duplicate code.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv4/netfilter/nft_fib_ipv4.c | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
index e50976e3c213..94eb25bc8d7e 100644
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -76,10 +76,7 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
.flowi4_iif = LOOPBACK_IFINDEX,
};
const struct net_device *oif;
- struct net_device *found;
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
- int i;
-#endif
+ const struct net_device *found;
/*
* Do not set flowi4_oif, it restricts results (for example, asking
@@ -146,25 +143,13 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
if (!oif) {
found = FIB_RES_DEV(res);
- goto ok;
- }
-
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
- for (i = 0; i < res.fi->fib_nhs; i++) {
- struct fib_nh *nh = &res.fi->fib_nh[i];
+ } else {
+ if (!fib_info_nh_uses_dev(res.fi, oif))
+ return;
- if (nh->nh_dev == oif) {
- found = nh->nh_dev;
- goto ok;
- }
+ found = oif;
}
- return;
-#else
- found = FIB_RES_DEV(res);
- if (found != oif)
- return;
-#endif
-ok:
+
switch (priv->result) {
case NFT_FIB_RESULT_OIF:
*dest = found->ifindex;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 0/3] net: wean netfilter from fib_nh
From: Florian Westphal @ 2018-09-20 20:53 UTC (permalink / raw)
To: dsahern; +Cc: netdev, netfilter-devel, pablo, fw, David Ahern
In-Reply-To: <20180920205049.15143-1-dsahern@kernel.org>
dsahern@kernel.org <dsahern@kernel.org> wrote:
> From: David Ahern <dsahern@gmail.com>
>
> Two netfilter modules reference fib_nh. In both cases the code is
> only checking if a nexthop in a fib_info uses a specific device.
> Both instances essentially duplicate code from __fib_validate_source,
> so move that code into a helper and flip the netfilter modules to
> use it.
Lokks good to me, thanks David.
^ permalink raw reply
* KMSAN: uninit-value in IP6_ECN_decapsulate
From: syzbot @ 2018-09-20 20:54 UTC (permalink / raw)
To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 88e0e95b30f1 kmsan: add a newline before "Uninit was creat..
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=146dfccf800000
kernel config: https://syzkaller.appspot.com/x/.config?x=848e40757852af3e
dashboard link: https://syzkaller.appspot.com/bug?extid=bf7e6250c7ce248f3ec9
compiler: clang version 7.0.0 (trunk 334104)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=177a9ce4400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14f68bd8400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bf7e6250c7ce248f3ec9@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KMSAN: uninit-value in INET_ECN_decapsulate include/net/inet_ecn.h:189
[inline]
BUG: KMSAN: uninit-value in IP6_ECN_decapsulate+0x421/0x970
include/net/inet_ecn.h:234
CPU: 0 PID: 4515 Comm: syz-executor162 Not tainted 4.17.0+ #8
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x185/0x1d0 lib/dump_stack.c:113
kmsan_report+0x188/0x2a0 mm/kmsan/kmsan.c:1122
__msan_warning_32+0x70/0xc0 mm/kmsan/kmsan_instr.c:620
INET_ECN_decapsulate include/net/inet_ecn.h:189 [inline]
IP6_ECN_decapsulate+0x421/0x970 include/net/inet_ecn.h:234
ip6ip6_dscp_ecn_decapsulate+0x1e0/0x250 net/ipv6/ip6_tunnel.c:719
__ip6_tnl_rcv+0xff9/0x1a10 net/ipv6/ip6_tunnel.c:829
ip6_tnl_rcv+0xe6/0x110 net/ipv6/ip6_tunnel.c:868
gre_rcv+0x1661/0x1a90 net/ipv6/ip6_gre.c:534
ip6_input_finish+0x1353/0x2260 net/ipv6/ip6_input.c:284
NF_HOOK include/linux/netfilter.h:288 [inline]
ip6_input+0x294/0x320 net/ipv6/ip6_input.c:327
dst_input include/net/dst.h:450 [inline]
ip6_rcv_finish+0x498/0x6e0 net/ipv6/ip6_input.c:71
NF_HOOK include/linux/netfilter.h:288 [inline]
ipv6_rcv+0x1d6b/0x2360 net/ipv6/ip6_input.c:208
__netif_receive_skb_core+0x47f3/0x4aa0 net/core/dev.c:4592
__netif_receive_skb net/core/dev.c:4657 [inline]
process_backlog+0x62d/0xe20 net/core/dev.c:5337
napi_poll net/core/dev.c:5735 [inline]
net_rx_action+0x766/0x1a80 net/core/dev.c:5801
__do_softirq+0x592/0x979 kernel/softirq.c:285
do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:1046
</IRQ>
do_softirq kernel/softirq.c:329 [inline]
__local_bh_enable_ip+0x114/0x140 kernel/softirq.c:182
local_bh_enable+0x36/0x40 include/linux/bottom_half.h:32
rcu_read_unlock_bh include/linux/rcupdate.h:728 [inline]
ip6_finish_output2+0x1ce8/0x2100 net/ipv6/ip6_output.c:121
ip6_finish_output+0xaf0/0xbb0 net/ipv6/ip6_output.c:154
NF_HOOK_COND include/linux/netfilter.h:277 [inline]
ip6_output+0x597/0x6c0 net/ipv6/ip6_output.c:171
dst_output include/net/dst.h:444 [inline]
ip6_local_out+0x164/0x1d0 net/ipv6/output_core.c:176
ip6_send_skb net/ipv6/ip6_output.c:1703 [inline]
ip6_push_pending_frames+0x218/0x4d0 net/ipv6/ip6_output.c:1723
rawv6_push_pending_frames net/ipv6/raw.c:616 [inline]
rawv6_sendmsg+0x4254/0x4fc0 net/ipv6/raw.c:935
inet_sendmsg+0x3fc/0x760 net/ipv4/af_inet.c:798
sock_sendmsg_nosec net/socket.c:629 [inline]
sock_sendmsg net/socket.c:639 [inline]
sock_write_iter+0x3bc/0x470 net/socket.c:908
call_write_iter include/linux/fs.h:1784 [inline]
new_sync_write fs/read_write.c:474 [inline]
__vfs_write+0x808/0x9f0 fs/read_write.c:487
vfs_write+0x467/0x8c0 fs/read_write.c:549
ksys_write fs/read_write.c:598 [inline]
__do_sys_write fs/read_write.c:610 [inline]
__se_sys_write fs/read_write.c:607 [inline]
__x64_sys_write+0x1bf/0x3e0 fs/read_write.c:607
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x441199
RSP: 002b:00007fff83b55688 EFLAGS: 00000217 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000441199
RDX: 0000000000000004 RSI: 00000000200001c0 RDI: 0000000000000003
RBP: 00000000006cc018 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000217 R12: 00000000004020a0
R13: 0000000000402130 R14: 0000000000000000 R15: 0000000000000000
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:279 [inline]
kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:189
kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:315
kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan.c:322
slab_post_alloc_hook mm/slab.h:446 [inline]
slab_alloc_node mm/slub.c:2753 [inline]
__kmalloc_node_track_caller+0xb35/0x11b0 mm/slub.c:4395
__kmalloc_reserve net/core/skbuff.c:138 [inline]
__alloc_skb+0x2cb/0x9e0 net/core/skbuff.c:206
alloc_skb include/linux/skbuff.h:988 [inline]
__ip6_append_data+0x364d/0x4fb0 net/ipv6/ip6_output.c:1434
ip6_append_data+0x40e/0x6b0 net/ipv6/ip6_output.c:1597
rawv6_sendmsg+0x2756/0x4fc0 net/ipv6/raw.c:928
inet_sendmsg+0x3fc/0x760 net/ipv4/af_inet.c:798
sock_sendmsg_nosec net/socket.c:629 [inline]
sock_sendmsg net/socket.c:639 [inline]
sock_write_iter+0x3bc/0x470 net/socket.c:908
call_write_iter include/linux/fs.h:1784 [inline]
new_sync_write fs/read_write.c:474 [inline]
__vfs_write+0x808/0x9f0 fs/read_write.c:487
vfs_write+0x467/0x8c0 fs/read_write.c:549
ksys_write fs/read_write.c:598 [inline]
__do_sys_write fs/read_write.c:610 [inline]
__se_sys_write fs/read_write.c:607 [inline]
__x64_sys_write+0x1bf/0x3e0 fs/read_write.c:607
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
==================================================================
---
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#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH net-next] net: micrel: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 2:42 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, netdev, krzk, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/micrel/ks8695net.c | 2 +-
drivers/net/ethernet/micrel/ks8851_mll.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/micrel/ks8695net.c b/drivers/net/ethernet/micrel/ks8695net.c
index bd51e05..b881f5d 100644
--- a/drivers/net/ethernet/micrel/ks8695net.c
+++ b/drivers/net/ethernet/micrel/ks8695net.c
@@ -1164,7 +1164,7 @@ static int ks8695_poll(struct napi_struct *napi, int budget)
* sk_buff and adds it to the TX ring. It then kicks the TX DMA
* engine to ensure transmission begins.
*/
-static int
+static netdev_tx_t
ks8695_start_xmit(struct sk_buff *skb, struct net_device *ndev)
{
struct ks8695_priv *ksp = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c
index 0e9719f..35f8c9e 100644
--- a/drivers/net/ethernet/micrel/ks8851_mll.c
+++ b/drivers/net/ethernet/micrel/ks8851_mll.c
@@ -1021,9 +1021,9 @@ static void ks_write_qmu(struct ks_net *ks, u8 *pdata, u16 len)
* spin_lock_irqsave is required because tx and rx should be mutual exclusive.
* So while tx is in-progress, prevent IRQ interrupt from happenning.
*/
-static int ks_start_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t ks_start_xmit(struct sk_buff *skb, struct net_device *netdev)
{
- int retv = NETDEV_TX_OK;
+ netdev_tx_t retv = NETDEV_TX_OK;
struct ks_net *ks = netdev_priv(netdev);
disable_irq(netdev->irq);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] ixgbe: check return value of napi_complete_done()
From: Jeff Kirsher @ 2018-09-20 21:01 UTC (permalink / raw)
To: Eric Dumazet, Song Liu, netdev; +Cc: intel-wired-lan, kernel-team, stable
In-Reply-To: <c1251d5d-4b18-a1de-acd2-6de829b7c7ac@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]
On Thu, 2018-09-20 at 13:35 -0700, Eric Dumazet wrote:
> On 09/20/2018 12:01 PM, Song Liu wrote:
> > The NIC driver should only enable interrupts when napi_complete_done()
> > returns true. This patch adds the check for ixgbe.
> >
> > Cc: stable@vger.kernel.org # 4.10+
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Song Liu <songliubraving@fb.com>
> > ---
>
>
> Well, unfortunately we do not know why this is needed,
> this is why I have not yet sent this patch formally.
>
> netpoll has correct synchronization :
>
> poll_napi() places into napi->poll_owner current cpu number before
> calling poll_one_napi()
>
> netpoll_poll_lock() does also use napi->poll_owner
>
> When netpoll calls ixgbe poll() method, it passed a budget of 0,
> meaning napi_complete_done() is not called.
>
> As long as we can not explain the problem properly in the changelog,
> we should investigate, otherwise we will probably see coming dozens of
> patches
> trying to fix a 'potential hazard'.
Agreed, which is why I have our validation and developers looking into it,
while we test the current patch from Song.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* general protection fault in ctnetlink_alloc_filter
From: syzbot @ 2018-09-20 21:04 UTC (permalink / raw)
To: coreteam, davem, fw, kadlec, linux-kernel, netdev,
netfilter-devel, pablo, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 3eb5358079d3 Add linux-next specific files for 20180918
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=171ce6f1400000
kernel config: https://syzkaller.appspot.com/x/.config?x=786006c5dafbadf6
dashboard link: https://syzkaller.appspot.com/bug?extid=e45eda8eda6e93a03959
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=114f76fa400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=102ed6c6400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+e45eda8eda6e93a03959@syzkaller.appspotmail.com
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: 5551 Comm: syz-executor610 Not tainted
4.19.0-rc4-next-20180918+ #74
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:nla_get_be32 include/net/netlink.h:1082 [inline]
RIP: 0010:ctnetlink_alloc_filter+0xb9/0x200
net/netfilter/nf_conntrack_netlink.c:843
Code: 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 57 01 00 00 48 b8 00 00 00 00 00
fc ff df 4d 8b 6c 24 40 49 8d 7d 04 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48
89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 06
RSP: 0018:ffff8801c48f71a0 EFLAGS: 00010247
RAX: dffffc0000000000 RBX: ffff8801d7acd580 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff864dfefd RDI: 0000000000000004
RBP: ffff8801c48f71b8 R08: ffff8801d957a180 R09: ffffed003b585b57
R10: ffffed003b585b57 R11: ffff8801dac2dabb R12: ffff8801c48f7500
R13: 0000000000000000 R14: ffff8801d90ef2b8 R15: ffff8801d90ef291
FS: 0000000000a90880(0000) GS:ffff8801dac00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000100 CR3: 00000001d9104000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
ctnetlink_start+0x10b/0x1b0 net/netfilter/nf_conntrack_netlink.c:857
__netlink_dump_start+0x43e/0x6f0 net/netlink/af_netlink.c:2312
netlink_dump_start include/linux/netlink.h:213 [inline]
ctnetlink_get_conntrack+0x777/0x9f0
net/netfilter/nf_conntrack_netlink.c:1320
nfnetlink_rcv_msg+0xdd3/0x10c0 net/netfilter/nfnetlink.c:228
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2447
nfnetlink_rcv+0x1c0/0x4d0 net/netfilter/nfnetlink.c:560
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x5a5/0x760 net/netlink/af_netlink.c:1336
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:622 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:632
___sys_sendmsg+0x7fd/0x930 net/socket.c:2117
__sys_sendmsg+0x11d/0x280 net/socket.c:2155
__do_sys_sendmsg net/socket.c:2164 [inline]
__se_sys_sendmsg net/socket.c:2162 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2162
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4400d9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff461c2298 EFLAGS: 00000213 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004400d9
RDX: 0000000000000000 RSI: 0000000020d65000 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
R10: 0000000000000000 R11: 0000000000000213 R12: 0000000000401960
R13: 00000000004019f0 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
---[ end trace 37c3db5bd5270e98 ]---
RIP: 0010:nla_get_be32 include/net/netlink.h:1082 [inline]
RIP: 0010:ctnetlink_alloc_filter+0xb9/0x200
net/netfilter/nf_conntrack_netlink.c:843
Code: 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 57 01 00 00 48 b8 00 00 00 00 00
fc ff df 4d 8b 6c 24 40 49 8d 7d 04 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48
89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 06
RSP: 0018:ffff8801c48f71a0 EFLAGS: 00010247
RAX: dffffc0000000000 RBX: ffff8801d7acd580 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff864dfefd RDI: 0000000000000004
RBP: ffff8801c48f71b8 R08: ffff8801d957a180 R09: ffffed003b585b57
R10: ffffed003b585b57 R11: ffff8801dac2dabb R12: ffff8801c48f7500
R13: 0000000000000000 R14: ffff8801d90ef2b8 R15: ffff8801d90ef291
FS: 0000000000a90880(0000) GS:ffff8801dac00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000100 CR3: 00000001d9104000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
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#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH net-next] net: freescale: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 2:50 UTC (permalink / raw)
To: davem, madalin.bucur, pantelis.antoniou, claudiu.manoil,
leoyang.li
Cc: linux-kernel, netdev, linuxppc-dev, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 3 ++-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 3 ++-
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 3 ++-
drivers/net/ethernet/freescale/gianfar.c | 4 ++--
drivers/net/ethernet/freescale/ucc_geth.c | 3 ++-
5 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index a5131a5..84843de 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2044,7 +2044,8 @@ static inline int dpaa_xmit(struct dpaa_priv *priv,
return 0;
}
-static int dpaa_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
+static netdev_tx_t
+dpaa_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
{
const int queue_mapping = skb_get_queue_mapping(skb);
bool nonlinear = skb_is_nonlinear(skb);
diff --git a/drivers/net/ethernet/freescale/fec_mpc52xx.c b/drivers/net/ethernet/freescale/fec_mpc52xx.c
index 6d7269d..b90bab7 100644
--- a/drivers/net/ethernet/freescale/fec_mpc52xx.c
+++ b/drivers/net/ethernet/freescale/fec_mpc52xx.c
@@ -305,7 +305,8 @@ static int mpc52xx_fec_close(struct net_device *dev)
* invariant will hold if you make sure that the netif_*_queue()
* calls are done at the proper times.
*/
-static int mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t
+mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
struct bcom_fec_bd *bd;
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 2c2976a..7c548ed 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -481,7 +481,8 @@ static struct sk_buff *tx_skb_align_workaround(struct net_device *dev,
}
#endif
-static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t
+fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct fs_enet_private *fep = netdev_priv(dev);
cbd_t __iomem *bdp;
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index c488d31..0bd21a4 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -110,7 +110,7 @@
const char gfar_driver_version[] = "2.0";
static int gfar_enet_open(struct net_device *dev);
-static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
static void gfar_reset_task(struct work_struct *work);
static void gfar_timeout(struct net_device *dev);
static int gfar_close(struct net_device *dev);
@@ -2332,7 +2332,7 @@ static inline bool gfar_csum_errata_76(struct gfar_private *priv,
/* This is called by the kernel when a frame is ready for transmission.
* It is pointed to by the dev->hard_start_xmit function pointer
*/
-static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
struct gfar_priv_tx_q *tx_queue = NULL;
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 9600837..32e0270 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3078,7 +3078,8 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
/* This is called by the kernel when a frame is ready for transmission. */
/* It is pointed to by the dev->hard_start_xmit function pointer */
-static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t
+ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
#ifdef CONFIG_UGETH_TX_ON_DEMAND
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] net: seeq: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 2:53 UTC (permalink / raw)
To: davem, linux; +Cc: linux-kernel, netdev, linux-arm-kernel, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/seeq/ether3.c | 5 +++--
drivers/net/ethernet/seeq/sgiseeq.c | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/seeq/ether3.c b/drivers/net/ethernet/seeq/ether3.c
index c5bc124..d1bb73b 100644
--- a/drivers/net/ethernet/seeq/ether3.c
+++ b/drivers/net/ethernet/seeq/ether3.c
@@ -77,7 +77,8 @@
static int ether3_rx(struct net_device *dev, unsigned int maxcnt);
static void ether3_tx(struct net_device *dev);
static int ether3_open (struct net_device *dev);
-static int ether3_sendpacket (struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t ether3_sendpacket(struct sk_buff *skb,
+ struct net_device *dev);
static irqreturn_t ether3_interrupt (int irq, void *dev_id);
static int ether3_close (struct net_device *dev);
static void ether3_setmulticastlist (struct net_device *dev);
@@ -481,7 +482,7 @@ static void ether3_timeout(struct net_device *dev)
/*
* Transmit a packet
*/
-static int
+static netdev_tx_t
ether3_sendpacket(struct sk_buff *skb, struct net_device *dev)
{
unsigned long flags;
diff --git a/drivers/net/ethernet/seeq/sgiseeq.c b/drivers/net/ethernet/seeq/sgiseeq.c
index 573691b..70cce63 100644
--- a/drivers/net/ethernet/seeq/sgiseeq.c
+++ b/drivers/net/ethernet/seeq/sgiseeq.c
@@ -578,7 +578,8 @@ static inline int sgiseeq_reset(struct net_device *dev)
return 0;
}
-static int sgiseeq_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t
+sgiseeq_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct sgiseeq_private *sp = netdev_priv(dev);
struct hpc3_ethregs *hregs = sp->hregs;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] net: cirrus: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 3:02 UTC (permalink / raw)
To: davem, hsweeten, fthain, geert, b.zolnierkie
Cc: linux-kernel, netdev, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/cirrus/ep93xx_eth.c | 2 +-
drivers/net/ethernet/cirrus/mac89x0.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index e2a7029..13dfdfc 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -332,7 +332,7 @@ static int ep93xx_poll(struct napi_struct *napi, int budget)
return rx;
}
-static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ep93xx_priv *ep = netdev_priv(dev);
struct ep93xx_tdesc *txd;
diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 3f8fe8f..6324e80 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -113,7 +113,7 @@ struct net_local {
/* Index to functions, as function prototypes. */
static int net_open(struct net_device *dev);
-static int net_send_packet(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t net_send_packet(struct sk_buff *skb, struct net_device *dev);
static irqreturn_t net_interrupt(int irq, void *dev_id);
static void set_multicast_list(struct net_device *dev);
static void net_rx(struct net_device *dev);
@@ -324,7 +324,7 @@ static int mac89x0_device_probe(struct platform_device *pdev)
return 0;
}
-static int
+static netdev_tx_t
net_send_packet(struct sk_buff *skb, struct net_device *dev)
{
struct net_local *lp = netdev_priv(dev);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] net: sgi: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 3:05 UTC (permalink / raw)
To: davem, ralf; +Cc: linux-kernel, netdev, linux-mips, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 4 ++--
drivers/net/ethernet/sgi/meth.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 18d533f..3140999 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -99,7 +99,7 @@ struct ioc3_private {
static int ioc3_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
static void ioc3_set_multicast_list(struct net_device *dev);
-static int ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev);
static void ioc3_timeout(struct net_device *dev);
static inline unsigned int ioc3_hash(const unsigned char *addr);
static inline void ioc3_stop(struct ioc3_private *ip);
@@ -1390,7 +1390,7 @@ static void ioc3_remove_one(struct pci_dev *pdev)
.remove = ioc3_remove_one,
};
-static int ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
unsigned long data;
struct ioc3_private *ip = netdev_priv(dev);
diff --git a/drivers/net/ethernet/sgi/meth.c b/drivers/net/ethernet/sgi/meth.c
index ea55abd..703fbbe 100644
--- a/drivers/net/ethernet/sgi/meth.c
+++ b/drivers/net/ethernet/sgi/meth.c
@@ -697,7 +697,7 @@ static void meth_add_to_tx_ring(struct meth_private *priv, struct sk_buff *skb)
/*
* Transmit a packet (called by the kernel)
*/
-static int meth_tx(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t meth_tx(struct sk_buff *skb, struct net_device *dev)
{
struct meth_private *priv = netdev_priv(dev);
unsigned long flags;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: Andrew Lunn @ 2018-09-21 3:12 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Arnd Bergmann, Ard Biesheuvel, Eric Biggers, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson
In-Reply-To: <CAHmME9rJq=q7xj_E_84GN5FuzSW2KdP4Ch=y527Lem0ME4GnLg@mail.gmail.com>
On Fri, Sep 21, 2018 at 02:11:43AM +0200, Jason A. Donenfeld wrote:
> Hey Arnd,
>
> On Thu, Sep 20, 2018 at 6:02 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > Right, if you hit a stack requirement like this, it's usually the compiler
> > doing something bad, not just using too much stack but also generating
> > rather slow object code in the process. It's better to fix the bug by
> > optimizing the code to not spill registers to the stack.
> >
> > In the long run, I'd like to reduce the stack frame size further, so
> > best assume that anything over 1024 bytes (on 32-bit) or 1280 bytes
> > (on 64-bit) is a bug in the code, and stay below that.
> >
> > For prototyping, you can just mark the broken functions individually
> > by setting the warning limit for a specific function that is known to
> > be misoptimized by the compiler (with a comment about which compiler
> > and architectures are affected), but not override the limit for the
> > entire file.
>
> Thanks for the explanation. Fortunately in my case, the issues were
> trivially fixable to get it under 1024/1280. (By the way, why does
> 64-bit have a slightly larger stack frame? To account for 32 pointers
> taking double the space or something?) That will be rectified in v6.
Hi Jason
Do you any stack usage information?
A VPN can be at the bottom of some deep stack calls. Swap on NFS over
the VPN? If you have one frame of 1K, you might be O.K. But if you
have a few of these, i can see there might be issues of overflowing
the stack.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-21 3:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: Arnd Bergmann, Ard Biesheuvel, Eric Biggers, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson
In-Reply-To: <20180921031255.GB11109@lunn.ch>
Hi Andrew,
On Fri, Sep 21, 2018 at 5:13 AM Andrew Lunn <andrew@lunn.ch> wrote:
> Do you any stack usage information?
>
> A VPN can be at the bottom of some deep stack calls. Swap on NFS over
> the VPN? If you have one frame of 1K, you might be O.K. But if you
> have a few of these, i can see there might be issues of overflowing
> the stack.
It's pretty small, and most of the work is punted onto workqueues
pretty early, so the depth isn't as much of an issue as it would be if
I did everything from ndo_start_xmit. I've done some stack tracking
using the debug option for that, and it's generally far from the
deepest thing running.
Jason
^ permalink raw reply
* Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: Andy Lutomirski @ 2018-09-21 3:23 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jason A. Donenfeld, Arnd Bergmann, Ard Biesheuvel, Eric Biggers,
LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <20180921031255.GB11109@lunn.ch>
> On Sep 20, 2018, at 8:12 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>
>> On Fri, Sep 21, 2018 at 02:11:43AM +0200, Jason A. Donenfeld wrote:
>> Hey Arnd,
>>
>>> On Thu, Sep 20, 2018 at 6:02 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>> Right, if you hit a stack requirement like this, it's usually the compiler
>>> doing something bad, not just using too much stack but also generating
>>> rather slow object code in the process. It's better to fix the bug by
>>> optimizing the code to not spill registers to the stack.
>>>
>>> In the long run, I'd like to reduce the stack frame size further, so
>>> best assume that anything over 1024 bytes (on 32-bit) or 1280 bytes
>>> (on 64-bit) is a bug in the code, and stay below that.
>>>
>>> For prototyping, you can just mark the broken functions individually
>>> by setting the warning limit for a specific function that is known to
>>> be misoptimized by the compiler (with a comment about which compiler
>>> and architectures are affected), but not override the limit for the
>>> entire file.
>>
>> Thanks for the explanation. Fortunately in my case, the issues were
>> trivially fixable to get it under 1024/1280. (By the way, why does
>> 64-bit have a slightly larger stack frame? To account for 32 pointers
>> taking double the space or something?) That will be rectified in v6.
>
> Hi Jason
>
> Do you any stack usage information?
>
> A VPN can be at the bottom of some deep stack calls. Swap on NFS over
> the VPN? If you have one frame of 1K, you might be O.K. But if you
> have a few of these, i can see there might be issues of overflowing
> the stack.
>
>
At the risk on suggesting something awful: on x86_64, since we turn preemption off for simd, it wouldn’t be *completely* insane to do the crypto on the irq stack. It would look like:
kernel_fpu_call(func, arg);
And this helper would disable preemption, enable FPU, switch to the irq stack, call func(arg), disable FPU, enable preemption, and return. And we can have large IRQ stacks.
I refuse to touch this with a ten-foot pole until the lazy FPU restore patches land.
All that being said, why are these frames so large? It sounds like something may be spilling that ought not to.
^ permalink raw reply
* Re: [PATCH net-next 0/7] add support for VSC8584 and VSC8574 Microsemi quad-port PHYs
From: Linus Walleij @ 2018-09-20 21:38 UTC (permalink / raw)
To: quentin.schulz
Cc: Alexandre Belloni, Ralf Baechle, paul.burton, James Hogan,
Rob Herring, Mark Rutland, David S. Miller, Andrew Lunn,
Florian Fainelli, allan.nielsen, Linux MIPS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, netdev, Thomas Petazzoni,
Antoine Tenart
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
Just as a drive-by comment this seems vaguely related to the Vitesse
DSA switch I merged in drivers/net/dsa/vitesse-vsc73xx.c
The VSC* product name handily gives away the origin in Vitesse's
product line.
The VSC73xx also have the 8051 CPU and internal RAM, but are
accessed (typically) over SPI, and AFAICT this thing is talking over
MDIO.
The Vitesse 73xx however also supports a WAN port and VLANs
which makes it significantly different, falling into switch class I
guess.
These VSC85*4's does have an SPI interface as well, according
to the data sheet but I assume your target boards don't even
connect it?
When it comes to 8051 code we have quite a lot of this in the kernel
these days, I suspect the 8051 snippets in this code could be
disassembled and put into linux-firmware in source form, but
that is maybe a bit overly ambitious. We have done that for a few
USB to serial controllers using the EzUSB 8051 though:
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/keyspan_pda
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/usbdux
These can rebuild their firmware using the as31 assembler.
https://github.com/nitsky/as31
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH net-next] net: wiznet: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 3:35 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, netdev, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/wiznet/w5100.c | 2 +-
drivers/net/ethernet/wiznet/w5300.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 2bdfb39..d8ba512 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -835,7 +835,7 @@ static void w5100_tx_work(struct work_struct *work)
w5100_tx_skb(priv->ndev, skb);
}
-static int w5100_start_tx(struct sk_buff *skb, struct net_device *ndev)
+static netdev_tx_t w5100_start_tx(struct sk_buff *skb, struct net_device *ndev)
{
struct w5100_priv *priv = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/wiznet/w5300.c b/drivers/net/ethernet/wiznet/w5300.c
index 56ae573..80fdbff 100644
--- a/drivers/net/ethernet/wiznet/w5300.c
+++ b/drivers/net/ethernet/wiznet/w5300.c
@@ -365,7 +365,7 @@ static void w5300_tx_timeout(struct net_device *ndev)
netif_wake_queue(ndev);
}
-static int w5300_start_tx(struct sk_buff *skb, struct net_device *ndev)
+static netdev_tx_t w5300_start_tx(struct sk_buff *skb, struct net_device *ndev)
{
struct w5300_priv *priv = netdev_priv(ndev);
--
1.8.3.1
^ permalink raw reply related
* [PATCH nf-next] netfilter: ctnetlink: must check mark attributes vs NULL
From: Florian Westphal @ 2018-09-20 21:53 UTC (permalink / raw)
To: netfilter-devel
Cc: syzbot+e45eda8eda6e93a03959, syzkaller-bugs, netdev,
kristian.evensen, Florian Westphal
In-Reply-To: <000000000000f4bf02057653dc0a@google.com>
else we will oops (null deref) when the attributes aren't present.
Also add back the EOPNOTSUPP in case MARK filtering is requested but
kernel doesn't support it.
Fixes: 59c08c69c2788 ("netfilter: ctnetlink: Support L3 protocol-filter on flush")
Reported-by: syzbot+e45eda8eda6e93a03959@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
---
only affects nf-next and linux-next.
net/netfilter/nf_conntrack_netlink.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index f8c74f31aa36..b7a6984c74d0 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -833,6 +833,11 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[], u8 family)
{
struct ctnetlink_filter *filter;
+#ifndef CONFIG_NF_CONNTRACK_MARK
+ if (cda[CTA_MARK] && cda[CTA_MARK_MASK])
+ return ERR_PTR(-EOPNOTSUPP);
+#endif
+
filter = kzalloc(sizeof(*filter), GFP_KERNEL);
if (filter == NULL)
return ERR_PTR(-ENOMEM);
@@ -840,8 +845,10 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[], u8 family)
filter->family = family;
#ifdef CONFIG_NF_CONNTRACK_MARK
- filter->mark.val = ntohl(nla_get_be32(cda[CTA_MARK]));
- filter->mark.mask = ntohl(nla_get_be32(cda[CTA_MARK_MASK]));
+ if (cda[CTA_MARK] && cda[CTA_MARK_MASK]) {
+ filter->mark.val = ntohl(nla_get_be32(cda[CTA_MARK]));
+ filter->mark.mask = ntohl(nla_get_be32(cda[CTA_MARK_MASK]));
+ }
#endif
return filter;
}
--
2.16.4
^ permalink raw reply related
* [PATCH net-next] net: i825xx: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 3:44 UTC (permalink / raw)
To: davem, linux, sammy; +Cc: linux-kernel, netdev, linux-arm-kernel, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/i825xx/ether1.c | 5 +++--
drivers/net/ethernet/i825xx/lib82596.c | 4 ++--
drivers/net/ethernet/i825xx/sun3_82586.c | 6 ++++--
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/i825xx/ether1.c b/drivers/net/ethernet/i825xx/ether1.c
index dc98345..35f6291 100644
--- a/drivers/net/ethernet/i825xx/ether1.c
+++ b/drivers/net/ethernet/i825xx/ether1.c
@@ -64,7 +64,8 @@
#define RX_AREA_END 0x0fc00
static int ether1_open(struct net_device *dev);
-static int ether1_sendpacket(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t ether1_sendpacket(struct sk_buff *skb,
+ struct net_device *dev);
static irqreturn_t ether1_interrupt(int irq, void *dev_id);
static int ether1_close(struct net_device *dev);
static void ether1_setmulticastlist(struct net_device *dev);
@@ -667,7 +668,7 @@
netif_wake_queue(dev);
}
-static int
+static netdev_tx_t
ether1_sendpacket (struct sk_buff *skb, struct net_device *dev)
{
int tmp, tst, nopaddr, txaddr, tbdaddr, dataddr;
diff --git a/drivers/net/ethernet/i825xx/lib82596.c b/drivers/net/ethernet/i825xx/lib82596.c
index f00a1dc..2f7ae11 100644
--- a/drivers/net/ethernet/i825xx/lib82596.c
+++ b/drivers/net/ethernet/i825xx/lib82596.c
@@ -347,7 +347,7 @@ struct i596_private {
0x7f /* *multi IA */ };
static int i596_open(struct net_device *dev);
-static int i596_start_xmit(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t i596_start_xmit(struct sk_buff *skb, struct net_device *dev);
static irqreturn_t i596_interrupt(int irq, void *dev_id);
static int i596_close(struct net_device *dev);
static void i596_add_cmd(struct net_device *dev, struct i596_cmd *cmd);
@@ -966,7 +966,7 @@ static void i596_tx_timeout (struct net_device *dev)
}
-static int i596_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t i596_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct i596_private *lp = netdev_priv(dev);
struct tx_cmd *tx_cmd;
diff --git a/drivers/net/ethernet/i825xx/sun3_82586.c b/drivers/net/ethernet/i825xx/sun3_82586.c
index 8bb15a8..1a86184 100644
--- a/drivers/net/ethernet/i825xx/sun3_82586.c
+++ b/drivers/net/ethernet/i825xx/sun3_82586.c
@@ -121,7 +121,8 @@
static irqreturn_t sun3_82586_interrupt(int irq,void *dev_id);
static int sun3_82586_open(struct net_device *dev);
static int sun3_82586_close(struct net_device *dev);
-static int sun3_82586_send_packet(struct sk_buff *,struct net_device *);
+static netdev_tx_t sun3_82586_send_packet(struct sk_buff *,
+ struct net_device *);
static struct net_device_stats *sun3_82586_get_stats(struct net_device *dev);
static void set_multicast_list(struct net_device *dev);
static void sun3_82586_timeout(struct net_device *dev);
@@ -1002,7 +1003,8 @@ static void sun3_82586_timeout(struct net_device *dev)
* send frame
*/
-static int sun3_82586_send_packet(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t
+sun3_82586_send_packet(struct sk_buff *skb, struct net_device *dev)
{
int len,i;
#ifndef NO_NOPCOMMANDS
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] net: apple: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-21 3:46 UTC (permalink / raw)
To: davem, keescook; +Cc: linux-kernel, netdev, fthain, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/apple/bmac.c | 4 ++--
drivers/net/ethernet/apple/mace.c | 4 ++--
drivers/net/ethernet/apple/macmace.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/apple/bmac.c b/drivers/net/ethernet/apple/bmac.c
index 024998d..6a8e256 100644
--- a/drivers/net/ethernet/apple/bmac.c
+++ b/drivers/net/ethernet/apple/bmac.c
@@ -154,7 +154,7 @@ struct bmac_data {
static irqreturn_t bmac_rxdma_intr(int irq, void *dev_id);
static void bmac_set_timeout(struct net_device *dev);
static void bmac_tx_timeout(struct timer_list *t);
-static int bmac_output(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t bmac_output(struct sk_buff *skb, struct net_device *dev);
static void bmac_start(struct net_device *dev);
#define DBDMA_SET(x) ( ((x) | (x) << 16) )
@@ -1456,7 +1456,7 @@ static int bmac_close(struct net_device *dev)
spin_unlock_irqrestore(&bp->lock, flags);
}
-static int
+static netdev_tx_t
bmac_output(struct sk_buff *skb, struct net_device *dev)
{
struct bmac_data *bp = netdev_priv(dev);
diff --git a/drivers/net/ethernet/apple/mace.c b/drivers/net/ethernet/apple/mace.c
index 0b5429d..68b9ee4 100644
--- a/drivers/net/ethernet/apple/mace.c
+++ b/drivers/net/ethernet/apple/mace.c
@@ -78,7 +78,7 @@ struct mace_data {
static int mace_open(struct net_device *dev);
static int mace_close(struct net_device *dev);
-static int mace_xmit_start(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t mace_xmit_start(struct sk_buff *skb, struct net_device *dev);
static void mace_set_multicast(struct net_device *dev);
static void mace_reset(struct net_device *dev);
static int mace_set_address(struct net_device *dev, void *addr);
@@ -525,7 +525,7 @@ static inline void mace_set_timeout(struct net_device *dev)
mp->timeout_active = 1;
}
-static int mace_xmit_start(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t mace_xmit_start(struct sk_buff *skb, struct net_device *dev)
{
struct mace_data *mp = netdev_priv(dev);
volatile struct dbdma_regs __iomem *td = mp->tx_dma;
diff --git a/drivers/net/ethernet/apple/macmace.c b/drivers/net/ethernet/apple/macmace.c
index 137cbb4..376f2c2 100644
--- a/drivers/net/ethernet/apple/macmace.c
+++ b/drivers/net/ethernet/apple/macmace.c
@@ -89,7 +89,7 @@ struct mace_frame {
static int mace_open(struct net_device *dev);
static int mace_close(struct net_device *dev);
-static int mace_xmit_start(struct sk_buff *skb, struct net_device *dev);
+static netdev_tx_t mace_xmit_start(struct sk_buff *skb, struct net_device *dev);
static void mace_set_multicast(struct net_device *dev);
static int mace_set_address(struct net_device *dev, void *addr);
static void mace_reset(struct net_device *dev);
@@ -444,7 +444,7 @@ static int mace_close(struct net_device *dev)
* Transmit a frame
*/
-static int mace_xmit_start(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t mace_xmit_start(struct sk_buff *skb, struct net_device *dev)
{
struct mace_data *mp = netdev_priv(dev);
unsigned long flags;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] af_key: free SKBs under RCU protection
From: Eric Dumazet @ 2018-09-20 22:10 UTC (permalink / raw)
To: stranche, Eric Dumazet; +Cc: netdev, steffen.klassert
In-Reply-To: <357e28c3fa0c7bacaffde4e960f58a87@codeaurora.org>
On 09/20/2018 12:25 PM, stranche@codeaurora.org wrote:
>>
>> I do not believe the changelog or the patch makes sense.
>>
>> Having skb still referencing a socket prevents this socket being released.
>>
>> If you think about it, what would prevent the freeing happening
>> _before_ the rcu_read_lock() in pfkey_broadcast() ?
>>
>> Maybe the correct fix is that pfkey_broadcast_one() should ensure the
>> socket is still valid.
>>
>> I would suggest something like :
>>
>> diff --git a/net/key/af_key.c b/net/key/af_key.c
>> index
>> 9d61266526e767770d9a1ce184ac8cdd59de309a..5ce309d020dda5e46e4426c4a639bfb551e2260d
>> 100644
>> --- a/net/key/af_key.c
>> +++ b/net/key/af_key.c
>> @@ -201,7 +201,9 @@ static int pfkey_broadcast_one(struct sk_buff
>> *skb, struct sk_buff **skb2,
>> {
>> int err = -ENOBUFS;
>>
>> - sock_hold(sk);
>> + if (!refcount_inc_not_zero(&sk->sk_refcnt))
>> + return -ENOENT;
>> +
>> if (*skb2 == NULL) {
>> if (refcount_read(&skb->users) != 1) {
>> *skb2 = skb_clone(skb, allocation);
>
> Hi Eric,
>
> I'm not sure that the socket getting freed before the rcu_read_lock() would
> be an issue, since then it would no longer be in the net_pkey->table that
> we loop through (since we call pfkey_remove() from pfkey_relase()). Because of
> that, all the sockets processed in pfkey_broadcast_one() have valid refcounts,
> so checking for zero there doesn't prevent the crash that I'm seeing.
>
> However, after going over the call flow again, I see that the actual problem
> occurs because of pfkey_broadcast_one(). Specifically, because of this check:
>
> if (*skb2 == NULL) {
> if (refcount_read(&skb->users) != 1) {
> *skb2 = skb_clone(skb, allocation);
> } else {
> *skb2 = skb;
> refcount_inc(&skb->users);
> }
> }
>
> Since we always pass a freshly cloned SKB to this function, skb->users is
> always 1, and skb2 just becomes skb. We then set skb2 (and thus skb) to
> belong to the socket.
>
> If the socket we queue skb2 to frees this SKB (thereby decrementing its
> refcount to 1) and the socket is freed before pfkey_broadcast() can
> execute the kfree_skb(skb) on line 284, we will then attempt to run
> sock_rfree() on an SKB with a dangling reference to this socket.
>
> Perhaps a cleaner solution here is to always clone the SKB in
> pfkey_broadcast_one(). That will ensure that the two kfree_skb() calls
> in pfkey_broadcast() will never be passed an SKB with sock_rfree() as
> its destructor, and we can avoid this race condition.
As long as one skb has sock_rfree has its destructor, the socket attached to
this skb can not be released. There is no race here.
Note that skb_clone() does not propagate the destructor.
The issue here is that in the rcu lookup, we can find a socket that has been
dismantled, with a 0 refcount.
We must not use sock_hold() in this case, since we are not sure the socket refcount is not already 0.
pfkey_broadcast() and pfkey_broadcast_one() violate basic RCU rules.
When in a RCU lookup, one want to increment an object refcount, it needs
to be extra-careful, as I did in my proposal.
Note that the race could be automatically detected with CONFIG_REFCOUNT_FULL=y
^ permalink raw reply
* Re: [PATCH net] af_key: free SKBs under RCU protection
From: Eric Dumazet @ 2018-09-20 22:29 UTC (permalink / raw)
To: Eric Dumazet, stranche; +Cc: netdev, steffen.klassert
In-Reply-To: <82b2ac27-0f61-3be6-b09e-ec31cf95881f@gmail.com>
On 09/20/2018 03:10 PM, Eric Dumazet wrote:
>
>
> On 09/20/2018 12:25 PM, stranche@codeaurora.org wrote:
>>>
>>> I do not believe the changelog or the patch makes sense.
>>>
>>> Having skb still referencing a socket prevents this socket being released.
>>>
>>> If you think about it, what would prevent the freeing happening
>>> _before_ the rcu_read_lock() in pfkey_broadcast() ?
>>>
>>> Maybe the correct fix is that pfkey_broadcast_one() should ensure the
>>> socket is still valid.
>>>
>>> I would suggest something like :
>>>
>>> diff --git a/net/key/af_key.c b/net/key/af_key.c
>>> index
>>> 9d61266526e767770d9a1ce184ac8cdd59de309a..5ce309d020dda5e46e4426c4a639bfb551e2260d
>>> 100644
>>> --- a/net/key/af_key.c
>>> +++ b/net/key/af_key.c
>>> @@ -201,7 +201,9 @@ static int pfkey_broadcast_one(struct sk_buff
>>> *skb, struct sk_buff **skb2,
>>> {
>>> int err = -ENOBUFS;
>>>
>>> - sock_hold(sk);
>>> + if (!refcount_inc_not_zero(&sk->sk_refcnt))
>>> + return -ENOENT;
>>> +
>>> if (*skb2 == NULL) {
>>> if (refcount_read(&skb->users) != 1) {
>>> *skb2 = skb_clone(skb, allocation);
>>
>> Hi Eric,
>>
>> I'm not sure that the socket getting freed before the rcu_read_lock() would
>> be an issue, since then it would no longer be in the net_pkey->table that
>> we loop through (since we call pfkey_remove() from pfkey_relase()). Because of
>> that, all the sockets processed in pfkey_broadcast_one() have valid refcounts,
>> so checking for zero there doesn't prevent the crash that I'm seeing.
>>
>> However, after going over the call flow again, I see that the actual problem
>> occurs because of pfkey_broadcast_one(). Specifically, because of this check:
>>
>> if (*skb2 == NULL) {
>> if (refcount_read(&skb->users) != 1) {
>> *skb2 = skb_clone(skb, allocation);
>> } else {
>> *skb2 = skb;
>> refcount_inc(&skb->users);
>> }
>> }
>>
>> Since we always pass a freshly cloned SKB to this function, skb->users is
>> always 1, and skb2 just becomes skb. We then set skb2 (and thus skb) to
>> belong to the socket.
>>
>> If the socket we queue skb2 to frees this SKB (thereby decrementing its
>> refcount to 1) and the socket is freed before pfkey_broadcast() can
>> execute the kfree_skb(skb) on line 284, we will then attempt to run
>> sock_rfree() on an SKB with a dangling reference to this socket.
>>
>> Perhaps a cleaner solution here is to always clone the SKB in
>> pfkey_broadcast_one(). That will ensure that the two kfree_skb() calls
>> in pfkey_broadcast() will never be passed an SKB with sock_rfree() as
>> its destructor, and we can avoid this race condition.
>
> As long as one skb has sock_rfree has its destructor, the socket attached to
> this skb can not be released. There is no race here.
>
> Note that skb_clone() does not propagate the destructor.
>
> The issue here is that in the rcu lookup, we can find a socket that has been
> dismantled, with a 0 refcount.
>
> We must not use sock_hold() in this case, since we are not sure the socket refcount is not already 0.
>
> pfkey_broadcast() and pfkey_broadcast_one() violate basic RCU rules.
>
> When in a RCU lookup, one want to increment an object refcount, it needs
> to be extra-careful, as I did in my proposal.
>
> Note that the race could be automatically detected with CONFIG_REFCOUNT_FULL=y
Bug was added in commit 7f6b9dbd5afb ("af_key: locking change")
^ permalink raw reply
* Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-21 4:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Lunn, Arnd Bergmann, Ard Biesheuvel, Eric Biggers, LKML,
Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <8FA361E3-FBCB-469E-88DC-F4085BD91175@amacapital.net>
Hi Andy,
On Fri, Sep 21, 2018 at 5:23 AM Andy Lutomirski <luto@amacapital.net> wrote:
> At the risk on suggesting something awful: on x86_64, since we turn preemption off for simd, it wouldn’t be *completely* insane to do the crypto on the irq stack. It would look like:
>
> kernel_fpu_call(func, arg);
>
> And this helper would disable preemption, enable FPU, switch to the irq stack, call func(arg), disable FPU, enable preemption, and return. And we can have large IRQ stacks.
>
> I refuse to touch this with a ten-foot pole until the lazy FPU restore patches land.
Haha. That's fun, and maybe we'll do that at some point, but I have
some other reasons too for being on a workqueue now.
>
> All that being said, why are these frames so large? It sounds like something may be spilling that ought not to.
They're not. Well, they're not anymore. I had a silly thing before
like "u8 buffer[1 << 12]" in some debugging code, which is what
prompted the ccflag-y addition. I cleaned up the mistakes like that
and frames are now reasonable everywhere. Non-issue.
Jason
^ permalink raw reply
* Re: array bounds warning in xfrm_output_resume
From: David Ahern @ 2018-09-20 22:53 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180920140617.uybaha3n7iz6ckvn@breakpoint.cc>
On 9/20/18 7:06 AM, Florian Westphal wrote:
> David Ahern <dsahern@gmail.com> wrote:
>>> $ make O=kbuild/perf -j 24 -s
>>> In file included from /home/dsa/kernel-3.git/include/linux/kernel.h:10:0,
>>> from /home/dsa/kernel-3.git/include/linux/list.h:9,
>>> from /home/dsa/kernel-3.git/include/linux/module.h:9,
>>> from /home/dsa/kernel-3.git/net/xfrm/xfrm_output.c:13:
>>> /home/dsa/kernel-3.git/net/xfrm/xfrm_output.c: In function
>>> ‘xfrm_output_resume’:
>>> /home/dsa/kernel-3.git/include/linux/compiler.h:252:20: warning: array
>>> subscript is above array bounds [-Warray-bounds]
>>> __read_once_size(&(x), __u.__c, sizeof(x)); \
>
> Does this thing avoid the warning?
nope, still see it.
^ permalink raw reply
* Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: Andy Lutomirski @ 2018-09-21 4:52 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Jason A. Donenfeld, Andrew Lunn, Arnd Bergmann, Eric Biggers,
LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <CAKv+Gu-sciHeWVij8hWGF78HV_CLddMv7Xdf1GRSY+6B2yt48A@mail.gmail.com>
> On Sep 20, 2018, at 9:30 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 20 September 2018 at 21:15, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> Hi Andy,
>>
>>> On Fri, Sep 21, 2018 at 5:23 AM Andy Lutomirski <luto@amacapital.net> wrote:
>>> At the risk on suggesting something awful: on x86_64, since we turn preemption off for simd, it wouldn’t be *completely* insane to do the crypto on the irq stack. It would look like:
>>>
>>> kernel_fpu_call(func, arg);
>>>
>>> And this helper would disable preemption, enable FPU, switch to the irq stack, call func(arg), disable FPU, enable preemption, and return. And we can have large IRQ stacks.
>>>
>>> I refuse to touch this with a ten-foot pole until the lazy FPU restore patches land.
>>
>> Haha. That's fun, and maybe we'll do that at some point, but I have
>> some other reasons too for being on a workqueue now.
>>
>
> Kernel mode crypto is callable from any context, and SIMD can be used
> in softirq context on arm64 (and on x86, even from hardirq context
> IIRC if the interrupt is taken from userland), in which case we'd
> already be on the irq stack.
The x86_64 irq stack handles nesting already.
^ permalink raw reply
* Re: [PATCH] net: netronome: remove redundant continue
From: zhong jiang @ 2018-09-21 5:21 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, dirk.vandermerwe, simon.horman, netdev, linux-kernel
In-Reply-To: <20180920093857.6227710f@cakuba.netronome.com>
On 2018/9/21 0:38, Jakub Kicinski wrote:
> On Thu, 20 Sep 2018 16:02:21 +0800, zhong jiang wrote:
>> The continue will not truely skip any code. hence it is safe to
>> remove it.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> I think this came up during review at some point. I still prefer to
> keep the continue. The body of the loop performs initialization of
> objects, if an object is removed we shouldn't carry on with the body.
> It's easy to miss that the object got freed otherwise, there is no
> error being set and no warning printed...
IMO, we should bring it back when the case truely occur you have said. At present.
We should not take too much into account. Maybe it will not occur.
Thanks,
zhong jiang
>> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
>> index 0b1ac9c..50d7b58 100644
>> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
>> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
>> @@ -230,10 +230,8 @@ static void nfp_net_pf_free_vnics(struct nfp_pf *pf)
>> ctrl_bar += NFP_PF_CSR_SLICE_SIZE;
>>
>> /* Kill the vNIC if app init marked it as invalid */
>> - if (nn->port && nn->port->type == NFP_PORT_INVALID) {
>> + if (nn->port && nn->port->type == NFP_PORT_INVALID)
>> nfp_net_pf_free_vnic(pf, nn);
>> - continue;
>> - }
>> }
>>
>> if (list_empty(&pf->vnics))
>
> .
>
^ permalink raw reply
* Re: [PATCH net] ixgbe: check return value of napi_complete_done()
From: Song Liu @ 2018-09-20 23:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jeff Kirsher, netdev, intel-wired-lan@lists.osuosl.org,
Kernel Team, stable@vger.kernel.org, Alexei Starovoitov
In-Reply-To: <ee9b9a55-c967-cec3-7df1-79f84b06154c@gmail.com>
> On Sep 20, 2018, at 4:22 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/20/2018 03:42 PM, Song Liu wrote:
>>
>>
>>> On Sep 20, 2018, at 2:01 PM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
>>>
>>> On Thu, 2018-09-20 at 13:35 -0700, Eric Dumazet wrote:
>>>> On 09/20/2018 12:01 PM, Song Liu wrote:
>>>>> The NIC driver should only enable interrupts when napi_complete_done()
>>>>> returns true. This patch adds the check for ixgbe.
>>>>>
>>>>> Cc: stable@vger.kernel.org # 4.10+
>>>>> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>>>> Suggested-by: Eric Dumazet <edumazet@google.com>
>>>>> Signed-off-by: Song Liu <songliubraving@fb.com>
>>>>> ---
>>>>
>>>>
>>>> Well, unfortunately we do not know why this is needed,
>>>> this is why I have not yet sent this patch formally.
>>>>
>>>> netpoll has correct synchronization :
>>>>
>>>> poll_napi() places into napi->poll_owner current cpu number before
>>>> calling poll_one_napi()
>>>>
>>>> netpoll_poll_lock() does also use napi->poll_owner
>>>>
>>>> When netpoll calls ixgbe poll() method, it passed a budget of 0,
>>>> meaning napi_complete_done() is not called.
>>>>
>>>> As long as we can not explain the problem properly in the changelog,
>>>> we should investigate, otherwise we will probably see coming dozens of
>>>> patches
>>>> trying to fix a 'potential hazard'.
>>>
>>> Agreed, which is why I have our validation and developers looking into it,
>>> while we test the current patch from Song.
>>
>> I figured out what is the issue here. And I have a proposal to fix it. I
>> have verified that this fixes the issue in our tests. But Alexei suggests
>> that it may not be the right way to fix.
>>
>> Here is what happened:
>>
>> netpoll tries to send skb with netpoll_start_xmit(). If that fails, it
>> calls netpoll_poll_dev(), which calls ndo_poll_controller(). Then, in
>> the driver, ndo_poll_controller() calls napi_schedule() for ALL NAPIs
>> within the same NIC.
>>
>> This is problematic, because at the end napi_schedule() calls:
>>
>> ____napi_schedule(this_cpu_ptr(&softnet_data), n);
>>
>> which attached these NAPIs to softnet_data on THIS CPU. This is done
>> via napi->poll_list.
>>
>> Then suddenly ksoftirqd on this CPU owns multiple NAPIs. And it will
>> not give up the ownership until it calls napi_complete_done(). However,
>> for a very busy server, we usually use 16 CPUs to poll NAPI, so this
>> CPU can easily be overloaded. And as a result, each call of napi->poll()
>> will hit budget (of 64), and it will not call napi_complete_done(),
>> and the NAPI stays in the poll_list of this CPU.
>>
>> When this happens, the host usually cannot get out of this state until
>> we throttle/stop client traffic.
>>
>>
>> I am pretty confident this is what happened. Please let me know if
>> anything above doesn't make sense.
>>
>>
>> Here is my proposal to fix it: Instead of polling all NAPIs within one
>> NIC, I would have netpoll to only poll the NAPI that will free space
>> for netpoll_start_xmit(). I attached my two RFC patches to the end of
>> this email.
>>
>> I chatted with Alexei about this. He think polling only one NAPI may
>> not guarantee netpoll make progress with the TX queue we are aiming
>> for. Also, the bigger problem may be the fact that NAPIs could get
>> pinned to one CPU and cannot get released.
>>
>> At this point, I really don't know what is the best way to fix this.
>>
>> I will also work on a repro with netperf.
>
> Thanks !
>
>>
>> Please let me know your suggestions.
>>
>
> Yeah, maybe that NICs using NAPI could not provide an ndo_poll_controller() method at all,
> since it is very risky (potentially grab many NAPI, and end up in this locked situation)
>
> poll_napi() could attempt to free skbs one napi at a time,
> without the current cpu stealing all NAPI.
>
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 57557a6a950cc9cdff959391576a03381d328c1a..a992971d366090ba69d5c1af32eadd554d6880cf 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -205,13 +205,8 @@ static void netpoll_poll_dev(struct net_device *dev)
> }
>
> ops = dev->netdev_ops;
> - if (!ops->ndo_poll_controller) {
> - up(&ni->dev_lock);
> - return;
> - }
> -
> - /* Process pending work on NIC */
> - ops->ndo_poll_controller(dev);
> + if (ops->ndo_poll_controller)
> + ops->ndo_poll_controller(dev);
>
> poll_napi(dev);
>
I tried to totally skip ndo_poll_controller() here. It did avoid hitting
the issue. However, netpoll will drop (fail to send) more packets.
Thanks,
Song
^ 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