RE: [EXTERNAL] Re: Feature request: provide ability to set a global callback function telling libcurl if IPv6 works on the system

From: Dmitry Karpov via curl-library <curl-library_at_lists.haxx.se>
Date: Fri, 23 Sep 2022 21:52:51 +0000

In addition to my previous arguments, a few comments about Daniel's PR (https://github.com/curl/curl/pull/9553).

While this PR helps IPv4-only mode to avoid delays caused by default Curl_ipv6works(), it eliminates some workarounds for dual-stack modes, which were available before the PR.

The current approach calls Curl_ipv6works() when a multi-handle is created and before any easy handle is added to it.
This allows applications to create a multi handle ahead of time and consume the Curl_ipv6works() delays not in time critical areas - potentially long before easy handles using dual-stack are added.

Then easy handles will not have connection delays in time critical areas due to Curl_ipv6works(), because it was already called during multi-handle creation.
Such approach maybe not easy to implement in all the cases, but at least it was available.

The PR moves the Curl_ipv6works() call to the time when the first dual-stack connection is established for the multi-handle.
This "lazy" approach doesn't allow to create a multi-handle ahead of time to avoid delays on dual-stack connections.

So, if someone was using this approach it will not work anymore after the PR is integrated.

I fully support the PR, but I also think that this is also another argument for adding the "IPv6 works" system callback.

Let's say someone doesn't want to change the Curl_ipv6works() default behavior, but he/she wants to have "IPv6 works on the system" check ahead of time.
So, when it is time to make dual-stack connections, the check (even if takes 30+ms) is already done, and the connection doesn't need to call Curl_ipv6works() in time-critical area.

The "IPv6 works" system callback will allow easily to achieve that.
Someone can do the default check once at some time and then set a global flag used in the callback thus eliminating any delays after the initial check.

Or maybe implement a more flexible approach when these checks are done periodically - by timer or when application detects some system configuration changes (i.e. network changes) and update the "IPv6 works" flag.

In other words, the "IPv6 works" system callback provides a nice flexibility for implementing different scenarios.

Thanks,
Dmitry Karpov

From: curl-library <curl-library-bounces_at_lists.haxx.se> On Behalf Of Dmitry Karpov via curl-library
Sent: Friday, September 23, 2022 1:35 PM
To: libcurl development <curl-library_at_lists.haxx.se>
Cc: Dmitry Karpov <dkarpov_at_roku.com>
Subject: RE: [EXTERNAL] Re: Feature request: provide ability to set a global callback function telling libcurl if IPv6 works on the system

I definitely agree with the observations, but not with the alternative approach.

Using a file like “. /etc/default/libcurl” will create more issues rather than helping.
It is like using a global “IPv6 works” variable, which was set during curl_global_init() in the past.

That approach was prone to multi-threading issues and race conditions when network settings affecting IPv6 stack were set after curl_global_init(),
and adding reading/wring to this “config” file (which has to be done somewhere) from different places just adds more problems.
It will be difficult to avoid file access issues if the code detecting if IPv6 works, needs to update this file when it is being read by libcurl.

So, I think that the “config” file approach is much more complex and has more implementation difficulties (i.e. just dealing with file names on Windows) than the approach using “IPv6 works” system callback.

Thanks,
Dmitry Karpov

From: curl-library <curl-library-bounces_at_lists.haxx.se<mailto:curl-library-bounces_at_lists.haxx.se>> On Behalf Of Timothe Litt via curl-library
Sent: Friday, September 23, 2022 1:15 PM
To: curl-library_at_lists.haxx.se<mailto:curl-library_at_lists.haxx.se>
Cc: Timothe Litt <litt_at_acm.org<mailto:litt_at_acm.org>>
Subject: Re: [EXTERNAL] Re: Feature request: provide ability to set a global callback function telling libcurl if IPv6 works on the system


This discussion revolves around some platforms on which it is difficult to efficiently determine if IPv6 is *not* available.

Observations:

  * The proposed solutions involve determining the converse.
  * On a given platform, answer is likely to be stable for a very long time. E.g. until a kernel/network/system change
  * The OP reports that getting the "no IPv6" result efficiently is very situation - and probably customer - dependent - e.g. is using heuristics that would not be a good fit in libcurl
  * libcurl isn't the right place for customer-dependent work-arounds; at best the detection would be once/process

Here's an alternative approach to consider:

  * Have libcurl read an optional configuration file; e.g. /etc/default/libcurl
  * The initial contents would be 'ipv6_disable:1"
  * When a similar issue arises for IPv4, or DNS, codesets, or some other system behavior, it's easy to add controls
  * If the file doesn't exist, libcurl uses its current defaults/behaviors - configure and runtime
  * Setting up the file becomes an installation-time issue on your platform. It can take as long as you like, and be as system specific as you like.
  * The overhead for libcurl is very small. On most systems, the file won't exist - and as strace will tell you, many, many files are looked-up and ignored when any non-trivial process starts. If the file does exist, the flags can be cached when the file is parsed. If performance ever becomes an issue (seems unlikely), the cache could be in a shared memory segment.
  * Optionally, libcurl could provide an API function to query a flag by name.

This would seem to meet the OP's requirements: no change to the application, easy to install on the affected platforms, efficient.

And it is minimally disruptive to libcurl, which should satisfy the developers.


Timothe Litt

ACM Distinguished Engineer

--------------------------

This communication may not represent the ACM or my employer's views,

if any, on the matters discussed.
On 23-Sep-22 15:15, Dmitry Karpov via curl-library wrote:

Let me reply a bit out of order and combine some arguments:



Exactly. Because it would create exactly the same outcome.



This would create the same outcome ONLY after your PR fixing IPv4 mode is applied.

Before that PR, the problem existed for ALL modes, so there was no work around even for IPv4 mode.



First: most developers won't write this kind of function at all.

curl has supported IPv6 since January 2001 and we are talking about doing this in a future release - after nearly 22 years of shipping IPv6 suppport. Clearly a lot of users manage decently without this.



Yes, it may be supported since a while ago, but when I started migrating to dual-stack more than a year ago, I stepped on multiple issues, which made me think that dual-stack has been hardly used on a

wide scale where performance was quite critical - at least on embedded platforms with large populations.



My applications run on different HW platforms, use a wide range of linux kernels with different configurations and user population is greater than 62M, which covers a very wide range of ISPs, routers and network setups. That's why maybe I stepped on the issues which other didn't see - or just didn't care.

The 30ms delay during connection setup maybe not a critical issue for everyone, but it is a still an issue and regression when migrating from a single-stack for time critical applications.



.They use the same code to determine IPv6 functionality: your function, and they do it roughly at the same time in the process: right before setting up a new connection.

The only difference is how that function is called.



That's a huge difference. Your proposal (which will work only after your PR is integrated) requires modifying transfer setup code in ALL the places (for current and new code ) whereas my proposal allows to do it in just one place and not worry about if someone will miss "manual" check in some place and create a timing issue.



For some reason you think this particular transfer detail deserves a completely new and different take, and this reason seems to be because of the way you have designed and setup your libcurl usage. In my view, it > does not seem to be particularly well founded from a libcurl architectural point of view.

I'm sorry, but I think consistency and simplicity is more important here.



I would argue that I see quite the opposite - that the "consistency and simplicity" is on my side rather than on yours.



The "IPv6 works" is not a transfer level detail - it is a system level detail.

It is important to outline that the "IPv6 works" check is not a IPv6 connectivity check - it is just a check if IPv6 is "enabled" on the system and can be used in connection setup.



This check is currently done when a multi handle is created and then its result is applied to every easy handle added to that multi-handle - regardless the resolve mode set on the easy handle.

(And a while ago that check was done only once in curl_global_init() which created a base for multi-threading and race condition issues)



What you suggest is essentially to consider "IPv6 doesn't work on a system" as a transfer level detail and do such check for every transfer setup.

And this just goes against consistency and simplicity, in my opinion.



So, if someone steps on the same connection delay issues using dual-stack libcurl as I did - your recommendation (again after you PR is integrated) would be:



    Because our default "IPv6 works" check doesn't work for you do the following:

    - write your own function doing " IPv6 works" check which works better for you (the same step as in my approach)

    - change ALL the places in the code where your transfers are set up, invoke your "IPv6 works" check function

       and force your transfers to use IPv4 resolve mode if that check fails.



    Fine print details:

        - be careful not to miss any single place where transfers might be using dual-stack in your code.

         - when you add new transfers to your code be careful not to miss the "IPv6 works" check function if they might be using dual-stack.

         - if you code integrates (or will do that future) some "3rd party" components which might be using libcurl and dual-stack resolve modes, which you can't modify

            then, sorry, you are out of luck. This is an "engineering issue", so the connection delays when "IPv6 doesn't work" will be there.



With my approach:

      Because our default "IPv6 works" check doesn't work for you do the following:

      - write your own function doing "IPv6 works" check which works better for you.

      - set your function as "IPv6 works" callback in curl_global_init_ipv6()



    Fine print details:

         None. It will work for current and future code using dual-stack modes and for "closed code" of 3rd party components

         without any modifications in the transfer setup code.



