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

Jump to bottom

x/sys/unix: missing function to marshal a Cmsghdr #59653

Open
marten-seemann opened this issue Apr 16, 2023 · 2 comments
Open

x/sys/unix: missing function to marshal a Cmsghdr #59653

marten-seemann opened this issue Apr 16, 2023 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@marten-seemann
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.20.3 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/src/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/bin/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/bin/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1782644412=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I'm implementing UDP GSO for quic-go (tracking issue). Since quic-go does DPLPMTUD, a server will send packets with different sizes to different clients. Therefore, we can't just set a single packet size per UDP socket. Instead, we have to pass the desired packet size to the kernel with every sendmsg call.

In Go, this is done using the WriteMsgUDP function on the net.UDPConn. The size is encoded as a control message, which is passed in the oob []byte parameter to that function:

cm = CMSG_FIRSTHDR(&msg);
cm->cmsg_level = SOL_UDP;
cm->cmsg_type = UDP_SEGMENT;
cm->cmsg_len = CMSG_LEN(sizeof(uint16_t));
*((uint16_t *) CMSG_DATA(cm)) = gso_size;

Proof of concept here: https://gist.github.com/marten-seemann/a549773b53f30960b966a9f4068b6e48#file-gso-go-L45-L50

What did you expect to see?

The proof of concept works, as can be confirmed by running tcpdump (e.g. tcpdump host 8.8.8.8 -n -v -i eth0).

What did you see instead?

Serializing the control message has to be done by hand (see the getCmsg function in my PoC). The precise format of the control message is highly dependent on the architecture, and my PoC will only work on amd64 (see definition). Other architectures serialize this message slightly different, there are 41 (!) architecture-dependent ztypes_ files in the unix package that define the Cmsghdr.

x/sys/unix already provides a deserialization function (via ParseOneSocketControlMessage / ParseSocketControlMessage). It would it would provide some kind of serialization function that works on all architectures as well.
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 16, 2023
@gopherbot gopherbot added this to the Unreleased milestone Apr 16, 2023
@marten-seemann
Copy link
Contributor Author

marten-seemann commented Apr 16, 2023 •
edited

Here's a proposal for an API:

Since this function is potentially called on every WriteMsgUDP call (although for the GSO use case, the result could obviously be cached for every packet size), it's essential that it doesn't cause any allocations. I therefore propose an append-style API:

func (Cmsghdr) Append(b []byte, data []byte) []byte

For example, for amd64, this would result in:

type Cmsghdr struct {
	Len   uint64
	Level int32
	Type  int32
}

func (h Cmsghdr) Append(b []byte, data []byte) []byte {
	b = binary.LittleEndian.AppendUint64(b, h.Len + uint64(len(data)))
	b = binary.LittleEndian.AppendUint32(b, int32(h.Level))
	b = binary.LittleEndian.AppendUint32(b, int32(h.Type))
	return append(b, data...)
}

This could also be exposed on the SocketControlMessage, e.g.

func (m SocketControlMessage) Append(b []byte) []byte {
    return m.Header.Append(b, m.Data)
}

@marten-seemann marten-seemann mentioned this issue Apr 16, 2023
Closed
@rittneje
Copy link

rittneje commented Apr 16, 2023 •
edited

x/sys/unix already supports most of what you need. See UnixRights for an example.

For your use case I believe it would be:

datalen := 2 // your payload is a uint16, which is 2 bytes
b := make([]byte, unix.CmsgSpace(datalen))
h := (*unix.Cmsghdr)(unsafe.Pointer(&b[0]))
h.Level = syscall.IPPROTO_UDP
h.Type = UDP_SEGMENT
h.SetLen(unix.CmsgLen(datalen))

// UnixRights uses the private `data` method, but I *think* this achieves the same goal.
offset := unix.CmsgSpace(0)
*(*uint16)(unsafe.Pointer(&b[offset])) = uint16(size)

return b

Since your message is always the same length, you could choose to allocate the whole byte slice once, and just overwrite the uint16 payload each time.

@mknyszek mknyszek added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest help wanted labels Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mknyszek @marten-seemann @gopherbot @rittneje