Fix forechannel attribute initialization
These two patches fix ca_maxresponsesize_cached, ca_maxresponsesize, and
ca_maxrequestsize calculations for the forechannel.
Tested:
Kerberos NFSv4.1 mount:
Passes Connectathon tests. Passes the expected pynfs tests when tests are run
individually.
When the pynfs tests are run all at once, there are too many sessions created,
and the server code (init_forechannel_attrs) works as planned and fails the
CREATE SESSION with NFS4ERR_SERVERFAULT.
The pynfs 4.1 tests need to be fixed to clean up after themselves!
-->Andy
0001-nfsd41-fix-NFSD_MIN_HDR_SEQ_SZ.patch
0002-nfsd41-add-RPC-header-size-to-fore-channel-negotiat.patch
From: Andy Adamson <[email protected]>
Both the max request and the max response size include the RPC header with
credential (request only) and verifier as well as the payload.
The RPCSEC_GSS credential and verifier are the largest. Kerberos is the only
supported GSS security mechansim, so the Kerberos GSS credential and verifier
sizes are used.
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfsd/nfs4state.c | 29 +++++++++++++++++++++++++----
1 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5ecb42c..e4d8f94 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -428,6 +428,27 @@ gen_sessionid(struct nfsd4_session *ses)
#define NFSD_MIN_HDR_SEQ_SZ (20 + 12 + 44)
/*
+ * The protocol defines ca_maxrequestsize as the XDR encoded size of the
+ * request including the RPC headers (including security flavor credentials
+ * and verifiers) but excludes any RPC transport framing headers.
+ * So we need to advertize a ca_maxrequestsize value that is the number of
+ * bytes of the maximum payload we support, plus some additional bytes to cover
+ * the maximum RPC header size. The RPCSEC_GSS security flavor has the
+ * largest credential and verifier, so we add 24 bytes for the RPC call header
+ * (xid through proceedure), 32 bytes of GSS credential, and 48 bytes of
+ * Kerberos GSS verifier.
+ */
+#define NFSD_MAX_CALL_HDR_SZ (24 + 32 + 48)
+
+/* The protocol defines ca_maxresponsesize as also including the RPC headers
+ * just as in ca_maxrequestsize. Once again, we use the maximum supported
+ * payload plus the largest RPC reply header which uses the RPCSEC_GSS
+ * security flavor. We add 12 bytes of RPC reply header (xid through
+ * reply state) and 48 bytes of GSS Kerberos verifier.
+ */
+#define NFSD_MAX_REPLY_HDR_SZ (12 + 48)
+
+/*
* Give the client the number of ca_maxresponsesize_cached slots it
* requests, of size bounded by NFSD_SLOT_CACHE_SIZE,
* NFSD_MAX_MEM_PER_SESSION, and nfsd_drc_max_mem. Do not allow more
@@ -488,12 +509,12 @@ static int init_forechannel_attrs(struct svc_rqst *rqstp,
/* headerpadsz set to zero in encode routine */
/* Use the client's max request and max response size if possible */
- if (fchan->maxreq_sz > maxcount)
- fchan->maxreq_sz = maxcount;
+ if (fchan->maxreq_sz > maxcount + NFSD_MAX_CALL_HDR_SZ)
+ fchan->maxreq_sz = maxcount + NFSD_MAX_CALL_HDR_SZ;
session_fchan->maxreq_sz = fchan->maxreq_sz;
- if (fchan->maxresp_sz > maxcount)
- fchan->maxresp_sz = maxcount;
+ if (fchan->maxresp_sz > maxcount + NFSD_MAX_REPLY_HDR_SZ)
+ fchan->maxresp_sz = maxcount + NFSD_MAX_REPLY_HDR_SZ;
session_fchan->maxresp_sz = fchan->maxresp_sz;
/* Use the client's maxops if possible */
--
1.6.2.5
From: Andy Adamson <[email protected]>
The reply RPC header is 12 bytes, the NULL verifier is 8 bytes giving 20
not 24 bytes in the NFSD_MIN_HDR_SEQ_SZ calculation,
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfsd/nfs4state.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 46e9ac5..5ecb42c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -420,12 +420,12 @@ gen_sessionid(struct nfsd4_session *ses)
* each time. Therefore we can advertise a ca_maxresponssize_cached
* value that is the number of bytes in our cache plus a few additional
* bytes. In order to stay on the safe side, and not promise more than
- * we can cache, those additional bytes must be the minimum possible: 24
+ * we can cache, those additional bytes must be the minimum possible: 20
* bytes of rpc header (xid through accept state, with AUTH_NULL
* verifier), 12 for the compound header (with zero-length tag), and 44
* for the SEQUENCE op response:
*/
-#define NFSD_MIN_HDR_SEQ_SZ (24 + 12 + 44)
+#define NFSD_MIN_HDR_SEQ_SZ (20 + 12 + 44)
/*
* Give the client the number of ca_maxresponsesize_cached slots it
--
1.6.2.5
On Fri, Sep 11, 2009 at 06:52:54PM -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> The reply RPC header is 12 bytes, the NULL verifier is 8 bytes giving 20
> not 24 bytes in the NFSD_MIN_HDR_SEQ_SZ calculation,
No, 24 is right. Maybe you forgot the accept stat at the end?:
xid
msg type
reply stat
verf flavor
verf length
accept stat
--b.
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 46e9ac5..5ecb42c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -420,12 +420,12 @@ gen_sessionid(struct nfsd4_session *ses)
> * each time. Therefore we can advertise a ca_maxresponssize_cached
> * value that is the number of bytes in our cache plus a few additional
> * bytes. In order to stay on the safe side, and not promise more than
> - * we can cache, those additional bytes must be the minimum possible: 24
> + * we can cache, those additional bytes must be the minimum possible: 20
> * bytes of rpc header (xid through accept state, with AUTH_NULL
> * verifier), 12 for the compound header (with zero-length tag), and 44
> * for the SEQUENCE op response:
> */
> -#define NFSD_MIN_HDR_SEQ_SZ (24 + 12 + 44)
> +#define NFSD_MIN_HDR_SEQ_SZ (20 + 12 + 44)
>
> /*
> * Give the client the number of ca_maxresponsesize_cached slots it
> --
> 1.6.2.5
>
On Fri, Sep 11, 2009 at 06:52:53PM -0400, [email protected] wrote:
> Fix forechannel attribute initialization
>
> These two patches fix ca_maxresponsesize_cached, ca_maxresponsesize, and
> ca_maxrequestsize calculations for the forechannel.
>
> Tested:
Actually the think I was interested to test was just to make sure that
the client does writes of the size it should. (It should be able to do
1MB reads and writes, but with the previous code, I believe it would
have been a bug for the client to do so, as we weren't advertising
maxresponse/requestsizes large enough.)
--b.
>
> Kerberos NFSv4.1 mount:
> Passes Connectathon tests. Passes the expected pynfs tests when tests are run
> individually.
> When the pynfs tests are run all at once, there are too many sessions created,
> and the server code (init_forechannel_attrs) works as planned and fails the
> CREATE SESSION with NFS4ERR_SERVERFAULT.
>
> The pynfs 4.1 tests need to be fixed to clean up after themselves!
>
> -->Andy
>
> 0001-nfsd41-fix-NFSD_MIN_HDR_SEQ_SZ.patch
> 0002-nfsd41-add-RPC-header-size-to-fore-channel-negotiat.patch
>
On Fri, Sep 11, 2009 at 06:52:55PM -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Both the max request and the max response size include the RPC header with
> credential (request only) and verifier as well as the payload.
>
> The RPCSEC_GSS credential and verifier are the largest. Kerberos is the only
> supported GSS security mechansim, so the Kerberos GSS credential and verifier
> sizes are used.
Rather than trying to estimate this is might be simplest just to use
what the server's using to allocate memory: RPCSVC_MAXPAGES. No, that
also takes into account space for the reply. You could do
PAGE_SIZE * (1 + (RPCSVC_MAXPAYLOAD+PAGE_SIZE-1)/PAGE_SIZE)
Actually, by design the server's real limit is actually on the sum of
the request and the reply sizes.
What happens if we get a request such that both the request and reply
are under our advertised limits, but the sum is too much? Can we just
declare that no client will be that weird and that we shouldn't have to
worry about it?
--b.
On Mon, Sep 14, 2009 at 2:51 PM, J. Bruce Fields <[email protected]>=
wrote:
> On Fri, Sep 11, 2009 at 06:52:54PM -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> The reply RPC header is 12 bytes, the NULL verifier is 8 bytes givin=
g 20
>> not 24 bytes in the NFSD_MIN_HDR_SEQ_SZ calculation,
>
> No, 24 is right. =A0Maybe you forgot the accept stat at the end?:
yes - missed the accept state.
-->Andy
>
> =A0 =A0 =A0 =A0xid
> =A0 =A0 =A0 =A0msg type
> =A0 =A0 =A0 =A0reply stat
> =A0 =A0 =A0 =A0verf flavor
> =A0 =A0 =A0 =A0verf length
> =A0 =A0 =A0 =A0accept stat
>
> --b.
>
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> =A0fs/nfsd/nfs4state.c | =A0 =A04 ++--
>> =A01 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 46e9ac5..5ecb42c 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -420,12 +420,12 @@ gen_sessionid(struct nfsd4_session *ses)
>> =A0 * each time. =A0Therefore we can advertise a ca_maxresponssize_c=
ached
>> =A0 * value that is the number of bytes in our cache plus a few addi=
tional
>> =A0 * bytes. =A0In order to stay on the safe side, and not promise m=
ore than
>> - * we can cache, those additional bytes must be the minimum possibl=
e: 24
>> + * we can cache, those additional bytes must be the minimum possibl=
e: 20
>> =A0 * bytes of rpc header (xid through accept state, with AUTH_NULL
>> =A0 * verifier), 12 for the compound header (with zero-length tag), =
and 44
>> =A0 * for the SEQUENCE op response:
>> =A0 */
>> -#define NFSD_MIN_HDR_SEQ_SZ =A0(24 + 12 + 44)
>> +#define NFSD_MIN_HDR_SEQ_SZ =A0(20 + 12 + 44)
>>
>> =A0/*
>> =A0 * Give the client the number of ca_maxresponsesize_cached slots =
it
>> --
>> 1.6.2.5
>>
> _______________________________________________
> pNFS mailing list
> [email protected]
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
On Mon, Sep 14, 2009 at 3:03 PM, J. Bruce Fields <[email protected]>=
wrote:
> On Fri, Sep 11, 2009 at 06:52:55PM -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Both the max request and the max response size include the RPC heade=
r with
>> credential (request only) =A0and verifier as well as the payload.
>>
>> The RPCSEC_GSS credential and verifier are the largest. Kerberos is =
the only
>> supported GSS security mechansim, so the Kerberos GSS credential and=
verifier
>> sizes are used.
>
> Rather than trying to estimate this is might be simplest just to use
> what the server's using to allocate memory: RPCSVC_MAXPAGES. =A0No, t=
hat
> also takes into account space for the reply. =A0You could do
>
> =A0 =A0 =A0 =A0PAGE_SIZE * (1 + (RPCSVC_MAXPAYLOAD+PAGE_SIZE-1)/PAGE_=
SIZE)
>
> Actually, by design the server's real limit is actually on the sum of
> the request and the reply sizes.
I think the actual limit is svc_max_payload rounded up to a multiple
of PAGE_SIZE plus PAGE_SIZE. which is a lot smaller than the sum of
the request and reply sizes. See below.
Note that svc_max_payload is what is returned in nfs4_encode_fattr for
MAXREAD and for MAXWRITE. These attributes use svc_max_payload in the
same way this patch does - the maximum data size not including rpc
headers.
I don't think the server wants is to advertise a MAXREAD/WRITE that it
can't supply because the fore channel maxrequest/maxresponse is too
small, so some additional space needs to be added to svc_max_payload
for the fore channel.
>
> What happens if we get a request such that both the request and reply
> are under our advertised limits, but the sum is too much? =A0Can we j=
ust
> declare that no client will be that weird and that we shouldn't have =
to
> worry about it?
I think the server already has this problem. In svc_init_buffer which
sets up the pages for a server thread request/response handling, it
uses sv_max_mesg / PAGE_SIZE + 1 with the comment
"extra page as we hold both request and reply. We assume one is at
most one page"
where
sv_max_mesg =3D roundup(serv->sv_max_payload + PAGE_SIZE, PAGE_SIZE).
-->Andy
>
> --b.
> _______________________________________________
> pNFS mailing list
> [email protected]
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>