openvpn-devel — OpenVPN developers list

[Openvpn-devel] [S] Change in openvpn[master]: add flag to print addresses in a consistent format during float

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

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

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


Change subject: add flag to print addresses in a consistent format during float
......................................................................

add flag to print addresses in a consistent format during float

Introduce the MAPF_SHOW_FAMILY flag to prepend the address family to the
address when printing an mroute_addr object, similar to
print_sockaddr_ex(). This ensures that when logging a float operation,
both the old and new addresses are printed in the same format:
$proto:[$family]$address:$port.

Note: when using this flag with an IPv4-mapped IPv6 address, the output
will appear as: [AF_INET6]a.b.c.d

Change-Id: I43cd3d564d8c6ad4e41de5a38130d90cb6778395
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32345.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/mroute.c
M src/openvpn/mroute.h
M src/openvpn/multi.c
3 files changed, 11 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1092/3

diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index a617b33..e3b1e9b 100644
--- a/src/openvpn/mroute.c
+++ b/src/openvpn/mroute.c
@@ -415,6 +415,10 @@
                 {
                     buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET, false));
                 }
+                if (flags & MAPF_SHOW_FAMILY)
+                {
+                    buf_printf(&out, "[AF_INET]");
+                }
                 buf_printf(&out, "%s", print_in_addr_t(ntohl(maddr.v4.addr),
                                                        (flags & MAPF_IA_EMPTY_IF_UNDEF) ? IA_EMPTY_IF_UNDEF : 0, gc));
                 if (maddr.type & MR_WITH_NETBITS)
@@ -442,6 +446,10 @@
                 {
                     buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET6, false));
                 }
+                if (flags & MAPF_SHOW_FAMILY)
+                {
+                    buf_printf(&out, "[AF_INET6]");
+                }
                 if (IN6_IS_ADDR_V4MAPPED( &maddr.v6.addr ) )
                 {
                     buf_printf(&out, "%s", print_in_addr_t(maddr.v4mappedv6.addr,
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index c359fd2..4f9fc03 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -150,6 +150,7 @@
 #define MAPF_SUBNET            (1<<0)
 #define MAPF_IA_EMPTY_IF_UNDEF (1<<1)
 #define MAPF_SHOW_ARP          (1<<2)
+#define MAPF_SHOW_FAMILY       (1<<3)
 const char *mroute_addr_print_ex(const struct mroute_addr *ma,
                                  const unsigned int flags,
                                  struct gc_arena *gc);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ec260a2..a62c57a 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3275,8 +3275,8 @@
     msg(D_MULTI_MEDIUM, "peer %" PRIu32 " (%s) floated from %s to %s",
         mi->context.c2.tls_multi->peer_id,
         tls_common_name(mi->context.c2.tls_multi, false),
-        mroute_addr_print(&mi->real, &gc),
-        print_link_socket_actual(&m->top.c2.from, &gc));
+        mroute_addr_print_ex(&mi->real, MAPF_SHOW_FAMILY, &gc),
+        mroute_addr_print_ex(&real, MAPF_SHOW_FAMILY, &gc));
 
     /* remove old address from hash table before changing address */
     ASSERT(hash_remove(m->hash, &mi->real));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1092?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: I43cd3d564d8c6ad4e41de5a38130d90cb6778395
Gerrit-Change-Number: 1092
Gerrit-PatchSet: 3
Gerrit-Owner: ralf_lici <ra...@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]: add flag to print addresses in a consistent format during float

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

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

Change subject: add flag to print addresses in a consistent format during float
......................................................................

add flag to print addresses in a consistent format during float

Introduce the MAPF_SHOW_FAMILY flag to prepend the address family to the
address when printing an mroute_addr object, similar to
print_sockaddr_ex(). This ensures that when logging a float operation,
both the old and new addresses are printed in the same format:
$proto:[$family]$address:$port.

Note: when using this flag with an IPv4-mapped IPv6 address, the output
will appear as: [AF_INET6]a.b.c.d

Change-Id: I43cd3d564d8c6ad4e41de5a38130d90cb6778395
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32345.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/mroute.c
M src/openvpn/mroute.h
M src/openvpn/multi.c
3 files changed, 11 insertions(+), 2 deletions(-)




diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index a617b33..e3b1e9b 100644
--- a/src/openvpn/mroute.c
+++ b/src/openvpn/mroute.c
@@ -415,6 +415,10 @@
                 {
                     buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET, false));
                 }
+                if (flags & MAPF_SHOW_FAMILY)
+                {
+                    buf_printf(&out, "[AF_INET]");
+                }
                 buf_printf(&out, "%s", print_in_addr_t(ntohl(maddr.v4.addr),
                                                        (flags & MAPF_IA_EMPTY_IF_UNDEF) ? IA_EMPTY_IF_UNDEF : 0, gc));
                 if (maddr.type & MR_WITH_NETBITS)
@@ -442,6 +446,10 @@
                 {
                     buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET6, false));
                 }
+                if (flags & MAPF_SHOW_FAMILY)
+                {
+                    buf_printf(&out, "[AF_INET6]");
+                }
                 if (IN6_IS_ADDR_V4MAPPED( &maddr.v6.addr ) )
                 {
                     buf_printf(&out, "%s", print_in_addr_t(maddr.v4mappedv6.addr,
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index c359fd2..4f9fc03 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -150,6 +150,7 @@
 #define MAPF_SUBNET            (1<<0)
 #define MAPF_IA_EMPTY_IF_UNDEF (1<<1)
 #define MAPF_SHOW_ARP          (1<<2)
+#define MAPF_SHOW_FAMILY       (1<<3)
 const char *mroute_addr_print_ex(const struct mroute_addr *ma,
                                  const unsigned int flags,
                                  struct gc_arena *gc);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ec260a2..a62c57a 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3275,8 +3275,8 @@
     msg(D_MULTI_MEDIUM, "peer %" PRIu32 " (%s) floated from %s to %s",
         mi->context.c2.tls_multi->peer_id,
         tls_common_name(mi->context.c2.tls_multi, false),
-        mroute_addr_print(&mi->real, &gc),
-        print_link_socket_actual(&m->top.c2.from, &gc));
+        mroute_addr_print_ex(&mi->real, MAPF_SHOW_FAMILY, &gc),
+        mroute_addr_print_ex(&real, MAPF_SHOW_FAMILY, &gc));
 
     /* remove old address from hash table before changing address */
     ASSERT(hash_remove(m->hash, &mi->real));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1092?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: I43cd3d564d8c6ad4e41de5a38130d90cb6778395
Gerrit-Change-Number: 1092
Gerrit-PatchSet: 3
Gerrit-Owner: ralf_lici <ra...@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

[Openvpn-devel] [PATCH applied] Re: add flag to print addresses in a consistent format during float

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

"Because it makes sense" - as agreed, this still distinguishes between
"v4 on a v6 socket" and "v4 on a v4 socket", because for troubleshooting
it can make a difference.  But having both old and new address printed
by the same function in the same format is definitely an improvement :-)

Tested v4 on a v4-only socket, v4+v6 on a dual-stack v6 socket, all looks
nice and consistent now.  Thanks.

Your patch has been applied to the master branch.

commit 7d5ec053f0f30c6cd27b60ed76859a09f6dbf5e4
Author: Ralf Lici
Date:   Fri Jul 25 21:41:39 2025 +0200

     add flag to print addresses in a consistent format during float

     Signed-off-by: Ralf Lici <ra...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32345.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v2] add flag to print addresses in a consistent format during float

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

From: Ralf Lici <ra...@ma...>

Introduce the MAPF_SHOW_FAMILY flag to prepend the address family to the
address when printing an mroute_addr object, similar to
print_sockaddr_ex(). This ensures that when logging a float operation,
both the old and new addresses are printed in the same format:
$proto:[$family]$address:$port.

Note: when using this flag with an IPv4-mapped IPv6 address, the output
will appear as: [AF_INET6]a.b.c.d

Change-Id: I43cd3d564d8c6ad4e41de5a38130d90cb6778395
Signed-off-by: Ralf Lici <ra...@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/+/1092
This mail reflects revision 2 of this Change.

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

        
diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index a617b33..e3b1e9b 100644
--- a/src/openvpn/mroute.c
+++ b/src/openvpn/mroute.c
@@ -415,6 +415,10 @@
                 {
                     buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET, false));
                 }
+                if (flags & MAPF_SHOW_FAMILY)
+                {
+                    buf_printf(&out, "[AF_INET]");
+                }
                 buf_printf(&out, "%s", print_in_addr_t(ntohl(maddr.v4.addr),
                                                        (flags & MAPF_IA_EMPTY_IF_UNDEF) ? IA_EMPTY_IF_UNDEF : 0, gc));
                 if (maddr.type & MR_WITH_NETBITS)
@@ -442,6 +446,10 @@
                 {
                     buf_printf(&out, "%s:", proto2ascii(maddr.proto, AF_INET6, false));
                 }
+                if (flags & MAPF_SHOW_FAMILY)
+                {
+                    buf_printf(&out, "[AF_INET6]");
+                }
                 if (IN6_IS_ADDR_V4MAPPED( &maddr.v6.addr ) )
                 {
                     buf_printf(&out, "%s", print_in_addr_t(maddr.v4mappedv6.addr,
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index c359fd2..4f9fc03 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -150,6 +150,7 @@
 #define MAPF_SUBNET            (1<<0)
 #define MAPF_IA_EMPTY_IF_UNDEF (1<<1)
 #define MAPF_SHOW_ARP          (1<<2)
+#define MAPF_SHOW_FAMILY       (1<<3)
 const char *mroute_addr_print_ex(const struct mroute_addr *ma,
                                  const unsigned int flags,
                                  struct gc_arena *gc);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 4696686..69ec2da 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3274,8 +3274,8 @@
     msg(D_MULTI_MEDIUM, "peer %" PRIu32 " (%s) floated from %s to %s",
         mi->context.c2.tls_multi->peer_id,
         tls_common_name(mi->context.c2.tls_multi, false),
-        mroute_addr_print(&mi->real, &gc),
-        print_link_socket_actual(&m->top.c2.from, &gc));
+        mroute_addr_print_ex(&mi->real, MAPF_SHOW_FAMILY, &gc),
+        mroute_addr_print_ex(&real, MAPF_SHOW_FAMILY, &gc));
 
     /* remove old address from hash table before changing address */
     ASSERT(hash_remove(m->hash, &mi->real));

[Openvpn-devel] [S] Change in openvpn[master]: add flag to print addresses in a consistent format during float

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

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

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

Change subject: add flag to print addresses in a consistent format during float
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1092?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: I43cd3d564d8c6ad4e41de5a38130d90cb6778395
Gerrit-Change-Number: 1092
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <ra...@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: ralf_lici <ra...@ma...>
Gerrit-Comment-Date: Fri, 25 Jul 2025 19:41:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: options: Clean up function setenv_foreign_option

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

Attention is currently required from: flichtenheld, plaisthos.

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

Change subject: options: Clean up function setenv_foreign_option
......................................................................


Patch Set 1:

(1 comment)

Patchset:

PS1: 
I'm not convinced we couldn't do something more radical, like, getting rid of the loop and `first` and all that, since all callers call `setenv_foreign_option()` with `len == 3`...  (unless I misread something)



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1112?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: I7efc77f63734501dfa8a8f5bed17b1a1b4e9e201
Gerrit-Change-Number: 1112
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Fri, 25 Jul 2025 18:19:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: dco_linux: fix async message reception

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

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

Change subject: dco_linux: fix async message reception
......................................................................

dco_linux: fix async message reception

Currently whenever we send a PEER_GET request to ovpn, we also
set the CB that is supposed to parse the reply.

However, due to the async nature of netlink messages, we could
get an unrelated notification, sent by ovpn (kernel) upon some event,
after userland has set the CB, but before parsing the awaited reply.

When this happens, the notification is then parsed with the
configured CB instead of the notification parser, thus effectively
rejecting the notification and losing the event.

To fix this inconsistency, make ovpn_handle_msg() the default and
only netlink parser CB. It is configured upon DCO initialization
and is never removed.

ovpn_handle_msg() will check the message type and will call the
corresponding handler. This way, no matter what message we get at
what time, we'll always parse it correctly.

As a bonus we can also simplify the nl_sendmsg() API as we
don't need to pass the cb and its argument anymore.

The ID of the NLCTRL family is now also stored in the DCO
context as we need it to check when we receive a mcast ID
lookup message.

Change-Id: I23ad79e14844aefde9ece34dadef0b75ff267201
Github: closes OpenVPN/openvpn#793
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.../msg32339.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
M src/openvpn/dco_linux.h
2 files changed, 110 insertions(+), 67 deletions(-)




diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 7c639d9..728fb7e 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -167,23 +167,19 @@
 }
 
 /**
- * Send a prepared netlink message and registers cb as callback if non-null.
+ * Send a prepared netlink message.
  *
  * The method will also free nl_msg
  * @param dco       The dco context to use
  * @param nl_msg    the message to use
- * @param cb        An optional callback if the caller expects an answer
- * @param cb_arg    An optional param to pass to the callback
  * @param prefix    A prefix to report in the error message to give the user context
  * @return          status of sending the message
  */
 static int
-ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, ovpn_nl_cb cb,
-                 void *cb_arg, const char *prefix)
+ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, const char *prefix)
 {
     dco->status = 1;
 
-    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, cb_arg);
     nl_send_auto(dco->nl_sock, nl_msg);
 
     while (dco->status == 1)
@@ -285,7 +281,7 @@
     }
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -385,6 +381,29 @@
 }
 
 static void
+ovpn_dco_register(dco_context_t *dco)
+{
+    msg(D_DCO_DEBUG, __func__);
+    ovpn_get_mcast_id(dco);
+
+    if (dco->ovpn_dco_mcast_id < 0)
+    {
+        msg(M_FATAL, "cannot get mcast group: %s",  nl_geterror(dco->ovpn_dco_mcast_id));
+    }
+
+    /* Register for ovpn-dco specific multicast messages that the kernel may
+     * send
+     */
+    int ret = nl_socket_add_membership(dco->nl_sock, dco->ovpn_dco_mcast_id);
+    if (ret)
+    {
+        msg(M_FATAL, "%s: failed to join groups: %d", __func__, ret);
+    }
+}
+
+static int ovpn_handle_msg(struct nl_msg *msg, void *arg);
+
+static void
 ovpn_dco_init_netlink(dco_context_t *dco)
 {
     dco->ovpn_dco_id = resolve_ovpn_netlink_id(M_FATAL);
@@ -420,11 +439,15 @@
 
     nl_socket_set_cb(dco->nl_sock, dco->nl_cb);
 
+    dco->dco_message_peer_id = -1;
     nl_cb_err(dco->nl_cb, NL_CB_CUSTOM, ovpn_nl_cb_error, &dco->status);
     nl_cb_set(dco->nl_cb, NL_CB_FINISH, NL_CB_CUSTOM, ovpn_nl_cb_finish,
               &dco->status);
     nl_cb_set(dco->nl_cb, NL_CB_ACK, NL_CB_CUSTOM, ovpn_nl_cb_finish,
               &dco->status);
+    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco);
+
+    ovpn_dco_register(dco);
 
     /* The async PACKET messages confuse libnl and it will drop them with
      * wrong sequence numbers (NLE_SEQ_MISMATCH), so disable libnl's sequence
@@ -476,27 +499,6 @@
     CLEAR(dco);
 }
 
-static void
-ovpn_dco_register(dco_context_t *dco)
-{
-    msg(D_DCO_DEBUG, __func__);
-    ovpn_get_mcast_id(dco);
-
-    if (dco->ovpn_dco_mcast_id < 0)
-    {
-        msg(M_FATAL, "cannot get mcast group: %s",  nl_geterror(dco->ovpn_dco_mcast_id));
-    }
-
-    /* Register for ovpn-dco specific multicast messages that the kernel may
-     * send
-     */
-    int ret = nl_socket_add_membership(dco->nl_sock, dco->ovpn_dco_mcast_id);
-    if (ret)
-    {
-        msg(M_FATAL, "%s: failed to join groups: %d", __func__, ret);
-    }
-}
-
 int
 open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev)
 {
@@ -516,10 +518,6 @@
         msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
     }
 
-    tt->dco.dco_message_peer_id = -1;
-
-    ovpn_dco_register(&tt->dco);
-
     return 0;
 }
 
@@ -548,7 +546,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_KEYCONF_PEER_ID, peerid);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -572,7 +570,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peerid);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -598,7 +596,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_KEYCONF_SLOT, slot);
     nla_nest_end(nl_msg, keyconf);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -657,7 +655,7 @@
     nla_nest_end(nl_msg, key_conf);
 
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -686,7 +684,7 @@
                 keepalive_timeout);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -754,7 +752,7 @@
 
     /* Even though 'nlctrl' is a constant, there seem to be no library
      * provided define for it */
-    int ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl");
+    dco->ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl");
 
     struct nl_msg *nl_msg = nlmsg_alloc();
     if (!nl_msg)
@@ -762,12 +760,12 @@
         return -ENOMEM;
     }
 
-    genlmsg_put(nl_msg, 0, 0, ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0);
+    genlmsg_put(nl_msg, 0, 0, dco->ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0);
 
     int ret = -EMSGSIZE;
     NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_FAMILY_NAME);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -879,31 +877,34 @@
 }
 
 static int
-dco_parse_peer_multi(struct nl_msg *msg, void *arg)
+ovpn_handle_peer_multi(dco_context_t *dco, struct nlattr *attrs[])
 {
-    struct nlattr *tb[OVPN_A_MAX + 1];
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
-
     msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
 
-    nla_parse(tb, OVPN_A_MAX, genlmsg_attrdata(gnlh, 0),
-              genlmsg_attrlen(gnlh, 0), NULL);
+    /* this function assumes openvpn is running in multipeer mode as
+     * it accesses c->multi
+     */
+    if (dco->ifmode != OVPN_MODE_MP)
+    {
+        msg(M_WARN, "%s: can't parse 'multi-peer' message on P2P instance", __func__);
+        return NL_SKIP;
+    }
 
-    if (!tb[OVPN_A_PEER])
+    if (!attrs[OVPN_A_PEER])
     {
         return NL_SKIP;
     }
 
     struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1];
-    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, tb[OVPN_A_PEER], NULL);
+    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL);
 
     if (!tb_peer[OVPN_A_PEER_ID])
     {
-        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        msg(M_WARN, "ovpn-dco: no peer-id provided in (MULTI) PEER_GET reply");
         return NL_SKIP;
     }
 
-    struct multi_context *m = arg;
+    struct multi_context *m = dco->c->multi;
     uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]);
 
     if (peer_id >= m->max_clients || !m->instances[peer_id])
@@ -919,39 +920,53 @@
 }
 
 static int
-dco_parse_peer(struct nl_msg *msg, void *arg)
+ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
 {
-    struct context *c = arg;
-    struct nlattr *tb[OVPN_A_MAX + 1];
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
-
     msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
 
-    nla_parse(tb, OVPN_A_MAX, genlmsg_attrdata(gnlh, 0),
-              genlmsg_attrlen(gnlh, 0), NULL);
-
-    if (!tb[OVPN_A_PEER])
+    if (!attrs[OVPN_A_PEER])
     {
         msg(D_DCO_DEBUG, "%s: malformed reply", __func__);
         return NL_SKIP;
     }
 
     struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1];
-    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, tb[OVPN_A_PEER], NULL);
+    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL);
 
     if (!tb_peer[OVPN_A_PEER_ID])
     {
-        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        msg(M_WARN, "ovpn-dco: no peer-id provided in PEER_GET reply");
         return NL_SKIP;
     }
 
     uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]);
-    if (c->c2.tls_multi->dco_peer_id != peer_id)
+    struct context_2 *c2;
+
+    if (dco->ifmode == OVPN_MODE_P2P)
+    {
+        c2 = &dco->c->c2;
+    }
+    else
+    {
+        struct multi_instance *mi = dco->c->multi->instances[peer_id];
+        if (!mi)
+        {
+            msg(M_WARN, "%s: received data for a non-existing peer %u", __func__, peer_id);
+            return NL_SKIP;
+        }
+
+        c2 = &mi->context.c2;
+    }
+
+    /* at this point this check should never fail for MP mode,
+     * but it's still fully valid for P2P mode
+     */
+    if (c2->tls_multi->dco_peer_id != peer_id)
     {
         return NL_SKIP;
     }
 
-    dco_update_peer_stat(&c->c2, tb_peer, peer_id);
+    dco_update_peer_stat(c2, tb_peer, peer_id);
 
     return NL_OK;
 }
@@ -1120,9 +1135,22 @@
 {
     dco_context_t *dco = arg;
 
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
     struct nlattr *attrs[OVPN_A_MAX + 1];
     struct nlmsghdr *nlh = nlmsg_hdr(msg);
+    struct genlmsghdr *gnlh = genlmsg_hdr(nlh);
+
+    msg(D_DCO_DEBUG, "ovpn-dco: received netlink message type=%u cmd=%u flags=%#.4x",
+        nlh->nlmsg_type, gnlh->cmd, nlh->nlmsg_flags);
+
+    /* if we get a message from the NLCTRL family, it means
+     * this is the reply to the mcast ID resolution request
+     * and we parse it accordingly.
+     */
+    if (nlh->nlmsg_type == dco->ctrlid)
+    {
+        msg(D_DCO_DEBUG, "ovpn-dco: received CTRLID message");
+        return mcast_family_handler(msg, dco);
+    }
 
     if (!genlmsg_valid_hdr(nlh, 0))
     {
@@ -1146,6 +1174,21 @@
      */
     switch (gnlh->cmd)
     {
+        case OVPN_CMD_PEER_GET:
+        {
+            /* this message is part of a peer list dump, hence triggered
+             * by a MP/server instance
+             */
+            if (nlh->nlmsg_flags & NLM_F_MULTI)
+            {
+                return ovpn_handle_peer_multi(dco, attrs);
+            }
+            else
+            {
+                return ovpn_handle_peer(dco, attrs);
+            }
+        }
+
         case OVPN_CMD_PEER_DEL_NTF:
         {
             return ovpn_handle_peer_del_ntf(dco, attrs);
@@ -1174,7 +1217,6 @@
 dco_do_read(dco_context_t *dco)
 {
     msg(D_DCO_DEBUG, __func__);
-    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco);
 
     return ovpn_nl_recvmsgs(dco, __func__);
 }
@@ -1189,7 +1231,7 @@
 
     nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP;
 
-    int ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer_multi, m, __func__);
+    int ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
     nlmsg_free(nl_msg);
 
@@ -1227,7 +1269,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer, c, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
diff --git a/src/openvpn/dco_linux.h b/src/openvpn/dco_linux.h
index 5e61cf1..cc14f45 100644
--- a/src/openvpn/dco_linux.h
+++ b/src/openvpn/dco_linux.h
@@ -66,6 +66,7 @@
     int status;
 
     struct context *c;
+    int ctrlid;
 
     enum ovpn_mode ifmode;
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1100?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: I23ad79e14844aefde9ece34dadef0b75ff267201
Gerrit-Change-Number: 1100
Gerrit-PatchSet: 6
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

[Openvpn-devel] [M] Change in openvpn[master]: dco_linux: fix async message reception

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

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

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


Change subject: dco_linux: fix async message reception
......................................................................

dco_linux: fix async message reception

Currently whenever we send a PEER_GET request to ovpn, we also
set the CB that is supposed to parse the reply.

However, due to the async nature of netlink messages, we could
get an unrelated notification, sent by ovpn (kernel) upon some event,
after userland has set the CB, but before parsing the awaited reply.

When this happens, the notification is then parsed with the
configured CB instead of the notification parser, thus effectively
rejecting the notification and losing the event.

To fix this inconsistency, make ovpn_handle_msg() the default and
only netlink parser CB. It is configured upon DCO initialization
and is never removed.

ovpn_handle_msg() will check the message type and will call the
corresponding handler. This way, no matter what message we get at
what time, we'll always parse it correctly.

As a bonus we can also simplify the nl_sendmsg() API as we
don't need to pass the cb and its argument anymore.

The ID of the NLCTRL family is now also stored in the DCO
context as we need it to check when we receive a mcast ID
lookup message.

Change-Id: I23ad79e14844aefde9ece34dadef0b75ff267201
Github: closes OpenVPN/openvpn#793
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.../msg32339.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
M src/openvpn/dco_linux.h
2 files changed, 110 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/00/1100/6

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 7c639d9..728fb7e 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -167,23 +167,19 @@
 }
 
 /**
- * Send a prepared netlink message and registers cb as callback if non-null.
+ * Send a prepared netlink message.
  *
  * The method will also free nl_msg
  * @param dco       The dco context to use
  * @param nl_msg    the message to use
- * @param cb        An optional callback if the caller expects an answer
- * @param cb_arg    An optional param to pass to the callback
  * @param prefix    A prefix to report in the error message to give the user context
  * @return          status of sending the message
  */
 static int
-ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, ovpn_nl_cb cb,
-                 void *cb_arg, const char *prefix)
+ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, const char *prefix)
 {
     dco->status = 1;
 
-    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, cb_arg);
     nl_send_auto(dco->nl_sock, nl_msg);
 
     while (dco->status == 1)
