Hi David- On Nov 17, 2014, at 10:08 AM, David Howells <dhowells@xxxxxxxxxx> wrote: > I'm not sure this patch actually solves your problem. > >> request_key_and_link() depends on getting an -EAGAIN result code to know >> when to perform an upcall to refresh an expired key. > > request_key_and_link() should return EKEYEXPIRED if it meets an expired key > until that key gets gc’d. That’s what the documenting comment says, but... nfs_idmap_request_key() considers any error return from request_key() to be a reason to fall back to using the legacy (non-keyring-based) ID mapper. nfs_idmap_get_key() considers any error return from key_validate() to be a permanent error. IOW the NFS idmapper logic depends on getting a valid key back from request_key() ie, one that has not expired, or one that was just refreshed. Getting back an expired key or getting -EKEYEXPIRED is unexpected. Thus it appears to be an API contract change. At least, the documenting comment in front of request_key_and_link() does not seem to match the current design of the NFS idmapper code. > What we lack is that bit to upcall to refresh the expired key. Currently request_key_and_link() expects -EAGAIN from search_process_keyrings() to know when it should upcall. But, maybe something else is needed there instead. > /sbin/request-key can support it - the first column has 'create' for key > creation and can hold other values for updating a key and KEYCTL_UPDATE can be > allowed to unexpire a key. > > Possibly I should be only returning EKEYEXPIRED if the key instantiation was > rejected so and simply invalidate the key if it's in-memory expiration > occurs. Making this so will cause failures in the testsuite, but I think > that's okay. > > Another option is to allow keys to be specifically marked at > immediate-gc-on-expire such that you never see them in the expired state > unless you're holding a ref on one inside the kernel. As long as request_key_and_link() can’t return an expired key. Another option is to alter the NFS idmapper logic to match the current keyring API contract. I didn’t choose that option before because it seemed like it wasn’t the intention to change that contract in 3.13. I'll hold off on testing the patches you sent over the weekend. -- Chuck Lever chuck[dot]lever[at]oracle[dot]com -- 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
