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

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

[Anna Schumaker]

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

[PATCH 1/3] nfs: Call nfs4_state_protect() from nfs4_proc_commit_setup()

[Anna Schumaker]

This keeps the function call internal to NFS v4, so the generic client
doesn't need to have a macro for the case that NFS v4 is compiled out.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs3proc.c       | 4 +++-
 fs/nfs/nfs4_fs.h        | 1 -
 fs/nfs/nfs4proc.c       | 5 ++++-
 fs/nfs/proc.c           | 3 ++-
 fs/nfs/write.c          | 5 +----
 include/linux/nfs_xdr.h | 3 ++-
 6 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 524f9f8..f6e6ee3 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -851,7 +851,9 @@ static int nfs3_commit_done(struct rpc_task *task, struct nfs_commit_data *data)
 	return 0;
 }
 
-static void nfs3_proc_commit_setup(struct nfs_commit_data *data, struct rpc_message *msg)
+static void nfs3_proc_commit_setup(struct rpc_clnt *clnt,
+				   struct nfs_commit_data *data,
+				   struct rpc_message *msg)
 {
 	msg->rpc_proc = &nfs3_procedures[NFS3PROC_COMMIT];
 }
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index a081787..5cc2edb 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -513,7 +513,6 @@ static inline bool nfs4_valid_open_stateid(const struct nfs4_state *state)
 
 #define nfs4_close_state(a, b) do { } while (0)
 #define nfs4_close_sync(a, b) do { } while (0)
-#define nfs4_state_protect(a, b, c, d) do { } while (0)
 #define nfs4_state_protect_write(a, b, c, d) do { } while (0)
 
 #endif /* CONFIG_NFS_V4 */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e7f8d5f..9a872d8 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4287,7 +4287,9 @@ static int nfs4_commit_done(struct rpc_task *task, struct nfs_commit_data *data)
 	return data->commit_done_cb(task, data);
 }
 
-static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_message *msg)
+static void nfs4_proc_commit_setup(struct rpc_clnt *clnt,
+				   struct nfs_commit_data *data,
+				   struct rpc_message *msg)
 {
 	struct nfs_server *server = NFS_SERVER(data->inode);
 
@@ -4296,6 +4298,7 @@ static void nfs4_proc_commit_setup(struct nfs_commit_data *data, struct rpc_mess
 	data->res.server = server;
 	msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT];
 	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
+	nfs4_state_protect(server->nfs_client, NFS_SP4_MACH_CRED_COMMIT, &clnt, msg);
 }
 
 struct nfs4_renewdata {
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index b09cc23..ae8e0f2 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -630,7 +630,8 @@ static void nfs_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit
 }
 
 static void
-nfs_proc_commit_setup(struct nfs_commit_data *data, struct rpc_message *msg)
+nfs_proc_commit_setup(struct rpc_clnt *clnt, struct nfs_commit_data *data,
+		      struct rpc_message *msg)
 {
 	BUG();
 }
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index af3af68..1c6d52b 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1486,13 +1486,10 @@ int nfs_initiate_commit(struct rpc_clnt *clnt, struct nfs_commit_data *data,
 		.priority = priority,
 	};
 	/* Set up the initial task struct.  */
-	NFS_PROTO(data->inode)->commit_setup(data, &msg);
+	NFS_PROTO(data->inode)->commit_setup(clnt, data, &msg);
 
 	dprintk("NFS: %5u initiated commit call\n", data->task.tk_pid);
 
-	nfs4_state_protect(NFS_SERVER(data->inode)->nfs_client,
-		NFS_SP4_MACH_CRED_COMMIT, &task_setup_data.rpc_client, &msg);
-
 	task = rpc_run_task(&task_setup_data);
 	if (IS_ERR(task))
 		return PTR_ERR(task);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 467c84e..f01b9a1 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1466,7 +1466,8 @@ struct nfs_rpc_ops {
 	int	(*read_done)(struct rpc_task *, struct nfs_pgio_header *);
 	void	(*write_setup)(struct nfs_pgio_header *, struct rpc_message *);
 	int	(*write_done)(struct rpc_task *, struct nfs_pgio_header *);
-	void	(*commit_setup) (struct nfs_commit_data *, struct rpc_message *);
+	void	(*commit_setup) (struct rpc_clnt *clnt, struct nfs_commit_data *,
+				 struct rpc_message *);
 	void	(*commit_rpc_prepare)(struct rpc_task *, struct nfs_commit_data *);
 	int	(*commit_done) (struct rpc_task *, struct nfs_commit_data *);
 	int	(*lock)(struct file *, int, struct file_lock *);
-- 
2.2.1

[PATCH 2/3] nfs: Call nfs4_state_protect_write() from nfs4_proc_write_setup()

[Anna Schumaker]

This makes the function call internal to the NFS v4 module, so the
generic client no longer needs a macro for the case where NFS v4 is
compiled out.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs3proc.c       | 3 ++-
 fs/nfs/nfs4_fs.h        | 1 -
 fs/nfs/nfs4proc.c       | 4 +++-
 fs/nfs/proc.c           | 3 ++-
 fs/nfs/write.c          | 5 +----
 include/linux/nfs_xdr.h | 3 ++-
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index f6e6ee3..f9b2a9d 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -832,7 +832,8 @@ static int nfs3_write_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
 	return 0;
 }
 
