[PATCH v1 00/12] Handling session trunking group membership

[PATCH v1 00/12] Handling session trunking group membership

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Client needs to handle session trunking group membership changes that
occur when a particular server leaves an established trunked group.
This results in a server sending a NFS4ERR_BAD_SESSION because that
server no longer has session's state.

This series proposes to deal with that situation in two fold. First
on DESTROY_SESSION, the client will offline all trunked connections
it has established to the server. Then on CREATE_SESSION it will
iterate thru offlined connections only and probe them again for
session trunking. If session trunking conditions still hold then
such transport would be made active again otherwise it will be
deleted from the trunked group.

Olga Kornievskaia (12):
  SUNRPC expose functions for offline remote xprt functionality
  SUNRPC add function to offline remove trunkable transports
  NFSv4.1 offline trunkable transports on DESTROY_SESSION
  SUNRPC create an iterator to list only OFFLINE xprts
  SUNRPC parameterize rpc_clnt_iterate_for_each_xprt with iterator init
    function
  SUNRPC enable back offline transports in trunking discovery
  SUNRPC create an rpc function that allows xprt removal from rpc_clnt
  NFSv4.1 remove xprt from xprt_switch if session trunking test fails
  SUNRPC restructure rpc_clnt_setup_test_and_add_xprt
  SUNRPC export xprt_iter_rewind function
  SUNRPC create a function that probes only offline transports
  NFSv4.1 probe offline transports for trunking on session creation

 fs/nfs/nfs4proc.c                    |  18 ++-
 include/linux/sunrpc/clnt.h          |   7 +-
 include/linux/sunrpc/xprt.h          |   3 +
 include/linux/sunrpc/xprtmultipath.h |   7 +-
 net/sunrpc/clnt.c                    | 204 ++++++++++++++++++++++-----
 net/sunrpc/debugfs.c                 |   3 +-
 net/sunrpc/stats.c                   |   2 +-
 net/sunrpc/sysfs.c                   |  28 +---
 net/sunrpc/xprt.c                    |  35 +++++
 net/sunrpc/xprtmultipath.c           | 109 +++++++++++---
 10 files changed, 338 insertions(+), 78 deletions(-)

-- 
2.27.0

[PATCH v1 01/12] SUNRPC expose functions for offline remote xprt functionality

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Re-arrange the code that make offline transport and delete transport
callable functions.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/sunrpc/xprt.h |  3 +++
 net/sunrpc/sysfs.c          | 28 +++++-----------------------
 net/sunrpc/xprt.c           | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 522bbf937957..0d51b9f9ea37 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -505,4 +505,7 @@ static inline int xprt_test_and_set_binding(struct rpc_xprt *xprt)
 	return test_and_set_bit(XPRT_BINDING, &xprt->state);
 }
 
+void xprt_set_offline_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps);
+void xprt_set_online_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps);
+void xprt_delete_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps);
 #endif /* _LINUX_SUNRPC_XPRT_H */
diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index a3a2f8aeb80e..7330eb9a70cf 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -314,32 +314,14 @@ static ssize_t rpc_sysfs_xprt_state_change(struct kobject *kobj,
 		goto release_tasks;
 	}
 	if (offline) {
-		if (!test_and_set_bit(XPRT_OFFLINE, &xprt->state)) {
-			spin_lock(&xps->xps_lock);
-			xps->xps_nactive--;
-			spin_unlock(&xps->xps_lock);
-		}
+		xprt_set_offline_locked(xprt, xps);
 	} else if (online) {
-		if (test_and_clear_bit(XPRT_OFFLINE, &xprt->state)) {
-			spin_lock(&xps->xps_lock);
-			xps->xps_nactive++;
-			spin_unlock(&xps->xps_lock);
-		}
+		xprt_set_online_locked(xprt, xps);
 	} else if (remove) {
-		if (test_bit(XPRT_OFFLINE, &xprt->state)) {
-			if (!test_and_set_bit(XPRT_REMOVE, &xprt->state)) {
-				xprt_force_disconnect(xprt);
-				if (test_bit(XPRT_CONNECTED, &xprt->state)) {
-					if (!xprt->sending.qlen &&
-					    !xprt->pending.qlen &&
-					    !xprt->backlog.qlen &&
-					    !atomic_long_read(&xprt->queuelen))
-						rpc_xprt_switch_remove_xprt(xps, xprt);
-				}
-			}
-		} else {
+		if (test_bit(XPRT_OFFLINE, &xprt->state))
+			xprt_delete_locked(xprt, xps);
+		else
 			count = -EINVAL;
-		}
 	}
 
 release_tasks:
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 86d62cffba0d..6480ae324b27 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -2152,3 +2152,38 @@ void xprt_put(struct rpc_xprt *xprt)
 		kref_put(&xprt->kref, xprt_destroy_kref);
 }
 EXPORT_SYMBOL_GPL(xprt_put);
+
+void xprt_set_offline_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps)
+{
+	if (!test_and_set_bit(XPRT_OFFLINE, &xprt->state)) {
+		spin_lock(&xps->xps_lock);
+		xps->xps_nactive--;
+		spin_unlock(&xps->xps_lock);
+	}
+}
+EXPORT_SYMBOL(xprt_set_offline_locked);
+
+void xprt_set_online_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps)
+{
+	if (test_and_clear_bit(XPRT_OFFLINE, &xprt->state)) {
+		spin_lock(&xps->xps_lock);
+		xps->xps_nactive++;
+		spin_unlock(&xps->xps_lock);
+	}
+}
+EXPORT_SYMBOL(xprt_set_online_locked);
+
+void xprt_delete_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps)
+{
+	if (test_and_set_bit(XPRT_REMOVE, &xprt->state))
+		return;
+
+	xprt_force_disconnect(xprt);
+	if (!test_bit(XPRT_CONNECTED, &xprt->state))
+		return;
+
+	if (!xprt->sending.qlen && !xprt->pending.qlen &&
+	    !xprt->backlog.qlen && !atomic_long_read(&xprt->queuelen))
+		rpc_xprt_switch_remove_xprt(xps, xprt);
+}
+EXPORT_SYMBOL(xprt_delete_locked);
-- 
2.27.0

Re: [PATCH v1 01/12] SUNRPC expose functions for offline remote xprt functionality

[Trond Myklebust]

On Mon, 2022-06-20 at 11:23 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Re-arrange the code that make offline transport and delete transport
> callable functions.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  include/linux/sunrpc/xprt.h |  3 +++
>  net/sunrpc/sysfs.c          | 28 +++++-----------------------
>  net/sunrpc/xprt.c           | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/sunrpc/xprt.h
> b/include/linux/sunrpc/xprt.h
> index 522bbf937957..0d51b9f9ea37 100644
> --- a/include/linux/sunrpc/xprt.h
> +++ b/include/linux/sunrpc/xprt.h
> @@ -505,4 +505,7 @@ static inline int
> xprt_test_and_set_binding(struct rpc_xprt *xprt)
>         return test_and_set_bit(XPRT_BINDING, &xprt->state);
>  }
>  
> +void xprt_set_offline_locked(struct rpc_xprt *xprt, struct
> rpc_xprt_switch *xps);
> +void xprt_set_online_locked(struct rpc_xprt *xprt, struct
> rpc_xprt_switch *xps);
> +void xprt_delete_locked(struct rpc_xprt *xprt, struct
> rpc_xprt_switch *xps);
>  #endif /* _LINUX_SUNRPC_XPRT_H */
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index a3a2f8aeb80e..7330eb9a70cf 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -314,32 +314,14 @@ static ssize_t
> rpc_sysfs_xprt_state_change(struct kobject *kobj,
>                 goto release_tasks;
>         }
>         if (offline) {
> -               if (!test_and_set_bit(XPRT_OFFLINE, &xprt->state)) {
> -                       spin_lock(&xps->xps_lock);
> -                       xps->xps_nactive--;
> -                       spin_unlock(&xps->xps_lock);
> -               }
> +               xprt_set_offline_locked(xprt, xps);
>         } else if (online) {
> -               if (test_and_clear_bit(XPRT_OFFLINE, &xprt->state)) {
> -                       spin_lock(&xps->xps_lock);
> -                       xps->xps_nactive++;
> -                       spin_unlock(&xps->xps_lock);
> -               }
> +               xprt_set_online_locked(xprt, xps);
>         } else if (remove) {
> -               if (test_bit(XPRT_OFFLINE, &xprt->state)) {
> -                       if (!test_and_set_bit(XPRT_REMOVE, &xprt-
> >state)) {
> -                               xprt_force_disconnect(xprt);
> -                               if (test_bit(XPRT_CONNECTED, &xprt-
> >state)) {
> -                                       if (!xprt->sending.qlen &&
> -                                           !xprt->pending.qlen &&
> -                                           !xprt->backlog.qlen &&
> -                                           !atomic_long_read(&xprt-
> >queuelen))
> -
>                                                rpc_xprt_switch_remove_
> xprt(xps, xprt);
> -                               }
> -                       }
> -               } else {
> +               if (test_bit(XPRT_OFFLINE, &xprt->state))
> +                       xprt_delete_locked(xprt, xps);
> +               else
>                         count = -EINVAL;
> -               }
>         }
>  
>  release_tasks:
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index 86d62cffba0d..6480ae324b27 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -2152,3 +2152,38 @@ void xprt_put(struct rpc_xprt *xprt)
>                 kref_put(&xprt->kref, xprt_destroy_kref);
>  }
>  EXPORT_SYMBOL_GPL(xprt_put);
> +
> +void xprt_set_offline_locked(struct rpc_xprt *xprt, struct
> rpc_xprt_switch *xps)
> +{
> +       if (!test_and_set_bit(XPRT_OFFLINE, &xprt->state)) {
> +               spin_lock(&xps->xps_lock);
> +               xps->xps_nactive--;
> +               spin_unlock(&xps->xps_lock);
> +       }
> +}
> +EXPORT_SYMBOL(xprt_set_offline_locked);
> +
> +void xprt_set_online_locked(struct rpc_xprt *xprt, struct
> rpc_xprt_switch *xps)
> +{
> +       if (test_and_clear_bit(XPRT_OFFLINE, &xprt->state)) {
> +               spin_lock(&xps->xps_lock);
> +               xps->xps_nactive++;
> +               spin_unlock(&xps->xps_lock);
> +       }
> +}
> +EXPORT_SYMBOL(xprt_set_online_locked);
> +
> +void xprt_delete_locked(struct rpc_xprt *xprt, struct
> rpc_xprt_switch *xps)
> +{
> +       if (test_and_set_bit(XPRT_REMOVE, &xprt->state))
> +               return;
> +
> +       xprt_force_disconnect(xprt);
> +       if (!test_bit(XPRT_CONNECTED, &xprt->state))
> +               return;
> +
> +       if (!xprt->sending.qlen && !xprt->pending.qlen &&
> +           !xprt->backlog.qlen && !atomic_long_read(&xprt-
> >queuelen))
> +               rpc_xprt_switch_remove_xprt(xps, xprt);
> +}
> +EXPORT_SYMBOL(xprt_delete_locked);

Why are these functions being exported? There is nothing outside the
main sunrpc layer that should be using them.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: [PATCH v1 01/12] SUNRPC expose functions for offline remote xprt functionality

[Olga Kornievskaia]

On Tue, Jul 12, 2022 at 11:12 AM Trond Myklebust
<trondmy@hammerspace.com> wrote:
>
> On Mon, 2022-06-20 at 11:23 -0400, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > Re-arrange the code that make offline transport and delete transport
> > callable functions.
> >
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> >  include/linux/sunrpc/xprt.h |  3 +++
> >  net/sunrpc/sysfs.c          | 28 +++++-----------------------
> >  net/sunrpc/xprt.c           | 35 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 43 insertions(+), 23 deletions(-)
> >
> > diff --git a/include/linux/sunrpc/xprt.h
> > b/include/linux/sunrpc/xprt.h
> > index 522bbf937957..0d51b9f9ea37 100644
> > --- a/include/linux/sunrpc/xprt.h
> > +++ b/include/linux/sunrpc/xprt.h
> > @@ -505,4 +505,7 @@ static inline int
> > xprt_test_and_set_binding(struct rpc_xprt *xprt)
> >         return test_and_set_bit(XPRT_BINDING, &xprt->state);
> >  }
> >
> > +void xprt_set_offline_locked(struct rpc_xprt *xprt, struct
> > rpc_xprt_switch *xps);
> > +void xprt_set_online_locked(struct rpc_xprt *xprt, struct
> > rpc_xprt_switch *xps);
> > +void xprt_delete_locked(struct rpc_xprt *xprt, struct
> > rpc_xprt_switch *xps);
> >  #endif /* _LINUX_SUNRPC_XPRT_H */
> > diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> > index a3a2f8aeb80e..7330eb9a70cf 100644
> > --- a/net/sunrpc/sysfs.c
> > +++ b/net/sunrpc/sysfs.c
> > @@ -314,32 +314,14 @@ static ssize_t
> > rpc_sysfs_xprt_state_change(struct kobject *kobj,
> >                 goto release_tasks;
> >         }
> >         if (offline) {
> > -               if (!test_and_set_bit(XPRT_OFFLINE, &xprt->state)) {
> > -                       spin_lock(&xps->xps_lock);
> > -                       xps->xps_nactive--;
> > -                       spin_unlock(&xps->xps_lock);
> > -               }
> > +               xprt_set_offline_locked(xprt, xps);
> >         } else if (online) {
> > -               if (test_and_clear_bit(XPRT_OFFLINE, &xprt->state)) {
> > -                       spin_lock(&xps->xps_lock);
> > -                       xps->xps_nactive++;
> > -                       spin_unlock(&xps->xps_lock);
> > -               }
> > +               xprt_set_online_locked(xprt, xps);
> >         } else if (remove) {
> > -               if (test_bit(XPRT_OFFLINE, &xprt->state)) {
> > -                       if (!test_and_set_bit(XPRT_REMOVE, &xprt-
> > >state)) {
> > -                               xprt_force_disconnect(xprt);
> > -                               if (test_bit(XPRT_CONNECTED, &xprt-
> > >state)) {
> > -                                       if (!xprt->sending.qlen &&
> > -                                           !xprt->pending.qlen &&
> > -                                           !xprt->backlog.qlen &&
> > -                                           !atomic_long_read(&xprt-
> > >queuelen))
> > -
> >                                                rpc_xprt_switch_remove_
> > xprt(xps, xprt);
> > -                               }
> > -                       }
> > -               } else {
> > +               if (test_bit(XPRT_OFFLINE, &xprt->state))
> > +                       xprt_delete_locked(xprt, xps);
> > +               else
> >                         count = -EINVAL;
> > -               }
> >         }
> >
> >  release_tasks:
> > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > index 86d62cffba0d..6480ae324b27 100644
> > --- a/net/sunrpc/xprt.c
> > +++ b/net/sunrpc/xprt.c
> > @@ -2152,3 +2152,38 @@ void xprt_put(struct rpc_xprt *xprt)
> >                 kref_put(&xprt->kref, xprt_destroy_kref);
> >  }
> >  EXPORT_SYMBOL_GPL(xprt_put);
> > +
> > +void xprt_set_offline_locked(struct rpc_xprt *xprt, struct
> > rpc_xprt_switch *xps)
> > +{
> > +       if (!test_and_set_bit(XPRT_OFFLINE, &xprt->state)) {
> > +               spin_lock(&xps->xps_lock);
> > +               xps->xps_nactive--;
> > +               spin_unlock(&xps->xps_lock);
> > +       }
> > +}
> > +EXPORT_SYMBOL(xprt_set_offline_locked);
> > +
> > +void xprt_set_online_locked(struct rpc_xprt *xprt, struct
> > rpc_xprt_switch *xps)
> > +{
> > +       if (test_and_clear_bit(XPRT_OFFLINE, &xprt->state)) {
> > +               spin_lock(&xps->xps_lock);
> > +               xps->xps_nactive++;
> > +               spin_unlock(&xps->xps_lock);
> > +       }
> > +}
> > +EXPORT_SYMBOL(xprt_set_online_locked);
> > +
> > +void xprt_delete_locked(struct rpc_xprt *xprt, struct
> > rpc_xprt_switch *xps)
> > +{
> > +       if (test_and_set_bit(XPRT_REMOVE, &xprt->state))
> > +               return;
> > +
> > +       xprt_force_disconnect(xprt);
> > +       if (!test_bit(XPRT_CONNECTED, &xprt->state))
> > +               return;
> > +
> > +       if (!xprt->sending.qlen && !xprt->pending.qlen &&
> > +           !xprt->backlog.qlen && !atomic_long_read(&xprt-
> > >queuelen))
> > +               rpc_xprt_switch_remove_xprt(xps, xprt);
> > +}
> > +EXPORT_SYMBOL(xprt_delete_locked);
>
> Why are these functions being exported? There is nothing outside the
> main sunrpc layer that should be using them.
>

Thank you for the comment. Make sense, will fix. It probably came from
looking at xprt_put above that code.

> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
>
>

[PATCH v1 02/12] SUNRPC add function to offline remove trunkable transports

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Iterate thru available transports in the xprt_switch for all
trunkable transports offline and possibly remote them as well.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/sunrpc/clnt.h |  1 +
 net/sunrpc/clnt.c           | 42 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 90501404fa49..e74a0740603b 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -234,6 +234,7 @@ int		rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
 			struct rpc_xprt_switch *,
 			struct rpc_xprt *,
 			void *);
+void		rpc_clnt_manage_trunked_xprts(struct rpc_clnt *, void *);
 
 const char *rpc_proc_name(const struct rpc_task *task);
 
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index e2c6eca0271b..544b55a3aa20 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2999,6 +2999,48 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
 
+static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
+				    struct rpc_xprt *xprt,
+				    void *data)
+{
+	struct rpc_xprt *main_xprt;
+	struct rpc_xprt_switch *xps;
+	int err = 0;
+	int *offline_destroy = (int *)data;
+
+	xprt_get(xprt);
+
+	rcu_read_lock();
+	main_xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
+	xps = xprt_switch_get(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
+	err = rpc_cmp_addr_port((struct sockaddr *)&xprt->addr,
+				(struct sockaddr *)&main_xprt->addr);
+	rcu_read_unlock();
+	xprt_put(main_xprt);
+	if (err)
+		goto out;
+
+	if (wait_on_bit_lock(&xprt->state, XPRT_LOCKED, TASK_KILLABLE)) {
+		err = -EINTR;
+		goto out;
+	}
+	xprt_set_offline_locked(xprt, xps);
+	if (*offline_destroy)
+		xprt_delete_locked(xprt, xps);
+
+	xprt_release_write(xprt, NULL);
+out:
+	xprt_put(xprt);
+	xprt_switch_put(xps);
+	return err;
+}
+
+void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void *data)
+{
+	rpc_clnt_iterate_for_each_xprt(clnt, rpc_xprt_offline_destroy, data);
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
+
 struct connect_timeout_data {
 	unsigned long connect_timeout;
 	unsigned long reconnect_timeout;
-- 
2.27.0

Re: [PATCH v1 02/12] SUNRPC add function to offline remove trunkable transports

[Trond Myklebust]

On Mon, 2022-06-20 at 11:23 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Iterate thru available transports in the xprt_switch for all
> trunkable transports offline and possibly remote them as well.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  include/linux/sunrpc/clnt.h |  1 +
>  net/sunrpc/clnt.c           | 42
> +++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/include/linux/sunrpc/clnt.h
> b/include/linux/sunrpc/clnt.h
> index 90501404fa49..e74a0740603b 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -234,6 +234,7 @@
> int         rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
>                         struct rpc_xprt_switch *,
>                         struct rpc_xprt *,
>                         void *);
> +void           rpc_clnt_manage_trunked_xprts(struct rpc_clnt *, void
> *);
>  
>  const char *rpc_proc_name(const struct rpc_task *task);
>  
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index e2c6eca0271b..544b55a3aa20 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2999,6 +2999,48 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
>  
> +static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
> +                                   struct rpc_xprt *xprt,
> +                                   void *data)
> +{
> +       struct rpc_xprt *main_xprt;
> +       struct rpc_xprt_switch *xps;
> +       int err = 0;
> +       int *offline_destroy = (int *)data;
> +
> +       xprt_get(xprt);
> +
> +       rcu_read_lock();
> +       main_xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
> +       xps = xprt_switch_get(rcu_dereference(clnt-
> >cl_xpi.xpi_xpswitch));
> +       err = rpc_cmp_addr_port((struct sockaddr *)&xprt->addr,
> +                               (struct sockaddr *)&main_xprt->addr);
> +       rcu_read_unlock();
> +       xprt_put(main_xprt);
> +       if (err)
> +               goto out;
> +
> +       if (wait_on_bit_lock(&xprt->state, XPRT_LOCKED,
> TASK_KILLABLE)) {
> +               err = -EINTR;
> +               goto out;
> +       }
> +       xprt_set_offline_locked(xprt, xps);
> +       if (*offline_destroy)
> +               xprt_delete_locked(xprt, xps);
> +
> +       xprt_release_write(xprt, NULL);
> +out:
> +       xprt_put(xprt);
> +       xprt_switch_put(xps);
> +       return err;
> +}
> +
> +void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void
> *data)
> +{
> +       rpc_clnt_iterate_for_each_xprt(clnt,
> rpc_xprt_offline_destroy, data);
> +}
> +EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);

Why is this function taking a 'void *' argument when
rpc_xprt_offline_destroy() won't accept anything other than an 'int *'.
It would be much cleaner to have 2 separate functions, neither or which
need more than one argument. Then you can hide the pointer to the 'int'
in each function and avoid exposing it as part of the API.

In addition, a function like this that is intended for use by a
different layer needs a proper kerneldoc comment so that we know what
the API is for, and what it does.

> +
>  struct connect_timeout_data {
>         unsigned long connect_timeout;
>         unsigned long reconnect_timeout;

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: [PATCH v1 02/12] SUNRPC add function to offline remove trunkable transports

[Olga Kornievskaia]

On Tue, Jul 12, 2022 at 11:24 AM Trond Myklebust
<trondmy@hammerspace.com> wrote:
>
> On Mon, 2022-06-20 at 11:23 -0400, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > Iterate thru available transports in the xprt_switch for all
> > trunkable transports offline and possibly remote them as well.
> >
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> >  include/linux/sunrpc/clnt.h |  1 +
> >  net/sunrpc/clnt.c           | 42
> > +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/include/linux/sunrpc/clnt.h
> > b/include/linux/sunrpc/clnt.h
> > index 90501404fa49..e74a0740603b 100644
> > --- a/include/linux/sunrpc/clnt.h
> > +++ b/include/linux/sunrpc/clnt.h
> > @@ -234,6 +234,7 @@
> > int         rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
> >                         struct rpc_xprt_switch *,
> >                         struct rpc_xprt *,
> >                         void *);
> > +void           rpc_clnt_manage_trunked_xprts(struct rpc_clnt *, void
> > *);
> >
> >  const char *rpc_proc_name(const struct rpc_task *task);
> >
> > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > index e2c6eca0271b..544b55a3aa20 100644
> > --- a/net/sunrpc/clnt.c
> > +++ b/net/sunrpc/clnt.c
> > @@ -2999,6 +2999,48 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
> >  }
> >  EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
> >
> > +static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
> > +                                   struct rpc_xprt *xprt,
> > +                                   void *data)
> > +{
> > +       struct rpc_xprt *main_xprt;
> > +       struct rpc_xprt_switch *xps;
> > +       int err = 0;
> > +       int *offline_destroy = (int *)data;
> > +
> > +       xprt_get(xprt);
> > +
> > +       rcu_read_lock();
> > +       main_xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
> > +       xps = xprt_switch_get(rcu_dereference(clnt-
> > >cl_xpi.xpi_xpswitch));
> > +       err = rpc_cmp_addr_port((struct sockaddr *)&xprt->addr,
> > +                               (struct sockaddr *)&main_xprt->addr);
> > +       rcu_read_unlock();
> > +       xprt_put(main_xprt);
> > +       if (err)
> > +               goto out;
> > +
> > +       if (wait_on_bit_lock(&xprt->state, XPRT_LOCKED,
> > TASK_KILLABLE)) {
> > +               err = -EINTR;
> > +               goto out;
> > +       }
> > +       xprt_set_offline_locked(xprt, xps);
> > +       if (*offline_destroy)
> > +               xprt_delete_locked(xprt, xps);
> > +
> > +       xprt_release_write(xprt, NULL);
> > +out:
> > +       xprt_put(xprt);
> > +       xprt_switch_put(xps);
> > +       return err;
> > +}
> > +
> > +void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void
> > *data)
> > +{
> > +       rpc_clnt_iterate_for_each_xprt(clnt,
> > rpc_xprt_offline_destroy, data);
> > +}
> > +EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
>
> Why is this function taking a 'void *' argument when
> rpc_xprt_offline_destroy() won't accept anything other than an 'int *'.
> It would be much cleaner to have 2 separate functions, neither or which
> need more than one argument. Then you can hide the pointer to the 'int'
> in each function and avoid exposing it as part of the API.

I could remove the void * altogether. As the following code only
offlines the transports. I wrote this function to be generic to be
able to do either if the need arises. It wasn't clear to me what you
meant by "have 2 separate functions". If you mean one for offline and
another for destroy, then perhaps that removes that need too. However,
if we were to have a generic one then since the majority of the code
is the same I don't see how having 2 functions is better.

> In addition, a function like this that is intended for use by
> different layer needs a proper kerneldoc comment so that we know what
> the API is for, and what it does.

Will add a comment above the function to explain what it does.

