* [PATCH v2 net-next] af_unix: ensure POLLOUT on remote close() for connected dgram socket
From: Jason Baron @ 2018-08-03 21:24 UTC (permalink / raw)
To: davem; +Cc: netdev, David Rientjes, Rainer Weikusat, Eric Dumazet
Applications use -ECONNREFUSED as returned from write() in order to
determine that a socket should be closed. However, when using connected
dgram unix sockets in a poll/write loop, a final POLLOUT event can be
missed when the remote end closes. Thus, the poll is stuck forever:
thread 1 (client) thread 2 (server)
connect() to server
write() returns -EAGAIN
unix_dgram_poll()
-> unix_recvq_full() is true
close()
->unix_release_sock()
->wake_up_interruptible_all()
unix_dgram_poll() (due to the
wake_up_interruptible_all)
-> unix_recvq_full() still is true
->free all skbs
Now thread 1 is stuck and will not receive anymore wakeups. In this
case, when thread 1 gets the -EAGAIN, it has not queued any skbs
otherwise the 'free all skbs' step would in fact cause a wakeup and
a POLLOUT return. So the race here is probably fairly rare because
it means there are no skbs that thread 1 queued and that thread 1
schedules before the 'free all skbs' step.
This issue was reported as a hang when /dev/log is closed.
The fix is to signal POLLOUT if the socket is marked as SOCK_DEAD, which
means a subsequent write() will get -ECONNREFUSED.
Reported-by: Ian Lance Taylor <iant@golang.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Rainer Weikusat <rweikusat@mobileactivedefense.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
v2: use check for SOCK_DEAD, since skb's can be purged in unix_sock_destructor()
---
net/unix/af_unix.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 1772a0e..d1edfa3 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -430,7 +430,12 @@ static int unix_dgram_peer_wake_me(struct sock *sk, struct sock *other)
connected = unix_dgram_peer_wake_connect(sk, other);
- if (unix_recvq_full(other))
+ /* If other is SOCK_DEAD, we want to make sure we signal
+ * POLLOUT, such that a subsequent write() can get a
+ * -ECONNREFUSED. Otherwise, if we haven't queued any skbs
+ * to other and its full, we will hang waiting for POLLOUT.
+ */
+ if (unix_recvq_full(other) && !sock_flag(other, SOCK_DEAD))
return 1;
if (connected)
--
1.9.1
^ permalink raw reply related
* Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue
From: Andrei Vagin @ 2018-08-03 21:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Nambiar, Amritha, netdev, davem, alexander.h.duyck,
willemdebruijn.kernel, sridhar.samudrala, alexander.duyck,
edumazet, hannes, tom, tom, jasowang, gaowanlong, virtualization,
rostedt
In-Reply-To: <20180803221212-mutt-send-email-mst@kernel.org>
On Fri, Aug 03, 2018 at 10:12:53PM +0300, Michael S. Tsirkin wrote:
> On Fri, Aug 03, 2018 at 12:06:51PM -0700, Andrei Vagin wrote:
> > On Fri, Aug 03, 2018 at 12:08:05AM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Aug 02, 2018 at 02:04:12PM -0700, Nambiar, Amritha wrote:
> > > > On 8/1/2018 5:11 PM, Andrei Vagin wrote:
> > > > > On Tue, Jul 10, 2018 at 07:28:49PM -0700, Nambiar, Amritha wrote:
> > > > >> With this patch series, I introduced static_key for XPS maps
> > > > >> (xps_needed), so static_key_slow_inc() is used to switch branches. The
> > > > >> definition of static_key_slow_inc() has cpus_read_lock in place. In the
> > > > >> virtio_net driver, XPS queues are initialized after setting the
> > > > >> queue:cpu affinity in virtnet_set_affinity() which is already protected
> > > > >> within cpus_read_lock. Hence, the warning here trying to acquire
> > > > >> cpus_read_lock when it is already held.
> > > > >>
> > > > >> A quick fix for this would be to just extract netif_set_xps_queue() out
> > > > >> of the lock by simply wrapping it with another put/get_online_cpus
> > > > >> (unlock right before and hold lock right after).
> > > > >
> > > > > virtnet_set_affinity() is called from virtnet_cpu_online(), which is
> > > > > called under cpus_read_lock too.
> > > > >
> > > > > It looks like now we can't call netif_set_xps_queue() from cpu hotplug
> > > > > callbacks.
> > > > >
> > > > > I can suggest a very straightforward fix for this problem. The patch is
> > > > > attached.
> > > > >
> > > >
> > > > Thanks for looking into this. I was thinking of fixing this in the
> > > > virtio_net driver by moving the XPS initialization (and have a new
> > > > get_affinity utility) in the ndo_open (so it is together with other tx
> > > > preparation) instead of probe. Your patch solves this in general for
> > > > setting up cpu hotplug callbacks which is under cpus_read_lock.
> > >
> > >
> > > I like this too. Could you repost in a standard way
> > > (inline, with your signoff etc) so we can ack this for
> > > net-next?
> >
> > When I was testing this patch, I got the following kasan warning. Michael,
> > could you take a look at it. Maybe you will understand what was going wrong there.
> >
> > https://api.travis-ci.org/v3/job/410701353/log.txt
> >
> > [ 7.275033] ==================================================================
> > [ 7.275226] BUG: KASAN: slab-out-of-bounds in virtnet_poll+0xaa1/0xd00
> > [ 7.275359] Read of size 8 at addr ffff8801d444a000 by task ip/370
> > [ 7.275488]
> > [ 7.275610] CPU: 1 PID: 370 Comm: ip Not tainted 4.18.0-rc6+ #1
> > [ 7.275613] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > [ 7.275616] Call Trace:
> > [ 7.275621] <IRQ>
> > [ 7.275630] dump_stack+0x71/0xab
> > [ 7.275640] print_address_description+0x6a/0x270
> > [ 7.275648] kasan_report+0x258/0x380
> > [ 7.275653] ? virtnet_poll+0xaa1/0xd00
> > [ 7.275661] virtnet_poll+0xaa1/0xd00
> > [ 7.275680] ? receive_buf+0x5920/0x5920
> > [ 7.275689] ? do_raw_spin_unlock+0x54/0x220
> > [ 7.275699] ? find_held_lock+0x32/0x1c0
> > [ 7.275710] ? rcu_process_callbacks+0xa60/0xd20
> > [ 7.275736] net_rx_action+0x2ee/0xad0
> > [ 7.275748] ? rcu_note_context_switch+0x320/0x320
> > [ 7.275754] ? napi_complete_done+0x300/0x300
> > [ 7.275763] ? native_apic_msr_write+0x27/0x30
> > [ 7.275768] ? lapic_next_event+0x5b/0x90
> > [ 7.275775] ? clockevents_program_event+0x21d/0x2f0
> > [ 7.275791] __do_softirq+0x19a/0x623
> > [ 7.275807] do_softirq_own_stack+0x2a/0x40
> > [ 7.275811] </IRQ>
> > [ 7.275818] do_softirq.part.18+0x6a/0x80
> > [ 7.275825] __local_bh_enable_ip+0x49/0x50
> > [ 7.275829] virtnet_open+0x129/0x440
> > [ 7.275841] __dev_open+0x189/0x2c0
> > [ 7.275848] ? dev_set_rx_mode+0x30/0x30
> > [ 7.275857] ? do_raw_spin_unlock+0x54/0x220
> > [ 7.275866] __dev_change_flags+0x3a9/0x4f0
> > [ 7.275873] ? dev_set_allmulti+0x10/0x10
> > [ 7.275889] dev_change_flags+0x7a/0x150
> > [ 7.275900] do_setlink+0x9fe/0x2e40
> > [ 7.275910] ? deref_stack_reg+0xad/0xe0
> > [ 7.275917] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.275922] ? find_held_lock+0x32/0x1c0
> > [ 7.275929] ? rtnetlink_put_metrics+0x460/0x460
> > [ 7.275935] ? virtqueue_add_sgs+0x9e2/0xde0
> > [ 7.275953] ? virtscsi_add_cmd+0x454/0x780
> > [ 7.275964] ? find_held_lock+0x32/0x1c0
> > [ 7.275973] ? deref_stack_reg+0xad/0xe0
> > [ 7.275979] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.275985] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.275993] ? memset+0x1f/0x40
> > [ 7.276008] ? nla_parse+0x33/0x290
> > [ 7.276016] rtnl_newlink+0x954/0x1120
> > [ 7.276030] ? rtnl_link_unregister+0x250/0x250
> > [ 7.276044] ? is_bpf_text_address+0x5/0x60
> > [ 7.276054] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.276057] ? lock_acquire+0x10b/0x2a0
> > [ 7.276072] ? deref_stack_reg+0xad/0xe0
> > [ 7.276078] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.276085] ? __kernel_text_address+0xe/0x30
> > [ 7.276090] ? unwind_get_return_address+0x5f/0xa0
> > [ 7.276103] ? find_held_lock+0x32/0x1c0
> > [ 7.276110] ? is_bpf_text_address+0x5/0x60
> > [ 7.276124] ? deref_stack_reg+0xad/0xe0
> > [ 7.276131] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> > [ 7.276136] ? depot_save_stack+0x2d9/0x460
> > [ 7.276142] ? deref_stack_reg+0xad/0xe0
> > [ 7.276156] ? find_held_lock+0x32/0x1c0
> > [ 7.276164] ? is_bpf_text_address+0x5/0x60
> > [ 7.276170] ? __lock_acquire.isra.29+0xe8/0x1bb0
> > [ 7.276212] ? rtnetlink_rcv_msg+0x5d6/0x930
> > [ 7.276222] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.276226] ? lock_acquire+0x10b/0x2a0
> > [ 7.276240] rtnetlink_rcv_msg+0x69e/0x930
> > [ 7.276249] ? rtnl_calcit.isra.31+0x2f0/0x2f0
> > [ 7.276255] ? __lock_acquire.isra.29+0xe8/0x1bb0
> > [ 7.276265] ? netlink_deliver_tap+0x8d/0x8e0
> > [ 7.276276] netlink_rcv_skb+0x127/0x350
> > [ 7.276281] ? rtnl_calcit.isra.31+0x2f0/0x2f0
> > [ 7.276289] ? netlink_ack+0x970/0x970
> > [ 7.276299] ? __alloc_skb+0xc2/0x520
> > [ 7.276311] netlink_unicast+0x40f/0x5d0
> > [ 7.276320] ? netlink_attachskb+0x580/0x580
> > [ 7.276325] ? _copy_from_iter_full+0x157/0x6f0
> > [ 7.276331] ? import_iovec+0x90/0x390
> > [ 7.276338] ? get_page_from_freelist+0x1e89/0x3e30
> > [ 7.276347] ? apparmor_socket_sock_rcv_skb+0x10/0x10
> > [ 7.276357] netlink_sendmsg+0x65e/0xb00
> > [ 7.276367] ? netlink_unicast+0x5d0/0x5d0
> > [ 7.276373] ? copy_msghdr_from_user+0x206/0x340
> > [ 7.276388] ? netlink_unicast+0x5d0/0x5d0
> > [ 7.276394] sock_sendmsg+0xb3/0xf0
> > [ 7.276401] ___sys_sendmsg+0x604/0x8b0
> > [ 7.276410] ? copy_msghdr_from_user+0x340/0x340
> > [ 7.276416] ? find_held_lock+0x32/0x1c0
> > [ 7.276424] ? __handle_mm_fault+0xc85/0x3140
> > [ 7.276433] ? lock_downgrade+0x5e0/0x5e0
> > [ 7.276439] ? mem_cgroup_commit_charge+0xb4/0xf80
> > [ 7.276453] ? _raw_spin_unlock+0x24/0x30
> > [ 7.276458] ? __handle_mm_fault+0xc85/0x3140
> > [ 7.276467] ? __pmd_alloc+0x430/0x430
> > [ 7.276473] ? find_held_lock+0x32/0x1c0
> > [ 7.276485] ? __fget_light+0x55/0x1f0
> > [ 7.276497] ? __sys_sendmsg+0xd2/0x170
> > [ 7.276502] __sys_sendmsg+0xd2/0x170
> > [ 7.276508] ? __ia32_sys_shutdown+0x70/0x70
> > [ 7.276516] ? handle_mm_fault+0x1f9/0x6a0
> > [ 7.276528] ? up_read+0x1c/0x110
> > [ 7.276534] ? __do_page_fault+0x4a6/0xa80
> > [ 7.276554] do_syscall_64+0xa0/0x280
> > [ 7.276558] ? prepare_exit_to_usermode+0x88/0x130
> > [ 7.276566] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [ 7.276572] RIP: 0033:0x7ffbe9a2f160
> > [ 7.276574] Code: c3 48 8b 05 2a 2d 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 1d 8f 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee cb 00 00 48 89 04 24
> > [ 7.276728] RSP: 002b:00007ffc5a2d6108 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > [ 7.276735] RAX: ffffffffffffffda RBX: 00007ffc5a2da220 RCX: 00007ffbe9a2f160
> > [ 7.276738] RDX: 0000000000000000 RSI: 00007ffc5a2d6150 RDI: 0000000000000003
> > [ 7.276741] RBP: 00007ffc5a2d6150 R08: 0000000000000000 R09: 0000000000000003
> > [ 7.276744] R10: 00007ffc5a2d5ed0 R11: 0000000000000246 R12: 000000005b6177de
> > [ 7.276748] R13: 0000000000000000 R14: 00000000006473a0 R15: 00007ffc5a2da918
> > [ 7.276763]
> > [ 7.276895] Allocated by task 1:
> > [ 7.277026] kasan_kmalloc+0xa0/0xd0
> > [ 7.277030] __kmalloc+0x13a/0x250
> > [ 7.277034] init_vqs+0xd0/0x11c0
> > [ 7.277038] virtnet_probe+0xb99/0x1ad0
> > [ 7.277045] virtio_dev_probe+0x3fc/0x890
> > [ 7.277052] driver_probe_device+0x6c4/0xcc0
> > [ 7.277056] __driver_attach+0x232/0x2c0
> > [ 7.277060] bus_for_each_dev+0x118/0x1a0
> > [ 7.277064] bus_add_driver+0x390/0x6e0
> > [ 7.277068] driver_register+0x18e/0x400
> > [ 7.277076] virtio_net_driver_init+0x6d/0x90
> > [ 7.277080] do_one_initcall+0xa8/0x348
> > [ 7.277085] kernel_init_freeable+0x42d/0x4c8
> > [ 7.277090] kernel_init+0xf/0x130
> > [ 7.277095] ret_from_fork+0x35/0x40
> > [ 7.277097]
> > [ 7.277223] Freed by task 0:
> > [ 7.277347] (stack is not available)
> > [ 7.277473]
> > [ 7.277596] The buggy address belongs to the object at ffff8801d4449100
> > [ 7.277596] which belongs to the cache kmalloc-4096 of size 4096
> > [ 7.277769] The buggy address is located 3840 bytes inside of
> > [ 7.277769] 4096-byte region [ffff8801d4449100, ffff8801d444a100)
> > [ 7.277932] The buggy address belongs to the page:
> > [ 7.278066] page:ffffea0007511200 count:1 mapcount:0 mapping:ffff8801db002600 index:0x0 compound_mapcount: 0
> > [ 7.278230] flags: 0x17fff8000008100(slab|head)
> > [ 7.278363] raw: 017fff8000008100 dead000000000100 dead000000000200 ffff8801db002600
> > [ 7.278516] raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
> > [ 7.278664] page dumped because: kasan: bad access detected
> > [ 7.278790]
> > [ 7.278904] Memory state around the buggy address:
> > [ 7.279031] ffff8801d4449f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279175] ffff8801d4449f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279323] >ffff8801d444a000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279468] ^
> > [ 7.279584] ffff8801d444a080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279729] ffff8801d444a100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > [ 7.279870] ==================================================================
> > [ 7.280011] Disabling lock debugging due to kernel taint
> > [ 7.632219] random: mktemp: uninitialized urandom read (6 bytes read)
> > [ 8.052850] random: mktemp: uninitialized urandom read (6 bytes read)
> > [ 8.335209] random: cloud-init: uninitialized urandom read (32 bytes read)
> > [ 8.796331] random: cloud-init: uninitialized urandom read (32 bytes read)
> > [ 9.162551] random: mktemp: uninitialized urandom read (12 bytes read)
> > [ 9.384504] random: ssh-keygen: uninitialized urandom read (32 bytes read)
> > [ 9.839586] init: failsafe main process (724) killed by TERM signal
> > [ 14.865443] postgres (1245): /proc/1245/oom_adj is deprecated, please use /proc/1245/oom_score_adj instead.
> > [ 17.213418] random: crng init done
> > [ 17.580892] init: plymouth-upstart-bridge main process ended, respawning
>
> I suspect an off by one somewhere. I'm looking at the patch
> and I don't see it but these things are hard to spot sometimes ...
>
It was reproduced only once, so the problem can be unrelated with this
patch.
> > >
> > > > >> But this may not a
> > > > >> clean solution. It'd help if I can get suggestions on what would be a
> > > > >> clean option to fix this without extensively changing the code in
> > > > >> virtio_net. Is it mandatory to protect the affinitization with
> > > > >> read_lock? I don't see similar lock in other drivers while setting the
> > > > >> affinity. I understand this warning should go away, but isn't it safe to
> > > > >> have multiple readers.
> > > > >>
> > > > >>> On Fri, Jun 29, 2018 at 09:27:07PM -0700, Amritha Nambiar wrote:
^ permalink raw reply
* Re: [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: Michael Tuexen @ 2018-08-03 20:56 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: David Laight, Konstantin Khorenko, oleg.babin@gmail.com,
netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
David S . Miller, Vlad Yasevich, Neil Horman, Xin Long,
Andrey Ryabinin
In-Reply-To: <20180803203009.GG5482@localhost.localdomain>
> On 3. Aug 2018, at 22:30, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
>
> On Fri, Aug 03, 2018 at 04:43:28PM +0000, David Laight wrote:
>> From: Konstantin Khorenko
>>> Sent: 03 August 2018 17:21
>>>
>>> Each SCTP association can have up to 65535 input and output streams.
>>> For each stream type an array of sctp_stream_in or sctp_stream_out
>>> structures is allocated using kmalloc_array() function. This function
>>> allocates physically contiguous memory regions, so this can lead
>>> to allocation of memory regions of very high order, i.e.:
>> ...
>>
>> Given how useless SCTP streams are, does anything actually use
>> more than about 4?
>
> Maybe Michael can help us with that. I'm also curious now.
In the context of SIGTRAN I have seen 17 streams...
In the context of WebRTC I have seen more streams. In general,
the streams concept seems to be useful. QUIC has lots of streams.
So I'm wondering why they are considered useless.
David, can you elaborate on this?
Best regards
Michael
>
> Marcelo
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Ben Pfaff @ 2018-08-03 23:01 UTC (permalink / raw)
To: Stefano Brivio
Cc: dev-yBygre7rU0TnMu66kgdUjQ, jpettit-pghWNbHTmq7QT0dZR+AlfA,
netdev, Jiri Benc
In-Reply-To: <20180803185241.4ac0d1e5@epycfail>
On Fri, Aug 03, 2018 at 06:52:41PM +0200, Stefano Brivio wrote:
> On Tue, 31 Jul 2018 15:06:57 -0700 Ben Pfaff <blp-LZ6Gd1LRuIk@public.gmane.org> wrote:
> > My current thought is that any fairness scheme we implement directly in
> > the kernel is going to need to evolve over time. Maybe we could do
> > something flexible with BPF and maps, instead of hard-coding it.
>
> Honestly, I fail to see what else we might want to do here, other than
> adding a simple mechanism for fairness, to solve the specific issue at
> hand. Flexibility would probably come at a higher cost. We could easily
> make limits configurable if needed. Do you have anything else in mind?
I think that a simple mechanism for fairness is fine. The direction of
extensibility that makes me anxious is how to decide what matters for
fairness. So far, we've talked about per-vport fairness. That works
pretty well for packets coming in from virtual interfaces where each
vport represents a separate VM. It does not work well if the traffic
filling your queues all comes from a single physical port because some
source of traffic is sending traffic at a high rate. In that case,
you'll do a lot better if you do fairness based on the source 5-tuple.
But if you're doing network virtualization, then the outer source
5-tuples won't necessarily vary much and you'd be better off looking at
the VNI and maybe some Geneve TLV options and maybe the inner 5-tuple...
I would be very pleased if we could integrate a simple mechanism for
fairness, based for now on some simple criteria like the source port,
but thinking ahead to how we could later make it gracefully extensible
to consider more general and possibly customizable criteria.
Thanks,
Ben.
^ permalink raw reply
* Re: [PATCH v2 1/2] net/sctp: Make wrappers for accessing in/out streams
From: Marcelo Ricardo Leitner @ 2018-08-03 20:40 UTC (permalink / raw)
To: Konstantin Khorenko
Cc: oleg.babin, netdev, linux-sctp, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin
In-Reply-To: <20180803162102.19540-2-khorenko@virtuozzo.com>
On Fri, Aug 03, 2018 at 07:21:01PM +0300, Konstantin Khorenko wrote:
> This patch introduces wrappers for accessing in/out streams indirectly.
> This will enable to replace physically contiguous memory arrays
> of streams with flexible arrays (or maybe any other appropriate
> mechanism) which do memory allocation on a per-page basis.
>
> Signed-off-by: Oleg Babin <obabin@virtuozzo.com>
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
>
> ---
> v2 changes:
> sctp_stream_in() users are updated to provide stream as an argument,
> sctp_stream_{in,out}_ptr() are now just sctp_stream_{in,out}().
> ---
...
>
> struct sctp_stream {
> - struct sctp_stream_out *out;
> - struct sctp_stream_in *in;
> + struct flex_array *out;
> + struct flex_array *in;
If this patch was meant to be a preparation, shouldn't this belong to
the next patch instead?
Marcelo
^ permalink raw reply
* Re: [PATCH net] dccp: fix undefined behavior with 'cwnd' shift in ccid2_cwnd_restart()
From: David Miller @ 2018-08-03 20:36 UTC (permalink / raw)
To: alexey.kodanev; +Cc: netdev, gerrit, dccp
In-Reply-To: <1533226925-16783-1-git-send-email-alexey.kodanev@oracle.com>
From: Alexey Kodanev <alexey.kodanev@oracle.com>
Date: Thu, 2 Aug 2018 19:22:05 +0300
> Make sure that the value of "(now - hc->tx_lsndtime) / hc->tx_rto" is
> properly limited when shifting 'u32 cwnd' with it, otherwise we can get:
...
> Fixes: 113ced1f52e5 ("dccp ccid-2: Perform congestion-window validation")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
...
> @@ -234,7 +234,7 @@ static void ccid2_cwnd_restart(struct sock *sk, const u32 now)
>
> /* don't reduce cwnd below the initial window (IW) */
> restart_cwnd = min(cwnd, iwnd);
> - cwnd >>= (now - hc->tx_lsndtime) / hc->tx_rto;
> + cwnd >>= min((now - hc->tx_lsndtime) / hc->tx_rto, 31U);
> hc->tx_cwnd = max(cwnd, restart_cwnd);
>
> hc->tx_cwnd_stamp = now;
Better to mimick the TCP cwnd validation code, something like:
s32 delta = now - hc->tx_lsndtime;
while ((delta -= hc->tx_rto) > 0 && cwnd > restart_cwnd)
cwnd >>= 1;
Thanks.
^ permalink raw reply
* Re: [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: Marcelo Ricardo Leitner @ 2018-08-03 20:30 UTC (permalink / raw)
To: David Laight
Cc: 'Konstantin Khorenko', oleg.babin@gmail.com,
netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
David S . Miller, Vlad Yasevich, Neil Horman, Xin Long,
Andrey Ryabinin, Michael Tuexen
In-Reply-To: <cd32848f0018432fad7246620d32c4e5@AcuMS.aculab.com>
On Fri, Aug 03, 2018 at 04:43:28PM +0000, David Laight wrote:
> From: Konstantin Khorenko
> > Sent: 03 August 2018 17:21
> >
> > Each SCTP association can have up to 65535 input and output streams.
> > For each stream type an array of sctp_stream_in or sctp_stream_out
> > structures is allocated using kmalloc_array() function. This function
> > allocates physically contiguous memory regions, so this can lead
> > to allocation of memory regions of very high order, i.e.:
> ...
>
> Given how useless SCTP streams are, does anything actually use
> more than about 4?
Maybe Michael can help us with that. I'm also curious now.
Marcelo
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: David Miller @ 2018-08-03 20:18 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, linville, andrew, vivien.didelot
In-Reply-To: <8f93154f-ab52-713c-e306-5a79b1dbfe47@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 3 Aug 2018 12:58:12 -0700
> For instance, in the current HW, you can program 128 filters through
> the switch, but only 8 of those could be wake-up capable at the
> CPU/management (SYSTEM PORT) level.
Yes, I noticed this in the driver patches.
> Let me cook something that does just that and re-post.
>
> Thanks for your feedback!
No problem.
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Florian Fainelli @ 2018-08-03 19:58 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linville, andrew, vivien.didelot
In-Reply-To: <20180803.120737.323954671047489933.davem@davemloft.net>
On 08/03/2018 12:07 PM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Fri, 3 Aug 2018 10:57:13 -0700
>
>> Does the current approach of specifying a bitmask of filters looks
>> reasonable to you though?
>
> So, in order to answer that, I need some clarification.
>
> The mask, as I see it, is a bit map of 48 possible positions
> (SOPASS_MAX * bits_per_byte). How do these bits map to individual
> rxnfc entries?
Correct about the size, it is 48-bits, each bit indeed does map to a
filter location. So if you programmed a filter a location 1, you would
pass 0x2 as the wake-on filter bitmask, etc.
>
> Are they locations? If so, how are special locations handled?
>
> What about "special" locations, where the driver and/or hardware
> are supposed to decide the location based upon the "special" type
> used?
I would not think they require special handling because the process is
kind of two step right now:
- first you program the desired filter (special location or not) and you
obtain an unique ID back
- second you program the desired filter mask with that ID as a bit
position that must be set
So the special location handling was kind of done by the kernel/driver
on the first filter insertion and you just pass that unique filter ID
around.
The reason why it was done as a two step process was largely because the
DSA switch driver, which is the one supporting the filter programming is
a discrete driver from the SYSTEM PORT driver which supports the
wake-on-filter thing. The two do communicate with one another through
the means of the DSA layer though.
Now that I think about it some more, see below, I prefer you approach
since it eliminates the "passing that ID around" step.
>
> If you considered the following, and you explained why it won't
> work, I apologize. But I'm wondering why you just don't find
> some way to specify this as a boolean of the flow spec in the
> rxnfc request or similar?
>
> That, at least semantically, seems to avoids several issues. And it
> is unambiguous what flow rule the wake filter boolean applies to.
>
> Right?
Yes, it would actually remove the need for having to specify a storage
location between user-space and kernel space and we would also be able
to valid ahead of time that we are not overflowing the wake-on-LAN
filter capacity. For instance, in the current HW, you can program 128
filters through the switch, but only 8 of those could be wake-up capable
at the CPU/management (SYSTEM PORT) level.
Let me cook something that does just that and re-post.
Thanks for your feedback!
--
Florian
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] ip: Use rb trees for IP frag queue
From: Peter Oskolkov @ 2018-08-03 19:57 UTC (permalink / raw)
To: joshhunt00; +Cc: davem, netdev, Eric Dumazet, fw
In-Reply-To: <CAKA=qzaxDaC39rKw7y_Y8uEYS=5LfdvqbKSwmyuFZ6zM8ZtMzA@mail.gmail.com>
On Fri, Aug 3, 2018 at 12:33 PM Josh Hunt <joshhunt00@gmail.com> wrote:
>
> On Thu, Aug 2, 2018 at 4:34 PM, Peter Oskolkov <posk@google.com> wrote:
>>
>> This patchset
>> * changes IPv4 defrag behavior to match that of IPv6: overlapping
>> fragments now cause the whole IP datagram to be discarded (suggested
>> by David Miller): there are no legitimate use cases for overlapping
>> fragments;
>> * changes IPv4 defrag queue from a list to a rb tree (suggested
>> by Eric Dumazet): this change removes a potential attach vector.
>>
>> Upcoming patches will contain similar changes for IPv6 frag queue,
>> as well as a comprehensive IP defrag self-test (temporarily delayed).
>>
>> Peter Oskolkov (3):
>> ip: discard IPv4 datagrams with overlapping segments.
>> net: modify skb_rbtree_purge to return the truesize of all purged
>> skbs.
>> ip: use rb trees for IP frag queue.
>>
>> include/linux/skbuff.h | 11 +-
>> include/net/inet_frag.h | 3 +-
>> include/uapi/linux/snmp.h | 1 +
>> net/core/skbuff.c | 6 +-
>> net/ipv4/inet_fragment.c | 16 +-
>> net/ipv4/ip_fragment.c | 239 +++++++++++-------------
>> net/ipv4/proc.c | 1 +
>> net/ipv6/netfilter/nf_conntrack_reasm.c | 1 +
>> net/ipv6/reassembly.c | 1 +
>> 9 files changed, 139 insertions(+), 140 deletions(-)
>>
>> --
>> 2.18.0.597.ga71716f1ad-goog
>>
>
> Peter
>
> I just tested your patches along with Florian's on top of net-next. Things look much better wrt this type of attack. Thanks for doing this. I'm wondering if we want to put an optional mechanism in place to limit the size of the tree in terms of skbs it can hold? Otherwise an attacker can send ~1400 8 byte frags and consume all frag memory (default high thresh is 4M) pretty easily and I believe also evict other frags which may have been pending? I am guessing this is what Florian's min MTU patches are trying to help with.
>
> --
> Josh
Hi Josh,
It will be really easy to limit the size of the queue/tree (e.g. based
on a sysctl parameter). I can send a follow-up patch if there is a
consensus that this behavior is needed/useful.
Thanks,
Peter
^ permalink raw reply
* Re: [PATCH v2 1/2] net/sctp: Make wrappers for accessing in/out streams
From: David Miller @ 2018-08-03 19:50 UTC (permalink / raw)
To: khorenko
Cc: marcelo.leitner, oleg.babin, netdev, linux-sctp, vyasevich,
nhorman, lucien.xin, aryabinin
In-Reply-To: <20180803162102.19540-2-khorenko@virtuozzo.com>
From: Konstantin Khorenko <khorenko@virtuozzo.com>
Date: Fri, 3 Aug 2018 19:21:01 +0300
> +struct sctp_stream_out *sctp_stream_out(const struct sctp_stream *stream,
> + __u16 sid)
> +{
> + return ((struct sctp_stream_out *)(stream->out)) + sid;
> +}
> +
> +struct sctp_stream_in *sctp_stream_in(const struct sctp_stream *stream,
> + __u16 sid)
> +{
> + return ((struct sctp_stream_in *)(stream->in)) + sid;
> +}
I agree with David that these should be in a header file, and marked
inline.
^ permalink raw reply
* Re: [PATCH net] l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()
From: David Miller @ 2018-08-03 19:42 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
In-Reply-To: <e0486aaa7f6a1ecdd12d0231d0b3ed560158a0f9.1533307527.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Fri, 3 Aug 2018 17:00:11 +0200
> If 'session' is not NULL and is not a PPP pseudo-wire, then we fail to
> drop the reference taken by l2tp_session_get().
>
> Fixes: ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> ---
> Sorry for the stupid mistake. I guess I got blinded by the apparent
> simplicity of the bug when I wrote the original patch.
Applied, thanks.
I'm pretty sure I backported the commit this fixes, so I'm queueing
this up for -stable as well.
^ permalink raw reply
* Re: [Patch net] ipv6: fix double refcount of fib6_metrics
From: David Miller @ 2018-08-03 19:38 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, sd, dsahern
In-Reply-To: <20180803062038.13272-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 2 Aug 2018 23:20:38 -0700
> All the callers of ip6_rt_copy_init()/rt6_set_from() hold refcnt
> of the "from" fib6_info, so there is no need to hold fib6_metrics
> refcnt again, because fib6_metrics refcnt is only released when
> fib6_info is gone, that is, they have the same life time, so the
> whole fib6_metrics refcnt can be removed actually.
>
> This fixes a kmemleak warning reported by Sabrina.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Reported-by: Sabrina Dubroca <sd@queasysnail.net>
> Cc: Sabrina Dubroca <sd@queasysnail.net>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Sabrina, please review!
^ permalink raw reply
* Re: [PATCH net 0/4] mlxsw: Fix ACL actions error condition handling
From: David Miller @ 2018-08-03 19:35 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, petrm, nird, mlxsw
In-Reply-To: <20180803125744.16200-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Fri, 3 Aug 2018 15:57:40 +0300
> Nir says:
>
> Two issues were lately noticed within mlxsw ACL actions error condition
> handling. The first patch deals with conflicting actions such as:
>
> # tc filter add dev swp49 parent ffff: \
> protocol ip pref 10 flower skip_sw dst_ip 192.168.101.1 \
> action goto chain 100 \
> action mirred egress redirect dev swp4
>
> The second action will never execute, however SW model allows this
> configuration, while the mlxsw driver cannot allow for it as it
> implements actions in sets of up to three actions per set with a single
> termination marking. Conflicting actions create a contradiction over
> this single marking and thus cannot be configured. The fix replaces a
> misplaced warning with an error code to be returned.
>
> Patches 2-4 fix a condition of duplicate destruction of resources. Some
> actions require allocation of specific resource prior to setting the
> action itself. On error condition this resource was destroyed twice,
> leading to a crash when using mirror action, and to a redundant
> destruction in other cases, since for error condition rule destruction
> also takes care of resource destruction. In order to fix this state a
> symmetry in behavior is added and resource destruction also takes care
> of removing the resource from rule's resource list.
Series applied, and queued up for -stable.
And thanks especially for the merge conflict heads up.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] ip: Use rb trees for IP frag queue
From: Josh Hunt @ 2018-08-03 19:33 UTC (permalink / raw)
To: Peter Oskolkov; +Cc: David Miller, netdev, Eric Dumazet, Florian Westphal
In-Reply-To: <20180802233439.51643-1-posk@google.com>
[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]
On Thu, Aug 2, 2018 at 4:34 PM, Peter Oskolkov <posk@google.com> wrote:
> This patchset
> * changes IPv4 defrag behavior to match that of IPv6: overlapping
> fragments now cause the whole IP datagram to be discarded (suggested
> by David Miller): there are no legitimate use cases for overlapping
> fragments;
> * changes IPv4 defrag queue from a list to a rb tree (suggested
> by Eric Dumazet): this change removes a potential attach vector.
>
> Upcoming patches will contain similar changes for IPv6 frag queue,
> as well as a comprehensive IP defrag self-test (temporarily delayed).
>
> Peter Oskolkov (3):
> ip: discard IPv4 datagrams with overlapping segments.
> net: modify skb_rbtree_purge to return the truesize of all purged
> skbs.
> ip: use rb trees for IP frag queue.
>
> include/linux/skbuff.h | 11 +-
> include/net/inet_frag.h | 3 +-
> include/uapi/linux/snmp.h | 1 +
> net/core/skbuff.c | 6 +-
> net/ipv4/inet_fragment.c | 16 +-
> net/ipv4/ip_fragment.c | 239 +++++++++++-------------
> net/ipv4/proc.c | 1 +
> net/ipv6/netfilter/nf_conntrack_reasm.c | 1 +
> net/ipv6/reassembly.c | 1 +
> 9 files changed, 139 insertions(+), 140 deletions(-)
>
> --
> 2.18.0.597.ga71716f1ad-goog
>
>
Peter
I just tested your patches along with Florian's on top of net-next. Things
look much better wrt this type of attack. Thanks for doing this. I'm
wondering if we want to put an optional mechanism in place to limit the
size of the tree in terms of skbs it can hold? Otherwise an attacker can
send ~1400 8 byte frags and consume all frag memory (default high thresh is
4M) pretty easily and I believe also evict other frags which may have been
pending? I am guessing this is what Florian's min MTU patches are trying to
help with.
--
Josh
[-- Attachment #2: Type: text/html, Size: 2675 bytes --]
^ permalink raw reply
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: Andy Lutomirski @ 2018-08-03 21:29 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Eric Biggers, Linux Crypto Mailing List, LKML, Netdev,
David Miller, Andrew Lutomirski, Greg Kroah-Hartman, Samuel Neves,
Daniel J . Bernstein, Tanja Lange, Jean-Philippe Aumasson,
Karthikeyan Bhargavan
In-Reply-To: <CAHmME9oThR-dE3gTW0UyqAGZO80qu19ktG4YTb4iL6CNpzNNaw@mail.gmail.com>
On Thu, Aug 2, 2018 at 7:48 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hey Andy,
>
> Thanks too for the feedback. Responses below:
>
> On Wed, Aug 1, 2018 at 7:09 PM Andy Lutomirski <luto@amacapital.net> wrote:
>> > I think the above changes would also naturally lead to a much saner
>> > patch series where each algorithm is added by its own patch, rather than
>> > one monster patch that adds many algorithms and 24000 lines of code.
>> >
>>
>> Yes, please.
>
> Ack, will be in v2.
>
>
>> I like this a *lot*. (But why are you passing have_simd? Shouldn't
>> that check live in chacha20_arch? If there's some init code needed,
>> then chacha20_arch() should just return false before the init code
>> runs. Once the arch does whatever feature detection it needs, it can
>> make chacha20_arch() start returning true.)
>
> The have_simd stuff is so that the FPU state can be amortized across
> several calls to the crypto functions. Here's a snippet from
> WireGuard's send.c:
>
> void packet_encrypt_worker(struct work_struct *work)
> {
> struct crypt_queue *queue = container_of(work, struct
> multicore_worker, work)->ptr;
> struct sk_buff *first, *skb, *next;
> bool have_simd = simd_get();
Gotcha. That was very hidden in the 24k lines. Please make this (and
any similar goodies) be their own patches.
Also, please consider spelling it differently:
simd_context_t simd_context = simd_get();
Because we'll feel very silly the first time some architecture has
more than one possible state. (It wouldn't be entirely insane for x86
to distinguish between "no SIMD", "XMM only", and "go to town!", for
example.)
^ permalink raw reply
* [PATCH net-next] net: sched: cls_flower: Fix an error code in fl_tmplt_create()
From: Dan Carpenter @ 2018-08-03 19:27 UTC (permalink / raw)
To: Jamal Hadi Salim, Jiri Pirko
Cc: Cong Wang, David S. Miller, netdev, kernel-janitors
We forgot to set the error code on this path, so we return NULL instead
of an error pointer. In the current code kzalloc() won't fail for small
allocations so this doesn't really affect runtime.
Fixes: b95ec7eb3b4d ("net: sched: cls_flower: implement chain templates")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index e8bd08ba998a..a3b69bb6f4b0 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1250,8 +1250,10 @@ static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
goto errout_tb;
tmplt = kzalloc(sizeof(*tmplt), GFP_KERNEL);
- if (!tmplt)
+ if (!tmplt) {
+ err = -ENOMEM;
goto errout_tb;
+ }
tmplt->chain = chain;
err = fl_set_key(net, tb, &tmplt->dummy_key, &tmplt->mask, extack);
if (err)
^ permalink raw reply related
* Re: [PATCH net-next 0/4] net: dsa and systemport WoL changes
From: David Miller @ 2018-08-03 19:13 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
In-Reply-To: <20180803180844.1010-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 3 Aug 2018 11:08:40 -0700
> This patch series extracts what was previously submitted as part of the
> "WAKE_FILTER" Wake-on-LAN patch series into patches that do not.
>
> Changes in this series:
>
> - properly align the dsa_is_cpu_port() check in first patch
Series applied, thanks for splitting these out into a separate series.
^ permalink raw reply
* Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue
From: Michael S. Tsirkin @ 2018-08-03 19:12 UTC (permalink / raw)
To: Andrei Vagin
Cc: Nambiar, Amritha, netdev, davem, alexander.h.duyck,
willemdebruijn.kernel, sridhar.samudrala, alexander.duyck,
edumazet, hannes, tom, tom, jasowang, gaowanlong, virtualization,
rostedt
In-Reply-To: <20180803190650.GA2157@outlook.office365.com>
On Fri, Aug 03, 2018 at 12:06:51PM -0700, Andrei Vagin wrote:
> On Fri, Aug 03, 2018 at 12:08:05AM +0300, Michael S. Tsirkin wrote:
> > On Thu, Aug 02, 2018 at 02:04:12PM -0700, Nambiar, Amritha wrote:
> > > On 8/1/2018 5:11 PM, Andrei Vagin wrote:
> > > > On Tue, Jul 10, 2018 at 07:28:49PM -0700, Nambiar, Amritha wrote:
> > > >> With this patch series, I introduced static_key for XPS maps
> > > >> (xps_needed), so static_key_slow_inc() is used to switch branches. The
> > > >> definition of static_key_slow_inc() has cpus_read_lock in place. In the
> > > >> virtio_net driver, XPS queues are initialized after setting the
> > > >> queue:cpu affinity in virtnet_set_affinity() which is already protected
> > > >> within cpus_read_lock. Hence, the warning here trying to acquire
> > > >> cpus_read_lock when it is already held.
> > > >>
> > > >> A quick fix for this would be to just extract netif_set_xps_queue() out
> > > >> of the lock by simply wrapping it with another put/get_online_cpus
> > > >> (unlock right before and hold lock right after).
> > > >
> > > > virtnet_set_affinity() is called from virtnet_cpu_online(), which is
> > > > called under cpus_read_lock too.
> > > >
> > > > It looks like now we can't call netif_set_xps_queue() from cpu hotplug
> > > > callbacks.
> > > >
> > > > I can suggest a very straightforward fix for this problem. The patch is
> > > > attached.
> > > >
> > >
> > > Thanks for looking into this. I was thinking of fixing this in the
> > > virtio_net driver by moving the XPS initialization (and have a new
> > > get_affinity utility) in the ndo_open (so it is together with other tx
> > > preparation) instead of probe. Your patch solves this in general for
> > > setting up cpu hotplug callbacks which is under cpus_read_lock.
> >
> >
> > I like this too. Could you repost in a standard way
> > (inline, with your signoff etc) so we can ack this for
> > net-next?
>
> When I was testing this patch, I got the following kasan warning. Michael,
> could you take a look at it. Maybe you will understand what was going wrong there.
>
> https://api.travis-ci.org/v3/job/410701353/log.txt
>
> [ 7.275033] ==================================================================
> [ 7.275226] BUG: KASAN: slab-out-of-bounds in virtnet_poll+0xaa1/0xd00
> [ 7.275359] Read of size 8 at addr ffff8801d444a000 by task ip/370
> [ 7.275488]
> [ 7.275610] CPU: 1 PID: 370 Comm: ip Not tainted 4.18.0-rc6+ #1
> [ 7.275613] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> [ 7.275616] Call Trace:
> [ 7.275621] <IRQ>
> [ 7.275630] dump_stack+0x71/0xab
> [ 7.275640] print_address_description+0x6a/0x270
> [ 7.275648] kasan_report+0x258/0x380
> [ 7.275653] ? virtnet_poll+0xaa1/0xd00
> [ 7.275661] virtnet_poll+0xaa1/0xd00
> [ 7.275680] ? receive_buf+0x5920/0x5920
> [ 7.275689] ? do_raw_spin_unlock+0x54/0x220
> [ 7.275699] ? find_held_lock+0x32/0x1c0
> [ 7.275710] ? rcu_process_callbacks+0xa60/0xd20
> [ 7.275736] net_rx_action+0x2ee/0xad0
> [ 7.275748] ? rcu_note_context_switch+0x320/0x320
> [ 7.275754] ? napi_complete_done+0x300/0x300
> [ 7.275763] ? native_apic_msr_write+0x27/0x30
> [ 7.275768] ? lapic_next_event+0x5b/0x90
> [ 7.275775] ? clockevents_program_event+0x21d/0x2f0
> [ 7.275791] __do_softirq+0x19a/0x623
> [ 7.275807] do_softirq_own_stack+0x2a/0x40
> [ 7.275811] </IRQ>
> [ 7.275818] do_softirq.part.18+0x6a/0x80
> [ 7.275825] __local_bh_enable_ip+0x49/0x50
> [ 7.275829] virtnet_open+0x129/0x440
> [ 7.275841] __dev_open+0x189/0x2c0
> [ 7.275848] ? dev_set_rx_mode+0x30/0x30
> [ 7.275857] ? do_raw_spin_unlock+0x54/0x220
> [ 7.275866] __dev_change_flags+0x3a9/0x4f0
> [ 7.275873] ? dev_set_allmulti+0x10/0x10
> [ 7.275889] dev_change_flags+0x7a/0x150
> [ 7.275900] do_setlink+0x9fe/0x2e40
> [ 7.275910] ? deref_stack_reg+0xad/0xe0
> [ 7.275917] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> [ 7.275922] ? find_held_lock+0x32/0x1c0
> [ 7.275929] ? rtnetlink_put_metrics+0x460/0x460
> [ 7.275935] ? virtqueue_add_sgs+0x9e2/0xde0
> [ 7.275953] ? virtscsi_add_cmd+0x454/0x780
> [ 7.275964] ? find_held_lock+0x32/0x1c0
> [ 7.275973] ? deref_stack_reg+0xad/0xe0
> [ 7.275979] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> [ 7.275985] ? lock_downgrade+0x5e0/0x5e0
> [ 7.275993] ? memset+0x1f/0x40
> [ 7.276008] ? nla_parse+0x33/0x290
> [ 7.276016] rtnl_newlink+0x954/0x1120
> [ 7.276030] ? rtnl_link_unregister+0x250/0x250
> [ 7.276044] ? is_bpf_text_address+0x5/0x60
> [ 7.276054] ? lock_downgrade+0x5e0/0x5e0
> [ 7.276057] ? lock_acquire+0x10b/0x2a0
> [ 7.276072] ? deref_stack_reg+0xad/0xe0
> [ 7.276078] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> [ 7.276085] ? __kernel_text_address+0xe/0x30
> [ 7.276090] ? unwind_get_return_address+0x5f/0xa0
> [ 7.276103] ? find_held_lock+0x32/0x1c0
> [ 7.276110] ? is_bpf_text_address+0x5/0x60
> [ 7.276124] ? deref_stack_reg+0xad/0xe0
> [ 7.276131] ? __read_once_size_nocheck.constprop.6+0x10/0x10
> [ 7.276136] ? depot_save_stack+0x2d9/0x460
> [ 7.276142] ? deref_stack_reg+0xad/0xe0
> [ 7.276156] ? find_held_lock+0x32/0x1c0
> [ 7.276164] ? is_bpf_text_address+0x5/0x60
> [ 7.276170] ? __lock_acquire.isra.29+0xe8/0x1bb0
> [ 7.276212] ? rtnetlink_rcv_msg+0x5d6/0x930
> [ 7.276222] ? lock_downgrade+0x5e0/0x5e0
> [ 7.276226] ? lock_acquire+0x10b/0x2a0
> [ 7.276240] rtnetlink_rcv_msg+0x69e/0x930
> [ 7.276249] ? rtnl_calcit.isra.31+0x2f0/0x2f0
> [ 7.276255] ? __lock_acquire.isra.29+0xe8/0x1bb0
> [ 7.276265] ? netlink_deliver_tap+0x8d/0x8e0
> [ 7.276276] netlink_rcv_skb+0x127/0x350
> [ 7.276281] ? rtnl_calcit.isra.31+0x2f0/0x2f0
> [ 7.276289] ? netlink_ack+0x970/0x970
> [ 7.276299] ? __alloc_skb+0xc2/0x520
> [ 7.276311] netlink_unicast+0x40f/0x5d0
> [ 7.276320] ? netlink_attachskb+0x580/0x580
> [ 7.276325] ? _copy_from_iter_full+0x157/0x6f0
> [ 7.276331] ? import_iovec+0x90/0x390
> [ 7.276338] ? get_page_from_freelist+0x1e89/0x3e30
> [ 7.276347] ? apparmor_socket_sock_rcv_skb+0x10/0x10
> [ 7.276357] netlink_sendmsg+0x65e/0xb00
> [ 7.276367] ? netlink_unicast+0x5d0/0x5d0
> [ 7.276373] ? copy_msghdr_from_user+0x206/0x340
> [ 7.276388] ? netlink_unicast+0x5d0/0x5d0
> [ 7.276394] sock_sendmsg+0xb3/0xf0
> [ 7.276401] ___sys_sendmsg+0x604/0x8b0
> [ 7.276410] ? copy_msghdr_from_user+0x340/0x340
> [ 7.276416] ? find_held_lock+0x32/0x1c0
> [ 7.276424] ? __handle_mm_fault+0xc85/0x3140
> [ 7.276433] ? lock_downgrade+0x5e0/0x5e0
> [ 7.276439] ? mem_cgroup_commit_charge+0xb4/0xf80
> [ 7.276453] ? _raw_spin_unlock+0x24/0x30
> [ 7.276458] ? __handle_mm_fault+0xc85/0x3140
> [ 7.276467] ? __pmd_alloc+0x430/0x430
> [ 7.276473] ? find_held_lock+0x32/0x1c0
> [ 7.276485] ? __fget_light+0x55/0x1f0
> [ 7.276497] ? __sys_sendmsg+0xd2/0x170
> [ 7.276502] __sys_sendmsg+0xd2/0x170
> [ 7.276508] ? __ia32_sys_shutdown+0x70/0x70
> [ 7.276516] ? handle_mm_fault+0x1f9/0x6a0
> [ 7.276528] ? up_read+0x1c/0x110
> [ 7.276534] ? __do_page_fault+0x4a6/0xa80
> [ 7.276554] do_syscall_64+0xa0/0x280
> [ 7.276558] ? prepare_exit_to_usermode+0x88/0x130
> [ 7.276566] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 7.276572] RIP: 0033:0x7ffbe9a2f160
> [ 7.276574] Code: c3 48 8b 05 2a 2d 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 1d 8f 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee cb 00 00 48 89 04 24
> [ 7.276728] RSP: 002b:00007ffc5a2d6108 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [ 7.276735] RAX: ffffffffffffffda RBX: 00007ffc5a2da220 RCX: 00007ffbe9a2f160
> [ 7.276738] RDX: 0000000000000000 RSI: 00007ffc5a2d6150 RDI: 0000000000000003
> [ 7.276741] RBP: 00007ffc5a2d6150 R08: 0000000000000000 R09: 0000000000000003
> [ 7.276744] R10: 00007ffc5a2d5ed0 R11: 0000000000000246 R12: 000000005b6177de
> [ 7.276748] R13: 0000000000000000 R14: 00000000006473a0 R15: 00007ffc5a2da918
> [ 7.276763]
> [ 7.276895] Allocated by task 1:
> [ 7.277026] kasan_kmalloc+0xa0/0xd0
> [ 7.277030] __kmalloc+0x13a/0x250
> [ 7.277034] init_vqs+0xd0/0x11c0
> [ 7.277038] virtnet_probe+0xb99/0x1ad0
> [ 7.277045] virtio_dev_probe+0x3fc/0x890
> [ 7.277052] driver_probe_device+0x6c4/0xcc0
> [ 7.277056] __driver_attach+0x232/0x2c0
> [ 7.277060] bus_for_each_dev+0x118/0x1a0
> [ 7.277064] bus_add_driver+0x390/0x6e0
> [ 7.277068] driver_register+0x18e/0x400
> [ 7.277076] virtio_net_driver_init+0x6d/0x90
> [ 7.277080] do_one_initcall+0xa8/0x348
> [ 7.277085] kernel_init_freeable+0x42d/0x4c8
> [ 7.277090] kernel_init+0xf/0x130
> [ 7.277095] ret_from_fork+0x35/0x40
> [ 7.277097]
> [ 7.277223] Freed by task 0:
> [ 7.277347] (stack is not available)
> [ 7.277473]
> [ 7.277596] The buggy address belongs to the object at ffff8801d4449100
> [ 7.277596] which belongs to the cache kmalloc-4096 of size 4096
> [ 7.277769] The buggy address is located 3840 bytes inside of
> [ 7.277769] 4096-byte region [ffff8801d4449100, ffff8801d444a100)
> [ 7.277932] The buggy address belongs to the page:
> [ 7.278066] page:ffffea0007511200 count:1 mapcount:0 mapping:ffff8801db002600 index:0x0 compound_mapcount: 0
> [ 7.278230] flags: 0x17fff8000008100(slab|head)
> [ 7.278363] raw: 017fff8000008100 dead000000000100 dead000000000200 ffff8801db002600
> [ 7.278516] raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
> [ 7.278664] page dumped because: kasan: bad access detected
> [ 7.278790]
> [ 7.278904] Memory state around the buggy address:
> [ 7.279031] ffff8801d4449f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 7.279175] ffff8801d4449f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 7.279323] >ffff8801d444a000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 7.279468] ^
> [ 7.279584] ffff8801d444a080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 7.279729] ffff8801d444a100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ 7.279870] ==================================================================
> [ 7.280011] Disabling lock debugging due to kernel taint
> [ 7.632219] random: mktemp: uninitialized urandom read (6 bytes read)
> [ 8.052850] random: mktemp: uninitialized urandom read (6 bytes read)
> [ 8.335209] random: cloud-init: uninitialized urandom read (32 bytes read)
> [ 8.796331] random: cloud-init: uninitialized urandom read (32 bytes read)
> [ 9.162551] random: mktemp: uninitialized urandom read (12 bytes read)
> [ 9.384504] random: ssh-keygen: uninitialized urandom read (32 bytes read)
> [ 9.839586] init: failsafe main process (724) killed by TERM signal
> [ 14.865443] postgres (1245): /proc/1245/oom_adj is deprecated, please use /proc/1245/oom_score_adj instead.
> [ 17.213418] random: crng init done
> [ 17.580892] init: plymouth-upstart-bridge main process ended, respawning
I suspect an off by one somewhere. I'm looking at the patch
and I don't see it but these things are hard to spot sometimes ...
> >
> > > >> But this may not a
> > > >> clean solution. It'd help if I can get suggestions on what would be a
> > > >> clean option to fix this without extensively changing the code in
> > > >> virtio_net. Is it mandatory to protect the affinitization with
> > > >> read_lock? I don't see similar lock in other drivers while setting the
> > > >> affinity. I understand this warning should go away, but isn't it safe to
> > > >> have multiple readers.
> > > >>
> > > >>> On Fri, Jun 29, 2018 at 09:27:07PM -0700, Amritha Nambiar wrote:
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: David Miller @ 2018-08-03 19:07 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, linville, andrew, vivien.didelot
In-Reply-To: <56447ed2-c441-81e5-3219-b2f6329366d1@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 3 Aug 2018 10:57:13 -0700
> Does the current approach of specifying a bitmask of filters looks
> reasonable to you though?
So, in order to answer that, I need some clarification.
The mask, as I see it, is a bit map of 48 possible positions
(SOPASS_MAX * bits_per_byte). How do these bits map to individual
rxnfc entries?
Are they locations? If so, how are special locations handled?
What about "special" locations, where the driver and/or hardware
are supposed to decide the location based upon the "special" type
used?
If you considered the following, and you explained why it won't
work, I apologize. But I'm wondering why you just don't find
some way to specify this as a boolean of the flow spec in the
rxnfc request or similar?
That, at least semantically, seems to avoids several issues. And it
is unambiguous what flow rule the wake filter boolean applies to.
Right?
^ permalink raw reply
* Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue
From: Andrei Vagin @ 2018-08-03 19:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Nambiar, Amritha, netdev, davem, alexander.h.duyck,
willemdebruijn.kernel, sridhar.samudrala, alexander.duyck,
edumazet, hannes, tom, tom, jasowang, gaowanlong, virtualization,
rostedt
In-Reply-To: <20180803000725-mutt-send-email-mst@kernel.org>
On Fri, Aug 03, 2018 at 12:08:05AM +0300, Michael S. Tsirkin wrote:
> On Thu, Aug 02, 2018 at 02:04:12PM -0700, Nambiar, Amritha wrote:
> > On 8/1/2018 5:11 PM, Andrei Vagin wrote:
> > > On Tue, Jul 10, 2018 at 07:28:49PM -0700, Nambiar, Amritha wrote:
> > >> With this patch series, I introduced static_key for XPS maps
> > >> (xps_needed), so static_key_slow_inc() is used to switch branches. The
> > >> definition of static_key_slow_inc() has cpus_read_lock in place. In the
> > >> virtio_net driver, XPS queues are initialized after setting the
> > >> queue:cpu affinity in virtnet_set_affinity() which is already protected
> > >> within cpus_read_lock. Hence, the warning here trying to acquire
> > >> cpus_read_lock when it is already held.
> > >>
> > >> A quick fix for this would be to just extract netif_set_xps_queue() out
> > >> of the lock by simply wrapping it with another put/get_online_cpus
> > >> (unlock right before and hold lock right after).
> > >
> > > virtnet_set_affinity() is called from virtnet_cpu_online(), which is
> > > called under cpus_read_lock too.
> > >
> > > It looks like now we can't call netif_set_xps_queue() from cpu hotplug
> > > callbacks.
> > >
> > > I can suggest a very straightforward fix for this problem. The patch is
> > > attached.
> > >
> >
> > Thanks for looking into this. I was thinking of fixing this in the
> > virtio_net driver by moving the XPS initialization (and have a new
> > get_affinity utility) in the ndo_open (so it is together with other tx
> > preparation) instead of probe. Your patch solves this in general for
> > setting up cpu hotplug callbacks which is under cpus_read_lock.
>
>
> I like this too. Could you repost in a standard way
> (inline, with your signoff etc) so we can ack this for
> net-next?
When I was testing this patch, I got the following kasan warning. Michael,
could you take a look at it. Maybe you will understand what was going wrong there.
https://api.travis-ci.org/v3/job/410701353/log.txt
[ 7.275033] ==================================================================
[ 7.275226] BUG: KASAN: slab-out-of-bounds in virtnet_poll+0xaa1/0xd00
[ 7.275359] Read of size 8 at addr ffff8801d444a000 by task ip/370
[ 7.275488]
[ 7.275610] CPU: 1 PID: 370 Comm: ip Not tainted 4.18.0-rc6+ #1
[ 7.275613] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[ 7.275616] Call Trace:
[ 7.275621] <IRQ>
[ 7.275630] dump_stack+0x71/0xab
[ 7.275640] print_address_description+0x6a/0x270
[ 7.275648] kasan_report+0x258/0x380
[ 7.275653] ? virtnet_poll+0xaa1/0xd00
[ 7.275661] virtnet_poll+0xaa1/0xd00
[ 7.275680] ? receive_buf+0x5920/0x5920
[ 7.275689] ? do_raw_spin_unlock+0x54/0x220
[ 7.275699] ? find_held_lock+0x32/0x1c0
[ 7.275710] ? rcu_process_callbacks+0xa60/0xd20
[ 7.275736] net_rx_action+0x2ee/0xad0
[ 7.275748] ? rcu_note_context_switch+0x320/0x320
[ 7.275754] ? napi_complete_done+0x300/0x300
[ 7.275763] ? native_apic_msr_write+0x27/0x30
[ 7.275768] ? lapic_next_event+0x5b/0x90
[ 7.275775] ? clockevents_program_event+0x21d/0x2f0
[ 7.275791] __do_softirq+0x19a/0x623
[ 7.275807] do_softirq_own_stack+0x2a/0x40
[ 7.275811] </IRQ>
[ 7.275818] do_softirq.part.18+0x6a/0x80
[ 7.275825] __local_bh_enable_ip+0x49/0x50
[ 7.275829] virtnet_open+0x129/0x440
[ 7.275841] __dev_open+0x189/0x2c0
[ 7.275848] ? dev_set_rx_mode+0x30/0x30
[ 7.275857] ? do_raw_spin_unlock+0x54/0x220
[ 7.275866] __dev_change_flags+0x3a9/0x4f0
[ 7.275873] ? dev_set_allmulti+0x10/0x10
[ 7.275889] dev_change_flags+0x7a/0x150
[ 7.275900] do_setlink+0x9fe/0x2e40
[ 7.275910] ? deref_stack_reg+0xad/0xe0
[ 7.275917] ? __read_once_size_nocheck.constprop.6+0x10/0x10
[ 7.275922] ? find_held_lock+0x32/0x1c0
[ 7.275929] ? rtnetlink_put_metrics+0x460/0x460
[ 7.275935] ? virtqueue_add_sgs+0x9e2/0xde0
[ 7.275953] ? virtscsi_add_cmd+0x454/0x780
[ 7.275964] ? find_held_lock+0x32/0x1c0
[ 7.275973] ? deref_stack_reg+0xad/0xe0
[ 7.275979] ? __read_once_size_nocheck.constprop.6+0x10/0x10
[ 7.275985] ? lock_downgrade+0x5e0/0x5e0
[ 7.275993] ? memset+0x1f/0x40
[ 7.276008] ? nla_parse+0x33/0x290
[ 7.276016] rtnl_newlink+0x954/0x1120
[ 7.276030] ? rtnl_link_unregister+0x250/0x250
[ 7.276044] ? is_bpf_text_address+0x5/0x60
[ 7.276054] ? lock_downgrade+0x5e0/0x5e0
[ 7.276057] ? lock_acquire+0x10b/0x2a0
[ 7.276072] ? deref_stack_reg+0xad/0xe0
[ 7.276078] ? __read_once_size_nocheck.constprop.6+0x10/0x10
[ 7.276085] ? __kernel_text_address+0xe/0x30
[ 7.276090] ? unwind_get_return_address+0x5f/0xa0
[ 7.276103] ? find_held_lock+0x32/0x1c0
[ 7.276110] ? is_bpf_text_address+0x5/0x60
[ 7.276124] ? deref_stack_reg+0xad/0xe0
[ 7.276131] ? __read_once_size_nocheck.constprop.6+0x10/0x10
[ 7.276136] ? depot_save_stack+0x2d9/0x460
[ 7.276142] ? deref_stack_reg+0xad/0xe0
[ 7.276156] ? find_held_lock+0x32/0x1c0
[ 7.276164] ? is_bpf_text_address+0x5/0x60
[ 7.276170] ? __lock_acquire.isra.29+0xe8/0x1bb0
[ 7.276212] ? rtnetlink_rcv_msg+0x5d6/0x930
[ 7.276222] ? lock_downgrade+0x5e0/0x5e0
[ 7.276226] ? lock_acquire+0x10b/0x2a0
[ 7.276240] rtnetlink_rcv_msg+0x69e/0x930
[ 7.276249] ? rtnl_calcit.isra.31+0x2f0/0x2f0
[ 7.276255] ? __lock_acquire.isra.29+0xe8/0x1bb0
[ 7.276265] ? netlink_deliver_tap+0x8d/0x8e0
[ 7.276276] netlink_rcv_skb+0x127/0x350
[ 7.276281] ? rtnl_calcit.isra.31+0x2f0/0x2f0
[ 7.276289] ? netlink_ack+0x970/0x970
[ 7.276299] ? __alloc_skb+0xc2/0x520
[ 7.276311] netlink_unicast+0x40f/0x5d0
[ 7.276320] ? netlink_attachskb+0x580/0x580
[ 7.276325] ? _copy_from_iter_full+0x157/0x6f0
[ 7.276331] ? import_iovec+0x90/0x390
[ 7.276338] ? get_page_from_freelist+0x1e89/0x3e30
[ 7.276347] ? apparmor_socket_sock_rcv_skb+0x10/0x10
[ 7.276357] netlink_sendmsg+0x65e/0xb00
[ 7.276367] ? netlink_unicast+0x5d0/0x5d0
[ 7.276373] ? copy_msghdr_from_user+0x206/0x340
[ 7.276388] ? netlink_unicast+0x5d0/0x5d0
[ 7.276394] sock_sendmsg+0xb3/0xf0
[ 7.276401] ___sys_sendmsg+0x604/0x8b0
[ 7.276410] ? copy_msghdr_from_user+0x340/0x340
[ 7.276416] ? find_held_lock+0x32/0x1c0
[ 7.276424] ? __handle_mm_fault+0xc85/0x3140
[ 7.276433] ? lock_downgrade+0x5e0/0x5e0
[ 7.276439] ? mem_cgroup_commit_charge+0xb4/0xf80
[ 7.276453] ? _raw_spin_unlock+0x24/0x30
[ 7.276458] ? __handle_mm_fault+0xc85/0x3140
[ 7.276467] ? __pmd_alloc+0x430/0x430
[ 7.276473] ? find_held_lock+0x32/0x1c0
[ 7.276485] ? __fget_light+0x55/0x1f0
[ 7.276497] ? __sys_sendmsg+0xd2/0x170
[ 7.276502] __sys_sendmsg+0xd2/0x170
[ 7.276508] ? __ia32_sys_shutdown+0x70/0x70
[ 7.276516] ? handle_mm_fault+0x1f9/0x6a0
[ 7.276528] ? up_read+0x1c/0x110
[ 7.276534] ? __do_page_fault+0x4a6/0xa80
[ 7.276554] do_syscall_64+0xa0/0x280
[ 7.276558] ? prepare_exit_to_usermode+0x88/0x130
[ 7.276566] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 7.276572] RIP: 0033:0x7ffbe9a2f160
[ 7.276574] Code: c3 48 8b 05 2a 2d 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 1d 8f 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee cb 00 00 48 89 04 24
[ 7.276728] RSP: 002b:00007ffc5a2d6108 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 7.276735] RAX: ffffffffffffffda RBX: 00007ffc5a2da220 RCX: 00007ffbe9a2f160
[ 7.276738] RDX: 0000000000000000 RSI: 00007ffc5a2d6150 RDI: 0000000000000003
[ 7.276741] RBP: 00007ffc5a2d6150 R08: 0000000000000000 R09: 0000000000000003
[ 7.276744] R10: 00007ffc5a2d5ed0 R11: 0000000000000246 R12: 000000005b6177de
[ 7.276748] R13: 0000000000000000 R14: 00000000006473a0 R15: 00007ffc5a2da918
[ 7.276763]
[ 7.276895] Allocated by task 1:
[ 7.277026] kasan_kmalloc+0xa0/0xd0
[ 7.277030] __kmalloc+0x13a/0x250
[ 7.277034] init_vqs+0xd0/0x11c0
[ 7.277038] virtnet_probe+0xb99/0x1ad0
[ 7.277045] virtio_dev_probe+0x3fc/0x890
[ 7.277052] driver_probe_device+0x6c4/0xcc0
[ 7.277056] __driver_attach+0x232/0x2c0
[ 7.277060] bus_for_each_dev+0x118/0x1a0
[ 7.277064] bus_add_driver+0x390/0x6e0
[ 7.277068] driver_register+0x18e/0x400
[ 7.277076] virtio_net_driver_init+0x6d/0x90
[ 7.277080] do_one_initcall+0xa8/0x348
[ 7.277085] kernel_init_freeable+0x42d/0x4c8
[ 7.277090] kernel_init+0xf/0x130
[ 7.277095] ret_from_fork+0x35/0x40
[ 7.277097]
[ 7.277223] Freed by task 0:
[ 7.277347] (stack is not available)
[ 7.277473]
[ 7.277596] The buggy address belongs to the object at ffff8801d4449100
[ 7.277596] which belongs to the cache kmalloc-4096 of size 4096
[ 7.277769] The buggy address is located 3840 bytes inside of
[ 7.277769] 4096-byte region [ffff8801d4449100, ffff8801d444a100)
[ 7.277932] The buggy address belongs to the page:
[ 7.278066] page:ffffea0007511200 count:1 mapcount:0 mapping:ffff8801db002600 index:0x0 compound_mapcount: 0
[ 7.278230] flags: 0x17fff8000008100(slab|head)
[ 7.278363] raw: 017fff8000008100 dead000000000100 dead000000000200 ffff8801db002600
[ 7.278516] raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
[ 7.278664] page dumped because: kasan: bad access detected
[ 7.278790]
[ 7.278904] Memory state around the buggy address:
[ 7.279031] ffff8801d4449f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.279175] ffff8801d4449f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.279323] >ffff8801d444a000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.279468] ^
[ 7.279584] ffff8801d444a080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.279729] ffff8801d444a100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.279870] ==================================================================
[ 7.280011] Disabling lock debugging due to kernel taint
[ 7.632219] random: mktemp: uninitialized urandom read (6 bytes read)
[ 8.052850] random: mktemp: uninitialized urandom read (6 bytes read)
[ 8.335209] random: cloud-init: uninitialized urandom read (32 bytes read)
[ 8.796331] random: cloud-init: uninitialized urandom read (32 bytes read)
[ 9.162551] random: mktemp: uninitialized urandom read (12 bytes read)
[ 9.384504] random: ssh-keygen: uninitialized urandom read (32 bytes read)
[ 9.839586] init: failsafe main process (724) killed by TERM signal
[ 14.865443] postgres (1245): /proc/1245/oom_adj is deprecated, please use /proc/1245/oom_score_adj instead.
[ 17.213418] random: crng init done
[ 17.580892] init: plymouth-upstart-bridge main process ended, respawning
>
> > >> But this may not a
> > >> clean solution. It'd help if I can get suggestions on what would be a
> > >> clean option to fix this without extensively changing the code in
> > >> virtio_net. Is it mandatory to protect the affinitization with
> > >> read_lock? I don't see similar lock in other drivers while setting the
> > >> affinity. I understand this warning should go away, but isn't it safe to
> > >> have multiple readers.
> > >>
> > >>> On Fri, Jun 29, 2018 at 09:27:07PM -0700, Amritha Nambiar wrote:
^ permalink raw reply
* Re: [PATCH v3 net-next 5/9] net: stmmac: Add MDIO related functions for XGMAC2
From: Florian Fainelli @ 2018-08-03 19:06 UTC (permalink / raw)
To: Jose Abreu, netdev
Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
Andrew Lunn
In-Reply-To: <53ac7d723d9d16a0b048433e85c2d7a8fafeef17.1533311285.git.joabreu@synopsys.com>
On 08/03/2018 08:50 AM, Jose Abreu wrote:
> Add the MDIO related funcionalities for the new IP block XGMAC2.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> ---
> +satic int stmmac_xgmac2_c22_format(struct stmmac_priv *priv, int phyaddr,
> + int phyreg, u32 *hw_addr)
> +{
> + unsigned int mii_data = priv->hw->mii.data;
> + u32 tmp;
> +
> + /* HW does not support C22 addr >= 4 */
> + if (phyaddr >= 4)
> + return -ENODEV;
It would be nice if this could be moved at probe time so you don't have
to wait until you connect to the PHY, read its PHY OUI and find out it
has a MDIO address >= 4. Not a blocker, but something that could be
improved further on.
In premise you could even scan the MDIO bus' device tree node, and find
that out ahead of time.
> + /* Wait until any existing MII operation is complete */
> + if (readl_poll_timeout(priv->ioaddr + mii_data, tmp,
> + !(tmp & MII_XGMAC_BUSY), 100, 10000))
> + return -EBUSY;
> +
> + /* Set port as Clause 22 */
> + tmp = readl(priv->ioaddr + XGMAC_MDIO_C22P);
> + tmp |= BIT(phyaddr);
Since the registers are being Read/Modify/Write here, don't you need to
clear the previous address bits as well?
You probably did not encounter any problems in your testing if you had
only one PHY on the MDIO bus, but this is not something that is
necessarily true, e.g: if you have an Ethernet switch, several MDIO bus
addresses are going to be responding.
Your MDIO bus implementation must be able to support one transaction
with one PHY address and the next transaction with another PHY address ,
etc...
That is something that should be easy to fix and be resubmitted as part
of v4.
--
Florian
^ permalink raw reply
* Re: [PATCH v3 net-next 3/9] net: stmmac: Add DMA related callbacks for XGMAC2
From: Florian Fainelli @ 2018-08-03 18:58 UTC (permalink / raw)
To: Jose Abreu, netdev
Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <dfb48f95fffd5929e09186243445db6746e31bf5.1533311285.git.joabreu@synopsys.com>
On 08/03/2018 08:50 AM, Jose Abreu wrote:
> Add the DMA related callbacks for the new IP block XGMAC2.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> ---
> + value &= ~XGMAC_RD_OSR_LMT;
> + value |= (axi->axi_rd_osr_lmt << XGMAC_RD_OSR_LMT_SHIFT) &
> + XGMAC_RD_OSR_LMT;
> +
> + for (i = 0; i < AXI_BLEN; i++) {
> + if (axi->axi_blen[i])
> + value &= ~XGMAC_UNDEF;
Should not you be you clearing all XGMAC_BLEN* values since you do a
logical or here? I am assuming this is not something that would likely
change from one open/close but still?
--
Florian
^ permalink raw reply
* Re: [PATCH v3 net-next 1/9] net: stmmac: Add XGMAC 2.10 HWIF entry
From: Florian Fainelli @ 2018-08-03 18:54 UTC (permalink / raw)
To: Jose Abreu, netdev
Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <5ff6dcc508726dec7d170315e2e3728667b6133f.1533311285.git.joabreu@synopsys.com>
On 08/03/2018 08:50 AM, Jose Abreu wrote:
> Add a new entry to HWIF table for XGMAC 2.10. For now we fill it with
> empty callbacks which will be added in posterior patches.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/common.h | 14 +++++++------
> drivers/net/ethernet/stmicro/stmmac/hwif.c | 31 ++++++++++++++++++++++++++--
> include/linux/stmmac.h | 1 +
> 3 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 78fd0f8b8e81..3fb81acbd274 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -36,12 +36,14 @@
> #include "mmc.h"
>
> /* Synopsys Core versions */
> -#define DWMAC_CORE_3_40 0x34
> -#define DWMAC_CORE_3_50 0x35
> -#define DWMAC_CORE_4_00 0x40
> -#define DWMAC_CORE_4_10 0x41
> -#define DWMAC_CORE_5_00 0x50
> -#define DWMAC_CORE_5_10 0x51
> +#define DWMAC_CORE_3_40 0x34
> +#define DWMAC_CORE_3_50 0x35
> +#define DWMAC_CORE_4_00 0x40
> +#define DWMAC_CORE_4_10 0x41
> +#define DWMAC_CORE_5_00 0x50
> +#define DWMAC_CORE_5_10 0x51
> +#define DWXGMAC_CORE_2_10 0x21
> +
> #define STMMAC_CHAN0 0 /* Always supported and default for all chips */
>
> /* These need to be power of two, and >= 4 */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> index 1f50e83cafb2..24f5ff175aa4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> @@ -72,6 +72,7 @@ static int stmmac_dwmac4_quirks(struct stmmac_priv *priv)
> static const struct stmmac_hwif_entry {
> bool gmac;
> bool gmac4;
> + bool xgmac;
> u32 min_id;
> const struct stmmac_regs_off regs;
> const void *desc;
> @@ -87,6 +88,7 @@ static const struct stmmac_hwif_entry {
> {
> .gmac = false,
> .gmac4 = false,
> + .xgmac = false,
In a future clean-up you would like want to remove this and replace this
an enumeration which is less error prone than having to define a boolean
for each of these previous generations only to say "this is not an xgmac".
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: Doron Roberts-Kedes @ 2018-08-03 18:49 UTC (permalink / raw)
To: Vakul Garg
Cc: David S . Miller, Dave Watson, Boris Pismenny, Aviad Yehezkel,
netdev@vger.kernel.org
In-Reply-To: <DB7PR04MB42528525C8E8B39AF3FB65E48B230@DB7PR04MB4252.eurprd04.prod.outlook.com>
On Fri, Aug 03, 2018 at 01:23:33AM +0000, Vakul Garg wrote:
>
>
> > -----Original Message-----
> > From: Doron Roberts-Kedes [mailto:doronrk@fb.com]
> > Sent: Friday, August 3, 2018 6:00 AM
> > To: David S . Miller <davem@davemloft.net>
> > Cc: Dave Watson <davejwatson@fb.com>; Vakul Garg
> > <vakul.garg@nxp.com>; Boris Pismenny <borisp@mellanox.com>; Aviad
> > Yehezkel <aviadye@mellanox.com>; netdev@vger.kernel.org; Doron
> > Roberts-Kedes <doronrk@fb.com>
> > Subject: [PATCH net-next] net/tls: Calculate nsg for zerocopy path without
> > skb_cow_data.
> >
> > decrypt_skb fails if the number of sg elements required to map is greater
> > than MAX_SKB_FRAGS. As noted by Vakul Garg, nsg must always be
> > calculated, but skb_cow_data adds unnecessary memcpy's for the zerocopy
> > case.
> >
> > The new function skb_nsg calculates the number of scatterlist elements
> > required to map the skb without the extra overhead of skb_cow_data. This
> > function mimics the structure of skb_to_sgvec.
> >
> > Fixes: c46234ebb4d1 ("tls: RX path for ktls")
> > Signed-off-by: Doron Roberts-Kedes <doronrk@fb.com>
> > ---
> > net/tls/tls_sw.c | 89
> > ++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 86 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
> > ff3a6904a722..c62793601cfc 100644
> > --- a/net/tls/tls_sw.c
> > +++ b/net/tls/tls_sw.c
> > @@ -43,6 +43,76 @@
> >
> > #define MAX_IV_SIZE TLS_CIPHER_AES_GCM_128_IV_SIZE
> >
> > +static int __skb_nsg(struct sk_buff *skb, int offset, int len,
> > + unsigned int recursion_level)
> > +{
> > + int start = skb_headlen(skb);
> > + int i, copy = start - offset;
> > + struct sk_buff *frag_iter;
> > + int elt = 0;
> > +
> > + if (unlikely(recursion_level >= 24))
> > + return -EMSGSIZE;
> > +
> > + if (copy > 0) {
> > + if (copy > len)
> > + copy = len;
> > + elt++;
> > + if ((len -= copy) == 0)
> > + return elt;
> > + offset += copy;
> > + }
> > +
> > + for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> > + int end;
> > +
> > + WARN_ON(start > offset + len);
> > +
> > + end = start + skb_frag_size(&skb_shinfo(skb)->frags[i]);
> > + if ((copy = end - offset) > 0) {
> > + if (copy > len)
> > + copy = len;
> > + elt++;
> > + if (!(len -= copy))
> > + return elt;
> > + offset += copy;
> > + }
> > + start = end;
> > + }
> > +
> > + skb_walk_frags(skb, frag_iter) {
> > + int end, ret;
> > +
> > + WARN_ON(start > offset + len);
> > +
> > + end = start + frag_iter->len;
> > + if ((copy = end - offset) > 0) {
> > +
> > + if (copy > len)
> > + copy = len;
> > + ret = __skb_nsg(frag_iter, offset - start, copy,
> > + recursion_level + 1);
> > + if (unlikely(ret < 0))
> > + return ret;
> > + elt += ret;
> > + if ((len -= copy) == 0)
> > + return elt;
> > + offset += copy;
> > + }
> > + start = end;
> > + }
> > + BUG_ON(len);
> > + return elt;
> > +}
> > +
> > +/* Return the number of scatterlist elements required to completely map
> > +the
> > + * skb, or -EMSGSIZE if the recursion depth is exceeded.
> > + */
> > +static int skb_nsg(struct sk_buff *skb, int offset, int len) {
> > + return __skb_nsg(skb, offset, len, 0); }
> > +
>
> These is generic function and useful elsewhere too.
> Should the above two functions be exported by skbuff.c?
True. Perhaps it can move into skbuff.c if/when there is a second
use case for it.
>
> > static int tls_do_decryption(struct sock *sk,
> > struct scatterlist *sgin,
> > struct scatterlist *sgout,
> > @@ -693,7 +763,7 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
> > struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
> > struct scatterlist *sgin = &sgin_arr[0];
> > struct strp_msg *rxm = strp_msg(skb);
> > - int ret, nsg = ARRAY_SIZE(sgin_arr);
> > + int ret, nsg;
> > struct sk_buff *unused;
> >
> > ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE, @@ -
> > 704,10 +774,23 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
> >
> > memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> > if (!sgout) {
> > - nsg = skb_cow_data(skb, 0, &unused) + 1;
> > + nsg = skb_cow_data(skb, 0, &unused);
> > + } else {
> > + nsg = skb_nsg(skb,
> > + rxm->offset + tls_ctx->rx.prepend_size,
> > + rxm->full_len - tls_ctx->rx.prepend_size);
> > + if (nsg <= 0)
> > + return nsg;
> Comparison should be (nsg < 1). TLS forbids '0' sized records.
Yes true, v2 incoming
>
> > + }
> > +
> > + // We need one extra for ctx->rx_aad_ciphertext
> > + nsg++;
> > +
> > + if (nsg > ARRAY_SIZE(sgin_arr))
> > sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
>
> Add check for kmalloc_array returnining NULL.
Yes true, v2 incoming.
>
> > +
> > + if (!sgout)
> > sgout = sgin;
> > - }
> >
> > sg_init_table(sgin, nsg);
> > sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
> > --
> > 2.17.1
>
^ 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