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

ci: Try to use NSS package on Linux #1900

Merged
merged 39 commits into from
Jul 17, 2024
Merged

Conversation

larseggert
Copy link
Collaborator

@larseggert larseggert commented May 13, 2024

This should speed up CI.

Which is still being rolled out, but has a bunch of advantages for us.
@larseggert larseggert changed the title Ci ubuntu 24.04 May 13, 2024
@larseggert
Copy link
Collaborator Author

Runners are ready, but actionlint needs an update.

Copy link

github-actions bot commented May 21, 2024

Firefox builds for this PR

The following builds are available for testing. Crossed-out builds did not succeed.

Copy link

github-actions bot commented Jun 17, 2024

Failed Interop Tests

QUIC Interop Runner, client vs. server

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

@larseggert
Copy link
Collaborator Author

@martinthomson
Copy link
Member

No. No idea at all. That failure is pretty troubling. If that were to happen in practice, it would mean a complete failure to encrypt, without hitting the error path.

Looking at the function that fails, we have a completely fresh HpKey instance that is successfully created. If something went wrong there, that's not likely to result in that sort of error, so we'll skip that.

The failure has to be in the hp.mask() call. That function takes a 16 byte buffer of zeros and encrypts it using ChaCha20. What we're seeing here is a complete failure to do anything to those zero values, but the function is returning OK. The only way I can see that happening is if NSS is somehow returning OK. Maybe someone on the crypto team would be able to help there. I can't spot the broken link.

@larseggert
Copy link
Collaborator Author

@martinthomson so this is not an ubuntu-24.04 issue. ubuntu-22.04 aka ubuntu-latest aka focal also has nss-3.98 available via apt now and hits the same issue.

I don't understand why I cant reproduce this locally in either a VM or in docker.

@larseggert larseggert changed the title ci: Bump Linux CI runners to ubuntu-24.04 Jul 12, 2024
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.97%. Comparing base (80c7969) to head (ca693d6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1900      +/-   ##
==========================================
- Coverage   94.97%   94.97%   -0.01%     
==========================================
  Files         112      112              
  Lines       36504    36504              
==========================================
- Hits        34670    34668       -2     
- Misses       1834     1836       +2     

☔ 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 July 12, 2024 13:36
Copy link

Benchmark results

Performance differences relative to 80c7969.

coalesce_acked_from_zero 1+1 entries: Change within noise threshold.
       time:   [191.73 ns 192.23 ns 192.75 ns]
       change: [-1.4693% -1.0117% -0.5614%] (p = 0.00 < 0.05)

Found 18 outliers among 100 measurements (18.00%)
10 (10.00%) high mild
8 (8.00%) high severe

coalesce_acked_from_zero 3+1 entries: Change within noise threshold.
       time:   [233.76 ns 234.32 ns 234.94 ns]
       change: [-1.0631% -0.6036% -0.1008%] (p = 0.01 < 0.05)

Found 13 outliers among 100 measurements (13.00%)
13 (13.00%) high severe

coalesce_acked_from_zero 10+1 entries: No change in performance detected.
       time:   [234.51 ns 237.92 ns 244.24 ns]
       change: [-0.7551% +0.7019% +3.0849%] (p = 0.66 > 0.05)

Found 13 outliers among 100 measurements (13.00%)
1 (1.00%) high mild
12 (12.00%) high severe

coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [216.96 ns 217.13 ns 217.33 ns]
       change: [-0.4507% +0.1516% +0.6982%] (p = 0.65 > 0.05)

Found 13 outliers among 100 measurements (13.00%)
5 (5.00%) high mild
8 (8.00%) high severe

RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [119.45 ms 119.61 ms 119.85 ms]
       change: [-1.0366% -0.7954% -0.5526%] (p = 0.00 < 0.05)

Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) high mild
2 (2.00%) high severe

transfer/Run multiple transfers with varying seeds: No change in performance detected.
       time:   [56.196 ms 59.348 ms 62.512 ms]
       thrpt:  [63.987 MiB/s 67.399 MiB/s 71.179 MiB/s]
change:
       time:   [-0.5491% +7.2542% +15.616%] (p = 0.07 > 0.05)
       thrpt:  [-13.507% -6.7636% +0.5522%]
transfer/Run multiple transfers with the same seed: No change in performance detected.
       time:   [61.263 ms 68.012 ms 74.651 ms]
       thrpt:  [53.583 MiB/s 58.813 MiB/s 65.293 MiB/s]
change:
       time:   [-16.242% -4.0743% +10.491%] (p = 0.55 > 0.05)
       thrpt:  [-9.4947% +4.2474% +19.391%]
1-conn/1-100mb-resp (aka. Download)/client: No change in performance detected.
       time:   [272.45 ms 279.05 ms 285.40 ms]
       thrpt:  [350.39 MiB/s 358.36 MiB/s 367.03 MiB/s]
change:
       time:   [-5.2245% -2.5643% -0.0756%] (p = 0.09 > 0.05)
       thrpt:  [+0.0756% +2.6318% +5.5125%]
1-conn/10_000-parallel-1b-resp (aka. RPS)/client: No change in performance detected.
       time:   [443.69 ms 446.97 ms 450.28 ms]
       thrpt:  [22.208 Kelem/s 22.373 Kelem/s 22.538 Kelem/s]
change:
       time:   [-0.0523% +1.0470% +2.1343%] (p = 0.06 > 0.05)
       thrpt:  [-2.0897% -1.0362% +0.0523%]
1-conn/1-1b-resp (aka. HPS)/client: No change in performance detected.
       time:   [68.348 ms 68.710 ms 69.121 ms]
       thrpt:  [14.467  elem/s 14.554  elem/s 14.631  elem/s]
change:
       time:   [-1.0226% -0.2098% +0.5982%] (p = 0.62 > 0.05)
       thrpt:  [-0.5946% +0.2102% +1.0332%]

Found 9 outliers among 100 measurements (9.00%)
1 (1.00%) high mild
8 (8.00%) high severe

Client/server transfer results

Transfer of 33554432 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 197.9 ± 131.9 97.8 651.4 1.00
neqo msquic reno on 339.8 ± 95.2 258.9 497.6 1.00
neqo msquic reno 300.0 ± 43.5 262.6 386.7 1.00
neqo msquic cubic on 289.4 ± 56.2 261.5 447.8 1.00
neqo msquic cubic 296.7 ± 44.9 259.5 385.4 1.00
msquic neqo reno on 194.8 ± 82.5 115.2 393.5 1.00
msquic neqo reno 204.4 ± 88.4 111.7 405.6 1.00
msquic neqo cubic on 562.6 ± 1709.4 137.8 7411.3 1.00
msquic neqo cubic 215.2 ± 95.4 118.2 416.3 1.00
neqo neqo reno on 232.6 ± 87.5 153.9 435.4 1.00
neqo neqo reno 215.2 ± 72.4 143.3 381.2 1.00
neqo neqo cubic on 232.7 ± 79.1 159.8 407.9 1.00
neqo neqo cubic 269.8 ± 111.3 172.3 530.4 1.00

⬇️ Download logs

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.

For what my review is worth on an NSS related change, this looks good to me.

@larseggert larseggert disabled auto-merge July 17, 2024 09:24
@larseggert larseggert merged commit 22ca6f7 into mozilla:main Jul 17, 2024
57 checks passed
@larseggert larseggert deleted the ci-ubuntu-24.04 branch July 17, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants