On Wed, 2007-10-24 at 23:22 +0100, David Howells wrote: > Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote: > > > > > Use the credential struct as the unique lookup key for an rpc_cred. > > > > > > That's not good enough. A cred struct may map to several rpc_creds as I > > > mentioned above. I suppose the nfs_client struct address could be added > > > to the lookup key. > > > > Huh? The RPC cred lookup function takes a pointer to an rpc_auth struct, > > which should already be tied to an rpc_client. > > So where do you get the rpc_auth struct from? > > How about we come at it this way: nfs_readpage() is changed to have a struct > cred * instead of a struct file *. How do we get the appropriate rpc_auth > struct (assuming it isn't held in the cred struct)? We need something from > the mapping to use as part of the lookup key, be that an nfs_client or an > rpc_client. You should be using the rpc_auth that is cached in the rpc_client: NFS_CLIENT(inode)->cl_auth W.r.t. the nfs_find_open_context(), btw, there is no reason why we couldn't change that to take a generic cred as its argument. > > In the case where keyrings are enabled and a key changes, then we should > > revalidate the rpc_cred and either dump it (replacing it with a new > > cred) or reuse it. We can only cache so much garbage... > > You can't necessarily just arbitrarily ditch an rpc_cred just because the key > it was derived from has been replaced. Someone may have a file open that's > using it. Unless, of course, you accept that it's okay to change the > credential that an open file is using... If we're happy to do that, that makes > my life a lot easier. > > Also, it occurs to me that if a cred struct acts as a lookup key to find cached > rpc_creds, what controls the lifetime of the latter with respect to the former? There is automatic garbage collection of the latter. If they are unused, they may be culled. > > Possibly, but I'm not accepting any single patch that completely > > rewrites the basic security model of NFS in one fell swoop. If you want > > to change the cred->rpc_cred mapping, then that will be done in > > incremental patches. > > Your first step is therefore to get the generic cred working with the > > _existing_ RPC security model. > > I would never have thought of that. Just making things crystal clear. I did also consult with a number of members of the NFS development team, and so far they all agree that we want to see the generic cred go in first before we start working on rewrites to the RPC auth code. We have yet to see any numbers comparing the current vs. cred performance. > However, there are two points you should consider: firstly, I've a patch that's > a quarter of a Meg just to add struct cred pointers into NFS so that the > appropriate credentials get down into the SunRPC layer rather than using > current's credentials; and secondly, I want some idea of where I'm going. If > you'd rather, I can just drop the credentials into the NFS VFS methods and > leave it for you to make work. The latter sounds good for the moment. There are several people who are working on the rpc auth code, and thus have an interest here, so it will take some time to work out a consensus for what needs to be done. Trond - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
