On Fri, Jul 9, 2021 at 2:09 PM Solomon Boulos <boulos@waymo.com> wrote:

I had meant to add lots of comments to the FSAL API header about how all this handle-to-wire / wire-to-host / handle-to-key stuff works… (particularly the memory lifetimes and ownership) because I struggled with it when implementing the v3 proxy.

The giant comment in fsal_api.h is not bad, but the real action is in nfs_filehandle_mgmt.c (there are other callers of wire_to_host, including the mdcache just passing it through to the underlying FSAL, and an NFSv4 digest variant, but HandleMap_GetFH is being called on the "get me the v3 handle" path):

https://github.com/nfs-ganesha/nfs-ganesha/blob/2ce0989828a1d3fc1f5dc6164ae1620243ed3320/src/support/nfs_filehandle_mgmt.c#L88

which shows that the buffer passed into wire_to_host is NFS3_FHSIZE long (which makes sense, the function is trying to compute an fh3 as output). The comment there incorrectly says "no larger than NFS4_FHSIZE" but it should say NFS3_FHSIZE. (How'd you get NFS4_FHSIZE+2?)

Prior to commit 3016c4cee4, when the netbuf was being used, the fh_data buffer was allocated with that size, and then the netbuf's "maxlen" property was set to sizeof(fh_data), effectively setting the max length to NFS4_FHSIZE + 2. I don't really know the rationale for that, either then or now, I'm basically just restoring the lengths that were there before. I'm certainly not saying this is the correct way to do it, nor that it is safe - just that it appears to work.

I think Frank is correct, though - it's worth trying to figure out what values we should be using and make sure things are sane - just going with it because it appears to work isn't necessarily a good idea.

-Nick