conn: fix StdNetBind fallback on Windows

[jwhited]

This is meant to address #64 (comment)

Should we remove the Windows code because it's not used? Or keep it because it could be used if for some reason you use bind_std on Windows instead of the winrio stuff? Or maybe the Windows code should use the control functions too?

Keep it for fallback, which was broken, and this commit addresses. The Windows code could eventually use the control functions. I don't know how SO_{RCV/SND}BUF sizing relates to a socket under the control of RIO and its ring. Something we will get deeper into with Windows work.

If RIO is unavailable, NewWinRingBind() falls back to StdNetBind. StdNetBind uses x/net/ipv{4,6}.PacketConn for sending and receiving datagrams, specifically via the {Read,Write}Batch methods. These methods are unimplemented on Windows and will return runtime errors as a result. This commit updates StdNetBind to fall back to the standard library net package for Windows.

[zx2c4]

Ooooof, that sure is ugly. What about fixing x/net to implement this as a simple loop operation?

[jwhited]

Ooooof, that sure is ugly. What about fixing x/net to implement this as a simple loop operation?

Have you seen this accepted proposal? golang/go#45886 (comment)

I'd rather see time invested there instead of more work in x/net. Would a TODO pointing to that issue + drying up these funcs to reduce the amount of ugliness be sufficient?

[zx2c4]

What do you mean by "drying up these funcs"?

Anyway, if you think golang/go#45886 (comment) is the way to go, I'm alright with an ugly bandaid like this. But it'd be nice if you could say, "I can get this done in net/ and such within the next N months" or whatever, just so we know it's not going to be a forever-TODO. IOW, I'd prefer if you took ownership of the inevitable conversion so it doesn't fall to be to remember at some point down the line.

Also, based on the platforms x/net supports, do you suppose it should be a positive test for "linux","freebsd","..." rather than a negative test for !="windows"? Or is it good as-is? I haven't looked.

[jwhited]

What do you mean by "drying up these funcs"?

Collapsing StdNetBind.receiveIPv4() and StdNetBind.receiveIPv4() together, e.g.

var (
	// If compilation fails here these are no longer the same underlying type.
	_ ipv4.Message = ipv6.Message{}
)

type batchReader interface {
	ReadBatch([]ipv6.Message, int) (int, error)
}

func makeReceive(pool *sync.Pool, br batchReader, conn *net.UDPConn) ReceiveFunc {
	return func(buffs [][]byte, sizes []int, eps []Endpoint) (n int, err error) {
	...
	}
}

There was a non-collapsed makeReceiveIPv{4,6}() pattern in place before, which served an important purpose at the time, when the ReceiveFuncs used a *net.UDPConn. The *net.UDPConn used in the ReceiveFunc had to be distinct from the pointer field on StdNetBind, which is nil'd on Close(). When x/net was added, we satisfy this by coincidence as *net.UDPConn is within a ipvX.PacketConn, but with this fix we are back to needing the pointers to be distinct.

So, what I'd like to do is return to this make..() pattern for ReceiveFuncs, and I can collapse them to not repeat the ugliness so much.

Anyway, if you think golang/go#45886 (comment) is the way to go, I'm alright with an ugly bandaid like this. But it'd be nice if you could say, "I can get this done in net/ and such within the next N months" or whatever, just so we know it's not going to be a forever-TODO. IOW, I'd prefer if you took ownership of the inevitable conversion so it doesn't fall to be to remember at some point down the line.

The std lib proposal improves upon x/net by removing allocations, which would require a redesign of the x/net API to accomplish. We are tracking moving to it as a TODO.

Also, based on the platforms x/net supports, do you suppose it should be a positive test for "linux","freebsd","..." rather than a negative test for !="windows"? Or is it good as-is? I haven't looked.

x/net calls down into x/sys/unix, which has support for everything except Windows and Plan9 afaict. I do like the idea of flipping this to a positive test for linux, as none of the other platforms benefit from the x/net API right now, and I bet they never will, as support would most likely be added to the std lib proposal first.

[zx2c4]

Alright! All of the above sounds good to me. (Though I'd expect for at least freebsd to also benefit from x/net, but maybe not.)

[jwhited]

Alright! All of the above sounds good to me. (Though I'd expect for at least freebsd to also benefit from x/net, but maybe not.)

FreeBSD doesn't benefit at the moment - https://cs.opensource.google/go/x/net/+/refs/tags/v0.8.0:ipv4/batch.go;l=86

So, what I'd like to do is return to this make..() pattern for ReceiveFuncs, and I can collapse them to not repeat the ugliness so much.

I returned to the make..() pattern, but was unable to collapse them. Collapsing breaks ReceiveFunc.prettyName() in that you end up unable to distinguish which address family func was started in the logs.

I added a 2nd commit in the series that is of a fix category and is in nearby code in StdNetBind.

[zx2c4]

func (s *StdNetBind) BatchSize() int {
	return IdealBatchSize
}

Should that now return 1 on !linux?

[zx2c4]

Applied these changes. Feel free to push a change to BatchSize() if you agree with the above comment, or not if you don't. You can also just dump things into jw/whatever/jr/for-jason branches as it suits you and poke me on Signal to merge.