Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1 00/25] Server-side RPC call header parsing overhaul
Thread-Topic: [PATCH v1 00/25] Server-side RPC call header parsing overhaul
Thread-Index: AQHZHsyO+pYu4CccG0ipBPGuVGU2ua6MyEaAgAAcTYA=
Date:   Tue, 3 Jan 2023 16:33:20 +0000
Message-ID: <14A17DBB-CD3D-41A4-B2C2-D46DEBB1243C@oracle.com>
References: <167267753484.112521.4826748148788735127.stgit@manet.1015granger.net>
 <159dd1253c7b49b6654cb4373477f69cc7387b1e.camel@kernel.org>
In-Reply-To: <159dd1253c7b49b6654cb4373477f69cc7387b1e.camel@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?wJVW79pgeegSzTPyK+SOO7ZW0kH1ITjAxoHJZUcG9BeNhL+A5XM1ydqE4qaR?=
 =?us-ascii?Q?kTv/MjdUbx/HYg9i+C/p8bb8KbiE9diCEUUkW28usiRmjAqD2B8iVOLX0ENT?=
 =?us-ascii?Q?4XZXQ6IHnr4Eqyben+4vfzPWbzAX/5u8a/wL/ITK92Uo/ubYkT69dno8ZWol?=
 =?us-ascii?Q?2gMjf/nWSbzEQdNQZYV8qxZe0/K8nBj3GiVAACgbli8uU+pSfidyw7bE47sP?=
 =?us-ascii?Q?m/Z5pxq8o6M7R9pqFFtDqmwJ0cYa1j3xFZmd843byrV1R1CdAxahfIffaMcs?=
 =?us-ascii?Q?JoBNICvLx3nygNq/X/Dxpnzdpd+SFb8x0lVAE7EL9OSDqPcNDc4x5+3zfJNv?=
 =?us-ascii?Q?+s+ZSaXeRbdRWW02oSgKDv1xAkhCrfP55PG6wlcthxSrMDVWBqwjYJtsX39b?=
 =?us-ascii?Q?d2Cl037lJ/jI6B5JZo6DJ9zJEIggul+wrnj/8qSI0tk9rEsrDNH4m0nanTNR?=
 =?us-ascii?Q?72WDpoPWHfYJ9jyd2YXTRAugSotr3h60VIVy2pssAhBnJzTht4HkHTKlhQvH?=
 =?us-ascii?Q?XZAcZp0dRykzqeTCFXza68bk3AVHWAaU+ARmw3dTw98gJwwZEtMiHgPPN9pQ?=
 =?us-ascii?Q?ik55h9z4Nd8b1alNKuTDqTK9yEAq6oFk0xhbB2PoDudzKrPCtTacI7bF37jr?=
 =?us-ascii?Q?DMKfE1XQjUWYR/VwEcx4W7hBKztBlaUTDMRINz/KqLX+C95Ey4dFrxWZWrzz?=
 =?us-ascii?Q?6TAIuqGypRCgcE0nX8uxaE55nXQFyFupu/NFr83d7fQTusS+9KHELvqqEFeb?=
 =?us-ascii?Q?TcWf1DALt3DejL8gEzGg0WeAofxxpBjYnEK2VILbQwJi4jhz42WYnT3Kbxfe?=
 =?us-ascii?Q?Qob54fStCP29m2gm9rWnY0dhAs41ZkBtZBYl8m01Kmuz0bdNkhNxCjle+yQK?=
 =?us-ascii?Q?UCvq8T8QgCFUmV0RLmKSkSJiHcK053bINDM8UZkWwxtQ+O2ItaZfznCHvX2u?=
 =?us-ascii?Q?bh9XNdCT8l+YOkYgMapHsVONdEkY0Q8NZVIBNKwN+AolUX2MmFnaobjSH3gR?=
 =?us-ascii?Q?VI+fQXpQfyWdj03W8Yd5+IIX5p3joxIC9xoKhSbqnnkx+xdNn2Gz5bQvwu+N?=
 =?us-ascii?Q?AJMZu2IAOG5bhsQ56HNGIg56fTfhBbBVoBhg85Zv7+K9rKL550YYgHGlHYH/?=
 =?us-ascii?Q?Xusx4vsBkZc6Vzhk4VZE7oyJlihG16bjulzrfnlLG4jvzbUdahvJVWduH4N3?=
 =?us-ascii?Q?5P5wMZcZY1xLnmgNqb2Q0zfeiosrWi5Fv0Yrri+Kt3vPERY9WZgbKEenvzqU?=
 =?us-ascii?Q?rnC9soMnkRsSq8IyBK3BhDIMLDFR2wiYI2fMxIJkz8m5PogcCUEZzIvxM3uY?=
 =?us-ascii?Q?kyY1weBNGoJ6WNEygvJDGzv7oNLNk/zR2DMARmCduYMi9fZM+yw4bMJ02d7h?=
 =?us-ascii?Q?xmJNqbIr2OhdmK/30qRr9yyQ/lDMdOwxIIkLYulSHTJPFwaZBTjQ3o2L2XB5?=
 =?us-ascii?Q?twVjSmohav2YKlNPZqEnNmnsBgOxjO1lD0CGxXS+MA8uVNAqbI0oPCe6G2ec?=
 =?us-ascii?Q?miYBs75ByfzXZrgw+fkZ3+S7J2DdaTHRa+rOJ/jJwZTBMT3jB9QNaicVb3DJ?=
 =?us-ascii?Q?00RCkx3l/DpVXgH8ysFAKezddxBcMHmPgZT/edxTGkJN9hMjevD6zZIdb1Nr?=
 =?us-ascii?Q?FQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <D683036747C97448A812257B0D0A6D25@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: N1cb/ZP9BrvRdHkPgMxinM6ZiJbp2vEVD2SuWN3VR5nrDLvsjZThEn6DTr7Zq9fXPeYmRrC7nhcGz+ZTjG4BQ/cqx7Iudvg6+etxEAImnkcdHvkI37jheNrQZLnygDBIBi9NEW7lPJIdSjr1N9koc0kSWHFyQUM9bcYSKndjg4JQ3xV65BQbeWU/t6YBjuOYz+L4xOFnQ8Igu6kx3NsiCVbrIWrRw/CMz1Fu17ja0wnaNV71jTg3h2eIJ5LD/Q90ND7kCKMU6x9/1MM/2VcFvyftwgwJgTwPqjZnIrbdfJVkq0ajUIypaTNA/XnD0ZEQcNg8/oWfcC26f6jW8bG8+XIuYCLIpkCqnHpMOy3dutbPThVPMtIwltvlkX3KRfnd2A3axGr+jQvuV4QaqQQ/4wOO70TS3Csipflt5s+mnDMctQPs6/mW8I2d/wfR2iSOEqNGJoLgvDRmyhRjQCL4NeX+fITXTVKDkHAz64gAeSa4oEikl4zqMx1HSzb9lnjKZ0e4p2WYm7/Xe5lS8aW7cmuvcnl68X9RQwZg4Ybl+BKLb9caE3O05iaHss6F6Rz/AF33DM5m34EEarBX0iAMFiyrJPqUlOQIYKVtM89+Ou/PwghsnTo28gfiwcuBroKCnhK/4BLCLxJW0VtsPbZTkMfP/A2dE4yiQKQUrW3fvb0vWN5gBxey0lPduBMg32dZgrsIMzivvPPKS0Wz0I6OPWHHZqXrorGCZVQNJL3MkY0iuyzi4LLWCBQN2JkFlP6P1n9/LIS4GSiYL5nX1dZ9xHh7i5v1j2uQ8CbNLw3lFmU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 22277524-8da3-4cd0-6a58-08daeda839c9
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jan 2023 16:33:20.5823
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: CkjqZsr9tM75LnbZzoq47gqLCGzJzXtFMNMxwcuMnPAxQSZ9ybVnYRUAUkfDMKXkqZMZRIObOL35CEMP1ejTaQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6968
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2023-01-03_05,2023-01-03_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0
 suspectscore=0 malwarescore=0 mlxscore=0 spamscore=0 adultscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2301030141
X-Proofpoint-ORIG-GUID: huYUcOAVwHGSk-PBIdELp4b_LkETURzw
X-Proofpoint-GUID: huYUcOAVwHGSk-PBIdELp4b_LkETURzw
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jan 3, 2023, at 9:52 AM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> On Mon, 2023-01-02 at 12:05 -0500, Chuck Lever wrote:
>> Happy new year, campers.
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> Yes, there are a lot of patches, but they are each small, easily
>> chewed mechanical changes.
>>=20
>> ---
>>=20
>> 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_i=
nit()
>>      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_strea=
m
>>      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()
>>=20
>>=20
>> 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(-)
>>=20
>> --
>> Chuck Lever
>>=20
>=20
> I went through the whole set and it's all a mostly logical set of
> methodical changes. You can add:
>=20
> 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