connman.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@metanate.com>
To: Daniel Wagner <wagi@monom.org>,
	"VAUTRIN Emmanuel (Canal Plus Prestataire)"
	<Emmanuel.VAUTRIN@cpexterne.org>
Cc: connman@lists.linux.dev
Subject: Re: [PATCH] service: Complete only after user connection retries
Date: Tue, 15 Jun 2021 12:09:30 +0100	[thread overview]
Message-ID: <20210615120930.2970caef.john@metanate.com> (raw)
In-Reply-To: <20210327124254.iorhou27y3u4pwih@beryllium.lan>

Hi,

On Sat, 27 Mar 2021 13:42:54 +0100
Daniel Wagner <wagi@monom.org> wrote:
> On Thu, Mar 11, 2021 at 01:04:11PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> > When a user connection fails, let the report_error_cb manage
> > exclusively service update, after all retries.  
> 
> Patch applied.

This patch seems to have totally broken connection attempts where the
wrong passphrase is entered:

        connmanctl> connect wifi_b0f1ecffb46f_7073475462456b44_managed_psk
        Agent RequestInput wifi_b0f1ecffb46f_7073475462456b44_managed_psk
          Passphrase = [ Type=psk, Requirement=mandatory ]
        Passphrase? wrong-passphrase
        Agent ReportError wifi_b0f1ecffb46f_7073475462456b44_managed_psk
          invalid-key
        Agent request cancelled by ConnMan
        connmanctl> connect wifi_b0f1ecffb46f_7073475462456b44_managed_psk
        Error /net/connman/service/wifi_b0f1ecffb46f_7073475462456b44_managed_psk: In progress

This happens because service->pending is never cleared because
service_cleared() is bypassed in this hunk:

@@ -6064,6 +6065,7 @@ static int service_indicate_state(struct connman_service *service)
                                                report_error_cb,
                                                get_dbus_sender(service),
                                                NULL);
+                       goto notifier;
                }
                service_complete(service);
                break;

It looks like something is missing because reporting the error only
after all retries makes sense, but after this patch the original connect
request doesn't get a response at all and any future attempt to connect
fails.

I'm not familiar enough with the code flow to suggest a follow-up
change, but does the above explanation make sense?  Is there an easy way
to ensure that the pending state is cleared and we reply to the original
message?


Thanks,
John

       reply	other threads:[~2021-06-15 11:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <PR1PR02MB4794EB6ADA4A23AA374EE58493909@PR1PR02MB4794.eurprd02.prod.outlook.com>
     [not found] ` <20210327124254.iorhou27y3u4pwih@beryllium.lan>
2021-06-15 11:09   ` John Keeping [this message]
2021-06-28  7:58     ` Daniel Wagner
2021-06-28 15:04       ` John Keeping
2021-06-28 19:54         ` Daniel Wagner
2021-06-29  7:35           ` Daniel Wagner

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=20210615120930.2970caef.john@metanate.com \
    --to=john@metanate.com \
    --cc=Emmanuel.VAUTRIN@cpexterne.org \
    --cc=connman@lists.linux.dev \
    --cc=wagi@monom.org \
    --subject='Re: [PATCH] service: Complete only after user connection retries' \
    /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
on how to clone and mirror all data and code used for this inbox