2014-08-22 19:10:44

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 0/4] Simple-minded server fixes for sidecar backchannel

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


2014-08-22 19:11:17

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 4/4] nfsd: Assert SEQ4_STATUS_CB_PATH_DOWN_SESSION

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 <[email protected]>
---

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;


2014-08-22 19:11:01

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 2/4] NFSD: Always initialize cl_cb_addr

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 <[email protected]>
---

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


2014-08-22 19:11:09

by Chuck Lever III

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

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 <[email protected]>
---

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);
/*


2014-08-27 21:16:50

by J. Bruce Fields

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

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 <[email protected]>
> ---
>
> 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;
> }
>
>

2014-08-22 19:10:52

by Chuck Lever III

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

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

Signed-off-by: Chuck Lever <[email protected]>
---

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;
}



2014-10-02 19:53:39

by J. Bruce Fields

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

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 <[email protected]>
> ---
>
> 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
>