Re: 11.0 stuck on high network load

2016-10-11 Thread Slawa Olhovchenkov
On Tue, Oct 11, 2016 at 09:20:17AM +0200, Julien Charbon wrote:

>  Then threads are competing for the INP_WLOCK lock.  For the example,
> let's say the thread A wants to run tcp_input()/in_pcblookup_mbuf() and
> racing for this INP_WLOCK:
> 
> https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/in_pcb.c#L1964
> 
>  And thread B wants to run tcp_timer_2msl()/tcp_close()/in_pcbdrop() and
> racing for this INP_WLOCK:
> 
> https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/tcp_timer.c#L323
> 
>  That leads to two cases:
> 
>  o Thread A wins the race:
> 
>   Thread A will continue tcp_input() as usal and INP_DROPPED flags is
> not set and inp is still in TCP hash table.
>   Thread B is waiting on thread A to release INP_WLOCK after finishing
> tcp_input() processing, and thread B will continue
> tcp_timer_2msl()/tcp_close()/in_pcbdrop() processing.
> 
>  o Thread B wins the race:
> 
>   Thread B runs tcp_timer_2msl()/tcp_close()/in_pcbdrop() and inp
> INP_DROPPED is set and inp being removed from TCP hash table.
>   In parallel, thread A has found the inp in TCP hash before is was
> removed, and waiting on the found inp INP_WLOCK lock.
>   Once thread B has released the INP_WLOCK lock, thread A gets this lock
> and sees the INP_DROPPED flag and do "goto findpcb" but here because the
> inp is not more in TCP hash table and it will not be find again by
> in_pcblookup_mbuf().
> 
>  Hopefully I am clear enough here.

Thanks, very clear.
Small qeustion: when both thread run on same CPU core, INP_WLOCK will
be re-schedule?

As I remeber race created by call tcp_twstart() at time of end
tcp_close(), at path sofree()-tcp_usr_detach() and unexpected
INP_TIMEWAIT state in the tcp_usr_detach(). INP_TIMEWAIT set in tcp_twstart()

After check source code I am found invocation of tcp_twstart() in
sys/netinet/tcp_stacks/fastpath.c, sys/netinet/tcp_input.c,
sys/dev/cxgb/ulp/tom/cxgb_cpl_io.c, sys/dev/cxgbe/tom/t4_cpl_io.c.

Invocation from sys/netinet/tcp_stacks/fastpath.c and
sys/netinet/tcp_input.c guarded by INP_WLOCK in tcp_input(), and now
will be OK.

Invocation from sys/dev/cxgb/ulp/tom/cxgb_cpl_io.c and
sys/dev/cxgbe/tom/t4_cpl_io.c is not clear to me, I am see independed
INP_WLOCK. Is this OK?

Can be thread A wants do_peer_close() directed from chelsio IRQ
handler, bypass tcp_input()?
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: aibs(4) / atk0110 support for newer systems

2016-10-11 Thread Andriy Gapon
On 06/10/2016 00:37, Andriy Gapon wrote:
> On 05/10/2016 23:28, Torfinn Ingolfsen wrote:
>> #6  0x80cd0081 in calltrap ()
>> at /usr/src/sys/amd64/amd64/exception.S:238
>> #7  0x81bcb078 in aibs_add_sensor () from /boot/kernel/aibs.ko
>> #8  0x81bcb4b4 in aibs_attach_sif () from /boot/kernel/aibs.ko
> 
> Argh, I've just spotted a very silly typo.
> Could you please replace '0' with 'o' in
>   err = aibs_add_sensor(sc, 0, [i], );
> ?

Ping.


-- 
Andriy Gapon
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: fix for use-after-free problem in 10.x

2016-10-11 Thread Pintér , Olivér
On Mon, Oct 10, 2016 at 7:07 AM, Julian Elischer  wrote:

