From: Benny Halevy <bhalevy@tonian.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "J. Bruce Fields" <bfields@redhat.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x share_access flags
Date: Thu, 20 Oct 2011 11:59:21 -0700 [thread overview]
Message-ID: <4EA06F89.8090207@tonian.com> (raw)
In-Reply-To: <20111020115629.GJ5444@fieldses.org>
On 2011-10-20 04:56, J. Bruce Fields wrote:
> On Wed, Oct 19, 2011 at 07:13:37PM -0700, Benny Halevy wrote:
>> From: Benny Halevy <bhalevy@tonian.com>
>>
>> Currently, it will not correctl ignore and nfsv4.1 signal flags if the client
>> sends them.
>
> Good catch, but since b6d2f1ca3c1162f51098969e9c52fd099720416a "nfsd4:
> more robust ignoring of WANT bits in OPEN" we're actually doing this
> right from the start, and this is redundant.
>
> (And remembering to mask out the other bits on every use is so
> error-prone, I'm wondering if we should actually put the other bits in a
> separate field entirely. Either that or maybe provide a little helper
> function to get the access bits and ensure they're never accessed any
> other way.)
Agreed, something along these lines? (untested)
>From 3eb3100dc8c8c3833ae24b09d356001245b7e691 Mon Sep 17 00:00:00 2001
From: Benny Halevy <bhalevy@tonian.com>
Date: Thu, 20 Oct 2011 07:12:47 -0700
Subject: [PATCH] nfsd4: split out share_access want and signel flags while decoding
Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
fs/nfsd/nfs4proc.c | 3 ---
fs/nfsd/nfs4state.c | 8 ++++----
fs/nfsd/nfs4xdr.c | 25 +++++++++++++++++++------
fs/nfsd/xdr4.h | 6 ++++--
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2f87ea5..65f6c86 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -319,9 +319,6 @@ static __be32 nfsd_check_obj_isreg(struct svc_fh *fh)
if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
return nfserr_inval;
- /* We don't yet support WANT bits: */
- open->op_share_access &= NFS4_SHARE_ACCESS_MASK;
-
/*
* RFC5661 18.51.3
* Before RECLAIM_COMPLETE done, server should deny new lock
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9701d71..d00c246 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2657,8 +2657,6 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
static int share_access_to_flags(u32 share_access)
{
- share_access &= NFS4_SHARE_ACCESS_MASK;
-
return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
}
@@ -3036,7 +3034,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
if (nfsd4_has_session(&resp->cstate)) {
open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
- if (open->op_share_access & NFS4_SHARE_WANT_NO_DELEG) {
+ if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
goto nodeleg;
}
@@ -3654,7 +3652,9 @@ static inline void nfs4_stateid_downgrade(struct nfs4_ol_stateid *stp, u32 to_ac
cstate->current_fh.fh_dentry->d_name.name);
/* We don't yet support WANT bits: */
- od->od_share_access &= NFS4_SHARE_ACCESS_MASK;
+ if (od->od_deleg_want)
+ dprintk("NFSD: %s: od_deleg_want=0x%x ignored\n", __func__,
+ od->od_deleg_want);
nfs4_lock_state();
status = nfs4_preprocess_confirmed_seqid_op(cstate, od->od_seqid,
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 79cb105..9b838e9 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -644,15 +644,19 @@ static __be32 nfsd4_decode_bind_conn_to_session(struct nfsd4_compoundargs *argp,
DECODE_TAIL;
}
-static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
+static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *share_access, u32 *deleg_want, u32 *deleg_when)
{
__be32 *p;
u32 w;
READ_BUF(4);
READ32(w);
- *x = w;
- switch (w & NFS4_SHARE_ACCESS_MASK) {
+ *share_access = w & NFS4_SHARE_ACCESS_MASK;
+ *deleg_want = w & NFS4_SHARE_WANT_MASK;
+ if (deleg_when)
+ *deleg_when = w & NFS4_SHARE_WHEN_MASK;
+
+ switch (w & NFS4_SHARE_WHEN_MASK) {
case NFS4_SHARE_ACCESS_READ:
case NFS4_SHARE_ACCESS_WRITE:
case NFS4_SHARE_ACCESS_BOTH:
@@ -660,11 +664,13 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
default:
return nfserr_bad_xdr;
}
- w &= !NFS4_SHARE_ACCESS_MASK;
+ w &= ~NFS4_SHARE_ACCESS_MASK;
if (!w)
return nfs_ok;
+
if (!argp->minorversion)
return nfserr_bad_xdr;
+
switch (w & NFS4_SHARE_WANT_MASK) {
case NFS4_SHARE_WANT_NO_PREFERENCE:
case NFS4_SHARE_WANT_READ_DELEG:
@@ -679,6 +685,9 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
w &= ~NFS4_SHARE_WANT_MASK;
if (!w)
return nfs_ok;
+
+ if (!deleg_when) /* open_downgrade */
+ return nfserr_inval;
switch (w) {
case NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL:
case NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED:
@@ -725,6 +734,7 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
{
DECODE_HEAD;
+ u32 dummy;
memset(open->op_bmval, 0, sizeof(open->op_bmval));
open->op_iattr.ia_valid = 0;
@@ -733,7 +743,9 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
/* seqid, share_access, share_deny, clientid, ownerlen */
READ_BUF(4);
READ32(open->op_seqid);
- status = nfsd4_decode_share_access(argp, &open->op_share_access);
+ /* decode, yet ignore deleg_when until supported */
+ status = nfsd4_decode_share_access(argp, &open->op_share_access,
+ &open->op_deleg_want, &dummy);
if (status)
goto xdr_error;
status = nfsd4_decode_share_deny(argp, &open->op_share_deny);
@@ -854,7 +866,8 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
return status;
READ_BUF(4);
READ32(open_down->od_seqid);
- status = nfsd4_decode_share_access(argp, &open_down->od_share_access);
+ status = nfsd4_decode_share_access(argp, &open_down->od_share_access,
+ &open_down->od_deleg_want, NULL);
if (status)
return status;
status = nfsd4_decode_share_deny(argp, &open_down->od_share_deny);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index bf4c9e7..1e5ca95 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -224,6 +224,7 @@ struct nfsd4_open {
u32 op_seqid; /* request */
u32 op_share_access; /* request */
u32 op_share_deny; /* request */
+ u32 op_deleg_want; /* request */
stateid_t op_stateid; /* response */
u32 op_recall; /* recall */
struct nfsd4_change_info op_cinfo; /* response */
@@ -244,8 +245,9 @@ struct nfsd4_open_confirm {
struct nfsd4_open_downgrade {
stateid_t od_stateid;
u32 od_seqid;
- u32 od_share_access;
- u32 od_share_deny;
+ u32 od_share_access; /* request */
+ u32 od_deleg_want; /* request */
+ u32 od_share_deny; /* request */
};
--
1.7.6
next prev parent reply other threads:[~2011-10-20 18:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-20 2:10 [PATCH 0/7] Bakeathon server fixes Benny Halevy
2011-10-20 2:12 ` [PATCH 1/7] nfsd4: implement new 4.1 open reclaim types Benny Halevy
2011-10-20 12:23 ` J. Bruce Fields
2011-10-20 2:12 ` [PATCH 2/7] nfsd41: use SEQ4_STATUS_BACKCHANNEL_FAULT when cb_sequence is invalid Benny Halevy
2011-10-20 2:13 ` [PATCH 3/7] nfsd4: seq->status_flags may be used unitialized Benny Halevy
2011-10-20 2:13 ` [PATCH 4/7] nfsd4: allow NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL | NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED Benny Halevy
2011-10-20 2:13 ` [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT Benny Halevy
2011-10-20 11:52 ` J. Bruce Fields
2011-10-20 12:36 ` Benny Halevy
2011-10-20 12:47 ` J. Bruce Fields
2011-10-20 19:16 ` Benny Halevy
2011-10-20 19:21 ` J. Bruce Fields
2011-10-20 2:13 ` [PATCH 6/7] nfsd4: typo logical vs bitwise negate for want_mask Benny Halevy
2011-10-20 11:53 ` J. Bruce Fields
2011-10-20 2:13 ` [PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x share_access flags Benny Halevy
2011-10-20 11:56 ` J. Bruce Fields
2011-10-20 18:59 ` Benny Halevy [this message]
2011-10-20 11:58 ` [PATCH 0/7] Bakeathon server fixes J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EA06F89.8090207@tonian.com \
--to=bhalevy@tonian.com \
--cc=bfields@fieldses.org \
--cc=bfields@redhat.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).