File 2836-Move-generic-hs_data-creation-to-inet_tcp_... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 2836-Move-generic-hs_data-creation-to-inet_tcp_dist.patch of Package erlang (Revision 3f96b7cdcbb0d4b7e1511f2bc0d71906)

Currently displaying revision 3f96b7cdcbb0d4b7e1511f2bc0d71906 , Show latest

From b9af2bd7849e019ac8abadbcd2f6c0298c116885 Mon Sep 17 00:00:00 2001
From: Raimo Niskanen <raimo@erlang.org>
Date: Wed, 17 Aug 2022 16:00:27 +0200
Subject: [PATCH 06/11] Move generic #hs_data{} creation to inet_tcp_dist

Create functions gen_hs_data/2 and nodelay/0 as OTP
internal in Kernel/inet_tcp_dist to be used by
e.g SSL/inet_tls_dist when creating a #hs_data{}
distribution handshake record.

This simplifies code both in inet_tcp_dist and in
inet_tls_dist.

Clarify where the distribution ?PROTOCOL name
shall be used (in #net_address.protocol, and in
the {accept, self, _, _, ?PROTOCOL} message).
---
 lib/kernel/src/inet_tcp_dist.erl  | 148 +++++++++++++---------------
 lib/ssl/src/inet_tls_dist.erl     | 156 +++++++++++-------------------
 lib/ssl/test/inet_crypto_dist.erl |  24 +----
 3 files changed, 129 insertions(+), 199 deletions(-)

diff --git a/lib/kernel/src/inet_tcp_dist.erl b/lib/kernel/src/inet_tcp_dist.erl
index b6a0cec0ab..968d286c3d 100644
--- a/lib/kernel/src/inet_tcp_dist.erl
+++ b/lib/kernel/src/inet_tcp_dist.erl
@@ -30,6 +30,8 @@
 %% Generalized dist API
 -export([gen_listen/3, gen_accept/2, gen_accept_connection/6,
 	 gen_setup/6, gen_select/2, gen_address/1]).
+%% OTP internal (e.g ssl)
+-export([gen_hs_data/2, nodelay/0]).
 
 %% internal exports
 
@@ -42,6 +44,8 @@
 -include("dist.hrl").
 -include("dist_util.hrl").
 
+-define(PROTOCOL, tcp).
+
 %% ------------------------------------------------------------
 %%  Select this protocol based on node name
 %%  select(Node) => Bool
@@ -71,6 +75,34 @@ address() ->
 gen_address(Driver) ->
     get_tcp_address(Driver).
 
+%% ------------------------------------------------------------
+%% Set up the general fields in #hs_data{}
+%% ------------------------------------------------------------
+gen_hs_data(Driver, Socket) ->
+    Nodelay = nodelay(),
+    #hs_data{
+       socket = Socket,
+       f_send = fun Driver:send/2,
+       f_recv = fun Driver:recv/3,
+       f_setopts_pre_nodeup =
+           fun (S) ->
+                   inet:setopts(
+                     S,
+                     [{active, false}, {packet, 4}, Nodelay])
+           end,
+       f_setopts_post_nodeup =
+           fun (S) ->
+                   inet:setopts(
+                     S,
+                     [{active, true}, {packet,4},
+                      {deliver, port}, binary, Nodelay])
+           end,
+       f_getll    = fun inet:getll/1,
+       mf_tick    = fun (S) -> ?MODULE:tick(Driver, S) end,
+       mf_getstat = fun ?MODULE:getstat/1,
+       mf_setopts = fun ?MODULE:setopts/2,
+       mf_getopts = fun ?MODULE:getopts/2}.
+
 %% ------------------------------------------------------------
 %% Create the listen socket, i.e. the port that this erlang
 %% node is accessible through.
@@ -167,7 +199,7 @@ gen_accept(Driver, Listen) ->
 accept_loop(Driver, Kernel, Listen) ->
     case Driver:accept(Listen) of
 	{ok, Socket} ->
-	    Kernel ! {accept,self(),Socket,Driver:family(),tcp},
+	    Kernel ! {accept,self(),Socket,Driver:family(),?PROTOCOL},
 	    _ = controller(Driver, Kernel, Socket),
 	    accept_loop(Driver, Kernel, Listen);
 	Error ->
@@ -216,40 +248,19 @@ do_accept(Driver, Kernel, AcceptPid, Socket, MyNode, Allowed, SetupTime) ->
 	    Timer = dist_util:start_timer(SetupTime),
 	    case check_ip(Driver, Socket) of
 		true ->
-		    HSData = #hs_data{
-		      kernel_pid = Kernel,
-		      this_node = MyNode,
-		      socket = Socket,
-		      timer = Timer,
-		      this_flags = 0,
-		      allowed = Allowed,
-		      f_send = fun Driver:send/2,
-		      f_recv = fun Driver:recv/3,
-		      f_setopts_pre_nodeup = 
-		      fun(S) ->
-			      inet:setopts(S, 
-					   [{active, false},
-					    {packet, 4},
-					    nodelay()])
-		      end,
-		      f_setopts_post_nodeup = 
-		      fun(S) ->
-			      inet:setopts(S, 
-					   [{active, true},
-					    {deliver, port},
-					    {packet, 4},
-                                            binary,
-					    nodelay()])
-		      end,
-		      f_getll = fun(S) ->
-					inet:getll(S)
-				end,
-		      f_address = fun(S, Node) -> get_remote_id(Driver, S, Node) end,
-		      mf_tick = fun(S) -> ?MODULE:tick(Driver, S) end,
-		      mf_getstat = fun ?MODULE:getstat/1,
-		      mf_setopts = fun ?MODULE:setopts/2,
-		      mf_getopts = fun ?MODULE:getopts/2
-		     },
+                    Family = Driver:family(),
+                    HSData =
+                        (gen_hs_data(Driver, Socket))
+                        #hs_data{
+                          kernel_pid = Kernel,
+                          this_node = MyNode,
+                          timer = Timer,
+                          this_flags = 0,
+                          allowed = Allowed,
+                          f_address =
+                              fun (S, Node) ->
+                                      get_remote_id(Family, S, Node)
+                              end},
 		    dist_util:handshake_other_started(HSData);
 		{false,IP} ->
 		    error_msg("** Connection attempt from "
@@ -277,13 +288,13 @@ nodelay() ->
 %% ------------------------------------------------------------
 %% Get remote information about a Socket.
 %% ------------------------------------------------------------
-get_remote_id(Driver, Socket, Node) ->
+get_remote_id(Family, Socket, Node) ->
     case inet:peername(Socket) of
 	{ok,Address} ->
 	    case split_node(atom_to_list(Node), $@, []) of
 		[_,Host] ->
 		    #net_address{address=Address,host=Host,
-				 protocol=tcp,family=Driver:family()};
+				 protocol=?PROTOCOL,family=Family};
 		_ ->
 		    %% No '@' or more than one '@' in node name.
 		    ?shutdown(no_node)
@@ -346,49 +357,24 @@ do_setup_connect(Driver, Kernel, Node, Address, AddressFamily,
 	  connect_options([{active, false}, {packet, 2}]))
 	of
 	{ok, Socket} ->
-		HSData = #hs_data{
-		  kernel_pid = Kernel,
-		  other_node = Node,
-		  this_node = MyNode,
-		  socket = Socket,
-		  timer = Timer,
-		  this_flags = 0,
-		  other_version = Version,
-		  f_send = fun Driver:send/2,
-		  f_recv = fun Driver:recv/3,
-		  f_setopts_pre_nodeup =
-		  fun(S) ->
-			  inet:setopts
-			(S,
-			 [{active, false},
-			  {packet, 4},
-			  nodelay()])
-		  end,
-		  f_setopts_post_nodeup =
-		  fun(S) ->
-			  inet:setopts
-			(S,
-			 [{active, true},
-			  {deliver, port},
-			  {packet, 4},
-			  nodelay()])
-		  end,
-
-		  f_getll = fun inet:getll/1,
-		  f_address =
-		  fun(_,_) ->
-			  #net_address{
-		   address = {Ip,TcpPort},
-		   host = Address,
-		   protocol = tcp,
-		   family = AddressFamily}
-		  end,
-		  mf_tick = fun(S) -> ?MODULE:tick(Driver, S) end,
-		  mf_getstat = fun ?MODULE:getstat/1,
-		  request_type = Type,
-		  mf_setopts = fun ?MODULE:setopts/2,
-		  mf_getopts = fun ?MODULE:getopts/2
-		 },
+                HSData =
+                    (gen_hs_data(Driver, Socket))
+                    #hs_data{
+                      kernel_pid = Kernel,
+                      other_node = Node,
+                      this_node = MyNode,
+                      timer = Timer,
+                      this_flags = 0,
+                      other_version = Version,
+                      f_address =
+                          fun(_,_) ->
+                                  #net_address{
+                                     address = {Ip,TcpPort},
+                                     host = Address,
+                                     protocol = ?PROTOCOL,
+                                     family = AddressFamily}
+                          end,
+                      request_type = Type},
 		dist_util:handshake_we_started(HSData);
 	_ ->
 		%% Other Node may have closed since
@@ -465,7 +451,7 @@ get_tcp_address(Driver) ->
     {ok, Host} = inet:gethostname(),
     #net_address {
 		  host = Host,
-		  protocol = tcp,
+		  protocol = ?PROTOCOL,
 		  family = Driver:family()
 		 }.
 
diff --git a/lib/ssl/src/inet_tls_dist.erl b/lib/ssl/src/inet_tls_dist.erl
index 6cc10c3bf4..be0357240e 100644
--- a/lib/ssl/src/inet_tls_dist.erl
+++ b/lib/ssl/src/inet_tls_dist.erl
@@ -29,8 +29,6 @@
 -export([gen_listen/3, gen_accept/2, gen_accept_connection/6,
 	 gen_setup/6, gen_close/2, gen_select/2, gen_address/1]).
 
--export([nodelay/0]).
-
 -export([verify_client/3, cert_nodes/1]).
 
 -export([dbg/0]). % Debug
@@ -45,6 +43,8 @@
 -include("ssl_internal.hrl").
 -include_lib("kernel/include/logger.hrl").
 
+-define(PROTOCOL, tls).
+
 %% -------------------------------------------------------------------------
 
 childspecs() ->
@@ -72,53 +72,25 @@ is_node_name(Node) ->
 
 %% -------------------------------------------------------------------------
 
-hs_data_common(Socket) when is_port(Socket) ->
-    {ok, {Ip, Port}} = inet:peername(Socket),
+hs_data_inet_tcp(Driver, Socket) ->
+    Family = Driver:family(),
+    {ok, Peername} = inet:peername(Socket),
+    (inet_tcp_dist:gen_hs_data(Driver, Socket))
     #hs_data{
