Skip to content

multi, signal network changed#17613

Closed
icing wants to merge 8 commits into
curl:masterfrom
icing:multi-network-changed
Closed

multi, signal network changed#17613
icing wants to merge 8 commits into
curl:masterfrom
icing:multi-network-changed

Conversation

@icing
Copy link
Copy Markdown
Contributor

@icing icing commented Jun 13, 2025

A modified version of #17246 for signalling changes in network:

New multi option CURLMOPT_NETWORK_CHANGED with a long bitmask value:

  • CURLM_NWCOPT_CLEAR_CONNS: do not reuse existing connections, close all idle connections.
  • CURLM_NWCOPT_CLEAR_DNS: clear the multi's DNS cache.

All other bits reserved for future extensions.

Comment thread tests/libtest/lib3033.c Outdated
@bagder
Copy link
Copy Markdown
Member

bagder commented Jun 15, 2025

But what if a user is doing many requests using a single handle with the easy interface and then changes network?

@icing
Copy link
Copy Markdown
Contributor Author

icing commented Jun 15, 2025

But what if a user is doing many requests using a single handle with the easy interface and then changes network?

Throw away the easy handle and make a new one, is probably the easiest way. Maybe do a SHARE with DNS caching one wants to keep that.

The question is if we want a SHARE option in a similar way. But we can do that if someone really has use for it.

@icing icing force-pushed the multi-network-changed branch from c173bda to 40b9fce Compare June 16, 2025 10:41
@bagder bagder added the feature-window A merge of this requires an open feature window label Jun 20, 2025
@bagder
Copy link
Copy Markdown
Member

bagder commented Jun 20, 2025

It also struck me that alt-svc has a parameter to flush the entry on "network change", that we could look into having a bit for

@bagder
Copy link
Copy Markdown
Member

bagder commented Jun 20, 2025

Anyone object to leaving this for the next feature window to allow some more feedback discussion?

I would love @Cering's comments on this take as compared to #17246

@Cering
Copy link
Copy Markdown
Contributor

Cering commented Jun 23, 2025

I would love @Cering's comments on this take as compared to #17246

Sorry for the delayed response.

Yes, this CURLMOPT_NETWORK_CHANGED is a better way to adapt network changing, and I prefer this PR to #17246.

@icing
Copy link
Copy Markdown
Contributor Author

icing commented Jun 23, 2025

Thanks for the feedback!

I believe we should make the value a bitmask with defines for CONN_REUSE, DNS and whatever else we might come up with.

@icing icing force-pushed the multi-network-changed branch from 40b9fce to d52fc5f Compare June 25, 2025 07:31
@icing
Copy link
Copy Markdown
Contributor Author

icing commented Jun 25, 2025

rebased, made the option value into a bitmask.

@gu2890961
Copy link
Copy Markdown

gu2890961 commented Jul 3, 2025

please approved these changes, thank you! @bagder

@icing
Copy link
Copy Markdown
Contributor Author

icing commented Jul 3, 2025

please approved these changes, thank you! @bagder

Since our feature window is closed for the release end of month, this will most likely become part of the September release.

Comment thread docs/libcurl/opts/CURLMOPT_NETWORK_CHANGED.md Outdated
Comment thread docs/libcurl/symbols-in-versions Outdated
Comment thread docs/libcurl/symbols-in-versions Outdated
@icing icing force-pushed the multi-network-changed branch from d52fc5f to 7d4b50e Compare July 28, 2025 07:02
@icing icing requested a review from bagder July 28, 2025 07:25
Comment thread docs/libcurl/opts/CURLMOPT_NETWORK_CHANGED.md Outdated
icing added 6 commits July 29, 2025 10:27
A modified version of curl#17246 for signalling changes in network:

New multi option `CURLMOPT_NETWORK_CHANGED` with a long value:
0 - does nothing
1 - do not reuse existing connections
2 - as 1, but also clear DNS cache

One could also make the long a bitmask, if people want more control.

Documentation to be added if this gets the vote to go forward.
@icing icing force-pushed the multi-network-changed branch from 7d4b50e to dbe068b Compare July 29, 2025 08:38
@bagder bagder closed this in 55c045c Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-window A merge of this requires an open feature window libcurl API tests

Development

Successfully merging this pull request may close these issues.

5 participants