* Re: VRF Issue Since kernel 5
From: Gowen @ 2019-09-12 6:50 UTC (permalink / raw)
To: David Ahern, netdev@vger.kernel.org
In-Reply-To: <b300235a-00f8-0689-8544-9db07cbd1e21@gmail.com>
Hi David -thanks for getting back to me
The DNS servers are 10.24.65.203 or 10.24.64.203 which you want to go
out mgmt-vrf. correct? No - 10.24.65.203 10.25.65.203, so should hit the route leak rule as below (if I've put the 10.24.64.0/24 subnet anywhere it is a typo)
vmAdmin@NETM06:~$ ip ro get 10.24.65.203 fibmatch
10.24.65.0/24 via 10.24.12.1 dev eth0
I've added the 127/8 route - no difference.
The reason for what you might think is an odd design is that I wanted any non-VRF aware users to be able to come in and run all commands in default context without issue, while production and mgmt traffic was separated still
DNS is now working as long as /etc/resolv.conf is populated with my DNS servers - a lot of people would be using this on Azure which uses netplan, so they'll have the same issue, is there documentation I could update or raise a bug to check the systemd-resolve servers as well?
Gareth
From: David Ahern <dsahern@gmail.com>
Sent: 11 September 2019 18:02
To: Gowen <gowen@potatocomputing.co.uk>; netdev@vger.kernel.org <netdev@vger.kernel.org>
Subject: Re: VRF Issue Since kernel 5
At LPC this week and just now getting a chance to process the data you sent.
On 9/9/19 8:46 AM, Gowen wrote:
> the production traffic is all in the 10.0.0.0/8 network (eth1 global VRF) except for a few subnets (DNS) which are routed out eth0 (mgmt-vrf)
>
>
> Admin@NETM06:~$ ip route show
> default via 10.24.12.1 dev eth0
> 10.0.0.0/8 via 10.24.12.1 dev eth1
> 10.24.12.0/24 dev eth1 proto kernel scope link src 10.24.12.9
> 10.24.65.0/24 via 10.24.12.1 dev eth0
> 10.25.65.0/24 via 10.24.12.1 dev eth0
> 10.26.0.0/21 via 10.24.12.1 dev eth0
> 10.26.64.0/21 via 10.24.12.1 dev eth0
interesting route table. This is default VRF but you have route leaking
through eth0 which is in mgmt-vrf.
>
>
> Admin@NETM06:~$ ip route show vrf mgmt-vrf
> default via 10.24.12.1 dev eth0
> unreachable default metric 4278198272
> 10.24.12.0/24 dev eth0 proto kernel scope link src 10.24.12.10
> 10.24.65.0/24 via 10.24.12.1 dev eth0
> 10.25.65.0/24 via 10.24.12.1 dev eth0
> 10.26.0.0/21 via 10.24.12.1 dev eth0
> 10.26.64.0/21 via 10.24.12.1 dev eth0
The DNS servers are 10.24.65.203 or 10.24.64.203 which you want to go
out mgmt-vrf. correct?
10.24.65.203 should hit the route "10.24.65.0/24 via 10.24.12.1 dev
eth0" for both default VRF and mgmt-vrf.
10.24.64.203 will NOT hit a route leak entry so traverse the VRF
associated with the context of the command (mgmt-vrf or default). Is
that intentional? (verify with: `ip ro get 10.24.64.203 fibmatch` and
`ip ro get 10.24.64.203 vrf mgmt-vrf fibmatch`)
>
>
>
> The strange activity occurs when I enter the command “sudo apt update” as I can resolve the DNS request (10.24.65.203 or 10.24.64.203, verified with tcpdump) out eth0 but for the actual update traffic there is no activity:
>
>
> sudo tcpdump -i eth0 '(host 10.24.65.203 or host 10.25.65.203) and port 53' -n
> <OUTPUT OMITTED FOR BREVITY>
> 10:06:05.268735 IP 10.24.12.10.39963 > 10.24.65.203.53: 48798+ [1au] A? security.ubuntu.com. (48)
> <OUTPUT OMITTED FOR BREVITY>
> 10:06:05.284403 IP 10.24.65.203.53 > 10.24.12.10.39963: 48798 13/0/1 A 91.189.91.23, A 91.189.88.24, A 91.189.91.26, A 91.189.88.162, A 91.189.88.149, A 91.189.91.24, A 91.189.88.173, A 91.189.88.177, A 91.189.88.31, A 91.189.91.14, A 91.189.88.176, A 91.189.88.175,
A 91.189.88.174 (256)
>
>
>
> You can see that the update traffic is returned but is not accepted by the stack and a RST is sent
>
>
> Admin@NETM06:~$ sudo tcpdump -i eth0 '(not host 168.63.129.16 and port 80)' -n
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on eth0, link-type EN10MB (Ethernet), capture size 262144 bytes
> 10:17:12.690658 IP 10.24.12.10.40216 > 91.189.88.175.80: Flags [S], seq 2279624826, win 64240, options [mss 1460,sackOK,TS val 2029365856 ecr 0,nop,wscale 7], length 0
> 10:17:12.691929 IP 10.24.12.10.52362 > 91.189.95.83.80: Flags [S], seq 1465797256, win 64240, options [mss 1460,sackOK,TS val 3833463674 ecr 0,nop,wscale 7], length 0
> 10:17:12.696270 IP 91.189.88.175.80 > 10.24.12.10.40216: Flags [S.], seq 968450722, ack 2279624827, win 28960, options [mss 1418,sackOK,TS val 81957103 ecr 2029365856,nop,wscale 7], length 0
> 10:17:12.696301 IP 10.24.12.10.40216 > 91.189.88.175.80: Flags [R], seq 2279624827, win 0, length 0
> 10:17:12.697884 IP 91.189.95.83.80 > 10.24.12.10.52362: Flags [S.], seq 4148330738, ack 1465797257, win 28960, options [mss 1418,sackOK,TS val 2257624414 ecr 3833463674,nop,wscale 8], length 0
> 10:17:12.697909 IP 10.24.12.10.52362 > 91.189.95.83.80: Flags [R], seq 1465797257, win 0, length 0
>
>
>
>
> I can emulate the DNS lookup using netcat in the vrf:
>
>
> sudo ip vrf exec mgmt-vrf nc -u 10.24.65.203 53
>
`ip vrf exec mgmt-vrf <COMMAND>` means that every IPv4 and IPv6 socket
opened by <COMMAND> is automatically bound to mgmt-vrf which causes
route lookups to hit the mgmt-vrf table.
Just running <COMMAND> (without binding to any vrf) means no socket is
bound to anything unless the command does a bind. In that case the
routing lookups determine which egress device is used.
Now the response comes back, if the ingress interface is a VRF then the
socket lookup wants to match on a device.
Now, a later response shows this for DNS lookups:
isc-worker0000 20261 [000] 2215.013849: fib:fib_table_lookup: table
10 oif 0 iif 0 proto 0 0.0.0.0/0 -> 127.0.0.1/0 tos 0 scope 0 flags 0
==> dev eth0 gw 10.24.12.1 src 10.24.12.10 err 0
isc-worker0000 20261 [000] 2215.013915: fib:fib_table_lookup: table
10 oif 4 iif 1 proto 17 0.0.0.0/52138 -> 127.0.0.53/53 tos 0 scope 0
flags 4 ==> dev eth0 gw 10.24.12.1 src 10.24.12.10 err 0
isc-worker0000 20261 [000] 2220.014006: fib:fib_table_lookup: table
10 oif 4 iif 1 proto 17 0.0.0.0/52138 -> 127.0.0.53/53 tos 0 scope 0
flags 4 ==> dev eth0 gw 10.24.12.1 src 10.24.12.10 err 0
which suggests your process is passing off the DNS lookup to a local
process (isc-worker) and it hits the default route for mgmt-vrf when it
is trying to connect to a localhost address.
For mgmt-vrf I suggest always adding 127.0.0.1/8 to the mgmt vrf device
(and ::1/128 for IPv6 starting with 5.x kernels - I forget the exact
kernel version).
That might solve your problem; it might not.
(BTW: Cumulus uses fib rules for DNS servers to force DNS packets out
the mgmt-vrf interface.)
^ permalink raw reply
* Re: VRF Issue Since kernel 5
From: Gowen @ 2019-09-12 6:54 UTC (permalink / raw)
To: David Ahern, Alexis Bauvin, mmanning@vyatta.att-mail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <0fd88da3-a7b1-d2e5-f5b8-0095220a7cc0@gmail.com>
currently:
vmAdmin@NETM06:~$ uname -r
5.0.0-1018-azure
vmAdmin@NETM06:~$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=18.04
DISTRIB_CODENAME=bionic
DISTRIB_DESCRIPTION="Ubuntu 18.04.3 LTS"
I don't keep a history of kernel versions on test but I noticed it had gone to kernel 5 and stopped working - I'm about 80% sure that happened at the same time - I'll try and dig out some logs today to see what I can find for you as Linux is fairly new to me
Gareth
From: David Ahern <dsahern@gmail.com>
Sent: 11 September 2019 17:09
To: Gowen <gowen@potatocomputing.co.uk>; Alexis Bauvin <abauvin@online.net>; mmanning@vyatta.att-mail.com <mmanning@vyatta.att-mail.com>
Cc: netdev@vger.kernel.org <netdev@vger.kernel.org>
Subject: Re: VRF Issue Since kernel 5
On 9/11/19 3:01 PM, Gowen wrote:
> Hi all,
>
> It looks like ip vrf exec checks /etc/resolv.conf (found with strace -e
> trace=file sudo ip vrf exec mgmt-vrf host www.google.co.uk &>
> ~/straceFileOfVrfHost.txt) , but as I'm on an Azure machine using
> netplan, this file isn't updated with DNS servers. I have added my DNS
> server to resolv.conf and now can update the cache with "sudo ip vrf
> exec sudo apt update", if I am correct (which I'm not sure about as not
> really my area) then this might be affecting more than just me.
>
> Also still not able to fix the updating cache from global VRF - which
> would cause bother in prod environment to others as well so think it
> would be good to get an RCA for it?
>
> thanks for your help so far, has been really interesting.
>
> Gareth
>
>
> ------------------------------------------------------------------------
> *From:* Gowen <gowen@potatocomputing.co.uk>
> *Sent:* 11 September 2019 13:48
> *To:* David Ahern <dsahern@gmail.com>; Alexis Bauvin
> <abauvin@online.net>; mmanning@vyatta.att-mail.com
> <mmanning@vyatta.att-mail.com>
> *Cc:* netdev@vger.kernel.org <netdev@vger.kernel.org>
> *Subject:* Re: VRF Issue Since kernel 5
>
> yep no problem:
>
> Admin@NETM06:~$ sudo sysctl -a | grep l3mdev
> net.ipv4.raw_l3mdev_accept = 1
> net.ipv4.tcp_l3mdev_accept = 1
> net.ipv4.udp_l3mdev_accept = 1
>
>
> The source of the DNS issue in the vrf exec command is something to do
> with networkd managing the DNS servers, I can fix it by explicitly
> mentioning the DNS server:
>
> systemd-resolve --status --no-page
>
> <OUTPUT OMITTED>
>
> Link 4 (mgmt-vrf)
> Current Scopes: none
> LLMNR setting: yes
> MulticastDNS setting: no
> DNSSEC setting: no
> DNSSEC supported: no
>
> Link 3 (eth1)
> Current Scopes: DNS
> LLMNR setting: yes
> MulticastDNS setting: no
> DNSSEC setting: no
> DNSSEC supported: no
> DNS Servers: 10.24.65.203
> 10.24.65.204
> 10.25.65.203
> 10.25.65.204
> DNS Domain: reddog.microsoft.com
>
> Link 2 (eth0)
> Current Scopes: DNS
> LLMNR setting: yes
> MulticastDNS setting: no
> DNSSEC setting: no
> DNSSEC supported: no
> DNS Servers: 10.24.65.203
> 10.24.65.204
> 10.25.65.203
> 10.25.65.204
> DNS Domain: reddog.microsoft.com
>
> there is no DNS server when I use ip vrf exec command (tcpdump shows
> only loopback traffic when invoked without my DNS sever explicitly
> entered) - odd as mgmt-vrf isnt L3 device so thought it would pick up
> eth0 DNS servers?
>
> I dont think this helps with my update cache traffic from global vrf
> though on port 80
>
Let's back up a bit: your subject line says vrf issue since kernel 5.
Did you update / change the OS as well?
ie., the previous version that worked what is the OS and kernel version?
What is the OS and kernel version with the problem?
^ permalink raw reply
* KASAN: use-after-free Read in __xfrm_decode_session
From: syzbot @ 2019-09-12 8:02 UTC (permalink / raw)
To: davem, herbert, linux-kernel, netdev, steffen.klassert,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 6d028043 Add linux-next specific files for 20190830
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=150de9b9600000
kernel config: https://syzkaller.appspot.com/x/.config?x=82a6bec43ab0cb69
dashboard link: https://syzkaller.appspot.com/bug?extid=55d9cf7c57894c1e4860
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+55d9cf7c57894c1e4860@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in decode_session6 net/xfrm/xfrm_policy.c:3390
[inline]
BUG: KASAN: use-after-free in __xfrm_decode_session+0x1cfb/0x2e90
net/xfrm/xfrm_policy.c:3482
Read of size 1 at addr ffff88805cdb288e by task syz-executor.0/24778
CPU: 0 PID: 24778 Comm: syz-executor.0 Not tainted 5.3.0-rc6-next-20190830
#75
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
__kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506
kasan_report+0x12/0x20 mm/kasan/common.c:634
__asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:129
decode_session6 net/xfrm/xfrm_policy.c:3390 [inline]
__xfrm_decode_session+0x1cfb/0x2e90 net/xfrm/xfrm_policy.c:3482
xfrm_decode_session_reverse include/net/xfrm.h:1144 [inline]
icmpv6_route_lookup+0x31b/0x4d0 net/ipv6/icmp.c:369
icmp6_send+0x129e/0x1e20 net/ipv6/icmp.c:557
icmpv6_send+0xec/0x230 net/ipv6/ip6_icmp.c:43
ip6_link_failure+0x2b/0x530 net/ipv6/route.c:2640
dst_link_failure include/net/dst.h:419 [inline]
ip6_tnl_xmit+0x45b/0x33f0 net/ipv6/ip6_tunnel.c:1222
ip6ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1376 [inline]
ip6_tnl_start_xmit+0x847/0x1c90 net/ipv6/ip6_tunnel.c:1402
__netdev_start_xmit include/linux/netdevice.h:4419 [inline]
netdev_start_xmit include/linux/netdevice.h:4433 [inline]
xmit_one net/core/dev.c:3280 [inline]
dev_hard_start_xmit+0x1a3/0x9c0 net/core/dev.c:3296
sch_direct_xmit+0x372/0xc30 net/sched/sch_generic.c:308
qdisc_restart net/sched/sch_generic.c:371 [inline]
__qdisc_run+0x586/0x19d0 net/sched/sch_generic.c:379
__dev_xmit_skb net/core/dev.c:3533 [inline]
__dev_queue_xmit+0x16f1/0x37c0 net/core/dev.c:3838
dev_queue_xmit+0x18/0x20 net/core/dev.c:3902
neigh_direct_output+0x16/0x20 net/core/neighbour.c:1530
neigh_output include/net/neighbour.h:511 [inline]
ip6_finish_output2+0x1034/0x2550 net/ipv6/ip6_output.c:116
__ip6_finish_output+0x444/0xaa0 net/ipv6/ip6_output.c:142
ip6_finish_output+0x38/0x1f0 net/ipv6/ip6_output.c:152
NF_HOOK_COND include/linux/netfilter.h:294 [inline]
ip6_output+0x235/0x7f0 net/ipv6/ip6_output.c:175
dst_output include/net/dst.h:436 [inline]
NF_HOOK include/linux/netfilter.h:305 [inline]
NF_HOOK include/linux/netfilter.h:299 [inline]
ip6_xmit+0xe35/0x20b0 net/ipv6/ip6_output.c:279
inet6_csk_xmit+0x2fb/0x5d0 net/ipv6/inet6_connection_sock.c:135
__tcp_transmit_skb+0x1a2f/0x3820 net/ipv4/tcp_output.c:1158
tcp_transmit_skb net/ipv4/tcp_output.c:1174 [inline]
tcp_send_syn_data net/ipv4/tcp_output.c:3505 [inline]
tcp_connect+0x1be7/0x4320 net/ipv4/tcp_output.c:3570
tcp_sendmsg_fastopen net/ipv4/tcp.c:1155 [inline]
tcp_sendmsg_locked+0x2898/0x3220 net/ipv4/tcp.c:1206
tcp_sendmsg+0x30/0x50 net/ipv4/tcp.c:1434
inet6_sendmsg+0x9e/0xe0 net/ipv6/af_inet6.c:576
sock_sendmsg_nosec net/socket.c:637 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:657
__sys_sendto+0x262/0x380 net/socket.c:1952
__do_sys_sendto net/socket.c:1964 [inline]
__se_sys_sendto net/socket.c:1960 [inline]
__x64_sys_sendto+0xe1/0x1a0 net/socket.c:1960
do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4598e9
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f83d1625c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000000000000006 RCX: 00000000004598e9
RDX: 0000000000000001 RSI: 0000000020000340 RDI: 0000000000000003
RBP: 000000000075bfc8 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000200400cf R11: 0000000000000246 R12: 00007f83d16266d4
R13: 00000000004c7880 R14: 00000000004dd188 R15: 00000000ffffffff
Allocated by task 3891:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:510 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:483
kasan_kmalloc+0x9/0x10 mm/kasan/common.c:524
__do_kmalloc mm/slab.c:3655 [inline]
__kmalloc+0x163/0x770 mm/slab.c:3664
kmalloc include/linux/slab.h:561 [inline]
tomoyo_realpath_from_path+0xcd/0x7b0 security/tomoyo/realpath.c:277
tomoyo_get_realpath security/tomoyo/file.c:151 [inline]
tomoyo_path_number_perm+0x1dd/0x520 security/tomoyo/file.c:723
tomoyo_file_ioctl+0x23/0x30 security/tomoyo/tomoyo.c:335
security_file_ioctl+0x77/0xc0 security/security.c:1409
ksys_ioctl+0x57/0xd0 fs/ioctl.c:711
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 3891:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
kasan_set_free_info mm/kasan/common.c:332 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:471
kasan_slab_free+0xe/0x10 mm/kasan/common.c:480
__cache_free mm/slab.c:3425 [inline]
kfree+0x10a/0x2c0 mm/slab.c:3756
tomoyo_realpath_from_path+0x1de/0x7b0 security/tomoyo/realpath.c:319
tomoyo_get_realpath security/tomoyo/file.c:151 [inline]
tomoyo_path_number_perm+0x1dd/0x520 security/tomoyo/file.c:723
tomoyo_file_ioctl+0x23/0x30 security/tomoyo/tomoyo.c:335
security_file_ioctl+0x77/0xc0 security/security.c:1409
ksys_ioctl+0x57/0xd0 fs/ioctl.c:711
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff88805cdb2000
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 2190 bytes inside of
4096-byte region [ffff88805cdb2000, ffff88805cdb3000)
The buggy address belongs to the page:
page:ffffea0001736c80 refcount:1 mapcount:0 mapping:ffff8880aa402000
index:0x0 compound_mapcount: 0
flags: 0x1fffc0000010200(slab|head)
raw: 01fffc0000010200 ffffea0001941688 ffffea0002364008 ffff8880aa402000
raw: 0000000000000000 ffff88805cdb2000 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88805cdb2780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88805cdb2800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff88805cdb2880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88805cdb2900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88805cdb2980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
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.
^ permalink raw reply
* Re: [PATCH V2 net-next 1/7] net: hns3: add ethtool_ops.set_channels support for HNS3 VF driver
From: tanhuazhong @ 2019-09-12 8:20 UTC (permalink / raw)
To: Michal Kubecek, netdev
Cc: davem, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski
In-Reply-To: <20190912062301.GE24779@unicorn.suse.cz>
Hi, Michal
On 2019/9/12 14:23, Michal Kubecek wrote:
> On Wed, Sep 11, 2019 at 10:40:33AM +0800, Huazhong Tan wrote:
>> From: Guangbin Huang <huangguangbin2@huawei.com>
>>
>> This patch adds ethtool_ops.set_channels support for HNS3 VF driver,
>> and updates related TQP information and RSS information, to support
>> modification of VF TQP number, and uses current rss_size instead of
>> max_rss_size to initialize RSS.
>>
>> Also, fixes a format error in hclgevf_get_rss().
>>
>> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
>> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
>> ---
>> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 1 +
>> .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 83 ++++++++++++++++++++--
>> 2 files changed, 79 insertions(+), 5 deletions(-)
>>
> ...
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
>> index 594cae8..e3090b3 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> ...
>> +static void hclgevf_update_rss_size(struct hnae3_handle *handle,
>> + u32 new_tqps_num)
>> +{
>> + struct hnae3_knic_private_info *kinfo = &handle->kinfo;
>> + struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
>> + u16 max_rss_size;
>> +
>> + kinfo->req_rss_size = new_tqps_num;
>> +
>> + max_rss_size = min_t(u16, hdev->rss_size_max,
>> + hdev->num_tqps / kinfo->num_tc);
>> +
>> + /* Use the user's configuration when it is not larger than
>> + * max_rss_size, otherwise, use the maximum specification value.
>> + */
>> + if (kinfo->req_rss_size != kinfo->rss_size && kinfo->req_rss_size &&
>> + kinfo->req_rss_size <= max_rss_size)
>> + kinfo->rss_size = kinfo->req_rss_size;
>> + else if (kinfo->rss_size > max_rss_size ||
>> + (!kinfo->req_rss_size && kinfo->rss_size < max_rss_size))
>> + kinfo->rss_size = max_rss_size;
>
> I don't think requested channel count can be larger than max_rss_size
> here. In ethtool_set_channels(), we check that requested channel counts
> do not exceed maximum channel counts as reported by ->get_channels().
> And hclgevf_get_max_channels() cannot return more than max_rss_size.
>
When we can modify the TC number (which PF has already supported, VF may
implement in the future) using lldptool or tc cmd,
hclgevf_update_rss_size will be called to update the rss information,
which may also change max_rss_size, so we will use max_rss_size instead
if the kinfo->rss_size configured using ethtool is bigger than max_rss_size.
>> +
>> + kinfo->num_tqps = kinfo->num_tc * kinfo->rss_size;
>> +}
>> +
>> +static int hclgevf_set_channels(struct hnae3_handle *handle, u32 new_tqps_num,
>> + bool rxfh_configured)
>> +{
>> + struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
>> + struct hnae3_knic_private_info *kinfo = &handle->kinfo;
>> + u16 cur_rss_size = kinfo->rss_size;
>> + u16 cur_tqps = kinfo->num_tqps;
>> + u32 *rss_indir;
>> + unsigned int i;
>> + int ret;
>> +
>> + hclgevf_update_rss_size(handle, new_tqps_num);
>> +
>> + ret = hclgevf_set_rss_tc_mode(hdev, kinfo->rss_size);
>> + if (ret)
>> + return ret;
>> +
>> + /* RSS indirection table has been configuared by user */
>> + if (rxfh_configured)
>> + goto out;
>> +
>> + /* Reinitializes the rss indirect table according to the new RSS size */
>> + rss_indir = kcalloc(HCLGEVF_RSS_IND_TBL_SIZE, sizeof(u32), GFP_KERNEL);
>> + if (!rss_indir)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < HCLGEVF_RSS_IND_TBL_SIZE; i++)
>> + rss_indir[i] = i % kinfo->rss_size;
>> +
>> + ret = hclgevf_set_rss(handle, rss_indir, NULL, 0);
>> + if (ret)
>> + dev_err(&hdev->pdev->dev, "set rss indir table fail, ret=%d\n",
>> + ret);
>> +
>> + kfree(rss_indir);
>> +
>> +out:
>> + if (!ret)
>> + dev_info(&hdev->pdev->dev,
>> + "Channels changed, rss_size from %u to %u, tqps from %u to %u",
>> + cur_rss_size, kinfo->rss_size,
>> + cur_tqps, kinfo->rss_size * kinfo->num_tc);
>> +
>> + return ret;
>> +}
>
> IIRC David asked you not to issue this log message in v1 review.
>
> Michal Kubecek
>
Sorry for missing this log.
Thanks.
> .
>
^ permalink raw reply
* [PATCH net,stable] cdc_ether: fix rndis support for Mediatek based smartphones
From: Bjørn Mork @ 2019-09-12 8:42 UTC (permalink / raw)
To: netdev; +Cc: Oliver Neukum, linux-usb, Lars Melin, Bjørn Mork
A Mediatek based smartphone owner reports problems with USB
tethering in Linux. The verbose USB listing shows a rndis_host
interface pair (e0/01/03 + 10/00/00), but the driver fails to
bind with
[ 355.960428] usb 1-4: bad CDC descriptors
The problem is a failsafe test intended to filter out ACM serial
functions using the same 02/02/ff class/subclass/protocol as RNDIS.
The serial functions are recognized by their non-zero bmCapabilities.
No RNDIS function with non-zero bmCapabilities were known at the time
this failsafe was added. But it turns out that some Wireless class
RNDIS functions are using the bmCapabilities field. These functions
are uniquely identified as RNDIS by their class/subclass/protocol, so
the failing test can safely be disabled. The same applies to the two
types of Misc class RNDIS functions.
Applying the failsafe to Communication class functions only retains
the original functionality, and fixes the problem for the Mediatek based
smartphone.
Tow examples of CDC functional descriptors with non-zero bmCapabilities
from Wireless class RNDIS functions are:
0e8d:000a Mediatek Crosscall Spider X5 3G Phone
CDC Header:
bcdCDC 1.10
CDC ACM:
bmCapabilities 0x0f
connection notifications
sends break
line coding and serial state
get/set/clear comm features
CDC Union:
bMasterInterface 0
bSlaveInterface 1
CDC Call Management:
bmCapabilities 0x03
call management
use DataInterface
bDataInterface 1
and
19d2:1023 ZTE K4201-z
CDC Header:
bcdCDC 1.10
CDC ACM:
bmCapabilities 0x02
line coding and serial state
CDC Call Management:
bmCapabilities 0x03
call management
use DataInterface
bDataInterface 1
CDC Union:
bMasterInterface 0
bSlaveInterface 1
The Mediatek example is believed to apply to most smartphones with
Mediatek firmware. The ZTE example is most likely also part of a larger
family of devices/firmwares.
Suggested-by: Lars Melin <larsm17@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ether.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 8458e88c18e9..32f53de5b1fe 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -206,7 +206,15 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
goto bad_desc;
}
skip:
- if (rndis && header.usb_cdc_acm_descriptor &&
+ /* Communcation class functions with bmCapabilities are not
+ * RNDIS. But some Wireless class RNDIS functions use
+ * bmCapabilities for their own purpose. The failsafe is
+ * therefore applied only to Communication class RNDIS
+ * functions. The rndis test is redundant, but a cheap
+ * optimization.
+ */
+ if (rndis && is_rndis(&intf->cur_altsetting->desc) &&
+ header.usb_cdc_acm_descriptor &&
header.usb_cdc_acm_descriptor->bmCapabilities) {
dev_dbg(&intf->dev,
"ACM capabilities %02x, not really RNDIS?\n",
--
2.20.1
^ permalink raw reply related
* [patch net-next v3 0/3] net: devlink: move reload fail indication to devlink core and expose to user
From: Jiri Pirko @ 2019-09-12 8:49 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, dsahern, jakub.kicinski, tariqt, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
First two patches are dependencies of the last one. That moves devlink
reload failure indication to the devlink code, so the drivers do not
have to track it themselves. Currently it is only mlxsw, but I will send
a follow-up patchset that introduces this in netdevsim too.
Jiri Pirko (3):
mlx4: Split restart_one into two functions
net: devlink: split reload op into two
net: devlink: move reload fail indication to devlink core and expose
to user
drivers/net/ethernet/mellanox/mlx4/catas.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 44 ++++++++++++++++++----
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 +-
drivers/net/ethernet/mellanox/mlxsw/core.c | 30 +++++++++------
drivers/net/netdevsim/dev.c | 13 +++++--
include/net/devlink.h | 8 +++-
include/uapi/linux/devlink.h | 2 +
net/core/devlink.c | 35 +++++++++++++++--
8 files changed, 106 insertions(+), 31 deletions(-)
--
2.21.0
^ permalink raw reply
* [patch net-next v3 1/3] mlx4: Split restart_one into two functions
From: Jiri Pirko @ 2019-09-12 8:49 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, dsahern, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <20190912084946.7468-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Split the function restart_one into two functions and separate teardown
and buildup.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/catas.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 25 ++++++++++++++++++----
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 +--
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/catas.c b/drivers/net/ethernet/mellanox/mlx4/catas.c
index 87e90b5d4d7d..5b11557f1ae4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/catas.c
+++ b/drivers/net/ethernet/mellanox/mlx4/catas.c
@@ -210,7 +210,7 @@ static void mlx4_handle_error_state(struct mlx4_dev_persistent *persist)
mutex_lock(&persist->interface_state_mutex);
if (persist->interface_state & MLX4_INTERFACE_STATE_UP &&
!(persist->interface_state & MLX4_INTERFACE_STATE_DELETION)) {
- err = mlx4_restart_one(persist->pdev, false, NULL);
+ err = mlx4_restart_one(persist->pdev);
mlx4_info(persist->dev, "mlx4_restart_one was ended, ret=%d\n",
err);
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 07c204bd3fc4..a39c647c12dc 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3931,6 +3931,10 @@ static void mlx4_devlink_param_load_driverinit_values(struct devlink *devlink)
}
}
+static void mlx4_restart_one_down(struct pci_dev *pdev);
+static int mlx4_restart_one_up(struct pci_dev *pdev, bool reload,
+ struct devlink *devlink);
+
static int mlx4_devlink_reload(struct devlink *devlink,
struct netlink_ext_ack *extack)
{
@@ -3941,9 +3945,11 @@ static int mlx4_devlink_reload(struct devlink *devlink,
if (persist->num_vfs)
mlx4_warn(persist->dev, "Reload performed on PF, will cause reset on operating Virtual Functions\n");
- err = mlx4_restart_one(persist->pdev, true, devlink);
+ mlx4_restart_one_down(persist->pdev);
+ err = mlx4_restart_one_up(persist->pdev, true, devlink);
if (err)
- mlx4_err(persist->dev, "mlx4_restart_one failed, ret=%d\n", err);
+ mlx4_err(persist->dev, "mlx4_restart_one_up failed, ret=%d\n",
+ err);
return err;
}
@@ -4163,7 +4169,13 @@ static int restore_current_port_types(struct mlx4_dev *dev,
return err;
}
-int mlx4_restart_one(struct pci_dev *pdev, bool reload, struct devlink *devlink)
+static void mlx4_restart_one_down(struct pci_dev *pdev)
+{
+ mlx4_unload_one(pdev);
+}
+
+static int mlx4_restart_one_up(struct pci_dev *pdev, bool reload,
+ struct devlink *devlink)
{
struct mlx4_dev_persistent *persist = pci_get_drvdata(pdev);
struct mlx4_dev *dev = persist->dev;
@@ -4175,7 +4187,6 @@ int mlx4_restart_one(struct pci_dev *pdev, bool reload, struct devlink *devlink)
total_vfs = dev->persist->num_vfs;
memcpy(nvfs, dev->persist->nvfs, sizeof(dev->persist->nvfs));
- mlx4_unload_one(pdev);
if (reload)
mlx4_devlink_param_load_driverinit_values(devlink);
err = mlx4_load_one(pdev, pci_dev_data, total_vfs, nvfs, priv, 1);
@@ -4194,6 +4205,12 @@ int mlx4_restart_one(struct pci_dev *pdev, bool reload, struct devlink *devlink)
return err;
}
+int mlx4_restart_one(struct pci_dev *pdev)
+{
+ mlx4_restart_one_down(pdev);
+ return mlx4_restart_one_up(pdev, false, NULL);
+}
+
#define MLX_SP(id) { PCI_VDEVICE(MELLANOX, id), MLX4_PCI_DEV_FORCE_SENSE_PORT }
#define MLX_VF(id) { PCI_VDEVICE(MELLANOX, id), MLX4_PCI_DEV_IS_VF }
#define MLX_GN(id) { PCI_VDEVICE(MELLANOX, id), 0 }
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 23f1b5b512c2..527b52e48276 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -1043,8 +1043,7 @@ int mlx4_catas_init(struct mlx4_dev *dev);
void mlx4_catas_end(struct mlx4_dev *dev);
int mlx4_crdump_init(struct mlx4_dev *dev);
void mlx4_crdump_end(struct mlx4_dev *dev);
-int mlx4_restart_one(struct pci_dev *pdev, bool reload,
- struct devlink *devlink);
+int mlx4_restart_one(struct pci_dev *pdev);
int mlx4_register_device(struct mlx4_dev *dev);
void mlx4_unregister_device(struct mlx4_dev *dev);
void mlx4_dispatch_event(struct mlx4_dev *dev, enum mlx4_dev_event type,
--
2.21.0
^ permalink raw reply related
* [patch net-next v3 2/3] net: devlink: split reload op into two
From: Jiri Pirko @ 2019-09-12 8:49 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, dsahern, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <20190912084946.7468-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
In order to properly implement failure indication during reload,
split the reload op into two ops, one for down phase and one for
up phase.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 19 +++++++++++++++----
drivers/net/ethernet/mellanox/mlxsw/core.c | 19 +++++++++++++++----
drivers/net/netdevsim/dev.c | 13 ++++++++++---
include/net/devlink.h | 5 ++++-
net/core/devlink.c | 16 ++++++++++++----
5 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index a39c647c12dc..ef3f3d06ff1e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3935,17 +3935,27 @@ static void mlx4_restart_one_down(struct pci_dev *pdev);
static int mlx4_restart_one_up(struct pci_dev *pdev, bool reload,
struct devlink *devlink);
-static int mlx4_devlink_reload(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+static int mlx4_devlink_reload_down(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
{
struct mlx4_priv *priv = devlink_priv(devlink);
struct mlx4_dev *dev = &priv->dev;
struct mlx4_dev_persistent *persist = dev->persist;
- int err;
if (persist->num_vfs)
mlx4_warn(persist->dev, "Reload performed on PF, will cause reset on operating Virtual Functions\n");
mlx4_restart_one_down(persist->pdev);
+ return 0;
+}
+
+static int mlx4_devlink_reload_up(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx4_priv *priv = devlink_priv(devlink);
+ struct mlx4_dev *dev = &priv->dev;
+ struct mlx4_dev_persistent *persist = dev->persist;
+ int err;
+
err = mlx4_restart_one_up(persist->pdev, true, devlink);
if (err)
mlx4_err(persist->dev, "mlx4_restart_one_up failed, ret=%d\n",
@@ -3956,7 +3966,8 @@ static int mlx4_devlink_reload(struct devlink *devlink,
static const struct devlink_ops mlx4_devlink_ops = {
.port_type_set = mlx4_devlink_port_type_set,
- .reload = mlx4_devlink_reload,
+ .reload_down = mlx4_devlink_reload_down,
+ .reload_up = mlx4_devlink_reload_up,
};
static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 963a2b4b61b1..c71a1d9ea17b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -984,16 +984,26 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
return 0;
}
-static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+static int
+mlxsw_devlink_core_bus_device_reload_down(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- int err;
if (!(mlxsw_core->bus->features & MLXSW_BUS_F_RESET))
return -EOPNOTSUPP;
mlxsw_core_bus_device_unregister(mlxsw_core, true);
+ return 0;
+}
+
+static int
+mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+ int err;
+
err = mlxsw_core_bus_device_register(mlxsw_core->bus_info,
mlxsw_core->bus,
mlxsw_core->bus_priv, true,
@@ -1066,7 +1076,8 @@ mlxsw_devlink_trap_group_init(struct devlink *devlink,
}
static const struct devlink_ops mlxsw_devlink_ops = {
- .reload = mlxsw_devlink_core_bus_device_reload,
+ .reload_down = mlxsw_devlink_core_bus_device_reload_down,
+ .reload_up = mlxsw_devlink_core_bus_device_reload_up,
.port_type_set = mlxsw_devlink_port_type_set,
.port_split = mlxsw_devlink_port_split,
.port_unsplit = mlxsw_devlink_port_unsplit,
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 39cdb6c18ec0..7fba7b271a57 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -521,8 +521,14 @@ static void nsim_dev_traps_exit(struct devlink *devlink)
kfree(nsim_dev->trap_data);
}
-static int nsim_dev_reload(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+static int nsim_dev_reload_down(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
+{
+ return 0;
+}
+
+static int nsim_dev_reload_up(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
{
enum nsim_resource_id res_ids[] = {
NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
@@ -638,7 +644,8 @@ nsim_dev_devlink_trap_action_set(struct devlink *devlink,
}
static const struct devlink_ops nsim_dev_devlink_ops = {
- .reload = nsim_dev_reload,
+ .reload_down = nsim_dev_reload_down,
+ .reload_up = nsim_dev_reload_up,
.flash_update = nsim_dev_flash_update,
.trap_init = nsim_dev_devlink_trap_init,
.trap_action_set = nsim_dev_devlink_trap_action_set,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 03e4d9244ff3..ab8d56d12ffd 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -642,7 +642,10 @@ enum devlink_trap_group_generic_id {
}
struct devlink_ops {
- int (*reload)(struct devlink *devlink, struct netlink_ext_ack *extack);
+ int (*reload_down)(struct devlink *devlink,
+ struct netlink_ext_ack *extack);
+ int (*reload_up)(struct devlink *devlink,
+ struct netlink_ext_ack *extack);
int (*port_type_set)(struct devlink_port *devlink_port,
enum devlink_port_type port_type);
int (*port_split)(struct devlink *devlink, unsigned int port_index,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4a2fb94c44cf..9e522639693d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2672,12 +2672,17 @@ devlink_resources_validate(struct devlink *devlink,
return err;
}
+static bool devlink_reload_supported(struct devlink *devlink)
+{
+ return devlink->ops->reload_down && devlink->ops->reload_up;
+}
+
static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
int err;
- if (!devlink->ops->reload)
+ if (!devlink_reload_supported(devlink))
return -EOPNOTSUPP;
err = devlink_resources_validate(devlink, NULL, info);
@@ -2685,7 +2690,10 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
return err;
}
- return devlink->ops->reload(devlink, info->extack);
+ err = devlink->ops->reload_down(devlink, info->extack);
+ if (err)
+ return err;
+ return devlink->ops->reload_up(devlink, info->extack);
}
static int devlink_nl_flash_update_fill(struct sk_buff *msg,
@@ -7150,7 +7158,7 @@ __devlink_param_driverinit_value_set(struct devlink *devlink,
int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
union devlink_param_value *init_val)
{
- if (!devlink->ops->reload)
+ if (!devlink_reload_supported(devlink))
return -EOPNOTSUPP;
return __devlink_param_driverinit_value_get(&devlink->param_list,
@@ -7197,7 +7205,7 @@ int devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
{
struct devlink *devlink = devlink_port->devlink;
- if (!devlink->ops->reload)
+ if (!devlink_reload_supported(devlink))
return -EOPNOTSUPP;
return __devlink_param_driverinit_value_get(&devlink_port->param_list,
--
2.21.0
^ permalink raw reply related
* [patch net-next v3 3/3] net: devlink: move reload fail indication to devlink core and expose to user
From: Jiri Pirko @ 2019-09-12 8:49 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, dsahern, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <20190912084946.7468-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Currently the fact that devlink reload failed is stored in drivers.
Move this flag into devlink core. Also, expose it to the user.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v2->v3:
- make arg of devlink_is_reload_failed const
v1->v2:
- s/devlink failed/devlink reload failed/ in description
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 15 +++++----------
include/net/devlink.h | 3 +++
include/uapi/linux/devlink.h | 2 ++
net/core/devlink.c | 21 ++++++++++++++++++++-
4 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index c71a1d9ea17b..3fa96076e8a5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -80,7 +80,6 @@ struct mlxsw_core {
struct mlxsw_thermal *thermal;
struct mlxsw_core_port *ports;
unsigned int max_ports;
- bool reload_fail;
bool fw_flash_in_progress;
unsigned long driver_priv[0];
/* driver_priv has to be always the last item */
@@ -1002,15 +1001,11 @@ mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink,
struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- int err;
-
- err = mlxsw_core_bus_device_register(mlxsw_core->bus_info,
- mlxsw_core->bus,
- mlxsw_core->bus_priv, true,
- devlink);
- mlxsw_core->reload_fail = !!err;
- return err;
+ return mlxsw_core_bus_device_register(mlxsw_core->bus_info,
+ mlxsw_core->bus,
+ mlxsw_core->bus_priv, true,
+ devlink);
}
static int mlxsw_devlink_flash_update(struct devlink *devlink,
@@ -1254,7 +1249,7 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
{
struct devlink *devlink = priv_to_devlink(mlxsw_core);
- if (mlxsw_core->reload_fail) {
+ if (devlink_is_reload_failed(devlink)) {
if (!reload)
/* Only the parts that were not de-initialized in the
* failed reload attempt need to be de-initialized.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index ab8d56d12ffd..23e4b65ec9df 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -38,6 +38,7 @@ struct devlink {
struct device *dev;
possible_net_t _net;
struct mutex lock;
+ bool reload_failed;
char priv[0] __aligned(NETDEV_ALIGN);
};
@@ -945,6 +946,8 @@ void
devlink_health_reporter_state_update(struct devlink_health_reporter *reporter,
enum devlink_health_reporter_state state);
+bool devlink_is_reload_failed(const struct devlink *devlink);
+
void devlink_flash_update_begin_notify(struct devlink *devlink);
void devlink_flash_update_end_notify(struct devlink *devlink);
void devlink_flash_update_status_notify(struct devlink *devlink,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 8da5365850cd..580b7a2e40e1 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -419,6 +419,8 @@ enum devlink_attr {
DEVLINK_ATTR_TRAP_METADATA, /* nested */
DEVLINK_ATTR_TRAP_GROUP_NAME, /* string */
+ DEVLINK_ATTR_RELOAD_FAILED, /* u8 0 or 1 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 9e522639693d..e48680efe54a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -471,6 +471,8 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
if (devlink_nl_put_handle(msg, devlink))
goto nla_put_failure;
+ if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_FAILED, devlink->reload_failed))
+ goto nla_put_failure;
genlmsg_end(msg, hdr);
return 0;
@@ -2677,6 +2679,21 @@ static bool devlink_reload_supported(struct devlink *devlink)
return devlink->ops->reload_down && devlink->ops->reload_up;
}
+static void devlink_reload_failed_set(struct devlink *devlink,
+ bool reload_failed)
+{
+ if (devlink->reload_failed == reload_failed)
+ return;
+ devlink->reload_failed = reload_failed;
+ devlink_notify(devlink, DEVLINK_CMD_NEW);
+}
+
+bool devlink_is_reload_failed(const struct devlink *devlink)
+{
+ return devlink->reload_failed;
+}
+EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
+
static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
@@ -2693,7 +2710,9 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
err = devlink->ops->reload_down(devlink, info->extack);
if (err)
return err;
- return devlink->ops->reload_up(devlink, info->extack);
+ err = devlink->ops->reload_up(devlink, info->extack);
+ devlink_reload_failed_set(devlink, !!err);
+ return err;
}
static int devlink_nl_flash_update_fill(struct sk_buff *msg,
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms
From: Ido Schimmel @ 2019-09-12 9:03 UTC (permalink / raw)
To: Robert Beckett
Cc: Florian Fainelli, netdev@vger.kernel.org, Andrew Lunn,
Vivien Didelot, David S. Miller, Jiri Pirko
In-Reply-To: <3f50ee51ec04a2d683a5338a68607824a3f45711.camel@collabora.com>
On Wed, Sep 11, 2019 at 12:49:03PM +0100, Robert Beckett wrote:
> On Wed, 2019-09-11 at 11:21 +0000, Ido Schimmel wrote:
> > On Tue, Sep 10, 2019 at 09:49:46AM -0700, Florian Fainelli wrote:
> > > +Ido, Jiri,
> > >
> > > On 9/10/19 8:41 AM, Robert Beckett wrote:
> > > > This patch-set adds support for some features of the Marvell
> > > > switch
> > > > chips that can be used to handle packet storms.
> > > >
> > > > The rationale for this was a setup that requires the ability to
> > > > receive
> > > > traffic from one port, while a packet storm is occuring on
> > > > another port
> > > > (via an external switch with a deliberate loop). This is needed
> > > > to
> > > > ensure vital data delivery from a specific port, while mitigating
> > > > any
> > > > loops or DoS that a user may introduce on another port (can't
> > > > guarantee
> > > > sensible users).
> > >
> > > The use case is reasonable, but the implementation is not really.
> > > You
> > > are using Device Tree which is meant to describe hardware as a
> > > policy
> > > holder for setting up queue priorities and likewise for queue
> > > scheduling.
> > >
> > > The tool that should be used for that purpose is tc and possibly an
> > > appropriately offloaded queue scheduler in order to map the desired
> > > scheduling class to what the hardware supports.
> > >
> > > Jiri, Ido, how do you guys support this with mlxsw?
> >
> > Hi Florian,
> >
> > Are you referring to policing traffic towards the CPU using a policer
> > on
> > the egress of the CPU port? At least that's what I understand from
> > the
> > description of patch 6 below.
> >
> > If so, mlxsw sets policers for different traffic types during its
> > initialization sequence. These policers are not exposed to the user
> > nor
> > configurable. While the default settings are good for most users, we
> > do
> > want to allow users to change these and expose current settings.
> >
> > I agree that tc seems like the right choice, but the question is
> > where
> > are we going to install the filters?
> >
>
> Before I go too far down the rabbit hole of tc traffic shaping, maybe
> it would be good to explain in more detail the problem I am trying to
> solve.
>
> We have a setup as follows:
>
> Marvell 88E6240 switch chip, accepting traffic from 4 ports. Port 1
> (P1) is critical priority, no dropped packets allowed, all others can
> be best effort.
>
> CPU port of swtich chip is connected via phy to phy of intel i210 (igb
> driver).
>
> i210 is connected via pcie switch to imx6.
>
> When too many small packets attempt to be delivered to CPU port (e.g.
> during broadcast flood) we saw dropped packets.
>
> The packets were being received by i210 in to rx descriptor buffer
> fine, but the CPU could not keep up with the load. We saw
> rx_fifo_errors increasing rapidly and ksoftirqd at ~100% CPU.
>
>
> With this in mind, I am wondering whether any amount of tc traffic
> shaping would help? Would tc shaping require that the packet reception
> manages to keep up before it can enact its policies? Does the
> infrastructure have accelerator offload hooks to be able to apply it
> via HW? I dont see how it would be able to inspect the packets to apply
> filtering if they were dropped due to rx descriptor exhaustion. (please
> bear with me with the basic questions, I am not familiar with this part
> of the stack).
>
> Assuming that tc is still the way to go, after a brief look in to the
> man pages and the documentation at largc.org, it seems like it would
> need to use the ingress qdisc, with some sort of system to segregate
> and priortise based on ingress port. Is this possible?
Hi Robert,
As I see it, you have two problems here:
1. Classification: Based on ingress port in your case
2. Scheduling: How to schedule between the different transmission queues
Where the port from which the packets should egress is the CPU port,
before they cross the PCI towards the imx6.
Both of these issues can be solved by tc. The main problem is that today
we do not have a netdev to represent the CPU port and therefore can't
use existing infra like tc. I believe we need to create one. Besides
scheduling, we can also use it to permit/deny certain traffic from
reaching the CPU and perform policing.
Drivers can run the received packets via taps using
dev_queue_xmit_nit(), so that users will see all the traffic directed at
the host when running tcpdump on this netdev.
>
>
>
> > >
> > > >
> > > > [patch 1/7] configures auto negotiation for CPU ports connected
> > > > with
> > > > phys to enable pause frame propogation.
> > > >
> > > > [patch 2/7] allows setting of port's default output queue
> > > > priority for
> > > > any ingressing packets on that port.
> > > >
> > > > [patch 3/7] dt-bindings for patch 2.
> > > >
> > > > [patch 4/7] allows setting of a port's queue scheduling so that
> > > > it can
> > > > prioritise egress of traffic routed from high priority ports.
> > > >
> > > > [patch 5/7] dt-bindings for patch 4.
> > > >
> > > > [patch 6/7] allows ports to rate limit their egress. This can be
> > > > used to
> > > > stop the host CPU from becoming swamped by packet delivery and
> > > > exhasting
> > > > descriptors.
> > > >
> > > > [patch 7/7] dt-bindings for patch 6.
> > > >
> > > >
> > > > Robert Beckett (7):
> > > > net/dsa: configure autoneg for CPU port
> > > > net: dsa: mv88e6xxx: add ability to set default queue
> > > > priorities per
> > > > port
> > > > dt-bindings: mv88e6xxx: add ability to set default queue
> > > > priorities
> > > > per port
> > > > net: dsa: mv88e6xxx: add ability to set queue scheduling
> > > > dt-bindings: mv88e6xxx: add ability to set queue scheduling
> > > > net: dsa: mv88e6xxx: add egress rate limiting
> > > > dt-bindings: mv88e6xxx: add egress rate limiting
> > > >
> > > > .../devicetree/bindings/net/dsa/marvell.txt | 38 +++++
> > > > drivers/net/dsa/mv88e6xxx/chip.c | 122
> > > > ++++++++++++---
> > > > drivers/net/dsa/mv88e6xxx/chip.h | 5 +-
> > > > drivers/net/dsa/mv88e6xxx/port.c | 140
> > > > +++++++++++++++++-
> > > > drivers/net/dsa/mv88e6xxx/port.h | 24 ++-
> > > > include/dt-bindings/net/dsa-mv88e6xxx.h | 22 +++
> > > > net/dsa/port.c | 10 ++
> > > > 7 files changed, 327 insertions(+), 34 deletions(-)
> > > > create mode 100644 include/dt-bindings/net/dsa-mv88e6xxx.h
> > > >
> > >
> > >
> > > --
> > > Florian
>
^ permalink raw reply
* Re: [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms
From: Ido Schimmel @ 2019-09-12 9:05 UTC (permalink / raw)
To: Andrew Lunn
Cc: Robert Beckett, Florian Fainelli, netdev@vger.kernel.org,
Vivien Didelot, David S. Miller, Jiri Pirko
In-Reply-To: <20190911225841.GB5710@lunn.ch>
On Thu, Sep 12, 2019 at 12:58:41AM +0200, Andrew Lunn wrote:
> So think about how your can model the Marvell switch capabilities
> using TC, and implement offload support for it.
+1 :)
^ permalink raw reply
* Re: [PATCH v3 2/2] tcp: Add rcv_wnd to TCP_INFO
From: Dave Taht @ 2019-09-12 9:14 UTC (permalink / raw)
To: Neal Cardwell
Cc: Thomas Higdon, netdev@vger.kernel.org, Jonathan Lemon, Dave Jones,
Eric Dumazet, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <CADVnQynNiTEAmA-++JL7kMeht+dzfh2b==R_UJnEdnX3W=3k8g@mail.gmail.com>
On Thu, Sep 12, 2019 at 1:59 AM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Wed, Sep 11, 2019 at 6:32 PM Thomas Higdon <tph@fb.com> wrote:
> >
> > Neal Cardwell mentioned that rcv_wnd would be useful for helping
> > diagnose whether a flow is receive-window-limited at a given instant.
> >
> > This serves the purpose of adding an additional __u32 to avoid the
> > would-be hole caused by the addition of the tcpi_rcvi_ooopack field.
> >
> > Signed-off-by: Thomas Higdon <tph@fb.com>
> > ---
>
> Thanks, Thomas.
>
> I know that when I mentioned this before I mentioned the idea of both
> tp->snd_wnd (send-side receive window) and tp->rcv_wnd (receive-side
> receive window) in tcp_info, and did not express a preference between
> the two. Now that we are faced with a decision between the two,
> personally I think it would be a little more useful to start with
> tp->snd_wnd. :-)
>
> Two main reasons:
>
> (1) Usually when we're diagnosing TCP performance problems, we do so
> from the sender, since the sender makes most of the
> performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc).
> From the sender-side the thing that would be most useful is to see
> tp->snd_wnd, the receive window that the receiver has advertised to
> the sender.
I am under the impression, that particularly in the mobile space, that
network behavior
is often governed by rcv_wnd. At least, there's been so many papers on
this that I'd
tended to assume so.
Given a desire to do both vars, is there a *third* u32 we could add to
fill in the next hole? :)
ecn marks?
>
> (2) From the receiver side, "ss" can already show a fair amount of
> info about receive-side buffer/window limits, like:
> info->tcpi_rcv_ssthresh, info->tcpi_rcv_space,
> skmeminfo[SK_MEMINFO_RMEM_ALLOC], skmeminfo[SK_MEMINFO_RCVBUF]. Often
> the rwin can be approximated by combining those.
>
> Hopefully Eric, Yuchung, and Soheil can weigh in on the question of
> snd_wnd vs rcv_wnd. Or we can perhaps think of another field, and add
> the tcpi_rcvi_ooopack, snd_wnd, rcv_wnd, and that final field, all
> together.
>
> thanks,
> neal
--
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
^ permalink raw reply
* Re: [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms
From: Andrew Lunn @ 2019-09-12 9:21 UTC (permalink / raw)
To: Ido Schimmel
Cc: Robert Beckett, Florian Fainelli, netdev@vger.kernel.org,
Vivien Didelot, David S. Miller, Jiri Pirko
In-Reply-To: <20190912090339.GA16311@splinter>
> 2. Scheduling: How to schedule between the different transmission queues
>
> Where the port from which the packets should egress is the CPU port,
> before they cross the PCI towards the imx6.
Hi Ido
This is DSA, so the switch is connected via Ethernet to the IMX6, not
PCI. Minor detail, but that really is the core of what makes DSA DSA.
Andrew
^ permalink raw reply
* Re: [PATCH net v2 01/11] net: core: limit nested device depth
From: David Miller @ 2019-09-12 9:38 UTC (permalink / raw)
To: ap420073
Cc: netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
manishc, rahulv, kys, haiyangz, sthemmin, sashal, hare, varun,
ubraun, kgraul, jay.vosburgh
In-Reply-To: <CAMArcTV-Qvfd7xA0huCh_dbtr7P4LA+cQ7CpnaBBhdq-tq5fZQ@mail.gmail.com>
From: Taehee Yoo <ap420073@gmail.com>
Date: Thu, 12 Sep 2019 12:56:19 +0900
> I tested with this reproducer commands without lockdep.
>
> ip link add dummy0 type dummy
> ip link add link dummy0 name vlan1 type vlan id 1
> ip link set vlan1 up
>
> for i in {2..200}
> do
> let A=$i-1
>
> ip link add name vlan$i link vlan$A type vlan id $i
> done
> ip link del vlan1 <-- this command is added.
Is there any other device type which allows arbitrary nesting depth
in this manner other than VLAN? Perhaps it is the VLAN nesting
depth that we should limit instead of all of this extra code.
^ permalink raw reply
* RE: [RFC PATCH 3/3] Enable ptp_kvm for arm64
From: Jianyong Wu (Arm Technology China) @ 2019-09-12 9:40 UTC (permalink / raw)
To: Marc Zyngier
Cc: netdev@vger.kernel.org, pbonzini@redhat.com,
sean.j.christopherson@intel.com, richardcochran@gmail.com,
Mark Rutland, Will Deacon, Suzuki Poulose,
linux-kernel@vger.kernel.org, Steve Capper,
Kaly Xin (Arm Technology China), Justin He (Arm Technology China)
In-Reply-To: <86ftl3rrxg.wl-maz@kernel.org>
Hi Marc,
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Wednesday, September 11, 2019 7:31 PM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; pbonzini@redhat.com;
> sean.j.christopherson@intel.com; richardcochran@gmail.com; Mark Rutland
> <Mark.Rutland@arm.com>; Will Deacon <Will.Deacon@arm.com>; Suzuki
> Poulose <Suzuki.Poulose@arm.com>; linux-kernel@vger.kernel.org; Steve
> Capper <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>
> Subject: Re: [RFC PATCH 3/3] Enable ptp_kvm for arm64
>
> On Wed, 11 Sep 2019 11:06:18 +0100,
> "Jianyong Wu (Arm Technology China)" <Jianyong.Wu@arm.com> wrote:
> >
> > Hi Marc,
> >
> > I think there are three points for the migration issue of ptp_kvm,
> > where a VM using ptp_kvm migrates to a host without ptp_kvm support.
> >
> > First: how does it impact the VM having migrated?
> > I run a VM with ptp_kvm support in guest but not support in host. the
> > ptp0 will return 0 when get time from it which can't pass the check of
> > chrony, then chrony will choose another clocksource.
> > From this point, VM will only get lost in precision of time sync.
>
> "only" is a bit of an understatement. Once the guest has started relying on a
> service, it seems rather harsh to pretend this service doesn't exist anymore.
> It could well be that the VM cannot perform its function if the precision is not
> good enough.
>
> The analogy is the Spectre-v2 mitigation, which is implemented as a hypercall.
> Nothing will break if you migrate to a host that doesn't support the mitigation,
> but the guest will now be unsafe. Is that acceptable? the answer is of course
> "no".
>
> > Second: how to check the failure of the ptp kvm service when there is
> > no ptp kvm service, hypercall will go into default ops, so we can
> > check the return value which can inform us the failure.
>
> Sure. But that's still an issue. The VM relied on the service, and the service
> isn't available anymore.
>
> > Third: how to inform VMM
> > There is ioctl cmd call "KVM_CHECK_EXTENSION" in kvm, which may do
> > that thing. Accordingly, qemu should be offered the support which will
> > block us. We can try to add this support in kvm but we are not sure
> > the response from qemu side.
>
> It doesn't matter whether QEMU implements that check or . The important
> thing is that we give userspace a way to check this for this, and having a
> capability that can be checked against is probably the right thing to do.
Ok, I agree.
Adding a new capability item under "KVM_CHECK_EXTENSION" in kvm for ptp_kvm will do and Using ioctl in userspace can check if the ptp service is available.
I will append this patch in this patch serial.
Thanks
Jianyong Wu
>
> Thanks,
>
> M.
>
> --
> Jazz is not dead, it just smells funny.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Linus Walleij @ 2019-09-12 9:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andy Shevchenko, Russell King - ARM Linux admin, Mika Westerberg,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
Andrew Lunn, David S. Miller, Florian Fainelli, Heiner Kallweit,
netdev
In-Reply-To: <20190911095149.GA108334@dtor-ws>
On Wed, Sep 11, 2019 at 10:51 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> If we are willing to sacrifice the custom label for the GPIO that
> fwnode_gpiod_get_index() allows us to set, then there are several
> drivers that could actually use gpiod_get() API.
We have:
gpiod_set_consumer_name(gpiod, "name");
to deal with that so no sacrifice is needed.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 00/11] Add support for software nodes to gpiolib
From: Linus Walleij @ 2019-09-12 9:55 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andy Shevchenko, Mika Westerberg, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM, Andrew Lunn, Andrzej Hajda,
Bartosz Golaszewski, Daniel Vetter, David Airlie, David S. Miller,
Florian Fainelli, Heiner Kallweit, Jernej Skrabec, Jonas Karlman,
Laurent Pinchart, Neil Armstrong, Russell King,
open list:DRM PANEL DRIVERS, ACPI Devel Maling List, netdev
In-Reply-To: <20190911075215.78047-1-dmitry.torokhov@gmail.com>
On Wed, Sep 11, 2019 at 8:52 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> If we agree in principle, I would like to have the very first 3 patches
> in an immutable branch off maybe -rc8 so that it can be pulled into
> individual subsystems so that patches switching various drivers to
> fwnode_gpiod_get_index() could be applied.
I think it seems a bit enthusiastic to have non-GPIO subsystems
pick up these changes this close to the merge window so my plan
is to merge patches 1.2.3 (1 already merged) and then you could
massage the other subsystems in v5.4-rc1.
But if other subsystems say "hey we want do fix this in like 3 days"
then I'm game for an immutable branch as well.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH net] net/sched: fix race between deactivation and dequeue for NOLOCK qdisc
From: Paolo Abeni @ 2019-09-12 10:02 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Davide Caratti, Li Shuang
The test implemented by some_qdisc_is_busy() is somewhat loosy for
NOLOCK qdisc, as we may hit the following scenario:
CPU1 CPU2
// in net_tx_action()
clear_bit(__QDISC_STATE_SCHED...);
// in some_qdisc_is_busy()
val = (qdisc_is_running(q) ||
test_bit(__QDISC_STATE_SCHED,
&q->state));
// here val is 0 but...
qdisc_run(q)
// ... CPU1 is going to run the qdisc next
As a conseguence qdisc_run() in net_tx_action() can race with qdisc_reset()
in dev_qdisc_reset(). Such race is not possible for !NOLOCK qdisc as
both the above bit operations are under the root qdisc lock().
After commit 021a17ed796b ("pfifo_fast: drop unneeded additional lock on dequeue")
the race can cause use after free and/or null ptr dereference, but the root
cause is likely older.
This patch addresses the issue explicitly checking for deactivation under
the seqlock for NOLOCK qdisc, so that the qdisc_run() in the critical
scenario becomes a no-op.
Note that the enqueue() op can still execute concurrently with dev_qdisc_reset(),
but that is safe due to the skb_array() locking, and we can't avoid that
for NOLOCK qdiscs.
Fixes: 021a17ed796b ("pfifo_fast: drop unneeded additional lock on dequeue")
Reported-by: Li Shuang <shuali@redhat.com>
Reported-and-tested-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/pkt_sched.h | 7 ++++++-
net/core/dev.c | 16 ++++++++++------
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index a16fbe9a2a67..aa99c73c3fbd 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -118,7 +118,12 @@ void __qdisc_run(struct Qdisc *q);
static inline void qdisc_run(struct Qdisc *q)
{
if (qdisc_run_begin(q)) {
- __qdisc_run(q);
+ /* NOLOCK qdisc must check 'state' under the qdisc seqlock
+ * to avoid racing with dev_qdisc_reset()
+ */
+ if (!(q->flags & TCQ_F_NOLOCK) ||
+ likely(!test_bit(__QDISC_STATE_DEACTIVATED, &q->state)))
+ __qdisc_run(q);
qdisc_run_end(q);
}
}
diff --git a/net/core/dev.c b/net/core/dev.c
index 0891f499c1bb..ef8f2f002e09 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3467,18 +3467,22 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
qdisc_calculate_pkt_len(skb, q);
if (q->flags & TCQ_F_NOLOCK) {
- if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
- __qdisc_drop(skb, &to_free);
- rc = NET_XMIT_DROP;
- } else if ((q->flags & TCQ_F_CAN_BYPASS) && q->empty &&
- qdisc_run_begin(q)) {
+ if ((q->flags & TCQ_F_CAN_BYPASS) && q->empty &&
+ qdisc_run_begin(q)) {
+ if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED,
+ &q->state))) {
+ __qdisc_drop(skb, &to_free);
+ rc = NET_XMIT_DROP;
+ goto end_run;
+ }
qdisc_bstats_cpu_update(q, skb);
+ rc = NET_XMIT_SUCCESS;
if (sch_direct_xmit(skb, q, dev, txq, NULL, true))
__qdisc_run(q);
+end_run:
qdisc_run_end(q);
- rc = NET_XMIT_SUCCESS;
} else {
rc = q->enqueue(skb, q, &to_free) & NET_XMIT_MASK;
qdisc_run(q);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v2 net-next 3/7] net: dsa: sja1105: Switch to hardware operations for PTP
From: David Miller @ 2019-09-12 10:12 UTC (permalink / raw)
To: olteanv; +Cc: f.fainelli, vivien.didelot, andrew, richardcochran, netdev
In-Reply-To: <20190910013501.3262-4-olteanv@gmail.com>
From: Vladimir Oltean <olteanv@gmail.com>
Date: Tue, 10 Sep 2019 04:34:57 +0300
> static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> {
> struct sja1105_private *priv = ptp_to_sja1105(ptp);
> + const struct sja1105_regs *regs = priv->info->regs;
> s64 clkrate;
> + int rc;
..
> -static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> -{
> - struct sja1105_private *priv = ptp_to_sja1105(ptp);
> + rc = sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkrate,
> + &clkrate, 4);
You're sending an arbitrary 4 bytes of a 64-bit value. This works on little endian
but will not on big endian.
Please properly copy this clkrate into a "u32" variable and pass that into
sja1105_spi_send_int().
It also seems to suggest that you want to use abs() to perform that weird
centering around 1 << 31 calculation.
Thank you.
^ permalink raw reply
* Re: [PATCH 1/7] net/dsa: configure autoneg for CPU port
From: Robert Beckett @ 2019-09-12 10:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, netdev, Vivien Didelot, David S. Miller,
bob.beckett
In-Reply-To: <20190911225252.GA5710@lunn.ch>
On Thu, 2019-09-12 at 00:52 +0200, Andrew Lunn wrote:
> > It is not just for broadcast storm protection. The original issue
> > that
> > made me look in to all of this turned out to be rx descritor ring
> > buffer exhaustion due to the CPU not being able to keep up with
> > packet
> > reception.
>
> Pause frames does not really solve this problem. The switch will at
> some point fill its buffers, and start throwing packets away. Or it
> needs to send pause packets it its peers. And then your whole switch
> throughput goes down. Packets will always get thrown away, so you
> need
> QoS in your network to give the network hints about which frames is
> should throw away first.
>
Indeed. This is the understanding I was working with.
This patch series enables pause frames, output queue prriority and
strict scheduling to egress the high priority queues first.
This means that when the switch starts dropping frames, it drops from
the lowest priority as the highest ones are delivered at line speed
without issue.
> ..
>
> > Fundamentally, with a phy to phy CPU connection, the CPU MAC may
> > well
> > wish to enable pause frames for various reasons, so we should
> > strive to
> > handle that I think.
>
> It actually has nothing to do with PHY to PHY connections. You can
> use
> pause frames with direct MAC to MAC connections. PHY auto-negotiation
> is one way to indicate both ends support it, but there are also other
> ways. e.g.
>
> ethtool -A|--pause devname [autoneg on|off] [rx on|off] [tx on|off]
>
> on the SoC you could do
>
> ethtool --pause eth0 autoneg off rx on tx on
>
> to force the SoC to send and process pause frames. Ideally i would
> prefer a solution like this, since it is not a change of behaviour
> for
> everybody else.
Good point, well made.
The reason for using autoneg in this series was due to having no netdev
to run ethtool against for the CPU port.
If we go down the route of creating a netdev for the CPU port, then we
could indeed force pause frames at both ends.
However, given that the phy on the marvell switch is capable of autoneg
, is it not reasonable to setup the advertisement and let autoneg take
care of it if using phy to phy connection?
>
> Andrew
^ permalink raw reply
* Re: [PATCH net v2 01/11] net: core: limit nested device depth
From: Taehee Yoo @ 2019-09-12 10:14 UTC (permalink / raw)
To: David Miller
Cc: Netdev, j.vosburgh, vfalico, Andy Gospodarek,
Jiří Pírko, sd, Roopa Prabhu, saeedm, manishc,
rahulv, kys, haiyangz, sthemmin, sashal, hare, varun, ubraun,
kgraul, Jay Vosburgh
In-Reply-To: <20190912.113807.52193745382103083.davem@davemloft.net>
On Thu, 12 Sep 2019 at 18:38, David Miller <davem@davemloft.net> wrote:
>
> From: Taehee Yoo <ap420073@gmail.com>
> Date: Thu, 12 Sep 2019 12:56:19 +0900
>
> > I tested with this reproducer commands without lockdep.
> >
> > ip link add dummy0 type dummy
> > ip link add link dummy0 name vlan1 type vlan id 1
> > ip link set vlan1 up
> >
> > for i in {2..200}
> > do
> > let A=$i-1
> >
> > ip link add name vlan$i link vlan$A type vlan id $i
> > done
> > ip link del vlan1 <-- this command is added.
>
> Is there any other device type which allows arbitrary nesting depth
> in this manner other than VLAN? Perhaps it is the VLAN nesting
> depth that we should limit instead of all of this extra code.
Below device types have the same problem.
VLAN, BONDING, TEAM, VXLAN, MACVLAN, and MACSEC.
All the below test commands reproduce a panic.
BONDING test commands:
ip link add bond0 type bond
for i in {1..200}
do
let A=$i-1
ip link add bond$i type bond
ip link set bond$i master bond$A
done
ip link set bond5 master bond0
TEAM test commands:
ip link add team0 type team
for i in {1..200}
do
let A=$i-1
ip link add team$i type team
ip link set team$i master team$A
done
MACSEC test commands:
ip link add link lo macsec0 type macsec
for i in {1..100}
do
let A=$i-1
ip link add link macsec$A macsec$i type macsec
done
ip link del macsec0
MACVLAN test commands:
ip link add dummy0 type dummy
ip link add macvlan1 link dummy0 type macvlan
ip link add vlan2 link macvlan1 type vlan id 2
let i=3
for j in {1..100}
do
let A=$i-1
ip link add macvlan$i link vlan$A type macvlan
let i=$i+1
let A=$i-1
ip link add vlan$i link macvlan$A type vlan id $i
let i=$i+1
done
ip link del dummy0
VXLAN test commands:
ip link add vxlan1 type vxlan dev lo id 1 dstport 1
for i in {2..100}
do
let A=$i-1
ip link add vxlan$i type vxlan dev vxlan$A id $i dstport $i
done
ip link del vxlan1
^ permalink raw reply
* Re: [PATCH v2 net-next 3/7] net: dsa: sja1105: Switch to hardware operations for PTP
From: Vladimir Oltean @ 2019-09-12 10:17 UTC (permalink / raw)
To: David Miller; +Cc: f.fainelli, vivien.didelot, andrew, richardcochran, netdev
In-Reply-To: <20190912.121203.1106283271122334199.davem@davemloft.net>
Hi Dave,
On 12/09/2019, David Miller <davem@davemloft.net> wrote:
> From: Vladimir Oltean <olteanv@gmail.com>
> Date: Tue, 10 Sep 2019 04:34:57 +0300
>
>> static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long
>> scaled_ppm)
>> {
>> struct sja1105_private *priv = ptp_to_sja1105(ptp);
>> + const struct sja1105_regs *regs = priv->info->regs;
>> s64 clkrate;
>> + int rc;
> ..
>> -static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>> -{
>> - struct sja1105_private *priv = ptp_to_sja1105(ptp);
>> + rc = sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkrate,
>> + &clkrate, 4);
>
> You're sending an arbitrary 4 bytes of a 64-bit value. This works on little
> endian
> but will not on big endian.
>
> Please properly copy this clkrate into a "u32" variable and pass that into
> sja1105_spi_send_int().
>
> It also seems to suggest that you want to use abs() to perform that weird
> centering around 1 << 31 calculation.
>
> Thank you.
>
It looks 'wrong' but it isn't. The driver uses the 'packing' framework
(lib/packing.c) which is endian-agnostic (converts between CPU and
peripheral endianness) and operates on u64 as the CPU word size. On
the contrary, u32 would not work with the 'packing' API in its current
form, but I don't see yet any reasons to extend it (packing64,
packing32 etc).
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH v4 2/2] net: phy: dp83867: Add SGMII mode type switching
From: Vitaly Gaiduk @ 2019-09-12 10:17 UTC (permalink / raw)
To: David Miller
Cc: robh+dt, f.fainelli, mark.rutland, andrew, hkallweit1, tpiepho,
netdev, devicetree, linux-kernel
In-Reply-To: <20190912.003754.663480494374990855.davem@davemloft.net>
Hello, David.
Should I patch commit as Trent Piepho suggested? He wrote about using
phy_modify_mmd() instead.
Vitaly.
On 12.09.2019 1:37, David Miller wrote:
> From: Vitaly Gaiduk <vitaly.gaiduk@cloudbear.ru>
> Date: Mon, 9 Sep 2019 20:19:24 +0300
>
>> This patch adds ability to switch beetween two PHY SGMII modes.
>> Some hardware, for example, FPGA IP designs may use 6-wire mode
>> which enables differential SGMII clock to MAC.
>>
>> Signed-off-by: Vitaly Gaiduk <vitaly.gaiduk@cloudbear.ru>
> Applied.
^ permalink raw reply
* Re: [PATCH v4] tun: fix use-after-free when register netdev failed
From: David Miller @ 2019-09-12 10:18 UTC (permalink / raw)
To: yangyingliang; +Cc: netdev, jasowang, eric.dumazet, xiyou.wangcong, weiyongjun1
In-Reply-To: <1568113017-79840-1-git-send-email-yangyingliang@huawei.com>
From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Sep 2019 18:56:57 +0800
> I got a UAF repport in tun driver when doing fuzzy test:
...
> tun_chr_read_iter() accessed the memory which freed by free_netdev()
> called by tun_set_iff():
>
> CPUA CPUB
> tun_set_iff()
> alloc_netdev_mqs()
> tun_attach()
> tun_chr_read_iter()
> tun_get()
> tun_do_read()
> tun_ring_recv()
> register_netdevice() <-- inject error
> goto err_detach
> tun_detach_all() <-- set RCV_SHUTDOWN
> free_netdev() <-- called from
> err_free_dev path
> netdev_freemem() <-- free the memory
> without check refcount
> (In this path, the refcount cannot prevent
> freeing the memory of dev, and the memory
> will be used by dev_put() called by
> tun_chr_read_iter() on CPUB.)
> (Break from tun_ring_recv(),
> because RCV_SHUTDOWN is set)
> tun_put()
> dev_put() <-- use the memory
> freed by netdev_freemem()
>
> Put the publishing of tfile->tun after register_netdevice(),
> so tun_get() won't get the tun pointer that freed by
> err_detach path if register_netdevice() failed.
>
> Fixes: eb0fb363f920 ("tuntap: attach queue 0 before registering netdevice")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] ipv6: Fix the link time qualifier of 'ping_v6_proc_exit_net()'
From: David Miller @ 2019-09-12 10:21 UTC (permalink / raw)
To: christophe.jaillet
Cc: kuznet, yoshfuji, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190910112959.9222-1-christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Tue, 10 Sep 2019 13:29:59 +0200
> The '.exit' functions from 'pernet_operations' structure should be marked
> as __net_exit, not __net_init.
>
> Fixes: d862e5461423 ("net: ipv6: Implement /proc/net/icmp6.")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Untested, but using __net_exit looks consistent with other
> pernet_operations.exit use case.
Looks good, applied.
^ 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