from:"Antonio Quartulli"

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
NOTE: not tested because I have no FreeBSD environment

Changes from v1:
* added boundary check on peer-id

Changes from v2:
* use one check only instead of two
---
 src/openvpn/dco_freebsd.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..54ec16ff 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,18 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
 {
-struct hash_element *he;
-struct hash_iterator hi;
 
-hash_iterator_init(m->hash, );
-
-while ((he = hash_iterator_next()))
+if (peer_id >= m->max_clients || !m->instances[peer_id])
 {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_WARN, "Cannot store DCO stats for peer %u",
+peerid);
 return;
 }
 
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);
+struct multi_instance *mi = m->instances[peer_id];
+
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
 }
 
 int
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v4] dco-linux: implement dco_get_peer_stats{, multi} API

With this API it is possible to retrieve the stats for a specific peer
or for all peers and then update the userspace counters with the value
reported by DCO.

Change-Id: Ia3990b86b1be7ca844fb1674b39ce0d60528ccff
Signed-off-by: Antonio Quartulli 
---

Changes from v1:
* use m->instances[] instead of iterating over m->hash

Changes from v2:
* added boundary check on peer-id

Changes from v3:
* use one check only instead of two
---
 src/openvpn/dco_linux.c  | 183 ---
 src/openvpn/ovpn_dco_linux.h |  14 ++-
 2 files changed, 179 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 47961849..317f9dc0 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -41,6 +41,7 @@
 #include "tun.h"
 #include "ssl.h"
 #include "fdmisc.h"
+#include "multi.h"
 #include "ssl_verify.h"
 
 #include "ovpn_dco_linux.h"
@@ -168,16 +169,17 @@ ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix)
  * @param dco   The dco context to use
  * @param nl_msgthe message to use
  * @param cbAn optional callback if the caller expects an answer
+ * @param cb_argAn optional param to pass to the callback
  * @param prefixA 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,
- const char *prefix)
+ void *cb_arg, const char *prefix)
 {
 dco->status = 1;
 
-nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, dco);
+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)
@@ -268,7 +270,7 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int 
sd,
 }
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -489,7 +491,7 @@ dco_swap_keys(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_SWAP_KEYS_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -513,7 +515,7 @@ dco_del_peer(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_DEL_PEER_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -539,7 +541,7 @@ dco_del_key(dco_context_t *dco, unsigned int peerid,
 NLA_PUT_U8(nl_msg, OVPN_DEL_KEY_ATTR_KEY_SLOT, slot);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -596,7 +598,7 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int 
keyid,
 
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -625,7 +627,7 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid,
 keepalive_timeout);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -706,7 +708,7 @@ ovpn_get_mcast_id(dco_context_t *dco)
 int ret = -EMSGSIZE;
 NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_NL_NAME);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -819,18 +821,173 @@ dco_do_read(dco_context_t *dco)
 return ovpn_nl_recvmsgs(dco, __func__);
 }
 
+static void
+dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id)
+{
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES])
+{
+c2->dco_read_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_read_bytes: %lu", __func__,
+c2->dco_read_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link RX bytes provided in reply for peer %u",
+__func__, id);
+}
+
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES])
+{
+c2->dco_write_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_write_bytes: %lu", __func__,
+c2->dco_write_bytes);
+}
+else
+{

[Openvpn-devel] [PATCH v3] dco-linux: implement dco_get_peer_stats{, multi} API

With this API it is possible to retrieve the stats for a specific peer
or for all peers and then update the userspace counters with the value
reported by DCO.

Change-Id: Ia3990b86b1be7ca844fb1674b39ce0d60528ccff
Signed-off-by: Antonio Quartulli 
---

Changes from v1:
* use m->instances[] instead of iterating over m->hash

Changes from v2:
* added boundary check on peer-id
---
 src/openvpn/dco_linux.c  | 190 ---
 src/openvpn/ovpn_dco_linux.h |  14 ++-
 2 files changed, 186 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 47961849..8e7b4eab 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -41,6 +41,7 @@
 #include "tun.h"
 #include "ssl.h"
 #include "fdmisc.h"
+#include "multi.h"
 #include "ssl_verify.h"
 
 #include "ovpn_dco_linux.h"
@@ -168,16 +169,17 @@ ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix)
  * @param dco   The dco context to use
  * @param nl_msgthe message to use
  * @param cbAn optional callback if the caller expects an answer
+ * @param cb_argAn optional param to pass to the callback
  * @param prefixA 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,
- const char *prefix)
+ void *cb_arg, const char *prefix)
 {
 dco->status = 1;
 
-nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, dco);
+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)
@@ -268,7 +270,7 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int 
sd,
 }
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -489,7 +491,7 @@ dco_swap_keys(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_SWAP_KEYS_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -513,7 +515,7 @@ dco_del_peer(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_DEL_PEER_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -539,7 +541,7 @@ dco_del_key(dco_context_t *dco, unsigned int peerid,
 NLA_PUT_U8(nl_msg, OVPN_DEL_KEY_ATTR_KEY_SLOT, slot);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -596,7 +598,7 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int 
keyid,
 
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -625,7 +627,7 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid,
 keepalive_timeout);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -706,7 +708,7 @@ ovpn_get_mcast_id(dco_context_t *dco)
 int ret = -EMSGSIZE;
 NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_NL_NAME);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -819,18 +821,180 @@ dco_do_read(dco_context_t *dco)
 return ovpn_nl_recvmsgs(dco, __func__);
 }
 
+static void
+dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id)
+{
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES])
+{
+c2->dco_read_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_read_bytes: %lu", __func__,
+c2->dco_read_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link RX bytes provided in reply for peer %u",
+__func__, id);
+}
+
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES])
+{
+c2->dco_write_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_write_bytes: %lu", __func__,
+c2->dco_write_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link TX bytes provided i

[Openvpn-devel] [PATCH v2] dco-freebsd: use m->instances[] instead of m->hash

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
NOTE: not tested because I have no FreeBSD environment

Changes from v1:
* added boundary check on peer-id
---
 src/openvpn/dco_freebsd.c | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..ec93eb61 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,24 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
 {
-struct hash_element *he;
-struct hash_iterator hi;
+struct multi_instance *mi;
 
-hash_iterator_init(m->hash, );
-
-while ((he = hash_iterator_next()))
+if (peer_id >= m->max_clients)
 {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
+msg(M_WARN, "Cannot retrieve stats for peer %u. ID is invalid (out of 
bound)!",
+peerid);
+return;
+}
 
+mi = m->instances[peer_id];
+if (!mi)
+{
+msg(M_INFO, "Peer %u returned by kernel, but not found locally", 
peerid);
 return;
 }
 
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
 }
 
 int
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco_freebsd: use m->instances[] instead of m->hash


Hi,

On 22/03/2023 08:14, Gert Doering wrote:

Hi,

On Wed, Mar 22, 2023 at 12:10:03AM +0100, Antonio Quartulli wrote:

+struct multi_instance *mi = m->instances[peer_id];
+if (!mi)
  {


This (and undoubtedly the same code in dco_linux.c) is trusting the
kernel to never return peer_id values that are outside the array
boundaries.


very good catch.



Is this what we want?


no. we should not trust an external source.

Will send v2 for this and v3 for dco-linux



I'd strongly prefer to have a check like this here

 if ((peer_id < m->max_clients) && (m->instances[peer_id]))
 {
...
 }

(which is what we do in multi_process_incoming_dco(), for example)


Note: in p2p mode, peer-id is something random, usually much bigger
than max_clients - now this *should* never be called in p2p mode, but
I still do not have a good feeling without the bounds check.

gert


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco_freebsd: use m->instances[] instead of m->hash


Hi,

On 22/03/2023 00:10, Antonio Quartulli wrote:

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

Cc: Kristof Provost 


If the patch is suitable for merging, please change the above to 
k...@freebsd.org before moving on.


Cheers,


Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
NOTE: not tested because I have no FreeBSD environment and I
can't find how to kick off the buildbot
---
  src/openvpn/dco_freebsd.c | 22 +-
  1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..ae8c1380 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,15 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
  static void
  dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
  {
-struct hash_element *he;
-struct hash_iterator hi;
-
-hash_iterator_init(m->hash, );
-
-while ((he = hash_iterator_next()))
+struct multi_instance *mi = m->instances[peer_id];
+if (!mi)
  {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_INFO, "Peer %d returned by kernel, but not found locally", 
peerid);
  return;
  }
  
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);

+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
  }
  
  int


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco_freebsd: use m->instances[] instead of m->hash

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
NOTE: not tested because I have no FreeBSD environment and I
can't find how to kick off the buildbot
---
 src/openvpn/dco_freebsd.c | 22 +-
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..ae8c1380 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,15 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
 {
-struct hash_element *he;
-struct hash_iterator hi;
-
-hash_iterator_init(m->hash, );
-
-while ((he = hash_iterator_next()))
+struct multi_instance *mi = m->instances[peer_id];
+if (!mi)
 {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_INFO, "Peer %d returned by kernel, but not found locally", 
peerid);
 return;
 }
 
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
 }
 
 int
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] dco-linux: implement dco_get_peer_stats{, multi} API

With this API it is possible to retrieve the stats for a specific peer
or for all peers and then update the userspace counters with the value
reported by DCO.

Change-Id: Ia3990b86b1be7ca844fb1674b39ce0d60528ccff
Signed-off-by: Antonio Quartulli 
---

Changes from v1:
* use m->instances[] instead of iterating over m->hash
---
 src/openvpn/dco_linux.c  | 183 ---
 src/openvpn/ovpn_dco_linux.h |  14 ++-
 2 files changed, 179 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 47961849..4bbe7e22 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -41,6 +41,7 @@
 #include "tun.h"
 #include "ssl.h"
 #include "fdmisc.h"
+#include "multi.h"
 #include "ssl_verify.h"
 
 #include "ovpn_dco_linux.h"
@@ -168,16 +169,17 @@ ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix)
  * @param dco   The dco context to use
  * @param nl_msgthe message to use
  * @param cbAn optional callback if the caller expects an answer
+ * @param cb_argAn optional param to pass to the callback
  * @param prefixA 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,
- const char *prefix)
+ void *cb_arg, const char *prefix)
 {
 dco->status = 1;
 
-nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, dco);
+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)
@@ -268,7 +270,7 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int 
sd,
 }
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -489,7 +491,7 @@ dco_swap_keys(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_SWAP_KEYS_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -513,7 +515,7 @@ dco_del_peer(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_DEL_PEER_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -539,7 +541,7 @@ dco_del_key(dco_context_t *dco, unsigned int peerid,
 NLA_PUT_U8(nl_msg, OVPN_DEL_KEY_ATTR_KEY_SLOT, slot);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -596,7 +598,7 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int 
keyid,
 
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -625,7 +627,7 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid,
 keepalive_timeout);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -706,7 +708,7 @@ ovpn_get_mcast_id(dco_context_t *dco)
 int ret = -EMSGSIZE;
 NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_NL_NAME);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -819,18 +821,173 @@ dco_do_read(dco_context_t *dco)
 return ovpn_nl_recvmsgs(dco, __func__);
 }
 
