Re: [PATCH 0/3] Remove function macros from nfs4_fs.h

[Weston Andros Adamson <dros@primarydata.com>]

> On Jan 5, 2015, at 4:51 PM, Weston Andros Adamson <dros@primarydata.com> wrote:
> 
>> 
>> On Jan 5, 2015, at 4:06 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>> 
>> On 01/05/2015 03:31 PM, Weston Andros Adamson wrote:
>>> These patches look good to me, but have you tested them? ;)
>>> 
>>> I mean, does anyone have a server that implements SP4_MACH_CRED to test against?
>> 
>> I've done basic (non SP4) testing, but I don't have an SP4_MACH_CRED server to test against.
>> 
>>> When I originally developed this feature, I tested against a hacked nfsd…
>>> that code was really ugly (not ready for upstreaming), but allowed me to test the client
>>> feature.
>>> 
>>> IRRC the server side is difficult because the server has to keep stateid to credential
>>> mappings, so when the machine cred was used it could check access against the acting cred. 
>>> 
>>> If there aren’t any servers to test this against, maybe we remove this feature? It can always
>>> be revived once there is a server to test against.
>>> 
>> I'm open to whatever!  Do you remember how complicated it was to set up the basic SP4 server when you did your testing?
> 
> Pretty complicated.
> 
> I hacked up knfsd to allow requests that use the machine credential instead of the expected
> user credential and when the machine credential was used, it would skip all credential permission
> checks in nfsd — again, only good for testing the client feature….
> 
> There were also some changes to nfsd to advertise the availability of SP4_MACH_CRED in
> the exchange_id.
> 
> I might be able to find these patches, but they’d need merging.
> 
> To test:
> - set up server with working krb5i share, obviously with configured machine credential
> - kinit as a user (not machine cred) for a short amount of time (see kinit’s -l / —lifetime flag).
> - do buffered writes past the lifetime of the kerberos ticket.
> - verify that the writes after expiration are using the machine credential (inspect rpc cred in
>    wireshark)
> 
> So, I think your cleanups look good - let’s go with them for now.
> 
> As far as removing SP4_MACH_CRED from the client, we should ask the list if there
> are any servers that implement it and if the client works against their implementation and go
> from there.

My sources tell me that NetApp servers might actually support SP4_MACH_CRED! Can you test
the current code against one?

-dros


>>> 
>>>> On Jan 5, 2015, at 2:17 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
>>>> 
>>>> While reviewing Tom's flexfile patches I found a few places where
>>>> nfs4_state_protect() was being called inside the generic client, rather
>>>> than in the nfsv4 module.  These patches move the function calls into
>>>> the correct layer and then tidy up nfs4_fs.h once everything has been
>>>> moved.
>>>> 
>>>> Thoughts?
>>>> 
>>>> Anna
>>>> 
>>>> 
>>>> Anna Schumaker (3):
>>>> nfs: Call nfs4_state_protect() from nfs4_proc_commit_setup()
>>>> nfs: Call nfs4_state_protect_write() from nfs4_proc_write_setup()
>>>> nfs: Remove unused v4 macros
>>>> 
>>>> fs/nfs/nfs3proc.c       |  7 +++++--
>>>> fs/nfs/nfs4_fs.h        |  7 -------
>>>> fs/nfs/nfs4proc.c       |  9 +++++++--
>>>> fs/nfs/proc.c           |  6 ++++--
>>>> fs/nfs/write.c          | 10 ++--------
>>>> include/linux/nfs_xdr.h |  6 ++++--
>>>> 6 files changed, 22 insertions(+), 23 deletions(-)
>>>> 
>>>> -- 
>>>> 2.2.1
>>>> 
>>>> --
>>>> 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