> On 8/10/2016 5:36 AM, Oliver Pinter wrote:
>
>> On 10/5/16, Julian Elischer  wrote:
>>
>>> In 11 and 12 the taskqueue code has been rewritten in this area but
>>> under 10 this bug still occurs.
>>>
>>> On our appliances this bug stops the system from mounting the ZFS
>>> root, so it is quite severe.
>>> Basically while the thread is sleeping during the ZFS mount of root
>>> (in the while loop), another thread can free the 'task' item it is
>>> checking in that while loop and it can be reused or filled with
>>> 'deadcode' etc., with the waiting code unaware of the change.. The fix
>>> is to refetch the item at the end of the queue each time around the loop.
>>> I don't really want to do the bigger change of MFCing the change in
>>> 11, as it is more extensive, though if someone else does, that's ok by
>>> me. (If it's ABI compatible)
>>>
>>> Any comments or suggestions?
>>>
>> Yes, please commit them. This patch fixes the ZFS + GELI + INVARIANTS
>> problem for us.
>> There is the FreeBSD PR about the issue:
>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209580
>>
>
> I committed a slightly better version to stable/10
> should I ask for a merge to releng/10.3?


Yes, it would be really nice! Thanks your effort!



>
>
>
>
>
>> here's the fix in diff form:
>>>
>>>
>>> [robot@porridge /usr/src]$ p4 diff -du ...
>>> --- //depot/pbranches/jelischer/FreeBSD-PZ/10.3/sys/kern/subr_ta
>>> skqueue.c
>>> 2016-09-27 09:14:59.0 -0700
>>> +++ /usr/src/sys/kern/subr_taskqueue.c  2016-09-27 09:14:59.0
>>> -0700
>>> @@ -441,9 +441,10 @@
>>>
>>>   TQ_LOCK(queue);
>>>   task = STAILQ_LAST(>tq_queue, task, ta_link);
>>> -   if (task != NULL)
>>> -   while (task->ta_pending != 0)
>>> -   TQ_SLEEP(queue, task, >tq_mutex, PWAIT,
>>> "-",
>>> 0);
>>> +   while (task != NULL && task->ta_pending != 0) {
>>> +   TQ_SLEEP(queue, task, >tq_mutex, PWAIT, "-", 0);
>>> +   task = STAILQ_LAST(>tq_queue, task, ta_link);
>>> +   }
>>>   taskqueue_drain_running(queue);
>>>   KASSERT(STAILQ_EMPTY(>tq_queue),
>>>   ("taskqueue queue is not empty after draining"));
>>>
>>> ___
>>> freebsd-hack...@freebsd.org mailing list
>>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@f
>>> reebsd.org"
>>>
>>>
> ___
> freebsd-stable@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
>
___
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: possible regression on i386

2016-10-11 Thread Marek Zarychta
On Sun, Oct 09, 2016 at 08:11:55PM -0400, Nathan Lay wrote:
> If your problem is anything like mine was, buildworld is trying to link
> with /usr/lib/librt.so rather than the new one built during the buildworld
> build. As per a recent commit, the new librt will have the additional
> mq_getfd_np() symbol while the original /usr/lib/librt.so will not. That
> causes those unresolved reference errors for new code trying to use the
> mq_getfd_np() function.
> 
> Try building and installing librt manually:
> cd /usr/src/lib/librt
> make && make install
> 
> Then try buildworld again.
> 
Thank you for reply and advice. It was, of course my fault, 
/etc/src.conf file was left unchanged since upgrade from 9.3 and 
setting "CC=clang" was causing compilation errors.
The first impression is very important: FreeBSD 11 runs pretty smoothly 
on this old machine being also more lightweight than 9.3.

-- 

Marek Zarychta


signature.asc
Description: PGP signature


Re: 11.0 stuck on high network load

2016-10-11 Thread Julien Charbon

 Hi Slawa,

On 10/10/16 7:35 PM, Slawa Olhovchenkov wrote:
> On Mon, Oct 10, 2016 at 05:44:21PM +0200, Julien Charbon wrote:
 can check the current other usages of goto findpcb in tcp_input().  The
 rational here being:

  - Behavior before the patch:  If the inp we found was deleted then goto
 findpcb.
  - Behavior after the patch:  If the inp we found was deleted or dropped
 then goto findpcb.

  I just prefer having the same behavior applied everywhere:  If
 tcp_input() loses the inp lock race and the inp was deleted or dropped
 then retry to find a new inpcb to deliver to.

  But you are right dropping the packet here will also fix the issue.

  Then the review process becomes quite helpful because people can argue:
  Dropping here is better because "blah", or goto findpcb is better
 because "bluh", etc.  And at the review end you have a nice final patch.

 https://reviews.freebsd.org/D8211
>>>
>>> I am not sure, I am see to
>>>
>>> sys/netinet/in_pcb.h:#defineINP_DROPPED 0x0400 /*
>> protocol drop flag */
>>>
>>> and think this is a flag 'all packets must be droped'
>>
>>  Hm, I believe this flag means "this inp has been dropped by the TCP
>> stack, so don't use it anymore".  Actually this flag is better described
>> in the function that sets it:
>>
>> "(INP_DROPPED) is used by TCP to mark an inpcb as unused and avoid
>> future packet delivery or event notification when a socket remains open
>> but TCP has closed."
>>
>> https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/in_pcb.c#L1320
>>
>> /*
>>  * in_pcbdrop() removes an inpcb from hashed lists, releasing its
>> address and
>>  * port reservation, and preventing it from being returned by inpcb lookups.
>>  *
>>  * It is used by TCP to mark an inpcb as unused and avoid future packet
>>  * delivery or event notification when a socket remains open but TCP has
>>  * closed.  This might occur as a result of a shutdown()-initiated TCP close
>>  * or a RST on the wire, and allows the port binding to be reused while
>> still
>>  * maintaining the invariant that so_pcb always points to a valid inpcb
>> until
>>  * in_pcbdetach().
>>  *
>>  */
>> void
>> in_pcbdrop(struct inpcb *inp)
>> {
>>   inp->inp_flags |= INP_DROPPED;
>>   ...
>>
>>  The classical example where "goto findpcb" is useful:  You receive a
>> new connection request with a TCP SYN packet and this packet is unlucky
>> and reached a inp being dropped:
>>
>>  - with "goto findpcb" approach, the next lookup will most likely find
>> the LISTEN inp and start the TCP hand-shake as usual
>>  - with "drop the packet" approach, the TCP client will need to
>> re-transmit a TCP SYN packet
>>
>>  It is not because a packet was unlucky once that it deserves to be
>> dropped. :)
> 
> Thanks for explaining, very helpfull.
> In this situation (TCP SYN with same 4-tuple as existing socket)
> allocate new PCB is best. But for this we must destroy current PCB. I
> am think INP_WUNLOCK(inp) don't destroy it and in_pcblookup_mbuf find
> it again (I am think in_pcblookup_mbuf find this PCB on first turn).
> I am assume for classical example in_pcbrele_wlocked(inp) free and
> destroy current PCB for possibility in_pcblookup_mbuf allocate new
> one.

 Astute question:  Here, the same inp cannot be find again by
in_pcblookup_mbuf(), the explanation is a bit long though:

in_pcbdrop() does two things under INP_WLOCK lock:
https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/in_pcb.c#L1334

1. Add INP_DROPPED flag
2. Remove the inp from the TCP hash table

 And once removed from the TCP hash table, in_pcblookup_mbuf() will
return NULL when doing the same TCP hash table lookup again.

 It means that under a INP_WLOCK lock, these two changes are atomic:

 - Either an inp does not have INP_DROPPED flag and can be in TCP hash table
 - Either an inp has INP_DROPPED flag and is not in TCP hash table

 But when you don't have the INP_WLOCK lock, then you can witness the
intermediate state where a inp is still in TCP hash table while a thread
is adding the INP_DROPPED flag.

 Nothing unusual here.

 Then threads are competing for the INP_WLOCK lock.  For the example,
let's say the thread A wants to run tcp_input()/in_pcblookup_mbuf() and
racing for this INP_WLOCK:

https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/in_pcb.c#L1964

 And thread B wants to run tcp_timer_2msl()/tcp_close()/in_pcbdrop() and
racing for this INP_WLOCK:

https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/tcp_timer.c#L323

 That leads to two cases:

 o Thread A wins the race:

  Thread A will continue tcp_input() as usal and INP_DROPPED flags is
not set and inp is still in TCP hash table.
  Thread B is waiting on thread A to release INP_WLOCK after finishing
tcp_input() processing, and thread B will continue
tcp_timer_2msl()/tcp_close()/in_pcbdrop() processing.

 o Thread B wins the race: