[PATCH net-next v1] net: mptcp, Fast Open Mechanism

[PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Dmytro Shytyi]

This set of patches will bring "Fast Open" Option support to MPTCP.
The aim of Fast Open Mechanism is to eliminate one round trip 
time from a TCP conversation by allowing data to be included as 
part of the SYN segment that initiates the connection. 

IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP.

[PATCH v1] includes "client" partial support for :
1. send request for cookie;
2. send syn+data+cookie.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index cd6b11c9b54d..1f9ef060e980 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1686,6 +1686,68 @@ static void mptcp_set_nospace(struct sock *sk)
        set_bit(MPTCP_NOSPACE, &mptcp_sk(sk)->flags);
 }

+static int mptcp_sendmsg_fastopen_cookie_req(struct sock *sk, struct msghdr *msg,
+                                            size_t *copied, size_t size,
+                                            struct ubuf_info *uarg)
+{
+       struct mptcp_sock *msk = mptcp_sk(sk);
+       struct socket *ssk = __mptcp_nmpc_socket(msk);
+       struct tcp_sock *tp = tcp_sk(ssk->sk);
+       struct sockaddr *uaddr = msg->msg_name;
+       struct tcp_fastopen_context *ctx;
+       const struct iphdr *iph;
+       struct sk_buff *skb;
+       int err;
+
+       skb = sk_stream_alloc_skb(ssk->sk, 0, ssk->sk->sk_allocation, true);
+       iph = ip_hdr(skb);
+       tcp_fastopen_init_key_once(sock_net(ssk->sk));
+       ctx = tcp_fastopen_get_ctx(ssk->sk);
+       tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req),
+                                  ssk->sk->sk_allocation);
+       tp->fastopen_req->data = msg;
+       tp->fastopen_req->size = size;
+       tp->fastopen_req->uarg = uarg;
+       err = mptcp_stream_connect(sk->sk_socket, uaddr, msg->msg_namelen, msg->msg_flags);
+       return err;
+}
+
+static int mptcp_sendmsg_fastopen_cookie_send(struct sock *sk, struct msghdr *msg,
+                                             size_t *copied, size_t size,
+                                             struct ubuf_info *uarg)
+{
+       struct tcp_fastopen_cookie *fastopen_cookie = kmalloc(sizeof(*fastopen_cookie),
+                                                             GFP_KERNEL);
+       struct mptcp_sock *msk = mptcp_sk(sk);
+       struct socket *ssk = __mptcp_nmpc_socket(msk);
+       struct tcp_sock *tp = tcp_sk(ssk->sk);
+       struct sockaddr *uaddr = msg->msg_name;
+       struct tcp_fastopen_context *ctx;
+       const struct iphdr *iph;
+       struct sk_buff *skb;
+       int err;
+
+       skb = sk_stream_alloc_skb(ssk->sk, 0, ssk->sk->sk_allocation, true);
+       iph = ip_hdr(skb);
+       tcp_fastopen_init_key_once(sock_net(ssk->sk));
+       ctx = tcp_fastopen_get_ctx(ssk->sk);
+
+       fastopen_cookie->val[0] = cpu_to_le64(siphash(&iph->saddr,
+                                             sizeof(iph->saddr) +
+                                             sizeof(iph->daddr),
+                                             &ctx->key[0]));
+       fastopen_cookie->len = TCP_FASTOPEN_COOKIE_SIZE;
+
+       tp->fastopen_req = kzalloc(sizeof(*tp->fastopen_req),
+                                  ssk->sk->sk_allocation);
+       tp->fastopen_req->data = msg;
+       tp->fastopen_req->size = size;
+       tp->fastopen_req->uarg = uarg;
+       memcpy(&tp->fastopen_req->cookie, fastopen_cookie, sizeof(tp->fastopen_req->cookie));
+       err = mptcp_stream_connect(sk->sk_socket, uaddr, msg->msg_namelen, msg->msg_flags);
+       return err;
+}
+
 static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 {
        struct mptcp_sock *msk = mptcp_sk(sk);
@@ -1694,9 +1756,22 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
        int ret = 0;
        long timeo;

-       /* we don't support FASTOPEN yet */
-       if (msg->msg_flags & MSG_FASTOPEN)
-               return -EOPNOTSUPP;
+       /* we don't fully support FASTOPEN yet */
+
+       if (msg->msg_flags & MSG_FASTOPEN) {
+               struct socket *ssk = __mptcp_nmpc_socket(msk);
+               struct tcp_sock *tp = tcp_sk(ssk->sk);
+
+               if (tp && tp->fastopen_req && tp->fastopen_req->cookie.len != 0) {
+                       // send cookie
+                       ret = mptcp_sendmsg_fastopen_cookie_send(sk, msg, &copied, len, uarg);
+               } else {
+                       struct tcp_fastopen_request *fastopen = tp->fastopen_req;
+                       //requests a cookie
+                       ret = mptcp_sendmsg_fastopen_cookie_req(sk, msg, &copied, len, uarg);
+               }
+       return ret;
+       }

        /* silently ignore everything else */
        msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Matthieu Baerts]

Hi Dmytro,

On 22/10/2021 07:15, Dmytro Shytyi wrote:
> This set of patches will bring "Fast Open" Option support to MPTCP.
> The aim of Fast Open Mechanism is to eliminate one round trip 
> time from a TCP conversation by allowing data to be included as 
> part of the SYN segment that initiates the connection. 
> 
> IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP.
> 
> [PATCH v1] includes "client" partial support for :
> 1. send request for cookie;
> 2. send syn+data+cookie.

Thank you for working on that and sharing this patch with us!

It looks like some modifications are going to be needed, e.g. after a
very quick look, it seems all tabs have been converted to spaces causing
the patch not being applicable on git[1]: did you not use git
format-patch to generate it? Also, some issues will be reported by
checkpatch.pl, the "reversed XMas tree" order is not respected for
variables declaration, etc.

Do you mind if we continue the discussion on MPTCP mailing list only? In
other words, without Netdev mailing list and Net maintainers, not to
bother everybody for MPTCP specific stuff?

Our usual way of working for MPTCP patches is to have the code review on
MPTCP ML only, then apply accepted patches in our tree first and once a
validation has been done, send patches later to Netdev ML for inclusion
in the -net or net-next tree.

Cheers,
Matt

[1]
https://patchew.org/logs/17ca66cd439.10a0a3ce11621928.1543611905599720914@shytyi.net/git/
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Dmytro Shytyi]

Hello Matthieu,

---- On Fri, 22 Oct 2021 09:22:05 +0200 Matthieu Baerts <matthieu.baerts@tessares.net> wrote ----

 > Hi Dmytro, 
 >  
 > On 22/10/2021 07:15, Dmytro Shytyi wrote: 
 > > This set of patches will bring "Fast Open" Option support to MPTCP. 
 > > The aim of Fast Open Mechanism is to eliminate one round trip 
 > > time from a TCP conversation by allowing data to be included as 
 > > part of the SYN segment that initiates the connection. 
 > > 
 > > IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP. 
 > > 
 > > [PATCH v1] includes "client" partial support for : 
 > > 1. send request for cookie; 
 > > 2. send syn+data+cookie. 
 >  
 > Thank you for working on that and sharing this patch with us! 

 Thank you for checking my patch!

 > It looks like some modifications are going to be needed, e.g. after a 
 > very quick look, it seems all tabs have been converted to spaces causing 
 > the patch not being applicable on git[1]: did you not use git 
 > format-patch to generate it? Also, some issues will be reported by 
 > checkpatch.pl, the "reversed XMas tree" order is not respected for 
 > variables declaration, etc. 

