osmux: Fix osmux seqnum incremented globally instead of per circuit

There's no real use in having a global seqnum. The seqnum, as specified
by the osmux protocol specification, relates to that of the RTP
seq+timestamp, hence it is per circuit.
Having it per circuit allows detecting gaps and lost batches on the
receiver side, applying the Marker bit on the re-assembled RTP packets.

As a resut of the fix, tests/osmux/osmux_test part validating Marker bit
started failing. It failed because it was wrongly written to start with,
since it used only one osmux_out_handle for the 4 CIDs in use, which was
wrong, since each CID must have its own osmux_out_handle as state is
kept there per circuit.

Related: SYS#5987
Change-Id: I562de6a871d8a35475c314ca107c2a12b55d6683
This commit is contained in:
Pau Espin 2022-09-28 17:59:01 +02:00
parent 9aa01ae7b5
commit d2810679c7
3 changed files with 589 additions and 587 deletions

View File

@ -298,7 +298,6 @@ struct osmux_batch {
struct osmux_hdr *osmuxh;
struct llist_head circuit_list;
unsigned int remaining_bytes;
uint8_t seq;
uint32_t nmsgs;
int ndummy;
};
@ -309,6 +308,7 @@ struct osmux_circuit {
struct llist_head msg_list;
int nmsgs;
int dummy;
uint8_t seq;
};
/* returns: 1 if batch is full, 0 if batch still not full, negative on error. */
@ -357,7 +357,7 @@ struct osmux_input_state {
struct rtp_hdr *rtph;
struct amr_hdr *amrh;
uint32_t amr_payload_len;
int ccid;
struct osmux_circuit *circuit;
int add_osmux_hdr;
};
@ -371,8 +371,8 @@ static int osmux_batch_put(struct osmux_batch *batch,
osmuxh->ft = OSMUX_FT_VOICE_AMR;
osmuxh->ctr = 0;
osmuxh->rtp_m = osmuxh->rtp_m || state->rtph->marker;
osmuxh->seq = batch->seq++;
osmuxh->circuit_id = state->ccid;
osmuxh->seq = state->circuit->seq++;
osmuxh->circuit_id = state->circuit->ccid;
osmuxh->amr_ft = state->amrh->ft;
/* annotate current osmux header */
@ -412,7 +412,7 @@ static void osmux_encode_dummy(struct osmux_batch *batch, uint8_t batch_factor,
osmuxh->amr_f = 0;
osmuxh->amr_q= 0;
osmuxh->seq = 0;
osmuxh->circuit_id = state->ccid;
osmuxh->circuit_id = state->circuit->ccid;
osmuxh->amr_cmr = 0;
osmuxh->amr_ft = AMR_FT_3;
msgb_put(state->out_msg, sizeof(struct osmux_hdr));
@ -445,7 +445,7 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
if (circuit->dummy) {
struct osmux_input_state state = {
.out_msg = batch_msg,
.ccid = circuit->ccid,
.circuit = circuit,
};
osmux_encode_dummy(batch, batch_factor, &state);
continue;
@ -455,7 +455,7 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
struct osmux_input_state state = {
.msg = cur,
.out_msg = batch_msg,
.ccid = circuit->ccid,
.circuit = circuit,
};
uint32_t amr_len;
#ifdef DEBUG_MSG

View File

@ -102,7 +102,7 @@ static void tx_cb(struct msgb *msg, void *data)
msgb_free(msg);
}
static struct osmux_out_handle *h_output;
static struct osmux_out_handle *h_output[4];
static void osmux_deliver(struct msgb *batch_msg, void *data)
{
@ -116,7 +116,7 @@ static void osmux_deliver(struct msgb *batch_msg, void *data)
* in a list. Then, reconstruct transmission timing.
*/
while((osmuxh = osmux_xfrm_output_pull(batch_msg)) != NULL)
osmux_xfrm_output_sched(h_output, osmuxh);
osmux_xfrm_output_sched(h_output[osmuxh->circuit_id], osmuxh);
msgb_free(batch_msg);
}
@ -132,7 +132,8 @@ static void sigalarm_handler(int foo)
exit(EXIT_FAILURE);
}
static void osmux_test_marker(int ccid) {
static void osmux_test_marker(int num_ccid)
{
struct msgb *msg;
struct rtp_hdr *rtph = (struct rtp_hdr *)rtp_pkt;
struct rtp_hdr *cpy_rtph;
@ -145,7 +146,7 @@ static void osmux_test_marker(int ccid) {
seq++;
rtph->sequence = htons(seq);
for (j=0; j<4; j++) {
for (j = 0; j < num_ccid; j++) {
msg = msgb_alloc(1500, "test");
if (!msg)
exit(EXIT_FAILURE);
@ -160,7 +161,7 @@ static void osmux_test_marker(int ccid) {
}
rtp_pkts++;
while (osmux_xfrm_input(&h_input, msg, j + ccid) > 0) {
while (osmux_xfrm_input(&h_input, msg, j) > 0) {
osmux_xfrm_input_deliver(&h_input);
}
}
@ -265,13 +266,15 @@ int main(void)
log_set_print_category_hex(osmo_stderr_target, 0);
log_set_use_color(osmo_stderr_target, 0);
h_output = osmux_xfrm_output_alloc(NULL);
osmux_xfrm_output_set_rtp_ssrc(h_output, 0x7000000);
osmux_xfrm_output_set_rtp_pl_type(h_output, 98);
osmux_xfrm_output_set_tx_cb(h_output, tx_cb, NULL);
/* These fields are set using random() */
h_output->rtp_seq = 9158;
h_output->rtp_timestamp = 1681692777;
for (i = 0; i < ARRAY_SIZE(h_output); i++) {
h_output[i] = osmux_xfrm_output_alloc(NULL);
osmux_xfrm_output_set_rtp_ssrc(h_output[i], 0x7000000 + i);
osmux_xfrm_output_set_rtp_pl_type(h_output[i], 98);
osmux_xfrm_output_set_tx_cb(h_output[i], tx_cb, NULL);
/* These fields are set using random() */
h_output[i]->rtp_seq = 9158;
h_output[i]->rtp_timestamp = 1681692777;
}
/* If the test takes longer than 10 seconds, abort it */
alarm(10);
@ -280,7 +283,7 @@ int main(void)
osmux_xfrm_input_init(&h_input);
for (i = 0; i < 4; i++)
osmux_xfrm_input_open_circuit(&h_input, i, 0);
osmux_test_marker(0);
osmux_test_marker(4);
for (i = 0; i < 4; i++)
osmux_xfrm_input_close_circuit(&h_input, i);
osmux_xfrm_input_fini(&h_input);

File diff suppressed because it is too large Load Diff