Re: [PATCH 13/17] NFS: Client implementation of Labeled-NFS

[Steve Dickson <SteveD@redhat.com>]

On 08/05/13 14:31, Myklebust, Trond wrote:
> On Wed, 2013-05-08 at 14:27 -0400, Steve Dickson wrote:
>>
>> On 08/05/13 14:07, Myklebust, Trond wrote:
>>> On Wed, 2013-05-08 at 13:39 -0400, Steve Dickson wrote:
>>>>
>>>> On 08/05/13 12:43, Myklebust, Trond wrote:
>>>>> On Wed, 2013-05-08 at 12:39 -0400, Steve Dickson wrote:
>>>>>>
>>>>>> On 01/05/13 15:03, Myklebust, Trond wrote:
>>>>>>>> @@ -2409,10 +2468,26 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>>>>>>>>>  			server->caps |= NFS_CAP_CTIME;
>>>>>>>>>  		if (res.attr_bitmask[1] & FATTR4_WORD1_TIME_MODIFY)
>>>>>>>>>  			server->caps |= NFS_CAP_MTIME;
>>>>>>>>> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL
>>>>>>>>> +		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
>>>>>>>>> +			server->caps |= NFS_CAP_SECURITY_LABEL;
>>>>>>>>> +#endif
>>>>>>>>> +		memcpy(server->attr_bitmask_nl, res.attr_bitmask, 
>>>>>>>>> +				sizeof(server->attr_bitmask));
>>>>>>>>> +
>>>>>>>>> +		if (server->caps & NFS_CAP_SECURITY_LABEL)
>>>>>>>>> +			server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
>>>>>>>>>  
>>>>>>>>>  		memcpy(server->cache_consistency_bitmask, res.attr_bitmask, sizeof(server->cache_consistency_bitmask));
>>>>>>>>>  		server->cache_consistency_bitmask[0] &= FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE;
>>>>>>>>> -		server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY;
>>>>>>>>> +		server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA |
>>>>>>>>> +							FATTR4_WORD1_TIME_MODIFY;
>>>>>>>>> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL
>>>>>>>>> +		server->cache_consistency_bitmask[2] &= FATTR4_WORD2_SECURITY_LABEL;
>>>>>>> Why? How is the security label relevant to cache consistency?
>>>>>> Its used to the set label bit in the GETATTR that goes out with ACCESS compound.
>>>>>
>>>>> The GETATTR that goes out with ACCESS is only there in order to get the
>>>>> change attribute so that we know when to invalidate the access cache. It
>>>>> is _only_ for cache consistency.
>>>>>
>>>>> Why do we need to fetch the label too?
>>>>>
>>>> I think I answer this in the other thread but in short 
>>>> access updates the inode and so it appears the goal 
>>>> is to synchronize inode updates and label updates.
>>>
>>> Those are not inode updates.
>> Ah... OK... but they all end up calling nfs_refresh_inode() with 
>> the valid label pointer... So there is an effort to keep the 
>> inode attribute cache updates synchronized with label updates... 
>>
>> So I guess the question is that needed... Is the setting of 
>> the label in nfs_fhget() and/or _nfs4_do_open() good enough.
> 
> Until someone comes up with a different cache consistency model, then
> I'd say yes. The only other case that comes to mind, is when our client
> actively changes the label...
On the server side as well, correct? What stops a process on the server side
from changing the label? 

I'm thinking that is reason the label was put in all those GETATTRs  

steved.