@@ -285,7 +281,7 @@
     }
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -385,6 +381,29 @@
 }
 
 static void
+ovpn_dco_register(dco_context_t *dco)
+{
+    msg(D_DCO_DEBUG, __func__);
+    ovpn_get_mcast_id(dco);
+
+    if (dco->ovpn_dco_mcast_id < 0)
+    {
+        msg(M_FATAL, "cannot get mcast group: %s",  nl_geterror(dco->ovpn_dco_mcast_id));
+    }
+
+    /* Register for ovpn-dco specific multicast messages that the kernel may
+     * send
+     */
+    int ret = nl_socket_add_membership(dco->nl_sock, dco->ovpn_dco_mcast_id);
+    if (ret)
+    {
+        msg(M_FATAL, "%s: failed to join groups: %d", __func__, ret);
+    }
+}
+
+static int ovpn_handle_msg(struct nl_msg *msg, void *arg);
+
+static void
 ovpn_dco_init_netlink(dco_context_t *dco)
 {
     dco->ovpn_dco_id = resolve_ovpn_netlink_id(M_FATAL);
@@ -420,11 +439,15 @@
 
     nl_socket_set_cb(dco->nl_sock, dco->nl_cb);
 
+    dco->dco_message_peer_id = -1;
     nl_cb_err(dco->nl_cb, NL_CB_CUSTOM, ovpn_nl_cb_error, &dco->status);
     nl_cb_set(dco->nl_cb, NL_CB_FINISH, NL_CB_CUSTOM, ovpn_nl_cb_finish,
               &dco->status);
     nl_cb_set(dco->nl_cb, NL_CB_ACK, NL_CB_CUSTOM, ovpn_nl_cb_finish,
               &dco->status);
+    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco);
+
+    ovpn_dco_register(dco);
 
     /* The async PACKET messages confuse libnl and it will drop them with
      * wrong sequence numbers (NLE_SEQ_MISMATCH), so disable libnl's sequence
@@ -476,27 +499,6 @@
     CLEAR(dco);
 }
 
-static void
-ovpn_dco_register(dco_context_t *dco)
-{
-    msg(D_DCO_DEBUG, __func__);
-    ovpn_get_mcast_id(dco);
-
-    if (dco->ovpn_dco_mcast_id < 0)
-    {
-        msg(M_FATAL, "cannot get mcast group: %s",  nl_geterror(dco->ovpn_dco_mcast_id));
-    }
-
-    /* Register for ovpn-dco specific multicast messages that the kernel may
-     * send
-     */
-    int ret = nl_socket_add_membership(dco->nl_sock, dco->ovpn_dco_mcast_id);
-    if (ret)
-    {
-        msg(M_FATAL, "%s: failed to join groups: %d", __func__, ret);
-    }
-}
-
 int
 open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev)
 {
@@ -516,10 +518,6 @@
         msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
     }
 
-    tt->dco.dco_message_peer_id = -1;
-
-    ovpn_dco_register(&tt->dco);
-
     return 0;
 }
 
@@ -548,7 +546,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_KEYCONF_PEER_ID, peerid);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -572,7 +570,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peerid);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -598,7 +596,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_KEYCONF_SLOT, slot);
     nla_nest_end(nl_msg, keyconf);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -657,7 +655,7 @@
     nla_nest_end(nl_msg, key_conf);
 
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -686,7 +684,7 @@
                 keepalive_timeout);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -754,7 +752,7 @@
 
     /* Even though 'nlctrl' is a constant, there seem to be no library
      * provided define for it */
-    int ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl");
+    dco->ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl");
 
     struct nl_msg *nl_msg = nlmsg_alloc();
     if (!nl_msg)
@@ -762,12 +760,12 @@
         return -ENOMEM;
     }
 
-    genlmsg_put(nl_msg, 0, 0, ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0);
+    genlmsg_put(nl_msg, 0, 0, dco->ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0);
 
     int ret = -EMSGSIZE;
     NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_FAMILY_NAME);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -879,31 +877,34 @@
 }
 
 static int
-dco_parse_peer_multi(struct nl_msg *msg, void *arg)
+ovpn_handle_peer_multi(dco_context_t *dco, struct nlattr *attrs[])
 {
-    struct nlattr *tb[OVPN_A_MAX + 1];
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
-
     msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
 
-    nla_parse(tb, OVPN_A_MAX, genlmsg_attrdata(gnlh, 0),
-              genlmsg_attrlen(gnlh, 0), NULL);
+    /* this function assumes openvpn is running in multipeer mode as
+     * it accesses c->multi
+     */
+    if (dco->ifmode != OVPN_MODE_MP)
+    {
+        msg(M_WARN, "%s: can't parse 'multi-peer' message on P2P instance", __func__);
+        return NL_SKIP;
+    }
 
-    if (!tb[OVPN_A_PEER])
+    if (!attrs[OVPN_A_PEER])
     {
         return NL_SKIP;
     }
 
     struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1];
-    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, tb[OVPN_A_PEER], NULL);
+    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL);
 
     if (!tb_peer[OVPN_A_PEER_ID])
     {
-        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        msg(M_WARN, "ovpn-dco: no peer-id provided in (MULTI) PEER_GET reply");
         return NL_SKIP;
     }
 
-    struct multi_context *m = arg;
+    struct multi_context *m = dco->c->multi;
     uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]);
 
     if (peer_id >= m->max_clients || !m->instances[peer_id])
@@ -919,39 +920,53 @@
 }
 
 static int
-dco_parse_peer(struct nl_msg *msg, void *arg)
+ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
 {
-    struct context *c = arg;
-    struct nlattr *tb[OVPN_A_MAX + 1];
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
-
     msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
 
-    nla_parse(tb, OVPN_A_MAX, genlmsg_attrdata(gnlh, 0),
-              genlmsg_attrlen(gnlh, 0), NULL);
-
-    if (!tb[OVPN_A_PEER])
+    if (!attrs[OVPN_A_PEER])
     {
         msg(D_DCO_DEBUG, "%s: malformed reply", __func__);
         return NL_SKIP;
     }
 
     struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1];
-    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, tb[OVPN_A_PEER], NULL);
+    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL);
 
     if (!tb_peer[OVPN_A_PEER_ID])
     {
-        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        msg(M_WARN, "ovpn-dco: no peer-id provided in PEER_GET reply");
         return NL_SKIP;
     }
 
     uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]);
-    if (c->c2.tls_multi->dco_peer_id != peer_id)
+    struct context_2 *c2;
+
+    if (dco->ifmode == OVPN_MODE_P2P)
+    {
+        c2 = &dco->c->c2;
+    }
+    else
+    {
+        struct multi_instance *mi = dco->c->multi->instances[peer_id];
+        if (!mi)
+        {
+            msg(M_WARN, "%s: received data for a non-existing peer %u", __func__, peer_id);
+            return NL_SKIP;
+        }
+
+        c2 = &mi->context.c2;
+    }
+
+    /* at this point this check should never fail for MP mode,
+     * but it's still fully valid for P2P mode
+     */
+    if (c2->tls_multi->dco_peer_id != peer_id)
     {
         return NL_SKIP;
     }
 
-    dco_update_peer_stat(&c->c2, tb_peer, peer_id);
+    dco_update_peer_stat(c2, tb_peer, peer_id);
 
     return NL_OK;
 }
@@ -1120,9 +1135,22 @@
 {
     dco_context_t *dco = arg;
 
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
     struct nlattr *attrs[OVPN_A_MAX + 1];
     struct nlmsghdr *nlh = nlmsg_hdr(msg);
+    struct genlmsghdr *gnlh = genlmsg_hdr(nlh);
+
+    msg(D_DCO_DEBUG, "ovpn-dco: received netlink message type=%u cmd=%u flags=%#.4x",
+        nlh->nlmsg_type, gnlh->cmd, nlh->nlmsg_flags);
+
+    /* if we get a message from the NLCTRL family, it means
+     * this is the reply to the mcast ID resolution request
+     * and we parse it accordingly.
+     */
+    if (nlh->nlmsg_type == dco->ctrlid)
+    {
+        msg(D_DCO_DEBUG, "ovpn-dco: received CTRLID message");
+        return mcast_family_handler(msg, dco);
+    }
 
     if (!genlmsg_valid_hdr(nlh, 0))
     {
@@ -1146,6 +1174,21 @@
      */
     switch (gnlh->cmd)
     {
+        case OVPN_CMD_PEER_GET:
+        {
+            /* this message is part of a peer list dump, hence triggered
+             * by a MP/server instance
+             */
+            if (nlh->nlmsg_flags & NLM_F_MULTI)
+            {
+                return ovpn_handle_peer_multi(dco, attrs);
+            }
+            else
+            {
+                return ovpn_handle_peer(dco, attrs);
+            }
+        }
+
         case OVPN_CMD_PEER_DEL_NTF:
         {
             return ovpn_handle_peer_del_ntf(dco, attrs);
@@ -1174,7 +1217,6 @@
 dco_do_read(dco_context_t *dco)
 {
     msg(D_DCO_DEBUG, __func__);
-    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco);
 
     return ovpn_nl_recvmsgs(dco, __func__);
 }
@@ -1189,7 +1231,7 @@
 
     nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP;
 
-    int ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer_multi, m, __func__);
+    int ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
     nlmsg_free(nl_msg);
 
@@ -1227,7 +1269,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer, c, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
diff --git a/src/openvpn/dco_linux.h b/src/openvpn/dco_linux.h
index 5e61cf1..cc14f45 100644
--- a/src/openvpn/dco_linux.h
+++ b/src/openvpn/dco_linux.h
@@ -66,6 +66,7 @@
     int status;
 
     struct context *c;
+    int ctrlid;
 
     enum ovpn_mode ifmode;
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1100?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: I23ad79e14844aefde9ece34dadef0b75ff267201
Gerrit-Change-Number: 1100
Gerrit-PatchSet: 6
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] [PATCH applied] Re: dco_linux: fix async message reception

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

Thanks for that work.

I tested all the "easy" notifications (DEL_PEER and FLOAT), and can
confirm that the race/mixup we observed in gerrit#1086 (issue #793) is
now fixed for good - with sufficient patience it's still possible to
inject a FLOAT message between "ask for peer stats" and "receive peer
stats", but the new dispatching logic handles things just fine, we're
no longer losing FLOAT messages, and life is good.

I did not test KEY_SWAP_NTF as that's not easy to trigger (without kernel
changes to lower the threshold) but as that code is not affected at all
by this commit I do not expect any surprises there.

Besides the manual test, this handled the DCO client/server test runs
just fine.

There's some suggestions to the code I still have, but "it works,
ship it" (so we get more exposure to the code we considered fixed) - the
cleanup can be a separate patch.

Your patch has been applied to the master branch.

commit f353b7100c859a02e70723c998594c3efd83c419
Author: Antonio Quartulli
Date:   Fri Jul 25 19:27:02 2025 +0200

     dco_linux: fix async message reception

     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.../msg32339.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v5] dco_linux: fix async message reception

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

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

Currently whenever we send a PEER_GET request to ovpn, we also
set the CB that is supposed to parse the reply.

However, due to the async nature of netlink messages, we could
get an unrelated notification, sent by ovpn upon some event,
after having set the CB, but before parsing the awaited reply.

When this happens, the notification is then parsed with the
configured CB instead of the notification parser, thus effectively
rejecting the notification and losing the event.

To fix this inconsistency, make ovpn_handle_msg() the default and
only netlink parser CB. It is configured upon DCO initialization
and is never removed.

ovpn_handle_msg() will check the message type and will call the
according parser. This way, no matter what message we get at
what time, but we'll always parse it correctly.

As a bonus we can also simplify the nl_sendmsg() API as we
don't need to pass the cb and its argument anymore.

The ID of the NLCTRL family is now also stored in the DCO
context as we need it to check when we receive a mcast ID
lookup message.

Change-Id: I23ad79e14844aefde9ece34dadef0b75ff267201
Github: OpenVPN/openvpn#793
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/+/1100
This mail reflects revision 5 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 7c639d9..728fb7e 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -167,23 +167,19 @@
 }
 
 /**
- * Send a prepared netlink message and registers cb as callback if non-null.
+ * Send a prepared netlink message.
  *
  * The method will also free nl_msg
  * @param dco       The dco context to use
  * @param nl_msg    the message to use
- * @param cb        An optional callback if the caller expects an answer
- * @param cb_arg    An optional param to pass to the callback
  * @param prefix    A prefix to report in the error message to give the user context
  * @return          status of sending the message
  */
 static int
-ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, ovpn_nl_cb cb,
-                 void *cb_arg, const char *prefix)
+ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, const char *prefix)
 {
     dco->status = 1;
 
-    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, cb_arg);
     nl_send_auto(dco->nl_sock, nl_msg);
 
     while (dco->status == 1)
@@ -285,7 +281,7 @@
     }
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -385,6 +381,29 @@
 }
 
 static void
+ovpn_dco_register(dco_context_t *dco)
+{
+    msg(D_DCO_DEBUG, __func__);
+    ovpn_get_mcast_id(dco);
+
+    if (dco->ovpn_dco_mcast_id < 0)
+    {
+        msg(M_FATAL, "cannot get mcast group: %s",  nl_geterror(dco->ovpn_dco_mcast_id));
+    }
+
+    /* Register for ovpn-dco specific multicast messages that the kernel may
+     * send
+     */
+    int ret = nl_socket_add_membership(dco->nl_sock, dco->ovpn_dco_mcast_id);
+    if (ret)
+    {
+        msg(M_FATAL, "%s: failed to join groups: %d", __func__, ret);
+    }
+}
+
+static int ovpn_handle_msg(struct nl_msg *msg, void *arg);
+
+static void
 ovpn_dco_init_netlink(dco_context_t *dco)
 {
     dco->ovpn_dco_id = resolve_ovpn_netlink_id(M_FATAL);
@@ -420,11 +439,15 @@
 
     nl_socket_set_cb(dco->nl_sock, dco->nl_cb);
 
+    dco->dco_message_peer_id = -1;
     nl_cb_err(dco->nl_cb, NL_CB_CUSTOM, ovpn_nl_cb_error, &dco->status);
     nl_cb_set(dco->nl_cb, NL_CB_FINISH, NL_CB_CUSTOM, ovpn_nl_cb_finish,
               &dco->status);
     nl_cb_set(dco->nl_cb, NL_CB_ACK, NL_CB_CUSTOM, ovpn_nl_cb_finish,
               &dco->status);
+    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco);
+
+    ovpn_dco_register(dco);
 
     /* The async PACKET messages confuse libnl and it will drop them with
      * wrong sequence numbers (NLE_SEQ_MISMATCH), so disable libnl's sequence
@@ -476,27 +499,6 @@
     CLEAR(dco);
 }
 
-static void
-ovpn_dco_register(dco_context_t *dco)
-{
-    msg(D_DCO_DEBUG, __func__);
-    ovpn_get_mcast_id(dco);
-
-    if (dco->ovpn_dco_mcast_id < 0)
-    {
-        msg(M_FATAL, "cannot get mcast group: %s",  nl_geterror(dco->ovpn_dco_mcast_id));
-    }
-
-    /* Register for ovpn-dco specific multicast messages that the kernel may
-     * send
-     */
-    int ret = nl_socket_add_membership(dco->nl_sock, dco->ovpn_dco_mcast_id);
-    if (ret)
-    {
-        msg(M_FATAL, "%s: failed to join groups: %d", __func__, ret);
-    }
-}
-
 int
 open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev)
 {
@@ -516,10 +518,6 @@
         msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
     }
 
-    tt->dco.dco_message_peer_id = -1;
-
-    ovpn_dco_register(&tt->dco);
-
     return 0;
 }
 
@@ -548,7 +546,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_KEYCONF_PEER_ID, peerid);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -572,7 +570,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peerid);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -598,7 +596,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_KEYCONF_SLOT, slot);
     nla_nest_end(nl_msg, keyconf);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -657,7 +655,7 @@
     nla_nest_end(nl_msg, key_conf);
 
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -686,7 +684,7 @@
                 keepalive_timeout);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -754,7 +752,7 @@
 
     /* Even though 'nlctrl' is a constant, there seem to be no library
      * provided define for it */
-    int ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl");
+    dco->ctrlid = genl_ctrl_resolve(dco->nl_sock, "nlctrl");
 
     struct nl_msg *nl_msg = nlmsg_alloc();
     if (!nl_msg)
@@ -762,12 +760,12 @@
         return -ENOMEM;
     }
 
-    genlmsg_put(nl_msg, 0, 0, ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0);
+    genlmsg_put(nl_msg, 0, 0, dco->ctrlid, 0, 0, CTRL_CMD_GETFAMILY, 0);
 
     int ret = -EMSGSIZE;
     NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_FAMILY_NAME);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
@@ -879,31 +877,34 @@
 }
 
 static int
-dco_parse_peer_multi(struct nl_msg *msg, void *arg)
+ovpn_handle_peer_multi(dco_context_t *dco, struct nlattr *attrs[])
 {
-    struct nlattr *tb[OVPN_A_MAX + 1];
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
-
     msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
 
-    nla_parse(tb, OVPN_A_MAX, genlmsg_attrdata(gnlh, 0),
-              genlmsg_attrlen(gnlh, 0), NULL);
+    /* this function assumes openvpn is running in multipeer mode as
+     * it accesses c->multi
+     */
+    if (dco->ifmode != OVPN_MODE_MP)
+    {
+        msg(M_WARN, "%s: can't parse 'multi-peer' message on P2P instance", __func__);
+        return NL_SKIP;
+    }
 
-    if (!tb[OVPN_A_PEER])
+    if (!attrs[OVPN_A_PEER])
     {
         return NL_SKIP;
     }
 
     struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1];
-    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, tb[OVPN_A_PEER], NULL);
+    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL);
 
     if (!tb_peer[OVPN_A_PEER_ID])
     {
-        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        msg(M_WARN, "ovpn-dco: no peer-id provided in (MULTI) PEER_GET reply");
         return NL_SKIP;
     }
 
-    struct multi_context *m = arg;
+    struct multi_context *m = dco->c->multi;
     uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]);
 
     if (peer_id >= m->max_clients || !m->instances[peer_id])
@@ -919,39 +920,53 @@
 }
 
 static int
-dco_parse_peer(struct nl_msg *msg, void *arg)
+ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[])
 {
-    struct context *c = arg;
-    struct nlattr *tb[OVPN_A_MAX + 1];
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
-
     msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
 
-    nla_parse(tb, OVPN_A_MAX, genlmsg_attrdata(gnlh, 0),
-              genlmsg_attrlen(gnlh, 0), NULL);
-
-    if (!tb[OVPN_A_PEER])
+    if (!attrs[OVPN_A_PEER])
     {
         msg(D_DCO_DEBUG, "%s: malformed reply", __func__);
         return NL_SKIP;
     }
 
     struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1];
-    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, tb[OVPN_A_PEER], NULL);
+    nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL);
 
     if (!tb_peer[OVPN_A_PEER_ID])
     {
-        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        msg(M_WARN, "ovpn-dco: no peer-id provided in PEER_GET reply");
         return NL_SKIP;
     }
 
     uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]);
-    if (c->c2.tls_multi->dco_peer_id != peer_id)
+    struct context_2 *c2;
+
+    if (dco->ifmode == OVPN_MODE_P2P)
+    {
+        c2 = &dco->c->c2;
+    }
+    else
+    {
+        struct multi_instance *mi = dco->c->multi->instances[peer_id];
+        if (!mi)
+        {
+            msg(M_WARN, "%s: received data for a non-existing peer %u", __func__, peer_id);
+            return NL_SKIP;
+        }
+
+        c2 = &mi->context.c2;
+    }
+
+    /* at this point this check should never fail for MP mode,
+     * but it's still fully valid for P2P mode
+     */
+    if (c2->tls_multi->dco_peer_id != peer_id)
     {
         return NL_SKIP;
     }
 
-    dco_update_peer_stat(&c->c2, tb_peer, peer_id);
+    dco_update_peer_stat(c2, tb_peer, peer_id);
 
     return NL_OK;
 }
@@ -1120,9 +1135,22 @@
 {
     dco_context_t *dco = arg;
 
-    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
     struct nlattr *attrs[OVPN_A_MAX + 1];
     struct nlmsghdr *nlh = nlmsg_hdr(msg);
+    struct genlmsghdr *gnlh = genlmsg_hdr(nlh);
+
+    msg(D_DCO_DEBUG, "ovpn-dco: received netlink message type=%u cmd=%u flags=%#.4x",
+        nlh->nlmsg_type, gnlh->cmd, nlh->nlmsg_flags);
+
+    /* if we get a message from the NLCTRL family, it means
+     * this is the reply to the mcast ID resolution request
+     * and we parse it accordingly.
+     */
+    if (nlh->nlmsg_type == dco->ctrlid)
+    {
+        msg(D_DCO_DEBUG, "ovpn-dco: received CTRLID message");
+        return mcast_family_handler(msg, dco);
+    }
 
     if (!genlmsg_valid_hdr(nlh, 0))
     {
@@ -1146,6 +1174,21 @@
      */
     switch (gnlh->cmd)
     {
+        case OVPN_CMD_PEER_GET:
+        {
+            /* this message is part of a peer list dump, hence triggered
+             * by a MP/server instance
+             */
+            if (nlh->nlmsg_flags & NLM_F_MULTI)
+            {
+                return ovpn_handle_peer_multi(dco, attrs);
+            }
+            else
+            {
+                return ovpn_handle_peer(dco, attrs);
+            }
+        }
+
         case OVPN_CMD_PEER_DEL_NTF:
         {
             return ovpn_handle_peer_del_ntf(dco, attrs);
@@ -1174,7 +1217,6 @@
 dco_do_read(dco_context_t *dco)
 {
     msg(D_DCO_DEBUG, __func__);
-    nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, ovpn_handle_msg, dco);
 
     return ovpn_nl_recvmsgs(dco, __func__);
 }
@@ -1189,7 +1231,7 @@
 
     nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP;
 
-    int ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer_multi, m, __func__);
+    int ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
     nlmsg_free(nl_msg);
 
@@ -1227,7 +1269,7 @@
     NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id);
     nla_nest_end(nl_msg, attr);
 
-    ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer, c, __func__);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
 
 nla_put_failure:
     nlmsg_free(nl_msg);
diff --git a/src/openvpn/dco_linux.h b/src/openvpn/dco_linux.h
index 5e61cf1..cc14f45 100644
--- a/src/openvpn/dco_linux.h
+++ b/src/openvpn/dco_linux.h
@@ -66,6 +66,7 @@
     int status;
 
     struct context *c;
+    int ctrlid;
 
     enum ovpn_mode ifmode;
 

[Openvpn-devel] [M] Change in openvpn[master]: dco_linux: fix async message reception

[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/+/1100?usp=email )

Change subject: dco_linux: fix async message reception
......................................................................


Patch Set 5: Code-Review+2

(1 comment)

Patchset:

PS5: 
Hah, gotcha

```
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_get_peer_stats_multi
Jul 25 19:18:56 ubuntu2004 tun-udp-p2p-tls-sha256[186220]: dco_do_read
Jul 25 19:18:56 ubuntu2004 tun-udp-p2p-tls-sha256[186220]: ovpn-dco: received netlink message type=31 cmd=11 flags=0000
Jul 25 19:18:56 ubuntu2004 tun-udp-p2p-tls-sha256[186220]: ovpn-dco: ignoring message for foreign ifindex 982
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: ovpn-dco: received netlink message type=31 cmd=11 flags=0000
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: ovpn-dco: received CMD_PEER_FLOAT_NTF, ifindex: 982, peer-id 2, address: [AF_INET]193.149.48.173:59979
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: ovpn-dco: received netlink message type=31 cmd=3 flags=0x0002
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: ovpn_handle_peer_multi: parsing message...
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / dco_read_bytes(0): 615704
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / dco_write_bytes(0): 625840
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / tun_read_bytes(0): 562992
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / tun_write_bytes(0): 568872
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: ovpn-dco: received netlink message type=31 cmd=3 flags=0x0002
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: ovpn_handle_peer_multi: parsing message...
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / dco_read_bytes(2): 11732
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / dco_write_bytes(2): 11392
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / tun_read_bytes(2): 9256
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_update_peer_stat / tun_write_bytes(2): 9256
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_do_read
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: dco_do_read: netlink reports blocking read - aborting wait
Jul 25 19:18:56 ubuntu2004 tun-udp-p2mp[186147]: peer 2 (cron2-freebsd-tc-amd64) floated from udp6:193.149.48.172:59979 to [AF_INET6]::ffff:193.149.48.173:59979
Jul 25 19:18:57 ubuntu2004 kernel: [294871.134548] tun1: peer 2 floated to 193.149.48.173:59979
```

this is precisely the situation that makes the pre-1100 code lose the "FLOAT!" message, because it arrives *after* `dco_get_peer_stats_multi()` starts running (or just before, but we do not look at DCO events) and before the actual cmd=3 reply comes in.

Good work!



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1100?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: I23ad79e14844aefde9ece34dadef0b75ff267201
Gerrit-Change-Number: 1100
Gerrit-PatchSet: 5
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, 25 Jul 2025 17:20:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: dco_linux: fix async message reception

[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/+/1100?usp=email )

Change subject: dco_linux: fix async message reception
......................................................................


Patch Set 5:

(1 comment)

File src/openvpn/dco_linux.c:

http://gerrit.openvpn.net/c/openvpn/+/1100/comment/acf86e82_8c8c0854 :
PS4, Line 1189:             }
> as mentioned above, MODE_MP may still need to call ovpn_handle_peer()
Now you got me started... if we need to have one function handle both P2P and P2MP modes anyway, why do we need two functions?  They look similar enough that using `ovpn_handle_peer()` for all `OVPN_CMD_PEER_GET` replies might just work (possibly moving the peer-id check inside the P2P clause to avoid having it in the MP path where it's more like an CANTHAPPEN...)?



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1100?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: I23ad79e14844aefde9ece34dadef0b75ff267201
Gerrit-Change-Number: 1100
Gerrit-PatchSet: 5
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, 25 Jul 2025 17:04:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>
Comment-In-Reply-To: ordex <an...@ma...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: management: resync timer on bytecount interval change

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

Attention is currently required from: flichtenheld, plaisthos.

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

Change subject: management: resync timer on bytecount interval change
......................................................................


Patch Set 2:

(1 comment)

Patchset:

PS1: 
> please rebase on current master to fix build issue
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1113?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: Ic0035d52e0ea123398318870d2f4d21af927a602
Gerrit-Change-Number: 1113
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <ra...@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-Comment-Date: Fri, 25 Jul 2025 16:46:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld <fr...@li...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: management: resync timer on bytecount interval change

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

Attention is currently required from: plaisthos, ralf_lici.

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

Change subject: management: resync timer on bytecount interval change
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

Patchset:

PS1: 
please rebase on current master to fix build issue



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1113?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: Ic0035d52e0ea123398318870d2f4d21af927a602
Gerrit-Change-Number: 1113
Gerrit-PatchSet: 1
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: ralf_lici <ra...@ma...>
Gerrit-Comment-Date: Fri, 25 Jul 2025 16:40:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: plugins: Clean up -Wconversion warnings

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

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

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


Change subject: plugins: Clean up -Wconversion warnings
......................................................................

plugins: Clean up -Wconversion warnings

Most of the are actually the same ones copied to every
single plugin.

Some drive-by fixes of other warnings and some
conversion cleanups that had no warnings because
they were suppressed by casts.

Change-Id: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32330.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M sample/sample-plugins/client-connect/sample-client-connect.c
M sample/sample-plugins/defer/multi-auth.c
M sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
M sample/sample-plugins/log/log.c
M sample/sample-plugins/log/log_v3.c
M sample/sample-plugins/simple/base64.c
M sample/sample-plugins/simple/simple.c
M src/plugins/auth-pam/auth-pam.c
M src/plugins/auth-pam/utils.c
M src/plugins/down-root/down-root.c
10 files changed, 42 insertions(+), 54 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/11/1111/2

diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c b/sample/sample-plugins/client-connect/sample-client-connect.c
index 18c2c6f..5194270 100644
--- a/sample/sample-plugins/client-connect/sample-client-connect.c
+++ b/sample/sample-plugins/client-connect/sample-client-connect.c
@@ -86,9 +86,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -309,7 +308,7 @@
 
     /* do mighty complicated work that will really take time here... */
     plugin_log(PLOG_NOTE, MODULE, "in async/deferred handler, sleep(%d)", seconds);
-    sleep(seconds);
+    sleep((unsigned int)seconds);
 
     /* write config options to openvpn */
     int ret = write_cc_options_file(name, envp);
diff --git a/sample/sample-plugins/defer/multi-auth.c b/sample/sample-plugins/defer/multi-auth.c
index 38db07f..c458346 100644
--- a/sample/sample-plugins/defer/multi-auth.c
+++ b/sample/sample-plugins/defer/multi-auth.c
@@ -124,9 +124,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -170,7 +169,7 @@
 
 /* Require a minimum OpenVPN Plugin API */
 OPENVPN_EXPORT int
-openvpn_plugin_min_version_required_v1()
+openvpn_plugin_min_version_required_v1(void)
 {
     return OPENVPN_PLUGIN_VERSION_MIN;
 }
@@ -349,9 +348,9 @@
      */
 
     /* do mighty complicated work that will really take time here... */
-    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%d)",
-         context->test_deferred_auth*1000);
-    usleep(context->test_deferred_auth*1000);
+    useconds_t wait_time = (useconds_t)context->test_deferred_auth*1000;
+    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%u)", wait_time);
+    usleep(wait_time);
 
     /* now signal success state to openvpn */
     int fd = open(auth_control_file, O_WRONLY);
diff --git a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
index cc256dd..f1f8868 100644
--- a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
+++ b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
@@ -69,9 +69,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log.c b/sample/sample-plugins/log/log.c
index 82595cf..b304f16 100644
--- a/sample/sample-plugins/log/log.c
+++ b/sample/sample-plugins/log/log.c
@@ -52,9 +52,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log_v3.c b/sample/sample-plugins/log/log_v3.c
index c90cc3d..a024027 100644
--- a/sample/sample-plugins/log/log_v3.c
+++ b/sample/sample-plugins/log/log_v3.c
@@ -55,9 +55,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/simple/base64.c b/sample/sample-plugins/simple/base64.c
index 6855966..e1cc791 100644
--- a/sample/sample-plugins/simple/base64.c
+++ b/sample/sample-plugins/simple/base64.c
@@ -59,9 +59,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -175,7 +174,7 @@
 
     /* test the BASE64 encode function */
     char *buf = NULL;
-    int r = ovpn_base64_encode(clcert_cn, strlen(clcert_cn), &buf);
+    int r = ovpn_base64_encode(clcert_cn, (int)strlen(clcert_cn), &buf);
     ovpn_log(PLOG_NOTE, PLUGIN_NAME, "BASE64 encoded '%s' (return value %i):  '%s'",
              clcert_cn, r, buf);
 
diff --git a/sample/sample-plugins/simple/simple.c b/sample/sample-plugins/simple/simple.c
index e17f3fa..2c096e2 100644
--- a/sample/sample-plugins/simple/simple.c
+++ b/sample/sample-plugins/simple/simple.c
@@ -54,9 +54,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 8692806..7264f95 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -165,31 +165,30 @@
     }
 }
 
-static int
-recv_string(int fd, char *buffer, int len)
+static ssize_t
+recv_string(int fd, char *buffer, size_t len)
 {
     if (len > 0)
     {
-        ssize_t size;
         memset(buffer, 0, len);
-        size = read(fd, buffer, len);
+        ssize_t size = read(fd, buffer, len);
         buffer[len-1] = 0;
         if (size >= 1)
         {
-            return (int)size;
+            return size;
         }
     }
     return -1;
 }
 
