Merge branch 'bmk/kernel/20221116/gen_udp_opts_processing/OTP-18323' into maint

3 files changed, 193 insertions, 49 deletions

diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c
index 78daf0a8ed..ec5f9a655c 100644
--- a/erts/emulator/drivers/common/inet_drv.c
+++ b/erts/emulator/drivers/common/inet_drv.c
@@ -140,9 +140,13 @@
 #define M2S(__M__)   (((__M__) == INET_MODE_LIST) ? "list" :      \
                       (((__M__) == INET_MODE_BINARY) ? "binary" : \
                        "undefined"))
-#define D2S(__D__)   (((__D__) == INET_DELIVER_PORT) ? "port" :  \
-                      (((__D__) == INET_DELIVER_TERM) ? "term" : \
+#define D2S(__D__)   (((__D__) == INET_DELIVER_PORT) ? "port" :         \
+                      (((__D__) == INET_DELIVER_TERM) ? "term" :        \
                        "undefined"))
+#define DOM2S(__D__)   (((__D__) == INET_AF_INET) ? "inet" :         \
+                        (((__D__) == INET_AF_INET6) ? "inet6" :      \
+                         (((__D__) == INET_AF_LOCAL) ? "local" :     \
+                          "undefined")))
 
 #if defined(__WIN32__) && defined(ARCH_64)
 #define SOCKET_FSTR "%lld"
@@ -7212,8 +7216,9 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
 	case UDP_OPT_ADD_MEMBERSHIP:
             DDBG(desc,
                  ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
-                  "inet_set_opts(add-membership) -> %d\r\n",
-                  __LINE__, desc->s, driver_caller(desc->port), ival) );
+                  "inet_set_opts(add-membership) -> domain: %d (%s)\r\n",
+                  __LINE__, desc->s, driver_caller(desc->port),
+                  ival, DOM2S(ival)) );
             if (ival == INET_AF_INET) {
                 proto = IPPROTO_IP;
                 type  = IP_ADD_MEMBERSHIP;
@@ -7236,8 +7241,9 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
 	case UDP_OPT_DROP_MEMBERSHIP:
             DDBG(desc,
                  ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
-                  "inet_set_opts(drop-membership) -> %d\r\n",
-                  __LINE__, desc->s, driver_caller(desc->port), ival) );
+                  "inet_set_opts(drop-membership) -> %d (%s)\r\n",
+                  __LINE__, desc->s, driver_caller(desc->port),
+                  ival, DOM2S(ival)) );
             if (ival == INET_AF_INET) {
                 proto = IPPROTO_IP;
                 type  = IP_DROP_MEMBERSHIP;
@@ -7264,13 +7270,16 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
                 ptr += 4;
                 len -= 4;
 
-                DEBUGF(("inet_set_opts(L_init_mreq) -> "
-                        "\r\n   proto:  %d"
-                        "\r\n   type:   %d"
-                        "\r\n   domain: %d"
-                        "\r\n   if:     %d"
-                        "\r\n",
-                        proto, type, domain, ival));
+                DDBG(desc,
+                     ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                      "inet_set_opts(L_init_mreq) -> "
+                      "\r\n   proto:  %d"
+                      "\r\n   type:   %d"
+                      "\r\n   domain: %d (%s)"
+                      "\r\n   if:     %d"
+                      "\r\n",
+                      __LINE__, desc->s, driver_caller(desc->port),
+                      proto, type, domain, DOM2S(domain), ival));
 
                 if ((domain == INET_AF_INET) && (desc->sfamily == AF_INET)) {
 
@@ -7283,7 +7292,10 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
                 
 #if defined(HAVE_STRUCT_IP_MREQN)
 
-                    DEBUGF(("inet_set_opts(L_init_mreq,inet) -> mreqn\r\n"));
+                    DDBG(desc,
+                         ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                          "inet_set_opts(L_init_mreq,inet) -> mreqn\r\n",
+                          __LINE__, desc->s, driver_caller(desc->port)));
 
                     mreq4.imr_ifindex          = sock_htonl(ival);
                     ival = get_int32(ptr);
@@ -7291,40 +7303,53 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
                     len -= 4;
                     mreq4.imr_multiaddr.s_addr = sock_htonl(ival);
                     ival = get_int32(ptr);
+                    ptr += 4;
+                    len -= 4;
                     mreq4.imr_address.s_addr   = sock_htonl(ival);
 
-                    DEBUGF(("inet_set_opts(L_init_mreq,inet) -> "
-                            "try setopt: "
-                            "\r\n   maddr: %x"
-                            "\r\n   addr:  %x"
-                            "\r\n   if:    %d"
-                            "\r\n   sz:    %d"
-                            "\r\n",
-                            mreq4.imr_multiaddr.s_addr,
-                            mreq4.imr_address.s_addr,
-                            mreq4.imr_ifindex,
-                            mreqSz));
+                    DDBG(desc,
+                         ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                          "inet_set_opts(L_init_mreq,inet) -> "
+                          "try setopt: "
+                          "\r\n   maddr: %x"
+                          "\r\n   addr:  %x"
+                          "\r\n   if:    %d"
+                          "\r\n   sz:    %d"
+                          "\r\n",
+                          __LINE__, desc->s, driver_caller(desc->port),
+                          mreq4.imr_multiaddr.s_addr,
+                          mreq4.imr_address.s_addr,
+                          mreq4.imr_ifindex,
+                          mreqSz));
 
 #else
 
-                    DEBUGF(("inet_set_opts(L_init_mreq,inet) -> mreq\r\n"));
+                    DDBG(desc,
+                         ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                          "inet_set_opts(L_init_mreq,inet) -> mreq\r\n",
+                          __LINE__, desc->s, driver_caller(desc->port)));
 
                     ival = get_int32(ptr);
                     ptr += 4;
                     len -= 4;
                     mreq4.imr_multiaddr.s_addr = sock_htonl(ival);
                     ival = get_int32(ptr);
+                    ptr += 4;
+                    len -= 4;
                     mreq4.imr_interface.s_addr = sock_htonl(ival);
 
-                    DEBUGF(("inet_set_opts(L_init_mreq,inet) -> "
-                            "try setopt: "
-                            "\r\n   maddr: %x"
-                            "\r\n   if:    %x"
-                            "\r\n   sz:    %d"
-                            "\r\n",
-                            mreq4.imr_multiaddr.s_addr,
-                            mreq4.imr_interface.s_addr,
-                            mreqSz));
+                    DDBG(desc,
+                         ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                          "inet_set_opts(L_init_mreq,inet) -> "
+                          "try setopt: "
+                          "\r\n   maddr: %x"
+                          "\r\n   if:    %x"
+                          "\r\n   sz:    %d"
+                          "\r\n",
+                          __LINE__, desc->s, driver_caller(desc->port),
+                          mreq4.imr_multiaddr.s_addr,
+                          mreq4.imr_interface.s_addr,
+                          mreqSz));
 
 #endif
 
@@ -7338,7 +7363,10 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
 
                     mreqSz = sizeof(mreq6);
 
-                    DEBUGF(("inet_set_opts(L_init_mreq,inet6) -> mreq\r\n"));
+                    DDBG(desc,
+                         ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                          "inet_set_opts(L_init_mreq,inet6) -> mreq\r\n",
+                          __LINE__, desc->s, driver_caller(desc->port)));
 
                     /* 0) Read out the ifindex
                      * 1) Read out the multiaddr
@@ -7355,13 +7383,16 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
                 }
 #endif
                 else {
-                    DEBUGF(("inet_set_opts(L_init_mreq) -> "
-                             "invalid domain: "
-                             "\r\n   domain:  %d (%d, %d)"
-                             "\r\n   sfmaily: %d (%d, %d)"
-                             "\r\n",
-                             domain, INET_AF_INET, INET_AF_INET6,
-                             desc->sfamily, AF_INET, AF_INET6));
+                    DDBG(desc,
+                         ("INET-DRV-DBG[%d][" SOCKET_FSTR ",%T] "
+                          "inet_set_opts(L_init_mreq) -> "
+                          "invalid domain: "
+                          "\r\n   domain:  %d (%d, %d)"
+                          "\r\n   sfmaily: %d (%d, %d)"
+                          "\r\n",
+                          __LINE__, desc->s, driver_caller(desc->port),
+                          domain, INET_AF_INET, INET_AF_INET6,
+                          desc->sfamily, AF_INET, AF_INET6));
                     return -1;
                 }
             }
diff --git a/lib/kernel/src/inet.erl b/lib/kernel/src/inet.erl
index 37f1706fb2..d836f6b367 100644
--- a/lib/kernel/src/inet.erl
+++ b/lib/kernel/src/inet.erl
@@ -1203,8 +1203,8 @@ udp_opt([Opt | Opts], #udp_opts{ifaddr = IfAddr} = R, As) ->
 		    {error, badarg}
 	    end;
         {active,N} when is_integer(N), N < 32768, N >= -32768 ->
-            NOpts = lists:keydelete(active, 1, R#udp_opts.opts),
-            udp_opt(Opts, R#udp_opts { opts = [{active,N}|NOpts] }, As);
+            POpts = lists:keydelete(active, 1, R#udp_opts.opts),
+            udp_opt(Opts, R#udp_opts { opts = [{active,N}|POpts] }, As);
 
         {Membership, {MAddr, If}}
           when ((Membership =:= add_membership) orelse
@@ -1212,7 +1212,8 @@ udp_opt([Opt | Opts], #udp_opts{ifaddr = IfAddr} = R, As) ->
                (tuple_size(MAddr) =:= 4) andalso
                ((If =:= any) orelse (tuple_size(If) =:= 4)) ->
             MembershipOpt = {Membership, {MAddr, If, 0}},
-            udp_opt(Opts, R#udp_opts{opts = [MembershipOpt|Opts]}, As);
+            POpts         = R#udp_opts.opts,
+            udp_opt(Opts, R#udp_opts{opts = [MembershipOpt|POpts]}, As);
 
 	{Name,Val} when is_atom(Name) -> udp_add(Name, Val, R, Opts, As);
 
diff --git a/lib/kernel/test/gen_udp_SUITE.erl b/lib/kernel/test/gen_udp_SUITE.erl
index c320b71ca0..9c5339b905 100644
--- a/lib/kernel/test/gen_udp_SUITE.erl
+++ b/lib/kernel/test/gen_udp_SUITE.erl
@@ -58,9 +58,14 @@
          t_simple_local_sockaddr_in_send_recv/1,
          t_simple_link_local_sockaddr_in_send_recv/1,
          t_simple_local_sockaddr_in6_send_recv/1,
-         t_simple_link_local_sockaddr_in6_send_recv/1
+         t_simple_link_local_sockaddr_in6_send_recv/1,
+
+         otp_18323_opts_processing/1,
+         otp_18323_open/1
+
 	]).
 
+-include_lib("kernel/src/inet_int.hrl").
 
 -define(TRY_TC(F), try_tc(F)).
                
@@ -97,7 +102,8 @@ groups() ->
      {local,                  [], local_cases()},
      {socket_monitor,         [], socket_monitor_cases()},
 
-     {sockaddr,               [], sockaddr_cases()}
+     {sockaddr,               [], sockaddr_cases()},
+     {otp18323,               [], otp18323_cases()}
     ].
 
 inet_backend_default_cases() ->
@@ -128,7 +134,8 @@ all_cases() ->
      recv_close,
      {group, socket_monitor},
      otp_17492,
-     {group, sockaddr}
+     {group, sockaddr},
+     {group, otp18323}
     ].
 
 recv_and_send_opts_cases() ->
@@ -165,6 +172,12 @@ sockaddr_cases() ->
      t_simple_link_local_sockaddr_in6_send_recv
     ].
 
+otp18323_cases() ->
+    [
+     otp_18323_opts_processing,
+     otp_18323_open
+    ].
+
 
 init_per_suite(Config0) ->
 
@@ -248,6 +261,14 @@ init_per_group(sockaddr = _GroupName, Config) ->
             ?P("init_per_group(sockaddr) -> 'socket' not configured"),
             {skip, "esock not configured"}
     end;
+init_per_group(otp18323 = _GroupName, Config) ->
+    ?P("init_per_group(otp18323) -> inet-drv specific bug(s)"),
+    case ?IS_SOCKET_BACKEND(Config) of
+        true ->
+            {skip, "Inet Drv specific bugs"};
+        false ->
+            ok
+    end;
 init_per_group(_GroupName, Config) ->
     Config.
 
@@ -2835,6 +2856,97 @@ do_simple_sockaddr_send_recv(#{family := _Fam} = SockAddr, _) ->
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
+%% Verify that the options [add|drop]_membership do not mess up
+%% the options (including 'ip' which could not be added *after*).
+%% This just attempts to very that the option processing is ok.
+otp_18323_opts_processing(Config) when is_list(Config) ->
+    ct:timetrap(?MINS(1)),
+    ?TC_TRY(?FUNCTION_NAME, fun() -> do_otp_18323_opts_processing(Config) end).
+
+do_otp_18323_opts_processing(_Config) ->
+    ?P("begin"),
+
+    do_otp_18323_opts_processing_verify(
+      {add_membership,  {{239,1,2,3},{0,0,0,0}}}),
+
+    do_otp_18323_opts_processing_verify(
+      {drop_membership, {{239,1,2,3},{0,0,0,0}}}),
+
+    ?P("done"),
+    ok.
+
+do_otp_18323_opts_processing_verify(MembershipOpt) ->
+    Port   = 4321,
+    RecBuf = 123456,
+    Active = 10,
+    IP     = {1,2,3,4},
+    Opts   = [binary, MembershipOpt, {ip, IP}, {active, Active}],
+
+    case inet:udp_options([{port, Port}, {recbuf, RecBuf} | Opts], inet_udp) of
+        {ok, #udp_opts{ifaddr = IP,
+                       port   = Port,
+                       opts   = SockOpts}} ->
+            ?P("Processed Socket Options: "
+               "~n   IfAddr:    ~p"
+               "~n   Port:      ~p"
+               "~n   Sock Opts: ~p", [IP, Port, SockOpts]),
+            %% Check that the recbuf and mode options are as expected
+            %% The option 'binary' is shorthand for {mode, binary}
+            {value, {recbuf, RecBuf}} = lists:keysearch(recbuf, 1, SockOpts),
+            {value, {mode,   binary}} = lists:keysearch(mode,   1, SockOpts),
+            {value, {active, Active}} = lists:keysearch(active, 1, SockOpts),
+            ok;
+        {error, Reason} ->
+            exit(?F("Failed processing options: ~p", [Reason]))
+    end.
+
+
+
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+
+%% Verify that the options [add|drop]_membership do not mess up
+%% the options (including 'ip' which could not be added *after*).
+otp_18323_open(Config) when is_list(Config) ->
+    ct:timetrap(?MINS(1)),
+    Pre  = fun() ->
+                   {ok, Addr} = ?LIB:which_local_addr(inet),
+                   #{local_addr => Addr}
+           end,
+    Case = fun(State) -> do_otp_18323_open(State) end,
+    Post = fun(_) -> ok end,
+    ?TC_TRY(?FUNCTION_NAME, Pre, Case, Post).
+
+do_otp_18323_open(#{local_addr := Addr}) ->
+    ?P("begin"),
+
+    ROpts = [binary,
+             {add_membership, {Addr,{0,0,0,0}}},
+             {ip, Addr},
+             {active,false},
+             {debug, true}],
+    SOpts = [{reuseaddr, true}, binary],
+
+    ?P("create received socket"),
+    {ok, R}     = gen_udp:open(0, ROpts),
+    ?P("extract received socket port"),
+    {ok, RPort} = inet:port(R),
+
+    ?P("create sender socket"),
+    {ok, S} = gen_udp:open(0, SOpts),
+
+    ?P("send to receiver (at port ~w)", [RPort]),
+    ok = gen_udp:send(S, Addr, RPort, <<"aaaaa">>),
+
+    ?P("attempt to receive data on specified format binary)"),
+    {ok, {_,_,<<"aaaaa">>}} = gen_udp:recv(R, 0, 200),
+
+    ?P("done"),
+    ok.
+
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+
 ok({ok,V}) -> V;
 ok(NotOk) ->
     try throw(not_ok)