Return-Path: Received: from userp1040.oracle.com ([156.151.31.81]:18768 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751430AbcCWSEG (ORCPT ); Wed, 23 Mar 2016 14:04:06 -0400 Message-ID: <56F2DA89.9040905@oracle.com> Date: Wed, 23 Mar 2016 11:03:53 -0700 From: Shirley Ma MIME-Version: 1.0 To: Chuck Lever , Trond Myklebust CC: Anna Schumaker , "J. Bruce Fields" , Leon Romanovsky , Linux NFS Mailing List Subject: Re: [PATCH] nfs: add nfs IPv6 rdma6 mount option support References: <56E9A650.7060409@oracle.com> <56F19F28.9020504@Netapp.com> <9414CE65-D41C-4497-9111-BCAB54FB6C7B@oracle.com> <4A9EA080-30CC-4DD8-9BD8-28D54F13C582@oracle.com> In-Reply-To: <4A9EA080-30CC-4DD8-9BD8-28D54F13C582@oracle.com> Content-Type: text/plain; charset=windows-1252 Sender: linux-nfs-owner@vger.kernel.org List-ID: On 03/22/2016 03:01 PM, Chuck Lever wrote: > >> On Mar 22, 2016, at 5:55 PM, Trond Myklebust wrote: >> >> On Tue, Mar 22, 2016 at 5:52 PM, Chuck Lever wrote: >>> >>>> On Mar 22, 2016, at 5:46 PM, Trond Myklebust wrote: >>>> >>>> On Tue, Mar 22, 2016 at 5:24 PM, Chuck Lever wrote: >>>>> >>>>>> On Mar 22, 2016, at 3:38 PM, Anna Schumaker 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 >>>>>> >>>>>> 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 > > >