Management: add `reload-push-options` & Fix `PUSH_UPDATE` logic
Overview
This PR introduces a new management command reload-push-options to allow dynamic updating of client configurations (routes, DNS, etc.) without restarting the server or disconnecting clients. It also includes a critical fix for PUSH_UPDATE handling when messages are split across multiple chunks (continuation messages).
1. Feature: reload-push-options
Administrators can now trigger a reload of push options from the server configuration file via the management interface.
Usage
- Without arguments: Reloads
pushoptions from the config file. New clients connecting after this will receive the new options. Existing clients are unaffected. - With
update-clients: Reloads options AND synchronizes them to currently connected clients.
Update Logic
When update-clients is used, the server:
- Parses the new configuration.
- Calculates the difference between the old and new push lists.
- Sends removal commands (e.g.,
-route) for option types that have changed or been removed. - Sends the new options via
PUSH_UPDATE.
This allows for seamless updates of routing tables and network settings on live VPNs.
2. Bug Fix: PUSH_UPDATE Continuation Logic
The Issue
Previously, the state tracking for which option types had been "reset" (e.g., OPT_P_U_ROUTE) was stored in a local variable update_options_found within apply_push_options().
If a PUSH_UPDATE message was large enough to be split into multiple fragments (using push-continuation), this state was lost between fragments.
- Result: The route table (or other list options) would be reset for every fragment that contained a route, rather than once for the entire update sequence.
- Symptom: Large route lists would fail to apply correctly, often resulting in only the last chunk's routes being present, or significant churn/flickering of routes during the update.
The Fix
The state variable has been moved to struct options as push_update_options_found. This ensures the "reset state" persists across the entire PUSH_UPDATE sequence and is only cleared once the full sequence is processed.
Testing
Integration Tests
Added a Docker-based integration test suite in tests/reload_push_options/.
- Coverage:
- Basic reload (no update-clients).
- Adding routes with update-clients.
- Removing routes.
- Replacing all routes.
- Stress Test: Verifies handling of 500+ routes (which triggers the continuation logic).
- How to run:
tests/reload_push_options/run.sh
Unit Tests
- Added
test_incoming_push_continuation_route_accumulationtotest_push_update_msg.cto verify that route lists accumulate correctly across multiple message chunks.
Thanks for working on this, and thanks for sending all the good stuff to the openvpn-devel list right away.
At this point in time, close to the 2.7 release, the big feature enhancement might be a bit too much for us to digest. The bugfix patch really should go in before 2.7.0 - @mrbff can you review?
Yes, I will review asap
It looks bigger than it is; most of it is just end-to-end tests. The code change itself is not that much....
Please have a closer look at the gc_arena stuff; I hope I got that right. I'm basically creating a new options.push_list and then switching the current with the new one.
regarding the management command:
reload-push-options [sync]
is the wording "sync" ok or should i change it to "broadcast" because there's "push-update-broad"?
broadcast does not sound really intuitive. Maybe reload-push-options update-clients instead?
Changed the naming, adjusted everything mentioned in the comments. Anything else? Any chance for 2.7.0?
The end result should be "one bugfix commit" (squashing all parts that are "bugfix" related), and "one feature commit" (new feature, with the 'sync' rename).
Bugfix can - and should - go into 2.7.0, "new feature" needs more review and (likely) more discussion.
ok @cron2, i'm opening a new PR here for bugfix only and send the diff to the mailing list
@maurice2k please do not open a new pr. You can update/force push to this PR to update it.
adjusted it here, merged the two patches (feature + bugfix) back into two commits. i made a new PR just for the bugfix (https://github.com/OpenVPN/openvpn/pull/925).
should i remove the bugfix commit from this PR? or remove the other PR?
Final patches have to be submitted to the mailing list. Once they are there, PRs on GitHub are not that useful anymore (they are good for a preliminary review, but nothing else) and can be closed.
So it doesn't matter how PRs are structured here, as long as they get the needed attention and patches are then submitted to the openvpn-devel mailing list.