Re: [netmp] socket accesses

[Aggelos Economopoulos <aoiko@xxxxxxxxxxxxxx>]

On Saturday 09 August 2008, Matthew Dillon wrote:
[...]
>     Ok, here are my suggestions:
[...]
I think I'm going to postpone answering to your suggestions until I start
writing the relevant code (that would be ASAP), so that we can talk specifics.

> :WRT SS_NOFDREF:
> :
> :- it is set on error by socreate(), but at this point the socket is not
> :  reachable yet
> :...
> :So, unless I'm missing something, SS_NOFDREF is only modified before the socket
> :becomes relevant or after the socket is no longer reachable.
> 
>     Not sure about this one.

I'm going to recheck that.

> :Moving to so_incomp:
> :
> :it's modified by sofree() (only called by protocol thread), sonewconn (same) and
> :soisconnected(). The latter is called from process context (fifo_open,
> :portal_connect at least, maybe sctp too) and from the bluetooth netisr.
> :
> :->so_comp is modified by soclose (called from process context), soisconnected,
> :sonewconn (see above), soaccept_predicate, sctp_get_peeloff (of course!),
> :ng_ksocket_finish_accept().
> :
> :Assuming we can move all list modification in protocol thread context, the
> :list traversal would still be tricky. Maybe a spinlock protecting the lists and
> :queue lengths is in order for now? The same lock could be reused for protecting
> :SS_INCOMP, SS_COMP. In the future we might try something more clever if this
> :becomes a performance issue. Opinions?
[...]
>     I think so_incomp is just used on listening sockets, right?  Protecting
>     it with a spinlock is probably just fine.

For now :)

>     The so_state flags themselves would have to be set and cleared with
>     atomic ops.  Any situation which requires notifying the protocol stack,
>     such as when userland sets CANTRCVMORE, can be done with a 'checkstate'
>     style message to the protocol stack.  The message would not contain the
>     flag, userland would simply set the flag and then tell the protocol
>     stack to check its flags state and do whatever needs to be done.

Not sure how to decide "what needs to be done".

>     How about read()/write()/send()/recv()/etc?  That is, so_rcvbuf and
>     so_sndbuf operation?  I'm thinking the mbuf chains are going to have
>     to be protected with a lock of some sort.  It seems like it should work,
>     except when an mbuf must be split up... I think even that could be made
>     to work if the new mbuf is pre-allocated/pre-cached so the split can
>     occur inside the spinlock protection.

Quoting my self:
> If you haven't been paying attention, see here too
>
> http://wiki.dragonflybsd.org/index.cgi/NetMP

Which will point you to
http://leaf.dragonflybsd.org/mailarchive/kernel/2008-05/msg00003.html
and
http://leaf.dragonflybsd.org/mailarchive/kernel/2008-06/msg00085.html

You can use the web interface at
http://repo.or.cz/w/dragonfly/netmp.git?a=shortlog;h=refs/heads/sockbuf
to browse the code or try

git clone git://repo.or.cz/dragonfly/netmp.git
cd netmp
git branch sockbuf origin/sockbuf
git checkout sockbuf

to get an on-disk copy. The code needs a simplification pass, but mostly
works now (known issues at http://wiki.dragonflybsd.org/index.cgi/NetMP).
That should take care of the so_rcvbuf, so_sndbuf issues.

Aggelos