On Jul. 16, 2008, 0:57 +0300, Trond Myklebust <trond.myklebust@xxxxxxxxxx> wrote:
> On Thu, 2008-07-03 at 20:49 +0300, Benny Halevy wrote:
>> Trond, following our conversation during the Connectathon
>> I reduced this patch as much as possible and restricted the
>> use of the compound header status to cases where op_hdr
>> decoding hit an error.
>>
>> This is needed for nfs41 for graceful fallback when trying
>> to mount a 4.0 server with 4.1. In this case the server
>> returns no ops and the hdr status is set to
>> NFS4ERR_MINOR_VERS_MISMATCH.
>>
>> Please consider these patches:
>>
>> [PATCH 1/2] nfs: return nfs4 compound header status on op header decoding error
>
> No, this patch doesn't look right either. If we overrun the end of the
> reply buffer, then xdr_inline_decode() will return a NULL pointer, so
> you should never hit the your (opnum != expected) case.
In patch 1, this is supposed to be handled like this:
decode_compound_hdr:
+ xdr->status = hdr->status;
decode_op_hdr:
+ p = xdr_inline_decode(xdr, 8);
+ if (unlikely(!p)) {
...
+ goto err;
+ }
+err:
+ if (xdr->status != NFS_OK)
+ return nfs4_stat_to_errno(xdr->status);
>
> So given that your concern is primarily the case where nops==0, why
> don't you just add that particular case to decode_compound_hdr?
>
> IOW: something like
>
>
> @@static int decode_compound_hdr(
> p += XDR_QUADLEN(hdr->taglen);
> READ32(hdr->nops);
> + if (hdr->nops < 1)
> + return nfs4_stat_to_errno(hdr->status);
> return 0;
> }
>
This certainly provides a shortcut for the nops==0 case.
However, it doesn't solve the OP_ILLEGAL case all other
cases where xdr_inline_decode failed or opnum != expected,
we can easily handle the OP_ILLEGAL case explicitly in
decode_op_hdr. Are you ok with the approach of
carrying the hdr.status in xdr->status for decode_op_hdr
use, or do you rather prefer to leave things as they are
for the invalid cases and explicitly handle only the
nops==0 and OP_ILLEGAL cases?
Benny
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
