On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
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.
The only means for userspace to separate an error message from info or
warnings is the error in nlmsgerr. If it is non-0, any extack message is
considered an error else it is a warning.
quoted hunk ↗ jump to hunk
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 <marcelo.leitner@gmail.com>
---
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
8 is way too many. If some change fails because of an error, why would a
single error message not be enough? If it is a not an error, why is more
than 1 warning message not enough? (I forget the details of the tc
'skip_sw' use case)
quoted hunk ↗ jump to hunk
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)
+
Adding separate macros for error versus warning is confusing since from
an extack perspective a message is a message and there is no uapi to
separate them.