Skip to content

Commit 591be7f

Browse files
neildneild
committed
quic: fix UDP on big-endian Linux, tests on various architectures
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
1 parent 34cc446 commit 591be7f

File tree

4 files changed

+102
-49
lines changed

4 files changed

+102
-49
lines changed

‎internal/quic/udp_darwin.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,33 @@
66

77
package quic
88

9+
import (
10+
"encoding/binary"
11+
12+
"golang.org/x/sys/unix"
13+
)
14+
915
// See udp.go.
1016
const (
1117
udpECNSupport = true
1218
udpInvalidLocalAddrIsError = true
1319
)
20+
21+
// Confusingly, on Darwin the contents of the IP_TOS option differ depending on whether
22+
// it is used as an inbound or outbound cmsg.
23+
24+
func parseIPTOS(b []byte) (ecnBits, bool) {
25+
// Single byte. The low two bits are the ECN field.
26+
if len(b) != 1 {
27+
return 0, false
28+
}
29+
return ecnBits(b[0] & ecnMask), true
30+
}
31+
32+
func appendCmsgECNv4(b []byte, ecn ecnBits) []byte {
33+
// 32-bit integer.
34+
// https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/netinet/in_tclass.c#L1062-L1073
35+
b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 4)
36+
binary.NativeEndian.PutUint32(data, uint32(ecn))
37+
return b
38+
}

‎internal/quic/udp_linux.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,28 @@
66

77
package quic
88

9+
import (
10+
"golang.org/x/sys/unix"
11+
)
12+
913
// See udp.go.
1014
const (
1115
udpECNSupport = true
1216
udpInvalidLocalAddrIsError = false
1317
)
18+
19+
// The IP_TOS socket option is a single byte containing the IP TOS field.
20+
// The low two bits are the ECN field.
21+
22+
func parseIPTOS(b []byte) (ecnBits, bool) {
23+
if len(b) != 1 {
24+
return 0, false
25+
}
26+
return ecnBits(b[0] & ecnMask), true
27+
}
28+
29+
func appendCmsgECNv4(b []byte, ecn ecnBits) []byte {
30+
b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 1)
31+
data[0] = byte(ecn)
32+
return b
33+
}

‎internal/quic/udp_msg.go

Lines changed: 44 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package quic
88

