Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(server): filter out /quic in favor of /quic-v1#4467

Merged
mergify[bot] merged 8 commits intolibp2p:masterfrom
mxinden:server-dedup-listen
Sep 12, 2023
Merged

fix(server): filter out /quic in favor of /quic-v1#4467
mergify[bot] merged 8 commits intolibp2p:masterfrom
mxinden:server-dedup-listen

Conversation

Copy link
Member

mxinden commented Sep 7, 2023 *
edited
Loading

Description

Configuration files generated by Kubo <= v0.22 list both /quic and /quic-v1 listen addresses with the same UDP port. Given that we enable draft-29, the two addresses are treated the same by rust-libp2p's QUIC implementation. Though calling listen_on with both results in an "Address already in use" error by the OS on the second call. To prevent this from happening filter out /quic addresses in favor of /quic-v1.

Notes & open questions

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

Configuration files generated by Kubo <= v0.22 list both `/quic` and `/quic-v1` listen
addresses with the same UDP port. Given that we enable draft-29, the two addresses are
treated the same by rust-libp2p's QUIC implementation. Though calling `listen_on` with
both results in an "Address already in use" error by the OS on the second call. To
prevent this from happening filter out `/quic` addresses in favor of `/quic-v1`.
mxinden requested review from mcamou and thomaseizinger September 7, 2023 11:07
mcamou reviewed Sep 7, 2023
mcamou approved these changes Sep 7, 2023
mcamou approved these changes Sep 7, 2023
thomaseizinger requested review from mcamou and removed request for thomaseizinger September 11, 2023 02:50
Copy link
Contributor

thomaseizinger commented Sep 11, 2023

@mcamou This disables draft-29 support from rust-libp2p-server which means for the /quic addresses in the config, we will log a warning that we can't listen on it anymore. Is that okay? Given that we will deprecate draft-29 anyway, I figured this is a much simpler solution.

mcamou approved these changes Sep 11, 2023
Copy link
Contributor

mcamou commented Sep 11, 2023

@mcamou This disables draft-29 support from rust-libp2p-server which means for the /quic addresses in the config, we will log a warning that we can't listen on it anymore. Is that okay? Given that we will deprecate draft-29 anyway, I figured this is a much simpler solution.

@thomaseizinger sounds good! I think this is what we need.

thomaseizinger reacted with thumbs up emoji

This comment was marked as resolved.

thomaseizinger approved these changes Sep 11, 2023
thomaseizinger added the send-it label Sep 11, 2023
Copy link
Contributor

mergify bot commented Sep 11, 2023

This pull request has merge conflicts. Could you please resolve them @mxinden?

mergify bot merged commit 0e64d71 into libp2p:master Sep 12, 2023
mxinden commented Sep 14, 2023
Copy link
Member Author

mxinden left a comment

Choose a reason for hiding this comment

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

Thank you. I find this an easier solution than the one I initially proposed.

Copy link
Contributor

thomaseizinger commented Sep 14, 2023

Thank you. I find this an easier solution than the one I initially proposed.

I forgot to update the PR description though so the commit message is all wrong :/

mxinden mentioned this pull request Sep 21, 2023
4 tasks
mxinden added a commit to mxinden/rust-libp2p that referenced this pull request Oct 10, 2023
Support for QUIC draft 29 was removed with libp2p#4467.

libp2p#4120 reintroduced it as a faulty merge.

This commit removes it again.
mxinden mentioned this pull request Oct 10, 2023
4 tasks
mergify bot pushed a commit that referenced this pull request Oct 10, 2023
Support for QUIC draft 29 was removed with #4467.

#4120 reintroduced it as a faulty merge.

This commit removes it again.

Pull-Request: #4622.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

thomaseizinger thomaseizinger approved these changes

+1 more reviewer

mcamou mcamou approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants