Bug #4663
closedosmo-e1d: SOCK_SEQPACKET and RAW timeslots
100%
Description
I actually think that the current code cannot work correctly for RAW timeslots.
As we use SOCK_SEQPACKET for the socketpair irrespective of the timeslot mode, this means that every read() of a ts-fd inside osmo-e1d will want to read one "packet" of whatever size the client has written.
However, in osmo-e1d we read only as many bytes as we have frames to send. With the current USB code this is 4..12 bytes per read().
In my understanding of SOCK_SEQPACKET semantics, this means that the first 4..12 bytes are read and the remainder of the packet is discarded.
We should probably eithe- use SOCK_STREAM sockets for RAW and keep SEQPACKET only for HDLC
- use larger read() sizes as we probably really don't want one syscall for every 4..12 bytes of every voice channel
I have other priorities right now, but I think it would be worthwhile to develop a test for this. With vpair this test could run without any hardware (vpair uses a nominal 10 frames-to-send granularity).
Updated by laforge almost 4 years ago
- Status changed from New to In Progress
- Assignee set to laforge
- % Done changed from 0 to 20
I used MSG_TRUNC in https://gerrit.osmocom.org/c/osmo-e1d/+/19218 to detect such situations, and a small test program reproduces it as expected.
Updated by tnt almost 4 years ago
One issue with SOCK_STREAM and larger reads is the buffering done by the kernel.
Having the read happen "as late as possible" is basically what provides back-pressure to the sender to make sure we don't end up with tons of latency added.
(that's one of the reason that osmo-e1d only reads when data is needed by the usb thread and not as soon as data is available).
From what I remember from early testing, the kernel buffered a max number of bytes and also a max number of "packets" which ever was reached first.
Updated by laforge almost 4 years ago
- % Done changed from 20 to 80
https://gerrit.osmocom.org/c/osmo-e1d/+/19220 moves RAW timeslots to SOCK_STREAM.
It still means we end up with ridiculously small individual read() system calls, but let's not conflate fixing the bug with optimization.
Updated by laforge almost 4 years ago
tnt wrote:
One issue with SOCK_STREAM and larger reads is the buffering done by the kernel.
Having the read happen "as late as possible" is basically what provides back-pressure to the sender to make sure we don't end up with tons of latency added.
sure. The way how we deal with this in libosmo-abis (with DAHDI) is to never mark the timeslot-fd as BSC_FD_WRITE, but always only send as many bytes as we have read. But still those reads/writes happen with 160 bytes per read/write in DAHDI. The trau frame is 40 bytes anyway, and 4 trau frames together form one 160 byte chunk.
So I am not saying applications should write all they can into the kernel buffer, by all means.
Updated by laforge almost 4 years ago
- Status changed from In Progress to Resolved
- % Done changed from 80 to 100
patches merged
