Re: [Gluster-devel] Fixing Address family mess

[Anand Babu Periasamy]

On Mon, May 7, 2012 at 6:16 AM, Jeff Darcy <address@hidden> wrote:
> On 05/07/2012 12:39 AM, Emmanuel Dreyfus wrote:
>> Quick summary of the problem: when using transport-type socket with
>> transport.address-family unspecified, glusterfs binds sockets with
>> AF_UNSPEC, which will use either AF_INET or AF_INET6 socket, whatever the
>> kernel prefers. At mine it uses AF_INET6, while the machine is not
>> configured to use IPv6. As a result, glusterfs client cannot connect
>> to glusterfs server.
>>
>> A workaround is to use option transport.address-family inet in
>> glusterfsd/glusterd.vol but that option must also be specified in
>> all volume files for all bricks and FUSE client, which is
>> unfortunate because they are automatically generated. I proposed a
>> patch so that glusterd transport.address-family setting is propagated
>> to various places: http://review.gluster.com/3261
>>
>> That did not meet consensus. Jeff Darcy notes that we should be able
>> to listen both on AF_INET and AF_INET6 sockets at the same time. I
>> had a look at the code, and indeed it could easily be done. The only
>> trouble is how to specify the listeners. For now option transport
>> defaults to socket,rdma. I suggest we add socket families in that
>> specification. We would then have this default:
>>    option transport socket/inet,socket/inet6,rdma
>>
>> With the following semantics:
>>    socket -> AF_UNSPEC socket (backward comaptibility)
>>    socket/inet -> AF_INET socket
>>    socket/inet6 -> AF_INET6 socket
>>    socket/sdp -> AF_SDP socket
>>    rdma -> sameas before
>>
>> Any opinion on that plan? Please comment before I writa code, it will
>> save me some time is the proposal is wrong.
>
> I think it looks like the right solution. I understand that keeping the
> address-family multiplexing entirely in the socket code would be more complex,
> since it changes the relationship between transport instances and file
> descriptors (and threads in the SSL/multi-thread case).  That's unfortunate,
> but far from the most unfortunate thing about our transport code.
>
> I do wonder whether we should use '/' as the separator, since it kind of
> implies the same kind of relationships between names and paths that we use for
> translator names - e.g. cluster/dht is actually used as part of the actual 
> path
> for dht.so - and in this case that relationship doesn't actually exist. 
> Another
> idea, which I don't actually like any better but which I'll suggest for
> completeness, would be to express the list of address families via an option:
>
>        option transport.socket.address-family inet6
>
> Now that I think about it, another benefit is that it supports multiple
> instances of the same address family with different options, e.g. to support
> segregated networks.  Obviously we lack higher-level support for that right
> now, but if that should ever change then it would be nice to have the right
> low-level infrastructure in place for it.
>

Yes this should be controlled through volume options.
"transport.address-family" is the right place to set it. Possible
values are "inet, inet6, unix, inet-sdp". I would have named those
user facing options as "ipv4, ipv6, sdp, all".

If transport.address-family is not set. then
    if remote-host is set
        default to AF_INET (ipv4) if
    if transport.socket.connect-path is set
        default to AF_UNIX (unix)

AF_UNSPEC is should be be taken as IPv4/IPv6. It is named
appropriately.  Default should be ipv4.

I have not tested the patch. It is simply to explain how the changes
should look like. I ignored legacy translators. When we implement
concurrent support for multiple address-family (likely via
mult-process model) we can worry about combinations.

I agree. Combinations should look like "inet | inet6 | .." and not
"inet / inet6 /.."

-- 
Anand Babu Periasamy
Blog [ http://www.unlocksmith.org ]
Twitter [ http://twitter.com/abperiasamy ]

Imagination is more important than knowledge --Albert Einstein

glusterfs-af-default-ipv4.diff
Description: Binary data