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

wasm: Add WAMR support #16057

Merged
merged 25 commits into from
May 31, 2021
Merged

wasm: Add WAMR support #16057

merged 25 commits into from
May 31, 2021

Conversation

leyao-daily
Copy link
Member

Commit Message: Enabling WAMR(Webassembly Micro Runtime) as wasm runtime in Envoy
Additional Description:

WebAssembly Micro Runtime (WAMR) is a standalone WebAssembly (WASM) runtime with a small footprint. It includes a few parts as below:

The "iwasm" VM core, supporting WebAssembly interpreter, ahead of time compilation (AoT) and Just-in-Time compilation (JIT)

The application framework and the supporting API's for the WASM applications

The dynamic management of the WASM applications

Risk Level: Medium
Testing: Runtime unit testing, integration testing and manual testing

...
//test/integration:http2_flood_integration_test                          PASSED in 13.5s
  Stats over 4 runs: max = 13.5s, min = 11.3s, avg = 12.7s, dev = 0.8s
//test/integration:multiplexed_integration_test                          PASSED in 19.1s
  Stats over 4 runs: max = 19.1s, min = 13.5s, avg = 16.5s, dev = 2.0s
//test/common/http/http2:codec_impl_test                                 PASSED in 4.9s
  Stats over 5 runs: max = 4.9s, min = 3.6s, avg = 4.2s, dev = 0.5s
//test/integration:protocol_integration_test                             PASSED in 40.3s
  Stats over 5 runs: max = 40.3s, min = 32.0s, avg = 35.4s, dev = 2.9s
//test/integration:quic_http_integration_test                            PASSED in 13.0s
  Stats over 6 runs: max = 13.0s, min = 9.6s, avg = 10.7s, dev = 1.2s
//test/integration:quic_protocol_integration_test                        PASSED in 40.3s
  Stats over 8 runs: max = 40.3s, min = 23.5s, avg = 29.4s, dev = 5.4s
//test/extensions/filters/http/wasm:config_test                          PASSED in 3.2s
  Stats over 50 runs: max = 3.2s, min = 0.4s, avg = 0.7s, dev = 0.6s
//test/extensions/filters/http/wasm:wasm_filter_test                     PASSED in 6.2s
  Stats over 50 runs: max = 6.2s, min = 0.9s, avg = 3.5s, dev = 1.2s

Executed 855 out of 855 tests: 855 tests pass.

Docs Changes: N/A
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link

Hi @leyao-daily, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #16057 was opened by leyao-daily.

see: more, trace.

@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #16057 was opened by leyao-daily.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Apr 19, 2021
bazel/external/wamr.BUILD Outdated Show resolved Hide resolved
Copy link
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. You're still missing:

test/extensions/bootstrap/wasm/wasm_speed_test.cc Outdated Show resolved Hide resolved
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #16057 was synchronize by leyao-daily.

see: more, trace.

@@ -704,6 +704,19 @@ REPOSITORY_LOCATIONS_SPEC = dict(
extensions = ["envoy.wasm.runtime.wavm"],
cpe = "cpe:2.3:a:webassembly_virtual_machine_project:webassembly_virtual_machine:*",
),
com_github_wamr = dict(
project_name = "Webassembly Micro Runtime",
Copy link
Member

Choose a reason for hiding this comment

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

@PiotrSikora what's our end game here? Will we eventually bring every Wasm runtime into the Envoy OSS build? Do we need this or can we be opinionated?

My main worry here is the increase in build time in CI, security risk envelope, etc.

CC @envoyproxy/dependency-shepherds

Copy link
Contributor

Choose a reason for hiding this comment

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

Those are good questions.

From Proxy-Wasm C++ Host's perspective, supporting any decent Wasm runtime is desireable, since they all have a bit different features, and most implement variants of Wasm C/C++ API, so both implementation and maintanance have relatively small cost.

From Envoy's perspective, it's definitely not neccessary, and we could be opinionated, but since those are optional alternatives, I don't think there are too many downsides, especially since the common logic is abstracted away.

Regarding build time - Wasm runtimes are updated once a month or once a quarter, and the alternative ones are built only in bazel.compile_time_options target, so their artifacts should be pretty much always cached, and the overhead shouldn't be much of an issue.

Regarding security risk - we could officially support only the default Wasm runtime (currently: V8), and be upfront that there won't be any security releases for other runtimes.

Regarding WAMR specifically, it has interpreted mode, and claims very small binary size overhead and low memory usage, so it might be more desirable in Envoy Mobile than V8, for example.

Copy link
Member

Choose a reason for hiding this comment

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

OK, can we add to this PR a change to threat model https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/security/threat_model#core-and-extensions stating this explicitly?

I think this is a pretty important thing to get clear as we add more runtimes; we won't support anything other than V8 for now from a security perspective.

name = "config",
srcs = ["config.cc"],
category = "envoy.wasm.runtime",
security_posture = "unknown",
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment that "This may never change from unknown until the threat model at https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/security/threat_model#core-and-extensions is updated to capture additional Wasm runtimes". This should also be done for all other Wasm runtimes other than V8.

Copy link
Member Author

Choose a reason for hiding this comment

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

So add an annotation above the unknown line to explain the meaning?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Could you add that comment here and to Wasmtime and WAVM?

Copy link
Contributor

Choose a reason for hiding this comment

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

Friendly ping.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still needed.

@leyao-daily
Copy link
Member Author

The PR is waiting for #16112 merging.

@PiotrSikora
Copy link
Contributor

The PR is waiting for #16112 merging.

The fix for removed getCustomSection() was merged in #16162, so you should be able to unblock this PR when you update your PR in Proxy-Wasm C++ Host repo, and merge with main branch here.

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label May 12, 2021
@leyao-daily
Copy link
Member Author

About the API change, what should I do to pass the check?

@PiotrSikora
Copy link
Contributor

About the API change, what should I do to pass the check?

Wait for one of the @envoyproxy/api-shepherds to approve.

@adisuissa
Copy link
Contributor

/lgtm api

@PiotrSikora
Copy link
Contributor

@leyao-daily this needs merge with master to resolve conflicts.

@htuch @lizan could one of you please take a look at this PR?

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label May 20, 2021
lum1n0us and others added 3 commits May 20, 2021 14:10
- generate llvm libraries list with "llvm-config --libnames"
```
$ llvm-config --version
10.0.0

$ llvm-config --libnames asmparser core debuginfodwarf engine lto
mcparser mirparser orcjit passes runtimedyld support x86asmparser
x86desc
```
- change WAMR default mode to JIT.

- change to latest bytecodealliance/wasm-micro-runtime commits

Signed-off-by: Liang He <[email protected]>
@htuch
Copy link
Member

htuch commented May 21, 2021

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label May 21, 2021
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label May 28, 2021
@leyao-daily
Copy link
Member Author

Once the CI/CD pass, I think this PR is ready to be merged.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label May 31, 2021
@htuch htuch merged commit a7fa331 into envoyproxy:main May 31, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
WebAssembly Micro Runtime (WAMR) is a standalone WebAssembly (WASM) runtime with a small footprint. It includes a few parts as below:

The "iwasm" VM core, supporting WebAssembly interpreter, ahead of time compilation (AoT) and Just-in-Time compilation (JIT)

The application framework and the supporting API's for the WASM applications

The dynamic management of the WASM applications

Risk Level: Medium
Testing: Runtime unit testing, integration testing and manual testing

Signed-off-by: Le Yao <[email protected]>
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.

9 participants