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

tinyemu: Slirp Socket support on WAMR #64

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

fredldotme
Copy link

@fredldotme fredldotme commented Jun 8, 2023

The Wasm Micro Runtime ships with an Apache 2.0 (with LLVM Exception) library which enables use of sockets in Wasm binaries. Integrating this library and fixing fallout issues enables networking support at least on WAMR.

@fredldotme
Copy link
Author

So as far as I can tell enabling SLIRP would result in the guest using VIRTIO_NET for networking. The module is actually loaded on my custom-built image but not used, at least it doesn't expose a networking interface.

@fredldotme fredldotme marked this pull request as draft June 8, 2023 20:03
Copy link
Owner

@ktock ktock left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines -70 to +76
EMU_OBJS+=$(addprefix slirp/, bootp.o ip_icmp.o mbuf.o slirp.o tcp_output.o cksum.o ip_input.o misc.o socket.o tcp_subr.o udp.o if.o ip_output.o sbuf.o tcp_input.o tcp_timer.o)
EMU_OBJS+=$(addprefix slirp/, bootp.o ip_icmp.o mbuf.o slirp.o tcp_output.o cksum.o ip_input.o misc.o socket.o tcp_subr.o udp.o if.o ip_output.o sbuf.o tcp_input.o tcp_timer.o wasi_socket_ext.o)
Copy link
Owner

Choose a reason for hiding this comment

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

You might also need to specify CONFIG_SLIRP=y to make command in Dockerfile.

container2wasm/Dockerfile

Lines 271 to 273 in bc5b068

RUN make -j $(nproc) -f Makefile \
CONFIG_FS_NET= CONFIG_SDL= CONFIG_INT128= CONFIG_X86EMU= CONFIG_SLIRP= OUTPUT_NAME=$JS_OUTPUT_NAME \
CC="emcc --preload-file /pack -s WASM=1 -s ASYNCIFY=1 -s ALLOW_MEMORY_GROWTH=1 -UEMSCRIPTEN -DON_BROWSER -sNO_EXIT_RUNTIME=1 -sFORCE_FILESYSTEM=1" && \

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Comment on lines +250 to +252
#else
#include "wasi_socket_ext.h"
#endif
Copy link
Owner

Choose a reason for hiding this comment

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

Are these socket-related WASM APIs wamr-specific? If so, it would be great if we can enable this slirp feature only when the user specify -net=wamr to the runtime flag.

Copy link
Author

Choose a reason for hiding this comment

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

I suppose it's c2w's .go that should generate a Dockerfile argument?

Copy link
Author

Choose a reason for hiding this comment

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

I've done a boolean flag for now since CONFIG_SLIRP also is just a boolean.

Comment on lines -70 to +76
EMU_OBJS+=$(addprefix slirp/, bootp.o ip_icmp.o mbuf.o slirp.o tcp_output.o cksum.o ip_input.o misc.o socket.o tcp_subr.o udp.o if.o ip_output.o sbuf.o tcp_input.o tcp_timer.o)
EMU_OBJS+=$(addprefix slirp/, bootp.o ip_icmp.o mbuf.o slirp.o tcp_output.o cksum.o ip_input.o misc.o socket.o tcp_subr.o udp.o if.o ip_output.o sbuf.o tcp_input.o tcp_timer.o wasi_socket_ext.o)
Copy link
Owner

Choose a reason for hiding this comment

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

As of now, it's fine that the guest VM and the container share the network namespace to make the network interface visible to the container. You can change the runc spec to do so:

s, err := ctdoci.GenerateSpecWithPlatform(ctdCtx, nil, p, &ctdcontainers.Container{})

-       s, err := ctdoci.GenerateSpecWithPlatform(ctdCtx, nil, "linux/riscv64", &ctdcontainers.Container{})
+       s, err := ctdoci.GenerateSpecWithPlatform(ctdCtx, nil, "linux/riscv64", &ctdcontainers.Container{},
+               ctdoci.WithHostNamespace(specs.NetworkNamespace),
+       )

Copy link
Author

Choose a reason for hiding this comment

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

Handled.

@fredldotme fredldotme marked this pull request as ready for review June 9, 2023 17:16
@fredldotme
Copy link
Author

fredldotme commented Jun 9, 2023

All comments should be somewhat handled now.

EDIT: First signs of user networking, though without a valid IP address (neither static IP nor DHCP work on my end, couldn't figure it out).

image

@ktock
Copy link
Owner

ktock commented Jun 10, 2023

Thanks!
TinyEMU seems to add the following addresses in the virtual network.

struct in_addr net_addr = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
struct in_addr mask = { .s_addr = htonl(0xffffff00) }; /* 255.255.255.0 */
struct in_addr host = { .s_addr = htonl(0x0a000202) }; /* 10.0.2.2 */
struct in_addr dhcp = { .s_addr = htonl(0x0a00020f) }; /* 10.0.2.15 */
struct in_addr dns = { .s_addr = htonl(0x0a000203) }; /* 10.0.2.3 */

In the guest VM, https://github.com/ktock/container2wasm/blob/main/cmd/init/main.go is executed before launching runc container so we can invoke some commands there for the interface configuration.

@fredldotme
Copy link
Author

Thanks, I'll set it up there and remove the capabilities as soon as it works properly.

Attempting the QEMU-preferred way of using DHCP (using dhclient), it seems as if only the first UDP packet gets forwarded through Virtio, then discarded because the checksum doesn't match, and no further UDP packets get forwarded afterwards. I wonder what's up here.

@ktock
Copy link
Owner

ktock commented Jun 10, 2023

it seems as if only the first UDP packet gets forwarded through Virtio, then discarded because the checksum doesn't match, and no further UDP packets get forwarded afterwards. I wonder what's up here.

Maybe we need some compile-time configurations for slirp? https://github.com/ktock/container2wasm/blob/main/patches/tinyemu/tinyemu/slirp/slirp_config.h (e.g. SIZEOF_CHAR_P = 4, HOST_LONG_BITS = 32, etc...)

@fredldotme
Copy link
Author

it seems as if only the first UDP packet gets forwarded through Virtio, then discarded because the checksum doesn't match, and no further UDP packets get forwarded afterwards. I wonder what's up here.

Maybe we need some compile-time configurations for slirp? https://github.com/ktock/container2wasm/blob/main/patches/tinyemu/tinyemu/slirp/slirp_config.h (e.g. SIZEOF_CHAR_P = 4, HOST_LONG_BITS = 32, etc...)

Tried that but those values already were accurate. I assume the device's virtio queue to not getting processed properly. Taking a deeper look now.

@fredldotme
Copy link
Author

fredldotme commented Jun 10, 2023

Seems as if writing arping responses always returns here: https://github.com/ktock/container2wasm/blob/main/patches/tinyemu/tinyemu/virtio.c#L1203

Now I wonder why that is, since other virtio devices seem to be increasing the last_avail_idx properly. Hacking around this by #if 0-ing out the condition allows the first packet to pass through, though no others.

Note that right now I'm setting up networking like this:

ip addr add 10.0.2.15/24 dev eth0
ip link set dev eth0 up
ip route add default via 10.0.2.2 dev eth0

@ktock
Copy link
Owner

ktock commented Jun 11, 2023

I haven't dig this deeply but IIRC napi_tx didn't work well on the emulator. Maybe virtio_net.napi_tx=false is needed to the kernel command line?

@fredldotme
Copy link
Author

I haven't dig this deeply but IIRC napi_tx didn't work well on the emulator. Maybe virtio_net.napi_tx=false is needed to the kernel command line?

That indeed was it! Will add the ip configuration commands and remove capabilities in a short moment.

@fredldotme
Copy link
Author

I hit a roadblock, apparently the host (10.0.2.2) can be pinged now, but setting a nameserver or even using a known public IP address doesn't result in curl fetching any valid data.

@ktock
Copy link
Owner

ktock commented Jun 14, 2023

setting a nameserver or even using a known public IP address doesn't result in curl fetching any valid data.

What error is curl reporting?

@fredldotme
Copy link
Author

setting a nameserver or even using a known public IP address doesn't result in curl fetching any valid data.

What error is curl reporting?

Sorry for the late response, I was busy. curl itself just times out, this is what I currently have:

alfredneumayer@Alfreds-Mac-mini build % ./iwasm --addr-pool=10.0.2.15/8 --allow-resolve="*" ~/Projects/open/container2wasm/out.wasm
root@localhost:/# ip addr add 10.0.2.15/8 dev eth0
ip addr add 10.0.2.15/8 dev eth0
root@localhost:/# ip link set up eth0
ip link set up eth0
root@localhost:/# ip addr
ip addr
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether 02:00:16:67:23:94 brd ff:ff:ff:ff:ff:ff
    inet 10.0.2.15/8 scope global eth0
       valid_lft forever preferred_lft forever
root@localhost:/# busybox ping -c 3 10.0.2.2 
busybox ping -c 3 10.0.2.2
PING 10.0.2.2 (10.0.2.2): 56 data bytes
64 bytes from 10.0.2.2: seq=0 ttl=255 time=9.825 ms
64 bytes from 10.0.2.2: seq=1 ttl=255 time=5.068 ms
64 bytes from 10.0.2.2: seq=2 ttl=255 time=5.148 ms

--- 10.0.2.2 ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 5.068/6.680/9.825 ms
root@localhost:/# curl http://10.0.2.2:5001
curl http://10.0.2.2:5001

curl: (28) Failed to connect to 10.0.2.2 port 5001 after 132671 ms: Connection timed out

Changing iwasm's address pool to something like 0.0.0.0/0 doesn't help either.

@atoonk
Copy link

atoonk commented Aug 5, 2023

Super interested in this!, ie. networking support from a c2w wasm binary running in my browser. I have a websocket to "network" service ready to go for something like this. Once ready for testing, feel free to ping me, happy to help testing, or help however i can.

@fredldotme
Copy link
Author

For those that want to make use of SLIRP without a networking proxy, tested on WebAssembly Micro Runtime, I finally figured it out!

Turns out Out-Of-Band communication doesn't seem to be a concept the current socket APIs understand. Working around this issue and treating connect()s synchronously does the job.

Right now the way to enable an image with networking support is by using the RISCV/TinyEMU backend and building the image with the c2w --net option. I'll leave the enablement on x86 up to the reader, I'm happy with the result. :)

Though the Go side probably needs some adjustments to fully be upstream-worthy, I guess those count as finishing touches only.

Copy link
Owner

@ktock ktock left a comment

Choose a reason for hiding this comment

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

Thanks! This is very cool!

Comment on lines +360 to +369
if (ret == 0) {
soisfconnected(so);
} else
{
/*
* If it's not in progress, it failed, so we just return 0,
* without clearing SS_NOFDREF
*/
soisfconnecting(so);
soisfconnecting(so);
}
Copy link
Owner

Choose a reason for hiding this comment

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

How is this change related to OOB? If this doesn't work, it rather feels like the socket doesn't support non-blocking connect (e.g. select on the socket doesn't set the writefd on ready).

Copy link
Author

Choose a reason for hiding this comment

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

It's a combination of both as far as I can tell, OOB flags stopping WAMR from continuing connecting as well as non-blocking not working at all.

@@ -778,7 +811,12 @@ ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags)
return len;
}
#endif

#ifdef WASI
return send(so->s, buf, len, 0);
Copy link
Owner

Choose a reason for hiding this comment

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

Can we clear OOB flag on the caller of slirp_send rather than entirely remove the flags?

return buf;
}

int get_dns_addr(struct in_addr *pdns_addr)
Copy link
Owner

Choose a reason for hiding this comment

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

How is this used?

struct addrinfo *res;
int count = 0;
struct addrinfo hints;
char *url = "google-public-dns-a.google.com";
Copy link
Owner

Choose a reason for hiding this comment

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

Can we make it a flag rather than hard-coding?

Comment on lines +69 to +72
cli.BoolFlag{
Name: "net",
Usage: "Build with networking capabilities (default: false)",
},
Copy link
Owner

Choose a reason for hiding this comment

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

This seems to rely on wamr-specific codes so I think we should enable this with the flag-net=wamr.

Copy link
Author

Choose a reason for hiding this comment

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

Will try to tackle this, keep in mind I'm not much of a Go developer.

if ((1 & (long) w) && (mlen > 0)) {
if ((1 & (uintptr_t) w) && (mlen > 0)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add an explanation comment here?

Copy link
Author

Choose a reason for hiding this comment

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

Not my change, applied from an upstream libslirp change. The commit message is detailed enough.

Comment on lines +237 to +240
#ifndef WASI
opt = 1;
setsockopt(so->s,SOL_SOCKET,SO_OOBINLINE,(char *)&opt,sizeof(int));
#endif
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't seem to be compiled by default.

Comment on lines 594 to 595
/*******************************************************/
/* socket */
Copy link
Owner

Choose a reason for hiding this comment

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

Please do not remove the original networking logic because it's needed for non-wamr runtimes (e.g. wasmtime, wazero and browser). We can enable the networking logic for wamr when -net=wamr is passed to the emulator flag during runtime.

Copy link
Author

Choose a reason for hiding this comment

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

Whoopsies, no idea how that happened, must have been through the rebase. Will check and reapply customizations again.

fredldotme and others added 9 commits September 22, 2023 19:44
…nditionally

- Rather than setting CONFIG_SLIRP=y, just undefine it to keep the Makefile simple.
- Only build WASI part if requested to do so by defining that one as well.
Casting a pointer to an integer value must use uintptr_t or intptr_t
(not long) for portable code. MinGW-w64 requires this because
sizeof(long) != sizeof(void *) for w64 hosts, so casting to long
raises a compiler warning.

I use uintptr_t instead of intptr_t because changing the sign does not
matter here and casting pointers to unsigned values seems more
reasonable (the unsigned value is a non negative offset.

Cc: Jan Kiszka <[email protected]>
Signed-off-by: Stefan Weil <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
Fixes checksum calculation for UDP/DHCP.
Hardcoded enablement for now. Enabling withNet itself results in
stuck communications (probably due to the missing networking proxy).
Also remove assumption that OOB communication is supported right now.
- Implement get_dns_addr for DNS lookups
- Avoid calling send() with flags since those are not supported universally,
  at least not in WebAssembly Micro Runtime
Instead of assuming OOB to be supported, just jump to connected-state after
a successful connect() and leave it at that.
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