connman.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Daniel Wagner <wagi@monom.org>
To: Jussi Laakkonen <jussi.laakkonen@jolla.com>
Cc: connman@lists.linux.dev
Subject: Re: [PATCH 2/5] vpn-provider: Ignore error adding when state is idle/unknown
Date: Mon, 13 Sep 2021 08:32:31 +0200	[thread overview]
Message-ID: <20210913063231.la2wfnnm5jh65hrd@beryllium.lan> (raw)
In-Reply-To: <20210902151124.4983-3-jussi.laakkonen@jolla.com>

On Thu, Sep 02, 2021 at 06:11:21PM +0300, Jussi Laakkonen wrote:
> Do not allow to add errors for provider that is already set into idle
> state or is in unknown state. This case may happen when networks are
> rapidly changed and VPN did not call the callback connect_cb() until the
> VPN is died and vpn.c:vpn_died() initiates cleanup in the VPN which
> eventually calls connect_cb() with an error.

I wonder if we should set the VPN state immediately when starting the
connection process to some value != unknown.

> ---
>  vpn/vpn-provider.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
> index f30ce04f..90e21e95 100644
> --- a/vpn/vpn-provider.c
> +++ b/vpn/vpn-provider.c
> @@ -2026,6 +2026,21 @@ int vpn_provider_indicate_error(struct vpn_provider *provider,
>  	DBG("provider %p id %s error %d", provider, provider->identifier,
>  									error);
>  
> +	/*
> +	 * Ignore adding of errors when the VPN is idle or not set. Calls may
> +	 * happen in a case when networks are rapidly changed and the call to
> +	 * vpn_died() is done before executing the connect_cb() from the
> +	 * plugin. Then vpn.c:vpn_died() executes the plugin specific died()
> +	 * function which may free the plugin private data, containing also
> +	 * the callback which hasn't yet been called. As a result the provider
> +	 * might already been reset to idle state when the callback is executed
> +	 * resulting in unnecessary reset of the previous successful connect
> +	 * timer and adding of an error for already disconnected VPN.
> +	 */
> +	if (provider->state == VPN_PROVIDER_STATE_IDLE ||
> +				provider->state == VPN_PROVIDER_STATE_UNKNOWN)
> +		return 0;

Checking the unknown state looks here a bit strange. The usual meaning
of unknown is it hasn't been initialized. But then this is exactly
what's happening here I assume.

  reply	other threads:[~2021-09-13  6:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 15:11 [PATCH 0/5] Add heurestic and customizable value for VPN auth errors Jussi Laakkonen
2021-09-02 15:11 ` [PATCH 1/5] vpn: Report EALREADY back to caller if VPN is already disconnecting Jussi Laakkonen
2021-09-02 15:11 ` [PATCH 2/5] vpn-provider: Ignore error adding when state is idle/unknown Jussi Laakkonen
2021-09-13  6:32   ` Daniel Wagner [this message]
2021-09-13  9:05     ` Jussi Laakkonen
2021-09-02 15:11 ` [PATCH 3/5] vpn-provider: Add auth error check heuristic to avoid losing creds Jussi Laakkonen
2021-09-02 15:11 ` [PATCH 4/5] openvpn: Default to 10 AuthErrorLimit unless set by user Jussi Laakkonen
2021-09-02 15:11 ` [PATCH 5/5] doc: Document AuthErrorLimit in VPN connection API Jussi Laakkonen
2021-09-13  6:43 ` [PATCH 0/5] Add heurestic and customizable value for VPN auth errors Daniel Wagner
2021-09-13  9:36   ` Jussi Laakkonen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210913063231.la2wfnnm5jh65hrd@beryllium.lan \
    --to=wagi@monom.org \
    --cc=connman@lists.linux.dev \
    --cc=jussi.laakkonen@jolla.com \
    --subject='Re: [PATCH 2/5] vpn-provider: Ignore error adding when state is idle/unknown' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).