2009-11-02 19:03:53

by Peter Staubach

[permalink] [raw]
Subject: [PATCH] register NFS_ACL with rpcbind

Hi.

Here is a patch to modify the NFS server to register the NFS_ACL
services with the rpcbind daemon. This allows the client to
ping for the existence of the NFS_ACL support via commands such
as "rpcinfo -t <server> nfs_acl".

The changelog for the patch which turned off this functionality
mentioned something about not registering the NFS_ACL as being
part of some tradition. I can't find this tradition and the
only other implementation which supports NFS_ACL does register
them with the rpcbind daemon.

Thanx...

ps

Signed-off-by: Peter Staubach <[email protected]>

--- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
@@ -346,5 +346,5 @@ struct svc_version nfsd_acl_version2 = {
.vs_proc = nfsd_acl_procedures2,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
- .vs_hidden = 1,
+ .vs_hidden = 0,
};
--- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
@@ -264,6 +264,6 @@ struct svc_version nfsd_acl_version3 = {
.vs_proc = nfsd_acl_procedures3,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
- .vs_hidden = 1,
+ .vs_hidden = 0,
};



2009-11-03 09:11:29

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

On Monday 02 November 2009 10:59:07 pm Peter Staubach wrote:
> Hi.
>
> Here is a patch to modify the NFS server to register the NFS_ACL
> services with the rpcbind daemon. This allows the client to
> ping for the existence of the NFS_ACL support via commands such
> as "rpcinfo -t <server> nfs_acl".
>
> This patch also modifies the NFS_ACL support so that responses
> to version 2 NULLPROC requests can be made.
>
> The changelog for the patch which turned off this functionality
> mentioned something about not registering the NFS_ACL as being
> part of some tradition. I can't find this tradition and the
> only other implementation which supports NFS_ACL does register
> them with the rpcbind daemon.

I don't understand the reasoning behind .vs_hidden for NFS_ACL, hopefully Olaf
can clarify. NFS_ACL is the only user of .vs_hidden as far as I can see
though, so if this is changeg, shouldn't the entire commit bc5fea4 which
introduced the flag be reverted?

Thanks,
Andreas

2009-11-03 09:19:27

by Olaf Kirch

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
> I don't understand the reasoning behind .vs_hidden for NFS_ACL, hopef=
ully
> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as I =
can
> see though, so if this is changeg, shouldn't the entire commit bc5fea=
4
> which introduced the flag be reverted?

I can't remember the details of that one. I do remember that this is
based on someone's request who told me that we shouldn't register nfsac=
l
with portmap. I didn't check myself whether Solaris did or did not do
it at that time.

I have no issue with reverting that change, and removing the whole
=2Evs_hidden kludge too.

Thanks,
Olaf
--=20
Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 N=FCrnberg
GF: Markus Rex, HRB 16746 (AG N=FCrnberg)


2009-11-03 15:28:45

by Peter Staubach

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

Olaf Kirch wrote:
> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>> I don't understand the reasoning behind .vs_hidden for NFS_ACL, hopefully
>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as I can
>> see though, so if this is changeg, shouldn't the entire commit bc5fea4
>> which introduced the flag be reverted?
>
> I can't remember the details of that one. I do remember that this is
> based on someone's request who told me that we shouldn't register nfsacl
> with portmap. I didn't check myself whether Solaris did or did not do
> it at that time.
>
> I have no issue with reverting that change, and removing the whole
> .vs_hidden kludge too.
>

It seems that vs_hidden is used in 1 place outside of the NFS_ACL
server code. It is used in the NFSv4 callback code.

I will look to see how difficult that might be to fix this spot
as well and then get rid of vs_hidden.

Thanx...

ps

2009-11-03 15:34:37

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

