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.

Sign up for GitHub

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

Upstreaming of proxy-wasm Part 1. #10262

Closed
wants to merge 20 commits into from
Closed

Upstreaming of proxy-wasm Part 1. #10262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
972166c
Upstreaming of proxy-wasm Part 1.
jplevyak Mar 5, 2020
8b5efdc
Format.
jplevyak Mar 6, 2020
65bd8ae
Address comments.
jplevyak Mar 6, 2020
b21914e
Address comments.
jplevyak Mar 6, 2020
3204e82
Address comments.
jplevyak Mar 6, 2020
3a25b41
Address comments.
jplevyak Mar 6, 2020
7548daf
Format.
jplevyak Mar 7, 2020
3518e10
Format.
jplevyak Mar 7, 2020
c9bfb2e
Format.
jplevyak Mar 9, 2020
b6f1ce6
Fix formatting.
jplevyak Mar 9, 2020
7832462
Remove local repo reference.
jplevyak Mar 9, 2020
489f8f8
Remove WasmState which is not needed in this PR.
jplevyak Mar 9, 2020
b21857a
Address comments.
jplevyak Mar 9, 2020
bbc7922
Format.
jplevyak Mar 10, 2020
6f29c2a
Move pointer using statement to core.
jplevyak Mar 10, 2020
b3a3072
Merge remote-tracking branch 'envoyproxy/master' into wasm-upstream-base
jplevyak Mar 10, 2020
e85d273
Add Random as required by RemoteAsyncDataProvider.
jplevyak Mar 11, 2020
167c344
Attempt to fix the @envoy dependency problem.
jplevyak Mar 12, 2020
06581c7
Sort alphabetically because that was sooooo important.
jplevyak Mar 12, 2020
4119e6b
Address some coverage.
jplevyak Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions bazel/external/proxy_wasm_cpp_host.BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
licenses(["notice"]) # Apache 2
jplevyak marked this conversation as resolved.
Show resolved Hide resolved

package(default_visibility = ["//visibility:public"])

cc_library(
name = "include",
Copy link
Member

Choose a reason for hiding this comment

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

I think that name = "proxy_wasm_interface" is better.

hdrs = [
"include/proxy-wasm/compat.h",
"include/proxy-wasm/context.h",
"include/proxy-wasm/exports.h",
"include/proxy-wasm/null.h",
"include/proxy-wasm/null_plugin.h",
"include/proxy-wasm/null_vm.h",
"include/proxy-wasm/null_vm_plugin.h",
"include/proxy-wasm/v8.h",
"include/proxy-wasm/wasm.h",
"include/proxy-wasm/wasm_api_impl.h",
"include/proxy-wasm/wasm_vm.h",
"include/proxy-wasm/word.h",
],
copts = ["-std=c++14"],
deps = [
"@proxy_wasm_cpp_sdk//:common_lib",
],
)

cc_library(
name = "lib",
Copy link
Member

Choose a reason for hiding this comment

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

name = "proxy_wasm_lib" may be better.

srcs = [
"src/base64.cc",
"src/base64.h",
"src/context.cc",
"src/exports.cc",
"src/foreign.cc",
"src/null/null.cc",
"src/null/null_plugin.cc",
"src/null/null_vm.cc",
"src/v8/v8.cc",
"src/wasm.cc",
],
copts = ["-std=c++14"],
deps = [
":include",
"//external:abseil_flat_hash_map",
"//external:abseil_optional",
"//external:abseil_strings",
"//external:protobuf",
"//external:wee8",
"//external:zlib",
"@boringssl//:ssl",
"@proxy_wasm_cpp_sdk//:api_lib",
"@proxy_wasm_cpp_sdk//:common_lib",
],
)
16 changes: 16 additions & 0 deletions bazel/external/proxy_wasm_cpp_sdk.BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
licenses(["notice"]) # Apache 2

package(default_visibility = ["//visibility:public"])

cc_library(
name = "api_lib",
hdrs = ["proxy_wasm_api.h"],
)

cc_library(
name = "common_lib",
hdrs = [
"proxy_wasm_common.h",
"proxy_wasm_enums.h",
],
)
14 changes: 14 additions & 0 deletions bazel/repositories.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ def envoy_dependencies(skip_targets = []):
_io_opentracing_cpp()
_net_zlib()
_upb()
_proxy_wasm_cpp_sdk()
_proxy_wasm_cpp_host()
_repository_impl("com_googlesource_code_re2")
_com_google_cel_cpp()
_repository_impl("bazel_toolchains")
Expand Down Expand Up @@ -741,6 +743,18 @@ def _com_github_gperftools_gperftools():
actual = "@envoy//bazel/foreign_cc:gperftools",
)

def _proxy_wasm_cpp_sdk():
_repository_impl(
name = "proxy_wasm_cpp_sdk",
build_file = "@envoy//bazel/external:proxy_wasm_cpp_sdk.BUILD",
)

def _proxy_wasm_cpp_host():
_repository_impl(
name = "proxy_wasm_cpp_host",
build_file = "@envoy//bazel/external:proxy_wasm_cpp_host.BUILD",
)

def _kafka_deps():
# This archive contains Kafka client source code.
# We are using request/response message format files to generate parser code.
Expand Down
10 changes: 10 additions & 0 deletions bazel/repository_locations.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,4 +319,14 @@ REPOSITORY_LOCATIONS = dict(
strip_prefix = "kafka-python-2.0.0",
urls = ["https://github.com/dpkp/kafka-python/archive/2.0.0.tar.gz"],
),
proxy_wasm_cpp_sdk = dict(
sha256 = "14f66f67e8f37ec81d28d7f5307be4407d206ac5f0daaf6d22fa5536797bcac1",
strip_prefix = "proxy-wasm-cpp-sdk-31f1fc5b7e09f231fa532d2d296e479a113c3e10",
urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-sdk/archive/31f1fc5b7e09f231fa532d2d296e479a113c3e10.tar.gz"],
),
proxy_wasm_cpp_host = dict(
sha256 = "4aaf28e2c3326f9f465ffc460162ee195ee3ceaba93241c58453ae57373191fe",
strip_prefix = "proxy-wasm-cpp-host-a9c0c47d8062aa36d955b8604b1b96ee9078253f",
urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/a9c0c47d8062aa36d955b8604b1b96ee9078253f.tar.gz"],
),
)
8 changes: 4 additions & 4 deletions include/abi/wasm/proxy_wasm_exports.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* Called when the VM starts by the first plugin to use the VM.
* @param root_context_id is an identifier for one or more related plugins.
* @param vm_configuration_size is the size of any configuration available via
* proxy_get_configuration during the lifetime of this call.
* proxy_get_buffer_bytes() during the lifetime of this call.
* @return non-zero on success and zero on failure (e.g. bad configuration).
*/
enum class WasmOnVmStartResult : uint32_t {
Expand All @@ -44,7 +44,7 @@ extern "C" WasmOnVmStartResult proxy_on_vm_start(uint32_t root_context_id,
* configuration.
* @param root_context_id is a unique identifier for the configuration verification context.
* @param configuration_size is the size of any configuration available via
* proxy_get_configuration().
* proxy_get_buffer_bytes().
* @return non-zero on success and zero on failure (i.e. bad configuration).
*/
enum class WasmOnValidateConfigurationResult : uint32_t {
Expand All @@ -57,7 +57,7 @@ proxy_validate_configuration(uint32_t root_context_id, uint32_t configuration_si
* Called when a plugin loads or when plugin configuration changes dynamically.
* @param root_context_id is an identifier for one or more related plugins.
* @param plugin_configuration_size is the size of any configuration available via
* proxy_get_configuration().
* proxy_get_buffer_bytes().
* @return non-zero on success and zero on failure (e.g. bad configuration).
*/
enum class WasmOnConfigureResult : uint32_t {
Expand All @@ -73,7 +73,7 @@ extern "C" WasmOnConfigureResult proxy_on_configure(uint32_t root_context_id,
* Called when a request, stream or other ephemeral context is created.
* @param context_id is an identifier of the ephemeral context.
* @param configuration_size is the size of any configuration available via
* proxy_get_configuration().
* proxy_get_buffer_bytes().
*/
extern "C" void proxy_on_context_create(uint32_t context_id, uint32_t root_context_id);

Expand Down
24 changes: 13 additions & 11 deletions include/abi/wasm/proxy_wasm_imports.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,25 @@
// Configuration and Status

/**
* Called from the VM to get any configuration. Valid only when in proxy_on_start() (where it will
* return a VM configuration), proxy_on_configure() (where it will return a plugin configuration) or
* in proxy_validate_configuration() (where it will return a VM configuration before
* proxy_on_start() has been called and a plugin configuration after).
* Called from the VM to get bytes from a buffer (e.g. a vm or plugin configuration).
* @param start is the offset of the first byte to retrieve.
* @param length is the number of the bytes to retrieve. If start + length exceeds the number of
* bytes available then configuration_size will be set to the number of bytes returned.
* @param configuration_ptr a pointer to a location which will be filled with either nullptr (if no
* configuration is available) or a pointer to a allocated block containing the configuration
* @param ptr a pointer to a location which will be filled with either nullptr (if no
* data is available) or a pointer to a allocated block containing the configuration
* bytes.
* @param configuration_size a pointer to a location containing the size (or zero) of any returned
* configuration byte block.
* @return a WasmResult: OK, InvalidMemoryAccess. Note: if OK is returned *configuration_ptr may
* @param size a pointer to a location containing the size (or zero) of any returned
* byte block.
* @return a WasmResult: OK, InvalidArgument, InvalidMemoryAccess. Note: if OK is returned *ptr may
* be nullptr.
*/
extern "C" WasmResult proxy_get_configuration((uint32_t start, uint32_t length,
const char** configuration_ptr, size_t* configuration_size);
enum class WasmBufferType : int32_t {
Copy link
Member

Choose a reason for hiding this comment

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

I think that WasmConfigurationBufferType may be easier to interpret.

VmConfiguration = 0,
PluginConfiguration = 1,
MAX = 2,
};
extern "C" WasmResult proxy_get_buffer_bytes(WasmBufferType type, uint32_t start, uint32_t length,
const char** ptr, size_t* size);

// Logging
//
Expand Down
1 change: 1 addition & 0 deletions source/common/stats/symbol_table_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,7 @@ class StatNameSet {
using StringStatNameMap = absl::flat_hash_map<std::string, Stats::StatName>;
StringStatNameMap builtin_stat_names_;
};
using StatNameSetSharedPtr = std::shared_ptr<Stats::StatNameSet>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect this is not a great pattern for using StatNameSet; I'm inclined not to institutionalize it by centralizing a shortcut for it.

But it's worth exploring what your data model is. Is the StatNameSet shared across all threads, all VMs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The VMs are running the same code and there are many of them (e.g 60 on a large system), so the replication would be large without sharing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

envoyproxy/envoy-wasm#452

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed on envoyproxy/envoy-wasm#452 we have a new solution and are testing this now. I will upstream that change assuming it works out. Let's not let that hold up this review.


} // namespace Stats
} // namespace Envoy
44 changes: 32 additions & 12 deletions source/extensions/common/wasm/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,49 @@ envoy_cc_library(
)

envoy_cc_library(
name = "wasm_vm_interface",
hdrs = ["wasm_vm.h"],
name = "wasm_vm_lib",
srcs = ["wasm_vm.cc"],
deps = [
":well_known_names",
"//source/common/common:minimal_logger_lib",
":wasm_hdr",
"//source/common/common:assert_lib",
"//source/common/stats:stats_lib",
"@proxy_wasm_cpp_host//:lib",
"@proxy_wasm_cpp_sdk//:common_lib",
],
)

# NB: Used to break the circular dependency between wasm_lib and null_plugin_lib.
envoy_cc_library(
name = "wasm_vm_base",
hdrs = ["wasm_vm_base.h"],
name = "wasm_hdr",
hdrs = [
"context.h",
"wasm.h",
"wasm_vm.h",
],
deps = [
":well_known_names",
"//include/envoy/upstream:cluster_manager_interface",
"//source/common/config:datasource_lib",
"//source/common/stats:stats_lib",
"@envoy_api//envoy/extensions/wasm/v3:pkg_cc_proto",
"@proxy_wasm_cpp_host//:include",
"@proxy_wasm_cpp_sdk//:common_lib",
],
)

envoy_cc_library(
name = "wasm_vm_lib",
srcs = ["wasm_vm.cc"],
name = "wasm_lib",
srcs = [
"context.cc",
"wasm.cc",
],
deps = [
":wasm_vm_interface",
"//source/common/common:assert_lib",
"//source/extensions/common/wasm/null:null_lib",
"//source/extensions/common/wasm/v8:v8_lib",
":wasm_hdr",
":wasm_vm_lib",
"//external:abseil_base",
"//external:abseil_node_hash_map",
"//source/common/common:base64_lib",
"//source/common/common:enum_to_int",
"@envoy_api//envoy/extensions/wasm/v3:pkg_cc_proto",
],
)
Loading