Skip to content

Add ListenPacketConn to accept connections on a supplied PacketConn - #837

Open
folbricht wants to merge 1 commit into
pion:mainfrom
folbricht:expose-listen-socket
Open

Add ListenPacketConn to accept connections on a supplied PacketConn#837
folbricht wants to merge 1 commit into
pion:mainfrom
folbricht:expose-listen-socket

Conversation

@folbricht

@folbricht folbricht commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Description

Adds ListenPacketConn and ListenPacketConnWithOptions, which create a DTLS listener that accepts connections on a caller-supplied net.PacketConn instead of opening a socket itself.

This complements #802: WithListenConfig covers socket options via Control, but some use cases need the socket created entirely outside the library, e.g. a socket created in another network namespace and passed over SCM_RIGHTS, or one bound/configured by other means.

Following the review discussion, this makes the supplied-connection path a first-class entry point rather than a ServerOption. Compared to the earlier WithPacketConn approach, there are no ignored network/address arguments and no interaction with WithListenConfig. The split mirrors crypto/tls, where tls.NewListener wraps an existing listener and tls.Listen is the convenience that opens the socket, and is similar to quic.Listen taking a net.PacketConn.

Internally, udp.ListenConfig gains a ListenPacketConn method holding the listener construction, with Listen reduced to opening the socket and delegating to it. The internal udp.listener only uses net.PacketConn methods, so its pConn field is widened from *net.UDPConn, which also makes the *net.UDPConn assertion after ListenPacket unnecessary.

The listener takes ownership of the supplied connection and closes it on Close. The existing public API is unchanged, so this is purely additive and no longer depends on a breaking release.

@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.75758% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.67%. Comparing base (179c353) to head (9d7e893).
⚠️ Report is 74 commits behind head on main.

Files with missing lines Patch % Lines
listener.go 70.37% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #837      +/-   ##
==========================================
+ Coverage   82.35%   82.67%   +0.32%     
==========================================
  Files         123      123              
  Lines        6999     7015      +16     
==========================================
+ Hits         5764     5800      +36     
+ Misses        820      807      -13     
+ Partials      415      408       -7     
Flag Coverage Δ
go 82.67% <75.75%> (+0.32%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@folbricht
folbricht force-pushed the expose-listen-socket branch from e5ecb0c to d1ee218 Compare June 2, 2026 07:31
@folbricht

Copy link
Copy Markdown
Contributor Author

The test-i386 (1.24) failure looks like a pre-existing flake rather than a regression from this change:

  • The failing test differs between runs (TestPionE2EChaCha20Poly1305RSACID, then TestPionE2ESimpleRSA), and the underlying error is handshake error: context deadline exceeded — the RSA handshake not completing in time on emulated i386.
  • The same job failed the same way on main in April (run 24338770289, TestPionE2ESimpleRSA, panic: timeout).
  • i386 with Go 1.25 passes consistently, as do all other platforms.
  • I also stress-tested locally under GOARCH=386 (47 runs of main, 15 runs of the changed listener code, interleaved): no failures attributable to the change.

@JoTurk

JoTurk commented Jun 2, 2026

Copy link
Copy Markdown
Member

@folbricht Hello we can maybe do a better API and break the current one, and keep this PR targeted at main or target it against v3 and merge it now, if you need this feature before waiting for dtls 1.3 release, what do you think?

@folbricht

Copy link
Copy Markdown
Contributor Author

Either works for me. Just let me know what you prefer and how I can update it. There's no urgency to it.

@folbricht
folbricht force-pushed the expose-listen-socket branch from d1ee218 to 7834c87 Compare June 4, 2026 07:29
@folbricht folbricht changed the title Add PacketConn option to Listener Add ListenPacketConn to accept connections on a supplied PacketConn Jun 4, 2026
@folbricht

folbricht commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@JoTurk Going with the first option, keeping this targeted at main. I reworked the PR so the supplied connection is a first-class entry point (ListenPacketConn / ListenPacketConnWithOptions) instead of a ServerOption. That removes the ignored network/address arguments and the interaction with WithListenConfig. The result ended up purely additive, so it doesn't strictly require a breaking release after all. Happy to adjust if you had a different API shape in mind.

This is what I went with:

  // New: supplied-conn path as first-class functions
  func ListenPacketConn(pconn net.PacketConn, config *Config) (net.Listener, error)
  func ListenPacketConnWithOptions(pconn net.PacketConn, opts ...ServerOption) (net.Listener, error)

  // Unchanged: address-based listening keeps its name and signature
  func Listen(network string, laddr *net.UDPAddr, config *Config) (net.Listener, error)         // Deprecated
  func ListenWithOptions(network string, laddr *net.UDPAddr, opts ...ServerOption) (net.Listener, error)

  // Unchanged: wrapping an existing demuxing PacketListener
  func NewListener(inner dtlsnet.PacketListener, config *Config) (net.Listener, error)          // Deprecated
  func NewListenerWithOptions(inner dtlsnet.PacketListener, opts ...ServerOption) (net.Listener, error)

If you prefer, this could work as well, just let me know:

 // The DTLS analogue would have been:
  func Listen(pconn net.PacketConn, config *Config) (net.Listener, error)                       // BREAKING: same name, new meaning
  func ListenWithOptions(pconn net.PacketConn, opts ...ServerOption) (net.Listener, error)      // BREAKING

  func ListenAddr(network string, laddr *net.UDPAddr, config *Config) (net.Listener, error)     // old behavior, new name
  func ListenAddrWithOptions(network string, laddr *net.UDPAddr, opts ...ServerOption) (net.Listener, error)

@folbricht
folbricht force-pushed the expose-listen-socket branch from 7834c87 to 9d7e893 Compare June 4, 2026 07:35
@przemyslaw0

Copy link
Copy Markdown

Could some collaborator in this repository review this PR?

@JoTurk

JoTurk commented Jun 9, 2026

Copy link
Copy Markdown
Member

@przemyslaw0 Sorry I saw your email the other day, I was going to response today, it takes time to introduce new APIs because it's not something we can revert easily in the future, and network APIs are tricky because we would want to normalize them across ice, stun etc
We also have a similar issue in #766 and i would rather if we introduce a single API to fix both issues.

@folbricht

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. I looked at #766 to see whether a single API could cover both. My take: the two issues only share one concept, "the caller controls the packet path", and they need it at different layers.

What each issue needs:

  • This PR needs the listener to accept a caller-created socket instead of opening one. Nothing about packet contents. The client side already supports this since Client/Server take a conn.
  • Allow intercepting and injecting DTLS packets #766 needs three things: observe and optionally divert outgoing handshake packets, inject inbound packets into the read path, and know flight boundaries (to invalidate a flight on timeout and resend). The first two are expressible at the transport layer; the flight signal is not, since it only exists inside writeHandshakePackets.

Why a single mechanism is hard to share directly: a Config callback approach (what #766 currently does) can't help here because this PR's problem is socket creation, not packet observation. And "just wrap a net.PacketConn" almost works for #766, but fippo's objections were concrete: the wrapper has to re-parse every datagram to classify it, and flight boundaries aren't observable at the wire level at all.

A unified design that covers both:

  1. ListenPacketConn (this PR) as the "bring your own transport" primitive for the listener, completing the matrix with Client/Server. This mirrors quic.Listen(net.PacketConn) and the crypto/tls split between NewListener (wraps) and Listen (opens).

  2. For Allow intercepting and injecting DTLS packets #766, instead of Config callbacks and InjectInboundPacket on Conn, an optional interface in pkg/net that a supplied conn can implement:

    // HandshakeWriter is optionally implemented by a PacketConn supplied to
    // Client/Server/ListenPacketConn. When implemented, handshake records are
    // written through it instead of WriteTo, with end marking the last
    // packet of a flight.
    type HandshakeWriter interface {
    	WriteHandshakeTo(p []byte, addr net.Addr, end bool) (int, error)
    }

    With that, outbound interception is the wrapper's WriteHandshakeTo (diverting is simply not forwarding to the inner conn), the flight-end flag comes for free, and inbound injection needs no API at all: once the caller owns the conn, ReadFrom returning an injected packet is injection. The library-side change is a single interface check in writeHandshakePackets, the pattern is standard Go interface upgrading (http.Flusher, io.ReaderFrom), and Conn and Config stay untouched. pion could ship a ready-made dtlsnet.InterceptPacketConn with callbacks so the Allow intercepting and injecting DTLS packets #766 use case is one constructor call.

  3. If you want something that normalizes across ice/stun/turn as well, the bigger-hammer version is a quic-go-style Transport:

    type Transport struct {
    	Conn net.PacketConn
    }
    func (t *Transport) Listen(config *Config) (net.Listener, error)
    func (t *Transport) Dial(ctx context.Context, raddr net.Addr, config *Config) (*Conn, error)

    That's a shape every pion library could adopt. But it's orthogonal rather than an alternative: Allow intercepting and injecting DTLS packets #766's flight signal still needs the optional interface either way, and ListenPacketConn would just become Transport.Listen internally.

One honest caveat: with the wrapper approach, classifying inbound datagrams still requires peeking at the record header content type (one byte per record), since there's no inbound equivalent of writeHandshakePackets. If #766 needs a zero-parse inbound notification too, that would be a second optional interface, at which point the weight starts to approach the callback design. Worth deciding how much #766 actually needs there.

So my suggestion: this PR is the first half of the unified design rather than something it would replace. It's purely additive, and the #766 work can layer on top of the supplied-conn path with one small optional interface. Happy to adjust the shape here if you'd prefer the Transport framing instead.

@JoTurk

JoTurk commented Jun 9, 2026

Copy link
Copy Markdown
Member

If you want something that normalizes across ice/stun/turn as well, the bigger-hammer version is a quic-go-style Transport:

Pion has pion/transport, which defines similar network abstraction interfaces and includes vnet, but that does not cleanly solve this DTLS issue by itself. The missing piece is a DTLS server listener that can accept an existing/multiplexed net.PacketConn and still provide listener-style accept or similar.

@przemyslaw0

Copy link
Copy Markdown

@JoTurk No problem, sorry, I'm really looking forward for support for setting socket options in listener. No hurry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants