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

test: Experiment with cargo fuzz #1764

Merged
merged 53 commits into from
Apr 18, 2024
Merged

test: Experiment with cargo fuzz #1764

merged 53 commits into from
Apr 18, 2024

Conversation

larseggert
Copy link
Collaborator

@larseggert larseggert commented Mar 21, 2024

Adds four simple fuzzing targets, for packets and frames, and for fuzzed data inside client and server initials.

larseggert added a commit to larseggert/neqo that referenced this pull request Mar 27, 2024
PR mozilla#1764 exports `frame` and `packet` if feature `fuzzing` is enabled.
This apparently turns on a bunch of clippy checks that are not on by
default? This PR fixes them.

Made this separate from mozilla#1764 to reduce that PR's size.
github-merge-queue bot pushed a commit that referenced this pull request Mar 27, 2024
* fix: Address more clippy warnings

PR #1764 exports `frame` and `packet` if feature `fuzzing` is enabled.
This apparently turns on a bunch of clippy checks that are not on by
default? This PR fixes them.

Made this separate from #1764 to reduce that PR's size.

* Fix clippy

* Remove comment
Copy link

codecov bot commented Mar 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.23%. Comparing base (d74ad5e) to head (ed79956).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1764      +/-   ##
==========================================
- Coverage   93.23%   93.23%   -0.01%     
==========================================
  Files         110      110              
  Lines       35669    35749      +80     
==========================================
+ Hits        33256    33330      +74     
- Misses       2413     2419       +6     

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

@larseggert larseggert marked this pull request as ready for review March 27, 2024 15:59
Copy link

github-actions bot commented Mar 27, 2024

Benchmark results

Performance differences relative to ccf0302.

  • drain a timer quickly time: [428.79 ns 434.44 ns 439.72 ns]
    change: [+38.065% +40.658% +43.230%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • coalesce_acked_from_zero 1+1 entries
    time: [197.08 ns 197.60 ns 198.13 ns]
    change: [-0.3722% -0.0223% +0.3213%] (p = 0.90 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 3+1 entries
    time: [239.13 ns 239.80 ns 240.48 ns]
    change: [-0.4614% +0.1057% +0.6371%] (p = 0.72 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 10+1 entries
    time: [238.21 ns 239.37 ns 240.67 ns]
    change: [-1.0492% -0.1759% +0.5088%] (p = 0.71 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1000+1 entries
    time: [218.69 ns 218.84 ns 219.02 ns]
    change: [-5.6412% -1.7086% +0.7100%] (p = 0.52 > 0.05)
    No change in performance detected.

  • RxStreamOrderer::inbound_frame()
    time: [118.91 ms 118.98 ms 119.05 ms]
    change: [-0.7655% -0.5679% -0.4288%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • transfer/Run multiple transfers with varying seeds
    time: [122.82 ms 123.13 ms 123.43 ms]
    thrpt: [32.406 MiB/s 32.486 MiB/s 32.568 MiB/s]
    change:
    time: [+1.5815% +1.9464% +2.3133%] (p = 0.00 < 0.05)
    thrpt: [-2.2610% -1.9092% -1.5569%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [123.01 ms 123.21 ms 123.43 ms]
    thrpt: [32.408 MiB/s 32.464 MiB/s 32.517 MiB/s]
    change:
    time: [+1.6664% +1.9052% +2.1581%] (p = 0.00 < 0.05)
    thrpt: [-2.1125% -1.8696% -1.6391%]
    Change within noise threshold.

  • 1-conn/1-100mb-resp (aka. Download)/client
    time: [1.1182 s 1.1265 s 1.1344 s]
    thrpt: [88.149 MiB/s 88.773 MiB/s 89.426 MiB/s]
    change:
    time: [+1.6146% +2.5454% +3.3059%] (p = 0.00 < 0.05)
    thrpt: [-3.2001% -2.4822% -1.5889%]
    💔 Performance has regressed.

  • 1-conn/10_000-parallel-1b-resp (aka. RPS)/client
    time: [425.12 ms 427.22 ms 429.37 ms]
    thrpt: [23.290 Kelem/s 23.407 Kelem/s 23.523 Kelem/s]
    change:
    time: [-0.8386% -0.1831% +0.4909%] (p = 0.60 > 0.05)
    thrpt: [-0.4886% +0.1834% +0.8457%]
    No change in performance detected.

  • 1-conn/1-1b-resp (aka. HPS)/client
    time: [50.511 ms 50.872 ms 51.241 ms]
    thrpt: [19.516 elem/s 19.657 elem/s 19.798 elem/s]
    change:
    time: [-0.3979% +0.5834% +1.5150%] (p = 0.24 > 0.05)
    thrpt: [-1.4923% -0.5801% +0.3995%]
    No change in performance detected.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 656.7 ± 243.0 385.4 1152.6 1.00
neqo msquic reno on 1114.2 ± 325.5 788.1 1790.2 1.00
neqo msquic reno 868.6 ± 113.3 757.3 1154.1 1.00
neqo msquic cubic on 983.5 ± 203.1 771.0 1360.9 1.00
neqo msquic cubic 940.6 ± 224.6 753.3 1389.5 1.00
msquic neqo reno on 4508.7 ± 272.9 4212.4 4988.5 1.00
msquic neqo reno 4510.8 ± 333.3 4125.1 5103.7 1.00
msquic neqo cubic on 4723.0 ± 346.5 4291.4 5293.0 1.00
msquic neqo cubic 4553.0 ± 215.7 4199.1 4842.5 1.00
neqo neqo reno on 3778.6 ± 430.5 3292.5 4404.0 1.00
neqo neqo reno 3747.8 ± 225.8 3484.9 4149.0 1.00
neqo neqo cubic on 4477.7 ± 538.7 3117.7 4968.8 1.00
neqo neqo cubic 4348.1 ± 224.8 4181.6 4952.3 1.00

⬇️ Download logs

@larseggert
Copy link
Collaborator Author

@KershawChang @martinthomson any chance for a review?

@mxinden mxinden mentioned this pull request Apr 3, 2024
Copy link
Collaborator

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

I don't think the use of unsafe is necessary here.

In case you like the suggestions, you can simply merge #1789.

fuzz/fuzz_targets/packet.rs Outdated Show resolved Hide resolved
fuzz/fuzz_targets/packet.rs Outdated Show resolved Hide resolved
test-fixture/src/sim/rng.rs Outdated Show resolved Hide resolved
fuzz/src/lib.rs Outdated Show resolved Hide resolved
neqo-common/src/fuzz.rs Outdated Show resolved Hide resolved
neqo-crypto/src/p11.rs Outdated Show resolved Hide resolved
neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved
neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved
neqo-common/src/fuzz.rs Outdated Show resolved Hide resolved
larseggert and others added 5 commits April 16, 2024 09:30
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

3766868 has a couple of comments.

I'm on vacation from tomorrow, so I trust you to do the right thing with this. This is generally OK.

(I did question whether it is the right decision to inline the write_to_corpus calls, rather than having specific code to generate that corpus, but this works.) Do you have a script for that?

neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved
neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved
neqo-common/src/lib.rs Outdated Show resolved Hide resolved
larseggert and others added 3 commits April 18, 2024 07:50
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>
@larseggert
Copy link
Collaborator Author

(I did question whether it is the right decision to inline the write_to_corpus calls, rather than having specific code to generate that corpus, but this works.) Do you have a script for that?

A corpus can get generated in a number of ways, it's basically just a collection of files. The fuzzer has commands to manage it (e.g., minify it, etc.), in Rust via cargo fuzz. For quant, I found it convenient to (re-)generate a corpus as a side effect of running tests, I just followed that approach here, too.

@larseggert larseggert merged commit 3797225 into main Apr 18, 2024
17 checks passed
@larseggert larseggert deleted the test-cargo-fuzz branch April 18, 2024 08:51
@larseggert larseggert mentioned this pull request Apr 18, 2024
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.

For fuzzing: we need to disable packet nember protection as well
3 participants