On Wed, Oct 12, 2016 at 11:42:38AM +0200, Julien Charbon wrote:
> On 10/12/16 11:29 AM, Slawa Olhovchenkov wrote:
> > On Wed, Oct 12, 2016 at 11:19:48AM +0200, Julien Charbon wrote:
> >
> >>> if INP_WLOCK is like spinlock -- this is dead lock.
> >>> if INP_WLOCK is like mutex -- thread1 resheduled.
> >>
> >> Thanks, I understand you question now. No an interrupt cannot
bypass a
> >> lock: Here INP_WLOCK is like mutex -- thread1 resheduled.
> >
> > Thanks, nice.
> >
> >>>>> 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()
> >>>>
> >>>> Exactly, thus the current fix is: If you already have
the INP_DROPPED
> >>>> flag set you are not allowed to call tcp_twstart(),
actually it is a
> >>>> good candidate for a new INVARIANT. Let me add that.
> >>>>
> >>>>> 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()?
> >>>>
> >>>> If you look carefully INP_WLOCK is used in cxgb_cpl_io.c
and
> >>>> t4_cpl_io.c before calling tcp_twstart().
> >>>
> >>> Yes, and you remeber: sys/netinet/tcp_subr.c
> >>>
> >>> 1535 struct tcpcb *
> >>> 1536 tcp_close(struct tcpcb *tp)
> >>> 1537 {
> >>> ...
> >>> 1569 INP_WUNLOCK(inp);
> >>> 1570 ACCEPT_LOCK();
> >>> 1571 SOCK_LOCK(so);
> >>> 1572 so->so_state &= ~SS_PROTOREF;
> >>> 1573 sofree(so);
> >>> 1574 return (NULL);
> >>>
> >>> sofree() call tcp_usr_detach() and in tcp_usr_detach() we have
> >>> unexpected INP_TIMEWAIT.
> >>
> >> I see, thus just for the context: The TCP stack in sys/dev/cxgb*
is a
> >> TOE (TCP Offload Engine?) TCP stack for Chelsio NICs, it is a
> >> separate/side TCP stack that is used only with TCP_OFFLOAD option.
> >>
> >> This TOE TCP stack actually has its own set of detach()/input()
> >> functions and seems to check INP_DROPPED flag properly. I guess
@np
> >> check fixes in socket TCP stack and decides which one can also
impact
> >> the Chelsio TOE TCP stack. Some bugs are only in socket TCP
stack, some
> >> are only in TOE TCP stack.
> >
> > I am fear about other direction -- setting INP_TIMEWAIT in Chelsio TOE
> > TCP stack and impact this to
> > tcp_timer_2msl()/tcp_close()/sofree()/tcp_usr_detach() path.
>
> I see, I expect no problem on this side as tcp_timer_2msl() checks the
> INP_TIMEWAIT flag and do not call tcp_close() if set.
I am about case when at time of first INP_WUNLOCK() tcp_timer_2msl()
don't see INP_TIMEWAIT, call tcp_close(), tcp_close() do INP_WUNLOCK()
and now Chelsio TOE take INP_WLOCK, do tcp_twstart() and set
INP_TIMEWAIT. After this tcp_timer_2msl resume and have unexpected
INP_TIMEWAIT in tcp_usr_detach().