Re: [PATCH 00/11] Add association state for VPNs

Hi all!

On 2/14/25 08:30, Christian Hewitt wrote:
>> On 24 Jan 2025, at 10:58 pm, Jussi Laakkonen <[email protected]> wrote:
>>
>> This patch set adds the association state also for the VPNs. This state is to
>> indicate that the VPN is waiting for VPN agent to provide input given by user.
>> In this state service.c must not do connect timeout checks as the timers for
>> both differ in length, default being 120s for connect timeout and 300s for VPN
>> agent dialog timeout.
>>
>> In order to facilitate this change the association state had to be implemented
>> also for VPNs. It is common state for services and like with services the
>> association state for VPNs preceeds the configuration state (on VPN side
>> connect state). Both vpn.c plugins on connmand and vpnd side require changes
>> to accommodate this state. When the VPN agent succeeds in getting the input
>> from the user the state transitions from association to connect (configuration)
>> state and, thus, requires no specific changes to VPN plugins.
>>
>> On connmand side the association state is the initial state when VPN is getting
>> connected and the state needs to be accounted as a connecting state in
>> plugins/vpn.c to not to lose transport ident for it and in provider.c as a
>> pre-configuration state to not to start the connect timeout for the VPN before
>> the VPN is in configuration state. The reason for the latter is that the
>> connect timeout should be exact and start from the point when
>> connect/configuration state is entered.
>>
>> On vpnd side association state is, like on connmand side, the initial state for
>> the VPN getting connected. After the VPN agent succeeds getting the information
>> from the user (credentials) the state transitions to connect (configuratioin).
>> There may be a possibility for a VPN plugin to run without VPN agent and thus
>> in these cases it is ensured that the vpn/plugins/vpn.c:vpn_notify() does
>> the state transition in such cases. It is allowed go back to association state
>> from connect state but not from other states.
>>
>> Jussi Laakkonen (11):
>>   agent: Cancel agent request on NoReply D-Bus error
>>   vpn-provider: Use association state for VPN agent input wait
>>   vpn: Add association state before connect state
>>   vpn-agent: Do connect state transition after input dialog check
>>   service: Explicit VPN connect timeout, ignore in VPN agent wait
>>   provider: Handle VPN configuration and association states
>>   vpn: Add support for association state, add state getter
>>   vpn: Check if connecting when setting state or disconnecting
>>   vpn: Add VPN agent use callback for plugins
>>   vpn-provider: Transition to CONNECT state with agentless VPNs
>>   doc: Update VPN documentation for association state
>>
>> doc/vpn-connection-api.txt |  4 +--
>> doc/vpn-overview.txt       |  7 ++++-
>> include/provider.h         |  9 +++---
>> plugins/vpn.c              | 23 ++++++++++++---
>> src/agent.c                |  4 ++-
>> src/connman.h              |  2 ++
>> src/provider.c             | 22 ++++++++++++++-
>> src/service.c              | 52 ++++++++++++++++++++++++++++++----
>> vpn/plugins/vpn.c          | 44 ++++++++++++++++++++++++++++-
>> vpn/plugins/vpn.h          | 12 ++++----
>> vpn/vpn-agent.c            |  6 +++-
>> vpn/vpn-provider.c         | 58 ++++++++++++++++++++++++++++++++++----
>> vpn/vpn-provider.h         |  7 +++++
>> 13 files changed, 220 insertions(+), 30 deletions(-)
> 
> I’ve been using this series combined with the following series and patch for several weeks:
> 
> https://patchwork.kernel.org/project/connman/list/?series=928220
> https://patchwork.kernel.org/project/connman/patch/DB6PR10MB1845EBB07DB785B580B3F6ECE1E02@DB6PR10MB1845.EURPRD10.PROD.OUTLOOK.COM/
> 
> Plus a revert of this commit which I’ve previously flagged as breaking the default “route all traffic down tunnel” with WireGuard (the main use-case for LibreELEC users):
> 
> https://git.kernel.org/pub/scm/network/connman/connman.git/commit/?id=9eb1772d31b6fcf78e8711976696491aec9ff5df
> 
> For easier testing all patches are in this branch: https://github.com/chewitt/connman/commits/wireguard
> 
> No issues observed, so:
> 
> Tested-by: Christian Hewitt <[email protected]>
> 

Denis: The testing of WireGuard changes was done with these changes as 
well, should I resend these patches? I'll try to find time 
today/tomorrow to check if they still apply, and if not, I'll send v2.

- Jussi