Netdev

[scottfeldman]

(I'm closing the original PR #3452 and submitting this one as a replacement).

This PR adds a network device driver model called netdev. There is a companion PR for TinyGo drivers to update the netdev drivers and network examples. This PR covers the core "net" and "net/http" packages.

An RFC for the work is here: #tinygo-org/drivers#487. Some things have changed from the RFC, but nothing major.

The "net" package is a partial port of Go's "net" package, copied and modified from Go version 1.19.3. See src/net/README for details on what is modified from Go's "net" package.

Most "net" features are working as they would in normal Go. TCP/UDP/TLS protocol support is there. As well as HTTP client and server support. Standard Go network packages such as golang.org/x/net/websockets and Paho MQTT client work as-is. Other packages are likely to work as-is.

Testing results are here.

[scottfeldman]

I don't understand the build failures. I ran "make test" and "make test-tinygo" before submitting. (Frustrated!)

[deadprogram]

@scottfeldman looks like now only Windows build is failing https://github.com/tinygo-org/tinygo/actions/runs/4546595651/jobs/8025444093?pr=3614#step:7:36

[deadprogram]

Also merge conflict in Makefile but that is a minor correction 😸

[soypat]

Looking amazing all around. Good job on keeping this interface lean! Looks solid as could be.

I got a nit regarding the file descriptor type on netdever which could give us a tad bit of more type hinting which you can never really have too much.

[deadprogram]

@scottfeldman if you can rebase against the latest dev, I think you will see that actually the net tests were removed under Windows. That is directly related to the Makefile merge conflicts as well.

This should allow all of the tests to pass.

[scottfeldman]

@scottfeldman if you can rebase against the latest dev, I think you will see that actually the net tests were removed under Windows. That is directly related to the Makefile merge conflicts as well.

This should allow all of the tests to pass.

@deadprogram Cool, I'll rebase but I need help on these instructions from here. The last step is where I've screwed up rebasing two times and end up having to create a new PR. What is "myfork" in the last line?

git checkout dev
git pull --rebase origin dev
git checkout my-feature-branch
git rebase dev
git push myfork my-feature-branch -f
         ^^^^^^

[deadprogram]

That last line would be:

git push scottfeldman netdev3 -f

Are you certain you have the latest dev branch?

[scottfeldman]

@deadprogram Still having trouble:

sfeldma@nuc:~/work/tinygo$ git push scottfeldman netdev3 -f
fatal: 'scottfeldman' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

sfeldma@nuc:~/work/tinygo$ git status
On branch netdev3
Your branch and 'origin/netdev3' have diverged,
and have 37 and 2 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

nothing to commit, working tree clean

sfeldma@nuc:~/work/tinygo$ git log --oneline | head -n5
e1de929d move IPPROTO_TLS to netdev to avoid src/syscall dependency
bc9dfb2c Add network device driver model, netdev
50d68135 main: set WASMTIME_BACKTRACE_DETAILS when running in wasmtime.
d50c54fc Makefile: compress/lzw seems to work on wasi now.
4a81cac5 main: make sure all testing output goes to the same place

I first synced my fork in scottfeldman/tinygo to tinygo-org/tinygo:dev, then did the following:

git checkout dev
git pull --rebase origin dev
git checkout netdev3
git rebase dev                        <-- resolved Makefile conflict
git push scottfeldman netdev3 -f      <-- failed

[scottfeldman]

Ok, I'm back on track with:

git push origin netdev3 -f

"net/mail" is the next failure. Removing it would get us thru, I believe. Feels like we're deferring a problem we'll have to fix eventually.

[scottfeldman]

Well, removing all the net package test paths from the Makefile didn't work.

Somehow the reflect package forces a compile of the net package, and this is breaking the Windows build because the net package needs syscall.SOL_TCP, etc. I'll revert the last commit.

I'm not sure what to do with Windows not finding syscall.SOL_TCP when it's clearly defined in src/syscall/net.go. Actually, looking at loader/goroot.go, I think adding "windows" tag to targets that need the syscall package merged. Let me try that.

[scottfeldman]

Year 2000 was the last time I did any development work on Windows. I don't want to break my 23-year streak of no Windows.

[soypat]

Does native (upstream) Go default to port 80 too?

[scottfeldman]

Yes, here's the doc for Addr in http.Server:

type Server struct {
	// Addr optionally specifies the TCP address for the server to listen on,
	// in the form "host:port". If empty, ":http" (port 80) is used.
	// The service names are defined in RFC 6335 and assigned by IANA.
	// See net.Dial for details of the address format.
	Addr string

[soypat]

Let's try to keep net compatible with native Go!

[deadprogram]

I have been thinking about how best to bring these changes into the code, because I really want the features that will be unlocked by the hard work put in by @scottfeldman

Basically, what do we do with the following?

• Are these people who will want to use the net package code from go1.19 vs some from go1.20+? How to handle those differences.
• What to do to update the net package quickly on new Go version releases, since many of those updates involve network security CVEs being patched.
• How can we make it reasonable for project maintainers to perform these updates? @scottfeldman might not want to be on the hook forever for this code.

One idea I had was to perhaps put the net package into a separate repo, and bring it into the main TinyGo repo using a git submodule. This might make it easier for people to choose which net package version to use, if they do not want to go with our default latest version for whatever reason. We could maintain a branch with 1.19 vs 1.20 vs whatever upcoming versions. I assume this would be needed?

Regarding how to update, perhaps some minimal automation could checkout the Go std lib net package someplace, and then try to update the specific files we need. At that point the person doing the update could see the diff in their uncommited changes and figure out what to do about it.

The excellent work that @scottfeldman put into this PR with the list of specific files changed made me think that something like this could be possible for handling the inevitable upstream changes in the main Go standard libs.

I really want to incorporate this incredible effort made by @scottfeldman and also want to keep things as maintainable as we can.

Thoughts?

[deadprogram]

One more thing, we will need to handle compatability with the upstream WASI changes being made by @achille-roussel @evanphx and friends such as https://cs.opensource.google/go/go/+/eb2bc919760d7a3e5ffd6022756cd7bda2f2dc63

[deadprogram]

I created https://github.com/tinygo-org/net so we can try this out. I'll get back to it later and let everything know what I find discover...

[deadprogram]

Step 1 is complete: here is branch with the netdev commits included https://github.com/tinygo-org/net/tree/netdev3

[deadprogram]

#3704 passed CI so I squashed a couple commits to clean up. It is ready for testing and any further review, as required.

[soypat]

My understanding on the subject of maintainability and security:

• We use the netdever model of doing things up until we could link netdevers into the sycalls used by upstream Go packages.
  • Why: My understanding is Scott tried to link using the syscalls and that was hard work that he did not have time to put in or that it was unfeasible due to the sheer size of the program including HTTP/2

In doing so we accept we may exclude security patches and optimizations due to the implementation mismatch with upstream. I was fine with it since I did not see another agile way forward.

@deadprogram Just to get me up to speed with the proposed changes-

• how would users swap net implementations?
• Is there evidence users would actually do this? I thought there was only one net implementation in existence compatible with tinygo
  • if there is not, should we put our efforts into maintaining multiple implementation? Is there another way of going about splitting implementations? maybe go build tags i.e. baremetal-net, tinynet, wasm
  • If we decide to keep everything inside net as detailed above maybe it'd be easier to work with since all context is in one place?
  • How is the proposed structural package change going to help in the security department?

Again, I ain't opposed- just want clarity on the reasons!