linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/8] For discussion of bug 198053
@ 2020-01-29 16:09 Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 1/8] nfsd: Fix NFSv4 READ on RDMA when using readv Chuck Lever
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Hi Bruce-

These are a proof-of-concept, not for merge.

This set of patches addresses the NFS/RDMA bug reported here:

https://bugzilla.kernel.org/show_bug.cgi?id=198053

However, applying them results in a regression of support for NFSv4
COMPOUNDs with multiple READ operations on TCP.

I think a different approach might be needed? I could introduce a
new transport method that would be called for READ/READLINK that
would enable the transport to determine how it wants to convey
the Reply payload. The TCP method would behave exactly as it does
today. The RDMA method would utilize a Write chunk if one is
available, otherwise, it would also behave as it does today.

If I can get that approach to work, it would both address 198053
and enable support for multiple READ operations in a COMPOUND for
both TCP and RDMA. Thoughts?

Sidebar: while working on this patch set, it occurred to me it
would be a good clean up if svc_alloc_arg could always set up a
page for rq_res->tail. Then there wouldn't have to be all the
duplicate logic for checking whether a tail exists, or if it's
large enough, etc. etc. Can you think of an easy way to grab
one of the rpc_rqst's rq_pages for this purpose?


---

Chuck Lever (8):
      nfsd: Fix NFSv4 READ on RDMA when using readv
      SUNRPC: Add XDR infrastructure for automatically padding xdr_buf::pages
      SUNRPC: TCP transport support for automated padding of xdr_buf::pages
      svcrdma: RDMA transport support for automated padding of xdr_buf::pages
      NFSD: NFSv2 support for automated padding of xdr_buf::pages
      NFSD: NFSv3 support for automated padding of xdr_buf::pages
      sunrpc: Add new contractual constraint on xdr_buf API
      SUNRPC: GSS support for automated padding of xdr_buf::pages


 fs/nfsd/nfs3xdr.c                     |   19 +-------
 fs/nfsd/nfs4xdr.c                     |   70 ++++++++-----------------------
 fs/nfsd/nfsxdr.c                      |   20 ++-------
 include/linux/sunrpc/xdr.h            |   74 +++++++++++++++++++++++++--------
 net/sunrpc/auth_gss/gss_krb5_crypto.c |   13 +++---
 net/sunrpc/auth_gss/gss_krb5_wrap.c   |   11 +++--
 net/sunrpc/auth_gss/svcauth_gss.c     |   51 +++++++++++++----------
 net/sunrpc/svc.c                      |    2 -
 net/sunrpc/svc_xprt.c                 |   14 ++++--
 net/sunrpc/svcsock.c                  |   39 +++++++++++------
 net/sunrpc/xdr.c                      |   15 ++++---
 net/sunrpc/xprtrdma/svc_rdma_rw.c     |   13 ++++++
 net/sunrpc/xprtrdma/svc_rdma_sendto.c |   27 ++++++++----
 13 files changed, 200 insertions(+), 168 deletions(-)

--
Chuck Lever

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH RFC 1/8] nfsd: Fix NFSv4 READ on RDMA when using readv
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
@ 2020-01-29 16:09 ` Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 2/8] SUNRPC: Add XDR infrastructure for automatically padding xdr_buf::pages Chuck Lever
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

svcrdma expects that the payload falls precisely into the xdr_buf
page vector. Adding "xdr->iov = NULL" forces xdr_reserve_space to
always use pages from xdr->buf->pages when calling nfsd_readv.

This code is called only when fops->splice_read is missing or when
RQ_SPLICE_OK is clear, so it's not a noticeable problem in many
common cases.

Fixes: b04209806384 ("nfsd4: allow exotic read compounds")
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=198053
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4xdr.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 75d1fc13a9c9..92a6ada60932 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3521,17 +3521,14 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 	u32 zzz = 0;
 	int pad;
 
+	/* Ensure xdr_reserve_space behaves itself */
+	if (xdr->iov == xdr->buf->head) {
+		xdr->iov = NULL;
+		xdr->end = xdr->p;
+	}
+
 	len = maxcount;
 	v = 0;
-
-	thislen = min_t(long, len, ((void *)xdr->end - (void *)xdr->p));
-	p = xdr_reserve_space(xdr, (thislen+3)&~3);
-	WARN_ON_ONCE(!p);
-	resp->rqstp->rq_vec[v].iov_base = p;
-	resp->rqstp->rq_vec[v].iov_len = thislen;
-	v++;
-	len -= thislen;
-
 	while (len) {
 		thislen = min_t(long, len, PAGE_SIZE);
 		p = xdr_reserve_space(xdr, (thislen+3)&~3);


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 2/8] SUNRPC: Add XDR infrastructure for automatically padding xdr_buf::pages
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 1/8] nfsd: Fix NFSv4 READ on RDMA when using readv Chuck Lever
@ 2020-01-29 16:09 ` Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 3/8] SUNRPC: TCP transport support for automated padding of xdr_buf::pages Chuck Lever
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Server/client agnostic API changes to support padding xdr_buf::pages
automatically.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/xdr.h |   74 ++++++++++++++++++++++++++++++++++----------
 net/sunrpc/svc.c           |    2 +
 net/sunrpc/svc_xprt.c      |   14 +++++---
 3 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index b41f34977995..47f55aad5a1e 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -24,6 +24,34 @@
  */
 #define XDR_QUADLEN(l)		(((l) + 3) >> 2)
 
