Re: [PATCH v5 07/21] nfsd: add callback encoding and decoding linkages for CB_NOTIFY

On Fri, May 22, 2026, at 3:42 PM, Jeff Layton wrote:
> Add routines for encoding and decoding CB_NOTIFY messages. These call
> into the code generated by xdrgen to do the actual encoding and
> decoding.

The commit message needs to explain that the encoder is not yet functional.
Something like: "The encoder is a stub; payload encoding (stateid, fh, and
cna_changes) is deferred."


> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 25bbf5b8814d..ea3e7deb06fa 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -865,6 +865,51 @@ static void encode_stateowner(struct xdr_stream 
> *xdr, struct nfs4_stateowner *so
>  	xdr_encode_opaque(p, so->so_owner.data, so->so_owner.len);
>  }
> 
> +static void nfs4_xdr_enc_cb_notify(struct rpc_rqst *req,
> +				   struct xdr_stream *xdr,
> +				   const void *data)
> +{
> +	const struct nfsd4_callback *cb = data;
> +	struct nfs4_cb_compound_hdr hdr = {
> +		.ident = 0,
> +		.minorversion = cb->cb_clp->cl_minorversion,
> +	};
> +	struct CB_NOTIFY4args args = { };
> +
> +	WARN_ON_ONCE(hdr.minorversion == 0);
> +
> +	encode_cb_compound4args(xdr, &hdr);
> +	encode_cb_sequence4args(xdr, cb, &hdr);
> +
> +	/*
> +	 * FIXME: get stateid and fh from delegation. Inline the cna_changes
> +	 * buffer, and zero it.
> +	 */
> +	WARN_ON_ONCE(!xdrgen_encode_CB_NOTIFY4args(xdr, &args));
> +
> +	hdr.nops++;
> +	encode_cb_nops(&hdr);
> +}

There are a number of problems with this, but since there are no
callers yet, we can let some of those issues stand.

What is problematic in the longer-term is that this is a client-side
encoder (since this is the server's NFSv4 callback client).

xdrgen_encode_CB_NOTIFY4args() is an argument encoder, which is
client-side functionality, but it resides in fs/nfsd/nfs4xdr_gen.c,
which is server-side. Let's not mix these purposes.

I replaced the comment and WARN_ON with this:

+       xdr_stream_encode_u32(xdr, OP_CB_NOTIFY);
+
+       /* FIXME: encode stateid, fh, and cna_changes from delegation */

You can use xdrgen functions for individual data items, but for
full argument and response structures, only server-side is supported
at the moment. In the later patch that completes this code, I'll cover
the other fields, which can be a mix of open code and xdrgen.


> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index e1c40f8b5d01..790282781243 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -190,6 +190,13 @@ struct nfs4_cb_fattr {
>  	u64 ncf_cur_fsize;
>  };
> 
> +/*
> + * FIXME: the current backchannel encoder can't handle a send buffer longer
> + *        than a single page (see bc_alloc/bc_free).
> + */

Nit: The allocator function name is bc_malloc


-- 
Chuck Lever