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: <20210312211826.360959-1-Anna.Schumaker@Netapp.com>
In-Reply-To: <20210312211826.360959-1-Anna.Schumaker@Netapp.com>
From:   Olga Kornievskaia <aglo@umich.edu>
Date:   Wed, 24 Mar 2021 13:29:59 -0400
Message-ID: <CAN-5tyFvHN_OHr20cJpE6Y5OATxLkm=OVHLnUpZ3zDh_6yBXCw@mail.gmail.com>
Subject: Re: [PATCH v3 0/5] SUNRPC: Create sysfs files for changing IP address
To:     Anna Schumaker <schumaker.anna@gmail.com>
Cc:     linux-nfs <linux-nfs@vger.kernel.org>,
        Anna Schumaker <Anna.Schumaker@netapp.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Mar 12, 2021 at 4:19 PM <schumaker.anna@gmail.com> wrote:
>
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>
> It's possible for an NFS server to go down but come back up with a
> different IP address. These patches provide a way for administrators to
> handle this issue by providing a new IP address for xprt sockets to
> connect to.
>
> Chuck has suggested some ideas for future work that could also use this
> interface, such as:
> - srcaddr: To move between network devices on the client
> - type: "tcp", "rdma", "local"
> - bound: 0 for autobind, or the result of the most recent rpcbind query
> - connected: either true or false
> - last: read-only timestamp of the last operation to use the transport
> - device: A symlink to the physical network device
>
> Changes in v3:
> - Rename functions and objects to make future expansion easier
> - Put files under /sys/kernel/sunrpc/client/ instead of
>   /sys/kernel/sunrpc/net/, again for future expansions
> - Clean up use of WARN_ON_ONCE() in xs_connect()
> - Fix up locking, reference counting, and RCU usage
> - Unconditionally create files so userspace tools don't need to guess
>   what is supported (We return an error message now instead)
>
> Changes in v2:
> - Put files under /sys/kernel/sunrpc/ instead of /sys/net/sunrpc/
> - Rename file from "address" to "dstaddr"
>
> Thoughts?

Reviewed-by/Tested-by this version. Works OK for me.

I would like to note that the interface doesn't or rather perhaps
cannot do any error checking. So if the "user" were to echo a
nonsensical data into the sysfs (echo foobar > <sysfspath>), that
breaks the existing connection. However, if a proper IP were to be
entered to correct it, things will go back to normal.

> Anna
>
>
> Anna Schumaker (5):
>   sunrpc: Create a sunrpc directory under /sys/kernel/
>   sunrpc: Create a client/ subdirectory in the sunrpc sysfs
>   sunrpc: Create per-rpc_clnt sysfs kobjects
>   sunrpc: Prepare xs_connect() for taking NULL tasks
>   sunrpc: Create a per-rpc_clnt file for managing the destination IP
>     address
>
>  include/linux/sunrpc/clnt.h |   1 +
>  net/sunrpc/Makefile         |   2 +-
>  net/sunrpc/clnt.c           |   5 +
>  net/sunrpc/sunrpc_syms.c    |   8 ++
>  net/sunrpc/sysfs.c          | 191 ++++++++++++++++++++++++++++++++++++
>  net/sunrpc/sysfs.h          |  20 ++++
>  net/sunrpc/xprtsock.c       |   2 +-
>  7 files changed, 227 insertions(+), 2 deletions(-)
>  create mode 100644 net/sunrpc/sysfs.c
>  create mode 100644 net/sunrpc/sysfs.h
>
> --
> 2.29.2
>