* [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
@ 2011-04-27 1:09 Mi Jinlong
2011-04-27 2:41 ` J. Bruce Fields
0 siblings, 1 reply; 5+ messages in thread
From: Mi Jinlong @ 2011-04-27 1:09 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: NFS
Make sure nfs server can distinguish request contains more ops
than channel allowed.
Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
---
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 fbde6f7..4f9fc68 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp,
if (!session)
goto out;
+ status = nfserr_too_many_ops;
+ if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt >
+ session->se_fchannel.maxops)
+ goto out;
+
status = nfserr_badslot;
if (seq->slotid >= session->se_fchannel.maxreqs)
goto out;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
2011-04-27 1:09 [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence Mi Jinlong
@ 2011-04-27 2:41 ` J. Bruce Fields
2011-04-27 5:48 ` Mi Jinlong
0 siblings, 1 reply; 5+ messages in thread
From: J. Bruce Fields @ 2011-04-27 2:41 UTC (permalink / raw)
To: Mi Jinlong; +Cc: NFS
On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote:
> Make sure nfs server can distinguish request contains more ops
> than channel allowed.
Yes, sequence looks like a reasonable op to catch this error. The spec
doesn't care as far as I can tell (and it's a buggy-client case, so why
should it), and we already check that any compound not starting with a
sequence has only one op.
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
> 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 fbde6f7..4f9fc68 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp,
> if (!session)
> goto out;
>
> + status = nfserr_too_many_ops;
> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt >
Kind of a cumbersome construction, though.
Eh, maybe overkill, but how about this?:
--b.
commit d5eee1629fb9d3a55e5793d156026248c14cb46c
Author: Mi Jinlong <mijinlong@cn.fujitsu.com>
Date: Wed Apr 27 09:09:58 2011 +0800
nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
Make sure nfs server errors out if request contains more ops
than channel allows.
Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
[bfields@redhat.com: use helper function]
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fbde6f7..487ba47 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi
return;
}
+static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session)
+{
+ struct nfsd4_compoundargs *args = rqstp->rq_argp;
+
+ return args->opcnt > session->se_fchannel.maxops;
+}
+
__be32
nfsd4_sequence(struct svc_rqst *rqstp,
struct nfsd4_compound_state *cstate,
@@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp,
if (!session)
goto out;
+ status = nfserr_too_many_ops;
+ if (nfsd4_session_too_many_ops(rqstp, session))
+ goto out;
+
status = nfserr_badslot;
if (seq->slotid >= session->se_fchannel.maxreqs)
goto out;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
2011-04-27 2:41 ` J. Bruce Fields
@ 2011-04-27 5:48 ` Mi Jinlong
2011-05-03 3:48 ` J. Bruce Fields
0 siblings, 1 reply; 5+ messages in thread
From: Mi Jinlong @ 2011-04-27 5:48 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: NFS
J. Bruce Fields:
> On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote:
>> Make sure nfs server can distinguish request contains more ops
>> than channel allowed.
>
> Yes, sequence looks like a reasonable op to catch this error. The spec
> doesn't care as far as I can tell (and it's a buggy-client case, so why
> should it), and we already check that any compound not starting with a
> sequence has only one op.
>
>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
>> ---
>> 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 fbde6f7..4f9fc68 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp,
>> if (!session)
>> goto out;
>>
>> + status = nfserr_too_many_ops;
>> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt >
>
> Kind of a cumbersome construction, though.
>
> Eh, maybe overkill, but how about this?:
That's great!
--
----
thanks
Mi Jinlong
>
> --b.
>
> commit d5eee1629fb9d3a55e5793d156026248c14cb46c
> Author: Mi Jinlong <mijinlong@cn.fujitsu.com>
> Date: Wed Apr 27 09:09:58 2011 +0800
>
> nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
>
> Make sure nfs server errors out if request contains more ops
> than channel allows.
>
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> [bfields@redhat.com: use helper function]
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index fbde6f7..487ba47 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi
> return;
> }
>
> +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session)
> +{
> + struct nfsd4_compoundargs *args = rqstp->rq_argp;
> +
> + return args->opcnt > session->se_fchannel.maxops;
> +}
> +
> __be32
> nfsd4_sequence(struct svc_rqst *rqstp,
> struct nfsd4_compound_state *cstate,
> @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp,
> if (!session)
> goto out;
>
> + status = nfserr_too_many_ops;
> + if (nfsd4_session_too_many_ops(rqstp, session))
> + goto out;
> +
> status = nfserr_badslot;
> if (seq->slotid >= session->se_fchannel.maxreqs)
> goto out;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
2011-04-27 5:48 ` Mi Jinlong
@ 2011-05-03 3:48 ` J. Bruce Fields
2011-05-09 9:26 ` Mi Jinlong
0 siblings, 1 reply; 5+ messages in thread
From: J. Bruce Fields @ 2011-05-03 3:48 UTC (permalink / raw)
To: Mi Jinlong; +Cc: NFS, iisaman
On Wed, Apr 27, 2011 at 01:48:09PM +0800, Mi Jinlong wrote:
>
>
> J. Bruce Fields:
> > On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote:
> >> Make sure nfs server can distinguish request contains more ops
> >> than channel allowed.
> >
> > Yes, sequence looks like a reasonable op to catch this error. The spec
> > doesn't care as far as I can tell (and it's a buggy-client case, so why
> > should it), and we already check that any compound not starting with a
> > sequence has only one op.
> >
> >> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> >> ---
> >> 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 fbde6f7..4f9fc68 100644
> >> --- a/fs/nfsd/nfs4state.c
> >> +++ b/fs/nfsd/nfs4state.c
> >> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp,
> >> if (!session)
> >> goto out;
> >>
> >> + status = nfserr_too_many_ops;
> >> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt >
> >
> > Kind of a cumbersome construction, though.
> >
> > Eh, maybe overkill, but how about this?:
>
> That's great!
But note we're making a bunch of the pynfs tests fail now, since they
aren't careful about how many ops they use in a compound. For example,
LKPP1d fails for me now. Note it might not fail for you if you put the
pynfs test directory right at the root of the export tree?
I don't think pynfs hsould be using absolute paths for everything. And
it should probably be doing single-component lookups in a loop instead
of depending on being able to do the whole lookup in one compound,
unless it's willing to adapt the compounds to take into account
maxops--which sounds to me like more trouble than it's worth.
The server could probably also raise its limit, though.
--b.
>
> --
> ----
> thanks
> Mi Jinlong
>
> >
> > --b.
> >
> > commit d5eee1629fb9d3a55e5793d156026248c14cb46c
> > Author: Mi Jinlong <mijinlong@cn.fujitsu.com>
> > Date: Wed Apr 27 09:09:58 2011 +0800
> >
> > nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
> >
> > Make sure nfs server errors out if request contains more ops
> > than channel allows.
> >
> > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> > [bfields@redhat.com: use helper function]
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index fbde6f7..487ba47 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi
> > return;
> > }
> >
> > +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session)
> > +{
> > + struct nfsd4_compoundargs *args = rqstp->rq_argp;
> > +
> > + return args->opcnt > session->se_fchannel.maxops;
> > +}
> > +
> > __be32
> > nfsd4_sequence(struct svc_rqst *rqstp,
> > struct nfsd4_compound_state *cstate,
> > @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp,
> > if (!session)
> > goto out;
> >
> > + status = nfserr_too_many_ops;
> > + if (nfsd4_session_too_many_ops(rqstp, session))
> > + goto out;
> > +
> > status = nfserr_badslot;
> > if (seq->slotid >= session->se_fchannel.maxreqs)
> > goto out;
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
2011-05-03 3:48 ` J. Bruce Fields
@ 2011-05-09 9:26 ` Mi Jinlong
0 siblings, 0 replies; 5+ messages in thread
From: Mi Jinlong @ 2011-05-09 9:26 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: NFS, iisaman
J. Bruce Fields:
> On Wed, Apr 27, 2011 at 01:48:09PM +0800, Mi Jinlong wrote:
>>
>> J. Bruce Fields:
>>> On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote:
>>>> Make sure nfs server can distinguish request contains more ops
>>>> than channel allowed.
>>> Yes, sequence looks like a reasonable op to catch this error. The spec
>>> doesn't care as far as I can tell (and it's a buggy-client case, so why
>>> should it), and we already check that any compound not starting with a
>>> sequence has only one op.
>>>
>>>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
>>>> ---
>>>> 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 fbde6f7..4f9fc68 100644
>>>> --- a/fs/nfsd/nfs4state.c
>>>> +++ b/fs/nfsd/nfs4state.c
>>>> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp,
>>>> if (!session)
>>>> goto out;
>>>>
>>>> + status = nfserr_too_many_ops;
>>>> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt >
>>> Kind of a cumbersome construction, though.
>>>
>>> Eh, maybe overkill, but how about this?:
>> That's great!
>
> But note we're making a bunch of the pynfs tests fail now, since they
> aren't careful about how many ops they use in a compound. For example,
> LKPP1d fails for me now. Note it might not fail for you if you put the
> pynfs test directory right at the root of the export tree?
Yes, I don't get LKPP1d fail.
> I don't think pynfs hsould be using absolute paths for everything. And
> it should probably be doing single-component lookups in a loop instead
> of depending on being able to do the whole lookup in one compound,
> unless it's willing to adapt the compounds to take into account
> maxops--which sounds to me like more trouble than it's worth.
I agree the second idea although it's more trouble.
Because simulating a NFS4.1 client, we should make sure pynfs41 can
make request with ops which less than maxops.
>
> The server could probably also raise its limit, though.
I think, we can do it as raising the maxops at CREATE_SESSION ops
when create session, I don't test it.
--
----
thanks
Mi Jinlong
>>> commit d5eee1629fb9d3a55e5793d156026248c14cb46c
>>> Author: Mi Jinlong <mijinlong@cn.fujitsu.com>
>>> Date: Wed Apr 27 09:09:58 2011 +0800
>>>
>>> nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence
>>>
>>> Make sure nfs server errors out if request contains more ops
>>> than channel allows.
>>>
>>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
>>> [bfields@redhat.com: use helper function]
>>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>>>
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index fbde6f7..487ba47 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi
>>> return;
>>> }
>>>
>>> +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session)
>>> +{
>>> + struct nfsd4_compoundargs *args = rqstp->rq_argp;
>>> +
>>> + return args->opcnt > session->se_fchannel.maxops;
>>> +}
>>> +
>>> __be32
>>> nfsd4_sequence(struct svc_rqst *rqstp,
>>> struct nfsd4_compound_state *cstate,
>>> @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp,
>>> if (!session)
>>> goto out;
>>>
>>> + status = nfserr_too_many_ops;
>>> + if (nfsd4_session_too_many_ops(rqstp, session))
>>> + goto out;
>>> +
>>> status = nfserr_badslot;
>>> if (seq->slotid >= session->se_fchannel.maxreqs)
>>> goto out;
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-09 9:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-27 1:09 [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence Mi Jinlong
2011-04-27 2:41 ` J. Bruce Fields
2011-04-27 5:48 ` Mi Jinlong
2011-05-03 3:48 ` J. Bruce Fields
2011-05-09 9:26 ` Mi Jinlong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).