99
import (
10+
"encoding/binary"
1011
"net"
1112
"net/netip"
1213
"sync"
@@ -141,15 +142,11 @@ func parseControl(d *datagram, control []byte) {
141142
case unix.IPPROTO_IP:
142143
switch hdr.Type {
143144
case unix.IP_TOS, unix.IP_RECVTOS:
144-
// Single byte containing the IP TOS field.
145-
// The low two bits are the ECN field.
146-
//
147145
// (Linux sets the type to IP_TOS, Darwin to IP_RECVTOS,
148-
// jus check for both.)
149-
if len(data) < 1 {
150-
break
146+
// just check for both.)
147+
if ecn, ok := parseIPTOS(data); ok {
148+
d.ecn = ecn
151149
}
152-
d.ecn = ecnBits(data[0] & ecnMask)
153150
case unix.IP_PKTINFO:
154151
if a, ok := parseInPktinfo(data); ok {
155152
d.localAddr = netip.AddrPortFrom(a, d.localAddr.Port())
@@ -158,12 +155,11 @@ func parseControl(d *datagram, control []byte) {
158155
case unix.IPPROTO_IPV6:
159156
switch hdr.Type {
160157
case unix.IPV6_TCLASS:
161-
// Single byte containing the traffic class field.
158+
// 32-bit integer containing the traffic class field.
162159
// The low two bits are the ECN field.
163-
if len(data) < 1 {
164-
break
160+
if ecn, ok := parseIPv6TCLASS(data); ok {
161+
d.ecn = ecn
165162
}
166-
d.ecn = ecnBits(data[0] & ecnMask)
167163
case unix.IPV6_PKTINFO:
168164
if a, ok := parseIn6Pktinfo(data); ok {
169165
d.localAddr = netip.AddrPortFrom(a, d.localAddr.Port())
@@ -173,27 +169,33 @@ func parseControl(d *datagram, control []byte) {
173169
}
174170
}
175171

176-
func parseInPktinfo(b []byte) (netip.Addr, bool) {
177-
// struct in_pktinfo {
178-
// unsigned int ipi_ifindex; /* send/recv interface index */
179-
// struct in_addr ipi_spec_dst; /* Local address */
180-
// struct in_addr ipi_addr; /* IP Header dst address */
181-
// };
182-
if len(b) != 12 {
183-
return netip.Addr{}, false
172+
// IPV6_TCLASS is specified by RFC 3542 as an int.
173+
174+
func parseIPv6TCLASS(b []byte) (ecnBits, bool) {
175+
if len(b) != 4 {
176+
return 0, false
184177
}
185-
return netip.AddrFrom4([4]byte(b[8:][:4])), true
178+
return ecnBits(binary.NativeEndian.Uint32(b) & ecnMask), true
186179
}
187180

188-
func parseIn6Pktinfo(b []byte) (netip.Addr, bool) {
189-
// struct in6_pktinfo {
190-
// struct in6_addr ipi6_addr; /* src/dst IPv6 address */
191-
// unsigned int ipi6_ifindex; /* send/recv interface index */
192-
// };
193-
if len(b) != 20 {
181+
func appendCmsgECNv6(b []byte, ecn ecnBits) []byte {
182+
b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_TCLASS, 4)
183+
binary.NativeEndian.PutUint32(data, uint32(ecn))
184+
return b
185+
}
186+
187+
// struct in_pktinfo {
188+
// unsigned int ipi_ifindex; /* send/recv interface index */
189+
// struct in_addr ipi_spec_dst; /* Local address */
190+
// struct in_addr ipi_addr; /* IP Header dst address */
191+
// };
192+
193+
// parseInPktinfo returns the destination address from an IP_PKTINFO.
194+
func parseInPktinfo(b []byte) (dst netip.Addr, ok bool) {
195+
if len(b) != 12 {
194196
return netip.Addr{}, false
195197
}
196-
return netip.AddrFrom16([16]byte(b[:16])).Unmap(), true
198+
return netip.AddrFrom4([4]byte(b[8:][:4])), true
197199
}
198200

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

213-
// appendCmsgIPSourceAddrV6 appends an IP_PKTINFO or IPV6_PKTINFO
214-
// setting the source address for an outbound datagram.
215+
// struct in6_pktinfo {
216+
// struct in6_addr ipi6_addr; /* src/dst IPv6 address */
217+
// unsigned int ipi6_ifindex; /* send/recv interface index */
218+
// };
219+
220+
// parseIn6Pktinfo returns the destination address from an IPV6_PKTINFO.
221+
func parseIn6Pktinfo(b []byte) (netip.Addr, bool) {
222+
if len(b) != 20 {
223+
return netip.Addr{}, false
224+
}
225+
return netip.AddrFrom16([16]byte(b[:16])).Unmap(), true
226+
}
227+
228+
// appendCmsgIPSourceAddrV6 appends an IPV6_PKTINFO setting the source address
229+
// for an outbound datagram.
215230
func appendCmsgIPSourceAddrV6(b []byte, src netip.Addr) []byte {
216-
// struct in6_pktinfo {
217-
// struct in6_addr ipi6_addr; /* src/dst IPv6 address */
218-
// unsigned int ipi6_ifindex; /* send/recv interface index */
219-
// };
220231
b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_PKTINFO, 20)
221232
ip := src.As16()
222233
copy(data[0:], ip[:])
223234
return b
224235
}
225236

226-
func appendCmsgECNv4(b []byte, ecn ecnBits) []byte {
227-
b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 4)
228-
data[0] = byte(ecn)
229-
return b
230-
}
231-
232-
func appendCmsgECNv6(b []byte, ecn ecnBits) []byte {
233-
b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_TCLASS, 4)
234-
data[0] = byte(ecn)
235-
return b
236-
}
237-
238237
// appendCmsg appends a cmsg with the given level, type, and size to b.
239238
// It returns the new buffer, and the data section of the cmsg.
240239
func appendCmsg(b []byte, level, typ int32, size int) (_, data []byte) {

‎internal/quic/udp_test.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ import (
1616
)
1717

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

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

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

7977
func TestUDPECN(t *testing.T) {
80-
t.Skip("https://go.dev/issue/65907 - temporarily skipped pending fix")
8178
if !udpECNSupport {
8279
t.Skipf("%v: no ECN support", runtime.GOOS)
8380
}
@@ -125,6 +122,18 @@ func runUDPTest(t *testing.T, f func(t *testing.T, u udpTest)) {
125122
spec = "unspec"
126123
}
127124
t.Run(fmt.Sprintf("%v/%v/%v", test.srcNet, test.dstNet, spec), func(t *testing.T) {
125+
// See: https://go.googlesource.com/go/+/refs/tags/go1.22.0/src/net/ipsock.go#47
126+
// On these platforms, conns with network="udp" cannot accept IPv6.
127+
switch runtime.GOOS {
128+
case "dragonfly", "openbsd":
129+
if test.srcNet == "udp6" && test.dstNet == "udp" {
130+
t.Skipf("%v: no support for mapping IPv4 address to IPv6", runtime.GOOS)
131+
}
132+
}
133+
if runtime.GOARCH == "wasm" && test.srcNet == "udp6" {
134+
t.Skipf("%v: IPv6 tests fail when using wasm fake net", runtime.GOARCH)
135+
}
136+
128137
srcAddr := netip.AddrPortFrom(netip.MustParseAddr(test.srcAddr), 0)
129138
srcConn, err := net.ListenUDP(test.srcNet, net.UDPAddrFromAddrPort(srcAddr))
130139
if err != nil {

0 commit comments

Comments
 (0)