linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] nfsd41: check the size of response
@ 2011-05-18  2:19 Mi Jinlong
  2011-05-19  1:07 ` J. Bruce Fields
  0 siblings, 1 reply; 3+ messages in thread
From: Mi Jinlong @ 2011-05-18  2:19 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: NFS

Compare response's size to the session se_fmaxresp_sz and 
se_fmaxresp_cached at the same function.

And modify function name nfsd4_check_drc_limit to
nfsd4_check_response_size.

Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
---
 fs/nfsd/nfs4xdr.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index c6766af..a7cf985 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3224,12 +3224,12 @@ static nfsd4_enc nfsd4_enc_ops[] = {
  * pad: add on 8 bytes for the next operation's op_code and status so that
  * there is room to cache a failure on the next operation.
  *
- * Compare this length to the session se_fmaxresp_cached.
+ * Compare this length to the session se_fmaxresp_sz and se_fmaxresp_cached.
  *
  * Our se_fmaxresp_cached will always be a multiple of PAGE_SIZE, and so
  * will be at least a page and will therefore hold the xdr_buf head.
  */
-static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
+static int nfsd4_check_response_size(struct nfsd4_compoundres *resp)
 {
 	int status = 0;
 	struct xdr_buf *xb = &resp->rqstp->rq_res;
@@ -3242,7 +3242,7 @@ static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
 		return status;
 
 	session = resp->cstate.session;
-	if (session == NULL || slot->sl_cachethis == 0)
+	if (session == NULL)
 		return status;
 
 	if (resp->opcnt >= args->opcnt)
@@ -3259,10 +3259,14 @@ static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
 	dprintk("%s length %u, xb->page_len %u tlen %u pad %u\n", __func__,
 		length, xb->page_len, tlen, pad);
 
-	if (length <= session->se_fchannel.maxresp_cached)
-		return status;
-	else
+	if (length > session->se_fchannel.maxresp_sz)
+		return nfserr_rep_too_big;
+
+	if (slot->sl_cachethis == 1 &&
+	    length > session->se_fchannel.maxresp_cached)
 		return nfserr_rep_too_big_to_cache;
+
+	return status;
 }
 
 void
@@ -3282,8 +3286,8 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
 	       !nfsd4_enc_ops[op->opnum]);
 	op->status = nfsd4_enc_ops[op->opnum](resp, op->status, &op->u);
 	/* nfsd4_check_drc_limit guarantees enough room for error status */
-	if (!op->status && nfsd4_check_drc_limit(resp))
-		op->status = nfserr_rep_too_big_to_cache;
+	if (!op->status)
+		op->status = nfsd4_check_response_size(resp);
 status:
 	/*
 	 * Note: We write the status directly, instead of using WRITE32(),
-- 
1.7.4.5



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] nfsd41: check the size of response
  2011-05-18  2:19 [PATCH] nfsd41: check the size of response Mi Jinlong
@ 2011-05-19  1:07 ` J. Bruce Fields
  2011-05-19  5:35   ` Mi Jinlong
  0 siblings, 1 reply; 3+ messages in thread
From: J. Bruce Fields @ 2011-05-19  1:07 UTC (permalink / raw)
  To: Mi Jinlong; +Cc: NFS

On Wed, May 18, 2011 at 10:19:23AM +0800, Mi Jinlong wrote:
> Compare response's size to the session se_fmaxresp_sz and 
> se_fmaxresp_cached at the same function.
> 
> And modify function name nfsd4_check_drc_limit to
> nfsd4_check_response_size.

Hm.  I guess that's OK.  Note we have a bigger problem, though, which is
that we're waiting too late to check these limits:

	http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#DRC_limit_checking

Have you looked at that list, by the way?:

	http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Highest_priority

Those are the 4.1 spec compliance problems that I think are the most
important to fix--the ones that I think we have to get done before we
can really declare the 4.1 server correct.

--b.

> 
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
>  fs/nfsd/nfs4xdr.c |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index c6766af..a7cf985 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3224,12 +3224,12 @@ static nfsd4_enc nfsd4_enc_ops[] = {
>   * pad: add on 8 bytes for the next operation's op_code and status so that
>   * there is room to cache a failure on the next operation.
>   *
> - * Compare this length to the session se_fmaxresp_cached.
> + * Compare this length to the session se_fmaxresp_sz and se_fmaxresp_cached.
>   *
>   * Our se_fmaxresp_cached will always be a multiple of PAGE_SIZE, and so
>   * will be at least a page and will therefore hold the xdr_buf head.
>   */
> -static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
> +static int nfsd4_check_response_size(struct nfsd4_compoundres *resp)
>  {
>  	int status = 0;
>  	struct xdr_buf *xb = &resp->rqstp->rq_res;
> @@ -3242,7 +3242,7 @@ static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
>  		return status;
>  
>  	session = resp->cstate.session;
> -	if (session == NULL || slot->sl_cachethis == 0)
> +	if (session == NULL)
>  		return status;
>  
>  	if (resp->opcnt >= args->opcnt)
> @@ -3259,10 +3259,14 @@ static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
>  	dprintk("%s length %u, xb->page_len %u tlen %u pad %u\n", __func__,
>  		length, xb->page_len, tlen, pad);
>  
> -	if (length <= session->se_fchannel.maxresp_cached)
> -		return status;
> -	else
> +	if (length > session->se_fchannel.maxresp_sz)
> +		return nfserr_rep_too_big;
> +
> +	if (slot->sl_cachethis == 1 &&
> +	    length > session->se_fchannel.maxresp_cached)
>  		return nfserr_rep_too_big_to_cache;
> +
> +	return status;
>  }
>  
>  void
> @@ -3282,8 +3286,8 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
>  	       !nfsd4_enc_ops[op->opnum]);
>  	op->status = nfsd4_enc_ops[op->opnum](resp, op->status, &op->u);
>  	/* nfsd4_check_drc_limit guarantees enough room for error status */
> -	if (!op->status && nfsd4_check_drc_limit(resp))
> -		op->status = nfserr_rep_too_big_to_cache;
> +	if (!op->status)
> +		op->status = nfsd4_check_response_size(resp);
>  status:
>  	/*
>  	 * Note: We write the status directly, instead of using WRITE32(),
> -- 
> 1.7.4.5
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] nfsd41: check the size of response
  2011-05-19  1:07 ` J. Bruce Fields
@ 2011-05-19  5:35   ` Mi Jinlong
  0 siblings, 0 replies; 3+ messages in thread
From: Mi Jinlong @ 2011-05-19  5:35 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: NFS



J. Bruce Fields:
> On Wed, May 18, 2011 at 10:19:23AM +0800, Mi Jinlong wrote:
>> Compare response's size to the session se_fmaxresp_sz and 
>> se_fmaxresp_cached at the same function.
>>
>> And modify function name nfsd4_check_drc_limit to
>> nfsd4_check_response_size.
> 
> Hm.  I guess that's OK.  Note we have a bigger problem, though, which is
> that we're waiting too late to check these limits:
> 
> 	http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#DRC_limit_checking
> 
> Have you looked at that list, by the way?:

  I will have a look, and try to solve this problem.
  Maybe it will take me some times.

-- 
----
thanks
Mi Jinlong

> 
> 	http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Highest_priority
> 
> Those are the 4.1 spec compliance problems that I think are the most
> important to fix--the ones that I think we have to get done before we
> can really declare the 4.1 server correct.
> 
> --b.
> 
>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
>> ---
>>  fs/nfsd/nfs4xdr.c |   20 ++++++++++++--------
>>  1 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>> index c6766af..a7cf985 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -3224,12 +3224,12 @@ static nfsd4_enc nfsd4_enc_ops[] = {
>>   * pad: add on 8 bytes for the next operation's op_code and status so that
>>   * there is room to cache a failure on the next operation.
>>   *
>> - * Compare this length to the session se_fmaxresp_cached.
>> + * Compare this length to the session se_fmaxresp_sz and se_fmaxresp_cached.
>>   *
>>   * Our se_fmaxresp_cached will always be a multiple of PAGE_SIZE, and so
>>   * will be at least a page and will therefore hold the xdr_buf head.
>>   */
>> -static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
>> +static int nfsd4_check_response_size(struct nfsd4_compoundres *resp)
>>  {
>>  	int status = 0;
>>  	struct xdr_buf *xb = &resp->rqstp->rq_res;
>> @@ -3242,7 +3242,7 @@ static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
>>  		return status;
>>  
>>  	session = resp->cstate.session;
>> -	if (session == NULL || slot->sl_cachethis == 0)
>> +	if (session == NULL)
>>  		return status;
>>  
>>  	if (resp->opcnt >= args->opcnt)
>> @@ -3259,10 +3259,14 @@ static int nfsd4_check_drc_limit(struct nfsd4_compoundres *resp)
>>  	dprintk("%s length %u, xb->page_len %u tlen %u pad %u\n", __func__,
>>  		length, xb->page_len, tlen, pad);
>>  
>> -	if (length <= session->se_fchannel.maxresp_cached)
>> -		return status;
>> -	else
>> +	if (length > session->se_fchannel.maxresp_sz)
>> +		return nfserr_rep_too_big;
>> +
>> +	if (slot->sl_cachethis == 1 &&
>> +	    length > session->se_fchannel.maxresp_cached)
>>  		return nfserr_rep_too_big_to_cache;
>> +
>> +	return status;
>>  }
>>  
>>  void
>> @@ -3282,8 +3286,8 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
>>  	       !nfsd4_enc_ops[op->opnum]);
>>  	op->status = nfsd4_enc_ops[op->opnum](resp, op->status, &op->u);
>>  	/* nfsd4_check_drc_limit guarantees enough room for error status */
>> -	if (!op->status && nfsd4_check_drc_limit(resp))
>> -		op->status = nfserr_rep_too_big_to_cache;
>> +	if (!op->status)
>> +		op->status = nfsd4_check_response_size(resp);
>>  status:
>>  	/*
>>  	 * Note: We write the status directly, instead of using WRITE32(),
>> -- 
>> 1.7.4.5
>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-05-19  5:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-18  2:19 [PATCH] nfsd41: check the size of response Mi Jinlong
2011-05-19  1:07 ` J. Bruce Fields
2011-05-19  5:35   ` Mi Jinlong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).