> On Aug 29, 2024, at 2:26 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Thu, 2024-08-29 at 11:13 -0400, Chuck Lever wrote: >> On Thu, Aug 29, 2024 at 09:26:46AM -0400, Jeff Layton wrote: >> >>> Create a new include/linux/sunrpc/xdrgen directory in which we can >>> keep autogenerated header files. >> >> I think the header files will have different content for the client >> and server. For example, the server-side header has function >> declarations for the procedure argument and result XDR functions, >> the client doesn't (because those functions are all static on the >> client side). >> >> Not sure we're ready for this level of sharing between client and >> server. >> > > Does that matter though? IMHO it does. Client and server are rather separate code bases, and when we have attempted to share things extensively between them in the past, it locks the other side into something that doesn't fit well. I'd rather be further along with client side code generation before making decisions about how to fit these pieces together. What I've toyed with so far suggests there are going to be a few substantive differences with the server side. > The constant and type definitions are the same between the client and > server. I think there is value in having a single source for that, like > we have today with nfs4.h. > > If we do decide that it's a problem, we can just split things up > further: The tool already does that now: xdrgen header --definitions gives you the protocol pieces, and xdrgen header --declarations gives you the function declarations. Specify both together and you get what you have in nfs4xdr_gen.h today. It might not be the most natural way to split these up, but <shrug>. (Note that this is going to get worse when Rust is introduced into the mix). > 1. one header file with constant, struct and type definitions > 2. one with server-side encode/decode prototypes that includes #1 > 3. one with client-side encode/decode prototypes that includes #1 Right, that's already the direction I'm going with xdrgen. I think we're on the same page with that. > include/linux/nfs4.h could still include #1 as well, but the client and > server could include the appropriate headers instead of or in addition > to it. That should work. >>> Move the new, generated nfs4xdr_gen.h file to nfs4_1.h in >>> that directory. >> >> I'd rather keep the current file name to indicate that it's >> generated code. >> > > I figured the "xdrgen" directory name would convey that. It does only if we move the header file to include/linux/sunrpc/xdrgen. I'm not sold on that idea yet though I guess it works for the protocol pieces that the Linux community doesn't directly control (ie --definitions). > This naming also makes it clearer that it's generated from > nfs4_1.x. Well the generated boilerplate could contain the spec file name -- it has the spec file's timestamp already as a kind of janky version number. Commit hash might be better. Note that the spec authors expect implementations to catenate all the disparate .x pieces (ie, all the minor versions and all the pNFS layout types) together into a single all-singing- all-dancing nfs4.x. Looking at it now, I'm not convinced we want to stay with the name nfs4_1.x. Also that's why I used the output file name nfs4xdr_gen.[ch]. > That said, I'm not too particular here. Can you lay out > how you think we ought to arrange things? I'm still musing about it. I'll mock something up in the 2/2 xdrgen patch for more feedback. Now that we've got most of the review of LOCALIO done, I'll have a bit more time to help you get xdrgen ready for server- side delstid. I will try not to hold you up. -- Chuck Lever
