Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Dmytro Shytyi <dmytro@shytyi.net>]

Hello,

I am sharing experience from using the patch with environment that is 
provided to test the patch with selftest ( As i mentioned before).
I take me some time to launch the patch. Hope this will help.

On 9/23/2022 12:58 PM, Matthieu Baerts wrote:
> 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

Could you please verify this with wireshark before the acceptence?

In evironment provided by the benjamins patch I cannot see the cookie, 
nither if I create sender in C  (that is not using mptcpize as in the 
exemple from packet drill:

mptcp: new fastopen-invalid-buf-ptr test · 
multipath-tcp/packetdrill@af86f4d (github.com) 
<https://github.com/multipath-tcp/packetdrill/commit/af86f4d67c2595b6edd667a89ba9a838308142e2>

or this

Support TCP Fast Open · Issue #49 · rust-lang/socket2 (github.com) 
<https://github.com/rust-lang/socket2/issues/49>

The cookie is not seen neither with curl:

  neither with *.c client, configured

neither with:

-netns client sysctl net.ipv4.tcp_fastopen=1

-netns server sysctl net.ipv4.tcp_fastopen=3

neighter with default Benjamin's setup.


I attach an exemple of c.* file in case if I made any mistake.

"

sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP);

I attach the code from *.C file:

setsockopt(sock_fd, SOL_TCP, TCP_FASTOPEN_CONNECT, &enable, sizeof(enable));

connect(sock_fd, (struct sockaddr *) &daddr, sizeof(daddr));

ret = sendto(sock_fd, sendline, strlen(sendline), 0,(struct sockaddr 
*)&daddr, sizeof(daddr));

"

>> 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

It seems this draft didn't get the consensus and was not accepted by 
IETF as RFC. How the abstract RFC related to the original idea to reuse 
TFO option from regular TCP in linux kernel implementation?