Bug #5568
openSMPP socket doesn't use TCP_NODELAY
80%
Description
The SMPP socket uses sockets directly (without relying on libosmo-netif/stream) and fails to setsockopt(TCP_NODELAY). This is not critical (SMS is sloooow), but it does add unintended latency to all SMPP messages sent.
The quick fix is to add the setsockopt for every socket we accept(). The clean-up would be to use libosmo-netif/stream...
Updated by msuraev over 1 year ago
- Status changed from New to In Progress
Indeed, libosmo-netif is better option - especially given that it's already used for SGs interface.
Updated by msuraev over 1 year ago
- % Done changed from 0 to 50
Draft is available in https://gerrit.osmocom.org/q/topic:SMPP it passes python SMPP tests locally but those are rather rudimentry. If you have an idea for better testsuite - I'd be happy to give it a shot.
Updated by fixeria over 1 year ago
Hi Max,
msuraev wrote in #note-3:
Draft is available in https://gerrit.osmocom.org/q/topic:SMPP it passes python SMPP tests locally but those are rather rudimentry. If you have an idea for better testsuite - I'd be happy to give it a shot.
we have rather complete SMPP testing coverage in ttcn3-msc-test, so I recommend running it.
Updated by laforge over 1 year ago
I'm actually quite concerned that this has turned out such a comprehensive (and supposedly time-consuming) set of patches.
Adding a TCP_NODELAY to SMPP sockets was the problem. But now we have major refactoring of not only the entire SMPP codebase in osmo-msc.git (with large potentail of fall-out, if not only that all of the branches/patches in the pipeline need to be manually rebased), but also even down into libosmocore now.
I don't know all of the details of the related rationale, but to me, this ticket was about setting a simple socket option, please explain why this requires such extensive refactoring and changes across projects.
Updated by msuraev over 1 year ago
laforge wrote in #note-5:
Adding a TCP_NODELAY to SMPP sockets was the problem. But now we have major refactoring of not only the entire SMPP codebase in osmo-msc.git
While looking at SMPP code in OsmoMSC I've noticed that there're 2 partially overlapping implementations (MSC itself and smpp mirror) with handful of TODO comments about merging them. Also some of the fixes were applied to one but not the other. Hence it felt better to merge them first and than work on converting shared codebase to libosmo-netif.
but also even down into libosmocore now.
There should be no related patches in libosmocore. Have I misattributed some submission? Apologies if so. Could you please point to the corresponding gerrit link?
https://gerrit.osmocom.org/q/topic:SMPP - those should be the only related patches (half are merged already). I've tried to split it into smaller chunks to make review easier.
Shall I proceed with the current series or abandon it and send simple TCP_NODELAY socket option patch without converting to libosmo-netif?
Updated by msuraev over 1 year ago
- Status changed from In Progress to Stalled
Implemented in https://gerrit.osmocom.org/c/osmo-msc/+/28849 - waiting for review.
Updated by laforge 6 months ago
- Assignee changed from msuraev to osmith
I'm not sure it was wise to conflate the setting of a simple TCP_NODELAY socket option with migrating an entire third-party library over to libosmo-netif.
Maybe a very simple patch to add the nodelay option would be quick to merge.
