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

net: use netdev package #3704

Merged
merged 6 commits into from
Dec 6, 2023
Merged

net: use netdev package #3704

merged 6 commits into from
Dec 6, 2023

Conversation

deadprogram
Copy link
Member

@deadprogram deadprogram commented May 4, 2023

This PR uses the net submodule branch https://github.com/tinygo-org/net/tree/netdev3 with all of the @scottfeldman changes and also brings in the other changed files from the commits in #3614

If this works, it was some pretty serious rebasing to get here. Not quite as serious as the work @scottfeldman did on the original PRs, of course.

UPDATE: now using https://github.com/tinygo-org/net/tree/dev

@deadprogram
Copy link
Member Author

The CI passed, so I squashed a few commits to clean things up. This PR is ready for testing and any additional review.

@deadprogram deadprogram mentioned this pull request May 4, 2023
@scottfeldman
Copy link
Contributor

Cool. Is there a way to link crypto/tls directory to tinygo-org/net/crypto/tls?

@deadprogram
Copy link
Member Author

Is there a way to link crypto/tls directory to tinygo-org/net/crypto/tls?

Not really, no. To do that would require another separate git submodule crypto

@deadprogram
Copy link
Member Author

Rebased against latest dev.

@deadprogram
Copy link
Member Author

The size difference check will fail until this is merged into dev branch due to switching to submodule.

@deadprogram
Copy link
Member Author

Now using https://github.com/tinygo-org/net/tree/dev branch. One more step closer to be able to be merged.

@deadprogram
Copy link
Member Author

Rebased against latest dev

@deadprogram
Copy link
Member Author

Rebased again against what will likely be the next release. Getting closer... 🏃

@scottfeldman
Copy link
Contributor

thank you

@scottfeldman
Copy link
Contributor

Upon further thought- could we not keep the netlink+netdev+probe packages out of tinygo-drivers until they mature enough that we are comfortable with their API? I feel like we really need to sit down and start using them. We can merge the tinygo-org/tinygo PR (this one) which would expose the needed functionality to make it work with the net package while not exposing any API that may be subject to future breakage. I think that's a sound path to follow given the scope of these changes- we're certainly going to need to make breaking changes in the future and frustrate users of the drivers package if we rush into this.

Rush? It's been almost a year since I sent on the RFC and no one is using the API except me because it never makes it to dev. How do we sit down and start using them? Frustrated.

Sorry, I know you're right to get the APIs right as best we can, even if it takes more time. Also sorry for being difficult to work with on this part of the project. If you could take over the driver PR, I would appreciate it and be relieved and can work on crytpo/tls port and ch9120 driver. Thanks for the review time.

@soypat
Copy link
Contributor

soypat commented Nov 28, 2023

I have what I need now, and if driver/stack interfaces change, I should be insulated from those changes since I'm building apps on top of "net".

Alright, we still need to merge netdev-3 on tinygo-org/net for this to work though with the useNetdev hack?

Would consider taking over on the driver/stack interfaces and develop a PR to replace mine?

As soon as I get around to finishing the TCP stack I'll be available. By then it I think I'll have a solid idea of where to take the API.

I think the tinygo-org/tinygo and tinygo-org/net PRs are ready to go;

I would think so as well. It is my understanding there is no exported API that break compatibility with upstream Go. I do have one reservation though regarding net.netdev interface: replace net.IP with netip.Addr and netip.AddrPort where appropiate. Proposal below:

type netdever interface {
	// GetHostByName returns the IP address of either a hostname or IPv4
	// address in standard dot notation
	GetHostByName(name string) (netip.Addr, error)

	// Addr returns IP address assigned to the interface, either by
	// DHCP or statically
	Addr() (netip.Addr, error)

	// Berkely Sockets-like interface, Go-ified.  See man page for socket(2), etc.
	Socket(domain int, stype int, protocol int) (int, error)
	Bind(sockfd int, addr netip.AddrPort) error
	Connect(sockfd int, host netip.AddrPort) error
	Listen(sockfd int, backlog int) error
	Accept(sockfd int, addr netip.AddrPort) (int, error)
	Send(sockfd int, buf []byte, flags int, deadline time.Time) (int, error)
	Recv(sockfd int, buf []byte, flags int, deadline time.Time) (int, error)
	Close(sockfd int) error
	SetSockOpt(sockfd int, level int, opt int, value interface{}) error
}

@scottfeldman Finally, as unsincere as it may sound I am sorry it took this long and is still taking this long to get these up to where they are now. I do feel like this took way more than it could have.

Copy link
Contributor

@soypat soypat left a comment

Choose a reason for hiding this comment

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

@deadprogram @scottfeldman This PR looks good. Are we ready to create the corresponding https://github.com/tinygo-org/net/tree/dev PR?

@deadprogram
Copy link
Member Author

I have merged the dev branch of net into main on that repo, so I can now update this PR to point to it.

@deadprogram
Copy link
Member Author

Actually, since the submodule already pointed to that SHA, no change required in this repo.

@soypat
Copy link
Contributor

soypat commented Nov 29, 2023

Aha! So it was merged, must have missed the netdev.go file first time I looked at it. I've created a PR to modify the netdever API to use the netip.Addr type which is a pretty good fit for embedded systems where heap allocations are a relatively large cost to pay. Be great to get eyes from you too @scottfeldman since this would impact your APIs which are already in use.

PR in question: tinygo-org/net#8

@scottfeldman
Copy link
Contributor

PR tinygo-org/net#8 LGTM. Need a corresponding PR in tinygo-drivers for the netdever interface changes.

@scottfeldman
Copy link
Contributor

Naming

The naming: probe.Probe does not really tell me or I think anyone of what is happening, why the call is needed. Having different packages for netlink/probe/netdev is confusing- these packages often need one another. This on top of the netdev.Netdever, probe.Probe, and netlink.Netlinker which is stuttersome at best and just confusing at worse because if I put myself in the place of someone who knows a thing or two about networking I don't think I could reliably always make out which is which. They are just too similarly named or vaguely named... and often are represented by the same underlying type, which adds to the confusing aspect of it all (see composing interfaces below)!

One comment about naming: The names netdev, netlink, and probe are all borrowed from Linux network driver kernel space, so they would be familiar to someone that's worked in that space. I'm from that space, but I didn't think about others not being familiar with those names. It's likely these Linux names have roots in BSD networking, so they should be familiar to folks with background in UNIX BSD networking drivers.

It could be worse! I could have used Microsoft NDIS names for everything, complete with VeryLongNamesWithCamelCase.

In any case, here's how I would define netdev, netlink, and probe:

netdev: an L3 interface; the thing that is assigned an IP address and can send/recv IP traffic. Has an UP/DOWN state.

netlink: an L2 interface; to send/recv eth traffic. Has a MAC address. Has an UP/DOWN state.

probe: OS function to probe system hardware, matching devices to drivers. Matching is done by some unique hardware ID like PCI device/vendor ID or USB device ID. Probe will create a driver instance for each matching device. We're not an OS, but we need some mechanism to bind a driver to a device without explicitly calling out the driver. Similar to how a tinygo app uses machine.LED without specifying the target hardware, an app should be able to use "net" package without specifying the target networking hardware. That is the intent of probe.

@scottfeldman
Copy link
Contributor

scottfeldman commented Dec 2, 2023

PR tinygo-org/net#8 LGTM. Need a corresponding PR in tinygo-drivers for the netdever interface changes.

Ok, I added tinygo-org/drivers#622 as a matching PR to match tinygo-org/net#8. Both need to go in together. @soypat Please review tinygo-org/drivers#622.

@deadprogram
Copy link
Member Author

Updated to point to latest main branch in net.

@deadprogram
Copy link
Member Author

One comment about naming: The names netdev, netlink, and probe are all borrowed from Linux network driver kernel space, so they would be familiar to someone that's worked in that space.

💯 this is probably very key to getting more people onboard with helping add/maintain this code.

@deadprogram deadprogram force-pushed the net-submodule-netdev3 branch 2 times, most recently from 003907b to 0277deb Compare December 4, 2023 13:42
Copy link

github-actions bot commented Dec 4, 2023

Size difference with the dev branch:

Binary size difference
drivers/sizes-dev.txt has more commands than drivers/sizes-pr.txt
    tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/ntpclient/main.go
 flash                          ram
 before   after   diff          before   after   diff
  60960   60960      0   0.00%    6164    6164      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adt7410/main.go
   9736    9736      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adxl345/main.go
  13268   13268      0   0.00%    6780    6780      0   0.00% tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/amg88xx
   8724    8724      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/main.go
  11612   11612      0   0.00%    6556    6556      0   0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/apds9960/proximity/main.go
   9784    9784      0   0.00%    4752    4752      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/itsybitsy-m0/main.go
   8168    8168      0   0.00%    2304    2304      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/at24cx/main.go
   8344    8344      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bh1750/main.go
   7632    7632      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/blinkm/main.go
  70492   70492      0   0.00%    3688    3688      0   0.00% tinygo build -size short -o ./build/test.hex -target=pinetime     ./examples/bma42x/main.go
  63444   63444      0   0.00%    6172    6172      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmi160/main.go
  27836   27836      0   0.00%    4772    4772      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp180/main.go
  63544   63544      0   0.00%    6204    6204      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp280/main.go
  12352   12352      0   0.00%    4804    4804      0   0.00% tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bmp388/main.go
   8128    8128      0   0.00%    3332    3332      0   0.00% tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/sram/main.go
  22100   22100      0   0.00%    3528    3528      0   0.00% tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/time/main.go
  69332   69332      0   0.00%    6352    6352      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/ds3231/main.go
   4704    4704      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/easystepper/main.go
  24932   24932      0   0.00%    6292    6292      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espconsole/main.go
  25080   25080      0   0.00%    6292    6292      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/esphub/main.go
  24932   24932      0   0.00%    6292    6292      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espstation/main.go
  68844   68844      0   0.00%    6944    6944      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/flash/console/spi
  64916   64916      0   0.00%    8984    8984      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/flash/console/qspi
   7040    7040      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/gc9a01/main.go
  68148   68148      0   0.00%    6340    6340      0   0.00% tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/i2c/main.go
  68544   68544      0   0.00%    6484    6484      0   0.00% tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/uart/main.go
   8372    8372      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/hcsr04/main.go
   5612    5612      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/customchar/main.go
   5656    5656      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/text/main.go
  10564   10564      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/hd44780i2c/main.go
  14516   14516      0   0.00%    6556    6556      0   0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/hts221/main.go
  16940   16940      0   0.00%    2344    2344      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hub75/main.go
  10052   10052      0   0.00%    6900    6900      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic
  10832   10832      0   0.00%    4852    4852      0   0.00% tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/basic
  29052   29052      0   0.00%   38060   38060      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/pyportal_boing
  10080   10080      0   0.00%    6908    6908      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/scroll
  10908   10908      0   0.00%    4860    4860      0   0.00% tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/scroll
 263564  263564      0   0.00%   46732   46732      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/slideshow
  12052   12052      0   0.00%    4772    4772      0   0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis3dh/main.go
  13912   13912      0   0.00%    6556    6556      0   0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/lps22hb/main.go
  26124   26124      0   0.00%    2312    2312      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/lsm303agr/main.go
  12520   12520      0   0.00%    4780    4780      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/lsm6ds3/main.go
  10996   10996      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mag3110/main.go
  10176   10176      0   0.00%    4764    4764      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017/main.go
  10604   10604      0   0.00%    4772    4772      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017-multiple/main.go
   9816    9816      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp3008/main.go
  66920   66920      0   0.00%    6172    6172      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp2515/main.go
  22984   22984      0   0.00%    2424    2424      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/microbitmatrix/main.go
  22936   22936      0   0.00%    4468    4468      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit-v2 ./examples/microbitmatrix/main.go
   8444    8444      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mma8653/main.go
   8356    8356      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mpu6050/main.go
  75196   75196      0   0.00%    7452    7452      0   0.00% tinygo build -size short -o ./build/test.hex -target=p1am-100 ./examples/p1am/main.go
  12148   12148      0   0.00%    3336    3336      0   0.00% tinygo build -size short -o ./build/test.hex -target=pico ./examples/pca9685/main.go
   6080    6080      0   0.00%    3272    3272      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setbuffer/main.go
   5100    5100      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setpixel/main.go
   2697    2697      0   0.00%     556     556      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino ./examples/servo
   7944    7944      0   0.00%    6780    6780      0   0.00% tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/shifter/main.go
  56608   56608      0   0.00%    3660    3660      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht3x/main.go
  56664   56664      0   0.00%    3668    3668      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht4x/main.go
  56580   56580      0   0.00%    3660    3660      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/shtc3/main.go
   6456    6456      0   0.00%    2272    2272      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/i2c_128x32/main.go
   5960    5960      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/spi_128x64/main.go
   5676    5676      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1331/main.go
   6376    6376      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7735/main.go
   6232    6232      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7789/main.go
  17156   17156      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/thermistor/main.go
  10312   10312      0   0.00%    4516    4516      0   0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-bluefruit ./examples/tone
  10096   10096      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/tm1637/main.go
   9404    9404      0   0.00%    6772    6772      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/fourwire/main.go
  12468   12468      0   0.00%    6984    6984      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/pyportal_touchpaint/main.go
  15804   15804      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl53l1x/main.go
  13848   13848      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl6180x/main.go
   6452    6452      0   0.00%    2304    2304      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13/main.go
   6004    6004      0   0.00%    2296    2296      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13x/main.go
   6260    6260      0   0.00%    2304    2304      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd4in2/main.go
1882105 1882105      0   0.00%  417764  417764      0   0.00%

@deadprogram deadprogram force-pushed the net-submodule-netdev3 branch 2 times, most recently from ecaaca8 to d0ce4cc Compare December 6, 2023 09:21
deadprogram and others added 6 commits December 6, 2023 10:23
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 (https://docs.google.com/spreadsheets/d/e/2PACX-1vT0cCjBvwXf9HJf6aJV2Sw198F2ief02gmbMV0sQocKT4y4RpfKv3dh6Jyew8lQW64FouZ8GwA2yjxI/pubhtml?gid=1013173032&single=true).
@deadprogram
Copy link
Member Author

The time has come to merge this! Great work @scottfeldman and thanks very much to @soypat for so much help reviewing and testing.

@deadprogram deadprogram merged commit a1a2d1a into dev Dec 6, 2023
24 checks passed
@deadprogram deadprogram deleted the net-submodule-netdev3 branch December 6, 2023 12:11
This was referenced Dec 6, 2023
@aykevl
Copy link
Member

aykevl commented Mar 12, 2024

I'm sorry I missed the discussion on the net package (it was a bit much to follow - sorry!), but why is this a submodule and not a regular package? Is there some benefit to keeping it in a submodule?

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