2013-06-25 18:37:42

by Andy Adamson

[permalink] [raw]
Subject: [PATCH 1/3] NFSv4.1 Add the RPC header and other overhead to session parameters request

From: Andy Adamson <[email protected]>

ca_maxresponsesize and ca_maxrequestsize include the RPC header. Add the
RPC header size and other xdr overhead in order to request NFS_MAX_FILE_IO_SIZE
of read and write data.

Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3e40876..b569feb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6189,9 +6189,9 @@ static void nfs4_init_channel_attrs(struct nfs41_create_session_args *args)
mxresp_sz = session->fc_target_max_resp_sz;

if (mxrqst_sz == 0)
- mxrqst_sz = NFS_MAX_FILE_IO_SIZE;
+ mxrqst_sz = NFS_MAX_FILE_IO_SIZE + nfs41_maxwrite_overhead;
if (mxresp_sz == 0)
- mxresp_sz = NFS_MAX_FILE_IO_SIZE;
+ mxresp_sz = NFS_MAX_FILE_IO_SIZE + nfs41_maxread_overhead;
/* Fore channel attributes */
args->fc_attrs.max_rqst_sz = mxrqst_sz;
args->fc_attrs.max_resp_sz = mxresp_sz;
--
1.7.11.7



2013-06-25 18:37:42

by Andy Adamson

[permalink] [raw]
Subject: [PATCH 2/3] NFSv4.1 Do not overwrite negotiated session sizes

From: Andy Adamson <[email protected]>

nfs4_init_session is called after CREATE_SESSION has set the ca_maxrequestsize
and ca_maxresponsesize values obtained from the server.

Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4session.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
index c4e225e..7ac14cc 100644
--- a/fs/nfs/nfs4session.c
+++ b/fs/nfs/nfs4session.c
@@ -499,13 +499,11 @@ int nfs4_init_session(struct nfs_server *server)
session = clp->cl_session;
spin_lock(&clp->cl_lock);
if (test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state)) {
- /* Initialise targets and channel attributes */
+ /* Initialise targets */
session->fc_target_max_rqst_sz = target_max_rqst_sz;
- session->fc_attrs.max_rqst_sz = target_max_rqst_sz;
session->fc_target_max_resp_sz = target_max_resp_sz;
- session->fc_attrs.max_resp_sz = target_max_resp_sz;
} else {
- /* Just adjust the targets */
+ /* Adjust the targets and perhaps reset the session */
if (target_max_rqst_sz > session->fc_target_max_rqst_sz) {
session->fc_target_max_rqst_sz = target_max_rqst_sz;
set_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state);
--
1.7.11.7


2013-06-25 18:52:56

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFSv4.1 Do not overwrite negotiated session sizes

On Tue, 2013-06-25 at 14:37 -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> nfs4_init_session is called after CREATE_SESSION has set the ca_maxrequestsize
> and ca_maxresponsesize values obtained from the server.

It had better not be, otherwise the values of
session->fc_target_max_rqst_sz and session->fc_target_max_resp_sz in
nfs4_init_channel_attrs() make no sense.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-06-25 18:37:43

by Andy Adamson

[permalink] [raw]
Subject: [PATCH 3/3] NFSv4.1 Fix gdia_maxcount calculation to fit in ca_maxresponsesize

From: Andy Adamson <[email protected]>

The GETDEVICEINFO gdia_maxcount represents all of the data being returned
within the GETDEVICEINFO4resok structure and includes the XDR overhead.

The CREATE_SESSION ca_maxresponsesize is the maximum reply and includes the RPC
headers (including security flavor credentials and verifiers).

Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/internal.h | 1 +
fs/nfs/nfs4filelayoutdev.c | 2 +-
fs/nfs/nfs4xdr.c | 6 ++++++
3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 97ec2ef..3c8373f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -255,6 +255,7 @@ extern int nfs4_decode_dirent(struct xdr_stream *,
#ifdef CONFIG_NFS_V4_1
extern const u32 nfs41_maxread_overhead;
extern const u32 nfs41_maxwrite_overhead;
+extern const u32 nfs41_maxgetdevinfo_overhead;
#endif

/* nfs4proc.c */
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index 0493dbd..749ca61 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -709,7 +709,7 @@ filelayout_get_device_info(struct inode *inode,
pdev->layout_type = LAYOUT_NFSV4_1_FILES;
pdev->pages = pages;
pdev->pgbase = 0;
- pdev->pglen = max_resp_sz;
+ pdev->pglen = max_resp_sz - nfs41_maxgetdevinfo_overhead;
pdev->mincount = 0;

rc = nfs4_proc_getdeviceinfo(server, pdev, cred);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 2a3f77e1..dde8e17 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -867,6 +867,12 @@ const u32 nfs41_maxread_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
decode_sequence_maxsz +
decode_putfh_maxsz) *
XDR_UNIT);
+
+const u32 nfs41_maxgetdevinfo_overhead = ((RPC_MAX_REPHEADER_WITH_AUTH +
+ compound_decode_hdr_maxsz +
+ decode_sequence_maxsz) *
+ XDR_UNIT);
+EXPORT_SYMBOL_GPL(nfs41_maxgetdevinfo_overhead);
#endif /* CONFIG_NFS_V4_1 */

static const umode_t nfs_type2fmt[] = {
--
1.7.11.7


2013-06-25 19:05:07

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFSv4.1 Do not overwrite negotiated session sizes


On Jun 25, 2013, at 2:52 PM, "Myklebust, Trond" <[email protected]>
wrote:

> On Tue, 2013-06-25 at 14:37 -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> nfs4_init_session is called after CREATE_SESSION has set the ca_maxrequestsize
>> and ca_maxresponsesize values obtained from the server.
>
> It had better not be, otherwise the values of
> session->fc_target_max_rqst_sz and session->fc_target_max_resp_sz in
> nfs4_init_channel_attrs() make no sense.

Well, it does. Shall I move the nfs4_init_session call to before nfs4_init_channel_attrs?

added dprintk and did a mount:

--> nfs41_init_clientid
--> nfs4_proc_create_session clp=ffff88005ff67800 session=ffff88005ff66c00
nfs4_init_channel_attrs: Fore Channel : max_rqst_sz=1048576 max_resp_sz=1048576 max_ops=8 max_reqs=16
nfs4_init_channel_attrs: BackChannel : max_rqst_sz=4096 max_resp_sz=4096 max_resp_sz_cached=0 max_ops=2 max_reqs=1

# ANDROS: set by server HERE
decode_chan_attrs max_resp_sz 81920
decode_chan_attrs max_resp_sz 4096

--> nfs4_verify_fore_channel_attrs rcvd->max_resp_sz 81920
--> nfs4_setup_session_slot_tables
--> nfs4_realloc_slot_table: max_reqs=16, tbl->max_slots 0
nfs4_realloc_slot_table: tbl=ffff88005ff66c40 slots=ffff88007a5edbc0 max_slots=16
<-- nfs4_realloc_slot_table: return 0
--> nfs4_realloc_slot_table: max_reqs=1, tbl->max_slots 0
nfs4_realloc_slot_table: tbl=ffff88005ff66e08 slots=ffff88007a5ed640 max_slots=1
<-- nfs4_realloc_slot_table: return 0
slot table setup returned 0
nfs4_proc_create_session client>seqid 2 sessionid 150994944:4550600:0:100925440
<-- nfs4_proc_create_session
nfs4_schedule_state_renewal: requeueing work. Lease period = 5

# ANDROS: Still correct when NFS4_CLNT_READY set:

<-- nfs41_init_clientid clp->cl_session->fc_attrs->mas_resp_sz 81920

Invoking bc_svc_process()

# ANDROS: Using session for RECLAIM COMPLETE

--> nfs41_proc_reclaim_complete

--> nfs41_setup_sequence
--> nfs4_alloc_slot used_slots=0000 highest_used=4294967295 max_slots=16
<-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=0
<-- nfs41_setup_sequence slotid=0 seqid=1
encode_sequence: sessionid=150994944:4550600:0:100925440 seqid=1 slotid=0 max_slotid=0 cache_this=0
bc_svc_process() returned w/ error code= 0
--> nfs4_reclaim_complete_done
--> nfs4_alloc_slot used_slots=0001 highest_used=0 max_slots=1
<-- nfs4_alloc_slot used_slots=0001 highest_used=0 slotid=-1
nfs41_sequence_done: Error 0 free the slot
nfs4_free_slot: slotid 0 highest_used_slotid -1
<-- nfs4_reclaim_complete_done


--> nfs_put_client({3})
--> nfs4_match_clientids client ID c86f450000000009 matches c86f450000000009
NFS: --> nfs4_match_serverowners server owners match
NFS: <-- nfs41_walk_client_list using nfs_client = ffff88005ff67800 ({3})
NFS: <-- nfs41_walk_client_list status = 0
--> nfs_put_client({3})
NFS: nfs4_discover_server_trunking: status = 0
--> nfs_put_client({2})
<-- nfs4_set_client() = 0 [new ffff88005ff67800]
<-- nfs4_init_server() = 0

#ANDROS: after using the session, Now we init it????

--> nfs4_init_session
nfs4_init_session taking cl_lock
nfs4_init_session test and clear case

#ANDROS - BUG - reset the max_resp_sz!!
<-- nfs4_init_session session->fc_attrs.max_resp_sz 1049480

> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com


2013-06-25 19:28:45

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSv4.1 Fix gdia_maxcount calculation to fit in ca_maxresponsesize

On Tue, 2013-06-25 at 14:37 -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> The GETDEVICEINFO gdia_maxcount represents all of the data being returned
> within the GETDEVICEINFO4resok structure and includes the XDR overhead.
>
> The CREATE_SESSION ca_maxresponsesize is the maximum reply and includes the RPC
> headers (including security flavor credentials and verifiers).

This still looks wrong.

The problem is that pdev->pglen is used both as the total buffer length,
as a function argument and as the reply message length (through the call
to xdr_init_decode_pages). We really should split out those 3 different
roles...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-06-25 19:44:07

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSv4.1 Fix gdia_maxcount calculation to fit in ca_maxresponsesize


On Jun 25, 2013, at 3:28 PM, "Myklebust, Trond" <[email protected]>
wrote:

> On Tue, 2013-06-25 at 14:37 -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> The GETDEVICEINFO gdia_maxcount represents all of the data being returned
>> within the GETDEVICEINFO4resok structure and includes the XDR overhead.
>>
>> The CREATE_SESSION ca_maxresponsesize is the maximum reply and includes the RPC
>> headers (including security flavor credentials and verifiers).
>
> This still looks wrong.
>
> The problem is that pdev->pglen is used both as the total buffer length,
> as a function argument and as the reply message length (through the call
> to xdr_init_decode_pages). We really should split out those 3 different
> roles?

Ah. OK, I'll split them out...

-->Andy

>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com


2013-06-25 19:38:01

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFSv4.1 Do not overwrite negotiated session sizes

On Tue, 2013-06-25 at 19:05 +0000, Adamson, Andy wrote:
> On Jun 25, 2013, at 2:52 PM, "Myklebust, Trond" <[email protected]>
> wrote:
>
> > On Tue, 2013-06-25 at 14:37 -0400, [email protected] wrote:
> >> From: Andy Adamson <[email protected]>
> >>
> >> nfs4_init_session is called after CREATE_SESSION has set the ca_maxrequestsize
> >> and ca_maxresponsesize values obtained from the server.
> >
> > It had better not be, otherwise the values of
> > session->fc_target_max_rqst_sz and session->fc_target_max_resp_sz in
> > nfs4_init_channel_attrs() make no sense.
>
> Well, it does. Shall I move the nfs4_init_session call to before nfs4_init_channel_attrs?
>

Either that, or change nfs4_init_channel_attrs() to always use
NFS_MAX_FILE_IO_SIZE. Right now, the code is just confused (and very
confusing).


--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com