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

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-QHcLZuEGTsvQT0dZR+***@public.gmane.org>
---

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


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[J. Bruce Fields]

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

Thanks, applying.--b.

Post by Chuck Lever
---
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;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[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-QHcLZuEGTsvQT0dZR+***@public.gmane.org>
---

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

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[J. Bruce Fields]

Post by 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.

OK, applying.--b.

Post by Chuck Lever
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[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-QHcLZuEGTsvQT0dZR+***@public.gmane.org>
---

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

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[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 backchanne=
l
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=E2=80=99s backchannel. This bit remains set on al=
l
SEQUENCE responses of the session until a connection is associate=
d
with the session=E2=80=99s 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-QHcLZuEGTsvQT0dZR+***@public.gmane.org>
---

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 =3D SEQ4_STATUS_CB_PATH_DOWN;
+ seq->status_flags =3D SEQ4_STATUS_CB_PATH_DOWN |
+ SEQ4_STATUS_CB_PATH_DOWN_SESSION;
break;
case NFSD4_CB_FAULT:
seq->status_flags =3D SEQ4_STATUS_BACKCHANNEL_FAULT;

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html