>
> > +
> >  struct connect_timeout_data {
> >         unsigned long connect_timeout;
> >         unsigned long reconnect_timeout;
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
>
>

[PATCH v1 03/12] NFSv4.1 offline trunkable transports on DESTROY_SESSION

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

When session is destroy, some of the transports might no longer be
valid trunks for the new session. Offline existing transports.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c0fdcf8c0032..cf898bea3bfd 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9273,7 +9273,7 @@ int nfs4_proc_destroy_session(struct nfs4_session *session,
 		.rpc_argp = session,
 		.rpc_cred = cred,
 	};
-	int status = 0;
+	int status = 0, offline = 0;
 
 	/* session is still being setup */
 	if (!test_and_clear_bit(NFS4_SESSION_ESTABLISHED, &session->session_state))
@@ -9286,6 +9286,7 @@ int nfs4_proc_destroy_session(struct nfs4_session *session,
 	if (status)
 		dprintk("NFS: Got error %d from the server on DESTROY_SESSION. "
 			"Session has been destroyed regardless...\n", status);
+	rpc_clnt_manage_trunked_xprts(session->clp->cl_rpcclient, &offline);
 	return status;
 }
 
-- 
2.27.0

[PATCH v1 04/12] SUNRPC create an iterator to list only OFFLINE xprts

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Create a new iterator helper that will go thru the all the transports
in the switch and return transports that are marked OFFLINE.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/sunrpc/xprtmultipath.h |  3 +
 net/sunrpc/clnt.c                    | 19 +++++-
 net/sunrpc/xprtmultipath.c           | 98 +++++++++++++++++++++++++---
 3 files changed, 108 insertions(+), 12 deletions(-)

diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index bbb8a5fa0816..688ca7eb1d01 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -63,6 +63,9 @@ extern void xprt_iter_init(struct rpc_xprt_iter *xpi,
 extern void xprt_iter_init_listall(struct rpc_xprt_iter *xpi,
 		struct rpc_xprt_switch *xps);
 
+extern void xprt_iter_init_listoffline(struct rpc_xprt_iter *xpi,
+		struct rpc_xprt_switch *xps);
+
 extern void xprt_iter_destroy(struct rpc_xprt_iter *xpi);
 
 extern struct rpc_xprt_switch *xprt_iter_xchg_switch(
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 544b55a3aa20..410bd6c352ad 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -785,7 +785,9 @@ int rpc_switch_client_transport(struct rpc_clnt *clnt,
 EXPORT_SYMBOL_GPL(rpc_switch_client_transport);
 
 static
-int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
+int _rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi,
+		void func(struct rpc_xprt_iter *xpi,
+			  struct rpc_xprt_switch *xps))
 {
 	struct rpc_xprt_switch *xps;
 
@@ -794,11 +796,24 @@ int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
 	rcu_read_unlock();
 	if (xps == NULL)
 		return -EAGAIN;
-	xprt_iter_init_listall(xpi, xps);
+	func(xpi, xps);
 	xprt_switch_put(xps);
 	return 0;
 }
 
+static
+int rpc_clnt_xprt_iter_init(struct rpc_clnt *clnt, struct rpc_xprt_iter *xpi)
+{
+	return _rpc_clnt_xprt_iter_init(clnt, xpi, xprt_iter_init_listall);
+}
+
+static
+int rpc_clnt_xprt_iter_offline_init(struct rpc_clnt *clnt,
+				    struct rpc_xprt_iter *xpi)
+{
+	return _rpc_clnt_xprt_iter_init(clnt, xpi, xprt_iter_init_listoffline);
+}
+
 /**
  * rpc_clnt_iterate_for_each_xprt - Apply a function to all transports
  * @clnt: pointer to client
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 1693f81aae37..4374cd6acc55 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -27,6 +27,7 @@ typedef struct rpc_xprt *(*xprt_switch_find_xprt_t)(struct rpc_xprt_switch *xps,
 static const struct rpc_xprt_iter_ops rpc_xprt_iter_singular;
 static const struct rpc_xprt_iter_ops rpc_xprt_iter_roundrobin;
 static const struct rpc_xprt_iter_ops rpc_xprt_iter_listall;
+static const struct rpc_xprt_iter_ops rpc_xprt_iter_listoffline;
 
 static void xprt_switch_add_xprt_locked(struct rpc_xprt_switch *xps,
 		struct rpc_xprt *xprt)
@@ -248,6 +249,18 @@ struct rpc_xprt *xprt_switch_find_first_entry(struct list_head *head)
 	return NULL;
 }
 
+static
+struct rpc_xprt *xprt_switch_find_first_entry_offline(struct list_head *head)
+{
+	struct rpc_xprt *pos;
+
+	list_for_each_entry_rcu(pos, head, xprt_switch) {
+		if (!xprt_is_active(pos))
+			return pos;
+	}
+	return NULL;
+}
+
 static
 struct rpc_xprt *xprt_iter_first_entry(struct rpc_xprt_iter *xpi)
 {
@@ -259,8 +272,8 @@ struct rpc_xprt *xprt_iter_first_entry(struct rpc_xprt_iter *xpi)
 }
 
 static
-struct rpc_xprt *xprt_switch_find_current_entry(struct list_head *head,
-		const struct rpc_xprt *cur)
+struct rpc_xprt *_xprt_switch_find_current_entry(struct list_head *head,
+		const struct rpc_xprt *cur, bool find_active)
 {
 	struct rpc_xprt *pos;
 	bool found = false;
@@ -268,14 +281,25 @@ struct rpc_xprt *xprt_switch_find_current_entry(struct list_head *head,
 	list_for_each_entry_rcu(pos, head, xprt_switch) {
 		if (cur == pos)
 			found = true;
-		if (found && xprt_is_active(pos))
+		if (found && ((find_active && xprt_is_active(pos)) ||
+				(!find_active && xprt_is_active(pos))))
 			return pos;
 	}
 	return NULL;
 }
 
 static
-struct rpc_xprt *xprt_iter_current_entry(struct rpc_xprt_iter *xpi)
+struct rpc_xprt *xprt_switch_find_current_entry(struct list_head *head,
+		const struct rpc_xprt *cur)
+{
+	return _xprt_switch_find_current_entry(head, cur, true);
+}
+
+static
+struct rpc_xprt * _xprt_iter_current_entry(struct rpc_xprt_iter *xpi,
+		struct rpc_xprt *first_entry(struct list_head *head),
+		struct rpc_xprt *current_entry(struct list_head *head,
+		const struct rpc_xprt *cur))
 {
 	struct rpc_xprt_switch *xps = rcu_dereference(xpi->xpi_xpswitch);
 	struct list_head *head;
@@ -284,8 +308,30 @@ struct rpc_xprt *xprt_iter_current_entry(struct rpc_xprt_iter *xpi)
 		return NULL;
 	head = &xps->xps_xprt_list;
 	if (xpi->xpi_cursor == NULL || xps->xps_nxprts < 2)
-		return xprt_switch_find_first_entry(head);
-	return xprt_switch_find_current_entry(head, xpi->xpi_cursor);
+		return first_entry(head);
+	return current_entry(head, xpi->xpi_cursor);
+}
+
+static
+struct rpc_xprt *xprt_iter_current_entry(struct rpc_xprt_iter *xpi)
+{
+	return _xprt_iter_current_entry(xpi, xprt_switch_find_first_entry,
+			xprt_switch_find_current_entry);
+}
+
+static
+struct rpc_xprt *xprt_switch_find_current_entry_offline(struct list_head *head,
+		const struct rpc_xprt *cur)
+{
+	return _xprt_switch_find_current_entry(head, cur, false);
+}
+
+static
+struct rpc_xprt *xprt_iter_current_entry_offline(struct rpc_xprt_iter *xpi)
+{
+	return _xprt_iter_current_entry(xpi,
+			xprt_switch_find_first_entry_offline,
+			xprt_switch_find_current_entry_offline);
 }
 
 bool rpc_xprt_switch_has_addr(struct rpc_xprt_switch *xps,
@@ -310,7 +356,7 @@ bool rpc_xprt_switch_has_addr(struct rpc_xprt_switch *xps,
 
 static
 struct rpc_xprt *xprt_switch_find_next_entry(struct list_head *head,
-		const struct rpc_xprt *cur)
+		const struct rpc_xprt *cur, bool check_active)
 {
 	struct rpc_xprt *pos, *prev = NULL;
 	bool found = false;
@@ -318,7 +364,12 @@ struct rpc_xprt *xprt_switch_find_next_entry(struct list_head *head,
 	list_for_each_entry_rcu(pos, head, xprt_switch) {
 		if (cur == prev)
 			found = true;
-		if (found && xprt_is_active(pos))
+		/* for request to return active transports return only
+		 * active, for request to return offline transports
+		 * return only offline
+		 */
+		if (found && ((check_active && xprt_is_active(pos)) ||
+			      (!check_active && !xprt_is_active(pos))))
 			return pos;
 		prev = pos;
 	}
@@ -355,7 +406,7 @@ struct rpc_xprt *__xprt_switch_find_next_entry_roundrobin(struct list_head *head
 {
 	struct rpc_xprt *ret;
 
-	ret = xprt_switch_find_next_entry(head, cur);
+	ret = xprt_switch_find_next_entry(head, cur, true);
 	if (ret != NULL)
 		return ret;
 	return xprt_switch_find_first_entry(head);
@@ -397,7 +448,14 @@ static
 struct rpc_xprt *xprt_switch_find_next_entry_all(struct rpc_xprt_switch *xps,
 		const struct rpc_xprt *cur)
 {
-	return xprt_switch_find_next_entry(&xps->xps_xprt_list, cur);
+	return xprt_switch_find_next_entry(&xps->xps_xprt_list, cur, true);
+}
+
+static
+struct rpc_xprt *xprt_switch_find_next_entry_offline(struct rpc_xprt_switch *xps,
+		const struct rpc_xprt *cur)
+{
+	return xprt_switch_find_next_entry(&xps->xps_xprt_list, cur, false);
 }
 
 static
@@ -407,6 +465,13 @@ struct rpc_xprt *xprt_iter_next_entry_all(struct rpc_xprt_iter *xpi)
 			xprt_switch_find_next_entry_all);
 }
 
+static
+struct rpc_xprt *xprt_iter_next_entry_offline(struct rpc_xprt_iter *xpi)
+{
+	return xprt_iter_next_entry_multiple(xpi,
+			xprt_switch_find_next_entry_offline);
+}
+
 /*
  * xprt_iter_rewind - Resets the xprt iterator
  * @xpi: pointer to rpc_xprt_iter
@@ -460,6 +525,12 @@ void xprt_iter_init_listall(struct rpc_xprt_iter *xpi,
 	__xprt_iter_init(xpi, xps, &rpc_xprt_iter_listall);
 }
 
+void xprt_iter_init_listoffline(struct rpc_xprt_iter *xpi,
+		struct rpc_xprt_switch *xps)
+{
+	__xprt_iter_init(xpi, xps, &rpc_xprt_iter_listoffline);
+}
+
 /**
  * xprt_iter_xchg_switch - Atomically swap out the rpc_xprt_switch
  * @xpi: pointer to rpc_xprt_iter
@@ -574,3 +645,10 @@ const struct rpc_xprt_iter_ops rpc_xprt_iter_listall = {
 	.xpi_xprt = xprt_iter_current_entry,
 	.xpi_next = xprt_iter_next_entry_all,
 };
+
+static
+const struct rpc_xprt_iter_ops rpc_xprt_iter_listoffline = {
+	.xpi_rewind = xprt_iter_default_rewind,
+	.xpi_xprt = xprt_iter_current_entry_offline,
+	.xpi_next = xprt_iter_next_entry_offline,
+};
-- 
2.27.0

[PATCH v1 05/12] SUNRPC parameterize rpc_clnt_iterate_for_each_xprt with iterator init function

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Allow for rpc_clnt_iterate_for_each_xprt() to take in an iterator
initialization function if no function passed in a default initiator
is used.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c           |  2 +-
 include/linux/sunrpc/clnt.h |  1 +
 net/sunrpc/clnt.c           | 17 ++++++++++++-----
 net/sunrpc/debugfs.c        |  2 +-
 net/sunrpc/stats.c          |  2 +-
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index cf898bea3bfd..5e4c32924347 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8532,7 +8532,7 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, const struct cred *cr
 		.clp = clp,
 		.cred = cred,
 	};
-	return rpc_clnt_iterate_for_each_xprt(clp->cl_rpcclient,
+	return rpc_clnt_iterate_for_each_xprt(clp->cl_rpcclient, NULL,
 			nfs4_proc_bind_conn_to_session_callback, &data);
 }
 
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index e74a0740603b..20aed14fe222 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -213,6 +213,7 @@ const char	*rpc_peeraddr2str(struct rpc_clnt *, enum rpc_display_format_t);
 int		rpc_localaddr(struct rpc_clnt *, struct sockaddr *, size_t);
 
 int 		rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
+			int (*setup)(struct rpc_clnt *, struct rpc_xprt_iter *),
 			int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void *),
 			void *data);
 
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 410bd6c352ad..b26267606de0 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -817,6 +817,8 @@ int rpc_clnt_xprt_iter_offline_init(struct rpc_clnt *clnt,
 /**
  * rpc_clnt_iterate_for_each_xprt - Apply a function to all transports
  * @clnt: pointer to client
+ * @setup: an optional iterator init function to use, if none supplied
+ *   default rpc_clnt_xprt_iter_init() iterator is used
  * @fn: function to apply
  * @data: void pointer to function data
  *
@@ -826,13 +828,17 @@ int rpc_clnt_xprt_iter_offline_init(struct rpc_clnt *clnt,
  * On error, the iteration stops, and the function returns the error value.
  */
 int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
+		int (*setup)(struct rpc_clnt *, struct rpc_xprt_iter *),
 		int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void *),
 		void *data)
 {
 	struct rpc_xprt_iter xpi;
 	int ret;
 
-	ret = rpc_clnt_xprt_iter_init(clnt, &xpi);
+	if (!setup)
+		ret = rpc_clnt_xprt_iter_init(clnt, &xpi);
+	else
+		ret = setup(clnt, &xpi);
 	if (ret)
 		return ret;
 	for (;;) {
@@ -3052,7 +3058,8 @@ static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
 
 void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void *data)
 {
-	rpc_clnt_iterate_for_each_xprt(clnt, rpc_xprt_offline_destroy, data);
+	rpc_clnt_iterate_for_each_xprt(clnt, NULL, rpc_xprt_offline_destroy,
+			data);
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
 
@@ -3084,7 +3091,7 @@ rpc_set_connect_timeout(struct rpc_clnt *clnt,
 		.connect_timeout = connect_timeout,
 		.reconnect_timeout = reconnect_timeout,
 	};
-	rpc_clnt_iterate_for_each_xprt(clnt,
+	rpc_clnt_iterate_for_each_xprt(clnt, NULL,
 			rpc_xprt_set_connect_timeout,
 			&timeout);
 }
@@ -3181,7 +3188,7 @@ rpc_clnt_swap_activate(struct rpc_clnt *clnt)
 	while (clnt != clnt->cl_parent)
 		clnt = clnt->cl_parent;
 	if (atomic_inc_return(&clnt->cl_swapper) == 1)
-		return rpc_clnt_iterate_for_each_xprt(clnt,
+		return rpc_clnt_iterate_for_each_xprt(clnt, NULL,
 				rpc_clnt_swap_activate_callback, NULL);
 	return 0;
 }
@@ -3200,7 +3207,7 @@ void
 rpc_clnt_swap_deactivate(struct rpc_clnt *clnt)
 {
 	if (atomic_dec_if_positive(&clnt->cl_swapper) == 0)
-		rpc_clnt_iterate_for_each_xprt(clnt,
+		rpc_clnt_iterate_for_each_xprt(clnt, NULL,
 				rpc_clnt_swap_deactivate_callback, NULL);
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_swap_deactivate);
diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
index 7dc9cc929bfd..ab60b4d3deb2 100644
--- a/net/sunrpc/debugfs.c
+++ b/net/sunrpc/debugfs.c
@@ -160,7 +160,7 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
 	debugfs_create_file("tasks", S_IFREG | 0400, clnt->cl_debugfs, clnt,
 			    &tasks_fops);
 
-	rpc_clnt_iterate_for_each_xprt(clnt, do_xprt_debugfs, &xprtnum);
+	rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_xprt_debugfs, &xprtnum);
 }
 
 void
diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
index 52908f9e6eab..e50f73a4aca5 100644
--- a/net/sunrpc/stats.c
+++ b/net/sunrpc/stats.c
@@ -258,7 +258,7 @@ void rpc_clnt_show_stats(struct seq_file *seq, struct rpc_clnt *clnt)
 	seq_printf(seq, "p/v: %u/%u (%s)\n",
 			clnt->cl_prog, clnt->cl_vers, clnt->cl_program->name);
 
-	rpc_clnt_iterate_for_each_xprt(clnt, do_print_stats, seq);
+	rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_print_stats, seq);
 
 	seq_printf(seq, "\tper-op statistics\n");
 	for (op = 0; op < maxproc; op++) {
-- 
2.27.0

Re: [PATCH v1 05/12] SUNRPC parameterize rpc_clnt_iterate_for_each_xprt with iterator init function

[Trond Myklebust]

On Mon, 2022-06-20 at 11:24 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Allow for rpc_clnt_iterate_for_each_xprt() to take in an iterator
> initialization function if no function passed in a default initiator
> is used.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4proc.c           |  2 +-
>  include/linux/sunrpc/clnt.h |  1 +
>  net/sunrpc/clnt.c           | 17 ++++++++++++-----
>  net/sunrpc/debugfs.c        |  2 +-
>  net/sunrpc/stats.c          |  2 +-
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index cf898bea3bfd..5e4c32924347 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -8532,7 +8532,7 @@ int nfs4_proc_bind_conn_to_session(struct
> nfs_client *clp, const struct cred *cr
>                 .clp = clp,
>                 .cred = cred,
>         };
> -       return rpc_clnt_iterate_for_each_xprt(clp->cl_rpcclient,
> +       return rpc_clnt_iterate_for_each_xprt(clp->cl_rpcclient,
> NULL,
>                         nfs4_proc_bind_conn_to_session_callback,
> &data);
>  }
>  
> diff --git a/include/linux/sunrpc/clnt.h
> b/include/linux/sunrpc/clnt.h
> index e74a0740603b..20aed14fe222 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -213,6 +213,7 @@ const char  *rpc_peeraddr2str(struct rpc_clnt *,
> enum rpc_display_format_t);
>  int            rpc_localaddr(struct rpc_clnt *, struct sockaddr *,
> size_t);
>  
>  int            rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
> +                       int (*setup)(struct rpc_clnt *, struct
> rpc_xprt_iter *),
>                         int (*fn)(struct rpc_clnt *, struct rpc_xprt
> *, void *),
>                         void *data);
>  
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 410bd6c352ad..b26267606de0 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -817,6 +817,8 @@ int rpc_clnt_xprt_iter_offline_init(struct
> rpc_clnt *clnt,
>  /**
>   * rpc_clnt_iterate_for_each_xprt - Apply a function to all
> transports
>   * @clnt: pointer to client
> + * @setup: an optional iterator init function to use, if none
> supplied
> + *   default rpc_clnt_xprt_iter_init() iterator is used
>   * @fn: function to apply
>   * @data: void pointer to function data
>   *
> @@ -826,13 +828,17 @@ int rpc_clnt_xprt_iter_offline_init(struct
> rpc_clnt *clnt,
>   * On error, the iteration stops, and the function returns the error
> value.
>   */
>  int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
> +               int (*setup)(struct rpc_clnt *, struct rpc_xprt_iter
> *),
>                 int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void
> *),
>                 void *data)
>  {
>         struct rpc_xprt_iter xpi;
>         int ret;
>  
> -       ret = rpc_clnt_xprt_iter_init(clnt, &xpi);
> +       if (!setup)
> +               ret = rpc_clnt_xprt_iter_init(clnt, &xpi);
> +       else
> +               ret = setup(clnt, &xpi);

Please create an internal function that is not exported outside the RPC
layer for this. It would be fine to share code with
rpc_clnt_iterate_for_each_xprt() where appropriate, but we should not
expose an API that expects the caller to have intimate knowledge of the
rpc client internals.

>         if (ret)
>                 return ret;
>         for (;;) {
> @@ -3052,7 +3058,8 @@ static int rpc_xprt_offline_destroy(struct
> rpc_clnt *clnt,
>  
>  void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void
> *data)
>  {
> -       rpc_clnt_iterate_for_each_xprt(clnt,
> rpc_xprt_offline_destroy, data);
> +       rpc_clnt_iterate_for_each_xprt(clnt, NULL,
> rpc_xprt_offline_destroy,
> +                       data);
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
>  
> @@ -3084,7 +3091,7 @@ rpc_set_connect_timeout(struct rpc_clnt *clnt,
>                 .connect_timeout = connect_timeout,
>                 .reconnect_timeout = reconnect_timeout,
>         };
> -       rpc_clnt_iterate_for_each_xprt(clnt,
> +       rpc_clnt_iterate_for_each_xprt(clnt, NULL,
>                         rpc_xprt_set_connect_timeout,
>                         &timeout);
>  }
> @@ -3181,7 +3188,7 @@ rpc_clnt_swap_activate(struct rpc_clnt *clnt)
>         while (clnt != clnt->cl_parent)
>                 clnt = clnt->cl_parent;
>         if (atomic_inc_return(&clnt->cl_swapper) == 1)
> -               return rpc_clnt_iterate_for_each_xprt(clnt,
> +               return rpc_clnt_iterate_for_each_xprt(clnt, NULL,
>                                 rpc_clnt_swap_activate_callback,
> NULL);
>         return 0;
>  }
> @@ -3200,7 +3207,7 @@ void
>  rpc_clnt_swap_deactivate(struct rpc_clnt *clnt)
>  {
>         if (atomic_dec_if_positive(&clnt->cl_swapper) == 0)
> -               rpc_clnt_iterate_for_each_xprt(clnt,
> +               rpc_clnt_iterate_for_each_xprt(clnt, NULL,
>                                 rpc_clnt_swap_deactivate_callback,
> NULL);
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_swap_deactivate);
> diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
> index 7dc9cc929bfd..ab60b4d3deb2 100644
> --- a/net/sunrpc/debugfs.c
> +++ b/net/sunrpc/debugfs.c
> @@ -160,7 +160,7 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
>         debugfs_create_file("tasks", S_IFREG | 0400, clnt-
> >cl_debugfs, clnt,
>                             &tasks_fops);
>  
> -       rpc_clnt_iterate_for_each_xprt(clnt, do_xprt_debugfs,
> &xprtnum);
> +       rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_xprt_debugfs,
> &xprtnum);
>  }
>  
>  void
> diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
> index 52908f9e6eab..e50f73a4aca5 100644
> --- a/net/sunrpc/stats.c
> +++ b/net/sunrpc/stats.c
> @@ -258,7 +258,7 @@ void rpc_clnt_show_stats(struct seq_file *seq,
> struct rpc_clnt *clnt)
>         seq_printf(seq, "p/v: %u/%u (%s)\n",
>                         clnt->cl_prog, clnt->cl_vers, clnt-
> >cl_program->name);
>  
> -       rpc_clnt_iterate_for_each_xprt(clnt, do_print_stats, seq);
> +       rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_print_stats,
> seq);
>  
>         seq_printf(seq, "\tper-op statistics\n");
>         for (op = 0; op < maxproc; op++) {

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

[PATCH v1 06/12] SUNRPC enable back offline transports in trunking discovery

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

When we are adding a transport to a xprt_switch that's already on
the list but has been marked OFFLINE, then make the state ONLINE
since it's been tested now.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/sunrpc/clnt.h |  1 +
 net/sunrpc/clnt.c           | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 20aed14fe222..319bcd3a3593 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -243,6 +243,7 @@ void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
 void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *, struct rpc_xprt *);
 bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
 			const struct sockaddr *sap);
+void rpc_clnt_xprt_set_online(struct rpc_clnt *clnt, struct rpc_xprt *xprt);
 void rpc_cleanup_clids(void);
 
 static inline int rpc_reply_expected(struct rpc_task *task)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index b26267606de0..1cbd598f596c 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -3105,8 +3105,22 @@ void rpc_clnt_xprt_switch_put(struct rpc_clnt *clnt)
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_put);
 
+void rpc_clnt_xprt_set_online(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
+{
+	struct rpc_xprt_switch *xps;
+
+	rcu_read_lock();
+	xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
+	rcu_read_unlock();
+	xprt_set_online_locked(xprt, xps);
+}
+
 void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
 {
+	if (rpc_clnt_xprt_switch_has_addr(clnt,
+		(const struct sockaddr *)&xprt->addr)) {
+		return rpc_clnt_xprt_set_online(clnt, xprt);
+	}
 	rcu_read_lock();
 	rpc_xprt_switch_add_xprt(rcu_dereference(clnt->cl_xpi.xpi_xpswitch),
 				 xprt);
-- 
2.27.0

[PATCH v1 07/12] SUNRPC create an rpc function that allows xprt removal from rpc_clnt

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Expose a function that allows a removal of xprt from the rpc_clnt.

When called from NFS that's running a trunked transport then don't
decrement the active transport counter.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/sunrpc/clnt.h          |  1 +
 include/linux/sunrpc/xprtmultipath.h |  2 +-
 net/sunrpc/clnt.c                    | 16 +++++++++++++++-
 net/sunrpc/xprt.c                    |  2 +-
 net/sunrpc/xprtmultipath.c           | 10 +++++-----
 5 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 319bcd3a3593..ac1024da86c5 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -241,6 +241,7 @@ const char *rpc_proc_name(const struct rpc_task *task);
 
 void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
 void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *, struct rpc_xprt *);
+void rpc_clnt_xprt_switch_remove_xprt(struct rpc_clnt *, struct rpc_xprt *);
 bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
 			const struct sockaddr *sap);
 void rpc_clnt_xprt_set_online(struct rpc_clnt *clnt, struct rpc_xprt *xprt);
diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index 688ca7eb1d01..9fff0768d942 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -55,7 +55,7 @@ extern void rpc_xprt_switch_set_roundrobin(struct rpc_xprt_switch *xps);
 extern void rpc_xprt_switch_add_xprt(struct rpc_xprt_switch *xps,
 		struct rpc_xprt *xprt);
 extern void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
-		struct rpc_xprt *xprt);
+		struct rpc_xprt *xprt, bool offline);
 
 extern void xprt_iter_init(struct rpc_xprt_iter *xpi,
 		struct rpc_xprt_switch *xps);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 1cbd598f596c..2b2515c121fa 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2157,7 +2157,8 @@ call_connect_status(struct rpc_task *task)
 				xprt_release(task);
 				value = atomic_long_dec_return(&xprt->queuelen);
 				if (value == 0)
-					rpc_xprt_switch_remove_xprt(xps, saved);
+					rpc_xprt_switch_remove_xprt(xps, saved,
+							true);
 				xprt_put(saved);
 				task->tk_xprt = NULL;
 				task->tk_action = call_start;
@@ -3128,6 +3129,19 @@ void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_add_xprt);
 
+void rpc_clnt_xprt_switch_remove_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
+{
+	struct rpc_xprt_switch *xps;
+
+	rcu_read_lock();
+	xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
+	rpc_xprt_switch_remove_xprt(rcu_dereference(clnt->cl_xpi.xpi_xpswitch),
+				    xprt, 0);
+	xps->xps_nunique_destaddr_xprts--;
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_remove_xprt);
+
 bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
 				   const struct sockaddr *sap)
 {
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 6480ae324b27..ac02bf6d109a 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -2184,6 +2184,6 @@ void xprt_delete_locked(struct rpc_xprt *xprt, struct rpc_xprt_switch *xps)
 
 	if (!xprt->sending.qlen && !xprt->pending.qlen &&
 	    !xprt->backlog.qlen && !atomic_long_read(&xprt->queuelen))
-		rpc_xprt_switch_remove_xprt(xps, xprt);
+		rpc_xprt_switch_remove_xprt(xps, xprt, true);
 }
 EXPORT_SYMBOL(xprt_delete_locked);
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 4374cd6acc55..41ec46e5f1a3 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -62,11 +62,11 @@ void rpc_xprt_switch_add_xprt(struct rpc_xprt_switch *xps,
 }
 
 static void xprt_switch_remove_xprt_locked(struct rpc_xprt_switch *xps,
-		struct rpc_xprt *xprt)
+		struct rpc_xprt *xprt, bool offline)
 {
 	if (unlikely(xprt == NULL))
 		return;
-	if (!test_bit(XPRT_OFFLINE, &xprt->state))
+	if (!test_bit(XPRT_OFFLINE, &xprt->state) && offline)
 		xps->xps_nactive--;
 	xps->xps_nxprts--;
 	if (xps->xps_nxprts == 0)
@@ -83,10 +83,10 @@ static void xprt_switch_remove_xprt_locked(struct rpc_xprt_switch *xps,
  * Removes xprt from the list of struct rpc_xprt in xps.
  */
 void rpc_xprt_switch_remove_xprt(struct rpc_xprt_switch *xps,
-		struct rpc_xprt *xprt)
+		struct rpc_xprt *xprt, bool offline)
 {
 	spin_lock(&xps->xps_lock);
-	xprt_switch_remove_xprt_locked(xps, xprt);
+	xprt_switch_remove_xprt_locked(xps, xprt, offline);
 	spin_unlock(&xps->xps_lock);
 	xprt_put(xprt);
 }
@@ -155,7 +155,7 @@ static void xprt_switch_free_entries(struct rpc_xprt_switch *xps)
 
 		xprt = list_first_entry(&xps->xps_xprt_list,
 				struct rpc_xprt, xprt_switch);
-		xprt_switch_remove_xprt_locked(xps, xprt);
+		xprt_switch_remove_xprt_locked(xps, xprt, true);
 		spin_unlock(&xps->xps_lock);
 		xprt_put(xprt);
 		spin_lock(&xps->xps_lock);
-- 
2.27.0

[PATCH v1 08/12] NFSv4.1 remove xprt from xprt_switch if session trunking test fails

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

If we are doing a session trunking test and it fails for the transport,
then remove this transport from the xprt_switch group.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5e4c32924347..152da2bc5100 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8917,6 +8917,9 @@ void nfs4_test_session_trunk(struct rpc_clnt *clnt, struct rpc_xprt *xprt,
 
 	if (status == 0)
 		rpc_clnt_xprt_switch_add_xprt(clnt, xprt);
+	else if (rpc_clnt_xprt_switch_has_addr(clnt,
+				(struct sockaddr *)&xprt->addr))
+		rpc_clnt_xprt_switch_remove_xprt(clnt, xprt);
 
 	rpc_put_task(task);
 }
-- 
2.27.0

[PATCH v1 09/12] SUNRPC restructure rpc_clnt_setup_test_and_add_xprt

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

In preparation for code re-use, pull out the part of the
rpc_clnt_setup_test_and_add_xprt() portion that sends a NULL rpc
and then calls a session trunking function into a helper function.

Re-organize the end of the function for code re-use.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 net/sunrpc/clnt.c | 53 ++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 2b2515c121fa..6b04b29bf842 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2887,6 +2887,31 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
 
+static int rpc_clnt_add_xprt_helper(struct rpc_clnt *clnt,
+				    struct rpc_xprt *xprt,
+				    void *data)
+{
+	struct rpc_task *task;
+	struct rpc_add_xprt_test *xtest = (struct rpc_add_xprt_test *)data;
+	int status = -EADDRINUSE;
+
+	/* Test the connection */
+	task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
+	if (IS_ERR(task))
+		return PTR_ERR(task);
+
+	status = task->tk_status;
+	rpc_put_task(task);
+
+	if (status < 0)
+		return status;
+
+	/* rpc_xprt_switch and rpc_xprt are deferrenced by add_xprt_test() */
+	xtest->add_xprt_test(clnt, xprt, xtest->data);
+
+	return 0;
+}
+
 /**
  * rpc_clnt_setup_test_and_add_xprt()
  *
@@ -2910,8 +2935,6 @@ int rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *clnt,
 				     struct rpc_xprt *xprt,
 				     void *data)
 {
-	struct rpc_task *task;
-	struct rpc_add_xprt_test *xtest = (struct rpc_add_xprt_test *)data;
 	int status = -EADDRINUSE;
 
 	xprt = xprt_get(xprt);
@@ -2920,31 +2943,19 @@ int rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *clnt,
 	if (rpc_xprt_switch_has_addr(xps, (struct sockaddr *)&xprt->addr))
 		goto out_err;
 
-	/* Test the connection */
-	task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
-	if (IS_ERR(task)) {
-		status = PTR_ERR(task);
-		goto out_err;
-	}
-	status = task->tk_status;
-	rpc_put_task(task);
-
+	status = rpc_clnt_add_xprt_helper(clnt, xprt, data);
 	if (status < 0)
 		goto out_err;
 
-	/* rpc_xprt_switch and rpc_xprt are deferrenced by add_xprt_test() */
-	xtest->add_xprt_test(clnt, xprt, xtest->data);
-
-	xprt_put(xprt);
-	xprt_switch_put(xps);
-
-	/* so that rpc_clnt_add_xprt does not call rpc_xprt_switch_add_xprt */
-	return 1;
+	status = 1;
 out_err:
 	xprt_put(xprt);
 	xprt_switch_put(xps);
-	pr_info("RPC:   rpc_clnt_test_xprt failed: %d addr %s not added\n",
-		status, xprt->address_strings[RPC_DISPLAY_ADDR]);
+	if (status < 0)
+		pr_info("RPC:   rpc_clnt_test_xprt failed: %d addr %s not "
+			"added\n", status,
+			xprt->address_strings[RPC_DISPLAY_ADDR]);
+	/* so that rpc_clnt_add_xprt does not call rpc_xprt_switch_add_xprt */
 	return status;
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_setup_test_and_add_xprt);
-- 
2.27.0

