2007-12-22 04:15:13

by Jeff Garzik

[permalink] [raw]
Subject: Linux client misses lack of open-confirm?

While debugging my NFS server, I may have caught a Linux client bug.

My server is currently buggy, in that, it never sets the
OPEN4_RESULT_CONFIRM bit after an OPEN with a new owner. Shockingly, I
can pass ~530 pynfs tests, fsx-linux [Linux v4 client], and build a
kernel [Linux v4 client] even with such brokenness. ;-)

Anyway, the Linux NFSv4 client (2.6.24-rc6) seems quite happy with this
state of affairs, right until CLOSE time, when it passes "seqid + 2" to
my server rather than the expected "seqid + 1".

Though I am quite happy that Linux managed to workaround my stupid
server and store data successfully _anyway_, I thought it was worth
commenting. I was assuming either

a) Linux would notice the lack of OPEN4_RESULT_CONFIRM and
complain accordingly, or,

b) Linux would generate a correct seqid, taking into account
the fact that it did not issue OPEN_CONFIRM.

As you can see from the wireshark-0.99.7-2.fc8 binary dump at

http://gtf.org/garzik/misc/dump.bz2 (33k compressed)

we see many examples of

C: OPEN (seqid == 0)
S: NFS4_OK

C: [perhaps some intervening READ or WRITE or *ATTR]
S: [replies as expected]

C: CLOSE (seqid == 2)
S: NFS4ERR_BAD_SEQID

If you feel this behavior is fine given a broken server, that's cool...
I just figured I would post in case somebody cared about this data point.

Jeff


P.S. I really really hate stateid/seqids at this point. RFC
nonwithstanding, they are basically undocumented. I am reduced to
poking through NFSv4 WG archives and Linux kernel code to find out what
my server should be doing. pynfs is no help here, either.


2007-12-22 15:27:34

by Trond Myklebust

[permalink] [raw]
Subject: Re: Linux client misses lack of open-confirm?


On Fri, 2007-12-21 at 23:15 -0500, Jeff Garzik wrote:
> While debugging my NFS server, I may have caught a Linux client bug.
>
> My server is currently buggy, in that, it never sets the
> OPEN4_RESULT_CONFIRM bit after an OPEN with a new owner. Shockingly, I
> can pass ~530 pynfs tests, fsx-linux [Linux v4 client], and build a
> kernel [Linux v4 client] even with such brokenness. ;-)
>
> Anyway, the Linux NFSv4 client (2.6.24-rc6) seems quite happy with this
> state of affairs, right until CLOSE time, when it passes "seqid + 2" to
> my server rather than the expected "seqid + 1".
>
> Though I am quite happy that Linux managed to workaround my stupid
> server and store data successfully _anyway_, I thought it was worth
> commenting. I was assuming either
>
> a) Linux would notice the lack of OPEN4_RESULT_CONFIRM and
> complain accordingly, or,
>
> b) Linux would generate a correct seqid, taking into account
> the fact that it did not issue OPEN_CONFIRM.
>
> As you can see from the wireshark-0.99.7-2.fc8 binary dump at
>
> http://gtf.org/garzik/misc/dump.bz2 (33k compressed)
>
> we see many examples of
>
> C: OPEN (seqid == 0)
> S: NFS4_OK
>
> C: [perhaps some intervening READ or WRITE or *ATTR]
> S: [replies as expected]
>
> C: CLOSE (seqid == 2)
> S: NFS4ERR_BAD_SEQID
>
> If you feel this behavior is fine given a broken server, that's cool...
> I just figured I would post in case somebody cared about this data point.

Hmm... That's not good. It is perfectly legal for a server to not
request OPEN4_RESULT_CONFIRM (although it is probably not a very good
idea), and the client should be able to cope with that.

I'll have a look at what is going on there.

> P.S. I really really hate stateid/seqids at this point. RFC
> nonwithstanding, they are basically undocumented. I am reduced to
> poking through NFSv4 WG archives and Linux kernel code to find out what
> my server should be doing. pynfs is no help here, either.

The primary function of seqids is to allow the server to distinguish
replayed non-idempotent RPC requests from new requests, so their
properties are really quite simple:

* If the seqid presented by the client is in sequence, then the
server is supposed to handle the request.
* If the seqid matches that of the last request, then the server
is supposed to replay the reply.
* If the seqid is completely out of sequence, then the server
should return the BAD_SEQID error.

As for stateids, their purpose is to allow the server to figure out to
which client it is talking, and to track what state the client thinks it
is holding. Apart from the seqid field (which is there in order to track
the ordering of OPEN requests), a stateid is an opaque structure.
The only really important requirement here is that you need to be able
to distinguish stale state from valid state so that you can fence off
RPC requests that refer to stale locks.

Cheers
Trond


2007-12-23 02:05:47

by Jeff Garzik

[permalink] [raw]
Subject: Re: Linux client misses lack of open-confirm?

Trond Myklebust wrote:
> On Fri, 2007-12-21 at 23:15 -0500, Jeff Garzik wrote:
>> While debugging my NFS server, I may have caught a Linux client bug.
>>
>> My server is currently buggy, in that, it never sets the
>> OPEN4_RESULT_CONFIRM bit after an OPEN with a new owner. Shockingly, I
>> can pass ~530 pynfs tests, fsx-linux [Linux v4 client], and build a
>> kernel [Linux v4 client] even with such brokenness. ;-)
>>
>> Anyway, the Linux NFSv4 client (2.6.24-rc6) seems quite happy with this
>> state of affairs, right until CLOSE time, when it passes "seqid + 2" to
>> my server rather than the expected "seqid + 1".
>>
>> Though I am quite happy that Linux managed to workaround my stupid
>> server and store data successfully _anyway_, I thought it was worth
>> commenting. I was assuming either
>>
>> a) Linux would notice the lack of OPEN4_RESULT_CONFIRM and
>> complain accordingly, or,
>>
>> b) Linux would generate a correct seqid, taking into account
>> the fact that it did not issue OPEN_CONFIRM.
>>
>> As you can see from the wireshark-0.99.7-2.fc8 binary dump at
>>
>> http://gtf.org/garzik/misc/dump.bz2 (33k compressed)
>>
>> we see many examples of
>>
>> C: OPEN (seqid == 0)
>> S: NFS4_OK
>>
>> C: [perhaps some intervening READ or WRITE or *ATTR]
>> S: [replies as expected]
>>
>> C: CLOSE (seqid == 2)
>> S: NFS4ERR_BAD_SEQID
>>
>> If you feel this behavior is fine given a broken server, that's cool...
>> I just figured I would post in case somebody cared about this data point.
>
> Hmm... That's not good. It is perfectly legal for a server to not
> request OPEN4_RESULT_CONFIRM (although it is probably not a very good
> idea), and the client should be able to cope with that.

If you want to reproduce, my server is open (though largely unannounced,
since its still in initial coding phase):
git://git.kernel.org/pub/scm/daemon/nfs/nfs4-ram.git

Commit b3f602203ab023aa559c4db5449448b9c7044f36 (HEAD~2 currently) can
reproduce the behavior nicely.

The server is currently a zero-configuration-file RAM server, so its
easy to test: just build and run (./nfs4_ramd). It binds to port 2049
with an empty filesystem, each time it is started. (--help for
alternate port or other options)


> I'll have a look at what is going on there.
>
>> P.S. I really really hate stateid/seqids at this point. RFC
>> nonwithstanding, they are basically undocumented. I am reduced to
>> poking through NFSv4 WG archives and Linux kernel code to find out what
>> my server should be doing. pynfs is no help here, either.
>
> The primary function of seqids is to allow the server to distinguish
> replayed non-idempotent RPC requests from new requests, so their
> properties are really quite simple:
>
> * If the seqid presented by the client is in sequence, then the
> server is supposed to handle the request.
> * If the seqid matches that of the last request, then the server
> is supposed to replay the reply.
> * If the seqid is completely out of sequence, then the server
> should return the BAD_SEQID error.
>
> As for stateids, their purpose is to allow the server to figure out to
> which client it is talking, and to track what state the client thinks it
> is holding. Apart from the seqid field (which is there in order to track
> the ordering of OPEN requests), a stateid is an opaque structure.
> The only really important requirement here is that you need to be able
> to distinguish stale state from valid state so that you can fence off
> RPC requests that refer to stale locks.

Yeah I figured out the purpose pretty quickly. The thing I missed was
that the seqid is per-lockowner, and not per-openfile. No surprise
things got weird, when I coded a server following that logic...

Plus there are a ton of undocumented -ordering- constraints you must
follow, with regards to validating seqid/stateid and then returning the
correct error.

Thanks for the response! Hope my buggy server helps you track down
client problems ;-)

Jeff