On Nov 3, 2009, at 10:28 AM, Peter Staubach wrote:
> Olaf Kirch wrote:
>> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>>> I don't understand the reasoning behind .vs_hidden for NFS_ACL,
>>> hopefully
>>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as
>>> I can
>>> see though, so if this is changeg, shouldn't the entire commit
>>> bc5fea4
>>> which introduced the flag be reverted?
>>
>> I can't remember the details of that one. I do remember that this is
>> based on someone's request who told me that we shouldn't register
>> nfsacl
>> with portmap. I didn't check myself whether Solaris did or did not do
>> it at that time.
>>
>> I have no issue with reverting that change, and removing the whole
>> .vs_hidden kludge too.
>>
>
> It seems that vs_hidden is used in 1 place outside of the NFS_ACL
> server code. It is used in the NFSv4 callback code.
>
> I will look to see how difficult that might be to fix this spot
> as well and then get rid of vs_hidden.

See archive of this mailing list from earlier in October. This change
was added because it's hard to get rid of the svc_unregister() call
done by svc_create().

I have another solution for that problem that I'm preparing for 2.6.33.

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




2009-11-04 18:44:25

by Peter Staubach

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

Chuck Lever wrote:
> On Nov 3, 2009, at 10:28 AM, Peter Staubach wrote:
>> Olaf Kirch wrote:
>>> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>>>> I don't understand the reasoning behind .vs_hidden for NFS_ACL,
>>>> hopefully
>>>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as I
>>>> can
>>>> see though, so if this is changeg, shouldn't the entire commit bc5fea4
>>>> which introduced the flag be reverted?
>>>
>>> I can't remember the details of that one. I do remember that this is
>>> based on someone's request who told me that we shouldn't register nfsacl
>>> with portmap. I didn't check myself whether Solaris did or did not do
>>> it at that time.
>>>
>>> I have no issue with reverting that change, and removing the whole
>>> .vs_hidden kludge too.
>>>
>>
>> It seems that vs_hidden is used in 1 place outside of the NFS_ACL
>> server code. It is used in the NFSv4 callback code.
>>
>> I will look to see how difficult that might be to fix this spot
>> as well and then get rid of vs_hidden.
>
> See archive of this mailing list from earlier in October. This change
> was added because it's hard to get rid of the svc_unregister() call done
> by svc_create().
>
> I have another solution for that problem that I'm preparing for 2.6.33.
>

Cool.

In the meantime, can we get this one in, Bruce?

Thanx...

ps

2009-11-04 18:58:40

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

On Mon, Nov 02, 2009 at 04:59:07PM -0500, Peter Staubach wrote:
> Hi.
>
> Here is a patch to modify the NFS server to register the NFS_ACL
> services with the rpcbind daemon. This allows the client to
> ping for the existence of the NFS_ACL support via commands such
> as "rpcinfo -t <server> nfs_acl".
>
> This patch also modifies the NFS_ACL support so that responses
> to version 2 NULLPROC requests can be made.
>
> The changelog for the patch which turned off this functionality
> mentioned something about not registering the NFS_ACL as being
> part of some tradition. I can't find this tradition and the
> only other implementation which supports NFS_ACL does register
> them with the rpcbind daemon.
>
> Thanx...
>
> ps
>
> Signed-off-by: Peter Staubach <[email protected]>
>
> --- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
> +++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
> @@ -217,6 +217,16 @@ static int nfsaclsvc_decode_accessargs(s
> * XDR encode functions
> */
>
> +/*
> + * There must be an encoding function for void results so svc_process
> + * will work properly.
> + */
> +int
> +nfsaclsvc_encode_voidres(struct svc_rqst *rqstp, __be32 *p, void *dummy)
> +{
> + return xdr_ressize_check(rqstp, p);
> +}

Out of curiosity: have you tested a null rpc?

Also, doesn't v3 have the same problem?

--b.

> +
> /* GETACL */
> static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
> struct nfsd3_getaclres *resp)
> @@ -308,7 +318,6 @@ static int nfsaclsvc_release_access(stru
> }
>
> #define nfsaclsvc_decode_voidargs NULL
> -#define nfsaclsvc_encode_voidres NULL
> #define nfsaclsvc_release_void NULL
> #define nfsd3_fhandleargs nfsd_fhandle
> #define nfsd3_attrstatres nfsd_attrstat
> @@ -346,5 +355,5 @@ struct svc_version nfsd_acl_version2 = {
> .vs_proc = nfsd_acl_procedures2,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> - .vs_hidden = 1,
> + .vs_hidden = 0,
> };
> --- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
> +++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
> @@ -264,6 +264,6 @@ struct svc_version nfsd_acl_version3 = {
> .vs_proc = nfsd_acl_procedures3,
> .vs_dispatch = nfsd_dispatch,
> .vs_xdrsize = NFS3_SVC_XDRSIZE,
> - .vs_hidden = 1,
> + .vs_hidden = 0,
> };
>

