This fault injection hook causes all SEQUENCE operation replies to have the
SEQ4_STATUS_CB_PATH_DOWN flag set until the client calls BIND_CONN_TO_SESSION
or creates a new session.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfsd/fault_inject.c | 4 ++++
fs/nfsd/fault_inject.h | 2 ++
fs/nfsd/nfs4state.c | 20 ++++++++++++++++++++
3 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index e6c3815..ab81144 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -38,6 +38,10 @@ static struct nfsd_fault_inject_op inject_ops[] = {
.file = "recall_delegations",
.func = nfsd_recall_delegations,
},
+ {
+ .file = "kill_backchannel",
+ .func = nfsd_kill_backchannel,
+ },
};
static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
diff --git a/fs/nfsd/fault_inject.h b/fs/nfsd/fault_inject.h
index 90bd057..b058e03 100644
--- a/fs/nfsd/fault_inject.h
+++ b/fs/nfsd/fault_inject.h
@@ -15,6 +15,7 @@ void nfsd_forget_locks(u64);
void nfsd_forget_openowners(u64);
void nfsd_forget_delegations(u64);
void nfsd_recall_delegations(u64);
+void nfsd_kill_backchannel(u64);
#else /* CONFIG_NFSD_FAULT_INJECTION */
static inline int nfsd_fault_inject_init(void) { return 0; }
static inline void nfsd_fault_inject_cleanup(void) {}
@@ -23,6 +24,7 @@ static inline void nfsd_forget_locks(u64 num) {}
static inline void nfsd_forget_openowners(u64 num) {}
static inline void nfsd_forget_delegations(u64 num) {}
static inline void nfsd_recall_delegations(u64 num) {}
+static inline void nfsd_kill_backchannel(u64 num) {}
#endif /* CONFIG_NFSD_FAULT_INJECTION */
#endif /* LINUX_NFSD_FAULT_INJECT_H */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 21266c7..9afc902 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4737,6 +4737,26 @@ void nfsd_recall_delegations(u64 num)
printk(KERN_INFO "NFSD: Recalled %d delegations", count);
}
+/*
+ * Force SEQUENCE operations to have SEQ4_STATUS_CB_PATH_DOWN flag set
+ * until backchannel is reestablished by BIND_CONN_TO_SESSION or
+ * DESTROY_SESSION/CREATE_SESSION with SP4_NONE.
+ *
+ * The argument 'num' is ignored, any value will trigger the fault on
+ * all clients.
+ */
+void nfsd_kill_backchannel(u64 num)
+{
+ struct nfs4_client *clp, *next;
+
+ nfs4_lock_state();
+ list_for_each_entry_safe(clp, next, &client_lru, cl_lru) {
+ clp->cl_cb_state = NFSD4_CB_DOWN;
+ }
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: killed backchannel");
+}
#endif /* CONFIG_NFSD_FAULT_INJECTION */
/* initialization to perform at module load time: */
--
1.7.4.4
Initiate a CB probe when a new connection with the correct direction is added
to a session (IFF backchannel is marked as down). Without this a
BIND_CONN_TO_SESSION has no effect on the internal backchannel state, which
causes the server to reply to every SEQUENCE op with the
SEQ4_STATUS_CB_PATH_DOWN flag set until DESTROY_SESSION.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
Hey Bruce -
In an earlier thread I mentioned that I was setting the backchannel state to
CB_UNKNOWN to clear the SEQ4_STATUS flag, but this was wrong - I don't believe
it would ever change (to CB_UP/CB_DOWN). This patch seems like the right way
to handle the internal backchannel state.
Also, I'm a bit confused as how we're supposed to maintain atomicity around
cl_cb_state:
from nfs4callback.c:nfsd4_probe_callback():
/* XXX: atomicity? Also, should we be using cl_flags? */
clp->cl_cb_state = NFSD4_CB_UNKNOWN;
and there are several places that check the value of cl_cb_state without holding
a lock (as far as I can tell).
fs/nfsd/nfs4state.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9afc902..99f092e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -862,6 +862,11 @@ static __be32 nfsd4_new_conn(struct svc_rqst *rqstp, struct nfsd4_session *ses,
if (ret)
/* oops; xprt is already down: */
nfsd4_conn_lost(&conn->cn_xpt_user);
+ if (ses->se_client->cl_cb_state == NFSD4_CB_DOWN &&
+ dir & NFS4_CDFC4_BACK) {
+ /* callback channel may be back up */
+ nfsd4_probe_callback(ses->se_client);
+ }
return nfs_ok;
}
--
1.7.4.4
That rings a bell...
----- "Dros Adamson" <[email protected]> wrote:
>
> but handling this error condition should be useful for
> future work (session trunking, if the client ever uses a different
> connection for back channel).
--
Matt Benjamin
The Linux Box
206 South Fifth Ave. Suite 150
Ann Arbor, MI 48104
http://linuxbox.com
tel. 734-761-4689
fax. 734-769-8938
cel. 734-216-5309
On Thu, May 24, 2012 at 03:42:16PM -0400, Weston Andros Adamson wrote:
> This fault injection hook causes all SEQUENCE operation replies to have the
> SEQ4_STATUS_CB_PATH_DOWN flag set until the client calls BIND_CONN_TO_SESSION
> or creates a new session.
I'm not necessarily opposed, but why do you need this? In what
"real-life" situation could a client see the CB_PATH_DOWN flag
unexpectedly set?
--b.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfsd/fault_inject.c | 4 ++++
> fs/nfsd/fault_inject.h | 2 ++
> fs/nfsd/nfs4state.c | 20 ++++++++++++++++++++
> 3 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
> index e6c3815..ab81144 100644
> --- a/fs/nfsd/fault_inject.c
> +++ b/fs/nfsd/fault_inject.c
> @@ -38,6 +38,10 @@ static struct nfsd_fault_inject_op inject_ops[] = {
> .file = "recall_delegations",
> .func = nfsd_recall_delegations,
> },
> + {
> + .file = "kill_backchannel",
> + .func = nfsd_kill_backchannel,
> + },
> };
>
> static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
> diff --git a/fs/nfsd/fault_inject.h b/fs/nfsd/fault_inject.h
> index 90bd057..b058e03 100644
> --- a/fs/nfsd/fault_inject.h
> +++ b/fs/nfsd/fault_inject.h
> @@ -15,6 +15,7 @@ void nfsd_forget_locks(u64);
> void nfsd_forget_openowners(u64);
> void nfsd_forget_delegations(u64);
> void nfsd_recall_delegations(u64);
> +void nfsd_kill_backchannel(u64);
> #else /* CONFIG_NFSD_FAULT_INJECTION */
> static inline int nfsd_fault_inject_init(void) { return 0; }
> static inline void nfsd_fault_inject_cleanup(void) {}
> @@ -23,6 +24,7 @@ static inline void nfsd_forget_locks(u64 num) {}
> static inline void nfsd_forget_openowners(u64 num) {}
> static inline void nfsd_forget_delegations(u64 num) {}
> static inline void nfsd_recall_delegations(u64 num) {}
> +static inline void nfsd_kill_backchannel(u64 num) {}
> #endif /* CONFIG_NFSD_FAULT_INJECTION */
>
> #endif /* LINUX_NFSD_FAULT_INJECT_H */
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 21266c7..9afc902 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4737,6 +4737,26 @@ void nfsd_recall_delegations(u64 num)
> printk(KERN_INFO "NFSD: Recalled %d delegations", count);
> }
>
> +/*
> + * Force SEQUENCE operations to have SEQ4_STATUS_CB_PATH_DOWN flag set
> + * until backchannel is reestablished by BIND_CONN_TO_SESSION or
> + * DESTROY_SESSION/CREATE_SESSION with SP4_NONE.
> + *
> + * The argument 'num' is ignored, any value will trigger the fault on
> + * all clients.
> + */
> +void nfsd_kill_backchannel(u64 num)
> +{
> + struct nfs4_client *clp, *next;
> +
> + nfs4_lock_state();
> + list_for_each_entry_safe(clp, next, &client_lru, cl_lru) {
> + clp->cl_cb_state = NFSD4_CB_DOWN;
> + }
> + nfs4_unlock_state();
> +
> + printk(KERN_INFO "NFSD: killed backchannel");
> +}
> #endif /* CONFIG_NFSD_FAULT_INJECTION */
>
> /* initialization to perform at module load time: */
> --
> 1.7.4.4
>
On Jun 5, 2012, at 4:19 PM, J. Bruce Fields wrote:
> On Thu, May 24, 2012 at 03:42:16PM -0400, Weston Andros Adamson wrote:
>> This fault injection hook causes all SEQUENCE operation replies to have the
>> SEQ4_STATUS_CB_PATH_DOWN flag set until the client calls BIND_CONN_TO_SESSION
>> or creates a new session.
>
> I'm not necessarily opposed, but why do you need this? In what
> "real-life" situation could a client see the CB_PATH_DOWN flag
> unexpectedly set?
We need this to test the client's handling of the SEQ4_STATUS_CB_PATH_DOWN flag :)
Real world scenarios are hard to come by at this point since the linux client always uses one TCP connection (for both fore and back channel), but handling this error condition should be useful for future work (session trunking, if the client ever uses a different connection for back channel).
Also, if nfsd's probe callback fails for whatever reason the CB_PATH_DOWN flag will be set.
Really, we want the client to be able to handle this error condition and this was the easiest way to test it.
-dros
>
> --b.
>
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> fs/nfsd/fault_inject.c | 4 ++++
>> fs/nfsd/fault_inject.h | 2 ++
>> fs/nfsd/nfs4state.c | 20 ++++++++++++++++++++
>> 3 files changed, 26 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
>> index e6c3815..ab81144 100644
>> --- a/fs/nfsd/fault_inject.c
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -38,6 +38,10 @@ static struct nfsd_fault_inject_op inject_ops[] = {
>> .file = "recall_delegations",
>> .func = nfsd_recall_delegations,
>> },
>> + {
>> + .file = "kill_backchannel",
>> + .func = nfsd_kill_backchannel,
>> + },
>> };
>>
>> static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
>> diff --git a/fs/nfsd/fault_inject.h b/fs/nfsd/fault_inject.h
>> index 90bd057..b058e03 100644
>> --- a/fs/nfsd/fault_inject.h
>> +++ b/fs/nfsd/fault_inject.h
>> @@ -15,6 +15,7 @@ void nfsd_forget_locks(u64);
>> void nfsd_forget_openowners(u64);
>> void nfsd_forget_delegations(u64);
>> void nfsd_recall_delegations(u64);
>> +void nfsd_kill_backchannel(u64);
>> #else /* CONFIG_NFSD_FAULT_INJECTION */
>> static inline int nfsd_fault_inject_init(void) { return 0; }
>> static inline void nfsd_fault_inject_cleanup(void) {}
>> @@ -23,6 +24,7 @@ static inline void nfsd_forget_locks(u64 num) {}
>> static inline void nfsd_forget_openowners(u64 num) {}
>> static inline void nfsd_forget_delegations(u64 num) {}
>> static inline void nfsd_recall_delegations(u64 num) {}
>> +static inline void nfsd_kill_backchannel(u64 num) {}
>> #endif /* CONFIG_NFSD_FAULT_INJECTION */
>>
>> #endif /* LINUX_NFSD_FAULT_INJECT_H */
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 21266c7..9afc902 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4737,6 +4737,26 @@ void nfsd_recall_delegations(u64 num)
>> printk(KERN_INFO "NFSD: Recalled %d delegations", count);
>> }
>>
>> +/*
>> + * Force SEQUENCE operations to have SEQ4_STATUS_CB_PATH_DOWN flag set
>> + * until backchannel is reestablished by BIND_CONN_TO_SESSION or
>> + * DESTROY_SESSION/CREATE_SESSION with SP4_NONE.
>> + *
>> + * The argument 'num' is ignored, any value will trigger the fault on
>> + * all clients.
>> + */
>> +void nfsd_kill_backchannel(u64 num)
>> +{
>> + struct nfs4_client *clp, *next;
>> +
>> + nfs4_lock_state();
>> + list_for_each_entry_safe(clp, next, &client_lru, cl_lru) {
>> + clp->cl_cb_state = NFSD4_CB_DOWN;
>> + }
>> + nfs4_unlock_state();
>> +
>> + printk(KERN_INFO "NFSD: killed backchannel");
>> +}
>> #endif /* CONFIG_NFSD_FAULT_INJECTION */
>>
>> /* initialization to perform at module load time: */
>> --
>> 1.7.4.4
>>
On Thu, May 24, 2012 at 03:42:17PM -0400, Weston Andros Adamson wrote:
> Initiate a CB probe when a new connection with the correct direction is added
> to a session (IFF backchannel is marked as down). Without this a
> BIND_CONN_TO_SESSION has no effect on the internal backchannel state, which
> causes the server to reply to every SEQUENCE op with the
> SEQ4_STATUS_CB_PATH_DOWN flag set until DESTROY_SESSION.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> Hey Bruce -
> In an earlier thread I mentioned that I was setting the backchannel state to
> CB_UNKNOWN to clear the SEQ4_STATUS flag, but this was wrong - I don't believe
> it would ever change (to CB_UP/CB_DOWN). This patch seems like the right way
> to handle the internal backchannel state.
Thanks, yes, this looks right, queueing up for 3.6.
> Also, I'm a bit confused as how we're supposed to maintain atomicity around
> cl_cb_state:
>
> from nfs4callback.c:nfsd4_probe_callback():
> /* XXX: atomicity? Also, should we be using cl_flags? */
> clp->cl_cb_state = NFSD4_CB_UNKNOWN;
>
> and there are several places that check the value of cl_cb_state without holding
> a lock (as far as I can tell).
I think that's probably OK.
We set cl_cb_state to UNKNOWN only before initiating a callback that
will eventually result in setting it to UP, DOWN, or FAULT.
And I think that's good enough for the readers.
So probably that comment should just go.
--b.
>
> fs/nfsd/nfs4state.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 9afc902..99f092e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -862,6 +862,11 @@ static __be32 nfsd4_new_conn(struct svc_rqst *rqstp, struct nfsd4_session *ses,
> if (ret)
> /* oops; xprt is already down: */
> nfsd4_conn_lost(&conn->cn_xpt_user);
> + if (ses->se_client->cl_cb_state == NFSD4_CB_DOWN &&
> + dir & NFS4_CDFC4_BACK) {
> + /* callback channel may be back up */
> + nfsd4_probe_callback(ses->se_client);
> + }
> return nfs_ok;
> }
>
> --
> 1.7.4.4
>