On Mon, 2022-06-06 at 10:51 -0400, Chuck Lever wrote:
> Pass the upper layer's rpc_create_args to the rpc_clnt_new()
> tracepoint so additional parts of the upper layer's request can be
> recorded.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> include/trace/events/sunrpc.h | 53 +++++++++++++++++++++++++++++++++--------
> net/sunrpc/clnt.c | 2 +-
> 2 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
> index a66aa1f59ed8..986e135e314f 100644
> --- a/include/trace/events/sunrpc.h
> +++ b/include/trace/events/sunrpc.h
> @@ -139,36 +139,69 @@ DEFINE_RPC_CLNT_EVENT(release);
> DEFINE_RPC_CLNT_EVENT(replace_xprt);
> DEFINE_RPC_CLNT_EVENT(replace_xprt_err);
>
> +TRACE_DEFINE_ENUM(RPC_XPRTSEC_NONE);
> +TRACE_DEFINE_ENUM(RPC_XPRTSEC_TLS_X509);
> +TRACE_DEFINE_ENUM(RPC_XPRTSEC_TLS_PSK);
> +
> +#define rpc_show_xprtsec_policy(policy) \
> + __print_symbolic(policy, \
> + { RPC_XPRTSEC_NONE, "none" }, \
> + { RPC_XPRTSEC_TLS_X509, "tls-x509" }, \
> + { RPC_XPRTSEC_TLS_PSK, "tls-psk" })
> +
> +#define rpc_show_create_flags(flags) \
> + __print_flags(flags, "|", \
> + { RPC_CLNT_CREATE_HARDRTRY, "HARDRTRY" }, \
> + { RPC_CLNT_CREATE_AUTOBIND, "AUTOBIND" }, \
> + { RPC_CLNT_CREATE_NONPRIVPORT, "NONPRIVPORT" }, \
> + { RPC_CLNT_CREATE_NOPING, "NOPING" }, \
> + { RPC_CLNT_CREATE_DISCRTRY, "DISCRTRY" }, \
> + { RPC_CLNT_CREATE_QUIET, "QUIET" }, \
> + { RPC_CLNT_CREATE_INFINITE_SLOTS, \
> + "INFINITE_SLOTS" }, \
> + { RPC_CLNT_CREATE_NO_IDLE_TIMEOUT, \
> + "NO_IDLE_TIMEOUT" }, \
> + { RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT, \
> + "NO_RETRANS_TIMEOUT" }, \
> + { RPC_CLNT_CREATE_SOFTERR, "SOFTERR" }, \
> + { RPC_CLNT_CREATE_REUSEPORT, "REUSEPORT" })
> +
> TRACE_EVENT(rpc_clnt_new,
> TP_PROTO(
> const struct rpc_clnt *clnt,
> const struct rpc_xprt *xprt,
> - const char *program,
> - const char *server
> + const struct rpc_create_args *args
> ),
>
> - TP_ARGS(clnt, xprt, program, server),
> + TP_ARGS(clnt, xprt, args),
>
> TP_STRUCT__entry(
> __field(unsigned int, client_id)
> + __field(unsigned long, xprtsec)
> + __field(unsigned long, flags)
> + __string(program, clnt->cl_program->name)
> + __string(server, xprt->servername)
> __string(addr, xprt->address_strings[RPC_DISPLAY_ADDR])
> __string(port, xprt->address_strings[RPC_DISPLAY_PORT])
> - __string(program, program)
> - __string(server, server)
> ),
>
> TP_fast_assign(
> __entry->client_id = clnt->cl_clid;
> + __entry->xprtsec = args->xprtsec;
> + __entry->flags = args->flags;
> + __assign_str(program, clnt->cl_program->name);
> + __assign_str(server, xprt->servername);
> __assign_str(addr, xprt->address_strings[RPC_DISPLAY_ADDR]);
> __assign_str(port, xprt->address_strings[RPC_DISPLAY_PORT]);
> - __assign_str(program, program);
> - __assign_str(server, server);
> ),
>
> - TP_printk("client=" SUNRPC_TRACE_CLID_SPECIFIER
> - " peer=[%s]:%s program=%s server=%s",
> + TP_printk("client=" SUNRPC_TRACE_CLID_SPECIFIER " peer=[%s]:%s"
> + " program=%s server=%s xprtsec=%s flags=%s",
> __entry->client_id, __get_str(addr), __get_str(port),
> - __get_str(program), __get_str(server))
> + __get_str(program), __get_str(server),
> + rpc_show_xprtsec_policy(__entry->xprtsec),
> + rpc_show_create_flags(__entry->flags)
> + )
> );
>
> TRACE_EVENT(rpc_clnt_new_err,
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 6dcc88d45f5d..0ca86c92968f 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -435,7 +435,7 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args,
> if (parent)
> refcount_inc(&parent->cl_count);
>
> - trace_rpc_clnt_new(clnt, xprt, program->name, args->servername);
> + trace_rpc_clnt_new(clnt, xprt, args);
> return clnt;
>
> out_no_path:
>
>
All of these tracing patches seem like a reasonable thing to add on
their own, IMO. It might be good to move them and some of the bugfixes
to a separate series and get those merged ahead of the parts that add
TLS support (which are more controversial).
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
