Skip to content

Support recvmsg() and sendmsg() #1042

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

Jump to bottom
Merged
wenyongh merged 2 commits into from
Mar 30, 2022
Merged

Support recvmsg() and sendmsg() #1042

wenyongh merged 2 commits into from
Mar 30, 2022

Conversation

@lum1n0us
Copy link
Contributor

@lum1n0us lum1n0us commented Mar 10, 2022

Add an example, send_recv.c, to test with new socket APIs. Before testing, please uncomment lines in CMakeLists.txt

@lum1n0us lum1n0us mentioned this pull request Mar 10, 2022
Closed
@lum1n0us
Copy link
Contributor Author

lum1n0us commented Mar 10, 2022

@wenyongh what kind of copyright declaration should I add into the example ?

@wenyongh wenyongh changed the base branch from dev/socket_api to main March 10, 2022 07:14
@lum1n0us
Copy link
Contributor Author

lum1n0us commented Mar 10, 2022

@wenyongh here is the question, the new API socketpair()

On Linux, the only supported domains for this call are AF_UNIX (or synonymously, AF_LOCAL) and AF_TIPC (since Linux 4.12)

And WAMR socket supporting only covers AF_INET, which means either we expand the socket address family or we reject the requirement.

BTW, the example send_recv.c will be failed in linux:

$ clang-13 -o send_recv ../wasm-src/send_recv.c
$ ./send_recv
socketpair err: Operation not supported

@lum1n0us lum1n0us force-pushed the a_new_socket_example branch from c8cf07a to 9fb4430 Compare March 10, 2022 07:48
@wenyongh
Copy link
Collaborator

wenyongh commented Mar 10, 2022

@wenyongh here is the question, the new API socketpair()

On Linux, the only supported domains for this call are AF_UNIX (or synonymously, AF_LOCAL) and AF_TIPC (since Linux 4.12)

And WAMR socket supporting only covers AF_INET, which means either we expand the socket address family or we reject the requirement.

BTW, the example send_recv.c will be failed in linux:

$ clang-13 -o send_recv ../wasm-src/send_recv.c
$ ./send_recv
socketpair err: Operation not supported

Please change AF_INET to AF_LOCAL.

@lum1n0us lum1n0us force-pushed the a_new_socket_example branch from 9fb4430 to 20e7dee Compare March 11, 2022 02:03
@lum1n0us
Copy link
Contributor Author

lum1n0us commented Mar 11, 2022 •
edited
Loading

In that case (socketpair() and AF_UNIX), we are looking at questions and they are all connected with rights control.

  • socketpair() creates two unnamed sockets. It required a new way to announce "unnamed socket are allowed" since neither --addr-pool nor --dir covers it now
  • because socketpair() brings AF_UNIX into WAMR socket APIs supporting range, are we going to expand AF_UNIX to socket() also? It should be OK to use '--dir' to cover "pathname sockets". (because pathname includes files and dirs, it's better to rename --dir to --path). Just need to figure out two options to cover unnamed sockets (created by socketpair()) and abstract sockets.

Reference:
AF_UNIX
socketpair

@lum1n0us
Copy link
Contributor Author

lum1n0us commented Mar 13, 2022

For the record, we plan to bypass the requirement of socketpair and only support recvmsg() and sendmsg().

@lum1n0us lum1n0us force-pushed the a_new_socket_example branch 3 times, most recently from a77fc63 to 1156129 Compare March 25, 2022 01:10
@lum1n0us lum1n0us changed the title [WIP] A new socket example Support recvmsg() and sendmsg() Mar 25, 2022
wenyongh
wenyongh reviewed Mar 25, 2022
View reviewed changes
core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c Outdated
Comment on lines 155 to 157
__wasi_iovec_t ri_data = { .buf = msg->msg_iov->iov_base,
.buf_len = msg->msg_iov->iov_len };
size_t ri_data_len = msg->msg_iovlen;
Copy link
Collaborator

@wenyongh wenyongh Mar 25, 2022

Choose a reason for hiding this comment

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

  1. Should check whether msg is NULL?
  2. Had better not initialize ri_data with non-const expression
    How about:
__wasi_iovec_t ri_data = { 0 };
size_t ri_data_len;

if (!msg) report error;
ri_data.buf = ..;
ri_data.buf_len = ..;
ri_data_len = ..;

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c Outdated
Comment on lines 178 to 180
__wasi_ciovec_t si_data = { .buf = msg->msg_iov->iov_base,
.buf_len = msg->msg_iov->iov_len };
size_t si_data_len = msg->msg_iovlen;
Copy link
Collaborator

@wenyongh wenyongh Mar 25, 2022

Choose a reason for hiding this comment

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

Same as above

@lum1n0us lum1n0us force-pushed the a_new_socket_example branch from 1156129 to 31d97cc Compare March 28, 2022 04:23
wenyongh
wenyongh reviewed Mar 28, 2022
View reviewed changes
samples/socket-api/README.md Outdated
```

`send_recv.wasm` contains a thread as a server and a thread as a client. They
sends and receives data via 127.0.0.1:1234.
Copy link
Collaborator

@wenyongh wenyongh Mar 28, 2022

Choose a reason for hiding this comment

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

send and receive

samples/socket-api/send_recv.dump Outdated
@@ -0,0 +1,16350 @@

Copy link
Collaborator

@wenyongh wenyongh Mar 28, 2022

Choose a reason for hiding this comment

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

Remove this file?

samples/socket-api/wasm-src/send_recv.c Outdated

if (pthread_cond_init(&cond, NULL)) {
perror("Initialize condition failed");
return EXIT_FAILURE;
Copy link
Collaborator

@wenyongh wenyongh Mar 28, 2022

Choose a reason for hiding this comment

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

Shall we pthread_mutex_destroy(&lock) before return?

samples/socket-api/wasm-src/send_recv.c Outdated

if (pthread_create(&cs[0], NULL, run_as_server, NULL)) {
perror("Create a server thread failed");
return EXIT_FAILURE;
Copy link
Collaborator

@wenyongh wenyongh Mar 28, 2022

Choose a reason for hiding this comment

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

Same as above, destroy cond and mutex before return?

@lum1n0us lum1n0us force-pushed the a_new_socket_example branch from 31d97cc to 041d43f Compare March 30, 2022 02:32
wenyongh
wenyongh reviewed Mar 30, 2022
View reviewed changes
@lum1n0us lum1n0us force-pushed the a_new_socket_example branch from 041d43f to d5fd6d6 Compare March 30, 2022 03:11
@wenyongh wenyongh merged commit 559a050 into bytecodealliance:main Mar 30, 2022
@lum1n0us lum1n0us deleted the a_new_socket_example branch May 10, 2022 14:41
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
@lum1n0us
Enable socket-api recvmsg() and sendmsg() (bytecodealliance#1042)
a9b7a94 
Implement socket-api `recvmsg()` and `sendmsg()` for wasm app,
add sample and update document.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@wenyongh wenyongh wenyongh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lum1n0us @wenyongh