netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: therbert@google.com, netdev@vger.kernel.org
Subject: Re: [PATCH] net: Provide SYN packet for passive connections
Date: Mon, 12 Mar 2012 21:01:59 -0700	[thread overview]
Message-ID: <1331611319.7787.37.camel@edumazet-glaptop> (raw)
In-Reply-To: <20120311.211424.996831768685832926.davem@davemloft.net>

On Sun, 2012-03-11 at 21:14 -0700, David Miller wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Sun, 11 Mar 2012 21:08:38 -0700
> 
> > For inet_connection_sock I believe there are fields that would only be
> > used for a listeners(e.g. icsk_accept_queue), and fields that would
> > only be used for a real connection (e.g. icsk_retransmits).  Would it
> > be worth it to split these into a union?
> 
> Indeed, it might.

OK I took a look at this idea, and it seems safe right now.

[PATCH net-next] inet: embed icsk_accept_queue in an union

icsk_accept_queue is currently used only for LISTEN sockets. We could
share its space with fields used for other kind of inet sockets.

For active connections, this area is zeroed in socket setup, and for
passive ones, we clear the whole area in inet_csk_clone_lock() to make
sure we dont use content inherited from (listener) parent.

/* Deinitialize accept_queue to trap illegal accesses. */
memset(&newicsk->icsk_accept_queue, 0, sizeof(newicsk->icsk_accept_queue));

For 100% safety, we could do the same in inet_csk_listen_stop(), or we can
double check that fields added in this union later are only accessed by non
LISTEN sockets.

All these fields must have a null default value.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Suggested-by: Tom Herbert <therbert@google.com>
---
 include/net/inet_connection_sock.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index dbf9aab..d3ccaf9 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -86,7 +86,9 @@ struct inet_connection_sock_af_ops {
 struct inet_connection_sock {
 	/* inet_sock has to be the first member! */
 	struct inet_sock	  icsk_inet;
-	struct request_sock_queue icsk_accept_queue;
+	union {
+		struct request_sock_queue icsk_accept_queue;
+	};
 	struct inet_bind_bucket	  *icsk_bind_hash;
 	unsigned long		  icsk_timeout;
  	struct timer_list	  icsk_retransmit_timer;

  reply	other threads:[~2012-03-13  4:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-12  2:48 [PATCH] net: Provide SYN packet for passive connections Tom Herbert
2012-03-12  2:55 ` David Miller
2012-03-12  4:08   ` Tom Herbert
2012-03-12  4:14     ` David Miller
2012-03-13  4:01       ` Eric Dumazet [this message]
2012-03-13  5:56         ` David Miller
2012-03-12  3:10 ` Eric Dumazet
2012-03-12  3:18   ` Tom Herbert

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=1331611319.7787.37.camel@edumazet-glaptop \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=therbert@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).