On Fri, Jan 15, 2010 at 06:17:38PM -0500, Chuck Lever wrote:
>
> 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@xxxxxxxxxx>
>>> ---
>>>
>>> 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.
OK, but svc_create_xprt has a few more.
>
> 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.
Right, I understand, but we could equally well do that by calling
svc_create_xprt twice, and depending on it (or __svc_xpo_create(), or
someone else there) to return -EAFNOSUPPORT in the !defined(CONFIG_IPV6)
&& !defined(CONFIG_IPV6_MODULE case. Right?
No change in behavior, I just want the #ifdef's buried a little further
if it's possible.
--b.
>
> 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
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
