Skip to contentSkip to conversation

NFS4.1: Fix bug server don't reply the right fore_channel to client at create_session

PATCH In review Mi Jinlong 3 participants 9 messages Nov 11, 2010 – Jan 5, 2011

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Mi Jinlong

author
#0 lore raw

PATCH 1/1 NFS4.1: Fix bug server don't reply the right fore_channel to client at create_session

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.

Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +2 -0
index f1e5ec6..3876a9c 100644
@@ -1556,6 +1556,8 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	status = nfs_ok;
 	memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
 	       NFS4_MAX_SESSIONID_LEN);
+	memcpy(&cr_ses->fore_channel, &new->se_fchannel,
+		sizeof(struct nfsd4_channel_attrs));
 	cs_slot->sl_seqid++;
 	cr_ses->seqid = cs_slot->sl_seqid;
Signature · 1 line
1.7.0.1
#1 lore raw
Quoted 7 lines · Mi Jinlong

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.

Signed-off-by Mi Jinlong <[email protected]>

Yeah, looks necessary to me too.

Benny

Quoted 17 lines
---
 fs/nfsd/nfs4state.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +2 -0
index f1e5ec6..3876a9c 100644
@@ -1556,6 +1556,8 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	status = nfs_ok;
 	memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
 	       NFS4_MAX_SESSIONID_LEN);
+	memcpy(&cr_ses->fore_channel, &new->se_fchannel,
+		sizeof(struct nfsd4_channel_attrs));
 	cs_slot->sl_seqid++;
 	cr_ses->seqid = cs_slot->sl_seqid;
#2 lore raw
In reply to Mi Jinlong
Quoted 5 lines · Mi Jinlong

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?

--b.

Quoted 23 lines
Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +2 -0
index f1e5ec6..3876a9c 100644
@@ -1556,6 +1556,8 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	status = nfs_ok;
 	memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
 	       NFS4_MAX_SESSIONID_LEN);
+	memcpy(&cr_ses->fore_channel, &new->se_fchannel,
+		sizeof(struct nfsd4_channel_attrs));
 	cs_slot->sl_seqid++;
 	cr_ses->seqid = cs_slot->sl_seqid;
Signature · 1 line
1.7.0.1
author
#3 lore raw

J. Bruce Fields:

Quoted 10 lines · Mi Jinlong
Quoted 5 lines

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

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_sessino, 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 <[email protected]>
---
 nfs4.1/server41tests/st_create_session.py |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
nfs4.1/server41tests/st_create_session.py +19 -0
index 5f425d4..174d31e 100644
@@ -156,6 +156,25 @@ def testReplay1b(t, env):
     if not nfs4lib.test_equal(res1, res2):
         fail("Replay results not equal")
 
+def testReplay1c(t, env):
+    """Replay a successful CREATE_SESSION with right maxreqs 
+        (less than NFSD_MAX_SLOTS_PER_SESSION)
+
+    FLAGS: create_session all
+    CODE: CSESS5c
+    """
+    NFSD_MAX_SLOTS_PER_SESSION = 160
+
+    c = env.c1.new_client(env.testname(t))
+    # CREATE_SESSION with fore_channel = NFSD_MAX_SLOTS_PER_SESSION + 1
+    chan_attrs = channel_attrs4(0,8192,8192,8192,128,
+                                NFSD_MAX_SLOTS_PER_SESSION + 1,[])
+    sess1 = c.create_session(fore_attrs=chan_attrs)
+
+    if nfs4lib.test_equal(sess1.fore_channel.maxrequests,
+                          chan_attrs.ca_maxrequests, "count4"):
+        fail("Replay wrong fore_channel maxreqs")
+
 def testReplay2(t, env):
     """Replay a unsuccessful CREATE_SESSION
Signature · 1 line
1.7.3.2
#4 lore raw
Quoted 7 lines · Mi Jinlong

J. Bruce Fields:

Quoted 4 lines · Mi Jinlong
Quoted 3 lines

At the latest kernel(2.6.37-rc1), server just initialize the forechannel at init_forechannel_attrs, but don't reflect it to reply.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

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?

Also, this is a "reply", not a "replay", test, so move it a little later; how about the following?

--b.

commit bf41254ab3583f6e41c96b1861956dd31595928d

Author: Mi Jinlong <[email protected]>
Date:   Mon Nov 15 16:12:43 2010 +0800

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 <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
nfs4.1/server41tests/st_create_session.py +19 -0
index 5f425d4..efb2641 100644
@@ -363,3 +363,23 @@ 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:
+    TOO_MANY_SLOTS = 1000000
+
+    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");
author
#5 lore raw

Hi Bruce:

J. Bruce Fields:

Quoted 9 lines · Mi Jinlong
Quoted 8 lines

J. Bruce Fields:

Quoted 6 lines · Mi Jinlong
Quoted 5 lines

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.

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

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

Quoted 3 lines

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.

Quoted 1 line

From 2f544fcef0eef7ac1d59bc428be3436ad7d1bbde Mon Sep 17 00:00:00 2001

From: Mi Jinlong <[email protected]>
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 <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_create_session.py |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
nfs4.1/server41tests/st_create_session.py +19 -0
index 5f425d4..5cfac14 100644
@@ -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")
Signature · 1 line
1.7.3.3
#6 lore raw
Quoted 17 lines · Mi Jinlong

Hi Bruce:

J. Bruce Fields:

Quoted 14 lines · Mi Jinlong
Quoted 12 lines

J. Bruce Fields:

Quoted 9 lines · Mi Jinlong
Quoted 5 lines

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.

Quoted 7 lines
Quoted 3 lines

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?

Quoted 9 lines

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.

Quoted 3 lines

Also, this is a "reply", not a "replay", test, so move it a little later; how about the following?

Quoted 6 lines

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.

Quoted 51 lines
Quoted 2 lines

From 2f544fcef0eef7ac1d59bc428be3436ad7d1bbde Mon Sep 17 00:00:00 2001

From: Mi Jinlong <[email protected]>
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 <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_create_session.py |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
nfs4.1/server41tests/st_create_session.py +19 -0
index 5f425d4..5cfac14 100644
@@ -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")
Signature · 1 line
1.7.3.3
author
#7 lore raw

J. Bruce Fields:

Quoted 16 lines · Mi Jinlong
Quoted 15 lines

Hi Bruce:

J. Bruce Fields:

Quoted 12 lines · Mi Jinlong
Quoted 11 lines

J. Bruce Fields:

Quoted 9 lines · Mi Jinlong
Quoted 5 lines

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.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

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.

Quoted 70 lines
Quoted 6 lines

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.

Quoted 8 lines
Quoted 2 lines

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.

Quoted 50 lines
Quoted 1 line

From 2f544fcef0eef7ac1d59bc428be3436ad7d1bbde Mon Sep 17 00:00:00 2001

From: Mi Jinlong <[email protected]>
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 <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_create_session.py |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
nfs4.1/server41tests/st_create_session.py +19 -0
index 5f425d4..5cfac14 100644
@@ -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")
Signature · 1 line
1.7.3.3
Signature · 3 lines
----
thanks
Mi Jinlong
#8 lore raw
Quoted 22 lines · Mi Jinlong

J. Bruce Fields:

Quoted 19 lines · Mi Jinlong
Quoted 18 lines

Hi Bruce:

J. Bruce Fields:

Quoted 15 lines · Mi Jinlong
Quoted 11 lines

J. Bruce Fields:

Quoted 9 lines · Mi Jinlong
Quoted 5 lines

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?

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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.

Thanks, I understand now. Sure, let's go with your solution--applied.

--b.

Quoted 77 lines
Quoted 71 lines
Quoted 7 lines

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.

Quoted 8 lines
Quoted 2 lines

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.

Quoted 50 lines
Quoted 1 line

From 2f544fcef0eef7ac1d59bc428be3436ad7d1bbde Mon Sep 17 00:00:00 2001

From: Mi Jinlong <[email protected]>
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 <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_create_session.py |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
nfs4.1/server41tests/st_create_session.py +19 -0
index 5f425d4..5cfac14 100644
@@ -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")
Signature · 1 line
1.7.3.3
Signature · 3 lines
----
thanks
Mi Jinlong