From patchwork Tue Sep 2 10:36:01 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4387 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:2a1c:b0:671:5a2c:6455 with SMTP id k28csp1851809maz; Tue, 2 Sep 2025 07:35:58 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCUzXx24QadCRRNA81TgSexArdJa2ILINe22Oyrr02UgVcEBlugUjpjBkDuXWGg+vlbt3IjEThpzRkY=@openvpn.net X-Google-Smtp-Source: AGHT+IGagrVrNJmHKtJ4M51NffmUEiYjuwt6PjMV1kmHFxBIYkbcP67eC7KWZr1Y67aL1RUlb+m8 X-Received: by 2002:a05:6870:6120:b0:315:7809:8999 with SMTP id 586e51a60fabf-319630d200bmr5873483fac.16.1756823758145; Tue, 02 Sep 2025 07:35:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1756823758; cv=none; d=google.com; s=arc-20240605; b=Cuhvd0MM2EcOG2kapWHE6dn9bPAWtq4TVnhNU5I1q+hP9O8X6s5msAC943gg5WKqw6 2HrWRm6falHY+FhalATPBBkRA1FXZi9/Z6Rqz73DoPVYvatK6lHKr81++j3qNxlEmKOB SPX1zMORc0QlzlRFyMbKuuTJOomDmHsPTGi65znfyYG2s0XhgTd2vhq7FLnWmyNoxBE1 ianTl4jTywwFz0grv0GXky6UP63guhlI9dhAf977AW94rZH1Aym4F+0ODxo6oFqRFsM7 MKAVU8F0OOb8tmfJ4z0PDyJne7MNmt+mQdjheduaBd0TxuLmqFdQWxr4SQtR5rDVrJzi rvOw== 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=TWnl8P4TLL5tVX3+nkhwEIiBINH33IpCtPSUjTN070c=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=Mk93pJHxUDaXJP/ugwiJkOjmxXzN7Bxfhj1LBysGzfW1i6fYLnhVRKyKVz8vdtnAbB uA0aC4wGqXvniHDTdzbYMgF4W71CID+1cl6i30rsYj9vMzB/sfPv1HRgNo0QDyMbD3jH FyFKUBifPPPtoKl2N9plcqImwL98+d/k5GBNkEtNMU6Qc5P3VvoSXIhBia19rmUT+kDm ykTMChSZbWJ9PIPQFTNqsBhdkQVlpZFk2/hz1QMC+uEE2QtiW/IGA9RonBF4+iUBRMcv jwCZp+bpo/N/dx8DE35wNGqc1tj3NP6oWrsu4/NNmBga4YfQHmCwCD7MGn34FMjXjFKR LgVw==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=fC7uWQa5; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=c0mMeyJ2; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=QxDUaehT; 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 586e51a60fabf-319c725db5bsi156541fac.391.2025.09.02.07.35.57 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Sep 2025 07:35:57 -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=fC7uWQa5; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=c0mMeyJ2; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=QxDUaehT; 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=TWnl8P4TLL5tVX3+nkhwEIiBINH33IpCtPSUjTN070c=; b=fC7uWQa5eaUv167e1jrjIQKpcM LmA+esfg4n5VihZMeZx8w+2FbBcO/RQn7/XjrBpnoFaFXGJGBKeqvKOACb55k5QPblxBF0xk9IkC1 tDNZEjXScNM2Wfoy8xBMuUKacMlK4E/Md5/nULkauIatfN2xKpjLhDou6t5P7coXe0wA=; Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1utS70-00072F-Pe; Tue, 02 Sep 2025 14:35:54 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1utS6y-000718-9d for openvpn-devel@lists.sourceforge.net; Tue, 02 Sep 2025 14:35:52 +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=2otYSdS8TL0gyvUSGrKfhAwmwWfTWBZ6Jja8nGLMlxs=; b=c0mMeyJ2LxmBBgLAW+OYjGSjST NTRtOsILJMd1E8v1elXb1kApHpKKJmOtYq1nKkEx3J0q32JaLgpza6ScpRnkTdjL6mSvpxC4VPhqm vJg88+eBqFgZFqw5+nlEWKsGIKjYItvpkjvAUgqvYw6XVqNp9nkQIEDUYFrdGy9li6AI=; 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=2otYSdS8TL0gyvUSGrKfhAwmwWfTWBZ6Jja8nGLMlxs=; b=QxDUaehTJynusmnmD0DKGgeriC al0WQ26PfdyMkSPzyU1XqzZZpUpwg5RnBNvbpjpE8sr5QMY29XNLYpV6lQ7dWDG6d96EPyzZ6i4vj Ve2zECCzl/bWiPo8LX3WTUSpEKmlOi9Zl/KvvZVNqOD06hpY+qUIhCIABBKyfNd6IzM8=; 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 1utS6x-00085W-5C for openvpn-devel@lists.sourceforge.net; Tue, 02 Sep 2025 14:35:52 +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 582Aa7FJ022195 for ; Tue, 2 Sep 2025 12:36:07 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.1/8.18.1/Submit) id 582Aa7mV022194 for openvpn-devel@lists.sourceforge.net; Tue, 2 Sep 2025 12:36:07 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Tue, 2 Sep 2025 12:36:01 +0200 Message-ID: <20250902103606.22181-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-2.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: 1utS6x-00085W-5C Subject: [Openvpn-devel] [PATCH v3] 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?1842163228974361414?= 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: Gert Doering --- 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 3 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering 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..5311701 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,82 @@ } 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 (man->persist.callback.flags & MCF_SERVER) + { + return; + } + if (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) +{ + if (!(management->persist.callback.flags & MCF_SERVER)) + { + return; + } + + struct timeval null; + CLEAR(null); + if (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) { + man->persist.bytes_in += c->c2.link_read_bytes; + man->persist.bytes_out += c->c2.link_write_bytes; + /* no need to raise SIGUSR1 since we are already closing the instance */ if (dco_enabled(&c->options) && (dco_get_peer_stats(c, false) == 0)) { - management_bytes_client(man, c->c2.dco_read_bytes, c->c2.dco_write_bytes); + man->persist.bytes_in += c->c2.dco_read_bytes; + man->persist.bytes_out += 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