Skip to content

Conversation

@cimacmillan
Copy link
Contributor

@cimacmillan cimacmillan commented Sep 8, 2022

Added a group of socket options that I found cURL & rust stdlib uses, as well as some UDP multicast options.

#1336

@cimacmillan cimacmillan force-pushed the cmmacmil/socket_options branch 2 times, most recently from 11c8b4c to bd711a8 Compare September 8, 2022 13:37
@cimacmillan cimacmillan force-pushed the cmmacmil/socket_options branch from bd711a8 to c13ff47 Compare September 8, 2022 13:52

static inline __wasi_errno_t
__wasi_sock_get_reuse_port(__wasi_fd_t fd, int8_t *reuse)
__wasi_sock_get_reuse_port(__wasi_fd_t fd, uint8_t *reuse)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a particular reason about from int8_t to uint8_t

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only reason is that for other functions I had used uint8_t for the enable/disable flag and I thought I would change this for consistency with those other set/get sock opts

__import_name__("sock_set_keep_alive")));

static inline __wasi_errno_t
__wasi_sock_set_keep_alive(__wasi_fd_t fd, uint8_t option)
Copy link
Contributor

Choose a reason for hiding this comment

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

may I ask about possible values of option?

Copy link
Contributor Author

@cimacmillan cimacmillan Sep 9, 2022

Choose a reason for hiding this comment

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

Just 0 and 1 like a bool. Wondering now whether I can just use std bool for the argument here. As it's be clearer what the values can be without a comment. Will have a look

@wenyongh
Copy link
Collaborator

@loganek @cimacmillan We have created the release WAMR-1.0.0, and merged the changes from branch main into branch dev/socket, as there is a conflict in file samples/socket-api/CMakeLists.txt, I did it manually with command line, please don't forget to sync up your local repo with the changes, thanks.

@loganek
Copy link
Contributor

loganek commented Sep 11, 2022

Sure, thanks for letting us know

@cimacmillan cimacmillan force-pushed the cmmacmil/socket_options branch 2 times, most recently from bfa6d70 to 0e84ec3 Compare September 12, 2022 13:25
Copy link
Contributor

@loganek loganek left a comment

Choose a reason for hiding this comment

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

Thanks for the change, overall looks good to me. I left a couple of comments. The major question I have is whether we could simplify some of the get/set functions that are different in BSD sockets and have single function with address family as an argument instead of two separate functions for every getter setter?

@cimacmillan cimacmillan force-pushed the cmmacmil/socket_options branch from 0e84ec3 to 5b80c8f Compare September 14, 2022 15:03
@cimacmillan cimacmillan force-pushed the cmmacmil/socket_options branch from 5b80c8f to 6972aed Compare September 14, 2022 15:16
@lum1n0us
Copy link
Contributor

LGTM

Copy link
Contributor

@loganek loganek left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. @wenyongh can we merge that?

@wenyongh wenyongh merged commit 4de5b52 into bytecodealliance:dev/socket Sep 15, 2022
wenyongh added a commit that referenced this pull request Sep 22, 2022
Implement more socket APIs, refer to #1336 and below PRs:
- Implement wasi_addr_resolve function (#1319)
- Fix socket-api byte order issue when host/network order are the same (#1327)
- Enhance sock_addr_local syscall (#1320)
- Implement sock_addr_remote syscall (#1360)
- Add support for IPv6 in WAMR (#1411)
- Implement ns lookup allowlist (#1420)
- Implement sock_send_to and sock_recv_from system calls (#1457)
- Added http downloader and multicast socket options (#1467)
- Fix `bind()` calls to receive the correct size of `sockaddr` structure (#1490)
- Assert on correct parameters (#1505)
- Copy only received bytes from socket recv buffer into the app buffer (#1497)

Co-authored-by: Marcin Kolny <mkolny@amazon.com>
Co-authored-by: Marcin Kolny <marcin.kolny@gmail.com>
Co-authored-by: Callum Macmillan <callumimacmillan@gmail.com>
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…1467)

Add a group of socket options used by cURL and rust stdlib, as well as some UDP multicast options.
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Implement more socket APIs, refer to bytecodealliance#1336 and below PRs:
- Implement wasi_addr_resolve function (bytecodealliance#1319)
- Fix socket-api byte order issue when host/network order are the same (bytecodealliance#1327)
- Enhance sock_addr_local syscall (bytecodealliance#1320)
- Implement sock_addr_remote syscall (bytecodealliance#1360)
- Add support for IPv6 in WAMR (bytecodealliance#1411)
- Implement ns lookup allowlist (bytecodealliance#1420)
- Implement sock_send_to and sock_recv_from system calls (bytecodealliance#1457)
- Added http downloader and multicast socket options (bytecodealliance#1467)
- Fix `bind()` calls to receive the correct size of `sockaddr` structure (bytecodealliance#1490)
- Assert on correct parameters (bytecodealliance#1505)
- Copy only received bytes from socket recv buffer into the app buffer (bytecodealliance#1497)

Co-authored-by: Marcin Kolny <mkolny@amazon.com>
Co-authored-by: Marcin Kolny <marcin.kolny@gmail.com>
Co-authored-by: Callum Macmillan <callumimacmillan@gmail.com>
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.

4 participants