Skip to content

Commit

Permalink
quic: fix UDP on big-endian Linux, tests on various architectures
Browse files Browse the repository at this point in the history 
The following cmsgs contain a native-endian 32-bit integer:

  - IP_TOS, passed to sendmsg
  - IPV6_TCLASS, always

IP_TOS received from recvmsg contains a single byte, because why not.

We were inadvertently assuming little-endian integers in all cases.
Add endianness conversion as appropriate.

Disable tests that rely on IPv4-in-IPv6 mapped sockets on dragonfly
and openbsd, which don't support this feature. (A "udp" socket cannot
receive IPv6 packets on these platforms.)

Disable IPv6 tests on wasm, where the simulated networking appears
to generally not support IPv6.

Fixes golang/go#65906
Fixes golang/go#65907

Change-Id: Ie50af12e182a1a5d685ce4fbdf008748f6aee339
Reviewed-on: https://go-review.googlesource.com/c/net/+/566296
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
  • Loading branch information
@neild
neild committed Feb 26, 2024
1 parent 34cc446 commit 591be7f
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 49 deletions.
25 changes: 25 additions & 0 deletions internal/quic/udp_darwin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,33 @@

package quic

import (
"encoding/binary"

"golang.org/x/sys/unix"
)

// See udp.go.
const (
udpECNSupport = true
udpInvalidLocalAddrIsError = true
)

// Confusingly, on Darwin the contents of the IP_TOS option differ depending on whether
// it is used as an inbound or outbound cmsg.

func parseIPTOS(b []byte) (ecnBits, bool) {
// Single byte. The low two bits are the ECN field.
if len(b) != 1 {
return 0, false
}
return ecnBits(b[0] & ecnMask), true
}

func appendCmsgECNv4(b []byte, ecn ecnBits) []byte {
// 32-bit integer.
// https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/netinet/in_tclass.c#L1062-L1073
b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 4)
binary.NativeEndian.PutUint32(data, uint32(ecn))
return b
}
20 changes: 20 additions & 0 deletions internal/quic/udp_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,28 @@

package quic

import (
"golang.org/x/sys/unix"
)

// See udp.go.
const (
udpECNSupport = true
udpInvalidLocalAddrIsError = false
)

// The IP_TOS socket option is a single byte containing the IP TOS field.
// The low two bits are the ECN field.

func parseIPTOS(b []byte) (ecnBits, bool) {
if len(b) != 1 {
return 0, false
}
return ecnBits(b[0] & ecnMask), true
}

func appendCmsgECNv4(b []byte, ecn ecnBits) []byte {
b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 1)
data[0] = byte(ecn)
return b
}
89 changes: 44 additions & 45 deletions internal/quic/udp_msg.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package quic

import (
"encoding/binary"
"net"
"net/netip"
"sync"
Expand Down Expand Up @@ -141,15 +142,11 @@ func parseControl(d *datagram, control []byte) {
case unix.IPPROTO_IP:
switch hdr.Type {
case unix.IP_TOS, unix.IP_RECVTOS:
// Single byte containing the IP TOS field.
// The low two bits are the ECN field.
//
// (Linux sets the type to IP_TOS, Darwin to IP_RECVTOS,
// jus check for both.)
if len(data) < 1 {
break
// just check for both.)
if ecn, ok := parseIPTOS(data); ok {
d.ecn = ecn
}
d.ecn = ecnBits(data[0] & ecnMask)
case unix.IP_PKTINFO:
if a, ok := parseInPktinfo(data); ok {
d.localAddr = netip.AddrPortFrom(a, d.localAddr.Port())
Expand All @@ -158,12 +155,11 @@ func parseControl(d *datagram, control []byte) {
case unix.IPPROTO_IPV6:
switch hdr.Type {
case unix.IPV6_TCLASS:
// Single byte containing the traffic class field.
// 32-bit integer containing the traffic class field.
// The low two bits are the ECN field.
if len(data) < 1 {
break
if ecn, ok := parseIPv6TCLASS(data); ok {
d.ecn = ecn
}
d.ecn = ecnBits(data[0] & ecnMask)
case unix.IPV6_PKTINFO:
if a, ok := parseIn6Pktinfo(data); ok {
d.localAddr = netip.AddrPortFrom(a, d.localAddr.Port())
Expand All @@ -173,27 +169,33 @@ func parseControl(d *datagram, control []byte) {
}
}

func parseInPktinfo(b []byte) (netip.Addr, bool) {
// struct in_pktinfo {
// unsigned int ipi_ifindex; /* send/recv interface index */
// struct in_addr ipi_spec_dst; /* Local address */
// struct in_addr ipi_addr; /* IP Header dst address */
// };
if len(b) != 12 {
return netip.Addr{}, false
// IPV6_TCLASS is specified by RFC 3542 as an int.

func parseIPv6TCLASS(b []byte) (ecnBits, bool) {
if len(b) != 4 {
return 0, false
}
return netip.AddrFrom4([4]byte(b[8:][:4])), true
return ecnBits(binary.NativeEndian.Uint32(b) & ecnMask), true
}

func parseIn6Pktinfo(b []byte) (netip.Addr, bool) {
// struct in6_pktinfo {
// struct in6_addr ipi6_addr; /* src/dst IPv6 address */
// unsigned int ipi6_ifindex; /* send/recv interface index */
// };
if len(b) != 20 {
func appendCmsgECNv6(b []byte, ecn ecnBits) []byte {
b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_TCLASS, 4)
binary.NativeEndian.PutUint32(data, uint32(ecn))
return b
}

// struct in_pktinfo {
// unsigned int ipi_ifindex; /* send/recv interface index */
// struct in_addr ipi_spec_dst; /* Local address */
// struct in_addr ipi_addr; /* IP Header dst address */
// };

// parseInPktinfo returns the destination address from an IP_PKTINFO.
func parseInPktinfo(b []byte) (dst netip.Addr, ok bool) {
if len(b) != 12 {
return netip.Addr{}, false
}
return netip.AddrFrom16([16]byte(b[:16])).Unmap(), true
return netip.AddrFrom4([4]byte(b[8:][:4])), true
}

// appendCmsgIPSourceAddrV4 appends an IP_PKTINFO setting the source address
Expand All @@ -210,31 +212,28 @@ func appendCmsgIPSourceAddrV4(b []byte, src netip.Addr) []byte {
return b
}

// appendCmsgIPSourceAddrV6 appends an IP_PKTINFO or IPV6_PKTINFO
// setting the source address for an outbound datagram.
// struct in6_pktinfo {
// struct in6_addr ipi6_addr; /* src/dst IPv6 address */
// unsigned int ipi6_ifindex; /* send/recv interface index */
// };

// parseIn6Pktinfo returns the destination address from an IPV6_PKTINFO.
func parseIn6Pktinfo(b []byte) (netip.Addr, bool) {
if len(b) != 20 {
return netip.Addr{}, false
}
return netip.AddrFrom16([16]byte(b[:16])).Unmap(), true
}

// appendCmsgIPSourceAddrV6 appends an IPV6_PKTINFO setting the source address
// for an outbound datagram.
func appendCmsgIPSourceAddrV6(b []byte, src netip.Addr) []byte {
// struct in6_pktinfo {
// struct in6_addr ipi6_addr; /* src/dst IPv6 address */
// unsigned int ipi6_ifindex; /* send/recv interface index */
// };
b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_PKTINFO, 20)
ip := src.As16()
copy(data[0:], ip[:])
return b
}

func appendCmsgECNv4(b []byte, ecn ecnBits) []byte {
b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 4)
data[0] = byte(ecn)
return b
}

func appendCmsgECNv6(b []byte, ecn ecnBits) []byte {
b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_TCLASS, 4)
data[0] = byte(ecn)
return b
}

// appendCmsg appends a cmsg with the given level, type, and size to b.
// It returns the new buffer, and the data section of the cmsg.
func appendCmsg(b []byte, level, typ int32, size int) (_, data []byte) {
Expand Down
17 changes: 13 additions & 4 deletions internal/quic/udp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import (
)

func TestUDPSourceUnspecified(t *testing.T) {
t.Skip("https://go.dev/issue/65906 - temporarily skipped pending fix")
// Send datagram with no source address set.
runUDPTest(t, func(t *testing.T, test udpTest) {
t.Logf("%v", test.dstAddr)
data := []byte("source unspecified")
if err := test.src.Write(datagram{
b: data,
Expand All @@ -34,7 +34,6 @@ func TestUDPSourceUnspecified(t *testing.T) {
}

func TestUDPSourceSpecified(t *testing.T) {
t.Skip("https://go.dev/issue/65906 - temporarily skipped pending fix")
// Send datagram with source address set.
runUDPTest(t, func(t *testing.T, test udpTest) {
data := []byte("source specified")
Expand All @@ -53,7 +52,6 @@ func TestUDPSourceSpecified(t *testing.T) {
}

func TestUDPSourceInvalid(t *testing.T) {
t.Skip("https://go.dev/issue/65906 - temporarily skipped pending fix")
// Send datagram with source address set to an address not associated with the connection.
if !udpInvalidLocalAddrIsError {
t.Skipf("%v: sending from invalid source succeeds", runtime.GOOS)
Expand All @@ -77,7 +75,6 @@ func TestUDPSourceInvalid(t *testing.T) {
}

func TestUDPECN(t *testing.T) {
t.Skip("https://go.dev/issue/65907 - temporarily skipped pending fix")
if !udpECNSupport {
t.Skipf("%v: no ECN support", runtime.GOOS)
}
Expand Down Expand Up @@ -125,6 +122,18 @@ func runUDPTest(t *testing.T, f func(t *testing.T, u udpTest)) {
spec = "unspec"
}
t.Run(fmt.Sprintf("%v/%v/%v", test.srcNet, test.dstNet, spec), func(t *testing.T) {
// See: https://go.googlesource.com/go/+/refs/tags/go1.22.0/src/net/ipsock.go#47
// On these platforms, conns with network="udp" cannot accept IPv6.
switch runtime.GOOS {
case "dragonfly", "openbsd":
if test.srcNet == "udp6" && test.dstNet == "udp" {
t.Skipf("%v: no support for mapping IPv4 address to IPv6", runtime.GOOS)
}
}
if runtime.GOARCH == "wasm" && test.srcNet == "udp6" {
t.Skipf("%v: IPv6 tests fail when using wasm fake net", runtime.GOARCH)
}

srcAddr := netip.AddrPortFrom(netip.MustParseAddr(test.srcAddr), 0)
srcConn, err := net.ListenUDP(test.srcNet, net.UDPAddrFromAddrPort(srcAddr))
if err != nil {
Expand Down

0 comments on commit 591be7f

Please sign in to comment.