Skip to content

[client] Handle IPv6 candidates in userspace bind #4123

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

lixmal
Copy link
Collaborator

@lixmal lixmal commented Jul 9, 2025

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

@Copilot Copilot AI review requested due to automatic review settings July 9, 2025 10:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances IPv6 support in the client’s userspace bind by introducing dual-stack packet connections and updating address formatting.

  • Extend LocalAddr to return appropriate IPv4/IPv6 unspecified addresses.
  • Introduce DualStackPacketConn and integrate it into UDPMux and ICEBind.
  • Add formatEndpoint to bracket IPv6 addresses and update ICE candidate logging.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sharedsock/sock_linux.go Updated LocalAddr to select IPv4/IPv6/dual-stack addresses
go.mod Bumped WireGuard-Go replacement version
client/internal/peer/worker_ice.go Added formatEndpoint for proper IPv6 bracket formatting
client/iface/bind/udp_mux_universal.go Detect and dispatch to DualStackPacketConn in three paths
client/iface/bind/udp_mux.go Added dual-stack dispatch in writeTo
client/iface/bind/ice_bind.go Refactored receiver creation, multiplex logic for IPv4/IPv6
client/iface/bind/dual_stack_conn.go New DualStackPacketConn type supporting IPv4 & IPv6
Comments suppressed due to low confidence (1)

client/iface/bind/ice_bind.go:256

  • The added parameter isIPv6 is never used in filterOutStunMessages, which will cause a compilation error for an unused variable. Either remove this parameter or incorporate isIPv6 into the filtering logic.
func (s *ICEBind) filterOutStunMessages(buffers [][]byte, n int, addr net.Addr, isIPv6 bool) (bool, error) {

Comment on lines 128 to +133
func (u *udpConn) WriteTo(b []byte, addr net.Addr) (int, error) {
// Check if this is a dual-stack connection and handle IPv6 addresses properly
if dualStackConn, ok := u.PacketConn.(*DualStackPacketConn); ok {
return dualStackConn.WriteTo(b, addr)
}

Copy link
Preview

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The dual-stack type assertion and WriteTo logic is duplicated in WriteTo, handleCachedAddress, and handleUncachedAddress. Consider extracting a helper to reduce duplication.

Suggested change
func (u *udpConn) WriteTo(b []byte, addr net.Addr) (int, error) {
// Check if this is a dual-stack connection and handle IPv6 addresses properly
if dualStackConn, ok := u.PacketConn.(*DualStackPacketConn); ok {
return dualStackConn.WriteTo(b, addr)
}
func (u *udpConn) writeToDualStack(b []byte, addr net.Addr) (int, error) {
if dualStackConn, ok := u.PacketConn.(*DualStackPacketConn); ok {
return dualStackConn.WriteTo(b, addr)
}
return 0, nil
}
func (u *udpConn) WriteTo(b []byte, addr net.Addr) (int, error) {
// Use helper to handle dual-stack connections
if n, err := u.writeToDualStack(b, addr); err != nil || n > 0 {
return n, err
}

Copilot uses AI. Check for mistakes.

Copy link

sonarqubecloud bot commented Jul 9, 2025

@pappz pappz self-requested a review July 9, 2025 13:45

// ReadFrom reads from both IPv4 and IPv6 connections
func (d *DualStackPacketConn) ReadFrom(b []byte) (n int, addr net.Addr, err error) {
// Prefer IPv4 if available
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we read from both?

@lixmal lixmal marked this pull request as draft July 14, 2025 11:39
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.

2 participants