-static int
+static ssize_t
 send_string(int fd, const char *string)
 {
-    const int len = strlen(string) + 1;
+    const size_t len = strlen(string) + 1;
     const ssize_t size = write(fd, string, len);
     if (size == len)
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -645,27 +644,26 @@
  * PAM conversation function
  */
 static int
-my_conv(int n, const struct pam_message **msg_array,
+my_conv(int num_msg, const struct pam_message **msg_array,
         struct pam_response **response_array, void *appdata_ptr)
 {
     const struct user_pass *up = ( const struct user_pass *) appdata_ptr;
     struct pam_response *aresp;
-    int i;
     int ret = PAM_SUCCESS;
 
     *response_array = NULL;
 
-    if (n <= 0 || n > PAM_MAX_NUM_MSG)
+    if (num_msg <= 0 || num_msg > PAM_MAX_NUM_MSG)
     {
         return (PAM_CONV_ERR);
     }
-    if ((aresp = calloc(n, sizeof *aresp)) == NULL)
+    if ((aresp = calloc((size_t)num_msg, sizeof *aresp)) == NULL)
     {
         return (PAM_BUF_ERR);
     }
 
     /* loop through each PAM-module query */
-    for (i = 0; i < n; ++i)
+    for (int i = 0; i < num_msg; ++i)
     {
         const struct pam_message *msg = msg_array[i];
         aresp[i].resp_retcode = 0;
@@ -683,9 +681,9 @@
         {
             /* use name/value list match method */
             const struct name_value_list *list = up->name_value_list;
-            int j;
 
             /* loop through name/value pairs */
+            int j; /* checked after loop */
             for (j = 0; j < list->len; ++j)
             {
                 const char *match_name = list->data[j].name;
diff --git a/src/plugins/auth-pam/utils.c b/src/plugins/auth-pam/utils.c
index a6ccfbf..ab5e17e 100644
--- a/src/plugins/auth-pam/utils.c
+++ b/src/plugins/auth-pam/utils.c
@@ -79,7 +79,7 @@
 
     while (scratch)
     {
-        strncat(temp, searching, scratch-searching);
+        strncat(temp, searching, (size_t)(scratch-searching));
         strcat(temp, replacewith);
 
         searching = scratch+strlen(searchfor);
@@ -93,9 +93,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/down-root/down-root.c b/src/plugins/down-root/down-root.c
index 253adcd..2c0faf4 100644
--- a/src/plugins/down-root/down-root.c
+++ b/src/plugins/down-root/down-root.c
@@ -88,9 +88,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -108,10 +107,10 @@
 /*
  * Return the length of a string array
  */
-static int
+static size_t
 string_array_len(const char *array[])
 {
-    int i = 0;
+    size_t i = 0;
     if (array)
     {
         while (array[i])
@@ -141,14 +140,14 @@
     }
 }
 
-static int
+static ssize_t
 send_control(int fd, int code)
 {
     unsigned char c = (unsigned char) code;
     const ssize_t size = write(fd, &c, sizeof(c));
     if (size == sizeof(c))
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -281,7 +280,6 @@
 openvpn_plugin_open_v1(unsigned int *type_mask, const char *argv[], const char *envp[])
 {
     struct down_root_context *context;
-    int i = 0;
 
     /*
      * Allocate our context
@@ -320,7 +318,7 @@
     }
 
     /* Ignore argv[0], as it contains just the plug-in file name */
-    for (i = 1; i < string_array_len(argv); i++)
+    for (int i = 1; i < string_array_len(argv); i++)
     {
         context->command[i-1] = (char *) argv[i];
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1111?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: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Gerrit-Change-Number: 1111
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [M] Change in openvpn[master]: plugins: Clean up -Wconversion warnings

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

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

Change subject: plugins: Clean up -Wconversion warnings
......................................................................

plugins: Clean up -Wconversion warnings

Most of the are actually the same ones copied to every
single plugin.

Some drive-by fixes of other warnings and some
conversion cleanups that had no warnings because
they were suppressed by casts.

Change-Id: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32330.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M sample/sample-plugins/client-connect/sample-client-connect.c
M sample/sample-plugins/defer/multi-auth.c
M sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
M sample/sample-plugins/log/log.c
M sample/sample-plugins/log/log_v3.c
M sample/sample-plugins/simple/base64.c
M sample/sample-plugins/simple/simple.c
M src/plugins/auth-pam/auth-pam.c
M src/plugins/auth-pam/utils.c
M src/plugins/down-root/down-root.c
10 files changed, 42 insertions(+), 54 deletions(-)




diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c b/sample/sample-plugins/client-connect/sample-client-connect.c
index 18c2c6f..5194270 100644
--- a/sample/sample-plugins/client-connect/sample-client-connect.c
+++ b/sample/sample-plugins/client-connect/sample-client-connect.c
@@ -86,9 +86,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -309,7 +308,7 @@
 
     /* do mighty complicated work that will really take time here... */
     plugin_log(PLOG_NOTE, MODULE, "in async/deferred handler, sleep(%d)", seconds);
-    sleep(seconds);
+    sleep((unsigned int)seconds);
 
     /* write config options to openvpn */
     int ret = write_cc_options_file(name, envp);
diff --git a/sample/sample-plugins/defer/multi-auth.c b/sample/sample-plugins/defer/multi-auth.c
index 38db07f..c458346 100644
--- a/sample/sample-plugins/defer/multi-auth.c
+++ b/sample/sample-plugins/defer/multi-auth.c
@@ -124,9 +124,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -170,7 +169,7 @@
 
 /* Require a minimum OpenVPN Plugin API */
 OPENVPN_EXPORT int
-openvpn_plugin_min_version_required_v1()
+openvpn_plugin_min_version_required_v1(void)
 {
     return OPENVPN_PLUGIN_VERSION_MIN;
 }
@@ -349,9 +348,9 @@
      */
 
     /* do mighty complicated work that will really take time here... */
-    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%d)",
-         context->test_deferred_auth*1000);
-    usleep(context->test_deferred_auth*1000);
+    useconds_t wait_time = (useconds_t)context->test_deferred_auth*1000;
+    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%u)", wait_time);
+    usleep(wait_time);
 
     /* now signal success state to openvpn */
     int fd = open(auth_control_file, O_WRONLY);
diff --git a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
index cc256dd..f1f8868 100644
--- a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
+++ b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
@@ -69,9 +69,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log.c b/sample/sample-plugins/log/log.c
index 82595cf..b304f16 100644
--- a/sample/sample-plugins/log/log.c
+++ b/sample/sample-plugins/log/log.c
@@ -52,9 +52,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log_v3.c b/sample/sample-plugins/log/log_v3.c
index c90cc3d..a024027 100644
--- a/sample/sample-plugins/log/log_v3.c
+++ b/sample/sample-plugins/log/log_v3.c
@@ -55,9 +55,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/simple/base64.c b/sample/sample-plugins/simple/base64.c
index 6855966..e1cc791 100644
--- a/sample/sample-plugins/simple/base64.c
+++ b/sample/sample-plugins/simple/base64.c
@@ -59,9 +59,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -175,7 +174,7 @@
 
     /* test the BASE64 encode function */
     char *buf = NULL;
-    int r = ovpn_base64_encode(clcert_cn, strlen(clcert_cn), &buf);
+    int r = ovpn_base64_encode(clcert_cn, (int)strlen(clcert_cn), &buf);
     ovpn_log(PLOG_NOTE, PLUGIN_NAME, "BASE64 encoded '%s' (return value %i):  '%s'",
              clcert_cn, r, buf);
 
diff --git a/sample/sample-plugins/simple/simple.c b/sample/sample-plugins/simple/simple.c
index e17f3fa..2c096e2 100644
--- a/sample/sample-plugins/simple/simple.c
+++ b/sample/sample-plugins/simple/simple.c
@@ -54,9 +54,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 8692806..7264f95 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -165,31 +165,30 @@
     }
 }
 
-static int
-recv_string(int fd, char *buffer, int len)
+static ssize_t
+recv_string(int fd, char *buffer, size_t len)
 {
     if (len > 0)
     {
-        ssize_t size;
         memset(buffer, 0, len);
-        size = read(fd, buffer, len);
+        ssize_t size = read(fd, buffer, len);
         buffer[len-1] = 0;
         if (size >= 1)
         {
-            return (int)size;
+            return size;
         }
     }
     return -1;
 }
 
-static int
+static ssize_t
 send_string(int fd, const char *string)
 {
-    const int len = strlen(string) + 1;
+    const size_t len = strlen(string) + 1;
     const ssize_t size = write(fd, string, len);
     if (size == len)
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -645,27 +644,26 @@
  * PAM conversation function
  */
 static int
-my_conv(int n, const struct pam_message **msg_array,
+my_conv(int num_msg, const struct pam_message **msg_array,
         struct pam_response **response_array, void *appdata_ptr)
 {
     const struct user_pass *up = ( const struct user_pass *) appdata_ptr;
     struct pam_response *aresp;
-    int i;
     int ret = PAM_SUCCESS;
 
     *response_array = NULL;
 
-    if (n <= 0 || n > PAM_MAX_NUM_MSG)
+    if (num_msg <= 0 || num_msg > PAM_MAX_NUM_MSG)
     {
         return (PAM_CONV_ERR);
     }
-    if ((aresp = calloc(n, sizeof *aresp)) == NULL)
+    if ((aresp = calloc((size_t)num_msg, sizeof *aresp)) == NULL)
     {
         return (PAM_BUF_ERR);
     }
 
     /* loop through each PAM-module query */
-    for (i = 0; i < n; ++i)
+    for (int i = 0; i < num_msg; ++i)
     {
         const struct pam_message *msg = msg_array[i];
         aresp[i].resp_retcode = 0;
@@ -683,9 +681,9 @@
         {
             /* use name/value list match method */
             const struct name_value_list *list = up->name_value_list;
-            int j;
 
             /* loop through name/value pairs */
+            int j; /* checked after loop */
             for (j = 0; j < list->len; ++j)
             {
                 const char *match_name = list->data[j].name;
diff --git a/src/plugins/auth-pam/utils.c b/src/plugins/auth-pam/utils.c
index a6ccfbf..ab5e17e 100644
--- a/src/plugins/auth-pam/utils.c
+++ b/src/plugins/auth-pam/utils.c
@@ -79,7 +79,7 @@
 
     while (scratch)
     {
-        strncat(temp, searching, scratch-searching);
+        strncat(temp, searching, (size_t)(scratch-searching));
         strcat(temp, replacewith);
 
         searching = scratch+strlen(searchfor);
@@ -93,9 +93,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/down-root/down-root.c b/src/plugins/down-root/down-root.c
index 253adcd..2c0faf4 100644
--- a/src/plugins/down-root/down-root.c
+++ b/src/plugins/down-root/down-root.c
@@ -88,9 +88,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -108,10 +107,10 @@
 /*
  * Return the length of a string array
  */
-static int
+static size_t
 string_array_len(const char *array[])
 {
-    int i = 0;
+    size_t i = 0;
     if (array)
     {
         while (array[i])
@@ -141,14 +140,14 @@
     }
 }
 
-static int
+static ssize_t
 send_control(int fd, int code)
 {
     unsigned char c = (unsigned char) code;
     const ssize_t size = write(fd, &c, sizeof(c));
     if (size == sizeof(c))
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -281,7 +280,6 @@
 openvpn_plugin_open_v1(unsigned int *type_mask, const char *argv[], const char *envp[])
 {
     struct down_root_context *context;
-    int i = 0;
 
     /*
      * Allocate our context
@@ -320,7 +318,7 @@
     }
 
     /* Ignore argv[0], as it contains just the plug-in file name */
-    for (i = 1; i < string_array_len(argv); i++)
+    for (int i = 1; i < string_array_len(argv); i++)
     {
         context->command[i-1] = (char *) argv[i];
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1111?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: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Gerrit-Change-Number: 1111
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [PATCH applied] Re: plugins: Clean up -Wconversion warnings

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

Stared long and hard at the changes, some of which clearly look like
I and others have been busy copy-pasting sample code around :-)

I haven't actually tested this beyond a test-compile ("make all" in
the sample-plugins directory, the BBs test-compiling the other
plugins) - easy and straightforward enough.  I took note there is
an "useconds_t" now, but I guess that all platforms having usleep()
will also have this...

Your patch has been applied to the master branch.

commit df3ac551259865a0826d4571a3ae48bb1bdf38e3
Author: Frank Lichtenheld
Date:   Fri Jul 25 14:44:09 2025 +0200

     plugins: Clean up -Wconversion warnings

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32330.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: management: resync timer on bytecount interval change

[ralf_lici (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/+/1113?usp=email

to review the following change.


Change subject: management: resync timer on bytecount interval change
......................................................................

management: resync timer on bytecount interval change

coarse_timer_wakeup tracks when the next timer-driven task will occur.
When a user issues `bytecount n` on the management interface but the
existing wakeup is more than n seconds ahead, bandwidth logging won’t
run until that original timer fires, delaying logs.

Introduce a flag to detect when the bytecount interval changes and,
when set, recalculate coarse_timer_wakeup so logging fires exactly
n seconds after the command. This guarantees bytecount adheres to the
user-specified interval.

Change-Id: Ic0035d52e0ea123398318870d2f4d21af927a602
Signed-off-by: Ralf Lici <ra...@ma...>
---
M src/openvpn/forward.c
M src/openvpn/manage.c
M src/openvpn/manage.h
3 files changed, 15 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/13/1113/1

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index a4f260a..192fff4 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -838,6 +838,19 @@
 static void
 check_coarse_timers(struct context *c)
 {
+#ifdef ENABLE_MANAGEMENT
+    /* The 'bytecount' command starts a timer at runtime, but it would not be
+     * processed if coarse_timer_wakeup was previously set to a higher value.
+     * Therefore, if the command has arrived, we reset coarse_timer_wakeup in
+     * to order to update it accordingly.
+     */
+    if (management && management->connection.bytecount_interval_changed)
+    {
+        reset_coarse_timers(c);
+        management->connection.bytecount_interval_changed = false;
+    }
+#endif /* ENABLE_MANAGEMENT */
+
     if (now < c->c2.coarse_timer_wakeup)
     {
         context_reschedule_sec(c, c->c2.coarse_timer_wakeup - now);
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 8836e79..0df78ee 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -503,6 +503,7 @@
         man->connection.bytecount_update_seconds = 0;
         event_timeout_clear(&man->connection.bytecount_update_interval);
     }
+    man->connection.bytecount_interval_changed = true;
     msg(M_CLIENT, "SUCCESS: bytecount interval changed");
 }
 
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index eb19a4e..00e3931 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -318,6 +318,7 @@
     bool state_realtime;
     bool log_realtime;
     bool echo_realtime;
+    bool bytecount_interval_changed;
     int bytecount_update_seconds;
     struct event_timeout bytecount_update_interval;
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1113?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: Ic0035d52e0ea123398318870d2f4d21af927a602
Gerrit-Change-Number: 1113
Gerrit-PatchSet: 1
Gerrit-Owner: ralf_lici <ra...@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] [PATCH v1] plugins: Clean up -Wconversion warnings

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

From: Frank Lichtenheld <fr...@li...>

Most of the are actually the same ones copied to every
single plugin.

Some drive-by fixes of other warnings and some
conversion cleanups that had no warnings because
they were suppressed by casts.

Change-Id: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Signed-off-by: Frank Lichtenheld <fr...@li...>
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/+/1111
This mail reflects revision 1 of this Change.

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

        
diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c b/sample/sample-plugins/client-connect/sample-client-connect.c
index 18c2c6f..5194270 100644
--- a/sample/sample-plugins/client-connect/sample-client-connect.c
+++ b/sample/sample-plugins/client-connect/sample-client-connect.c
@@ -86,9 +86,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -309,7 +308,7 @@
 
     /* do mighty complicated work that will really take time here... */
     plugin_log(PLOG_NOTE, MODULE, "in async/deferred handler, sleep(%d)", seconds);
-    sleep(seconds);
+    sleep((unsigned int)seconds);
 
     /* write config options to openvpn */
     int ret = write_cc_options_file(name, envp);
diff --git a/sample/sample-plugins/defer/multi-auth.c b/sample/sample-plugins/defer/multi-auth.c
index 38db07f..c458346 100644
--- a/sample/sample-plugins/defer/multi-auth.c
+++ b/sample/sample-plugins/defer/multi-auth.c
@@ -124,9 +124,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -170,7 +169,7 @@
 
 /* Require a minimum OpenVPN Plugin API */
 OPENVPN_EXPORT int
-openvpn_plugin_min_version_required_v1()
+openvpn_plugin_min_version_required_v1(void)
 {
     return OPENVPN_PLUGIN_VERSION_MIN;
 }
@@ -349,9 +348,9 @@
      */
 
     /* do mighty complicated work that will really take time here... */
-    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%d)",
-         context->test_deferred_auth*1000);
-    usleep(context->test_deferred_auth*1000);
+    useconds_t wait_time = (useconds_t)context->test_deferred_auth*1000;
+    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%u)", wait_time);
+    usleep(wait_time);
 
     /* now signal success state to openvpn */
     int fd = open(auth_control_file, O_WRONLY);
diff --git a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
index cc256dd..f1f8868 100644
--- a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
+++ b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
@@ -69,9 +69,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log.c b/sample/sample-plugins/log/log.c
index 82595cf..b304f16 100644
--- a/sample/sample-plugins/log/log.c
+++ b/sample/sample-plugins/log/log.c
@@ -52,9 +52,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log_v3.c b/sample/sample-plugins/log/log_v3.c
index c90cc3d..a024027 100644
--- a/sample/sample-plugins/log/log_v3.c
+++ b/sample/sample-plugins/log/log_v3.c
@@ -55,9 +55,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/simple/base64.c b/sample/sample-plugins/simple/base64.c
index 6855966..e1cc791 100644
--- a/sample/sample-plugins/simple/base64.c
+++ b/sample/sample-plugins/simple/base64.c
@@ -59,9 +59,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -175,7 +174,7 @@
 
     /* test the BASE64 encode function */
     char *buf = NULL;
-    int r = ovpn_base64_encode(clcert_cn, strlen(clcert_cn), &buf);
+    int r = ovpn_base64_encode(clcert_cn, (int)strlen(clcert_cn), &buf);
     ovpn_log(PLOG_NOTE, PLUGIN_NAME, "BASE64 encoded '%s' (return value %i):  '%s'",
              clcert_cn, r, buf);
 
diff --git a/sample/sample-plugins/simple/simple.c b/sample/sample-plugins/simple/simple.c
index e17f3fa..2c096e2 100644
--- a/sample/sample-plugins/simple/simple.c
+++ b/sample/sample-plugins/simple/simple.c
@@ -54,9 +54,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 8692806..7264f95 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -165,31 +165,30 @@
     }
 }
 
-static int
-recv_string(int fd, char *buffer, int len)
+static ssize_t
+recv_string(int fd, char *buffer, size_t len)
 {
     if (len > 0)
     {
-        ssize_t size;
         memset(buffer, 0, len);
-        size = read(fd, buffer, len);
+        ssize_t size = read(fd, buffer, len);
         buffer[len-1] = 0;
         if (size >= 1)
         {
-            return (int)size;
+            return size;
         }
     }
     return -1;
 }
 
-static int
+static ssize_t
 send_string(int fd, const char *string)
 {
-    const int len = strlen(string) + 1;
+    const size_t len = strlen(string) + 1;
     const ssize_t size = write(fd, string, len);
     if (size == len)
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -645,27 +644,26 @@
  * PAM conversation function
  */
 static int
-my_conv(int n, const struct pam_message **msg_array,
+my_conv(int num_msg, const struct pam_message **msg_array,
         struct pam_response **response_array, void *appdata_ptr)
 {
     const struct user_pass *up = ( const struct user_pass *) appdata_ptr;
     struct pam_response *aresp;
-    int i;
     int ret = PAM_SUCCESS;
 
     *response_array = NULL;
 
-    if (n <= 0 || n > PAM_MAX_NUM_MSG)
+    if (num_msg <= 0 || num_msg > PAM_MAX_NUM_MSG)
     {
         return (PAM_CONV_ERR);
     }
-    if ((aresp = calloc(n, sizeof *aresp)) == NULL)
+    if ((aresp = calloc((size_t)num_msg, sizeof *aresp)) == NULL)
     {
         return (PAM_BUF_ERR);
     }
 
     /* loop through each PAM-module query */
-    for (i = 0; i < n; ++i)
+    for (int i = 0; i < num_msg; ++i)
     {
         const struct pam_message *msg = msg_array[i];
         aresp[i].resp_retcode = 0;
@@ -683,9 +681,9 @@
         {
             /* use name/value list match method */
             const struct name_value_list *list = up->name_value_list;
-            int j;
 
             /* loop through name/value pairs */
+            int j; /* checked after loop */
             for (j = 0; j < list->len; ++j)
             {
                 const char *match_name = list->data[j].name;
diff --git a/src/plugins/auth-pam/utils.c b/src/plugins/auth-pam/utils.c
index a6ccfbf..ab5e17e 100644
--- a/src/plugins/auth-pam/utils.c
+++ b/src/plugins/auth-pam/utils.c
@@ -79,7 +79,7 @@
 
     while (scratch)
     {
-        strncat(temp, searching, scratch-searching);
+        strncat(temp, searching, (size_t)(scratch-searching));
         strcat(temp, replacewith);
 
         searching = scratch+strlen(searchfor);
@@ -93,9 +93,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/down-root/down-root.c b/src/plugins/down-root/down-root.c
index 253adcd..2c0faf4 100644
--- a/src/plugins/down-root/down-root.c
+++ b/src/plugins/down-root/down-root.c
@@ -88,9 +88,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -108,10 +107,10 @@
 /*
  * Return the length of a string array
  */
-static int
+static size_t
 string_array_len(const char *array[])
 {
-    int i = 0;
+    size_t i = 0;
     if (array)
     {
         while (array[i])
@@ -141,14 +140,14 @@
     }
 }
 
-static int
+static ssize_t
 send_control(int fd, int code)
 {
     unsigned char c = (unsigned char) code;
     const ssize_t size = write(fd, &c, sizeof(c));
     if (size == sizeof(c))
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -281,7 +280,6 @@
 openvpn_plugin_open_v1(unsigned int *type_mask, const char *argv[], const char *envp[])
 {
     struct down_root_context *context;
-    int i = 0;
 
     /*
      * Allocate our context
@@ -320,7 +318,7 @@
     }
 
     /* Ignore argv[0], as it contains just the plug-in file name */
-    for (i = 1; i < string_array_len(argv); i++)
+    for (int i = 1; i < string_array_len(argv); i++)
     {
         context->command[i-1] = (char *) argv[i];
     }

[Openvpn-devel] [M] Change in openvpn[master]: plugins: Clean up -Wconversion warnings

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

Attention is currently required from: flichtenheld, plaisthos.

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

Change subject: plugins: Clean up -Wconversion warnings
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1111?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: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Gerrit-Change-Number: 1111
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Fri, 25 Jul 2025 12:42:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: options: Clean up function setenv_foreign_option

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

Attention is currently required from: plaisthos.

Hello plaisthos,

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

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

to review the following change.


Change subject: options: Clean up function setenv_foreign_option
......................................................................

options: Clean up function setenv_foreign_option

- Remove unnecessary len > 0 check by reordering
  the code slightly.
- Remove -Wconversion warning by making the len argument
  size_t.

Change-Id: I7efc77f63734501dfa8a8f5bed17b1a1b4e9e201
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/options.c
1 file changed, 25 insertions(+), 29 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1112/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 70b5799..4d94211 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1067,41 +1067,37 @@
 
 #ifndef _WIN32
 static void
-setenv_foreign_option(struct options *o, const char *argv[], int len, struct env_set *es)
+setenv_foreign_option(struct options *o, const char *argv[], size_t len, struct env_set *es)
 {
-    if (len > 0)
-    {
-        struct gc_arena gc = gc_new();
-        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
-        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
-        int i;
-        bool first = true;
-        bool good = true;
+    struct gc_arena gc = gc_new();
+    struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+    struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+    bool first = true;
+    bool good = true;
 
-        good &= buf_printf(&name, "foreign_option_%d", o->foreign_option_index + 1);
-        ++o->foreign_option_index;
-        for (i = 0; i < len; ++i)
+    good &= buf_printf(&name, "foreign_option_%d", o->foreign_option_index + 1);
+    for (size_t i = 0; i < len; ++i)
+    {
+        if (argv[i])
         {
-            if (argv[i])
+            if (!first)
             {
-                if (!first)
-                {
-                    good &= buf_printf(&value, " ");
-                }
-                good &= buf_printf(&value, "%s", argv[i]);
-                first = false;
+                good &= buf_printf(&value, " ");
             }
+            good &= buf_printf(&value, "%s", argv[i]);
+            first = false;
         }
-        if (good)
-        {
-            setenv_str(es, BSTR(&name), BSTR(&value));
-        }
-        else
-        {
-            msg(M_WARN, "foreign_option: name/value overflow");
-        }
-        gc_free(&gc);
     }
+    if (good)
+    {
+        setenv_str(es, BSTR(&name), BSTR(&value));
+        ++o->foreign_option_index;
+    }
+    else
+    {
+        msg(M_WARN, "foreign_option: name/value overflow");
+    }
+    gc_free(&gc);
 }
 #endif /* ifndef _WIN32 */
 
@@ -3679,7 +3675,7 @@
     {
         /* Set foreign option env vars from --dns config */
         const char *p[] = { "dhcp-option", NULL, NULL };
-        size_t p_len = sizeof(p) / sizeof(p[0]);
+        const size_t p_len = sizeof(p) / sizeof(p[0]);
 
         p[1] = "DOMAIN";
         const struct dns_domain *d = dns->search_domains;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1112?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: I7efc77f63734501dfa8a8f5bed17b1a1b4e9e201
Gerrit-Change-Number: 1112
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [M] Change in openvpn[master]: plugins: Clean up -Wconversion warnings

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

Attention is currently required from: plaisthos.

Hello plaisthos,

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

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

to review the following change.


Change subject: plugins: Clean up -Wconversion warnings
......................................................................

plugins: Clean up -Wconversion warnings

Most of the are actually the same ones copied to every
single plugin.

Some drive-by fixes of other warnings and some
conversion cleanups that had no warnings because
they were suppressed by casts.

Change-Id: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M sample/sample-plugins/client-connect/sample-client-connect.c
M sample/sample-plugins/defer/multi-auth.c
M sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
M sample/sample-plugins/log/log.c
M sample/sample-plugins/log/log_v3.c
M sample/sample-plugins/simple/base64.c
M sample/sample-plugins/simple/simple.c
M src/plugins/auth-pam/auth-pam.c
M src/plugins/auth-pam/utils.c
M src/plugins/down-root/down-root.c
10 files changed, 42 insertions(+), 54 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/11/1111/1

diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c b/sample/sample-plugins/client-connect/sample-client-connect.c
index 18c2c6f..5194270 100644
--- a/sample/sample-plugins/client-connect/sample-client-connect.c
+++ b/sample/sample-plugins/client-connect/sample-client-connect.c
@@ -86,9 +86,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -309,7 +308,7 @@
 
     /* do mighty complicated work that will really take time here... */
     plugin_log(PLOG_NOTE, MODULE, "in async/deferred handler, sleep(%d)", seconds);
-    sleep(seconds);
+    sleep((unsigned int)seconds);
 
     /* write config options to openvpn */
     int ret = write_cc_options_file(name, envp);
diff --git a/sample/sample-plugins/defer/multi-auth.c b/sample/sample-plugins/defer/multi-auth.c
index 38db07f..c458346 100644
--- a/sample/sample-plugins/defer/multi-auth.c
+++ b/sample/sample-plugins/defer/multi-auth.c
@@ -124,9 +124,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -170,7 +169,7 @@
 
 /* Require a minimum OpenVPN Plugin API */
 OPENVPN_EXPORT int
-openvpn_plugin_min_version_required_v1()
+openvpn_plugin_min_version_required_v1(void)
 {
     return OPENVPN_PLUGIN_VERSION_MIN;
 }
@@ -349,9 +348,9 @@
      */
 
     /* do mighty complicated work that will really take time here... */
-    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%d)",
-         context->test_deferred_auth*1000);
-    usleep(context->test_deferred_auth*1000);
+    useconds_t wait_time = (useconds_t)context->test_deferred_auth*1000;
+    plog(context, PLOG_NOTE, "in async/deferred handler, usleep(%u)", wait_time);
+    usleep(wait_time);
 
     /* now signal success state to openvpn */
     int fd = open(auth_control_file, O_WRONLY);
diff --git a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
index cc256dd..f1f8868 100644
--- a/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
+++ b/sample/sample-plugins/keying-material-exporter-demo/keyingmaterialexporter.c
@@ -69,9 +69,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log.c b/sample/sample-plugins/log/log.c
index 82595cf..b304f16 100644
--- a/sample/sample-plugins/log/log.c
+++ b/sample/sample-plugins/log/log.c
@@ -52,9 +52,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/log/log_v3.c b/sample/sample-plugins/log/log_v3.c
index c90cc3d..a024027 100644
--- a/sample/sample-plugins/log/log_v3.c
+++ b/sample/sample-plugins/log/log_v3.c
@@ -55,9 +55,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/sample/sample-plugins/simple/base64.c b/sample/sample-plugins/simple/base64.c
index 6855966..e1cc791 100644
--- a/sample/sample-plugins/simple/base64.c
+++ b/sample/sample-plugins/simple/base64.c
@@ -59,9 +59,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -175,7 +174,7 @@
 
     /* test the BASE64 encode function */
     char *buf = NULL;
-    int r = ovpn_base64_encode(clcert_cn, strlen(clcert_cn), &buf);
+    int r = ovpn_base64_encode(clcert_cn, (int)strlen(clcert_cn), &buf);
     ovpn_log(PLOG_NOTE, PLUGIN_NAME, "BASE64 encoded '%s' (return value %i):  '%s'",
              clcert_cn, r, buf);
 
diff --git a/sample/sample-plugins/simple/simple.c b/sample/sample-plugins/simple/simple.c
index e17f3fa..2c096e2 100644
--- a/sample/sample-plugins/simple/simple.c
+++ b/sample/sample-plugins/simple/simple.c
@@ -54,9 +54,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 8692806..7264f95 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -165,31 +165,30 @@
     }
 }
 
-static int
-recv_string(int fd, char *buffer, int len)
+static ssize_t
+recv_string(int fd, char *buffer, size_t len)
 {
     if (len > 0)
     {
-        ssize_t size;
         memset(buffer, 0, len);
-        size = read(fd, buffer, len);
+        ssize_t size = read(fd, buffer, len);
         buffer[len-1] = 0;
         if (size >= 1)
         {
-            return (int)size;
+            return size;
         }
     }
     return -1;
 }
 
-static int
+static ssize_t
 send_string(int fd, const char *string)
 {
-    const int len = strlen(string) + 1;
+    const size_t len = strlen(string) + 1;
     const ssize_t size = write(fd, string, len);
     if (size == len)
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -645,27 +644,26 @@
  * PAM conversation function
  */
 static int
-my_conv(int n, const struct pam_message **msg_array,
+my_conv(int num_msg, const struct pam_message **msg_array,
         struct pam_response **response_array, void *appdata_ptr)
 {
     const struct user_pass *up = ( const struct user_pass *) appdata_ptr;
     struct pam_response *aresp;
-    int i;
     int ret = PAM_SUCCESS;
 
     *response_array = NULL;
 
-    if (n <= 0 || n > PAM_MAX_NUM_MSG)
+    if (num_msg <= 0 || num_msg > PAM_MAX_NUM_MSG)
     {
         return (PAM_CONV_ERR);
     }
-    if ((aresp = calloc(n, sizeof *aresp)) == NULL)
+    if ((aresp = calloc((size_t)num_msg, sizeof *aresp)) == NULL)
     {
         return (PAM_BUF_ERR);
     }
 
     /* loop through each PAM-module query */
-    for (i = 0; i < n; ++i)
+    for (int i = 0; i < num_msg; ++i)
     {
         const struct pam_message *msg = msg_array[i];
         aresp[i].resp_retcode = 0;
@@ -683,9 +681,9 @@
         {
             /* use name/value list match method */
             const struct name_value_list *list = up->name_value_list;
-            int j;
 
             /* loop through name/value pairs */
+            int j; /* checked after loop */
             for (j = 0; j < list->len; ++j)
             {
                 const char *match_name = list->data[j].name;
diff --git a/src/plugins/auth-pam/utils.c b/src/plugins/auth-pam/utils.c
index a6ccfbf..ab5e17e 100644
--- a/src/plugins/auth-pam/utils.c
+++ b/src/plugins/auth-pam/utils.c
@@ -79,7 +79,7 @@
 
     while (scratch)
     {
-        strncat(temp, searching, scratch-searching);
+        strncat(temp, searching, (size_t)(scratch-searching));
         strcat(temp, replacewith);
 
         searching = scratch+strlen(searchfor);
@@ -93,9 +93,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
diff --git a/src/plugins/down-root/down-root.c b/src/plugins/down-root/down-root.c
index 253adcd..2c0faf4 100644
--- a/src/plugins/down-root/down-root.c
+++ b/src/plugins/down-root/down-root.c
@@ -88,9 +88,8 @@
 {
     if (envp)
     {
-        int i;
-        const int namelen = strlen(name);
-        for (i = 0; envp[i]; ++i)
+        const size_t namelen = strlen(name);
+        for (int i = 0; envp[i]; ++i)
         {
             if (!strncmp(envp[i], name, namelen))
             {
@@ -108,10 +107,10 @@
 /*
  * Return the length of a string array
  */
-static int
+static size_t
 string_array_len(const char *array[])
 {
-    int i = 0;
+    size_t i = 0;
     if (array)
     {
         while (array[i])
@@ -141,14 +140,14 @@
     }
 }
 
-static int
+static ssize_t
 send_control(int fd, int code)
 {
     unsigned char c = (unsigned char) code;
     const ssize_t size = write(fd, &c, sizeof(c));
     if (size == sizeof(c))
     {
-        return (int) size;
+        return size;
     }
     else
     {
@@ -281,7 +280,6 @@
 openvpn_plugin_open_v1(unsigned int *type_mask, const char *argv[], const char *envp[])
 {
     struct down_root_context *context;
-    int i = 0;
 
     /*
      * Allocate our context
@@ -320,7 +318,7 @@
     }
 
     /* Ignore argv[0], as it contains just the plug-in file name */
-    for (i = 1; i < string_array_len(argv); i++)
+    for (int i = 1; i < string_array_len(argv); i++)
     {
         context->command[i-1] = (char *) argv[i];
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1111?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: Id61df43bd79fc794a55e107daa0218c8441c2b2c
Gerrit-Change-Number: 1111
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE c...

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

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

Hello cron2, flichtenheld, plaisthos, stipa, 

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

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

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


Change subject: PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE control messages
......................................................................

PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE control messages

Using the management interface you can now target one or more clients (via broadcast,
via cid, via common name, via address) and send a PUSH_UPDATE control message
to update some options.

Change-Id: Ie82bcc7a8e583de9156b185d71d1a323ed8df3fc
Signed-off-by: Marco Baffo <ma...@ma...>
---
M CMakeLists.txt
M doc/management-notes.txt
M src/openvpn/manage.c
M src/openvpn/manage.h
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/push.h
M src/openvpn/push_util.c
M tests/unit_tests/openvpn/Makefile.am
M tests/unit_tests/openvpn/test_push_update_msg.c
10 files changed, 857 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/69/869/20

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3866e21..97f0310 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -862,6 +862,7 @@
 	    src/openvpn/push_util.c
 	    src/openvpn/options_util.c
 	    src/openvpn/otime.c
+        src/openvpn/list.c
         )
 
     if (TARGET test_argv)
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index f1d2930..58393da 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -1028,6 +1028,51 @@
 stored outside of the filesystem (e.g. in Mac OS X Keychain)
 with OpenVPN via the management interface.
 
+COMMAND -- push-update-broad (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Send a message to every connected client to update options at runtime.
+The updatable options are: "block-ipv6", "block-outside-dns", "dhcp-option",
+"dns", "ifconfig", "ifconfig-ipv6", "redirect-gateway", "redirect-private",
+"route", "route-gateway", "route-ipv6", "route-metric", "topology",
+"tun-mtu", "keepalive". When a valid option is pushed, the receiving client will
+delete every previous value and set new value, so the update of the option will 
+not be incremental even when theoretically possible (ex. with "redirect-gateway").
+The '-' symbol in front of an option means the option should be removed.
+When an option is used with '-', it cannot take any parameter.
+The '?' symbol in front of an option means the option's update is optional
+so if the client do not support it, that option will just be ignored without
+making fail the entire command. The '-' and '?' symbols can be used together.
+
+Option Format Ex.
+  `-?option`, `-option`, `?option parameters` are valid formats,
+  `?-option` is not a valid format.
+
+Example
+  push-update-broad "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
+COMMAND -- push-update-cid (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Same as push-update-broad but you must target a single client using client id.
+
+Example
+  push-update-cid 42 "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
+COMMAND -- push-update-cn (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Same as push-update-broad but target the clients based on the provided common name
+(usually just one client per common name is permitted except if "duplicate-cn" option is used).
+
+Example
+  push-update-cid Client0 "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
+COMMAND -- push-update-addr (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Same as push-update-broad but target only the client(s) connecting from the
+provided address (real address). Support both IPv4 and IPv6.
+
+Example
+  push-update-addr 9.9.9.9 1234 "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
 OUTPUT FORMAT
 -------------
 
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 8836e79..42b0331 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -24,7 +24,6 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
-
 #include "syshead.h"
 
 #ifdef ENABLE_MANAGEMENT
@@ -42,6 +41,7 @@
 #include "manage.h"
 #include "openvpn.h"
 #include "dco.h"
+#include "push.h"
 
 #include "memdbg.h"
 
@@ -124,6 +124,11 @@
     msg(M_CLIENT, "username type u        : Enter username u for a queried OpenVPN username.");
     msg(M_CLIENT, "verb [n]               : Set log verbosity level to n, or show if n is absent.");
     msg(M_CLIENT, "version [n]            : Set client's version to n or show current version of daemon.");
+    msg(M_CLIENT, "push-update-broad options : Broadcast a message to update the specified options.");
+    msg(M_CLIENT, "                            Ex. push-update-broad \"route something, -dns\"");
+    msg(M_CLIENT, "push-update-cid CID options : Send an update message to the client identified by CID.");
+    msg(M_CLIENT, "push-update-cn CN options : Send an update message to the client(s) with the specified Common Name.");
+    msg(M_CLIENT, "push-update-addr ip port options : Send an update message to the client(s) connecting from the provided address.");
     msg(M_CLIENT, "END");
 }
 
@@ -1335,6 +1340,129 @@
 }
 
 static void
+man_push_update(struct management *man, const char **p, const push_update_type type)
+{
+    bool status = false;
+
+    if (type == UPT_BROADCAST)
+    {
+        if (!man->persist.callback.push_update_broadcast)
+        {
+            man_command_unsupported("push-update-broad");
+            return;
+        }
+
+        status = (*man->persist.callback.push_update_broadcast)(man->persist.callback.arg, p[1]);
+    }
+    else if (type == UPT_BY_CID)
+    {
+        if (!man->persist.callback.push_update_by_cid)
+        {
+            man_command_unsupported("push-update-cid");
+            return;
+        }
+
+        unsigned long cid = 0;
+
+        if (!parse_cid(p[1], &cid))
+        {
+            msg(M_CLIENT, "ERROR: push-update-cid fail during cid parsing");
+            return;
+        }
+
+        status = (*man->persist.callback.push_update_by_cid)(man->persist.callback.arg, cid, p[2]);
+    }
+    else if (type == UPT_BY_CN)
+    {
+        if (!man->persist.callback.push_update_by_cn)
+        {
+            man_command_unsupported("push-update-cn");
+            return;
+        }
+
+        status = (*man->persist.callback.push_update_by_cn)(man->persist.callback.arg, p[1], p[2]);
+    }
+    else if (type == UPT_BY_ADDR)
+    {
+        if (!man->persist.callback.push_update_by_addr)
+        {
+            man_command_unsupported("push-update-addr");
+            return;
+        }
+
+        const char *ip_str = p[1];
+        const char *port_str = p[2];
+        const char *options = p[3];
+
+        if (!strlen(ip_str) || !strlen(port_str))
+        {
+            msg(M_CLIENT, "ERROR: push-update-addr parse");
+            return;
+        }
+
+        struct addrinfo *res = NULL;
+        int port = atoi(port_str);
+
+        if (port < 1 || port > 65535)
+        {
+            msg(M_CLIENT, "ERROR: port number is out of range: %s", port_str);
+            return;
+        }
+
+        int addr_status = openvpn_getaddrinfo(GETADDR_MSG_VIRT_OUT, ip_str, port_str, 0, NULL, AF_UNSPEC, &res);
+
+        if (addr_status != 0 || !res)
+        {
+            msg(M_CLIENT, "ERROR: error resolving address: %s (%s)", ip_str, gai_strerror(addr_status));
+            return;
+        }
+
+        struct addrinfo *rp;
+
+        /* Iterate through resolved addresses */
+        for (rp = res; rp != NULL; rp = rp->ai_next)
+        {
+            struct openvpn_sockaddr saddr;
+            struct mroute_addr maddr;
+
+            CLEAR(saddr);
+            switch (rp->ai_family)
+            {
+                case AF_INET:
+                    saddr.addr.in4 = *((struct sockaddr_in *)rp->ai_addr);
+                    break;
+
+                case AF_INET6:
+                    saddr.addr.in6 = *((struct sockaddr_in6 *)rp->ai_addr);
+                    break;
+
+                default:
+                    continue;
+            }
+
+            if (!mroute_extract_openvpn_sockaddr(&maddr, &saddr, true))
+            {
+                continue;
+            }
+
+            status = (*man->persist.callback.push_update_by_addr)(man->persist.callback.arg, &maddr, options);
+            if (status)
+            {
+                break;
+            }
+        }
+        freeaddrinfo(res);
+    }
+
+    if (status)
+    {
+        msg(M_CLIENT, "SUCCESS: push-update command succeeded");
+        return;
+    }
+    msg(M_CLIENT, "ERROR: push-update command failed");
+}
+
+static void
 man_dispatch_command(struct management *man, struct status_output *so, const char **p, const int nparms)
 {
     struct gc_arena gc = gc_new();
@@ -1656,6 +1784,34 @@
             man_remote(man, p);
         }
     }
+    else if (streq(p[0], "push-update-broad"))
+    {
+        if (man_need(man, p, 1, 0))
+        {
+            man_push_update(man, p, UPT_BROADCAST);
+        }
+    }
+    else if (streq(p[0], "push-update-cid"))
+    {
+        if (man_need(man, p, 2, 0))
+        {
+            man_push_update(man, p, UPT_BY_CID);
+        }
+    }
+    else if (streq(p[0], "push-update-cn"))
+    {
+        if (man_need(man, p, 2, 0))
+        {
+            man_push_update(man, p, UPT_BY_CN);
+        }
+    }
+    else if (streq(p[0], "push-update-addr"))
+    {
+        if (man_need(man, p, 3, 0))
+        {
+            man_push_update(man, p, UPT_BY_ADDR);
+        }
+    }
 #if 1
     else if (streq(p[0], "test"))
     {
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index eb19a4e..fd7cb11 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -44,7 +44,6 @@
 #define MF_EXTERNAL_KEY_PSSPAD      (1<<16)
 #define MF_EXTERNAL_KEY_DIGEST      (1<<17)
 
-
 #ifdef ENABLE_MANAGEMENT
 
 #include "misc.h"
@@ -205,6 +204,10 @@
 #endif
     unsigned int (*remote_entry_count)(void *arg);
     bool (*remote_entry_get)(void *arg, unsigned int index, char **remote);
+    bool (*push_update_broadcast)(void *arg, const char *options);
+    bool (*push_update_by_cid)(void *arg, unsigned long cid, const char *options);
+    bool (*push_update_by_cn)(void *arg, const char *cn, const char *options);
+    bool (*push_update_by_addr)(void *arg, const struct mroute_addr *maddr, const char *options);
 };
 
 /*
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 26f95ec..5972311 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -4100,7 +4100,7 @@
     }
 }
 
-static struct multi_instance *
+struct multi_instance *
 lookup_by_cid(struct multi_context *m, const unsigned long cid)
 {
     if (m)
@@ -4248,6 +4248,10 @@
         cb.client_auth = management_client_auth;
         cb.client_pending_auth = management_client_pending_auth;
         cb.get_peer_info = management_get_peer_info;
+        cb.push_update_broadcast = management_callback_send_push_update_broadcast;
+        cb.push_update_by_cid = management_callback_send_push_update_by_cid;
+        cb.push_update_by_cn = management_callback_send_push_update_by_cn;
+        cb.push_update_by_addr = management_callback_send_push_update_by_addr;
         management_set_callback(management, &cb);
     }
 #endif /* ifdef ENABLE_MANAGEMENT */
@@ -4373,3 +4377,47 @@
     close_instance(top);
 
 }
+
+/**
+ * Update the vhash with new IP/IPv6 addresses in the multi_context when a
+ * push-update message containing ifconfig/ifconfig-ipv6 options is sent
+ * from the server. This function should be called after a push-update
+ * and old_ip/old_ipv6 are the previous addresses of the client in
+ * ctx->options.ifconfig_local and ctx->options.ifconfig_ipv6_local.
+ */
+void
+update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6)
+{
+    struct in_addr addr;
+    struct in6_addr new_ipv6;
+
+    if ((mi->context.options.ifconfig_local && (!old_ip || strcmp(old_ip, mi->context.options.ifconfig_local)))
+        && inet_pton(AF_INET, mi->context.options.ifconfig_local, &addr) == 1)
+    {
+        in_addr_t new_ip = ntohl(addr.s_addr);
+
+        /* Add new, remove old if exist */
+        multi_learn_in_addr_t(m, mi, new_ip, 0, true);
+    }
+
+    /* TO DO:
+     *  else if (old_ip && !mi->context.options.ifconfig_local)
+     *  {
+     *      // remove old ip
+     *  }
+     */
+
+    if ((mi->context.options.ifconfig_ipv6_local && (!old_ipv6 || strcmp(old_ipv6, mi->context.options.ifconfig_ipv6_local)))
+        && inet_pton(AF_INET6, mi->context.options.ifconfig_ipv6_local, &new_ipv6) == 1)
+    {
+        /* Add new, remove old if exist */
+        multi_learn_in6_addr(m, mi, new_ipv6, 0, true);
+    }
+
+    /* TO DO:
+     *  else if (old_ipv6 && !mi->context.options.ifconfig_ipv6_local)
+     *  {
+     *      // remove old IPv6
+     *  }
+     */
+}
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 8b2704c..01c05ba 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -702,5 +702,13 @@
  */
 void multi_assign_peer_id(struct multi_context *m, struct multi_instance *mi);
 
+#ifdef ENABLE_MANAGEMENT
+struct multi_instance *
+lookup_by_cid(struct multi_context *m, const unsigned long cid);
+
+#endif
+
+void
+update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6);
 
 #endif /* MULTI_H */
diff --git a/src/openvpn/push.h b/src/openvpn/push.h
index 18dfcd8..e67c3ac 100644
--- a/src/openvpn/push.h
+++ b/src/openvpn/push.h
@@ -42,6 +42,16 @@
 #define PUSH_OPT_TO_REMOVE (1<<0)
 #define PUSH_OPT_OPTIONAL (1<<1)
 
+/* Push-update message sender modes */
+typedef enum {
+    UPT_BROADCAST = 0,
+    UPT_BY_ADDR = 1,
+    UPT_BY_CN = 2,
+#ifdef ENABLE_MANAGEMENT
+    UPT_BY_CID = 3
+#endif
+} push_update_type;
+
 int process_incoming_push_request(struct context *c);
 
 /**
@@ -134,4 +144,33 @@
 void
 receive_auth_pending(struct context *c, const struct buffer *buffer);
 
+/**
+ * @brief A function to send a PUSH_UPDATE control message from server to client(s).
+ *
+ * @param m the multi_context, contains all the clients connected to this server.
+ * @param target the target to which to send the message. It should be:
+ * `NULL` if `type == UPT_BROADCAST`,
+ * a `mroute_addr *` if `type == UPT_BY_ADDR`,
+ * a `char *` if `type == UPT_BY_CN`,
+ * an `unsigned long *` if `type == UPT_BY_CID`.
+ * @param msg a string containing the options to send.
+ * @param type the way to address the message (broadcast, by cid, by cn, by address).
+ * @param push_bundle_size the maximum size of a bundle of pushed option. Just use PUSH_BUNDLE_SIZE macro.
+ * @return the number of clients to which the message was sent.
+ */
+int
+send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const int push_bundle_size);
+
+#ifdef ENABLE_MANAGEMENT
+
+bool management_callback_send_push_update_broadcast(void *arg, const char *options);
+
+bool management_callback_send_push_update_by_cid(void *arg, unsigned long cid, const char *options);
+
+bool management_callback_send_push_update_by_cn(void *arg, const char *cn, const char *options);
+
+bool management_callback_send_push_update_by_addr(void *arg, const struct mroute_addr *maddr, const char *options);
+
+#endif /* ifdef ENABLE_MANAGEMENT*/
+
 #endif /* ifndef PUSH_H */
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
index b4d1e8b..245a29b 100644
--- a/src/openvpn/push_util.c
+++ b/src/openvpn/push_util.c
@@ -3,6 +3,9 @@
 #endif
 
 #include "push.h"
+#include "multi.h"
+#include "ssl_verify.h"
+#include "buffer.h"
 
 int
 process_incoming_push_update(struct context *c,
@@ -42,3 +45,281 @@
 
     return ret;
 }
+
+/**
+ * Return index of last `,` or `0` if it didn't find any.
+ * If there is a comma at index `0` it's an error anyway
+ */
+static int
+find_first_comma_of_next_bundle(const char *str, int ix)
+{
+    while (ix > 0)
+    {
+        if (str[ix] == ',')
+        {
+            return ix;
+        }
+        ix--;
+    }
+    return 0;
+}
+
+/* Allocate memory and assemble the final message */
+static struct buffer
+forge_msg(const char *src, const char *continuation, struct gc_arena *gc)
+{
+    int src_len = strlen(src);
+    int con_len = continuation ? strlen(continuation) : 0;
+    struct buffer buf = alloc_buf_gc(src_len + sizeof(push_update_cmd) + con_len + 2, gc);
+
+    buf_printf(&buf, "%s,%s%s", push_update_cmd, src, continuation ? continuation : "");
+
+    return buf;
+}
+
+static char *
+gc_strdup(const char *src, struct gc_arena *gc)
+{
+    char *ret = gc_malloc((strlen(src) + 1) * sizeof(char), true, gc);
+
+    strcpy(ret, src);
+    return ret;
+}
+
+/* It split the messagge (if necessay) and fill msgs with the message chunks.
+ * Return `false` on failure an `true` on success.
+ */
+static bool
+message_splitter(const char *s, struct buffer *msgs, struct gc_arena *gc, const int safe_cap)
+{
+    if (!s || !*s)
+    {
+        return false;
+    }
+
+    char *str = gc_strdup(s, gc);
+    int i = 0;
+    int im = 0;
+
+    while (*str)
+    {
+        /* + ',' - '/0' */
+        if (strlen(str) > safe_cap)
+        {
+            int ci = find_first_comma_of_next_bundle(str, safe_cap);
+            if (!ci)
+            {
+                /* if no commas were found go to fail, do not send any message */
+                return false;
+            }
+            str[ci] = '\0';
+            /* copy from i to (ci -1) */
+            msgs[im] = forge_msg(str, ",push-continuation 2", gc);
+            i = ci + 1;
+        }
+        else
+        {
+            if (im)
+            {
+                msgs[im] = forge_msg(str, ",push-continuation 1", gc);
+            }
+            else
+            {
+                msgs[im] = forge_msg(str, NULL, gc);
+            }
+            i = strlen(str);
+        }
+        str = &str[i];
+        im++;
+    }
+    return true;
+}
+
+/* send the message(s) prepared to one single client */
+static bool
+send_single_push_update(struct context *c, struct buffer *msgs, unsigned int *option_types_found)
+{
+    if (!msgs[0].data || !*(msgs[0].data))
+    {
+        return false;
+    }
+    int i = -1;
+
+    while (msgs[++i].data && *(msgs[i].data))
+    {
+        if (!send_control_channel_string(c, BSTR(&msgs[i]), D_PUSH))
+        {
+            return false;
+        }
+
+        /* After sending the control message, we update the options server-side in the client's context
+         * so pushed options like ifconfig/ifconfig-ipv6 can actually work.
+         * If we don't do that, the packets arriving from the client with the new address will be
+         * rejected because the value in the option is an old one.
+         * For the same reason we later update the vhash too in `send_push_update()` function. */
+        buf_string_compare_advance(&msgs[i], push_update_cmd);
+        if (process_incoming_push_update(c, pull_permission_mask(c), option_types_found, &msgs[i]) == PUSH_MSG_ERROR)
+        {
+            msg(M_WARN, "Failed to process push update message sent to client ID: %u",
+                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
+            continue;
+        }
+        c->options.push_option_types_found |= *option_types_found;
+        if (!options_postprocess_pull(&c->options, c->c2.es))
+        {
+            msg(M_WARN, "Failed to post-process push update message sent to client ID: %u",
+                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
+        }
+    }
+    return true;
+}
+
+int
+send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const int push_bundle_size)
+{
+    if (!msg || !*msg || !m
+        || (!target && type != UPT_BROADCAST))
+    {
+        return -EINVAL;
+    }
+
+    struct gc_arena gc = gc_new();
+    /* extra space for possible trailing ifconfig and push-continuation */
+    const int extra = 84 + sizeof(push_update_cmd);
+    /* push_bundle_size is the maximum size of a message, so if the message
+     * we want to send exceeds that size we have to split it into smaller messages */
+    const int safe_cap = push_bundle_size - extra;
+    int msgs_num = (strlen(msg) / safe_cap) + ((strlen(msg) % safe_cap) != 0);
+    struct buffer *msgs = gc_malloc((msgs_num + 1) * sizeof(struct buffer), true, &gc);
+
+    unsigned int option_types_found = 0;
+
+    msgs[msgs_num].data = NULL;
+    if (!message_splitter(msg, msgs, &gc, safe_cap))
+    {
+        gc_free(&gc);
+        return -EINVAL;
+    }
+
+#ifdef ENABLE_MANAGEMENT
+    if (type == UPT_BY_CID)
+    {
+        struct multi_instance *mi = lookup_by_cid(m, *((unsigned long *)target));
+
+        if (!mi)
+        {
+            return -ENOENT;
+        }
+
+        const char *old_ip = mi->context.options.ifconfig_local;
+        const char *old_ipv6 = mi->context.options.ifconfig_ipv6_local;
+        if (!mi->halt
+            && send_single_push_update(&mi->context, msgs, &option_types_found))
+        {
+            if (option_types_found & OPT_P_UP)
+            {
+                update_vhash(m, mi, old_ip, old_ipv6);
+            }
+            gc_free(&gc);
+            return 1;
+        }
+        else
+        {
+            gc_free(&gc);
+            return 0;
+        }
+    }
+#endif /* ifdef ENABLE_MANAGEMENT */
+
+    int count = 0;
+    struct hash_iterator hi;
+    const struct hash_element *he;
+
+    hash_iterator_init(m->iter, &hi);
+    while ((he = hash_iterator_next(&hi)))
+    {
+        struct multi_instance *curr_mi = he->value;
+
+        if (curr_mi->halt)
+        {
+            continue;
+        }
+        if (type == UPT_BY_ADDR && !mroute_addr_equal(target, &curr_mi->real))
+        {
+            continue;
+        }
+        else if (type == UPT_BY_CN)
+        {
+            const char *curr_cn = tls_common_name(curr_mi->context.c2.tls_multi, false);
+            if (strcmp(curr_cn, target))
+            {
+                continue;
+            }
+        }
+        /* Either we found a matching client or type is UPT_BROADCAST so we update every client */
+        option_types_found = 0;
+        const char *old_ip = curr_mi->context.options.ifconfig_local;
+        const char *old_ipv6 = curr_mi->context.options.ifconfig_ipv6_local;
+        if (!send_single_push_update(&curr_mi->context, msgs, &option_types_found))
+        {
+            msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated",
+                curr_mi->context.c2.tls_multi ? curr_mi->context.c2.tls_multi->peer_id : UINT32_MAX);
+            continue;
+        }
+        if (option_types_found & OPT_P_UP)
+        {
+            update_vhash(m, curr_mi, old_ip, old_ipv6);
+        }
+        count++;
+    }
+
+    hash_iterator_free(&hi);
+    gc_free(&gc);
+    return count;
+}
+
+#ifdef ENABLE_MANAGEMENT
+#define RETURN_UPDATE_STATUS(n_sent) \
+    do { \
+        if ((n_sent) > 0) { \
+            msg(M_CLIENT, "SUCCESS: %d client(s) updated", (n_sent)); \
+            return true; \
+        } else { \
+            msg(M_CLIENT, "ERROR: no client updated"); \
+            return false; \
+        } \
+    } while (0)
+
+
+bool
+management_callback_send_push_update_broadcast(void *arg, const char *options)
+{
+    int n_sent = send_push_update(arg, NULL, options, UPT_BROADCAST, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+
+bool
+management_callback_send_push_update_by_cid(void *arg, unsigned long cid, const char *options)
+{
+    int n_sent = send_push_update(arg, &cid, options, UPT_BY_CID, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+
+bool
+management_callback_send_push_update_by_cn(void *arg, const char *cn, const char *options)
+{
+    int n_sent = send_push_update(arg, cn, options, UPT_BY_CN, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+
+bool
+management_callback_send_push_update_by_addr(void *arg, const struct mroute_addr *maddr, const char *options)
+{
+    int n_sent = send_push_update(arg, maddr, options, UPT_BY_ADDR, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+#endif /* ifdef ENABLE_MANAGEMENT */
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index b24e03c..9a40512 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -343,4 +343,5 @@
 	$(top_srcdir)/src/openvpn/platform.c \
 	$(top_srcdir)/src/openvpn/push_util.c \
 	$(top_srcdir)/src/openvpn/options_util.c \
-	$(top_srcdir)/src/openvpn/otime.c
\ No newline at end of file
+	$(top_srcdir)/src/openvpn/otime.c \
+	$(top_srcdir)/src/openvpn/list.c
\ No newline at end of file
diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c
index d47286a..9341c34 100644
--- a/tests/unit_tests/openvpn/test_push_update_msg.c
+++ b/tests/unit_tests/openvpn/test_push_update_msg.c
@@ -8,6 +8,7 @@
 #include <cmocka.h>
 #include "push.h"
 #include "options_util.h"
+#include "multi.h"
 
 /* mocks */
 
@@ -36,6 +37,18 @@
     return flags;
 }
 
+void
+update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6)
+{
+    return;
+}
+
+bool
+options_postprocess_pull(struct options *options, struct env_set *es)
+{
+    return true;
+}
+
 bool
 apply_push_options(struct context *c,
                    struct options *options,
@@ -109,6 +122,49 @@
     }
 }
 
+const char *
+tls_common_name(const struct tls_multi *multi, const bool null)
+{
+    return NULL;
+}
+
+#ifndef ENABLE_MANAGEMENT
+bool
+send_control_channel_string(struct context *c, const char *str, int msglevel)
+{
+    return true;
+}
+#else  /* ifndef ENABLE_MANAGEMENT */
+char **res;
+int i;
+
+bool
+send_control_channel_string(struct context *c, const char *str, int msglevel)
+{
+    if (res && res[i] && strcmp(res[i], str))
+    {
+        printf("\n\nexpected: %s\n\n  actual: %s\n\n", res[i], str);
+        return false;
+    }
+    i++;
+    return true;
+}
+
+struct multi_instance *
+lookup_by_cid(struct multi_context *m, const unsigned long cid)
+{
+    return *(m->instances);
+}
+
+bool
+mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,
+                                const struct openvpn_sockaddr *osaddr,
+                                bool use_port)
+{
+    return true;
+}
+#endif /* ifndef ENABLE_MANAGEMENT */
+
 /* tests */
 
 static void
@@ -139,7 +195,6 @@
     free_buf(&buf);
 }
 
-
 static void
 test_incoming_push_message_error2(void **state)
 {
@@ -224,6 +279,207 @@
     free_buf(&buf);
 }
 
+#ifdef ENABLE_MANAGEMENT
+char *r0[] = {
+    "PUSH_UPDATE,redirect-gateway local,route 192.168.1.0 255.255.255.0"
+};
+char *r1[] = {
+    "PUSH_UPDATE,-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8,redirect-gateway local,push-continuation 2",
+    "PUSH_UPDATE,route 192.168.1.0 255.255.255.0,push-continuation 1"
+};
+char *r3[] = {
+    "PUSH_UPDATE,,,"
+};
+char *r4[] = {
+    "PUSH_UPDATE,-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local,push-continuation 2",
+    "PUSH_UPDATE, route 192.168.1.0 255.255.255.0,,push-continuation 1"
+};
+char *r5[] = {
+    "PUSH_UPDATE,,-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local,push-continuation 2",
+    "PUSH_UPDATE, route 192.168.1.0 255.255.255.0,push-continuation 1"
+};
+char *r6[] = {
+    "PUSH_UPDATE,-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8, redirect-gateway 10.10.10.10,,push-continuation 2",
+    "PUSH_UPDATE, route 192.168.1.0 255.255.255.0,,push-continuation 1"
+};
+char *r7[] = {
+    "PUSH_UPDATE,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,push-continuation 2",
+    "PUSH_UPDATE,,,,,,,,,,,,,,,,,,,push-continuation 1"
+};
+char *r8[] = {
+    "PUSH_UPDATE,-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8,redirect-gateway\n local,push-continuation 2",
+    "PUSH_UPDATE,route 192.168.1.0 255.255.255.0\n\n\n,push-continuation 1"
+};
+char *r9[] = {
+    "PUSH_UPDATE,,"
+};
+
+
+const char *msg0 = "redirect-gateway local,route 192.168.1.0 255.255.255.0";
+const char *msg1 = "-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,"
+                   " akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8,redirect-gateway local,route 192.168.1.0 255.255.255.0";
+const char *msg2 = "";
+const char *msg3 = ",,";
+const char *msg4 = "-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,"
+                   " akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local, route 192.168.1.0 255.255.255.0,";
+const char *msg5 = ",-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,"
+                   " akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local, route 192.168.1.0 255.255.255.0";
+const char *msg6 = "-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf, akakakakakakakakakakakaf,"
+                   " dhcp-option DNS 8.8.8.8, redirect-gateway 10.10.10.10,, route 192.168.1.0 255.255.255.0,";
+const char *msg7 = ",,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,";
+const char *msg8 = "-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf, akakakakakakakakakakakaf,"
+                   " dhcp-option DNS 8.8.8.8,redirect-gateway\n local,route 192.168.1.0 255.255.255.0\n\n\n";
+const char *msg9 = ",";
+
+const char *msg10 = "abandon ability able about above absent absorb abstract absurd abuse access accident account accuse achieve"
+                    "acid acoustic acquire across act action actor actress actual adapt add addict address adjust"
+                    "baby bachelor bacon badge bag balance balcony ball bamboo banana banner bar barely bargain barrel base basic"
+                    "basket battle beach bean beauty because become beef before begin behave behind"
+                    "cabbage cabin cable cactus cage cake call calm camera camp can canal cancel candy cannon canoe canvas canyon"
+                    "capable capital captain car carbon card cargo carpet carry cart case"
+                    "daisy damage damp dance danger daring dash daughter dawn day deal debate debris decade december decide decline"
+                    "decorate decrease deer defense define defy degree delay deliver demand demise denial";
+
+#define PUSH_BUNDLE_SIZE_TEST 184
+
+static void
+test_send_push_msg0(void **state)
+{
+    i = 0;
+    res = r0;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg0, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+static void
+test_send_push_msg1(void **state)
+{
+    i = 0;
+    res = r1;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg1, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg2(void **state)
+{
+    i = 0;
+    res = NULL;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg2, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), -EINVAL);
+}
+
+static void
+test_send_push_msg3(void **state)
+{
+    i = 0;
+    res = r3;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg3, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg4(void **state)
+{
+    i = 0;
+    res = r4;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg4, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg5(void **state)
+{
+    i = 0;
+    res = r5;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg5, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg6(void **state)
+{
+    i = 0;
+    res = r6;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg6, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg7(void **state)
+{
+    i = 0;
+    res = r7;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg7, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg8(void **state)
+{
+    i = 0;
+    res = r8;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg8, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg9(void **state)
+{
+    i = 0;
+    res = r9;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg9, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg10(void **state)
+{
+    i = 0;
+    res = NULL;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg10, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), -EINVAL);
+}
+
+#undef PUSH_BUNDLE_SIZE_TEST
+
+static int
+setup2(void **state)
+{
+    struct multi_context *m = calloc(1, sizeof(struct multi_context));
+    m->instances = calloc(1, sizeof(struct multi_instance *));
+    struct multi_instance *mi = calloc(1, sizeof(struct multi_instance));
+    *(m->instances) = mi;
+    *state = m;
+    return 0;
+}
+
+static int
+teardown2(void **state)
+{
+    struct multi_context *m = *state;
+    free(*(m->instances));
+    free(m->instances);
+    free(m);
+    return 0;
+}
+#endif /* ifdef ENABLE_MANAGEMENT */
+
 static int
 setup(void **state)
 {
@@ -253,7 +509,20 @@
         cmocka_unit_test_setup_teardown(test_incoming_push_message_1, setup, teardown),
         cmocka_unit_test_setup_teardown(test_incoming_push_message_bad_format, setup, teardown),
         cmocka_unit_test_setup_teardown(test_incoming_push_message_mix, setup, teardown),
-        cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown)
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown),
+#ifdef ENABLE_MANAGEMENT
+        cmocka_unit_test_setup_teardown(test_send_push_msg0, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg1, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg2, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg3, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg4, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg5, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg6, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg7, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg8, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg9, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg10, setup2, teardown2)
+#endif
     };
 
     return cmocka_run_group_tests(tests, NULL, NULL);

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/869?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: Ie82bcc7a8e583de9156b185d71d1a323ed8df3fc
Gerrit-Change-Number: 869
Gerrit-PatchSet: 20
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [L] Change in openvpn[master]: Clean up the big msglevel type confusion

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

Attention is currently required from: plaisthos.

Hello plaisthos,

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

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

to review the following change.


Change subject: Clean up the big msglevel type confusion
......................................................................

Clean up the big msglevel type confusion

msglevel was definitely unsigned as the first
argument to msg(), but many parts of the code
had it as signed. So this produced a LOT of
warnings when enabling -Wsign-conversion.

There is one exception in struct status_output
where -1 is a valid value in the API. Only
positive values are translated into standard
message levels.

Change-Id: Id492cb774c6d022d06bb3cf5fec2a4bdd410e619
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/argv.c
M src/openvpn/argv.h
M src/openvpn/buffer.c
M src/openvpn/clinat.c
M src/openvpn/clinat.h
M src/openvpn/comp.c
M src/openvpn/comp.h
M src/openvpn/crypto.c
M src/openvpn/crypto.h
M src/openvpn/dco.c
M src/openvpn/dco.h
M src/openvpn/dco_freebsd.c
M src/openvpn/dco_linux.c
M src/openvpn/dco_win.c
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/env_set.c
M src/openvpn/env_set.h
M src/openvpn/error.c
M src/openvpn/error.h
M src/openvpn/forward.c
M src/openvpn/forward.h
M src/openvpn/init.c
M src/openvpn/init.h
M src/openvpn/manage.h
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/options_util.c
M src/openvpn/options_util.h
M src/openvpn/packet_id.c
M src/openvpn/plugin.c
M src/openvpn/plugin.h
M src/openvpn/pool.c
M src/openvpn/pool.h
M src/openvpn/push.c
M src/openvpn/push.h
M src/openvpn/route.c
M src/openvpn/route.h
M src/openvpn/run_command.c
M src/openvpn/run_command.h
M src/openvpn/sig.c
M src/openvpn/sig.h
M src/openvpn/socket.c
M src/openvpn/socket.h
M src/openvpn/ssl_verify_backend.h
M src/openvpn/ssl_verify_mbedtls.c
M src/openvpn/ssl_verify_openssl.c
M src/openvpn/status.c
M src/openvpn/status.h
M src/openvpn/tun.c
M src/openvpn/tun.h
M tests/unit_tests/openvpn/mock_ssl_dependencies.c
M tests/unit_tests/openvpn/test_argv.c
M tests/unit_tests/openvpn/test_misc.c
M tests/unit_tests/openvpn/test_pkcs11.c
M tests/unit_tests/openvpn/test_pkt.c
M tests/unit_tests/openvpn/test_tls_crypt.c
M tests/unit_tests/openvpn/test_user_pass.c
59 files changed, 185 insertions(+), 185 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/1110/1

diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 95215c0..2db510c 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -236,14 +236,14 @@
 /**
  *  Write the arguments stored in a struct argv via the msg() command.
  *
- *  @param msglev  Integer with the message level used by msg().
- *  @param a       Valid pointer to the struct argv with the arguments to write.
+ *  @param msglevel  Integer with the message level used by msg().
+ *  @param a         Valid pointer to the struct argv with the arguments to write.
  */
 void
-argv_msg(const int msglev, const struct argv *a)
+argv_msg(const unsigned int msglevel, const struct argv *a)
 {
     struct gc_arena gc = gc_new();
-    msg(msglev, "%s", argv_str(a, &gc, 0));
+    msg(msglevel, "%s", argv_str(a, &gc, 0));
     gc_free(&gc);
 }
 
@@ -251,16 +251,16 @@
  *  Similar to argv_msg() but prefixes the messages being written with a
  *  given string.
  *
- *  @param msglev  Integer with the message level used by msg().
- *  @param a       Valid pointer to the struct argv with the arguments to write
- *  @param prefix  Valid pointer to the prefix string
+ *  @param msglevel  Integer with the message level used by msg().
+ *  @param a         Valid pointer to the struct argv with the arguments to write
+ *  @param prefix    Valid pointer to the prefix string
  *
  */
 void
-argv_msg_prefix(const int msglev, const struct argv *a, const char *prefix)
+argv_msg_prefix(const unsigned int msglevel, const struct argv *a, const char *prefix)
 {
     struct gc_arena gc = gc_new();
-    msg(msglev, "%s: %s", prefix, argv_str(a, &gc, 0));
+    msg(msglevel, "%s: %s", prefix, argv_str(a, &gc, 0));
     gc_free(&gc);
 }
 
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index 098a1cb..866b405 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -47,9 +47,9 @@
 
 struct argv argv_insert_head(const struct argv *a, const char *head);
 
-void argv_msg(const int msglev, const struct argv *a);
+void argv_msg(const unsigned int msglevel, const struct argv *a);
 
-void argv_msg_prefix(const int msglev, const struct argv *a, const char *prefix);
+void argv_msg_prefix(const unsigned int msglevel, const struct argv *a, const char *prefix);
 
 void argv_parse_cmd(struct argv *a, const char *s);
 
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index dd6044b..456f591 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -1130,7 +1130,7 @@
 {
     if (buf && buf->len)
     {
-        int msglevel = D_ALIGN_DEBUG;
+        unsigned int msglevel = D_ALIGN_DEBUG;
         const unsigned int u = (unsigned int) BPTR(buf);
 
         if (u & (PAYLOAD_ALIGN-1))
diff --git a/src/openvpn/clinat.c b/src/openvpn/clinat.c
index f040724..81af08c 100644
--- a/src/openvpn/clinat.c
+++ b/src/openvpn/clinat.c
@@ -49,7 +49,7 @@
 }
 
 void
-print_client_nat_list(const struct client_nat_option_list *list, int msglevel)
+print_client_nat_list(const struct client_nat_option_list *list, unsigned int msglevel)
 {
     struct gc_arena gc = gc_new();
     int i;
@@ -108,7 +108,7 @@
                               const char *network,
                               const char *netmask,
                               const char *foreign_network,
-                              int msglevel)
+                              unsigned int msglevel)
 {
     struct client_nat_entry e;
     bool ok;
@@ -166,7 +166,7 @@
 #endif
 
 static void
-print_pkt(struct openvpn_iphdr *iph, const char *prefix, const int direction, const int msglevel)
+print_pkt(struct openvpn_iphdr *iph, const char *prefix, const int direction, const unsigned int msglevel)
 {
     struct gc_arena gc = gc_new();
 
diff --git a/src/openvpn/clinat.h b/src/openvpn/clinat.h
index 334b3f2..c099857 100644
--- a/src/openvpn/clinat.h
+++ b/src/openvpn/clinat.h
@@ -51,14 +51,14 @@
 
 void copy_client_nat_option_list(struct client_nat_option_list *dest, const struct client_nat_option_list *src);
 
-void print_client_nat_list(const struct client_nat_option_list *list, int msglevel);
+void print_client_nat_list(const struct client_nat_option_list *list, unsigned int msglevel);
 
 void add_client_nat_to_option_list(struct client_nat_option_list *dest,
                                    const char *type,
                                    const char *network,
                                    const char *netmask,
                                    const char *foreign_network,
-                                   int msglevel);
+                                   unsigned int msglevel);
 
 void client_nat_transform(const struct client_nat_option_list *list,
                           struct buffer *ipbuf,
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index b2aa07e..2e17af4 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -160,7 +160,7 @@
 #endif /* USE_COMP */
 
 bool
-check_compression_settings_valid(struct compress_options *info, int msglevel)
+check_compression_settings_valid(struct compress_options *info, unsigned int msglevel)
 {
     /*
      * We also allow comp-stub-v2 here as it technically allows escaping of
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index ee9c16a..37f3be7 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -91,7 +91,7 @@
  * in
  */
 bool
-check_compression_settings_valid(struct compress_options *info, int msglevel);
+check_compression_settings_valid(struct compress_options *info, unsigned int msglevel);
 
 #ifdef USE_COMP
 #include "buffer.h"
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 7564f82..91b5da04 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1650,7 +1650,7 @@
 }
 
 int
-ascii2keydirection(int msglevel, const char *str)
+ascii2keydirection(unsigned int msglevel, const char *str)
 {
     if (!str)
     {
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 345e7ce..cd32bb4 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -627,7 +627,7 @@
 
 void must_have_n_keys(const char *filename, const char *option, const struct key2 *key2, int n);
 
-int ascii2keydirection(int msglevel, const char *str);
+int ascii2keydirection(unsigned int msglevel, const char *str);
 
 const char *keydirection2ascii(int kd, bool remote, bool humanreadable);
 
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 98cbb72..6ce86a2 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -239,7 +239,7 @@
 }
 
 static bool
-dco_check_option_ce(const struct connection_entry *ce, int msglevel, int mode)
+dco_check_option_ce(const struct connection_entry *ce, unsigned int msglevel, int mode)
 {
     if (ce->fragment)
     {
@@ -297,7 +297,7 @@
 }
 
 bool
-dco_check_startup_option(int msglevel, const struct options *o)
+dco_check_startup_option(unsigned int msglevel, const struct options *o)
 {
     /* check if no dev name was specified at all. In the case,
      * later logic will most likely stop OpenVPN, so no need to
@@ -433,7 +433,7 @@
 }
 
 bool
-dco_check_option(int msglevel, const struct options *o)
+dco_check_option(unsigned int msglevel, const struct options *o)
 {
     /* At this point the ciphers have already been normalised */
     if (o->enable_ncp_fallback
@@ -479,7 +479,7 @@
 }
 
 bool
-dco_check_pull_options(int msglevel, const struct options *o)
+dco_check_pull_options(unsigned int msglevel, const struct options *o)
 {
     if (!o->use_peer_id)
     {
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index f38316d..3dbf19c 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -56,7 +56,7 @@
  * @param msglevel      level to print messages to
  * @return              true if ovpn-dco is available, false otherwise
  */
-bool dco_available(int msglevel);
+bool dco_available(unsigned int msglevel);
 
 
 /**
@@ -76,7 +76,7 @@
  * @param o         the options struct that hold the options
  * @return          true if no conflict was detected, false otherwise
  */
-bool dco_check_option(int msglevel, const struct options *o);
+bool dco_check_option(unsigned int msglevel, const struct options *o);
 
 /**
  * Check whether the options struct has any further option that is not supported
@@ -88,7 +88,7 @@
  * @param o         the options struct that hold the options
  * @return          true if no conflict was detected, false otherwise
  */
-bool dco_check_startup_option(int msglevel, const struct options *o);
+bool dco_check_startup_option(unsigned int msglevel, const struct options *o);
 
 /**
  * Check whether any of the options pushed by the server is not supported by
@@ -99,7 +99,7 @@
  * @param o         the options struct that hold the options
  * @return          true if no conflict was detected, false otherwise
  */
-bool dco_check_pull_options(int msglevel, const struct options *o);
+bool dco_check_pull_options(unsigned int msglevel, const struct options *o);
 
 /**
  * Initialize the DCO context
@@ -267,7 +267,7 @@
 typedef void *dco_context_t;
 
 static inline bool
-dco_available(int msglevel)
+dco_available(unsigned int msglevel)
 {
     return false;
 }
@@ -279,19 +279,19 @@
 }
 
 static inline bool
-dco_check_option(int msglevel, const struct options *o)
+dco_check_option(unsigned int msglevel, const struct options *o)
 {
     return false;
 }
 
 static inline bool
-dco_check_startup_option(int msglevel, const struct options *o)
+dco_check_startup_option(unsigned int msglevel, const struct options *o)
 {
     return false;
 }
 
 static inline bool
-dco_check_pull_options(int msglevel, const struct options *o)
+dco_check_pull_options(unsigned int msglevel, const struct options *o)
 {
     return false;
 }
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index b8816c6..bcd10b7 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -582,7 +582,7 @@
 }
 
 bool
-dco_available(int msglevel)
+dco_available(unsigned int msglevel)
 {
     struct if_clonereq ifcr;
     char *buf = NULL;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 3652a49..43753a5 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -76,7 +76,7 @@
  * @return ID on success, negative error code on error
  */
 static int
-resolve_ovpn_netlink_id(int msglevel)
+resolve_ovpn_netlink_id(unsigned int msglevel)
 {
     int ret;
     struct nl_sock *nl_sock = nl_socket_alloc();
@@ -1196,7 +1196,7 @@
 }
 
 bool
-dco_available(int msglevel)
+dco_available(unsigned int msglevel)
 {
     if (resolve_ovpn_netlink_id(D_DCO_DEBUG) < 0)
     {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 83db739..15f7237 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -598,7 +598,7 @@
 }
 
 bool
-dco_available(int msglevel)
+dco_available(unsigned int msglevel)
 {
     /* try to open device by symbolic name */
     HANDLE h = CreateFile("\\\\.\\ovpn-dco", GENERIC_READ | GENERIC_WRITE,
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index ea3d91b..271186e 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -201,7 +201,7 @@
 }
 
 bool
-dns_options_verify(int msglevel, const struct dns_options *o)
+dns_options_verify(unsigned int msglevel, const struct dns_options *o)
 {
     const struct dns_server *server =
         o->servers ? o->servers : o->servers_prepull;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index d33f64e..ee70ed8 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -157,7 +157,7 @@
  * @param   o           Pointer to the DNS options to validate
  * @return              True if no error was found
  */
-bool dns_options_verify(int msglevel, const struct dns_options *o);
+bool dns_options_verify(unsigned int msglevel, const struct dns_options *o);
 
 /**
  * Makes a deep copy of the passed DNS options.
diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c
index f20ee8a..cbf7d8f 100644
--- a/src/openvpn/env_set.c
+++ b/src/openvpn/env_set.c
@@ -210,7 +210,7 @@
 }
 
 void
-env_set_print(int msglevel, const struct env_set *es)
+env_set_print(unsigned int msglevel, const struct env_set *es)
 {
     if (check_debug_level(msglevel))
     {
diff --git a/src/openvpn/env_set.h b/src/openvpn/env_set.h
index 52c5312..42ebff0 100644
--- a/src/openvpn/env_set.h
+++ b/src/openvpn/env_set.h
@@ -89,7 +89,7 @@
 
 const char *env_set_get(const struct env_set *es, const char *name);
 
-void env_set_print(int msglevel, const struct env_set *es);
+void env_set_print(unsigned int msglevel, const struct env_set *es);
 
 /**
  * Write a struct env_set to a file. Each item on one line.
diff --git a/src/openvpn/error.c b/src/openvpn/error.c
index 049cd92..170076a 100644
--- a/src/openvpn/error.c
+++ b/src/openvpn/error.c
@@ -409,8 +409,8 @@
     bool ret = true;
     if (mute_cutoff > 0 && !(flags & M_NOMUTE))
     {
-        const int mute_level = DECODE_MUTE_LEVEL(flags);
-        if (mute_level > 0 && mute_level == mute_category)
+        const unsigned int mute_level = DECODE_MUTE_LEVEL(flags);
+        if (mute_level == mute_category)
         {
             if (mute_count == mute_cutoff)
             {
diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index e0a2730..7744ef8 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -84,22 +84,22 @@
 
 /* msg() flags */
 
-#define M_DEBUG_LEVEL     (0x0F)         /* debug level mask */
+#define M_DEBUG_LEVEL     (0x0Fu)         /* debug level mask */
 
-#define M_FATAL           (1<<4)         /* exit program */
-#define M_NONFATAL        (1<<5)         /* non-fatal error */
-#define M_WARN            (1<<6)         /* call syslog with LOG_WARNING */
-#define M_DEBUG           (1<<7)
+#define M_FATAL           (1u<<4)         /* exit program */
+#define M_NONFATAL        (1u<<5)         /* non-fatal error */
+#define M_WARN            (1u<<6)         /* call syslog with LOG_WARNING */
+#define M_DEBUG           (1u<<7)
 
-#define M_ERRNO           (1<<8)         /* show errno description */
+#define M_ERRNO           (1u<<8)         /* show errno description */
 
-#define M_NOMUTE          (1<<11)        /* don't do mute processing */
-#define M_NOPREFIX        (1<<12)        /* don't show date/time prefix */
-#define M_USAGE_SMALL     (1<<13)        /* fatal options error, call usage_small */
-#define M_MSG_VIRT_OUT    (1<<14)        /* output message through msg_status_output callback */
-#define M_OPTERR          (1<<15)        /* print "Options error:" prefix */
-#define M_NOLF            (1<<16)        /* don't print new line */
-#define M_NOIPREFIX       (1<<17)        /* don't print instance prefix */
+#define M_NOMUTE          (1u<<11)        /* don't do mute processing */
+#define M_NOPREFIX        (1u<<12)        /* don't show date/time prefix */
+#define M_USAGE_SMALL     (1u<<13)        /* fatal options error, call usage_small */
+#define M_MSG_VIRT_OUT    (1u<<14)        /* output message through msg_status_output callback */
+#define M_OPTERR          (1u<<15)        /* print "Options error:" prefix */
+#define M_NOLF            (1u<<16)        /* don't print new line */
+#define M_NOIPREFIX       (1u<<17)        /* don't print instance prefix */
 
 /* flag combinations which are frequently used */
 #define M_ERR     (M_FATAL | M_ERRNO)
@@ -113,7 +113,7 @@
  * A mute level of 0 is always printed.
  */
 #define MUTE_LEVEL_SHIFT 24
-#define MUTE_LEVEL_MASK 0xFF
+#define MUTE_LEVEL_MASK 0xFFu
 
 #define ENCODE_MUTE_LEVEL(mute_level) (((mute_level) & MUTE_LEVEL_MASK) << MUTE_LEVEL_SHIFT)
 #define DECODE_MUTE_LEVEL(flags) (((flags) >> MUTE_LEVEL_SHIFT) & MUTE_LEVEL_MASK)
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index dfc6708..aab195d 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -379,7 +379,7 @@
 
 bool
 send_control_channel_string_dowork(struct tls_session *session,
-                                   const char *str, int msglevel)
+                                   const char *str, unsigned int msglevel)
 {
     struct gc_arena gc = gc_new();
     bool stat;
@@ -407,7 +407,7 @@
 }
 
 bool
-send_control_channel_string(struct context *c, const char *str, int msglevel)
+send_control_channel_string(struct context *c, const char *str, unsigned int msglevel)
 {
     if (c->c2.tls_multi)
     {
diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index b3e424c..aaf47fe 100644
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -287,7 +287,7 @@
  * @param msglevel   - Message level to use for logging
  */
 bool
-send_control_channel_string(struct context *c, const char *str, int msglevel);
+send_control_channel_string(struct context *c, const char *str, unsigned int msglevel);
 
 /*
  * Send a string to remote over the TLS control channel.
@@ -307,7 +307,7 @@
 
 bool
 send_control_channel_string_dowork(struct tls_session *session,
-                                   const char *str, int msglevel);
+                                   const char *str, unsigned int msglevel);
 
 
 /**
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 77747a2..641d7ee 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -4379,7 +4379,7 @@
 }
 
 void
-management_show_net_callback(void *arg, const int msglevel)
+management_show_net_callback(void *arg, const unsigned int msglevel)
 {
 #ifdef _WIN32
     show_routes(msglevel);
diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 5c6b9c1..40d8d04 100644
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -125,7 +125,7 @@
 
 void close_management(void);
 
-void management_show_net_callback(void *arg, const int msglevel);
+void management_show_net_callback(void *arg, const unsigned int msglevel);
 
 #endif
 
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index e25a615..3b4c18c 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -178,7 +178,7 @@
     unsigned int flags;
 
     void (*status) (void *arg, const int version, struct status_output *so);
-    void (*show_net) (void *arg, const int msglevel);
+    void (*show_net) (void *arg, const unsigned int msglevel);
     int (*kill_by_cn) (void *arg, const char *common_name);
     int (*kill_by_addr) (void *arg, const in_addr_t addr, const int port, const int proto);
     void (*delete_event) (void *arg, event_t event);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ec260a2..b84f663 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3419,7 +3419,7 @@
     }
     else
     {
-        int msglevel = D_DCO;
+        unsigned int msglevel = D_DCO;
         if (dco->dco_message_type == OVPN_CMD_DEL_PEER
             && dco->dco_del_peer_reason == OVPN_DEL_PEER_REASON_USERSPACE)
         {
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 0662b49..70b5799 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1106,7 +1106,7 @@
 #endif /* ifndef _WIN32 */
 
 static in_addr_t
-get_ip_addr(const char *ip_string, int msglevel, bool *error)
+get_ip_addr(const char *ip_string, unsigned int msglevel, bool *error)
 {
     unsigned int flags = GETADDR_HOST_ORDER;
     bool succeeded = false;
@@ -1187,7 +1187,7 @@
  * @param   gc          The returned object will be allocated in this gc
  */
 static struct verify_hash_list *
-parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct gc_arena *gc)
+parse_hash_fingerprint(const char *str, int nbytes, unsigned int msglevel, struct gc_arena *gc)
 {
     int i = 0;
     const char *cp = str;
@@ -1241,7 +1241,7 @@
  * @param   gc          The returned list items will be allocated in this gc
  */
 static struct verify_hash_list *
-parse_hash_fingerprint_multiline(const char *str, int nbytes, int msglevel,
+parse_hash_fingerprint_multiline(const char *str, int nbytes, unsigned int msglevel,
                                  struct gc_arena *gc)
 {
     struct gc_arena gc_temp = gc_new();
@@ -1335,7 +1335,7 @@
 #endif /* ifdef _WIN32 */
 
 static void
-dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
+dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, unsigned int msglevel)
 {
     struct in6_addr addr;
     if (*len >= N_DHCP_ADDR)
@@ -1349,7 +1349,7 @@
     }
 }
 static void
-dhcp_option_address_parse(const char *name, const char *parm, in_addr_t *array, int *len, int msglevel)
+dhcp_option_address_parse(const char *name, const char *parm, in_addr_t *array, int *len, unsigned int msglevel)
 {
     if (*len >= N_DHCP_ADDR)
     {
@@ -1480,7 +1480,7 @@
 option_iroute(struct options *o,
               const char *network_str,
               const char *netmask_str,
-              int msglevel)
+              unsigned int msglevel)
 {
     struct iroute *ir;
 
@@ -1509,7 +1509,7 @@
 static void
 option_iroute_ipv6(struct options *o,
                    const char *prefix_str,
-                   int msglevel)
+                   unsigned int msglevel)
 {
     struct iroute_ipv6 *ir;
 
@@ -2068,7 +2068,7 @@
 }
 
 static struct local_entry *
-alloc_local_entry(struct connection_entry *ce, const int msglevel,
+alloc_local_entry(struct connection_entry *ce, const unsigned int msglevel,
                   struct gc_arena *gc)
 {
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
@@ -2108,7 +2108,7 @@
 }
 
 static struct connection_entry *
-alloc_connection_entry(struct options *options, const int msglevel)
+alloc_connection_entry(struct options *options, const unsigned int msglevel)
 {
     struct connection_list *l = alloc_connection_list_if_undef(options);
     struct connection_entry *e;
@@ -2141,7 +2141,7 @@
 }
 
 static struct remote_entry *
-alloc_remote_entry(struct options *options, const int msglevel)
+alloc_remote_entry(struct options *options, const unsigned int msglevel)
 {
     struct remote_list *l = alloc_remote_list_if_undef(options);
     struct remote_entry *e;
@@ -2698,7 +2698,7 @@
 
     if (!options->tls_server && !options->tls_client)
     {
-        int msglevel = M_USAGE;
+        unsigned int msglevel = M_USAGE;
         if (options->allow_deprecated_insecure_static_crypto)
         {
             msglevel = M_INFO;
@@ -4598,7 +4598,7 @@
 }
 
 static void
-options_warning_safe_scan2(const int msglevel,
+options_warning_safe_scan2(const unsigned int msglevel,
                            const int delim,
                            const bool report_inconsistent,
                            const char *p1,
@@ -4666,7 +4666,7 @@
 }
 
 static void
-options_warning_safe_scan1(const int msglevel,
+options_warning_safe_scan1(const unsigned int msglevel,
                            const int delim,
                            const bool report_inconsistent,
                            const struct buffer *b1_src,
@@ -4687,7 +4687,7 @@
 }
 
 static void
-options_warning_safe_ml(const int msglevel, char *actual, const char *expected, size_t actual_n)
+options_warning_safe_ml(const unsigned int msglevel, char *actual, const char *expected, size_t actual_n)
 {
     struct gc_arena gc = gc_new();
 
@@ -4784,7 +4784,7 @@
  */
 
 int
-parse_topology(const char *str, const int msglevel)
+parse_topology(const char *str, const unsigned int msglevel)
 {
     if (streq(str, "net30"))
     {
@@ -4840,7 +4840,7 @@
 }
 
 bool
-auth_retry_set(const int msglevel, const char *option)
+auth_retry_set(const unsigned int msglevel, const char *option)
 {
     if (streq(option, "interact"))
     {
@@ -5004,7 +5004,7 @@
 
 #if 0
 static void
-ping_rec_err(int msglevel)
+ping_rec_err(unsigned int msglevel)
 {
     msg(msglevel, "only one of --ping-exit or --ping-restart options may be specified");
 }
@@ -5032,7 +5032,7 @@
            const int n,
            const char *file,
            const int line_num,
-           int msglevel,
+           unsigned int msglevel,
            struct gc_arena *gc)
 {
     const int STATE_INITIAL = 0;
@@ -5337,7 +5337,7 @@
            const char *file,
            int line,
            const int level,
-           const int msglevel,
+           const unsigned int msglevel,
            const unsigned int permission_mask,
            unsigned int *option_types_found,
            struct env_set *es);
@@ -5348,7 +5348,7 @@
                  int level,
                  const char *top_file,
                  const int top_line,
-                 const int msglevel,
+                 const unsigned int msglevel,
                  const unsigned int permission_mask,
                  unsigned int *option_types_found,
                  struct env_set *es)
@@ -5421,7 +5421,7 @@
 read_config_string(const char *prefix,
                    struct options *options,
                    const char *config,
-                   const int msglevel,
+                   const unsigned int msglevel,
                    const unsigned int permission_mask,
                    unsigned int *option_types_found,
                    struct env_set *es)
@@ -5454,7 +5454,7 @@
 parse_argv(struct options *options,
            const int argc,
            char *argv[],
-           const int msglevel,
+           const unsigned int msglevel,
            const unsigned int permission_mask,
            unsigned int *option_types_found,
            struct env_set *es)
@@ -5573,7 +5573,7 @@
     char line[OPTION_PARM_SIZE];
     int line_num = 0;
     const char *file = "[PUSH-OPTIONS]";
-    const int msglevel = D_PUSH_ERRORS|M_OPTERR;
+    const unsigned int msglevel = D_PUSH_ERRORS|M_OPTERR;
 
     while (buf_parse(buf, ',', line, sizeof(line)))
     {
@@ -5596,7 +5596,7 @@
 void
 options_server_import(struct options *o,
                       const char *filename,
-                      int msglevel,
+                      unsigned int msglevel,
                       unsigned int permission_mask,
                       unsigned int *option_types_found,
                       struct env_set *es)
@@ -5616,7 +5616,7 @@
 void
 options_string_import(struct options *options,
                       const char *config,
-                      const int msglevel,
+                      const unsigned int msglevel,
                       const unsigned int permission_mask,
                       unsigned int *option_types_found,
                       struct env_set *es)
@@ -5639,7 +5639,7 @@
                   const unsigned int type,
                   const unsigned int allowed,
                   unsigned int *found,
-                  const int msglevel,
+                  const unsigned int msglevel,
                   struct options *options,
                   bool is_inline)
 {
@@ -5693,7 +5693,7 @@
 #define NM_QUOTE_HINT (1<<0)
 
 static bool
-no_more_than_n_args(const int msglevel,
+no_more_than_n_args(const unsigned int msglevel,
                     char *p[],
                     const int max,
                     const unsigned int flags)
@@ -5720,8 +5720,8 @@
     }
 }
 
-static inline int
-msglevel_forward_compatible(struct options *options, const int msglevel)
+static inline unsigned int
+msglevel_forward_compatible(struct options *options, const unsigned int msglevel)
 {
     return options->forward_compatible ? M_WARN : msglevel;
 }
@@ -5790,14 +5790,14 @@
            const char *file,
            int line,
            const int level,
-           const int msglevel,
+           const unsigned int msglevel,
            const unsigned int permission_mask,
            unsigned int *option_types_found,
            struct env_set *es)
 {
     struct gc_arena gc = gc_new();
     const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);
-    int msglevel_fc = msglevel_forward_compatible(options, msglevel);
+    unsigned int msglevel_fc = msglevel_forward_compatible(options, msglevel);
 
     ASSERT(MAX_PARMS >= 7);
 
@@ -9657,7 +9657,7 @@
     else
     {
         int i;
-        int msglevel_unknown = msglevel_fc;
+        unsigned int msglevel_unknown = msglevel_fc;
         /* Check if an option is in --ignore-unknown-option and
          * set warning level to non fatal */
         for (i = 0; options->ignore_unknown_option && options->ignore_unknown_option[i]; i++)
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 4ba8e3d..78b0d7c 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -797,7 +797,7 @@
 void parse_argv(struct options *options,
                 const int argc,
                 char *argv[],
-                const int msglevel,
+                const unsigned int msglevel,
                 const unsigned int permission_mask,
                 unsigned int *option_types_found,
                 struct env_set *es);
@@ -872,7 +872,7 @@
 
 void options_server_import(struct options *o,
                            const char *filename,
-                           int msglevel,
+                           unsigned int msglevel,
                            unsigned int permission_mask,
                            unsigned int *option_types_found,
                            struct env_set *es);
@@ -886,14 +886,14 @@
                const int n,
                const char *file,
                const int line_num,
-               int msglevel,
+               unsigned int msglevel,
                struct gc_arena *gc);
 
 /*
  * parse/print topology coding
  */
 
-int parse_topology(const char *str, const int msglevel);
+int parse_topology(const char *str, const unsigned int msglevel);
 
 const char *print_topology(const int topology);
 
@@ -907,13 +907,13 @@
 
 int auth_retry_get(void);
 
-bool auth_retry_set(const int msglevel, const char *option);
+bool auth_retry_set(const unsigned int msglevel, const char *option);
 
 const char *auth_retry_print(void);
 
 void options_string_import(struct options *options,
                            const char *config,
-                           const int msglevel,
+                           const unsigned int msglevel,
                            const unsigned int permission_mask,
                            unsigned int *option_types_found,
                            struct env_set *es);
diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c
index 80d0c08..cb2a59c 100644
--- a/src/openvpn/options_util.c
+++ b/src/openvpn/options_util.c
@@ -116,7 +116,7 @@
 }
 
 int
-positive_atoi(const char *str, int msglevel)
+positive_atoi(const char *str, unsigned int msglevel)
 {
     char *endptr;
     long long i = strtoll(str, &endptr, 10);
@@ -132,7 +132,7 @@
 }
 
 int
-atoi_warn(const char *str, int msglevel)
+atoi_warn(const char *str, unsigned int msglevel)
 {
     char *endptr;
     long long i = strtoll(str, &endptr, 10);
diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h
index 2474e7f..2f0b29b 100644
--- a/src/openvpn/options_util.h
+++ b/src/openvpn/options_util.h
@@ -41,13 +41,13 @@
  * integer number. Otherwise print a warning with msglevel and return 0
  */
 int
-positive_atoi(const char *str, int msglevel);
+positive_atoi(const char *str, unsigned int msglevel);
 
 /**
  * Converts a str to an integer if the string can be represented as an
  * integer number. Otherwise print a warning with msglevel and return 0
  */
 int
-atoi_warn(const char *str, int msglevel);
+atoi_warn(const char *str, unsigned int msglevel);
 
 #endif /* ifndef OPTIONS_UTIL_H_ */
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index 76a81c6..057fdbd 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -54,7 +54,7 @@
 #define SEQ_EXPIRED ((time_t)1)
 
 #ifdef ENABLE_DEBUG
-static void packet_id_debug_print(int msglevel,
+static void packet_id_debug_print(unsigned int msglevel,
                                   const struct packet_id_rec *p,
                                   const struct packet_id_net *pin,
                                   const char *message,
@@ -63,7 +63,7 @@
 #endif /* ENABLE_DEBUG */
 
 static inline void
-packet_id_debug(int msglevel,
+packet_id_debug(unsigned int msglevel,
                 const struct packet_id_rec *p,
                 const struct packet_id_net *pin,
                 const char *message,
@@ -592,7 +592,7 @@
 #ifdef ENABLE_DEBUG
 
 static void
-packet_id_debug_print(int msglevel,
+packet_id_debug_print(unsigned int msglevel,
                       const struct packet_id_rec *p,
                       const struct packet_id_net *pin,
                       const char *message,
diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c
index bafa469..3aafccf 100644
--- a/src/openvpn/plugin.c
+++ b/src/openvpn/plugin.c
@@ -51,7 +51,7 @@
 static struct plugin_common *static_plugin_common = NULL; /* GLOBAL */
 
 static void
-plugin_show_string_array(int msglevel, const char *name, const char *array[])
+plugin_show_string_array(unsigned int msglevel, const char *name, const char *array[])
 {
     int i;
     for (i = 0; array[i]; ++i)
@@ -64,7 +64,7 @@
 }
 
 static void
-plugin_show_args_env(int msglevel, const char *argv[], const char *envp[])
+plugin_show_args_env(unsigned int msglevel, const char *argv[], const char *envp[])
 {
     if (check_debug_level(msglevel))
     {
@@ -186,7 +186,7 @@
 
 #ifndef ENABLE_SMALL
 void
-plugin_option_list_print(const struct plugin_option_list *list, int msglevel)
+plugin_option_list_print(const struct plugin_option_list *list, unsigned int msglevel)
 {
     int i;
     struct gc_arena gc = gc_new();
@@ -1024,7 +1024,7 @@
 
 #ifdef ENABLE_DEBUG
 void
-plugin_return_print(const int msglevel, const char *prefix, const struct plugin_return *pr)
+plugin_return_print(const unsigned int msglevel, const char *prefix, const struct plugin_return *pr)
 {
     int i;
     msg(msglevel, "PLUGIN_RETURN_PRINT %s", prefix);
diff --git a/src/openvpn/plugin.h b/src/openvpn/plugin.h
index 5e91c60..003fcbb 100644
--- a/src/openvpn/plugin.h
+++ b/src/openvpn/plugin.h
@@ -110,7 +110,7 @@
                             struct gc_arena *gc);
 
 #ifndef ENABLE_SMALL
-void plugin_option_list_print(const struct plugin_option_list *list, int msglevel);
+void plugin_option_list_print(const struct plugin_option_list *list, unsigned int msglevel);
 
 #endif
 
@@ -144,7 +144,7 @@
 void plugin_return_free(struct plugin_return *pr);
 
 #ifdef ENABLE_DEBUG
-void plugin_return_print(const int msglevel, const char *prefix, const struct plugin_return *pr);
+void plugin_return_print(const unsigned int msglevel, const char *prefix, const struct plugin_return *pr);
 
 #endif
 
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index ced172c..c144c01 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -118,7 +118,7 @@
  * Verify start/end range
  */
 bool
-ifconfig_pool_verify_range(const int msglevel, const in_addr_t start, const in_addr_t end)
+ifconfig_pool_verify_range(const unsigned int msglevel, const in_addr_t start, const in_addr_t end)
 {
     struct gc_arena gc = gc_new();
     bool ret = true;
@@ -534,7 +534,7 @@
 }
 
 static void
-ifconfig_pool_msg(const struct ifconfig_pool *pool, int msglevel)
+ifconfig_pool_msg(const struct ifconfig_pool *pool, unsigned int msglevel)
 {
     struct status_output *so = status_open(NULL, 0, msglevel, NULL, 0);
     ASSERT(so);
diff --git a/src/openvpn/pool.h b/src/openvpn/pool.h
index 9c3905d..b47cff0 100644
--- a/src/openvpn/pool.h
+++ b/src/openvpn/pool.h
@@ -79,7 +79,7 @@
 
 void ifconfig_pool_free(struct ifconfig_pool *pool);
 
-bool ifconfig_pool_verify_range(const int msglevel, const in_addr_t start, const in_addr_t end);
+bool ifconfig_pool_verify_range(const unsigned int msglevel, const in_addr_t start, const in_addr_t end);
 
 ifconfig_pool_handle ifconfig_pool_acquire(struct ifconfig_pool *pool, in_addr_t *local, in_addr_t *remote, struct in6_addr *remote_ipv6, const char *common_name);
 
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index ad8fa3d7..34e5b7a 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -378,7 +378,7 @@
  * @return true on success, false on failure.
  */
 static bool push_option_fmt(struct gc_arena *gc, struct push_list *push_list,
-                            int msglevel, const char *fmt, ...)
+                            unsigned int msglevel, const char *fmt, ...)
 #ifdef __GNUC__
 #if __USE_MINGW_ANSI_STDIO
 __attribute__ ((format(gnu_printf, 4, 5)))
@@ -852,7 +852,7 @@
 
 static void
 push_option_ex(struct gc_arena *gc, struct push_list *push_list,
-               const char *opt, bool enable, int msglevel)
+               const char *opt, bool enable, unsigned int msglevel)
 {
     if (!string_class(opt, CC_ANY, CC_COMMA))
     {
@@ -880,7 +880,7 @@
 }
 
 void
-push_option(struct options *o, const char *opt, int msglevel)
+push_option(struct options *o, const char *opt, unsigned int msglevel)
 {
     push_option_ex(&o->gc, &o->push_list, opt, true, msglevel);
 }
@@ -902,7 +902,7 @@
 }
 
 void
-push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc)
+push_options(struct options *o, char **p, unsigned int msglevel, struct gc_arena *gc)
 {
     const char **argv = make_extended_arg_array(p, false, gc);
     char *opt = print_argv(argv, gc, 0);
@@ -911,7 +911,7 @@
 
 static bool
 push_option_fmt(struct gc_arena *gc, struct push_list *push_list,
-                int msglevel, const char *format, ...)
+                unsigned int msglevel, const char *format, ...)
 {
     va_list arglist;
     char tmp[256] = {0};
diff --git a/src/openvpn/push.h b/src/openvpn/push.h
index 048b4c4..86074d2 100644
--- a/src/openvpn/push.h
+++ b/src/openvpn/push.h
@@ -58,9 +58,9 @@
 
 void clone_push_list(struct options *o);
 
-void push_option(struct options *o, const char *opt, int msglevel);
+void push_option(struct options *o, const char *opt, unsigned int msglevel);
 
-void push_options(struct options *o, char **p, int msglevel,
+void push_options(struct options *o, char **p, unsigned int msglevel,
                   struct gc_arena *gc);
 
 void push_reset(struct options *o);
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 156262a..2b624ba 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1340,7 +1340,7 @@
 }
 
 void
-print_default_gateway(const int msglevel,
+print_default_gateway(const unsigned int msglevel,
                       const struct route_gateway_info *rgi,
                       const struct route_ipv6_gateway_info *rgi6)
 {
@@ -3314,18 +3314,18 @@
  * Show current routing table
  */
 void
-show_routes(int msglev)
+show_routes(unsigned int msglevel)
 {
     struct gc_arena gc = gc_new();
 
     const MIB_IPFORWARDTABLE *rt = get_windows_routing_table(&gc);
 
-    msg(msglev, "SYSTEM ROUTING TABLE");
+    msg(msglevel, "SYSTEM ROUTING TABLE");
     if (rt)
     {
         for (DWORD i = 0; i < rt->dwNumEntries; ++i)
         {
-            msg(msglev, "%s", format_route_entry(&rt->table[i], &gc));
+            msg(msglevel, "%s", format_route_entry(&rt->table[i], &gc));
         }
     }
     gc_free(&gc);
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index f015a12..faae424 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -356,7 +356,7 @@
                               const struct in6_addr *dest,
                               openvpn_net_ctx_t *ctx);
 
-void print_default_gateway(const int msglevel,
+void print_default_gateway(const unsigned int msglevel,
                            const struct route_gateway_info *rgi,
                            const struct route_ipv6_gateway_info *rgi6);
 
@@ -381,7 +381,7 @@
 
 #ifdef _WIN32
 
-void show_routes(int msglev);
+void show_routes(unsigned int msglevel);
 
 bool test_routes(const struct route_list *rl, const struct tuntap *tt);
 
diff --git a/src/openvpn/run_command.c b/src/openvpn/run_command.c
index 8b5fa4f..a41b05322 100644
--- a/src/openvpn/run_command.c
+++ b/src/openvpn/run_command.c
@@ -108,7 +108,7 @@
 
 #ifndef WIN32
 bool
-openvpn_waitpid_check(pid_t pid, const char *msg_prefix, int msglevel)
+openvpn_waitpid_check(pid_t pid, const char *msg_prefix, unsigned int msglevel)
 {
     if (pid == 0)
     {
diff --git a/src/openvpn/run_command.h b/src/openvpn/run_command.h
index e9a6fe1..5570065 100644
--- a/src/openvpn/run_command.h
+++ b/src/openvpn/run_command.h
@@ -75,7 +75,7 @@
  */
 bool
 openvpn_waitpid_check(pid_t pid, const char *msg_prefix,
-                      int msglevel);
+                      unsigned int msglevel);
 
 #endif
 
diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index 9334b10..d8b6954 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -291,7 +291,7 @@
 }
 
 void
-print_signal(const struct signal_info *si, const char *title, int msglevel)
+print_signal(const struct signal_info *si, const char *title, unsigned int msglevel)
 {
     if (si)
     {
diff --git a/src/openvpn/sig.h b/src/openvpn/sig.h
index fe81af9..8c808f6 100644
--- a/src/openvpn/sig.h
+++ b/src/openvpn/sig.h
@@ -67,7 +67,7 @@
 
 void restore_signal_state(void);
 
-void print_signal(const struct signal_info *si, const char *title, int msglevel);
+void print_signal(const struct signal_info *si, const char *title, unsigned int msglevel);
 
 void print_status(struct context *c, struct status_output *so);
 
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 980d8b4..2f31aca 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -81,7 +81,7 @@
 get_addr_generic(sa_family_t af, unsigned int flags, const char *hostname,
                  void *network, unsigned int *netbits,
                  int resolve_retry_seconds, struct signal_info *sig_info,
-                 int msglevel)
+                 unsigned int msglevel)
 {
     char *endp, *sep, *var_host = NULL;
     struct addrinfo *ai = NULL;
@@ -224,7 +224,7 @@
 
 bool
 get_ipv6_addr(const char *hostname, struct in6_addr *network,
-              unsigned int *netbits, int msglevel)
+              unsigned int *netbits, unsigned int msglevel)
 {
     if (get_addr_generic(AF_INET6, GETADDR_RESOLVE, hostname, network, netbits,
                          0, NULL, msglevel) < 0)
@@ -477,7 +477,7 @@
     struct addrinfo hints;
     int status;
     struct signal_info sigrec = {0};
-    int msglevel = (flags & GETADDR_FATAL) ? M_FATAL : D_RESOLVE_ERRORS;
+    unsigned int msglevel = (flags & GETADDR_FATAL) ? M_FATAL : D_RESOLVE_ERRORS;
     struct gc_arena gc = gc_new();
     const char *print_hostname;
     const char *print_servname;
@@ -541,7 +541,7 @@
         int resolve_retries = (flags & GETADDR_TRY_ONCE) ? 1 :
                               ((resolve_retry_seconds + 4)/ fail_wait_interval);
         const char *fmt;
-        int level = 0;
+        unsigned int level = 0;
 
         /* this is not a numeric IP, therefore force resolution using the
          * provided ai_family */
@@ -689,7 +689,7 @@
 done:
     if (sig_info && sig_info->signal_received)
     {
-        int level = 0;
+        unsigned int level = 0;
         if (flags & GETADDR_FATAL_ON_SIGNAL)
         {
             level = M_FATAL;
@@ -2059,7 +2059,7 @@
 linksock_print_addr(struct link_socket *sock)
 {
     struct gc_arena gc = gc_new();
-    const int msglevel = (sock->mode == LS_MODE_TCP_ACCEPT_FROM) ? D_INIT_MEDIUM : M_INFO;
+    const unsigned int msglevel = (sock->mode == LS_MODE_TCP_ACCEPT_FROM) ? D_INIT_MEDIUM : M_INFO;
 
     /* print local address */
     if (sock->bind_local)
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index 6d71e83..26a13d6 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -545,7 +545,7 @@
  * Translate an IPv6 addr or hostname from string form to in6_addr
  */
 bool get_ipv6_addr(const char *hostname, struct in6_addr *network,
-                   unsigned int *netbits, int msglevel);
+                   unsigned int *netbits, unsigned int msglevel);
 
 int openvpn_getaddrinfo(unsigned int flags,
                         const char *hostname,
diff --git a/src/openvpn/ssl_verify_backend.h b/src/openvpn/ssl_verify_backend.h
index eb80cc8..b4711a5 100644
--- a/src/openvpn/ssl_verify_backend.h
+++ b/src/openvpn/ssl_verify_backend.h
@@ -195,7 +195,7 @@
  *
  */
 void x509_track_add(const struct x509_track **ll_head, const char *name,
-                    int msglevel, struct gc_arena *gc);
+                    unsigned int msglevel, struct gc_arena *gc);
 
 /*
  * Save X509 fields to environment, using the naming convention:
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index 25ad09c..2db7b17 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -358,7 +358,7 @@
 }
 
 void
-x509_track_add(const struct x509_track **ll_head, const char *name, int msglevel, struct gc_arena *gc)
+x509_track_add(const struct x509_track **ll_head, const char *name, unsigned int msglevel, struct gc_arena *gc)
 {
     struct x509_track *xt;
     ALLOC_OBJ_CLEAR_GC(xt, struct x509_track, gc);
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index 1546815..57a2570 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -421,7 +421,7 @@
  */
 
 void
-x509_track_add(const struct x509_track **ll_head, const char *name, int msglevel, struct gc_arena *gc)
+x509_track_add(const struct x509_track **ll_head, const char *name, unsigned int msglevel, struct gc_arena *gc)
 {
     struct x509_track *xt;
     ALLOC_OBJ_CLEAR_GC(xt, struct x509_track, gc);
diff --git a/src/openvpn/status.c b/src/openvpn/status.c
index 4b0e516..06bb916 100644
--- a/src/openvpn/status.c
+++ b/src/openvpn/status.c
@@ -239,14 +239,13 @@
 
         if (so->msglevel >= 0 && !so->errors)
         {
-            msg(so->msglevel, "%s", buf);
+            msg((unsigned int)so->msglevel, "%s", buf);
         }
 
         if (so->fd >= 0 && !so->errors)
         {
-            int len;
             strcat(buf, "\n");
-            len = strlen(buf);
+            size_t len = strlen(buf);
             if (len > 0)
             {
                 if (write(so->fd, buf, len) != len)
diff --git a/src/openvpn/status.h b/src/openvpn/status.h
index eaed22a..9cd9325 100644
--- a/src/openvpn/status.h
+++ b/src/openvpn/status.h
@@ -53,6 +53,7 @@
 
     char *filename;
     int fd;
+    /* NB: -1 is used to indicate that output should only go to the file */
     int msglevel;
     const struct virtual_output *vout;
 
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index d15d9eb6..d8c292d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -108,9 +108,9 @@
                                    const int addr_len,
                                    DWORD adapter_index);
 
-static void netsh_command(const struct argv *a, int n, int msglevel);
+static void netsh_command(const struct argv *a, int n, unsigned int msglevel);
 
-static void exec_command(const char *prefix, const struct argv *a, int n, int msglevel);
+static void exec_command(const char *prefix, const struct argv *a, int n, unsigned int msglevel);
 
 static const char *netsh_get_id(const char *dev_node, struct gc_arena *gc);
 
@@ -4201,7 +4201,7 @@
 }
 
 void
-show_tap_win_adapters(int msglev, int warnlev)
+show_tap_win_adapters(unsigned int msglevel, unsigned int warnlevel)
 {
     struct gc_arena gc = gc_new();
 
@@ -4218,7 +4218,7 @@
     const struct tap_reg *tap_reg = get_tap_reg(&gc);
     const struct panel_reg *panel_reg = get_panel_reg(&gc);
 
-    msg(msglev, "Available adapters [name, GUID, driver]:");
+    msg(msglevel, "Available adapters [name, GUID, driver]:");
 
     /* loop through each TAP-Windows adapter registry entry */
     for (tr = tap_reg; tr != NULL; tr = tr->next)
@@ -4230,7 +4230,7 @@
         {
             if (!strcmp(tr->guid, pr->guid))
             {
-                msg(msglev, "'%s' %s %s", pr->name, tr->guid, print_tun_backend_driver(tr->windows_driver));
+                msg(msglevel, "'%s' %s %s", pr->name, tr->guid, print_tun_backend_driver(tr->windows_driver));
                 ++links;
             }
         }
@@ -4244,7 +4244,7 @@
             /* a TAP adapter exists without a link from the network
              * connections control panel */
             warn_panel_null = true;
-            msg(msglev, "[NULL] %s", tr->guid);
+            msg(msglevel, "[NULL] %s", tr->guid);
         }
     }
 
@@ -4263,17 +4263,17 @@
     /* warn on registry inconsistencies */
     if (warn_tap_dup)
     {
-        msg(warnlev, "WARNING: Some TAP-Windows adapters have duplicate GUIDs");
+        msg(warnlevel, "WARNING: Some TAP-Windows adapters have duplicate GUIDs");
     }
 
     if (warn_panel_dup)
     {
-        msg(warnlev, "WARNING: Some TAP-Windows adapters have duplicate links from the Network Connections control panel");
+        msg(warnlevel, "WARNING: Some TAP-Windows adapters have duplicate links from the Network Connections control panel");
     }
 
     if (warn_panel_null)
     {
-        msg(warnlev, "WARNING: Some TAP-Windows adapters have no link from the Network Connections control panel");
+        msg(warnlevel, "WARNING: Some TAP-Windows adapters have no link from the Network Connections control panel");
     }
 
     gc_free(&gc);
@@ -5023,31 +5023,31 @@
  * Show info for a single adapter
  */
 static void
-show_adapter(int msglev, const IP_ADAPTER_INFO *a, struct gc_arena *gc)
+show_adapter(unsigned int msglevel, const IP_ADAPTER_INFO *a, struct gc_arena *gc)
 {
-    msg(msglev, "%s", a->Description);
-    msg(msglev, "  Index = %d", (int)a->Index);
-    msg(msglev, "  GUID = %s", a->AdapterName);
-    msg(msglev, "  IP = %s", format_ip_addr_string(&a->IpAddressList, gc));
-    msg(msglev, "  MAC = %s", format_hex_ex(a->Address, a->AddressLength, 0, 1, ":", gc));
-    msg(msglev, "  GATEWAY = %s", format_ip_addr_string(&a->GatewayList, gc));
+    msg(msglevel, "%s", a->Description);
+    msg(msglevel, "  Index = %d", (int)a->Index);
+    msg(msglevel, "  GUID = %s", a->AdapterName);
+    msg(msglevel, "  IP = %s", format_ip_addr_string(&a->IpAddressList, gc));
+    msg(msglevel, "  MAC = %s", format_hex_ex(a->Address, a->AddressLength, 0, 1, ":", gc));
+    msg(msglevel, "  GATEWAY = %s", format_ip_addr_string(&a->GatewayList, gc));
     if (a->DhcpEnabled)
     {
-        msg(msglev, "  DHCP SERV = %s", format_ip_addr_string(&a->DhcpServer, gc));
-        msg(msglev, "  DHCP LEASE OBTAINED = %s", time_string(a->LeaseObtained, 0, false, gc));
-        msg(msglev, "  DHCP LEASE EXPIRES  = %s", time_string(a->LeaseExpires, 0, false, gc));
+        msg(msglevel, "  DHCP SERV = %s", format_ip_addr_string(&a->DhcpServer, gc));
+        msg(msglevel, "  DHCP LEASE OBTAINED = %s", time_string(a->LeaseObtained, 0, false, gc));
+        msg(msglevel, "  DHCP LEASE EXPIRES  = %s", time_string(a->LeaseExpires, 0, false, gc));
     }
     if (a->HaveWins)
     {
-        msg(msglev, "  PRI WINS = %s", format_ip_addr_string(&a->PrimaryWinsServer, gc));
-        msg(msglev, "  SEC WINS = %s", format_ip_addr_string(&a->SecondaryWinsServer, gc));
+        msg(msglevel, "  PRI WINS = %s", format_ip_addr_string(&a->PrimaryWinsServer, gc));
+        msg(msglevel, "  SEC WINS = %s", format_ip_addr_string(&a->SecondaryWinsServer, gc));
     }
 
     {
         const IP_PER_ADAPTER_INFO *pai = get_per_adapter_info(a->Index, gc);
         if (pai)
         {
-            msg(msglev, "  DNS SERV = %s", format_ip_addr_string(&pai->DnsServerList, gc));
+            msg(msglevel, "  DNS SERV = %s", format_ip_addr_string(&pai->DnsServerList, gc));
         }
     }
 }
@@ -5056,12 +5056,12 @@
  * Show current adapter list
  */
 void
-show_adapters(int msglev)
+show_adapters(unsigned int msglevel)
 {
     struct gc_arena gc = gc_new();
     const IP_ADAPTER_INFO *ai = get_adapter_info_list(&gc);
 
-    msg(msglev, "SYSTEM ADAPTER LIST");
+    msg(msglevel, "SYSTEM ADAPTER LIST");
     if (ai)
     {
         const IP_ADAPTER_INFO *a;
@@ -5069,7 +5069,7 @@
         /* find index in the linked list */
         for (a = ai; a != NULL; a = a->Next)
         {
-            show_adapter(msglev, a, &gc);
+            show_adapter(msglevel, a, &gc);
         }
     }
     gc_free(&gc);
@@ -5286,7 +5286,7 @@
 }
 
 static void
-exec_command(const char *prefix, const struct argv *a, int n, int msglevel)
+exec_command(const char *prefix, const struct argv *a, int n, unsigned int msglevel)
 {
     int i;
     for (i = 0; i < n; ++i)
@@ -5307,7 +5307,7 @@
 }
 
 static void
-netsh_command(const struct argv *a, int n, int msglevel)
+netsh_command(const struct argv *a, int n, unsigned int msglevel)
 {
     exec_command("NETSH", a, n, msglevel);
 }
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 5407e47..77c1d76 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -449,9 +449,9 @@
                           int *count,
                           in_addr_t *netmask);
 
-void show_tap_win_adapters(int msglev, int warnlev);
+void show_tap_win_adapters(unsigned int msglevel, unsigned int warnlevel);
 
-void show_adapters(int msglev);
+void show_adapters(unsigned int msglevel);
 
 void tap_allow_nonadmin_access(const char *dev_node);
 
diff --git a/tests/unit_tests/openvpn/mock_ssl_dependencies.c b/tests/unit_tests/openvpn/mock_ssl_dependencies.c
index 5cfe8d0..c8612ab 100644
--- a/tests/unit_tests/openvpn/mock_ssl_dependencies.c
+++ b/tests/unit_tests/openvpn/mock_ssl_dependencies.c
@@ -39,7 +39,7 @@
 
 int
 parse_line(const char *line, char **p, const int n, const char *file,
-           const int line_num, int msglevel, struct gc_arena *gc)
+           const int line_num, unsigned int msglevel, struct gc_arena *gc)
 {
     /* Dummy function to get the linker happy, should never be called */
     assert_true(false);
diff --git a/tests/unit_tests/openvpn/test_argv.c b/tests/unit_tests/openvpn/test_argv.c
index 33b3dec..eb232dd 100644
--- a/tests/unit_tests/openvpn/test_argv.c
+++ b/tests/unit_tests/openvpn/test_argv.c
@@ -23,7 +23,7 @@
 
 int
 __wrap_parse_line(const char *line, char **p, const int n, const char *file,
-                  const int line_num, int msglevel, struct gc_arena *gc)
+                  const int line_num, unsigned int msglevel, struct gc_arena *gc)
 {
     p[0] = PATH1 PATH2;
     p[1] = PARAM1;
diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c
index 0c604ff..3272f6b 100644
--- a/tests/unit_tests/openvpn/test_misc.c
+++ b/tests/unit_tests/openvpn/test_misc.c
@@ -335,7 +335,7 @@
     assert_false(valid_integer("-2147483653", false));
 
 
-    int msglevel = D_LOW;
+    unsigned int msglevel = D_LOW;
     int saved_log_level = mock_get_debug_level();
     mock_set_debug_level(D_LOW);
 
diff --git a/tests/unit_tests/openvpn/test_pkcs11.c b/tests/unit_tests/openvpn/test_pkcs11.c
index adad40b..052e910 100644
--- a/tests/unit_tests/openvpn/test_pkcs11.c
+++ b/tests/unit_tests/openvpn/test_pkcs11.c
@@ -60,7 +60,7 @@
 /* stubs for some unused functions instead of pulling in too many dependencies */
 int
 parse_line(const char *line, char **p, const int n, const char *file,
-           const int line_num, int msglevel, struct gc_arena *gc)
+           const int line_num, unsigned int msglevel, struct gc_arena *gc)
 {
     assert_true(0);
     return 0;
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index ebffabe..5f741f6 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -46,7 +46,7 @@
 
 int
 parse_line(const char *line, char **p, const int n, const char *file,
-           const int line_num, int msglevel, struct gc_arena *gc)
+           const int line_num, unsigned int msglevel, struct gc_arena *gc)
 {
     /* Dummy function to get the linker happy, should never be called */
     assert_true(false);
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index cc415c8..998e10d 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -100,7 +100,7 @@
 
 int
 __wrap_parse_line(const char *line, char **p, const int n, const char *file,
-                  const int line_num, int msglevel, struct gc_arena *gc)
+                  const int line_num, unsigned int msglevel, struct gc_arena *gc)
 {
     p[0] = PATH1 PATH2;
     p[1] = PARAM1;
diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c
index 400c0f4..bc9ba54 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -77,7 +77,7 @@
 /* stubs for some unused functions instead of pulling in too many dependencies */
 int
 parse_line(const char *line, char **p, const int n, const char *file,
-           const int line_num, int msglevel, struct gc_arena *gc)
+           const int line_num, unsigned int msglevel, struct gc_arena *gc)
 {
     assert_true(0);
     return 0;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1110?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: Id492cb774c6d022d06bb3cf5fec2a4bdd410e619
Gerrit-Change-Number: 1110
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: route: Make sure various route flags are treated as unsigned

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

Attention is currently required from: plaisthos.

Hello plaisthos,

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

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

to review the following change.


Change subject: route: Make sure various route flags are treated as unsigned
......................................................................

route: Make sure various route flags are treated as unsigned

The variables that hold them are already unsigned, make sure
the flags are as well to avoid spurious conversion warnings.

Change-Id: Ib7f78abbcd52c00a32afdea36ef635681ac8e127
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/route.h
1 file changed, 14 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/09/1109/1

diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 237375c..f015a12 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -83,14 +83,14 @@
 };
 
 /* redirect-gateway flags */
-#define RG_ENABLE         (1<<0)
-#define RG_LOCAL          (1<<1)
-#define RG_DEF1           (1<<2)
-#define RG_BYPASS_DHCP    (1<<3)
-#define RG_BYPASS_DNS     (1<<4)
-#define RG_REROUTE_GW     (1<<5)
-#define RG_AUTO_LOCAL     (1<<6)
-#define RG_BLOCK_LOCAL    (1<<7)
+#define RG_ENABLE         (1u<<0)
+#define RG_LOCAL          (1u<<1)
+#define RG_DEF1           (1u<<2)
+#define RG_BYPASS_DHCP    (1u<<3)
+#define RG_BYPASS_DNS     (1u<<4)
+#define RG_REROUTE_GW     (1u<<5)
+#define RG_AUTO_LOCAL     (1u<<6)
+#define RG_BLOCK_LOCAL    (1u<<7)
 
 struct route_option_list {
     unsigned int flags; /* RG_x flags */
@@ -113,9 +113,9 @@
 };
 
 struct route_ipv4 {
-#define RT_DEFINED        (1<<0)
-#define RT_ADDED          (1<<1)
-#define RT_METRIC_DEFINED (1<<2)
+#define RT_DEFINED        (1u<<0)
+#define RT_ADDED          (1u<<1)
+#define RT_METRIC_DEFINED (1u<<2)
     struct route_ipv4 *next;
     unsigned int flags;
     const struct route_option *option;
@@ -216,9 +216,9 @@
 };
 
 struct route_list {
-#define RL_DID_REDIRECT_DEFAULT_GATEWAY (1<<0)
-#define RL_DID_LOCAL                    (1<<1)
-#define RL_ROUTES_ADDED                 (1<<2)
+#define RL_DID_REDIRECT_DEFAULT_GATEWAY (1u<<0)
+#define RL_DID_LOCAL                    (1u<<1)
+#define RL_ROUTES_ADDED                 (1u<<2)
     unsigned int iflags;
 
     struct route_special_addr spec;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1109?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: Ib7f78abbcd52c00a32afdea36ef635681ac8e127
Gerrit-Change-Number: 1109
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange