linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mi Jinlong <mijinlong@cn.fujitsu.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: NFSv3 list <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] NFS4.1: Fix bug server don't reply the right fore_channel to client at create_session
Date: Wed, 05 Jan 2011 13:56:18 +0800	[thread overview]
Message-ID: <4D240802.7050307@cn.fujitsu.com> (raw)
In-Reply-To: <20110105010034.GB14744@fieldses.org>



J. Bruce Fields:
> On Thu, Dec 30, 2010 at 11:16:44AM +0800, Mi Jinlong wrote:
>> Hi Bruce:
>>
>> J. Bruce Fields:
>>> On Mon, Nov 15, 2010 at 04:12:43PM +0800, Mi Jinlong wrote:
>>>> J. Bruce Fields:
>>>>> On Thu, Nov 11, 2010 at 06:03:40PM +0800, Mi Jinlong wrote:
>>>>>> At the latest kernel(2.6.37-rc1), server just initialize the forechannel
>>>>>> at init_forechannel_attrs, but don't reflect it to reply.
>>>>>>
>>>>>> After initialize the session success, we should copy the forechannel info
>>>>>> to nfsd4_create_session struct.
>>>>> Thanks!
>>>>>
>>>>> Is there a chance you could write a pynfs test for this?
>>>> Maybe the following one is OK.
>>> Yes, thanks very much.  But could we use a larger MAX_SLOTS_PER_SESSION
>>> value, to increase the chances this test will make sense for any
>>> server?
>>   Yes, but we can't use a huge value as 1000000 for that the test site
>>   will allocate replay_cache for each fore_channel.maxrequests when get
>>   the reply of CREATE_SESSION.
> 
> pynfs should only allocate enough for the returned maxrequests, right?

  Yes, it does like that. If server return a wrong maxrequests as 1000000, 
  it will allocate 1000000 replay_cache. So we should set a small one here.

> 
>> If setting to 1000000 or more huger, 
>>   when the test case fail, the test site will eat many memory. 
>>
>>   So, we should set it less than 1000 and add an explain.
>>
>>> Also, this is a "reply", not a "replay", test, so move it a little
>>> later; how about the following?
>>   Sorry for that, it's a typo. ^_^
>>
>>   Maybe we should set the TOO_MANY_SLOTS less than 1000, after testing,
>>   the patch is as following:
>>
>>   Ps: delete the semicolon at the latest line.
> 
> Whoops, I obviously don't do much python....
> 
> --b.
> 
>> >From 2f544fcef0eef7ac1d59bc428be3436ad7d1bbde Mon Sep 17 00:00:00 2001
>> From: Mi Jinlong <mijinlong@cn.fujitsu.com>
>> Date: Wed, 29 Dec 2010 16:06:30 +0800
>> Subject: [PATCH] SRVR: Can server return fore_channel maxreqs correctly
>>
>>     SRVR: Can server return fore_channel maxreqs correctly
>>     
>>     When client set the fore_channel maxreqs larger than server's
>>     NFSD_MAX_SLOTS_PER_SESSION at create_session, the fore_channel
>>     maxreqs at the reply must less than NFSD_MAX_SLOTS_PER_SESSION.
>>     
>>     At 2.6.37-rc1, the NFSD_MAX_SLOTS_PER_SESSION is 160.
>>     
>>     Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
>>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>>
>> ---
>>  nfs4.1/server41tests/st_create_session.py |   19 +++++++++++++++++++
>>  1 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/nfs4.1/server41tests/st_create_session.py b/nfs4.1/server41tests/st_create_session.py
>> index 5f425d4..5cfac14 100644
>> --- a/nfs4.1/server41tests/st_create_session.py
>> +++ b/nfs4.1/server41tests/st_create_session.py
>> @@ -363,3 +363,22 @@ def testCallbackVersion(t, env):
>>                   (cb_occurred.low, cb_occurred.hi, cb_occurred.vers))
>>      finally:
>>          env.c1._check_version = orig
>> +
>> +def testMaxreqs(t, env):
>> +    """A CREATE_SESSION with maxreqs too large should return
>> +       a modified value
>> +
>> +    FLAGS: create_session all
>> +    CODE: CSESS22
>> +    """
>> +    # Assuming this is too large for any server; increase if necessary:
>> +    # but too huge will eat many memory for replay_cache, becareful it!
>> +    TOO_MANY_SLOTS = 500
>> +
>> +    c = env.c1.new_client(env.testname(t))
>> +    # CREATE_SESSION with fore_channel = TOO_MANY_SLOTS
>> +    chan_attrs = channel_attrs4(0,8192,8192,8192,128, TOO_MANY_SLOTS, [])
>> +    sess1 = c.create_session(fore_attrs=chan_attrs)
>> +    if nfs4lib.test_equal(sess1.fore_channel.maxrequests,
>> +                          chan_attrs.ca_maxrequests, "count4"):
>> +        fail("Server allows surprisingly large fore_channel maxreqs")
>> -- 
>> 1.7.3.3
>>
> 

-- 
----
thanks
Mi Jinlong


  reply	other threads:[~2011-01-05  5:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-11 10:03 [PATCH] NFS4.1: Fix bug server don't reply the right fore_channel to client at create_session Mi Jinlong
2010-11-11 13:32 ` Benny Halevy
2010-11-12 20:45 ` J. Bruce Fields
2010-11-15  8:12   ` Mi Jinlong
2010-12-29 19:35     ` J. Bruce Fields
2010-12-30  3:16       ` Mi Jinlong
2011-01-05  1:00         ` J. Bruce Fields
2011-01-05  5:56           ` Mi Jinlong [this message]
2011-01-05 17:35             ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D240802.7050307@cn.fujitsu.com \
    --to=mijinlong@cn.fujitsu.com \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).