[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-nfs
Subject:    RE: [PATCH 09/14] SQUASHME: nfs41: sunrpc: Add RPC direction back into the XDR buffer
From:       "Labiaga, Ricardo" <Ricardo.Labiaga () netapp ! com>
Date:       2009-06-12 15:07:46
Message-ID: 273FE88A07F5D445824060902F70034406334658 () SACMVEXC1-PRD ! hq ! netapp ! com
[Download RAW message or body]

> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy@panasas.com]
> Sent: Friday, June 12, 2009 7:22 AM
> To: Labiaga, Ricardo
> Cc: Myklebust, Trond; pnfs@linux-nfs.org; linux-nfs@vger.kernel.org
> Subject: Re: [PATCH 09/14] SQUASHME: nfs41: sunrpc: Add RPC direction
back
> into the XDR buffer
> 
> On Jun. 12, 2009, 1:54 -0400, Ricardo Labiaga
<Ricardo.Labiaga@netapp.com>
> wrote:
> > [squash with: nfs41: Skip past the RPC call direction]
> >
> > An earlier nfs41/sunrpc patch incorrectly assumed that all
transports
> will
> > read the RPC direction and route callbacks or replies to the
processing
> > routines.  This is only currently implemented for TCP, so
> rpc_verify_header()
> > needs to continue verifying that it is processing an RPC_REPLY.
> >
> > Reading and storing the RPC direction is a three step process.
> >
> > 1. xs_tcp_read_calldir() reads the RPC direction, but it will not
store
> it
> > in the XDR buffer since the 'struct rpc_rqst' is not yet available.
> >
> > 2. The 'struct rpc_rqst' is obtained during the TCP_RCV_COPY_DATA
state.
> > This state need not necessarily be preceeded by the
> TCP_RCV_READ_CALLDIR.
> > For example, we may be reading a continuation packet to a large
reply.
> > Therefore, we can't simply obtain the 'struct rpc_rqst' during the
> > TCP_RCV_READ_CALLDIR state and assume it's available during
> TCP_RCV_COPY_DATA.
> >
> > This patch adds a new TCP_RCV_READ_CALLDIR flag to indicate the need
to
> > read the RPC direction.  It then uses TCP_RCV_COPY_CALLDIR to
indicate
> the
> > RPC direction needs to be saved after the 'struct rpc_rqst' has been
> allocated.
> >
> > 3. The 'struct rpc_rqst' is obtained by the xs_tcp_read_data()
helper
> > functions.  xs_tcp_read_common() then saves the RPC direction in the
XDR
> > buffer if TCP_RCV_COPY_CALLDIR is set.  This will happen when we're
> reading
> > the data immediately after the direction was read.
xs_tcp_read_common()
> > then clears this flag.
> >
> > Signed-off-by: Ricardo Labiaga <Ricardo.Labiaga@netapp.com>
> > ---
> >  net/sunrpc/xprtsock.c |   31 +++++++++++++++++++++++++------
> >  1 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index 8ec2600..fe57ebd 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -275,12 +275,13 @@ struct sock_xprt {
> >  #define TCP_RCV_COPY_FRAGHDR	(1UL << 1)
> >  #define TCP_RCV_COPY_XID	(1UL << 2)
> >  #define TCP_RCV_COPY_DATA	(1UL << 3)
> > -#define TCP_RCV_COPY_CALLDIR	(1UL << 4)
> > +#define TCP_RCV_READ_CALLDIR	(1UL << 4)
> > +#define TCP_RCV_COPY_CALLDIR	(1UL << 5)
> >
> >  /*
> >   * TCP RPC flags
> >   */
> > -#define TCP_RPC_REPLY		(1UL << 5)
> > +#define TCP_RPC_REPLY		(1UL << 6)
> >
> >  static inline struct sockaddr *xs_addr(struct rpc_xprt *xprt)
> >  {
> > @@ -1002,7 +1003,7 @@ static inline void xs_tcp_read_xid(struct
> sock_xprt *transport, struct xdr_skb_r
> >  	if (used != len)
> >  		return;
> >  	transport->tcp_flags &= ~TCP_RCV_COPY_XID;
> > -	transport->tcp_flags |= TCP_RCV_COPY_CALLDIR;
> > +	transport->tcp_flags |= TCP_RCV_READ_CALLDIR;
> >  	transport->tcp_copied = 4;
> >  	dprintk("RPC:       reading %s XID %08x\n",
> >  			(transport->tcp_flags & TCP_RPC_REPLY) ? "reply
for"
> > @@ -1031,9 +1032,13 @@ static inline void xs_tcp_read_calldir(struct
> sock_xprt *transport,
> >  	transport->tcp_offset += used;
> >  	if (used != len)
> >  		return;
> > -	transport->tcp_flags &= ~TCP_RCV_COPY_CALLDIR;
> > +	transport->tcp_flags &= ~TCP_RCV_READ_CALLDIR;
> > +	transport->tcp_flags |= TCP_RCV_COPY_CALLDIR;
> >  	transport->tcp_flags |= TCP_RCV_COPY_DATA;
> > -	transport->tcp_copied += 4;
> > +	/*
> > +	 * We don't yet have the XDR buffer, so we will write the
calldir
> > +	 * out after we get the buffer from the 'struct rpc_rqst'
> > +	 */
> >  	if (ntohl(calldir) == RPC_REPLY)
> >  		transport->tcp_flags |= TCP_RPC_REPLY;
> >  	else
> > @@ -1055,6 +1060,20 @@ static inline void xs_tcp_read_common(struct
> rpc_xprt *xprt,
> >  	ssize_t r;
> >
> >  	rcvbuf = &req->rq_private_buf;
> > +
> > +	if (transport->tcp_flags & TCP_RCV_COPY_CALLDIR) {
> > +		/*
> > +		 * Save the RPC direction in the XDR buffer
> > +		 */
> > +		__be32	calldir = transport->tcp_flags & TCP_RPC_REPLY ?
> > +					htonl(RPC_REPLY) : 0;
> > +
> > +		memcpy(rcvbuf->head[0].iov_base + transport->tcp_copied,
> > +			&calldir, sizeof(calldir));
> > +		transport->tcp_copied += sizeof(calldir);
> > +		transport->tcp_flags &= ~TCP_RCV_COPY_CALLDIR;
> 
> Can this be done in xs_tcp_read_calldir()?
> 

No, because we don't yet have the XDR buffer.  The XDR buffer is in the
'struct rpc_rqst' which we can only obtain after we read the RPC
direction.  This because we need to see if we get it from the list of
pending replies, or from the list of preallocated callback buffers.  We
can not search for this in xs_tcp_read_calldir() because we also need to
obtain it for RPC continuation fragments that will not have the RPC call
direction bit set.

I played with another version of the patch that created a new state for
allocation of the rpc_rqst buffer, but ran into problems.  I'll work
some more on that version of the patch in the next few weeks, but I'd
like to get the submitted version in right now.

- ricardo


> Benny
> 
> > +	}
> > +
> >  	len = desc->count;
> >  	if (len > transport->tcp_reclen - transport->tcp_offset) {
> >  		struct xdr_skb_reader my_desc;
> > @@ -1258,7 +1277,7 @@ static int xs_tcp_data_recv(read_descriptor_t
> *rd_desc, struct sk_buff *skb, uns
> >  			continue;
> >  		}
> >  		/* Read in the call/reply flag */
> > -		if (transport->tcp_flags & TCP_RCV_COPY_CALLDIR) {
> > +		if (transport->tcp_flags & TCP_RCV_READ_CALLDIR) {
> >  			xs_tcp_read_calldir(transport, &desc);
> >  			continue;
> >  		}

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]