Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210616011013.50547-1-olga.kornievskaia@gmail.com> <20210616011013.50547-7-olga.kornievskaia@gmail.com>
In-Reply-To: <20210616011013.50547-7-olga.kornievskaia@gmail.com>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Fri, 6 Aug 2021 16:12:22 -0400
Message-ID: <CAN-5tyGque598rzk2GQK3u=sWP35DHndJw50mWM-zfzF46L6oQ@mail.gmail.com>
Subject: Re: [PATCH v3 6/6] NFSv4 allow for nconnect value of trunkable transport
To:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        Chuck Lever <chuck.lever@oracle.com>
Cc:     linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello folks (Chuck, Trond have graciously participated previously and
I look forward to hearing back from you),

I would like to restart the conversation about the client's behaviour
with regards to discovered truncable transports.

I have put forth a version and patch 6 is the one that deals with the
combination of trunkable transport and nconnect and I look for
feedback from the community. I have written my views in the comments
in the patch.

My current position is: given that it's unclear how to reconcile
managing connections to the same IP together with connections to other
IPs. I propose we don't bring in nconnect  in play when truncable
transport is discovered (basically option #2 in my comments below). I
believe that's the simplest thing to do now and we can add nconnect
behaviour later.

Thank you.


On Tue, Jun 15, 2021 at 9:10 PM Olga Kornievskaia
<olga.kornievskaia@gmail.com> wrote:
>
> From: Olga Kornievskaia <kolga@netapp.com>
>
> If the new mount asked for nconnect mount, then create old client
> number of connections to the destination address that has been
> established as the same server with trunkable address.
>
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>
> --- There might be a number of objection to this patch. One
> I can think of is that this patch creates the nconnects based on
> whether or not the new mount asked for nconnect instead of
> unconditionally creating nconnect number of connections. The patch
> still creates nconnect connections based on the original value
> instead of picking the value of clp->cl_nconnect. I would have
> preferred that would be done. I don't see what can be wrong with
> using the new value. But I feared to go against what was objected
> before. My preference would be to (1) create clp->cl_nconnect
> connections or (2) not use this patch at all or (3) use as is
> here (meaning at least not create extra connections unless asked
> for by the mount).
> ---
>  fs/nfs/nfs4client.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index af57332503be..50fa9d53b444 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -427,6 +427,15 @@ static void nfs4_add_trunk(struct nfs_client *clp, struct nfs_client *old)
>
>         rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
>                           rpc_clnt_test_and_add_xprt, NULL);
> +
> +       if (clp->cl_nconnect > 1) {
> +               int i;
> +
> +               for (i = 0; i < old->cl_nconnect - 1; i++)
> +                       if (rpc_clnt_add_xprt(old->cl_rpcclient, &xprt_args,
> +                                             NULL, NULL) < 0)
> +                               break;
> +       }
>  }
>
>  /**
> --
> 2.27.0
>