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

fix: Exit with ProtocolViolation if a packet has no frames #1779

Merged
merged 6 commits into from
Apr 2, 2024
Merged

Conversation

larseggert
Copy link
Collaborator

Fixes #1476

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.06%. Comparing base (3151adc) to head (f5caaa8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1779   +/-   ##
=======================================
  Coverage   93.05%   93.06%           
=======================================
  Files         117      117           
  Lines       36368    36372    +4     
=======================================
+ Hits        33843    33849    +6     
+ Misses       2525     2523    -2     

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

Copy link

github-actions bot commented Mar 28, 2024

Benchmark results

Performance differences relative to 3151adc.

  • drain a timer quickly time: [348.05 ns 356.26 ns 363.95 ns]
    change: [-12.726% -11.220% -9.6153%] (p = 0.00 < 0.05)
    💚 Performance has improved.

  • coalesce_acked_from_zero 1+1 entries
    time: [195.15 ns 195.57 ns 196.02 ns]
    change: [-2.0110% -1.6060% -1.2326%] (p = 0.00 < 0.05)
    💚 Performance has improved.

  • coalesce_acked_from_zero 3+1 entries
    time: [234.83 ns 235.43 ns 236.07 ns]
    change: [-1.0831% -0.6845% -0.3026%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • coalesce_acked_from_zero 10+1 entries
    time: [234.59 ns 235.55 ns 236.64 ns]
    change: [-2.4201% -0.9672% -0.0253%] (p = 0.12 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1000+1 entries
    time: [216.49 ns 216.64 ns 216.81 ns]
    change: [-0.8321% -0.2219% +0.3473%] (p = 0.49 > 0.05)
    No change in performance detected.

  • RxStreamOrderer::inbound_frame()
    time: [119.31 ms 119.37 ms 119.44 ms]
    change: [-0.1639% -0.0869% -0.0145%] (p = 0.02 < 0.05)
    Change within noise threshold.

  • transfer/Run multiple transfers with varying seeds
    time: [117.84 ms 118.14 ms 118.44 ms]
    thrpt: [33.773 MiB/s 33.857 MiB/s 33.944 MiB/s]
    change:
    time: [-1.0124% -0.7047% -0.3498%] (p = 0.00 < 0.05)
    thrpt: [+0.3510% +0.7097% +1.0227%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [118.62 ms 118.78 ms 118.95 ms]
    thrpt: [33.626 MiB/s 33.675 MiB/s 33.722 MiB/s]
    change:
    time: [-1.1013% -0.9126% -0.7208%] (p = 0.00 < 0.05)
    thrpt: [+0.7261% +0.9210% +1.1136%]
    Change within noise threshold.

  • 1-conn/1-100mb-resp (aka. Download)/client
    time: [1.1152 s 1.1317 s 1.1555 s]
    thrpt: [86.540 MiB/s 88.362 MiB/s 89.669 MiB/s]
    change:
    time: [+2.4327% +4.6437% +7.1631%] (p = 0.00 < 0.05)
    thrpt: [-6.6843% -4.4377% -2.3749%]
    💔 Performance has regressed.

  • 1-conn/10_000-1b-seq-resp (aka. RPS)/client
    time: [427.69 ms 429.81 ms 431.94 ms]
    thrpt: [23.151 Kelem/s 23.266 Kelem/s 23.381 Kelem/s]
    change:
    time: [-0.1437% +0.5186% +1.1808%] (p = 0.13 > 0.05)
    thrpt: [-1.1670% -0.5159% +0.1439%]
    No change in performance detected.

  • 100-seq-conn/1-1b-resp (aka. HPS)/client
    time: [3.3699 s 3.3730 s 3.3762 s]
    thrpt: [29.619 elem/s 29.647 elem/s 29.674 elem/s]
    change:
    time: [-1.0145% -0.8906% -0.7739%] (p = 0.00 < 0.05)
    thrpt: [+0.7800% +0.8986% +1.0249%]
    Change within noise threshold.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 460.2 ± 63.8 394.0 586.8 1.00
neqo msquic reno on 1931.4 ± 32.6 1879.6 1995.8 1.00
neqo msquic reno 1989.6 ± 202.3 1856.9 2558.4 1.00
neqo msquic cubic on 1934.5 ± 25.2 1880.9 1964.4 1.00
neqo msquic cubic 1926.3 ± 70.4 1786.3 2063.2 1.00
msquic neqo reno on 4376.8 ± 137.2 4177.7 4572.4 1.00
msquic neqo reno 4384.7 ± 148.6 4158.8 4613.2 1.00
msquic neqo cubic on 4417.5 ± 163.6 4185.0 4661.3 1.00
msquic neqo cubic 4347.2 ± 183.6 4110.5 4589.0 1.00
neqo neqo reno on 3767.6 ± 210.6 3497.3 4205.0 1.00
neqo neqo reno 3501.5 ± 354.6 2767.9 3998.3 1.00
neqo neqo cubic on 4289.4 ± 152.0 4043.5 4489.8 1.00
neqo neqo cubic 4294.1 ± 138.5 4109.2 4470.9 1.00

⬇️ Download logs

neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved
@larseggert larseggert added this pull request to the merge queue Apr 2, 2024
Merged via the queue into main with commit 27a7250 Apr 2, 2024
15 checks passed
@larseggert larseggert deleted the fix-1476 branch April 2, 2024 08:04
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.

Neqo server does not close the connection when receiving a packet without any frames.
2 participants