| Message ID | 4546b63e67b2989789d146498b13cc09e1fdc543.1612403190.git.marcelo.leitner@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7e3ce05e7f650371061d0b9eec1e1cf74ed6fca0 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] netlink: add tracepoint at NL_SET_ERR_MSG | expand |
On Wed, Feb 03, 2021 at 10:48:16PM -0300, Marcelo Ricardo Leitner wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > Often userspace won't request the extack information, or they don't log it > because of log level or so, and even when they do, sometimes it's not > enough to know exactly what caused the error. > > Netlink extack is the standard way of reporting erros with descriptive > error messages. With a trace point on it, we then can know exactly where > the error happened, regardless of userspace app. Also, we can even see if > the err msg was overwritten. > > The wrapper do_trace_netlink_extack() is because trace points shouldn't be > called from .h files, as trace points are not that small, and the function > call to do_trace_netlink_extack() on the macros is not protected by > tracepoint_enabled() because the macros are called from modules, and this > would require exporting some trace structs. As this is error path, it's > better to export just the wrapper instead. > > v2: removed leftover tracepoint declaration Whoops, missed a blank line here. Please just let me know if I should send a new one. Thanks. > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
On 2/3/21 6:48 PM, Marcelo Ricardo Leitner wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > Often userspace won't request the extack information, or they don't log it > because of log level or so, and even when they do, sometimes it's not > enough to know exactly what caused the error. > > Netlink extack is the standard way of reporting erros with descriptive > error messages. With a trace point on it, we then can know exactly where > the error happened, regardless of userspace app. Also, we can even see if > the err msg was overwritten. > > The wrapper do_trace_netlink_extack() is because trace points shouldn't be > called from .h files, as trace points are not that small, and the function > call to do_trace_netlink_extack() on the macros is not protected by > tracepoint_enabled() because the macros are called from modules, and this > would require exporting some trace structs. As this is error path, it's > better to export just the wrapper instead. > > v2: removed leftover tracepoint declaration > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > --- > include/linux/netlink.h | 6 ++++++ > include/trace/events/netlink.h | 29 +++++++++++++++++++++++++++++ > net/netlink/af_netlink.c | 8 ++++++++ > 3 files changed, 43 insertions(+) > create mode 100644 include/trace/events/netlink.h > Reviewed-by: David Ahern <dsahern@kernel.org>
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Wed, 3 Feb 2021 22:48:16 -0300 you wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > Often userspace won't request the extack information, or they don't log it > because of log level or so, and even when they do, sometimes it's not > enough to know exactly what caused the error. > > Netlink extack is the standard way of reporting erros with descriptive > error messages. With a trace point on it, we then can know exactly where > the error happened, regardless of userspace app. Also, we can even see if > the err msg was overwritten. > > [...] Here is the summary with links: - [net-next,v2] netlink: add tracepoint at NL_SET_ERR_MSG https://git.kernel.org/netdev/net-next/c/7e3ce05e7f65 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/include/linux/netlink.h b/include/linux/netlink.h index 9f118771e24808623287d46157046749ec96a2b5..0bcf98098c5a01dd3e2c373692d8f28c6dc5e0f8 100644 --- a/include/linux/netlink.h +++ b/include/linux/netlink.h @@ -11,6 +11,8 @@ struct net; +void do_trace_netlink_extack(const char *msg); + static inline struct nlmsghdr *nlmsg_hdr(const struct sk_buff *skb) { return (struct nlmsghdr *)skb->data; @@ -90,6 +92,8 @@ struct netlink_ext_ack { static const char __msg[] = msg; \ struct netlink_ext_ack *__extack = (extack); \ \ + do_trace_netlink_extack(__msg); \ + \ if (__extack) \ __extack->_msg = __msg; \ } while (0) @@ -110,6 +114,8 @@ struct netlink_ext_ack { static const char __msg[] = msg; \ struct netlink_ext_ack *__extack = (extack); \ \ + do_trace_netlink_extack(__msg); \ + \ if (__extack) { \ __extack->_msg = __msg; \ __extack->bad_attr = (attr); \ diff --git a/include/trace/events/netlink.h b/include/trace/events/netlink.h new file mode 100644 index 0000000000000000000000000000000000000000..3b7be3b386a4f3976738a107fe4b7e0915ae58bb --- /dev/null +++ b/include/trace/events/netlink.h @@ -0,0 +1,29 @@ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM netlink + +#if !defined(_TRACE_NETLINK_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_NETLINK_H + +#include <linux/tracepoint.h> + +TRACE_EVENT(netlink_extack, + + TP_PROTO(const char *msg), + + TP_ARGS(msg), + + TP_STRUCT__entry( + __string( msg, msg ) + ), + + TP_fast_assign( + __assign_str(msg, msg); + ), + + TP_printk("msg=%s", __get_str(msg)) +); + +#endif /* _TRACE_NETLINK_H */ + +/* This part must be outside protection */ +#include <trace/define_trace.h> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index daca50d6bb1283f3b04b585217f2aea6ba279b8b..dd488938447f9735daf1fb727c339a9874bab38b 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -67,6 +67,8 @@ #include <net/sock.h> #include <net/scm.h> #include <net/netlink.h> +#define CREATE_TRACE_POINTS +#include <trace/events/netlink.h> #include "af_netlink.h" @@ -147,6 +149,12 @@ static BLOCKING_NOTIFIER_HEAD(netlink_chain); static const struct rhashtable_params netlink_rhashtable_params; +void do_trace_netlink_extack(const char *msg) +{ + trace_netlink_extack(msg); +} +EXPORT_SYMBOL(do_trace_netlink_extack); + static inline u32 netlink_group_mask(u32 group) { return group ? 1 << (group - 1) : 0;