On Dec 15, 2010, at 10:29 AM, Benny Halevy wrote:
> On 2010-12-15 16:11, Fred Isaman wrote:
>>
>> On Dec 15, 2010, at 8:57 AM, Benny Halevy wrote:
>>
>>> On 2010-12-10 03:22, Fred Isaman wrote:
>>>> It was checking that at least one known bit was set. It needs to check
>>>> no unknown bit was set. From RFC5661, section 20.6.3:
>>>>
>>>> When a bit is set in the type mask that corresponds to an undefined
>>>> type of recallable object, NFS4ERR_INVAL MUST be returned.
>>>>
>>>> Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx>
>>>> ---
>>>> fs/nfs/callback.h | 1 +
>>>> fs/nfs/callback_proc.c | 27 ++++-----------------------
>>>> 2 files changed, 5 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
>>>> index b16dd1f..616c5c1 100644
>>>> --- a/fs/nfs/callback.h
>>>> +++ b/fs/nfs/callback.h
>>>> @@ -126,6 +126,7 @@ extern int nfs41_validate_delegation_stateid(struct nfs_delegation *delegation,
>>>> #define RCA4_TYPE_MASK_OBJ_LAYOUT_MAX 9
>>>> #define RCA4_TYPE_MASK_OTHER_LAYOUT_MIN 12
>>>> #define RCA4_TYPE_MASK_OTHER_LAYOUT_MAX 15
>>>> +#define RCA4_TYPE_MASK_ALL 0xf31f
>>>>
>>>> struct cb_recallanyargs {
>>>> struct sockaddr *craa_addr;
>>>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>>>> index 61b3c66..d4aec46 100644
>>>> --- a/fs/nfs/callback_proc.c
>>>> +++ b/fs/nfs/callback_proc.c
>>>> @@ -661,28 +661,10 @@ out_putclient:
>>>> goto out;
>>>> }
>>>>
>>>> -static inline bool
>>>> -validate_bitmap_values(const unsigned long *mask)
>>>> +static bool
>>>> +validate_bitmap_values(unsigned long mask)
>>>> {
>>>> - int i;
>>>> -
>>>> - if (*mask == 0)
>>>> - return true;
>>>> - if (test_bit(RCA4_TYPE_MASK_RDATA_DLG, mask) ||
>>>> - test_bit(RCA4_TYPE_MASK_WDATA_DLG, mask) ||
>>>> - test_bit(RCA4_TYPE_MASK_DIR_DLG, mask) ||
>>>> - test_bit(RCA4_TYPE_MASK_FILE_LAYOUT, mask) ||
>>>> - test_bit(RCA4_TYPE_MASK_BLK_LAYOUT, mask))
>>>> - return true;
>>>> - for (i = RCA4_TYPE_MASK_OBJ_LAYOUT_MIN;
>>>> - i <= RCA4_TYPE_MASK_OBJ_LAYOUT_MAX; i++)
>>>> - if (test_bit(i, mask))
>>>> - return true;
>>>> - for (i = RCA4_TYPE_MASK_OTHER_LAYOUT_MIN;
>>>> - i <= RCA4_TYPE_MASK_OTHER_LAYOUT_MAX; i++)
>>>> - if (test_bit(i, mask))
>>>> - return true;
>>>> - return false;
>>>> + return mask & ~RCA4_TYPE_MASK_ALL;
>>>
>>> Hmm, shouldn't that be
>>> return (mask & ~RCA4_TYPE_MASK_ALL) == 0;
>>>
>>> Benny
>>>
>>
>> Yes, you are right.
>
> OK. This is fixed in my branch to be released asap.
Thanks. I have a bunch more minor code cleanups that I'll send once I see what you have, unless you want them immediately. I'll also have a rebase of the pnfs-submit branch with the wave2 patches pushed down to the bottom shortly after I see your branch.
> I've reverted large parts of this patchset in the post-submit stream
> to restore layoutcommit and layoutreturn, but not their embedding in
> the CLOSE compound. I also kept the cleanups and bug fixes.
> I'll send out the post-submit when it's ready.
> Some more work will be required to restore the original patches
> author and signoffees.
>
> This is the list as of now:
>
> pick af44531 Revert "pnfs-submit: wave2: remove forgotten layoutreturn struct definitions"
> pick c465549 Revert "pnfs-submit: Turn off layoutcommits"
> pick 0f4ba67 Revert "pnfs-submit: wave2: remove all LAYOUTRETURN code"
> pick 486db47 Revert "pnfs-submit: wave2: Remove LAYOUTRETURN from return on close"
>
> pick 484c935 FIXME: roc should return layout on last close
> (This patch just adds a FIXME comment.)
>
The above looks good.
> pick 8698772 Revert "pnfs-submit: wave2: remove cl_layoutrecalls list"
> pick 263879b Revert "pnfs-submit: wave2: Pull out all recall initiated LAYOUTRETURNS"
> pick 693765f Revert "pnfs-submit: wave2: Don't wait in layoutget"
Just note the Trond was highly resistant to the above.
> pick de56e11 Revert "pnfs-submit: wave2: check that partial LAYOUTGET return is ignored"
>
We need some sort of check that we got what we asked for, given that the xdr code can chop the servers reply.
Fred
> Anything else you had in mind?
>
> Benny
>
>>
>> Fred
>>
>>>> }
>>>>
>>>> __be32 nfs4_callback_recallany(struct cb_recallanyargs *args, void *dummy,
>>>> @@ -702,8 +684,7 @@ __be32 nfs4_callback_recallany(struct cb_recallanyargs *args, void *dummy,
>>>> rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR));
>>>>
>>>> status = cpu_to_be32(NFS4ERR_INVAL);
>>>> - if (!validate_bitmap_values((const unsigned long *)
>>>> - &args->craa_type_mask))
>>>> + if (!validate_bitmap_values(args->craa_type_mask))
>>>> goto out;
>>>>
>>>> status = cpu_to_be32(NFS4_OK);
>>
>> --
>> 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
--
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
