-
Notifications
You must be signed in to change notification settings - Fork 914
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
Netdev2 #3452
Netdev2 #3452
Conversation
src/net/net.go
Outdated
"time" | ||
|
||
"tinygo.org/x/drivers/netdev" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think maybe this shouldn't be referencing the drivers module; it might be preferable to have a copy of the netdev.Netdever interface in TinyGo instead of in drivers
(or alternatively, have the primary definition be in TinyGo and referenced from drivers
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had it that way at one point, but changed it as to not add any new exports to "net" package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the motivation, but we for sure do not wish for the circular reference. Either use a subpackage, just add another file with the exported interface, or copy the interface definition into both the tinygo repo and also a duplicate the drivers package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, new commit moving drivers/netdev to tinygo/src/net/netdev. This should decouple "net" and "netdev" and remove the circular reference.
src/net/udpsock.go
Outdated
// UDPConn is the implementation of the Conn and PacketConn interfaces | ||
// for UDP network connections. | ||
type UDPConn struct { | ||
fd netdev.Sockfd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment as for netdev.Netdever ... Sockfd should maybe be defined in the TinyGo repo instead of drivers
. Also wondering if Sockfd should be uintptr
instead of int
(in os
for instance file descriptors are uintptr
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int was what socket(2) calls return, so just copied that. And -1 is a common error return value for socket(2).
Great work, this is very exciting :) |
Seems like some dependency problem here @scottfeldman https://github.com/tinygo-org/tinygo/actions/runs/4190521808/jobs/7264005556#step:13:32 |
@deadprogram I'm stuck. Both netdev PRs for tinygo and drivers builds are failing because they depend on each other. tinygo needs to import drivers/netdev and drivers needs to import tinygo "net" for net.useNetdev. These build fine in my go workspace where they're both present. I'm at a loss on what to do. |
@deadprogram I started over in a fresh go workspace with a fresh clone of tinygo and drivers, checked out netdev branch for each and built everything again. Ran make test on tinygo and drivers and both PASS. It seems both PRs need to be merged at the same time and folks would have to refresh both to avoid hitting the build issue we're hitting here. |
… extra rebuilds (#3453) builds/macos, linux, windows: update to explicit restore/save for LLVM source and LLVM builds
This is a big commit that changes the way runtime type information is stored in the binary. Instead of compressing it and storing it in a number of sidetables, it is stored similar to how the Go compiler toolchain stores it (but still more compactly). This has a number of advantages: * It is much easier to add new features to reflect support. They can simply be added to these structs without requiring massive changes (especially in the reflect lowering pass). * It removes the reflect lowering pass, which was a large amount of hard to understand and debug code. * The reflect lowering pass also required merging all LLVM IR into one module, which is terrible for performance especially when compiling large amounts of code. See issue 2870 for details. * It is (probably!) easier to reason about for the compiler. The downside is that it increases code size a bit, especially when reflect is involved. I hope to fix some of that in later patches.
machine/usb/hid: add MediaKey support
gen: Better handling of sub-clusters in SVD
…ctions Signed-off-by: deadprogram <[email protected]>
Signed-off-by: deadprogram <[email protected]>
Signed-off-by: deadprogram <[email protected]>
…d rebuilding Signed-off-by: deadprogram <[email protected]>
Running binaries in QEMU (when debugging on Linux for example) did not work correctly as qemu-user expects the `-g` flag to be first on the command line before the program name. Putting it after will make it a command line parameter for the emulated program, which is not what we want. I don't think this ever worked correctly.
… first Signed-off-by: deadprogram <[email protected]>
This is unsafe and should never be done.
Some vector registers must be preserved across calls, but this wasn't happening on Linux and MacOS. When I added support for windows/arm64, I saw that it required these vector registers to be preserved and assumed this was Windows deviating from the standard calling convention. But actually, Windows was just implementing the standard calling convention and the bug was on Linux and MacOS. This commit fixes the bug on Linux and MacOS and at the same time merges the Go and assembly files as they no longer need to be separate.
It can be difficult to find what went wrong in a test. Omitting -v should make it easier to see the failing tests and the output for them (note that output is still printed for tests that fail).
Hi @soypat, pester you on the review...how's it going? |
My apologies @scottfeldman, I totally missed the updated version. Thanks for letting me know. I'll get to it ASAP |
Changes in the commit based on feedback from Ben and Patricio: 0) Move netdever interface to "net" package 1) Mirror netdever interface with same in drivers repo 2) Use go:linkname to link to net.useNetdev() 3) Don't add any new Exports to "net" or "net/http" packages 4) Use native Go types in netdever interface 5) Add some common syscall types for things like AF_INET and SOCK_DGRAM. 6) Update documentation
Scott, the way the go tool works is that it interprets files that end with |
I have some updates in my local tree to this PR. Should I push those or wait until your review? Changes are:
|
Push em! |
Hello @scottfeldman looks like some actual CI errors like this https://github.com/tinygo-org/tinygo/actions/runs/4535422239/jobs/7990849307?pr=3452#step:20:34 |
I messed up my git repo for this PR, so I'm closing this PR and replacing it with a fresh one: #3614. |
This PR adds a network device driver model called netdev. There will be a companion PR for TinyGo drivers to update the netdev drivers and network examples. This PR covers the core "net" package.
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, version 1.19.3. The src/net/README file has 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.