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.
>
> 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.leit...@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)
> 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.
