From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt0-f169.google.com ([209.85.216.169]:34320 "EHLO mail-qt0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752563AbeCPTXd (ORCPT ); Fri, 16 Mar 2018 15:23:33 -0400 Received: by mail-qt0-f169.google.com with SMTP id l25so12056945qtj.1 for ; Fri, 16 Mar 2018 12:23:33 -0700 (PDT) From: Marcelo Ricardo Leitner To: netdev@vger.kernel.org Cc: marcelo.leitner@gmail.com, Alexander Aring , Jiri Pirko , Jakub Kicinski Subject: [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message Date: Fri, 16 Mar 2018 16:23:10 -0300 Message-Id: <673abaddb26351826ca454f46d1271f1f4814c56.1521226621.git.marcelo.leitner@gmail.com> In-Reply-To: References: Sender: netdev-owner@vger.kernel.org List-ID: Currently extack can carry only a single message, which is usually the error message. This imposes a limitation on a more verbose error reporting. For example, it's not able to carry warning messages together with the error message, or 2 warning messages. One use case is when dealing with tc offloading. If it failed to offload, and also failed to install on software, it will report only regarding the error about the software datapath, but the error from the hardware path would also have been welcomed. This patch extends extack so it now can carry up to 8 messages and these messages may be prefixed similarly to printk/pr_warning, so thus they can be tagged either was warning or error. Fixed number of messages because supporting a dynamic limit seem to be an overkill for the moment. Remember that this is not meant to be a trace tool, but an error reporting one. Signed-off-by: Marcelo Ricardo Leitner --- include/linux/netlink.h | 50 +++++++++++++++++++++++++++++------------------- net/netlink/af_netlink.c | 12 +++++++----- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/include/linux/netlink.h b/include/linux/netlink.h index f3075d6c7e8229c999ab650537f1e3b11e1f457b..d9780836cf263d4c436d732e9b7a8cde0739ac23 100644 --- a/include/linux/netlink.h +++ b/include/linux/netlink.h @@ -71,43 +71,53 @@ netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg) * @cookie: cookie data to return to userspace (for success) * @cookie_len: actual cookie data length */ +#define NETLINK_MAX_EXTACK_MSGS 8 struct netlink_ext_ack { - const char *_msg; + const char *_msg[NETLINK_MAX_EXTACK_MSGS]; const struct nlattr *bad_attr; u8 cookie[NETLINK_MAX_COOKIE_LEN]; u8 cookie_len; + u8 _msg_count; }; -/* Always use this macro, this allows later putting the - * message into a separate section or such for things - * like translation or listing all possible messages. - * Currently string formatting is not supported (due - * to the lack of an output buffer.) +/* Always use these macros, this allows later putting + * the message into a separate section or such for + * things like translation or listing all possible + * messages. Currently string formatting is not + * supported (due to the lack of an output buffer.) */ -#define NL_SET_ERR_MSG(extack, msg) do { \ - static const char __msg[] = msg; \ - struct netlink_ext_ack *__extack = (extack); \ - \ - if (__extack) \ - __extack->_msg = __msg; \ +#define NL_SET_MSG(extack, msg) do { \ + static const char __msg[] = msg; \ + struct netlink_ext_ack *__extack = (extack); \ + \ + if (__extack && \ + !WARN_ON(__extack->_msg_count >= NETLINK_MAX_EXTACK_MSGS)) \ + __extack->_msg[__extack->_msg_count++] = __msg; \ } while (0) +#define NL_SET_ERR_MSG(extack, msg) NL_SET_MSG(extack, msg) +#define NL_SET_WARN_MSG(extack, msg) NL_SET_MSG(extack, KERN_WARNING msg) + #define NL_SET_ERR_MSG_MOD(extack, msg) \ NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " msg) +#define NL_SET_WARN_MSG_MOD(extack, msg) \ + NL_SET_WARN_MSG((extack), KBUILD_MODNAME ": " msg) + #define NL_SET_BAD_ATTR(extack, attr) do { \ if ((extack)) \ (extack)->bad_attr = (attr); \ } while (0) -#define NL_SET_ERR_MSG_ATTR(extack, attr, msg) do { \ - static const char __msg[] = msg; \ - struct netlink_ext_ack *__extack = (extack); \ - \ - if (__extack) { \ - __extack->_msg = __msg; \ - __extack->bad_attr = (attr); \ - } \ +#define NL_SET_ERR_MSG_ATTR(extack, attr, msg) do { \ + static const char __msg[] = msg; \ + struct netlink_ext_ack *__extack = (extack); \ + \ + if (__extack) { \ + if (!WARN_ON(__extack->_msg_count >= NETLINK_MAX_EXTACK_MSGS)) \ + __extack->_msg[__extack->_msg_count++] = __msg; \ + __extack->bad_attr = (attr); \ + } \ } while (0) extern void netlink_kernel_release(struct sock *sk); diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 5d10dcfe6411e745e2abcda925fe7b6fabdac0fc..71b4ece5c760979e082b0c644af9d2f222e1b721 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2350,13 +2350,15 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk); unsigned int flags = 0; bool nlk_has_extack = nlk->flags & NETLINK_F_EXT_ACK; + int i; /* Error messages get the original request appened, unless the user * requests to cap the error message, and get extra error data if * requested. */ - if (nlk_has_extack && extack && extack->_msg) - tlvlen += nla_total_size(strlen(extack->_msg) + 1); + if (nlk_has_extack && extack) + for (i = 0; i < extack->_msg_count; i++) + tlvlen += nla_total_size(strlen(extack->_msg[i]) + 1); if (err) { if (!(nlk->flags & NETLINK_F_CAP_ACK)) @@ -2389,10 +2391,10 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg) ? nlh->nlmsg_len : sizeof(*nlh)); if (nlk_has_extack && extack) { - if (extack->_msg) { + for (i = 0; i < extack->_msg_count; i++) WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG, - extack->_msg)); - } + extack->_msg[i])); + if (err) { if (extack->bad_attr && !WARN_ON((u8 *)extack->bad_attr < in_skb->data || -- 2.14.3