[pnfs] [PATCH 3/8] Added a function to read the call/reply flag

[Benny Halevy]

It seems like your implementation might break if only the xid is sent.
The current code for xs_tcp_read_request() will deal with that correctly
since it reads up to xprt->tcp_reclen, but xs_tcp_read_calldir() simply reads
sizeof(xprt->tcp_calldir) (similar to xs_tcp_read_xid()).  Therefore
I suggest requiring tcp_reclen >= 8 rather than 4 in xs_tcp_read_fraghdr().
I.e.:

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 441bd53..9d679d6 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -604,7 +604,7 @@ static inline void xs_tcp_read_fraghdr(struct rpc_xprt *xprt, skb_reader_t
        xprt->tcp_offset = 0;

        /* Sanity check of the record length */
-       if (unlikely(xprt->tcp_reclen < 4)) {
+       if (unlikely(xprt->tcp_reclen < 8)) {
                dprintk("RPC:      invalid TCP record fragment length\n");
                xprt_disconnect(xprt);
                return;


iyer at netapp.com wrote:
> From: Rahul Iyer <iyer at netapp.com>
> 
> Added a function xs_tcp_read_calldir() to read in the call/reply flag and store
> it in xprt->tcp_calldir.
> 
> Signed-off-by: Rahul Iyer <iyer at netapp.com>
> ---
>  net/sunrpc/xprtsock.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 441bd53..ab1e8fb 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -648,6 +648,34 @@ static inline void xs_tcp_read_xid(struct rpc_xprt *xprt, skb_reader_t *desc)
>  	xs_tcp_check_recm(xprt);
>  }
>  
> +static inline void xs_tcp_read_calldir(struct rpc_xprt *xprt, skb_reader_t *desc)
> +{
> +	size_t len, used;
> +	u32 offset;
> +	char *p;
> +
> +	/* The reason for this rather odd calculation is that we want
> +	 * xprt->tcp_offset to be 8 at the end of this routine (4 bytes for the
> +	 * xid and 4 bytes for the call/reply flag). When coming into this
> +	 * function for the first time, xprt->tcp_offset is 4 (after reading
> +	 * the xid).
> +	 */
> +	offset = xprt->tcp_offset - sizeof(xprt->tcp_xid);
> +	len = sizeof(xprt->tcp_calldir) - offset;
> +	dprintk("RPC:      reading CALL/REPLY flag (%Zu bytes)\n", len);
> +	p = ((char *) &xprt->tcp_calldir) + offset;
> +	used = xs_tcp_copy_data(desc, p, len);
> +	xprt->tcp_offset += used;
> +	if (used != len)
> +		return;
> +	xprt->tcp_flags &= ~XPRT_COPY_CALLDIR;
> +	xprt->tcp_flags |= XPRT_COPY_DATA;
> +	xprt->tcp_copied += 4;
> +	dprintk("RPC:      reading reply for CALL/REPLY flag %08x\n",
> +						ntohl(xprt->tcp_xid));
> +	xs_tcp_check_recm(xprt);
> +}
> +
>  static inline void xs_tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc)
>  {
>  	struct rpc_rqst *req;