From a0579152c6d774f9e9e1c05bd8e441a4ae80c833 Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Tue, 11 Aug 2026 13:08:06 +0200 Subject: [PATCH 1/2] feat: take the socket options callers actually pass The list was what a socket happens to be settable to. That is how `inet` came to be refused, and the fix for it left the same shape behind: `{ip, _}`, `{linger, _}`, `{priority, _}` and friends were still refused, waiting for the first caller who passes one. Sorted by what the option decides rather than by what it looks like. Read before connecting, because that is the only moment they mean anything: `inet`, `inet6`, and now `{ip, _}` and `{port, _}`, which choose the local end to bind - how a multi homed host picks the interface the server sees. Set on the socket: the four that already were, and now `{priority, _}`, `{reuseaddr, _}`, `{tos, _}`, `{bind_to_device, _}` and `{linger, _}`, which takes `m:gen_tcp`'s `{OnOff, Seconds}` and gives the socket module the struct the OS wants. Accepted and ignored, and said out loud: `{delay_send, _}`, `{high_watermark, _}`, `{low_watermark, _}`, `{exit_on_close, _}` and `{show_econnreset, _}` configure a port's send queue and a port's reporting. There is no port here. Anything else is still refused rather than dropped. The doc now says that a refusal for an option the driver takes is a gap in this list rather than a decision. Taking `[gen_tcp:connect_option()]` rather than `[term()]` is what let the option values be typed at all, which is how `{ip, _}` turned out to be wider than an address: `any`, `loopback` and `{local, Path}` are all legal and now handled. --- guides/configuration.md | 20 +++- src/minato_socket.erl | 174 +++++++++++++++++++++++++------- test/minato_transport_SUITE.erl | 51 +++++++++- 3 files changed, 201 insertions(+), 44 deletions(-) diff --git a/guides/configuration.md b/guides/configuration.md index 77b7c43..d9e1b26 100644 --- a/guides/configuration.md +++ b/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 diff --git a/src/minato_socket.erl b/src/minato_socket.erl index b584bb4..c840bae 100644 --- a/src/minato_socket.erl +++ b/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,28 +299,68 @@ 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}}. +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, {priority, Value}) -> + socket:setopt(Socket, {socket, priority}, Value); +setopt(Socket, {reuseaddr, Value}) -> + socket:setopt(Socket, {socket, reuseaddr}, Value); +setopt(Socket, {tos, Value}) -> + socket:setopt(Socket, {ip, tos}, Value); +setopt(Socket, {bind_to_device, Value}) -> + socket:setopt(Socket, {socket, bindtodevice}, Value); +setopt(Socket, {linger, {OnOff, Seconds}}) -> + 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}}. %%---------------------------------------------------------------------- %% 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 @@ -309,25 +368,66 @@ opened(Domain, 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 @@ -335,7 +435,7 @@ connecting(Socket, Address, Opts, Timeout) -> {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}; diff --git a/test/minato_transport_SUITE.erl b/test/minato_transport_SUITE.erl index 0b582a2..a1d0139 100644 --- a/test/minato_transport_SUITE.erl +++ b/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,58 @@ 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}]}, + Opts = (opts(Config))#{socket_options => [{netns, "/var/run/netns/none"}]}, ok = judged(transport(Config), minato_conn:connect(Opts)). %% 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; +judged(socket, {error, {socket, {unsupported_socket_option, {netns, _Namespace}}}}) -> ok; +judged(inet, {error, _Driver_refuses_it_too}) -> ok; judged(inet, {ok, Conn}) -> minato_conn:close(Conn). %%---------------------------------------------------------------------- From 5f719f2d2f4f438602c9893f1130ff8ac210a2dd Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Tue, 11 Aug 2026 13:33:46 +0200 Subject: [PATCH 2/2] fix: an option this machine lacks is not an option the caller got wrong Windows found both of these, which is what it is there for. `priority` and `tos` are Linux's. Setting them on Windows answers `{invalid, {socket_option, _}}`, and refusing the connection over that would mean a config that works on one machine breaks on another - the same shape as refusing `inet`, one layer along. The driver does not fail a connection over an option the OS has never heard of, and neither does this now. The refusal case assumed the driver answers `{error, _}` for an option it will not take. On Windows it raises instead. Either is a refusal; what the case is about is the transport that has to decide for itself. --- src/minato_socket.erl | 30 +++++++++++++++++++++--------- test/minato_transport_SUITE.erl | 14 ++++++++++++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/minato_socket.erl b/src/minato_socket.erl index c840bae..3ffe008 100644 --- a/src/minato_socket.erl +++ b/src/minato_socket.erl @@ -299,28 +299,32 @@ setopts(_Socket, []) -> ok. -spec setopt(socket(), term()) -> ok | {error, term()}. +%% 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}) -> - socket:setopt(Socket, {tcp, nodelay}, Value); + supported(socket:setopt(Socket, {tcp, nodelay}, Value)); setopt(Socket, {keepalive, Value}) -> - socket:setopt(Socket, {socket, keepalive}, Value); + supported(socket:setopt(Socket, {socket, keepalive}, Value)); setopt(Socket, {recbuf, Value}) -> - socket:setopt(Socket, {socket, rcvbuf}, Value); + supported(socket:setopt(Socket, {socket, rcvbuf}, Value)); setopt(Socket, {sndbuf, Value}) -> - socket:setopt(Socket, {socket, sndbuf}, Value); + supported(socket:setopt(Socket, {socket, sndbuf}, Value)); setopt(Socket, {priority, Value}) -> - socket:setopt(Socket, {socket, priority}, Value); + supported(socket:setopt(Socket, {socket, priority}, Value)); setopt(Socket, {reuseaddr, Value}) -> - socket:setopt(Socket, {socket, reuseaddr}, Value); + supported(socket:setopt(Socket, {socket, reuseaddr}, Value)); setopt(Socket, {tos, Value}) -> - socket:setopt(Socket, {ip, tos}, Value); + supported(socket:setopt(Socket, {ip, tos}, Value)); setopt(Socket, {bind_to_device, Value}) -> - socket:setopt(Socket, {socket, bindtodevice}, Value); + supported(socket:setopt(Socket, {socket, bindtodevice}, Value)); setopt(Socket, {linger, {OnOff, Seconds}}) -> - socket:setopt(Socket, {socket, linger}, #{onoff => OnOff, linger => Seconds}); + supported(socket:setopt(Socket, {socket, linger}, #{onoff => OnOff, linger => Seconds})); setopt(_Socket, binary) -> ok; setopt(_Socket, {mode, binary}) -> @@ -356,6 +360,14 @@ setopt(_Socket, {send_timeout_close, _Read_by_the_connection}) -> 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 %%---------------------------------------------------------------------- diff --git a/test/minato_transport_SUITE.erl b/test/minato_transport_SUITE.erl index a1d0139..f88194a 100644 --- a/test/minato_transport_SUITE.erl +++ b/test/minato_transport_SUITE.erl @@ -260,14 +260,24 @@ a_local_address_is_bound_not_ignored(Config) -> an_option_this_transport_cannot_honour_is_refused(Config) -> Opts = (opts(Config))#{socket_options => [{netns, "/var/run/netns/none"}]}, - ok = judged(transport(Config), minato_conn:connect(Opts)). + 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. +%% 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, _Driver_refuses_it_too}) -> ok; +judged(inet, {error, _Answered}) -> ok; +judged(inet, {raised, _Refused}) -> ok; judged(inet, {ok, Conn}) -> minato_conn:close(Conn). %%----------------------------------------------------------------------