Neil Brown wrote: > On Wednesday November 14, chuck.lever@oracle.com wrote: >> Trond Myklebust wrote: >>> On Wed, 2007-11-14 at 14:00 -0500, Chuck Lever wrote: >>>> That's correct. I'm just trying to understand why, historically, >>>> rq_daddr was just the 32-bit address and not a full sockaddr to begin >>>> with. There may be something we're missing, like "we didn't want to add >>>> another large field to this structure due to memory alignment or >>>> allocation efficiency concerns". :-) >>> Actually, rq_daddr by definition pretty much has to be of the same >>> address family as rq_addr, since they are the two endpoints for the same >>> socket. >>> >>> However I can't see where rq_addr is being initialised for UDP sockets. >>> That is sort of worrying given that it is used among other things by the >>> nfsd duplicate reply cache... >> That's the other half of my question. Why isn't nlmsvc_lookup_host >> using rq_addr (without the d)? > > Git is your friend. > > commit c98451bdb2f3e6d6cc1e03adad641e9497512b49 > Author: Frank van Maarseveen > Date: Mon Jul 9 22:25:29 2007 +0200 > > NLM: fix source address of callback to client > > Use the destination address of the original NLM request as the > source address in callbacks to the client. > > Signed-off-by: Frank van Maarseveen > Signed-off-by: Trond Myklebust Silly me. I had assumed that it had always been that way, and thus git would not be helpful (git's history truncates at 2.6.12). Unfortunately Frank's patch description doesn't explain *why* this change was made. I assume this fixes a bug with multi-homed servers? > Also, other places that need to interpret rq_daddr use: > > struct svc_sock *svsk = > container_of(rqstp->rq_xprt, struct svc_sock, sk_xprt); > switch (svsk->sk_sk->sk_family) { > case AF_INET:... > case AF_INET6: .... > > (see svcsock.c). > > Why wouldn't that work here? Should lockd be poking around in network layer data structures? I would argue "no" especially because that would make lockd transport-dependent. This approach wouldn't work at all for, say, RDMA transports. What do you think of changing the rq_daddr field to be a sockaddr_storage?