[PATCH] Args need to be the same for replay cache

[PATCH] Args need to be the same for replay cache

[Thomas Haynes]

From: Tom Haynes <loghyr@primarydata.com>

2.10.6.1.3.1.  False Retry

  If a requester sent a Sequence operation with a slot ID and sequence
  ID that are in the reply cache but the replier detected that the
  retried request is not the same as the original request, including a
  retry that has different operations or different arguments in the
  operations from the original and a retry that uses a different
   principal in the RPC request's credential field that translates to a
  different user, then this is a false retry.  When the replier detects
  a false retry, it is permitted (but not always obligated) to return
  NFS4ERR_SEQ_FALSE_RETRY in response to the Sequence operation when it
  detects a false retry.

Or in other words, sa_cachethis needs to be set or a
server can respond with an error.

Signed-off-by: Tom Haynes <loghyr@primarydata.com>
---
 nfs4.1/server41tests/st_sequence.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/nfs4.1/server41tests/st_sequence.py b/nfs4.1/server41tests/st_=
sequence.py
index d8d460c..e1e5f06 100644
--- a/nfs4.1/server41tests/st_sequence.py
+++ b/nfs4.1/server41tests/st_sequence.py
@@ -115,7 +115,7 @@ def testReplayCache001(t, env):
     sess1 =3D c1.create_session()
     res1 =3D sess1.compound([op.putrootfh()], cache_this=3DTrue)
     check(res1)
-    res2 =3D sess1.compound([op.putrootfh()], seq_delta=3D0)
+    res2 =3D sess1.compound([op.putrootfh()], cache_this=3DTrue, seq_delta=
=3D0)
     check(res2)
     res1.tag =3D res2.tag =3D ""
     if not nfs4lib.test_equal(res1, res2):
@@ -137,7 +137,7 @@ def testReplayCache002(t, env):
           op.rename("%s_1" % env.testname(t), "%s_2" % env.testname(t))]
     res1 =3D sess1.compound(ops, cache_this=3DTrue)
     check(res1)
-    res2 =3D sess1.compound(ops, seq_delta=3D0)
+    res2 =3D sess1.compound(ops, cache_this=3DTrue, seq_delta=3D0)
     check(res2)
     res1.tag =3D res2.tag =3D ""
     if not nfs4lib.test_equal(res1, res2):
@@ -158,7 +158,7 @@ def testReplayCache003(t, env):
     sess1 =3D c1.create_session()
     res1 =3D sess1.compound([op.putrootfh(), op.lookup("")], cache_this=3D=
True)
     check(res1, NFS4ERR_INVAL)
-    res2 =3D sess1.compound([op.putrootfh(), op.lookup("")], seq_delta=3D0=
)
+    res2 =3D sess1.compound([op.putrootfh(), op.lookup("")], cache_this=3D=
True, seq_delta=3D0)
     check(res2, NFS4ERR_INVAL)
     res1.tag =3D res2.tag =3D ""
     if not nfs4lib.test_equal(res1, res2):
@@ -176,7 +176,7 @@ def testReplayCache004(t, env):
     ops +=3D [op.savefh(), op.rename("", "foo")]
     res1 =3D sess1.compound(ops, cache_this=3DTrue)
     check(res1, NFS4ERR_INVAL)
-    res2 =3D sess1.compound(ops, seq_delta=3D0)
+    res2 =3D sess1.compound(ops, cache_this=3DTrue, seq_delta=3D0)
     check(res2, NFS4ERR_INVAL)
     res1.tag =3D res2.tag =3D ""
     if not nfs4lib.test_equal(res1, res2):
@@ -192,7 +192,7 @@ def testReplayCache005(t, env):
     sess1 =3D c1.create_session()
     res1 =3D sess1.compound([op.illegal()], cache_this=3DTrue)
     check(res1, NFS4ERR_OP_ILLEGAL)
-    res2 =3D sess1.compound([op.illegal()], seq_delta=3D0)
+    res2 =3D sess1.compound([op.illegal()], cache_this=3DTrue, seq_delta=
=3D0)
     check(res2, NFS4ERR_OP_ILLEGAL)
     res1.tag =3D res2.tag =3D ""
     if not nfs4lib.test_equal(res1, res2):
@@ -208,7 +208,7 @@ def testReplayCache006(t, env):
     sess =3D c.create_session()
     res1 =3D sess.compound([])
     check(res1)
-    res2 =3D sess.compound([], seq_delta=3D0)
+    res2 =3D sess.compound([], cache_this=3DTrue, seq_delta=3D0)
     check(res2)
     res1.tag =3D res2.tag =3D ""
     if not nfs4lib.test_equal(res1, res2):
--=20
2.3.6

pynfs replay cache test SEQ9f

[Thomas Haynes]

Hi Bruce,

In this test:

def testReplayCache006(t, env):
   """Send two solo sequence compounds with same seqid

   FLAGS: sequence all
   CODE: SEQ9f
   """
   c =3D env.c1.new_client(env.testname(t))
   sess =3D c.create_session()
   res1 =3D sess.compound([])
   check(res1)
   res2 =3D sess.compound([], cache_this=3DTrue, seq_delta=3D0)
   check(res2)
   res1.tag =3D res2.tag =3D ""
   if not nfs4lib.test_equal(res1, res2):
       fail("Replay results not equal")

I don't see why the result should be NFS4_OK.

The first compound does not set sa_cachethis and the second
does set it. According to RFC5661, the server is not required
to cache the first request if the client does not request it.

And if the server does not cache the response, when it gets
a request to replay it, it can return NFS4ERR_RETRY_UNCACHED_REP:

2.10.6.1.3.  Optional Reply Caching

  On a per-request basis, the requester can choose to direct the
  replier to cache the reply to all operations after the first
  operation (SEQUENCE or CB_SEQUENCE) via the sa_cachethis or
  csa_cachethis fields of the arguments to SEQUENCE or CB_SEQUENCE.
  The reason it would not direct the replier to cache the entire reply
  is that the request is composed of all idempotent operations [34].
  Caching the reply may offer little benefit.  If the reply is too
  large (see Section 2.10.6.4), it may not be cacheable anyway.  Even
  if the reply to idempotent request is small enough to cache,
  unnecessarily caching the reply slows down the server and increases
  RPC latency.

  Whether or not the requester requests the reply to be cached has no
  effect on the slot processing.  If the results of SEQUENCE or
  CB_SEQUENCE are NFS4_OK, then the slot's sequence ID MUST be
  incremented by one.  If a requester does not direct the replier to
  cache the reply, the replier MUST do one of following:

  o  The replier can cache the entire original reply.  Even though
     sa_cachethis or csa_cachethis is FALSE, the replier is always free
     to cache.  It may choose this approach in order to simplify
     implementation.

  o  The replier enters into its reply cache a reply consisting of the
     original results to the SEQUENCE or CB_SEQUENCE operation, and
     with the next operation in COMPOUND or CB_COMPOUND having the
     error NFS4ERR_RETRY_UNCACHED_REP.  Thus, if the requester later
     retries the request, it will get NFS4ERR_RETRY_UNCACHED_REP.  If a
     replier receives a retried Sequence operation where the reply to
     the COMPOUND or CB_COMPOUND was not cached, then the replier,

Further, since the parameters to SEQUENCE are different it is a false
retry according to this:

2.10.6.1.3.1.  False Retry

If a requester sent a Sequence operation with a slot ID and sequence
ID that are in the reply cache but the replier detected that the
retried request is not the same as the original request, including a
retry that has different operations or different arguments in the
operations from the original and a retry that uses a different
 principal in the RPC request's credential field that translates to a
different user, then this is a false retry.  When the replier detects
a false retry, it is permitted (but not always obligated) to return
NFS4ERR_SEQ_FALSE_RETRY in response to the Sequence operation when it
detects a false retry.

I'm not sure if the test should be fixed to either

1) allow either NFS4_OK or NFS4ERR_RETRY_UNCACHED_REP
2) just remove the test.

Thoughts?

Re: pynfs replay cache test SEQ9f

[Trond Myklebust]

T24gVGh1LCAyMDE3LTEwLTEyIGF0IDE4OjMyICswMDAwLCBUaG9tYXMgSGF5bmVzIHdyb3RlOg0K
PiBIaSBCcnVjZSwNCj4gDQo+IEluIHRoaXMgdGVzdDoNCj4gDQo+IGRlZiB0ZXN0UmVwbGF5Q2Fj
aGUwMDYodCwgZW52KToNCj4gICAgIiIiU2VuZCB0d28gc29sbyBzZXF1ZW5jZSBjb21wb3VuZHMg
d2l0aCBzYW1lIHNlcWlkDQo+IA0KPiAgICBGTEFHUzogc2VxdWVuY2UgYWxsDQo+ICAgIENPREU6
IFNFUTlmDQo+ICAgICIiIg0KPiAgICBjID0gZW52LmMxLm5ld19jbGllbnQoZW52LnRlc3RuYW1l
KHQpKQ0KPiAgICBzZXNzID0gYy5jcmVhdGVfc2Vzc2lvbigpDQo+ICAgIHJlczEgPSBzZXNzLmNv
bXBvdW5kKFtdKQ0KPiAgICBjaGVjayhyZXMxKQ0KPiAgICByZXMyID0gc2Vzcy5jb21wb3VuZChb
XSwgY2FjaGVfdGhpcz1UcnVlLCBzZXFfZGVsdGE9MCkNCj4gICAgY2hlY2socmVzMikNCj4gICAg
cmVzMS50YWcgPSByZXMyLnRhZyA9ICIiDQo+ICAgIGlmIG5vdCBuZnM0bGliLnRlc3RfZXF1YWwo
cmVzMSwgcmVzMik6DQo+ICAgICAgICBmYWlsKCJSZXBsYXkgcmVzdWx0cyBub3QgZXF1YWwiKQ0K
PiANCj4gSSBkb24ndCBzZWUgd2h5IHRoZSByZXN1bHQgc2hvdWxkIGJlIE5GUzRfT0suDQo+IA0K
PiBUaGUgZmlyc3QgY29tcG91bmQgZG9lcyBub3Qgc2V0IHNhX2NhY2hldGhpcyBhbmQgdGhlIHNl
Y29uZA0KPiBkb2VzIHNldCBpdC4gQWNjb3JkaW5nIHRvIFJGQzU2NjEsIHRoZSBzZXJ2ZXIgaXMg
bm90IHJlcXVpcmVkDQo+IHRvIGNhY2hlIHRoZSBmaXJzdCByZXF1ZXN0IGlmIHRoZSBjbGllbnQg
ZG9lcyBub3QgcmVxdWVzdCBpdC4NCj4gDQo+IEFuZCBpZiB0aGUgc2VydmVyIGRvZXMgbm90IGNh
Y2hlIHRoZSByZXNwb25zZSwgd2hlbiBpdCBnZXRzDQo+IGEgcmVxdWVzdCB0byByZXBsYXkgaXQs
IGl0IGNhbiByZXR1cm4gTkZTNEVSUl9SRVRSWV9VTkNBQ0hFRF9SRVA6DQo+IA0KPiAyLjEwLjYu
MS4zLiAgT3B0aW9uYWwgUmVwbHkgQ2FjaGluZw0KPiANCj4gICBPbiBhIHBlci1yZXF1ZXN0IGJh
c2lzLCB0aGUgcmVxdWVzdGVyIGNhbiBjaG9vc2UgdG8gZGlyZWN0IHRoZQ0KPiAgIHJlcGxpZXIg
dG8gY2FjaGUgdGhlIHJlcGx5IHRvIGFsbCBvcGVyYXRpb25zIGFmdGVyIHRoZSBmaXJzdA0KPiAg
IG9wZXJhdGlvbiAoU0VRVUVOQ0Ugb3IgQ0JfU0VRVUVOQ0UpIHZpYSB0aGUgc2FfY2FjaGV0aGlz
IG9yDQo+ICAgY3NhX2NhY2hldGhpcyBmaWVsZHMgb2YgdGhlIGFyZ3VtZW50cyB0byBTRVFVRU5D
RSBvciBDQl9TRVFVRU5DRS4NCj4gICBUaGUgcmVhc29uIGl0IHdvdWxkIG5vdCBkaXJlY3QgdGhl
IHJlcGxpZXIgdG8gY2FjaGUgdGhlIGVudGlyZQ0KPiByZXBseQ0KPiAgIGlzIHRoYXQgdGhlIHJl
cXVlc3QgaXMgY29tcG9zZWQgb2YgYWxsIGlkZW1wb3RlbnQgb3BlcmF0aW9ucyBbMzRdLg0KPiAg
IENhY2hpbmcgdGhlIHJlcGx5IG1heSBvZmZlciBsaXR0bGUgYmVuZWZpdC4gIElmIHRoZSByZXBs
eSBpcyB0b28NCj4gICBsYXJnZSAoc2VlIFNlY3Rpb24gMi4xMC42LjQpLCBpdCBtYXkgbm90IGJl
IGNhY2hlYWJsZSBhbnl3YXkuICBFdmVuDQo+ICAgaWYgdGhlIHJlcGx5IHRvIGlkZW1wb3RlbnQg
cmVxdWVzdCBpcyBzbWFsbCBlbm91Z2ggdG8gY2FjaGUsDQo+ICAgdW5uZWNlc3NhcmlseSBjYWNo
aW5nIHRoZSByZXBseSBzbG93cyBkb3duIHRoZSBzZXJ2ZXIgYW5kIGluY3JlYXNlcw0KPiAgIFJQ
QyBsYXRlbmN5Lg0KPiANCj4gICBXaGV0aGVyIG9yIG5vdCB0aGUgcmVxdWVzdGVyIHJlcXVlc3Rz
IHRoZSByZXBseSB0byBiZSBjYWNoZWQgaGFzIG5vDQo+ICAgZWZmZWN0IG9uIHRoZSBzbG90IHBy
b2Nlc3NpbmcuICBJZiB0aGUgcmVzdWx0cyBvZiBTRVFVRU5DRSBvcg0KPiAgIENCX1NFUVVFTkNF
IGFyZSBORlM0X09LLCB0aGVuIHRoZSBzbG90J3Mgc2VxdWVuY2UgSUQgTVVTVCBiZQ0KPiAgIGlu
Y3JlbWVudGVkIGJ5IG9uZS4gIElmIGEgcmVxdWVzdGVyIGRvZXMgbm90IGRpcmVjdCB0aGUgcmVw
bGllciB0bw0KPiAgIGNhY2hlIHRoZSByZXBseSwgdGhlIHJlcGxpZXIgTVVTVCBkbyBvbmUgb2Yg
Zm9sbG93aW5nOg0KPiANCj4gICBvICBUaGUgcmVwbGllciBjYW4gY2FjaGUgdGhlIGVudGlyZSBv
cmlnaW5hbCByZXBseS4gIEV2ZW4gdGhvdWdoDQo+ICAgICAgc2FfY2FjaGV0aGlzIG9yIGNzYV9j
YWNoZXRoaXMgaXMgRkFMU0UsIHRoZSByZXBsaWVyIGlzIGFsd2F5cw0KPiBmcmVlDQo+ICAgICAg
dG8gY2FjaGUuICBJdCBtYXkgY2hvb3NlIHRoaXMgYXBwcm9hY2ggaW4gb3JkZXIgdG8gc2ltcGxp
ZnkNCj4gICAgICBpbXBsZW1lbnRhdGlvbi4NCj4gDQo+ICAgbyAgVGhlIHJlcGxpZXIgZW50ZXJz
IGludG8gaXRzIHJlcGx5IGNhY2hlIGEgcmVwbHkgY29uc2lzdGluZyBvZg0KPiB0aGUNCj4gICAg
ICBvcmlnaW5hbCByZXN1bHRzIHRvIHRoZSBTRVFVRU5DRSBvciBDQl9TRVFVRU5DRSBvcGVyYXRp
b24sIGFuZA0KPiAgICAgIHdpdGggdGhlIG5leHQgb3BlcmF0aW9uIGluIENPTVBPVU5EIG9yIENC
X0NPTVBPVU5EIGhhdmluZyB0aGUNCj4gICAgICBlcnJvciBORlM0RVJSX1JFVFJZX1VOQ0FDSEVE
X1JFUC4gIFRodXMsIGlmIHRoZSByZXF1ZXN0ZXIgbGF0ZXINCj4gICAgICByZXRyaWVzIHRoZSBy
ZXF1ZXN0LCBpdCB3aWxsIGdldCBORlM0RVJSX1JFVFJZX1VOQ0FDSEVEX1JFUC4gIElmDQo+IGEN
Cj4gICAgICByZXBsaWVyIHJlY2VpdmVzIGEgcmV0cmllZCBTZXF1ZW5jZSBvcGVyYXRpb24gd2hl
cmUgdGhlIHJlcGx5IHRvDQo+ICAgICAgdGhlIENPTVBPVU5EIG9yIENCX0NPTVBPVU5EIHdhcyBu
b3QgY2FjaGVkLCB0aGVuIHRoZSByZXBsaWVyLA0KPiANCj4gRnVydGhlciwgc2luY2UgdGhlIHBh
cmFtZXRlcnMgdG8gU0VRVUVOQ0UgYXJlIGRpZmZlcmVudCBpdCBpcyBhIGZhbHNlDQo+IHJldHJ5
IGFjY29yZGluZyB0byB0aGlzOg0KPiANCj4gMi4xMC42LjEuMy4xLiAgRmFsc2UgUmV0cnkNCj4g
DQo+IElmIGEgcmVxdWVzdGVyIHNlbnQgYSBTZXF1ZW5jZSBvcGVyYXRpb24gd2l0aCBhIHNsb3Qg
SUQgYW5kIHNlcXVlbmNlDQo+IElEIHRoYXQgYXJlIGluIHRoZSByZXBseSBjYWNoZSBidXQgdGhl
IHJlcGxpZXIgZGV0ZWN0ZWQgdGhhdCB0aGUNCj4gcmV0cmllZCByZXF1ZXN0IGlzIG5vdCB0aGUg
c2FtZSBhcyB0aGUgb3JpZ2luYWwgcmVxdWVzdCwgaW5jbHVkaW5nIGENCj4gcmV0cnkgdGhhdCBo
YXMgZGlmZmVyZW50IG9wZXJhdGlvbnMgb3IgZGlmZmVyZW50IGFyZ3VtZW50cyBpbiB0aGUNCj4g
b3BlcmF0aW9ucyBmcm9tIHRoZSBvcmlnaW5hbCBhbmQgYSByZXRyeSB0aGF0IHVzZXMgYSBkaWZm
ZXJlbnQNCj4gIHByaW5jaXBhbCBpbiB0aGUgUlBDIHJlcXVlc3QncyBjcmVkZW50aWFsIGZpZWxk
IHRoYXQgdHJhbnNsYXRlcyB0byBhDQo+IGRpZmZlcmVudCB1c2VyLCB0aGVuIHRoaXMgaXMgYSBm
YWxzZSByZXRyeS4gIFdoZW4gdGhlIHJlcGxpZXIgZGV0ZWN0cw0KPiBhIGZhbHNlIHJldHJ5LCBp
dCBpcyBwZXJtaXR0ZWQgKGJ1dCBub3QgYWx3YXlzIG9ibGlnYXRlZCkgdG8gcmV0dXJuDQo+IE5G
UzRFUlJfU0VRX0ZBTFNFX1JFVFJZIGluIHJlc3BvbnNlIHRvIHRoZSBTZXF1ZW5jZSBvcGVyYXRp
b24gd2hlbiBpdA0KPiBkZXRlY3RzIGEgZmFsc2UgcmV0cnkuDQo+IA0KPiBJJ20gbm90IHN1cmUg
aWYgdGhlIHRlc3Qgc2hvdWxkIGJlIGZpeGVkIHRvIGVpdGhlcg0KPiANCj4gMSkgYWxsb3cgZWl0
aGVyIE5GUzRfT0sgb3IgTkZTNEVSUl9SRVRSWV9VTkNBQ0hFRF9SRVANCj4gMikganVzdCByZW1v
dmUgdGhlIHRlc3QuDQo+IA0KPiBUaG91Z2h0cz8NCg0KSG1tbS4uLi4NCg0KMi4xMC42LjEuMw0K
Li4uDQoNCiAgICAgICogIE1VU1QgTk9UIHJldHVybiBORlM0RVJSX1JFVFJZX1VOQ0FDSEVEX1JF
UCBpbiByZXBseSB0byBhDQogICAgICAgICBTZXF1ZW5jZSBvcGVyYXRpb24gaWYgdGhlIFNlcXVl
bmNlIG9wZXJhdGlvbiBpcyB0aGUgZmlyc3QNCiAgICAgICAgIG9wZXJhdGlvbi4NCi4uLi4NCg0K
Li4uc28gaXQgbG9va3MgYXMgaWYgdGhlIGZpcnN0IHNlcXVlbmNlIGluIGEgY29tcG91bmQgYWx3
YXlzIGhhcyB0bw0KcmV0dXJuIGVpdGhlciBORlNfT0ssIG9yIHBvc3NpYmx5IE5GUzRFUlJfU0VR
X0ZBTFNFX1JFVFJZLiBUaGUgbGF0dGVyDQpzaG91bGQgcHJlc3VtYWJseSBiZSBhbGxvd2VkLCBz
aW5jZSB0aGVyZSBpcyBubyBleGNlcHRpb24gY2FydmVkIG91dCBpbg0KMi4xMC42LjEuMy4xIGZv
ciB0aGUgc2VxdWVuY2Ugb3Agdy5yLnQuIGJlaGF2aW91ciB3aXRoIGRpZmZlcmVudA0Kb3BlcmF0
aW9uIGFyZ3VtZW50cy4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQg
bWFpbnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20N
Cg==

Re: pynfs replay cache test SEQ9f

[J. Bruce Fields]

On Thu, Oct 12, 2017 at 06:32:09PM +0000, Thomas Haynes wrote:
> Hi Bruce,
> 
> In this test:
> 
> def testReplayCache006(t, env):
>    """Send two solo sequence compounds with same seqid
> 
>    FLAGS: sequence all
>    CODE: SEQ9f
>    """
>    c = env.c1.new_client(env.testname(t))
>    sess = c.create_session()
>    res1 = sess.compound([])
>    check(res1)
>    res2 = sess.compound([], cache_this=True, seq_delta=0)
>    check(res2)
>    res1.tag = res2.tag = ""
>    if not nfs4lib.test_equal(res1, res2):
>        fail("Replay results not equal")
> 
> I don't see why the result should be NFS4_OK.
> 
> The first compound does not set sa_cachethis and the second
> does set it. According to RFC5661, the server is not required
> to cache the first request if the client does not request it.
> 
> And if the server does not cache the response, when it gets
> a request to replay it, it can return NFS4ERR_RETRY_UNCACHED_REP:

If I'm reading that section correctly, it can't return
NFS4ERR_RETRY_UNCACHED_REP, that can only be returned on the next op of
the compound (and there is no next op in this case, only the one
SEQUENCE).

So I *think* the only correct options OK or FALSE_RETRY?

> I'm not sure if the test should be fixed to either
> 
> 1) allow either NFS4_OK or NFS4ERR_RETRY_UNCACHED_REP
> 2) just remove the test.

Maybe just add cache_this to the first?

Looks like the following test is off too--I think a server's allowed to
return a cached reply instead of RETRY_UNCACHED_REP?

--b.

Re: [nfsv4] pynfs replay cache test SEQ9f

[Thomas Haynes]

[-- Attachment #1.1: Type: text/plain, Size: 1772 bytes --]



On Oct 12, 2017, at 12:49 PM, J. Bruce Fields <bfields@fieldses.org<mailto:bfields@fieldses.org>> wrote:

On Thu, Oct 12, 2017 at 06:32:09PM +0000, Thomas Haynes wrote:
Hi Bruce,

In this test:

def testReplayCache006(t, env):
  """Send two solo sequence compounds with same seqid

  FLAGS: sequence all
  CODE: SEQ9f
  """
  c = env.c1.new_client(env.testname(t))
  sess = c.create_session()
  res1 = sess.compound([])
  check(res1)
  res2 = sess.compound([], cache_this=True, seq_delta=0)
  check(res2)
  res1.tag = res2.tag = ""
  if not nfs4lib.test_equal(res1, res2):
      fail("Replay results not equal")

I don't see why the result should be NFS4_OK.

The first compound does not set sa_cachethis and the second
does set it. According to RFC5661, the server is not required
to cache the first request if the client does not request it.

And if the server does not cache the response, when it gets
a request to replay it, it can return NFS4ERR_RETRY_UNCACHED_REP:

If I'm reading that section correctly, it can't return
NFS4ERR_RETRY_UNCACHED_REP, that can only be returned on the next op of
the compound (and there is no next op in this case, only the one
SEQUENCE).

So I *think* the only correct options OK or FALSE_RETRY?


It can’t be OK if the parameters to SEQUENCE differ.



I'm not sure if the test should be fixed to either

1) allow either NFS4_OK or NFS4ERR_RETRY_UNCACHED_REP
2) just remove the test.

Maybe just add cache_this to the first?

Ack. I’ll send in a patch for this approach.


Looks like the following test is off too--I think a server's allowed to
return a cached reply instead of RETRY_UNCACHED_REP?


Yes, it is allowed to do so since it can always cache.


--b.


[-- Attachment #1.2: Type: text/html, Size: 11666 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
nfsv4 mailing list
nfsv4@ietf.org
https://www.ietf.org/mailman/listinfo/nfsv4

Re: pynfs replay cache test SEQ9f

[J. Bruce Fields]

Your mailer's not quoting right, it's a little hard for me to find your
replies.  Wading in:

On Thu, Oct 12, 2017 at 09:39:04PM +0000, Thomas Haynes wrote:
> On Oct 12, 2017, at 12:49 PM, J. Bruce Fields <bfields@fieldses.org<mailto:bfields@fieldses.org>> wrote:
> > So I *think* the only correct options OK or FALSE_RETRY?
> 
> It can’t be OK if the parameters to SEQUENCE differ.

I'm getting that from: "When the replier detects a false retry, it is
permitted (but not always obligated) to return NFS4ERR_FALSE_RETRY in
response to the Sequence operation when it detects a false retry."

If i understand the following language, we're required to return
FALSE_RETRY in the case the rpc credentials of the caller map to
different principals, but not otherwise.

--b

Re: pynfs replay cache test SEQ9f

[Tom Haynes]

On Thu, Oct 12, 2017 at 05:44:54PM -0400, J. Bruce Fields wrote:
> Your mailer's not quoting right, it's a little hard for me to find your
> replies.  Wading in:
> 
> On Thu, Oct 12, 2017 at 09:39:04PM +0000, Thomas Haynes wrote:
> > On Oct 12, 2017, at 12:49 PM, J. Bruce Fields <bfields@fieldses.org<mailto:bfields@fieldses.org>> wrote:
> > > So I *think* the only correct options OK or FALSE_RETRY?
> > 
> > It can’t be OK if the parameters to SEQUENCE differ.
> 
> I'm getting that from: "When the replier detects a false retry, it is
> permitted (but not always obligated) to return NFS4ERR_FALSE_RETRY in
> response to the Sequence operation when it detects a false retry."

I think you are agreeing with me that OK is not appropriate here?


> 
> If i understand the following language, we're required to return
> FALSE_RETRY in the case the rpc credentials of the caller map to
> different principals, but not otherwise.

This one drove me crazy:

   If a requester sent a Sequence operation with a slot ID and sequence
   ID that are in the reply cache but the replier detected that the
   retried request is not the same as the original request, including a
   retry that has different operations or different arguments in the
   operations from the original

SEQUENCE is not special - both the compounds in this example
only have the SEQUENCE op and they differ only in that in the
first sa_cachethis is False and in the second it is True.

So we have to return FALSE_SEQ_RETRY here...

> 
> --b
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: pynfs replay cache test SEQ9f

[J. Bruce Fields]

On Thu, Oct 12, 2017 at 03:00:51PM -0700, Tom Haynes wrote:
> On Thu, Oct 12, 2017 at 05:44:54PM -0400, J. Bruce Fields wrote:
> > Your mailer's not quoting right, it's a little hard for me to find your
> > replies.  Wading in:
> > 
> > On Thu, Oct 12, 2017 at 09:39:04PM +0000, Thomas Haynes wrote:
> > > On Oct 12, 2017, at 12:49 PM, J. Bruce Fields <bfields@fieldses.org<mailto:bfields@fieldses.org>> wrote:
> > > > So I *think* the only correct options OK or FALSE_RETRY?
> > > 
> > > It can’t be OK if the parameters to SEQUENCE differ.
> > 
> > I'm getting that from: "When the replier detects a false retry, it is
> > permitted (but not always obligated) to return NFS4ERR_FALSE_RETRY in
> > response to the Sequence operation when it detects a false retry."
> 
> I think you are agreeing with me that OK is not appropriate here?

No, I think OK is OK:

> > If i understand the following language, we're required to return
> > FALSE_RETRY in the case the rpc credentials of the caller map to
> > different principals, but not otherwise.
> 
> This one drove me crazy:
> 
>    If a requester sent a Sequence operation with a slot ID and sequence
>    ID that are in the reply cache but the replier detected that the
>    retried request is not the same as the original request, including a
>    retry that has different operations or different arguments in the
>    operations from the original
> 
> SEQUENCE is not special - both the compounds in this example
> only have the SEQUENCE op and they differ only in that in the
> first sa_cachethis is False and in the second it is True.
> 
> So we have to return FALSE_SEQ_RETRY here...

It says "if the replier detected" a difference, not "if there is" a
difference.  So the replier is not required to do such detection.  This
agrees with the "not always obligated" above.

So I think it's allowed for the server to just return an old cached
response here (with the cached OK).  And I can't see any practical
problem that would create--a client shouldn't be sending a different
request with the same (slot, sequence) anyway.  The only potential risk
is the malicious client trying to snoop somebody else's reply cache,
hence the requirement in the case principals differ.

--b.

Re: pynfs replay cache test SEQ9f

[Trond Myklebust]

T24gVGh1LCAyMDE3LTEwLTEyIGF0IDIxOjUyIC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6
DQo+IE9uIFRodSwgT2N0IDEyLCAyMDE3IGF0IDAzOjAwOjUxUE0gLTA3MDAsIFRvbSBIYXluZXMg
d3JvdGU6DQo+ID4gT24gVGh1LCBPY3QgMTIsIDIwMTcgYXQgMDU6NDQ6NTRQTSAtMDQwMCwgSi4g
QnJ1Y2UgRmllbGRzIHdyb3RlOg0KPiA+ID4gWW91ciBtYWlsZXIncyBub3QgcXVvdGluZyByaWdo
dCwgaXQncyBhIGxpdHRsZSBoYXJkIGZvciBtZSB0bw0KPiA+ID4gZmluZCB5b3VyDQo+ID4gPiBy
ZXBsaWVzLiAgV2FkaW5nIGluOg0KPiA+ID4gDQo+ID4gPiBPbiBUaHUsIE9jdCAxMiwgMjAxNyBh
dCAwOTozOTowNFBNICswMDAwLCBUaG9tYXMgSGF5bmVzIHdyb3RlOg0KPiA+ID4gPiBPbiBPY3Qg
MTIsIDIwMTcsIGF0IDEyOjQ5IFBNLCBKLiBCcnVjZSBGaWVsZHMgPGJmaWVsZHNAZmllbGRzZXMN
Cj4gPiA+ID4gLm9yZzxtYWlsdG86YmZpZWxkc0BmaWVsZHNlcy5vcmc+PiB3cm90ZToNCj4gPiA+
ID4gPiBTbyBJICp0aGluayogdGhlIG9ubHkgY29ycmVjdCBvcHRpb25zIE9LIG9yIEZBTFNFX1JF
VFJZPw0KPiA+ID4gPiANCj4gPiA+ID4gSXQgY2Fu4oCZdCBiZSBPSyBpZiB0aGUgcGFyYW1ldGVy
cyB0byBTRVFVRU5DRSBkaWZmZXIuDQo+ID4gPiANCj4gPiA+IEknbSBnZXR0aW5nIHRoYXQgZnJv
bTogIldoZW4gdGhlIHJlcGxpZXIgZGV0ZWN0cyBhIGZhbHNlIHJldHJ5LA0KPiA+ID4gaXQgaXMN
Cj4gPiA+IHBlcm1pdHRlZCAoYnV0IG5vdCBhbHdheXMgb2JsaWdhdGVkKSB0byByZXR1cm4NCj4g
PiA+IE5GUzRFUlJfRkFMU0VfUkVUUlkgaW4NCj4gPiA+IHJlc3BvbnNlIHRvIHRoZSBTZXF1ZW5j
ZSBvcGVyYXRpb24gd2hlbiBpdCBkZXRlY3RzIGEgZmFsc2UNCj4gPiA+IHJldHJ5LiINCj4gPiAN
Cj4gPiBJIHRoaW5rIHlvdSBhcmUgYWdyZWVpbmcgd2l0aCBtZSB0aGF0IE9LIGlzIG5vdCBhcHBy
b3ByaWF0ZSBoZXJlPw0KPiANCj4gTm8sIEkgdGhpbmsgT0sgaXMgT0s6DQo+IA0KPiA+ID4gSWYg
aSB1bmRlcnN0YW5kIHRoZSBmb2xsb3dpbmcgbGFuZ3VhZ2UsIHdlJ3JlIHJlcXVpcmVkIHRvIHJl
dHVybg0KPiA+ID4gRkFMU0VfUkVUUlkgaW4gdGhlIGNhc2UgdGhlIHJwYyBjcmVkZW50aWFscyBv
ZiB0aGUgY2FsbGVyIG1hcCB0bw0KPiA+ID4gZGlmZmVyZW50IHByaW5jaXBhbHMsIGJ1dCBub3Qg
b3RoZXJ3aXNlLg0KPiA+IA0KPiA+IFRoaXMgb25lIGRyb3ZlIG1lIGNyYXp5Og0KPiA+IA0KPiA+
ICAgIElmIGEgcmVxdWVzdGVyIHNlbnQgYSBTZXF1ZW5jZSBvcGVyYXRpb24gd2l0aCBhIHNsb3Qg
SUQgYW5kDQo+ID4gc2VxdWVuY2UNCj4gPiAgICBJRCB0aGF0IGFyZSBpbiB0aGUgcmVwbHkgY2Fj
aGUgYnV0IHRoZSByZXBsaWVyIGRldGVjdGVkIHRoYXQgdGhlDQo+ID4gICAgcmV0cmllZCByZXF1
ZXN0IGlzIG5vdCB0aGUgc2FtZSBhcyB0aGUgb3JpZ2luYWwgcmVxdWVzdCwNCj4gPiBpbmNsdWRp
bmcgYQ0KPiA+ICAgIHJldHJ5IHRoYXQgaGFzIGRpZmZlcmVudCBvcGVyYXRpb25zIG9yIGRpZmZl
cmVudCBhcmd1bWVudHMgaW4NCj4gPiB0aGUNCj4gPiAgICBvcGVyYXRpb25zIGZyb20gdGhlIG9y
aWdpbmFsDQo+ID4gDQo+ID4gU0VRVUVOQ0UgaXMgbm90IHNwZWNpYWwgLSBib3RoIHRoZSBjb21w
b3VuZHMgaW4gdGhpcyBleGFtcGxlDQo+ID4gb25seSBoYXZlIHRoZSBTRVFVRU5DRSBvcCBhbmQg
dGhleSBkaWZmZXIgb25seSBpbiB0aGF0IGluIHRoZQ0KPiA+IGZpcnN0IHNhX2NhY2hldGhpcyBp
cyBGYWxzZSBhbmQgaW4gdGhlIHNlY29uZCBpdCBpcyBUcnVlLg0KPiA+IA0KPiA+IFNvIHdlIGhh
dmUgdG8gcmV0dXJuIEZBTFNFX1NFUV9SRVRSWSBoZXJlLi4uDQo+IA0KPiBJdCBzYXlzICJpZiB0
aGUgcmVwbGllciBkZXRlY3RlZCIgYSBkaWZmZXJlbmNlLCBub3QgImlmIHRoZXJlIGlzIiBhDQo+
IGRpZmZlcmVuY2UuICBTbyB0aGUgcmVwbGllciBpcyBub3QgcmVxdWlyZWQgdG8gZG8gc3VjaA0K
PiBkZXRlY3Rpb24uICBUaGlzDQo+IGFncmVlcyB3aXRoIHRoZSAibm90IGFsd2F5cyBvYmxpZ2F0
ZWQiIGFib3ZlLg0KPiANCj4gU28gSSB0aGluayBpdCdzIGFsbG93ZWQgZm9yIHRoZSBzZXJ2ZXIg
dG8ganVzdCByZXR1cm4gYW4gb2xkIGNhY2hlZA0KPiByZXNwb25zZSBoZXJlICh3aXRoIHRoZSBj
YWNoZWQgT0spLiAgQW5kIEkgY2FuJ3Qgc2VlIGFueSBwcmFjdGljYWwNCj4gcHJvYmxlbSB0aGF0
IHdvdWxkIGNyZWF0ZS0tYSBjbGllbnQgc2hvdWxkbid0IGJlIHNlbmRpbmcgYSBkaWZmZXJlbnQN
Cj4gcmVxdWVzdCB3aXRoIHRoZSBzYW1lIChzbG90LCBzZXF1ZW5jZSkgYW55d2F5LiAgVGhlIG9u
bHkgcG90ZW50aWFsDQo+IHJpc2sNCj4gaXMgdGhlIG1hbGljaW91cyBjbGllbnQgdHJ5aW5nIHRv
IHNub29wIHNvbWVib2R5IGVsc2UncyByZXBseSBjYWNoZSwNCj4gaGVuY2UgdGhlIHJlcXVpcmVt
ZW50IGluIHRoZSBjYXNlIHByaW5jaXBhbHMgZGlmZmVyLg0KDQpDbGllbnRzIG1heSBpbmRlZWQg
c2VuZCBhIGRpZmZlcmVudCByZXF1ZXN0IHdpdGggdGhlIHNhbWUgc2xvdCBhbmQNCnNlcXVlbmNl
IGlmIHRoZXkgZG9uJ3Qga25vdyB0aGF0IHRoZSBzZXJ2ZXIgcmVjZWl2ZWQgdGhlIGxhc3QgcmVx
dWVzdC4NClRoaXMgaXMgdGJlICJ1c2VyIHByZXNzZWQgXkMiIHNjZW5hcmlvLi4uDQoNClNlcnZl
cnMgTUFZIGlnbm9yZSB0aGF0IGZhY3QsIGJ1dCB0aGV5J2QgYmUgcmVhbGx5IHN0dXBpZCB0byBk
byBzby4uLg0KDQo+IA0KPiAtLWIuDQo+IC0tDQo+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBs
aXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC1uZnMiDQo+IGluDQo+IHRoZSBi
b2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnDQo+IE1vcmUgbWFq
b3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRt
bA0KPiANCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIs
IFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=

Re: pynfs replay cache test SEQ9f

[bfields]

On Fri, Oct 13, 2017 at 01:34:28PM +0000, Trond Myklebust wrote:
> On Thu, 2017-10-12 at 21:52 -0400, J. Bruce Fields wrote:
> > On Thu, Oct 12, 2017 at 03:00:51PM -0700, Tom Haynes wrote:
> > > On Thu, Oct 12, 2017 at 05:44:54PM -0400, J. Bruce Fields wrote:
> > > > Your mailer's not quoting right, it's a little hard for me to
> > > > find your
> > > > replies.  Wading in:
> > > > 
> > > > On Thu, Oct 12, 2017 at 09:39:04PM +0000, Thomas Haynes wrote:
> > > > > On Oct 12, 2017, at 12:49 PM, J. Bruce Fields <bfields@fieldses
> > > > > .org<mailto:bfields@fieldses.org>> wrote:
> > > > > > So I *think* the only correct options OK or FALSE_RETRY?
> > > > > 
> > > > > It can’t be OK if the parameters to SEQUENCE differ.
> > > > 
> > > > I'm getting that from: "When the replier detects a false retry,
> > > > it is
> > > > permitted (but not always obligated) to return
> > > > NFS4ERR_FALSE_RETRY in
> > > > response to the Sequence operation when it detects a false
> > > > retry."
> > > 
> > > I think you are agreeing with me that OK is not appropriate here?
> > 
> > No, I think OK is OK:
> > 
> > > > If i understand the following language, we're required to return
> > > > FALSE_RETRY in the case the rpc credentials of the caller map to
> > > > different principals, but not otherwise.
> > > 
> > > This one drove me crazy:
> > > 
> > >    If a requester sent a Sequence operation with a slot ID and
> > > sequence
> > >    ID that are in the reply cache but the replier detected that the
> > >    retried request is not the same as the original request,
> > > including a
> > >    retry that has different operations or different arguments in
> > > the
> > >    operations from the original
> > > 
> > > SEQUENCE is not special - both the compounds in this example
> > > only have the SEQUENCE op and they differ only in that in the
> > > first sa_cachethis is False and in the second it is True.
> > > 
> > > So we have to return FALSE_SEQ_RETRY here...
> > 
> > It says "if the replier detected" a difference, not "if there is" a
> > difference.  So the replier is not required to do such
> > detection.  This
> > agrees with the "not always obligated" above.
> > 
> > So I think it's allowed for the server to just return an old cached
> > response here (with the cached OK).  And I can't see any practical
> > problem that would create--a client shouldn't be sending a different
> > request with the same (slot, sequence) anyway.  The only potential
> > risk
> > is the malicious client trying to snoop somebody else's reply cache,
> > hence the requirement in the case principals differ.
> 
> Clients may indeed send a different request with the same slot and
> sequence if they don't know that the server received the last request.
> This is tbe "user pressed ^C" scenario...
> 
> Servers MAY ignore that fact, but they'd be really stupid to do so...

OK, OK, I'll look into fixing the server (I'm pretty sure we get this
wrong).

You've explained the ctrl-C case before and I don't think I understood
it.  I guess otherwise the only way for the client to sort out the
situation would be to retry the original request.  And that requires
keeping the arguments and credentials around to handle potential
retries.  And that's impractical if the process is going away?  OK.

--b.

Re: pynfs replay cache test SEQ9f

[Trond Myklebust]

T24gRnJpLCAyMDE3LTEwLTEzIGF0IDExOjAwIC0wNDAwLCBiZmllbGRzQGZpZWxkc2VzLm9yZyB3
cm90ZToNCj4gT24gRnJpLCBPY3QgMTMsIDIwMTcgYXQgMDE6MzQ6MjhQTSArMDAwMCwgVHJvbmQg
TXlrbGVidXN0IHdyb3RlOg0KPiA+IE9uIFRodSwgMjAxNy0xMC0xMiBhdCAyMTo1MiAtMDQwMCwg
Si4gQnJ1Y2UgRmllbGRzIHdyb3RlOg0KPiA+ID4gT24gVGh1LCBPY3QgMTIsIDIwMTcgYXQgMDM6
MDA6NTFQTSAtMDcwMCwgVG9tIEhheW5lcyB3cm90ZToNCj4gPiA+ID4gT24gVGh1LCBPY3QgMTIs
IDIwMTcgYXQgMDU6NDQ6NTRQTSAtMDQwMCwgSi4gQnJ1Y2UgRmllbGRzDQo+ID4gPiA+IHdyb3Rl
Og0KPiA+ID4gPiA+IFlvdXIgbWFpbGVyJ3Mgbm90IHF1b3RpbmcgcmlnaHQsIGl0J3MgYSBsaXR0
bGUgaGFyZCBmb3IgbWUgdG8NCj4gPiA+ID4gPiBmaW5kIHlvdXINCj4gPiA+ID4gPiByZXBsaWVz
LiAgV2FkaW5nIGluOg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IE9uIFRodSwgT2N0IDEyLCAyMDE3
IGF0IDA5OjM5OjA0UE0gKzAwMDAsIFRob21hcyBIYXluZXMNCj4gPiA+ID4gPiB3cm90ZToNCj4g
PiA+ID4gPiA+IE9uIE9jdCAxMiwgMjAxNywgYXQgMTI6NDkgUE0sIEouIEJydWNlIEZpZWxkcyA8
YmZpZWxkc0BmaWVsDQo+ID4gPiA+ID4gPiBkc2VzDQo+ID4gPiA+ID4gPiAub3JnPG1haWx0bzpi
ZmllbGRzQGZpZWxkc2VzLm9yZz4+IHdyb3RlOg0KPiA+ID4gPiA+ID4gPiBTbyBJICp0aGluayog
dGhlIG9ubHkgY29ycmVjdCBvcHRpb25zIE9LIG9yIEZBTFNFX1JFVFJZPw0KPiA+ID4gPiA+ID4g
DQo+ID4gPiA+ID4gPiBJdCBjYW7igJl0IGJlIE9LIGlmIHRoZSBwYXJhbWV0ZXJzIHRvIFNFUVVF
TkNFIGRpZmZlci4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBJJ20gZ2V0dGluZyB0aGF0IGZyb206
ICJXaGVuIHRoZSByZXBsaWVyIGRldGVjdHMgYSBmYWxzZQ0KPiA+ID4gPiA+IHJldHJ5LA0KPiA+
ID4gPiA+IGl0IGlzDQo+ID4gPiA+ID4gcGVybWl0dGVkIChidXQgbm90IGFsd2F5cyBvYmxpZ2F0
ZWQpIHRvIHJldHVybg0KPiA+ID4gPiA+IE5GUzRFUlJfRkFMU0VfUkVUUlkgaW4NCj4gPiA+ID4g
PiByZXNwb25zZSB0byB0aGUgU2VxdWVuY2Ugb3BlcmF0aW9uIHdoZW4gaXQgZGV0ZWN0cyBhIGZh
bHNlDQo+ID4gPiA+ID4gcmV0cnkuIg0KPiA+ID4gPiANCj4gPiA+ID4gSSB0aGluayB5b3UgYXJl
IGFncmVlaW5nIHdpdGggbWUgdGhhdCBPSyBpcyBub3QgYXBwcm9wcmlhdGUNCj4gPiA+ID4gaGVy
ZT8NCj4gPiA+IA0KPiA+ID4gTm8sIEkgdGhpbmsgT0sgaXMgT0s6DQo+ID4gPiANCj4gPiA+ID4g
PiBJZiBpIHVuZGVyc3RhbmQgdGhlIGZvbGxvd2luZyBsYW5ndWFnZSwgd2UncmUgcmVxdWlyZWQg
dG8NCj4gPiA+ID4gPiByZXR1cm4NCj4gPiA+ID4gPiBGQUxTRV9SRVRSWSBpbiB0aGUgY2FzZSB0
aGUgcnBjIGNyZWRlbnRpYWxzIG9mIHRoZSBjYWxsZXIgbWFwDQo+ID4gPiA+ID4gdG8NCj4gPiA+
ID4gPiBkaWZmZXJlbnQgcHJpbmNpcGFscywgYnV0IG5vdCBvdGhlcndpc2UuDQo+ID4gPiA+IA0K
PiA+ID4gPiBUaGlzIG9uZSBkcm92ZSBtZSBjcmF6eToNCj4gPiA+ID4gDQo+ID4gPiA+ICAgIElm
IGEgcmVxdWVzdGVyIHNlbnQgYSBTZXF1ZW5jZSBvcGVyYXRpb24gd2l0aCBhIHNsb3QgSUQgYW5k
DQo+ID4gPiA+IHNlcXVlbmNlDQo+ID4gPiA+ICAgIElEIHRoYXQgYXJlIGluIHRoZSByZXBseSBj
YWNoZSBidXQgdGhlIHJlcGxpZXIgZGV0ZWN0ZWQgdGhhdA0KPiA+ID4gPiB0aGUNCj4gPiA+ID4g
ICAgcmV0cmllZCByZXF1ZXN0IGlzIG5vdCB0aGUgc2FtZSBhcyB0aGUgb3JpZ2luYWwgcmVxdWVz
dCwNCj4gPiA+ID4gaW5jbHVkaW5nIGENCj4gPiA+ID4gICAgcmV0cnkgdGhhdCBoYXMgZGlmZmVy
ZW50IG9wZXJhdGlvbnMgb3IgZGlmZmVyZW50IGFyZ3VtZW50cw0KPiA+ID4gPiBpbg0KPiA+ID4g
PiB0aGUNCj4gPiA+ID4gICAgb3BlcmF0aW9ucyBmcm9tIHRoZSBvcmlnaW5hbA0KPiA+ID4gPiAN
Cj4gPiA+ID4gU0VRVUVOQ0UgaXMgbm90IHNwZWNpYWwgLSBib3RoIHRoZSBjb21wb3VuZHMgaW4g
dGhpcyBleGFtcGxlDQo+ID4gPiA+IG9ubHkgaGF2ZSB0aGUgU0VRVUVOQ0Ugb3AgYW5kIHRoZXkg
ZGlmZmVyIG9ubHkgaW4gdGhhdCBpbiB0aGUNCj4gPiA+ID4gZmlyc3Qgc2FfY2FjaGV0aGlzIGlz
IEZhbHNlIGFuZCBpbiB0aGUgc2Vjb25kIGl0IGlzIFRydWUuDQo+ID4gPiA+IA0KPiA+ID4gPiBT
byB3ZSBoYXZlIHRvIHJldHVybiBGQUxTRV9TRVFfUkVUUlkgaGVyZS4uLg0KPiA+ID4gDQo+ID4g
PiBJdCBzYXlzICJpZiB0aGUgcmVwbGllciBkZXRlY3RlZCIgYSBkaWZmZXJlbmNlLCBub3QgImlm
IHRoZXJlIGlzIg0KPiA+ID4gYQ0KPiA+ID4gZGlmZmVyZW5jZS4gIFNvIHRoZSByZXBsaWVyIGlz
IG5vdCByZXF1aXJlZCB0byBkbyBzdWNoDQo+ID4gPiBkZXRlY3Rpb24uICBUaGlzDQo+ID4gPiBh
Z3JlZXMgd2l0aCB0aGUgIm5vdCBhbHdheXMgb2JsaWdhdGVkIiBhYm92ZS4NCj4gPiA+IA0KPiA+
ID4gU28gSSB0aGluayBpdCdzIGFsbG93ZWQgZm9yIHRoZSBzZXJ2ZXIgdG8ganVzdCByZXR1cm4g
YW4gb2xkDQo+ID4gPiBjYWNoZWQNCj4gPiA+IHJlc3BvbnNlIGhlcmUgKHdpdGggdGhlIGNhY2hl
ZCBPSykuICBBbmQgSSBjYW4ndCBzZWUgYW55DQo+ID4gPiBwcmFjdGljYWwNCj4gPiA+IHByb2Js
ZW0gdGhhdCB3b3VsZCBjcmVhdGUtLWEgY2xpZW50IHNob3VsZG4ndCBiZSBzZW5kaW5nIGENCj4g
PiA+IGRpZmZlcmVudA0KPiA+ID4gcmVxdWVzdCB3aXRoIHRoZSBzYW1lIChzbG90LCBzZXF1ZW5j
ZSkgYW55d2F5LiAgVGhlIG9ubHkNCj4gPiA+IHBvdGVudGlhbA0KPiA+ID4gcmlzaw0KPiA+ID4g
aXMgdGhlIG1hbGljaW91cyBjbGllbnQgdHJ5aW5nIHRvIHNub29wIHNvbWVib2R5IGVsc2UncyBy
ZXBseQ0KPiA+ID4gY2FjaGUsDQo+ID4gPiBoZW5jZSB0aGUgcmVxdWlyZW1lbnQgaW4gdGhlIGNh
c2UgcHJpbmNpcGFscyBkaWZmZXIuDQo+ID4gDQo+ID4gQ2xpZW50cyBtYXkgaW5kZWVkIHNlbmQg
YSBkaWZmZXJlbnQgcmVxdWVzdCB3aXRoIHRoZSBzYW1lIHNsb3QgYW5kDQo+ID4gc2VxdWVuY2Ug
aWYgdGhleSBkb24ndCBrbm93IHRoYXQgdGhlIHNlcnZlciByZWNlaXZlZCB0aGUgbGFzdA0KPiA+
IHJlcXVlc3QuDQo+ID4gVGhpcyBpcyB0YmUgInVzZXIgcHJlc3NlZCBeQyIgc2NlbmFyaW8uLi4N
Cj4gPiANCj4gPiBTZXJ2ZXJzIE1BWSBpZ25vcmUgdGhhdCBmYWN0LCBidXQgdGhleSdkIGJlIHJl
YWxseSBzdHVwaWQgdG8gZG8NCj4gPiBzby4uLg0KPiANCj4gT0ssIE9LLCBJJ2xsIGxvb2sgaW50
byBmaXhpbmcgdGhlIHNlcnZlciAoSSdtIHByZXR0eSBzdXJlIHdlIGdldCB0aGlzDQo+IHdyb25n
KS4NCj4gDQo+IFlvdSd2ZSBleHBsYWluZWQgdGhlIGN0cmwtQyBjYXNlIGJlZm9yZSBhbmQgSSBk
b24ndCB0aGluayBJDQo+IHVuZGVyc3Rvb2QNCj4gaXQuICBJIGd1ZXNzIG90aGVyd2lzZSB0aGUg
b25seSB3YXkgZm9yIHRoZSBjbGllbnQgdG8gc29ydCBvdXQgdGhlDQo+IHNpdHVhdGlvbiB3b3Vs
ZCBiZSB0byByZXRyeSB0aGUgb3JpZ2luYWwgcmVxdWVzdC4gIEFuZCB0aGF0IHJlcXVpcmVzDQo+
IGtlZXBpbmcgdGhlIGFyZ3VtZW50cyBhbmQgY3JlZGVudGlhbHMgYXJvdW5kIHRvIGhhbmRsZSBw
b3RlbnRpYWwNCj4gcmV0cmllcy4gIEFuZCB0aGF0J3MgaW1wcmFjdGljYWwgaWYgdGhlIHByb2Nl
c3MgaXMgZ29pbmcgYXdheT8gIE9LLg0KPiANCg0KUmlnaHQsIHdlJ3JlIG5vdCBnb2luZyB0byBk
byB0aGF0IGp1c3QgZm9yIGRhdGEgdGhhdCBpcyBqdXN0IGdvaW5nIHRvDQpiZSB0b3NzZWQgYXdh
eSBhbnl3YXkuIFdlIGFscmVhZHkgZ3VhcmFudGVlIHRoYXQgbm9uLWlkZW1wb3RlbnQNCm9wZXJh
dGlvbnMgKHRoZSBvbmVzIHRoYXQgd2UgYWN0dWFsbHkgZG8gYXNrIHRoZSBzZXJ2ZXIgdG8gY2Fj
aGUpIGFyZQ0KZ3VhcmFudGVlZCB0byBjb21wbGV0ZSB3aGV0aGVyIG9yIG5vdCB0aGUgdXNlciBw
cmVzc2VzIF5DLCBzbyB0aGlzIGlzDQptYWlubHkgYWJvdXQgd2hhdCBoYXBwZW5zIHdoZW4gc29t
ZWJvZHkgaW50ZXJydXB0cyBhbiBvcGVyYXRpb24gdGhhdCB3ZQ0KZGlkIG5vdCB3YW50IHRoZSBz
ZXJ2ZXIgdG8gY2FjaGUuDQoNCkkgaGF2ZSBhIHBhdGNoIG91dCB0aGVyZSB0aGF0IGp1c3QgcmVw
bGF5cyBhIFNFUVVFTkNFIG9wIGlmIHdlIGRldGVjdA0KdGhhdCBhbiBSUEMgY2FsbCB3YXMgaW50
ZXJydXB0ZWQuIFRoYXQgc2hvdWxkIGJlIHN1ZmZpY2llbnQgdG8gZGVhbA0Kd2l0aCBzZXJ2ZXJz
IHRoYXQgY2FjaGUgZXZlcnl0aGluZyAod2hldGhlciBvciBub3QgdGhlIGNsaWVudCBzZXRzDQpz
YV9jYWNoZXRoaXMpLCBidXQgZG9uJ3Qgd2FudCB0byBkbyBORlM0RVJSX1NFUV9GQUxTRV9SRVRS
WS4gVGhhdA0KcGFydGljdWxhciBjb21iaW5hdGlvbiBoYXMgYmVlbiBzZWVuIHRvIGJlIGV4dHJl
bWVseSB0b3hpYyB0byB0aGUNCmN1cnJlbnQgY2xpZW50LCBiZWNhdXNlIGl0IGNhbiBnZXQgcmVw
bGF5ZWQgTE9PS1VQIG9yIEdFVEFUVFIgcmVxdWVzdHMNCmFmdGVyIHNvbWVvbmUgcHJlc3NlcyBe
Qy4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwg
UHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==

Re: pynfs replay cache test SEQ9f

[bfields]

On Fri, Oct 13, 2017 at 03:26:51PM +0000, Trond Myklebust wrote:
> On Fri, 2017-10-13 at 11:00 -0400, bfields@fieldses.org wrote:
> > OK, OK, I'll look into fixing the server (I'm pretty sure we get this
> > wrong).
> > 
> > You've explained the ctrl-C case before and I don't think I
> > understood
> > it.  I guess otherwise the only way for the client to sort out the
> > situation would be to retry the original request.  And that requires
> > keeping the arguments and credentials around to handle potential
> > retries.  And that's impractical if the process is going away?  OK.
> > 
> 
> Right, we're not going to do that just for data that is just going to
> be tossed away anyway. We already guarantee that non-idempotent
> operations (the ones that we actually do ask the server to cache) are
> guaranteed to complete whether or not the user presses ^C, so this is
> mainly about what happens when somebody interrupts an operation that we
> did not want the server to cache.
> 
> I have a patch out there that just replays a SEQUENCE op if we detect
> that an RPC call was interrupted. That should be sufficient to deal
> with servers that cache everything (whether or not the client sets
> sa_cachethis), but don't want to do NFS4ERR_SEQ_FALSE_RETRY. That
> particular combination has been seen to be extremely toxic to the
> current client, because it can get replayed LOOKUP or GETATTR requests
> after someone presses ^C.

Those all involve uncached compounds with more than one op.  My reading
of knfsd code is that it will return RETRY_UNCACHED_REP in this case,
and I think (I might be misunderstanding) that the client will bump the
slot seqid and retry in that case.  So I *think* you shouldn't be seeing
that problem with knfsd?

--b.

Re: pynfs replay cache test SEQ9f

[bfields]

On Fri, Oct 13, 2017 at 02:50:15PM -0400, bfields@fieldses.org wrote:
> Those all involve uncached compounds with more than one op.  My reading
> of knfsd code is that it will return RETRY_UNCACHED_REP in this case,
> and I think (I might be misunderstanding) that the client will bump the
> slot seqid and retry in that case.  So I *think* you shouldn't be seeing
> that problem with knfsd?

So to bring it back to the original question, I'm still not sure it's
actually a problem if a server returns NFS_OK when cache_this is set
differently betwen two compound calls each containing just a single
SEQUENCE.

But, maybe it's worth at least warning about.

--b.

Re: pynfs replay cache test SEQ9f

[bfields]

On Fri, Oct 13, 2017 at 02:50:15PM -0400, bfields@fieldses.org wrote:
> On Fri, Oct 13, 2017 at 03:26:51PM +0000, Trond Myklebust wrote:
> > On Fri, 2017-10-13 at 11:00 -0400, bfields@fieldses.org wrote:
> > > OK, OK, I'll look into fixing the server (I'm pretty sure we get this
> > > wrong).
> > > 
> > > You've explained the ctrl-C case before and I don't think I
> > > understood
> > > it.  I guess otherwise the only way for the client to sort out the
> > > situation would be to retry the original request.  And that requires
> > > keeping the arguments and credentials around to handle potential
> > > retries.  And that's impractical if the process is going away?  OK.
> > > 
> > 
> > Right, we're not going to do that just for data that is just going to
> > be tossed away anyway. We already guarantee that non-idempotent
> > operations (the ones that we actually do ask the server to cache) are
> > guaranteed to complete whether or not the user presses ^C, so this is
> > mainly about what happens when somebody interrupts an operation that we
> > did not want the server to cache.
> > 
> > I have a patch out there that just replays a SEQUENCE op if we detect
> > that an RPC call was interrupted. That should be sufficient to deal
> > with servers that cache everything (whether or not the client sets
> > sa_cachethis), but don't want to do NFS4ERR_SEQ_FALSE_RETRY. That
> > particular combination has been seen to be extremely toxic to the
> > current client, because it can get replayed LOOKUP or GETATTR requests
> > after someone presses ^C.
> 
> Those all involve uncached compounds with more than one op.  My reading
> of knfsd code is that it will return RETRY_UNCACHED_REP in this case,
> and I think (I might be misunderstanding) that the client will bump the
> slot seqid and retry in that case.  So I *think* you shouldn't be seeing
> that problem with knfsd?

Argh, no, you're sending a bare SEQUENCE so of course there's just one
op.

And looking at Olga's COPY example and the code....  The server gets
confused in this case and returns a reply to the SEQUENCE, nothing else,
but sets the reply's opcnt to the count taken from the original call,
for some reason.

So, the server's returning a corrupt reply.  It needs to return a reply
that's actually legal xdr and SEQUENCE results that match the call.
Beyond that it probably doesn't matter exactly what it returns--either
it handles it as a replay and doesn't bump the seqid, or a new call and
does, but either way the seqid ends up in the same place, which is the
goal here.  OK.

--b.

RE: [nfsv4] pynfs replay cache test SEQ9f

[Frank Filz]

> On Fri, 2017-10-13 at 11:00 -0400, bfields@fieldses.org wrote:
> > On Fri, Oct 13, 2017 at 01:34:28PM +0000, Trond Myklebust wrote:
> > > On Thu, 2017-10-12 at 21:52 -0400, J. Bruce Fields wrote:
> > > > On Thu, Oct 12, 2017 at 03:00:51PM -0700, Tom Haynes wrote:
> > > > > On Thu, Oct 12, 2017 at 05:44:54PM -0400, J. Bruce Fields
> > > > > wrote:
> > > > > > Your mailer's not quoting right, it's a little hard for me to
> > > > > > find your replies.  Wading in:
> > > > > >
> > > > > > On Thu, Oct 12, 2017 at 09:39:04PM +0000, Thomas Haynes
> > > > > > wrote:
> > > > > > > On Oct 12, 2017, at 12:49 PM, J. Bruce Fields <bfields@fiel
> > > > > > > dses .org<mailto:bfields@fieldses.org>> wrote:
> > > > > > > > So I *think* the only correct options OK or FALSE_RETRY?
> > > > > > >
> > > > > > > It can=E2=80=99t be OK if the parameters to SEQUENCE differ.
> > > > > >
> > > > > > I'm getting that from: "When the replier detects a false
> > > > > > retry, it is permitted (but not always obligated) to return
> > > > > > NFS4ERR_FALSE_RETRY in response to the Sequence operation
> when
> > > > > > it detects a false retry."
> > > > >
> > > > > I think you are agreeing with me that OK is not appropriate
> > > > > here?
> > > >
> > > > No, I think OK is OK:
> > > >
> > > > > > If i understand the following language, we're required to
> > > > > > return FALSE_RETRY in the case the rpc credentials of the
> > > > > > caller map to different principals, but not otherwise.
> > > > >
> > > > > This one drove me crazy:
> > > > >
> > > > >    If a requester sent a Sequence operation with a slot ID and
> > > > > sequence
> > > > >    ID that are in the reply cache but the replier detected that
> > > > > the
> > > > >    retried request is not the same as the original request,
> > > > > including a
> > > > >    retry that has different operations or different arguments in
> > > > > the
> > > > >    operations from the original
> > > > >
> > > > > SEQUENCE is not special - both the compounds in this example
> > > > > only have the SEQUENCE op and they differ only in that in the
> > > > > first sa_cachethis is False and in the second it is True.
> > > > >
> > > > > So we have to return FALSE_SEQ_RETRY here...
> > > >
> > > > It says "if the replier detected" a difference, not "if there is"
> > > > a
> > > > difference.  So the replier is not required to do such detection.
> > > > This agrees with the "not always obligated" above.
> > > >
> > > > So I think it's allowed for the server to just return an old
> > > > cached response here (with the cached OK).  And I can't see any
> > > > practical problem that would create--a client shouldn't be sending
> > > > a different request with the same (slot, sequence) anyway.  The
> > > > only potential risk is the malicious client trying to snoop
> > > > somebody else's reply cache, hence the requirement in the case
> > > > principals differ.
> > >
> > > Clients may indeed send a different request with the same slot and
> > > sequence if they don't know that the server received the last
> > > request.
> > > This is tbe "user pressed ^C" scenario...
> > >
> > > Servers MAY ignore that fact, but they'd be really stupid to do
> > > so...
> >
> > OK, OK, I'll look into fixing the server (I'm pretty sure we get this
> > wrong).
> >
> > You've explained the ctrl-C case before and I don't think I understood
> > it.  I guess otherwise the only way for the client to sort out the
> > situation would be to retry the original request.  And that requires
> > keeping the arguments and credentials around to handle potential
> > retries.  And that's impractical if the process is going away?  OK.
> >
> 
> Right, we're not going to do that just for data that is just going to be =
tossed
> away anyway. We already guarantee that non-idempotent operations (the
> ones that we actually do ask the server to cache) are guaranteed to compl=
ete
> whether or not the user presses ^C, so this is mainly about what happens
> when somebody interrupts an operation that we did not want the server to
> cache.
> 
> I have a patch out there that just replays a SEQUENCE op if we detect tha=
t an
> RPC call was interrupted. That should be sufficient to deal with servers =
that
> cache everything (whether or not the client sets sa_cachethis), but don't=

> want to do NFS4ERR_SEQ_FALSE_RETRY. That particular combination has
> been seen to be extremely toxic to the current client, because it can get=

> replayed LOOKUP or GETATTR requests after someone presses ^C.

I'm wondering if Ganesha does this right...

Bruce, if there's something not covered by the pynfs tests, can we make sur=
e to add one?

Frank


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH] Args need to be the same for replay cache

[J. Bruce Fields]

This prompted a long discussion on the correct server behavior in the
case of false retries of bare SEQUENCE calls, which was kind of
irrelevant to your actual pynfs patch.  Somebody just reminded me that I
never applied that.

Applied now, thanks.--b.

