From patchwork Fri Aug 29 14:43:23 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4380 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:664d:b0:671:5a2c:6455 with SMTP id eu13csp414596mac; Fri, 29 Aug 2025 07:43:50 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCUY/uMTTm6o42VhLkHjf5UuBhZ993WPBG94o+HIphqoBdbnY+sbBUh2LDMUC+w5nEujqwZQvU2xbBQ=@openvpn.net X-Google-Smtp-Source: AGHT+IFgObwAaJKCwQmwE/lcchgtiHMROeKX+zTZITqkkD93NVXGrr5DsHBfkC+/9/U1/72j666K X-Received: by 2002:a05:6830:2472:b0:745:4fe0:e164 with SMTP id 46e09a7af769-7454fe0e849mr2424501a34.25.1756478630098; Fri, 29 Aug 2025 07:43:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1756478630; cv=none; d=google.com; s=arc-20240605; b=VOA45GeSympmq9hl1kNdg6PZL32rUxXcy14mF6odamik8Z7i7Uiu5HeDLpGaCtW3Ud Ryd+aHwB4fTIABmJZuRyBFkYZhCt8pL44dY43xVycOVBHXCSfKsxq3aVbR4E+RwCunHs j0OK2RS9p2gHiKyNRDUPBgfWZgEg4aLdrWI+LQBuqAIVBVN7Yy5RlHhix6Hvp08f84uN YsOYFZ7WJBZrvfIoCLCuIu1ddWgYDch5+IgCiA8hHXzNcxINcrtpU1SRIzk93L442sSS esLt8IKO4PPevSGI+jLG7hi5Gu4cyoWpE6ZH+xHTtgsC1csCXAumUNrBuX9Dfee5RHuP imQg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=qigksuGl9mgOiX+AwsdVkKPGTa5Ljzdlv/g1QTyPYLU=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=NN4vaGvQzHMt8W+AHok9JQBS7tHp4gm+RjW/C5EAvnK94zm1EzjpZlOFNZvqj5ED17 LCK5NS/JpeK/rYpsqEUSUmfOrJ2w+AIojLAf5QJlegSKxJ9coKxiBamJZbdKFMc+AujJ jMDdZWltgK81lxY0DUDY96FNJQ9L7UxiE9p4TeNnrpB8e0vKRfeucOpZiJDwMGFVUDX/ xi74ebJmYDZVz5ZFU0BR0lRF3qVOeVeabB+ecT2XlcVMETPCiqDlnY5lTxj1wYlKERAV E3OAWH4tJy4t6OZKaqQi+4G61+FjBqkU3ot4aA69xszPiDFT+Ea/g6dAE3yy8LDys5wF LneQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b="Sad/cLzF"; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=ISPri6gO; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="ZmMW/1q7"; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 46e09a7af769-745585c0924si445807a34.344.2025.08.29.07.43.49 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 29 Aug 2025 07:43:49 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b="Sad/cLzF"; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=ISPri6gO; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="ZmMW/1q7"; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-ID:Date:To:From:Sender: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=qigksuGl9mgOiX+AwsdVkKPGTa5Ljzdlv/g1QTyPYLU=; b=Sad/cLzF2xfiAmklQX906DAuH8 O0l8HM7Zpx/yPLKbVtPKdghzFR8HnRofuR56lNuDHvzy59XdxKrwNKzUTQB81fdcdWIWIK5ouoF3X gnf2w9QgEtwtTGljKxEB3Qe8KGpkdL8oH4DLIStgJWCiqb9efRxvlxolLnWn8omAG1HY=; Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1us0KP-0003C5-CZ; Fri, 29 Aug 2025 14:43:46 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1us0KN-0003Bu-Te for openvpn-devel@lists.sourceforge.net; Fri, 29 Aug 2025 14:43:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=gg5ciAbd2IwBQS5bTDJyhfkzpvt/L6e9pxWJ5b0lCD0=; b=ISPri6gOdwSbEVBxVAp/8sVB1T aMv1xBtvjiqsmNIzO/bTtoc8qEjgAuonxZ4H7fqEHebAVrRvWC1g1gdNp/JuA5pDQfqNo44jK8DLp vHSNSuCHO55WeIyRXolEOWACphSD6tf45JEZDq43LQGKbOHe+SrWswGcwS+8MuKZ+KVA=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=gg5ciAbd2IwBQS5bTDJyhfkzpvt/L6e9pxWJ5b0lCD0=; b=ZmMW/1q7j8Bu9qxgP/wfGhHNxo iD3w8fZDefsfq70SQw2AA9202l6VyqYZ/YwX0FL6MwGa95qXvY8CLEdaHLDerYSb/fnVHKP3HPy3P 5GW8FmqvAeWlqv11IJzMnaT6byK9GfZvJwtmjtQxg3ruLMjYUQHEwzZn8rlgbnpn9t7o=; Received: from [193.149.48.143] (helo=blue.greenie.muc.de) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1us0KN-0002HM-7T for openvpn-devel@lists.sourceforge.net; Fri, 29 Aug 2025 14:43:44 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.18.1/8.18.1) with ESMTP id 57TEhVWr024503 for ; Fri, 29 Aug 2025 16:43:31 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.1/8.18.1/Submit) id 57TEhVvC024502 for openvpn-devel@lists.sourceforge.net; Fri, 29 Aug 2025 16:43:31 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Fri, 29 Aug 2025 16:43:23 +0200 Message-ID: <20250829144331.24487-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.49.1 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: 1.3 (+) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-1.hosts.colo.sdot.me", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Lev Stipakov There are few issues with it: - when using DCO, the server part doesn't output BYTECOUNT_CLI since process_incoming_link_part1/process_outgoing_link are not called Content analysis details: (1.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 1.3 RDNS_NONE Delivered to internal network by a host with no rDNS X-Headers-End: 1us0KN-0002HM-7T Subject: [Openvpn-devel] [PATCH v1] Refactor management bytecount tracking X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1841801336050797445?= X-GMAIL-MSGID: =?utf-8?q?1841801336050797445?= From: Lev Stipakov There are few issues with it: - when using DCO, the server part doesn't output BYTECOUNT_CLI since process_incoming_link_part1/process_outgoing_link are not called - when using DCO, the server part applies bytecount timer to the each connection, unneccessary making too many calls to the kernel and also uses incorect BYTECOUNT output. - client part outputs counters using timer, server part utilizes traffic activity -> inconsistency Following changes have been made: - Use timer to output counters in client and server mode. Code which deals with bytecount on traffic activity has been removed. This unifies DCO and non-DCO, as well as client and server mode - In server mode, peers stats are fetched with the single ioctl call - Per-packet stats are not persisted anymore in the client mode during traffic activity. Instead cumulative stats (including DCO stats) are persisted when the session closes. GitHub: https://github.com/OpenVPN/openvpn/issues/820 Change-Id: I43a93f0d84f01fd808a64115e1b8c3b806706491 Signed-off-by: Lev Stipakov Acked-by: Ralf Lici --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1162 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Ralf Lici diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 75ca9d5..03b6a0c 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -818,7 +818,7 @@ #ifdef ENABLE_MANAGEMENT if (management) { - management_check_bytecount(c, management, &c->c2.timeval); + management_check_bytecount_client(c, management, &c->c2.timeval); } #endif /* ENABLE_MANAGEMENT */ } @@ -998,14 +998,6 @@ } #endif c->c2.original_recv_size = c->c2.buf.len; -#ifdef ENABLE_MANAGEMENT - if (management) - { - management_bytes_client(management, c->c2.buf.len, 0); - management_bytes_server(management, &c->c2.link_read_bytes, &c->c2.link_write_bytes, - &c->c2.mda_context); - } -#endif } else { @@ -1823,14 +1815,6 @@ mmap_stats->link_write_bytes = link_write_bytes_global; } #endif -#ifdef ENABLE_MANAGEMENT - if (management) - { - management_bytes_client(management, 0, size); - management_bytes_server(management, &c->c2.link_read_bytes, - &c->c2.link_write_bytes, &c->c2.mda_context); - } -#endif } } diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index aed04f5..bcc7250 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -41,6 +41,7 @@ #include "manage.h" #include "openvpn.h" #include "dco.h" +#include "multi.h" #include "memdbg.h" @@ -517,29 +518,27 @@ } static void -man_bytecount_output_client(struct management *man, counter_type dco_read_bytes, - counter_type dco_write_bytes) +man_bytecount_output_client(counter_type bytes_in_total, counter_type bytes_out_total) { char in[32]; char out[32]; /* do in a roundabout way to work around possible mingw or mingw-glibc bug */ - snprintf(in, sizeof(in), counter_format, man->persist.bytes_in + dco_read_bytes); - snprintf(out, sizeof(out), counter_format, man->persist.bytes_out + dco_write_bytes); + snprintf(in, sizeof(in), counter_format, bytes_in_total); + snprintf(out, sizeof(out), counter_format, bytes_out_total); msg(M_CLIENT, ">BYTECOUNT:%s,%s", in, out); } -void -man_bytecount_output_server(const counter_type *bytes_in_total, const counter_type *bytes_out_total, +static void +man_bytecount_output_server(const counter_type bytes_in_total, const counter_type bytes_out_total, struct man_def_auth_context *mdac) { char in[32]; char out[32]; /* do in a roundabout way to work around possible mingw or mingw-glibc bug */ - snprintf(in, sizeof(in), counter_format, *bytes_in_total); - snprintf(out, sizeof(out), counter_format, *bytes_out_total); + snprintf(in, sizeof(in), counter_format, bytes_in_total); + snprintf(out, sizeof(out), counter_format, bytes_out_total); msg(M_CLIENT, ">BYTECOUNT_CLI:%lu,%s,%s", mdac->cid, in, out); - mdac->bytecount_last_update = now; } static void @@ -4065,42 +4064,70 @@ } void -management_check_bytecount(struct context *c, struct management *man, struct timeval *timeval) +management_check_bytecount_client(struct context *c, struct management *man, struct timeval *timeval) { - if (event_timeout_trigger(&man->connection.bytecount_update_interval, timeval, ETT_DEFAULT)) + if (!(man->persist.callback.flags & MCF_SERVER) && event_timeout_trigger(&man->connection.bytecount_update_interval, timeval, ETT_DEFAULT)) { - counter_type dco_read_bytes = 0; - counter_type dco_write_bytes = 0; - if (dco_enabled(&c->options)) { if (dco_get_peer_stats(c, true) < 0) { return; } - - dco_read_bytes = c->c2.dco_read_bytes; - dco_write_bytes = c->c2.dco_write_bytes; } - if (!(man->persist.callback.flags & MCF_SERVER)) - { - man_bytecount_output_client(man, dco_read_bytes, dco_write_bytes); - } + man_bytecount_output_client(c->c2.dco_read_bytes + man->persist.bytes_in + c->c2.link_read_bytes, + c->c2.dco_write_bytes + man->persist.bytes_out + c->c2.link_write_bytes); } } -/* DCO resets stats on reconnect. Since client expects stats - * to be preserved across reconnects, we need to save DCO +void +management_check_bytecount_server(struct multi_context *multi) +{ + struct timeval null; + CLEAR(null); + + if ((management->persist.callback.flags & MCF_SERVER) && event_timeout_trigger(&management->connection.bytecount_update_interval, &null, ETT_DEFAULT)) + { + /* fetch counters from dco */ + if (dco_enabled(&multi->top.options)) + { + if (dco_get_peer_stats_multi(&multi->top.c1.tuntap->dco, true) < 0) + { + return; + } + } + + /* iterate over peers and report counters for each connected peer */ + struct hash_iterator hi; + struct hash_element *he; + hash_iterator_init(multi->hash, &hi); + while ((he = hash_iterator_next(&hi))) + { + struct multi_instance *mi = (struct multi_instance *)he->value; + struct context_2 *c2 = &mi->context.c2; + + if ((c2->mda_context.flags & (DAF_CONNECTION_ESTABLISHED | DAF_CONNECTION_CLOSED)) == DAF_CONNECTION_ESTABLISHED) + { + man_bytecount_output_server(c2->dco_read_bytes + c2->link_read_bytes, c2->dco_write_bytes + c2->link_write_bytes, &c2->mda_context); + } + } + hash_iterator_free(&hi); + } +} + +/* context_2 stats are reset on reconnect. Since client expects stats + * to be preserved across reconnects, we need to save context_2 * stats before tearing the tunnel down. */ void man_persist_client_stats(struct management *man, struct context *c) { /* no need to raise SIGUSR1 since we are already closing the instance */ - if (dco_enabled(&c->options) && (dco_get_peer_stats(c, false) == 0)) + if (!(man->persist.callback.flags & MCF_SERVER)) { - management_bytes_client(man, c->c2.dco_read_bytes, c->c2.dco_write_bytes); + man->persist.bytes_in += (c->c2.link_read_bytes + c->c2.dco_read_bytes); + man->persist.bytes_out += (c->c2.link_write_bytes + c->c2.dco_write_bytes); } } diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h index 083caf5..bbf0630 100644 --- a/src/openvpn/manage.h +++ b/src/openvpn/manage.h @@ -70,8 +70,6 @@ unsigned int flags; unsigned int mda_key_id_counter; - - time_t bytecount_last_update; }; /* @@ -492,34 +490,9 @@ * These functions drive the bytecount in/out counters. */ -void management_check_bytecount(struct context *c, struct management *man, struct timeval *timeval); +void management_check_bytecount_client(struct context *c, struct management *man, struct timeval *timeval); -static inline void -management_bytes_client(struct management *man, const int size_in, const int size_out) -{ - if (!(man->persist.callback.flags & MCF_SERVER)) - { - man->persist.bytes_in += size_in; - man->persist.bytes_out += size_out; - } -} - -void man_bytecount_output_server(const counter_type *bytes_in_total, - const counter_type *bytes_out_total, - struct man_def_auth_context *mdac); - -static inline void -management_bytes_server(struct management *man, const counter_type *bytes_in_total, - const counter_type *bytes_out_total, struct man_def_auth_context *mdac) -{ - if (man->connection.bytecount_update_seconds > 0 - && now >= mdac->bytecount_last_update + man->connection.bytecount_update_seconds - && (mdac->flags & (DAF_CONNECTION_ESTABLISHED | DAF_CONNECTION_CLOSED)) - == DAF_CONNECTION_ESTABLISHED) - { - man_bytecount_output_server(bytes_in_total, bytes_out_total, mdac); - } -} +void management_check_bytecount_server(struct multi_context *multi); void man_persist_client_stats(struct management *man, struct context *c); diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index e1ce32a..f1abdbe 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3812,6 +3812,13 @@ { check_stale_routes(m); } + +#ifdef ENABLE_MANAGEMENT + if (management) + { + management_check_bytecount_server(m); + } +#endif /* ENABLE_MANAGEMENT */ } static void