So, I guess from this side-by-side comparison it is pretty obvious which approach adheres more to the principles of consistency and simplicity.

Also, there are already "system" level callbacks which allow to set system wide memory management functions in curl_global_init_mem().



And I just propose to add a similar system wide "IPv6 works on the system" check callback, so I am quite consistent here with what libcurl does.



Thanks,

Dmitry Karpov



-----Original Message-----

From: Daniel Stenberg <daniel_at_haxx.se><mailto:daniel_at_haxx.se>

Sent: Friday, September 23, 2022 12:54 AM

To: Dmitry Karpov <dkarpov_at_roku.com><mailto:dkarpov_at_roku.com>

Cc: Dmitry Karpov via curl-library <curl-library_at_lists.haxx.se><mailto:curl-library_at_lists.haxx.se>

Subject: RE: [EXTERNAL] Re: Feature request: provide ability to set a global callback function telling libcurl if IPv6 works on the system



On Thu, 22 Sep 2022, Dmitry Karpov wrote:



If I understand what you suggest, then in all the places where the "AUTO"

(CURL_IPRESOLVE_WHATEVER) is used, I would need to use something like:



long resolve_mode = my_ipv6_works() ? CURL_IPRESOLVE_WHATEVER :

CURL_IPRESOLVE_V4; curl_easy_setopt(curl, CURLOPT_IPRESOLVE,

resolve_mode);



Exactly. Because it would create exactly the same outcome.



First one is that the "AUTO" will stop being the most efficient mode

for dual-stack, because its efficiency will depend on how "IPv6 works"

check (provided by "factory default" Curl_ipv6works()) works on some

systems (kernels, network configs etc).



Not at all. You say you can write a "ipv6works" function (as a callback) that can detect/set the status, so clearly then it can be called by your own application as well. There is no difference in effectiveness between these two approaches. They use the same code to determine IPv6 functionality: your function, and they do it roughly at the same time in the process: right before setting up a new connection.



The only difference is how that function is called.



And if some curl application accidentally steps on this issue and

needs to implement some custom "IPv6 works" mechanism, then it is

suggested that after implementing it, application developer should go

through all the places where transfer options are set, explicitly use

application specific

"IPv6 works" function and explicitly select IPv4 mode if that function

returns "false".



Exactly like you do when you set other properties and control other characteristcs of libcurl transfers. There are *300* options for curl_easy_setopt(), I'm sure you realized by now that this is the libcurl API model.



For some reason you think this particular transfer detail deserves a completely new and different take, and this reason seems to be because of the way you have designed and setup your libcurl usage. In my view, it does not seem to be particularly well founded from a libcurl architectural point of view.



I'm sorry, but I think consistency and simplicity is more important here.



Frankly, I think it is just cruel to make developers do that when

everything can be solved in just one system level "IPv6 works" callback.



First: most developers won't write this kind of function at all. curl has supported IPv6 since January 2001 and we are talking about doing this in a future release - after nearly 22 years of shipping IPv6 suppport. Clearly a lot of users manage decently without this.



Then, many users do their setopt logic in a single place or at least offers mechanics to extend the set without going through hoops. Doing this logic as a callback or as a setopt then means roughly the same effort.



I don't think insisting on consistency is cruel.



And for me the biggest problem is that I just can't change the code of

certain curl-based components used in my application. They are written

by some other developers and closed for any modifications.



This is the core of your issue, not necessarily related to the libcurl issue at hand. I would argue that this is a fragile way to go about libcurl use (and fix). What if you find another flaw tomorrow where a setopt is used wrongly?

Would we solve those as well with additional work-arounds and coming in through the backdoor just because someone else has the key to the front door?



In other words, it is much easier and cleaner to override how

Curl_ipv6works() to adjust to demands of certain systems/scenarios

than do checks if "IPv6 works" and change resolve mode in every place

where application sets a transfer.



Easier and cleaner for you. I maintain that it is dirtier for libcurl and its API (design).



Also, Curl_ipv6works() is called only once per multi-handle, and the

"IPv6 works" setting is then used for all easy handles added to that multi-handle.

So, essentially there is no need to do "IPv6 works" for every easy

handle - that's also what the system "IPv6 works" callback efficiently

allows to avoid.



When you write your own ipv6works function, you can decide yourself how often and when it needs to rerun the check. There's nothing that says it needs to do anything for every easy handle, it could just return the previous result.

-- 
Unsubscribe: https://lists.haxx.se/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Received on 2022-09-23