[PATCH v1 10/12] SUNRPC export xprt_iter_rewind function

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Make xprt_iter_rewind callable outside of xprtmultipath.c

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/sunrpc/xprtmultipath.h | 2 ++
 net/sunrpc/xprtmultipath.c           | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/sunrpc/xprtmultipath.h b/include/linux/sunrpc/xprtmultipath.h
index 9fff0768d942..c0514c684b2c 100644
--- a/include/linux/sunrpc/xprtmultipath.h
+++ b/include/linux/sunrpc/xprtmultipath.h
@@ -68,6 +68,8 @@ extern void xprt_iter_init_listoffline(struct rpc_xprt_iter *xpi,
 
 extern void xprt_iter_destroy(struct rpc_xprt_iter *xpi);
 
+extern void xprt_iter_rewind(struct rpc_xprt_iter *xpi);
+
 extern struct rpc_xprt_switch *xprt_iter_xchg_switch(
 		struct rpc_xprt_iter *xpi,
 		struct rpc_xprt_switch *newswitch);
diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index 41ec46e5f1a3..154bb6cf27ad 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -479,7 +479,6 @@ struct rpc_xprt *xprt_iter_next_entry_offline(struct rpc_xprt_iter *xpi)
  * Resets xpi to ensure that it points to the first entry in the list
  * of transports.
  */
-static
 void xprt_iter_rewind(struct rpc_xprt_iter *xpi)
 {
 	rcu_read_lock();
-- 
2.27.0

[PATCH v1 11/12] SUNRPC create a function that probes only offline transports

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

For only offline transports, attempt to check connectivity via
a NULL call and, if that succeeds, call a provided session trunking
detection function.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c           |  2 +-
 include/linux/sunrpc/clnt.h |  3 ++-
 net/sunrpc/clnt.c           | 47 +++++++++++++++++++++++++++++++++----
 net/sunrpc/debugfs.c        |  3 ++-
 net/sunrpc/stats.c          |  2 +-
 5 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 152da2bc5100..00778f351283 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8533,7 +8533,7 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, const struct cred *cr
 		.cred = cred,
 	};
 	return rpc_clnt_iterate_for_each_xprt(clp->cl_rpcclient, NULL,
-			nfs4_proc_bind_conn_to_session_callback, &data);
+			nfs4_proc_bind_conn_to_session_callback, &data, false);
 }
 
 /*
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index ac1024da86c5..a0160b83d4a4 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -215,7 +215,7 @@ int		rpc_localaddr(struct rpc_clnt *, struct sockaddr *, size_t);
 int 		rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
 			int (*setup)(struct rpc_clnt *, struct rpc_xprt_iter *),
 			int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void *),
-			void *data);
+			void *data, bool do_rewind);
 
 int 		rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 			struct rpc_xprt_switch *xps,
@@ -236,6 +236,7 @@ int		rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
 			struct rpc_xprt *,
 			void *);
 void		rpc_clnt_manage_trunked_xprts(struct rpc_clnt *, void *);
+void		rpc_probe_trunked_xprts(struct rpc_clnt *, void *);
 
 const char *rpc_proc_name(const struct rpc_task *task);
 
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 6b04b29bf842..348d0772c91d 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -830,7 +830,7 @@ int rpc_clnt_xprt_iter_offline_init(struct rpc_clnt *clnt,
 int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
 		int (*setup)(struct rpc_clnt *, struct rpc_xprt_iter *),
 		int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void *),
-		void *data)
+		void *data, bool do_rewind)
 {
 	struct rpc_xprt_iter xpi;
 	int ret;
@@ -850,6 +850,9 @@ int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
 		xprt_put(xprt);
 		if (ret < 0)
 			break;
+
+		if (do_rewind)
+			xprt_iter_rewind(&xpi);
 	}
 	xprt_iter_destroy(&xpi);
 	return ret;
@@ -3032,6 +3035,40 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
 
+static int rpc_xprt_probe_trunked(struct rpc_clnt *clnt,
+				  struct rpc_xprt *xprt,
+				  void *data)
+{
+	struct rpc_xprt_switch *xps;
+	struct rpc_xprt *main_xprt;
+	int status = 0;
+
+	xprt_get(xprt);
+
+	rcu_read_lock();
+	main_xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
+	xps = xprt_switch_get(rcu_dereference(clnt->cl_xpi.xpi_xpswitch));
+	status = rpc_cmp_addr_port((struct sockaddr *)&xprt->addr,
+				(struct sockaddr *)&main_xprt->addr);
+	rcu_read_unlock();
+	xprt_put(main_xprt);
+	if (status || !test_bit(XPRT_OFFLINE, &xprt->state))
+		goto out;
+
+	status = rpc_clnt_add_xprt_helper(clnt, xprt, data);
+out:
+	xprt_put(xprt);
+	xprt_switch_put(xps);
+	return status;
+}
+
+void rpc_probe_trunked_xprts(struct rpc_clnt *clnt, void *data)
+{
+	rpc_clnt_iterate_for_each_xprt(clnt, rpc_clnt_xprt_iter_offline_init,
+			rpc_xprt_probe_trunked, data, true);
+}
+EXPORT_SYMBOL_GPL(rpc_probe_trunked_xprts);
+
 static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
 				    struct rpc_xprt *xprt,
 				    void *data)
@@ -3071,7 +3108,7 @@ static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
 void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void *data)
 {
 	rpc_clnt_iterate_for_each_xprt(clnt, NULL, rpc_xprt_offline_destroy,
-			data);
+			data, false);
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
 
@@ -3105,7 +3142,7 @@ rpc_set_connect_timeout(struct rpc_clnt *clnt,
 	};
 	rpc_clnt_iterate_for_each_xprt(clnt, NULL,
 			rpc_xprt_set_connect_timeout,
-			&timeout);
+			&timeout, false);
 }
 EXPORT_SYMBOL_GPL(rpc_set_connect_timeout);
 
@@ -3228,7 +3265,7 @@ rpc_clnt_swap_activate(struct rpc_clnt *clnt)
 		clnt = clnt->cl_parent;
 	if (atomic_inc_return(&clnt->cl_swapper) == 1)
 		return rpc_clnt_iterate_for_each_xprt(clnt, NULL,
-				rpc_clnt_swap_activate_callback, NULL);
+				rpc_clnt_swap_activate_callback, NULL, false);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_swap_activate);
@@ -3247,7 +3284,7 @@ rpc_clnt_swap_deactivate(struct rpc_clnt *clnt)
 {
 	if (atomic_dec_if_positive(&clnt->cl_swapper) == 0)
 		rpc_clnt_iterate_for_each_xprt(clnt, NULL,
-				rpc_clnt_swap_deactivate_callback, NULL);
+				rpc_clnt_swap_deactivate_callback, NULL, false);
 }
 EXPORT_SYMBOL_GPL(rpc_clnt_swap_deactivate);
 #endif /* CONFIG_SUNRPC_SWAP */
diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
index ab60b4d3deb2..9c700bad1ec5 100644
--- a/net/sunrpc/debugfs.c
+++ b/net/sunrpc/debugfs.c
@@ -160,7 +160,8 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
 	debugfs_create_file("tasks", S_IFREG | 0400, clnt->cl_debugfs, clnt,
 			    &tasks_fops);
 
-	rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_xprt_debugfs, &xprtnum);
+	rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_xprt_debugfs, &xprtnum,
+			false);
 }
 
 void
diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
index e50f73a4aca5..60e2d738a8f1 100644
--- a/net/sunrpc/stats.c
+++ b/net/sunrpc/stats.c
@@ -258,7 +258,7 @@ void rpc_clnt_show_stats(struct seq_file *seq, struct rpc_clnt *clnt)
 	seq_printf(seq, "p/v: %u/%u (%s)\n",
 			clnt->cl_prog, clnt->cl_vers, clnt->cl_program->name);
 
-	rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_print_stats, seq);
+	rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_print_stats, seq, false);
 
 	seq_printf(seq, "\tper-op statistics\n");
 	for (op = 0; op < maxproc; op++) {
-- 
2.27.0

Re: [PATCH v1 11/12] SUNRPC create a function that probes only offline transports

[Trond Myklebust]

On Mon, 2022-06-20 at 11:24 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> For only offline transports, attempt to check connectivity via
> a NULL call and, if that succeeds, call a provided session trunking
> detection function.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4proc.c           |  2 +-
>  include/linux/sunrpc/clnt.h |  3 ++-
>  net/sunrpc/clnt.c           | 47 +++++++++++++++++++++++++++++++++--
> --
>  net/sunrpc/debugfs.c        |  3 ++-
>  net/sunrpc/stats.c          |  2 +-
>  5 files changed, 48 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 152da2bc5100..00778f351283 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -8533,7 +8533,7 @@ int nfs4_proc_bind_conn_to_session(struct
> nfs_client *clp, const struct cred *cr
>                 .cred = cred,
>         };
>         return rpc_clnt_iterate_for_each_xprt(clp->cl_rpcclient,
> NULL,
> -                       nfs4_proc_bind_conn_to_session_callback,
> &data);
> +                       nfs4_proc_bind_conn_to_session_callback,
> &data, false);
>  }
>  
>  /*
> diff --git a/include/linux/sunrpc/clnt.h
> b/include/linux/sunrpc/clnt.h
> index ac1024da86c5..a0160b83d4a4 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -215,7 +215,7 @@ int         rpc_localaddr(struct rpc_clnt *,
> struct sockaddr *, size_t);
>  int            rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
>                         int (*setup)(struct rpc_clnt *, struct
> rpc_xprt_iter *),
>                         int (*fn)(struct rpc_clnt *, struct rpc_xprt
> *, void *),
> -                       void *data);
> +                       void *data, bool do_rewind);
>  
>  int            rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>                         struct rpc_xprt_switch *xps,
> @@ -236,6 +236,7 @@
> int         rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
>                         struct rpc_xprt *,
>                         void *);
>  void           rpc_clnt_manage_trunked_xprts(struct rpc_clnt *, void
> *);
> +void           rpc_probe_trunked_xprts(struct rpc_clnt *, void *);
>  
>  const char *rpc_proc_name(const struct rpc_task *task);
>  
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 6b04b29bf842..348d0772c91d 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -830,7 +830,7 @@ int rpc_clnt_xprt_iter_offline_init(struct
> rpc_clnt *clnt,
>  int rpc_clnt_iterate_for_each_xprt(struct rpc_clnt *clnt,
>                 int (*setup)(struct rpc_clnt *, struct rpc_xprt_iter
> *),
>                 int (*fn)(struct rpc_clnt *, struct rpc_xprt *, void
> *),
> -               void *data)
> +               void *data, bool do_rewind)
>  {
>         struct rpc_xprt_iter xpi;
>         int ret;
> @@ -850,6 +850,9 @@ int rpc_clnt_iterate_for_each_xprt(struct
> rpc_clnt *clnt,
>                 xprt_put(xprt);
>                 if (ret < 0)
>                         break;
> +
> +               if (do_rewind)
> +                       xprt_iter_rewind(&xpi);

This really needs to be another separate function. You are not
iterating over each xprt here, you are always looking at the first
valid entry.

>         }
>         xprt_iter_destroy(&xpi);
>         return ret;
> @@ -3032,6 +3035,40 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
>  
> +static int rpc_xprt_probe_trunked(struct rpc_clnt *clnt,
> +                                 struct rpc_xprt *xprt,
> +                                 void *data)
> +{
> +       struct rpc_xprt_switch *xps;
> +       struct rpc_xprt *main_xprt;
> +       int status = 0;
> +
> +       xprt_get(xprt);
> +
> +       rcu_read_lock();
> +       main_xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
> +       xps = xprt_switch_get(rcu_dereference(clnt-
> >cl_xpi.xpi_xpswitch));
> +       status = rpc_cmp_addr_port((struct sockaddr *)&xprt->addr,
> +                               (struct sockaddr *)&main_xprt->addr);
> +       rcu_read_unlock();
> +       xprt_put(main_xprt);
> +       if (status || !test_bit(XPRT_OFFLINE, &xprt->state))
> +               goto out;
> +
> +       status = rpc_clnt_add_xprt_helper(clnt, xprt, data);
> +out:
> +       xprt_put(xprt);
> +       xprt_switch_put(xps);
> +       return status;
> +}
> +
> +void rpc_probe_trunked_xprts(struct rpc_clnt *clnt, void *data)

'data' is not a 'void *'. You patch 447574a510fc ("SUNRPC restructure
rpc_clnt_setup_test_and_add_xprt") means that only a 'struct
rpc_add_xprt_test' argument is allowed here.

> +{
> +       rpc_clnt_iterate_for_each_xprt(clnt,
> rpc_clnt_xprt_iter_offline_init,
> +                       rpc_xprt_probe_trunked, data, true);
> +}
> +EXPORT_SYMBOL_GPL(rpc_probe_trunked_xprts);
> +
>  static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
>                                     struct rpc_xprt *xprt,
>                                     void *data)
> @@ -3071,7 +3108,7 @@ static int rpc_xprt_offline_destroy(struct
> rpc_clnt *clnt,
>  void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void
> *data)
>  {
>         rpc_clnt_iterate_for_each_xprt(clnt, NULL,
> rpc_xprt_offline_destroy,
> -                       data);
> +                       data, false);
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
>  
> @@ -3105,7 +3142,7 @@ rpc_set_connect_timeout(struct rpc_clnt *clnt,
>         };
>         rpc_clnt_iterate_for_each_xprt(clnt, NULL,
>                         rpc_xprt_set_connect_timeout,
> -                       &timeout);
> +                       &timeout, false);
>  }
>  EXPORT_SYMBOL_GPL(rpc_set_connect_timeout);
>  
> @@ -3228,7 +3265,7 @@ rpc_clnt_swap_activate(struct rpc_clnt *clnt)
>                 clnt = clnt->cl_parent;
>         if (atomic_inc_return(&clnt->cl_swapper) == 1)
>                 return rpc_clnt_iterate_for_each_xprt(clnt, NULL,
> -                               rpc_clnt_swap_activate_callback,
> NULL);
> +                               rpc_clnt_swap_activate_callback,
> NULL, false);
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_swap_activate);
> @@ -3247,7 +3284,7 @@ rpc_clnt_swap_deactivate(struct rpc_clnt *clnt)
>  {
>         if (atomic_dec_if_positive(&clnt->cl_swapper) == 0)
>                 rpc_clnt_iterate_for_each_xprt(clnt, NULL,
> -                               rpc_clnt_swap_deactivate_callback,
> NULL);
> +                               rpc_clnt_swap_deactivate_callback,
> NULL, false);
>  }
>  EXPORT_SYMBOL_GPL(rpc_clnt_swap_deactivate);
>  #endif /* CONFIG_SUNRPC_SWAP */
> diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
> index ab60b4d3deb2..9c700bad1ec5 100644
> --- a/net/sunrpc/debugfs.c
> +++ b/net/sunrpc/debugfs.c
> @@ -160,7 +160,8 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
>         debugfs_create_file("tasks", S_IFREG | 0400, clnt-
> >cl_debugfs, clnt,
>                             &tasks_fops);
>  
> -       rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_xprt_debugfs,
> &xprtnum);
> +       rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_xprt_debugfs,
> &xprtnum,
> +                       false);
>  }
>  
>  void
> diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
> index e50f73a4aca5..60e2d738a8f1 100644
> --- a/net/sunrpc/stats.c
> +++ b/net/sunrpc/stats.c
> @@ -258,7 +258,7 @@ void rpc_clnt_show_stats(struct seq_file *seq,
> struct rpc_clnt *clnt)
>         seq_printf(seq, "p/v: %u/%u (%s)\n",
>                         clnt->cl_prog, clnt->cl_vers, clnt-
> >cl_program->name);
>  
> -       rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_print_stats,
> seq);
> +       rpc_clnt_iterate_for_each_xprt(clnt, NULL, do_print_stats,
> seq, false);
>  
>         seq_printf(seq, "\tper-op statistics\n");
>         for (op = 0; op < maxproc; op++) {

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

[PATCH v1 12/12] NFSv4.1 probe offline transports for trunking on session creation

[Olga Kornievskaia]

From: Olga Kornievskaia <kolga@netapp.com>

Once the session is established call into the SUNRPC layer to check
if any offlined trunking connections should be re-enabled.

Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 00778f351283..6bf21bc8c074 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9244,6 +9244,13 @@ int nfs4_proc_create_session(struct nfs_client *clp, const struct cred *cred)
 	int status;
 	unsigned *ptr;
 	struct nfs4_session *session = clp->cl_session;
+	struct nfs4_add_xprt_data xprtdata = {
+		.clp = clp,
+	};
+	struct rpc_add_xprt_test rpcdata = {
+		.add_xprt_test = clp->cl_mvops->session_trunk,
+		.data = &xprtdata,
+	};
 
 	dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
 
@@ -9260,6 +9267,7 @@ int nfs4_proc_create_session(struct nfs_client *clp, const struct cred *cred)
 	ptr = (unsigned *)&session->sess_id.data[0];
 	dprintk("%s client>seqid %d sessionid %u:%u:%u:%u\n", __func__,
 		clp->cl_seqid, ptr[0], ptr[1], ptr[2], ptr[3]);
+	rpc_probe_trunked_xprts(clp->cl_rpcclient, &rpcdata);
 out:
 	return status;
 }
-- 
2.27.0