Re: [PATCH 3/3] NFSD: Enable NFS server use of PF_INET6

Chuck Lever
2010-01-15T23:18:07Z

On Jan 15, 2010, at 5:44 PM, J. Bruce Fields wrote:

> On Wed, Jan 13, 2010 at 04:10:48PM -0500, Chuck Lever wrote:
>> Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the
>> kernel.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>> fs/nfsd/nfsctl.c | 36 +++++++++++++++++++++++++++++-------
>> fs/nfsd/nfssvc.c | 27 ++++++++++++++++++++++-----
>> 2 files changed, 51 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 2604c3e..7ebb7a5 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
>> return len;
>> }
>>
>> +static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
>> + const char *transport,
>> + const int family,
>> + const unsigned short port)
>> +{
>> + int err;
>> +
>> + err = svc_create_xprt(serv, transport, family, port,
>> + SVC_SOCK_ANONYMOUS);
>> +
>> + if (err < 0) {
>> + /* Give a reasonable perror msg for bad transport string */
>> + if (err == -ENOENT)
>> + err = -EPROTONOSUPPORT;
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /*
>> * A transport listener is added by writing it's transport name and
>> * a port number.
>> @@ -1000,14 +1020,16 @@ static ssize_t __write_ports_addxprt(char
>> *buf)
>> if (err != 0)
>> return err;
>>
>> - err = svc_create_xprt(nfsd_serv, transport,
>> - PF_INET, port, SVC_SOCK_ANONYMOUS);
>> - if (err < 0) {
>> - /* Give a reasonable perror msg for bad transport string */
>> - if (err == -ENOENT)
>> - err = -EPROTONOSUPPORT;
>> + err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET,
>> port);
>> + if (err < 0)
>> return err;
>> - }
>> +
>> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>> + err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET6,
>> port);
>> + if (err < 0 && err != -EAFNOSUPPORT)
>> + return err;
>> +#endif /* CONFIG_IPV6 || CONFIG_IPV6_MODULE */
>
> Does every caller of __write_ports_create_xprt() with PF_INET6 end up
> being under this ifdef? Could we instead just move the ifdef inside
> it
> (or inside __svc_xpo_create())? (As usual I'd rather keep any
> necessary
> hidden away to the extent possible.)

The only two call sites are right here: one with PF_INET and one with
PF_INET6.

When IPv6 support is not enabled, we want only the first call (with
PF_INET). When IPv6 support is enabled, we want two calls; one with
PF_INET and then one with PF_INET6.

This particular interface is for creating a named transport. We don't
name our IPv6 transports separately from IPv4 transports, so this is a
simple way to get IPv6 enabled listeners. It's actually been a while
since I sat down and thought through this.

> --b.
>
>> +
>> return 0;
>> }
>>
>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> index 171699e..7af14c1 100644
>> --- a/fs/nfsd/nfssvc.c
>> +++ b/fs/nfsd/nfssvc.c
>> @@ -275,14 +275,32 @@ int nfsd_create_serv(void)
>> return err;
>> }
>>
>> -static int nfsd_init_socks(int port)
>> +static int nfsd_create_xprt(const char *transport, const unsigned
>> short port)
>> +{
>> + int ret;
>> +
>> + ret = svc_create_xprt(nfsd_serv, transport, PF_INET, port,
>> + SVC_SOCK_DEFAULTS);
>> + if (ret < 0)
>> + return ret;
>> +
>> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>> + ret = svc_create_xprt(nfsd_serv, transport, PF_INET6, port,
>> + SVC_SOCK_DEFAULTS);
>> + if (ret < 0 && ret != -EAFNOSUPPORT)
>> + return ret;
>> +#endif /* CONFIG_IPV6 || CONFIG_IPV6_MODULE */
>> +
>> + return 0;
>> +}
>> +
>> +static int nfsd_init_socks(const unsigned short port)
>> {
>> int error;
>> if (!list_empty(&nfsd_serv->sv_permsocks))
>> return 0;
>>
>> - error = svc_create_xprt(nfsd_serv, "udp", PF_INET, port,
>> - SVC_SOCK_DEFAULTS);
>> + error = nfsd_create_xprt("udp", port);
>> if (error < 0)
>> return error;
>>
>> @@ -290,8 +308,7 @@ static int nfsd_init_socks(int port)
>> if (error < 0)
>> return error;
>>
>> - error = svc_create_xprt(nfsd_serv, "tcp", PF_INET, port,
>> - SVC_SOCK_DEFAULTS);
>> + error = nfsd_create_xprt("tcp", port);
>> if (error < 0)
>> return error;
>>
>>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com