On Wed, Oct 11, 2017 at 09:48:22AM -0700, Thomas Haynes wrote:
> From: Tom Haynes <loghyr@primarydata.com>
> 
> 2.10.6.1.3.1.  False Retry
> 
>   If a requester sent a Sequence operation with a slot ID and sequence
>   ID that are in the reply cache but the replier detected that the
>   retried request is not the same as the original request, including a
>   retry that has different operations or different arguments in the
>   operations from the original and a retry that uses a different
>    principal in the RPC request's credential field that translates to a
>   different user, then this is a false retry.  When the replier detects
>   a false retry, it is permitted (but not always obligated) to return
>   NFS4ERR_SEQ_FALSE_RETRY in response to the Sequence operation when it
>   detects a false retry.
> 
> Or in other words, sa_cachethis needs to be set or a
> server can respond with an error.
> 
> Signed-off-by: Tom Haynes <loghyr@primarydata.com>
> ---
>  nfs4.1/server41tests/st_sequence.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/nfs4.1/server41tests/st_sequence.py b/nfs4.1/server41tests/st_sequence.py
> index d8d460c..e1e5f06 100644
> --- a/nfs4.1/server41tests/st_sequence.py
> +++ b/nfs4.1/server41tests/st_sequence.py
> @@ -115,7 +115,7 @@ def testReplayCache001(t, env):
>      sess1 = c1.create_session()
>      res1 = sess1.compound([op.putrootfh()], cache_this=True)
>      check(res1)
> -    res2 = sess1.compound([op.putrootfh()], seq_delta=0)
> +    res2 = sess1.compound([op.putrootfh()], cache_this=True, seq_delta=0)
>      check(res2)
>      res1.tag = res2.tag = ""
>      if not nfs4lib.test_equal(res1, res2):
> @@ -137,7 +137,7 @@ def testReplayCache002(t, env):
>            op.rename("%s_1" % env.testname(t), "%s_2" % env.testname(t))]
>      res1 = sess1.compound(ops, cache_this=True)
>      check(res1)
> -    res2 = sess1.compound(ops, seq_delta=0)
> +    res2 = sess1.compound(ops, cache_this=True, seq_delta=0)
>      check(res2)
>      res1.tag = res2.tag = ""
>      if not nfs4lib.test_equal(res1, res2):
> @@ -158,7 +158,7 @@ def testReplayCache003(t, env):
>      sess1 = c1.create_session()
>      res1 = sess1.compound([op.putrootfh(), op.lookup("")], cache_this=True)
>      check(res1, NFS4ERR_INVAL)
> -    res2 = sess1.compound([op.putrootfh(), op.lookup("")], seq_delta=0)
> +    res2 = sess1.compound([op.putrootfh(), op.lookup("")], cache_this=True, seq_delta=0)
>      check(res2, NFS4ERR_INVAL)
>      res1.tag = res2.tag = ""
>      if not nfs4lib.test_equal(res1, res2):
> @@ -176,7 +176,7 @@ def testReplayCache004(t, env):
>      ops += [op.savefh(), op.rename("", "foo")]
>      res1 = sess1.compound(ops, cache_this=True)
>      check(res1, NFS4ERR_INVAL)
> -    res2 = sess1.compound(ops, seq_delta=0)
> +    res2 = sess1.compound(ops, cache_this=True, seq_delta=0)
>      check(res2, NFS4ERR_INVAL)
>      res1.tag = res2.tag = ""
>      if not nfs4lib.test_equal(res1, res2):
> @@ -192,7 +192,7 @@ def testReplayCache005(t, env):
>      sess1 = c1.create_session()
>      res1 = sess1.compound([op.illegal()], cache_this=True)
>      check(res1, NFS4ERR_OP_ILLEGAL)
> -    res2 = sess1.compound([op.illegal()], seq_delta=0)
> +    res2 = sess1.compound([op.illegal()], cache_this=True, seq_delta=0)
>      check(res2, NFS4ERR_OP_ILLEGAL)
>      res1.tag = res2.tag = ""
>      if not nfs4lib.test_equal(res1, res2):
> @@ -208,7 +208,7 @@ def testReplayCache006(t, env):
>      sess = c.create_session()
>      res1 = sess.compound([])
>      check(res1)
> -    res2 = sess.compound([], seq_delta=0)
> +    res2 = sess.compound([], cache_this=True, seq_delta=0)
>      check(res2)
>      res1.tag = res2.tag = ""
>      if not nfs4lib.test_equal(res1, res2):
> -- 
> 2.3.6

Re: [PATCH] Args need to be the same for replay cache

[Olga Kornievskaia]

Hi Bruce,

Do you by any chance have a reference to this discussion?

I would like to reference it. For now I'm hijacking this thread to
bring this up. I'm still concerned about the case where client sent a
request and slot got interrupted (so by default the client doesn't
increment the seq#). Then the client re-used the slot for the same
kind of operation (WRITE is very interesting) with same arguments but
say different FH. Is the server obligated the cache the whole call to
address and check that? You have a patch to check for false retries
that checks for different creds but I don't think you have something
that would catch this case?

Thank you.


On Tue, Apr 10, 2018 at 3:49 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> This prompted a long discussion on the correct server behavior in the
> case of false retries of bare SEQUENCE calls, which was kind of
> irrelevant to your actual pynfs patch.  Somebody just reminded me that I
> never applied that.
>
> Applied now, thanks.--b.
>
> On Wed, Oct 11, 2017 at 09:48:22AM -0700, Thomas Haynes wrote:
>> From: Tom Haynes <loghyr@primarydata.com>
>>
>> 2.10.6.1.3.1.  False Retry
>>
>>   If a requester sent a Sequence operation with a slot ID and sequence
>>   ID that are in the reply cache but the replier detected that the
>>   retried request is not the same as the original request, including a
>>   retry that has different operations or different arguments in the
>>   operations from the original and a retry that uses a different
>>    principal in the RPC request's credential field that translates to a
>>   different user, then this is a false retry.  When the replier detects
>>   a false retry, it is permitted (but not always obligated) to return
>>   NFS4ERR_SEQ_FALSE_RETRY in response to the Sequence operation when it
>>   detects a false retry.
>>
>> Or in other words, sa_cachethis needs to be set or a
>> server can respond with an error.
>>
>> Signed-off-by: Tom Haynes <loghyr@primarydata.com>
>> ---
>>  nfs4.1/server41tests/st_sequence.py | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/nfs4.1/server41tests/st_sequence.py b/nfs4.1/server41tests/st_sequence.py
>> index d8d460c..e1e5f06 100644
>> --- a/nfs4.1/server41tests/st_sequence.py
>> +++ b/nfs4.1/server41tests/st_sequence.py
>> @@ -115,7 +115,7 @@ def testReplayCache001(t, env):
>>      sess1 = c1.create_session()
>>      res1 = sess1.compound([op.putrootfh()], cache_this=True)
>>      check(res1)
>> -    res2 = sess1.compound([op.putrootfh()], seq_delta=0)
>> +    res2 = sess1.compound([op.putrootfh()], cache_this=True, seq_delta=0)
>>      check(res2)
>>      res1.tag = res2.tag = ""
>>      if not nfs4lib.test_equal(res1, res2):
>> @@ -137,7 +137,7 @@ def testReplayCache002(t, env):
>>            op.rename("%s_1" % env.testname(t), "%s_2" % env.testname(t))]
>>      res1 = sess1.compound(ops, cache_this=True)
>>      check(res1)
>> -    res2 = sess1.compound(ops, seq_delta=0)
>> +    res2 = sess1.compound(ops, cache_this=True, seq_delta=0)
>>      check(res2)
>>      res1.tag = res2.tag = ""
>>      if not nfs4lib.test_equal(res1, res2):
>> @@ -158,7 +158,7 @@ def testReplayCache003(t, env):
>>      sess1 = c1.create_session()
>>      res1 = sess1.compound([op.putrootfh(), op.lookup("")], cache_this=True)
>>      check(res1, NFS4ERR_INVAL)
>> -    res2 = sess1.compound([op.putrootfh(), op.lookup("")], seq_delta=0)
>> +    res2 = sess1.compound([op.putrootfh(), op.lookup("")], cache_this=True, seq_delta=0)
>>      check(res2, NFS4ERR_INVAL)
>>      res1.tag = res2.tag = ""
>>      if not nfs4lib.test_equal(res1, res2):
>> @@ -176,7 +176,7 @@ def testReplayCache004(t, env):
>>      ops += [op.savefh(), op.rename("", "foo")]
>>      res1 = sess1.compound(ops, cache_this=True)
>>      check(res1, NFS4ERR_INVAL)
>> -    res2 = sess1.compound(ops, seq_delta=0)
>> +    res2 = sess1.compound(ops, cache_this=True, seq_delta=0)
>>      check(res2, NFS4ERR_INVAL)
>>      res1.tag = res2.tag = ""
>>      if not nfs4lib.test_equal(res1, res2):
>> @@ -192,7 +192,7 @@ def testReplayCache005(t, env):
>>      sess1 = c1.create_session()
>>      res1 = sess1.compound([op.illegal()], cache_this=True)
>>      check(res1, NFS4ERR_OP_ILLEGAL)
>> -    res2 = sess1.compound([op.illegal()], seq_delta=0)
>> +    res2 = sess1.compound([op.illegal()], cache_this=True, seq_delta=0)
>>      check(res2, NFS4ERR_OP_ILLEGAL)
>>      res1.tag = res2.tag = ""
>>      if not nfs4lib.test_equal(res1, res2):
>> @@ -208,7 +208,7 @@ def testReplayCache006(t, env):
>>      sess = c.create_session()
>>      res1 = sess.compound([])
>>      check(res1)
>> -    res2 = sess.compound([], seq_delta=0)
>> +    res2 = sess.compound([], cache_this=True, seq_delta=0)
>>      check(res2)
>>      res1.tag = res2.tag = ""
>>      if not nfs4lib.test_equal(res1, res2):
>> --
>> 2.3.6
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Args need to be the same for replay cache

[J. Bruce Fields]

On Tue, Apr 24, 2018 at 04:10:29PM -0400, Olga Kornievskaia wrote:
> Do you by any chance have a reference to this discussion?

This:
	http://lkml.kernel.org/r/1507740502-5151-1-git-send-email-Thomas.Haynes@primarydata.com

and this:

	http://lkml.kernel.org/r/E0161195-9F4A-4B36-A71D-6A924498C893@primarydata.com

and followups.

> I would like to reference it. For now I'm hijacking this thread to
> bring this up. I'm still concerned about the case where client sent a
> request and slot got interrupted (so by default the client doesn't
> increment the seq#). Then the client re-used the slot for the same
> kind of operation (WRITE is very interesting) with same arguments but
> say different FH. Is the server obligated the cache the whole call to
> address and check that? You have a patch to check for false retries
> that checks for different creds but I don't think you have something
> that would catch this case?

Right.  I don't believe the spec requires us to catch false retries in
every possible case.  That may mean we return a pretty bizarre reply
that doesn't match the request, but that's the client's own fault....

--b.