Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(quic): allow disabling of path MTU discovery #4823

Merged
merged 6 commits into from
Nov 12, 2023

Conversation

nazar-pc
Copy link
Contributor

@nazar-pc nazar-pc commented Nov 8, 2023

Description

Path MTU discovery seems to be broken in QUIC right now on Windows and we had to disable it in Subspace.

Related: autonomys/subspace#2195.

Attributions

Co-authored-by: Shamil Gadelshin shamilgadelshin@gmail.com

Notes & open questions

It is following default behavior of quinn and just adds an option to change that if necessary.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 works for me. Can you add a changelog entry?

transports/quic/src/config.rs Outdated Show resolved Hide resolved
@thomaseizinger

This comment was marked as resolved.

@nazar-pc nazar-pc force-pushed the mtu-path-discovery-configuration branch from a3b7c8d to 5a50ef8 Compare November 10, 2023 20:28
@thomaseizinger thomaseizinger changed the title feat(quic): Add path MTU discovery configuration feat(quic): allow disabling of path MTU discovery Nov 12, 2023
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! just one formality.

transports/quic/CHANGELOG.md Outdated Show resolved Hide resolved
transports/quic/CHANGELOG.md Show resolved Hide resolved
@nazar-pc nazar-pc force-pushed the mtu-path-discovery-configuration branch from f4b8218 to 12ec98b Compare November 12, 2023 05:42
@nazar-pc
Copy link
Contributor Author

Yep, apologies, this trivial PR will certainly need to be squashed

@nazar-pc nazar-pc force-pushed the mtu-path-discovery-configuration branch from 12ec98b to 43d6211 Compare November 12, 2023 05:45
@thomaseizinger
Copy link
Contributor

Yep, apologies, this trivial PR will certainly need to be squashed

We always squash-merge! :)

@mergify mergify bot merged commit 3ec575a into libp2p:master Nov 12, 2023
71 checks passed
@nazar-pc nazar-pc deleted the mtu-path-discovery-configuration branch November 17, 2023 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants