diff mbox series

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

Message ID 20230322192757.20767-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel,v4] dco-linux: implement dco_get_peer_stats{, multi} API | expand

Commit Message

Antonio Quartulli March 22, 2023, 7:27 p.m. UTC 
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 <a@unstable.cc>
---

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(-)

Comments

Gert Doering March 23, 2023, 12:03 p.m. UTC | #1 
Hi,

On Wed, Mar 22, 2023 at 08:27:57PM +0100, Antonio Quartulli wrote:
> 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 <a@unstable.cc>

This *looks* very reasonable, but only half of it works for me.

...
tun-udp-p2mp[492453]: OpenVPN 2.7_git [git:vw/master/5a8fb55ac8cf4019] x86_64-pc-linux-gnu [SSL (mbed TLS)] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] [DCO] built on Mar 23 2023
tun-udp-p2mp[492453]: DCO version: 0.2.20230313
...
tun-udp-p2mp[492454]: dco_get_peer_stats_multi
tun-udp-p2mp[492454]: dco_parse_peer_multi: parsing message...
tun-udp-p2mp[492454]: dco_update_peer_stat: no link RX bytes provided in reply for peer 0
tun-udp-p2mp[492454]: dco_update_peer_stat: no link TX bytes provided in reply for peer 0
tun-udp-p2mp[492454]: dco_update_peer_stat / tun_read_bytes: 240
tun-udp-p2mp[492454]: dco_update_peer_stat / tun_write_bytes: 96
tun-udp-p2mp[492454]: dco_parse_peer_multi: parsing message...
tun-udp-p2mp[492454]: dco_update_peer_stat: no link RX bytes provided in reply for peer 1
tun-udp-p2mp[492454]: dco_update_peer_stat: no link TX bytes provided in reply for peer 1
tun-udp-p2mp[492454]: dco_update_peer_stat / tun_read_bytes: 776
tun-udp-p2mp[492454]: dco_update_peer_stat / tun_write_bytes: 80


So neither for p2p tls nor for --server setups, I am receiving "link bytes",
only tun*bytes.

I have not checked the kernel side if I *should* receive anything, but
this looks incomplete - either the kernel side is not yet ready (not
a showstopper) or there is a desync in the tags used, which might or
might not prevent merging...

Antonio, any ideas?

gert
Antonio Quartulli March 23, 2023, 12:15 p.m. UTC | #2 
Spot on and sorry for forgetting to mentioning it:

You need ovpn-dco at this commit:

commit 726fdfe0fa21aa4e87c5a60294ea0365ce7b6809 (HEAD -> master, 
origin/master)
Author: Antonio Quartulli <antonio@openvpn.net>
Date:   Mon Mar 20 23:50:52 2023 +0100

     ovpn-dco: store and report transport rx/tx stats as well

     Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

Which comes right after the one you are using.

It is not a breaking change (as you see there is no real *failure*, just 
missing information from the kernel side).

Regards,

On 23/03/2023 13:03, Gert Doering wrote:
> Hi,
> 
> On Wed, Mar 22, 2023 at 08:27:57PM +0100, Antonio Quartulli wrote:
>> 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 <a@unstable.cc>
> 
> This *looks* very reasonable, but only half of it works for me.
> 
> ...
> tun-udp-p2mp[492453]: OpenVPN 2.7_git [git:vw/master/5a8fb55ac8cf4019] x86_64-pc-linux-gnu [SSL (mbed TLS)] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] [DCO] built on Mar 23 2023
> tun-udp-p2mp[492453]: DCO version: 0.2.20230313
> ...
> tun-udp-p2mp[492454]: dco_get_peer_stats_multi
> tun-udp-p2mp[492454]: dco_parse_peer_multi: parsing message...
> tun-udp-p2mp[492454]: dco_update_peer_stat: no link RX bytes provided in reply for peer 0
> tun-udp-p2mp[492454]: dco_update_peer_stat: no link TX bytes provided in reply for peer 0
> tun-udp-p2mp[492454]: dco_update_peer_stat / tun_read_bytes: 240
> tun-udp-p2mp[492454]: dco_update_peer_stat / tun_write_bytes: 96
> tun-udp-p2mp[492454]: dco_parse_peer_multi: parsing message...
> tun-udp-p2mp[492454]: dco_update_peer_stat: no link RX bytes provided in reply for peer 1
> tun-udp-p2mp[492454]: dco_update_peer_stat: no link TX bytes provided in reply for peer 1
> tun-udp-p2mp[492454]: dco_update_peer_stat / tun_read_bytes: 776
> tun-udp-p2mp[492454]: dco_update_peer_stat / tun_write_bytes: 80
> 
> 
> So neither for p2p tls nor for --server setups, I am receiving "link bytes",
> only tun*bytes.
> 
> I have not checked the kernel side if I *should* receive anything, but
> this looks incomplete - either the kernel side is not yet ready (not
> a showstopper) or there is a desync in the tags used, which might or
> might not prevent merging...
> 
> Antonio, any ideas?
> 
> gert
Gert Doering March 23, 2023, 12:29 p.m. UTC | #3 
Acked-by: Gert Doering <gert@greenie.muc.de>

This only touches linux only files, so only tested on Linux (builds with
and without DCO).  The patch looks larger than it is because of a new
argument to ovpn_nl_msg_send(), but for the "existing code" this is
unused (extra argument to the callback function), so no change.  The
actual new code looks quite reasonable.

Testing confirms that we have counters now, for both server (status file)
and client (SIGUSR2) use.

There is a catch to it - the userland side has support for "link bytes"
and "tun bytes" (which end up in dco*bytes), but the "recommended kernel
version" from ChangeLog does not have the link counters yet - so all
queries end up with output like this:

tun-udp-p2mp[492454]: dco_update_peer_stat: no link RX bytes provided in reply for peer 0
tun-udp-p2mp[492454]: dco_update_peer_stat: no link TX bytes provided in reply for peer 0
tun-udp-p2mp[492454]: dco_update_peer_stat / tun_read_bytes: 5600
tun-udp-p2mp[492454]: dco_update_peer_stat / tun_write_bytes: 3592

upgrading kernel commit to commit 726fdfe0f brings both counters (and
the values correlate to "which peer did what?").

tun-udp-p2mp[493935]: dco_update_peer_stat / dco_read_bytes: 132176
tun-udp-p2mp[493935]: dco_update_peer_stat / dco_write_bytes: 131600
tun-udp-p2mp[493935]: dco_update_peer_stat / tun_read_bytes: 128096
tun-udp-p2mp[493935]: dco_update_peer_stat / tun_write_bytes: 127640



Your patch has been applied to the master and release/2.6 branch.

commit 5a8fb55ac8cf4019afee884d3be545ddf87435a4 (master)
commit 1fd69b9085f4c21542ec506d101eb73b3cd0abc4 (release/2.6)
Author: Antonio Quartulli
Date:   Wed Mar 22 20:27:57 2023 +0100

     dco-linux: implement dco_get_peer_stats{, multi} API

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20230322192757.20767-1-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26481.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

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_msg    the message to use
  * @param cb        An optional callback if the caller expects an answer
+ * @param cb_arg    An optional param to pass to the callback
  * @param prefix    A prefix to report in the error message to give the user context
  * @return          status of sending the message
  */
 static int
 ovpn_nl_msg_send(dco_context_t *dco, struct nl_msg *nl_msg, ovpn_nl_cb cb,
-                 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",
+            __func__, id);
+    }
+
+    if (tb[OVPN_GET_PEER_RESP_ATTR_VPN_RX_BYTES])
+    {
+        c2->tun_read_bytes = nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_VPN_RX_BYTES]);
+        msg(D_DCO_DEBUG, "%s / tun_read_bytes: %lu", __func__,
+            c2->tun_read_bytes);
+    }
+    else
+    {
+        msg(M_WARN, "%s: no VPN RX bytes provided in reply for peer %u",
+            __func__, id);
+    }
+
+    if (tb[OVPN_GET_PEER_RESP_ATTR_VPN_TX_BYTES])
+    {
+        c2->tun_write_bytes = nla_get_u64(tb[OVPN_GET_PEER_RESP_ATTR_VPN_TX_BYTES]);
+        msg(D_DCO_DEBUG, "%s / tun_write_bytes: %lu", __func__,
+            c2->tun_write_bytes);
+    }
+    else
+    {
+        msg(M_WARN, "%s: no VPN TX bytes provided in reply for peer %u",
+            __func__, id);
+    }
+}
+
+int
+dco_parse_peer_multi(struct nl_msg *msg, void *arg)
+{
+    struct nlattr *tb[OVPN_ATTR_MAX + 1];
+    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
+
+    msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
+
+    nla_parse(tb, OVPN_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
+              genlmsg_attrlen(gnlh, 0), NULL);
+
+    if (!tb[OVPN_ATTR_GET_PEER])
+    {
+        return NL_SKIP;
+    }
+
+    struct nlattr *tb_peer[OVPN_GET_PEER_RESP_ATTR_MAX + 1];
+
+    nla_parse(tb_peer, OVPN_GET_PEER_RESP_ATTR_MAX,
+              nla_data(tb[OVPN_ATTR_GET_PEER]),
+              nla_len(tb[OVPN_ATTR_GET_PEER]), NULL);
+
+    if (!tb_peer[OVPN_GET_PEER_RESP_ATTR_PEER_ID])
+    {
+        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        return NL_SKIP;
+    }
+
+    struct multi_context *m = arg;
+    uint32_t peer_id = nla_get_u32(tb_peer[OVPN_GET_PEER_RESP_ATTR_PEER_ID]);
+
+    if (peer_id >= m->max_clients || !m->instances[peer_id])
+    {
+        msg(M_WARN, "%s: cannot store DCO stats for peer %u", __func__,
+            peer_id);
+        return NL_SKIP;
+    }
+
+    dco_update_peer_stat(&m->instances[peer_id]->context.c2, tb_peer, peer_id);
+
+    return NL_OK;
+}
+
 int
 dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m)
 {
-    /* Not implemented. */
-    return 0;
+    msg(D_DCO_DEBUG, "%s", __func__);
+
+    struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_GET_PEER);
+
+    nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP;
+
+    return ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer_multi, m, __func__);
+}
+
+static int
+dco_parse_peer(struct nl_msg *msg, void *arg)
+{
+    struct context *c = arg;
+    struct nlattr *tb[OVPN_ATTR_MAX + 1];
+    struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
+
+    msg(D_DCO_DEBUG, "%s: parsing message...", __func__);
+
+    nla_parse(tb, OVPN_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
+              genlmsg_attrlen(gnlh, 0), NULL);
+
+    if (!tb[OVPN_ATTR_GET_PEER])
+    {
+        msg(D_DCO_DEBUG, "%s: malformed reply", __func__);
+        return NL_SKIP;
+    }
+
+    struct nlattr *tb_peer[OVPN_GET_PEER_RESP_ATTR_MAX + 1];
+
+    nla_parse(tb_peer, OVPN_GET_PEER_RESP_ATTR_MAX,
+              nla_data(tb[OVPN_ATTR_GET_PEER]),
+              nla_len(tb[OVPN_ATTR_GET_PEER]), NULL);
+
+    if (!tb_peer[OVPN_GET_PEER_RESP_ATTR_PEER_ID])
+    {
+        msg(M_WARN, "%s: no peer-id provided in reply", __func__);
+        return NL_SKIP;
+    }
+
+    uint32_t peer_id = nla_get_u32(tb_peer[OVPN_GET_PEER_RESP_ATTR_PEER_ID]);
+    if (c->c2.tls_multi->dco_peer_id != peer_id)
+    {
+        return NL_SKIP;
+    }
+
+    dco_update_peer_stat(&c->c2, tb_peer, peer_id);
+
+    return NL_OK;
 }
 
 int
 dco_get_peer_stats(struct context *c)
 {
-    /* Not implemented. */
-    return 0;
+    uint32_t peer_id = c->c2.tls_multi->dco_peer_id;
+    msg(D_DCO_DEBUG, "%s: peer-id %d", __func__, peer_id);
+
+    dco_context_t *dco = &c->c1.tuntap->dco;
+    struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_GET_PEER);
+    struct nlattr *attr = nla_nest_start(nl_msg, OVPN_ATTR_GET_PEER);
+    int ret = -EMSGSIZE;
+
+    NLA_PUT_U32(nl_msg, OVPN_GET_PEER_ATTR_PEER_ID, peer_id);
+    nla_nest_end(nl_msg, attr);
+
+    ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer, c, __func__);
+
+nla_put_failure:
+    nlmsg_free(nl_msg);
+    return ret;
 }
 
 bool
diff --git a/src/openvpn/ovpn_dco_linux.h b/src/openvpn/ovpn_dco_linux.h
index d3fd9a89..73e19b59 100644
--- a/src/openvpn/ovpn_dco_linux.h
+++ b/src/openvpn/ovpn_dco_linux.h
@@ -2,7 +2,7 @@ 
 /*
  *  OpenVPN data channel accelerator
  *
- *  Copyright (C) 2019-2022 OpenVPN, Inc.
+ *  Copyright (C) 2019-2023 OpenVPN, Inc.
  *
  *  Author:	James Yonan <james@openvpn.net>
  *		Antonio Quartulli <antonio@openvpn.net>
@@ -188,10 +188,14 @@  enum ovpn_netlink_get_peer_response_attrs {
 	OVPN_GET_PEER_RESP_ATTR_LOCAL_PORT,
 	OVPN_GET_PEER_RESP_ATTR_KEEPALIVE_INTERVAL,
 	OVPN_GET_PEER_RESP_ATTR_KEEPALIVE_TIMEOUT,
-	OVPN_GET_PEER_RESP_ATTR_RX_BYTES,
-	OVPN_GET_PEER_RESP_ATTR_TX_BYTES,
-	OVPN_GET_PEER_RESP_ATTR_RX_PACKETS,
-	OVPN_GET_PEER_RESP_ATTR_TX_PACKETS,
+	OVPN_GET_PEER_RESP_ATTR_VPN_RX_BYTES,
+	OVPN_GET_PEER_RESP_ATTR_VPN_TX_BYTES,
+	OVPN_GET_PEER_RESP_ATTR_VPN_RX_PACKETS,
+	OVPN_GET_PEER_RESP_ATTR_VPN_TX_PACKETS,
+	OVPN_GET_PEER_RESP_ATTR_LINK_RX_BYTES,
+	OVPN_GET_PEER_RESP_ATTR_LINK_TX_BYTES,
+	OVPN_GET_PEER_RESP_ATTR_LINK_RX_PACKETS,
+	OVPN_GET_PEER_RESP_ATTR_LINK_TX_PACKETS,
 
 	__OVPN_GET_PEER_RESP_ATTR_AFTER_LAST,
 	OVPN_GET_PEER_RESP_ATTR_MAX = __OVPN_GET_PEER_RESP_ATTR_AFTER_LAST - 1,