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

Add option to increase SO_RCVBUF on dnstap listen socket #61

Closed
tarko opened this issue Mar 27, 2022 · 7 comments
Closed

Add option to increase SO_RCVBUF on dnstap listen socket #61

tarko opened this issue Mar 27, 2022 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@tarko
Copy link

tarko commented Mar 27, 2022

hey,

Unbound is super aggressive (for good reasons) in closing the blocking TCP sockets should there be even a tiny receive queue on collector side (due to IOPS peaks, for example).

Perhaps add config option to manually set setReadBuffer on the dnstap collector listening socket so kernel could at least buffer more data in such scenarios?

@dmachard dmachard modified the milestones: 0.17.0, 0.18.0 Mar 28, 2022
@dmachard dmachard added the enhancement New feature or request label Mar 28, 2022
@dmachard
Copy link
Owner

dmachard commented Apr 8, 2022

good idea, I need to check if it's easy to implement.

@dmachard dmachard removed this from the 0.18.0 milestone Apr 17, 2022
@dmachard
Copy link
Owner

I am not an expert in this topic, can you help me to implement that ?
Can you confirm that if you change this value, you need to be < to the max value defined by the OS ?

@dmachard dmachard added the help wanted Extra attention is needed label Jul 30, 2022
@tarko
Copy link
Author

tarko commented Aug 1, 2022

You can set any value without error and if you exceed the net.core.rmem_max then it'll just get clamped to max value. Good programs produce a warning if you get clamped so operator can adjust settings.

Golang socket API does not really make it easy to set and get socket options. Rought PoC I hacked together:

package main

import (
	"fmt"
	"net"
	"syscall"
	"context"
)

func main() {
	lc := net.ListenConfig{
		Control: func(network, address string, conn syscall.RawConn) error {
			conn.Control(func(fd uintptr) {
				desired :=  1024 * 1024 * 100
				syscall.SetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF, desired)
				actual, _ := syscall.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF)
				fmt.Println(desired, actual)
			})
			return nil
		},
	}

	//listener, _ := net.Listen("tcp", "0.0.0.0:1234")
	listener, _ := lc.Listen(context.Background(), "tcp", "0.0.0.0:1234")

	fmt.Println(listener)

	for {
		conn, _ := listener.Accept()
		fmt.Println(conn, conn.(*net.TCPConn))
		//conn.(*net.TCPConn).SetReadBuffer(1024 * 1024 * 100)
	}
}

With net.core.rmem_max = 16000000 it'll get clamped to 32000000 (double value is expected, other half of the memory is used for socket metadata) and you can also inspect actual socket receive buffer:

$ ss -n -p -m -l '( sport = 1234 or dport = 1234 )'
Netid       State        Recv-Q       Send-Q               Local Address:Port               Peer Address:Port       Process       
tcp         LISTEN       0            4096                             *:1234                          *:*           users:(("test",pid=1207135,fd=3))
         skmem:(r0,rb32000000,t0,tb16384,f0,w0,o0,bl0,d0)

Note that TCPConn interface also provides SetReadBuffer but you have to adjust this for every accepted socket and there is no way to get the actual value so no way to produce a warning.

@tarko
Copy link
Author

tarko commented Aug 1, 2022

Some other languages make this pretty easy, example from my own code :)

                        rcvbuf = 134217728
                        @sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, rcvbuf)
                        if (actual = @sock.getsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF).int) < rcvbuf
                                Logger.warn("Unable to set SO_RCVBUF to %d, actual %d" % [rcvbuf, actual])
                        end

@dmachard
Copy link
Owner

FYI, the following adaptation of your proposal will be implement on next release

// Configure SO_RCVBUF, thanks to https://github.com/dmachard/go-dns-collector/issues/61#issuecomment-1201199895
func SetSock_RCVBUF(conn net.Conn, desired int, is_tls bool) (int, int, error) {
	var file *os.File
	var err error
	if is_tls {
		tlsConn := conn.(*tls.Conn).NetConn()
		file, err = tlsConn.(*net.TCPConn).File()
		if err != nil {
			return 0, 0, err
		}
	} else {
		file, err = conn.(*net.TCPConn).File()
		if err != nil {
			return 0, 0, err
		}
	}

	// get the before value
	before, err := syscall.GetsockoptInt(int(file.Fd()), syscall.SOL_SOCKET, syscall.SO_RCVBUF)
	if err != nil {
		return 0, 0, err
	}

	// set the new one and check the new actual value
	syscall.SetsockoptInt(int(file.Fd()), syscall.SOL_SOCKET, syscall.SO_RCVBUF, desired)
	actual, err := syscall.GetsockoptInt(int(file.Fd()), syscall.SOL_SOCKET, syscall.SO_RCVBUF)
	if err != nil {
		return 0, 0, err
	}
	return before, actual, nil
}

Output:
INFO: 2023/02/24 18:41:33.830156 [tap] dnstap collector - set SO_RCVBUF option, value before: 131072, desired: 104857600, actual: 425984

@dmachard dmachard added this to the v0.29.0 milestone Feb 25, 2023
@tarko
Copy link
Author

tarko commented Feb 26, 2023

Thanks, looks good to me

ESTAB     0          0               [::ffff:.1.2.3.4]:6000            [::ffff:5.6.7.8]:54756     
         skmem:(r0,rb268435456,t0,tb87040,f36864,w0,o0,bl0,d0)     

@dmachard
Copy link
Owner

dmachard commented Mar 5, 2023

Implemented in v0.29. 0 release

@dmachard dmachard closed this as completed Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants