From: Marcel Holtmann <marcel@holtmann.org>
To: Grant Erickson <gerickson@nuovations.com>
Cc: connman@lists.linux.dev
Subject: Re: [PATCH v2 00/17] Address Redundant IPv4 Reachability Checks
Date: Thu, 23 Nov 2023 12:19:34 +0100 [thread overview]
Message-ID: <4B140B42-25C2-4A6A-A772-166D075E4FD0@holtmann.org> (raw)
In-Reply-To: <20231119170714.775270-1-gerickson@nuovations.com>
Hi Grant,
> This addresses an issue in which, when 'EnableOnlineCheck' is
> asserted, two (2) concurrent IPv4 and one (1) IPv6 reachability checks
> get queued.
>
> The issue is addressed by introducing and leveraging
> '__connman_wispr_cancel' in 'cancel_online_check' to ensure that a
> pending or in-flight WISPr portal detection request or an "online"
> HTTP-based Internet reachability check is canceled not only in the
> service module but in the WISPr module as well.
>
> This prevents two concurrent, outstanding, and redundant IPv4 WISPr
> requests from being in-flight when EnableOnlineCheck is asserted.
>
> Prior to these changes, this redundancy was set in motion by:
>
> 1. The two back-to-back IPv4 probes get triggered by both of the
> following paths:
>
> a. Triggered from 'default_changed' and
> 'start_wispr_if_connected'. This is the newer path,
> chronologically, in the code base.
>
> b. Triggered from 'service_ip_bound', 'address_updated', and
> 'start_online_check'. This is the older path, chronologically,
> in the code base.
>
> 2. With the following commits addressed:
>
> a. 812e171 ("Close three '__connman_wisp_start' failure closure
> holes.")
>
> * With this fix alone, at minimum, the first probe above (1)
> would have resulted, eventually in a failure closure with
> 'complete_online_check'.
>
> b. 5f95851 ("wispr: Avoid 'connman_proxy_lookup' call for
> 'UNKNOWN' proxy method.") fixed, the first IPv4 probe from
> (1) no longer "falls down a 'hole'" and gets lost (closure-
> and accounting-wise).
>
> * With this fix, the first IPv4 probe is no longer a
> complete throwaway--it actually succeeds. However, since
> it does so, it becomes the first of the two redundant IPv4
> checks.
>
> By employing '__connman_wispr_cancel', 'cancel_online_check' quells
> the first IPv4 check triggered from 'default_changed' and
> 'start_wispr_if_connected' and replaces it, fully (in both the service
> and WISpr modules), with the second IPv4 check triggered from
> 'service_ip_bound', 'address_updated', and 'start_online_check'.
>
> Grant Erickson (17):
> wispr: Rename context proxy-related data members.
> wispr: Add and leverage 'is_wispr_supported'.
> wispr: Add documentation to 'is_wispr_supported'.
> service: Document service unref calls in 'cancel_online_check'.
> wispr: Add documentation to 'wispr_portal_detect'.
> wispr: Add documentation to '__connman_wispr_stop'.
> wispr: Add and leverage 'cancel_connman_wispr_portal_context'.
> wispr: Add documentation to 'cancel_connman_wispr_portal_context'.
> gweb: Add optional OS error status parameter to 'g_web_request_*'.
> wispr: Add and leverage an OS error parameter to
> '__connman_wispr_cb_t'.
> service: Add IP configuration type to 'cancel_online_check'.
> gweb: Add reference count debug statements.
> gweb: Factor out GWeb destruction into 'g_web_free'.
> wispr: Add '__connman_wispr_cancel' interface.
> wispr: Add documentation to '__connman_wispr_cancel'.
> wispr: Avoid double-free in 'free_wispr_routes'.
> service: Leverage '__connman_wispr_cancel'.
>
> gweb/gweb.c | 94 +++++++++----
> gweb/gweb.h | 8 +-
> src/6to4.c | 2 +-
> src/connman.h | 5 +-
> src/service.c | 70 ++++++++--
> src/wispr.c | 342 ++++++++++++++++++++++++++++++++++++++++-------
> tools/web-test.c | 3 +-
> tools/wispr.c | 8 +-
> 8 files changed, 440 insertions(+), 92 deletions(-)
all 17 patches have been applied.
Regards
Marcel
prev parent reply other threads:[~2023-11-23 11:19 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-16 1:02 [PATCH 00/17] Address Redundant IPv4 Reachability Checks Grant Erickson
2023-11-16 1:02 ` [PATCH 01/17] wispr: Rename context proxy-related data members Grant Erickson
2023-11-16 1:02 ` [PATCH 02/17] wispr: Add and leverage 'is_wispr_supported' Grant Erickson
2023-11-16 1:02 ` [PATCH 03/17] wispr: Add documentation to 'is_wispr_supported' Grant Erickson
2023-11-16 1:02 ` [PATCH 04/17] service: Document service unref calls in 'cancel_online_check' Grant Erickson
2023-11-16 1:02 ` [PATCH 05/17] wispr: Add documentation to 'wispr_portal_detect' Grant Erickson
2023-11-16 1:02 ` [PATCH 06/17] wispr: Add documentation to '__connman_wispr_stop' Grant Erickson
2023-11-16 1:02 ` [PATCH 07/17] wispr: Add and leverage 'cancel_connman_wispr_portal_context' Grant Erickson
2023-11-16 1:02 ` [PATCH 08/17] wispr: Add documentation to 'cancel_connman_wispr_portal_context' Grant Erickson
2023-11-16 1:02 ` [PATCH 09/17] gweb: Add optional OS error status parameter to 'g_web_request_*' Grant Erickson
2023-11-16 1:02 ` [PATCH 10/17] wispr: Add and leverage an OS error parameter to '__connman_wispr_cb_t' Grant Erickson
2023-11-16 1:02 ` [PATCH 11/17] service: Add IP configuration type to 'cancel_online_check' Grant Erickson
2023-11-16 1:02 ` [PATCH 12/17] gweb: Add reference count debug statements Grant Erickson
2023-11-16 1:02 ` [PATCH 13/17] gweb: Factor out GWeb destruction into 'g_web_free' Grant Erickson
2023-11-16 1:02 ` [PATCH 14/17] wispr: Add '__connman_wispr_cancel' interface Grant Erickson
2023-11-16 1:02 ` [PATCH 15/17] wispr: Add documentation to '__connman_wispr_cancel' Grant Erickson
2023-11-16 1:02 ` [PATCH 16/17] wispr: Avoid double-free in 'free_wispr_routes' Grant Erickson
2023-11-16 1:02 ` [PATCH 17/17] service: Leverage '__connman_wispr_cancel' Grant Erickson
2023-11-19 14:12 ` [PATCH 00/17] Address Redundant IPv4 Reachability Checks Marcel Holtmann
2023-11-19 17:08 ` Grant Erickson
2023-11-19 17:06 ` [PATCH v2 " Grant Erickson
2023-11-19 17:06 ` [PATCH v2 01/17] wispr: Rename context proxy-related data members Grant Erickson
2023-11-19 17:06 ` [PATCH v2 02/17] wispr: Add and leverage 'is_wispr_supported' Grant Erickson
2023-11-19 17:06 ` [PATCH v2 03/17] wispr: Add documentation to 'is_wispr_supported' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 04/17] service: Document service unref calls in 'cancel_online_check' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 05/17] wispr: Add documentation to 'wispr_portal_detect' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 06/17] wispr: Add documentation to '__connman_wispr_stop' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 07/17] wispr: Add and leverage 'cancel_connman_wispr_portal_context' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 08/17] wispr: Add documentation to 'cancel_connman_wispr_portal_context' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 09/17] gweb: Add optional OS error status parameter to 'g_web_request_*' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 10/17] wispr: Add and leverage an OS error parameter to '__connman_wispr_cb_t' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 11/17] service: Add IP configuration type to 'cancel_online_check' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 12/17] gweb: Add reference count debug statements Grant Erickson
2023-11-19 17:07 ` [PATCH v2 13/17] gweb: Factor out GWeb destruction into 'g_web_free' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 14/17] wispr: Add '__connman_wispr_cancel' interface Grant Erickson
2023-11-19 17:07 ` [PATCH v2 15/17] wispr: Add documentation to '__connman_wispr_cancel' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 16/17] wispr: Avoid double-free in 'free_wispr_routes' Grant Erickson
2023-11-19 17:07 ` [PATCH v2 17/17] service: Leverage '__connman_wispr_cancel' Grant Erickson
2023-11-23 11:19 ` Marcel Holtmann [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=4B140B42-25C2-4A6A-A772-166D075E4FD0@holtmann.org \
--to=marcel@holtmann.org \
--cc=connman@lists.linux.dev \
--cc=gerickson@nuovations.com \
/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).