Listener only allows one session per client addr

[Eudi4H]

Listener only supports one session per client address. (Let's say per IP:port tuple, though with custom net.PacketConn it could be something else.) The Listener.sessions map is keyed by address, and if Listener.packetInput sees a different conv than the one it knows about for an address, it closes the existing session and replaces it with a new one under the new conv.
https://github.com/xtaci/kcp-go/blob/v5.5.9/sess.go#L810-L813

I think it should be possible to have multiple sessions for the same client address, as long as the sessions use different convs.

Here is a patch to show what I mean. It makes Listener.sessions be keyed by (addr, conv), not just addr. I am not sure the patch is entirely correct, especially regarding FEC as I have never tried using that.
https://gist.github.com/Eudi4H/5e5bc37f1277f124e01553753a9736d0

Here is a demonstration program. It creates a net.PacketConn and starts a UDPSession on it. It uses the UDPSession for five seconds, then creates a second UDPSession using the same net.PacketConn. When the second UDPSession connects, the first UDPSession stops working with an io: read/write on closed pipe error.

Possibly related but I'm not sure: #54 .

go.mod

module example.com/kcp-simul

require (
	github.com/klauspost/cpuid v1.2.3 // indirect
	github.com/pkg/errors v0.9.1 // indirect
	github.com/tjfoc/gmsm v1.3.0 // indirect
	github.com/xtaci/kcp-go/v5 v5.5.9
	golang.org/x/crypto v0.0.0-20200311171314-f7b00557c8c4 // indirect
	golang.org/x/net v0.0.0-20200301022130-244492dfa37a // indirect
)

package main

import (
	"fmt"
	"io"
	"net"
	"os"
	"time"

	"github.com/xtaci/kcp-go/v5"
)

func acceptSessions(ln *kcp.Listener) error {
	for {
		conn, err := ln.AcceptKCP()
		if err != nil {
			return err
		}
		go func() {
			defer conn.Close()
			n, err := io.Copy(os.Stdout, conn)
			fmt.Printf("%08x io.Copy -> %v, %v\n", conn.GetConv(), n, err)
		}()
	}
}

func clientLoop(s string, conn *kcp.UDPSession) error {
	for {
		n, err := conn.Write([]byte(s))
		fmt.Printf("%08x Write -> %v, %v\n", conn.GetConv(), n, err)
		if err != nil {
			return err
		}
		time.Sleep(1 * time.Second)
	}
}

func main() {
	serverConn, err := net.ListenPacket("udp", "127.0.0.1:0")
	if err != nil {
		panic(err)
	}
	defer serverConn.Close()
	ln, err := kcp.ServeConn(nil, 0, 0, serverConn)
	if err != nil {
		panic(err)
	}
	go acceptSessions(ln)

	// Create a net.PacketConn to give to kcp.NewConn2. We will use the same
	// net.PacketConn for two KCP connections.
	clientConn, err := net.ListenPacket("udp", ":0")
	if err != nil {
		panic(err)
	}

	clientA, err := kcp.NewConn3(0xaaaaaaaa, serverConn.LocalAddr(), nil, 0, 0, clientConn)
	if err != nil {
		panic(err)
	}
	go clientLoop("clientA\n", clientA)

	time.Sleep(5 * time.Second)

	clientB, err := kcp.NewConn3(0xbbbbbbbb, serverConn.LocalAddr(), nil, 0, 0, clientConn)
	if err != nil {
		panic(err)
	}
	clientLoop("clientB\n", clientB)
}

Without the patch, the output is

aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
bbbbbbbb Write -> 8, <nil>
aaaaaaaa io.Copy -> 40, io: read/write on closed pipe
clientB
aaaaaaaa Write -> 8, <nil>
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>

With the patch, the output is

aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
aaaaaaaa Write -> 8, <nil>
clientA
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
clientA
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
clientA
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
clientA
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
clientA
bbbbbbbb Write -> 8, <nil>
clientB
aaaaaaaa Write -> 8, <nil>
clientA

[Eudi4H]

Here is some background on my use case if you're curious: https://bugs.torproject.org/33519#comment:3. We are implementing a proxy and for us, creating a new net.PacketConn is expensive--it can take around 30 seconds--so we don't want to make a new one for every new outgoing connection. We would prefer to be able to make just one net.PacketConn (with an abstract random string in place of an IP:port address) and share it among all our outgoing connections.

The situation I am picturing is one PacketConn, multiple UDPSessions, one smux.Stream per UDPSession. An alternative design is one PacketConn, one UDPSession, multiple smux.Streams in the UDPSession. That second design may also work, but we would need a plan for what to do if the global UDPSession ever fails.

[xtaci]

could you make a PR to this and the other packetconn issue?

[xtaci]

I need to do some tests

[xtaci]

For this issue, I have doubts on the reliability on this conv field to multiplex streams into a connection, will conv be duplicated for some time later, or will there be a problem with FEC?

So I chose to create a overlay, which was https://github.com/xtaci/smux.

What's your thought?

[Eudi4H]

While I was working on a pull request (https://github.com/Eudi4H/kcp-go/tree/listener-multi-sessions), I realized that the approach I suggested (keying by (addr, conv)) won't work. It makes the sample program above work, but it doesn't work in the more detailed test I wrote. The reason is that both client UDPSessions run their own readLoop and read from the net.PacketConn in an uncoordinated way. Each one may read packets that are intended for the other. If both connections happen to get past the handshake, retransmissions may compensate for the missing packets, but performance will be poor.

So I think you are right. We will have to find a way to do it using multiple smux streams per UDPSession. (We are already using smux, but currently only with one stream per UDPSession.) Thanks for the consideration.