After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793261 - [RFE] Platform: support netlink extended acks
[RFE] Platform: support netlink extended acks
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks: nm-review
 
 
Reported: 2018-02-07 15:08 UTC by Beniamino Galvani
Modified: 2018-03-09 16:57 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
[PATCH] platform: print error message from netlink extended ack (5.03 KB, patch)
2018-03-09 09:41 UTC, Beniamino Galvani 		none Details | Review

Description Beniamino Galvani 2018-02-07 15:08:53 UTC 
Recent kernel versions report an "extended ack" for netlink failures, that contains a text describing the reason for the failure. This is useful for debugging failures, for example -EINVAL errors, where the extended ack describes better what went wrong.
Comment 1 Beniamino Galvani 2018-03-09 09:41:02 UTC 
Created attachment 369484 [details] [review]
[PATCH] platform: print error message from netlink extended ack
Comment 2 Thomas Haller 2018-03-09 10:12:48 UTC 
+    err = setsockopt (sk->s_fd, SOL_NETLINK, NETLINK_EXT_ACK, &enable, sizeof (enable));

gboolean is indeed a typedef of "int", and that won't change in glib. But could you still use an explicit "int" type variable to make that clear?


-                   _LOGD ("netlink: recvmsg: error message from kernel: %s (%d) for request %d",
+                   _LOGE ("netlink: recvmsg: error message from kernel: %s (%d)%s%s%s for request %d",

The platform layer does not know whether a netlink failure is really a serious error or something that can happen under non-erroneous conditions. Only the caller can know that. Hence, logging from platform at this point should be at most at debug level.
Comment 3 Beniamino Galvani 2018-03-09 15:50:01 UTC 
(In reply to Thomas Haller from comment #2)
> +    err = setsockopt (sk->s_fd, SOL_NETLINK, NETLINK_EXT_ACK, &enable,
> sizeof (enable));
> 
> gboolean is indeed a typedef of "int", and that won't change in glib. But
> could you still use an explicit "int" type variable to make that clear?

Fixed.

> -                   _LOGD ("netlink: recvmsg: error message from kernel: %s
> (%d) for request %d",
> +                   _LOGE ("netlink: recvmsg: error message from kernel: %s
> (%d)%s%s%s for request %d",
> 
> The platform layer does not know whether a netlink failure is really a
> serious error or something that can happen under non-erroneous conditions.
> Only the caller can know that. Hence, logging from platform at this point
> should be at most at debug level.

Yeah, I forgot to change it back to _LOGD after debugging.

Pushed with other commits to branch bg/extack-bgo793261.
Comment 4 Thomas Haller 2018-03-09 16:47:58 UTC 
lgtm