Re: [PATCH 2/2] NFSv4: encode_attrs should not backfill the bitmap and attribute length

[Chuck Lever <chuck.lever@oracle.com>]

On Jul 23, 2013, at 1:00 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Tue, 2013-07-23 at 17:59 +0200, Andre Heider wrote:
>> Trond,
>> 
>> On Wed, Jul 17, 2013 at 11:59 PM, Trond Myklebust
>> <Trond.Myklebust@netapp.com> wrote:
>>> The attribute length is already calculated in advance. There is no
>>> reason why we cannot calculate the bitmap in advance too so that
>>> we don't have to play pointer games.
>> 
>> I'm sorry to report that this patch seems to be more than just a cleanup.
>> 
>> I just tested 3.11-rc2 against my FreeBSD server, and with just patch
>> 1/2 (as in -rc2) I still get the failure upon `touch`. It fails with
>> or without Rick's server patch.
>> 
>> Applying this one on top of -rc2 fixes it.
> 
> How about the attached instead of the cleanup?

Even with this patch applied, cthon basic tests fail immediately for me.  The mkdir to create the test directory fails with EIO.

The wire trace shows that the CREATE operation is malformed: the client sends a length of 8 for the 4-octet mode attribute value at the end of the operation.

This causes the server to skip over the following GETFH operation -- it thinks the client has sent 3 operations, when the client told it to expect 4 in this compound.

Here:

1064         *p++ = cpu_to_be32(bmval_len);
1065         q = p;
1066         /* Skip bitmap entries + attrlen */
1067         p += bmval_len + 1;

You've skipped over the 4-octet attrlen field, but here:

1125         *q++ = htonl(bmval0);
1126         *q++ = htonl(bmval1);
1127         if (bmval_len == 3)
1128                 *q++ = htonl(bmval2);
1129         len = (char *)p - (char *)q;
1130         *q = htonl(len);

have you failed to take the attrlen field into account when computing the length of the attribute values?

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com