Re: Use of delayed request information in nlmsvc_lookup_host

[Chuck Lever <chuck.lever@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

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 <frankvm@frankvm.com>
> 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 <frankvm@frankvm.com>
>     Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

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?

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 259 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs