feat: take the socket options callers actually pass

guides/configuration.md

@@ -44,11 +44,21 @@ for where that is and is not true. `inet` is the driver, and is one option away
 for a machine or a workload that disagrees.
 
 The two transports take the same `socket_options`, but the socket transport has
-no driver behind it to hold the ones it does not use. `inet` and `inet6` choose
-which address a hostname resolves to; `nodelay`, `keepalive`, `recbuf` and
-`sndbuf` are set on the socket; `binary`, `{packet, raw}`, `{active, false}` and
-`buffer` are accepted and ignored; anything else is refused at connect rather
-than silently dropped.
+no driver behind it to hold the ones it does not use, so it sorts them by what
+they decide.
+
+`inet`, `inet6`, `ip` and `port` are read before connecting: the first two
+choose which address a hostname resolves to, the other two the local end to bind
+to. `nodelay`, `keepalive`, `recbuf`, `sndbuf`, `priority`, `reuseaddr`, `tos`,
+`bind_to_device` and `linger` are set on the socket, and `send_timeout` with
+`send_timeout_close` are applied to the write. `binary`, `{packet, raw}`,
+`{active, false}`, `buffer`, `delay_send`, `high_watermark`, `low_watermark`,
+`exit_on_close` and `show_econnreset` are accepted and ignored, because this
+transport already does them or configures a port's send queue that is not there.
+
+Anything else is refused at connect rather than silently dropped. If you hit
+that with an option the driver takes, it is a gap in the list rather than a
+decision - the list is meant to be what callers pass.
 
 `inet6` means v6 or nothing. A host with both an A and a AAAA record resolves to
 v4 unless you say otherwise, and a caller that asked for v6 gets an error rather

src/minato_socket.erl

@@ -86,22 +86,41 @@ Open a connection.
 Takes the option list `m:gen_tcp` would take, so a caller does not have to know
 which transport it got.
 
-`inet` and `inet6` choose the address family a hostname resolves to, as they do
-for the driver. `{nodelay, _}`, `{keepalive, _}`, `{recbuf, _}` and
-`{sndbuf, _}` are set on the socket. `{send_timeout, _}` and
-`{send_timeout_close, _}` are read by `m:minato_conn` and applied to `send/3`,
-because there is no driver here to hold them. `binary`, `{mode, binary}`,
-`{active, false}`, `{packet, raw}` and `{buffer, _}` are what this module always
-does or has no equivalent of, and are accepted and ignored.
+Three kinds of option, and the difference is what the option decides.
+
+**Read before connecting**, because that is the only time they can be honoured:
+`inet` and `inet6` choose which address a hostname resolves to; `{ip, _}` and
+`{port, _}` choose the local end to bind, which is how a multi homed host picks
+the interface the server sees.
+
+**Set on the socket**: `{nodelay, _}`, `{keepalive, _}`, `{recbuf, _}`,
+`{sndbuf, _}`, `{priority, _}`, `{reuseaddr, _}`, `{tos, _}`,
+`{bind_to_device, _}` and `{linger, _}`. `{send_timeout, _}` and
+`{send_timeout_close, _}` belong here in spirit but are read by `m:minato_conn`
+and applied to `send/3`, because there is no driver underneath to hold them.
+
+**Accepted and ignored**, because this module already does them or the thing
+they configure does not exist here: `binary`, `{mode, binary}`,
+`{active, false}`, `{packet, raw}`, `{buffer, _}`, `{delay_send, _}`,
+`{high_watermark, _}`, `{low_watermark, _}`, `{exit_on_close, _}` and
+`{show_econnreset, _}`. The last five configure a port's send queue and a
+port's reporting, and there is no port here.
 
 Anything else is refused rather than silently dropped: this is the default
 transport, and an option that did something through the driver must not quietly
-stop doing it here. That cuts both ways, so the list above is what real callers
-pass rather than what a socket happens to be settable to - `inet` is neither,
-and refusing it broke every deployment that names its address family.
+stop doing it here.
+
+The lists above are what callers pass, not what a socket happens to be settable
+to. Getting that backwards is how `inet` came to be refused, which broke every
+deployment that names its address family - so when a real caller passes
+something missing, the answer is to add it here rather than to tell them to
+stop.
 """.
 -spec connect(
-    inet:socket_address() | inet:hostname(), inet:port_number(), [term()], timeout()
+    inet:socket_address() | inet:hostname(),
+    inet:port_number(),
+    [gen_tcp:connect_option()],
+    timeout()
 ) -> {ok, socket()} | {error, term()}.
 connect(Host, Port, Opts, Timeout) ->
     case address(Host, Port, family(Opts)) of
@@ -110,7 +129,7 @@ connect(Host, Port, Opts, Timeout) ->
     end.
 
 -doc false.
--spec family([term()]) -> inet | inet6 | any.
+-spec family([gen_tcp:connect_option()]) -> inet | inet6 | any.
 %% `inet` and `inet6` are not options a socket is set to, which is why asking
 %% `setopt/2` about them was the wrong question: they choose which address a
 %% name resolves to, and the driver reads them before it connects. `any` is
@@ -280,62 +299,155 @@ setopts(_Socket, []) ->
     ok.
 
 -spec setopt(socket(), term()) -> ok | {error, term()}.
-setopt(_Socket, {active, false}) -> ok;
-setopt(_Socket, {active, _Mode}) -> {error, active_mode_unsupported};
-setopt(Socket, {nodelay, Value}) -> socket:setopt(Socket, {tcp, nodelay}, Value);
-setopt(Socket, {keepalive, Value}) -> socket:setopt(Socket, {socket, keepalive}, Value);
-setopt(Socket, {recbuf, Value}) -> socket:setopt(Socket, {socket, rcvbuf}, Value);
-setopt(Socket, {sndbuf, Value}) -> socket:setopt(Socket, {socket, sndbuf}, Value);
-setopt(_Socket, binary) -> ok;
-setopt(_Socket, {mode, binary}) -> ok;
-setopt(_Socket, {packet, raw}) -> ok;
-setopt(_Socket, {packet, 0}) -> ok;
-setopt(_Socket, {buffer, _Driver_side}) -> ok;
-setopt(_Socket, inet) -> ok;
-setopt(_Socket, inet6) -> ok;
-setopt(_Socket, {send_timeout, _Read_by_the_connection}) -> ok;
-setopt(_Socket, {send_timeout_close, _Read_by_the_connection}) -> ok;
-setopt(_Socket, Option) -> {error, {unsupported_socket_option, Option}}.
+%% An option the OS does not have is not the caller asking for something wrong,
+%% it is a machine that cannot do it, and the driver does not fail a connection
+%% over that either. `priority` and `tos` are Linux's; Windows answers
+%% `{invalid, {socket_option, _}}` and the connection carries on without them.
+setopt(_Socket, {active, false}) ->
+    ok;
+setopt(_Socket, {active, _Mode}) ->
+    {error, active_mode_unsupported};
+setopt(Socket, {nodelay, Value}) ->
+    supported(socket:setopt(Socket, {tcp, nodelay}, Value));
+setopt(Socket, {keepalive, Value}) ->
+    supported(socket:setopt(Socket, {socket, keepalive}, Value));
+setopt(Socket, {recbuf, Value}) ->
+    supported(socket:setopt(Socket, {socket, rcvbuf}, Value));
+setopt(Socket, {sndbuf, Value}) ->
+    supported(socket:setopt(Socket, {socket, sndbuf}, Value));
+setopt(Socket, {priority, Value}) ->
+    supported(socket:setopt(Socket, {socket, priority}, Value));
+setopt(Socket, {reuseaddr, Value}) ->
+    supported(socket:setopt(Socket, {socket, reuseaddr}, Value));
+setopt(Socket, {tos, Value}) ->
+    supported(socket:setopt(Socket, {ip, tos}, Value));
+setopt(Socket, {bind_to_device, Value}) ->
+    supported(socket:setopt(Socket, {socket, bindtodevice}, Value));
+setopt(Socket, {linger, {OnOff, Seconds}}) ->
+    supported(socket:setopt(Socket, {socket, linger}, #{onoff => OnOff, linger => Seconds}));
+setopt(_Socket, binary) ->
+    ok;
+setopt(_Socket, {mode, binary}) ->
+    ok;
+setopt(_Socket, {packet, raw}) ->
+    ok;
+setopt(_Socket, {packet, 0}) ->
+    ok;
+setopt(_Socket, {buffer, _Driver_side}) ->
+    ok;
+setopt(_Socket, inet) ->
+    ok;
+setopt(_Socket, inet6) ->
+    ok;
+setopt(_Socket, {ip, _Bound_before_connecting}) ->
+    ok;
+setopt(_Socket, {port, _Bound_before_connecting}) ->
+    ok;
+setopt(_Socket, {delay_send, _No_send_queue_here}) ->
+    ok;
+setopt(_Socket, {high_watermark, _No_send_queue_here}) ->
+    ok;
+setopt(_Socket, {low_watermark, _No_send_queue_here}) ->
+    ok;
+setopt(_Socket, {exit_on_close, _Not_a_port}) ->
+    ok;
+setopt(_Socket, {show_econnreset, _Driver_reporting}) ->
+    ok;
+setopt(_Socket, {send_timeout, _Read_by_the_connection}) ->
+    ok;
+setopt(_Socket, {send_timeout_close, _Read_by_the_connection}) ->
+    ok;
+setopt(_Socket, Option) ->
+    {error, {unsupported_socket_option, Option}}.
+
+%% An option the OS does not have is not the caller asking for something wrong,
+%% it is a machine that cannot do it, and the driver does not fail a connection
+%% over that either. `priority` and `tos` are Linux's, and Windows answers
+%% `{invalid, {socket_option, _}}` rather than setting them.
+-spec supported(ok | {error, term()}) -> ok | {error, term()}.
+supported({error, {invalid, {socket_option, _Not_on_this_machine}}}) -> ok;
+supported(Answer) -> Answer.
 
 %%----------------------------------------------------------------------
 %% Opening
 %%----------------------------------------------------------------------
 
--spec opened(socket:domain(), socket:sockaddr(), [term()], timeout()) ->
+-spec opened(socket:domain(), socket:sockaddr(), [gen_tcp:connect_option()], timeout()) ->
     {ok, socket()} | {error, term()}.
 opened(Domain, Address, Opts, Timeout) ->
     case socket:open(Domain, stream, tcp) of
         {ok, Socket} -> started(Socket, Address, Opts, Timeout);
         {error, Reason} -> {error, Reason}
     end.
 
--spec started(socket(), socket:sockaddr(), [term()], timeout()) ->
+-spec started(socket(), socket:sockaddr(), [gen_tcp:connect_option()], timeout()) ->
     {ok, socket()} | {error, term()}.
 started(Socket, Address, Opts, Timeout) ->
-    case bound(Socket, Address) of
+    case bound(Socket, Address, local(Opts)) of
         ok -> connecting(Socket, Address, Opts, Timeout);
         {error, Reason} -> failed(Socket, Reason)
     end.
 
 -doc false.
--spec bound(socket(), socket:sockaddr()) -> ok | {error, term()}.
+-spec bound(socket(), socket:sockaddr(), socket:sockaddr() | any | loopback) ->
+    ok | {error, term()}.
 %% Windows refuses to connect a socket that was never bound, and answers
 %% `not_bound` rather than binding it for you. Binding to the wildcard is what
 %% the driver does on every machine, and costs nothing where it was not needed.
-bound(Socket, #{family := Family}) when Family =:= inet; Family =:= inet6 ->
-    socket:bind(Socket, any);
-bound(_Socket, _Local) ->
+%%
+%% `{ip, _}` and `{port, _}` are the caller asking for a particular local end
+%% instead, which a multi homed host uses to choose which interface the server
+%% sees. They are read before the connect because that is when a bind is
+%% possible at all.
+bound(Socket, #{family := Family}, Local) when Family =:= inet; Family =:= inet6 ->
+    socket:bind(Socket, Local);
+bound(_Socket, _Address, _Local) ->
     ok.
 
--spec connecting(socket(), socket:sockaddr(), [term()], timeout()) ->
+-spec local([gen_tcp:connect_option()]) -> socket:sockaddr() | any | loopback.
+local(Opts) ->
+    bound_to(local(ip, Opts), local(port, Opts)).
+
+-spec bound_to(inet:socket_address() | undefined, inet:port_number() | undefined) ->
+    socket:sockaddr() | any | loopback.
+bound_to(undefined, undefined) ->
+    any;
+bound_to(undefined, Port) ->
+    #{family => inet, addr => any, port => Port};
+bound_to(Address, undefined) ->
+    local_address(Address, 0);
+bound_to(Address, Port) ->
+    local_address(Address, Port).
+
+-spec local_address(inet:socket_address(), inet:port_number()) ->
+    socket:sockaddr() | any | loopback.
+local_address({local, Path}, _Port) ->
+    #{family => local, path => iolist_to_binary(Path)};
+local_address(any, Port) ->
+    #{family => inet, addr => any, port => Port};
+local_address(loopback, Port) ->
+    #{family => inet, addr => loopback, port => Port};
+local_address(Address, Port) ->
+    {_Domain, Sockaddr} = target(Address, Port),
+    Sockaddr.
+
+-spec local
+    (ip, [gen_tcp:connect_option()]) -> inet:socket_address() | undefined;
+    (port, [gen_tcp:connect_option()]) -> inet:port_number() | undefined.
+local(ip, [{ip, Address} | _Rest]) -> Address;
+local(port, [{port, Port} | _Rest]) -> Port;
+local(Name, [_Other | Rest]) -> local(Name, Rest);
+local(_Name, []) -> undefined.
+
+-spec connecting(socket(), socket:sockaddr(), [gen_tcp:connect_option()], timeout()) ->
     {ok, socket()} | {error, term()}.
 connecting(Socket, Address, Opts, Timeout) ->
     case socket:connect(Socket, Address, Timeout) of
         ok -> tuned(Socket, Opts);
         {error, Reason} -> failed(Socket, Reason)
     end.
 
--spec tuned(socket(), [term()]) -> {ok, socket()} | {error, term()}.
+-spec tuned(socket(), [gen_tcp:connect_option()]) -> {ok, socket()} | {error, term()}.
 tuned(Socket, Opts) ->
     case setopts(Socket, Opts) of
         ok -> {ok, Socket};

test/minato_transport_SUITE.erl

@@ -34,6 +34,8 @@ Skips when no server answers.
     an_address_family_is_chosen_not_refused/1,
     an_address_family_is_not_quietly_swapped/1,
     a_send_to_a_peer_that_stopped_reading_gives_up/1,
+    every_socket_option_the_list_claims_is_taken/1,
+    a_local_address_is_bound_not_ignored/1,
     an_option_this_transport_cannot_honour_is_refused/1,
     a_pool_lends_a_connection_and_takes_it_back/1,
     a_pooled_connection_outlives_the_process_that_opened_it/1,
@@ -68,6 +70,8 @@ cases() ->
         an_address_family_is_chosen_not_refused,
         an_address_family_is_not_quietly_swapped,
         a_send_to_a_peer_that_stopped_reading_gives_up,
+        every_socket_option_the_list_claims_is_taken,
+        a_local_address_is_bound_not_ignored,
         an_option_this_transport_cannot_honour_is_refused,
         a_pool_lends_a_connection_and_takes_it_back,
         a_pooled_connection_outlives_the_process_that_opened_it,
@@ -212,15 +216,68 @@ filled(Conn, Bytes, Left) ->
         {error, Reason} -> {error, Reason}
     end.
 
+%% The whole list, in one connection, on both transports. A driver option this
+%% transport quietly dropped was the defect that took an outage to find, and a
+%% driver option it refuses is the same defect wearing the other coat: either
+%% way the caller asked for something and did not get it. The cheapest guard is
+%% to keep passing everything real callers pass.
+every_socket_option_the_list_claims_is_taken(Config) ->
+    Opts = (opts(Config))#{
+        socket_options => [
+            inet,
+            {ip, {127, 0, 0, 1}},
+            {nodelay, true},
+            {keepalive, true},
+            {recbuf, 65536},
+            {sndbuf, 65536},
+            {priority, 0},
+            {reuseaddr, true},
+            {tos, 8},
+            {linger, {true, 5}},
+            {send_timeout, 5000},
+            {send_timeout_close, true},
+            {delay_send, true},
+            {high_watermark, 131072},
+            {low_watermark, 65536},
+            {exit_on_close, true},
+            {buffer, 65536},
+            binary,
+            {packet, raw},
+            {active, false}
+        ]
+    },
+    {ok, Conn} = minato_conn:connect(Opts),
+    {ok, #{rows := [{1}]}, Ready} = minato_query:query(Conn, ~"SELECT $1::int4", [1]),
+    ok = minato_conn:close(Ready).
+
+%% `{ip, _}` is bound rather than accepted and forgotten. Asking the server which
+%% address it sees would prove nothing through a NAT, and a container is one, so
+%% the proof is an address this machine cannot have: binding it has to fail.
+%% TEST-NET-1 is reserved and never assigned.
+a_local_address_is_bound_not_ignored(Config) ->
+    Opts = (opts(Config))#{socket_options => [{ip, {192, 0, 2, 1}}]},
+    ?assertMatch({error, {socket, eaddrnotavail}}, minato_conn:connect(Opts)).
+
 an_option_this_transport_cannot_honour_is_refused(Config) ->
-    Opts = (opts(Config))#{socket_options => [{priority, 3}]},
-    ok = judged(transport(Config), minato_conn:connect(Opts)).
+    Opts = (opts(Config))#{socket_options => [{netns, "/var/run/netns/none"}]},
+    Answer =
+        try minato_conn:connect(Opts) of
+            Result -> Result
+        catch
+            _Class:Raised -> {raised, Raised}
+        end,
+    ok = judged(transport(Config), Answer).
 
 %% The refusal is the point: an option that did something through the driver has
 %% to either keep doing it or say that it stopped, never quietly become nothing.
 %% Which option this is matters less than that the answer is loud - and the list
 %% of what is honoured grows as real callers turn out to pass things.
-judged(socket, {error, {socket, {unsupported_socket_option, {priority, 3}}}}) -> ok;
+%% The driver refuses it by answering on one machine and by raising on another,
+%% and either is a refusal. What this case is about is the transport that has to
+%% decide for itself.
+judged(socket, {error, {socket, {unsupported_socket_option, {netns, _Namespace}}}}) -> ok;
+judged(inet, {error, _Answered}) -> ok;
+judged(inet, {raised, _Refused}) -> ok;
 judged(inet, {ok, Conn}) -> minato_conn:close(Conn).
 
 %%----------------------------------------------------------------------