Received-SPF: pass (google.com: domain of linux-nfs+bounces-2396-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 19 Mar 2024 11:02:52 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Neil Brown <neilb@suse.de>, Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>, linux-nfs@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nfsd: trivial GET_DIR_DELEGATION support
Message-ID: <ZfmpHFMYFmnIiBD1@manet.1015granger.net>
References: <20240318-nfs-gdd-trivial-v1-1-158924b9e00d@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240318-nfs-gdd-trivial-v1-1-158924b9e00d@kernel.org>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?BzE+nqL4Nc2Ysfiw8rOxvkYI+/pWmVqPOAtUxZiW8whzRxKhX2lsfelqE2LK?=
 =?us-ascii?Q?Wp4p+ilhftNZOe3QAA+xKtS4P3lFCgPrWMbjdFeBvQx4CZLAJ+7t7A8gDVDi?=
 =?us-ascii?Q?LI5V/heOTsrdswANLUIxlY0asz+pfU+bWEhuWgopk7H2ca6CP9I0/pLPK264?=
 =?us-ascii?Q?d9q64yLgZivtLduD1+Ka3GiRpqoN/3411mnakJ3XE2VykYo1pIELhpo/q1uk?=
 =?us-ascii?Q?ILl9LznG9T+Zydqli+sTTFcwCVZbBJM5JwOfcqUfsyVN+K4Ghof9rpADo+nH?=
 =?us-ascii?Q?ghAli+O0VGBhlUuzt6TDrm/PLKAksVJAa/vWPHYfGBCUTXO2QWc4OW/k+29l?=
 =?us-ascii?Q?AS/Fh03lJ9aVySGSfEQbmlTA8gNpVIu0kVRdpzm8RShu9FkOnFq40cVW0Jri?=
 =?us-ascii?Q?eV4i2VL3E682D6yD35Ku9LwagP5etq6zMwgomQmpUmOUvcfvg3Nwgl3/kRPZ?=
 =?us-ascii?Q?7ZjtWYwLzY1tTI84ozLnywofHpjfeu7a+JhqBoOFnugjiaVqqmiv59t+8YpJ?=
 =?us-ascii?Q?XRX8mrStnKjxY4LvXvGPXgkJ5jsKlYXBNqLX8q9E0B9IPN7PTzet+hClhc11?=
 =?us-ascii?Q?XTxvlLSY3Wc3ZI2VerMoJtkE94FTdT2bwGOQaXEHJYhQzohETtwHSJwFDvrr?=
 =?us-ascii?Q?NtdFKtxk3F+kegsoTkycSToY3ZAjpGu+4W4FP2OGqGcTpoD5D0DMNvgZxe8R?=
 =?us-ascii?Q?mjJTrW0l8e40t9OXnXLvYXTg1Vr5KNau4iY4HbE+cakupn4VrQ3vnXiDrOzy?=
 =?us-ascii?Q?/QvpqEDjXWfptlDw4G1EQf2hC+P2NSXV36IyPCv88vRmXrfO10qj3Ra3WRbj?=
 =?us-ascii?Q?N1xcZZ2E+sQGivohf3eXh9TWsjjeqASuzQDlyj7R5qrsdSi2eeX3jkUfsia3?=
 =?us-ascii?Q?vzkQGeM1ROSjS42neKKwPUhLC4PA/ZEVynRhWtWaHtGLDvfz+o5mzg8z8TTi?=
 =?us-ascii?Q?c5UgcGfRwE8PdbZEw+OghAj/y9eBZEtjrmUv3bhdWCUhSf19VTLgKbOQwhSS?=
 =?us-ascii?Q?D7svr3I+XSk6ls8gyygGxgLjunzuihEApS25/+t9+4bQSKRSd8YM+1u2Ff6r?=
 =?us-ascii?Q?emj+pP+yUcDCnelxyo7YKe7Ol/znPcU/+SnnvcUWR6Bh8IkyA/sap9F7/mcn?=
 =?us-ascii?Q?V+BBYbssVeIgEFax8QazBI20bmYuZOphgXQdSHeWYkwdZBQTo/Rb6jyQrYRB?=
 =?us-ascii?Q?JtSI9VAumhwpEL3XalSchK3Sm/ulFemh2WvRig48cgepoJ32DzqIV//O2WOQ?=
 =?us-ascii?Q?qv7cgQLPUT73d9+HyWY5DmC2D8MxIHGXbeSRc5jwEGOwDbE8Wia7GhwMsrLV?=
 =?us-ascii?Q?mZaPpa30prtNsKvI5Uai1/mEBJAKEUokrLzdprwLkkUgOH97LDnchZpLV67+?=
 =?us-ascii?Q?WO+28nlkS8QVahlr7F7h6OFovHnvEdd7zjFuMPG70sAyHtXfqt2MR0WhaX1L?=
 =?us-ascii?Q?AKI6AMvJjxt2ZdPjcI7gXQS0hZirRWTHds8rFWyV3kB/0tYXTu8A4OBWY2cE?=
 =?us-ascii?Q?Fkjblg/6LAjecHxvtDqN+Hcc13Kk5vjKKkyiVhMbScDFYRN6pCMP++9WXdfh?=
 =?us-ascii?Q?dUMCRctMzN62Opngc+MDUsdG/s7phMzzfyWGAtyqCA8t3QPuTDf1FXRpF35Y?=
 =?us-ascii?Q?hQ=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	9JtpsRuYrcoH+tBaOTsPFNxvPM4sBzvYdQ5eg+vpdtmnLRQ3dE4k8Ebzt9XhzS/qWhtxiKX0bmbchKkPwrGN/kDRK5VQW3+qaavQb7B9KU5hiKQvaJnJwIIanuMM/Gmp4cF3/++fKW3EvSY/TZ5uTU7WtiDIE+xOsz61+zVhjcOaDwy8QF/+ZwaSO36fLGGtk/a3LLbfCxrDVjYmmrQTxz6xW9H2BBD9Ng8HPKsOuOBrgxS+2wWYNuHnEPak7m5bmlb2I36ghCT4p+ZglCok9QSJ06q+KwmgJ+kGEv0xO0nVr4QE2MHsK934vLk3wLv72k4v/R5hTh6Npju91G8lZE+c3onS0sAqJPhvXLf+Q9KvVoFUuVvunZ9L9SI3/4FY4qGhXq3WKaMuF9kyLrHRTKsV/WmCuKgCS9/7RCOJWWeAQJLjTNZ7rH1O07xiJUjFDfuj762uTlXat6PCaukE2XWoizEg7ylhTXQIQmtTlOhKMbS9NMg3eUcSAxZcQxkX74zo4WM9DeEMpzbw+ua5YV27DbfJLg44ejKcSKw3omArBpmCkxKa6XCfuDJxvDJNEszkd9N3EjcnbB5Xb1Mn4h7/e8SlCtfP6j7XACBRKiM=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a3350a4-7f6c-491f-0af8-08dc4825a86a
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Mar 2024 15:02:55.7790
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9nnvHOi16hpnevuui90BYqHabe/5XiGq89eZ4ypT8opNZPLgzC7W+z5t7gIZ0RG6yg9hEo//KFiEsAgFhsYVsg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYXPR10MB7897
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-03-19_04,2024-03-18_03,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0
 malwarescore=0 spamscore=0 mlxscore=0 phishscore=0 mlxlogscore=614
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2403140000 definitions=main-2403190114
X-Proofpoint-GUID: BDKexg7P745vIxZHhw7f_H3jpJYM9Xon
X-Proofpoint-ORIG-GUID: BDKexg7P745vIxZHhw7f_H3jpJYM9Xon

On Mon, Mar 18, 2024 at 12:48:04PM -0400, Jeff Layton wrote:
> This adds basic infrastructure for handing GET_DIR_DELEGATION calls from
> clients, including the decoders and encoders. For now, the server side
> always just returns that the  delegation is GDDR_UNAVAIL (and that we
> won't call back).
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Please consider this for v6.10. Eventually clients may start sending
> this operation, and it's better if we can return GDD4_UNAVAIL instead of
> having to abort the whole compound.

Applied to my private "for 6.10" tree, which will become nfsd-next
once the v6.9 merge window closes.


> ---
>  fs/nfsd/nfs4proc.c   | 30 +++++++++++++++++
>  fs/nfsd/nfs4xdr.c    | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  fs/nfsd/xdr4.h       | 19 +++++++++++
>  include/linux/nfs4.h |  6 ++++
>  4 files changed, 144 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 2927b1263f08..46b3d99c2786 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2154,6 +2154,18 @@ nfsd4_verify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	return status == nfserr_same ? nfs_ok : status;
>  }
>  
> +static __be32
> +nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
> +			 struct nfsd4_compound_state *cstate,
> +			 union nfsd4_op_u *u)
> +{
> +	struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation;
> +
> +	/* FIXME: implement directory delegations */
> +	gdd->gddrnf_status = GDD4_UNAVAIL;
> +	return nfs_ok;
> +}
> +
>  #ifdef CONFIG_NFSD_PNFS
>  static const struct nfsd4_layout_ops *
>  nfsd4_layout_verify(struct svc_export *exp, unsigned int layout_type)
> @@ -3082,6 +3094,18 @@ static u32 nfsd4_copy_notify_rsize(const struct svc_rqst *rqstp,
>  		* sizeof(__be32);
>  }
>  
> +static u32 nfsd4_get_dir_delegation_rsize(const struct svc_rqst *rqstp,
> +					  const struct nfsd4_op *op)
> +{
> +	return (op_encode_hdr_size +
> +		1 /* gddr_status */ +
> +		op_encode_verifier_maxsz +
> +		op_encode_stateid_maxsz +
> +		2 /* gddr_notification */ +
> +		2 /* gddr_child_attributes */ +
> +		2 /* gddr_dir_attributes */);
> +}
> +
>  #ifdef CONFIG_NFSD_PNFS
>  static u32 nfsd4_getdeviceinfo_rsize(const struct svc_rqst *rqstp,
>  				     const struct nfsd4_op *op)
> @@ -3470,6 +3494,12 @@ static const struct nfsd4_operation nfsd4_ops[] = {
>  		.op_get_currentstateid = nfsd4_get_freestateid,
>  		.op_rsize_bop = nfsd4_only_status_rsize,
>  	},
> +	[OP_GET_DIR_DELEGATION] = {
> +		.op_func = nfsd4_get_dir_delegation,
> +		.op_flags = OP_MODIFIES_SOMETHING,
> +		.op_name = "OP_GET_DIR_DELEGATION",
> +		.op_rsize_bop = nfsd4_get_dir_delegation_rsize,
> +	},
>  #ifdef CONFIG_NFSD_PNFS
>  	[OP_GETDEVICEINFO] = {
>  		.op_func = nfsd4_getdeviceinfo,
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index fac938f563ad..369b85e42440 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1732,6 +1732,40 @@ nfsd4_decode_free_stateid(struct nfsd4_compoundargs *argp,
>  	return nfsd4_decode_stateid4(argp, &free_stateid->fr_stateid);
>  }
>  
> +static __be32
> +nfsd4_decode_get_dir_delegation(struct nfsd4_compoundargs *argp,
> +		union nfsd4_op_u *u)
> +{
> +	struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation;
> +	__be32 status;
> +
> +	memset(gdd, 0, sizeof(*gdd));
> +
> +	if (xdr_stream_decode_bool(argp->xdr, &gdd->gdda_signal_deleg_avail) < 0)
> +		return nfserr_bad_xdr;
> +
> +	status = nfsd4_decode_bitmap4(argp, gdd->gdda_notification_types,
> +				      ARRAY_SIZE(gdd->gdda_notification_types));
> +	if (status)
> +		return status;
> +
> +	status = nfsd4_decode_nfstime4(argp, &gdd->gdda_child_attr_delay);
> +	if (status)
> +		return status;
> +
> +	status = nfsd4_decode_nfstime4(argp, &gdd->gdda_dir_attr_delay);
> +	if (status)
> +		return status;
> +
> +	status = nfsd4_decode_bitmap4(argp, gdd->gdda_child_attributes,
> +					ARRAY_SIZE(gdd->gdda_child_attributes));
> +	if (status)
> +		return status;
> +
> +	return nfsd4_decode_bitmap4(argp, gdd->gdda_dir_attributes,
> +					ARRAY_SIZE(gdd->gdda_dir_attributes));
> +}
> +
>  #ifdef CONFIG_NFSD_PNFS
>  static __be32
>  nfsd4_decode_getdeviceinfo(struct nfsd4_compoundargs *argp,
> @@ -2370,7 +2404,7 @@ static const nfsd4_dec nfsd4_dec_ops[] = {
>  	[OP_CREATE_SESSION]	= nfsd4_decode_create_session,
>  	[OP_DESTROY_SESSION]	= nfsd4_decode_destroy_session,
>  	[OP_FREE_STATEID]	= nfsd4_decode_free_stateid,
> -	[OP_GET_DIR_DELEGATION]	= nfsd4_decode_notsupp,
> +	[OP_GET_DIR_DELEGATION]	= nfsd4_decode_get_dir_delegation,
>  #ifdef CONFIG_NFSD_PNFS
>  	[OP_GETDEVICEINFO]	= nfsd4_decode_getdeviceinfo,
>  	[OP_GETDEVICELIST]	= nfsd4_decode_notsupp,
> @@ -5002,6 +5036,59 @@ nfsd4_encode_device_addr4(struct xdr_stream *xdr,
>  	return nfserr_toosmall;
>  }
>  
> +static __be32
> +nfsd4_encode_get_dir_delegation(struct nfsd4_compoundres *resp, __be32 nfserr,
> +				union nfsd4_op_u *u)
> +{
> +	struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation;
> +	struct xdr_stream *xdr = resp->xdr;
> +	__be32 status = nfserr_resource;
> +
> +	switch(gdd->gddrnf_status) {
> +	case GDD4_OK:
> +		if (xdr_stream_encode_u32(xdr, GDD4_OK) != XDR_UNIT)
> +			break;
> +
> +		status = nfsd4_encode_verifier4(xdr, &gdd->gddr_cookieverf);
> +		if (status)
> +			break;
> +
> +		status = nfsd4_encode_stateid4(xdr, &gdd->gddr_stateid);
> +		if (status)
> +			break;
> +
> +		status = nfsd4_encode_bitmap4(xdr, gdd->gddr_notification[0], 0, 0);
> +		if (status)
> +			break;
> +
> +		status = nfsd4_encode_bitmap4(xdr, gdd->gddr_child_attributes[0],
> +						   gdd->gddr_child_attributes[1],
> +						   gdd->gddr_child_attributes[2]);
> +		if (status)
> +			break;
> +
> +		status = nfsd4_encode_bitmap4(xdr, gdd->gddr_dir_attributes[0],
> +						   gdd->gddr_dir_attributes[1],
> +						   gdd->gddr_dir_attributes[2]);
> +		break;
> +	default:
> +		/*
> +		 * If we don't recognize the gddrnf_status value, just treat it
> +		 * like unavail + no notification, but print a warning too.
> +		 */
> +		pr_warn("nfsd: bad gddrnf_status (%u)\n", gdd->gddrnf_status);
> +		gdd->gddrnf_will_signal_deleg_avail = 0;
> +		fallthrough;
> +	case GDD4_UNAVAIL:
> +		if (xdr_stream_encode_u32(xdr, GDD4_UNAVAIL) != XDR_UNIT)
> +			break;
> +
> +		status = nfsd4_encode_bool(xdr, gdd->gddrnf_will_signal_deleg_avail);
> +		break;
> +	}
> +	return status;
> +}
> +
>  static __be32
>  nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
>  		union nfsd4_op_u *u)
> @@ -5580,7 +5667,7 @@ static const nfsd4_enc nfsd4_enc_ops[] = {
>  	[OP_CREATE_SESSION]	= nfsd4_encode_create_session,
>  	[OP_DESTROY_SESSION]	= nfsd4_encode_noop,
>  	[OP_FREE_STATEID]	= nfsd4_encode_noop,
> -	[OP_GET_DIR_DELEGATION]	= nfsd4_encode_noop,
> +	[OP_GET_DIR_DELEGATION]	= nfsd4_encode_get_dir_delegation,
>  #ifdef CONFIG_NFSD_PNFS
>  	[OP_GETDEVICEINFO]	= nfsd4_encode_getdeviceinfo,
>  	[OP_GETDEVICELIST]	= nfsd4_encode_noop,
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 415516c1b27e..446e72b0385e 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -518,6 +518,24 @@ struct nfsd4_free_stateid {
>  	stateid_t	fr_stateid;         /* request */
>  };
>  
> +struct nfsd4_get_dir_delegation {
> +	/* request */
> +	u32			gdda_signal_deleg_avail;
> +	u32			gdda_notification_types[1];
> +	struct timespec64	gdda_child_attr_delay;
> +	struct timespec64	gdda_dir_attr_delay;
> +	u32			gdda_child_attributes[3];
> +	u32			gdda_dir_attributes[3];
> +	/* response */
> +	u32			gddrnf_status;
> +	nfs4_verifier		gddr_cookieverf;
> +	stateid_t		gddr_stateid;
> +	u32			gddr_notification[1];
> +	u32			gddr_child_attributes[3];
> +	u32			gddr_dir_attributes[3];
> +	bool			gddrnf_will_signal_deleg_avail;
> +};
> +
>  /* also used for NVERIFY */
>  struct nfsd4_verify {
>  	u32		ve_bmval[3];        /* request */
> @@ -797,6 +815,7 @@ struct nfsd4_op {
>  		struct nfsd4_reclaim_complete	reclaim_complete;
>  		struct nfsd4_test_stateid	test_stateid;
>  		struct nfsd4_free_stateid	free_stateid;
> +		struct nfsd4_get_dir_delegation	get_dir_delegation;
>  		struct nfsd4_getdeviceinfo	getdeviceinfo;
>  		struct nfsd4_layoutget		layoutget;
>  		struct nfsd4_layoutcommit	layoutcommit;
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index ef8d2d618d5b..0d896ce296ce 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -701,6 +701,12 @@ enum state_protect_how4 {
>  	SP4_SSV		= 2
>  };
>  
> +/* GET_DIR_DELEGATION non-fatal status codes */
> +enum gddrnf4_status {
> +	GDD4_OK		= 0,
> +	GDD4_UNAVAIL	= 1
> +};
> +
>  enum pnfs_layouttype {
>  	LAYOUT_NFSV4_1_FILES  = 1,
>  	LAYOUT_OSD2_OBJECTS = 2,
> 
> ---
> base-commit: c442a42363b2ce5c3eb2b0ff1e052ee956f0a29f
> change-id: 20240318-nfs-gdd-trivial-19b6ca653841
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 

-- 
Chuck Lever