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

["Chuck Lever" <cel@kernel.org>]

On Wed, Jun 10, 2026, at 11:19 AM, Jeff Layton wrote:
> On Mon, 2026-06-08 at 12:52 -0400, Chuck Lever wrote:

>> > 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.
>> 
>
> The full argument encoder and decoder works just fine. When you say
> "supported" what do you mean, specifically?

"encode argument" is a client-side mechanism

"decode argument" is a server-side mechanism

"encode result" is a server-side mechanism

"decode result" is a client-side mechanism

The reason that matters is that the client-side and server-side
XDR implementation for the top level arguments and results (not
the individual data items) in the kernel have different calling
conventions -- eg, server side wants to see a struct svc_rqst *


> I'd really rather not go back to open-coding the encoders and decoders,
> particularly since CB_NOTIFY has one of the most complex argument
> structures in the protocol.

Go look at how I implemented this in the subsequent patch. You can
use the xdrgen notify4 encoder, that's where all the complexity is.

-- 
Chuck Lever