All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick McManus <mcmanus@ducksong.com>
To: Eric Dumazet <dada1@cosmosbay.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: RFC [PATCH 3/3] TCP_DEFER_ACCEPT updates: more accurate timers and	resets
Date: Sat, 01 Mar 2008 15:59:02 -0500	[thread overview]
Message-ID: <1204405142.5792.26.camel@tng> (raw)
In-Reply-To: <47C935BC.9060409@cosmosbay.com>


On Sat, 2008-03-01 at 11:53 +0100, Eric Dumazet wrote:
> Patrick McManus a écrit :
> > Signed-off-by: Patrick McManus <mcmanus@ducksong.com>
> > 
> >   Change TCP_DEFER_ACCEPT implementation so that it transitions a
> >     connection to ESTABLISHED after handshake is complete instead of
> >     leaving it in SYN-RECV until some data arrvies. Place connection in
> >     accept queue when first data packet arrives from slow path.
> >
> This all makes sense Patrick.
> 
> Your patch is quite large and difficult to review (for me :) )
> 

Eric, thanks for the feedback!

> 1) Adding "struct tcp_deferred_accept_info" on "struct tcp_sock" (24 bytes on 
> 64 bit arches) is a rather high cost to pay for an obscure TCP_DEFER_ACCEPT.
> 
> But then, many "struct tcp_sock" fields are used only at socket establishment.
> 

It occurs to me the child socket pointer in that struct is superfulous,
so I can cut it down to two ptrs instead of three.. I'll include that in
a respin - and as you point out struct tcp_sock is already full of stuff
that is really only used once.

> 2) About MAX_TCP_ACCEPT_DEFERRED test in do_tcp_setsockopt(), I am not sure we 
> can return -EINVAL.
> 

> setsockopt(TCP_DEFER_ACCEPT, 100000) is a hint given by application, and could 
> be mapped to setsockopt(TCP_DEFER_ACCEPT, 65535) silently.

good point, I agree. I'll include that change in the respin too.


-Patrick



      reply	other threads:[~2008-03-01 20:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-27  1:49 RFC [PATCH 3/3] TCP_DEFER_ACCEPT updates: more accurate timers and resets Patrick McManus
2008-03-01 10:53 ` Eric Dumazet
2008-03-01 20:59   ` Patrick McManus [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1204405142.5792.26.camel@tng \
    --to=mcmanus@ducksong.com \
    --cc=dada1@cosmosbay.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.