[gnutls-devel] [PATCH] Re: TCP Fast Open

[Nikos Mavrogiannopoulos]

On Thu, Jul 14, 2016 at 12:45 PM, Tim Ruehsen <tim.ruehsen at gmx.de> wrote:
> Here is my patch, first version.
>
> Please review and comment.
>
> - I have no platform without TFO to test with... so compilation might break on
> such platforms, could anyone give it a try ?

I've committed it on a special branch and run through the CI. The
windows and freebsd builds fail:
https://gitlab.com/gnutls/gnutls/builds/2370926
https://gitlab.com/gnutls/gnutls/builds/2370929

> - Please check the docs... I am unsure about completeness

It would be best to mention that optimization in the manual, while
also discussing other optimizations such as false start and session
resumption. I could take care of that.

> - To check the timings / test TFO, I added 'return 0;' in line 1245 of src/
> cli.c and run it with and without --fastopen against www.google.com 443.
> I have ~35ms ping here and had a reproducable gain of ~32ms when using --
> fastopen (152ms vs. 120ms).

That's quite nice. Have you noticed firewalls or middle boxes blocking
that approach? Is there anything we can fix for them or fallback?

> @@ -496,8 +496,17 @@ _gnutls_writev(gnutls_session_t session, const giovec_t * giovec,
>         }
>
>         if (no_writev == 0) {
> -            i = session->internals.vec_push_func(fd, giovec,
> -                                 giovec_cnt);
> +            if (session->internals.connect_addr) {
> +                if (session->internals.vec_push_func == system_writev)
> +                    i = system_writev_tfo(session, giovec, giovec_cnt);
> +#ifdef MSG_NOSIGNAL
> +                else if (session->internals.vec_push_func == system_writev_nosignal)
> +                    i = system_writev_nosignal_tfo(session, giovec, giovec_cnt);
> +#endif
> +                else
> +                    i = session->internals.vec_push_func(fd, giovec, giovec_cnt);
> +            } else
> +                i = session->internals.vec_push_func(fd, giovec, giovec_cnt);

This path is a bit worrying. Why not have
gnutls_transport_set_fastopen() replace all the pull/push functions
with its own version?
A test program utilizing this code path should also be added to avoid
breaking that functionality in the future.

regards,
Nikos