I used scripts/checkpatch.pl. I meant no disrespect, and checked 
the patch before submission. I will do it one more time before next submission.
I used "git diff". Thanks for pointing that out. I will use "git format-patch".



 > Do you mind if we continue the discussion on MPTCP mailing list only? In 
 > other words, without Netdev mailing list and Net maintainers, not to 
 > bother everybody for MPTCP specific stuff? 

 I do not mind. Starting from this message, this is only on mptcp list.
These mail addresses I got by executing ./scripts/get_maintainer.pl

 > Our usual way of working for MPTCP patches is to have the code review on 
 > MPTCP ML only, then apply accepted patches in our tree first and once a 
 > validation has been done, send patches later to Netdev ML for inclusion 
 > in the -net or net-next tree. 

Great,  

 > Cheers, 
 > Matt 

Take care,
Dmytro SHYTYI.

 >  
 > [1] 
 > https://patchew.org/logs/17ca66cd439.10a0a3ce11621928.1543611905599720914@shytyi.net/git/ 
 > -- 
 > Tessares | Belgium | Hybrid Access Solutions 
 > www.tessares.net 
 > 

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Matthieu Baerts]

Hi Dmytro,

(without netdev ML and net maintainers)

On 22/10/2021 07:15, Dmytro Shytyi wrote:
> This set of patches will bring "Fast Open" Option support to MPTCP.
> The aim of Fast Open Mechanism is to eliminate one round trip 
> time from a TCP conversation by allowing data to be included as 
> part of the SYN segment that initiates the connection. 
> 
> IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP.
> 
> [PATCH v1] includes "client" partial support for :
> 1. send request for cookie;
> 2. send syn+data+cookie.
> 
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>

Again, thank you for looking at that!

Here is a very quick review about the code style, just to get that out
from under our feet and not being distracted by that later ;-)
This review is not about the "logic" behind, more about how the code
should look like.

> ---
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index cd6b11c9b54d..1f9ef060e980 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1686,6 +1686,68 @@ static void mptcp_set_nospace(struct sock *sk)
>         set_bit(MPTCP_NOSPACE, &mptcp_sk(sk)->flags);

It looks like the patch has not been formatting properly: we can see all
tabs have been converted to spaces.
The documentation about how to submit patches [1] is quite long but most
of the time these two commands can be enough to send one patch to MPTCP ML:

  $ git format-patch --subject-prefix="PATCH mptcp-next" -v2 -1
  $ git send-email --annotate --to "mptcp@lists.linux.dev" <file|dir>

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html

>  }
> 
> +static int mptcp_sendmsg_fastopen_cookie_req(struct sock *sk, struct msghdr *msg,
> +                                            size_t *copied, size_t size,
> +                                            struct ubuf_info *uarg)
> +{
> +       struct mptcp_sock *msk = mptcp_sk(sk);
> +       struct socket *ssk = __mptcp_nmpc_socket(msk);

For variables declaration in the net tree, we have to follow the
"reversed xmas tree" order: ordered by char size, longest lines first.
So you need to flip the two lines above. Same for the two lines below
and in the next function.

Sadly, checkpatch doesn't warn you about that.

> +       struct tcp_sock *tp = tcp_sk(ssk->sk);
> +       struct sockaddr *uaddr = msg->msg_name;
> +       struct tcp_fastopen_context *ctx;
> +       const struct iphdr *iph;
> +       struct sk_buff *skb;
> +       int err;
> +
> +       skb = sk_stream_alloc_skb(ssk->sk, 0, ssk->sk->sk_allocation, true);
> +       iph = ip_hdr(skb);
> +       tcp_fastopen_init_key_once(sock_net(ssk->sk));
> +       ctx = tcp_fastopen_get_ctx(ssk->sk);

It looks like you don't use it. Does the compiler not complain about that?

> +       tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req),
> +                                  ssk->sk->sk_allocation);

After something got allocated, you need to check if you got what you
wanted. If not, you need to revert some actions, usually done at the end
of a function with goto: you can look around in this file for some examples.
In general, you need to look for all possible errors if the function you
call can return some.

Also, Do not hesitate to create "logical blocks" of code separated by
empty lines, e.g. assigning values in tp->fastopen_req can be done in
one block, the 'return' part in another one, allocating a new skb in
another one, etc. Do not hesitate to look at how it is done around.

> +       tp->fastopen_req->data = msg;
> +       tp->fastopen_req->size = size;
> +       tp->fastopen_req->uarg = uarg;
> +       err = mptcp_stream_connect(sk->sk_socket, uaddr, msg->msg_namelen, msg->msg_flags);
> +       return err;

Maybe clearer not to use 'err' if you don't do anything special with it.

> +}
> +
> +static int mptcp_sendmsg_fastopen_cookie_send(struct sock *sk, struct msghdr *msg,
> +                                             size_t *copied, size_t size,
> +                                             struct ubuf_info *uarg)
> +{
> +       struct tcp_fastopen_cookie *fastopen_cookie = kmalloc(sizeof(*fastopen_cookie),
> +                                                             GFP_KERNEL);
> +       struct mptcp_sock *msk = mptcp_sk(sk);
> +       struct socket *ssk = __mptcp_nmpc_socket(msk);
> +       struct tcp_sock *tp = tcp_sk(ssk->sk);
> +       struct sockaddr *uaddr = msg->msg_name;
> +       struct tcp_fastopen_context *ctx;
> +       const struct iphdr *iph;
> +       struct sk_buff *skb;
> +       int err;
> +
> +       skb = sk_stream_alloc_skb(ssk->sk, 0, ssk->sk->sk_allocation, true);
> +       iph = ip_hdr(skb);
> +       tcp_fastopen_init_key_once(sock_net(ssk->sk));
> +       ctx = tcp_fastopen_get_ctx(ssk->sk);
> +
> +       fastopen_cookie->val[0] = cpu_to_le64(siphash(&iph->saddr,
> +                                             sizeof(iph->saddr) +
> +                                             sizeof(iph->daddr),
> +                                             &ctx->key[0]));
> +       fastopen_cookie->len = TCP_FASTOPEN_COOKIE_SIZE;
> +
> +       tp->fastopen_req = kzalloc(sizeof(*tp->fastopen_req),
> +                                  ssk->sk->sk_allocation);
> +       tp->fastopen_req->data = msg;
> +       tp->fastopen_req->size = size;
> +       tp->fastopen_req->uarg = uarg;
> +       memcpy(&tp->fastopen_req->cookie, fastopen_cookie, sizeof(tp->fastopen_req->cookie));
> +       err = mptcp_stream_connect(sk->sk_socket, uaddr, msg->msg_namelen, msg->msg_flags);
> +       return err;

(Most of the comments above apply to the code here too. Also, please
check if it is not possible to avoid code duplication by creating other
static functions.)

> +}
> +
>  static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>  {
>         struct mptcp_sock *msk = mptcp_sk(sk);
> @@ -1694,9 +1756,22 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>         int ret = 0;
>         long timeo;
> 
> -       /* we don't support FASTOPEN yet */
> -       if (msg->msg_flags & MSG_FASTOPEN)
> -               return -EOPNOTSUPP;
> +       /* we don't fully support FASTOPEN yet */
> +
> +       if (msg->msg_flags & MSG_FASTOPEN) {
> +               struct socket *ssk = __mptcp_nmpc_socket(msk);
> +               struct tcp_sock *tp = tcp_sk(ssk->sk);
> +
> +               if (tp && tp->fastopen_req && tp->fastopen_req->cookie.len != 0) {
> +                       // send cookie

C++-style comments are usually not allowed in the kernel. I thought
checkpatch was saying something about that but maybe it doesn't :)

> +                       ret = mptcp_sendmsg_fastopen_cookie_send(sk, msg, &copied, len, uarg);
> +               } else {
> +                       struct tcp_fastopen_request *fastopen = tp->fastopen_req;
> +                       //requests a cookie
> +                       ret = mptcp_sendmsg_fastopen_cookie_req(sk, msg, &copied, len, uarg);
> +               }
> +       return ret;

The alignement is not OK here.
Please also add a new line before to clearly identify the 'return' part
after the 'if/else'.

I hope this might help reviewing the new versions ;-)

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

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Mat Martineau]