+/**
+ * xdr_align_size - Calculate padded size of an object
+ * @n: Size of an object being XDR encoded (in bytes)
+ *
+ * Return value:
+ *   Size (in bytes) of the object including xdr padding
+ */
+static inline size_t
+xdr_align_size(size_t n)
+{
+	const size_t mask = sizeof(__u32) - 1;
+
+	return (n + mask) & ~mask;
+}
+
+/**
+ * xdr_pad_size - Calculate XDR padding size
+ * @len: Actual size of object being XDR encoded (in bytes)
+ *
+ * Return value:
+ *   Size (in bytes) of needed XDR padding
+ */
+static inline size_t
+xdr_pad_size(size_t len)
+{
+	return xdr_align_size(len) - len;
+}
+
 /*
  * Generic opaque `network object.' At the kernel level, this type
  * is used only by lockd.
@@ -39,9 +67,6 @@ struct xdr_netobj {
  * Features a header (for a linear buffer containing RPC headers
  * and the data payload for short messages), and then an array of
  * pages.
- * The tail iovec allows you to append data after the page array. Its
- * main interest is for appending padding to the pages in order to
- * satisfy the int_32-alignment requirements in RFC1832.
  *
  * For the future, we might want to string several of these together
  * in a list if anybody wants to make use of NFSv4 COMPOUND
@@ -55,6 +80,7 @@ struct xdr_buf {
 	struct page **	pages;		/* Array of pages */
 	unsigned int	page_base,	/* Start of page data */
 			page_len,	/* Length of page data */
+			page_pad,	/* XDR padding needed for pages */
 			flags;		/* Flags for data disposition */
 #define XDRBUF_READ		0x01		/* target of file read */
 #define XDRBUF_WRITE		0x02		/* source of file write */
@@ -72,11 +98,39 @@ struct xdr_buf {
 	buf->tail[0].iov_len = 0;
 	buf->pages = NULL;
 	buf->page_len = 0;
+	buf->page_pad = 0;
 	buf->flags = 0;
 	buf->len = 0;
 	buf->buflen = len;
 }
 
+/**
+ * xdr_buf_set_pagelen - Set the length of the page list
+ * @buf: XDR buffer containing a message
+ * @len: Size of @buf's page list (in bytes)
+ *
+ */
+static inline void
+xdr_buf_set_pagelen(struct xdr_buf *buf, size_t len)
+{
+	buf->page_len = len;
+	buf->page_pad = xdr_pad_size(len);
+}
+
+/**
+ * xdr_buf_msglen - Return the length of the content in @buf
+ * @buf: XDR buffer containing an XDR-encoded message
+ *
+ * Return value:
+ *   Size (in bytes) of the content in @buf
+ */
+static inline size_t
+xdr_buf_msglen(const struct xdr_buf *buf)
+{
+	return buf->head[0].iov_len + buf->page_len + buf->page_pad +
+		buf->tail[0].iov_len;
+}
+
 /*
  * pre-xdr'ed macros.
  */
@@ -285,20 +339,6 @@ ssize_t xdr_stream_decode_string(struct xdr_stream *xdr, char *str,
 		size_t size);
 ssize_t xdr_stream_decode_string_dup(struct xdr_stream *xdr, char **str,
 		size_t maxlen, gfp_t gfp_flags);
-/**
- * xdr_align_size - Calculate padded size of an object
- * @n: Size of an object being XDR encoded (in bytes)
- *
- * Return value:
- *   Size (in bytes) of the object including xdr padding
- */
-static inline size_t
-xdr_align_size(size_t n)
-{
-	const size_t mask = sizeof(__u32) - 1;
-
-	return (n + mask) & ~mask;
-}
 
 /**
  * xdr_stream_encode_u32 - Encode a 32-bit integer
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 187dd4e73d64..63a3afac7100 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1516,7 +1516,7 @@ static __printf(2,3) void svc_printk(struct svc_rqst *rqstp, const char *fmt, ..
 	rqstp->rq_res.pages = rqstp->rq_respages + 1;
 	rqstp->rq_res.len = 0;
 	rqstp->rq_res.page_base = 0;
-	rqstp->rq_res.page_len = 0;
+	xdr_buf_set_pagelen(&rqstp->rq_res, 0);
 	rqstp->rq_res.buflen = PAGE_SIZE;
 	rqstp->rq_res.tail[0].iov_base = NULL;
 	rqstp->rq_res.tail[0].iov_len = 0;
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index de3c077733a7..032c3bc91d43 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -510,7 +510,7 @@ static void svc_xprt_release(struct svc_rqst *rqstp)
 	rqstp->rq_deferred = NULL;
 
 	svc_free_res_pages(rqstp);
-	rqstp->rq_res.page_len = 0;
+	xdr_buf_set_pagelen(&rqstp->rq_res, 0);
 	rqstp->rq_res.page_base = 0;
 
 	/* Reset response buffer and release
@@ -666,7 +666,7 @@ static int svc_alloc_arg(struct svc_rqst *rqstp)
 	arg->pages = rqstp->rq_pages + 1;
 	arg->page_base = 0;
 	/* save at least one page for response */
-	arg->page_len = (pages-2)*PAGE_SIZE;
+	xdr_buf_set_pagelen(arg, (pages - 2) << PAGE_SHIFT);
 	arg->len = (pages-1)*PAGE_SIZE;
 	arg->tail[0].iov_len = 0;
 	return 0;
@@ -902,9 +902,13 @@ int svc_send(struct svc_rqst *rqstp)
 
 	/* calculate over-all length */
 	xb = &rqstp->rq_res;
-	xb->len = xb->head[0].iov_len +
-		xb->page_len +
-		xb->tail[0].iov_len;
+	xb->len = xdr_buf_msglen(xb);
+	if ((xb->head[0].iov_len & 3) != 0)
+		trace_printk("head=[%p,%zu] page=%u/%u tail=[%p,%zu] msglen=%zu buflen=%u",
+			xb->head[0].iov_base, xb->head[0].iov_len,
+			xb->page_len, xb->page_pad,
+			xb->tail[0].iov_base, xb->tail[0].iov_len,
+			xdr_buf_msglen(xb), xb->buflen);
 
 	/* Grab mutex to serialize outgoing data. */
 	mutex_lock(&xprt->xpt_mutex);


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 3/8] SUNRPC: TCP transport support for automated padding of xdr_buf::pages
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 1/8] nfsd: Fix NFSv4 READ on RDMA when using readv Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 2/8] SUNRPC: Add XDR infrastructure for automatically padding xdr_buf::pages Chuck Lever
@ 2020-01-29 16:09 ` Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 4/8] svcrdma: RDMA " Chuck Lever
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svcsock.c |   39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 2934dd711715..966ea431f845 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -187,14 +187,12 @@ int svc_send_common(struct socket *sock, struct xdr_buf *xdr,
 	size_t		base = xdr->page_base;
 	unsigned int	pglen = xdr->page_len;
 	unsigned int	flags = MSG_MORE | MSG_SENDPAGE_NOTLAST;
-	int		slen;
+	int		slen = xdr_buf_msglen(xdr);
 	int		len = 0;
 
-	slen = xdr->len;
-
 	/* send head */
 	if (slen == xdr->head[0].iov_len)
-		flags = 0;
+		flags = MSG_EOR;
 	len = kernel_sendpage(sock, headpage, headoffset,
 				  xdr->head[0].iov_len, flags);
 	if (len != xdr->head[0].iov_len)
@@ -207,7 +205,7 @@ int svc_send_common(struct socket *sock, struct xdr_buf *xdr,
 	size = PAGE_SIZE - base < pglen ? PAGE_SIZE - base : pglen;
 	while (pglen > 0) {
 		if (slen == size)
-			flags = 0;
+			flags = MSG_EOR;
 		result = kernel_sendpage(sock, *ppage, base, size, flags);
 		if (result > 0)
 			len += result;
@@ -219,11 +217,21 @@ int svc_send_common(struct socket *sock, struct xdr_buf *xdr,
 		base = 0;
 		ppage++;
 	}
+	if (xdr->page_pad) {
+		if (!xdr->tail[0].iov_len)
+			flags = MSG_EOR;
+		result = kernel_sendpage(sock, ZERO_PAGE(0), 0,
+					 xdr->page_pad, flags);
+		if (result > 0)
+			len += result;
+		if (result != xdr->page_pad)
+			goto out;
+	}
 
 	/* send tail */
 	if (xdr->tail[0].iov_len) {
 		result = kernel_sendpage(sock, tailpage, tailoffset,
-				   xdr->tail[0].iov_len, 0);
+					 xdr->tail[0].iov_len, MSG_EOR);
 		if (result > 0)
 			len += result;
 	}
@@ -272,9 +280,9 @@ static int svc_sendto(struct svc_rqst *rqstp, struct xdr_buf *xdr)
 			       rqstp->rq_respages[0], tailoff);
 
 out:
-	dprintk("svc: socket %p sendto([%p %zu... ], %d) = %d (addr %s)\n",
+	dprintk("svc: socket %p sendto([%p %zu... ], %zu) = %d (addr %s)\n",
 		svsk, xdr->head[0].iov_base, xdr->head[0].iov_len,
-		xdr->len, len, svc_print_addr(rqstp, buf, sizeof(buf)));
+		xdr_buf_msglen(xdr), len, svc_print_addr(rqstp, buf, sizeof(buf)));
 
 	return len;
 }
@@ -1134,24 +1142,25 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
 static int svc_tcp_sendto(struct svc_rqst *rqstp)
 {
 	struct xdr_buf	*xbufp = &rqstp->rq_res;
+	u32 reclen = xdr_buf_msglen(xbufp);
+	__be32 marker;
 	int sent;
-	__be32 reclen;
 
 	/* Set up the first element of the reply kvec.
 	 * Any other kvecs that may be in use have been taken
 	 * care of by the server implementation itself.
 	 */
-	reclen = htonl(0x80000000|((xbufp->len ) - 4));
-	memcpy(xbufp->head[0].iov_base, &reclen, 4);
+	marker = cpu_to_be32(0x80000000 | (reclen - sizeof(marker)));
+	memcpy(xbufp->head[0].iov_base, &marker, sizeof(marker));
 
 	sent = svc_sendto(rqstp, &rqstp->rq_res);
-	if (sent != xbufp->len) {
+	if (sent != reclen) {
 		printk(KERN_NOTICE
-		       "rpc-srv/tcp: %s: %s %d when sending %d bytes "
+		       "rpc-srv/tcp: %s: %s %d when sending %u bytes "
 		       "- shutting down socket\n",
 		       rqstp->rq_xprt->xpt_server->sv_name,
-		       (sent<0)?"got error":"sent only",
-		       sent, xbufp->len);
+		       (sent<0)?"got error":"sent",
+		       sent, reclen);
 		set_bit(XPT_CLOSE, &rqstp->rq_xprt->xpt_flags);
 		svc_xprt_enqueue(rqstp->rq_xprt);
 		sent = -EAGAIN;


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 4/8] svcrdma: RDMA transport support for automated padding of xdr_buf::pages
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
                   ` (2 preceding siblings ...)
  2020-01-29 16:09 ` [PATCH RFC 3/8] SUNRPC: TCP transport support for automated padding of xdr_buf::pages Chuck Lever
@ 2020-01-29 16:09 ` Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 5/8] NFSD: NFSv2 " Chuck Lever
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs


---
 net/sunrpc/xprtrdma/svc_rdma_rw.c     |   13 +++++++++++++
 net/sunrpc/xprtrdma/svc_rdma_sendto.c |   27 +++++++++++++++++++--------
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 467d40a1dffa..a7fb886ea136 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -14,6 +14,8 @@
 #include "xprt_rdma.h"
 #include <trace/events/rpcrdma.h>
 
+static const __be32 xdr_padding = xdr_zero;
+
 static void svc_rdma_write_done(struct ib_cq *cq, struct ib_wc *wc);
 static void svc_rdma_wc_read_done(struct ib_cq *cq, struct ib_wc *wc);
 
