[pnfs] [PATCH 12/17] Reroute a callback to be processed by thecallback service.
Iyer, Rahul
Rahul.Iyer at netapp.com
Mon May 21 16:00:57 EDT 2007
> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy at panasas.com]
> Sent: Monday, May 21, 2007 12:03 PM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] [PATCH 12/17] Reroute a callback to be
> processed by thecallback service.
>
> Iyer, Rahul wrote:
> >
> >
> >> -----Original Message-----
> >> From: Iyer, Rahul
> >> Sent: Monday, May 21, 2007 10:55 AM
> >> To: Benny Halevy
> >> Cc: pnfs at linux-nfs.org
> >> Subject: Re: [pnfs] [PATCH 12/17] Reroute a callback to be
> processed
> >> by thecallback service.
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Benny Halevy [mailto:bhalevy at panasas.com]
> >>> Sent: Monday, May 21, 2007 6:27 AM
> >>> To: Iyer, Rahul
> >>> Cc: pnfs at linux-nfs.org
> >>> Subject: Re: [PATCH 12/17] Reroute a callback to be
> >> processed by the
> >>> callback service.
> >>>
> >>> Iyer, Rahul wrote:
> >>>
> >>>> From 205cbe5b3b6371126973ba1a682b0b922c1d9195 Mon Sep 17
> >>> 00:00:00 2001
> >>>> Message-Id:
> >>> <205cbe5b3b6371126973ba1a682b0b922c1d9195.1179276403.git.iyer@
> >>> netapp.com>
> >>>> In-Reply-To:
> >>> <19b0fc3bc27cc1b1527dd685f71b79cd249a3779.1179276402.git.iyer@
> >>> netapp.com>
> >>>> References:
> >>> <19b0fc3bc27cc1b1527dd685f71b79cd249a3779.1179276402.git.iyer@
> >>> netapp.com>
> >>>> From: Rahul Iyer <iyer at netapp.com>
> >>>> Date: Tue, 15 May 2007 16:57:28 -0700
> >>>> Subject: [PATCH 12/17] Reroute a callback to be processed
> >>> by the callback service.
> >>>> This routine is the heart of the mux/demux of the
> >>> callbacks. When a RPC request
> >>>> arrives on the channel and is determined to be a callback,
> >>> the following thinfs
> >>>> happen:
> >>>>
> >>>> 1. A request (rpc_rqst struct) is allocated for the rpc
> >>> request and the request
> >>>> is read in.
> >>>>
> >>>> 2. The request is queued onto the callback request list of
> >>> the corresponding
> >>>> svc_serv for processing by the callback service
> >>>>
> >>>> Signed-off-by: Rahul Iyer <iyer at netapp.com>
> >>>> ---
> >>>> net/sunrpc/xprtsock.c | 30 ++++++++++++++++++++++++------
> >>>> 1 files changed, 24 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index
> >>>> 50e000e..0c0d7e0 100644
> >>>> --- a/net/sunrpc/xprtsock.c
> >>>> +++ b/net/sunrpc/xprtsock.c
> >>>> @@ -29,6 +29,7 @@
> >>>> #include <linux/tcp.h>
> >>>> #include <linux/sunrpc/clnt.h>
> >>>> #include <linux/sunrpc/sched.h>
> >>>> +#include <linux/sunrpc/bc_xprt.h>
> >>>> #include <linux/file.h>
> >>>>
> >>>> #include <net/sock.h>
> >>>> @@ -689,11 +690,20 @@ static inline void
> >>> xs_tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc
> >>>> spin_lock(&xprt->transport_lock);
> >>>> calldir = ntohl(xprt->tcp_calldir);
> >>>> if (calldir == RPC_CALL) {
> >>>> - dprintk("Callback received on backchannel!\n");
> >>>> - spin_unlock(&xprt->transport_lock);
> >>>>
> >>>> - /* Ignore callbacks for now */
> >>>> - return;
> >>>> + req = xprt_alloc_bc_request(xprt);
> >>>> +
> >>>> + if (!req) {
> >>>> + printk(KERN_NOTICE "Couldn't get
> >>> rpc_rqst for the allback! Dropping callback...\n");
> >>>> + xprt->tcp_flags &= ~XPRT_COPY_DATA;
> >>>> + spin_unlock(&xprt->transport_lock);
> >>>> +
> >>>> + /*
> >>>> + * Ignore the callback because we
> >>> couldn't allocate a
> >>>> + * request
> >>>> + */
> >>>> + return;
> >>>> + }
> >>> The spec says you should close the connection in this
> case assuming
> >>> returning an appropriate error here is too complicated for now.
> >>> In any case the client MUST NOT silently drop a request
> >> (and a printk
> >>> counts as silence from the server POV ;-)
> >>>
> >>> Benny
> >>>
> >> Ok. A possible solution is to set the size of the mempool to
> >> ca_maxrequests. Any allocation that fails means that the
> >> ca_maxrequests has been exceeded (though exceeding the
> limit doesn't
> >> mean allocation failure, because it is a mempool). In this
> scenario,
> >> we can alert the xprt that this happened and return an
> error. I can
> >> look at this.
> >
> > Actually, scratch that. Since this is in softirq context,
> there is no
> > guarantee any allocation will succeed. If the size of the
> mempool is
> > at least 2 greater than the ca_maxrequests, then the
> CB_SEQUENCE logic
> > will notice that the slotid has exceeded the max_slots (or
> there is a
> > duplicate slotid). Thus it will return an error. In such a
> case, the
> > server should realize that there is a problem with its
> sequence logic
> > and probably close the connection. Despite this, if it keeps on
> > sending CB_COMPOUND requests, this is a genuine case of resource
> > exhaustion and I believe it should be ok to start dropping
> requests.
> > The server has overwhelmed the client's resources and
> there's not much we can do.
> > Does this make sense?
>
> I don't think so. The idea behind sessions is that they
> provide EOS and the receiver must not drop requests.
>
> See secion 2.9.2 in draft-10:
>
> In order to reduce congestion, if a connection-oriented
> transport is
> used, and the request is not the NULL procedure,
>
> o A requester MUST NOT retry a request unless the connection the
> request was issued over was disconnected before the reply was
> received.
>
> o A replier MUST NOT silently drop a request, even if the
> request is
> a retry. (The silent drop behavior of RPCSEC_GSS [5] does not
> apply because this behavior happens at the RPCSEC_GSS layer, a
> lower layer in the request processing). Instead, the replier
> SHOULD return an appropriate error (see Section 2.10.4.1) or it
> MAY disconnect the connection.
I understand and agree with this. But we're at the point where the
sender isn't following session rules anyways, and has been sent at least
one error telling it that the slot management is messed up. Despite
this, the sender is still sending requests. We're dealing with an
illegal client here.
Regars
Rahul
>
More information about the pNFS
mailing list