Thanks for v4! Took a heavier look. Code applies cleanly to
033f39e694872d8d74e670a20093db781eb0bf61, is indented properly, and passes
make check.

doc/src/sgml/libpq.sgml
> <term><literal>MPTCP</literal><indexterm><primary>MultiPath
TCP</primary></indexterm></term>

s/MultiPath/Multipath/ (other places like the commit message too)


> (multiplexing) over mulitple network paths, provided that remote also

s/mulitple/multiple/


src/backend/libpq/pqcomm.c
> if (addr->ai_family != AF_UNIX)
> #ifdef IPPROTO_MPTCP
>   ipprotocol = ListenMPTCP ? IPPROTO_MPTCP : 0;
> #else
>   ipprotocol = 0;
> #endif

It's already 0 from the top of the loop, so it should be enough to do:

#ifdef IPPROTO_MPTCP
  if (ListenMPTCP && addr->ai_family != AF_UNIX)
    socket_protocol = IPPROTO_MPTCP;
#endif


src/backend/utils/misc/postgresql.conf.sample
> #listen_mptcp = off                     # whether to enable Multipathing
TCP or not

s/Multipathing/Multipath/

Or just: # enable Multipath TCP

Can we move to a less prominent place - perhaps under TCP, after
client_connection_check_interval?


src/interfaces/libpq/fe-connect.c
> {"mptcp", "PGMPTCP", "0", NULL,

Does this need freeing in freePGconn()?


src/interfaces/libpq/fe-connect.c
> if (addr_cur->family != AF_UNIX && conn->mptcp && conn->mptcp[0] == '1')
> {
> #ifdef IPPROTO_MPTCP
>   fprintf(stderr, "enabling MPTCP client\n");
>   ip_protocol = IPPROTO_MPTCP;
> #else
>   fprintf(stderr, "MPTCP client is not supported on this platform\n");
> #endif

We should throw a proper message to the user if they attempt mptcp via tcp,
but don't have it enabled by using the libpq_append_conn_error function.
Don't know if a warning for attempting mptcp on via unix socket is worth it.

I'm not clear on the mptcp[0] == '1' bit - does that mean the only way to
invoke it is exactly this?:

PGMPTCP=1

(or I suppose, any other string starting with "1")


Big picture: is it worth making this more generic, in case other protocols
appear some time in the future?

listen_mptcp = on -> listen_protocol = mptcp
PGMPTCP=1 -> PGSOCKETPROTOCOL=mptcp

On re-reading this email, that doesn't allow us to handle different
protocols for different families, so listen_mptcp is fine.


This new ENV should be added to the lists at
src/test/perl/PostgreSQL/Test/Utils.pm
and src/test/regress/pg_regress.c

Ideally also some tests.

Cheers,
Greg

Reply via email to