diff mbox

[net-next-2.6,v6,4/7,RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's

Message ID 4B03FCE4.5020403@gmail.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

William Allen Simpson Nov. 18, 2009, 1:55 p.m. UTC 
Ilpo Järvinen wrote:
> You both are right (and that's what is causing confusion)...
> 
In this case, for /this/ code, *none* of you are correct, and _that's_
causing confusion.  There is no independent retransmission of SYNACK data!
None!!  Nada!!!  There is no retransmission queue for SYNACK data.

SYN with SACK should never be sent, and should never be received -- and
already should be discarded and ignored (outside the scope of this patch).

As I've already mentioned in this thread 2 days ago, in my earlier patches
(now deferred to part 2 after the functional split requested by Eric and
Ilpo), the request_sock was removed entirely (just like syncookies).
There wasn't (and won't be) any struct or timer lying around to allow
SYNACK data retransmissions to occur.

Even now, the entire s_data_* edifice isn't passed to be retransmitted.
Note the code (in part 1a), already reviewed and Ack'd:

See that NULL?  There's no cookie data structure for retransmission!

Could we end this diversion into rampant speculation?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ilpo Järvinen Nov. 18, 2009, 2:08 p.m. UTC | #1 
On Wed, 18 Nov 2009, William Allen Simpson wrote:

> Ilpo Järvinen wrote:
> > You both are right (and that's what is causing confusion)...
> > 
> In this case, for /this/ code, *none* of you are correct, and _that's_
> causing confusion. There is no independent retransmission of SYNACK data!
> None!!  Nada!!!  There is no retransmission queue for SYNACK data.
> 
> SYN with SACK should never be sent, and should never be received -- and
> already should be discarded and ignored (outside the scope of this patch).
> 
> As I've already mentioned in this thread 2 days ago, in my earlier patches
> (now deferred to part 2 after the functional split requested by Eric and
> Ilpo), the request_sock was removed entirely (just like syncookies).
> There wasn't (and won't be) any struct or timer lying around to allow
> SYNACK data retransmissions to occur.
> 
> Even now, the entire s_data_* edifice isn't passed to be retransmitted.
> Note the code (in part 1a), already reviewed and Ack'd:
> 
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 4be2228..7a42990 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -537,7 +537,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff
> *skb,
>  		 * Enforce "SYN-ACK" according to figure 8, figure 6
>  		 * of RFC793, fixed by RFC1122.
>  		 */
> -		req->rsk_ops->rtx_syn_ack(sk, req);
> +		req->rsk_ops->rtx_syn_ack(sk, req, NULL);
>  		return NULL;
>  	}
> 
> See that NULL?  There's no cookie data structure for retransmission!
> 
> Could we end this diversion into rampant speculation?

...I was just commenting on the disagreement in the interpretation of what 
DaveM said vs comments you mentioned... :-)
diff mbox

Patch

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 4be2228..7a42990 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -537,7 +537,7 @@  struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
  		 * Enforce "SYN-ACK" according to figure 8, figure 6
  		 * of RFC793, fixed by RFC1122.
  		 */
-		req->rsk_ops->rtx_syn_ack(sk, req);
+		req->rsk_ops->rtx_syn_ack(sk, req, NULL);
  		return NULL;
  	}