netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: William Allen Simpson <william.allen.simpson@gmail.com>
To: Linux Kernel Network Developers <netdev@vger.kernel.org>
Subject: Re: [net-next-2.6 PATCH 1/4 revised] TCPCT part 1a: extend struct tcp_request_sock
Date: Sun, 18 Oct 2009 11:57:52 -0400	[thread overview]
Message-ID: <4ADB3B00.7040409@gmail.com> (raw)
In-Reply-To: <4AD8AFC0.1090101@gmail.com>

William Allen Simpson wrote:
> Pass additional parameters associated with sending SYNACK.  This
> is not as straightforward or architecturally clean as previously
> proposed, and has the unfortunate side effect of potentially
> including otherwise unneeded headers for related protocols, but
> that problem will affect very few files.
> ---
>  include/net/extend_request_sock.h |   37 
> +++++++++++++++++++++++++++++++++++++
>  1 files changed, 37 insertions(+), 0 deletions(-)
>  create mode 100644 include/net/extend_request_sock.h
> 
This technique appears to be unworkable:

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9971870..30c4808 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -71,6 +71,7 @@
  #include <net/timewait_sock.h>
  #include <net/xfrm.h>
  #include <net/netdma.h>
+#include <net/extend_request_sock.h>

  #include <linux/inet.h>
  #include <linux/ipv6.h>
@@ -1195,6 +1196,15 @@ struct request_sock_ops tcp_request_sock_ops __read_mostly = {
  	.send_reset	=	tcp_v4_send_reset,
  };

+struct request_sock_ops tcp4_extend_request_sock_ops __read_mostly = {
+	.family		=	PF_INET,
+	.obj_size	=	sizeof(struct extend_request_sock),
+	.rtx_syn_ack	=	tcp_v4_send_synack,
+	.send_ack	=	tcp_v4_reqsk_send_ack,
+	.destructor	=	tcp_v4_reqsk_destructor,
+	.send_reset	=	tcp_v4_send_reset,
+};
+

...

+		req = inet_reqsk_alloc(&tcp4_extend_request_sock_ops);
+		if (NULL == req)
+			goto drop;
+

Many hours of investigation demonstrated that the obj_size isn't actually
used to allocate the structure.  Heck, it's not even checked to determine
whether there's enough room!  Instead, the kernel crashes later, as the
extended variables are accessed!

Returning to the architecturally clean parameters of the previous patch
series, that has the distinct advantage of actually working....

      reply	other threads:[~2009-10-18 15:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-16 17:39 [net-next-2.6 PATCH 1/4 revised] TCPCT part 1a: extend struct tcp_request_sock William Allen Simpson
2009-10-18 15:57 ` William Allen Simpson [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=4ADB3B00.7040409@gmail.com \
    --to=william.allen.simpson@gmail.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 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).