-static void nfs3_proc_write_setup(struct nfs_pgio_header *hdr,
+static void nfs3_proc_write_setup(struct rpc_clnt *clnt,
+				  struct nfs_pgio_header *hdr,
 				  struct rpc_message *msg)
 {
 	msg->rpc_proc = &nfs3_procedures[NFS3PROC_WRITE];
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 5cc2edb..3694895 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -513,7 +513,6 @@ static inline bool nfs4_valid_open_stateid(const struct nfs4_state *state)
 
 #define nfs4_close_state(a, b) do { } while (0)
 #define nfs4_close_sync(a, b) do { } while (0)
-#define nfs4_state_protect_write(a, b, c, d) do { } while (0)
 
 #endif /* CONFIG_NFS_V4 */
 #endif /* __LINUX_FS_NFS_NFS4_FS.H */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 9a872d8..ff3c36f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4239,7 +4239,8 @@ bool nfs4_write_need_cache_consistency_data(struct nfs_pgio_header *hdr)
 	return nfs4_have_delegation(hdr->inode, FMODE_READ) == 0;
 }
 
-static void nfs4_proc_write_setup(struct nfs_pgio_header *hdr,
+static void nfs4_proc_write_setup(struct rpc_clnt *clnt,
+				  struct nfs_pgio_header *hdr,
 				  struct rpc_message *msg)
 {
 	struct nfs_server *server = NFS_SERVER(hdr->inode);
@@ -4257,6 +4258,7 @@ static void nfs4_proc_write_setup(struct nfs_pgio_header *hdr,
 
 	msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_WRITE];
 	nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 1);
+	nfs4_state_protect_write(server->nfs_client, &clnt, msg, hdr);
 }
 
 static void nfs4_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index ae8e0f2..18ead52 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -616,7 +616,8 @@ static int nfs_write_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
 	return 0;
 }
 
-static void nfs_proc_write_setup(struct nfs_pgio_header *hdr,
+static void nfs_proc_write_setup(struct rpc_clnt *clnt,
+				 struct nfs_pgio_header *hdr,
 				 struct rpc_message *msg)
 {
 	/* Note: NFSv2 ignores @stable and always uses NFS_FILE_SYNC */
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 1c6d52b..0d37c5d 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1246,10 +1246,7 @@ static void nfs_initiate_write(struct nfs_pgio_header *hdr,
 	int priority = flush_task_priority(how);
 
 	task_setup_data->priority = priority;
-	NFS_PROTO(inode)->write_setup(hdr, msg);
-
-	nfs4_state_protect_write(NFS_SERVER(inode)->nfs_client,
-				 &task_setup_data->rpc_client, msg, hdr);
+	NFS_PROTO(inode)->write_setup(task_setup_data->rpc_client, hdr, msg);
 }
 
 /* If a nfs_flush_* function fails, it should remove reqs from @head and
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index f01b9a1..96c9fdf 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1464,7 +1464,8 @@ struct nfs_rpc_ops {
 				    struct nfs_pgio_header *);
 	void	(*read_setup)(struct nfs_pgio_header *, struct rpc_message *);
 	int	(*read_done)(struct rpc_task *, struct nfs_pgio_header *);
-	void	(*write_setup)(struct nfs_pgio_header *, struct rpc_message *);
+	void	(*write_setup)(struct rpc_clnt *, struct nfs_pgio_header *,
+			       struct rpc_message *);
 	int	(*write_done)(struct rpc_task *, struct nfs_pgio_header *);
 	void	(*commit_setup) (struct rpc_clnt *clnt, struct nfs_commit_data *,
 				 struct rpc_message *);
-- 
2.2.1

[PATCH 3/3] nfs: Remove unused v4 macros

[Anna Schumaker]

These functions are never called in the case that NFS v4 is not enabled.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/nfs4_fs.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 3694895..f39314b 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -509,10 +509,5 @@ static inline bool nfs4_valid_open_stateid(const struct nfs4_state *state)
 	return test_bit(NFS_STATE_RECOVERY_FAILED, &state->flags) == 0;
 }
 
-#else
-
-#define nfs4_close_state(a, b) do { } while (0)
-#define nfs4_close_sync(a, b) do { } while (0)
-
 #endif /* CONFIG_NFS_V4 */
 #endif /* __LINUX_FS_NFS_NFS4_FS.H */
-- 
2.2.1

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

[Weston Andros Adamson]

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?
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.

-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

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

[Anna Schumaker]

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?

Anna

> -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
> 

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

[Weston Andros Adamson]

> 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.

-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
>> 
> 

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

[Weston Andros Adamson]

> 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

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

[J. Bruce Fields]

On Mon, Jan 05, 2015 at 03:31:46PM -0500, 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?
> 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.

The Linux server should support MACH_CRED as of
57266a6e916e2522ea61758a3ee5576b60156791 "nfsd4: implement minimal
SP4_MACH_CRED".  (Well, plus some later bugfixes.)  But I think anything
since 3.14 should be OK.

That said, I wouldn't be surprised if it has problems.  But please do
test against that and let me know....

--b.

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

[Weston Andros Adamson]

> On Jan 6, 2015, at 2:08 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Mon, Jan 05, 2015 at 03:31:46PM -0500, 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?
>> 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.
> 
> The Linux server should support MACH_CRED as of
> 57266a6e916e2522ea61758a3ee5576b60156791 "nfsd4: implement minimal
> SP4_MACH_CRED".  (Well, plus some later bugfixes.)  But I think anything
> since 3.14 should be OK.
> 
> That said, I wouldn't be surprised if it has problems.  But please do
> test against that and let me know....
> 
> --b.

Ah, right, but only for state operations that don’t touch the filesystem:

OP_BIND_CONN_TO_SESSION
OP_EXCHANGE_ID
OP_CREATE_SESSION
OP_DESTROY_SESSION
OP_DESTROY_CLIENTID

Which is not that interesting, since the client should already be using the machine cred
with these operations.

What is interesting is supporting write and commit (and associated ops, i.e. sequence).
That way when a client is doing buffered writes and the user cred expires, it can flush the
locally cached data. This is what the linux client SP4_MACH_CRED feature focused on.

I think implementing SP4_MACH_CRED for these operations has the issue I mentioned
earlier: the fh_verify path will have to check credentials against some cached credential
(tied to the stateid), because request will contain the machine credential and not the user
credential that previous writes (before cred expiration) used.

-dros

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

[J. Bruce Fields]

On Wed, Jan 07, 2015 at 01:47:53PM -0500, Weston Andros Adamson wrote:
> Ah, right, but only for state operations that don’t touch the filesystem:
> 
> OP_BIND_CONN_TO_SESSION
> OP_EXCHANGE_ID
> OP_CREATE_SESSION
> OP_DESTROY_SESSION
> OP_DESTROY_CLIENTID
> 
> Which is not that interesting, since the client should already be using the machine cred
> with these operations.
> 
> What is interesting is supporting write and commit (and associated ops, i.e. sequence).
> That way when a client is doing buffered writes and the user cred expires, it can flush the
> locally cached data. This is what the linux client SP4_MACH_CRED feature focused on.
> 
> I think implementing SP4_MACH_CRED for these operations has the issue I mentioned
> earlier: the fh_verify path will have to check credentials against some cached credential
> (tied to the stateid), because request will contain the machine credential and not the user
> credential that previous writes (before cred expiration) used.

Oh, I see.  Yeah, that sounds like a bigger project.

--b.

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

[J. Bruce Fields]

On Wed, Jan 07, 2015 at 01:55:25PM -0500, J. Bruce Fields wrote:
> On Wed, Jan 07, 2015 at 01:47:53PM -0500, Weston Andros Adamson wrote:
> > Ah, right, but only for state operations that don’t touch the filesystem:
> > 
> > OP_BIND_CONN_TO_SESSION
> > OP_EXCHANGE_ID
> > OP_CREATE_SESSION
> > OP_DESTROY_SESSION
> > OP_DESTROY_CLIENTID
> > 
> > Which is not that interesting, since the client should already be using the machine cred
> > with these operations.
> > 
> > What is interesting is supporting write and commit (and associated ops, i.e. sequence).
> > That way when a client is doing buffered writes and the user cred expires, it can flush the
> > locally cached data. This is what the linux client SP4_MACH_CRED feature focused on.
> > 
> > I think implementing SP4_MACH_CRED for these operations has the issue I mentioned
> > earlier: the fh_verify path will have to check credentials against some cached credential
> > (tied to the stateid), because request will contain the machine credential and not the user
> > credential that previous writes (before cred expiration) used.
> 
> Oh, I see.  Yeah, that sounds like a bigger project.

(And I'd be curious what the security model is.)

--b.