Skip to content

dco: backport immediate message processing to 2.6#945

Closed
alloc33 wants to merge 1 commit intoOpenVPN:release/2.6from
alloc33:release/2.6
Closed

dco: backport immediate message processing to 2.6#945
alloc33 wants to merge 1 commit intoOpenVPN:release/2.6from
alloc33:release/2.6

Conversation

@alloc33
Copy link
Copy Markdown

@alloc33 alloc33 commented Dec 26, 2025

Backport of commit 7791f53 from master.

Currently, reading and processing of incoming DCO messages are decoupled: notifications are read, parsed, and the relevant information is stored in fields of dco_context_t for later processing. This approach is problematic on Linux, since libnl does not allow reading a single netlink message at a time, which can result in loss of information when multiple notifications are available.

This change adopts a read -> parse -> process paradigm. On Linux, processing is now invoked directly from within the parsing callback.

On Linux, a DEL_PEER notification from the kernel triggers a GET_PEER request from userspace, which can lead to errors when multiple simultaneous DEL_PEER notifications are received. To avoid this, introduce a lock that prevents requesting stats while still busy parsing other messages.

Note: The 2.6 backport requires additional changes not present in the master commit because the multi context linkage infrastructure differs:

  • Added multi pointer to struct context (openvpn.h)
  • Set context linkages in init.c, mtcp.c, mudp.c
  • Added dco_linux.h changes for context pointer

Reported-by: Stefan Baranoff stefan.baranoff@trinitycyber.com
Github: #900
Github: #918
Github: #931
Github: fixes #919
(backport of commit 7791f53)

Thank you for your contribution

You are welcome to open PR, but they are used for discussion only. All
patches must eventually go to the openvpn-devel mailing list for review:

Please send your patch using git-send-email. For example to send your latest commit to the list:

$ git send-email --to=openvpn-devel@lists.sourceforge.net HEAD~1

For details, see these Wiki articles:

Backport of commit 7791f53 from master.

Currently, reading and processing of incoming DCO messages are
decoupled: notifications are read, parsed, and the relevant information
is stored in fields of dco_context_t for later processing. This approach
is problematic on Linux, since libnl does not allow reading a single
netlink message at a time, which can result in loss of information when
multiple notifications are available.

This change adopts a read -> parse -> process paradigm. On Linux,
processing is now invoked directly from within the parsing callback.

On Linux, a DEL_PEER notification from the kernel triggers a GET_PEER
request from userspace, which can lead to errors when multiple
simultaneous DEL_PEER notifications are received. To avoid this,
introduce a lock that prevents requesting stats while still busy
parsing other messages.

Note: The 2.6 backport requires additional changes not present in the
master commit because the multi context linkage infrastructure differs:
- Added multi pointer to struct context (openvpn.h)
- Set context linkages in init.c, mtcp.c, mudp.c
- Added dco_linux.h changes for context pointer

Reported-by: Stefan Baranoff <stefan.baranoff@trinitycyber.com>
Github: OpenVPN#900
Github: OpenVPN#918
Github: OpenVPN#931
Github: fixes OpenVPN#919
(backport of commit 7791f53)

Signed-off-by: Nikolai Shelekhov <nickshv13@icloud.com>
@cron2
Copy link
Copy Markdown
Contributor

cron2 commented Apr 1, 2026

Thanks for the PR. It got lost in the preparations for 2.7.0 and 2.7.1 release.

We discussed this today, and try to get it done "soon", so it can be released as 2.6.20.

@ralflici
Copy link
Copy Markdown
Contributor

ralflici commented Apr 7, 2026

This looks good to me. I tested it with 50 clients all connecting and disconnecting simultaneously, and it showed no issues. The only suggestion I have is to add a debug print (like we have in v2.7) when the lock is taken and we cannot request stats, so the logs are easier to interpret.

@cron2
Copy link
Copy Markdown
Contributor

cron2 commented Apr 19, 2026

So, I have taken the patch from here, tried to apply it to release/2.6 "as of 246bc0e", and it fails with

forward.c:1277:30: error: no member named 'c' in 'struct dco_context'
 1277 |     struct context *c = dco->c;
      |                         ~~~  ^
1 error generated.

@ralflici can you help me figure out which commits I need to cherrypick to 2.6? What did you have in your tree when you tested this?

@ralflici
Copy link
Copy Markdown
Contributor

I actually used the PR tree for testing and did not try applying the patches on top of v2.6. Technically, the PR includes the needed change in the DCO context, but only for Linux, while that part in forward.c is shared with FreeBSD as well.

From master, I can see that the commits adding the c reference in dco_context_t are d1f2afc for FreeBSD and 6d45008 for Windows, but they include much more than just that change.

Looking more carefully, this line might also be incorrect, because it accesses a field that is defined only for Linux and is outside any #ifdef guard (in master we handle this in ovpn_dco_init). Similarly, this asserts on the same c field, which is not defined on FreeBSD.

So I think this PR should also be tested on the other platforms, and on a build with DCO disabled.

cron2 pushed a commit that referenced this pull request Apr 21, 2026
…791f53

Change ovpn_dco_init() to take a reference to struct context, add a
backlink from the platform DCO contexts to the owning openvpn context,
and store the top multi context in struct context for server mode.

This prepares the tree for the follow-up backend changes from commit
7791f53 ("dco: process messages immediately after read") where Linux and
FreeBSD process DCO notifications directly from their backend read
paths.

It is part of a reworked backport of PR #945 originally proposed by
Nikolai Shelekhov <nickshv13@icloud.com>.

The original commits in master are

    commit a699681
    Author: Antonio Quartulli <antonio@mandelbit.com>
    Date:   Wed Jul 23 15:39:11 2025 +0200

	dco: only pass struct context to init function

    commit 7f5a6de
    Author: Antonio Quartulli <antonio@mandelbit.com>
    Date:   Wed Jul 23 08:10:25 2025 +0200

	multi: store multi_context address inside top instance

Change-Id: I974e10ec91a0b63f52387f1406ce1b49145eb0be
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1631
Message-Id: <20260420165923.14226-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36691.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
cron2 pushed a commit that referenced this pull request Apr 21, 2026
Backport the immediate DCO message processing model from commit 7791f53
("dco: process messages immediately after read").

Change the core DCO API from dco_do_read() to dco_read_and_process(),
and make the backend read paths own the parsing and immediate handling
of kernel notifications before handing the resulting state to
process_incoming_dco() or multi_process_incoming_dco().

On Linux, install a permanent netlink callback and process GET_PEER and
DEL_PEER messages as they are received, instead of switching callbacks
for each read and deferring handling through shared DCO message state.
Also add a small guard to avoid requesting peer stats while libnl is
still parsing a batch of notifications.

On FreeBSD, move notification handling into dco_read_and_process() and
update peer statistics directly from the backend instead of storing
temporary byte counters in dco_context_t.

This commit is part of a reworked backport of PR #945 originally
proposed by Nikolai Shelekhov <nickshv13@icloud.com>.

Github: #900
Github: #918
Github: #931
Github: fixes #919
Github: closes #945

Change-Id: I92c5f74a27b40fede204f714b042a6cc80b3703e
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1632
Message-Id: <20260421084906.5720-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36707.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
@cron2
Copy link
Copy Markdown
Contributor

cron2 commented Apr 22, 2026

So, @ralflici split the work into two - infrastructure ("introduce dco->c") and actual event handling. Then we added a 3rd commit to fix the bugs :-) -

commit adece45
Author: Ralf Lici ralf@mandelbit.com
Date: Wed Apr 22 07:56:30 2026 +0200

dco-linux: enforce ifindex only for DEL_PEER notifications

commit e78a8af
Author: Ralf Lici ralf@mandelbit.com
Date: Tue Apr 21 10:49:01 2026 +0200

dco: backport immediate notification processing on Linux and FreeBSD
...
It is part of a reworked backport of PR #945 originally proposed by
Nikolai Shelekhov <nickshv13@icloud.com>.

commit 876a8cf
Author: Ralf Lici ralf@mandelbit.com
Date: Mon Apr 20 18:59:17 2026 +0200

dco: port core/context infrastructure needed for backport of commit 7791f53

... so we should have properly-functioning DCO event handling for Linux and FreeBSD in the release/2.6 tree now, without breaking compilation on FreeBSD or Windows.

Thanks, @alloc33 for your help in actually making it here ;-) - I will close this PR now (as there is no more code to be merged) and leave open the issues until we have confirmation that things work correctly now.

2.6.20 with the fixes is supposed to arrive in a few hours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants