diff mbox

nfs: add nfs IPv6 rdma6 mount option support

Message ID 56E9A650.7060409@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shirley Ma March 16, 2016, 6:30 p.m. UTC
Add rdma6 option to support NFS/RDMA IPv6.

Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
---


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Bruce Fields March 16, 2016, 6:46 p.m. UTC | #1
On Wed, Mar 16, 2016 at 11:30:40AM -0700, Shirley Ma wrote:
> Add rdma6 option to support NFS/RDMA IPv6.

This is client-side: cc'ing Trond and Anna.--b.

> 
> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
> ---
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index f126828..62a55d0 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>  
>  enum {
>  	Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
> +	Opt_xprt_rdma6,
>  
>  	Opt_xprt_err
>  };
> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>  	{ Opt_xprt_tcp, "tcp" },
>  	{ Opt_xprt_tcp6, "tcp6" },
>  	{ Opt_xprt_rdma, "rdma" },
> +	{ Opt_xprt_rdma6, "rdma6" },
>  
>  	{ Opt_xprt_err, NULL }
>  };
> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>  				mnt->flags |= NFS_MOUNT_TCP;
>  				mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>  				break;
> +			case Opt_xprt_rdma6:
> +				protofamily = AF_INET6;
>  			case Opt_xprt_rdma:
>  				/* vector side protocols to TCP */
>  				mnt->flags |= NFS_MOUNT_TCP;
> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>  			case Opt_xprt_tcp:
>  				mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>  				break;
> +			case Opt_xprt_rdma6:
> +				mountfamily = AF_INET6;
>  			case Opt_xprt_rdma: /* not used for side protocols */
>  			default:
>  				dfprintk(MOUNT, "NFS:   unrecognized "
> diff --git a/include/linux/sunrpc/msg_prot.h b/include/linux/sunrpc/msg_prot.h
> index 8073713..49b8433 100644
> --- a/include/linux/sunrpc/msg_prot.h
> +++ b/include/linux/sunrpc/msg_prot.h
> @@ -149,6 +149,7 @@ typedef __be32	rpc_fraghdr;
>  #define RPCBIND_NETID_UDP	"udp"
>  #define RPCBIND_NETID_TCP	"tcp"
>  #define RPCBIND_NETID_RDMA	"rdma"
> +#define RPCBIND_NETID_RDMA6	"rdma6"
>  #define RPCBIND_NETID_SCTP	"sctp"
>  #define RPCBIND_NETID_UDP6	"udp6"
>  #define RPCBIND_NETID_TCP6	"tcp6"
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Schumaker, Anna March 22, 2016, 7:38 p.m. UTC | #2
Hi Shirley,

Sorry for the delay in looking at this patch.  Comments are below:

On 03/16/2016 02:30 PM, Shirley Ma wrote:
> Add rdma6 option to support NFS/RDMA IPv6.
> 
> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>

Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?

> ---
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index f126828..62a55d0 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>  
>  enum {
>  	Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
> +	Opt_xprt_rdma6,
>  
>  	Opt_xprt_err
>  };
> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>  	{ Opt_xprt_tcp, "tcp" },
>  	{ Opt_xprt_tcp6, "tcp6" },
>  	{ Opt_xprt_rdma, "rdma" },
> +	{ Opt_xprt_rdma6, "rdma6" },
>  
>  	{ Opt_xprt_err, NULL }
>  };
> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>  				mnt->flags |= NFS_MOUNT_TCP;
>  				mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>  				break;
> +			case Opt_xprt_rdma6:
> +				protofamily = AF_INET6;
>  			case Opt_xprt_rdma:
>  				/* vector side protocols to TCP */
>  				mnt->flags |= NFS_MOUNT_TCP;
> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>  			case Opt_xprt_tcp:
>  				mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>  				break;
> +			case Opt_xprt_rdma6:
> +				mountfamily = AF_INET6;
>  			case Opt_xprt_rdma: /* not used for side protocols */
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.

>  			default:
>  				dfprintk(MOUNT, "NFS:   unrecognized "
> diff --git a/include/linux/sunrpc/msg_prot.h b/include/linux/sunrpc/msg_prot.h
> index 8073713..49b8433 100644
> --- a/include/linux/sunrpc/msg_prot.h
> +++ b/include/linux/sunrpc/msg_prot.h
> @@ -149,6 +149,7 @@ typedef __be32	rpc_fraghdr;
>  #define RPCBIND_NETID_UDP	"udp"
>  #define RPCBIND_NETID_TCP	"tcp"
>  #define RPCBIND_NETID_RDMA	"rdma"
> +#define RPCBIND_NETID_RDMA6	"rdma6"

This is defined right after tcp6, so we probably don't need it twice :).

Thanks,
Anna

>  #define RPCBIND_NETID_SCTP	"sctp"
>  #define RPCBIND_NETID_UDP6	"udp6"
>  #define RPCBIND_NETID_TCP6	"tcp6"
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever March 22, 2016, 9:24 p.m. UTC | #3
> On Mar 22, 2016, at 3:38 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
> 
> Hi Shirley,
> 
> Sorry for the delay in looking at this patch.  Comments are below:
> 
> On 03/16/2016 02:30 PM, Shirley Ma wrote:
>> Add rdma6 option to support NFS/RDMA IPv6.
>> 
>> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
> 
> Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?
> 
>> ---
>> 
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index f126828..62a55d0 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>> 
>> enum {
>> 	Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
>> +	Opt_xprt_rdma6,
>> 
>> 	Opt_xprt_err
>> };
>> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>> 	{ Opt_xprt_tcp, "tcp" },
>> 	{ Opt_xprt_tcp6, "tcp6" },
>> 	{ Opt_xprt_rdma, "rdma" },
>> +	{ Opt_xprt_rdma6, "rdma6" },
>> 
>> 	{ Opt_xprt_err, NULL }
>> };
>> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>> 				mnt->flags |= NFS_MOUNT_TCP;
>> 				mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>> 				break;
>> +			case Opt_xprt_rdma6:
>> +				protofamily = AF_INET6;
>> 			case Opt_xprt_rdma:
>> 				/* vector side protocols to TCP */
>> 				mnt->flags |= NFS_MOUNT_TCP;
>> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>> 			case Opt_xprt_tcp:
>> 				mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>> 				break;
>> +			case Opt_xprt_rdma6:
>> +				mountfamily = AF_INET6;
>> 			case Opt_xprt_rdma: /* not used for side protocols */
>                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.

I wondered this too.

But what's going on is that for NFSv3 on RDMA, when IPv6 is
used for the main protocol, IPv6 needs to be used for the
mount protocol as well, even though it is going over TCP.


>> 			default:
>> 				dfprintk(MOUNT, "NFS:   unrecognized "
>> diff --git a/include/linux/sunrpc/msg_prot.h b/include/linux/sunrpc/msg_prot.h
>> index 8073713..49b8433 100644
>> --- a/include/linux/sunrpc/msg_prot.h
>> +++ b/include/linux/sunrpc/msg_prot.h
>> @@ -149,6 +149,7 @@ typedef __be32	rpc_fraghdr;
>> #define RPCBIND_NETID_UDP	"udp"
>> #define RPCBIND_NETID_TCP	"tcp"
>> #define RPCBIND_NETID_RDMA	"rdma"
>> +#define RPCBIND_NETID_RDMA6	"rdma6"
> 
> This is defined right after tcp6, so we probably don't need it twice :).
> 
> Thanks,
> Anna
> 
>> #define RPCBIND_NETID_SCTP	"sctp"
>> #define RPCBIND_NETID_UDP6	"udp6"
>> #define RPCBIND_NETID_TCP6	"tcp6"
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust March 22, 2016, 9:46 p.m. UTC | #4
On Tue, Mar 22, 2016 at 5:24 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>
>> On Mar 22, 2016, at 3:38 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>>
>> Hi Shirley,
>>
>> Sorry for the delay in looking at this patch.  Comments are below:
>>
>> On 03/16/2016 02:30 PM, Shirley Ma wrote:
>>> Add rdma6 option to support NFS/RDMA IPv6.
>>>
>>> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
>>
>> Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?
>>
>>> ---
>>>
>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>> index f126828..62a55d0 100644
>>> --- a/fs/nfs/super.c
>>> +++ b/fs/nfs/super.c
>>> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>>>
>>> enum {
>>>      Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
>>> +    Opt_xprt_rdma6,
>>>
>>>      Opt_xprt_err
>>> };
>>> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>>>      { Opt_xprt_tcp, "tcp" },
>>>      { Opt_xprt_tcp6, "tcp6" },
>>>      { Opt_xprt_rdma, "rdma" },
>>> +    { Opt_xprt_rdma6, "rdma6" },
>>>
>>>      { Opt_xprt_err, NULL }
>>> };
>>> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>>>                              mnt->flags |= NFS_MOUNT_TCP;
>>>                              mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>                              break;
>>> +                    case Opt_xprt_rdma6:
>>> +                            protofamily = AF_INET6;
>>>                      case Opt_xprt_rdma:
>>>                              /* vector side protocols to TCP */
>>>                              mnt->flags |= NFS_MOUNT_TCP;
>>> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>>>                      case Opt_xprt_tcp:
>>>                              mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>>>                              break;
>>> +                    case Opt_xprt_rdma6:
>>> +                            mountfamily = AF_INET6;
>>>                      case Opt_xprt_rdma: /* not used for side protocols */
>>                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.
>
> I wondered this too.
>
> But what's going on is that for NFSv3 on RDMA, when IPv6 is
> used for the main protocol, IPv6 needs to be used for the
> mount protocol as well, even though it is going over TCP.

It causes nfs_mount_parse_options to immediately stop further parsing
and return '0' == error encountered. The mount is supposed to fail in
that case.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever March 22, 2016, 9:52 p.m. UTC | #5
> On Mar 22, 2016, at 5:46 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> 
> On Tue, Mar 22, 2016 at 5:24 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>>> On Mar 22, 2016, at 3:38 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>>> 
>>> Hi Shirley,
>>> 
>>> Sorry for the delay in looking at this patch.  Comments are below:
>>> 
>>> On 03/16/2016 02:30 PM, Shirley Ma wrote:
>>>> Add rdma6 option to support NFS/RDMA IPv6.
>>>> 
>>>> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
>>> 
>>> Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?
>>> 
>>>> ---
>>>> 
>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>> index f126828..62a55d0 100644
>>>> --- a/fs/nfs/super.c
>>>> +++ b/fs/nfs/super.c
>>>> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>>>> 
>>>> enum {
>>>>     Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
>>>> +    Opt_xprt_rdma6,
>>>> 
>>>>     Opt_xprt_err
>>>> };
>>>> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>>>>     { Opt_xprt_tcp, "tcp" },
>>>>     { Opt_xprt_tcp6, "tcp6" },
>>>>     { Opt_xprt_rdma, "rdma" },
>>>> +    { Opt_xprt_rdma6, "rdma6" },
>>>> 
>>>>     { Opt_xprt_err, NULL }
>>>> };
>>>> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>                             mnt->flags |= NFS_MOUNT_TCP;
>>>>                             mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>>                             break;
>>>> +                    case Opt_xprt_rdma6:
>>>> +                            protofamily = AF_INET6;
>>>>                     case Opt_xprt_rdma:
>>>>                             /* vector side protocols to TCP */
>>>>                             mnt->flags |= NFS_MOUNT_TCP;
>>>> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>                     case Opt_xprt_tcp:
>>>>                             mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>>>>                             break;
>>>> +                    case Opt_xprt_rdma6:
>>>> +                            mountfamily = AF_INET6;
>>>>                     case Opt_xprt_rdma: /* not used for side protocols */
>>>                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.
>> 
>> I wondered this too.
>> 
>> But what's going on is that for NFSv3 on RDMA, when IPv6 is
>> used for the main protocol, IPv6 needs to be used for the
>> mount protocol as well, even though it is going over TCP.
> 
> It causes nfs_mount_parse_options to immediately stop further parsing
> and return '0' == error encountered. The mount is supposed to fail in
> that case.

Ah, a break; is needed there too.

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust March 22, 2016, 9:55 p.m. UTC | #6
On Tue, Mar 22, 2016 at 5:52 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>
>> On Mar 22, 2016, at 5:46 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>>
>> On Tue, Mar 22, 2016 at 5:24 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>>
>>>> On Mar 22, 2016, at 3:38 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>>>>
>>>> Hi Shirley,
>>>>
>>>> Sorry for the delay in looking at this patch.  Comments are below:
>>>>
>>>> On 03/16/2016 02:30 PM, Shirley Ma wrote:
>>>>> Add rdma6 option to support NFS/RDMA IPv6.
>>>>>
>>>>> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
>>>>
>>>> Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?
>>>>
>>>>> ---
>>>>>
>>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>>> index f126828..62a55d0 100644
>>>>> --- a/fs/nfs/super.c
>>>>> +++ b/fs/nfs/super.c
>>>>> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>>>>>
>>>>> enum {
>>>>>     Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
>>>>> +    Opt_xprt_rdma6,
>>>>>
>>>>>     Opt_xprt_err
>>>>> };
>>>>> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>>>>>     { Opt_xprt_tcp, "tcp" },
>>>>>     { Opt_xprt_tcp6, "tcp6" },
>>>>>     { Opt_xprt_rdma, "rdma" },
>>>>> +    { Opt_xprt_rdma6, "rdma6" },
>>>>>
>>>>>     { Opt_xprt_err, NULL }
>>>>> };
>>>>> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>>                             mnt->flags |= NFS_MOUNT_TCP;
>>>>>                             mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>>>                             break;
>>>>> +                    case Opt_xprt_rdma6:
>>>>> +                            protofamily = AF_INET6;
>>>>>                     case Opt_xprt_rdma:
>>>>>                             /* vector side protocols to TCP */
>>>>>                             mnt->flags |= NFS_MOUNT_TCP;
>>>>> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>>                     case Opt_xprt_tcp:
>>>>>                             mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>>>>>                             break;
>>>>> +                    case Opt_xprt_rdma6:
>>>>> +                            mountfamily = AF_INET6;
>>>>>                     case Opt_xprt_rdma: /* not used for side protocols */
>>>>                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.
>>>
>>> I wondered this too.
>>>
>>> But what's going on is that for NFSv3 on RDMA, when IPv6 is
>>> used for the main protocol, IPv6 needs to be used for the
>>> mount protocol as well, even though it is going over TCP.
>>
>> It causes nfs_mount_parse_options to immediately stop further parsing
>> and return '0' == error encountered. The mount is supposed to fail in
>> that case.
>
> Ah, a break; is needed there too.
>

I'm saying that makes no sense to allow "rdma6" here in the first
place, since we've already banned the use of "rdma" through that same
mechanism.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever March 22, 2016, 10:01 p.m. UTC | #7
> On Mar 22, 2016, at 5:55 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> 
> On Tue, Mar 22, 2016 at 5:52 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>>> On Mar 22, 2016, at 5:46 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>>> 
>>> On Tue, Mar 22, 2016 at 5:24 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>>> 
>>>>> On Mar 22, 2016, at 3:38 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>>>>> 
>>>>> Hi Shirley,
>>>>> 
>>>>> Sorry for the delay in looking at this patch.  Comments are below:
>>>>> 
>>>>> On 03/16/2016 02:30 PM, Shirley Ma wrote:
>>>>>> Add rdma6 option to support NFS/RDMA IPv6.
>>>>>> 
>>>>>> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
>>>>> 
>>>>> Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?
>>>>> 
>>>>>> ---
>>>>>> 
>>>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>>>> index f126828..62a55d0 100644
>>>>>> --- a/fs/nfs/super.c
>>>>>> +++ b/fs/nfs/super.c
>>>>>> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>>>>>> 
>>>>>> enum {
>>>>>>    Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
>>>>>> +    Opt_xprt_rdma6,
>>>>>> 
>>>>>>    Opt_xprt_err
>>>>>> };
>>>>>> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>>>>>>    { Opt_xprt_tcp, "tcp" },
>>>>>>    { Opt_xprt_tcp6, "tcp6" },
>>>>>>    { Opt_xprt_rdma, "rdma" },
>>>>>> +    { Opt_xprt_rdma6, "rdma6" },
>>>>>> 
>>>>>>    { Opt_xprt_err, NULL }
>>>>>> };
>>>>>> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>>>                            mnt->flags |= NFS_MOUNT_TCP;
>>>>>>                            mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>>>>                            break;
>>>>>> +                    case Opt_xprt_rdma6:
>>>>>> +                            protofamily = AF_INET6;
>>>>>>                    case Opt_xprt_rdma:
>>>>>>                            /* vector side protocols to TCP */
>>>>>>                            mnt->flags |= NFS_MOUNT_TCP;
>>>>>> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>>>                    case Opt_xprt_tcp:
>>>>>>                            mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>>>>>>                            break;
>>>>>> +                    case Opt_xprt_rdma6:
>>>>>> +                            mountfamily = AF_INET6;
>>>>>>                    case Opt_xprt_rdma: /* not used for side protocols */
>>>>>                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>> Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.
>>>> 
>>>> I wondered this too.
>>>> 
>>>> But what's going on is that for NFSv3 on RDMA, when IPv6 is
>>>> used for the main protocol, IPv6 needs to be used for the
>>>> mount protocol as well, even though it is going over TCP.
>>> 
>>> It causes nfs_mount_parse_options to immediately stop further parsing
>>> and return '0' == error encountered. The mount is supposed to fail in
>>> that case.
>> 
>> Ah, a break; is needed there too.
>> 
> 
> I'm saying that makes no sense to allow "rdma6" here in the first
> place, since we've already banned the use of "rdma" through that same
> mechanism.