On Fri, 22 Oct 2021, Matthieu Baerts wrote:

> Hi Dmytro,
>
> (without netdev ML and net maintainers)
>
> On 22/10/2021 07:15, Dmytro Shytyi wrote:
>> This set of patches will bring "Fast Open" Option support to MPTCP.
>> The aim of Fast Open Mechanism is to eliminate one round trip
>> time from a TCP conversation by allowing data to be included as
>> part of the SYN segment that initiates the connection.
>>
>> IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP.
>>
>> [PATCH v1] includes "client" partial support for :
>> 1. send request for cookie;
>> 2. send syn+data+cookie.
>>
>> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
>
> Again, thank you for looking at that!
>

Thanks for covering the patch submission and formatting guidelines, 
Matthieu.

Dymtro, it will also be important to add some 'fast open' coverage to the 
kernel self tests in this directory of the kernel source tree:

tools/testing/selftests/net/mptcp/

There's a short guide to running these tests at 
https://github.com/multipath-tcp/mptcp_net-next/wiki/Testing#kselftest


We appreciate your work to add this feature to MPTCP in the Linux kernel!

--
Mat Martineau
Intel

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Dmytro Shytyi]

---- On Fri, 22 Oct 2021 11:22:31 +0200 Matthieu Baerts <matthieu.baerts@tessares.net> wrote ----

 > Hi Dmytro, 
 >  
 > (without netdev ML and net maintainers) 
 >  
 > On 22/10/2021 07:15, Dmytro Shytyi wrote: 
 > > This set of patches will bring "Fast Open" Option support to MPTCP. 
 > > The aim of Fast Open Mechanism is to eliminate one round trip 
 > > time from a TCP conversation by allowing data to be included as 
 > > part of the SYN segment that initiates the connection. 
 > > 
 > > IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP. 
 > > 
 > > [PATCH v1] includes "client" partial support for : 
 > > 1. send request for cookie; 
 > > 2. send syn+data+cookie. 
 > > 
 > > Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net> 
 >  
 > Again, thank you for looking at that! 
 >  
 > Here is a very quick review about the code style, just to get that out 
 > from under our feet and not being distracted by that later ;-) 
 > This review is not about the "logic" behind, more about how the code 
 > should look like. 
 >  
 > > --- 
 > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c 
 > > index cd6b11c9b54d..1f9ef060e980 100644 
 > > --- a/net/mptcp/protocol.c 
 > > +++ b/net/mptcp/protocol.c 
 > > @@ -1686,6 +1686,68 @@ static void mptcp_set_nospace(struct sock *sk) 
 > >         set_bit(MPTCP_NOSPACE, &mptcp_sk(sk)->flags); 
 >  
 > It looks like the patch has not been formatting properly: we can see all 
 > tabs have been converted to spaces. 
 > The documentation about how to submit patches [1] is quite long but most 
 > of the time these two commands can be enough to send one patch to MPTCP ML: 
 >  
 >  $ git format-patch --subject-prefix="PATCH mptcp-next" -v2 -1 
 >  $ git send-email --annotate --to "mptcp@lists.linux.dev" <file|dir> 
 >  
 > [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html 
 >  
 > >  } 
 > > 
 > > +static int mptcp_sendmsg_fastopen_cookie_req(struct sock *sk, struct msghdr *msg, 
 > > +                                            size_t *copied, size_t size, 
 > > +                                            struct ubuf_info *uarg) 
 > > +{ 
 > > +       struct mptcp_sock *msk = mptcp_sk(sk); 
 > > +       struct socket *ssk = __mptcp_nmpc_socket(msk); 
 >  
 > For variables declaration in the net tree, we have to follow the 
 > "reversed xmas tree" order: ordered by char size, longest lines first. 
 > So you need to flip the two lines above. Same for the two lines below 
 > and in the next function. 
 >  
 > Sadly, checkpatch doesn't warn you about that. 
 >  
 > > +       struct tcp_sock *tp = tcp_sk(ssk->sk); 
 > > +       struct sockaddr *uaddr = msg->msg_name; 
 > > +       struct tcp_fastopen_context *ctx; 
 > > +       const struct iphdr *iph; 
 > > +       struct sk_buff *skb; 
 > > +       int err; 
 > > + 
 > > +       skb = sk_stream_alloc_skb(ssk->sk, 0, ssk->sk->sk_allocation, true); 
 > > +       iph = ip_hdr(skb); 
 > > +       tcp_fastopen_init_key_once(sock_net(ssk->sk)); 
 > > +       ctx = tcp_fastopen_get_ctx(ssk->sk); 
 >  
 > It looks like you don't use it. Does the compiler not complain about that? 
 >  
 > > +       tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req), 
 > > +                                  ssk->sk->sk_allocation); 
 >  
 > After something got allocated, you need to check if you got what you 
 > wanted. If not, you need to revert some actions, usually done at the end 
 > of a function with goto: you can look around in this file for some examples. 
 > In general, you need to look for all possible errors if the function you 
 > call can return some. 
 >  
 > Also, Do not hesitate to create "logical blocks" of code separated by 
 > empty lines, e.g. assigning values in tp->fastopen_req can be done in 
 > one block, the 'return' part in another one, allocating a new skb in 
 > another one, etc. Do not hesitate to look at how it is done around. 
 >  
 > > +       tp->fastopen_req->data = msg; 
 > > +       tp->fastopen_req->size = size; 
 > > +       tp->fastopen_req->uarg = uarg; 
 > > +       err = mptcp_stream_connect(sk->sk_socket, uaddr, msg->msg_namelen, msg->msg_flags); 
 > > +       return err; 
 >  
 > Maybe clearer not to use 'err' if you don't do anything special with it. 
 >  
 > > +} 
 > > + 
 > > +static int mptcp_sendmsg_fastopen_cookie_send(struct sock *sk, struct msghdr *msg, 
 > > +                                             size_t *copied, size_t size, 
 > > +                                             struct ubuf_info *uarg) 
 > > +{ 
 > > +       struct tcp_fastopen_cookie *fastopen_cookie = kmalloc(sizeof(*fastopen_cookie), 
 > > +                                                             GFP_KERNEL); 
 > > +       struct mptcp_sock *msk = mptcp_sk(sk); 
 > > +       struct socket *ssk = __mptcp_nmpc_socket(msk); 
 > > +       struct tcp_sock *tp = tcp_sk(ssk->sk); 
 > > +       struct sockaddr *uaddr = msg->msg_name; 
 > > +       struct tcp_fastopen_context *ctx; 
 > > +       const struct iphdr *iph; 
 > > +       struct sk_buff *skb; 
 > > +       int err; 
 > > + 
 > > +       skb = sk_stream_alloc_skb(ssk->sk, 0, ssk->sk->sk_allocation, true); 
 > > +       iph = ip_hdr(skb); 
 > > +       tcp_fastopen_init_key_once(sock_net(ssk->sk)); 
 > > +       ctx = tcp_fastopen_get_ctx(ssk->sk); 
 > > + 
 > > +       fastopen_cookie->val[0] = cpu_to_le64(siphash(&iph->saddr, 
 > > +                                             sizeof(iph->saddr) + 
 > > +                                             sizeof(iph->daddr), 
 > > +                                             &ctx->key[0])); 
 > > +       fastopen_cookie->len = TCP_FASTOPEN_COOKIE_SIZE; 
 > > + 
 > > +       tp->fastopen_req = kzalloc(sizeof(*tp->fastopen_req), 
 > > +                                  ssk->sk->sk_allocation); 
 > > +       tp->fastopen_req->data = msg; 
 > > +       tp->fastopen_req->size = size; 
 > > +       tp->fastopen_req->uarg = uarg; 
 > > +       memcpy(&tp->fastopen_req->cookie, fastopen_cookie, sizeof(tp->fastopen_req->cookie)); 
 > > +       err = mptcp_stream_connect(sk->sk_socket, uaddr, msg->msg_namelen, msg->msg_flags); 
 > > +       return err; 
 >  
 > (Most of the comments above apply to the code here too. Also, please 
 > check if it is not possible to avoid code duplication by creating other 
 > static functions.) 
 >  
 > > +} 
 > > + 
 > >  static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) 
 > >  { 
 > >         struct mptcp_sock *msk = mptcp_sk(sk); 
 > > @@ -1694,9 +1756,22 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) 
 > >         int ret = 0; 
 > >         long timeo; 
 > > 
 > > -       /* we don't support FASTOPEN yet */ 
 > > -       if (msg->msg_flags & MSG_FASTOPEN) 
 > > -               return -EOPNOTSUPP; 
 > > +       /* we don't fully support FASTOPEN yet */ 
 > > + 
 > > +       if (msg->msg_flags & MSG_FASTOPEN) { 
 > > +               struct socket *ssk = __mptcp_nmpc_socket(msk); 
 > > +               struct tcp_sock *tp = tcp_sk(ssk->sk); 
 > > + 
 > > +               if (tp && tp->fastopen_req && tp->fastopen_req->cookie.len != 0) { 
 > > +                       // send cookie 
 >  
 > C++-style comments are usually not allowed in the kernel. I thought 
 > checkpatch was saying something about that but maybe it doesn't :) 
 >  
 > > +                       ret = mptcp_sendmsg_fastopen_cookie_send(sk, msg, &copied, len, uarg); 
 > > +               } else { 
 > > +                       struct tcp_fastopen_request *fastopen = tp->fastopen_req; 
 > > +                       //requests a cookie 
 > > +                       ret = mptcp_sendmsg_fastopen_cookie_req(sk, msg, &copied, len, uarg); 
 > > +               } 
 > > +       return ret; 
 >  
 > The alignement is not OK here. 
 > Please also add a new line before to clearly identify the 'return' part 
 > after the 'if/else'. 
 >  
 > I hope this might help reviewing the new versions ;-) 
 
