| Message ID | 167267753484.112521.4826748148788735127.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | Server-side RPC call header parsing overhaul | expand |
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>
> 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
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