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

[EC] P-256/384/521 s2n-bignum scalar multiplication #2036

Merged
merged 9 commits into from
Dec 13, 2024

Conversation

dkostic
Copy link
Contributor

@dkostic dkostic commented Dec 5, 2024

Issues:

N/A

Description of changes:

For curves P-256/384/521 we use s2n-bignum implementation
of scalar multiplication of an arbitrary point. This gives the following
performance improvements (measurements in ops/s):

__Apple M1__| before |  after | speedup |
P-256 MUL   |  27871 |  31607 |  1.13x  |
P-256 ECDH  |  20804 |  22778 |  1.11x  |
P-384 MUL   |   7245 |   8618 |  1.19x  |
P-384 ECDH  |   5367 |   5986 |  1.11x  |
P-521 MUL   |   5040 |   5806 |  1.15x  |
P-521 ECDH  |   3696 |   4053 |  1.10x  |

____Intel___| before |  after | speedup |
P-256 MUL   |  21913 |  25650 |  1.17x  |
P-256 ECDH  |  17188 |  19453 |  1.13x  |
P-384 MUL   |   6554 |   7691 |  1.17x  |
P-384 ECDH  |   4731 |   5321 |  1.12x  |
P-521 MUL   |   4400 |   5151 |  1.17x  |
P-521 ECDH  |   3192 |   3514 |  1.10x  |

where Apple M1 is a M1 based macbook laptop, and
Intel is Intel(R) Xeon(R) Platinum 8488C.

Call-outs:

N/A

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@dkostic dkostic requested a review from a team as a code owner December 5, 2024 23:43
@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.76%. Comparing base (c21a05c) to head (84f677e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2036      +/-   ##
==========================================
- Coverage   78.77%   78.76%   -0.01%     
==========================================
  Files         598      598              
  Lines      103683   103689       +6     
  Branches    14742    14743       +1     
==========================================
- Hits        81672    81669       -3     
- Misses      21359    21366       +7     
- Partials      652      654       +2     

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

@dkostic dkostic force-pushed the s2n-bignum-scalar-mul branch from c00d023 to 84f677e Compare December 11, 2024 19:30
@dkostic dkostic changed the title [EC] P-256 and P-384 s2n-bignum scalar multiplication [EC] P-256/384/521 s2n-bignum scalar multiplication Dec 11, 2024
@nebeid nebeid self-requested a review December 12, 2024 00:01
@dkostic dkostic enabled auto-merge (squash) December 12, 2024 15:43
Copy link
Contributor

@nebeid nebeid left a comment

Choose a reason for hiding this comment

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

Comments on updates that dkostic will address in another PR as we discussed:

  • p384_felem_mul_scalar_rwnaf is now only mentioned at the bottom of p384.c, but it's now scalar_rwnaf in ec_nistp.c so I wanted to suggest an updated writing there.
  • update the mention of the ECCKiila project and how we defer from them in starting from the most significant digit.

Added point: I think we need to clarify that p256-nistz contains two assembly implementations while p384.c and p521.c contain just one: s2n-bignum and the alternative is Fiat C

@dkostic dkostic merged commit 02ea4c4 into aws:main Dec 13, 2024
121 of 124 checks passed
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.

4 participants