Thank you, Metthieu. 
I'm working on PATCH net-next v2 net: mptcp. It will include all your comments, except this one:

 > It looks like you don't use it. Does the compiler not complain about that? 
 >  
 > > +       tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req), 
 > > +                                  ssk->sk->sk_allocation); 
Compiler not compalin about that because it is used in the code.


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

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Dmytro Shytyi]

---- On Fri, 22 Oct 2021 20:26:35 +0200 Mat Martineau <mathew.j.martineau@linux.intel.com> wrote ----

 > On Fri, 22 Oct 2021, Matthieu Baerts wrote: 
 >  
 > > Hi Dmytro, 
 > > 
 > > (without netdev ML and net maintainers) 
 > > 
 > > On 22/10/2021 07:15, Dmytro Shytyi wrote: 
 > >> This set of patches will bring "Fast Open" Option support to MPTCP. 
 > >> The aim of Fast Open Mechanism is to eliminate one round trip 
 > >> time from a TCP conversation by allowing data to be included as 
 > >> part of the SYN segment that initiates the connection. 
 > >> 
 > >> IETF RFC 8684: Appendix B.  TCP Fast Open and MPTCP. 
 > >> 
 > >> [PATCH v1] includes "client" partial support for : 
 > >> 1. send request for cookie; 
 > >> 2. send syn+data+cookie. 
 > >> 
 > >> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net> 
 > > 
 > > Again, thank you for looking at that! 
 > > 
 >  
 > Thanks for covering the patch submission and formatting guidelines, 
 > Matthieu. 
 >  
 > Dymtro, it will also be important to add some 'fast open' coverage to the 
 > kernel self tests in this directory of the kernel source tree: 
 >  
 > tools/testing/selftests/net/mptcp/ 
 >  
 > There's a short guide to running these tests at 
 > https://github.com/multipath-tcp/mptcp_net-next/wiki/Testing#kselftest 

Hello Mat,

I will add 'fast open' coverage to the kernel self tests.
Thank you for pointing that out.
 >  
 > We appreciate your work to add this feature to MPTCP in the Linux kernel! 
Thanks!

Take care,
Dmytro SHYTYI

 > -- 
 > Mat Martineau 
 > Intel 
 > 

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Matthieu Baerts]

Hi Dmytro,

On 25/10/2021 13:34, Dmytro Shytyi wrote:
> 
> ---- On Fri, 22 Oct 2021 11:22:31 +0200 Matthieu Baerts <matthieu.baerts@tessares.net> wrote ----
> 
>  > Hi Dmytro, 
>  >  

Small detail: it looks like your email client is adding unusual spaces
before ">" to quote the previous message. That's not a big issue but it
makes other email clients and even lore [1] not interpreting that as
quotes, e.g. on lore, you can see the quotes are in blue in my reply and
Mat's one but not in yours.

[1]
https://lore.kernel.org/mptcp/17cb73b1343.c752fede1729351.6463099199494289591@shytyi.net/T/#t

(...)

>  > I hope this might help reviewing the new versions ;-) 
>  
> Thank you, Metthieu. 
> I'm working on PATCH net-next v2 net: mptcp. It will include all your comments, except this one:
> 
>  > It looks like you don't use it. Does the compiler not complain about that? 
>  >  
>  > > +       tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req), 
>  > > +                                  ssk->sk->sk_allocation); 
> Compiler not compalin about that because it is used in the code.

In the version you sent, in mptcp_sendmsg_fastopen_cookie_send() you are
indeed using it but not it in mptcp_sendmsg_fastopen_cookie_req(), no?

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

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Dmytro Shytyi]

[-- Attachment #1: Type: text/plain, Size: 10609 bytes --]

Hello all,

I'm writing this message to post an update regarding the work on fastopen mechanism.
Evolution of fastopen PATCH is far away from submission yet (packet retransmissions are present), but there are good signs.
Notably I attach the pcap wireshark that includes client + server in 2 different namespaces:
 > > 1. MPTCP cookie request from client.
 > > 2. MPTCP cookie offering from server.
 > > 3. MPTCP SYN+DATA+COOKIE from client.
 > > 4. subsequent write + read on the opened socket.
Finally the userspace application receives the data send in the SYN+DATA+COOKIE. You may find the examples in the prinf+tcpdump log on the bottom of the message as a bonus...

I'm looking what I have to adjust to fix packet retransmissions. If you have in mind some ideas/suggestions, you are welcome to provide some comments (in mailing list or maybe in private),

awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client
info: 0
ACCEPT_SUCCESS: Success
SERVER_READ_SUCCESS: Success
received z from client
SERVER_READ_SUCCESS: Success
received z from client
SERVER_READ_SUCCESS: Success
received z from client
SERVER_READ_SUCCESS: Success
received a from client
SERVER_READ_SUCCESS: Success
received f from client

22:49:21.410344 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [S], seq 3325457504, win 64240, options [mss 1460,sackOK,TS val 1084403317 ecr 0,nop,wscale 7,tfo cookiereq,nop,nop,mptcp capable[bad opt]>
22:49:21.410367 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [S.], seq 1929681786, ack 3325457505, win 65160, options [mss 1460,sackOK,TS val 1487352723 ecr 1084403317,nop,wscale 7,tfo cookie d0f85d19,nop,nop,mptcp capable Unknown Version (1)], length 0
22:49:21.412560 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 1:2, ack 1, win 502, options [nop,nop,TS val 1084403319 ecr 1487352723,mptcp capable[bad opt]>
22:49:21.412621 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 2, win 510, options [nop,nop,TS val 1487352725 ecr 1084403319,mptcp dss ack 12880089298653898158], length 0
22:49:21.413113 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403319 ecr 1487352725,mptcp dss ack 3168804441], length 0
22:49:21.413134 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 2:3, ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352725,mptcp dss ack 3168804441 seq 12880089298653898158 subseq 2 len 1,nop,nop], length 1
22:49:21.413139 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 3, win 510, options [nop,nop,TS val 1487352726 ecr 1084403320,mptcp dss ack 12880089298653898159], length 0
22:49:21.413394 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441], length 0
22:49:21.413408 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 3:4, ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441 seq 12880089298653898159 subseq 3 len 1,nop,nop], length 1
22:49:21.413411 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 4, win 510, options [nop,nop,TS val 1487352726 ecr 1084403320,mptcp dss ack 12880089298653898160], length 0
22:49:21.413674 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441], length 0
22:49:21.413699 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 4:5, ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441 seq 12880089298653898160 subseq 4 len 1,nop,nop], length 1
22:49:21.413944 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441], length 0
22:49:21.414206 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487352727 ecr 1084403321,mptcp dss fin ack 12880089298653898162 seq 11302478206648458841 subseq 0 len 1,nop,nop], length 0


awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client
ACCEPT_SUCCESS: Success
SERVER_READ_SUCCESS: Success
received a from client <<<<<<<<<<<<<<< THIS "A" CHARACTER IS A PART OF THE SYN+DATA+COOKIE.
info: 0
SERVER_READ_SUCCESS: Success
received z from client
SERVER_READ_SUCCESS: Success
received z from client
SERVER_READ_SUCCESS: Success
received z from client
SERVER_READ_SUCCESS: Success
received a from client
SERVER_READ_SUCCESS: Success
received f from client
22:52:11.262622 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [S], seq 549537183:549537184, win 64240, options [mss 1460,sackOK,TS val 1084573169 ecr 0,nop,wscale 7,tfo cookie d0f85d19,nop,nop,mptcp capable[bad opt]>
22:52:11.262767 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [S.], seq 4092504081, ack 549537185, win 65160, options [mss 1460,sackOK,TS val 1487522575 ecr 1084573169,nop,wscale 7,mptcp capable Unknown Version (1)], length 0
22:52:11.262810 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573169 ecr 1487522575,mptcp capable Unknown Version (1)], length 0
22:52:11.266377 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 1:2, ack 1, win 502, options [nop,nop,TS val 1084573173 ecr 1487522575,mptcp capable[bad opt]>
22:52:11.266451 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 2, win 510, options [nop,nop,TS val 1487522579 ecr 1084573173,mptcp dss ack 12881747887342348535], length 0
22:52:11.266904 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573173 ecr 1487522579,mptcp dss ack 3798297942], length 0
22:52:11.267039 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 2:3, ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522579,mptcp dss ack 3798297942 seq 746326162666601844 subseq 2 len 1,nop,nop], length 1
22:52:11.267050 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 3, win 510, options [nop,nop,TS val 1487522580 ecr 1084573174,mptcp dss ack 12881747887342348536], length 0
22:52:11.267427 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522580,mptcp dss ack 3798297942], length 0
22:52:11.267454 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 3:4, ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522580,mptcp dss ack 3798297942 seq 746326162666601845 subseq 3 len 1,nop,nop], length 1
22:52:11.267461 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 4, win 510, options [nop,nop,TS val 1487522580 ecr 1084573174,mptcp dss ack 12881747887342348537], length 0
22:52:11.267957 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522580,mptcp dss ack 3798297942], length 0
22:52:11.267988 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 4:5, ack 1, win 502, options [nop,nop,TS val 1084573175 ecr 1487522580,mptcp dss ack 3798297942 seq 746326162666601846 subseq 4 len 1,nop,nop], length 1
22:52:11.267995 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 5, win 510, options [nop,nop,TS val 1487522581 ecr 1084573175,mptcp dss ack 12881747887342348538], length 0
22:52:11.268546 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573175 ecr 1487522581,mptcp dss ack 3798297942], length 0
22:52:11.268594 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 5:6, ack 1, win 502, options [nop,nop,TS val 1084573175 ecr 1487522581,mptcp dss ack 3798297942 seq 746326162666601847 subseq 5 len 1,nop,nop], length 1
22:52:11.268603 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487522581 ecr 1084573175,mptcp dss ack 12881747887342348539], length 0
22:52:11.269735 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487522581 ecr 1084573175,mptcp dss fin ack 12881747887342348539 seq 9620565311902543190 subseq 0 len 1,nop,nop], length 0
22:52:11.269846 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573176 ecr 1487522581,mptcp dss ack 3798297942], length 0
22:52:11.469004 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573177 ecr 1487522583,mptcp dss fin ack 3798297943 seq 746326162666601848 subseq 0 len 1,nop,nop], length 0
22:52:11.472189 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487522782 ecr 1084573177,mptcp dss ack 12881747887342348539], length 0
22:53:11.672337 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573379 ecr 1487522782,mptcp dss fin ack 3798297943 seq 746326162666601848 subseq 0 len 1,nop,nop], length 0
22:53:11.672425 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [F.], seq 6, ack 1, win 502, options [nop,nop,TS val 1084633579 ecr 1487522782,mptcp dss fin ack 3798297943 seq 746326162666601848 subseq 0 len 1,nop,nop], length 0
22:53:11.672461 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 7, win 510, options [nop,nop,TS val 1487582985 ecr 1084633579,mptcp dss ack 12881747887342348539], length 0
22:53:11.672515 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [F.], seq 1, ack 7, win 510, options [nop,nop,TS val 1487582985 ecr 1084633579,mptcp dss ack 12881747887342348539], length 0
22:53:11.672545 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 2, win 502, options [nop,nop,TS val 1084633579 ecr 1487582985], length 0

Thanks,
Dmytro.
> 
> Hi Dmytro,
>
> On 25/10/2021 13:34, Dmytro Shytyi wrote:
> >
> > ---- On Fri, 22 Oct 2021 11:22:31 +0200 Matthieu Baerts <matthieu.baerts@tessares.net> wrote ----
> >
> > > Hi Dmytro,
> > >
>
> Small detail: it looks like your email client is adding unusual spaces
> before ">" to quote the previous message. That's not a big issue but it
> makes other email clients and even lore [1] not interpreting that as
> quotes, e.g. on lore, you can see the quotes are in blue in my reply and
> Mat's one but not in yours.
>
> [1]
> https://lore.kernel.org/mptcp/17cb73b1343.c752fede1729351.6463099199494289591@shytyi.net/T/#t
>
> (...)
>
> > > I hope this might help reviewing the new versions ;-)
> >
> > Thank you, Metthieu.
> > I'm working on PATCH net-next v2 net: mptcp. It will include all your comments, except this one:
> >
> > > It looks like you don't use it. Does the compiler not complain about that?
> > >
> > > > + tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req),
> > > > + ssk->sk->sk_allocation);
> > Compiler not compalin about that because it is used in the code.
>
> In the version you sent, in mptcp_sendmsg_fastopen_cookie_send() you are
> indeed using it but not it in mptcp_sendmsg_fastopen_cookie_req(), no?
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net

[-- Attachment #2: fastopen (1).pcap --]
[-- Type: application/octet-stream, Size: 7203 bytes --]

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Dmytro Shytyi]

[-- Attachment #1.1: Type: text/plain, Size: 18943 bytes --]

Hello all,



I'm writing this message to post another update regarding the work on fastopen mechanism in MPTCP.



Changelog:

1. Packet retransmissions are fixed.

2. The final ack (was missing) is now present during the exchange 3way handshake during TFO cookie request.



Work is in process...



If you find any anomaly, comments are welcome.



Tcpdump log + pcap trace are attached.





Tcpdump log:


awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client

17:23:50.587922 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [S], seq 3910070489, win 64240, options [mss 1460,sackOK,TS val 2969564598 ecr 0,nop,wscale 7,tfo  cookiereq,nop,nop,mptcp capable[bad opt]>

17:23:50.587955 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [S.], seq 3806637430, ack 3910070490, win 65160, options [mss 1460,sackOK,TS val 2284154585 ecr 2969564598,nop,wscale 7,tfo  cookie 5e09b5dd,nop,nop,mptcp capable Unknown Version (1)], length 0

17:23:50.587997 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 2969564598 ecr 2284154585,mptcp capable Unknown Version (1)], length 0

17:23:50.589091 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [P.], seq 1:4, ack 1, win 502, options [nop,nop,TS val 2969564599 ecr 2284154585,mptcp capable[bad opt]>

17:23:50.589149 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [.], ack 4, win 510, options [nop,nop,TS val 2284154586 ecr 2969564599,mptcp dss ack 6653452424296905113], length 0

17:23:50.591278 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [P.], seq 4:7, ack 1, win 502, options [nop,nop,TS val 2969564601 ecr 2284154586,mptcp dss ack 3417001499 seq 6653452424296905113 subseq 4 len 3,nop,nop], length 3

17:23:50.591382 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [.], ack 7, win 510, options [nop,nop,TS val 2284154588 ecr 2969564601,mptcp dss ack 6653452424296905116], length 0

17:23:50.591433 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [P.], seq 7:10, ack 1, win 502, options [nop,nop,TS val 2969564601 ecr 2284154588,mptcp dss ack 3417001499 seq 6653452424296905116 subseq 7 len 3,nop,nop], length 3

17:23:50.591441 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [.], ack 10, win 510, options [nop,nop,TS val 2284154588 ecr 2969564601,mptcp dss ack 6653452424296905119], length 0

17:23:50.591458 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [P.], seq 10:13, ack 1, win 502, options [nop,nop,TS val 2969564601 ecr 2284154588,mptcp dss ack 3417001499 seq 6653452424296905119 subseq 10 len 3,nop,nop], length 3

17:23:50.591465 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [.], ack 13, win 510, options [nop,nop,TS val 2284154588 ecr 2969564601,mptcp dss ack 6653452424296905122], length 0

17:23:50.591479 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [P.], seq 13:16, ack 1, win 502, options [nop,nop,TS val 2969564601 ecr 2284154588,mptcp dss ack 3417001499 seq 6653452424296905122 subseq 13 len 3,nop,nop], length 3

17:23:50.591485 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [.], ack 16, win 510, options [nop,nop,TS val 2284154588 ecr 2969564601,mptcp dss ack 6653452424296905125], length 0

17:24:51.757234 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [F.], seq 1, ack 16, win 510, options [nop,nop,TS val 2284215754 ecr 2969564601,mptcp dss fin ack 6653452424296905125 seq 12259594063739309595 subseq 0 len 1,nop,nop], length 0

17:24:51.757252 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [F.], seq 16, ack 1, win 502, options [nop,nop,TS val 2969625767 ecr 2284154588,mptcp dss fin ack 3417001499 seq 6653452424296905125 subseq 0 len 1,nop,nop], length 0

17:24:51.757364 IP 10.0.0.2.7003 > 10.0.0.1.35282: Flags [.], ack 17, win 510, options [nop,nop,TS val 2284215754 ecr 2969625767,mptcp dss fin ack 6653452424296905125 seq 12259594063739309595 subseq 0 len 1,nop,nop], length 0

17:24:51.757367 IP 10.0.0.1.35282 > 10.0.0.2.7003: Flags [.], ack 2, win 502, options [nop,nop,TS val 2969625767 ecr 2284215754,mptcp dss fin ack 3417001499 seq 6653452424296905125 subseq 0 len 1,nop,nop], length 0




awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client

17:24:00.975725 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [S], seq 1823106728:1823106731, win 64240, options [mss 1460,sackOK,TS val 2969574986 ecr 0,nop,wscale 7,tfo  cookie 5e09b5dd,nop,nop,mptcp capable[bad opt]>

17:24:00.975783 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [S.], seq 3107129198, ack 1823106732, win 65160, options [mss 1460,sackOK,TS val 2284164973 ecr 2969574986,nop,wscale 7,mptcp capable Unknown Version (1)], length 0

17:24:00.975896 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 2969574986 ecr 2284164973,mptcp capable Unknown Version (1)], length 0

17:24:00.976880 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [P.], seq 1:4, ack 1, win 502, options [nop,nop,TS val 2969574987 ecr 2284164973,mptcp capable[bad opt]>

17:24:00.977097 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [.], ack 4, win 510, options [nop,nop,TS val 2284164974 ecr 2969574987,mptcp dss ack 17277082426285855013], length 0

17:24:00.977148 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [P.], seq 4:7, ack 1, win 502, options [nop,nop,TS val 2969574987 ecr 2284164974,mptcp dss ack 3342174256 seq 777308174741155629 subseq 4 len 3,nop,nop], length 3

17:24:00.977163 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [.], ack 7, win 510, options [nop,nop,TS val 2284164974 ecr 2969574987,mptcp dss ack 17277082426285855016], length 0

17:24:00.977184 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [P.], seq 7:10, ack 1, win 502, options [nop,nop,TS val 2969574987 ecr 2284164974,mptcp dss ack 3342174256 seq 777308174741155632 subseq 7 len 3,nop,nop], length 3

17:24:00.977192 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [.], ack 10, win 510, options [nop,nop,TS val 2284164974 ecr 2969574987,mptcp dss ack 17277082426285855019], length 0

17:24:00.977230 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [P.], seq 10:13, ack 1, win 502, options [nop,nop,TS val 2969574987 ecr 2284164974,mptcp dss ack 3342174256 seq 777308174741155635 subseq 10 len 3,nop,nop], length 3

17:24:00.977243 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [.], ack 13, win 510, options [nop,nop,TS val 2284164974 ecr 2969574987,mptcp dss ack 17277082426285855022], length 0

17:25:01.997117 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [F.], seq 1, ack 13, win 510, options [nop,nop,TS val 2284225994 ecr 2969574987,mptcp dss fin ack 17277082426285855022 seq 1417219265470498864 subseq 0 len 1,nop,nop], length 0

17:25:01.997180 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [F.], seq 13, ack 1, win 502, options [nop,nop,TS val 2969636007 ecr 2284164974,mptcp dss fin ack 3342174256 seq 777308174741155638 subseq 0 len 1,nop,nop], length 0

17:25:01.997232 IP 10.0.0.2.7003 > 10.0.0.1.35284: Flags [.], ack 14, win 510, options [nop,nop,TS val 2284225994 ecr 2969636007,mptcp dss fin ack 17277082426285855022 seq 1417219265470498864 subseq 0 len 1,nop,nop], length 0

17:25:01.997233 IP 10.0.0.1.35284 > 10.0.0.2.7003: Flags [.], ack 2, win 502, options [nop,nop,TS val 2969636007 ecr 2284225994,mptcp dss fin ack 3342174256 seq 777308174741155638 subseq 0 len 1,nop,nop], length 0



Userspace data exchange:





awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client

received abc from client

received zzz from client

received zzz from client

received zzz from client

received fff from client



awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client

received abc from client

received zzz from client

received zzz from client

received zzz from client

received fff from client





Best regards,
Dmytro SHYTYI.



> Hello all,

> 

> I'm writing this message to post an update regarding the work on fastopen mechanism.

> Evolution of fastopen PATCH is far away from submission yet (packet retransmissions are present), but there are good signs.

> Notably I attach the pcap wireshark that includes client + server in 2 different namespaces:

>  > > 1. MPTCP cookie request from client.

>  > > 2. MPTCP cookie offering from server.

>  > > 3. MPTCP SYN+DATA+COOKIE from client.

>  > > 4. subsequent write + read on the opened socket.

> Finally the userspace application receives the data send in the SYN+DATA+COOKIE. You may find the examples in the prinf+tcpdump log on the bottom of the message as a bonus...

> 

> I'm looking what I have to adjust to fix packet retransmissions. If you have in mind some ideas/suggestions, you are welcome to provide some comments (in mailing list or maybe in private),

> 

> awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client

> info: 0

> ACCEPT_SUCCESS: Success

> SERVER_READ_SUCCESS: Success

> received z from client

> SERVER_READ_SUCCESS: Success

> received z from client

> SERVER_READ_SUCCESS: Success

> received z from client

> SERVER_READ_SUCCESS: Success

> received a from client

> SERVER_READ_SUCCESS: Success

> received f from client

> 

> 22:49:21.410344 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [S], seq 3325457504, win 64240, options [mss 1460,sackOK,TS val 1084403317 ecr 0,nop,wscale 7,tfo cookiereq,nop,nop,mptcp capable[bad opt]>

> 22:49:21.410367 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [S.], seq 1929681786, ack 3325457505, win 65160, options [mss 1460,sackOK,TS val 1487352723 ecr 1084403317,nop,wscale 7,tfo cookie d0f85d19,nop,nop,mptcp capable Unknown Version (1)], length 0

> 22:49:21.412560 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 1:2, ack 1, win 502, options [nop,nop,TS val 1084403319 ecr 1487352723,mptcp capable[bad opt]>

> 22:49:21.412621 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 2, win 510, options [nop,nop,TS val 1487352725 ecr 1084403319,mptcp dss ack 12880089298653898158], length 0

> 22:49:21.413113 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403319 ecr 1487352725,mptcp dss ack 3168804441], length 0

> 22:49:21.413134 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 2:3, ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352725,mptcp dss ack 3168804441 seq 12880089298653898158 subseq 2 len 1,nop,nop], length 1

> 22:49:21.413139 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 3, win 510, options [nop,nop,TS val 1487352726 ecr 1084403320,mptcp dss ack 12880089298653898159], length 0

> 22:49:21.413394 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441], length 0

> 22:49:21.413408 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 3:4, ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441 seq 12880089298653898159 subseq 3 len 1,nop,nop], length 1

> 22:49:21.413411 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 4, win 510, options [nop,nop,TS val 1487352726 ecr 1084403320,mptcp dss ack 12880089298653898160], length 0

> 22:49:21.413674 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441], length 0

> 22:49:21.413699 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [P.], seq 4:5, ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441 seq 12880089298653898160 subseq 4 len 1,nop,nop], length 1

> 22:49:21.413944 IP 10.0.0.1.37980 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084403320 ecr 1487352726,mptcp dss ack 3168804441], length 0

> 22:49:21.414206 IP 10.0.0.2.7003 > 10.0.0.1.37980: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487352727 ecr 1084403321,mptcp dss fin ack 12880089298653898162 seq 11302478206648458841 subseq 0 len 1,nop,nop], length 0

> 

> 

> awesome@ferby:~/workspace/new-ns$ sudo ip netns exec client ./client

> ACCEPT_SUCCESS: Success

> SERVER_READ_SUCCESS: Success

> received a from client <<<<<<<<<<<<<<< THIS "A" CHARACTER IS A PART OF THE SYN+DATA+COOKIE.

> info: 0

> SERVER_READ_SUCCESS: Success

> received z from client

> SERVER_READ_SUCCESS: Success

> received z from client

> SERVER_READ_SUCCESS: Success

> received z from client

> SERVER_READ_SUCCESS: Success

> received a from client

> SERVER_READ_SUCCESS: Success

> received f from client

> 22:52:11.262622 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [S], seq 549537183:549537184, win 64240, options [mss 1460,sackOK,TS val 1084573169 ecr 0,nop,wscale 7,tfo cookie d0f85d19,nop,nop,mptcp capable[bad opt]>

> 22:52:11.262767 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [S.], seq 4092504081, ack 549537185, win 65160, options [mss 1460,sackOK,TS val 1487522575 ecr 1084573169,nop,wscale 7,mptcp capable Unknown Version (1)], length 0

> 22:52:11.262810 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573169 ecr 1487522575,mptcp capable Unknown Version (1)], length 0

> 22:52:11.266377 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 1:2, ack 1, win 502, options [nop,nop,TS val 1084573173 ecr 1487522575,mptcp capable[bad opt]>

> 22:52:11.266451 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 2, win 510, options [nop,nop,TS val 1487522579 ecr 1084573173,mptcp dss ack 12881747887342348535], length 0

> 22:52:11.266904 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573173 ecr 1487522579,mptcp dss ack 3798297942], length 0

> 22:52:11.267039 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 2:3, ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522579,mptcp dss ack 3798297942 seq 746326162666601844 subseq 2 len 1,nop,nop], length 1

> 22:52:11.267050 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 3, win 510, options [nop,nop,TS val 1487522580 ecr 1084573174,mptcp dss ack 12881747887342348536], length 0

> 22:52:11.267427 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522580,mptcp dss ack 3798297942], length 0

> 22:52:11.267454 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 3:4, ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522580,mptcp dss ack 3798297942 seq 746326162666601845 subseq 3 len 1,nop,nop], length 1

> 22:52:11.267461 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 4, win 510, options [nop,nop,TS val 1487522580 ecr 1084573174,mptcp dss ack 12881747887342348537], length 0

> 22:52:11.267957 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573174 ecr 1487522580,mptcp dss ack 3798297942], length 0

> 22:52:11.267988 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 4:5, ack 1, win 502, options [nop,nop,TS val 1084573175 ecr 1487522580,mptcp dss ack 3798297942 seq 746326162666601846 subseq 4 len 1,nop,nop], length 1

> 22:52:11.267995 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 5, win 510, options [nop,nop,TS val 1487522581 ecr 1084573175,mptcp dss ack 12881747887342348538], length 0

> 22:52:11.268546 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573175 ecr 1487522581,mptcp dss ack 3798297942], length 0

> 22:52:11.268594 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [P.], seq 5:6, ack 1, win 502, options [nop,nop,TS val 1084573175 ecr 1487522581,mptcp dss ack 3798297942 seq 746326162666601847 subseq 5 len 1,nop,nop], length 1

> 22:52:11.268603 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487522581 ecr 1084573175,mptcp dss ack 12881747887342348539], length 0

> 22:52:11.269735 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487522581 ecr 1084573175,mptcp dss fin ack 12881747887342348539 seq 9620565311902543190 subseq 0 len 1,nop,nop], length 0

> 22:52:11.269846 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573176 ecr 1487522581,mptcp dss ack 3798297942], length 0

> 22:52:11.469004 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573177 ecr 1487522583,mptcp dss fin ack 3798297943 seq 746326162666601848 subseq 0 len 1,nop,nop], length 0

> 22:52:11.472189 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 6, win 510, options [nop,nop,TS val 1487522782 ecr 1084573177,mptcp dss ack 12881747887342348539], length 0

> 22:53:11.672337 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 1, win 502, options [nop,nop,TS val 1084573379 ecr 1487522782,mptcp dss fin ack 3798297943 seq 746326162666601848 subseq 0 len 1,nop,nop], length 0

> 22:53:11.672425 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [F.], seq 6, ack 1, win 502, options [nop,nop,TS val 1084633579 ecr 1487522782,mptcp dss fin ack 3798297943 seq 746326162666601848 subseq 0 len 1,nop,nop], length 0

> 22:53:11.672461 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [.], ack 7, win 510, options [nop,nop,TS val 1487582985 ecr 1084633579,mptcp dss ack 12881747887342348539], length 0

> 22:53:11.672515 IP 10.0.0.2.7003 > 10.0.0.1.37982: Flags [F.], seq 1, ack 7, win 510, options [nop,nop,TS val 1487582985 ecr 1084633579,mptcp dss ack 12881747887342348539], length 0

> 22:53:11.672545 IP 10.0.0.1.37982 > 10.0.0.2.7003: Flags [.], ack 2, win 502, options [nop,nop,TS val 1084633579 ecr 1487582985], length 0

> 

> Thanks,

> Dmytro.

> >

> > Hi Dmytro,

> >

> > On 25/10/2021 13:34, Dmytro Shytyi wrote:

> > >

> > > ---- On Fri, 22 Oct 2021 11:22:31 +0200 Matthieu Baerts <mailto:matthieu.baerts@tessares.net> wrote ----

> > >

> > > > Hi Dmytro,

> > > >

> >

> > Small detail: it looks like your email client is adding unusual spaces

> > before ">" to quote the previous message. That's not a big issue but it

> > makes other email clients and even lore [1] not interpreting that as

> > quotes, e.g. on lore, you can see the quotes are in blue in my reply and

> > Mat's one but not in yours.

> >

> > [1]

> > https://lore.kernel.org/mptcp/17cb73b1343.c752fede1729351.6463099199494289591@shytyi.net/T/#t

> >

> > (...)

> >

> > > > I hope this might help reviewing the new versions ;-)

> > >

> > > Thank you, Metthieu.

> > > I'm working on PATCH net-next v2 net: mptcp. It will include all your comments, except this one:

> > >

> > > > It looks like you don't use it. Does the compiler not complain about that?

> > > >

> > > > > + tp->fastopen_req = kzalloc(sizeof(*tp->fatopen_req),

> > > > > + ssk->sk->sk_allocation);

> > > Compiler not compalin about that because it is used in the code.

> >

> > In the version you sent, in mptcp_sendmsg_fastopen_cookie_send() you are

> > indeed using it but not it in mptcp_sendmsg_fastopen_cookie_req(), no?

> >

> > Cheers,

> > Matt

> > --

> > Tessares | Belgium | Hybrid Access Solutions

> >http://www.tessares.net

[-- Attachment #1.2: Type: text/html, Size: 22736 bytes --]

[-- Attachment #2: fastopen.pcap --]
[-- Type: application/octet-stream, Size: 5590 bytes --]

Re: [PATCH net-next v1] net: mptcp, Fast Open Mechanism

[Paolo Abeni]

Hello,

On Sat, 2022-01-08 at 18:58 +0100, Dmytro Shytyi wrote:
> I'm writing this message to post another update regarding the work on
> fastopen mechanism in MPTCP.
> 
> Changelog:
> 1. Packet retransmissions are fixed.
> 2. The final ack (was missing) is now present during the exchange
> 3way handshake during TFO cookie request.
> 
> Work is in process...

Thank you for progressing on this.

Could you please additionally share your WIP code? Even if in an early
draft status, sharing it could help detecting eventual pitfalls (e.g.
the non-fastopen case should be impacted as little as possible)

Thanks!

Paolo