Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
MIME-Version: 1.0
References: <20230927192712.317799-1-trondmy@kernel.org>
In-Reply-To: <20230927192712.317799-1-trondmy@kernel.org>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Thu, 28 Sep 2023 10:58:06 -0400
Message-ID: <CAN-5tyF9rKdu0D-7nUFQtq1BWQABb+mdY3sLrDY1-sU_Q2p8fA@mail.gmail.com>
Subject: Re: [PATCH] SUNRPC: Don't retry using the same source port if
 connection failed
To:     trondmy@kernel.org
Cc:     linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Sep 27, 2023 at 3:35=E2=80=AFPM <trondmy@kernel.org> wrote:
>
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> If the TCP connection attempt fails without ever establishing a
> connection, then assume the problem may be the server is rejecting us
> due to port reuse.

Doesn't this break 4.0 replay cache? Seems too general to assume that
any unsuccessful SYN was due to a server reboot and it's ok for the
client to change the port.

>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  net/sunrpc/xprtsock.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 71848ab90d13..1a96777f0ed5 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -62,6 +62,7 @@
>  #include "sunrpc.h"
>
>  static void xs_close(struct rpc_xprt *xprt);
> +static void xs_reset_srcport(struct sock_xprt *transport);
>  static void xs_set_srcport(struct sock_xprt *transport, struct socket *s=
ock);
>  static void xs_tcp_set_socket_timeouts(struct rpc_xprt *xprt,
>                 struct socket *sock);
> @@ -1565,8 +1566,10 @@ static void xs_tcp_state_change(struct sock *sk)
>                 break;
>         case TCP_CLOSE:
>                 if (test_and_clear_bit(XPRT_SOCK_CONNECTING,
> -                                       &transport->sock_state))
> +                                      &transport->sock_state)) {
> +                       xs_reset_srcport(transport);
>                         xprt_clear_connecting(xprt);
> +               }
>                 clear_bit(XPRT_CLOSING, &xprt->state);
>                 /* Trigger the socket release */
>                 xs_run_error_worker(transport, XPRT_SOCK_WAKE_DISCONNECT)=
;
> @@ -1722,6 +1725,11 @@ static void xs_set_port(struct rpc_xprt *xprt, uns=
igned short port)
>         xs_update_peer_port(xprt);
>  }
>
> +static void xs_reset_srcport(struct sock_xprt *transport)
> +{
> +       transport->srcport =3D 0;
> +}
> +
>  static void xs_set_srcport(struct sock_xprt *transport, struct socket *s=
ock)
>  {
>         if (transport->srcport =3D=3D 0 && transport->xprt.reuseport)
> --
> 2.41.0
>