netdev - Re: [PATCH v2 3/7] net-tcp: Fast Open client

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 18 Jul 2012 23:23:24 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Yuchung Cheng <ycheng@...gle.com>
Cc:	davem@...emloft.net, hkchu@...gle.com, edumazet@...gle.com,
	ncardwell@...gle.com, sivasankar@...ucsd.edu,
	netdev@...r.kernel.org
Subject: Re: [PATCH v2 3/7] net-tcp: Fast Open client - sending SYN-data

On Wed, 2012-07-18 at 14:01 -0700, Yuchung Cheng wrote:
> This patch implements sending SYN-data in tcp_connect(). The data is
> from tcp_sendmsg() with flag MSG_FASTOPEN (implemented in a later patch).
> 
> The length of the cookie in tcp_fastopen_req, init'd to 0, controls the
> type of the SYN. If the cookie is not cached (len==0), the host sends
> data-less SYN with Fast Open cookie request option to solicit a cookie
> from the remote. If cookie is not available (len > 0), the host sends
> a SYN-data with Fast Open cookie option. If cookie length is negative,
>   the SYN will not include any Fast Open option (for fall back operations).
> 
> To deal with middleboxes that may drop SYN with data or experimental TCP
> option, the SYN-data is only sent once. SYN retransmits do not include
> data or Fast Open options. The connection will fall back to regular TCP
> handshake.
> 
> Signed-off-by: Yuchung Cheng <ycheng@...gle.com>

...

>  
>  static long inet_wait_for_connect(struct sock *sk, long timeo)
>  {
> +	const bool write = (sk->sk_protocol == IPPROTO_TCP) &&
> +			   tcp_sk(sk)->fastopen_req &&
> +			   tcp_sk(sk)->fastopen_req->data;
>  	DEFINE_WAIT(wait);
>  
>  	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
> +	if (write)
> +		sk->sk_write_pending++;
>  
>  	/* Basic assumption: if someone sets sk->sk_err, he _must_
>  	 * change state of the socket from TCP_SYN_*.
> @@ -576,6 +581,8 @@ static long inet_wait_for_connect(struct sock *sk, long timeo)
>  		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
>  	}
>  	finish_wait(sk_sleep(sk), &wait);
> +	if (write)
> +		sk->sk_write_pending--;
>  	return timeo;
>  }
>  

It might be cleaner to move the TCP stuff out of this function and put
it in inet_stream_connect(), since inet_stream_connect() is known to
already have TCP magic.

So I suggest you add a "int writebias" argument to this function, and
use :


static long inet_wait_for_connect(struct sock *sk, long timeo,
+                                 int writebias)
 {
        DEFINE_WAIT(wait);
 
        prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+       sk->sk_write_pending += writebias;
 
        /* Basic assumption: if someone sets sk->sk_err, he _must_
         * change state of the socket from TCP_SYN_*.
@@ -576,6 +581,8 @@ static long inet_wait_for_connect(struct sock *sk,
long timeo)
                prepare_to_wait(sk_sleep(sk), &wait,
TASK_INTERRUPTIBLE);
        }
        finish_wait(sk_sleep(sk), &wait);
+       sk->sk_write_pending -= writebias;
        return timeo;
 }



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html