| Message ID | 20221205164103.9190-3-kprovost@netgate.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,1/4] Read DCO traffic stats from the kernel | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> This is indeed an important missing piece for correct stats (... to be handed to --client-disconnect scripts, etc). I have not tested the feature as such (kernel side support has landed but I have not yet rebuilt that system). I *have* tested the tree on Linux/FreeBSD with/without DCO, and everything works as before. Looking at the code: storing these in dco_context "to be able to transport to the caller" sounds like an reasonable approach, because the dco_do_read() functions know "nothing else" - we could introduce call-by-ref variables, but that won't make the code easier to read. The nested nvlists (bytes->{in,out}) here add a level of trust to the running kernel - should "in" be missing, nvlist_get_number(9) documents "the program will be aborted". The existance of "bytes" is checked, so old/new kernel compat is fine (OTOH if we do not trust the kernel module here, we're lost anyway). Just sayin'. Your patch has been applied to the master and release/2.6 branch. commit 6674963debfb88c0dd3dd4eae4533010ffc319b1 (master) commit f05c7a8a11329cc579d3751469b597e0eab1317f (HEAD -> release/2.6) Author: Kristof Provost Date: Mon Dec 5 17:41:01 2022 +0100 dco: Update counters when a client disconnects Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20221205164103.9190-3-kprovost@netgate.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25614.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 5b352859..2ae46589 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -528,6 +528,15 @@ dco_do_read(dco_context_t *dco) else { dco->dco_del_peer_reason = OVPN_DEL_PEER_REASON_EXPIRED; + + if (nvlist_exists_nvlist(nvl, "bytes")) + { + const nvlist_t *bytes = nvlist_get_nvlist(nvl, "bytes"); + + dco->dco_read_bytes = nvlist_get_number(bytes, "in"); + dco->dco_write_bytes = nvlist_get_number(bytes, "out"); + } + dco->dco_message_type = OVPN_CMD_DEL_PEER; } diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h index 7de11697..0d059dda 100644 --- a/src/openvpn/dco_freebsd.h +++ b/src/openvpn/dco_freebsd.h @@ -55,6 +55,8 @@ typedef struct dco_context { int dco_message_type; int dco_message_peer_id; int dco_del_peer_reason; + uint64_t dco_read_bytes; + uint64_t dco_write_bytes; } dco_context_t; #endif /* defined(ENABLE_DCO) && defined(TARGET_FREEBSD) */ diff --git a/src/openvpn/dco_linux.h b/src/openvpn/dco_linux.h index 416ea30a..7d56308b 100644 --- a/src/openvpn/dco_linux.h +++ b/src/openvpn/dco_linux.h @@ -53,6 +53,8 @@ typedef struct int dco_message_type; int dco_message_peer_id; int dco_del_peer_reason; + uint64_t dco_read_bytes; + uint64_t dco_write_bytes; } dco_context_t; #endif /* defined(ENABLE_DCO) && defined(TARGET_LINUX) */ diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 38da87b8..74671303 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3245,6 +3245,8 @@ process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi, * installed, and we do not need to clean up the state in the kernel */ mi->context.c2.tls_multi->dco_peer_id = -1; 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); } @@ -3278,6 +3280,8 @@ multi_process_incoming_dco(struct multi_context *m) dco->dco_message_type = 0; dco->dco_message_peer_id = -1; + dco->dco_read_bytes = 0; + dco->dco_write_bytes = 0; return ret > 0; } #endif /* if defined(ENABLE_DCO) && defined(TARGET_LINUX) */