Re: [PATCH v1 00/25] Server-side RPC call header parsing overhaul

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jan 3, 2023, at 9:52 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Mon, 2023-01-02 at 12:05 -0500, Chuck Lever wrote:
>> Happy new year, campers.
>> 
>> The following series has been percolating for quite a while, thanks
>> to the extended 6.1-rc cycle. I'd like to get this work reviewed
>> for possible inclusion in v6.3, so I'm starting early.
>> 
>> The purpose of this series is to replace the svc_get* macros in the
>> Linux kernel server's RPC call header parsing code with xdr_stream
>> helpers. I've measured no change in CPU utilization after the
>> overhaul; svc_recv() and friends remain the highest CPU consumers by
>> an order of magnitude.
>> 
>> Memory safety: Buffer bounds checking after decoding each XDR item
>> is more memory-safe than the current decoding mechanism. Subsequent
>> memory safety improvements to the xdr_stream helpers will benefit
>> all who use them.
>> 
>> Audit friendliness: The new code has lots of comments and other
>> clean-up to help align it with the relevant RPC specifications. The
>> use of common helpers also makes the decoders easier to audit.
>> 
>> I've split the full series in half to make it easier to review. The
>> patches posted here handle RPC call header decoding. Remaining
>> patches, to be posted later, deal with RPC reply header encoding.
>> 
>> Yes, there are a lot of patches, but they are each small, easily
>> chewed mechanical changes.
>> 
>> ---
>> 
>> Chuck Lever (25):
>>      SUNRPC: Push svcxdr_init_decode() into svc_process_common()
>>      SUNRPC: Move svcxdr_init_decode() into ->accept methods
>>      SUNRPC: Add an XDR decoding helper for struct opaque_auth
>>      SUNRPC: Convert svcauth_null_accept() to use xdr_stream
>>      SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
>>      SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
>>      SUNRPC: Move the server-side GSS upcall to a noinline function
>>      SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
>>      SUNRPC: Remove gss_read_common_verf()
>>      SUNRPC: Remove gss_read_verf()
>>      SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
>>      SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
>>      SUNRPC: Rename automatic variables in unwrap_integ_data()
>>      SUNRPC: Convert unwrap_integ_data() to use xdr_stream
>>      SUNRPC: Rename automatic variables in unwrap_priv_data()
>>      SUNRPC: Convert unwrap_priv_data() to use xdr_stream
>>      SUNRPC: Convert gss_verify_header() to use xdr_stream
>>      SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
>>      SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
>>      SUNRPC: Hoist init_decode out of svc_authenticate()
>>      SUNRPC: Re-order construction of the first reply fields
>>      SUNRPC: Eliminate unneeded variable
>>      SUNRPC: Decode most of RPC header with xdr_stream
>>      SUNRPC: Remove svc_process_common's argv parameter
>>      SUNRPC: Hoist svcxdr_init_decode() into svc_process()
>> 
>> 
>> fs/lockd/svc.c                    |   1 -
>> fs/nfs/callback_xdr.c             |   1 -
>> fs/nfsd/nfssvc.c                  |   1 -
>> include/linux/sunrpc/msg_prot.h   |   5 +
>> include/linux/sunrpc/xdr.h        |   5 +-
>> net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
>> net/sunrpc/svc.c                  |  69 ++--
>> net/sunrpc/svc_xprt.c             |   1 -
>> net/sunrpc/svcauth.c              |  13 +-
>> net/sunrpc/svcauth_unix.c         | 132 +++++---
>> net/sunrpc/xdr.c                  |  50 ++-
>> 11 files changed, 468 insertions(+), 322 deletions(-)
>> 
>> --
>> Chuck Lever
>> 
> 
> I went through the whole set and it's all a mostly logical set of
> methodical changes. You can add:
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Thanks Jeff! I've applied these and pushed them to nfsd's for-next,
but comments are still open, and testing results are welcome.


--
Chuck Lever