netdev - Re: [RESEND PATCH 1/2] TCP: note received valid-cookie Fast Open option

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CADVnQyn7i_ZHwZNm5gxwHuAcSAF9NdYZyNZyQ_2abr79oytT4g@mail.gmail.com>
Date: Tue, 22 Apr 2025 11:43:37 -0400
From: Neal Cardwell <ncardwell@...gle.com>
To: Jeremy Harris <jgh@...m.org>
Cc: netdev@...r.kernel.org, edumazet@...gle.com
Subject: Re: [RESEND PATCH 1/2] TCP: note received valid-cookie Fast Open option

On Wed, Apr 16, 2025 at 5:15 AM Jeremy Harris <jgh@...m.org> wrote:
>
> Signed-off-by: Jeremy Harris <jgh@...m.org>

The suggested commit title is:

  TCP: note received valid-cookie Fast Open option

The "TCP:" prefix is not the typical prefix for Linux TCP changes. A
"tcp:"  is much more common.

Please follow the convention that we try to adhere to for TCP TFO
changes by using something like:

  tcp: fastopen: note received valid-cookie Fast Open option

> ---
>  include/linux/tcp.h     | 3 ++-
>  net/ipv4/tcp_fastopen.c | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 1669d95bb0f9..a96c38574bce 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -385,7 +385,8 @@ struct tcp_sock {
>                 syn_fastopen:1, /* SYN includes Fast Open option */
>                 syn_fastopen_exp:1,/* SYN includes Fast Open exp. option */
>                 syn_fastopen_ch:1, /* Active TFO re-enabling probe */
> -               syn_data_acked:1;/* data in SYN is acked by SYN-ACK */
> +               syn_data_acked:1,/* data in SYN is acked by SYN-ACK */
> +               syn_fastopen_in:1; /* Received SYN includes Fast Open option */

IMHO this field name and comment are slightly misleading.

Sometimes when a SYN is received with a TFO option the server will
fail to create a child because the TFO cookie is incorrect.

When this bit is set, we know not only that the "Received SYN includes
Fast Open option", but we also know that the TFO cookie was correct
and a child socket was created.

So I would suggest a more specific comment and field name, like:

  syn_fastopen_child:1; /* created TFO passive child socket */

thanks,
neal