dCache/jpnfs — pull request #2 by ato (2014-03-04 05:49:43+00:00)
Title: nfs4: verify seqid in OPEN_CONFIRM as per RFC

I've run into a problem with jpnfs with the NFS 4.0 (not 4.1) protocol. When opening and reading a file twice, the first time works fine, the second time we see these kernel messages on RHEL5 (Linux 2.6.18):
```
NFS: v4 server returned a bad sequence-id error!
NFS: v4 server returned a bad sequence-id error!
NFS: v4 server returned a bad sequence-id error!
NFS: v4 server returned a bad sequence-id error!
```

Or on RHEL6 (Linux 2.6.32) (with -o vers=4 not v4.1):
```
NFS: v4 server returned a bad sequence-id error on an unconfirmed sequence ffff881933529080!
NFS: v4 server 127.0.0.1 returned a bad sequence-id error!
```

After retrying a few thousand times the client gives up with:
```
cat: /mnt/tmp/foo: Remote I/O error
```

Investigating with a packet sniffer I see this sequence of calls the first time the file is read:

```
client OPEN {seqid: 0}
server OPEN reply NFS4_OK {stateid: {seqid: 1, data=...}}
client OPEN_CONFIRM {seqid: 1, stateid: {seqid: 1, data=...}}
server OPEN_CONFIRM reply NFS4_OK {stateid: {seqid: 2, data=...}}
```

But then the second time the file is read:
```
client OPEN {seqid: 0}
server OPEN reply NFS4_OK {stateid: {seqid: 2, data=...}}
client OPEN_CONFIRM {seqid: 1, stateid: {seqid: 2, data=...}}
server OPEN_CONFIRM reply NFS4ERR_BAD_SEQID
```

Then the client goes on to retry a large number of times:
```
client OPEN {seqid: 0}
server OPEN reply NFS4_OK {stateid: {seqid: 3, data=...}}
client OPEN_CONFIRM {seqid: 1, stateid: {seqid: 3, data=...}}
server OPEN_CONFIRM reply NFS4ERR_BAD_SEQID
client OPEN {seqid: 0}
server OPEN reply NFS4_OK {stateid: {seqid: 4, data=...}}
client OPEN_CONFIRM {seqid: 1, stateid: {seqid: 4, data=...}}
server OPEN_CONFIRM reply NFS4ERR_BAD_SEQID
.... and so on until stateid.seqid reaches 0x4001.
```

Looking at [OperationOPEN_CONFIRM.java](https://github.com/dCache/jpnfs/blob/f7fdc97e98827b738907afd6171bb7b9956c96f6/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java#L69) this is how seqid is verified:

```java
stateid4 stateid = _args.opopen_confirm.open_stateid;
NFS4Client client = context.getStateHandler().getClientIdByStateId(stateid);
...
NFS4State state = client.state(stateid);
if (state.stateid().seqid.value != _args.opopen_confirm.seqid.value.value) {
throw new ChimeraNFSException(nfsstat.NFSERR_BAD_SEQID, "bad seqid.");
}
```

If I'm interpreting the code correctly the server is therefore checking \_args.opopen_confirm.seqid against its remembered stateid.seqid (i.e. the stateid.seqid the server returned in the OPEN response, not the seqid the client sent in the OPEN request).

I checked RFC3530 and it says:
```
14.2.18. Operation 20: OPEN_CONFIRM - Confirm Open

SYNOPSIS

(cfh), seqid, stateid-> stateid

ARGUMENT

struct OPEN_CONFIRM4args {
/* CURRENT_FH: opened file */
stateid4 open_stateid;
seqid4 seqid;
};
...

DESCRIPTION

This operation is used to confirm the sequence id usage for the first
time that a open_owner is used by a client. The stateid returned
from the OPEN operation is used as the argument for this operation
along with the next sequence id for the open_owner. The sequence id
passed to the OPEN_CONFIRM must be 1 (one) greater than the seqid
passed to the OPEN operation from which the open_confirm value was
obtained. If the server receives an unexpected sequence id with
respect to the original open, then the server assumes that the client
will not confirm the original OPEN and all state associated with the
original OPEN is released by the server.
```

If I understand the RFC correctly, when making an OPEN_CONFIRM call, the client should echo the stateid that was returned by the server in the OPEN response. It should additionally send opopen_confirm.seqid as one plus the value the client sent for opopen.seqid.

However it seems then that the jpnfs server is verifying opopen_confirm.seqid against the wrong value. In fact I couldn't find anywhere in the server code where the seqid the client sends in the OPEN call (opopen.seqid) is used at all.

Is this a bug or am I misunderstanding the RFC? We're currently working around it with this patch but I'm unsure if it is the correct solution.