mptcp.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Matthieu Baerts <matthieu.baerts@tessares.net>
To: Dmytro Shytyi <dmytro@shytyi.net>,
	benjamin.hesmans@tessares.net, mptcp@lists.linux.dev,
	pabeni@redhat.com
Subject: Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only
Date: Fri, 23 Sep 2022 12:58:53 +0200	[thread overview]
Message-ID: <3d37f47b-dd2e-fe80-caab-d1863028d7f1@tessares.net> (raw)
In-Reply-To: <709a843b-b7f7-438d-39c6-f139aaa55a10@shytyi.net>

Hi Dmytro,

On 23/09/2022 01:23, Dmytro Shytyi wrote:
> Hello Benjamin, All,
> 
> I excuse for the later if I made any mistake.
> 
> My thought is comming from experience with the patch.
> 
> 
> Will huge respect, I think this patch _*MUST NOT*_ be accepted because
> of multiple reasons:
> 
> 1. it violates the RFC 8684 [1] section B1:
> 
> "When a TFO initiator first connects to a listener, it cannot
> immediately include data in the SYN for security reasons[RFC7413
> <https://www.rfc-editor.org/rfc/rfc8684.html#RFC7413>]. Instead, it
> requests a cookie that will be used in subsequent connections."
> 
> 
> Also I created environment[3] using commit[2], I tested v0, v2 and I do
> not see the mptcp capable option in SYN.

From what I see in your setup, you set net.ipv4.tcp_fastopen:
- The sender has the 0x4 flag to send data in the opening SYN regardless
of cookie availability and without a cookie option.
- The receiver has the 0x200 flag to accept data-in-SYN w/o any cookie
option present

See the link below for more details about the bitmap:

https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html

If you change these values, I guess you will see the TFO cookie option.

Please note that the proper way to validate exchanged packet is to use
Packetdrill. The following PR proves the implementation works with TFO
cookies:

https://github.com/multipath-tcp/packetdrill/pull/87

> 2. Abscense of MP_CAPABLE in SYN  violates the RFC 8684 [1] section B3.

The Packetdrill tests also shows everything is OK with MPTCP.
I suppose there is an issue with your test environment and some paths
(e.g. libmptcpwrap.so) are probably wrong, justifying why you get a 404
error when doing the curl:

  "GET /tfo.sh HTTP/1.1" 404

Please do the validation without mptcpize, e.g. with packetdrill:

https://github.com/multipath-tcp/packetdrill/blob/f3672b80a687e0e2a59926992f28c165783ecf8b/gtests/net/mptcp/fastopen/client-TCP_FASTOPEN_CONNECT.pkt

> 3. Patch uses an original Idea of another autor from mailing list (Reuse
> the TCP FASTOPEN option in MPTCP).

A v3 mentioning you is going to be sent as discussed at the last meeting.

We should indeed mention the authors of the original idea to have TFO in
MPTCP:

https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

  reply	other threads:[~2022-09-23 10:58 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-21 12:55 [RFC PATCH mptcp-next v9 0/6] mptcp: Fast Open Mechanism Dmytro Shytyi
2022-09-21 12:55 ` [RFC PATCH mptcp-next v9 1/6] mptcp: add mptcp_stream_connect to protocol.h Dmytro Shytyi
2022-09-21 12:55 ` [RFC PATCH mptcp-next v9 2/6] mptcp: add mptcp_setsockopt_fastopen Dmytro Shytyi
2022-09-21 12:55 ` [RFC PATCH mptcp-next v9 3/6] mptcp: reuse tcp_sendmsg_fastopen() Dmytro Shytyi
2022-09-21 12:55 ` [RFC PATCH mptcp-next v9 4/6] mptcp: fix retrans., add mptfo vars to msk Dmytro Shytyi
2022-09-21 18:02   ` Paolo Abeni
2022-09-22 11:53     ` Dmytro Shytyi
2022-09-21 12:55 ` [RFC PATCH mptcp-next v9 5/6] mptcp: add subflow_v(4,6)_send_synack() Dmytro Shytyi
2022-09-21 18:00   ` Paolo Abeni
2022-09-22 11:50     ` Dmytro Shytyi
2022-09-21 12:55 ` [RFC PATCH mptcp-next v9 6/6] selftests: mptfo initiator/listener Dmytro Shytyi
2022-09-21 13:38   ` selftests: mptfo initiator/listener: Build Failure MPTCP CI
2022-09-21 15:01   ` selftests: mptfo initiator/listener: Tests Results MPTCP CI
2022-09-22 23:23 ` [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only Dmytro Shytyi
2022-09-23 10:58   ` Matthieu Baerts [this message]
2022-09-23 13:41     ` Dmytro Shytyi
2022-09-23 14:08       ` Matthieu Baerts
2022-09-23 14:17         ` Dmytro Shytyi
2022-09-22 13:56 Benjamin Hesmans
2022-09-22 14:36 ` Matthieu Baerts

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=3d37f47b-dd2e-fe80-caab-d1863028d7f1@tessares.net \
    --to=matthieu.baerts@tessares.net \
    --cc=benjamin.hesmans@tessares.net \
    --cc=dmytro@shytyi.net \
    --cc=mptcp@lists.linux.dev \
    --cc=pabeni@redhat.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).