[pnfs] [PATCH 12/17] Reroute a callback to be processed by thecallback service.

[Iyer, Rahul]

> -----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?
Regards
Rahul

> Regards
> Rahul
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>