2009-11-04 19:46:09

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

On Nov 4, 2009, at 1:44 PM, Peter Staubach wrote:
> Chuck Lever wrote:
>> On Nov 3, 2009, at 10:28 AM, Peter Staubach wrote:
>>> Olaf Kirch wrote:
>>>> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>>>>> I don't understand the reasoning behind .vs_hidden for NFS_ACL,
>>>>> hopefully
>>>>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far
>>>>> as I
>>>>> can
>>>>> see though, so if this is changeg, shouldn't the entire commit
>>>>> bc5fea4
>>>>> which introduced the flag be reverted?
>>>>
>>>> I can't remember the details of that one. I do remember that this
>>>> is
>>>> based on someone's request who told me that we shouldn't register
>>>> nfsacl
>>>> with portmap. I didn't check myself whether Solaris did or did
>>>> not do
>>>> it at that time.
>>>>
>>>> I have no issue with reverting that change, and removing the whole
>>>> .vs_hidden kludge too.
>>>>
>>>
>>> It seems that vs_hidden is used in 1 place outside of the NFS_ACL
>>> server code. It is used in the NFSv4 callback code.
>>>
>>> I will look to see how difficult that might be to fix this spot
>>> as well and then get rid of vs_hidden.
>>
>> See archive of this mailing list from earlier in October. This
>> change
>> was added because it's hard to get rid of the svc_unregister() call
>> done
>> by svc_create().
>>
>> I have another solution for that problem that I'm preparing for
>> 2.6.33.
>>
>
> Cool.
>
> In the meantime, can we get this one in, Bruce?

As far as I know, there is no "meantime" in this case. 2.6.33 is the
next merge window.

I don't have a problem with getting rid of .vs_hidden anyway... all
I'm saying is maybe you don't have to work too hard at it. :-)

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




2009-11-04 19:54:14

by Peter Staubach

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

J. Bruce Fields wrote:
> On Mon, Nov 02, 2009 at 04:59:07PM -0500, Peter Staubach wrote:
>> Hi.
>>
>> Here is a patch to modify the NFS server to register the NFS_ACL
>> services with the rpcbind daemon. This allows the client to
>> ping for the existence of the NFS_ACL support via commands such
>> as "rpcinfo -t <server> nfs_acl".
>>
>> This patch also modifies the NFS_ACL support so that responses
>> to version 2 NULLPROC requests can be made.
>>
>> The changelog for the patch which turned off this functionality
>> mentioned something about not registering the NFS_ACL as being
>> part of some tradition. I can't find this tradition and the
>> only other implementation which supports NFS_ACL does register
>> them with the rpcbind daemon.
>>
>> Thanx...
>>
>> ps
>>
>> Signed-off-by: Peter Staubach <[email protected]>
>>
>> --- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
>> +++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
>> @@ -217,6 +217,16 @@ static int nfsaclsvc_decode_accessargs(s
>> * XDR encode functions
>> */
>>
>> +/*
>> + * There must be an encoding function for void results so svc_process
>> + * will work properly.
>> + */
>> +int
>> +nfsaclsvc_encode_voidres(struct svc_rqst *rqstp, __be32 *p, void *dummy)
>> +{
>> + return xdr_ressize_check(rqstp, p);
>> +}
>
> Out of curiosity: have you tested a null rpc?
>

Yup. "rpcinfo -t <server> nfs_acl" works like a charm
with this patch applied for both versions 2 and 3.

> Also, doesn't v3 have the same problem?
>

Nope. It already had a similar XDR routine. Which is
similar to the NFS XDR routine to do the same sort of
thing. I suppose that someday, we could clean them up,
but these routines aren't doing any harm for the moment.

Thanx...

ps

> --b.
>
>> +
>> /* GETACL */
>> static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
>> struct nfsd3_getaclres *resp)
>> @@ -308,7 +318,6 @@ static int nfsaclsvc_release_access(stru
>> }
>>
>> #define nfsaclsvc_decode_voidargs NULL
>> -#define nfsaclsvc_encode_voidres NULL
>> #define nfsaclsvc_release_void NULL
>> #define nfsd3_fhandleargs nfsd_fhandle
>> #define nfsd3_attrstatres nfsd_attrstat
>> @@ -346,5 +355,5 @@ struct svc_version nfsd_acl_version2 = {
>> .vs_proc = nfsd_acl_procedures2,
>> .vs_dispatch = nfsd_dispatch,
>> .vs_xdrsize = NFS3_SVC_XDRSIZE,
>> - .vs_hidden = 1,
>> + .vs_hidden = 0,
>> };
>> --- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
>> +++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
>> @@ -264,6 +264,6 @@ struct svc_version nfsd_acl_version3 = {
>> .vs_proc = nfsd_acl_procedures3,
>> .vs_dispatch = nfsd_dispatch,
>> .vs_xdrsize = NFS3_SVC_XDRSIZE,
>> - .vs_hidden = 1,
>> + .vs_hidden = 0,
>> };
>>


2009-11-05 16:56:06

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v2] register NFS_ACL with rpcbind

On Wed, Nov 04, 2009 at 02:54:12PM -0500, Peter Staubach wrote:
> J. Bruce Fields wrote:
> > Out of curiosity: have you tested a null rpc?
> >
>
> Yup. "rpcinfo -t <server> nfs_acl" works like a charm
> with this patch applied for both versions 2 and 3.
>
> > Also, doesn't v3 have the same problem?
> >
>
> Nope. It already had a similar XDR routine. Which is
> similar to the NFS XDR routine to do the same sort of
> thing. I suppose that someday, we could clean them up,

Yes.

> but these routines aren't doing any harm for the moment.

OK. Applied.

--b.

2009-11-02 21:59:05

by Peter Staubach

[permalink] [raw]
Subject: [PATCH v2] register NFS_ACL with rpcbind

Hi.

Here is a patch to modify the NFS server to register the NFS_ACL
services with the rpcbind daemon. This allows the client to
ping for the existence of the NFS_ACL support via commands such
as "rpcinfo -t <server> nfs_acl".

This patch also modifies the NFS_ACL support so that responses
to version 2 NULLPROC requests can be made.

The changelog for the patch which turned off this functionality
mentioned something about not registering the NFS_ACL as being
part of some tradition. I can't find this tradition and the
only other implementation which supports NFS_ACL does register
them with the rpcbind daemon.

Thanx...

ps

Signed-off-by: Peter Staubach <[email protected]>

--- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
@@ -217,6 +217,16 @@ static int nfsaclsvc_decode_accessargs(s
* XDR encode functions
*/

+/*
+ * There must be an encoding function for void results so svc_process
+ * will work properly.
+ */
+int
+nfsaclsvc_encode_voidres(struct svc_rqst *rqstp, __be32 *p, void *dummy)
+{
+ return xdr_ressize_check(rqstp, p);
+}
+
/* GETACL */
static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
struct nfsd3_getaclres *resp)
@@ -308,7 +318,6 @@ static int nfsaclsvc_release_access(stru
}

#define nfsaclsvc_decode_voidargs NULL
-#define nfsaclsvc_encode_voidres NULL
#define nfsaclsvc_release_void NULL
#define nfsd3_fhandleargs nfsd_fhandle
#define nfsd3_attrstatres nfsd_attrstat
@@ -346,5 +355,5 @@ struct svc_version nfsd_acl_version2 = {
.vs_proc = nfsd_acl_procedures2,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
- .vs_hidden = 1,
+ .vs_hidden = 0,
};
--- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
@@ -264,6 +264,6 @@ struct svc_version nfsd_acl_version3 = {
.vs_proc = nfsd_acl_procedures3,
.vs_dispatch = nfsd_dispatch,
.vs_xdrsize = NFS3_SVC_XDRSIZE,
- .vs_hidden = 1,
+ .vs_hidden = 0,
};