[erlang-patches] [PATCH] Fix SCTP multihoming for IPv6

Tomas Abrahamsson <>
Mon Apr 23 23:21:08 CEST 2012


On Mon, Apr 23, 2012 at 16:16 +02:00, Raimo Niskanen wrote:
> Is it so that the sctp_bindx argument 2 declared as (struct sockaddr *)
> is in fact misused to be able to contain IPv6 addresses by packing
> them unaligned?
>
> If so, I would like this fact more visible in the code by defining 'addrs' as:
>        char addrs[256 * sizeof(struct inet_address)]
>
> If not, the code looks erroneous (but since there are test cases in the patch
> I would guess the code is right and it is the sctp_bindx call that is weird.
>
> I would have expected argument 2 to be (struct sockaddr_storage *) but on
> my Linux it is not so...

I think I see what you mean, but it seems the 2nd arg
is supposed to be either an array of sockaddr_in or
an array of sockaddr_in6, not the union of them:

       addrs is a pointer to an array of one or more socket
       addresses. Each address is contained in its appropriate
       structure(i.e.  struct sockaddr_in or struct
       sockaddr_in6).

(quoted from the Linux man page, the Solaris
man page reads similarly)

Looking at it now again, I first thought that addrs should be
either sockaddr_in or sockaddr_in6, not inet_address,
which is the union, but then I re-read the inet_set_address
function, which seems to do just what the man page for
sctp_bindx requires, if I read it correctly.

So I declared the addrs to be of type inet_address to be able
to use the inet_set_address function, and the casting to
(struct sockaddr *) was simply to avoid a compiler warning.

Btw, I did develop the test first, and it failed until
I changed inet_drv.c, so I think it actually works as intended.

Please, let me know if there's anything that I need to do
to get the patch accepted.

BRs
Tomas

> On Fri, Dec 30, 2011 at 09:10:58PM +0100, Tomas Abrahamsson wrote:
>> Setting several ip addresses for an SCTP socket worked for IPv4, but
>> for IPv6 the call to gen_sctp:open/1 failed with badarg.
>> ---
>>  erts/emulator/drivers/common/inet_drv.c |   12 +++---
>>  lib/kernel/test/gen_sctp_SUITE.erl      |   64 ++++++++++++++++++++++++++++++-
>>  2 files changed, 68 insertions(+), 8 deletions(-)
>>
>> diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c
>> index eeaa4d2..f712653 100644
>> --- a/erts/emulator/drivers/common/inet_drv.c
>> +++ b/erts/emulator/drivers/common/inet_drv.c
>> @@ -10111,8 +10111,9 @@ static ErlDrvSSizeT packet_inet_ctl(ErlDrvData e, unsigned int cmd, char* buf,
>>           /* Construct the list of addresses we bind to. The curr limit is
>>              256 addrs. Buff structure: Flags(1), ListItem,...:
>>           */
>> -         struct sockaddr addrs[256];
>> +         inet_address addrs[256];
>>           char* curr;
>> +         char* addr_dest;
>>           int   add_flag, n, rflag;
>>
>>           if (!IS_SCTP(desc))
>> @@ -10122,6 +10123,7 @@ static ErlDrvSSizeT packet_inet_ctl(ErlDrvData e, unsigned int cmd, char* buf,
>>           add_flag = get_int8(curr);
>>           curr++;
>>
>> +         addr_dest = (char *)addrs;
>>           for(n=0; n < 256 && curr < buf+len; n++)
>>               {
>>                   /* List item format: Port(2), IP(4|16) -- compatible with
>> @@ -10132,16 +10134,14 @@ static ErlDrvSSizeT packet_inet_ctl(ErlDrvData e, unsigned int cmd, char* buf,
>>                   if (curr == NULL)
>>                       return ctl_error(EINVAL, rbuf, rsize);
>>
>> -                 /* Now: we need to squeeze "tmp" into the size of "sockaddr",
>> -                    which is smaller than "tmp" for IPv6 (extra IN6 info will
>> -                    be cut off): */
>> -                 memcpy(addrs + n, &tmp, sizeof(struct sockaddr));
>> +                 memcpy(addr_dest, &tmp, alen);
>> +                 addr_dest += alen;
>>               }
>>           /* Make the real flags: */
>>           rflag = add_flag ? SCTP_BINDX_ADD_ADDR : SCTP_BINDX_REM_ADDR;
>>
>>           /* Invoke the call: */
>> -         if (p_sctp_bindx(desc->s, addrs, n, rflag) < 0)
>> +         if (p_sctp_bindx(desc->s, (struct sockaddr *)addrs, n, rflag) < 0)
>>               return ctl_error(sock_errno(), rbuf, rsize);
>>
>>           desc->state = INET_STATE_BOUND;
>> diff --git a/lib/kernel/test/gen_sctp_SUITE.erl b/lib/kernel/test/gen_sctp_SUITE.erl
>> index 8f490b6..6d6b154 100644
>> --- a/lib/kernel/test/gen_sctp_SUITE.erl
>> +++ b/lib/kernel/test/gen_sctp_SUITE.erl
>> @@ -31,14 +31,16 @@
>>     [basic/1,
>>      api_open_close/1,api_listen/1,api_connect_init/1,api_opts/1,
>>      xfer_min/1,xfer_active/1,def_sndrcvinfo/1,implicit_inet6/1,
>> -    basic_stream/1, xfer_stream_min/1, peeloff/1, buffers/1]).
>> +    basic_stream/1, xfer_stream_min/1, peeloff/1, buffers/1,
>> +    open_multihoming_ipv4_socket/1, open_multihoming_ipv6_socket/1]).
>>
>>  suite() -> [{ct_hooks,[ts_install_cth]}].
>>
>>  all() ->
>>      [basic, api_open_close, api_listen, api_connect_init,
>>       api_opts, xfer_min, xfer_active, def_sndrcvinfo, implicit_inet6,
>> -     basic_stream, xfer_stream_min, peeloff, buffers].
>> +     basic_stream, xfer_stream_min, peeloff, buffers,
>> +     open_multihoming_ipv4_socket, open_multihoming_ipv6_socket].
>>
>>  groups() ->
>>      [].
>> @@ -1329,3 +1331,61 @@ match_unless_solaris(A, B) ->
>>       {unix,sunos} -> B;
>>       _ -> A = B
>>      end.
>> +
>> +open_multihoming_ipv4_socket(doc) ->
>> +    "Test opening a multihoming ipv4 socket";
>> +open_multihoming_ipv4_socket(suite) ->
>> +    [];
>> +open_multihoming_ipv4_socket(Config) when is_list(Config) ->
>> +    ?line Hostname = ok(inet:gethostname()),
>> +    ?line HostAddrIPv4 = ok(inet:getaddr(Hostname, inet)),
>> +    ?line S = ok(gen_sctp:open(0, [{ip,HostAddrIPv4},{ip,{127,0,0,1}}])),
>> +    ?line ok = gen_sctp:listen(S, true),
>> +    ?line setup_connection(S, HostAddrIPv4, inet),
>> +    ?line ok = gen_sctp:close(S).
>> +
>> +open_multihoming_ipv6_socket(doc) ->
>> +    "Test opening a multihoming ipv6 socket";
>> +open_multihoming_ipv6_socket(suite) ->
>> +    [];
>> +open_multihoming_ipv6_socket(Config) when is_list(Config) ->
>> +    ?line Hostname = ok(inet:gethostname()),
>> +    ?line LoopbackIPv6 = {0,0,0,0,0,0,0,1},
>> +    ?line
>> +     case inet:getaddr(Hostname, inet6) of
>> +         {ok,HostAddrIPv6} when HostAddrIPv6 =/= LoopbackIPv6 ->
>> +             ?line
>> +                 case is_good_ipv6_address(HostAddrIPv6) of
>> +                     true ->
>> +                         ?line io:format("using ipv6 addresses ~p and ~p~n",
>> +                                         [HostAddrIPv6, LoopbackIPv6]),
>> +                         ?line S = ok(gen_sctp:open(
>> +                                        0, [{ip,HostAddrIPv6},
>> +                                            {ip,LoopbackIPv6}, inet6])),
>> +                         ?line ok = gen_sctp:listen(S, true),
>> +                         ?line setup_connection(S, HostAddrIPv6, inet6),
>> +                         ?line ok = gen_sctp:close(S);
>> +                     false ->
>> +                         {skip,"Need 2 good IPv6 addresses"}
>> +                 end;
>> +         {ok,HostAddrIPv6} when HostAddrIPv6 =:= LoopbackIPv6 ->
>> +             {skip,"Need 2 different IPv6 addresses, found only ::1"};
>> +         {error,eafnosupport} ->
>> +             {skip,"Can not look up IPv6 address"}
>> +        end.
>> +
>> +is_good_ipv6_address({0,0,0,0,0,16#ffff,_,_}) -> false; %% ipv4 mapped
>> +is_good_ipv6_address({16#fe80,_,_,_,_,_,_,_}) -> false; %% link-local
>> +is_good_ipv6_address(_)                       -> true.
>> +
>> +setup_connection(S1, Addr, IpFamily) ->
>> +    ?line P1 = ok(inet:port(S1)),
>> +    ?line S2 = ok(gen_sctp:open(0, [IpFamily])),
>> +    ?line P2 = ok(inet:port(S2)),
>> +    ?line #sctp_assoc_change{state=comm_up} =
>> +     ok(gen_sctp:connect(S2, Addr, P1, [])),
>> +    ?line case ok(gen_sctp:recv(S1)) of
>> +           {Addr,P2,[],#sctp_assoc_change{state=comm_up}} ->
>> +               ok
>> +       end,
>> +    ?line ok = gen_sctp:close(S2).
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> erlang-patches mailing list
>> 
>> http://erlang.org/mailman/listinfo/erlang-patches
>
> --
>
> / Raimo Niskanen, Erlang/OTP, Ericsson AB


More information about the erlang-patches mailing list