Skip to content

fix: bind SIP media sockets to signaling interface - #784

Open
AlexanderMatveev wants to merge 4 commits into
livekit:mainfrom
AlexanderMatveev:fix/sip-bindip
Open

fix: bind SIP media sockets to signaling interface#784
AlexanderMatveev wants to merge 4 commits into
livekit:mainfrom
AlexanderMatveev:fix/sip-bindip

Conversation

@AlexanderMatveev

@AlexanderMatveev AlexanderMatveev commented Aug 8, 2026

Copy link
Copy Markdown

Summary

Adds an opt-in way to bind SIP media (RTP) sockets to a specific local interface, instead of always listening on 0.0.0.0.

This is a rebased revival of #659 (closed without merge), updated for review feedback: do not change the default bind address.

Problem

On multi-homed hosts, SIP signaling may use one interface/IP while RTP sockets bound to 0.0.0.0 egress via another. Some SIP providers require signaling and media to originate from the same IP.

nat_1_to_1_ip / media_nat_1_to_1_ip only affect announce; they do not bind the RTP socket. listen_ip only covers SIP signaling.

Change

  • Add BindIP to MediaOptions
  • Add optional config media_listen_ip
  • Resolve RTP bind IP only from explicit config: media_listen_ip → specific listen_ip → else 0.0.0.0
  • No inference from SignalingIPLocal (announce/discovery value; unsafe as a default bind)
  • Unit tests for resolver + explicit BindIP socket bind

Operators who need same-IP media/signaling on multi-homed hosts set media_listen_ip (or a specific listen_ip). Everyone else keeps today’s 0.0.0.0 behavior.

Test plan

  • go test ./pkg/sip/ -run 'TestParseSpecificListenIP|TestResolveMediaBindIP|TestMediaPortBindIP'
  • Optional: multi-homed smoke with media_listen_ip set to the signaling NIC

RTP previously always listened on 0.0.0.0, so on multi-homed hosts media
could egress via a different IP than SIP signaling. Bind media sockets to
SignalingIPLocal so announce and source IP stay aligned.
@AlexanderMatveev
AlexanderMatveev requested a review from a team as a code owner August 8, 2026 22:46
@CLAassistant

CLAassistant commented Aug 8, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

devin-ai-integration[bot]

This comment was marked as resolved.

@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.04%. Comparing base (0460b40) to head (a96065e).
⚠️ Report is 346 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #784      +/-   ##
==========================================
+ Coverage   65.25%   67.04%   +1.78%     
==========================================
  Files          51       41      -10     
  Lines        6588     8122    +1534     
==========================================
+ Hits         4299     5445    +1146     
- Misses       1915     2183     +268     
- Partials      374      494     +120     

☔ 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.

SignalingIPLocal is an announce value and may equal nat_1_to_1_ip when that
address is not local. Bind media via media_listen_ip, then listen_ip, then
SignalingIPLocal only if present on an interface; otherwise keep 0.0.0.0.
devin-ai-integration[bot]

This comment was marked as resolved.

Do not infer BindIP from SignalingIPLocal. Default remains 0.0.0.0 so
multi-interface receive behavior is unchanged unless operators explicitly
pin the media NIC.
devin-ai-integration[bot]

This comment was marked as resolved.

Reject unparseable or unspecified media_listen_ip at startup so typos
fail fast instead of silently falling back to 0.0.0.0.
@AlexanderMatveev
AlexanderMatveev marked this pull request as draft August 9, 2026 00:51
@AlexanderMatveev
AlexanderMatveev marked this pull request as ready for review August 9, 2026 13: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