I see, this is the parsing logic just for "mountproto=".

This hunk should be dropped, and maybe the
"case Opt_xprt_rdma:" can be removed, too.


--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shirley Ma March 23, 2016, 6:03 p.m. UTC | #8
On 03/22/2016 03:01 PM, Chuck Lever wrote:
> 
>> On Mar 22, 2016, at 5:55 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>>
>> On Tue, Mar 22, 2016 at 5:52 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>>
>>>> On Mar 22, 2016, at 5:46 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>>>>
>>>> On Tue, Mar 22, 2016 at 5:24 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>>>>
>>>>>> On Mar 22, 2016, at 3:38 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>>>>>>
>>>>>> Hi Shirley,
>>>>>>
>>>>>> Sorry for the delay in looking at this patch.  Comments are below:
>>>>>>
>>>>>> On 03/16/2016 02:30 PM, Shirley Ma wrote:
>>>>>>> Add rdma6 option to support NFS/RDMA IPv6.
>>>>>>>
>>>>>>> Signed-off-by: Shirley Ma <shirley.ma@oracle.com>
>>>>>>
>>>>>> Can you add a little more to the patch description to describe when RDMA with IPv6 would be used?
>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>>>>> index f126828..62a55d0 100644
>>>>>>> --- a/fs/nfs/super.c
>>>>>>> +++ b/fs/nfs/super.c
>>>>>>> @@ -191,6 +191,7 @@ static const match_table_t nfs_mount_option_tokens = {
>>>>>>>
>>>>>>> enum {
>>>>>>>    Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
>>>>>>> +    Opt_xprt_rdma6,
>>>>>>>
>>>>>>>    Opt_xprt_err
>>>>>>> };
>>>>>>> @@ -201,6 +202,7 @@ static const match_table_t nfs_xprt_protocol_tokens = {
>>>>>>>    { Opt_xprt_tcp, "tcp" },
>>>>>>>    { Opt_xprt_tcp6, "tcp6" },
>>>>>>>    { Opt_xprt_rdma, "rdma" },
>>>>>>> +    { Opt_xprt_rdma6, "rdma6" },
>>>>>>>
>>>>>>>    { Opt_xprt_err, NULL }
>>>>>>> };
>>>>>>> @@ -1456,6 +1458,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>>>>                            mnt->flags |= NFS_MOUNT_TCP;
>>>>>>>                            mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
>>>>>>>                            break;
>>>>>>> +                    case Opt_xprt_rdma6:
>>>>>>> +                            protofamily = AF_INET6;
>>>>>>>                    case Opt_xprt_rdma:
>>>>>>>                            /* vector side protocols to TCP */
>>>>>>>                            mnt->flags |= NFS_MOUNT_TCP;
>>>>>>> @@ -1490,6 +1494,8 @@ static int nfs_parse_mount_options(char *raw,
>>>>>>>                    case Opt_xprt_tcp:
>>>>>>>                            mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
>>>>>>>                            break;
>>>>>>> +                    case Opt_xprt_rdma6:
>>>>>>> +                            mountfamily = AF_INET6;
>>>>>>>                    case Opt_xprt_rdma: /* not used for side protocols */
>>>>>>                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>> Do we need to be setting mountfamily here?  The comment next to Opt_xprt_rdma makes it sound like this code doesn't apply to RDMA.
>>>>>
>>>>> I wondered this too.
>>>>>
>>>>> But what's going on is that for NFSv3 on RDMA, when IPv6 is
>>>>> used for the main protocol, IPv6 needs to be used for the
>>>>> mount protocol as well, even though it is going over TCP.
>>>>
>>>> It causes nfs_mount_parse_options to immediately stop further parsing
>>>> and return '0' == error encountered. The mount is supposed to fail in
>>>> that case.
>>>
>>> Ah, a break; is needed there too.
>>>
>>
>> I'm saying that makes no sense to allow "rdma6" here in the first
>> place, since we've already banned the use of "rdma" through that same
>> mechanism.
> 
> I see, this is the parsing logic just for "mountproto=".
> 
> This hunk should be dropped, and maybe the
> "case Opt_xprt_rdma:" can be removed, too.

Thanks for the review. I will drop mountfamily option case.
 
> 
> --
> Chuck Lever
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index f126828..62a55d0 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -191,6 +191,7 @@  static const match_table_t nfs_mount_option_tokens = {
 
 enum {
 	Opt_xprt_udp, Opt_xprt_udp6, Opt_xprt_tcp, Opt_xprt_tcp6, Opt_xprt_rdma,
+	Opt_xprt_rdma6,
 
 	Opt_xprt_err
 };
@@ -201,6 +202,7 @@  static const match_table_t nfs_xprt_protocol_tokens = {
 	{ Opt_xprt_tcp, "tcp" },
 	{ Opt_xprt_tcp6, "tcp6" },
 	{ Opt_xprt_rdma, "rdma" },
+	{ Opt_xprt_rdma6, "rdma6" },
 
 	{ Opt_xprt_err, NULL }
 };
@@ -1456,6 +1458,8 @@  static int nfs_parse_mount_options(char *raw,
 				mnt->flags |= NFS_MOUNT_TCP;
 				mnt->nfs_server.protocol = XPRT_TRANSPORT_TCP;
 				break;
+			case Opt_xprt_rdma6:
+				protofamily = AF_INET6;
 			case Opt_xprt_rdma:
 				/* vector side protocols to TCP */
 				mnt->flags |= NFS_MOUNT_TCP;
@@ -1490,6 +1494,8 @@  static int nfs_parse_mount_options(char *raw,
 			case Opt_xprt_tcp:
 				mnt->mount_server.protocol = XPRT_TRANSPORT_TCP;
 				break;
+			case Opt_xprt_rdma6:
+				mountfamily = AF_INET6;
 			case Opt_xprt_rdma: /* not used for side protocols */
 			default:
 				dfprintk(MOUNT, "NFS:   unrecognized "
diff --git a/include/linux/sunrpc/msg_prot.h b/include/linux/sunrpc/msg_prot.h
index 8073713..49b8433 100644
--- a/include/linux/sunrpc/msg_prot.h
+++ b/include/linux/sunrpc/msg_prot.h
@@ -149,6 +149,7 @@  typedef __be32	rpc_fraghdr;
 #define RPCBIND_NETID_UDP	"udp"
 #define RPCBIND_NETID_TCP	"tcp"
 #define RPCBIND_NETID_RDMA	"rdma"
+#define RPCBIND_NETID_RDMA6	"rdma6"
 #define RPCBIND_NETID_SCTP	"sctp"
 #define RPCBIND_NETID_UDP6	"udp6"
 #define RPCBIND_NETID_TCP6	"tcp6"