Re: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock request

Thanks for testing it, Frank.

FWIW, if the unmodified test still passes on ganesha then that's
probably an indicator that it's not doing adequate vetting of the lock
seqid with v4.0.

Cheers,
Jeff

On Mon, 2023-03-13 at 11:51 -0700, Frank Filz wrote:

Looks good to me, tested against Ganesha and the updated patch passes.

Frank

quoted

-----Original Message-----
From: Jeff Layton [mailto:jlayton@kernel.org]
Sent: Monday, March 13, 2023 4:24 AM
To: calum.mackay@oracle.com
Cc: bfields@fieldses.org; ffilzlnx@mindspring.com;

linux-nfs@vger.kernel.org;

quoted

Frank Filz [off-list ref]
Subject: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock

request

quoted

This test currently fails against Linux nfsd, but I think it's the test

that's wrong. It

quoted

basically does:

open for read
read lock
unlock
open upgrade
write lock

The write lock above is sent with a lock_seqid of 0, which is wrong.
RFC7530/16.10.5 says:

   o  In the case in which the state has been created and the [new
      lockowner] boolean is true, the server rejects the request with the
      error NFS4ERR_BAD_SEQID.  The only exception is where there is a
      retransmission of a previous request in which the boolean was
      true.  In this case, the lock_seqid will match the original
      request, and the response will reflect the final case, below.

Since the above is not a retransmission, knfsd is correct to reject this

call. This

quoted

patch fixes the open_sequence object to track the lock seqid and set it

correctly

quoted

in the LOCK request.

With this, LOCK24 passes against knfsd.

Cc: Frank Filz <redacted>
Fixes: 4299316fb357 (Add LOCK24 test case to test open uprgade/downgrade
scenario)
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.0/servertests/st_lock.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/nfs4.0/servertests/st_lock.py b/nfs4.0/servertests/st_lock.py

index

quoted

468672403ffe..9d650ab017b9 100644

--- a/nfs4.0/servertests/st_lock.py
+++ b/nfs4.0/servertests/st_lock.py

@@ -886,6 +886,7 @@ class open_sequence:
         self.client = client
         self.owner = owner
         self.lockowner = lockowner
+        self.lockseqid = 0
     def open(self, access):
         self.fh, self.stateid = self.client.create_confirm(self.owner,
 						access=access,

@@ -900,14 +901,17 @@ class open_sequence:
         self.client.close_file(self.owner, self.fh, self.stateid)
     def lock(self, type):
         res = self.client.lock_file(self.owner, self.fh, self.stateid,
-                    type=type, lockowner=self.lockowner)
+                                    type=type, lockowner=self.lockowner,
+                                    lockseqid=self.lockseqid)
         check(res)
         if res.status == NFS4_OK:
             self.lockstateid = res.lockid
+            self.lockseqid += 1
     def unlock(self):
         res = self.client.unlock_file(1, self.fh, self.lockstateid)
         if res.status == NFS4_OK:
             self.lockstateid = res.lockid
+            self.lockseqid += 1

 def testOpenUpgradeLock(t, env):
     """Try open, lock, open, downgrade, close
--

2.39.2

-- 
Jeff Layton [off-list ref]