Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <4c6c46419d34cbe0a408aea4705383edecffa00d.camel@poochiereds.net>
Subject: Re: [PATCH v1 3/7] NFSD: Add nfsd4_encode_open_read_delegation4()
From:   Jeff Layton <jlayton@poochiereds.net>
To:     Chuck Lever <cel@kernel.org>, linux-nfs@vger.kernel.org
Cc:     Chuck Lever <chuck.lever@oracle.com>
Date:   Fri, 29 Sep 2023 12:21:28 -0400
In-Reply-To: <169599594587.5622.94747681619250190.stgit@manet.1015granger.net>
References: <169599581942.5622.15965175797823365235.stgit@manet.1015granger.net>
         <169599594587.5622.94747681619250190.stgit@manet.1015granger.net>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-09-29 at 09:59 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>=20
> Refactor nfsd4_encode_open() so the open_read_delegation4 type is
> encoded in a separate function. This makes it more straightforward
> to later add support for returning an nfsace4 in OPEN responses that
> offer a delegation.
>=20
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4state.c |    6 +++--
>  fs/nfsd/nfs4xdr.c   |   61 ++++++++++++++++++++++++++++++++++++++-------=
------
>  fs/nfsd/xdr4.h      |    2 +-
>  3 files changed, 49 insertions(+), 20 deletions(-)
>=20
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 22e95b9ae82f..b1118050ff52 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5688,11 +5688,11 @@ nfs4_open_delegation(struct nfsd4_open *open, str=
uct nfs4_ol_stateid *stp,
>  	struct path path;
> =20
>  	cb_up =3D nfsd4_cb_channel_good(oo->oo_owner.so_client);
> -	open->op_recall =3D 0;
> +	open->op_recall =3D false;
>  	switch (open->op_claim_type) {
>  		case NFS4_OPEN_CLAIM_PREVIOUS:
>  			if (!cb_up)
> -				open->op_recall =3D 1;
> +				open->op_recall =3D true;
>  			break;
>  		case NFS4_OPEN_CLAIM_NULL:
>  			parent =3D currentfh;
> @@ -5746,7 +5746,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struc=
t nfs4_ol_stateid *stp,
>  	if (open->op_claim_type =3D=3D NFS4_OPEN_CLAIM_PREVIOUS &&
>  	    open->op_delegate_type !=3D NFS4_OPEN_DELEGATE_NONE) {
>  		dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> -		open->op_recall =3D 1;
> +		open->op_recall =3D true;
>  	}
> =20
>  	/* 4.1 client asking for a delegation? */
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index ee8a7989f54f..f411fcc435f6 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -4074,6 +4074,49 @@ nfsd4_encode_link(struct nfsd4_compoundres *resp, =
__be32 nfserr,
>  	return nfsd4_encode_change_info4(xdr, &link->li_cinfo);
>  }
> =20
> +/*
> + * This implementation does not yet support returning an ACE in an
> + * OPEN that offers a delegation.
> + */
> +static __be32
> +nfsd4_encode_open_nfsace4(struct xdr_stream *xdr)
> +{
> +	__be32 status;
> +
> +	/* type */
> +	status =3D nfsd4_encode_acetype4(xdr, NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE)=
;
> +	if (status !=3D nfs_ok)
> +		return nfserr_resource;
> +	/* flag */
> +	status =3D nfsd4_encode_aceflag4(xdr, 0);
> +	if (status !=3D nfs_ok)
> +		return nfserr_resource;
> +	/* access mask */
> +	status =3D nfsd4_encode_acemask4(xdr, 0);
> +	if (status !=3D nfs_ok)
> +		return nfserr_resource;
> +	/* who - empty for now */
> +	if (xdr_stream_encode_u32(xdr, 0) !=3D XDR_UNIT)
> +		return nfserr_resource;
> +	return nfs_ok;
> +}
> +
> +static __be32
> +nfsd4_encode_open_read_delegation4(struct xdr_stream *xdr, struct nfsd4_=
open *open)
> +{
> +	__be32 status;
> +
> +	/* stateid */
> +	status =3D nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> +	if (status !=3D nfs_ok)
> +		return status;
> +	/* recall */
> +	status =3D nfsd4_encode_bool(xdr, open->op_recall);
> +	if (status !=3D nfs_ok)
> +		return status;
> +	/* permissions */
> +	return nfsd4_encode_open_nfsace4(xdr);
> +}
> =20
>  static __be32
>  nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr,
> @@ -4106,22 +4149,8 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, =
__be32 nfserr,
>  	case NFS4_OPEN_DELEGATE_NONE:
>  		break;
>  	case NFS4_OPEN_DELEGATE_READ:
> -		nfserr =3D nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
> -		if (nfserr)
> -			return nfserr;
> -		p =3D xdr_reserve_space(xdr, 20);
> -		if (!p)
> -			return nfserr_resource;
> -		*p++ =3D cpu_to_be32(open->op_recall);
> -
> -		/*
> -		 * TODO: ACE's in delegations
> -		 */
> -		*p++ =3D cpu_to_be32(NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE);
> -		*p++ =3D cpu_to_be32(0);
> -		*p++ =3D cpu_to_be32(0);
> -		*p++ =3D cpu_to_be32(0);   /* XXX: is NULL principal ok? */
> -		break;
> +		/* read */
> +		return nfsd4_encode_open_read_delegation4(xdr, open);
>  	case NFS4_OPEN_DELEGATE_WRITE:
>  		nfserr =3D nfsd4_encode_stateid4(xdr, &open->op_delegate_stateid);
>  		if (nfserr)
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index aba07d5378fc..c142a9b5ab98 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -389,7 +389,7 @@ struct nfsd4_open {
>  	u32		op_deleg_want;      /* request */
>  	stateid_t	op_stateid;         /* response */
>  	__be32		op_xdr_error;       /* see nfsd4_open_omfg() */
> -	u32		op_recall;          /* recall */
> +	bool		op_recall;          /* response */

nit: this would probably pack better if you moved op_recall down beside
op_truncate.

>  	struct nfsd4_change_info  op_cinfo; /* response */
>  	u32		op_rflags;          /* response */
>  	bool		op_truncate;        /* used during processing */
>=20
>=20

--=20
Jeff Layton <jlayton@poochiereds.net>