> On Feb 1, 2019, at 9:30 PM, Tom Talpey <tom@xxxxxxxxxx> wrote:
>
> On 2/1/2019 2:57 PM, Chuck Lever wrote:
>> Byte-swapping causes a CPU pipeline bubble on some processors. When
>> a decoder is comparing an on-the-wire value for equality, byte-
>> swapping can be avoided by comparing it directly to a pre-byte-
>> swapped constant value.
>> The current set of pre-xdr'd constants is missing some common values
>> used in the RPC header. Fill those out.
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> include/linux/sunrpc/auth_gss.h | 5 ++-
>> include/linux/sunrpc/xdr.h | 66 ++++++++++++++++++++++++---------------
>> 2 files changed, 45 insertions(+), 26 deletions(-)
>> diff --git a/include/linux/sunrpc/auth_gss.h b/include/linux/sunrpc/auth_gss.h
>> index 30427b7..adc4be2 100644
>> --- a/include/linux/sunrpc/auth_gss.h
>> +++ b/include/linux/sunrpc/auth_gss.h
>> @@ -19,7 +19,10 @@
>> #include <linux/sunrpc/svc.h>
>> #include <linux/sunrpc/gss_api.h>
>> -#define RPC_GSS_VERSION 1
>> +enum {
>> + RPC_GSS_VERSION = 1,
>> + rpc_gss_version = cpu_to_be32(RPC_GSS_VERSION)
>> +};
>> #define MAXSEQ 0x80000000 /* maximum legal sequence number, from rfc 2203 */
>> diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
>> index 787939d..69161cb 100644
>> --- a/include/linux/sunrpc/xdr.h
>> +++ b/include/linux/sunrpc/xdr.h
>> @@ -17,6 +17,7 @@
>> #include <asm/byteorder.h>
>> #include <asm/unaligned.h>
>> #include <linux/scatterlist.h>
>> +#include <linux/sunrpc/msg_prot.h>
>> struct bio_vec;
>> struct rpc_rqst;
>> @@ -79,31 +80,46 @@ struct xdr_buf {
>> buf->buflen = len;
>> }
>> -/*
>> - * pre-xdr'ed macros.
>> - */
>> -
>> -#define xdr_zero cpu_to_be32(0)
>> -#define xdr_one cpu_to_be32(1)
>> -#define xdr_two cpu_to_be32(2)
>> -
>> -#define rpc_success cpu_to_be32(RPC_SUCCESS)
>> -#define rpc_prog_unavail cpu_to_be32(RPC_PROG_UNAVAIL)
>> -#define rpc_prog_mismatch cpu_to_be32(RPC_PROG_MISMATCH)
>> -#define rpc_proc_unavail cpu_to_be32(RPC_PROC_UNAVAIL)
>> -#define rpc_garbage_args cpu_to_be32(RPC_GARBAGE_ARGS)
>> -#define rpc_system_err cpu_to_be32(RPC_SYSTEM_ERR)
>> -#define rpc_drop_reply cpu_to_be32(RPC_DROP_REPLY)
>> -
>> -#define rpc_auth_ok cpu_to_be32(RPC_AUTH_OK)
>> -#define rpc_autherr_badcred cpu_to_be32(RPC_AUTH_BADCRED)
>> -#define rpc_autherr_rejectedcred cpu_to_be32(RPC_AUTH_REJECTEDCRED)
>> -#define rpc_autherr_badverf cpu_to_be32(RPC_AUTH_BADVERF)
>> -#define rpc_autherr_rejectedverf cpu_to_be32(RPC_AUTH_REJECTEDVERF)
>> -#define rpc_autherr_tooweak cpu_to_be32(RPC_AUTH_TOOWEAK)
>> -#define rpcsec_gsserr_credproblem cpu_to_be32(RPCSEC_GSS_CREDPROBLEM)
>> -#define rpcsec_gsserr_ctxproblem cpu_to_be32(RPCSEC_GSS_CTXPROBLEM)
>> -#define rpc_autherr_oldseqnum cpu_to_be32(101)
>> +enum xdr_be32_equivalents {
>> + xdr_zero = cpu_to_be32(0),
>> + xdr_one = cpu_to_be32(1),
>> + xdr_two = cpu_to_be32(2),
>
> It is clever to use an enum to pre-compute these values, but
Perhaps not clever; it is a current Linux kernel coding
practice to use an enum in favor of a C macro for constants.
> it becomes a concern that the type (and size) of an enum may
> not be the same as values they may be compared to.
Indeed, an enum is a variably-sized signed integer, IIUC.
> Commonly, code may compare them to int32, uint32, etc. What
> guarantees are there that such comparisons will yield the
> appropriate result, especially if a < or > test is performed?
I believe for the purposes of assignment and equality comparison
the compiler will promote these to the size and sign of the
variable. We would never perform a greater or less than test with
these values, obviously.
However, they probably should have an obvious and well-defined
type, and I should leave the already-defined macros as they
are.
> Tom.
>
>> +
>> + rpc_version = cpu_to_be32(RPC_VERSION),
>> +
>> + rpc_auth_null = cpu_to_be32(RPC_AUTH_NULL),
>> + rpc_auth_unix = cpu_to_be32(RPC_AUTH_UNIX),
>> + rpc_auth_short = cpu_to_be32(RPC_AUTH_SHORT),
>> + rpc_auth_des = cpu_to_be32(RPC_AUTH_DES),
>> + rpc_auth_krb = cpu_to_be32(RPC_AUTH_KRB),
>> + rpc_auth_gss = cpu_to_be32(RPC_AUTH_GSS),
>> +
>> + rpc_call = cpu_to_be32(RPC_CALL),
>> + rpc_reply = cpu_to_be32(RPC_REPLY),
>> +
>> + rpc_msg_accepted = cpu_to_be32(RPC_MSG_ACCEPTED),
>> + rpc_msg_denied = cpu_to_be32(RPC_MSG_DENIED),
>> +
>> + rpc_success = cpu_to_be32(RPC_SUCCESS),
>> + rpc_prog_unavail = cpu_to_be32(RPC_PROG_UNAVAIL),
>> + rpc_prog_mismatch = cpu_to_be32(RPC_PROG_MISMATCH),
>> + rpc_proc_unavail = cpu_to_be32(RPC_PROC_UNAVAIL),
>> + rpc_garbage_args = cpu_to_be32(RPC_GARBAGE_ARGS),
>> + rpc_system_err = cpu_to_be32(RPC_SYSTEM_ERR),
>> + rpc_drop_reply = cpu_to_be32(RPC_DROP_REPLY),
>> +
>> + rpc_mismatch = cpu_to_be32(RPC_MISMATCH),
>> + rpc_auth_error = cpu_to_be32(RPC_AUTH_ERROR),
>> +
>> + rpc_auth_ok = cpu_to_be32(RPC_AUTH_OK),
>> + rpc_autherr_badcred = cpu_to_be32(RPC_AUTH_BADCRED),
>> + rpc_autherr_rejectedcred = cpu_to_be32(RPC_AUTH_REJECTEDCRED),
>> + rpc_autherr_badverf = cpu_to_be32(RPC_AUTH_BADVERF),
>> + rpc_autherr_rejectedverf = cpu_to_be32(RPC_AUTH_REJECTEDVERF),
>> + rpc_autherr_tooweak = cpu_to_be32(RPC_AUTH_TOOWEAK),
>> + rpcsec_gsserr_credproblem = cpu_to_be32(RPCSEC_GSS_CREDPROBLEM),
>> + rpcsec_gsserr_ctxproblem = cpu_to_be32(RPCSEC_GSS_CTXPROBLEM),
>> +};
>> /*
>> * Miscellaneous XDR helper functions
--
Chuck Lever
