lkml.org 
[lkml]   [2006]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [NFS] [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies
    From
    Date
    On Fri, 2006-09-01 at 14:39 +1000, NeilBrown wrote:
    > From: Olaf Kirch <okir@suse.de>
    >
    > When we send a GRANTED_MSG call, we current copy the NLM cookie
    > provided in the original LOCK call - because in 1996, some broken
    > clients seemed to rely on this bug. However, this means the cookies
    > are not unique, so that when the client's GRANTED_RES message comes
    > back, we cannot simply match it based on the cookie, but have to
    > use the client's IP address in addition. Which breaks when you have
    > a multi-homed NFS client.
    >
    > The X/Open spec explicitly mentions that clients should not expect the
    > same cookie; so one may hope that any clients that were broken in 1996
    > have either been fixed or rendered obsolete.

    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.

    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.

    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.

    > Signed-off-by: Olaf Kirch <okir@suse.de>
    > Signed-off-by: Neil Brown <neilb@suse.de>
    >
    > ### Diffstat output
    > ./fs/lockd/svc4proc.c | 2 +-
    > ./fs/lockd/svclock.c | 24 +++++++++++++-----------
    > ./fs/lockd/svcproc.c | 2 +-
    > ./include/linux/lockd/lockd.h | 2 +-
    > 4 files changed, 16 insertions(+), 14 deletions(-)
    >
    > diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
    > --- .prev/fs/lockd/svc4proc.c 2006-09-01 12:11:01.000000000 +1000
    > +++ ./fs/lockd/svc4proc.c 2006-09-01 12:17:21.000000000 +1000
    > @@ -455,7 +455,7 @@ nlm4svc_proc_granted_res(struct svc_rqst
    >
    > dprintk("lockd: GRANTED_RES called\n");
    >
    > - nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
    > + nlmsvc_grant_reply(&argp->cookie, argp->status);
    > return rpc_success;
    > }
    >
    >
    > diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
    > --- .prev/fs/lockd/svclock.c 2006-09-01 12:11:01.000000000 +1000
    > +++ ./fs/lockd/svclock.c 2006-09-01 12:17:21.000000000 +1000
    > @@ -139,19 +139,19 @@ static inline int nlm_cookie_match(struc
    > * Find a block with a given NLM cookie.
    > */
    > static inline struct nlm_block *
    > -nlmsvc_find_block(struct nlm_cookie *cookie, struct sockaddr_in *sin)
    > +nlmsvc_find_block(struct nlm_cookie *cookie)
    > {
    > struct nlm_block *block;
    >
    > list_for_each_entry(block, &nlm_blocked, b_list) {
    > - if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie)
    > - && nlm_cmp_addr(sin, &block->b_host->h_addr))
    > + if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie))
    > goto found;
    > }
    >
    > return NULL;
    >
    > found:
    > + dprintk("nlmsvc_find_block(%s): block=%p\n", nlmdbg_cookie2a(cookie), block);
    > kref_get(&block->b_count);
    > return block;
    > }
    > @@ -165,6 +165,11 @@ found:
    > * request, but (as I found out later) that's because some implementations
    > * do just this. Never mind the standards comittees, they support our
    > * logging industries.
    > + *
    > + * 10 years later: I hope we can safely ignore these old and broken
    > + * clients by now. Let's fix this so we can uniquely identify an incoming
    > + * GRANTED_RES message by cookie, without having to rely on the client's IP
    > + * address. --okir
    > */
    > static inline struct nlm_block *
    > nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
    > @@ -197,7 +202,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
    > /* Set notifier function for VFS, and init args */
    > call->a_args.lock.fl.fl_flags |= FL_SLEEP;
    > call->a_args.lock.fl.fl_lmops = &nlmsvc_lock_operations;
    > - call->a_args.cookie = *cookie; /* see above */
    > + nlmclnt_next_cookie(&call->a_args.cookie);
    >
    > dprintk("lockd: created block %p...\n", block);
    >
    > @@ -640,17 +645,14 @@ static const struct rpc_call_ops nlmsvc_
    > * block.
    > */
    > void
    > -nlmsvc_grant_reply(struct svc_rqst *rqstp, struct nlm_cookie *cookie, u32 status)
    > +nlmsvc_grant_reply(struct nlm_cookie *cookie, u32 status)
    > {
    > struct nlm_block *block;
    > - struct nlm_file *file;
    >
    > - dprintk("grant_reply: looking for cookie %x, host (%08x), s=%d \n",
    > - *(unsigned int *)(cookie->data),
    > - ntohl(rqstp->rq_addr.sin_addr.s_addr), status);
    > - if (!(block = nlmsvc_find_block(cookie, &rqstp->rq_addr)))
    > + dprintk("grant_reply: looking for cookie %x, s=%d \n",
    > + *(unsigned int *)(cookie->data), status);
    > + if (!(block = nlmsvc_find_block(cookie)))
    > return;
    > - file = block->b_file;
    >
    > if (block) {
    > if (status == NLM_LCK_DENIED_GRACE_PERIOD) {
    >
    > diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
    > --- .prev/fs/lockd/svcproc.c 2006-09-01 12:11:01.000000000 +1000
    > +++ ./fs/lockd/svcproc.c 2006-09-01 12:17:21.000000000 +1000
    > @@ -484,7 +484,7 @@ nlmsvc_proc_granted_res(struct svc_rqst
    >
    > dprintk("lockd: GRANTED_RES called\n");
    >
    > - nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
    > + nlmsvc_grant_reply(&argp->cookie, argp->status);
    > return rpc_success;
    > }
    >
    >
    > diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
    > --- .prev/include/linux/lockd/lockd.h 2006-09-01 12:17:03.000000000 +1000
    > +++ ./include/linux/lockd/lockd.h 2006-09-01 12:17:21.000000000 +1000
    > @@ -193,7 +193,7 @@ u32 nlmsvc_cancel_blocked(struct nlm_
    > unsigned long nlmsvc_retry_blocked(void);
    > void nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
    > nlm_host_match_fn_t match);
    > -void nlmsvc_grant_reply(struct svc_rqst *, struct nlm_cookie *, u32);
    > +void nlmsvc_grant_reply(struct nlm_cookie *, u32);
    >
    > /*
    > * File handling for the server personality
    >
    > -------------------------------------------------------------------------
    > Using Tomcat but need to do more? Need to support web services, security?
    > Get stuff done quickly with pre-integrated technology to make your job easier
    > Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
    > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
    > _______________________________________________
    > NFS maillist - NFS@lists.sourceforge.net
    > https://lists.sourceforge.net/lists/listinfo/nfs

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-09-01 18:07    [W:3.847 / U:0.196 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site