Support TCP for protocol messages by softins · Pull Request #3636 · jamulussoftware/jamulus

softins · 2026-03-11T14:36:42Z

Short description of changes

Support fallback to TCP for protocol messages, in order to overcome potential loss of large messages due to UDP fragmentation. ~~Currently an incomplete draft, for comment as development continues.~~

CHANGELOG: Client/Server: Support TCP fallback for protocol messages.

Context: Fixes an issue?

Discussed in issue #3242.

Does this change need documentation? What needs to be documented and how?

It will need documentation once design and development are complete. Particularly need to explain the firewall requirements for a server or directory.

Status of this Pull Request

~~Incomplete, still under development. Main server side complete and working. Client side development in progress.~~ Complete and ready for review and testing. ~~Still marked draft as~~ it needs some of the debug messages to be commented out before merging.

What is missing until this pull request can be merged?

A lot of testing of both server and client. Intended for Jamulus 4.0.0.

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want
My code follows the style guide
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

softins · 2026-03-11T14:40:45Z

So far, this implements the server side of the design described here and here

softins · 2026-03-28T12:07:21Z

So the next stage of implementation has been achieved: client-side support in the Connect dialog.

If the server list has not been received via UDP when the associated message indicating TCP support has arrived, the client will retry fetching the server list over TCP.
If the client list for a server has not been received via UDP when the associated message indicating TCP support has arrived, the client will retry fetching the client list over TCP, and will continue to use TCP for that server while the Connect dialog is open.
A directory or server that does not have TCP support will not send the TCP supported message, and will continue to be handled as in current versions.
If the server list or client list is successfully received over UDP, there is no need for the client to try TCP.

It has been tested by using nft to drop outbound Jamulus UDP messages with a specific message ID, to simulate loss due to fragmentation.

Examples for a directory-enabled server running on port 22120:

drop UDP server list: nft add rule inet filter output udp sport 22120 @ih,16,16 0xee03 drop
drop UDP client list: nft add rule inet filter output udp sport 22120 @ih,16,16 0xf503 drop
drop UDP "TCP supported" msg: nft add rule inet filter output udp sport 22120 @ih,16,16 0xfb03 drop

Note that nft rules require network byte order (big-endian), but Jamulus IDs are little-endian:

CLM_SERVER_LIST = 1006 = 0x03ee => 0xee03 (LE byte order)
CLM_RED_SERVER_LIST = 1018 = 0x03fa => 0xfa03 (LE byte order)
CLM_CONN_CLIENTS_LIST = 1013 = 0x03f5 => 0xf503 (LE byte order)
CLM_TCP_SUPPORTED = 1019 = 0x03fb => 0xfb03 (LE byte order)

softins · 2026-03-28T17:56:02Z

The next step is to try implementing the connected-mode TCP described here

ann0see · 2026-04-07T14:55:58Z

    bool         bUseTranslation             = true;
    bool         bCustomPortNumberGiven      = false;
    bool         bEnableIPv6                 = false;
+    bool         bEnableTcp                  = false;


Since we'll have a long time for the 4.0 release, I'd enable it by default soon (of course once we've tested that the basics work)

No, I disagree. It's a server-only option, and most servers operators will not need to enable TCP support. Only those running large directories or large servers will need to, and they also need to understand and configure their firewall requirements.

TCP support in the client will indeed be enabled by default, but will only take effect when talking to a directory or server that has enabled it.

If a server operator enables TCP without having configured their firewall correctly, client users could have problems as the server would advertise TCP support to the client, but the client could be unable to connect.

Can we not give an error message or fallback procedure in case the TCP connection timed out?

Yes, I'm sure we can. I haven't yet tested that scenario.

But it doesn't negate my view that server-side TCP support needs to be an explicit option.

softins · 2026-04-09T22:48:19Z

Well I've finished implementing everything I intended to, for directory, server and client, so it's ready for reviewing and trying out, as and when time permits (post 3.12.0).

I have a private directory and server built and running with TCP support, at newjam.softins.co.uk on the standard port 22124.

In order to demonstrate the use of TCP in a new client's connect dialog, it will be necessary to use custom firewall filters on the client end to temporarily drop incoming UDP Jamulus protocol messages containing a server list or connected clients list.

There is full forward and backward compatibility between clients and servers built with TCP support and older versions.

softins · 2026-04-10T06:34:38Z

Keeping as draft, because it will need quite a few debug messages removed before merging.

Constructor for CTcpConnection made polymorphic for client and server.

softins · 2026-05-25T13:13:57Z

This is rebased and ready for thorough review now. A few comments:

I know there are a lot of commits. I plan to squash to a lot fewer (maybe even one?) before merging, but I thought it might be useful to get the granular view while reviewing.
The TCP connections for fetching server lists from directories, and client lists from servers, are only used where UDP has failed. This is only likely to happen with very large lists, and most operators of small directories and servers will never need to specify --enabletcp.
A server or directory operator who does specify --enabletcp must also allow the host's firewall to accept TCP connections on the same port as for UDP.
The client always has TCP support enabled, and will only try to use TCP when it is enabled on the server.
- When fetching server or client lists in the Connect Dialog, if the client detects that a list message has probably been lost, it will use a short-lived TCP connection for each request and then close it.
- When the client establishes an audio connection to a TCP-enabled server, it will establish a long-lived TCP connection, so that the server can reliably send updated client lists to the client during the session. It will close the TCP connection on disconnect.
In order to simulate UDP message loss for testing, I have created a repo of test scripts and documentation at https://github.com/softins/jamulus-tcp-tests. As they will likely only be used during review, I didn't want to include them in the main repo, but am happy to do so if considered useful.
The Wireshark Jamulus dissector understands the CLM_TCP_SUPPORTED message, but does not yet support dissecting Jamulus messages sent over TCP.

pljones · 2026-05-25T17:01:14Z

I'd rather wait now until #3710 is completed, so we can include this new feature in the message being proposed there.

Other than that, I can't see anything wrong with the code: that doesn't mean I fully understand it :).

softins · 2026-05-25T17:07:57Z

I'd rather wait now until #3710 is completed, so we can include this new feature in the message being proposed there.

I disagree. The CLM_TCP_SUPPORTED message is not just to let the client know we support TCP. It also enables the client to know that it didn't receive the expected message via UDP that was sent immediately before it.

(The point is that the client only uses TCP when necessary, not every time in any case)

So I've no problem with using #3710 purely for information that the server supports TCP connection, but that can't replace the CLM_TCP_SUPPORTED mechanism.

So this PR can, or maybe even should, precede #3710, and the latter can then include the fact the feature is enabled in its own information.

pljones · 2026-05-25T17:49:52Z

The client always has TCP support enabled, and will only try to use TCP when it is enabled on the server.

When fetching server or client lists in the Connect Dialog, if the client detects that a list message has probably been lost, it will use a short-lived TCP connection for each request and then close it.

When the client establishes an audio connection to a TCP-enabled server, it will establish a long-lived TCP connection, so that the server can reliably send updated client lists to the client during the session. It will close the TCP connection on disconnect.

OK, I guess I don't follow this. Somewhere (docs/tcp.md or something), could you add an explanation with a flow diagram of how the different scenarios will work, showing protocol messages exchanged and comparing the current flow with the new proposed flow.

(As you've got new files, I'd like them in after the AGPL 3.0+ change, too, so you can be first to put just the AGPL 3.0+ banner on a file to show how it's done.)

softins · 2026-05-26T10:19:52Z

OK, I guess I don't follow this. Somewhere (docs/tcp.md or something), could you add an explanation with a flow diagram of how the different scenarios will work, showing protocol messages exchanged and comparing the current flow with the new proposed flow.

OK, I'll put something together over the next few days.

(As you've got new files, I'd like them in after the AGPL 3.0+ change, too, so you can be first to put just the AGPL 3.0+ banner on a file to show how it's done.)

Cool, I can do that. For new files, does the header need to include lines 10-17, the part about pre/post 3.12.1dev? Or is that only applicable to existing files?

pljones · 2026-05-26T15:54:31Z

For new files, no - just keep the AGPL bits.

ann0see · 2026-05-27T20:00:11Z

So I think I'll need significant time to review this. I won't have a lot of time for this in the next week. In the meantime we should make this production ready and cleaned up if needed.

softins · 2026-05-28T14:15:09Z

I've added docs/TCP.md describing how and when TCP is used, both for the Connect Dialog and for Connected Mode.

softins · 2026-05-28T14:17:36Z

So I think I'll need significant time to review this. I won't have a lot of time for this in the next week. In the meantime we should make this production ready and cleaned up if needed.

I want to leave the commit history and the qDebug messages in place while it is being reviewed and tested. Once the team is happy with the operation, I will tidy up the commits and comment out the qDebug messages that would be too noisy for production.

pljones · 2026-05-28T16:34:43Z

Thanks, makes sense. Next to try and work out how the fits around it 🙂 .

pljones · 2026-05-28T16:38:56Z

+        // create a TCP client connection and send message
+        QTcpSocket* pSocket = new QTcpSocket ( this );
+
+        // timer for TCP connect timeout shorter than Qt default 30 seconds


If this is the keep alive timer - say that and say it's 15 seconds - rather than saying what it isn't.

No, it's the connect timeout. Qt by default returns "connect timeout" after no acceptance for 30 seconds. There's no point waiting that long - it will either succeed within a couple of seconds or never, nowadays.

The keepalive timer is separate from that.

OK - so still say what the number is.

Also, even though it's the client, I'd issue a warning level message with the source and dest ip/port numbers.

pljones · 2026-05-28T16:43:22Z

+            pTimer->deleteLater();
+
+            qWarning() << "- TCP connection error:" << pSocket->errorString();
+            // may want to specifically handle ConnectionRefusedError?


The problem is knowing why the connection was refused. Saying "Maybe check your firewall" isn't much help. Client-side is always tricky because most diagnostics never get looked at.

pljones · 2026-05-28T16:52:38Z

    TimerPing.start ( PING_UPDATE_TIME_SERVER_LIST_MS );
 }

+void CConnectDlg::SetTcpSupported ( const CHostAddress& InetAddr, int iID )


I don't like this being UI-specific code.

The RPC interface will have the same problem.

pljones · 2026-05-28T16:54:43Z

+    {
+        // ID out of range or channel not connected - reject connection
+        pTcpConnection->disconnectFromHost();
+        qDebug() << "- rejected invalid client ID";


Warning not Debug. This should never happen.

pljones · 2026-05-28T16:55:12Z

+    {
+        // IP address mismatch - reject connection
+        pTcpConnection->disconnectFromHost();
+        qDebug() << "- rejected mismatched IP address";


Warning not Debug, as above.

pljones · 2026-05-28T16:55:52Z

+
+    CChannel* pChannel = &vecChannels[iChanID];
+
+    qDebug() << "- request to link TCP connection with UDP client at" << pChannel->GetAddress().toString();


If this is the long lived connection, this should be info level.

pljones · 2026-05-28T16:56:48Z

    void OnCLReqVersionAndOS ( CHostAddress InetAddr ) { ConnLessProtocol.CreateCLVersionAndOSMes ( InetAddr ); }

-    void OnCLReqConnClientsList ( CHostAddress InetAddr ) { ConnLessProtocol.CreateCLConnClientsListMes ( InetAddr, CreateChannelList() ); }
+    void OnCLReqConnClientsList ( CHostAddress InetAddr, CTcpConnection* pTcpConnection )


We should not really allow .h files to have multi-line methods...

pljones · 2026-05-28T16:57:55Z

+
+void CTcpConnection::OnDisconnected()
+{
+    qDebug() << "- Jamulus-TCP: disconnected from:" << tcpAddress.toString();


pljones · 2026-05-28T16:59:18Z

+
+void CTcpConnection::OnTimerIdleTimeout()
+{
+    // qDebug() << "- ConnTimeout timer" << this << "from TCP" << tcpAddress.toString();


Hm. Should we ever normally get an idle timeout? Doesn't it mean something didn't clear down the connection properly?

No, not normally. It's just to prevent a connection hanging around if a user's internet goes down suddenly without disconnecting, for example. Many types of servers implement an idle timeout for such a scenario.

Any "should never happen", I'd like to have warnings for, really. If they do happen regularly, we'll want to know whether there's a flaw in the logic.

pljones · 2026-05-28T17:01:50Z

+            QString ( "- Jamulus-TCP: Server started on %1:%2" ).arg ( pTcpServer->serverAddress().toString() ).arg ( pTcpServer->serverPort() ) );
+        return true;
+    }
+    qInfo() << "- Jamulus-TCP: Unable to start server:" << pTcpServer->errorString();


Logging source IP, source port, target IP and target port would be useful, though. The operator could use them to determine the cause of the problem.

softins added this to the Release 4.0.0 milestone Mar 11, 2026

softins self-assigned this Mar 11, 2026

softins mentioned this pull request Mar 11, 2026

Support TCP for protocol messages #3242

Open

softins force-pushed the tcp-protocol branch 4 times, most recently from 5e1a658 to 0ae51e2 Compare March 16, 2026 13:05

softins linked an issue Mar 16, 2026 that may be closed by this pull request

Support TCP for protocol messages #3242

Open

softins added the feature request Feature request label Mar 16, 2026

softins force-pushed the tcp-protocol branch 3 times, most recently from 7ad1d1f to d939e5b Compare March 26, 2026 17:38

ann0see self-requested a review April 7, 2026 14:51

ann0see reviewed Apr 7, 2026

View reviewed changes

Comment thread src/tcpserver.h

ann0see reviewed Apr 7, 2026

View reviewed changes

Comment thread src/connectdlg.cpp Outdated

ann0see reviewed Apr 7, 2026

View reviewed changes

Comment thread src/connectdlg.cpp Outdated

ann0see assigned ann0see and pljones Apr 9, 2026

ann0see added the bug Something isn't working label Apr 9, 2026

github-project-automation Bot added this to Tracking Apr 9, 2026

github-project-automation Bot moved this to Triage in Tracking Apr 9, 2026

ann0see moved this from Triage to In Progress in Tracking Apr 9, 2026

softins marked this pull request as ready for review April 9, 2026 22:48

softins marked this pull request as draft April 10, 2026 06:30

softins added 8 commits May 25, 2026 11:24

Rework TCP session-mode connection

20c70ce

Minor comment updates

8e295fa

Add support for sending Empty Message over TCP

e0ac6fa

Implement keepalive over session long TCP connection

1ec2e88

Clarify comment

3356dee

Make CTcpConnection work in serveronly mode.

fdcef54

Constructor for CTcpConnection made polymorphic for client and server.

Add timeout for TCP connection

3c9d5cc

Add an idle timeout on the server side

fe7b02c

softins force-pushed the tcp-protocol branch from 2c09cb1 to fe7b02c Compare May 25, 2026 10:59

Add document describing TCP operation

2d5ec89

pljones reviewed May 28, 2026

View reviewed changes

Comment thread docs/TCP.md

Copy link
Copy Markdown

Collaborator

pljones May 28, 2026

Choose a reason for hiding this comment

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

Thanks, makes sense. Next to try and work out how the fits around it 🙂 .

pljones reviewed May 28, 2026

View reviewed changes


		CChannel* pChannel = &vecChannels[iChanID];

		qDebug() << "- request to link TCP connection with UDP client at" << pChannel->GetAddress().toString();

Conversation

softins commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

softins commented Mar 11, 2026

Uh oh!

softins commented Mar 28, 2026

Uh oh!

softins commented Mar 28, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

softins commented Apr 9, 2026

Uh oh!

softins commented Apr 10, 2026

Uh oh!

softins commented May 25, 2026

Uh oh!

pljones commented May 25, 2026

Uh oh!

softins commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pljones commented May 25, 2026

Uh oh!

softins commented May 26, 2026

Uh oh!

pljones commented May 26, 2026

Uh oh!

ann0see commented May 27, 2026

Uh oh!

softins commented May 28, 2026

Uh oh!

softins commented May 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pljones May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softins May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pljones May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

softins commented Mar 11, 2026 •

edited

Loading

softins commented May 25, 2026 •

edited

Loading

pljones May 28, 2026 •

edited

Loading

softins May 28, 2026 •

edited

Loading

pljones May 28, 2026 •

edited

Loading