Hello,
I would go for NET_RLOCK() instead of NET_LOCK(), which is currently alias
to NET_WLOCK(). My point is the icmp6_reflect(), which is the workhorse for
icmp6_error(), is a typical 'READER-user' of network stack. It does not
change any network configuration. So it should be fine to let it run
as a reader.
I'm sure mpi@ can provide authoritative OK here as nothing should go wrong
with pure NET_LOCK() you have in your diff.
regards
sasha
On Tue, Nov 14, 2017 at 01:44:18PM +0100, Alexander Bluhm wrote:
> Hi,
>
> On my regress test machine I see this trace:
>
> splassert: rt_match: want 2 have 0
> Starting stack trace...
> rt_match(2,0,d0bb032e,604a56c3) at rt_match+0x45
> rt_match(f5475540,0,0,0) at rt_match+0x45
> rtalloc(f5475540,0,0) at rtalloc+0x13
> icmp6_reflect(d818de00,28) at icmp6_reflect+0x1f7
> icmp6_error(d8181900,3,1,0) at icmp6_error+0x39a
> frag6_freef(d717b0b4) at frag6_freef+0xb0
> frag6_slowtimo() at frag6_slowtimo+0x24c
> pfslowtimo(d0c4cf78) at pfslowtimo+0x40
> softclock_thread(d77f5590) at softclock_thread+0xc2
> End of stack trace.
>
> It is triggered by regress/sys/netinet6/frag6. A net lock is missing
> when we send the icmp6 reply.
>
> ok?
>
> bluhm
>
> Index: netinet6/frag6.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/frag6.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 frag6.c
> --- netinet6/frag6.c 13 Nov 2017 07:16:35 -0000 1.80
> +++ netinet6/frag6.c 14 Nov 2017 12:00:37 -0000
> @@ -593,8 +593,12 @@ frag6_slowtimo(void)
>
> mtx_leave(&frag6_mutex);
>
> - while ((q6 = TAILQ_FIRST(&rmq6)) != NULL) {
> - TAILQ_REMOVE(&rmq6, q6, ip6q_queue);
> - frag6_freef(q6);
> + if (!TAILQ_EMPTY(&rmq6)) {
> + NET_LOCK();
> + while ((q6 = TAILQ_FIRST(&rmq6)) != NULL) {
> + TAILQ_REMOVE(&rmq6, q6, ip6q_queue);
> + frag6_freef(q6);
> + }
> + NET_UNLOCK();
> }
> }
>
