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

x/net/ipv4: SetTOS should return errNotImplemented on Windows #67474

Open
eelcocramer opened this issue May 17, 2024 · 5 comments
Open

x/net/ipv4: SetTOS should return errNotImplemented on Windows #67474

eelcocramer opened this issue May 17, 2024 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@eelcocramer
Copy link

eelcocramer commented May 17, 2024

Go version

go version go1.22.3 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN='/Users/gopher/go/bin'
GOCACHE='/Users/gopher/Library/Caches/go-build'
GOENV='/Users/gopher/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/gopher/go/pkg/mod'
GOOS='darwin'
GOPATH='/Users/gopher/go'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/gopher/reproduce/go.mod'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/68/ch3mzzs55bz8x9mzk1dsd35c0000gn/T/go-build1659040423=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I cross build on MacOS for Windows so the version and env above is all output for MacOS but this issue is created for the Windows implementation of SetTOS.

See the following code:

package main

import (
	"flag"
	"log"
	"net"

	"golang.org/x/net/ipv4"
)

func main() {
	name := flag.String("i", "Ethernet", "interface name")
	flag.Parse()

	iface, err := net.InterfaceByName(*name)
	if err != nil {
		log.Fatalf("%v", err)
	}

	group := &net.UDPAddr{IP: net.ParseIP("224.0.0.1"), Port: 1234}
	conn, err := net.ListenPacket("udp4", "224.0.0.1:1234")
	if err != nil {
		log.Fatalf("%v", err)
	}

	p := ipv4.NewPacketConn(conn)
	p.SetMulticastInterface(iface)

	if err := p.JoinGroup(iface, &net.UDPAddr{IP: group.IP}); err != nil {
		log.Fatalf("%v", err)
	}

	err = p.SetTOS(0x40)
	if err != nil {
		log.Fatalf("%v", err)
	}
	_, err = p.WriteTo([]byte("test1"), nil, group)
	if err != nil {
		log.Fatalf("%v", err)
	}
}

What did you see happen?

The code runs successful on Windows but does not set the TOS value on the outgoing UDP packet.

What did you expect to see?

According to the document that is referenced by the source code the IP_TOS is described as Do not use as the TOS cannot be set programmatically.

I would think it would be better to remove this line from the code and make calling SetTOS on Windows fail with an error because currently the call seems to succeed without any effect to the actual packet.

@gopherbot gopherbot added this to the Unreleased milestone May 17, 2024
@dmitshur
Copy link
Contributor

CC @golang/windows, @ianlancetaylor.

@dmitshur dmitshur added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 17, 2024
@bjorndm
Copy link

bjorndm commented May 17, 2024

It seems the TOS field itself is obsolete: https://en.m.wikipedia.org/wiki/Differentiated_services

It should be replaced by a combination of the DS field and ECN field which winsock does support : https://learn.microsoft.com/en-us/windows/win32/winsock/winsock-ecn

@eelcocramer
Copy link
Author

Fair point. Actually what I am trying to achieve is to set a value on the DS field part of the DS/ECN byte.

The TOS field has been absolute well before Go development was even started ;-). TOS and DS/ECN refer to the same byte in the IP header. In the MS document you refer is mentioned how to set the ECN field using winsock but it does not mention setting the DS field of the byte?

@bjorndm
Copy link

bjorndm commented May 18, 2024

It seems to set the TOS bits directly since windows2000 you need admin privileges. To set the diffserve quality of service there is a different API https://learn.microsoft.com/en-us/windows/win32/api/qosobjs/ns-qosobjs-qos_diffserv

@eelcocramer
Copy link
Author

I tried running the code with admin privileges on windows 10 but the TOS bits were not set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

4 participants