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

Add external Wasm modules #1232

Merged
merged 2 commits into from
Jul 7, 2020
Merged

Conversation

ipetr0v
Copy link
Contributor

@ipetr0v ipetr0v commented Jul 3, 2020

This change:

  • Adds the ability to load external Wasm modules with oak_config_serializer
  • Makes TIR example use an external precompiled Wasm module

Ref #1183

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have checked that these tests are run by Cloudbuild

@ipetr0v ipetr0v marked this pull request as ready for review July 6, 2020 13:48
@ipetr0v ipetr0v requested a review from tiziano88 July 6, 2020 13:48
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
.context("Couldn't parse URL")?;

debug!("Downloading module from: {}", url);
let response = reqwest::get(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess at some point we need to have a cache of modules, since this will re-download them every time, perhaps add a TODO + issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an issue #1240

sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
sdk/rust/oak_config_serializer/src/main.rs Outdated Show resolved Hide resolved
@ipetr0v ipetr0v requested a review from tiziano88 July 7, 2020 13:41
Fix URL

Fix hash

Change after review

Fix Cargo.lock

Change after review

Change after review
Copy link
Collaborator

@tiziano88 tiziano88 left a comment

Choose a reason for hiding this comment

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

neat

fn get_sha256(data: &[u8]) -> String {
let mut hasher = Sha256::new();
hasher.update(data);
hex::encode(hasher.finalize().as_slice().to_vec())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this works

Suggested change
hex::encode(hasher.finalize().as_slice().to_vec())
hex::encode(hasher.finalize().collect())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finalize returns a GenericArray and it doesn't have collect.

sdk/rust/oak_config_serializer/src/main.rs Show resolved Hide resolved

debug!("Downloading module from: {}", url);
// TODO(#1240): Add a Wasm module cache.
let response = reqwest::get(url.clone())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why can url not be passed by reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used in .with_context(|| format!("Couldn't download module from {}", url))?; that moves it in the context of lambda.

@codecov-commenter
Copy link

Codecov Report

Merging #1232 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1232   +/-   ##
=======================================
  Coverage   31.39%   31.39%           
=======================================
  Files          61       61           
  Lines        7521     7521           
  Branches     3542     3542           
=======================================
  Hits         2361     2361           
  Misses       2929     2929           
  Partials     2231     2231           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a35998c...f13bd03. Read the comment docs.

@ipetr0v ipetr0v merged commit 9c82168 into project-oak:main Jul 7, 2020
@ipetr0v ipetr0v deleted the external_modules branch July 7, 2020 16:59
@github-actions
Copy link

github-actions bot commented Jul 7, 2020

Reproducibility Index:

27b563913abba69ab96699ca4576a5406231800739297d43f14647dfe28182d3  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
aaa701e9c0fc58fd65e5956bf07dbc17a9fe30170d776f8c734e833ecc30718e  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
4b88fc85c633e5413002b05ff66bec9ec653397f07f6799f889aeb7af14fb6e1  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
1e8c5a8ff13e83e228fa461eb049e7eb37600b980ef17e1e42aed2b9b14c8cc1  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
400c13da142185bb700bf6daaed1de58f38a6ebc98d52a67a0c93f425a806c77  ./examples/target/wasm32-unknown-unknown/release/database_proxy.wasm
9035e13788e682a01e22c1ba42c125fccbfae4df296d7bd46318fb56044e19a2  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
c5646c404e5d4f4d354f1961e39bc9ca23989922741efee90b9f270625bdcd29  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
3267614a8c860eef6449c40856cef55b04aceb95a206fe9d3cd70d2b3931d967  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
656e89a34c1d964fd003e7421d204f96fd5c3853d27b142a9eec619f6f86f96b  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
aea5aa8876b8158f0d781665e089307e9de53b59104407e71422a58ed4cff6d2  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
080c53ca4491dbd9e279725cc32abddca83d0da14e645915981455b5b9f8ecd2  ./examples/target/wasm32-unknown-unknown/release/trusted_information_retrieval.wasm
f7e04fec862016c4c4dfdb816062cd5227cf8325b3340e5cdbf7d29ff7d5a714  ./oak/server/target/x86_64-unknown-linux-musl/release/oak_loader

Reproducibility Index diff:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants