Add support for warnings to extack

Currently we have the limitation that warnings cannot be reported though extack. For example, when tc flower failed to get offloaded but got installed on software datapath. The hardware failure is not fatal and thus extack is not even shared with the driver, so the error is simply omitted from any logging.

The idea here is to allow such kind of warnings to get through and be available for the sysadmin or the tool managing such commands (like Open vSwitch), so that if this happens, we will have such log message in a file later.

The first patch extends extack to support more than one message and with different log level (currently only error and warning). The second shares extack with the drivers regardless of skip_sw.

The iproute patch also follows.

This kernel change is backward compatible with older iproute because iproute will only process the last message, which should be the error one in case of failure, or a warning if it suceeded.

The iproute change is compatible with older kernels because it will find only one message to be processed and will handle it properly.

With this patches, this is now possible:

# tc qdisc add dev p7p1 ingress
# tc filter add dev p7p1 parent ffff: protocol ip prio 1 flower \
	src_mac ec:13:db:00:00:00 dst_mac ec:14:c2:00:00:00 \
	src_ip 56.0.0.0 dst_ip 55.0.0.0 action drop
Warning: TC offload is disabled on net device.
# echo $?
0

Marcelo Ricardo Leitner (2):
netlink: extend extack so it can carry more than one message
sched: pass extack through even if skip_sw is not set

 include/linux/netlink.h  | 50 +++++++++++++++++++++++++++++-------------------
 include/net/pkt_cls.h    |  3 +--
 net/netlink/af_netlink.c | 12 +++++++-----
 3 files changed, 38 insertions(+), 27 deletions(-)

Signature · 1 line

2.14.3

Currently we have the limitation that warnings cannot be reported though extack. For example, when tc flower failed to get offloaded but got installed on software datapath. The hardware failure is not fatal and thus extack is not even shared with the driver, so the error is simply omitted from any logging.

The idea here is to allow such kind of warnings to get through and be available for the sysadmin or the tool managing such commands (like Open vSwitch), so that if this happens, we will have such log message in a file later.

The first patch extends extack to support more than one message and with different log level (currently only error and warning). The second shares extack with the drivers regardless of skip_sw.

The iproute patch also follows.

This kernel change is backward compatible with older iproute because iproute will only process the last message, which should be the error one in case of failure, or a warning if it suceeded.

The iproute change is compatible with older kernels because it will find only one message to be processed and will handle it properly.

With this patches, this is now possible:

# tc qdisc add dev p7p1 ingress
# tc filter add dev p7p1 parent ffff: protocol ip prio 1 flower \
	src_mac ec:13:db:00:00:00 dst_mac ec:14:c2:00:00:00 \
	src_ip 56.0.0.0 dst_ip 55.0.0.0 action drop
Warning: TC offload is disabled on net device.
# echo $?
0

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 <[email protected]>

---
 include/linux/netlink.h  | 50 +++++++++++++++++++++++++++++-------------------
 net/netlink/af_netlink.c | 12 +++++++-----
 2 files changed, 37 insertions(+), 25 deletions(-)

include/linux/netlink.h +0 -0

index f3075d6c7e8229c999ab650537f1e3b11e1f457b..d9780836cf263d4c436d732e9b7a8cde0739ac23 100644

include/linux/netlink.h +14 -12

 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;		\

Currently we have the limitation that warnings cannot be reported though extack. For example, when tc flower failed to get offloaded but got installed on software datapath. The hardware failure is not fatal and thus extack is not even shared with the driver, so the error is simply omitted from any logging.

CC David Ahern <[email protected]>

Quoted 24 lines · Marcelo Ricardo Leitner

Currently we have the limitation that warnings cannot be reported though extack. For example, when tc flower failed to get offloaded but got installed on software datapath. The hardware failure is not fatal and thus extack is not even shared with the driver, so the error is simply omitted from any logging.

The idea here is to allow such kind of warnings to get through and be available for the sysadmin or the tool managing such commands (like Open vSwitch), so that if this happens, we will have such log message in a file later.

The first patch extends extack to support more than one message and with different log level (currently only error and warning). The second shares extack with the drivers regardless of skip_sw.

The iproute patch also follows.

This kernel change is backward compatible with older iproute because iproute will only process the last message, which should be the error one in case of failure, or a warning if it suceeded.

The iproute change is compatible with older kernels because it will find only one message to be processed and will handle it properly.

Quoted 8 lines

With this patches, this is now possible:

# tc qdisc add dev p7p1 ingress
# tc filter add dev p7p1 parent ffff: protocol ip prio 1 flower \
	src_mac ec:13:db:00:00:00 dst_mac ec:14:c2:00:00:00 \
	src_ip 56.0.0.0 dst_ip 55.0.0.0 action drop
Warning: TC offload is disabled on net device.
# echo $?
0

IMHO this set does more and less than is required to solve the problem.

The way I understand it is we don't want HW offload errors/warnings to be printed to unsuspecting users who didn't specify any skip_* flags. What carries the message and whether it's explicitly marked as warning or error does not change the fact that user of the SW fwd path may not want to not be bothered by offload warnings.

There maybe well be value in ability to report multiple messages. But for opt-in warning messages I would be leaning towards:

include/net/pkt_cls.h +1 -1

index e828d31be5dae0ae8c69016dfde50379296484aa..7cec393bb47974b48a6d510b8aa84534a7a98594 100644
@@ -705,8 +705,7 @@ tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common,
 	cls_common->chain_index = tp->chain->index;
 	cls_common->protocol = tp->protocol;
 	cls_common->prio = tp->prio;
-	if (tc_skip_sw(flags))
+	if (tc_skip_sw(flags) || flags & TCA_CLS_FLAGS_OFFLOAD_VERBOSE)
 		cls_common->extack = extack;
 }
 
 enum tc_fl_command {

That is admittedly quite conservative. Esp. in case of flower, cls_bpf is used in SW far more than HW, not to mention qdisc offload (although flag would be different there)!

Quoted 3 lines · Marcelo Ricardo Leitner

Currently extack can carry only a single message, which is usually the error message.

Quoted 3 lines

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 22 lines

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 <[email protected]>

---
 include/linux/netlink.h  | 50 +++++++++++++++++++++++++++++-------------------
 net/netlink/af_netlink.c | 12 +++++++-----
 2 files changed, 37 insertions(+), 25 deletions(-)

include/linux/netlink.h netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg) · line 74

Quoted 9 lines

include/linux/netlink.h +1 -0

index f3075d6c7e8229c999ab650537f1e3b11e1f457b..d9780836cf263d4c436d732e9b7a8cde0739ac23 100644
@@ -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 37 lines

include/linux/netlink.h +14 -12

 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)
 

include/linux/netlink.h netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg) · line 105

Quoted 8 lines

include/linux/netlink.h +6 -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.

Quoted 5 lines · Marcelo Ricardo Leitner

Currently we have the limitation that warnings cannot be reported though extack. For example, when tc flower failed to get offloaded but got installed on software datapath. The hardware failure is not fatal and thus extack is not even shared with the driver, so the error is simply omitted from any logging.

If this set ends up moving forward, the above statement needs to be
corrected: extack allows non-error messages to be sent back to the user, so the above must be talking about some other limitation local to tc.

Quoted 5 lines · Marcelo Ricardo Leitner

Currently we have the limitation that warnings cannot be reported though extack. For example, when tc flower failed to get offloaded but got installed on software datapath. The hardware failure is not fatal and thus extack is not even shared with the driver, so the error is simply omitted from any logging.

If this set ends up moving forward, the above statement needs to be
corrected: extack allows non-error messages to be sent back to the user, so the above must be talking about some other limitation local to tc.

Quoted 4 lines · David Ahern

Quoted 3 lines · Marcelo Ricardo Leitner

Currently extack can carry only a single message, which is usually the error message.

Quoted 8 lines

Quoted 3 lines

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.

I don't see your point here.

The proposed patch extends what you said to:
- include warnings on error reports
- allow more than 1 message

With the proposed patch, if nlmsgerr is 0 all messages are considered as warnings. If it's non-zero, some may be marked as warnings.

Quoted 4 lines

Quoted 4 lines

include/linux/netlink.h +3 -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)			\

Quoted 8 lines

include/linux/netlink.h netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg) · line 105

Quoted 4 lines

include/linux/netlink.h +3 -0

 	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.

Are you saying the markings at beginning of the messages are not possible? If that's the case, we probably can think of something else, as I see value in being able to deliver warnings together with errors.

Quoted 14 lines · Marcelo Ricardo Leitner

Quoted 4 lines · David Ahern

Quoted 3 lines · Marcelo Ricardo Leitner

Currently extack can carry only a single message, which is usually the error message.

Quoted 8 lines

Quoted 3 lines

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 8 lines

I don't see your point here.

The proposed patch extends what you said to:
- include warnings on error reports
- allow more than 1 message

With the proposed patch, if nlmsgerr is 0 all messages are considered as warnings. If it's non-zero, some may be marked as warnings.

It's the 'some' that I was referring to, but ...

Quoted 13 lines

Quoted 12 lines

include/linux/netlink.h netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg) · line 105

Quoted 8 lines

include/linux/netlink.h +6 -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.

Quoted 3 lines

Are you saying the markings at beginning of the messages are not possible? If that's the case, we probably can think of something else, as I see value in being able to deliver warnings together with errors.

... I did miss the KERN_WARNIN above. That means that warning messages are prefixed by 0x1 (KERN_SOH) and "4" (warning loglevel). There will be cases missed by iproute2 as current versions won't catch the 2 new characters.

Converting code to be able to continue generating error messages yet ultimately fail seems overly complex to me. I get the intent of returning as much info as possible, but most of that feels (e.g., in the mlx5 example you referenced) like someone learning how to do something the first time in which case 1 at a time errors seems reasonable - in fact might drive home some lessons. ;-)

This patch introduces support for reading more than one message from extack's and to adjust their level (warning/error) accordingly.

Yes, there is a FIXME tag in the callback call for now.

Signed-off-by Marcelo Ricardo Leitner <[email protected]>

lib/libnetlink.c +16 -11

 static int err_attr_cb(const struct nlattr *attr, void *data)
 {
-	const struct nlattr **tb = data;
+	struct extack_args *tb = data;
 	uint16_t type;
 
 	if (mnl_attr_type_valid(attr, NLMSGERR_ATTR_MAX) < 0) {

@@ -60,19 +67,23 @@ static int err_attr_cb(const struct nlattr *attr, void *data)
 		return MNL_CB_ERROR;
 	}
 
-	tb[type] = attr;
+	if (type == NLMSGERR_ATTR_OFFS)
+		tb->offs = attr;
+	else if (tb->msg_count < NETLINK_MAX_EXTACK_MSGS)
+		tb->msg[tb->msg_count++] = attr;
 	return MNL_CB_OK;
 }
 
 /* dump netlink extended ack error message */
 int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 {
-	struct nlattr *tb[NLMSGERR_ATTR_MAX + 1] = {};
+	struct extack_args tb = {};
 	const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
 	const struct nlmsghdr *err_nlh = NULL;
 	unsigned int hlen = sizeof(*err);
-	const char *msg = NULL;
+	const char *msg[NETLINK_MAX_EXTACK_MSGS] = {};
 	uint32_t off = 0;
+	int ret, i;
 
 	/* no TLVs, nothing to do here */
 	if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS))

@@ -82,14 +93,14 @@ int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 	if (!(nlh->nlmsg_flags & NLM_F_CAPPED))
 		hlen += mnl_nlmsg_get_payload_len(&err->msg);
 