-        socket = Socket,
-        f_send = fun inet_tcp:send/2,
-        f_recv = fun inet_tcp:recv/3,
-        f_setopts_pre_nodeup =
-            fun(S) ->
-                inet:setopts(
-                    S,
-                    [
-                        {active, false},
-                        {packet, 4},
-                        nodelay()
-                    ]
-                )
-            end,
-        f_setopts_post_nodeup =
-            fun(S) ->
-                inet:setopts(
-                    S,
-                    [
-                        {active, true},
-                        {deliver, port},
-                        {packet, 4},
-                        nodelay()
-                    ]
-                )
-            end,
-
-        f_getll = fun inet:getll/1,
-        f_address =
-            fun(_, Node) ->
-                {node, _, Host} = dist_util:split_node(Node),
-                #net_address{
-                    address = {Ip, Port},
-                    host = Host,
-                    protocol = tls,
-                    family = inet
-                }
-            end,
-        mf_tick = fun(S) -> inet_tcp_dist:tick(inet_tcp, S) end,
-        mf_getstat = fun inet_tcp_dist:getstat/1,
-        mf_setopts = fun inet_tcp_dist:setopts/2,
-        mf_getopts = fun inet_tcp_dist:getopts/2
-    };
-hs_data_common(#sslsocket{pid = [_, DistCtrl|_]} = SslSocket) ->
+      f_address =
+          fun(_, Node) ->
+                  {node, _, Host} = dist_util:split_node(Node),
+                  #net_address{
+                     address  = Peername,
+                     host     = Host,
+                     protocol = ?PROTOCOL,
+                     family   = Family
+                    }
+          end}.
+
+hs_data_ssl(Driver, #sslsocket{pid = [_, DistCtrl|_]} = SslSocket) ->
+    Family = Driver:family(),
+    {ok, Peername} = ssl:peername(SslSocket),
     #hs_data{
        socket = DistCtrl,
        f_send =
@@ -144,7 +116,7 @@ hs_data_common(#sslsocket{pid = [_, DistCtrl|_]} = SslSocket) ->
            end,
        f_address =
            fun (Ctrl, Node) when Ctrl == DistCtrl ->
-                   f_address(SslSocket, Node)
+                   f_address(Family, Peername, Node)
            end,
        mf_tick =
            fun (Ctrl) when Ctrl == DistCtrl ->
@@ -182,22 +154,19 @@ f_setopts_pre_nodeup(_SslSocket) ->
     ok.
 
 f_setopts_post_nodeup(SslSocket) ->
-    ssl:setopts(SslSocket, [nodelay()]).
+    ssl:setopts(SslSocket, [inet_tcp_dist:nodelay()]).
 
 f_getll(DistCtrl) ->
     {ok, DistCtrl}.
 
-f_address(SslSocket, Node) ->
-    case ssl:peername(SslSocket) of
-        {ok, Address} ->
-            case dist_util:split_node(Node) of
-                {node,_,Host} ->
-                    #net_address{
-                       address=Address, host=Host,
-                       protocol=tls, family=inet};
-                _ ->
-                    {error, no_node}
-            end
+f_address(Family, Address, Node) ->
+    case dist_util:split_node(Node) of
+        {node,_,Host} ->
+            #net_address{
+               address=Address, host=Host,
+               protocol=?PROTOCOL, family=Family};
+        _ ->
+            {error, no_node}
     end.
 
 mf_tick(DistCtrl) ->
@@ -249,7 +218,7 @@ gen_listen(Driver, Name, Host) ->
     case inet_tcp_dist:gen_listen(Driver, Name, Host) of
         {ok, {Socket, Address, Creation}} ->
             inet:setopts(Socket, [{packet, 4}, {nodelay, true}]),
-            {ok, {Socket, Address#net_address{protocol=tls}, Creation}};
+            {ok, {Socket, Address#net_address{protocol=?PROTOCOL}, Creation}};
         Other ->
             Other
     end.
@@ -365,7 +334,7 @@ accept_one(Driver, Kernel, Socket) ->
     end.
 %%
 accept_one(Driver, Kernel, DistCtrl, ControllingProcessFun, DistSocket) ->
-    trace(Kernel ! {accept, self(), DistCtrl, Driver:family(), tls}),
+    trace(Kernel ! {accept, self(), DistCtrl, Driver:family(), ?PROTOCOL}),
     receive
         {Kernel, controller, Pid} ->
             case ControllingProcessFun(DistSocket, Pid) of
@@ -514,20 +483,23 @@ gen_accept_connection(
         dist_util:net_ticker_spawn_options())).
 
 do_accept(
-  _Driver, AcceptPid, DistCtrl, MyNode, Allowed, SetupTime, Kernel) ->
+  Driver, AcceptPid, DistCtrl, MyNode, Allowed, SetupTime, Kernel) ->
     MRef = erlang:monitor(process, AcceptPid),
     receive
 	{AcceptPid, controller} ->
             erlang:demonitor(MRef, [flush]),
             Timer = dist_util:start_timer(SetupTime),
-            {HSData0, NewAllowed} = case is_port(DistCtrl) of
-                true ->
-                    {hs_data_common(DistCtrl), Allowed};
-                false ->
-                    {ok, SslSocket} = tls_sender:dist_tls_socket(DistCtrl),
-                    link(DistCtrl),
-                    {hs_data_common(SslSocket), allowed_nodes(SslSocket, Allowed)}
-            end,
+            {HSData0, NewAllowed} =
+                case is_port(DistCtrl) of
+                    true ->
+                        {hs_data_inet_tcp(Driver, DistCtrl),
+                         Allowed};
+                    false ->
+                        {ok, SslSocket} = tls_sender:dist_tls_socket(DistCtrl),
+                        link(DistCtrl),
+                        {hs_data_ssl(Driver, SslSocket),
+                         allowed_nodes(SslSocket, Allowed)}
+                end,
             HSData =
                 HSData0#hs_data{
                   kernel_pid = Kernel,
@@ -657,12 +629,13 @@ do_setup_connect(Driver, Kernel, Node, Address, Ip, TcpPort, Version, Type, MyNo
     dist_util:reset_timer(Timer),
     case ssl:connect(
         Ip, TcpPort,
-        [binary, {active, false}, {packet, 4}, {server_name_indication, Address},
+        [binary, {active, false}, {packet, 4},
+         {server_name_indication, Address},
             Driver:family(), {nodelay, true}] ++ Opts,
         net_kernel:connecttime()
     ) of
         {ok, #sslsocket{pid = [Receiver, Sender| _]} = SslSocket} ->
-            DistSocket =
+            HSData =
                 case KTLS of
                     true ->
                         {ok, KtlsInfo} =
@@ -670,7 +643,7 @@ do_setup_connect(Driver, Kernel, Node, Address, Ip, TcpPort, Version, Type, MyNo
                         case set_ktls(KtlsInfo) of
                             ok ->
                                 #{socket := Socket} = KtlsInfo,
-                                Socket;
+                                hs_data_inet_tcp(Driver, Socket);
                             {error, KtlsReason} ->
                                 ?shutdown2(
                                    Node,
@@ -680,10 +653,8 @@ do_setup_connect(Driver, Kernel, Node, Address, Ip, TcpPort, Version, Type, MyNo
                         _ = monitor_pid(Sender),
                         ok = ssl:controlling_process(SslSocket, self()),
                         link(Sender),
-                        SslSocket
-                end,
-            HSData =
-                (hs_data_common(DistSocket))
+                        hs_data_ssl(Driver, SslSocket)
+                end
                 #hs_data{
                   kernel_pid = Kernel,
                   other_node = Node,
@@ -883,21 +854,6 @@ connect_options(Opts) ->
 	    Opts
     end.
 
-%% we may not always want the nodelay behaviour
-%% for performance reasons
-nodelay() ->
-    case application:get_env(kernel, dist_nodelay) of
-	undefined ->
-	    {nodelay, true};
-	{ok, true} ->
-	    {nodelay, true};
-	{ok, false} ->
-	    {nodelay, false};
-	_ ->
-	    {nodelay, true}
-    end.
-
-
 get_ssl_options(Type) ->
     try ets:lookup(ssl_dist_opts, Type) of
         [{Type, Opts0}] ->
@@ -1014,13 +970,17 @@ set_ktls_1(
     %%
     SOL_TCP = 6,
     TCP_ULP = 31,
-    _ = inet:setopts(Socket, [{raw, SOL_TCP, TCP_ULP, <<"tls">>}]),
+    KtlsMod = <<"tls">>, % Linux kernel module name
+    KtlsModSize = byte_size(KtlsMod),
+    _ = inet:setopts(Socket, [{raw, SOL_TCP, TCP_ULP, KtlsMod}]),
     %%
     %% Check if kernel module loaded,
-    %% i.e if getopts SOL_TCP,TCP_ULP returns "tls"
+    %% i.e if getopts SOL_TCP,TCP_ULP returns KtlsMod
     %%
-    case inet:getopts(Socket, [{raw, SOL_TCP, TCP_ULP, 4}]) of
-        {ok, [{raw, SOL_TCP, TCP_ULP, <<"tls",0>>}]} ->
+    case
+        inet:getopts(Socket, [{raw, SOL_TCP, TCP_ULP, KtlsModSize + 1}])
+    of
+        {ok, [{raw, SOL_TCP, TCP_ULP, <<KtlsMod:KtlsModSize/binary,0>>}]} ->
             set_ktls_2(KtlsInfo, Socket);
         Other ->
             {error, {ktls_not_supported, Other}}
diff --git a/lib/ssl/test/inet_crypto_dist.erl b/lib/ssl/test/inet_crypto_dist.erl
index 58a303f48c..d0fa550948 100644
--- a/lib/ssl/test/inet_crypto_dist.erl
+++ b/lib/ssl/test/inet_crypto_dist.erl
@@ -36,8 +36,6 @@
 -export([gen_listen/2, gen_accept/2, gen_accept_connection/6,
 	 gen_setup/6, gen_close/2, gen_select/2]).
 
--export([nodelay/0]).
-
 %% Debug
 %%%-compile(export_all).
 -export([dbg/0, test_server/0, test_client/1]).
@@ -439,7 +437,7 @@ do_accept(
 	{Acceptor, controller, Socket} ->
 	    Timer = dist_util:start_timer(SetupTime),
             HSData =
-                hs_data_common(
+                hs_data(
                   NetKernel, MyNode, DistCtrl, Timer,
                   Socket, Driver:family()),
             HSData_1 =
@@ -529,7 +527,7 @@ do_setup_connect(
                 end,
             %% DistCtrl is a "socket"
             HSData =
-                hs_data_common(
+                hs_data(
                   NetKernel, MyNode, DistCtrl, Timer,
                   Socket, Driver:family()),
             HSData_1 =
@@ -558,7 +556,7 @@ gen_close(Socket, Driver) ->
 %% -------------------------------------------------------------------------
 
 
-hs_data_common(NetKernel, MyNode, DistCtrl, Timer, Socket, Family) ->
+hs_data(NetKernel, MyNode, DistCtrl, Timer, Socket, Family) ->
     %% Field 'socket' below is set to DistCtrl, which makes
     %% the distribution handshake process (ticker) call
     %% the funs below with DistCtrl as the S argument.
@@ -739,20 +737,6 @@ connect_options(Opts) ->
 	    Opts
     end.
 
-%% we may not always want the nodelay behaviour
-%% for performance reasons
-nodelay() ->
-    case application:get_env(kernel, dist_nodelay) of
-	undefined ->
-	    {nodelay, true};
-	{ok, true} ->
-	    {nodelay, true};
-	{ok, false} ->
-	    {nodelay, false};
-	_ ->
-	    {nodelay, true}
-    end.
-
 %% %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%
 %% The DistCtrl process(es).
@@ -1401,7 +1401,7 @@ input_handler(#params{socket = Socket} = Params, Seq, DistHandle) ->
     try
         ok =
             inet:setopts(
-              Socket, [{active, ?TCP_ACTIVE}, nodelay()]),
+              Socket, [{active, ?TCP_ACTIVE}, inet_tcp_dist:nodelay()]),
         input_handler(
           Params#params{dist_handle = DistHandle},
           Seq)
-- 
2.35.3

Places

File 2836-Move-generic-hs_data-creation-to-inet_tcp_dist.patch of Package erlang (Revision 3f96b7cdcbb0d4b7e1511f2bc0d71906)

Places