From: Grant Erickson <gerickson@nuovations.com>
To: connman@lists.linux.dev
Subject: Re: When 'EnableOnlineCheck' Is Asserted Two Recurring IPv4 and One IPv6 Reachability Checks Get Scheduled
Date: Tue, 14 Nov 2023 07:45:03 -0800 [thread overview]
Message-ID: <A1E9F9EF-0B21-470D-AE8C-BBF1548ABB53@nuovations.com> (raw)
In-Reply-To: <3497BF07-2F1A-4D75-A955-45BC604F022F@nuovations.com>
On Nov 13, 2023, at 3:16 PM, Grant Erickson <gerickson@nuovations.com> wrote:
> On Nov 13, 2023, at 10:38 AM, Grant Erickson <gerickson@nuovations.com> wrote:
>> The latest issue I am jumping into in top-of-tree now that the two heap-use-after-free issues have been
>> addressed is "When 'EnableOnlineCheck' Is Asserted Two (2) Recurring IPv4 and One (1) IPv6
>> Reachability Checks Get Scheduled” rather than one of each. Feel free to track progress here:
>>
>> https://github.com/Nuovations/connman/issues/74
>
> As each of the above probes come back around to ‘complete_online_check' and then, eventually, ‘reschedule_online_check', from a design perspective, we are left with three opportunities to correct what is likely regarded as misbehavior here:
>
> 1. Prevent two parallel outstanding IPv4 probes from ever being rescheduled. However, this only handles the ‘EnableOnlineToReadyTransition' is asserted case.
> * Given this impartial solve, this seems like like a non-starter.
> 2. Prevent the second outstanding IPv4 probe from being issued.
> a. This would cover BOTH ‘EnableOnlineToReadyTransition' being asserted AND not being asserted.
> b. Infrastructure-wise, this would require more state in the service module to track outstanding probes or different treatment the WISPr module in '__connman_wispr_start' wherein an existing, in flight context is displaced.
> * ‘-EALREADY' could be returned in such a case.
> 3. Prevent the first outstanding IPv4 probe from completing once the second outstanding IPv4 probe is issued:
> a. This would also cover both EnableOnlineToReadyTransition being asserted and not being asserted.
> b. Again, in the WISPr module in ‘__connman_wispr_start', when the existing ‘wp_context' context is displaced ‘g_web_unref' would need to be called to destroy the GWeb instance, closing the in flight connection which would ensure that the first probe never completes, either successfully or unsuccessfully (though you could argue the request should still be closed but perhaps expanded to include system status, such as -ECANCELED, such that the WISPr closure callback could elect to ignore the failure).
>
> One could imagine a currently-absent API to help with some of these use cases: '__connman_wispr_cancel(service, type)’.
I ended up prototyping approach (3) above, including implementing the as-yet-missing '__connman_wispr_cancel(service, type)’, and it appears to be working well with very tactical changes, per the raw and annotated log traces at:
https://github.com/Nuovations/connman/issues/74#issuecomment-1810434718
Patches forthcoming.
Best,
Grant
--
Principal
Nuovations
gerickson@nuovations.com
http://www.nuovations.com/
prev parent reply other threads:[~2023-11-14 15:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-13 18:38 When 'EnableOnlineCheck' Is Asserted Two Recurring IPv4 and One IPv6 Reachability Checks Get Scheduled Grant Erickson
2023-11-13 23:16 ` Grant Erickson
2023-11-14 15:45 ` Grant Erickson [this message]
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=A1E9F9EF-0B21-470D-AE8C-BBF1548ABB53@nuovations.com \
--to=gerickson@nuovations.com \
--cc=connman@lists.linux.dev \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).