-	if (mnl_attr_parse(nlh, hlen, err_attr_cb, tb) != MNL_CB_OK)
+	if (mnl_attr_parse(nlh, hlen, err_attr_cb, &tb) != MNL_CB_OK)
 		return 0;
 
-	if (tb[NLMSGERR_ATTR_MSG])
-		msg = mnl_attr_get_str(tb[NLMSGERR_ATTR_MSG]);
+	for (i = 0; i < NETLINK_MAX_EXTACK_MSGS && tb.msg[i]; i++)
+		msg[i] = mnl_attr_get_str(tb.msg[i]);
 
-	if (tb[NLMSGERR_ATTR_OFFS]) {
-		off = mnl_attr_get_u32(tb[NLMSGERR_ATTR_OFFS]);
+	if (tb.offs) {
+		off = mnl_attr_get_u32(tb.offs);
 
 		if (off > nlh->nlmsg_len) {
 			fprintf(stderr,

@@ -100,21 +111,35 @@ int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 	}
 
 	if (errfn)
-		return errfn(msg, off, err_nlh);
+		return errfn(*msg, off, err_nlh); /* FIXME */
 
-	if (msg && *msg != '\0') {
+	ret = 0;
+	for (i = 0; i < NETLINK_MAX_EXTACK_MSGS && msg[i]; i++) {

lib/libnetlink.c +8 -4

+		/* But we can't have Error if it didn't fail. */
+		if (is_err && !err->error)
+			is_err = 0;
 
 		fprintf(stderr, "%s: %s",
-			is_err ? "Error" : "Warning", msg);
-		if (msg[strlen(msg) - 1] != '.')
+			is_err ? "Error" : "Warning", _msg);
+		if (_msg[strlen(_msg) - 1] != '.')
 			fprintf(stderr, ".");
 		fprintf(stderr, "\n");
 
-		return is_err ? 1 : 0;
+		if (is_err)
+			ret = 1;
 	}
 
-	return 0;
+	return ret;
 }
 #else
 #warning "libmnl required for error support"

Quoted 6 lines · On Fri, 16 Mar 2018 16:23:09 -0300 Marcelo Ricardo Leitner…

This patch introduces support for reading more than one message from extack's and to adjust their level (warning/error) accordingly.

Yes, there is a FIXME tag in the callback call for now.

Signed-off-by Marcelo Ricardo Leitner <[email protected]>

Make sense, can hold off until kernel supports warnings.

Quoted 13 lines

---
 lib/libnetlink.c | 55 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 15 deletions(-)

lib/libnetlink.c +0 -0

index 928de1dd16d84b7802c06d0a659f5d73e1bbcb4b..7c4c81e11e02ea857888190eb5e7a9e99d159bb3 100644
@@ -43,9 +43,16 @@ static const enum mnl_attr_data_type extack_policy[NLMSGERR_ATTR_MAX + 1] = {
 	[NLMSGERR_ATTR_OFFS]	= MNL_TYPE_U32,
 };
 

lib/libnetlink.c static const enum mnl_attr_data_type extack_policy[NLMSGERR_ATTR_MAX + 1] = { · line 46

Quoted 1 line

lib/libnetlink.c +1 -0

+#define NETLINK_MAX_EXTACK_MSGS 8

Would rather not have fixed maximums

lib/libnetlink.c static const enum mnl_attr_data_type extack_policy[NLMSGERR_ATTR_MAX + 1] = { · line 51

Quoted 6 lines

lib/libnetlink.c +5 -0

+struct extack_args {
+	const struct nlattr *msg[NETLINK_MAX_EXTACK_MSGS];
+	const struct nlattr *offs;
+	int msg_count;
+};

If you put msg[] last in structure, it could be variable length.

Quoted 62 lines

lib/libnetlink.c +14 -10

 static int err_attr_cb(const struct nlattr *attr, void *data)
 {
-	const struct nlattr **tb = data;
+	struct extack_args *tb = data;
 	uint16_t type;
 
 	if (mnl_attr_type_valid(attr, NLMSGERR_ATTR_MAX) < 0) {

@@ -60,19 +67,23 @@ static int err_attr_cb(const struct nlattr *attr, void *data)
 		return MNL_CB_ERROR;
 	}
 
-	tb[type] = attr;
+	if (type == NLMSGERR_ATTR_OFFS)
+		tb->offs = attr;
+	else if (tb->msg_count < NETLINK_MAX_EXTACK_MSGS)
+		tb->msg[tb->msg_count++] = attr;
 	return MNL_CB_OK;
 }
 
 /* dump netlink extended ack error message */
 int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 {
-	struct nlattr *tb[NLMSGERR_ATTR_MAX + 1] = {};
+	struct extack_args tb = {};
 	const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
 	const struct nlmsghdr *err_nlh = NULL;
 	unsigned int hlen = sizeof(*err);
-	const char *msg = NULL;
+	const char *msg[NETLINK_MAX_EXTACK_MSGS] = {};
 	uint32_t off = 0;
+	int ret, i;
 
 	/* no TLVs, nothing to do here */
 	if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS))

@@ -82,14 +93,14 @@ int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 	if (!(nlh->nlmsg_flags & NLM_F_CAPPED))
 		hlen += mnl_nlmsg_get_payload_len(&err->msg);
 
-	if (mnl_attr_parse(nlh, hlen, err_attr_cb, tb) != MNL_CB_OK)
+	if (mnl_attr_parse(nlh, hlen, err_attr_cb, &tb) != MNL_CB_OK)
 		return 0;
 
-	if (tb[NLMSGERR_ATTR_MSG])
-		msg = mnl_attr_get_str(tb[NLMSGERR_ATTR_MSG]);
+	for (i = 0; i < NETLINK_MAX_EXTACK_MSGS && tb.msg[i]; i++)
+		msg[i] = mnl_attr_get_str(tb.msg[i]);
 
-	if (tb[NLMSGERR_ATTR_OFFS]) {
-		off = mnl_attr_get_u32(tb[NLMSGERR_ATTR_OFFS]);
+	if (tb.offs) {
+		off = mnl_attr_get_u32(tb.offs);
 
 		if (off > nlh->nlmsg_len) {
 			fprintf(stderr,

@@ -100,21 +111,35 @@ int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 	}
 
 	if (errfn)
-		return errfn(msg, off, err_nlh);
+		return errfn(*msg, off, err_nlh); /* FIXME */
 

lib/libnetlink.c int nl_dump_ext_ack(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn) · line 127

Quoted 13 lines

lib/libnetlink.c +11 -1

-	if (msg && *msg != '\0') {
+	ret = 0;
+	for (i = 0; i < NETLINK_MAX_EXTACK_MSGS && msg[i]; i++) {
 		bool is_err = !!err->error;
+		const char *_msg = msg[i];
+
+		/* Message tagging has precedence.
+		 * KERN_WARNING = ASCII Start Of Header ('\001') + '4'
+		 */
+		if (!strncmp(_msg, "\0014", 2)) {
+			is_err = false;
+			_msg += 2;
+		}

If you are going to have an API that has levels, it must be the same as existing syslog kernel log format and maybe even get some code reuse.

Quoted 23 lines

lib/libnetlink.c +8 -4

+		/* But we can't have Error if it didn't fail. */
+		if (is_err && !err->error)
+			is_err = 0;
 
 		fprintf(stderr, "%s: %s",
-			is_err ? "Error" : "Warning", msg);
-		if (msg[strlen(msg) - 1] != '.')
+			is_err ? "Error" : "Warning", _msg);
+		if (_msg[strlen(_msg) - 1] != '.')
 			fprintf(stderr, ".");
 		fprintf(stderr, "\n");
 
-		return is_err ? 1 : 0;
+		if (is_err)
+			ret = 1;
 	}
 
-	return 0;
+	return ret;
 }
 #else
 #warning "libmnl required for error support"