Re: [PATCH 06/45] provider: Handle VPN configuration and association states

Hi Denis,

On 8/1/25 22:34, Denis Kenzior wrote:
> Hi Jussi,
> 
> On 7/11/25 9:26 AM, Jussi Laakkonen wrote:
>> Set the association state when VPN is waiting for user input as an
>> initial state after connecting the provider. Set the configuration
>> state (as it is declaced to be the string to connect state in VPN)
>> accordingly as well. Start VPN connect timeout in configuration
>> state with restart option to ensure that the timeout begins from the
>> last known configuration (connect) state.
>> ---
>>   include/provider.h |  9 +++++----
>>   src/provider.c     | 22 +++++++++++++++++++++-
>>   2 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/provider.h b/include/provider.h
>> index 3f2e36ad..aac47527 100644
>> --- a/include/provider.h
>> +++ b/include/provider.h
>> @@ -44,10 +44,11 @@ enum connman_provider_type {
>>   enum connman_provider_state {
>>       CONNMAN_PROVIDER_STATE_UNKNOWN       = 0,
>>       CONNMAN_PROVIDER_STATE_IDLE          = 1,
>> -    CONNMAN_PROVIDER_STATE_CONNECT       = 2,
>> -    CONNMAN_PROVIDER_STATE_READY         = 3,
>> -    CONNMAN_PROVIDER_STATE_DISCONNECT    = 4,
>> -    CONNMAN_PROVIDER_STATE_FAILURE       = 5,
>> +    CONNMAN_PROVIDER_STATE_ASSOCIATION   = 2,
>> +    CONNMAN_PROVIDER_STATE_CONNECT       = 3,
>> +    CONNMAN_PROVIDER_STATE_READY         = 4,
>> +    CONNMAN_PROVIDER_STATE_DISCONNECT    = 5,
>> +    CONNMAN_PROVIDER_STATE_FAILURE       = 6,
>>   };
>>   enum connman_provider_error {
>> diff --git a/src/provider.c b/src/provider.c
>> index 1f0ce10d..ab4aeafb 100644
>> --- a/src/provider.c
>> +++ b/src/provider.c
>> @@ -126,6 +126,22 @@ static int provider_indicate_state(struct 
>> connman_provider *provider,
>>   {
>>       DBG("state %d", state);
>> +    switch (state) {
>> +    case CONNMAN_SERVICE_STATE_UNKNOWN:
>> +    case CONNMAN_SERVICE_STATE_IDLE:
>> +    case CONNMAN_SERVICE_STATE_ASSOCIATION:
>> +        break;
>> +    case CONNMAN_SERVICE_STATE_CONFIGURATION:
>> +        __connman_service_start_connect_timeout(provider->vpn_service,
>> +                                true);
> 
> Possibly handle this inside service.c?
> 

Could be. However, I kept this here mainly because the VPN related 
operations can be extended based on the actual service state. Like, we 
have an IPv6 disable option that was not yet accepted by upstream, which 
simply disables IPv6 from the transport and all connected when VPN setup 
requests so. It was discussed whether undef routes could be used but 
after trying  this approach, also with desktop, it seems that 
applications work really weirdly with it. It is not related to this but 
makes it more convenient to make certain VPN related actions within 
provider.c and not to clutter service.c with more VPN specifics, right?

>> +        break;
>> +    case CONNMAN_SERVICE_STATE_READY:
>> +    case CONNMAN_SERVICE_STATE_ONLINE:
>> +    case CONNMAN_SERVICE_STATE_DISCONNECT:
>> +    case CONNMAN_SERVICE_STATE_FAILURE:
>> +        break;
>> +    }
>> +
>>       __connman_service_ipconfig_indicate_state(provider->vpn_service, 
>> state,
>>                       CONNMAN_IPCONFIG_TYPE_IPV4);
> 
> Regards,
> -Denis

BR,
  Jussi