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

Netdevdemo #3500

Closed
wants to merge 12 commits into from
Closed

Netdevdemo #3500

wants to merge 12 commits into from

Conversation

soypat
Copy link
Contributor

@soypat soypat commented Mar 1, 2023

@scottfeldman @aykevl @bgould @deadprogram
As of now this does not compile throwing a convoluted stream of llvm errors. It serves the purpose to illustrate the artistic direction the netdev PR could take.

To set the netdev it would suffice to do the following in a tinygo program. Note the changes in the interface. There is a working proof of concept attached as well to illustrate at a simple level what is being done and that the core idea works.

I have not been able to actually get this to work due to the errors which seem to be on the net package internal side of things. Probably due to the net.Addr type which has some excessively complex methods that may be breaking tinygo. I'll try to reduce the net.Addr

Proof of concept of linking functions that accept different interfaces with identical method sets
package main

import (
	"strings"
	_ "unsafe" // unsafe package needed to link function.
)

type reeder interface {
	Read(b []byte) (int, error)
}

func main() {
	b, _ := ReedAll(strings.NewReader("blabla"))
	println(string(b))
	// output:
	// blabla
}

//go:linkname ReedAll io.ReadAll
func ReedAll(d reeder) ([]byte, error)
usage of netdever
package main

import (
	"net"
	"time"
	_ "unsafe"
)

func main() {
	UseDev(nil)
}

//go:linkname UseDev net.useDev
func UseDev(d dev)

// dev drivers implement the net.dev interface.
//
// A Netdever is passed to the "net" package using netdev.Use().
//
// Just like a net.Conn, multiple goroutines may invoke methods on a Netdever
// simultaneously.
type dev interface {

	// NetConnect device to IP network
	NetConnect() error

	// NetDisconnect device from IP network
	NetDisconnect()

	// NetNotify to register callback for network events
	// NetNotify(func(Event))

	// GetHostByName returns the IP address of either a hostname or IPv4
	// address in standard dot notation
	GetHostByName(name string) (net.IP, error)

	// GetHardwareAddr returns device MAC address
	GetHardwareAddr() (net.HardwareAddr, error)

	// GetIPAddr returns IP address assigned to device, either by DHCP or
	// statically
	GetIPAddr() (net.IP, error)

	// Socketer is a Berkely Sockets-like interface
	socketer
}

// Berkely Sockets-like interface.  See man page for socket(2), etc.
//
// Multiple goroutines may invoke methods on a Socketer simultaneously.
type socketer interface {
	// # Socket Address family argument
	//
	// Socket address families specifies a communication domain:
	//  - AF_UNIX, AF_LOCAL(synonyms): Local communication For further information, see unix(7).
	//  - AF_INET: IPv4 Internet protocols.  For further information, see ip(7).
	//
	// # Socket type argument
	//
	// Socket types which specifies the communication semantics.
	//  - SOCK_STREAM: Provides sequenced, reliable, two-way, connection-based
	//  byte streams.  An out-of-band data transmission mechanism may be supported.
	//  - SOCK_DGRAM: Supports datagrams (connectionless, unreliable messages of
	//  a fixed maximum length).
	//
	// The type argument serves a second purpose: in addition to specifying a
	// socket type, it may include the bitwise OR of any of the following values,
	// to modify the behavior of socket():
	//  - SOCK_NONBLOCK: Set the O_NONBLOCK file status flag on the open file description.
	//
	// # Socket protocol argument
	//
	// The protocol specifies a particular protocol to be used with the
	// socket.  Normally only a single protocol exists to support a
	// particular socket type within a given protocol family, in which
	// case protocol can be specified as 0. However, it is possible
	// that many protocols may exist, in which case a particular
	// protocol must be specified in this manner.
	//
	// # Return value
	//
	// On success, a file descriptor for the new socket is returned. Quoting man pages:
	// "On error, -1 is returned, and errno is set to indicate the error." Since
	// this is not C we may use a error type native to Go to represent the error
	// ocurred which by itself not only notifies of an error but also provides
	// information on the error as a human readable string when calling the Error method.
	Socket(family int, sockType uint8, protocol int) (fd uintptr, err error)

	Bind(sockfd uintptr, addr net.Addr) error
	Connect(sockfd uintptr, servaddr net.Addr) error
	Listen(sockfd uintptr, backlog int) error
	Accept(sockfd uintptr, peer net.Addr) (uintptr, error)
	// # Flags argument
	//
	// The flags argument is formed by ORing one or more of the following values:
	//  - MSG_CMSG_CLOEXEC: Set the close-on-exec flag for the file descriptor. Unix.
	//  - MSG_DONTWAIT: Enables nonblocking operation. If call would block then returns error.
	//  - MSG_ERRQUEUE: (see manpage) his flag specifies that queued errors should be received
	//  from the socket error queue.
	//  - MSG_OOB: his flag requests receipt of out-of-band data that would not be received in the normal data stream.
	//  - MSG_PEEK: This flag causes the receive operation to return data from
	//  the beginning of the receive queue without removing that data from the queue.
	//  - MSG_TRUNC: Ask for real length of datagram even when it was longer than passed buffer.
	//  - MSG_WAITALL: This flag requests that the operation block until the full request is satisfied.
	Send(sockfd uintptr, buf []byte, flags uint16, timeout time.Duration) (int, error)
	Recv(sockfd uintptr, buf []byte, flags uint16, timeout time.Duration) (int, error)
	Close(sockfd uintptr) error
	// SetSockOpt manipulates options for the socket
	// referred to by the file descriptor sockfd.  Options may exist at
	// multiple protocol levels; they are always present at the
	// uppermost socket level.
	//
	// # Level argument
	//
	// When manipulating socket options, the level at which the option
	// resides and the name of the option must be specified.  To
	// manipulate options at the sockets API level, level is specified
	// as SOL_SOCKET.  To manipulate options at any other level the
	// protocol number of the appropriate protocol controlling the
	// option is supplied.  For example, to indicate that an option is
	// to be interpreted by the TCP protocol, level should be set to the
	// protocol number of TCP; see getprotoent(3).
	//
	// # Option argument
	//
	// The arguments optval and optlen are used to access option values
	// for setsockopt().  For getsockopt() they identify a buffer in
	// which the value for the requested option(s) are to be returned.
	// In Go we provide developers with an `any` interface to be able
	// to pass driver-specific configurations.
	SetSockOpt(sockfd uintptr, level, opt int, optionValue any) error
}
errors
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"internal/testlog$pack" to i8*), i8* undef), !dbg !117
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %2, 0, !dbg !153
 i8*  call void @"interface:{Chdir:func:{basic:string}{},Getenv:func:{basic:string}{},Open:func:{basic:string}{},Stat:func:{basic:string}{}}.Getenv$invoke"(i8* %invoke.func.value, i8* %6, i64 %7, i64 %invoke.func.typecode, i8* undef), !dbg !153
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %2, 0, !dbg !167
 i8*  call void @"interface:{Chdir:func:{basic:string}{},Getenv:func:{basic:string}{},Open:func:{basic:string}{},Stat:func:{basic:string}{}}.Open$invoke"(i8* %invoke.func.value, i8* %6, i64 %7, i64 %invoke.func.typecode, i8* undef), !dbg !167
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %2, 0, !dbg !181
 i8*  call void @"interface:{Chdir:func:{basic:string}{},Getenv:func:{basic:string}{},Open:func:{basic:string}{},Stat:func:{basic:string}{}}.Stat$invoke"(i8* %invoke.func.value, i8* %6, i64 %7, i64 %invoke.func.typecode, i8* undef), !dbg !181
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:pointer:named:error", i32 0, i32 0)
 i64  %0 = call %runtime._interface @"internal/reflectlite.TypeOf"(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:pointer:named:error", i32 0, i32 0), i8* null, i8* undef), !dbg !91
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %0, 0, !dbg !94
 i8*  %1 = call %runtime._interface @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.Elem$invoke"(i8* %invoke.func.value, i64 %invoke.func.typecode, i8* undef), !dbg !94
Call parameter type does not match function signature!
  %interface.type = extractvalue %runtime._interface %1, 0, !dbg !134
 i8*  %2 = call i1 @"interface:{Unwrap:func:{}{named:error}}.$typeassert"(i64 %interface.type), !dbg !134
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %5, 0, !dbg !140
 i8*  %7 = call %runtime._interface @"interface:{Unwrap:func:{}{named:error}}.Unwrap$invoke"(i8* %invoke.func.value, i64 %invoke.func.typecode, i8* undef), !dbg !140
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %13, 0, !dbg !162
 i8*  %14 = call i1 @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.Comparable$invoke"(i8* %invoke.func.value, i64 %invoke.func.typecode, i8* undef), !dbg !162
Call parameter type does not match function signature!
  %interface.type = extractvalue %runtime._interface %15, 0, !dbg !171
 i8*  %21 = call i1 @"interface:{Is:func:{named:error}{basic:bool}}.$typeassert"(i64 %interface.type), !dbg !171
Call parameter type does not match function signature!
  %invoke.func.typecode7 = extractvalue %runtime._interface %24, 0, !dbg !177
 i8*  %28 = call i1 @"interface:{Is:func:{named:error}{basic:bool}}.Is$invoke"(i8* %invoke.func.value8, i64 %26, i8* %27, i64 %invoke.func.typecode7, i8* undef), !dbg !177
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"errors$pack" to i8*), i8* undef), !dbg !208
Call parameter type does not match function signature!
  %invoke.func.typecode = extractvalue %runtime._interface %13, 0, !dbg !216
 i8*  %14 = call i64 @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.Kind$invoke"(i8* %invoke.func.value, i64 %invoke.func.typecode, i8* undef), !dbg !216
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"errors$pack.2" to i8*), i8* undef), !dbg !218
Call parameter type does not match function signature!
  %invoke.func.typecode9 = extractvalue %runtime._interface %13, 0, !dbg !222
 i8*  %21 = call %runtime._interface @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.Elem$invoke"(i8* %invoke.func.value10, i64 %invoke.func.typecode9, i8* undef), !dbg !222
Call parameter type does not match function signature!
  %invoke.func.typecode11 = extractvalue %runtime._interface %21, 0, !dbg !225
 i8*  %22 = call i64 @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.Kind$invoke"(i8* %invoke.func.value12, i64 %invoke.func.typecode11, i8* undef), !dbg !225
Call parameter type does not match function signature!
  %invoke.func.typecode13 = extractvalue %runtime._interface %21, 0, !dbg !229
 i8*  %27 = call i1 @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.Implements$invoke"(i8* %invoke.func.value14, i64 %25, i8* %26, i64 %invoke.func.typecode13, i8* undef), !dbg !229
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"errors$pack.4" to i8*), i8* undef), !dbg !230
Call parameter type does not match function signature!
  %invoke.func.typecode15 = extractvalue %runtime._interface %34, 0, !dbg !237
 i8*  %37 = call i1 @"interface:{Align:func:{}{basic:int},AssignableTo:func:{named:reflect.Type}{basic:bool},Bits:func:{}{basic:int},ChanDir:func:{}{named:reflect.ChanDir},Comparable:func:{}{basic:bool},ConvertibleTo:func:{named:reflect.Type}{basic:bool},Elem:func:{}{named:reflect.Type},Field:func:{basic:int}{named:reflect.StructField},FieldAlign:func:{}{basic:int},FieldByIndex:func:{slice:basic:int}{named:reflect.StructField},FieldByName:func:{basic:string}{named:reflect.StructField,basic:bool},Implements:func:{named:reflect.Type}{basic:bool},In:func:{basic:int}{named:reflect.Type},IsVariadic:func:{}{basic:bool},Key:func:{}{named:reflect.Type},Kind:func:{}{named:reflect.Kind},Len:func:{}{basic:int},MethodByName:func:{basic:string}{named:reflect.Method,basic:bool},Name:func:{}{basic:string},NumField:func:{}{basic:int},NumIn:func:{}{basic:int},NumMethod:func:{}{basic:int},NumOut:func:{}{basic:int},Out:func:{basic:int}{named:reflect.Type},PkgPath:func:{}{basic:string},Size:func:{}{basic:uintptr},String:func:{}{basic:string}}.AssignableTo$invoke"(i8* %invoke.func.value16, i64 %35, i8* %36, i64 %invoke.func.typecode15, i8* undef), !dbg !237
 .....
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"strconv$pack.143" to i8*), i8* undef), !dbg !7303
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"strconv$pack.159" to i8*), i8* undef), !dbg !7850
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"strconv$pack.153" to i8*), i8* undef), !dbg !8039
Call parameter type does not match function signature!
i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0)
 i64  call void @runtime._panic(i8* getelementptr inbounds ({ i8, i8* }, { i8, i8* }* @"reflect/types.type:basic:string", i32 0, i32 0), i8* bitcast ({ %runtime._string }* @"strconv$pack.155" to i8*), i8* undef), !dbg !8272
FAIL	net	0.000s
error: verification error after compiling package internal/testlog

@scottfeldman
Copy link
Contributor

scottfeldman commented Mar 1, 2023

Same comment I left at the end of my PR (#3452):

"You lost me here. How does a driver in the drivers repo get access to non-exported interfaces in the tinygo repo? The drivers have to be written to some interface".

I think you'll need to pull the driver PR (#523) also, and update the drivers and test the examples to match the changes you're making to tinygo when proposing alternative ideas.

One comment on your PR: it doesn't feel right for every tinygo application that wants to use "net" to import "unsafe". I'm grimacing thinking about writing the user documentation for that requirement.

Actually, I'm not understanding the problem your PR is trying to solve, fundamentally. Maybe you can describe a scenario that illustrates the problem before we spend more time in solution space. To set the problem up, imagine my PR went in as-is. Time passes. Now, in the future, an issue comes up but somehow we shot ourselves in the foot with my PR and can't move forward or have to invent some distasteful work-around and live with it. What is the scenario you have in mind? Sorry for being dense; I'm just not coming up with anything.

@scottfeldman
Copy link
Contributor

Same comment I left at the end of my PR (#3452):

"You lost me here. How does a driver in the drivers repo get access to non-exported interfaces in the tinygo repo? The drivers have to be written to some interface".

Ok, I'm an idiot. I see it now, the driver doesn't need to know the interface definition to implement it. Duh, Go 101. Let me try this out on tinygo/drivers and update the PRs. Sorry for being a knucklehead; I just couldn't see what now seems obvious. Thanks @soypat for being persistent.

PS. I don't like this "unsafe" business, but the scale seems tipped.

@soypat
Copy link
Contributor Author

soypat commented Mar 2, 2023

OK- so we have TinyGo stdlib, which I'll just call tinygo, the native Go stdlib or upstream for short, and third party libraries, which I'll call libs.

So to quickly answer your doubts two key points:

  1. Users will not have to import unsafe. There can be a netdev lib for development outside of tinygo which does the dirty work. This package can be anywhere really, users can build their own libs. It sets the stage for more contributions. The example I've shown below could be such a minimalist library.
  2. Your PR violates Tinygo <-> upstream compatibility. I believe it is not necessary and there is more to gain from this approach.

The beauty of this PR is that there's not much more to it. It will allow users to begin developing drivers outside of the tinygo repo in external libs. If you want typed interfaces you can do that on the lib side of things by creating an interface that wraps the net.dev interface.

The way I see it not much has changed since your first proposed RFC where you had a function called netdev.UseNetdev under the drivers package. Same user experience!

Actually, I'm not understanding the problem your PR is trying to solve, fundamentally. Maybe you can describe a scenario that illustrates the problem before we spend more time in solution space. To set the problem up, imagine my PR went in as-is. Time passes. Now, in the future, an issue comes up but somehow we shot ourselves in the foot with my PR and can't move forward or have to invent some distasteful work-around and live with it. What is the scenario you have in mind? Sorry for being dense; I'm just not coming up with anything.

So the problem this solves is losing compatibility with Go. Why should we take that step if we can avoid it completely? There's also the issue of duplicated types in your PR which introduces another HardwareAddr and Addr types. This PR also elimanates that. The net code also becomes simpler since the argument to TCPDial and other Dial functions can be directly passed into the dev methods. Overall code reduction, gains in simplicity at the cost of unsafe linking... I think it's a cheap price to pay. The only person that should ideally be linking is the lib creator. I'll provide an example of the lib side of things using non-native types to demonstrate the flexibility of this idea.

@soypat
Copy link
Contributor Author

soypat commented Mar 2, 2023

Netdev lib with helpful types

Here's an example of a third party library that provides users with your proposed types for the socket(2) interface. If I'm not missing any crucial bit of information, I think we keep the best of all worlds with this approach! We can develop on top of the socket(2) bare-bones implementation however we please and learn more about what works and what does not while keeping compatibility with upstream and tinygo.

@soypat
Copy link
Contributor Author

soypat commented Mar 2, 2023

Thanks @soypat for being persistent.

@scottfeldman I just saw your response! Didn't mean to pile on you after you had understood.

@scottfeldman
Copy link
Contributor

I have one wifinina example working! It'll take me probably all day tomorrow to finish the conversion and test all the examples and devices and update the documentation. But it's looking good. Lots of stuff fell out.

@scottfeldman
Copy link
Contributor

Ok, I've made tinygo & drivers updates based on this demo, specifically:

tinygo repo

  1. Move netdever interface to "net" package
  2. Mirror netdever interface with same in drivers repo
  3. Use go:linkname to link to net.useNetdev()
  4. Don't add any new Exports to "net" or "net/http" packages
  5. Use native Go types in netdever interface
  6. Add some common syscall types for things like AF_INET and SOCK_DGRAM.
  7. Update documentation

drivers repo

  1. Move netdever interface to "drivers" package
  2. Mirror netdever interface with same in drivers repo
  3. Use go:linkname to link to net.useNetdev()
  4. Use native Go types in netdever interface
  5. Add Netlinker interface to "drivers" package
  6. Update documentation, add new README-net.md file

@deadprogram
Copy link
Member

Now that #3704 has been merged I am going to close this PR. Thanks everyone!

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.

3 participants