Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Dec 2012 18:02:12 -0500
From:      Karim Fodil-Lemelin <fodillemlinkarim@gmail.com>
To:        Vijay Singh <vijju.singh@gmail.com>
Cc:        freebsd-net@freebsd.org, Navdeep Parhar <nparhar@gmail.com>
Subject:   Re: use of V_tcbinfo lock for TCP syncache
Message-ID:  <50D24774.80800@gmail.com>
In-Reply-To: <CALCNsJS8j0wvv4dc1Q6rLckmr0RE%2BgW0%2BmUDdmSvyfjXzKugsQ@mail.gmail.com>
References:  <CALCNsJRFyQ9ZmfJ3quX2-cUuFHjs2rXw63Tq5ZH-uP1%2BoQmjLw@mail.gmail.com> <50D218BA.7080301@FreeBSD.org> <50D21C3B.8020803@gmail.com> <CALCNsJS8j0wvv4dc1Q6rLckmr0RE%2BgW0%2BmUDdmSvyfjXzKugsQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 19/12/2012 4:01 PM, Vijay Singh wrote:
>>> Holding the pcbinfo lock prevents races between syncache_socket() and
>>> accept().  See rwatson's comment just above tcp_usr_accept.  I noted
>>> this too (the comment above tod->tod_offload_socket() in tcp_syncache.c)
>>> back when I updated the TOE code in the kernel.
>> er, I think I told you why tcp_usr_accept holds the pcbinfo lock, which
>> wasn't your original question... :-)
> But it helped.
>
> So I am thinking about trying a change where syncache_socket() would
> call soalloc() first, get a socket, setup the inp, and then do a
> (modified) sonewconn to place the socket in the listener's queue.
> Robert's comment indicated that this would be a better way to
> eliminate the race since we wouldn't need the pcblock when we make the
> sonewconn call.
>
>
Sure but syncache_expand() is entered with the tcbinfo already write 
locked which also protects the unlocking of the listening connection and 
the locking of the newly created socket. Around this part:

             /*
              * Socket is created in state SYN_RECEIVED.
              * Unlock the listen socket, lock the newly
              * created socket and update the tp variable.
              */
             INP_WUNLOCK(inp);    /* listen socket */
             inp = sotoinpcb(so);
             INP_WLOCK(inp);        /* new connection */
             tp = intotcpcb(inp);

  Without the tcbinfo lock the new socket could be closed (getting a 
reset) which would put it in INP_TIMEWAIT or INP_DROPPED _after_ the 
check is made in tcp_usr_accept since there is a period of time where 
tcbinfo is not locked and the new socket inp is not locked either.

I could be wrong but it seems that without the tcbinfo lock a lot could 
happen between the unlocking of the listen socket and the locking of the 
new one.

Karim.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50D24774.80800>