[PATCH V3] svcrdma: advertise the correct max payload

[Steve Wise]

Svcrdma currently advertises 1MB, which is too large. The correct value
is the minimum of RPCSVC_MAXPAYLOAD and the max scatter-gather allowed
in an NFSRDMA IO chunk * the host page size. This bug is usually benign
because the Linux X64 NFSRDMA client correctly limits the payload size to
the correct value (64*4096 = 256KB). But if the Linux client is PPC64
with a 64KB page size, then the client will indeed use a payload size
that will overflow the server.

Signed-off-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+***@public.gmane.org>
---

net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
net/sunrpc/xprtrdma/xprt_rdma.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 374feb4..4e61880 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -91,7 +91,7 @@ struct svc_xprt_class svc_rdma_class = {
.xcl_name = "rdma",
.xcl_owner = THIS_MODULE,
.xcl_ops = &svc_rdma_ops,
- .xcl_max_payload = RPCSVC_MAXPAYLOAD_TCP,
+ .xcl_max_payload = RPCSVC_MAXPAYLOAD_RDMA,
.xcl_ident = XPRT_TRANSPORT_RDMA,
};

diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index c419498..ac7fc9a 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -51,6 +51,7 @@
#include <linux/sunrpc/clnt.h> /* rpc_xprt */
#include <linux/sunrpc/rpc_rdma.h> /* RPC/RDMA protocol */
#include <linux/sunrpc/xprtrdma.h> /* xprt parameters */
+#include <linux/sunrpc/svc.h> /* RPCSVC_MAXPAYLOAD */

#define RDMA_RESOLVE_TIMEOUT (5000) /* 5 seconds */
#define RDMA_CONNECT_RETRY_MAX (2) /* retries if no listener backlog */
@@ -392,4 +393,10 @@ extern struct kmem_cache *svc_rdma_ctxt_cachep;
/* Workqueue created in svc_rdma.c */
extern struct workqueue_struct *svc_rdma_wq;

+#if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_DATA_SEGS << PAGE_SHIFT)
+#define RPCSVC_MAXPAYLOAD_RDMA RPCSVC_MAXPAYLOAD
+#else
+#define RPCSVC_MAXPAYLOAD_RDMA (RPCRDMA_MAX_DATA_SEGS << PAGE_SHIFT)
+#endif
+
#endif /* _LINUX_SUNRPC_XPRT_RDMA_H */

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Sagi Grimberg]

Post by Steve Wise
Svcrdma currently advertises 1MB, which is too large. The correct value
is the minimum of RPCSVC_MAXPAYLOAD and the max scatter-gather allowed
in an NFSRDMA IO chunk * the host page size. This bug is usually benign
because the Linux X64 NFSRDMA client correctly limits the payload size to
the correct value (64*4096 = 256KB). But if the Linux client is PPC64
with a 64KB page size, then the client will indeed use a payload size
that will overflow the server.

Maybe I'm a bit late with this, but can you explain why SG table is
limited to 64 (fastreg MR and page_list probably bound it). Where is it
coming from? Naturally it needs to be bound by some figure, but why 64?

Sorry if I'm nit-picking on this pretty straight-forward bug fix...

Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Chuck Lever]

Post by Sagi Grimberg

Svcrdma currently advertises 1MB, which is too large. The correct v=

alue

Post by Sagi Grimberg

is the minimum of RPCSVC_MAXPAYLOAD and the max scatter-gather allow=

ed

Post by Sagi Grimberg

in an NFSRDMA IO chunk * the host page size. This bug is usually ben=

ign

Post by Sagi Grimberg

because the Linux X64 NFSRDMA client correctly limits the payload si=

ze to

Post by Sagi Grimberg

the correct value (64*4096 =3D 256KB). But if the Linux client is P=

PC64

Post by Sagi Grimberg

with a 64KB page size, then the client will indeed use a payload siz=

e

Post by Sagi Grimberg

that will overflow the server.
=20

=20
Maybe I'm a bit late with this, but can you explain why SG table is
limited to 64 (fastreg MR and page_list probably bound it). Where is =

it

Post by Sagi Grimberg
coming from? Naturally it needs to be bound by some figure, but why 6=

4?

It=92s arbitrary, AFAIK. It has been used to allocate some data structu=
res
on the stack, for example, so it can=92t be too large.

It=92s something we need to look at.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Steve Wise]

Hey Bruce, is this version acceptable for 3.18?

Thanks,

Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[J. Bruce Fields]

Post by Steve Wise
Hey Bruce, is this version acceptable for 3.18?

Yes, applying, thanks for the reminder.--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html