+static void
+dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id)
+{
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES])
+{
+c2->dco_read_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_read_bytes: %lu", __func__,
+c2->dco_read_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link RX bytes provided in reply for peer %u",
+__func__, id);
+}
+
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES])
+{
+c2->dco_write_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_write_bytes: %lu", __func__,
+c2->dco_write_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link TX bytes provided in reply for peer %u",
+

[Openvpn-devel] [PATCH] multi: don't call DCO APIs if DCO is disabled

The agreement with the DCO submodule is that no API should be called if
DCO is actually disabled. For this reason, every invocation must happen
only after having checked that dco_enabled() returns true.

Add missing checks before invoking dco_get_peer_stats_multi()

Reported-by: Lev Stipakov 
Signed-off-by: Antonio Quartulli 
---
 src/openvpn/multi.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 53c17b3a..1f0a9c01 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -549,7 +549,10 @@ multi_del_iroutes(struct multi_context *m,
 static void
 setenv_stats(struct multi_context *m, struct context *c)
 {
-dco_get_peer_stats_multi(>top.c1.tuntap->dco, m);
+if (dco_enabled(>top.options))
+{
+dco_get_peer_stats_multi(>top.c1.tuntap->dco, m);
+}
 
 setenv_counter(c->c2.es, "bytes_received", c->c2.link_read_bytes + 
c->c2.dco_read_bytes);
 setenv_counter(c->c2.es, "bytes_sent", c->c2.link_write_bytes + 
c->c2.dco_write_bytes);
@@ -849,7 +852,10 @@ multi_print_status(struct multi_context *m, struct 
status_output *so, const int
 
 status_reset(so);
 
-dco_get_peer_stats_multi(>top.c1.tuntap->dco, m);
+if (dco_enabled(>top.options))
+{
+dco_get_peer_stats_multi(>top.c1.tuntap->dco, m);
+}
 
 if (version == 1)
 {
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco-linux: implement dco_get_peer_stats{, multi} API

2023-03-20 Thread Antonio Quartulli

With this API it is possible to retrieve the stats for a specific peer
or for all peers and then update the userspace counters with the value
reported by DCO.

Change-Id: Ia3990b86b1be7ca844fb1674b39ce0d60528ccff
Signed-off-by: Antonio Quartulli 
---

Pleas, use the latest ovpn-dco master branch!

 src/openvpn/dco_linux.c  | 194 ---
 src/openvpn/ovpn_dco_linux.h |  14 ++-
 2 files changed, 190 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 47961849..1f18fa81 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -41,6 +41,7 @@
 #include "tun.h"
 #include "ssl.h"
 #include "fdmisc.h"
+#include "multi.h"
 #include "ssl_verify.h"
 
 #include "ovpn_dco_linux.h"
@@ -168,16 +169,17 @@ ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix)
  * @param dco   The dco context to use
  * @param nl_msgthe message to use
  * @param cbAn optional callback if the caller expects an answer
+ * @param cb_argAn optional param to pass to the callback
  * @param prefixA 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,
- const char *prefix)
+ void *cb_arg, const char *prefix)
 {
 dco->status = 1;
 
-nl_cb_set(dco->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, cb, dco);
+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)
@@ -268,7 +270,7 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int 
sd,
 }
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -489,7 +491,7 @@ dco_swap_keys(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_SWAP_KEYS_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -513,7 +515,7 @@ dco_del_peer(dco_context_t *dco, unsigned int peerid)
 NLA_PUT_U32(nl_msg, OVPN_DEL_PEER_ATTR_PEER_ID, peerid);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -539,7 +541,7 @@ dco_del_key(dco_context_t *dco, unsigned int peerid,
 NLA_PUT_U8(nl_msg, OVPN_DEL_KEY_ATTR_KEY_SLOT, slot);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -596,7 +598,7 @@ dco_new_key(dco_context_t *dco, unsigned int peerid, int 
keyid,
 
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -625,7 +627,7 @@ dco_set_peer(dco_context_t *dco, unsigned int peerid,
 keepalive_timeout);
 nla_nest_end(nl_msg, attr);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, NULL, NULL, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -706,7 +708,7 @@ ovpn_get_mcast_id(dco_context_t *dco)
 int ret = -EMSGSIZE;
 NLA_PUT_STRING(nl_msg, CTRL_ATTR_FAMILY_NAME, OVPN_NL_NAME);
 
-ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, __func__);
+ret = ovpn_nl_msg_send(dco, nl_msg, mcast_family_handler, dco, __func__);
 
 nla_put_failure:
 nlmsg_free(nl_msg);
@@ -819,18 +821,184 @@ dco_do_read(dco_context_t *dco)
 return ovpn_nl_recvmsgs(dco, __func__);
 }
 
+static void
+dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id)
+{
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES])
+{
+c2->dco_read_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_read_bytes: %lu", __func__,
+c2->dco_read_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link RX bytes provided in reply for peer %u",
+__func__, id);
+}
+
+if (tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES])
+{
+c2->dco_write_bytes = 
nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES]);
+msg(D_DCO_DEBUG, "%s / dco_write_bytes: %lu", __func__,
+c2->dco_write_bytes);
+}
+else
+{
+msg(M_WARN, "%s: no link TX bytes provided in reply for peer %u",
+__func__, id);
+}
+
+if (tb[OVPN_GET_PEER_RES

[Openvpn-devel] [PATCH] dco-linux: remove M_ERRNO flag when printing netlink error message

2023-03-20 Thread Antonio Quartulli

Netlink has its own error space and reports errors via the return
value of its functions.

For this reason remove the M_ERRNO flag when printing its errors.
At the moment we get something like this:

netlink reports error (-7): Invalid input data or parameter: Interrupted system 
call (errno=4)

where the errno=4 (and its human readable representation) is a leftover
from the previous recv() interrupted by a signal and it is totally
unrelated to this netlink failure.

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/dco_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 98b2aae3..0e479beb 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -154,7 +154,7 @@ ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix)
 default:
 if (ret)
 {
-msg(M_NONFATAL|M_ERRNO, "%s: netlink reports error (%d): %s", 
prefix, ret, nl_geterror(-ret));
+msg(M_NONFATAL, "%s: netlink reports error (%d): %s", prefix, 
ret, nl_geterror(-ret));
 }
 break;
 }
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3] dco: print version to log if available

2023-03-10 Thread Antonio Quartulli


Ignore the last message - it was meant for another patch *shrug*

On 09/03/2023 16:02, Antonio Quartulli wrote:

This is being discussed on Gerrit at:

https://gerrit.openvpn.net/c/openvpn/+/28

On 09/03/2023 14:14, Antonio Quartulli wrote:

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 
'v0'.

This should be improved with a follow-up patch.

For Linux we directly fetch the module version from /sys and print
something like:

DCO version: 0.1.20230206-15-g580608ec7c59

Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd
Signed-off-by: Antonio Quartulli 
---
Changes from v1:
* beautify usage of buf with some helpers
* don't apply -1 to buffer capacity as fgets will already read one byte
   less than provided length and will add a null terminator there

Changes from v2:
* fix style of type returned by dco_version_string() (uncrustify doesn't
   work anymore on my box..due to incompatibler config)
---
  src/openvpn/dco.h | 15 +++
  src/openvpn/dco_freebsd.c |  6 ++
  src/openvpn/dco_linux.c   | 27 +++
  src/openvpn/dco_win.c |  6 ++
  src/openvpn/openvpn.c |  2 ++
  src/openvpn/options.c | 11 +++
  src/openvpn/options.h |  2 ++
  7 files changed, 69 insertions(+)

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 2fe671bf..96d95c21 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -58,6 +58,15 @@ struct tuntap;
   */
  bool dco_available(int msglevel);
+
+/**
+ * Return a human readable string representing the DCO version
+ *
+ * @param gc    the garbage collector to use for any dynamic allocation
+ * @return  a pointer to the string (allocated via gc) containing 
the string

+ */
+const char *dco_version_string(struct gc_arena *gc);
+
  /**
   * Check whether the options struct has any option that is not 
supported by
   * our current dco implementation. If so print a warning at warning 
level

@@ -250,6 +259,12 @@ dco_available(int msglevel)
  return false;
  }
+static inline const char *
+dco_version_string(struct gc_arena *gc)
+{
+    return "not-compiled";
+}
+
  static inline bool
  dco_check_option(int msglevel, const struct options *o)
  {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 92de5f04..cbd2ce20 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -614,6 +614,12 @@ out:
  return available;
  }
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+    return "v0";
+}
+
  void
  dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
  {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 308abfc4..9f3531d5 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -841,9 +841,36 @@ dco_available(int msglevel)
  "Note: Kernel support for ovpn-dco missing, disabling 
data channel offload.");

  return false;
  }
+
  return true;
  }
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+    struct buffer out = alloc_buf_gc(256, gc);
+    FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r");
+    if (!fp)
+    {
+    return "N/A";
+    }
+
+    if (!fgets(BSTR(), BCAP(), fp))
+    {
+    return "ERR";
+    }
+
+    /* remove potential newline at the end of the string */
+    char *str = BSTR();
+    char *nl = strchr(str, '\n');
+    if (nl)
+    {
+    *nl = '\0';
+    }
+
+    return BSTR();
+}
+
  void
  dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
  {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index a805c2a0..26463b38 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -385,6 +385,12 @@ dco_available(int msglevel)
  return false;
  }
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+    return "v0";
+}
+
  int
  dco_do_read(dco_context_t *dco)
  {
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index cba58276..1aaddcdf 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[])
  #endif
  show_library_versions(M_INFO);
+    show_dco_version(M_INFO);
+
  /* misc stuff */
  pre_setup();
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index ce7bd0a7..8f0e2194 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags)
  }
  #endif
+void
+show_dco_version(const unsigned int flags)
+{
+#ifdef ENABLE_DCO
+    struct gc_arena gc = gc_new();
+    msg(flags, "DCO version: %s", dco_version_string());
+    gc_free();
+#endif
+}
+
  void
  show_library_versions(const unsigned int flags)
  {
@@

[Openvpn-devel] [PATCH v2] dco: don't use NetLink to exchange control packets

Using NetLink for control messages did not work out as it did lead to
kernel side buffer congestion during heavy client activity.

With this patch DCO will redirect control packets directly to the
transport socket without altering them, so that userspace can
happily process them as usual.

Change-Id: Ia1297c3ae9a28b188ed21ad21ae96fff3d02ee4d
[l...@openvpn.net: ensure win_dco flag is still exposed]
Signed-off-by: Antonio Quartulli 
---
Changes from v1:
* improved comments
* improved commit message

This patch was also reviewed and approved on gerrit at:
https://gerrit.openvpn.net/c/openvpn/+/28


 src/openvpn/dco.c|  12 -
 src/openvpn/dco.h|  16 --
 src/openvpn/dco_freebsd.c|  10 
 src/openvpn/dco_freebsd.h|   2 -
 src/openvpn/dco_linux.c  | 101 ---
 src/openvpn/dco_linux.h  |   2 -
 src/openvpn/dco_win.c|   8 ---
 src/openvpn/forward.c|  63 +++---
 src/openvpn/init.c   |   3 +-
 src/openvpn/mtcp.c   |  22 +---
 src/openvpn/multi.c  |  39 +-
 src/openvpn/ovpn_dco_linux.h |  16 +-
 src/openvpn/socket.c |   8 +--
 src/openvpn/socket.h |  29 ++
 14 files changed, 35 insertions(+), 296 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index b53332a8..308578b4 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -485,7 +485,6 @@ dco_p2p_add_new_peer(struct context *c)
 }
 
 c->c2.tls_multi->dco_peer_id = multi->peer_id;
-c->c2.link_socket->dco_installed = true;
 
 return 0;
 }
@@ -605,17 +604,6 @@ dco_multi_add_new_peer(struct multi_context *m, struct 
multi_instance *mi)
 
 c->c2.tls_multi->dco_peer_id = peer_id;
 
-if (c->mode == CM_CHILD_TCP)
-{
-multi_tcp_dereference_instance(m->mtcp, mi);
-if (close(sd))
-{
-msg(D_DCO|M_ERRNO, "error closing TCP socket after DCO handover");
-}
-c->c2.link_socket->dco_installed = true;
-c->c2.link_socket->sd = SOCKET_UNDEFINED;
-}
-
 return 0;
 }
 
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 18a9d78b..2fe671bf 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -127,15 +127,6 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t 
*ctx);
  */
 int dco_do_read(dco_context_t *dco);
 
-/**
- * Write data to the DCO communication channel (control packet expected)
- *
- * @param dco   the DCO context
- * @param peer_id   the ID of the peer to send the data to
- * @param buf   the buffer containing the data to send
- */
-int dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf);
-
 /**
  * Install a DCO in the main event loop
  */
@@ -301,13 +292,6 @@ dco_do_read(dco_context_t *dco)
 return 0;
 }
 
-static inline int
-dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf)
-{
-ASSERT(false);
-return 0;
-}
-
 static inline void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index cd4083c4..92de5f04 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -142,7 +142,6 @@ open_fd(dco_context_t *dco)
 {
 dco->open = true;
 }
-dco->dco_packet_in = alloc_buf(PAGE_SIZE);
 
 return dco->fd;
 }
@@ -560,15 +559,6 @@ dco_do_read(dco_context_t *dco)
 return 0;
 }
 
-int
-dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf)
-{
-/* Control packets are passed through the socket, so this should never get
- * called. See should_use_dco_socket(). */
-ASSERT(0);
-return -EINVAL;
-}
-
 bool
 dco_available(int msglevel)
 {
diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
index 970beca0..a07f9b69 100644
--- a/src/openvpn/dco_freebsd.h
+++ b/src/openvpn/dco_freebsd.h
@@ -51,8 +51,6 @@ typedef struct dco_context {
 
 char ifname[IFNAMSIZ];
 
-struct buffer dco_packet_in;
-
 int dco_message_type;
 int dco_message_peer_id;
 int dco_del_peer_reason;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index c84f9cfe..308abfc4 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -434,24 +434,6 @@ ovpn_dco_register(dco_context_t *dco)
 {
 msg(M_ERR, "%s: failed to join groups: %d", __func__, ret);
 }
-
-/* Register for non-data packets that ovpn-dco may receive. They will be
- * forwarded to userspace
- */
-struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, 
OVPN_CMD_REGISTER_PACKET);
-if (!nl_msg)
-{
-msg(M_ERR, "%s: cannot allocate message to register for control 
packets",
-__func__);
-}
-
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
-if (ret)
-{
-msg(M_ERR, "%s: failed to register for control packets: %d", __func__,
-ret);
-

Re: [Openvpn-devel] [PATCH v3] dco: print version to log if available


This is being discussed on Gerrit at:

https://gerrit.openvpn.net/c/openvpn/+/28

On 09/03/2023 14:14, Antonio Quartulli wrote:

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.

For Linux we directly fetch the module version from /sys and print
something like:

DCO version: 0.1.20230206-15-g580608ec7c59

Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd
Signed-off-by: Antonio Quartulli 
---
Changes from v1:
* beautify usage of buf with some helpers
* don't apply -1 to buffer capacity as fgets will already read one byte
   less than provided length and will add a null terminator there

Changes from v2:
* fix style of type returned by dco_version_string() (uncrustify doesn't
   work anymore on my box..due to incompatibler config)
---
  src/openvpn/dco.h | 15 +++
  src/openvpn/dco_freebsd.c |  6 ++
  src/openvpn/dco_linux.c   | 27 +++
  src/openvpn/dco_win.c |  6 ++
  src/openvpn/openvpn.c |  2 ++
  src/openvpn/options.c | 11 +++
  src/openvpn/options.h |  2 ++
  7 files changed, 69 insertions(+)

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 2fe671bf..96d95c21 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -58,6 +58,15 @@ struct tuntap;
   */
  bool dco_available(int msglevel);
  
+

+/**
+ * Return a human readable string representing the DCO version
+ *
+ * @param gcthe garbage collector to use for any dynamic allocation
+ * @return  a pointer to the string (allocated via gc) containing the 
string
+ */
+const char *dco_version_string(struct gc_arena *gc);
+
  /**
   * Check whether the options struct has any option that is not supported by
   * our current dco implementation. If so print a warning at warning level
@@ -250,6 +259,12 @@ dco_available(int msglevel)
  return false;
  }
  
+static inline const char *

+dco_version_string(struct gc_arena *gc)
+{
+return "not-compiled";
+}
+
  static inline bool
  dco_check_option(int msglevel, const struct options *o)
  {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 92de5f04..cbd2ce20 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -614,6 +614,12 @@ out:
  return available;
  }
  
+const char *

+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
  void
  dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
  {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 308abfc4..9f3531d5 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -841,9 +841,36 @@ dco_available(int msglevel)
  "Note: Kernel support for ovpn-dco missing, disabling data channel 
offload.");
  return false;
  }
+
  return true;
  }
  
+const char *

+dco_version_string(struct gc_arena *gc)
+{
+struct buffer out = alloc_buf_gc(256, gc);
+FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r");
+if (!fp)
+{
+return "N/A";
+}
+
+if (!fgets(BSTR(), BCAP(), fp))
+{
+return "ERR";
+}
+
+/* remove potential newline at the end of the string */
+char *str = BSTR();
+char *nl = strchr(str, '\n');
+if (nl)
+{
+*nl = '\0';
+}
+
+return BSTR();
+}
+
  void
  dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
  {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index a805c2a0..26463b38 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -385,6 +385,12 @@ dco_available(int msglevel)
  return false;
  }
  
+const char *

+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
  int
  dco_do_read(dco_context_t *dco)
  {
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index cba58276..1aaddcdf 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[])
  #endif
  show_library_versions(M_INFO);
  
+show_dco_version(M_INFO);

+
  /* misc stuff */
  pre_setup();
  
diff --git a/src/openvpn/options.c b/src/openvpn/options.c

index ce7bd0a7..8f0e2194 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags)
  }
  #endif
  
+void

+show_dco_version(const unsigned int flags)
+{
+#ifdef ENABLE_DCO
+struct gc_arena gc = gc_new();
+msg(flags, "DCO version: %s", dco_version_string());
+gc_free();
+#endif
+}
+
  void
  show_library_versions(const unsigned int flags)
  {
@@ -4839,6 +4849,7 @@ usage_version(void)
  #ifdef _WIN32
  show_windows_version(

[Openvpn-devel] [PATCH v3] dco: print version to log if available

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.

For Linux we directly fetch the module version from /sys and print
something like:

DCO version: 0.1.20230206-15-g580608ec7c59

Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd
Signed-off-by: Antonio Quartulli 
---
Changes from v1:
* beautify usage of buf with some helpers
* don't apply -1 to buffer capacity as fgets will already read one byte
  less than provided length and will add a null terminator there

Changes from v2:
* fix style of type returned by dco_version_string() (uncrustify doesn't
  work anymore on my box..due to incompatibler config)
---
 src/openvpn/dco.h | 15 +++
 src/openvpn/dco_freebsd.c |  6 ++
 src/openvpn/dco_linux.c   | 27 +++
 src/openvpn/dco_win.c |  6 ++
 src/openvpn/openvpn.c |  2 ++
 src/openvpn/options.c | 11 +++
 src/openvpn/options.h |  2 ++
 7 files changed, 69 insertions(+)

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 2fe671bf..96d95c21 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -58,6 +58,15 @@ struct tuntap;
  */
 bool dco_available(int msglevel);
 
+
+/**
+ * Return a human readable string representing the DCO version
+ *
+ * @param gcthe garbage collector to use for any dynamic allocation
+ * @return  a pointer to the string (allocated via gc) containing the 
string
+ */
+const char *dco_version_string(struct gc_arena *gc);
+
 /**
  * Check whether the options struct has any option that is not supported by
  * our current dco implementation. If so print a warning at warning level
@@ -250,6 +259,12 @@ dco_available(int msglevel)
 return false;
 }
 
+static inline const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "not-compiled";
+}
+
 static inline bool
 dco_check_option(int msglevel, const struct options *o)
 {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 92de5f04..cbd2ce20 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -614,6 +614,12 @@ out:
 return available;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
 void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 308abfc4..9f3531d5 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -841,9 +841,36 @@ dco_available(int msglevel)
 "Note: Kernel support for ovpn-dco missing, disabling data channel 
offload.");
 return false;
 }
+
 return true;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+struct buffer out = alloc_buf_gc(256, gc);
+FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r");
+if (!fp)
+{
+return "N/A";
+}
+
+if (!fgets(BSTR(), BCAP(), fp))
+{
+return "ERR";
+}
+
+/* remove potential newline at the end of the string */
+char *str = BSTR();
+char *nl = strchr(str, '\n');
+if (nl)
+{
+*nl = '\0';
+}
+
+return BSTR();
+}
+
 void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index a805c2a0..26463b38 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -385,6 +385,12 @@ dco_available(int msglevel)
 return false;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
 int
 dco_do_read(dco_context_t *dco)
 {
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index cba58276..1aaddcdf 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[])
 #endif
 show_library_versions(M_INFO);
 
+show_dco_version(M_INFO);
+
 /* misc stuff */
 pre_setup();
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index ce7bd0a7..8f0e2194 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags)
 }
 #endif
 
+void
+show_dco_version(const unsigned int flags)
+{
+#ifdef ENABLE_DCO
+struct gc_arena gc = gc_new();
+msg(flags, "DCO version: %s", dco_version_string());
+gc_free();
+#endif
+}
+
 void
 show_library_versions(const unsigned int flags)
 {
@@ -4839,6 +4849,7 @@ usage_version(void)
 #ifdef _WIN32
 show_windows_version( M_INFO|M_NOPREFIX );
 #endif
+show_dco_version(M_INFO | M_NOPREFIX);
 msg(M_INFO|M_NOPREFIX, "Originally developed by James Yonan");
 msg(M_INFO|M_NOPREFIX, "Copyright (C) 20

Re: [Openvpn-devel] [PATCH] Ensure n = 2 is set in key2 structer in tls_crypt_v2_unwrap_client_key


Hi,

On 09/03/2023 13:00, Arne Schwabe wrote:

The ASSERT in xor_key2 assumes that all methods that load a key2 struct
correctly set n=2. However, tls_crypt_v2_unwrap_client_key loads a key
without setting n = 2, trigerring the assert.


trigerring -> triggering



Closes and reported in https://github.com/OpenVPN/openvpn/issues/272

Change-Id: Iaeb163d83b95818e0b26faf9d25e7737dc8ecb23
Signed-off-by: Arne Schwabe 


I can easily reproduce the issue ad verify that indeed the patch fixes it.

By checking the code I can see that we set 'n' whenever we initialize 
the keys member. However, this was not happening in 
tls_crypt_v2_unwrap_client_key() thus leading to the assert being triggered.


Acked-by: Antonio Quartulli 


---
  src/openvpn/tls_crypt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 8882d5de0..4f22f8af7 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -533,6 +533,7 @@ tls_crypt_v2_unwrap_client_key(struct key2 *client_key, 
struct buffer *metadata,
  }
  memcpy(_key->keys, BPTR(), sizeof(client_key->keys));
  ASSERT(buf_advance(, sizeof(client_key->keys)));
+client_key->n = 2;
  
  if (!buf_copy(metadata, ))

  {


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] dco: print version to log if available

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.

For Linux we directly fetch the module version from /sys and print
something like:

DCO version: 0.1.20230206-15-g580608ec7c59

Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd
Signed-off-by: Antonio Quartulli 
---
Changes from v1:
* beautify usage of buf with some helpers
* don't apply -1 to buffer capacity as fgets will already read one byte
  less than provided length and will add a null terminator there
---
 src/openvpn/dco.h | 15 +++
 src/openvpn/dco_freebsd.c |  6 ++
 src/openvpn/dco_linux.c   | 27 +++
 src/openvpn/dco_win.c |  6 ++
 src/openvpn/openvpn.c |  2 ++
 src/openvpn/options.c | 11 +++
 src/openvpn/options.h |  2 ++
 7 files changed, 69 insertions(+)

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 2fe671bf..96d95c21 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -58,6 +58,15 @@ struct tuntap;
  */
 bool dco_available(int msglevel);
 
+
+/**
+ * Return a human readable string representing the DCO version
+ *
+ * @param gcthe garbage collector to use for any dynamic allocation
+ * @return  a pointer to the string (allocated via gc) containing the 
string
+ */
+const char *dco_version_string(struct gc_arena *gc);
+
 /**
  * Check whether the options struct has any option that is not supported by
  * our current dco implementation. If so print a warning at warning level
@@ -250,6 +259,12 @@ dco_available(int msglevel)
 return false;
 }
 
+static inline const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "not-compiled";
+}
+
 static inline bool
 dco_check_option(int msglevel, const struct options *o)
 {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 92de5f04..cbd2ce20 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -614,6 +614,12 @@ out:
 return available;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
 void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 308abfc4..00d32d05 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -841,9 +841,36 @@ dco_available(int msglevel)
 "Note: Kernel support for ovpn-dco missing, disabling data channel 
offload.");
 return false;
 }
+
 return true;
 }
 
+const char
+*dco_version_string(struct gc_arena *gc)
+{
+struct buffer out = alloc_buf_gc(256, gc);
+FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r");
+if (!fp)
+{
+return "N/A";
+}
+
+if (!fgets(BSTR(), BCAP(), fp))
+{
+return "ERR";
+}
+
+/* remove potential newline at the end of the string */
+char *str = BSTR();
+char *nl = strchr(str, '\n');
+if (nl)
+{
+*nl = '\0';
+}
+
+return BSTR();
+}
+
 void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index a805c2a0..26463b38 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -385,6 +385,12 @@ dco_available(int msglevel)
 return false;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
 int
 dco_do_read(dco_context_t *dco)
 {
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index cba58276..1aaddcdf 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[])
 #endif
 show_library_versions(M_INFO);
 
+show_dco_version(M_INFO);
+
 /* misc stuff */
 pre_setup();
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index ce7bd0a7..8f0e2194 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags)
 }
 #endif
 
+void
+show_dco_version(const unsigned int flags)
+{
+#ifdef ENABLE_DCO
+struct gc_arena gc = gc_new();
+msg(flags, "DCO version: %s", dco_version_string());
+gc_free();
+#endif
+}
+
 void
 show_library_versions(const unsigned int flags)
 {
@@ -4839,6 +4849,7 @@ usage_version(void)
 #ifdef _WIN32
 show_windows_version( M_INFO|M_NOPREFIX );
 #endif
+show_dco_version(M_INFO | M_NOPREFIX);
 msg(M_INFO|M_NOPREFIX, "Originally developed by James Yonan");
 msg(M_INFO|M_NOPREFIX, "Copyright (C) 2002-2023 OpenVPN Inc 
");
 #ifndef ENABLE_SMALL
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 7df717f7..2f25f5d0 1006

Re: [Openvpn-devel] [PATCH] dco: print FreeBSD version


Hi,

On 09/03/2023 13:13, Kristof Provost via Openvpn-devel wrote:

This should use BSTR(data) instead.


I copied Antonio’s code here, but that is better, so I’ll fix that too.



dang! with one email Arne spoiled two patches!

Cheers,

--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco: print version to log if available


Hi,

On 09/03/2023 10:03, Kristof Provost wrote:

On 9 Mar 2023, at 9:57, Antonio Quartulli wrote:

On 09/03/2023 09:36, Kristof Provost wrote:

On 9 Mar 2023, at 1:52, Antonio Quartulli wrote:

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.


FreeBSD is a bit special in this regard, because the DCO driver is part of the 
base operating system and not an add-on as in Linux and Windows.

Arguably the thing to do for FreeBSD is to log the OS version.


Isn't it possible to install DCO "out-of-tree" (as we can do on Linux)?


With some effort it could be done, but any version (in the source tree) before 
DCO was actually imported isn’t going to work. I made a few (small) tweaks to 
the network stack to make things work.

Realistically this isn’t something people will do, and if they do decide to the 
usual warranty applies (i.e. they get to keep all of the pieces it breaks into).


Yeah, same problem on Linux. This is why we have to ship a compatibility 
layer in order to compile DCO on older kernels.


On Linux, though, compiling a new module on an older kernel is pretty 
common, this is why we have put some effort there too.





However, what you said will also apply to linux in the future: we will also 
need to report the kernel version, *if* DCO is compiled in (to be implemented).

This said, reporting the kernel/os version is absolutely appropriate if that's 
what identifies the DCO version.


There’s no API in FreeBSD’s DCO to identify its version either, which is 
another reason to just use the OS version.


yup, makes sense!




Is that something you could implement in dco_freebsd.c?


Sure. I’ll send a patch later today.


Thanks!

Cheers,



Best regards,
Kristof


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco: print version to log if available


Hi,

On 09/03/2023 09:36, Kristof Provost wrote:

On 9 Mar 2023, at 1:52, Antonio Quartulli wrote:

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.


FreeBSD is a bit special in this regard, because the DCO driver is part of the 
base operating system and not an add-on as in Linux and Windows.

Arguably the thing to do for FreeBSD is to log the OS version.


Isn't it possible to install DCO "out-of-tree" (as we can do on Linux)?

However, what you said will also apply to linux in the future: we will 
also need to report the kernel version, *if* DCO is compiled in (to be 
implemented).


This said, reporting the kernel/os version is absolutely appropriate if 
that's what identifies the DCO version.


Is that something you could implement in dco_freebsd.c?


Cheers,



Kristof


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco: print version to log if available

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.

For Linux we directly fetch the module version from /sys and print
something like:

DCO version: 0.1.20230206-15-g580608ec7c59

Cc: Lev Stipakov 
Cc: Kristof Provost 
Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd
Signed-off-by: Antonio Quartulli 
---
 src/openvpn/dco.h | 15 +++
 src/openvpn/dco_freebsd.c |  6 ++
 src/openvpn/dco_linux.c   | 30 ++
 src/openvpn/dco_win.c |  6 ++
 src/openvpn/openvpn.c |  2 ++
 src/openvpn/options.c | 11 +++
 src/openvpn/options.h |  2 ++
 7 files changed, 72 insertions(+)

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 2fe671bf..96d95c21 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -58,6 +58,15 @@ struct tuntap;
  */
 bool dco_available(int msglevel);
 
+
+/**
+ * Return a human readable string representing the DCO version
+ *
+ * @param gcthe garbage collector to use for any dynamic allocation
+ * @return  a pointer to the string (allocated via gc) containing the 
string
+ */
+const char *dco_version_string(struct gc_arena *gc);
+
 /**
  * Check whether the options struct has any option that is not supported by
  * our current dco implementation. If so print a warning at warning level
@@ -250,6 +259,12 @@ dco_available(int msglevel)
 return false;
 }
 
+static inline const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "not-compiled";
+}
+
 static inline bool
 dco_check_option(int msglevel, const struct options *o)
 {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 92de5f04..cbd2ce20 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -614,6 +614,12 @@ out:
 return available;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
 void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 308abfc4..c9cd9df1 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -841,9 +841,39 @@ dco_available(int msglevel)
 "Note: Kernel support for ovpn-dco missing, disabling data channel 
offload.");
 return false;
 }
+
 return true;
 }
 
+const char
+*dco_version_string(struct gc_arena *gc)
+{
+struct buffer out = alloc_buf_gc(256, gc);
+FILE *fp = fopen("/sys/module/ovpn_dco_v2/version", "r");
+if (!fp)
+{
+buf_printf(, "N/A");
+goto out;
+}
+
+if (!fgets((char *)buf_bptr(), out.capacity - 1, fp))
+{
+buf_printf(, "ERR");
+goto out;
+}
+
+/* remove potential newline at the end of the string */
+char *str = (char *)buf_bptr();
+char *nl = strchr(str, '\n');
+if (nl)
+{
+*nl = '\0';
+}
+
+out:
+return (char *)out.data;
+}
+
 void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index a805c2a0..26463b38 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -385,6 +385,12 @@ dco_available(int msglevel)
 return false;
 }
 
+const char *
+dco_version_string(struct gc_arena *gc)
+{
+return "v0";
+}
+
 int
 dco_do_read(dco_context_t *dco)
 {
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index cba58276..1aaddcdf 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -262,6 +262,8 @@ openvpn_main(int argc, char *argv[])
 #endif
 show_library_versions(M_INFO);
 
+show_dco_version(M_INFO);
+
 /* misc stuff */
 pre_setup();
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index ce7bd0a7..8f0e2194 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4816,6 +4816,16 @@ show_windows_version(const unsigned int flags)
 }
 #endif
 
+void
+show_dco_version(const unsigned int flags)
+{
+#ifdef ENABLE_DCO
+struct gc_arena gc = gc_new();
+msg(flags, "DCO version: %s", dco_version_string());
+gc_free();
+#endif
+}
+
 void
 show_library_versions(const unsigned int flags)
 {
@@ -4839,6 +4849,7 @@ usage_version(void)
 #ifdef _WIN32
 show_windows_version( M_INFO|M_NOPREFIX );
 #endif
+show_dco_version(M_INFO | M_NOPREFIX);
 msg(M_INFO|M_NOPREFIX, "Originally developed by James Yonan");
 msg(M_INFO|M_NOPREFIX, "Copyright (C) 2002-2023 OpenVPN Inc 
");
 #ifndef ENABLE_SMALL
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 7df717f7..2f25f5d0 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@

[Openvpn-devel] [PATCH] dco: don't use NetLink to exchange control packets

Using NetLink has proved to be overkill and performance critical.
The amount of control traffic can also easily overrun the NetLink buffer
when a server has enough clients connected.

Stop using NetLink to send/receive control packets and just use the
transport socket as if DCO was not there at all.

Under the hood DCO will redirect control packets to the transport socket
without altering them, so that userspace can happily process them as
usual.

Change-Id: Ia1297c3ae9a28b188ed21ad21ae96fff3d02ee4d
[l...@openvpn.net: ensure win_dco flag is still exposed]
Signed-off-by: Antonio Quartulli 
---
 src/openvpn/dco.c|  12 -
 src/openvpn/dco.h|  16 --
 src/openvpn/dco_freebsd.c|  10 
 src/openvpn/dco_freebsd.h|   2 -
 src/openvpn/dco_linux.c  | 101 ---
 src/openvpn/dco_linux.h  |   2 -
 src/openvpn/dco_win.c|   8 ---
 src/openvpn/forward.c|  63 +++---
 src/openvpn/init.c   |   3 +-
 src/openvpn/mtcp.c   |  22 +---
 src/openvpn/multi.c  |  39 +-
 src/openvpn/ovpn_dco_linux.h |  16 +-
 src/openvpn/socket.c |   8 +--
 src/openvpn/socket.h |  22 
 14 files changed, 29 insertions(+), 295 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index b53332a8..308578b4 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -485,7 +485,6 @@ dco_p2p_add_new_peer(struct context *c)
 }
 
 c->c2.tls_multi->dco_peer_id = multi->peer_id;
-c->c2.link_socket->dco_installed = true;
 
 return 0;
 }
@@ -605,17 +604,6 @@ dco_multi_add_new_peer(struct multi_context *m, struct 
multi_instance *mi)
 
 c->c2.tls_multi->dco_peer_id = peer_id;
 
-if (c->mode == CM_CHILD_TCP)
-{
-multi_tcp_dereference_instance(m->mtcp, mi);
-if (close(sd))
-{
-msg(D_DCO|M_ERRNO, "error closing TCP socket after DCO handover");
-}
-c->c2.link_socket->dco_installed = true;
-c->c2.link_socket->sd = SOCKET_UNDEFINED;
-}
-
 return 0;
 }
 
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 18a9d78b..2fe671bf 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -127,15 +127,6 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t 
*ctx);
  */
 int dco_do_read(dco_context_t *dco);
 
-/**
- * Write data to the DCO communication channel (control packet expected)
- *
- * @param dco   the DCO context
- * @param peer_id   the ID of the peer to send the data to
- * @param buf   the buffer containing the data to send
- */
-int dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf);
-
 /**
  * Install a DCO in the main event loop
  */
@@ -301,13 +292,6 @@ dco_do_read(dco_context_t *dco)
 return 0;
 }
 
-static inline int
-dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf)
-{
-ASSERT(false);
-return 0;
-}
-
 static inline void
 dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index cd4083c4..92de5f04 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -142,7 +142,6 @@ open_fd(dco_context_t *dco)
 {
 dco->open = true;
 }
-dco->dco_packet_in = alloc_buf(PAGE_SIZE);
 
 return dco->fd;
 }
@@ -560,15 +559,6 @@ dco_do_read(dco_context_t *dco)
 return 0;
 }
 
-int
-dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf)
-{
-/* Control packets are passed through the socket, so this should never get
- * called. See should_use_dco_socket(). */
-ASSERT(0);
-return -EINVAL;
-}
-
 bool
 dco_available(int msglevel)
 {
diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
index 970beca0..a07f9b69 100644
--- a/src/openvpn/dco_freebsd.h
+++ b/src/openvpn/dco_freebsd.h
@@ -51,8 +51,6 @@ typedef struct dco_context {
 
 char ifname[IFNAMSIZ];
 
-struct buffer dco_packet_in;
-
 int dco_message_type;
 int dco_message_peer_id;
 int dco_del_peer_reason;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index c84f9cfe..308abfc4 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -434,24 +434,6 @@ ovpn_dco_register(dco_context_t *dco)
 {
 msg(M_ERR, "%s: failed to join groups: %d", __func__, ret);
 }
-
-/* Register for non-data packets that ovpn-dco may receive. They will be
- * forwarded to userspace
- */
-struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, 
OVPN_CMD_REGISTER_PACKET);
-if (!nl_msg)
-{
-msg(M_ERR, "%s: cannot allocate message to register for control 
packets",
-__func__);
-}
-
-ret = ovpn_nl_msg_send(dco, nl_msg, NULL, __func__);
-if (ret)
-{
-msg(M_ERR, "%s: failed to register for control packets: %d", __func__,
-ret);
-

Re: [Openvpn-devel] [PATCH] Set netlink socket to be non-blocking


Hi,

On 08/03/2023 16:19, Arne Schwabe wrote:

Even though we use select/poll to explicitly query when the nextlink


nextlink -> netlink


socket is ready for read, sometimes we end up reading from the socket
when it is not ready to read and then the process hangs for several
seoneds (20-30s). Avoid this situation by setting the socket to be
non-blocking, so we get a status in this case that allows us to continue.

Change-Id: I35447c23a9350176007df5455bf9451021e9856d
Signed-off-by: Arne Schwabe 


Well spotted!

Acked-by: Antonio Quartulli 


---
  src/openvpn/dco_linux.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 1a6566aad..6f1b999bb 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -359,7 +359,9 @@ ovpn_dco_init_netlink(dco_context_t *dco)
  nl_geterror(ret));
  }
  
+/* set close on exec and non-block on the netlink socket */

  set_cloexec(nl_socket_get_fd(dco->nl_sock));
+set_nonblock(nl_socket_get_fd(dco->nl_sock));
  
  dco->nl_cb = nl_cb_alloc(NL_CB_DEFAULT);

  if (!dco->nl_cb)


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Avoid warning about missing braces when initialising key struct

Signed-off-by: Antonio Quartulli 
---

As concluded on IRC, this version does what we want it to do.
We also quickly tested with some sample program to make sure we weren't
making this up.


 src/openvpn/tls_crypt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 81098355..3b68d186 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -348,7 +348,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct 
key2 *original_key,
 msg(M_FATAL, "ERROR: invalid tls-crypt-v2 client key format");
 }
 
-struct key2 key2 = { .n = 2, .keys = { 0 } };
+struct key2 key2 = { .n = 2 };
 if (!buf_read(_key, , sizeof(key2.keys)))
 {
 msg(M_FATAL, "ERROR: not enough data in tls-crypt-v2 client key");
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Avoid warning about missing braces when initialising key struct


Hi,

On 08/03/2023 00:57, Arne Schwabe wrote:

This avoids the warning from gcc about initialising the key2 struct

Change-Id: Ia73d24923b1efd99263f33ce13d90e04b59bd980
Signed-off-by: Arne Schwabe 
---
  src/openvpn/tls_crypt.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 81098355e..e84d979e9 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -348,7 +348,8 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct 
key2 *original_key,
  msg(M_FATAL, "ERROR: invalid tls-crypt-v2 client key format");
  }
  
-struct key2 key2 = { .n = 2, .keys = { 0 } };

+struct key2 key2 = { .n = 2, .keys = {{{ 0 }}} };


why not just struct key2 key2 = { .n = 2 } ?

Cheers,


+
  if (!buf_read(_key, , sizeof(key2.keys)))
  {
  msg(M_FATAL, "ERROR: not enough data in tls-crypt-v2 client key");


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD

2023-03-03 Thread Antonio Quartulli


Hi,

On 03/03/2023 12:27, Antonio Quartulli wrote:

Hi,

On 03/03/2023 12:05, Kristof Provost via Openvpn-devel wrote:

From: Kristof Provost 

FreeBSD's if_ovpn will never emit this as a peer deletion reason
(because it doesn't support TCP), but this allows us to align the
defines between Linux and FreeBSD, and remove a Linux-specific case from
process_incoming_del_peer().


SoB missing


Sorry for being very condensed, however the patch looks good and gets my 
ACK:


Acked-by: Antonio Quartulli 

I normally put my ACK under the SoB, so when I couldn't find it, my 
brain just threw that exception :-P


Cheers,




---
  src/openvpn/dco_freebsd.h | 1 +
  src/openvpn/multi.c   | 3 ---
  2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
index 2e35f3ac..970beca0 100644
--- a/src/openvpn/dco_freebsd.h
+++ b/src/openvpn/dco_freebsd.h
@@ -41,6 +41,7 @@ enum ovpn_del_reason_t {
  OVPN_DEL_PEER_REASON_EXPIRED,
  OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
  OVPN_DEL_PEER_REASON_USERSPACE,
+    OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
  };
  typedef struct dco_context {
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f2559016..99123c39 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3244,12 +3244,9 @@ process_incoming_del_peer(struct multi_context 
*m, struct multi_instance *mi,

  reason = "ovpn-dco: transport error";
  break;
-#ifdef TARGET_LINUX
-    /* FIXME: this is linux-only today and breaks FreeBSD 
compilation */

  case OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT:
  reason = "ovpn-dco: transport disconnected";
  break;
-#endif
  case OVPN_DEL_PEER_REASON_USERSPACE:
  /* We assume that is ourselves. Unfortunately, sometimes 
these




--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD

2023-03-03 Thread Antonio Quartulli


Hi,

On 03/03/2023 12:05, Kristof Provost via Openvpn-devel wrote:

From: Kristof Provost 

FreeBSD's if_ovpn will never emit this as a peer deletion reason
(because it doesn't support TCP), but this allows us to align the
defines between Linux and FreeBSD, and remove a Linux-specific case from
process_incoming_del_peer().


SoB missing


---
  src/openvpn/dco_freebsd.h | 1 +
  src/openvpn/multi.c   | 3 ---
  2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
index 2e35f3ac..970beca0 100644
--- a/src/openvpn/dco_freebsd.h
+++ b/src/openvpn/dco_freebsd.h
@@ -41,6 +41,7 @@ enum ovpn_del_reason_t {
  OVPN_DEL_PEER_REASON_EXPIRED,
  OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
  OVPN_DEL_PEER_REASON_USERSPACE,
+OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
  };
  
  typedef struct dco_context {

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f2559016..99123c39 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3244,12 +3244,9 @@ process_incoming_del_peer(struct multi_context *m, 
struct multi_instance *mi,
  reason = "ovpn-dco: transport error";
  break;
  
-#ifdef TARGET_LINUX

-/* FIXME: this is linux-only today and breaks FreeBSD compilation */
  case OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT:
  reason = "ovpn-dco: transport disconnected";
  break;
-#endif
  
  case OVPN_DEL_PEER_REASON_USERSPACE:

  /* We assume that is ourselves. Unfortunately, sometimes these


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Update issue templates

2023-02-26 Thread Antonio Quartulli

With this change we extend the text exposed to people opening a bug in
the OpenVPN project.

Hopefully they will read and immediately understand that GH is not the
right place to report ossues about commercial products.

Change-Id: Idd039612698a6b08f9544450885d1a5f77fd95c6
Signed-off-by: Antonio Quartulli 
---
 .github/ISSUE_TEMPLATE/bug_report.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.github/ISSUE_TEMPLATE/bug_report.md 
b/.github/ISSUE_TEMPLATE/bug_report.md
index 6cbd3ba4..3bf6f233 100644
--- a/.github/ISSUE_TEMPLATE/bug_report.md
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -7,6 +7,9 @@ assignees: ''
 
 ---
 
+**IMPORTANT NOTE**
+Bugs about OpenVPN Access Server, OpenVPN Connect or any other product by 
OpenVPN Inc. should be directly reported to OpenVPN Inc. at 
https://support.openvpn.net
+
 **Describe the bug**
 A clear and concise description of what the bug is.
 
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Avoid management log loop with verb >= 6

2023-02-26 Thread Antonio Quartulli


Hi,

On 17/02/2023 13:21, Lev Stipakov wrote:

From: Lev Stipakov 

This log message is printed within check_tls(),
which is called by pre_select(), which is called
on every iteration of event loop.

When management is attached (and doesn't use own event loop),
this message sets management state to "wait write",
which arms event loop. When on the next iteration iowait
returns with "management write event is set", we call
pre_select() and print that message again, causing the loop.

Fix by simply removing this log message.

Signed-off-by: Lev Stipakov 


As discussed on IRC, removing this message is a good thing as it serves 
no real purpose.


On top of that this message is causing this log infinite recursion, 
therefore it should just go.


OTOH we may still have *some* recursion due to other messages printed by 
this function. However, this messages will print only once, therefore 
they won't cause the recursion to continue indefinitely.


Acked-by: Antonio Quartulli 


---
  src/openvpn/dco.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 3087a0df..b53332a8 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -133,8 +133,6 @@ dco_get_secondary_key(struct tls_multi *multi, const struct 
key_state *primary)
  bool
  dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
  {
-msg(D_DCO_DEBUG, "%s: peer_id=%d", __func__, multi->dco_peer_id);
-
  /* this function checks if keys have to be swapped or erased, therefore it
   * can't do much if we don't have any key installed
   */


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] Disabe DCO if proxy is set via management

2023-02-20 Thread Antonio Quartulli


Hi,

On 20/02/2023 10:06, Lev Stipakov wrote:

From: Lev Stipakov 

DCO doesn't support proxy and we already disable DCO
is proxy is set in profile.

Signed-off-by: Lev Stipakov 


Acked-by: Antonio Quartulli 


---
  v2: use dco_enabled() helper function

  src/openvpn/init.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b500d354..622239f6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -223,6 +223,12 @@ management_callback_proxy_cmd(void *arg, const char **p)
  }
  else if (p[2] && p[3])
  {
+if (dco_enabled(>options))
+{
+msg(M_INFO, "Proxy set via management, disabling Data Channel 
Offload.");
+c->options.tuntap_options.disable_dco = true;
+}
+
  if (streq(p[1], "HTTP"))
  {
  struct http_proxy_options *ho;


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Improve format specifier for socket handle in Windows

2023-02-10 Thread Antonio Quartulli


Hi,

On 10/02/2023 20:54, Selva Nair wrote:


I also discussed this with Lev and, despite this being different from
what we do in the *nix world (where decimal representations make sense
for file descriptors), it seems to be the right hting to do on Windows
when using HANDLEs (shrug).


Windows kernel handles are very much "like" fds though the handle table 
also covers processes, threads,  semaphores etc. They are not pointers, 
are generally smallish numbers or ~0 (== -1). But not as small as fds in 
the Unix/Linux world --- mainly because several objects share the name 
space and they have to be multiples of 4 for some arcane reason.


The problem here was not printing as "decimal" but as "unsigned 
decimal". Use of the latter would have caused the same issue on Linux too.


Using hex is a compromise here as handles are defined as "unsigned ints" 
of the same size as pointers. So PRIdPTR somehow looks improper though I 
would have chosen that.


Given your explanation then it sounds like we should truly use PRIdPTR.
Wouldn't you agree Lev?

Cheers,



Selva


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Improve format specifier for socket handle in Windows

2023-02-10 Thread Antonio Quartulli


Hi,

On 10/02/2023 14:31, Lev Stipakov wrote:

From: Lev Stipakov 

Socket is a handle on Windows, which is usually logged in hex.
Also an interesting value is INVALID_SOCKET, which is ~0.

PRIuPTR prints decimals, and for INVALID_SOCKET it prints something like

   2023-02-10 14:45:21 us=906000 write to TUN/TAP : Jrjestelmkutsulle
annettu data-alue on liian pieni.   (fd=18446744073709551615,code=122)

PRIxPTR prints hex, and INVALID_SOCKET looks a bit nicer:

   2023-02-10 15:17:11 us=828000 write to TUN/TAP : Jrjestelmkutsulle
annettu data-alue on liian pieni.   (fd=,code=122)

Reported-by: Selva Nair 
Signed-off-by: Lev Stipakov 


I also discussed this with Lev and, despite this being different from 
what we do in the *nix world (where decimal representations make sense 
for file descriptors), it seems to be the right hting to do on Windows 
when using HANDLEs (shrug).


Acked-by: Antonio Quartulli 


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3] configure: enable DCO by default on FreeBSD/Linux


Hi,

On 07/02/2023 14:20, Frank Lichtenheld wrote:

Automatically disabled when
- iproute2 is enabled
   (Don't want to force people specifying --disable-dco explicitely)
- libnv is missing on FreeBSD
   (FreeBSD version too old anyway)

Will still error out if libnl-genl is missing on Linux to
make people aware of the new dependency.

Signed-off-by: Frank Lichtenheld 


Acked-by: Antonio Quartulli 


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3 release/2.6] Allow certain DHCP options to be used without DHCP server


Hi,

On 07/02/2023 15:54, Lev Stipakov wrote:

From: Lev Stipakov 

Followin DHCP options:

   DOMAIN, ADAPTER_DOMAIN_SUFFIX, DNS, WINS

don't require DHCP server in order to be used.

This change allows those options to be used with dco and wintun
drivers. If an option specified which requires DHCP server and
tap-windows6 driver is not used, print a clear error message
instead of obscure reference to --ip-win32.

Reported-by: Marek Zarychta
Signed-off-by: Lev Stipakov 


Code makes sense and does what it says.

Acked-by: Antonio Quartulli 

However, please not that I did not test this code path explicitly as my 
testing capabilities on Windows are pretty limited.


Cheers,


---
  v3: replace SHOW_INT with SHOW_UNSIGNED
  v2: replace enum with defines, which are more suitable
  as bit flags

  src/openvpn/options.c | 39 +++
  src/openvpn/tun.h |  6 +-
  2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ae3faf8..8cbffc5c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1290,7 +1290,7 @@ show_tuntap_options(const struct tuntap_options *o)
  SHOW_INT(dhcp_masq_offset);
  SHOW_INT(dhcp_lease_time);
  SHOW_INT(tap_sleep);
-SHOW_BOOL(dhcp_options);
+SHOW_UNSIGNED(dhcp_options);
  SHOW_BOOL(dhcp_renew);
  SHOW_BOOL(dhcp_pre_release);
  SHOW_STR(domain);
@@ -2478,12 +2478,20 @@ options_postprocess_verify_ce(const struct options 
*options,
  msg(M_USAGE, "On Windows, --ip-win32 doesn't make sense unless --ifconfig 
is also used");
  }
  
-if (options->tuntap_options.dhcp_options

-&& options->windows_driver != WINDOWS_DRIVER_WINTUN
-&& options->tuntap_options.ip_win32_type != IPW32_SET_DHCP_MASQ
-&& options->tuntap_options.ip_win32_type != IPW32_SET_ADAPTIVE)
+if (options->tuntap_options.dhcp_options & DHCP_OPTIONS_DHCP_REQUIRED)
  {
-msg(M_USAGE, "--dhcp-option requires --ip-win32 dynamic or adaptive");
+const char *prefix = "Some dhcp-options require DHCP server";
+if (options->windows_driver != WINDOWS_DRIVER_TAP_WINDOWS6)
+{
+msg(M_USAGE, "%s, which is not supported by selected %s driver",
+prefix, print_windows_driver(options->windows_driver));
+}
+else if (options->tuntap_options.ip_win32_type != IPW32_SET_DHCP_MASQ
+ && options->tuntap_options.ip_win32_type != 
IPW32_SET_ADAPTIVE)
+{
+msg(M_USAGE, "%s, which requires --ip-win32 dynamic or adaptive",
+prefix);
+}
  }
  
  if (options->windows_driver == WINDOWS_DRIVER_WINTUN && dev != DEV_TYPE_TUN)

@@ -8083,16 +8091,17 @@ add_option(struct options *options,
  {
  struct tuntap_options *o = >tuntap_options;
  VERIFY_PERMISSION(OPT_P_DHCPDNS);
-bool ipv6dns = false;
  
  if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))

  && p[2] && !p[3])
  {
  o->domain = p[2];
+o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
  }
  else if (streq(p[1], "NBS") && p[2] && !p[3])
  {
  o->netbios_scope = p[2];
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
  }
  else if (streq(p[1], "NBT") && p[2] && !p[3])
  {
@@ -8104,31 +8113,35 @@ add_option(struct options *options,
  goto err;
  }
  o->netbios_node_type = t;
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
  }
  else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
   && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
  {
  if (strstr(p[2], ":"))
  {
-ipv6dns = true;
  dhcp_option_dns6_parse(p[2], o->dns6, >dns6_len, msglevel);
  }
  else
  {
  dhcp_option_address_parse("DNS", p[2], o->dns, >dns_len, 
msglevel);
+o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
  }
  }
  else if (streq(p[1], "WINS") && p[2] && !p[3])
  {
  dhcp_option_address_parse("WINS", p[2], o->wins, >wins_len, 
msglevel);
+o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
  }
  else if (streq(p[1], "NTP") && p[2] && !p[3])
  {
  dhcp_option_address_parse("NTP", p[2], o->ntp, >ntp_len, 
msglevel);
+o-&g

Re: [Openvpn-devel] [PATCH v2 release/2.6] Allow certain DHCP options to be used without DHCP server


Hi,

On 07/02/2023 14:59, Lev Stipakov wrote:

This definition seems to be unused by the way :)

I am not sure if "0x0003" is better than "3" (we have only two
flags). Something like 0011b would be clearer, but we don't have
SHOW_BINARY.


Personally I find the hex representation better when you want to print a 
bitfield.


The fact we are using hex kinda tells me already that it's not the value 
"3" that we care about.


And later is may become "10". Imho it just gets more confusing.

Cheers,



ti 7. helmik. 2023 klo 15.36 Antonio Quartulli (a...@unstable.cc) kirjoitti:


Hi,

On 07/02/2023 10:42, Lev Stipakov wrote:

From: Lev Stipakov 

Followin DHCP options:

DOMAIN, ADAPTER_DOMAIN_SUFFIX, DNS, WINS

don't require DHCP server in order to be used.

This change allows those options to be used with dco and wintun
drivers. If an option specified which requires DHCP server and
tap-windows6 driver is not used, print a clear error message
instead of obscure reference to --ip-win32.

This fixes https://github.com/OpenVPN/openvpn/issues/239

Reported-by: Marek Zarychta
Signed-off-by: Lev Stipakov 
---
   v2: replace enum with defines, which are more suitable
   as bit flags

   src/openvpn/options.c | 39 +++
   src/openvpn/tun.h |  6 +-
   2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ae3faf8..9c05217c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1290,7 +1290,7 @@ show_tuntap_options(const struct tuntap_options *o)
   SHOW_INT(dhcp_masq_offset);
   SHOW_INT(dhcp_lease_time);
   SHOW_INT(tap_sleep);
-SHOW_BOOL(dhcp_options);
+SHOW_INT(dhcp_options);


wouldn't SHOW_UNSIGNED be more appropriate since this is a bitfield?

This is its definition:

options.c:987:#define SHOW_UNSIGNED(var)  SHOW_PARM(var, o->var, "0x%08x")

Cheers,


   SHOW_BOOL(dhcp_renew);
   SHOW_BOOL(dhcp_pre_release);
   SHOW_STR(domain);
@@ -2478,12 +2478,20 @@ options_postprocess_verify_ce(const struct options 
*options,
   msg(M_USAGE, "On Windows, --ip-win32 doesn't make sense unless --ifconfig 
is also used");
   }

-if (options->tuntap_options.dhcp_options
-&& options->windows_driver != WINDOWS_DRIVER_WINTUN
-&& options->tuntap_options.ip_win32_type != IPW32_SET_DHCP_MASQ
-&& options->tuntap_options.ip_win32_type != IPW32_SET_ADAPTIVE)
+if (options->tuntap_options.dhcp_options & DHCP_OPTIONS_DHCP_REQUIRED)
   {
-msg(M_USAGE, "--dhcp-option requires --ip-win32 dynamic or adaptive");
+const char *prefix = "Some dhcp-options require DHCP server";
+if (options->windows_driver != WINDOWS_DRIVER_TAP_WINDOWS6)
+{
+msg(M_USAGE, "%s, which is not supported by selected %s driver",
+prefix, print_windows_driver(options->windows_driver));
+}
+else if (options->tuntap_options.ip_win32_type != IPW32_SET_DHCP_MASQ
+ && options->tuntap_options.ip_win32_type != 
IPW32_SET_ADAPTIVE)
+{
+msg(M_USAGE, "%s, which requires --ip-win32 dynamic or adaptive",
+prefix);
+}
   }

   if (options->windows_driver == WINDOWS_DRIVER_WINTUN && dev != 
DEV_TYPE_TUN)
@@ -8083,16 +8091,17 @@ add_option(struct options *options,
   {
   struct tuntap_options *o = >tuntap_options;
   VERIFY_PERMISSION(OPT_P_DHCPDNS);
-bool ipv6dns = false;

   if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
   && p[2] && !p[3])
   {
   o->domain = p[2];
+o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
   }
   else if (streq(p[1], "NBS") && p[2] && !p[3])
   {
   o->netbios_scope = p[2];
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
   }
   else if (streq(p[1], "NBT") && p[2] && !p[3])
   {
@@ -8104,31 +8113,35 @@ add_option(struct options *options,
   goto err;
   }
   o->netbios_node_type = t;
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
   }
   else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
&& (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
   {
   if (strstr(p[2], ":"))
   {
-ipv6dns = true;
   dhcp_option_dns6_parse(p[2], o->dns6, >dns6_len, 
msglevel);
   }
   else
   {

Re: [Openvpn-devel] [PATCH v2 release/2.6] Allow certain DHCP options to be used without DHCP server

o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
  }
  else if (streq(p[1], "NBDD") && p[2] && !p[3])
  {
  dhcp_option_address_parse("NBDD", p[2], o->nbdd, >nbdd_len, 
msglevel);
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
  }
  else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
  {
@@ -8141,10 +8154,12 @@ add_option(struct options *options,
  msg(msglevel, "--dhcp-option %s: maximum of %d search entries can 
be specified",
  p[1], N_SEARCH_LIST_LEN);
  }
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
  }
  else if (streq(p[1], "DISABLE-NBT") && !p[2])
  {
  o->disable_nbt = 1;
+o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
  }
  #if defined(TARGET_ANDROID)
  else if (streq(p[1], "PROXY_HTTP") && p[3] && !p[4])
@@ -8158,14 +8173,6 @@ add_option(struct options *options,
  msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or 
unknown parameter", p[1]);
  goto err;
  }
-
-/* flag that we have options to give to the TAP driver's DHCPv4 server
- *  - skipped for "DNS6", as that's not a DHCPv4 option
- */
-if (!ipv6dns)
-{
-o->dhcp_options = true;
-}
  }
  #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
  #ifdef _WIN32
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 3b0a0d24..e19e1a2e 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -62,6 +62,10 @@ enum windows_driver_type {
  #define IPW32_SET_ADAPTIVE_DELAY_WINDOW 300
  #define IPW32_SET_ADAPTIVE_TRY_NETSH20
  
+/* bit flags for DHCP options */

+#define DHCP_OPTIONS_DHCP_OPTIONAL (1<<0)
+#define DHCP_OPTIONS_DHCP_REQUIRED (1<<1)
+
  struct tuntap_options {
  /* --ip-win32 options */
  bool ip_win32_defined;
@@ -90,7 +94,7 @@ struct tuntap_options {
  
  /* --dhcp-option options */
  
-bool dhcp_options;

+int dhcp_options;
  
  const char *domain;  /* DOMAIN (15) */
  


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco_linux: update license for ovpn_dco_linux.h

2023-01-25 Thread Antonio Quartulli

The linux userspace API header has acquired the MIT license (check the
ovpn-dco repository for the related change), therefore we simply bring
this change in our local copy to ensure compliancy.

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/ovpn_dco_linux.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/ovpn_dco_linux.h b/src/openvpn/ovpn_dco_linux.h
index f9a3b827..96395886 100644
--- a/src/openvpn/ovpn_dco_linux.h
+++ b/src/openvpn/ovpn_dco_linux.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: (GPL-2.0-only WITH Linux-syscall-note) OR MIT */
 /*
  *  OpenVPN data channel accelerator
  *
-- 
2.39.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Workaround: make ovpn-dco more reliable

2023-01-18 Thread Antonio Quartulli


Hi,

On 12/01/2023 17:37, Arne Schwabe wrote:

This workaround avoids the kernel trigger ENOBUFS when the kernel
internal queue is overrun with events of disconnectingh clients or
similar. This is a workaround until we come up with a more permanent
solution.

Signed-off-by: Arne Schwabe 


After further discussion and deeper testing, we concluded that it is 
possible to generate so much netlink traffic that we can easily fill the 
buffers and start losing message or get desync'd with kernelspace.


The long term solution is improving ovpn-dco (kernel module) to reduce 
such traffic, however, for now it makes sense to extend the userspace 
buffer in order to decrease the likelihood of filling it up during 
normal operations.


Therefore this patch gets my ACK:

Acked-by: Antonio Quartulli 

[please add spaces around the '*' operator]


---
  src/openvpn/dco_linux.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 5524cdbcd..c84f9cfe1 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -375,6 +375,11 @@ ovpn_dco_init_netlink(dco_context_t *dco)
   * wrong sequence numbers (NLE_SEQ_MISMATCH), so disable libnl's sequence
   * number check */
  nl_socket_disable_seq_check(dco->nl_sock);
+
+/* nl library sets the buffer size to 32k/32k by default which is sometimes
+ * overrun with very fast connecting/disconnecting clients.
+ * TODO: fix this in a better and more reliable way */
+ASSERT(!nl_socket_set_buffer_size(dco->nl_sock, 1024*1024, 1024*1024));
  }
  
  bool


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT breakage on FreeBSD+DCO

2023-01-13 Thread Antonio Quartulli


Hi,

On 13/01/2023 09:07, Gert Doering wrote:

commit 67c4eebdae introduces a new peer disconnect reason (transport
disconnected, aka "TCP session closed") which breaks compilation on
FreeBSD - OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT not part of the
enum in freebsd_dco.h, and no kernel support for TCP anyway.

This patch is an intermediate bandaid, making the offending code in
multi.c "linux only" while a better solution is discussed.

Signed-off-by: Gert Doering 


Hi,

as we are discussing in a nother thread and on IRC, we need to come up 
with a better solution that is flexible enough to prevent this from 
happening in the future.


There are some floating ideas.
Anyway, the discussion will continue in the other thread.

Regarding this patch:
Acked-by: Antonio Quartulli 

Maybe we need a fbsd14 buildbot?

Cheers,


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco: print proper message in case of transport disconnection

2023-01-13 Thread Antonio Quartulli


Hi,

On 13/01/2023 09:44, Gert Doering wrote:

Hi,

On Fri, Jan 13, 2023 at 09:37:49AM +0100, Antonio Quartulli wrote:

On 13/01/2023 09:32, Kristof Provost wrote:

I???m not sure how we???d cope with supporting building on older releases
though. Not a worry just yet, because FreeBSD main is the only version
with DCO support in it, but it???ll be a problem sooner or later.


OpenVPN ships its own "recent"| version of the if_ovpn.h file, no?
So as long as the file contains all the enums it will compile just fine.


Yes, these defines are mirrored in our dco_freebsd.h - so I could just
add it there, but we need to be careful that we never have conflicting
enums in OpenVPN dco_freebsd.h vs. kernel if_ovpn.h.

Or, as Antonio explained to me how it works on linux uapi headers "enums
are only ever appended to, never ever reordered, nothing is ever deleted".

I'm open for anything that lets me ship rc2+patch as FreeBSD port :-)


As I was saying on IRC, IMHO we took this path of using the same names 
for enums on linux and freebsd because it was convenient.

But here we are hitting a shortcoming of this approach.

If we assume that enums are not supposed to be the same across platforms 
(the latter may have different logic, or whatever), we could implement a 
conversion function that takes a platform specific enum as input and 
returns a userspace specific enum.


Platform independent code will then always only deal with userspace 
enums and won't care about what is truly written in the 
platform-specific headers.


This way we draw a clear boundary between "what kernel on platform X 
supports" and "what userspace openvpn can deal with". And situations 
like this should never happen again.


This conversion function would be implemented in the platform specific 
code (i.e. dco_linux.c)


Does it make sense?


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] dco: print proper message in case of transport disconnection

2023-01-13 Thread Antonio Quartulli


Hi,

On 13/01/2023 09:32, Kristof Provost wrote:
I’m not sure how we’d cope with supporting building on older releases 
though. Not a worry just yet, because FreeBSD main is the only version 
with DCO support in it, but it’ll be a problem sooner or later.


OpenVPN ships its own "recent"| version of the if_ovpn.h file, no?
So as long as the file contains all the enums it will compile just fine.

Cheers,


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Repair special-casing of EEXIST for Linux/SITNL route install

2023-01-11 Thread Antonio Quartulli


Hi,

for the netlink/sitnl bits: this makes sense to me.
I agree with Selva that the v6 variant could benefit from the same 
treatment.


However, this patch can also be hacked on its own

Acked-by: Antonio Quartulli 

On 11/01/2023 17:08, Gert Doering wrote:

The code in sitnl_route_set() used to treat "route can not be installed
because it already exists" (EEXIST) as "not an error".

This is arguably a reasonable approach, but needs to handled higher
up - if the low level add_route() function say "no error", we will try
to remove that route later on in delete_route(), possibly removing
someone else's "already existing" route then.

So:
  - remove special case in sitnl_route_set()
  - do not pass NLM_F_REPLACE flag to sitnl_route_set() call - this would
cause netlink to just replace existing routes, never return EEXIST
(see "man netlink(7)")
  - add detailed return code handling to route_add(), assign "2" on "-EEXIST"
(and log appropriate message).

(Note: sitnl_route_set() is a common function for sitnl route add and
delete, but EEXIST can not happen on delete - so this change has no
impact for the "delete" case)

Signed-off-by: Gert Doering 
---
  src/openvpn/networking_sitnl.c |  6 +-
  src/openvpn/route.c| 10 --
  2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index fe124616..92f30044 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -944,10 +944,6 @@ sitnl_route_set(int cmd, uint32_t flags, int ifindex, 
sa_family_t af_family,
  }
  
  ret = sitnl_send(, 0, 0, NULL, NULL);

-if (ret == -EEXIST)
-{
-ret = 0;
-}
  err:
  return ret;
  }
@@ -1177,7 +1173,7 @@ sitnl_route_add(const char *iface, sa_family_t af_family, 
const void *dst,
  scope = RT_SCOPE_LINK;
  }
  
-return sitnl_route_set(RTM_NEWROUTE, NLM_F_CREATE | NLM_F_REPLACE, ifindex,

+return sitnl_route_set(RTM_NEWROUTE, NLM_F_CREATE, ifindex,
 af_family, dst, prefixlen, gw, table, metric, 
scope,
 RTPROT_BOOT, RTN_UNICAST);
  }
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 99f948ba..2db127bb 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1594,8 +1594,14 @@ add_route(struct route_ipv4 *r,
  }
  
  status = 1;

-if (net_route_v4_add(ctx, >network, netmask_to_netbits2(r->netmask),
- >gateway, iface, 0, metric) < 0)
+int ret = net_route_v4_add(ctx, >network, 
netmask_to_netbits2(r->netmask),
+   >gateway, iface, 0, metric);
+if (ret == -EEXIST)
+{
+msg(D_ROUTE, "NOTE: Linux route add command failed because route 
exists");
+status = 2;
+}
+else if (ret < 0)
  {
  msg(M_WARN, "ERROR: Linux route add command failed");
  status = 0;


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco: send SIGUSR1 upon ping timeout

2023-01-11 Thread Antonio Quartulli

When a peer is removed with reason "ping expire", we should kill the
instance with SIGUSR1 and not SIGTERM

Cc: Arne Schwabe 
Signed-off-by: Antonio Quartulli 
--

Arne, I am not 100% sure why but it seems for ping-restart we always use
SIGUSR1, right? but the DCO handling code was apparently using SIGTERM.

What do you think?
---
 src/openvpn/multi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 99123c39..10efffec 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3234,10 +3234,12 @@ process_incoming_del_peer(struct multi_context *m, 
struct multi_instance *mi,
   dco_context_t *dco)
 {
 const char *reason = "ovpn-dco: unknown reason";
+int signal = SIGTERM;
 switch (dco->dco_del_peer_reason)
 {
 case OVPN_DEL_PEER_REASON_EXPIRED:
 reason = "ovpn-dco: ping expired";
+signal = SIGUSR1;
 break;
 
 case OVPN_DEL_PEER_REASON_TRANSPORT_ERROR:
@@ -3270,7 +3272,7 @@ process_incoming_del_peer(struct multi_context *m, struct 
multi_instance *mi,
 mi->context.sig->signal_text = reason;
 mi->context.c2.dco_read_bytes = dco->dco_read_bytes;
 mi->context.c2.dco_write_bytes = dco->dco_write_bytes;
-multi_signal_instance(m, mi, SIGTERM);
+multi_signal_instance(m, mi, signal);
 }
 
 bool
-- 
2.38.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] dco: print proper message in case of transport disconnection

2023-01-11 Thread Antonio Quartulli

Signed-off-by: Antonio Quartulli 
---
--no-verify is required upon commit due to changes in ovpn_dco_linux.h

Little logging improvement for https://github.com/OpenVPN/ovpn-dco/issues/9
---
 src/openvpn/multi.c  | 4 
 src/openvpn/ovpn_dco_linux.h | 5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 77dcaa60..99123c39 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3244,6 +3244,10 @@ process_incoming_del_peer(struct multi_context *m, 
struct multi_instance *mi,
 reason = "ovpn-dco: transport error";
 break;
 
+case OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT:
+reason = "ovpn-dco: transport disconnected";
+break;
+
 case OVPN_DEL_PEER_REASON_USERSPACE:
 /* We assume that is ourselves. Unfortunately, sometimes these
  * events happen with enough delay that they can have an order of
diff --git a/src/openvpn/ovpn_dco_linux.h b/src/openvpn/ovpn_dco_linux.h
index beca1beb..f9a3b827 100644
--- a/src/openvpn/ovpn_dco_linux.h
+++ b/src/openvpn/ovpn_dco_linux.h
@@ -1,8 +1,8 @@
-/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
 /*
  *  OpenVPN data channel accelerator
  *
- *  Copyright (C) 2019-2021 OpenVPN, Inc.
+ *  Copyright (C) 2019-2022 OpenVPN, Inc.
  *
  *  Author:James Yonan 
  *     Antonio Quartulli 
@@ -85,6 +85,7 @@ enum ovpn_del_peer_reason {
OVPN_DEL_PEER_REASON_USERSPACE,
OVPN_DEL_PEER_REASON_EXPIRED,
OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
+   OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
__OVPN_DEL_PEER_REASON_AFTER_LAST
 };
 
-- 
2.38.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode

2023-01-10 Thread Antonio Quartulli


Hi,

On 09/01/2023 21:00, Gert Doering wrote:

p2p --tls-server with no active client/peer logs once per second

   "dco_update_keys: peer_id=-1"

which does exactly nothing, except fill the disk.  So skip the call to
dco_update_keys() if peer_id == -1.

Signed-off-by: Gert Doering 
---
  src/openvpn/forward.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index ae0512fc..2ba8b0fa 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -151,6 +151,12 @@ check_dco_key_status(struct context *c)
  return;
  }
  
+/* no active peer (p2p tls-server mode) */

+if (c->c2.tls_multi->dco_peer_id == -1 )


Please remove the space after -1 (not sure why uncrustify hasn't caught it).


+{
+return;
+}
+
  if (!dco_update_keys(>c1.tuntap->dco, c->c2.tls_multi))
  {
  /* Something bad happened. Kill the connection to



Rest looks good. Thanks!

Acked-by: Antonio Quartulli 

However, as discussed on IRC: *why* are we running the check_tls code is 
the peer has gone away and we have switched the peer-id to -1?


This is the real question.

--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/3] dco: improve comment about hidden debug message

2023-01-03 Thread Antonio Quartulli

While at it also improve the debug message itself
to be more self-explanatory.

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/multi.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index b10a6d8d..8facc66f 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3296,12 +3296,17 @@ multi_process_incoming_dco(struct multi_context *m)
 if (dco->dco_message_type == OVPN_CMD_DEL_PEER
 && dco->dco_del_peer_reason == OVPN_DEL_PEER_REASON_USERSPACE)
 {
-/* we get notified after we kill the peer ourselves and probably
- * have already forgotten about it. This is expected */
+/* we receive OVPN_CMD_DEL_PEER message with reason USERSPACE
+ * after we kill the peer ourselves. This peer may have already
+ * been deleted, so we end up here.
+ * In this case, print the following debug message with DCO_DEBUG
+ * level only to avoid polluting the standard DCO level with this
+ * harmless event.
+ */
 msglevel = D_DCO_DEBUG;
 }
-msg(msglevel, "Received packet for peer-id unknown to OpenVPN: %d, "
-"type %d, reason %d", peer_id, dco->dco_message_type,
+msg(msglevel, "Received DCO message for unknown peer-id: %d, "
+"type %d, del_peer_reason %d", peer_id, dco->dco_message_type,
 dco->dco_del_peer_reason);
 /* Also clear the buffer if this was incoming packet for a dropped 
peer */
 buf_init(>dco_packet_in, 0);
-- 
2.38.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/3] dco: bail out when no peer-specific message is delivered

2023-01-03 Thread Antonio Quartulli

multi_process_incoming_dco() is currently partly processing
messages that were actually discarded. This results in a bogus
message being printed:

"Received packet for peer-id unknown to OpenVPN: -1, type 0, reason 2"

Change the flow so that we bail out immediately when we know that no
message was truly delivered by DCO.
Currently this can be verified by chacking that the peed_is is greater
than -1.

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/multi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 27676de5..b10a6d8d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3270,7 +3270,15 @@ multi_process_incoming_dco(struct multi_context *m)
 
 int peer_id = dco->dco_message_peer_id;
 
-if ((peer_id >= 0) && (peer_id < m->max_clients) && 
(m->instances[peer_id]))
+/* no peer-specific message delivered -> nothing to process.
+ * bail out right away
+ */
+if (peer_id < 0)
+{
+return ret > 0;
+}
+
+if ((peer_id < m->max_clients) && (m->instances[peer_id]))
 {
 mi = m->instances[peer_id];
 if (dco->dco_message_type == OVPN_CMD_PACKET)
-- 
2.38.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/3] dco: properly re-initialize dco_del_peer_reason

2023-01-03 Thread Antonio Quartulli

After processing a message, all fields of the dco object should be
re-initialized so that future processings are not affected by stale
values.

This includes dco_del_peer_reason.

Since its values can start at 0, re-initialize it with -1.

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/multi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 3658e1d5..27676de5 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3301,6 +3301,7 @@ multi_process_incoming_dco(struct multi_context *m)
 
 dco->dco_message_type = 0;
 dco->dco_message_peer_id = -1;
+dco->dco_del_peer_reason = -1;
 dco->dco_read_bytes = 0;
 dco->dco_write_bytes = 0;
 return ret > 0;
-- 
2.38.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Introduce dco_get_peer_stats API and Windows implementation

2022-12-15 Thread Antonio Quartulli


Hi,

On 15/12/2022 13:55, Gert Doering wrote:

Hi,

On Thu, Dec 15, 2022 at 07:49:53AM -0500, Selva Nair wrote:

Possibly we can merge the current patch
for openvpn.exe and then improve it later in bug-fix releases of 2.6?


This was my thinking as well - so the patch is in, and provides at
least some statistics, and room for improvements :-)


Yeah, this was the best course of action in my opinion as well.

Cheers,


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Introduce dco_get_peer_stats API and Windows implementation

2022-12-15 Thread Antonio Quartulli

Hi,

On 15/12/2022 10:31, Lev Stipakov wrote:

Hi,

In non-dco use, the stats as persisted by the management interface are not reset throughout
the lifetime of the process. With dco, what the driver provides is "Peer Stats"
which is reset in OvpnPeerNew() (linux appears to do the same). A quick option > would
be to move the zero-ing to OvpnEvtFileCleanup() which, I guess, would work at least when
persist-tun is in use.

Yeah, without persist-tun client closes the handle, which triggers
OvpnEvtFileCleanup(), which would reset stats.

Or in a new callback that gets called on tun-open. But then it wont be strictly
"Peer Stats".

Yeah that might be an option, to have RESET_STATS ioctl.

Also I wonder if the driver could remember, say, the last pid, and
then reset stats on NEW_PEER if pid != last pid. Since pids are
recycled, we might "leak" stats in unfortunate cases, but that's okay
I guess?

Sorry for jumping in the discussion a bit late.

IMHO the stats belong to the Peer object.
Let's not forget that, apart from windows, DCO supports multi-peer mode,
therefore we have to consider that case as well.

Due to the above, I'd "reset" the stats when a new peer object is
instantiated. Basically when a new peer object is instantiated it should
come with empty stats, as expected.

If the desired behvaiour in userspace is to keep the stats persistent
across a number of events, it should simply be userspace to keep those
stats.

This way we don't mix up the logic of counting the bytes per peer, and
keeping a general picture of the VPN process.

Cheers,

--
Antonio Quartulli

___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 1/3] Improve debug logging of DCO swap key message and Linux dco_new_peer





On 13/12/2022 23:54, Arne Schwabe wrote:

Signed-off-by: Arne Schwabe 
---
  src/openvpn/dco.c   | 18 ++
  src/openvpn/dco_linux.c | 10 --
  2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index feb38cd02..2396bcbf0 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -55,8 +55,8 @@ dco_install_key(struct tls_multi *multi, struct key_state *ks,
  const char *ciphername)
  
  {

-msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d", __func__, multi->dco_peer_id,
-ks->key_id);
+msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d, currently installed %d",


I'd make this "num. installed" because "currently installed" makes me 
think as if what follows is the ID of something that is installed. While 
we should make it explicit that this is the number of installed keys.



+__func__, multi->dco_peer_id, ks->key_id, multi->dco_keys_installed);
  
  /* Install a key in the PRIMARY slot only when no other key exist.

   * From that moment on, any new key will be installed in the SECONDARY
@@ -181,8 +181,18 @@ dco_update_keys(dco_context_t *dco, struct tls_multi 
*multi)
   */
  if (primary->dco_status == DCO_INSTALLED_SECONDARY)
  {
-msg(D_DCO_DEBUG, "Swapping primary and secondary keys, now: id1=%d 
id2=%d",
-primary->key_id, secondary ? secondary->key_id : -1);
+if (secondary)
+{
+msg(D_DCO_DEBUG, "Swapping primary and secondary keys to "
+"primary-id=%d secondary-id=%d",
+primary->key_id, secondary->key_id);
+}
+else
+{
+msg(D_DCO_DEBUG, "Swapping primary and secondary keys to"
+"primary-id=%d secondary-id=(to be deleted)",
+primary->key_id);
+}
  
  int ret = dco_swap_keys(dco, multi->dco_peer_id);

  if (ret < 0)
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 109358205..fbd940c28 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -216,9 +216,15 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int 
sd,
   struct sockaddr *localaddr, struct sockaddr *remoteaddr,
   struct in_addr *remote_in4, struct in6_addr *remote_in6)
  {
-msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d", __func__, peerid, sd);
-
  struct gc_arena gc = gc_new();
+const char *remotestr = "[undefined]";
+if (remoteaddr)
+{
+remotestr = print_sockaddr(remoteaddr, );
+}
+msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d, remote addr: %s", __func__,
+peerid, sd, remotestr);
+
  struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_NEW_PEER);
  struct nlattr *attr = nla_nest_start(nl_msg, OVPN_ATTR_NEW_PEER);
  int ret = -EMSGSIZE;



The rest makes sense:

Acked-by: Antonio Quartulli 

--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 3/3] Set DCO_NOT_INSTALLED also for keys not in the get_key_scan range


Hi,

On 13/12/2022 23:54, Arne Schwabe wrote:

We have 6 key slots but normally only consider 3 of them to be
active/valid keys. Especially the secondary key of TM_LAME_DUCK can
in rare corner cases have a key that is still installed in the kernel.

While this should not cause any issues since I do not see way for this
key to become active ever again, it is better to keep the state correctly.

Signed-off-by: Arne Schwabe 


I am fine with this because it is just "safer".
But I truly dislike that we have to "Defend ourselves" from conditions 
which we don't even know when they may happen

Still, this is a rant for another patch/cleanup.

This patch makes sense
Acked-by: Antonio Quartulli 


---
  src/openvpn/dco.c | 14 +-
  1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 36bfbf10a..20196fe5d 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -221,13 +221,17 @@ dco_update_keys(dco_context_t *dco, struct tls_multi 
*multi)
  multi->dco_keys_installed = 1;
  }
  
-/* all keys that are not installed are set to NOT installed */

-for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+/* all keys that are not installed are set to NOT installed. Include also
+ * keys that might even be considered as active keys to be sure*/
+for (int i = 0; i < TM_SIZE; ++i)
  {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks != primary && ks != secondary)
+for (int j = 0; j < KS_SIZE; j++)
  {
-ks->dco_status = DCO_NOT_INSTALLED;
+struct key_state *ks = >session[i].key[j];
+if (ks != primary && ks != secondary)
+{
+ks->dco_status = DCO_NOT_INSTALLED;
+        }
  }
  }
  return true;


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/3] Trigger a USR1 if dco_update_keys fails


Hi,

On 13/12/2022 23:54, Arne Schwabe wrote:

When dco_update_keys fails, we are in some weird state that we are
unlikely to recover since what userspace and kernel space think of
the keys is very likely to not in sync anymore. So abandon the
connection if this happens.

Signed-off-by: Arne Schwabe 


This makes sense to me.
We didn't do that earlier because we weren't sure about what to doing 
this case, but issuing USR1 and bailing out is actually sensible.


Acked-by: Antonio Quartulli 


---
  src/openvpn/dco.c | 15 ---
  src/openvpn/dco.h |  9 ++---
  src/openvpn/forward.c |  7 ++-
  3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 2396bcbf0..36bfbf10a 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -130,7 +130,7 @@ dco_get_secondary_key(struct tls_multi *multi, const struct 
key_state *primary)
  return NULL;
  }
  
-void

+bool
  dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
  {
  msg(D_DCO_DEBUG, "%s: peer_id=%d", __func__, multi->dco_peer_id);
@@ -140,7 +140,7 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
   */
  if (multi->dco_keys_installed == 0)
  {
-return;
+return true;
  }
  
  struct key_state *primary = tls_select_encryption_key(multi);

@@ -155,18 +155,18 @@ dco_update_keys(dco_context_t *dco, struct tls_multi 
*multi)
  if (ret < 0)
  {
  msg(D_DCO, "Cannot delete primary key during wipe: %s (%d)", 
strerror(-ret), ret);
-return;
+return false;
  }
  
  ret = dco_del_key(dco, multi->dco_peer_id, OVPN_KEY_SLOT_SECONDARY);

  if (ret < 0)
  {
  msg(D_DCO, "Cannot delete secondary key during wipe: %s (%d)", 
strerror(-ret), ret);
-return;
+return false;
  }
  
  multi->dco_keys_installed = 0;

-return;
+return true;
  }
  
  /* if we have a primary key, it must have been installed already (keys

@@ -198,7 +198,7 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
  if (ret < 0)
  {
  msg(D_DCO, "Cannot swap keys: %s (%d)", strerror(-ret), ret);
-return;
+return false;
  }
  
  primary->dco_status = DCO_INSTALLED_PRIMARY;

@@ -216,7 +216,7 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
  if (ret < 0)
  {
  msg(D_DCO, "Cannot delete secondary key: %s (%d)", 
strerror(-ret), ret);
-return;
+return false;
  }
  multi->dco_keys_installed = 1;
  }
@@ -230,6 +230,7 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
  ks->dco_status = DCO_NOT_INSTALLED;
  }
  }
+return true;
  }
  
  static bool

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index e051db068..7e1febaa3 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -164,9 +164,11 @@ int init_key_dco_bi(struct tls_multi *multi, struct 
key_state *ks,
   *
   * @param dco   DCO device context
   * @param multi TLS multi instance
+ *
+ * @return  returns false if an error occurred that is not
+ *  recoverable and should reset the connection
   */
-void dco_update_keys(dco_context_t *dco, struct tls_multi *multi);
-
+bool dco_update_keys(dco_context_t *dco, struct tls_multi *multi);
  /**
   * Install a new peer in DCO - to be called by a CLIENT (or P2P) instance
   *
@@ -304,10 +306,11 @@ init_key_dco_bi(struct tls_multi *multi, struct key_state 
*ks,
  return 0;
  }
  
-static inline void

+static inline bool
  dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
  {
  ASSERT(false);
+return false;
  }
  
  static inline int

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 5cd7eaa6e..8c1e49a34 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -151,7 +151,12 @@ check_dco_key_status(struct context *c)
  return;
  }
  
-dco_update_keys(>c1.tuntap->dco, c->c2.tls_multi);

+if (!dco_update_keys(>c1.tuntap->dco, c->c2.tls_multi))
+{
+/* Something bad happened. Kill the connection to
+ * be able to recover. */
+    register_signal(c, SIGUSR1, "dco update keys error");
+}
  }
  
  /*


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 1/4] Read DCO traffic stats from the kernel


Hi,

On 05/12/2022 17:41, Kristof Provost via Openvpn-devel wrote:

From: Kristof Provost 

When DCO is active userspace doesn't see all of the traffic, so when we
access these stats we must update them.

Retrieve kernel statistics every time we access the
link_(read|write)_bytes values.

Introduce a dco_(read|write)_bytes so that we don't clobber the existing
statistics, which still count control packets, sent or received directly
through the socket.

Signed-off-by: Kristof Provost 


Acked-by: Antonio Quartulli 


---
  src/openvpn/dco.h  |  8 
  src/openvpn/dco_freebsd.c  | 78 ++
  src/openvpn/dco_linux.c|  7 +++
  src/openvpn/dco_win.c  |  7 +++
  src/openvpn/multi.c| 30 +++--
  src/openvpn/openvpn.h  |  2 +
  src/openvpn/ovpn_dco_freebsd.h |  1 +
  7 files changed, 120 insertions(+), 13 deletions(-)

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index e051db06..e5d89358 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -225,6 +225,14 @@ void dco_install_iroute(struct multi_context *m, struct 
multi_instance *mi,
   */
  void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi);
  
+/**

+ * Update traffic statistics for all peers
+ *
+ * @param dco  DCO device context
+ * @param mthe server context
+ **/
+int dco_get_peer_stats(dco_context_t *dco, struct multi_context *m);
+
  /**
   * Retrieve the list of ciphers supported by the current platform
   *
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index a52ac8c1..5b352859 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -37,6 +37,7 @@
  #include "dco.h"
  #include "tun.h"
  #include "crypto.h"
+#include "multi.h"
  #include "ssl_common.h"
  
  static nvlist_t *

@@ -641,6 +642,83 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
  nvlist_destroy(nvl);
  }
  
+static void

+dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
+{
+struct hash_element *he;
+struct hash_iterator hi;
+
+hash_iterator_init(m->hash, );
+
+while ((he = hash_iterator_next()))
+{
+struct multi_instance *mi = (struct multi_instance *) he->value;
+
+if (mi->context.c2.tls_multi->peer_id != peerid)
+continue;
+
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
+
+return;
+}
+
+msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);
+}
+
+int
+dco_get_peer_stats(dco_context_t *dco, struct multi_context *m)
+{
+
+struct ifdrv drv;
+uint8_t buf[4096];
+nvlist_t *nvl;
+const nvlist_t *const *nvpeers;
+size_t npeers;
+int ret;
+
+if (!dco || !dco->open)
+{
+return 0;
+}
+
+CLEAR(drv);
+snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname);
+drv.ifd_cmd = OVPN_GET_PEER_STATS;
+drv.ifd_len = sizeof(buf);
+drv.ifd_data = buf;
+
+ret = ioctl(dco->fd, SIOCGDRVSPEC, );
+if (ret)
+{
+msg(M_WARN | M_ERRNO, "Failed to get peer stats");
+return -EINVAL;
+}
+
+nvl = nvlist_unpack(buf, drv.ifd_len, 0);
+if (! nvl)
+{
+msg(M_WARN, "Failed to unpack nvlist");
+return -EINVAL;
+}
+
+if (! nvlist_exists_nvlist_array(nvl, "peers")) {
+/* no peers */
+return 0;
+}
+
+nvpeers = nvlist_get_nvlist_array(nvl, "peers", );
+for (size_t i = 0; i < npeers; i++)
+{
+const nvlist_t *peer = nvpeers[i];
+uint32_t peerid = nvlist_get_number(peer, "peerid");
+
+dco_update_peer_stat(m, peerid, nvlist_get_nvlist(peer, "bytes"));
+}
+
+return 0;
+}
+
  const char *
  dco_get_supported_ciphers()
  {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 10935820..0306cec3 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -911,6 +911,13 @@ nla_put_failure:
  return ret;
  }
  
+int

+dco_get_peer_stats(dco_context_t *dco, struct multi_context *m)
+{
+/* Not implemented. */
+return 0;
+}
+
  bool
  dco_available(int msglevel)
  {
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 48a1755a..68ec931c 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -399,6 +399,13 @@ dco_do_write(dco_context_t *dco, int peer_id, struct 
buffer *buf)
  return 0;
  }
  
+int

+dco_get_peer_stats(dco_context_t *dco, struct multi_context *m)
+{
+/* Not implemented. */
+return 0;
+}
+
  void
  dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
  {
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 0a23c2bc..38da87b8 100644
--- a/src/openvpn/multi.c
+++ b/src/o

Re: [Openvpn-devel] [PATCH 1/4] Read DCO traffic stats from the kernel