@@ -559,6 +561,9 @@ int svc_rdma_send_reply_chunk(struct svcxprt_rdma *rdma, __be32 *rp_ch,
 {
 	struct svc_rdma_write_info *info;
 	int consumed, ret;
+	struct kvec pad = {
+		.iov_base = (void *)&xdr_padding,
+	};
 
 	info = svc_rdma_write_info_alloc(rdma, rp_ch);
 	if (!info)
@@ -577,6 +582,14 @@ int svc_rdma_send_reply_chunk(struct svcxprt_rdma *rdma, __be32 *rp_ch,
 		if (ret < 0)
 			goto out_err;
 		consumed += xdr->page_len;
+
+		if (xdr->page_pad) {
+			pad.iov_len = xdr->page_pad;
+			ret = svc_rdma_send_xdr_kvec(info, &pad);
+			if (ret < 0)
+				goto out_err;
+			consumed += pad.iov_len;
+		}
 	}
 
 	if (xdr->tail[0].iov_len) {
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 33f817519964..d0f9acfe60a6 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -112,6 +112,8 @@
 #include "xprt_rdma.h"
 #include <trace/events/rpcrdma.h>
 
+static const __be32 xdr_padding = xdr_zero;
+
 static void svc_rdma_wc_send(struct ib_cq *cq, struct ib_wc *wc);
 
 static inline struct svc_rdma_send_ctxt *
@@ -320,11 +322,6 @@ int svc_rdma_send(struct svcxprt_rdma *rdma, struct ib_send_wr *wr)
 	return ret;
 }
 
-static u32 xdr_padsize(u32 len)
-{
-	return (len & 3) ? (4 - (len & 3)) : 0;
-}
-
 /* Returns length of transport header, in bytes.
  */
 static unsigned int svc_rdma_reply_hdr_len(__be32 *rdma_resp)
@@ -561,6 +558,8 @@ static bool svc_rdma_pull_up_needed(struct svcxprt_rdma *rdma,
 					   remaining);
 			pageoff = 0;
 		}
+		if (xdr->page_pad)
+			++elements;
 	}
 
 	/* xdr->tail */
@@ -593,7 +592,7 @@ static int svc_rdma_pull_up_reply_msg(struct svcxprt_rdma *rdma,
 	if (wr_lst) {
 		u32 xdrpad;
 
-		xdrpad = xdr_padsize(xdr->page_len);
+		xdrpad = xdr_pad_size(xdr->page_len);
 		if (taillen && xdrpad) {
 			tailbase += xdrpad;
 			taillen -= xdrpad;
@@ -614,12 +613,16 @@ static int svc_rdma_pull_up_reply_msg(struct svcxprt_rdma *rdma,
 			dst += len;
 			pageoff = 0;
 		}
+		if (xdr->page_pad) {
+			memcpy(dst, &xdr_padding, xdr->page_pad);
+			dst += xdr->page_pad;
+		}
 	}
 
 	if (taillen)
 		memcpy(dst, tailbase, taillen);
 
-	ctxt->sc_sges[0].length += xdr->len;
+	ctxt->sc_sges[0].length += xdr_buf_msglen(xdr);
 	ib_dma_sync_single_for_device(rdma->sc_pd->device,
 				      ctxt->sc_sges[0].addr,
 				      ctxt->sc_sges[0].length,
@@ -668,7 +671,7 @@ int svc_rdma_map_reply_msg(struct svcxprt_rdma *rdma,
 	if (wr_lst) {
 		base = xdr->tail[0].iov_base;
 		len = xdr->tail[0].iov_len;
-		xdr_pad = xdr_padsize(xdr->page_len);
+		xdr_pad = xdr_pad_size(xdr->page_len);
 
 		if (len && xdr_pad) {
 			base += xdr_pad;
@@ -693,6 +696,14 @@ int svc_rdma_map_reply_msg(struct svcxprt_rdma *rdma,
 		remaining -= len;
 		page_off = 0;
 	}
+	if (xdr->page_pad) {
+		++ctxt->sc_cur_sge_no;
+		ret = svc_rdma_dma_map_buf(rdma, ctxt,
+					   (unsigned char *)&xdr_padding,
+					   xdr->page_pad);
+		if (ret < 0)
+			return ret;
+	}
 
 	base = xdr->tail[0].iov_base;
 	len = xdr->tail[0].iov_len;


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 5/8] NFSD: NFSv2 support for automated padding of xdr_buf::pages
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
                   ` (3 preceding siblings ...)
  2020-01-29 16:09 ` [PATCH RFC 4/8] svcrdma: RDMA " Chuck Lever
@ 2020-01-29 16:09 ` Chuck Lever
  2020-01-29 16:09 ` [PATCH RFC 6/8] NFSD: NFSv3 " Chuck Lever
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfsxdr.c |   20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index b51fe515f06f..06e3a021b87a 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -455,13 +455,7 @@ __be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *f
 
 	*p++ = htonl(resp->len);
 	xdr_ressize_check(rqstp, p);
-	rqstp->rq_res.page_len = resp->len;
-	if (resp->len & 3) {
-		/* need to pad the tail */
-		rqstp->rq_res.tail[0].iov_base = p;
-		*p = 0;
-		rqstp->rq_res.tail[0].iov_len = 4 - (resp->len&3);
-	}
+	xdr_buf_set_pagelen(&rqstp->rq_res, resp->len);
 	return 1;
 }
 
@@ -475,13 +469,7 @@ __be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *f
 	xdr_ressize_check(rqstp, p);
 
 	/* now update rqstp->rq_res to reflect data as well */
-	rqstp->rq_res.page_len = resp->count;
-	if (resp->count & 3) {
-		/* need to pad the tail */
-		rqstp->rq_res.tail[0].iov_base = p;
-		*p = 0;
-		rqstp->rq_res.tail[0].iov_len = 4 - (resp->count&3);
-	}
+	xdr_buf_set_pagelen(&rqstp->rq_res, resp->count);
 	return 1;
 }
 
@@ -494,8 +482,8 @@ __be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *f
 	p = resp->buffer;
 	*p++ = 0;			/* no more entries */
 	*p++ = htonl((resp->common.err == nfserr_eof));
-	rqstp->rq_res.page_len = (((unsigned long)p-1) & ~PAGE_MASK)+1;
-
+	xdr_buf_set_pagelen(&rqstp->rq_res,
+			    (((unsigned long)p - 1) & ~PAGE_MASK) + 1);
 	return 1;
 }
 


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 6/8] NFSD: NFSv3 support for automated padding of xdr_buf::pages
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
                   ` (4 preceding siblings ...)
  2020-01-29 16:09 ` [PATCH RFC 5/8] NFSD: NFSv2 " Chuck Lever
@ 2020-01-29 16:09 ` Chuck Lever
  2020-01-29 16:10 ` [PATCH RFC 7/8] sunrpc: Add new contractual constraint on xdr_buf API Chuck Lever
  2020-01-29 16:10 ` [PATCH RFC 8/8] SUNRPC: GSS support for automated padding of xdr_buf::pages Chuck Lever
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:09 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs3xdr.c |   19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 195ab7a0fc89..91d96a329033 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -709,13 +709,7 @@ void fill_post_wcc(struct svc_fh *fhp)
 	if (resp->status == 0) {
 		*p++ = htonl(resp->len);
 		xdr_ressize_check(rqstp, p);
-		rqstp->rq_res.page_len = resp->len;
-		if (resp->len & 3) {
-			/* need to pad the tail */
-			rqstp->rq_res.tail[0].iov_base = p;
-			*p = 0;
-			rqstp->rq_res.tail[0].iov_len = 4 - (resp->len&3);
-		}
+		xdr_buf_set_pagelen(&rqstp->rq_res, resp->len);
 		return 1;
 	} else
 		return xdr_ressize_check(rqstp, p);
@@ -733,14 +727,7 @@ void fill_post_wcc(struct svc_fh *fhp)
 		*p++ = htonl(resp->eof);
 		*p++ = htonl(resp->count);	/* xdr opaque count */
 		xdr_ressize_check(rqstp, p);
-		/* now update rqstp->rq_res to reflect data as well */
-		rqstp->rq_res.page_len = resp->count;
-		if (resp->count & 3) {
-			/* need to pad the tail */
-			rqstp->rq_res.tail[0].iov_base = p;
-			*p = 0;
-			rqstp->rq_res.tail[0].iov_len = 4 - (resp->count & 3);
-		}
+		xdr_buf_set_pagelen(&rqstp->rq_res, resp->count);
 		return 1;
 	} else
 		return xdr_ressize_check(rqstp, p);
@@ -817,7 +804,7 @@ void fill_post_wcc(struct svc_fh *fhp)
 		xdr_ressize_check(rqstp, p);
 		if (rqstp->rq_res.head[0].iov_len + (2<<2) > PAGE_SIZE)
 			return 1; /*No room for trailer */
-		rqstp->rq_res.page_len = (resp->count) << 2;
+		xdr_buf_set_pagelen(&rqstp->rq_res, resp->count << 2);
 
 		/* add the 'tail' to the end of the 'head' page - page 0. */
 		rqstp->rq_res.tail[0].iov_base = p;


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 7/8] sunrpc: Add new contractual constraint on xdr_buf API
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
                   ` (5 preceding siblings ...)
  2020-01-29 16:09 ` [PATCH RFC 6/8] NFSD: NFSv3 " Chuck Lever
@ 2020-01-29 16:10 ` Chuck Lever
  2020-01-29 16:10 ` [PATCH RFC 8/8] SUNRPC: GSS support for automated padding of xdr_buf::pages Chuck Lever
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:10 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Server-side only.

Let's move some XDR padding logic into the transports, where it
can be handled automatically. Essentially, all three sections of
the xdr_buf always begin on XDR alignment, and the RPC layer
will insert padding between the sections to enforce that
guarantee, as the buffer is transmitted.

o head[0] has always begun on an XDR boundary, so no change there.
o Insertion should be "almost never" for the boundary between
  head[0] and the page list. This is determined by checking if
  head[0].iov_len is XDR-aligned.
o Insertion might be somewhat frequent for the boundary between
  the page list and tail[0]. This is determined by checking if
  (head[0].iov_len + page_len) is XDR-aligned.

Whatever is contained in each section of the xdr_buf remains the
responsibility of the upper layer to form correctly.

So if nfsd4_encode_readv wants to stuff a bunch of XDR items into
the page list, it is free to do so without confusing the transport.
The only alignment concern is the last item in the page list,
which will be automatically handled by RPC and the transport layers.

One benefit of this change is that padding logic duplicated
throughout the NFSD Reply encoders can be eliminated. Those changes
will occur in subsequent patches.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4xdr.c |   55 +++++++++++++----------------------------------------
 net/sunrpc/xdr.c  |   12 ++++++------
 2 files changed, 19 insertions(+), 48 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 92a6ada60932..c4d5595dd38e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3457,10 +3457,6 @@ static __be32 nfsd4_encode_splice_read(
 	__be32 nfserr;
 	__be32 *p = xdr->p - 2;
 
-	/* Make sure there will be room for padding if needed */
-	if (xdr->end - xdr->p < 1)
-		return nfserr_resource;
-
 	nfserr = nfsd_splice_read(read->rd_rqstp, read->rd_fhp,
 				  file, read->rd_offset, &maxcount, &eof);
 	read->rd_length = maxcount;
@@ -3470,32 +3466,18 @@ static __be32 nfsd4_encode_splice_read(
 		 * page length; reset it so as not to confuse
 		 * xdr_truncate_encode:
 		 */
-		buf->page_len = 0;
+		xdr_buf_set_pagelen(buf, 0);
 		return nfserr;
 	}
 
 	*(p++) = htonl(eof);
 	*(p++) = htonl(maxcount);
 
-	buf->page_len = maxcount;
-	buf->len += maxcount;
+	xdr_buf_set_pagelen(buf, maxcount);
+	buf->len = xdr_buf_msglen(buf);
 	xdr->page_ptr += (buf->page_base + maxcount + PAGE_SIZE - 1)
 							/ PAGE_SIZE;
 
-	/* Use rest of head for padding and remaining ops: */
-	buf->tail[0].iov_base = xdr->p;
-	buf->tail[0].iov_len = 0;
-	xdr->iov = buf->tail;
-	if (maxcount&3) {
-		int pad = 4 - (maxcount&3);
-
-		*(xdr->p++) = 0;
-
-		buf->tail[0].iov_base += maxcount&3;
-		buf->tail[0].iov_len = pad;
-		buf->len += pad;
-	}
-
 	space_left = min_t(int, (void *)xdr->end - (void *)xdr->p,
 				buf->buflen - buf->len);
 	buf->buflen = buf->len + space_left;
@@ -3518,8 +3500,6 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 	__be32 nfserr;
 	__be32 tmp;
 	__be32 *p;
-	u32 zzz = 0;
-	int pad;
 
 	/* Ensure xdr_reserve_space behaves itself */
 	if (xdr->iov == xdr->buf->head) {
@@ -3531,7 +3511,7 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 	v = 0;
 	while (len) {
 		thislen = min_t(long, len, PAGE_SIZE);
-		p = xdr_reserve_space(xdr, (thislen+3)&~3);
+		p = xdr_reserve_space(xdr, thislen);
 		WARN_ON_ONCE(!p);
 		resp->rqstp->rq_vec[v].iov_base = p;
 		resp->rqstp->rq_vec[v].iov_len = thislen;
@@ -3540,23 +3520,18 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 	}
 	read->rd_vlen = v;
 
-	len = maxcount;
 	nfserr = nfsd_readv(resp->rqstp, read->rd_fhp, file, read->rd_offset,
 			    resp->rqstp->rq_vec, read->rd_vlen, &maxcount,
 			    &eof);
 	read->rd_length = maxcount;
 	if (nfserr)
 		return nfserr;
-	xdr_truncate_encode(xdr, starting_len + 8 + ((maxcount+3)&~3));
+	xdr_truncate_encode(xdr, starting_len + 8 + maxcount);
 
 	tmp = htonl(eof);
 	write_bytes_to_xdr_buf(xdr->buf, starting_len    , &tmp, 4);
 	tmp = htonl(maxcount);
 	write_bytes_to_xdr_buf(xdr->buf, starting_len + 4, &tmp, 4);
-
-	pad = (maxcount&3) ? 4 - (maxcount&3) : 0;
-	write_bytes_to_xdr_buf(xdr->buf, starting_len + 8 + maxcount,
-								&zzz, pad);
 	return 0;
 
 }
@@ -3610,8 +3585,7 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_readlink *readlink)
 {
 	int maxcount;
-	__be32 wire_count;
-	int zero = 0;
+	__be32 tmp;
 	struct xdr_stream *xdr = &resp->xdr;
 	int length_offset = xdr->buf->len;
 	__be32 *p;
@@ -3621,9 +3595,12 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 		return nfserr_resource;
 	maxcount = PAGE_SIZE;
 
+	/* XXX: This is probably going to result
+	 * in the same bad behavior of RPC/RDMA */
 	p = xdr_reserve_space(xdr, maxcount);
 	if (!p)
 		return nfserr_resource;
+
 	/*
 	 * XXX: By default, vfs_readlink() will truncate symlinks if they
 	 * would overflow the buffer.  Is this kosher in NFSv4?  If not, one
@@ -3639,12 +3616,9 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 		return nfserr;
 	}
 
-	wire_count = htonl(maxcount);
-	write_bytes_to_xdr_buf(xdr->buf, length_offset, &wire_count, 4);
-	xdr_truncate_encode(xdr, length_offset + 4 + ALIGN(maxcount, 4));
-	if (maxcount & 3)
-		write_bytes_to_xdr_buf(xdr->buf, length_offset + 4 + maxcount,
-						&zero, 4 - (maxcount&3));
+	tmp = cpu_to_be32(maxcount);
+	write_bytes_to_xdr_buf(xdr->buf, length_offset, &tmp, 4);
+	xdr_truncate_encode(xdr, length_offset + 4 + xdr_align_size(maxcount));
 	return 0;
 }
 
@@ -3718,6 +3692,7 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
 	}
 	if (nfserr)
 		goto err_no_verf;
+	WARN_ON(maxcount != xdr_align_size(maxcount));
 
 	if (readdir->cookie_offset) {
 		wire_offset = cpu_to_be64(offset);
@@ -4568,10 +4543,6 @@ void nfsd4_release_compoundargs(struct svc_rqst *rqstp)
 	 * All that remains is to write the tag and operation count...
 	 */
 	struct nfsd4_compoundres *resp = rqstp->rq_resp;
-	struct xdr_buf *buf = resp->xdr.buf;
-
-	WARN_ON_ONCE(buf->len != buf->head[0].iov_len + buf->page_len +
-				 buf->tail[0].iov_len);
 
 	rqstp->rq_next_page = resp->xdr.page_ptr + 1;
 
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index f3104be8ff5d..d2dadb200024 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -675,15 +675,15 @@ void xdr_truncate_encode(struct xdr_stream *xdr, size_t len)
 	int fraglen;
 	int new;
 
-	if (len > buf->len) {
+	if (len > xdr_buf_msglen(buf)) {
 		WARN_ON_ONCE(1);
 		return;
 	}
 	xdr_commit_encode(xdr);
 
-	fraglen = min_t(int, buf->len - len, tail->iov_len);
+	fraglen = min_t(int, xdr_buf_msglen(buf) - len, tail->iov_len);
 	tail->iov_len -= fraglen;
-	buf->len -= fraglen;
+	buf->len = xdr_buf_msglen(buf);
 	if (tail->iov_len) {
 		xdr->p = tail->iov_base + tail->iov_len;
 		WARN_ON_ONCE(!xdr->end);
@@ -691,12 +691,12 @@ void xdr_truncate_encode(struct xdr_stream *xdr, size_t len)
 		return;
 	}
 	WARN_ON_ONCE(fraglen);
-	fraglen = min_t(int, buf->len - len, buf->page_len);
+	fraglen = min_t(int, xdr_buf_msglen(buf) - len, buf->page_len);
 	buf->page_len -= fraglen;
-	buf->len -= fraglen;
+	buf->page_pad = xdr_pad_size(buf->page_len);
+	buf->len = xdr_buf_msglen(buf);
 
 	new = buf->page_base + buf->page_len;
-
 	xdr->page_ptr = buf->pages + (new >> PAGE_SHIFT);
 
 	if (buf->page_len) {


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 8/8] SUNRPC: GSS support for automated padding of xdr_buf::pages
  2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
                   ` (6 preceding siblings ...)
  2020-01-29 16:10 ` [PATCH RFC 7/8] sunrpc: Add new contractual constraint on xdr_buf API Chuck Lever
@ 2020-01-29 16:10 ` Chuck Lever
  7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-01-29 16:10 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/gss_krb5_crypto.c |   13 ++++----
 net/sunrpc/auth_gss/gss_krb5_wrap.c   |   11 ++++---
 net/sunrpc/auth_gss/svcauth_gss.c     |   51 +++++++++++++++++++--------------
 net/sunrpc/xdr.c                      |    3 ++
 4 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
index 6f2d30d7b766..eb5a43b61b42 100644
--- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
+++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
@@ -412,8 +412,9 @@
 	err = crypto_ahash_init(req);
 	if (err)
 		goto out;
-	err = xdr_process_buf(body, body_offset, body->len - body_offset,
-			      checksummer, req);
+	err = xdr_process_buf(body, body_offset,
+			      xdr_buf_msglen(body) - body_offset, checksummer,
+			      req);
 	if (err)
 		goto out;
 	if (header != NULL) {
@@ -682,12 +683,10 @@ struct decryptor_desc {
 	SYNC_SKCIPHER_REQUEST_ON_STACK(req, cipher);
 	u8 *data;
 	struct page **save_pages;
-	u32 len = buf->len - offset;
+	u32 len = xdr_buf_msglen(buf) - offset;
 
-	if (len > GSS_KRB5_MAX_BLOCKSIZE * 2) {
-		WARN_ON(0);
+	if (len > GSS_KRB5_MAX_BLOCKSIZE * 2)
 		return -ENOMEM;
-	}
 	data = kmalloc(GSS_KRB5_MAX_BLOCKSIZE * 2, GFP_NOFS);
 	if (!data)
 		return -ENOMEM;
@@ -800,7 +799,7 @@ struct decryptor_desc {
 	if (err)
 		return GSS_S_FAILURE;
 
-	nbytes = buf->len - offset - GSS_KRB5_TOK_HDR_LEN;
+	nbytes = xdr_buf_msglen(buf) - offset - GSS_KRB5_TOK_HDR_LEN;
 	nblocks = (nbytes + blocksize - 1) / blocksize;
 	cbcbytes = 0;
 	if (nblocks > 2)
diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
index 14a0aff0cd84..8d71d561f430 100644
--- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
+++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
@@ -405,12 +405,13 @@ static void rotate_buf_a_little(struct xdr_buf *buf, unsigned int shift)
 	BUG_ON(shift > LOCAL_BUF_LEN);
 
 	read_bytes_from_xdr_buf(buf, 0, head, shift);
-	for (i = 0; i + shift < buf->len; i += LOCAL_BUF_LEN) {
-		this_len = min(LOCAL_BUF_LEN, buf->len - (i + shift));
+	for (i = 0; i + shift < xdr_buf_msglen(buf); i += LOCAL_BUF_LEN) {
+		this_len = min_t(unsigned int, LOCAL_BUF_LEN,
+				 xdr_buf_msglen(buf) - (i + shift));
 		read_bytes_from_xdr_buf(buf, i+shift, tmp, this_len);
 		write_bytes_to_xdr_buf(buf, i, tmp, this_len);
 	}
-	write_bytes_to_xdr_buf(buf, buf->len - shift, head, shift);
+	write_bytes_to_xdr_buf(buf, xdr_buf_msglen(buf) - shift, head, shift);
 }
 
 static void _rotate_left(struct xdr_buf *buf, unsigned int shift)
@@ -418,7 +419,7 @@ static void _rotate_left(struct xdr_buf *buf, unsigned int shift)
 	int shifted = 0;
 	int this_shift;
 
-	shift %= buf->len;
+	shift %= xdr_buf_msglen(buf);
 	while (shifted < shift) {
 		this_shift = min(shift - shifted, LOCAL_BUF_LEN);
 		rotate_buf_a_little(buf, this_shift);
@@ -430,7 +431,7 @@ static void rotate_left(u32 base, struct xdr_buf *buf, unsigned int shift)
 {
 	struct xdr_buf subbuf;
 
-	xdr_buf_subsegment(buf, &subbuf, base, buf->len - base);
+	xdr_buf_subsegment(buf, &subbuf, base, xdr_buf_msglen(buf) - base);
 	_rotate_left(&subbuf, shift);
 }
 
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index c62d1f10978b..893b9114cb8a 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -907,12 +907,6 @@ u32 svcauth_gss_flavor(struct auth_domain *dom)
 	return stat;
 }
 
-static inline int
-total_buf_len(struct xdr_buf *buf)
-{
-	return buf->head[0].iov_len + buf->page_len + buf->tail[0].iov_len;
-}
-
 static void
 fix_priv_head(struct xdr_buf *buf, int pad)
 {
@@ -941,7 +935,7 @@ u32 svcauth_gss_flavor(struct auth_domain *dom)
 	/* buf->len is the number of bytes from the original start of the
 	 * request to the end, where head[0].iov_len is just the bytes
 	 * not yet read from the head, so these two values are different: */
-	remaining_len = total_buf_len(buf);
+	remaining_len = xdr_buf_msglen(buf);
 	if (priv_len > remaining_len)
 		return -EINVAL;
 	pad = remaining_len - priv_len;
@@ -961,7 +955,7 @@ u32 svcauth_gss_flavor(struct auth_domain *dom)
 	/* XXX: This is very inefficient.  It would be better to either do
 	 * this while we encrypt, or maybe in the receive code, if we can peak
 	 * ahead and work out the service and mechanism there. */
-	offset = buf->head[0].iov_len % 4;
+	offset = buf->head[0].iov_len & 3;
 	if (offset) {
 		buf->buflen = RPCSVC_MAXPAYLOAD;
 		xdr_shift_buf(buf, offset);
@@ -1671,12 +1665,30 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 	int integ_offset, integ_len;
 	int stat = -EINVAL;
 
+	/* Fill in pad bytes for xdr_buf::pages: */
+	if (resbuf->page_pad) {
+		if (resbuf->tail[0].iov_base != NULL) {
+			memmove((u8 *)resbuf->tail[0].iov_base + sizeof(__be32),
+				resbuf->tail[0].iov_base,
+				resbuf->tail[0].iov_len);
+		} else {
+			resbuf->tail[0].iov_base =
+					(u8 *)resbuf->head[0].iov_base +
+					resbuf->head[0].iov_len;
+			resbuf->tail[0].iov_len = 0;
+		}
+		memset(resbuf->tail[0].iov_base, 0, sizeof(__be32));
+		resbuf->tail[0].iov_base = (u8 *)resbuf->tail[0].iov_base +
+					   (4 - resbuf->page_pad);
+		resbuf->tail[0].iov_len += resbuf->page_pad;
+		resbuf->page_pad = 0;
+	}
+
 	p = svcauth_gss_prepare_to_wrap(resbuf, gsd);
 	if (p == NULL)
 		goto out;
 	integ_offset = (u8 *)(p + 1) - (u8 *)resbuf->head[0].iov_base;
-	integ_len = resbuf->len - integ_offset;
-	BUG_ON(integ_len % 4);
+	integ_len = xdr_buf_msglen(resbuf) - integ_offset;
 	*p++ = htonl(integ_len);
 	*p++ = htonl(gc->gc_seq);
 	if (xdr_buf_subsegment(resbuf, &integ_buf, integ_offset, integ_len)) {
@@ -1716,7 +1728,6 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 	struct page **inpages = NULL;
 	__be32 *p, *len;
 	int offset;
-	int pad;
 
 	p = svcauth_gss_prepare_to_wrap(resbuf, gsd);
 	if (p == NULL)
@@ -1735,7 +1746,7 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 	 * there is RPC_MAX_AUTH_SIZE slack space available in
 	 * both the head and tail.
 	 */
-	if (resbuf->tail[0].iov_base) {
+	if (resbuf->tail[0].iov_base != NULL) {
 		BUG_ON(resbuf->tail[0].iov_base >= resbuf->head[0].iov_base
 							+ PAGE_SIZE);
 		BUG_ON(resbuf->tail[0].iov_base < resbuf->head[0].iov_base);
@@ -1746,6 +1757,7 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 			resbuf->tail[0].iov_base,
 			resbuf->tail[0].iov_len);
 		resbuf->tail[0].iov_base += RPC_MAX_AUTH_SIZE;
+		/* XXX: insert padding for resbuf->pages */
 	}
 	/*
 	 * If there is no current tail data, make sure there is
@@ -1754,21 +1766,18 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 	 * is RPC_MAX_AUTH_SIZE slack space available in both the
 	 * head and tail.
 	 */
-	if (resbuf->tail[0].iov_base == NULL) {
+	else {
 		if (resbuf->head[0].iov_len + 2*RPC_MAX_AUTH_SIZE > PAGE_SIZE)
 			return -ENOMEM;
 		resbuf->tail[0].iov_base = resbuf->head[0].iov_base
 			+ resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE;
-		resbuf->tail[0].iov_len = 0;
+		memset(resbuf->tail[0].iov_base, 0, sizeof(__be32));
+		resbuf->tail[0].iov_len = resbuf->page_pad;
+		resbuf->page_pad = 0;
 	}
 	if (gss_wrap(gsd->rsci->mechctx, offset, resbuf, inpages))
 		return -ENOMEM;
-	*len = htonl(resbuf->len - offset);
-	pad = 3 - ((resbuf->len - offset - 1)&3);
-	p = (__be32 *)(resbuf->tail[0].iov_base + resbuf->tail[0].iov_len);
-	memset(p, 0, pad);
-	resbuf->tail[0].iov_len += pad;
-	resbuf->len += pad;
+	*len = cpu_to_be32(xdr_buf_msglen(resbuf) - offset);
 	return 0;
 }
 
@@ -1789,7 +1798,7 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 	/* normally not set till svc_send, but we need it here: */
 	/* XXX: what for?  Do we mess it up the moment we call svc_putu32
 	 * or whatever? */
-	resbuf->len = total_buf_len(resbuf);
+	resbuf->len = xdr_buf_msglen(resbuf);
 	switch (gc->gc_svc) {
 	case RPC_GSS_SVC_NONE:
 		break;
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index d2dadb200024..798ebb406058 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1132,6 +1132,9 @@ void xdr_enter_page(struct xdr_stream *xdr, unsigned int len)
 		base -= buf->page_len;
 		subbuf->page_len = 0;
 	}
+	/* XXX: Still need to deal with case where buf->page_pad is non-zero */
+	WARN_ON(buf->page_pad);
+	subbuf->page_pad = 0;
 
 	if (base < buf->tail[0].iov_len) {
 		subbuf->tail[0].iov_base = buf->tail[0].iov_base + base;


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-01-29 16:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 16:09 [PATCH RFC 0/8] For discussion of bug 198053 Chuck Lever
2020-01-29 16:09 ` [PATCH RFC 1/8] nfsd: Fix NFSv4 READ on RDMA when using readv Chuck Lever
2020-01-29 16:09 ` [PATCH RFC 2/8] SUNRPC: Add XDR infrastructure for automatically padding xdr_buf::pages Chuck Lever
2020-01-29 16:09 ` [PATCH RFC 3/8] SUNRPC: TCP transport support for automated padding of xdr_buf::pages Chuck Lever
2020-01-29 16:09 ` [PATCH RFC 4/8] svcrdma: RDMA " Chuck Lever
2020-01-29 16:09 ` [PATCH RFC 5/8] NFSD: NFSv2 " Chuck Lever
2020-01-29 16:09 ` [PATCH RFC 6/8] NFSD: NFSv3 " Chuck Lever
2020-01-29 16:10 ` [PATCH RFC 7/8] sunrpc: Add new contractual constraint on xdr_buf API Chuck Lever
2020-01-29 16:10 ` [PATCH RFC 8/8] SUNRPC: GSS support for automated padding of xdr_buf::pages Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).