linux-next: manual merge of the nfs-anna tree with the nfs tree

linux-next: manual merge of the nfs-anna tree with the nfs tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 4054 bytes --]

Hi all,

Today's linux-next merge of the nfs-anna tree got a conflict in:

  net/sunrpc/xprtrdma/rpc_rdma.c

between commit:

  a0db57fee35ec ("xprtrdma: Fix Write chunk payload corruption on a short reply header")

from the nfs tree and commit:

  9ee534a759123 ("xprtrdma: trim head iovec when the payload arrives via a Write chunk")

from the nfs-anna tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

This one definitely needs double checking.

diff --combined net/sunrpc/xprtrdma/rpc_rdma.c
index ba8bdca8a49fb,7e5713367291e..0000000000000
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@@ -984,12 -984,16 +984,16 @@@ void rpcrdma_reset_cwnd(struct rpcrdma_
   * @rqst: controlling RPC request
   * @srcp: points to RPC message payload in receive buffer
   * @copy_len: remaining length of receive buffer content
-  * @pad: Write chunk pad bytes needed (zero for pure inline)
+  * @writelist: bytes conveyed by the Write chunk (zero for pure inline)
   *
   * The upper layer has set the maximum number of bytes it can
   * receive in each component of rq_rcv_buf. These values are set in
   * the head.iov_len, page_len, tail.iov_len, and buflen fields.
   *
+  * For READ replies with a Write chunk, the payload is already in
+  * the page list. An overestimated head length and buflen can be
+  * reduced without moving reply data.
+  *
   * Unlike the TCP equivalent (xdr_partial_copy_from_skb), in
   * many cases this function simply updates iov_base pointers in
   * rq_rcv_buf to point directly to the received reply data, to
@@@ -998,13 -1002,16 +1002,16 @@@
   * Returns the count of bytes which had to be memcopied.
   */
  static unsigned long
- rpcrdma_inline_fixup(struct rpc_rqst *rqst, char *srcp, int copy_len, int pad)
+ rpcrdma_inline_fixup(struct rpc_rqst *rqst, char *srcp, int copy_len,
+ 		     u32 writelist)
  {
  	unsigned long fixup_copy_count;
  	int i, npages, curlen;
  	char *destp;
  	struct page **ppages;
  	int page_base;
+ 	int pad = writelist & 3;
+ 	unsigned int delta;
  
  	/* The head iovec is redirected to the RPC reply message
  	 * in the receive buffer, to avoid a memcopy.
@@@ -1016,8 -1023,20 +1023,20 @@@
  	 * head.iov_len bytes are copied into the page list.
  	 */
  	curlen = rqst->rq_rcv_buf.head[0].iov_len;
- 	if (curlen > copy_len)
+ 	if (curlen > copy_len) {
+ 		/* These READ replies have no inline fields after the payload.
+ 		 * Trim the head without moving the directly-placed data.
+ 		 */
+ 		if (writelist) {
+ 			delta = curlen - copy_len;
+ 
+ 			rqst->rq_rcv_buf.head[0].iov_len = copy_len;
+ 			rqst->rq_private_buf.head[0].iov_len = copy_len;
+ 			rqst->rq_rcv_buf.buflen -= delta;
+ 			rqst->rq_private_buf.buflen -= delta;
+ 		}
  		curlen = copy_len;
+ 	}
  	srcp += curlen;
  	copy_len -= curlen;
  
@@@ -1243,19 -1262,9 +1262,19 @@@ rpcrdma_decode_msg(struct rpcrdma_xprt 
  
  	/* Build the RPC reply's Payload stream in rqst->rq_rcv_buf */
  	base = (char *)xdr_inline_decode(xdr, 0);
 -	rpclen = xdr_stream_remaining(xdr);
 +	rpclen = rep->rr_hdrbuf.len - xdr_stream_pos(xdr);
 +	if (writelist)
 +		rpclen &= ~3;
 +	if (unlikely(!rpclen))
 +		return -EIO;
 +	if (writelist && rpclen < rqst->rq_rcv_buf.head[0].iov_len) {
 +		rqst->rq_rcv_buf.head[0].iov_len = rpclen;
 +		rqst->rq_private_buf.head[0].iov_len = rpclen;
 +	}
 +
  	r_xprt->rx_stats.fixup_copy_count +=
 -		rpcrdma_inline_fixup(rqst, base, rpclen, writelist);
 +		rpcrdma_inline_fixup(rqst, base, rpclen,
 +				     xdr_pad_size(writelist));
  
  	r_xprt->rx_stats.total_rdma_reply += writelist;
  	return rpclen + xdr_align_size(writelist);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the nfs-anna tree with the nfs tree

[Tim Menninger]

I tested the original reproducer on v7.3-rc4 and confirmed the issue was
present. I then applied the combined patch from Mark's merge resolution,
and the reproducer passed.

So at least for the issue I originally reported, the merge resolution
appears correct.

Thanks, Tim

[PATCH linux-next] xprtrdma: fix up the nfs/nfs-anna merge of the Write chunk head trim

[Sasha Levin]

Two fixes for the same Write chunk payload corruption meet in
linux-next. In the nfs tree, commit a0db57fee35e ("xprtrdma: Fix Write
chunk payload corruption on a short reply header") trims
head[0].iov_len in rpcrdma_decode_msg(). In the nfs-anna tree,
commit 9ee534a75912 ("xprtrdma: trim head iovec when the payload
arrives via a Write chunk") trims it in rpcrdma_inline_fixup() and
reduces buflen by the same amount.

The merge resolution keeps the caller-side trim. By the time
rpcrdma_inline_fixup() runs, head[0].iov_len already equals copy_len,
so the block added by the nfs-anna commit never runs and its buflen
reduction in rq_rcv_buf and rq_private_buf is lost. The helper is also
still passed xdr_pad_size(writelist), while its kernel-doc now
describes @writelist as the number of bytes conveyed by the Write
chunk.

Drop the caller-side trim and pass the full Write chunk length, so
that rpcrdma_inline_fixup() is the only place that trims head[0] and
reduces buflen. Compute the pad with xdr_pad_size() in the helper, so
the padding from the nfs commit stays correct for lengths that are not
a multiple of four. Its rpclen calculation and empty reply check stay
in rpcrdma_decode_msg().

Fixes: 53ab189bdc14 ("Merge branch 'linux-next' of git://git.linux-nfs.org/projects/anna/linux-nfs.git")
Link: https://lore.kernel.org/all/as4aacUf2Q78FMvY@sirena.org.uk/
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Mark, this applies on top of next-20261020 and is meant to be folded into the
nfs-anna merge resolution.

Tim's reproducer passes with either resolution, since head[0] is trimmed in
both. The only behavioral difference is the buflen reduction. I found no reader
on the Write chunk READ decode path that uses the extra buflen, so this
restores the nfs-anna buflen change as reviewed rather than fixing an observed
failure.

Chuck, Anna, Trond: does this match what you want Linus to see?  Neither tree
is in mainline yet, so whichever pull request goes second will hit this
conflict.

This comes from our llminus-assisted integration tree. Build tested with W=1
only; I have no RPC/RDMA setup to run it.

 net/sunrpc/xprtrdma/rpc_rdma.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index c0d5042f3b660..69d4e35d49ef5 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -1010,7 +1010,7 @@ rpcrdma_inline_fixup(struct rpc_rqst *rqst, char *srcp, int copy_len,
 	char *destp;
 	struct page **ppages;
 	int page_base;
-	int pad = writelist & 3;
+	int pad = xdr_pad_size(writelist);
 	unsigned int delta;
 
 	/* The head iovec is redirected to the RPC reply message
@@ -1267,14 +1267,9 @@ rpcrdma_decode_msg(struct rpcrdma_xprt *r_xprt, struct rpcrdma_rep *rep,
 		rpclen &= ~3;
 	if (unlikely(!rpclen))
 		return -EIO;
-	if (writelist && rpclen < rqst->rq_rcv_buf.head[0].iov_len) {
-		rqst->rq_rcv_buf.head[0].iov_len = rpclen;
-		rqst->rq_private_buf.head[0].iov_len = rpclen;
-	}
 
 	r_xprt->rx_stats.fixup_copy_count +=
-		rpcrdma_inline_fixup(rqst, base, rpclen,
-				     xdr_pad_size(writelist));
+		rpcrdma_inline_fixup(rqst, base, rpclen, writelist);
 
 	r_xprt->rx_stats.total_rdma_reply += writelist;
 	return rpclen + xdr_align_size(writelist);
-- 
2.53.0