Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Feb 2021 17:47:14 -0500
From:   "J. Bruce Fields" <bfields@redhat.com>
To:     Calum Mackay <calum.mackay@oracle.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [RFC] pynfs: add courteous server tests
Message-ID: <YCxLcm8FFFWzZELr@pick.fieldses.org>
References: <47d31c15-7467-6abb-9e62-96ffca1c6ec0@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <47d31c15-7467-6abb-9e62-96ffca1c6ec0@oracle.com>
Precedence: bulk

On Tue, Feb 16, 2021 at 10:04:05PM +0000, Calum Mackay wrote:
> hi Bruce,
> 
> At Chuck's suggestion, I've added an initial PyNFS test to aid work on a
> courteous server. A simple test, along the lines you indicated, that locks a
> file, waits twice the lease period, and tries to unlock:
> 
> OK -> PASS (courteous server)
> BADSESSION -> WARNING (discourteous server)
> 
> 
> Before sending my patch, Chuck asked me to add the second test you
> suggested:
> 
> 	- A second test creates a new client, acquires a file lock, and
> 	  waits two lease periods.  Then creates a second client, which
> 	  attempts to acquire the lock.  The second client should
> 	  succeed.
> 
> 
> 
> This doesn't seem to differentiate between these three cases:
> 
> 1. a discourteous server, which invalidates the client 1 state, and frees
> all client 1's locks, upon lease expiry, then allows client 2 to lock the
> file. The above test spec would result in a PASS for a discourteous server,
> which doesn't seem right.

Apologies for the confusing suggestion.  I think all I really wanted to
verify was that the server will grant the lock to a second client after
a lease period has gone by.

That's a simple test that *any* server (courteous or discourteous)
should pass.  (We probably do have a test for that at least in the 4.0
case.  It'd be nice to have one in the 4.1 case.  Maybe just look into
porting some 4.0 tests over to 4.1.)

Anyway, it seems like a simple thing that would be useful to verify
while doing courteous server implementation.  I mean, we want to make
sure we don't accidentally implement a "courteous" server that works by
just never dropping any client's state ever.

> 2. a broad-grained courteous server, which invalidates the client 1 state,
> and frees all client 1's locks, because of conflicting access from client 2
> (after client 1's lease expiry), who is then granted the lock. A PASS here
> would be correct.
> 
> 3. a fine-grained courteous server, which persists the session, but revokes
> that particular client 1 lock, because of conflicting access from client 2
> (after client 1's lease expiry), who is granted the lock. A PASS here would
> be correct.

> If I've read it right, the test could differentiate between cases 2) and 3),
> by having client 1 try to unlock, after client 2 successfully locks, where
> client 1 will see either BADSESSION (case 2) or SOME_STATE_REVOKED / EXPIRED
> (case 3).

Sounds like a good idea to test those two cases.

> But we don't need to differentiate cases 2) and 3), since a PASS
> would be correct in either case.

But, yes, they're both "courteous" cases.

I do wish sometimes that we had states other than "PASS/FAIL/WARN";
sometimes you want to know stuff about a server that isn't just whether
it's "right" or not.

Bit it's not really a big deal.  Note if you leave the "all" flag off a
test, it won't be run by default.  And if a test flagged "courteous" but
not "all" fails on a correct but uncourteous server, that's probably
fine.

I think it'd also be fine to WARN about anything short of the
finest-grained possible behavior.  You can give more details in the
wording of the warning.

> However that won't differentiate between cases 1) and 2), where client 1
> will see BADSESSION in each case. Yet case 1) ought to result in a WARNING,
> and case 2) in a PASS?

We could also do something like:

	client 1 acquires two different locks.
	wait a lease period or two
	client 2 attempts to acquire one of the locks.
	client 1 attempts to unlock the other one.

And that'd be another way to test whether we have a coarse- or fine-
grained courteous server.

My test suggestions were very off-the-cuff, if you have ideas then go
for it.

--b.