public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: Olaf Kirch <okir@suse.de>
Cc: NeilBrown <neilb@suse.de>, Andrew Morton <akpm@osdl.org>,
	nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [NFS] [PATCH 016 of 19] knfsd: match GRANTED_RES replies using	cookies
Date: Tue, 05 Sep 2006 12:12:30 -0400	[thread overview]
Message-ID: <1157472751.8238.4.camel@localhost> (raw)
In-Reply-To: <20060904090939.GC28400@suse.de>

On Mon, 2006-09-04 at 11:09 +0200, Olaf Kirch wrote:
> On Fri, Sep 01, 2006 at 12:03:38PM -0400, Trond Myklebust wrote:
> > Vetoed. The reason why we need the client's IP address as an argument
> > for nlmsvc_find_block() is 'cos the cookie value is unique to the
> > _client_ only.
> 
> In NLM, a cookie can be used to identify the asynchronous reply to the
> original request. Previously, there was a hack in the code that
> sends GRANT replies to reuse the original cookie from the client's
> LOCK request. The protocol spec explicitly says you must not rely
> on this behavior; the only reason I added this kludge was that some
> HPUX and SunOS versions did just that.
> 
> The down side of that kludge is that we are using a client-provided cookie
> in one of our calls - which means we keep our fingers crossed it doesn't
> collide with the cookie we generate ourselves. And to reduce the risk,
> we also check the client IP when searching the nlm_blocked list. But it
> is incorrect, and a kludge, and the longer I look at this code the
> more I'm amazed it hasn't blown up.
> 
> This patch changes the code so that the only cookies we ever use
> to look up something are those we generate ourselves, so they
> are globally unique. As a consequence, we can stop using the client's
> IP when searching the list.
> 
> > IOW: when we go searching a global list of blocks for a given cookie
> > value that was sent to us by a given client, then we want to know that
> > we're only searching through that particular client's blocks.
> 
> This is no longer true after applying this change.

Sorry, I missed that change. I see your point now.

> > A better way, BTW, would be to get rid of the global list nlm_blocked,
> > and just move the list of blocks into a field in the nlm_host for each
> > client.
> 
> Given an incoming NLM_GRANTED_RES, how can you look up the nlm_host
> and the pending NLM_GRANTED_MSG?
> The reply may not come from any IP address you know of. The whole
> reason for introducing this cookie nonsense in the NLM specification
> was that the RPC layer doesn't always give you enough clues to
> match a callback to the original message.

Right. The question is how many clients out there do still rely on the
current behaviour?

Looking at Brent's 'NFS Illustrated', I see that he notes that for
NLM_GRANTED, the cookie is "An opaque value that is normally the same as
the client sent in the LOCK request, though the client cannot depend on
it". Which sounds like weasel words for "some clients still do depend on
it".

Cheers
  Trond


  reply	other threads:[~2006-09-05 16:12 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-01  4:38 [PATCH 000 of 19] knfsd: lockd improvements NeilBrown
2006-09-01  4:38 ` [PATCH 001 of 19] knfsd: Hide use of lockd's h_monitored flag NeilBrown
2006-09-01  4:38 ` [PATCH 002 of 19] knfsd: Consolidate common code for statd->lockd notification NeilBrown
2006-09-01  4:38 ` [PATCH 003 of 19] knfsd: When looking up a lockd host, pass hostname & length NeilBrown
2006-09-01  4:38 ` [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle NeilBrown
2006-09-01  6:17   ` Andrew Morton
2006-09-01 23:48     ` Neil Brown
2006-09-01  6:20   ` Andrew Morton
2006-09-01 23:50     ` Neil Brown
2006-09-01 15:50   ` [NFS] " Trond Myklebust
2006-09-01 16:11     ` Olaf Kirch
2006-09-01 16:41       ` Trond Myklebust
2006-09-04  8:48         ` Olaf Kirch
2006-09-01  4:38 ` [PATCH 005 of 19] knfsd: Misc minor fixes, indentation changes NeilBrown
2006-09-01  4:38 ` [PATCH 006 of 19] knfsd: lockd: Make nlm_host_rebooted use the nsm_handle NeilBrown
2006-09-01  4:38 ` [PATCH 007 of 19] knfsd: lockd: make the nsm upcalls " NeilBrown
2006-09-01  4:38 ` [PATCH 008 of 19] knfsd: lockd: make the hash chains use a hlist_node NeilBrown
2006-09-01  4:38 ` [PATCH 009 of 19] knfsd: lockd: Change list of blocked list to list_node NeilBrown
2006-09-01  4:39 ` [PATCH 010 of 19] knfsd: Change nlm_file to use a hlist NeilBrown
2006-09-01  4:39 ` [PATCH 011 of 19] knfsd: lockd: make nlm_traverse_* more flexible NeilBrown
2006-09-01  4:39 ` [PATCH 012 of 19] knfsd: lockd: Add nlm_destroy_host NeilBrown
2006-09-01  4:39 ` [PATCH 013 of 19] knfsd: Simplify nlmsvc_invalidate_all NeilBrown
2006-09-01  4:39 ` [PATCH 014 of 19] knfsd: lockd: optionally use hostnames for identifying peers NeilBrown
2006-09-01  4:39 ` [PATCH 015 of 19] knfsd: make nlmclnt_next_cookie SMP safe NeilBrown
2006-09-01  4:39 ` [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies NeilBrown
2006-09-01 16:03   ` [NFS] " Trond Myklebust
2006-09-04  9:09     ` Olaf Kirch
2006-09-05 16:12       ` Trond Myklebust [this message]
2006-09-05 17:39         ` Olaf Kirch
2006-09-01  4:39 ` [PATCH 017 of 19] knfsd: Export nsm_local_state to user space via sysctl NeilBrown
2006-09-01  4:39 ` [PATCH 018 of 19] knfsd: lockd: fix use of h_nextrebind NeilBrown
2006-09-01 16:05   ` [NFS] " Trond Myklebust
2006-09-01  4:39 ` [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default NeilBrown
2006-09-01 13:25   ` [NFS] " Peter Staubach
2006-09-01 13:29   ` Peter Staubach
2006-09-01 13:47     ` Olaf Kirch
2006-09-01 15:31   ` Chuck Lever
2006-09-01 15:54     ` Olaf Kirch
2006-09-01 16:08       ` Chuck Lever
2006-09-01 16:34         ` Peter Staubach
2006-09-01 16:13     ` Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1157472751.8238.4.camel@localhost \
    --to=trond.myklebust@fys.uio.no \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=nfs@lists.sourceforge.net \
    --cc=okir@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox