feat: add basic WASI `net.Listener` via `sock_accept` #2748

rvolosatovs · 2022-04-05T18:34:59Z

Description

Add support for sock_accept recently added to WASI WebAssembly/WASI#458

The implementation still lacks polling support, but that is something best left for another PR, since that would be a magnitudes bigger change.

Relevant documentation on flags can be found here https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md#fdflags

I followed the upstream Go implementation as much as possible with minimal changes

Testing

Server

Save the following program to main.go in your working directory:

package main

import (
	"fmt"
	"io"
	"log"
	"net"
	"os"
	"strconv"
	"time"
)

func handle(conn net.Conn) error {
	defer conn.Close()

	var b [128]byte
	n, err := conn.Read(b[:])
	if err != nil {
		if err == io.EOF {
			return nil
		}
		return fmt.Errorf("failed to read from connection: %s", err)
	}
	log.Printf("Read '%s'\n", b[:n])

	res := fmt.Sprintf("Hello, %s", b[:n])
	n, err = conn.Write([]byte(res))
	if err != nil {
		return fmt.Errorf("failed to write to connection: %s", err)
	}
	log.Printf("Wrote '%s'\n", res[:n])
	return nil
}

func run() error {
	fds, err := strconv.Atoi(os.Getenv("FD_COUNT"))
	if err != nil {
		return fmt.Errorf("failed to parse FD_COUNT: %w", err)
	}
	if fds != 4 {
		return fmt.Errorf("FD_COUNT must be 4, got %d", fds)
	}

	ln, err := net.FileListener(os.NewFile(uintptr(3), "socket"))
	if err != nil {
		return fmt.Errorf("failed to listen on fd 3: %w", err)
	}

	for {
		log.Println("Waiting for connection...")
		conn, err := ln.Accept()
		if err != nil {
			if err, ok := err.(*os.SyscallError); ok && err.Timeout() {
				log.Println("Accept timed out, retrying in a second...")
				time.Sleep(time.Second)
				continue
			}
			return fmt.Errorf("failed to accept connection: %w", err)
		}
		log.Println("Accepted connection")

		if err := handle(conn); err != nil {
			return fmt.Errorf("failed to handle connection: %w", err)
		}
		log.Println("---")
	}
}

func init() {
	log.SetOutput(os.Stderr)
	log.SetFlags(0)
}

func main() {
	if err := run(); err != nil {
		log.Fatalf("Failed to run: %s", err)
	}
}

Compile with:

$ tinygo build -target wasi main.go

Wasmtime

Works on wasmtime 0.38.0:

$ wasmtime run --env FD_COUNT=4 --tcplisten 127.0.0.1:9000 main.wasm
Waiting for connection...
Accept timed out, retrying in a millisecond...
Waiting for connection...
Accept timed out, retrying in a millisecond...
Waiting for connection...
Accept timed out, retrying in a millisecond...
Waiting for connection...
Accepted connection
Read 'test
'
Wrote 'Hello, test
'
---

Enarx

Works on https://github.com/enarx/enarx with the following config:

[[files]]
kind = "stdin"

[[files]]
kind = "stdout"

[[files]]
kind = "stderr"

[[files]]
kind = "listen"
prot = "tcp"
port = 9000

For example, on Linux:

$ curl -sL -o enarx https://github.com/enarx/enarx/releases/download/v0.6.0/enarx-x86_64-unknown-linux-musl
$ echo 26823747c19c21fc7c5a7a6d44337d731735c780044b9da90365e63aae15f68d enarx | sha256sum -c
$ chmod +x enarx
$ ./enarx run --wasmcfgfile Enarx.toml main.wasm
Waiting for connection...
Accepted connection
Read 'test
'
Wrote 'Hello, test
'
---

Client

Start in a terminal window:

while :; do sleep 1 && echo test | nc 127.0.0.1 9000; done
Hello, test

deadprogram · 2022-05-18T14:14:58Z

Hello @rvolosatovs could you please rebase against the latest dev branch so this can be more easily reviewed?

cc/ @dkegel-fastly @dgryski @aykevl

npmccallum · 2022-06-29T17:12:38Z

@rvolosatovs Can you rebase this?

rvolosatovs · 2022-07-01T16:50:20Z

@deadprogram apologies for the delay, I was on vacation right after we spoke and then had a few very busy weeks.
I've rebased, updated the implementation and added an example for testing the PR is ready for review, PTAL!
cc @dkegel-fastly @dgryski @aykevl

There is a CI build failure on MacOS, but it does not seem to be caused by this PR - I may be wrong however

npmccallum · 2022-07-22T14:44:17Z

@deadprogram Are you able to review this?

rvolosatovs · 2022-08-05T16:33:21Z

I've extracted some of the required changes into smaller scoped PRs in hopes of a simpler review.
@dkegel-fastly @dgryski @aykevl @deadprogram

This PR is now blocked by:
#3051
#3050
#3052
#3053
#3054

Signed-off-by: Roman Volosatovs <[email protected]>

Directly duplicate from Go 1.18 implementation, use fake netFD for all systems (which is only used for js&wasm in stdlib) Signed-off-by: Roman Volosatovs <[email protected]>

Signed-off-by: Roman Volosatovs <[email protected]>

rvolosatovs · 2022-08-07T09:57:32Z

@deadprogram would further splitting this PR into smaller scoped chunks assist review?

rvolosatovs · 2022-08-07T10:11:12Z

Filed #3061 , marking as draft again

codefromthecrypt · 2022-09-06T10:17:26Z

@rvolosatovs interesting (esp that the snapshot01 version was updated after the fact!).

I was curious the motivation of this? Is this a proof of concept or are folks making raw socket listeners? Reason I ask is often there are higher level libs that need some changes to support even if sock_accept works.

Finally, I noticed in the past due to lack of description in the spec sometimes you need to look at another impl to see how they handle things. Did you notice how this is implemented in rust or clang?? Ex the resulting wasm (can inspect backwards via wasm2wat) looks similar between here and rust or C of the same?

Cheers!

codefromthecrypt

Added some review to help this along.

I suspect when things stabilize, unit tests would make sense, though I notice Go doesn't really test these files directly either.

codefromthecrypt · 2022-09-06T10:51:00Z

src/net/fd_wasi.go

+
+// LocalAddr implements the Conn LocalAddr method.
+func (*netFD) LocalAddr() Addr {
+	return nil


Suggested change

return nil

return laddr

rvolosatovs · 2022-09-26T10:34:55Z

src/net/fd_wasi.go

+
+// RemoteAddr implements the Conn RemoteAddr method.
+func (*netFD) RemoteAddr() Addr {
+	return nil


This is not available on WASI

rvolosatovs · 2022-09-26T10:34:28Z

src/net/fd_wasi.go

+
+// LocalAddr implements the Conn LocalAddr method.
+func (*netFD) LocalAddr() Addr {
+	return nil


This is not available on WASI

codefromthecrypt · 2022-09-06T11:12:55Z

src/net/fd_wasi.go

+	if err != nil {
+		return nil, wrapSyscallError("sock_accept", err)
+	}
+	return &netFD{File: os.NewFile(uintptr(fd), "conn"), net: "file+net", laddr: nil, raddr: nil}, nil


maybe a fake addr instead of nil? or comment why? I think usually these are derived from fd, right?

None of this is available on WASI

I'll delete all my comments which summarize as "please comment this" :D for now, I guess people will know that if something is nil, it is intentional.

codefromthecrypt · 2022-09-06T11:27:58Z

src/syscall/syscall_libc_wasi.go

@@ -309,6 +309,14 @@ func Getpagesize() int {
 	return 65536
 }

+func SockAccept(fd int, flags int) (nfd int, err error) {
+	var retptr0 __wasi_fd_t


verify flags or return EINVAL?

Standard library does not do that for Accept4 https://github.com/golang/go/blob/a0441c7ae3dea57a0553c9ea77e184c34b7da40f/src/syscall/syscall_linux.go#L661-L677 🤔
Is it really responsibility of TinyGo standard library or rather the WASI implementation?

WASI has no guidance on things like this. the spec is very sparse. There's no discussion of host vs guest responsibilities. knowing this, yeah most hosts will also validate and return EINVAL conventionally https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md

lacking guidance, what I do is look at what wasi-libc does and prefer that, though agree you can also look at how go does it. this isn't a huge deal which way, imho

codefromthecrypt · 2022-09-06T11:32:32Z

src/net/fd_wasi.go

+
+// Read implements the Conn Read method.
+func (c *netFD) Read(b []byte) (int, error) {
+	// TODO: Handle EAGAIN and perform poll


Suggested change

// TODO: Handle EAGAIN and perform poll

// TODO: Handle EAGAIN and perform poll similar to poll/fd_unix.go

Well, not necessarily same as https://github.com/golang/go/blob/master/src/internal/poll/fd_unix.go
It's circling back to https://github.com/tinygo-org/tinygo/pull/2748/files#r979834379 in my head - is it a goal of this project to be as similar to upstream Go and minimize diff or not? A simpler and more minimal poll implementation could probably suit this project better, but I am not sure what's the preferred approach. I was hoping this PR could start a discussion

I'll leave this one to the maintainers to decide. thanks for clarifying!

To the style question, my impression is that the project tries in this order:

directly use upstream files (no copying needed)

borrow a subset of upstream files, minimizing diffs but only borrowing the lines that are needed

replace upstream files with something more suited to tinygo's use case (but still kind of minimizing diffs if possible)

Upstream's networking is... tricky because of its interaction with goroutines, as you've noticed.
How useful is networking without getting that bit right?
Will your code (e.g. netfd.Read()) pause all goroutines while blocking on networking?
I haven't checked yet.

codefromthecrypt · 2022-09-06T11:35:46Z

src/net/fd_wasi.go

+	"time"
+)
+
+// Network file descriptor.


Suggested change

// Network file descriptor.

// wasiCon implements Con with a file returned by syscall.SockAccept.

This is directly copy-pasted and adapted from https://github.com/golang/go/blob/a0441c7ae3dea57a0553c9ea77e184c34b7da40f/src/net/fd_posix.go#L16-L17, that's the approach I saw throughout this project so I followed that.

Changing this comment would make it inconsistent with upstream, which means it would not be as trivial to sync this with upstream by copy-pasting.

If we're changing this, then we should change the whole implementation as well, since the way this is designed in the standard library is not necessarily the best approach here given the set of use cases this project tries to cover.

ok understand what you mean. I think in another world I would probably comment things like this in the source, but ack that's not the practice. I'll dismiss my feedback as most of them are me being curious why things are defining unused symbols or otherwise. People who know the conventions already somehow won't need to see comments like this.

rvolosatovs · 2022-09-26T10:46:37Z

src/net/fd_wasi.go

+	return nil
+}
+
+func fileListener(f *os.File) (Listener, error) {


Upstream Go does not do this https://github.com/golang/go/blob/a0441c7ae3dea57a0553c9ea77e184c34b7da40f/src/net/file_unix.go#L89-L102

If a structure of the code is confusing, I don't think that explaining everything in comments is the right solution - restructuring the code is, however.
As I said, I tried to keep as close as possible to upstream Go implementation - if that's not a requirement, then I'd rather rewrite this whole thing in a way that makes much more sense especially for this use case

Personally, I would love to see your impl, but for tinygo whatever maintainers suggest is best as they have the long term stake.

rvolosatovs · 2022-09-26T10:16:58Z

@rvolosatovs interesting (esp that the snapshot01 version was updated after the fact!).

I was curious the motivation of this? Is this a proof of concept or are folks making raw socket listeners? Reason I ask is often there are higher level libs that need some changes to support even if sock_accept works.

The motivation is to be able to accept a connection from within a WebAssembly module via WASI. In Wasmtime and Enarx this works by calling sock_accept on pre-opened file descriptors

Finally, I noticed in the past due to lack of description in the spec sometimes you need to look at another impl to see how they handle things. Did you notice how this is implemented in rust or clang?? Ex the resulting wasm (can inspect backwards via wasm2wat) looks similar between here and rust or C of the same?

Both Rust (rust-lang/rust#93158) and C (WebAssembly/wasi-libc#287) support was added by @haraldh, who we work together with at @profianinc. We're making sure to have all implementations consistent.

Note, that big difference of sock_accept from accept family, is that the peer address cannot be determined (at least for now) as part of the "accept".

rvolosatovs · 2022-09-26T10:34:28Z

src/net/fd_wasi.go

+
+// LocalAddr implements the Conn LocalAddr method.
+func (*netFD) LocalAddr() Addr {
+	return nil


This is not available on WASI

rvolosatovs · 2022-09-26T10:34:55Z

src/net/fd_wasi.go

+
+// RemoteAddr implements the Conn RemoteAddr method.
+func (*netFD) RemoteAddr() Addr {
+	return nil


This is not available on WASI

rvolosatovs · 2022-09-26T10:46:37Z

src/net/fd_wasi.go

+	return nil
+}
+
+func fileListener(f *os.File) (Listener, error) {


Upstream Go does not do this https://github.com/golang/go/blob/a0441c7ae3dea57a0553c9ea77e184c34b7da40f/src/net/file_unix.go#L89-L102

If a structure of the code is confusing, I don't think that explaining everything in comments is the right solution - restructuring the code is, however.
As I said, I tried to keep as close as possible to upstream Go implementation - if that's not a requirement, then I'd rather rewrite this whole thing in a way that makes much more sense especially for this use case

hlubek · 2023-03-06T13:52:22Z

I played around with this PR and tried to use it to start an HTTP server with net/http over the FileListener. Basically my WASM/WASI code looks like this (following the example from this PR):

func run() error {
	ln, err := net.FileListener(os.NewFile(uintptr(3), "socket"))
	if err != nil {
		return fmt.Errorf("failed to listen on fd 3: %w", err)
	}

	mux := http.NewServeMux()
	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		fmt.Fprintf(w, "Hello, %s", r.URL.Path[1:])
	})

	srv := &http.Server{
		Handler: mux,
	}

	err = srv.Serve(ln)
	if err != nil {
		return fmt.Errorf("failed to serve: %w", err)
	}

	return nil
}

It seems to work after I did the following changes:

Added a dummy LocalAddr and RemoteAddr, otherwise the Go HTTP implementation had a nil dereference issue

Implement a very naive polling via checking for syscall.EAGAIN and sleeping for a few milliseconds (in Read, Write and Accept):

func (c *netFD) Read(b []byte) (int, error) {
  var (
  	n   int
  	err error
  )
  for {
  	n, err = c.File.Read(b)
  	if errors.Is(err, syscall.EAGAIN) {
  		time.Sleep(30 * time.Millisecond)
  		continue
  	}
  	break
  }
  return n, err
}

Doing this I could run the server via wasmtime run --tcplisten 127.0.0.1:9000 server.wasm and a Curl succeeded. 😁

What would be a better way to perform the polling? Is poll_oneoff supposed to work for sock_accept and reading / writing from the file descriptor?

I could imagine that the Listener returned by FileListener should not poll by default but just return EAGAIN / a syscall error indicating a timeout and the user code could wrap the listener to control the polling behaviour.

It would be really great to have some kind of support of this in TinyGo, so web services with Go could be run in WASM/WASI.

codefromthecrypt · 2023-03-07T00:10:54Z

@hlubek if were me I would probably scour GitHub for anyone using wasi-libc in C and see how they are doing it. https://github.com/WebAssembly/wasi-libc/blob/63e4489d01ad0262d995c6d9a5f1a1bab719c917/libc-bottom-half/sources/accept.c then compare polling approaches. I think these network functions are more used in rust for various docker demos.

chriso · 2023-05-11T23:49:21Z

We're proposing this be added to Go (with GOOS=wasip1). See https://go-review.googlesource.com/c/go/+/493358. It'll use the native netpoll facility so you won't have to manually handle EAGAIN.

chriso · 2023-05-26T21:05:21Z

This landed in Go via golang/go@a17de43e (and prerequisites golang/go@41893389 and golang/go@c5c21845)

rvolosatovs · 2023-05-26T21:09:56Z

This landed in Go via golang/go@a17de43e (and prerequisites golang/go@41893389 and golang/go@c5c21845)

Yes, it finally happened! Closing

codefromthecrypt · 2023-07-22T03:07:36Z

@deadprogram I think this issue is not closed in TinyGo, even if it is solved for Go. There will be users who can't use Go, yet due to lack of exports. Should this be re-opened as an issue until closed? or this codebase repurposed to complete regardless of what Go does?

rvolosatovs force-pushed the feat/wasi-sock-accept branch 4 times, most recently from 36cec4e to 3614793 Compare April 5, 2022 19:09

rvolosatovs mentioned this pull request Apr 5, 2022

feat: add a Go server example enarx/enarx#1639

Closed

rvolosatovs force-pushed the feat/wasi-sock-accept branch from 3614793 to f0645b0 Compare April 8, 2022 12:29

rvolosatovs force-pushed the feat/wasi-sock-accept branch 2 times, most recently from 9efdb97 to fedcfaf Compare July 1, 2022 14:51

rvolosatovs marked this pull request as ready for review July 1, 2022 16:50

rvolosatovs force-pushed the feat/wasi-sock-accept branch from 77aa8ad to 57de1cd Compare July 8, 2022 15:25

rvolosatovs changed the title ~~feat: add WASI sock_accept support~~ feat: add basic WASI net support via sock_accept Jul 13, 2022

rvolosatovs changed the title ~~feat: add basic WASI net support via sock_accept~~ feat: add basic WASI net.Listener via sock_accept Jul 13, 2022

rvolosatovs force-pushed the feat/wasi-sock-accept branch from 57de1cd to 823a275 Compare August 5, 2022 16:31

rvolosatovs marked this pull request as draft August 5, 2022 16:31

rvolosatovs force-pushed the feat/wasi-sock-accept branch from 823a275 to bfe82d6 Compare August 6, 2022 11:38

rvolosatovs added 5 commits August 7, 2022 11:51

syscall: add WASI SockAccept

Unverified

The committer email address is not verified.

Learn about vigilant mode

7958868

Signed-off-by: Roman Volosatovs <[email protected]>

net: wrap netFD in conn

Unverified

The committer email address is not verified.

Learn about vigilant mode

6c775cd

Directly duplicate from Go 1.18 implementation, use fake netFD for all systems (which is only used for js&wasm in stdlib) Signed-off-by: Roman Volosatovs <[email protected]>

net: add wrapSyscallError

Unverified

The committer email address is not verified.

Learn about vigilant mode

f9495a0

Signed-off-by: Roman Volosatovs <[email protected]>

net: implement netFD for WASI

Unverified

The committer email address is not verified.

Learn about vigilant mode

519a89d

Signed-off-by: Roman Volosatovs <[email protected]>

net: add FileListener for WASI

Unverified

The committer email address is not verified.

Learn about vigilant mode

902c2a2

Signed-off-by: Roman Volosatovs <[email protected]>

rvolosatovs force-pushed the feat/wasi-sock-accept branch from bfe82d6 to 902c2a2 Compare August 7, 2022 09:51

rvolosatovs marked this pull request as ready for review August 7, 2022 09:52

rvolosatovs mentioned this pull request Aug 7, 2022

syscall: add WASI sock_accept #3061

Closed

rvolosatovs marked this pull request as draft August 7, 2022 10:11

codefromthecrypt mentioned this pull request Sep 6, 2022

feat: add support for accept and accept4 WebAssembly/wasi-libc#287

Merged

codefromthecrypt reviewed Sep 6, 2022

View reviewed changes

rvolosatovs commented Sep 26, 2022

View reviewed changes

rvolosatovs closed this May 26, 2023

	// TODO: Handle EAGAIN and perform poll
	// TODO: Handle EAGAIN and perform poll similar to poll/fd_unix.go

	// Network file descriptor.
	// wasiCon implements Con with a file returned by syscall.SockAccept.

feat: add basic WASI net.Listener via sock_accept #2748

feat: add basic WASI net.Listener via sock_accept #2748

Conversation

rvolosatovs commented Apr 5, 2022 • edited Loading

Description

Testing

Server

Wasmtime

Enarx

Client

deadprogram commented May 18, 2022

npmccallum commented Jun 29, 2022

rvolosatovs commented Jul 1, 2022

npmccallum commented Jul 22, 2022

rvolosatovs commented Aug 5, 2022

rvolosatovs commented Aug 7, 2022

rvolosatovs commented Aug 7, 2022

codefromthecrypt commented Sep 6, 2022

codefromthecrypt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvolosatovs Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvolosatovs commented Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlubek commented Mar 6, 2023

codefromthecrypt commented Mar 7, 2023

chriso commented May 11, 2023

chriso commented May 26, 2023

rvolosatovs commented May 26, 2023

codefromthecrypt commented Jul 22, 2023

feat: add basic WASI `net.Listener` via `sock_accept` #2748

feat: add basic WASI `net.Listener` via `sock_accept` #2748

rvolosatovs commented Apr 5, 2022 •

edited

Loading

rvolosatovs Sep 26, 2022 •

edited

Loading

rvolosatovs commented Sep 26, 2022 •

edited

Loading