[PATCH v2 iproute2] lib: Dump ext-ack string by default

Subsystems: library code, the rest

STALE3354d

3 messages, 2 authors, 2017-08-09 · open the first message on its own page

[PATCH v2 iproute2] lib: Dump ext-ack string by default

From: David Ahern <hidden>
Date: 2017-08-08 14:30:23

In time, errfn can be implemented for link, route, etc commands to
give a much more detailed response (e.g., point to the attribute
that failed). Doing so is much more complicated to process the
message and convert attribute ids to names.

In any case the error string returned by the kernel should be dumped
to the user, so make that happen now.

Signed-off-by: David Ahern <redacted>
---
v2
- check that errmsg is non-null

 lib/libnetlink.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 145de2cb0ccf..063f5cd6c7b1 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -61,7 +61,6 @@ static int err_attr_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
-
 /* dump netlink extended ack error message */
 static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 {
@@ -72,9 +71,6 @@ static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 	const char *errmsg = NULL;
 	uint32_t off = 0;
 
-	if (!errfn)
-		return 0;
-
 	/* no TLVs, nothing to do here */
 	if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS))
 		return 0;
@@ -99,7 +95,16 @@ static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
 			err_nlh = &err->msg;
 	}
 
-	return errfn(errmsg, off, err_nlh);
+	if (errfn)
+		return errfn(errmsg, off, err_nlh);
+
+	if (errmsg) {
+		fprintf(stderr, "Error: %s\n", errmsg);
+
+		return 1;
+	}
+
+	return 0;
 }
 #else
 #warning "libmnl required for error support"
-- 
2.1.4

Re: [PATCH v2 iproute2] lib: Dump ext-ack string by default

From: Girish Moodalbail <hidden>
Date: 2017-08-09 07:38:39

On 8/8/17 7:30 AM, David Ahern wrote:
quoted hunk
In time, errfn can be implemented for link, route, etc commands to
give a much more detailed response (e.g., point to the attribute
that failed). Doing so is much more complicated to process the
message and convert attribute ids to names.

In any case the error string returned by the kernel should be dumped
to the user, so make that happen now.

Signed-off-by: David Ahern <redacted>
---
v2
- check that errmsg is non-null

  lib/libnetlink.c | 15 ++++++++++-----
  1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 145de2cb0ccf..063f5cd6c7b1 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -61,7 +61,6 @@ static int err_attr_cb(const struct nlattr *attr, void *data)
  	return MNL_CB_OK;
  }
  
-
  /* dump netlink extended ack error message */
  static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
  {
@@ -72,9 +71,6 @@ static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
  	const char *errmsg = NULL;
  	uint32_t off = 0;
  
-	if (!errfn)
-		return 0;
-
  	/* no TLVs, nothing to do here */
  	if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS))
  		return 0;
@@ -99,7 +95,16 @@ static int nl_dump_ext_err(const struct nlmsghdr *nlh, nl_ext_ack_fn_t errfn)
  			err_nlh = &err->msg;
  	}
  
-	return errfn(errmsg, off, err_nlh);
+	if (errfn)
+		return errfn(errmsg, off, err_nlh);
+
+	if (errmsg) {
+		fprintf(stderr, "Error: %s\n", errmsg);
Should the above output end with a period '.'? All the error messages in the Kernel are statements without a terminating period, so the output will look something like this..

Error: Minimally Geneve ID and Remote IP address are required
Error: Provided Ethernet address is not unicast
Error: Provided link layer address is not Ethernet

Thanks,
~Girish



quoted hunk
+
+		return 1;
+	}
+
+	return 0;
  }
  #else
  #warning "libmnl required for error support"

Re: [PATCH v2 iproute2] lib: Dump ext-ack string by default

From: David Ahern <hidden>
Date: 2017-08-09 14:03:24

On 8/9/17 1:38 AM, Girish Moodalbail wrote:
quoted
@@ -99,7 +95,16 @@ static int nl_dump_ext_err(const struct nlmsghdr
*nlh, nl_ext_ack_fn_t errfn)
              err_nlh = &err->msg;
      }
  -    return errfn(errmsg, off, err_nlh);
+    if (errfn)
+        return errfn(errmsg, off, err_nlh);
+
+    if (errmsg) {
+        fprintf(stderr, "Error: %s\n", errmsg);
Should the above output end with a period '.'? All the error messages in
the Kernel are statements without a terminating period, so the output
no guarantee of that, but iproute2 could check for it and add it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help