[PATCH v2] svcrdma: Ignore source port when computing DRC hash

[PATCH v2] svcrdma: Ignore source port when computing DRC hash

[Chuck Lever]

The DRC appears to be effectively empty after an RPC/RDMA transport
reconnect. The problem is that each connection uses a different
source port, which defeats the DRC hash.

Clients always have to disconnect before they send retransmissions
to reset the connection's credit accounting, thus every retransmit
on NFS/RDMA will miss the DRC.

An NFS/RDMA client's IP source port is meaningless for RDMA
transports. The transport layer typically sets the source port value
on the connection to a random ephemeral port. The server already
ignores it for the "secure port" check. See commit 16e4d93f6de7
("NFSD: Ignore client's source port on RDMA transports").

The Linux NFS server's DRC resolves XID collisions from the same
source IP address by using the checksum of the first 200 bytes of
the RPC call header.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Cc: stable@vger.kernel.org # v4.14+
---
 net/sunrpc/xprtrdma/svc_rdma_transport.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 027a3b0..0004535 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -211,9 +211,14 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id,
 	/* Save client advertised inbound read limit for use later in accept. */
 	newxprt->sc_ord = param->initiator_depth;
 
-	/* Set the local and remote addresses in the transport */
 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
 	svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
+	/* The remote port is arbitrary and not under the control of the
+	 * client ULP. Set it to a fixed value so that the DRC continues
+	 * to be effective after a reconnect.
+	 */
+	rpc_set_port((struct sockaddr *)&newxprt->sc_xprt.xpt_remote, 0);
+
 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
 	svc_xprt_set_local(&newxprt->sc_xprt, sa, svc_addr_len(sa));
 

Re: [PATCH v2] svcrdma: Ignore source port when computing DRC hash

[Chuck Lever]

> On Jun 11, 2019, at 11:01 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> The DRC appears to be effectively empty after an RPC/RDMA transport
> reconnect. The problem is that each connection uses a different
> source port, which defeats the DRC hash.
> 
> Clients always have to disconnect before they send retransmissions
> to reset the connection's credit accounting, thus every retransmit
> on NFS/RDMA will miss the DRC.
> 
> An NFS/RDMA client's IP source port is meaningless for RDMA
> transports. The transport layer typically sets the source port value
> on the connection to a random ephemeral port. The server already
> ignores it for the "secure port" check. See commit 16e4d93f6de7
> ("NFSD: Ignore client's source port on RDMA transports").
> 
> The Linux NFS server's DRC resolves XID collisions from the same
> source IP address by using the checksum of the first 200 bytes of
> the RPC call header.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> Cc: stable@vger.kernel.org # v4.14+
> ---
> net/sunrpc/xprtrdma/svc_rdma_transport.c |    7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> index 027a3b0..0004535 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -211,9 +211,14 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id,
> 	/* Save client advertised inbound read limit for use later in accept. */
> 	newxprt->sc_ord = param->initiator_depth;
> 
> -	/* Set the local and remote addresses in the transport */
> 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
> 	svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
> +	/* The remote port is arbitrary and not under the control of the
> +	 * client ULP. Set it to a fixed value so that the DRC continues
> +	 * to be effective after a reconnect.
> +	 */
> +	rpc_set_port((struct sockaddr *)&newxprt->sc_xprt.xpt_remote, 0);
> +
> 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
> 	svc_xprt_set_local(&newxprt->sc_xprt, sa, svc_addr_len(sa));

Hi Bruce, what's the disposition of this patch?


--
Chuck Lever

Re: [PATCH v2] svcrdma: Ignore source port when computing DRC hash

[Bruce Fields]

On Wed, Jun 19, 2019 at 04:25:57PM -0400, Chuck Lever wrote:
> 
> > On Jun 11, 2019, at 11:01 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
> > 
> > The DRC appears to be effectively empty after an RPC/RDMA transport
> > reconnect. The problem is that each connection uses a different
> > source port, which defeats the DRC hash.
> > 
> > Clients always have to disconnect before they send retransmissions
> > to reset the connection's credit accounting, thus every retransmit
> > on NFS/RDMA will miss the DRC.
> > 
> > An NFS/RDMA client's IP source port is meaningless for RDMA
> > transports. The transport layer typically sets the source port value
> > on the connection to a random ephemeral port. The server already
> > ignores it for the "secure port" check. See commit 16e4d93f6de7
> > ("NFSD: Ignore client's source port on RDMA transports").
> > 
> > The Linux NFS server's DRC resolves XID collisions from the same
> > source IP address by using the checksum of the first 200 bytes of
> > the RPC call header.
> > 
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > Cc: stable@vger.kernel.org # v4.14+
> > ---
> > net/sunrpc/xprtrdma/svc_rdma_transport.c |    7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > index 027a3b0..0004535 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > @@ -211,9 +211,14 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id,
> > 	/* Save client advertised inbound read limit for use later in accept. */
> > 	newxprt->sc_ord = param->initiator_depth;
> > 
> > -	/* Set the local and remote addresses in the transport */
> > 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
> > 	svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
> > +	/* The remote port is arbitrary and not under the control of the
> > +	 * client ULP. Set it to a fixed value so that the DRC continues
> > +	 * to be effective after a reconnect.
> > +	 */
> > +	rpc_set_port((struct sockaddr *)&newxprt->sc_xprt.xpt_remote, 0);
> > +
> > 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
> > 	svc_xprt_set_local(&newxprt->sc_xprt, sa, svc_addr_len(sa));
> 
> Hi Bruce, what's the disposition of this patch?

I've queued it up locally for 5.2 and stable.  Apologies, it got a
little buried in my inbox, thanks for the reminder.

--b.

Re: [PATCH v2] svcrdma: Ignore source port when computing DRC hash

[Chuck Lever]

> On Jun 19, 2019, at 4:43 PM, Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Wed, Jun 19, 2019 at 04:25:57PM -0400, Chuck Lever wrote:
>> 
>>> On Jun 11, 2019, at 11:01 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>> The DRC appears to be effectively empty after an RPC/RDMA transport
>>> reconnect. The problem is that each connection uses a different
>>> source port, which defeats the DRC hash.
>>> 
>>> Clients always have to disconnect before they send retransmissions
>>> to reset the connection's credit accounting, thus every retransmit
>>> on NFS/RDMA will miss the DRC.
>>> 
>>> An NFS/RDMA client's IP source port is meaningless for RDMA
>>> transports. The transport layer typically sets the source port value
>>> on the connection to a random ephemeral port. The server already
>>> ignores it for the "secure port" check. See commit 16e4d93f6de7
>>> ("NFSD: Ignore client's source port on RDMA transports").
>>> 
>>> The Linux NFS server's DRC resolves XID collisions from the same
>>> source IP address by using the checksum of the first 200 bytes of
>>> the RPC call header.
>>> 
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> Cc: stable@vger.kernel.org # v4.14+
>>> ---
>>> net/sunrpc/xprtrdma/svc_rdma_transport.c |    7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> index 027a3b0..0004535 100644
>>> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> @@ -211,9 +211,14 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id,
>>> 	/* Save client advertised inbound read limit for use later in accept. */
>>> 	newxprt->sc_ord = param->initiator_depth;
>>> 
>>> -	/* Set the local and remote addresses in the transport */
>>> 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
>>> 	svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
>>> +	/* The remote port is arbitrary and not under the control of the
>>> +	 * client ULP. Set it to a fixed value so that the DRC continues
>>> +	 * to be effective after a reconnect.
>>> +	 */
>>> +	rpc_set_port((struct sockaddr *)&newxprt->sc_xprt.xpt_remote, 0);
>>> +
>>> 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
>>> 	svc_xprt_set_local(&newxprt->sc_xprt, sa, svc_addr_len(sa));
>> 
>> Hi Bruce, what's the disposition of this patch?
> 
> I've queued it up locally for 5.2 and stable.

Great, thank you!


> Apologies, it got a
> little buried in my inbox, thanks for the reminder.
> 
> --b.

--
Chuck Lever