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

[WIP][Wasi] add Wasi-Socket #3147

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

LFsWang
Copy link
Contributor

@LFsWang LFsWang commented Jan 9, 2024

Wasi threads

framework

  • resource manager
  • access control
    • enable/disable network
    • network allow ip check

network

  • network (resource)
    • instance
    • drop

udp-create-socket

  • create_udp_socket(address-family)

udp

  • udpsocket (resource)

    • instance
    • drop
    • subscribe (poll function, will remove in preview 3)
    • start_bind(address, address-family)
      • check ip is allowed
    • finish_bind(handle)
    • stream(remote-address)
      • check child stream (resource)
      • check remote address allowed
    • local_address()
    • remote_address()
    • AddressFamily()
    • UnicastHopLimit()
    • SetUnicastHopLimit()
    • GetReceiveBufferSize()
    • SetReceiveBufferSize()
    • GetSendBufferSize()
    • SetSendBufferSize()
  • incoming-datagram-stream

    • instance (user can not use this)
    • drop
    • subscribe (poll function, will remove in preview 3)
    • receive(max-results)
  • outgoing-datagram-stream

    • instance (user can not use this)
    • drop
    • subscribe (poll function, will remove in preview 3)
    • check-send()
    • send(list)

tcp-create-socket

  • create_tcp_socket(address-family)

tcp

  • tcp-socket
    • instance
    • drop
    • subscribe (poll function, will remove in preview 3)
    • start-bind
    • finish-bind
    • start-connect
    • finish-connect
    • start-listen
    • finish-listen
    • accept
    • local-address
    • remote-address
    • is_listening
    • address_family
    • set_listen_backlog_size
    • keep-alive-enabled
    • set-keep-alive-enabled
    • keep-alive-idle-time
    • setkeep-alive-idle-time
    • keep-alive-interval
    • set-keep-alive-interval
    • keep-alive-count
    • set-keep-alive-count
    • hop-limit
    • set-hop-limit
    • receive-buffer-size
    • set-receive-buffer-size
    • send-buffer-size
    • set-send-buffer-size
    • shutdown

IpNameLookup

  • resolve-addresses
  • resolve-address-stream
    • instance
    • drop
    • subscribe (poll function, will remove in preview 3)
    • resolve-next-address

Copy link
Member

juntao commented Jan 9, 2024

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Commit acc7e23a2b1b8152ee7433f773790fd825d8c07a

The pull request "[WIP][Wasi] add Wasi-Socket" adds socket support to the Wasi (WebAssembly System Interface) in the WasmEdge project. The following are the major changes:

New headers files were added under the include/host/preview2/wasi-sockets/ directory, including api.h, base.h, env.h, etc. Each of these headers declares classes or types with corresponding functions for managing sockets.

New classes are implemented in 5 new source files under lib/host/preview2/wasi-sockets/ directory, including inode-linux.cpp, ipnamelookup.cpp, tcp.cpp, udp.cpp, and util.cpp. These classes handle udp/tcp sockets and utility functions.

There's an addition to the CMakeLists.txt file to handle the build configuration for the new code.

The critical file api.h provides robust error handling for sockets operations like access denied, invalid argument, operation timeout, concurrency conflict, etc. It also defines enums for Address family i.e., IPv4 and IPv6 and types for DGRAM and STREAM.

The env.h header file indicates an environment class WasiSocketsEnvironment has been added, which extends the WASI::Environ class.

The terms TCP, UDP, and IPNameLookup imply there is support for both UDP and TCP type sockets, and potential DNS resolution functionality.

Potential Issues:

  1. This is a Work-In-Progress (WiP) and the code may not be ready for a production environment.

  2. It's unclear whether there are corresponding unit tests to validate this new functionality.

  3. Without comments in the code, it can be challenging to understand the purpose and functionality of some sections of code.

  4. There might be potential issues with multi-threading as the resource table implementation is not thread-safe.

  5. It's unclear how this integration has an impact on the overall performance of the software.

  6. The PR does not delete any existing code which is a good sign. However, it might lead to breaks in other parts of the software that might use the previous implementation.

@github-actions github-actions bot added the c-WASI label Jan 9, 2024
@LFsWang LFsWang force-pushed the wasi-socket branch 3 times, most recently from fb72ae7 to 2cce340 Compare January 22, 2024 14:37
@LFsWang LFsWang force-pushed the wasi-socket branch 5 times, most recently from e21da8b to fa8919f Compare February 21, 2024 08:31
@LFsWang LFsWang force-pushed the wasi-socket branch 4 times, most recently from cea1632 to ef09b1e Compare February 26, 2024 14:09
Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 1419 lines in your changes are missing coverage. Please review.

Project coverage is 73.21%. Comparing base (26de10d) to head (acc7e23).
Report is 3 commits behind head on master.

Files Patch % Lines
lib/host/preview2/wasi-sockets/tcp.cpp 0.00% 486 Missing ⚠️
lib/host/preview2/wasi-sockets/udp.cpp 0.00% 289 Missing ⚠️
lib/host/wasi/inode-linux.cpp 0.00% 269 Missing ⚠️
include/host/wasi/environ.h 0.00% 127 Missing ⚠️
include/host/preview2/wasi-sockets/util.h 0.00% 55 Missing ⚠️
include/host/wasi/vinode.h 0.00% 46 Missing ⚠️
lib/host/preview2/wasi-sockets/ipnamelookup.cpp 0.00% 44 Missing ⚠️
lib/host/preview2/wasi-sockets/util.cpp 0.00% 42 Missing ⚠️
...nclude/host/preview2/wasi-sockets/resource_table.h 0.00% 30 Missing ⚠️
include/host/preview2/wasi-sockets/env.h 0.00% 22 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3147      +/-   ##
==========================================
- Coverage   77.31%   73.21%   -4.11%     
==========================================
  Files         185      192       +7     
  Lines       25599    27018    +1419     
  Branches     5639     5999     +360     
==========================================
- Hits        19791    19780      -11     
- Misses       4382     5803    +1421     
- Partials     1426     1435       +9     

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

Signed-off-by: Sylveon <sylveon@secondstate.io>
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.

None yet

2 participants