[PATCH 0/4] Simple-minded server fixes for sidecar backchannel

[PATCH 0/4] Simple-minded server fixes for sidecar backchannel

[Chuck Lever]

Hi Bruce-

I've been testing these for my client backchannel work. These are a
naive first pass at fixing server issues with multiple connections
per session.

---

Chuck Lever (4):
      nfsd: Assert SEQ4_STATUS_CB_PATH_DOWN_SESSION
      nfsd: Mark back channel down if SESSION4_BACK_CHAN is clear
      NFSD: Always initialize cl_cb_addr
      sunrpc: fix byte-swapping of displayed XID


 fs/nfsd/nfs4state.c  |    8 ++++++--
 net/sunrpc/svcsock.c |    2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
Chuck Lever

[PATCH 1/4] sunrpc: fix byte-swapping of displayed XID

[Chuck Lever]

xprt_lookup_rqst() and bc_send_request() display a byte-swapped XID,
but receive_cb_reply() does not.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/svcsock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index c24a8ff..6ed8729 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1036,7 +1036,7 @@ static int receive_cb_reply(struct svc_sock *svsk, struct svc_rqst *rqstp)
 			"%s: Got unrecognized reply: "
 			"calldir 0x%x xpt_bc_xprt %p xid %08x\n",
 			__func__, ntohl(calldir),
-			bc_xprt, xid);
+			bc_xprt, ntohl(xid));
 		return -EAGAIN;
 	}
 

[PATCH 2/4] NFSD: Always initialize cl_cb_addr

[Chuck Lever]

A client may not want to use the back channel on a transport it sent
CREATE_SESSION on, in which case it clears SESSION4_BACK_CHAN.

However, cl_cb_addr should be populated anyway, to be used if the
client binds other connections to this session. If cl_cb_addr is
not initialized, rpc_create() fails when the server attempts to
set up a back channel on such secondary transports.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfs4state.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2e80a59..a45f244 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1429,7 +1429,7 @@ static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, stru
 	list_add(&new->se_perclnt, &clp->cl_sessions);
 	spin_unlock(&clp->cl_lock);
 
-	if (cses->flags & SESSION4_BACK_CHAN) {
+	{
 		struct sockaddr *sa = svc_addr(rqstp);
 		/*
 		 * This is a little silly; with sessions there's no real

[PATCH 3/4] nfsd: Mark back channel down if SESSION4_BACK_CHAN is clear

[Chuck Lever]

If a client indicates via CREATE_SESSION that no back channel
should be used on the existing connection, mark the client's
cl_cb_state DOWN instead of leaving it UNKNOWN.

Otherwise, a fresh session is created without an operational
backchannel, but the server never asserts SEQ4_STATUS_CB_PATH_DOWN.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfs4state.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a45f244..d7cfd8b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1429,6 +1429,9 @@ static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, stru
 	list_add(&new->se_perclnt, &clp->cl_sessions);
 	spin_unlock(&clp->cl_lock);
 
+	if (!(cses->flags & SESSION4_BACK_CHAN))
+		clp->cl_cb_state = NFSD4_CB_DOWN;
+
 	{
 		struct sockaddr *sa = svc_addr(rqstp);
 		/*

[PATCH 4/4] nfsd: Assert SEQ4_STATUS_CB_PATH_DOWN_SESSION

[Chuck Lever]

The NFS server should assert both CB_PATH_DOWN and CB_PATH_DOWN_SESSION
when there is no backchannel.

RFC 5661 section 18.46.3:

   SEQ4_STATUS_CB_PATH_DOWN
      When set, indicates that the client has no operational backchannel
      path for any session associated with the client ID, making it
      necessary for the client to re-establish one.  This bit remains
      set on all SEQUENCE responses on all sessions associated with the
      client ID until at least one backchannel is available on any
      session associated with the client ID.


   SEQ4_STATUS_CB_PATH_DOWN_SESSION
      When set, indicates that the session has no operational
      backchannel. . .
                . . . The SEQ4_STATUS_CB_PATH_DOWN_SESSION bit is the
      indication to the client that it needs to associate a connection
      to the session’s backchannel.  This bit remains set on all
      SEQUENCE responses of the session until a connection is associated
      with the session’s a backchannel.

The Linux server doesn't currently make a distinction between these
cases, so both flags should be asserted unconditionally.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfs4state.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d7cfd8b..ea50ac0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2805,7 +2805,8 @@ nfsd4_sequence(struct svc_rqst *rqstp,
 out:
 	switch (clp->cl_cb_state) {
 	case NFSD4_CB_DOWN:
-		seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN;
+		seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN |
+				    SEQ4_STATUS_CB_PATH_DOWN_SESSION;
 		break;
 	case NFSD4_CB_FAULT:
 		seq->status_flags = SEQ4_STATUS_BACKCHANNEL_FAULT;

Re: [PATCH 1/4] sunrpc: fix byte-swapping of displayed XID

[J. Bruce Fields]

On Fri, Aug 22, 2014 at 03:10:50PM -0400, Chuck Lever wrote:
> xprt_lookup_rqst() and bc_send_request() display a byte-swapped XID,
> but receive_cb_reply() does not.

Thanks, applying.--b.

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  net/sunrpc/svcsock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index c24a8ff..6ed8729 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1036,7 +1036,7 @@ static int receive_cb_reply(struct svc_sock *svsk, struct svc_rqst *rqstp)
>  			"%s: Got unrecognized reply: "
>  			"calldir 0x%x xpt_bc_xprt %p xid %08x\n",
>  			__func__, ntohl(calldir),
> -			bc_xprt, xid);
> +			bc_xprt, ntohl(xid));
>  		return -EAGAIN;
>  	}
>  
> 

Re: [PATCH 2/4] NFSD: Always initialize cl_cb_addr

[J. Bruce Fields]

On Fri, Aug 22, 2014 at 03:10:59PM -0400, Chuck Lever wrote:
> A client may not want to use the back channel on a transport it sent
> CREATE_SESSION on, in which case it clears SESSION4_BACK_CHAN.
> 
> However, cl_cb_addr should be populated anyway, to be used if the
> client binds other connections to this session. If cl_cb_addr is
> not initialized, rpc_create() fails when the server attempts to
> set up a back channel on such secondary transports.

OK, applying.--b.

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfsd/nfs4state.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 2e80a59..a45f244 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1429,7 +1429,7 @@ static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, stru
>  	list_add(&new->se_perclnt, &clp->cl_sessions);
>  	spin_unlock(&clp->cl_lock);
>  
> -	if (cses->flags & SESSION4_BACK_CHAN) {
> +	{
>  		struct sockaddr *sa = svc_addr(rqstp);
>  		/*
>  		 * This is a little silly; with sessions there's no real
>