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

md5_mb (multibuffer) using isa-l_crypto #6037

Merged
merged 2 commits into from
Jun 4, 2020
Merged

Conversation

guymguym
Copy link
Member

@guymguym guymguym commented May 27, 2020

Explain the changes

All mains now import src/util/fips module on startup which auto detects if running in fips mode based on /proc/crypto/fips. This can also be forced using the FIPS=1 env.

In fips mode in order to allow md5 for non cryptographic use cases needed for the S3 protocol, we use md5_mb from isa-l_crypto. Since crypto.createHash is called from our code and also node modules, we monkey-patch the crypto library and replace the createHash('md5'), and also use it natively in the upload pipeline (ChunkSplitter).

NOTE: Using md5_mb with a single stream provides lower performance compared to openssl MD5. See this issue on single buffer performance - intel/isa-l_crypto#45. We try to leverage the multibuffer capability by sharing a thread_local ctx mgr and deferring the flushing of contexes so that if multiple streams are processed concurrently it will be flushed together. The single CPU core performance gain can be very significant - see the performance report here - https://github.com/intel/isa-l/wiki/Documentation.

Issues: Fixed #xxx / Gap #xxx

  1. NA

Testing Instructions:

  1. Run on FIPS enabled host.

@@ -0,0 +1,34 @@
FROM centos:8
Copy link
Contributor

@liranmauda liranmauda May 27, 2020

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I am ignoring this for now. Will get back to it later.

src/tools/coding_speed.js Outdated Show resolved Hide resolved
@guymguym guymguym force-pushed the guy-md5 branch 2 times, most recently from 13923c0 to 6baa16a Compare June 3, 2020 03:16
@guymguym guymguym changed the title MD5_MULTIBUFFER mode using isa-l_crypto md5_mb (multibuffer) using isa-l_crypto Jun 3, 2020
@nimrod-becker
Copy link
Contributor

Don't forget

const ZERO_SIZE_ETAG = 'd41d8cd98f00b204e9800998ecf8427e';

in object_server

@guymguym
Copy link
Member Author

guymguym commented Jun 3, 2020

Don't forget

const ZERO_SIZE_ETAG = 'd41d8cd98f00b204e9800998ecf8427e';

in object_server

@nimrod-becker no need anymore, monkey-patching works for that as well.

TypeError: Cannot read property 'unwrap' of undefined
Co-Authored-By: liranmauda <[email protected]>
@guymguym guymguym merged commit 4c87f61 into noobaa:master Jun 4, 2020
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.

3 participants