listeners: Use net.UDPConn instead

[francislavoie]

See #3998

How does this look @marten-seemann? Seems to compile, but untested.

[marten-seemann]

I think this should work. You could easily test it by just starting Caddy with HTTP/3 enabled.

[mholt]

Technically, I believe Packet sockets can be either UDP or IP networks: https://golang.org/pkg/net/#ListenPacket

The network must be "udp", "udp4", "udp6", "unixgram", or an IP transport. The IP transports are "ip", "ip4", or "ip6" followed by a colon and a literal protocol number or a protocol name, as in "ip:1" or "ip:icmp".

Although Caddy doesn't yet have a module that uses IP sockets, it could definitely happen (a layer 3 app, for example).

I wonder if we could either use a type-assertion to return the right fakeClose* type (i.e. if a UDPConn, return a *fakeCloseUDPConn, otherwise *fakeClosePacketConn)? I'm just trying to not shut out future modules from IP listeners.

[francislavoie]

🤔 I'm not sure if I know how to resolve this, actually. The net.UDPConn changes I made are pretty wide-reaching, cause it also changes globalListener. And we're creating the UDPConn ourselves based on the network given. Right now network is just "udp" all the time. What would it be for IP sockets? I think I'm just not entirely sure how to make the changes you're suggesting 😞

[mholt]

Ok -- no problem. I'll take a closer look as soon as I have a chance.

[mholt]

I've implemented what I think is a much cleaner, flexible workaround -- though it is still a hack, it should work for 99.9% of use cases -- like so:

func (fcpc fakeClosePacketConn) SetReadBuffer(bytes int) error {
	switch conn := fcpc.PacketConn.(type) {
	case *net.UDPConn:
		return conn.SetReadBuffer(bytes)
	case *net.IPConn:
		return conn.SetReadBuffer(bytes)
	case *net.UnixConn:
		return conn.SetReadBuffer(bytes)
	}
	return fmt.Errorf("not implemented for %T", fcpc.PacketConn)
}

func (fcpc fakeClosePacketConn) SyscallConn() (syscall.RawConn, error) {
	switch conn := fcpc.PacketConn.(type) {
	case *net.UDPConn:
		return conn.SyscallConn()
	case *net.IPConn:
		return conn.SyscallConn()
	case *net.UnixConn:
		return conn.SyscallConn()
	}
	return nil, fmt.Errorf("not implemented for %T", fcpc.PacketConn)
}

I'm ready to commit and push this as it resolves the original warnings, but now I'm seeing this error produced from quic-go:

failed to sufficiently increase receive buffer size (was: 208 kiB, wanted: 2048 kiB, got: 416 kiB). See https://github.com/lucas-clemente/quic-go/wiki/UDP-Receive-Buffer-Size for details.

Any suggestions @marten-seemann ?

[mholt]

By the way @marten-seemann, this error seems like a copy-paste mistake: https://github.com/lucas-clemente/quic-go/blob/6533c6f6e549979726581e7cd28e8f0c7e0213f1/packet_handler_map.go#L78

[marten-seemann]

@mholt Thanks, I'll submit a fix.

[mholt]

Thanks @marten-seemann ! What about the new error we're seeing though? Is it a problem? I feel like we're going to get bug reports.

[marten-seemann]

You should probably run the sysctl command to increase the buffer size, if you expect to receive larger amounts of data.
Regarding your code, why do you use a type switch instead of an interface assertion?

[mholt]

@marten-seemann That is a GREAT idea, totally didn't think of that! Thanks! Got it down to this:

func (fcpc fakeClosePacketConn) SetReadBuffer(bytes int) error {
	if conn, ok := fcpc.PacketConn.(interface{ SetReadBuffer(int) error }); ok {
		return conn.SetReadBuffer(bytes)
	}
	return fmt.Errorf("SetReadBuffer() not implemented for %T", fcpc.PacketConn)
}

func (fcpc fakeClosePacketConn) SyscallConn() (syscall.RawConn, error) {
	if conn, ok := fcpc.PacketConn.(interface {
		SyscallConn() (syscall.RawConn, error)
	}); ok {
		return conn.SyscallConn()
	}
	return nil, fmt.Errorf("SyscallConn() not implemented for %T", fcpc.PacketConn)
}

And thanks for the tip, I'll refer anyone who asks about it to that system call then for their individual systems.

[mholt]

Thanks for the initial contribution, Francis; closing in favor of ec3ac84.