openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[ordex (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email

to review the following change.


Change subject: dco_linux: enable extended netlink error reporting
......................................................................

dco_linux: enable extended netlink error reporting

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/dco_linux.c
1 file changed, 5 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/40/1040/1

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..fcca9dc 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange

[Openvpn-devel] [XS] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, ordex, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email )

Change subject: dco_linux: enable extended netlink error reporting
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

Patchset:

PS1: 
This patch works and does what it says on the lid.  Alas, when we're starting to fix error message printing, we also need to fix it for good...

```
    if (tb_msg[NLMSGERR_ATTR_MSG])
    {
        len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                      nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
        msg(M_WARN, "kernel error: %*s\n", len,
            (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
    }
```

there is an extra `\n` which should not be there.  Please :-)



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Comment-Date: Fri, 06 Jun 2025 12:54:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[ordex (C. Review) <ge...@op...>]

Attention is currently required from: cron2, flichtenheld, ordex, plaisthos.

Hello cron2, flichtenheld, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by cron2


Change subject: dco_linux: enable extended netlink error reporting
......................................................................

dco_linux: enable extended netlink error reporting

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/dco_linux.c
1 file changed, 8 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/40/1040/2

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..0345413 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -367,19 +367,19 @@
     {
         len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                       nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
-        msg(M_WARN, "kernel error: %*s\n", len,
+        msg(M_WARN, "kernel error: %*s", len,
             (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST])
     {
-        msg(M_WARN, "kernel error: missing required nesting type %u\n",
+        msg(M_WARN, "kernel error: missing required nesting type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE])
     {
-        msg(M_WARN, "kernel error: missing required attribute type %u\n",
+        msg(M_WARN, "kernel error: missing required attribute type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE]));
     }
 
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, ordex, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email )

Change subject: dco_linux: enable extended netlink error reporting
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Comment-Date: Mon, 09 Jun 2025 10:01:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [PATCH v2] dco_linux: enable extended netlink error reporting

[Gert D. <ge...@gr...>]

From: Antonio Quartulli <an...@ma...>

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
Acked-by: Gert Doering <ge...@gr...>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1040
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..0345413 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -367,19 +367,19 @@
     {
         len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                       nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
-        msg(M_WARN, "kernel error: %*s\n", len,
+        msg(M_WARN, "kernel error: %*s", len,
             (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST])
     {
-        msg(M_WARN, "kernel error: missing required nesting type %u\n",
+        msg(M_WARN, "kernel error: missing required nesting type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE])
     {
-        msg(M_WARN, "kernel error: missing required attribute type %u\n",
+        msg(M_WARN, "kernel error: missing required attribute type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE]));
     }
 
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

[Openvpn-devel] [PATCH applied] Re: dco_linux: enable extended netlink error reporting

[Gert D. <ge...@gr...>]

Tested, makes sense - I intentionally broke some code parts in dco_linux.c
that set up a peer, and instead of a not very helpful "-7" userspace now
gets a proper error string.  Discovered the extra "\n" testing this, so
thanks for the v2 fix :-)

Your patch has been applied to the master branch.

commit ddbfbeca603eedddc12556b28e5cd9b022e9ea63
Author: Antonio Quartulli
Date:   Mon Jun 9 12:02:10 2025 +0200

     dco_linux: enable extended netlink error reporting

     Signed-off-by: Antonio Quartulli <an...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31885.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#3) to the change originally created by ordex. ( http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: dco_linux: enable extended netlink error reporting
......................................................................

dco_linux: enable extended netlink error reporting

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31885.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
1 file changed, 8 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/40/1040/3

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..0345413 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -367,19 +367,19 @@
     {
         len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                       nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
-        msg(M_WARN, "kernel error: %*s\n", len,
+        msg(M_WARN, "kernel error: %*s", len,
             (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST])
     {
-        msg(M_WARN, "kernel error: missing required nesting type %u\n",
+        msg(M_WARN, "kernel error: missing required nesting type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE])
     {
-        msg(M_WARN, "kernel error: missing required attribute type %u\n",
+        msg(M_WARN, "kernel error: missing required attribute type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE]));
     }
 
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 3
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email )

Change subject: dco_linux: enable extended netlink error reporting
......................................................................

dco_linux: enable extended netlink error reporting

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31885.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
1 file changed, 8 insertions(+), 3 deletions(-)




diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..0345413 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -367,19 +367,19 @@
     {
         len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                       nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
-        msg(M_WARN, "kernel error: %*s\n", len,
+        msg(M_WARN, "kernel error: %*s", len,
             (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST])
     {
-        msg(M_WARN, "kernel error: missing required nesting type %u\n",
+        msg(M_WARN, "kernel error: missing required nesting type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE])
     {
-        msg(M_WARN, "kernel error: missing required attribute type %u\n",
+        msg(M_WARN, "kernel error: missing required attribute type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE]));
     }
 
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 3
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged