Re: [PATCH] sunrpc: include gid in the rpc_cred_cache hash

[Frank Sorenson <sorenson@redhat.com>]

----- Original Message -----
> From: "Trond Myklebust" <trondmy@primarydata.com>
> To: "Frank Sorenson" <sorenson@redhat.com>
> Cc: "List Linux NFS Mailing" <linux-nfs@vger.kernel.org>
> Sent: Friday, September 16, 2016 4:37:39 PM
> Subject: Re: [PATCH] sunrpc: include gid in the rpc_cred_cache hash

> > +rpcauth_hash_acred(struct auth_cred *acred, unsigned int hashbits)
> > +{
> > +	return hash_64(from_kgid(&init_user_ns, acred->gid) |
> > +		(from_kuid(&init_user_ns, acred->uid) << (sizeof(gid_t) * 8)),
> > +		hashbits);
> > +}
> > +

> NACK. The choice of only using the uid when hashing was deliberate;
> RPCSEC_GSS is keyed only on the uid…
> If you want to do this in order to accelerate AUTH_SYS lookups, then you need
> to push the hashing down to the auth flavour ops.

I recognize that RPCSEC_GSS only uses the uid as a key.  However, RPCSEC_GSS
calls rpcauth_lookup_credcache with an auth_cred, just like AUTH_SYS, only with
the gid set to 0.  Including the gid in the hash has no effect on RPCSEC_GSS;
if the function is flipped to shift the gid instead of the uid, it even hashes
to the same result as it did previously.

Adding a shift and bitwise OR to the hash is more straightforward and
efficient than adding the logic to provide a per-auth flavour hash op that
differs only in that it doesn't shift and OR a 0 value.

Or are there additional benefits to be gained from each having its own hash
function?


Thanks,

Frank
--
Frank Sorenson
sorenson@redhat.com
Senior Software Maintenance Engineer
Global Support Services - filesystems
Red Hat