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

refactor(server): replace mio with tokio #1581

Merged
merged 12 commits into from
Feb 1, 2024
Merged

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Jan 24, 2024

Use tokio instead of mio and mio-extras in neqo-server.

See #1535 for details.

@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2024

Codecov Report

Attention: 70 lines in your changes are missing coverage. Please review.

Comparison is base (5e32696) 87.51% compared to head (3240b64) 87.61%.

Files Patch % Lines
neqo-server/src/main.rs 0.00% 70 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1581      +/-   ##
==========================================
+ Coverage   87.51%   87.61%   +0.09%     
==========================================
  Files         118      118              
  Lines       38656    38615      -41     
==========================================
  Hits        33831    33831              
+ Misses       4825     4784      -41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator Author

@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.

This is ready for a first review.

Quic Interop tests with Neqo itself and ngtcp2 succeed.

neqo-server/src/main.rs Show resolved Hide resolved
neqo-server/src/main.rs Show resolved Hide resolved
@mxinden mxinden marked this pull request as ready for review January 30, 2024 15:50
KershawChang
KershawChang previously approved these changes Jan 30, 2024
Copy link
Collaborator

@KershawChang KershawChang left a comment

Choose a reason for hiding this comment

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

Looks good to me.

neqo-server/src/main.rs Show resolved Hide resolved
neqo-server/src/main.rs Outdated Show resolved Hide resolved
neqo-server/src/main.rs Show resolved Hide resolved
@mxinden
Copy link
Collaborator Author

mxinden commented Jan 31, 2024

All comments addressed. Would you mind taking another look?

I can look into converting neqo-client next, unless there are any objections?

@larseggert
Copy link
Collaborator

LGTM. No objection from my side on doing neqo-client. Thanks for your contributions!

@larseggert
Copy link
Collaborator

@mxinden are you planning to use this PR for neqo-client, or should it be merged?

@mxinden
Copy link
Collaborator Author

mxinden commented Feb 1, 2024

Preference to merge here as is and do the client side through a separate pull request @larseggert.

I have the neqo-client changes locally. I have not yet opened a pull request as one interop test (http3) is failing i.e. needs debugging first.

Signed-off-by: Max Inden <mail@max-inden.de>
@larseggert
Copy link
Collaborator

OK. I'm prepared to merge this once the CI is green.

@larseggert larseggert merged commit 20c8e8c into mozilla:main Feb 1, 2024
10 checks passed
mxinden added a commit to mxinden/neqo that referenced this pull request May 4, 2024
There are two server implementations based on neqo:

1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server
  - http3 and http09 implementation
  - used for manual testing and QUIC Interop

2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs
  - used to test Firefox

I assume one was once an exact copy of the other. Both implement their own I/O,
event loop, ... Since then, the two implementations diverged significantly.
Especially (1) saw a lot of improvements in recent months:

- mozilla#1564
- mozilla#1569
- mozilla#1578
- mozilla#1581
- mozilla#1604
- mozilla#1612
- mozilla#1676
- mozilla#1692
- mozilla#1707
- mozilla#1708
- mozilla#1727
- mozilla#1753
- mozilla#1756
- mozilla#1766
- mozilla#1772
- mozilla#1786
- mozilla#1787
- mozilla#1788
- mozilla#1794
- mozilla#1806
- mozilla#1808
- mozilla#1848
- mozilla#1866

At this point, bugs in (2) are hard to fix, see e.g.
mozilla#1801.

This commit merges (2) into (1), thus removing all duplicate logic and
having (2) benefit from all the recent improvements to (1).
KershawChang pushed a commit to KershawChang/neqo that referenced this pull request May 7, 2024
There are two server implementations based on neqo:

1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server
  - http3 and http09 implementation
  - used for manual testing and QUIC Interop

2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs
  - used to test Firefox

I assume one was once an exact copy of the other. Both implement their own I/O,
event loop, ... Since then, the two implementations diverged significantly.
Especially (1) saw a lot of improvements in recent months:

- mozilla#1564
- mozilla#1569
- mozilla#1578
- mozilla#1581
- mozilla#1604
- mozilla#1612
- mozilla#1676
- mozilla#1692
- mozilla#1707
- mozilla#1708
- mozilla#1727
- mozilla#1753
- mozilla#1756
- mozilla#1766
- mozilla#1772
- mozilla#1786
- mozilla#1787
- mozilla#1788
- mozilla#1794
- mozilla#1806
- mozilla#1808
- mozilla#1848
- mozilla#1866

At this point, bugs in (2) are hard to fix, see e.g.
mozilla#1801.

This commit merges (2) into (1), thus removing all duplicate logic and
having (2) benefit from all the recent improvements to (1).
github-merge-queue bot pushed a commit that referenced this pull request May 8, 2024
* refactor(bin): introduce server/http3.rs and server/http09.rs

The QUIC Interop Runner requires an http3 and http09 implementation for both
client and server. The client code is already structured into an http3 and an
http09 implementation since #1727.

This commit does the same for the server side, i.e. splits the http3 and http09
implementation into separate Rust modules.

* refactor: merge mozilla-central http3 server into neqo-bin

There are two server implementations based on neqo:

1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server
  - http3 and http09 implementation
  - used for manual testing and QUIC Interop

2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs
  - used to test Firefox

I assume one was once an exact copy of the other. Both implement their own I/O,
event loop, ... Since then, the two implementations diverged significantly.
Especially (1) saw a lot of improvements in recent months:

- #1564
- #1569
- #1578
- #1581
- #1604
- #1612
- #1676
- #1692
- #1707
- #1708
- #1727
- #1753
- #1756
- #1766
- #1772
- #1786
- #1787
- #1788
- #1794
- #1806
- #1808
- #1848
- #1866

At this point, bugs in (2) are hard to fix, see e.g.
#1801.

This commit merges (2) into (1), thus removing all duplicate logic and
having (2) benefit from all the recent improvements to (1).

* Move firefox.rs to mozilla-central

* Reduce HttpServer trait functions

* Extract constructor

* Remove unused deps

* Remove clap color feature

Nice to have. Adds multiple dependencies. Hard to justify for mozilla-central.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants