21 Jul
2015
21 Jul
'15
4:23 p.m.
From: Pablo Neira Ayuso <pablo(a)netfilter.org>
The following patchset, although it looks a bit large, it is composed of many
small incremental changes to improve the existing osmux infrastructure and to
add dummy padding support.
The dummy padding consists of a osmux header and payload that looks like this:
OSMUX seq=000 ccid=002 ft=2 ctr=3 amr_f=0 amr_q=0 amr_ft=03 amr_cmr=00 ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ]
The frame type is OSMUX_FT_DUMMY (=2) in the batch that whose payload is
padded with 0xff. Note that the message above is padding the circuit ID 2.
This is used for early bandwidth preallocation in links that involve relatively
slow dynamic bandwidth allocation such as Iridium/OpenPort.
The osmux test infrastructure has been improved (to resolve some memleaks) and
enhanced to cover this new dummy support.
The OpenBSC changeset is composed of one very small patch that will come after
this batch.
If no major concerns, I'll push this initial batch soon into the repository.
Thanks.
Pablo Neira Ayuso (18):
osmux: discard non voice osmux message
osmux: add osmux_input_state structure
osmux: add circuit helper functions
osmux: rename struct batch_list_node to osmux_circuit
osmux: rename circuit->list to circuit->rtp_list
osmux: pass up struct osmux_batch
osmux: count pending messages to be transformed in the batch
osmux: introduce osmux_xfrm_input_close_circuit()
osmux: introduce osmux_xfrm_input_open_circuit()
tests: osmux: adapt it to use the new circuit API
tests: osmux: factor out main test loop
tests: osmux: test online deactivation of dummy padding
tests: osmux: validate dummy padding with no voice data interaction
tests: osmux: test circuit reopening after closure
osmux: kill osmux_get_hdr()
tests: osmux: fix msgb leaks
tests: compile tests with debugging symbols, ie. -g
tests: osmux: iterate 64 times in osmo_test_loop()
include/osmocom/netif/osmux.h | 8 +-
src/osmux.c | 374 ++++++++++++++++++++++++++---------------
tests/Makefile.am | 2 +-
tests/osmux/osmux_test.c | 80 ++++++---
4 files changed, 304 insertions(+), 160 deletions(-)
--
1.7.10.4
21 Jul
21 Jul
4:23 p.m.
New subject: [PATCH libosmo-netif 01/18] osmux: discard non voice osmux message
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
We only support voice osmux messages by now. Discard unsupported types.
---
src/osmux.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/osmux.c b/src/osmux.c
index eb2c683..a3eccaa 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -65,6 +65,11 @@ struct osmux_hdr *osmux_xfrm_output_pull(struct msgb *msg)
osmuxh = (struct osmux_hdr *)msg->data;
+ if (osmuxh->ft != OSMUX_FT_VOICE_AMR) {
+ LOGP(DLMIB, LOGL_ERROR, "Discarding unsupported Osmux FT %d\n",
+ osmuxh->ft);
+ return NULL;
+ }
if (!osmo_amr_ft_valid(osmuxh->amr_ft)) {
LOGP(DLMIB, LOGL_ERROR, "Discarding bad AMR FT %d\n",
osmuxh->amr_ft);
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 02/18] osmux: add osmux_input_state structure
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
This new structure serves as container of the internal osmux state during
transformation from RTP AMR to the compressed osmux format.
This reduces the footprint of several functions and it makes them easier to
extend if we need to pass new information between functions.
---
src/osmux.c | 77 +++++++++++++++++++++++++++++++----------------------------
1 file changed, 40 insertions(+), 37 deletions(-)
diff --git a/src/osmux.c b/src/osmux.c
index a3eccaa..938b9a6 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -221,63 +221,65 @@ static void osmux_batch_dequeue(struct msgb *msg, struct
batch_list_node *node)
node->nmsgs--;
}
-static int
-osmux_batch_put(struct osmux_in_handle *h, struct msgb *out_msg,
- struct msgb *msg, struct rtp_hdr *rtph,
- struct amr_hdr *amrh, uint32_t amr_payload_len,
- int ccid, int add_osmux_header)
+struct osmux_input_state {
+ struct msgb *out_msg;
+ struct msgb *msg;
+ struct rtp_hdr *rtph;
+ struct amr_hdr *amrh;
+ uint32_t amr_payload_len;
+ int ccid;
+ int add_osmux_hdr;
+};
+
+static int osmux_batch_put(struct osmux_in_handle *h,
+ struct osmux_input_state *state)
{
struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
struct osmux_hdr *osmuxh;
- if (add_osmux_header) {
- osmuxh = (struct osmux_hdr *)out_msg->tail;
+ if (state->add_osmux_hdr) {
+ osmuxh = (struct osmux_hdr *)state->out_msg->tail;
osmuxh->ft = OSMUX_FT_VOICE_AMR;
osmuxh->ctr = 0;
- osmuxh->amr_f = amrh->f;
- osmuxh->amr_q= amrh->q;
+ osmuxh->amr_f = state->amrh->f;
+ osmuxh->amr_q= state->amrh->q;
osmuxh->seq = batch->seq++;
- osmuxh->circuit_id = ccid;
- osmuxh->amr_cmr = amrh->cmr;
- osmuxh->amr_ft = amrh->ft;
- msgb_put(out_msg, sizeof(struct osmux_hdr));
+ osmuxh->circuit_id = state->ccid;
+ osmuxh->amr_cmr = state->amrh->cmr;
+ osmuxh->amr_ft = state->amrh->ft;
+ msgb_put(state->out_msg, sizeof(struct osmux_hdr));
/* annotate current osmux header */
batch->osmuxh = osmuxh;
} else {
if (batch->osmuxh->ctr == 0x7) {
LOGP(DLMIB, LOGL_ERROR, "cannot add msg=%p, "
- "too many messages for this RTP ssrc=%u\n",
- msg, rtph->ssrc);
+ "too many messages for this RTP ssrc=%u\n",
+ state->msg, state->rtph->ssrc);
return 0;
}
batch->osmuxh->ctr++;
}
- memcpy(out_msg->tail, osmo_amr_get_payload(amrh), amr_payload_len);
- msgb_put(out_msg, amr_payload_len);
+ memcpy(state->out_msg->tail, osmo_amr_get_payload(state->amrh),
+ state->amr_payload_len);
+ msgb_put(state->out_msg, state->amr_payload_len);
return 0;
}
-static int
-osmux_xfrm_encode_amr(struct osmux_in_handle *h,
- struct msgb *out_msg,
- struct rtp_hdr *rtph, struct msgb *msg,
- int ccid, int add_osmux_header)
+static int osmux_xfrm_encode_amr(struct osmux_in_handle *h,
+ struct osmux_input_state *state)
{
- struct amr_hdr *amrh;
uint32_t amr_len;
- uint32_t amr_payload_len;
- amrh = osmo_rtp_get_payload(rtph, msg, &amr_len);
- if (amrh == NULL)
+ state->amrh = osmo_rtp_get_payload(state->rtph, state->msg, &amr_len);
+ if (state->amrh == NULL)
return -1;
- amr_payload_len = amr_len - sizeof(struct amr_hdr);
+ state->amr_payload_len = amr_len - sizeof(struct amr_hdr);
- if (osmux_batch_put(h, out_msg, msg, rtph, amrh, amr_payload_len,
- ccid, add_osmux_header) < 0)
+ if (osmux_batch_put(h, state) < 0)
return -1;
return 0;
@@ -304,9 +306,11 @@ static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
int ctr = 0;
llist_for_each_entry_safe(cur, tmp, &node->list, list) {
- struct rtp_hdr *rtph;
- int add_osmux_hdr = 0;
-
+ struct osmux_input_state state = {
+ .msg = cur,
+ .out_msg = batch_msg,
+ .ccid = node->ccid,
+ };
#ifdef DEBUG_MSG
char buf[4096];
@@ -315,19 +319,18 @@ static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
LOGP(DLMIB, LOGL_DEBUG, "to BSC-NAT: %s\n", buf);
#endif
- rtph = osmo_rtp_get_hdr(cur);
- if (rtph == NULL)
+ state.rtph = osmo_rtp_get_hdr(cur);
+ if (state.rtph == NULL)
return NULL;
if (ctr == 0) {
#ifdef DEBUG_MSG
LOGP(DLMIB, LOGL_DEBUG, "add osmux header\n");
#endif
- add_osmux_hdr = 1;
+ state.add_osmux_hdr = 1;
}
- osmux_xfrm_encode_amr(h, batch_msg, rtph, cur,
- node->ccid, add_osmux_hdr);
+ osmux_xfrm_encode_amr(h, &state);
osmux_batch_dequeue(cur, node);
msgb_free(cur);
ctr++;
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 03/18] osmux: add circuit helper functions
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Add osmux_batch_add_circuit() and osmux_batch_find_circuit() helper functions.
---
src/osmux.c | 65 ++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 19 deletions(-)
diff --git a/src/osmux.c b/src/osmux.c
index 938b9a6..c3e68a3 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -450,19 +450,50 @@ static void osmux_replay_lost_packets(struct batch_list_node *node,
}
}
+static struct batch_list_node *
+osmux_batch_find_circuit(struct osmux_batch *batch, int ccid)
+{
+ struct batch_list_node *circuit;
+
+ llist_for_each_entry(circuit, &batch->node_list, head) {
+ if (circuit->ccid == ccid)
+ return circuit;
+ }
+ return NULL;
+}
+
+static struct batch_list_node *
+osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
+{
+ struct batch_list_node *circuit;
+
+ circuit = osmux_batch_find_circuit(batch, ccid);
+ if (circuit != NULL) {
+ LOGP(DLMIB, LOGL_ERROR, "circuit %u already exists!\n", ccid);
+ return NULL;
+ }
+
+ circuit = talloc_zero(osmux_ctx, struct batch_list_node);
+ if (circuit == NULL) {
+ LOGP(DLMIB, LOGL_ERROR, "OOM on circuit %u\n", ccid);
+ return NULL;
+ }
+
+ circuit->ccid = ccid;
+ INIT_LLIST_HEAD(&circuit->list);
+ llist_add_tail(&circuit->head, &batch->node_list);
+
+ return circuit;
+}
+
static int
osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
struct rtp_hdr *rtph, int ccid)
{
- struct batch_list_node *node;
- int found = 0, bytes = 0, amr_payload_len;
+ int bytes = 0, amr_payload_len;
+ struct batch_list_node *circuit;
- llist_for_each_entry(node, &batch->node_list, head) {
- if (node->ccid == ccid) {
- found = 1;
- break;
- }
- }
+ circuit = osmux_batch_find_circuit(batch, ccid);
amr_payload_len = osmux_rtp_amr_payload_len(msg, rtph);
if (amr_payload_len < 0)
@@ -470,21 +501,21 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
/* First check if there is room for this message in the batch */
bytes += amr_payload_len;
- if (!found)
+ if (!circuit)
bytes += sizeof(struct osmux_hdr);
/* No room, sorry. You'll have to retry */
if (bytes > batch->remaining_bytes)
return 1;
- if (found) {
+ if (circuit) {
struct msgb *cur;
/* Extra validation: check if this message already exists,
* should not happen but make sure we don't propagate
* duplicated messages.
*/
- llist_for_each_entry(cur, &node->list, list) {
+ llist_for_each_entry(cur, &circuit->list, list) {
struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur);
if (rtph2 == NULL)
return -1;
@@ -498,21 +529,17 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
}
}
/* Handle RTP packet loss scenario */
- osmux_replay_lost_packets(node, rtph);
+ osmux_replay_lost_packets(circuit, rtph);
} else {
/* This is the first message with that ssrc we've seen */
- node = talloc_zero(osmux_ctx, struct batch_list_node);
- if (node == NULL)
+ circuit = osmux_batch_add_circuit(batch, ccid);
+ if (!circuit)
return -1;
-
- node->ccid = ccid;
- INIT_LLIST_HEAD(&node->list);
- llist_add_tail(&node->head, &batch->node_list);
}
/* This batch is full, force batch delivery */
- if (osmux_batch_enqueue(msg, node) < 0)
+ if (osmux_batch_enqueue(msg, circuit) < 0)
return 1;
#ifdef DEBUG_MSG
--
1.7.10.4
8 Aug
8 Aug
8:56 p.m.
New subject: [PATCH libosmo-netif 03/18] osmux: add circuit helper functions
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org wrote:
Hi!
+static struct batch_list_node *
+osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
+{
+ struct batch_list_node *circuit;
+
+ circuit = osmux_batch_find_circuit(batch, ccid);
+ if (circuit != NULL) {
+ LOGP(DLMIB, LOGL_ERROR, "circuit %u already exists!\n", ccid);
+ return NULL;
+ }
Okay very defensive but no performance issue right now. Patches 1-3 look good!
holger
21 Jul
21 Jul
4:23 p.m.
New subject: [PATCH libosmo-netif 04/18] osmux: rename struct batch_list_node to osmux_circuit
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
I think this is a better name for this object. Basically, an input handle
represents a batch that is composed of one or more circuit objects.
Each circuit object contains a list of RTP messages that are pending to be
converted to the osmux format in one single batch.
---
src/osmux.c | 66 +++++++++++++++++++++++++++++------------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/src/osmux.c b/src/osmux.c
index c3e68a3..81ad9dc 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -181,24 +181,24 @@ int osmux_xfrm_output(struct osmux_hdr *osmuxh, struct
osmux_out_handle *h,
struct osmux_batch {
struct osmo_timer_list timer;
struct osmux_hdr *osmuxh;
- struct llist_head node_list;
+ struct llist_head circuit_list;
unsigned int remaining_bytes;
uint8_t seq;
};
-struct batch_list_node {
+struct osmux_circuit {
struct llist_head head;
int ccid;
struct llist_head list;
int nmsgs;
};
-static int osmux_batch_enqueue(struct msgb *msg, struct batch_list_node *node)
+static int osmux_batch_enqueue(struct msgb *msg, struct osmux_circuit *circuit)
{
/* Too many messages per batch, discard it. The counter field of the
* osmux header is just 3 bits long, so make sure it doesn't overflow.
*/
- if (node->nmsgs >= 8) {
+ if (circuit->nmsgs >= 8) {
struct rtp_hdr *rtph;
rtph = osmo_rtp_get_hdr(msg);
@@ -210,15 +210,15 @@ static int osmux_batch_enqueue(struct msgb *msg, struct
batch_list_node *node)
return -1;
}
- llist_add_tail(&msg->list, &node->list);
- node->nmsgs++;
+ llist_add_tail(&msg->list, &circuit->list);
+ circuit->nmsgs++;
return 0;
}
-static void osmux_batch_dequeue(struct msgb *msg, struct batch_list_node *node)
+static void osmux_batch_dequeue(struct msgb *msg, struct osmux_circuit *circuit)
{
llist_del(&msg->list);
- node->nmsgs--;
+ circuit->nmsgs--;
}
struct osmux_input_state {
@@ -288,7 +288,7 @@ static int osmux_xfrm_encode_amr(struct osmux_in_handle *h,
static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
{
struct msgb *batch_msg;
- struct batch_list_node *node, *tnode;
+ struct osmux_circuit *circuit, *next;
struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
#ifdef DEBUG_MSG
@@ -301,15 +301,15 @@ static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
return NULL;
}
- llist_for_each_entry_safe(node, tnode, &batch->node_list, head) {
+ llist_for_each_entry_safe(circuit, next, &batch->circuit_list, head) {
struct msgb *cur, *tmp;
int ctr = 0;
- llist_for_each_entry_safe(cur, tmp, &node->list, list) {
+ llist_for_each_entry_safe(cur, tmp, &circuit->list, list) {
struct osmux_input_state state = {
.msg = cur,
.out_msg = batch_msg,
- .ccid = node->ccid,
+ .ccid = circuit->ccid,
};
#ifdef DEBUG_MSG
char buf[4096];
@@ -331,12 +331,12 @@ static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
}
osmux_xfrm_encode_amr(h, &state);
- osmux_batch_dequeue(cur, node);
+ osmux_batch_dequeue(cur, circuit);
msgb_free(cur);
ctr++;
}
- llist_del(&node->head);
- talloc_free(node);
+ llist_del(&circuit->head);
+ talloc_free(circuit);
}
return batch_msg;
}
@@ -394,7 +394,7 @@ static int osmux_rtp_amr_payload_len(struct msgb *msg, struct rtp_hdr
*rtph)
return amr_payload_len;
}
-static void osmux_replay_lost_packets(struct batch_list_node *node,
+static void osmux_replay_lost_packets(struct osmux_circuit *circuit,
struct rtp_hdr *cur_rtph)
{
int16_t diff;
@@ -403,11 +403,11 @@ static void osmux_replay_lost_packets(struct batch_list_node *node,
int i;
/* Have we see any RTP packet in this batch before? */
- if (llist_empty(&node->list))
+ if (llist_empty(&circuit->list))
return;
/* Get last RTP packet seen in this batch */
- last = llist_entry(node->list.prev, struct msgb, list);
+ last = llist_entry(circuit->list.prev, struct msgb, list);
rtph = osmo_rtp_get_hdr(last);
if (rtph == NULL)
return;
@@ -441,7 +441,7 @@ static void osmux_replay_lost_packets(struct batch_list_node *node,
DELTA_RTP_TIMESTAMP);
/* No more room in this batch, skip padding with more clones */
- if (osmux_batch_enqueue(clone, node) < 0) {
+ if (osmux_batch_enqueue(clone, circuit) < 0) {
msgb_free(clone);
break;
}
@@ -450,22 +450,22 @@ static void osmux_replay_lost_packets(struct batch_list_node *node,
}
}
-static struct batch_list_node *
+static struct osmux_circuit *
osmux_batch_find_circuit(struct osmux_batch *batch, int ccid)
{
- struct batch_list_node *circuit;
+ struct osmux_circuit *circuit;
- llist_for_each_entry(circuit, &batch->node_list, head) {
+ llist_for_each_entry(circuit, &batch->circuit_list, head) {
if (circuit->ccid == ccid)
return circuit;
}
return NULL;
}
-static struct batch_list_node *
+static struct osmux_circuit *
osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
{
- struct batch_list_node *circuit;
+ struct osmux_circuit *circuit;
circuit = osmux_batch_find_circuit(batch, ccid);
if (circuit != NULL) {
@@ -473,7 +473,7 @@ osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
return NULL;
}
- circuit = talloc_zero(osmux_ctx, struct batch_list_node);
+ circuit = talloc_zero(osmux_ctx, struct osmux_circuit);
if (circuit == NULL) {
LOGP(DLMIB, LOGL_ERROR, "OOM on circuit %u\n", ccid);
return NULL;
@@ -481,7 +481,7 @@ osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
circuit->ccid = ccid;
INIT_LLIST_HEAD(&circuit->list);
- llist_add_tail(&circuit->head, &batch->node_list);
+ llist_add_tail(&circuit->head, &batch->circuit_list);
return circuit;
}
@@ -491,7 +491,7 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
struct rtp_hdr *rtph, int ccid)
{
int bytes = 0, amr_payload_len;
- struct batch_list_node *circuit;
+ struct osmux_circuit *circuit;
circuit = osmux_batch_find_circuit(batch, ccid);
@@ -597,7 +597,7 @@ int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int
ccid)
/* This is the first message in the batch, start the
* batch timer to deliver it.
*/
- first_rtp_msg = llist_empty(&batch->node_list) ? 1 : 0;
+ first_rtp_msg = llist_empty(&batch->circuit_list) ? 1 : 0;
/* Add this RTP to the OSMUX batch */
ret = osmux_batch_add(batch, msg, rtph, ccid);
@@ -638,7 +638,7 @@ void osmux_xfrm_input_init(struct osmux_in_handle *h)
if (batch == NULL)
return;
- INIT_LLIST_HEAD(&batch->node_list);
+ INIT_LLIST_HEAD(&batch->circuit_list);
batch->remaining_bytes = h->batch_size;
batch->timer.cb = osmux_batch_timer_expired;
batch->timer.data = h;
@@ -651,11 +651,11 @@ void osmux_xfrm_input_init(struct osmux_in_handle *h)
void osmux_xfrm_input_fini(struct osmux_in_handle *h)
{
struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
- struct batch_list_node *node, *next;
+ struct osmux_circuit *circuit, *next;
- llist_for_each_entry_safe(node, next, &batch->node_list, head) {
- llist_del(&node->head);
- talloc_free(node);
+ llist_for_each_entry_safe(circuit, next, &batch->circuit_list, head) {
+ llist_del(&circuit->head);
+ talloc_free(circuit);
}
osmo_timer_del(&batch->timer);
talloc_free(batch);
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 05/18] osmux: rename circuit->list to circuit->rtp_list
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
A circuit object contains a list of pending RTP messages to be converted to the
osmux format.
---
src/osmux.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/osmux.c b/src/osmux.c
index 81ad9dc..187ca1c 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -189,7 +189,7 @@ struct osmux_batch {
struct osmux_circuit {
struct llist_head head;
int ccid;
- struct llist_head list;
+ struct llist_head msg_list;
int nmsgs;
};
@@ -210,7 +210,7 @@ static int osmux_batch_enqueue(struct msgb *msg, struct osmux_circuit
*circuit)
return -1;
}
- llist_add_tail(&msg->list, &circuit->list);
+ llist_add_tail(&msg->list, &circuit->msg_list);
circuit->nmsgs++;
return 0;
}
@@ -305,7 +305,7 @@ static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
struct msgb *cur, *tmp;
int ctr = 0;
- llist_for_each_entry_safe(cur, tmp, &circuit->list, list) {
+ llist_for_each_entry_safe(cur, tmp, &circuit->msg_list, list) {
struct osmux_input_state state = {
.msg = cur,
.out_msg = batch_msg,
@@ -403,11 +403,11 @@ static void osmux_replay_lost_packets(struct osmux_circuit
*circuit,
int i;
/* Have we see any RTP packet in this batch before? */
- if (llist_empty(&circuit->list))
+ if (llist_empty(&circuit->msg_list))
return;
/* Get last RTP packet seen in this batch */
- last = llist_entry(circuit->list.prev, struct msgb, list);
+ last = llist_entry(circuit->msg_list.prev, struct msgb, list);
rtph = osmo_rtp_get_hdr(last);
if (rtph == NULL)
return;
@@ -480,7 +480,7 @@ osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
}
circuit->ccid = ccid;
- INIT_LLIST_HEAD(&circuit->list);
+ INIT_LLIST_HEAD(&circuit->msg_list);
llist_add_tail(&circuit->head, &batch->circuit_list);
return circuit;
@@ -515,7 +515,7 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
* should not happen but make sure we don't propagate
* duplicated messages.
*/
- llist_for_each_entry(cur, &circuit->list, list) {
+ llist_for_each_entry(cur, &circuit->msg_list, list) {
struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur);
if (rtph2 == NULL)
return -1;
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 06/18] osmux: pass up struct osmux_batch
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Instead of struct osmux_in_handle. This object contains the internal batching
state information.
---
src/osmux.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/osmux.c b/src/osmux.c
index 187ca1c..bd400f6 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -231,10 +231,9 @@ struct osmux_input_state {
int add_osmux_hdr;
};
-static int osmux_batch_put(struct osmux_in_handle *h,
+static int osmux_batch_put(struct osmux_batch *batch,
struct osmux_input_state *state)
{
- struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
struct osmux_hdr *osmuxh;
if (state->add_osmux_hdr) {
@@ -268,7 +267,7 @@ static int osmux_batch_put(struct osmux_in_handle *h,
return 0;
}
-static int osmux_xfrm_encode_amr(struct osmux_in_handle *h,
+static int osmux_xfrm_encode_amr(struct osmux_batch *batch,
struct osmux_input_state *state)
{
uint32_t amr_len;
@@ -279,23 +278,23 @@ static int osmux_xfrm_encode_amr(struct osmux_in_handle *h,
state->amr_payload_len = amr_len - sizeof(struct amr_hdr);
- if (osmux_batch_put(h, state) < 0)
+ if (osmux_batch_put(batch, state) < 0)
return -1;
return 0;
}
-static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
+static struct msgb *osmux_build_batch(struct osmux_batch *batch,
+ uint32_t batch_size)
{
struct msgb *batch_msg;
struct osmux_circuit *circuit, *next;
- struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
#ifdef DEBUG_MSG
LOGP(DLMIB, LOGL_DEBUG, "Now building batch\n");
#endif
- batch_msg = msgb_alloc(h->batch_size, "osmux");
+ batch_msg = msgb_alloc(batch_size, "osmux");
if (batch_msg == NULL) {
LOGP(DLMIB, LOGL_ERROR, "Not enough memory\n");
return NULL;
@@ -330,7 +329,7 @@ static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
state.add_osmux_hdr = 1;
}
- osmux_xfrm_encode_amr(h, &state);
+ osmux_xfrm_encode_amr(batch, &state);
osmux_batch_dequeue(cur, circuit);
msgb_free(cur);
ctr++;
@@ -349,7 +348,7 @@ void osmux_xfrm_input_deliver(struct osmux_in_handle *h)
#ifdef DEBUG_MSG
LOGP(DLMIB, LOGL_DEBUG, "invoking delivery function\n");
#endif
- batch_msg = osmux_build_batch(h);
+ batch_msg = osmux_build_batch(batch, h->batch_size);
h->stats.output_osmux_msgs++;
h->stats.output_osmux_bytes += batch_msg->len;
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 07/18] osmux: count pending messages to be transformed in the batch
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Add a new field to count the number of messages in the batch that are pending
to be transformed to osmux. Use this new field to check when to enable the
osmux timer.
The follow up patch keeps circuit objects in the batch until they are closed,
so we won't be able to rely on this to know when to enable the timer anymore.
---
src/osmux.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/src/osmux.c b/src/osmux.c
index bd400f6..82ec56e 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -184,6 +184,7 @@ struct osmux_batch {
struct llist_head circuit_list;
unsigned int remaining_bytes;
uint8_t seq;
+ uint32_t nmsgs;
};
struct osmux_circuit {
@@ -333,6 +334,7 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
osmux_batch_dequeue(cur, circuit);
msgb_free(cur);
ctr++;
+ batch->nmsgs--;
}
llist_del(&circuit->head);
talloc_free(circuit);
@@ -486,7 +488,7 @@ osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
}
static int
-osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
+osmux_batch_add(struct osmux_batch *batch, int batch_factor, struct msgb *msg,
struct rtp_hdr *rtph, int ccid)
{
int bytes = 0, amr_payload_len;
@@ -549,6 +551,15 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
/* Update remaining room in this batch */
batch->remaining_bytes -= bytes;
+ if (batch->nmsgs == 0) {
+#ifdef DEBUG_MSG
+ LOGP(DLMIB, LOGL_DEBUG, "osmux start timer batch\n");
+#endif
+ osmo_timer_schedule(&batch->timer, 0,
+ batch_factor * DELTA_RTP_MSG);
+ }
+ batch->nmsgs++;
+
return 0;
}
@@ -565,7 +576,7 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg,
*/
int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int ccid)
{
- int ret, first_rtp_msg;
+ int ret;
struct rtp_hdr *rtph;
struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
@@ -593,13 +604,9 @@ int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int
ccid)
* receive AMR traffic.
*/
- /* This is the first message in the batch, start the
- * batch timer to deliver it.
- */
- first_rtp_msg = llist_empty(&batch->circuit_list) ? 1 : 0;
-
/* Add this RTP to the OSMUX batch */
- ret = osmux_batch_add(batch, msg, rtph, ccid);
+ ret = osmux_batch_add(batch, h->batch_factor,
+ msg, rtph, ccid);
if (ret < 0) {
/* Cannot put this message into the batch.
* Malformed, duplicated, OOM. Drop it and tell
@@ -611,15 +618,6 @@ int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int
ccid)
h->stats.input_rtp_msgs++;
h->stats.input_rtp_bytes += msg->len;
-
- if (first_rtp_msg) {
-#ifdef DEBUG_MSG
- LOGP(DLMIB, LOGL_DEBUG,
- "osmux start timer batch\n");
-#endif
- osmo_timer_schedule(&batch->timer, 0,
- h->batch_factor * DELTA_RTP_MSG);
- }
break;
}
return ret;
--
1.7.10.4
8 Aug
8 Aug
9 p.m.
New subject: [PATCH libosmo-netif 07/18] osmux: count pending messages to be transformed in the batch
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org wrote:
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Add a new field to count the number of messages in the batch that are pending
to be transformed to osmux. Use this new field to check when to enable the
osmux timer.
okay, when one counts a list one needs to be careful to have it balanced. While
having a look if it is balanced I noticed this in the original code:
static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
{
…
llist_for_each_entry_safe(node, tnode, &batch->node_list, head) {
struct msgb *cur, *tmp;
int ctr = 0;
llist_for_each_entry_safe(cur, tmp, &node->list, list) {
struct rtp_hdr *rtph;
int add_osmux_hdr = 0;
#ifdef DEBUG_MSG
...
#endif
rtph = osmo_rtp_get_hdr(cur);
if (rtph == NULL)
return NULL;
When and how will this package be dropped?
Patches 4-7 look good too!
10 Aug
10 Aug
10:10 a.m.
New subject: [PATCH libosmo-netif 07/18] osmux: count pending messages to be transformed in the batch
On Sat, Aug 08, 2015 at 09:00:30PM +0200, Holger Freyther wrote:
If the RTP message that we received is malformed.
Although we already checked this when enqueuing the packet into the
circuit list before the transformation happens, so it should be safe
to remove the NULL check.
But let me revisit this in a follow up patch.
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org
wrote:
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Add a new field to count the number of messages in the batch that are pending
to be transformed to osmux. Use this new field to check when to enable the
osmux timer.
okay, when one counts a list one needs to be careful to have it balanced. While
having a look if it is balanced I noticed this in the original code:
static struct msgb *osmux_build_batch(struct osmux_in_handle *h)
{
…
llist_for_each_entry_safe(node, tnode, &batch->node_list, head) {
struct msgb *cur, *tmp;
int ctr = 0;
llist_for_each_entry_safe(cur, tmp, &node->list, list) {
struct rtp_hdr *rtph;
int add_osmux_hdr = 0;
#ifdef DEBUG_MSG
...
#endif
rtph = osmo_rtp_get_hdr(cur);
if (rtph == NULL)
return NULL;
When and how will this package be dropped? Patches 4-7 look good too!
Thanks Holger.
21 Jul
21 Jul
4:23 p.m.
New subject: [PATCH libosmo-netif 08/18] osmux: introduce osmux_xfrm_input_close_circuit()
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Add this new function to explicitly remove an existing circuit. Thus, the
client application (openbsc) is in full control to release circuits.
Before this patch, the circuit object was added when the first RTP messages was
seen, and it was removed when transforming the list of pending RTP messages to
the Osmux message (once the timer expired).
Moreover, check circuit->nmsgs to account bytes that are consumed by the osmux
header, given that !circuit doesn't mean "this is the first packet"
anymore.
---
include/osmocom/netif/osmux.h | 2 ++
src/osmux.c | 27 ++++++++++++++++++++++-----
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/osmocom/netif/osmux.h b/include/osmocom/netif/osmux.h
index 0f6f5c9..14c967f 100644
--- a/include/osmocom/netif/osmux.h
+++ b/include/osmocom/netif/osmux.h
@@ -84,6 +84,8 @@ int osmux_snprintf(char *buf, size_t size, struct msgb *msg);
void osmux_xfrm_input_init(struct osmux_in_handle *h);
void osmux_xfrm_input_fini(struct osmux_in_handle *h);
+void osmux_xfrm_input_close_circuit(struct osmux_in_handle *h, int ccid);
+
int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int ccid);
void osmux_xfrm_input_deliver(struct osmux_in_handle *h);
diff --git a/src/osmux.c b/src/osmux.c
index 82ec56e..4451b5a 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -289,7 +289,7 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
uint32_t batch_size)
{
struct msgb *batch_msg;
- struct osmux_circuit *circuit, *next;
+ struct osmux_circuit *circuit;
#ifdef DEBUG_MSG
LOGP(DLMIB, LOGL_DEBUG, "Now building batch\n");
@@ -301,7 +301,7 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
return NULL;
}
- llist_for_each_entry_safe(circuit, next, &batch->circuit_list, head) {
+ llist_for_each_entry(circuit, &batch->circuit_list, head) {
struct msgb *cur, *tmp;
int ctr = 0;
@@ -336,8 +336,6 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
ctr++;
batch->nmsgs--;
}
- llist_del(&circuit->head);
- talloc_free(circuit);
}
return batch_msg;
}
@@ -487,6 +485,18 @@ osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
return circuit;
}
+static void osmux_batch_del_circuit(struct osmux_batch *batch, int ccid)
+{
+ struct osmux_circuit *circuit;
+
+ circuit = osmux_batch_find_circuit(batch, ccid);
+ if (circuit == NULL)
+ return;
+
+ llist_del(&circuit->head);
+ talloc_free(circuit);
+}
+
static int
osmux_batch_add(struct osmux_batch *batch, int batch_factor, struct msgb *msg,
struct rtp_hdr *rtph, int ccid)
@@ -502,7 +512,7 @@ osmux_batch_add(struct osmux_batch *batch, int batch_factor, struct
msgb *msg,
/* First check if there is room for this message in the batch */
bytes += amr_payload_len;
- if (!circuit)
+ if (!circuit || circuit->nmsgs == 0)
bytes += sizeof(struct osmux_hdr);
/* No room, sorry. You'll have to retry */
@@ -645,6 +655,13 @@ void osmux_xfrm_input_init(struct osmux_in_handle *h)
LOGP(DLMIB, LOGL_DEBUG, "initialized osmux input converter\n");
}
+void osmux_xfrm_input_close_circuit(struct osmux_in_handle *h, int ccid)
+{
+ struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
+
+ osmux_batch_del_circuit(batch, ccid);
+}
+
void osmux_xfrm_input_fini(struct osmux_in_handle *h)
{
struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
--
1.7.10.4
8 Aug
8 Aug
9:04 p.m.
New subject: [PATCH libosmo-netif 08/18] osmux: introduce osmux_xfrm_input_close_circuit()
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org wrote:
+ llist_del(&circuit->head);
+ talloc_free(circuit);
What happens if there is a single circuit and the batch timer was running? I assume
it will timeout not be rescheduled?
10 Aug
10 Aug
10:26 a.m.
New subject: [PATCH libosmo-netif 08/18] osmux: introduce osmux_xfrm_input_close_circuit()
On Sat, Aug 08, 2015 at 09:04:12PM +0200, Holger Freyther wrote:
Good point.
Looking at osmux_xfrm_input_deliver() and assuming the ndummy counter
became zero, it will not be rescheduled.
Regarding the normal voice path, I think it should be possible to pass
an empty msgb to the deliver() callback if the timer is called after
the circuit is gone, let me have a careful look at this.
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org
wrote:
+ llist_del(&circuit->head);
+ talloc_free(circuit);
What happens if there is a single circuit and the batch timer was
running? I assume it will timeout not be rescheduled?
21 Jul
21 Jul
4:23 p.m.
New subject: [PATCH libosmo-netif 09/18] osmux: introduce osmux_xfrm_input_open_circuit()
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
This new function allows you to create a circuit on an existing input handle.
We don't create the circuit anymore from the first packet seen, instead the
client application is in full control of opening and closing the circuit.
This change includes a new feature to pad a circuit until we see the first
packet that contains voice data. This is useful to preallocate bandwidth on
satellite links such as Iridium/OpenPort.
---
include/osmocom/netif/osmux.h | 4 +-
src/osmux.c | 133 +++++++++++++++++++++++++++++++----------
2 files changed, 103 insertions(+), 34 deletions(-)
diff --git a/include/osmocom/netif/osmux.h b/include/osmocom/netif/osmux.h
index 14c967f..c1f527a 100644
--- a/include/osmocom/netif/osmux.h
+++ b/include/osmocom/netif/osmux.h
@@ -5,7 +5,7 @@
/* OSmux header:
*
- * ft (3 bits): 0=signalling, 1=voice
+ * ft (3 bits): 0=signalling, 1=voice, 2=dummy
* ctr (3 bits): Number of batched AMR payloads (starting 0)
* amr_f (1 bit): AMR F field (RFC3267)
* amr_q (1 bit): AMR Q field (RFC3267)
@@ -17,6 +17,7 @@
#define OSMUX_FT_SIGNAL 0
#define OSMUX_FT_VOICE_AMR 1
+#define OSMUX_FT_DUMMY 2
struct osmux_hdr {
#if OSMO_IS_BIG_ENDIAN
@@ -84,6 +85,7 @@ int osmux_snprintf(char *buf, size_t size, struct msgb *msg);
void osmux_xfrm_input_init(struct osmux_in_handle *h);
void osmux_xfrm_input_fini(struct osmux_in_handle *h);
+int osmux_xfrm_input_open_circuit(struct osmux_in_handle *h, int ccid, int dummy);
void osmux_xfrm_input_close_circuit(struct osmux_in_handle *h, int ccid);
int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int ccid);
diff --git a/src/osmux.c b/src/osmux.c
index 4451b5a..74883d9 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -56,16 +56,29 @@ static uint32_t osmux_get_payload_len(struct osmux_hdr *osmuxh)
return osmo_amr_bytes(osmuxh->amr_ft) * (osmuxh->ctr+1);
}
-struct osmux_hdr *osmux_xfrm_output_pull(struct msgb *msg)
+static uint32_t osmux_ft_dummy_size(uint8_t amr_ft, uint32_t batch_factor)
{
- struct osmux_hdr *osmuxh = NULL;
+ return sizeof(struct osmux_hdr) + (osmo_amr_bytes(amr_ft) * batch_factor);
+}
+struct osmux_hdr *osmux_xfrm_output_pull(struct msgb *msg)
+{
+ struct osmux_hdr *osmuxh;
+next:
+ osmuxh = NULL;
if (msg->len > sizeof(struct osmux_hdr)) {
size_t len;
osmuxh = (struct osmux_hdr *)msg->data;
- if (osmuxh->ft != OSMUX_FT_VOICE_AMR) {
+ switch (osmuxh->ft) {
+ case OSMUX_FT_VOICE_AMR:
+ break;
+ case OSMUX_FT_DUMMY:
+ msgb_pull(msg, osmux_ft_dummy_size(osmuxh->amr_ft,
+ osmuxh->ctr + 1));
+ goto next;
+ default:
LOGP(DLMIB, LOGL_ERROR, "Discarding unsupported Osmux FT %d\n",
osmuxh->ft);
return NULL;
@@ -185,6 +198,7 @@ struct osmux_batch {
unsigned int remaining_bytes;
uint8_t seq;
uint32_t nmsgs;
+ int dummy;
};
struct osmux_circuit {
@@ -192,6 +206,7 @@ struct osmux_circuit {
int ccid;
struct llist_head msg_list;
int nmsgs;
+ int dummy;
};
static int osmux_batch_enqueue(struct msgb *msg, struct osmux_circuit *circuit)
@@ -285,8 +300,31 @@ static int osmux_xfrm_encode_amr(struct osmux_batch *batch,
return 0;
}
+static void osmux_encode_dummy(struct osmux_batch *batch, uint32_t batch_factor,
+ struct osmux_input_state *state)
+{
+ struct osmux_hdr *osmuxh;
+ /* TODO: This should be configurable at some point. */
+ uint32_t payload_size = osmux_ft_dummy_size(AMR_FT_3, batch_factor) -
+ sizeof(struct osmux_hdr);
+
+ osmuxh = (struct osmux_hdr *)state->out_msg->tail;
+ osmuxh->ft = OSMUX_FT_DUMMY;
+ osmuxh->ctr = batch_factor - 1;
+ osmuxh->amr_f = 0;
+ osmuxh->amr_q= 0;
+ osmuxh->seq = 0;
+ osmuxh->circuit_id = state->ccid;
+ osmuxh->amr_cmr = 0;
+ osmuxh->amr_ft = AMR_FT_3;
+ msgb_put(state->out_msg, sizeof(struct osmux_hdr));
+
+ memset(state->out_msg->tail, 0xff, payload_size);
+ msgb_put(state->out_msg, payload_size);
+}
+
static struct msgb *osmux_build_batch(struct osmux_batch *batch,
- uint32_t batch_size)
+ uint32_t batch_size, uint32_t batch_factor)
{
struct msgb *batch_msg;
struct osmux_circuit *circuit;
@@ -305,6 +343,15 @@ static struct msgb *osmux_build_batch(struct osmux_batch *batch,
struct msgb *cur, *tmp;
int ctr = 0;
+ if (circuit->dummy) {
+ struct osmux_input_state state = {
+ .out_msg = batch_msg,
+ .ccid = circuit->ccid,
+ };
+ osmux_encode_dummy(batch, batch_factor, &state);
+ continue;
+ }
+
llist_for_each_entry_safe(cur, tmp, &circuit->msg_list, list) {
struct osmux_input_state state = {
.msg = cur,
@@ -348,7 +395,7 @@ void osmux_xfrm_input_deliver(struct osmux_in_handle *h)
#ifdef DEBUG_MSG
LOGP(DLMIB, LOGL_DEBUG, "invoking delivery function\n");
#endif
- batch_msg = osmux_build_batch(batch, h->batch_size);
+ batch_msg = osmux_build_batch(batch, h->batch_size, h->batch_factor);
h->stats.output_osmux_msgs++;
h->stats.output_osmux_bytes += batch_msg->len;
@@ -356,6 +403,11 @@ void osmux_xfrm_input_deliver(struct osmux_in_handle *h)
h->deliver(batch_msg, h->data);
osmo_timer_del(&batch->timer);
batch->remaining_bytes = h->batch_size;
+
+ if (batch->dummy) {
+ osmo_timer_schedule(&batch->timer, 0,
+ h->batch_factor * DELTA_RTP_MSG);
+ }
}
static void osmux_batch_timer_expired(void *data)
@@ -462,7 +514,8 @@ osmux_batch_find_circuit(struct osmux_batch *batch, int ccid)
}
static struct osmux_circuit *
-osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
+osmux_batch_add_circuit(struct osmux_batch *batch, int ccid, int dummy,
+ int batch_factor)
{
struct osmux_circuit *circuit;
@@ -482,6 +535,13 @@ osmux_batch_add_circuit(struct osmux_batch *batch, int ccid)
INIT_LLIST_HEAD(&circuit->msg_list);
llist_add_tail(&circuit->head, &batch->circuit_list);
+ if (dummy) {
+ circuit->dummy = dummy;
+ batch->dummy++;
+ if (!osmo_timer_pending(&batch->timer))
+ osmo_timer_schedule(&batch->timer, 0,
+ batch_factor * DELTA_RTP_MSG);
+ }
return circuit;
}
@@ -493,6 +553,8 @@ static void osmux_batch_del_circuit(struct osmux_batch *batch, int
ccid)
if (circuit == NULL)
return;
+ if (circuit->dummy)
+ batch->dummy--;
llist_del(&circuit->head);
talloc_free(circuit);
}
@@ -503,51 +565,48 @@ osmux_batch_add(struct osmux_batch *batch, int batch_factor, struct
msgb *msg,
{
int bytes = 0, amr_payload_len;
struct osmux_circuit *circuit;
+ struct msgb *cur;
circuit = osmux_batch_find_circuit(batch, ccid);
+ if (!circuit)
+ return -1;
+ /* We've seen the first RTP message, disable dummy padding */
+ if (circuit->dummy) {
+ circuit->dummy = 0;
+ batch->dummy--;
+ }
amr_payload_len = osmux_rtp_amr_payload_len(msg, rtph);
if (amr_payload_len < 0)
return -1;
/* First check if there is room for this message in the batch */
bytes += amr_payload_len;
- if (!circuit || circuit->nmsgs == 0)
+ if (circuit->nmsgs == 0)
bytes += sizeof(struct osmux_hdr);
/* No room, sorry. You'll have to retry */
if (bytes > batch->remaining_bytes)
return 1;
- if (circuit) {
- struct msgb *cur;
-
- /* Extra validation: check if this message already exists,
- * should not happen but make sure we don't propagate
- * duplicated messages.
- */
- llist_for_each_entry(cur, &circuit->msg_list, list) {
- struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur);
- if (rtph2 == NULL)
- return -1;
-
- /* Already exists message with this sequence, skip */
- if (rtph2->sequence == rtph->sequence) {
- LOGP(DLMIB, LOGL_ERROR, "already exists "
- "message with seq=%u, skip it\n",
- rtph->sequence);
- return -1;
- }
- }
- /* Handle RTP packet loss scenario */
- osmux_replay_lost_packets(circuit, rtph);
+ /* Extra validation: check if this message already exists, should not
+ * happen but make sure we don't propagate duplicated messages.
+ */
+ llist_for_each_entry(cur, &circuit->msg_list, list) {
+ struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur);
+ if (rtph2 == NULL)
+ return -1;
- } else {
- /* This is the first message with that ssrc we've seen */
- circuit = osmux_batch_add_circuit(batch, ccid);
- if (!circuit)
+ /* Already exists message with this sequence, skip */
+ if (rtph2->sequence == rtph->sequence) {
+ LOGP(DLMIB, LOGL_ERROR, "already exists "
+ "message with seq=%u, skip it\n",
+ rtph->sequence);
return -1;
+ }
}
+ /* Handle RTP packet loss scenario */
+ osmux_replay_lost_packets(circuit, rtph);
/* This batch is full, force batch delivery */
if (osmux_batch_enqueue(msg, circuit) < 0)
@@ -655,6 +714,14 @@ void osmux_xfrm_input_init(struct osmux_in_handle *h)
LOGP(DLMIB, LOGL_DEBUG, "initialized osmux input converter\n");
}
+int osmux_xfrm_input_open_circuit(struct osmux_in_handle *h, int ccid,
+ int dummy)
+{
+ struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
+
+ return osmux_batch_add_circuit(batch, ccid, dummy, h->batch_factor) ? 0 : -1;
+}
+
void osmux_xfrm_input_close_circuit(struct osmux_in_handle *h, int ccid)
{
struct osmux_batch *batch = (struct osmux_batch *)h->internal_data;
--
1.7.10.4
10 Aug
10 Aug
10:27 a.m.
New subject: [PATCH libosmo-netif 09/18] osmux: introduce osmux_xfrm_input_open_circuit()
On Sat, Aug 08, 2015 at 09:08:53PM +0200, Holger Freyther wrote:
OK, will rename this.
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org
wrote:
@@ -185,6 +198,7 @@ struct osmux_batch {
unsigned int remaining_bytes;
uint8_t seq;
uint32_t nmsgs;
+ int dummy;
ndummy?
21 Jul
21 Jul
4:23 p.m.
New subject: [PATCH libosmo-netif 10/18] tests: osmux: adapt it to use the new circuit API
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
This also introduces a spare circuit that contains no voice data to test
bandwidth preallocation through the new osmux dummy frame type.
---
tests/osmux/osmux_test.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index dae9aa2..1b87db7 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -135,6 +135,13 @@ int main(void)
/* If the test takes longer than 10 seconds, abort it */
alarm(10);
+ for (i = 0; i < 2; i++)
+ osmux_xfrm_input_open_circuit(&h_input, i, 0);
+
+ /* Add two circuits with dummy padding */
+ osmux_xfrm_input_open_circuit(&h_input, 2, 1);
+ osmux_xfrm_input_open_circuit(&h_input, 3, 1);
+
for (i=1; i<64; i++) {
msg = msgb_alloc(1500, "test");
if (!msg)
@@ -178,6 +185,12 @@ int main(void)
k = 0;
}
}
+
+ for (i = 0; i < 4; i++)
+ osmux_xfrm_input_close_circuit(&h_input, i);
+
+ osmux_xfrm_input_fini(&h_input);
+
fprintf(stdout, "OK: Test passed\n");
return EXIT_SUCCESS;
}
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 11/18] tests: osmux: factor out main test loop
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Move main test loop routine to the new osmux_test_loop() function.
---
tests/osmux/osmux_test.c | 64 ++++++++++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 28 deletions(-)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index 1b87db7..51f026c 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -112,7 +112,7 @@ static void sigalarm_handler(int foo)
exit(EXIT_FAILURE);
}
-int main(void)
+static void osmux_test_loop(int ccid)
{
struct msgb *msg;
char buf[1024];
@@ -120,32 +120,10 @@ int main(void)
uint16_t seq;
int i, j, k = 0;
- if (signal(SIGALRM, sigalarm_handler) == SIG_ERR) {
- perror("signal");
- exit(EXIT_FAILURE);
- }
-
- /* This test doesn't use it, but osmux requires it internally. */
- osmo_init_logging(&osmux_test_log_info);
- log_set_log_level(osmo_stderr_target, LOGL_DEBUG);
-
- osmux_xfrm_input_init(&h_input);
- osmux_xfrm_output_init(&h_output, 0x7000000);
-
- /* If the test takes longer than 10 seconds, abort it */
- alarm(10);
-
- for (i = 0; i < 2; i++)
- osmux_xfrm_input_open_circuit(&h_input, i, 0);
-
- /* Add two circuits with dummy padding */
- osmux_xfrm_input_open_circuit(&h_input, 2, 1);
- osmux_xfrm_input_open_circuit(&h_input, 3, 1);
-
- for (i=1; i<64; i++) {
+ for (i = 1; i < 64; i++) {
msg = msgb_alloc(1500, "test");
if (!msg)
- return 0;
+ exit(EXIT_FAILURE);
memcpy(msg->data, rtp_pkt, sizeof(rtp_pkt));
msgb_put(msg, sizeof(rtp_pkt));
@@ -155,7 +133,7 @@ int main(void)
rtph->sequence = htons(seq);
osmo_rtp_snprintf(buf, sizeof(buf), msg);
- fprintf(stderr, "adding to ccid=%u %s\n", i % 2, buf);
+ fprintf(stderr, "adding to ccid=%u %s\n", (i % 2) + ccid, buf);
rtp_pkts++;
k++;
@@ -164,7 +142,7 @@ int main(void)
* gaps between two messages to test the osmux replaying
* feature.
*/
- osmux_xfrm_input(&h_input, msg, i % 2);
+ osmux_xfrm_input(&h_input, msg, (i % 2) + ccid);
if (i % 4 == 0) {
gettimeofday(&last, NULL);
@@ -179,12 +157,42 @@ int main(void)
* messages that are extracted from OSMUX has been
* delivered.
*/
- for (j=0; j<k-2; j++)
+ for (j = 0; j < k-2; j++)
osmo_select_main(0);
k = 0;
}
}
+}
+
+int main(void)
+{
+ int i;
+
+ if (signal(SIGALRM, sigalarm_handler) == SIG_ERR) {
+ perror("signal");
+ exit(EXIT_FAILURE);
+ }
+
+ /* This test doesn't use it, but osmux requires it internally. */
+ osmo_init_logging(&osmux_test_log_info);
+ log_set_log_level(osmo_stderr_target, LOGL_DEBUG);
+
+ osmux_xfrm_input_init(&h_input);
+ osmux_xfrm_output_init(&h_output, 0x7000000);
+
+ /* If the test takes longer than 10 seconds, abort it */
+ alarm(10);
+
+ for (i = 0; i < 2; i++)
+ osmux_xfrm_input_open_circuit(&h_input, i, 0);
+
+ /* Add two circuits with dummy padding */
+ osmux_xfrm_input_open_circuit(&h_input, 2, 1);
+ osmux_xfrm_input_open_circuit(&h_input, 3, 1);
+
+ /* Start pushing voice data to circuits 0 and 1 */
+ osmux_test_loop(0);
for (i = 0; i < 4; i++)
osmux_xfrm_input_close_circuit(&h_input, i);
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 12/18] tests: osmux: test online deactivation of dummy padding
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Add RTP packets to circuit with dummy padding enabled to test automatic
deactivation once when start seeing real voice traffic.
---
tests/osmux/osmux_test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index 51f026c..09ef9fd 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -193,6 +193,10 @@ int main(void)
/* Start pushing voice data to circuits 0 and 1 */
osmux_test_loop(0);
+ /* ... now start pushing voice data to circuits 2 and 3. This circuits
+ * comes with dummy padding enabled.
+ */
+ osmux_test_loop(2);
for (i = 0; i < 4; i++)
osmux_xfrm_input_close_circuit(&h_input, i);
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 13/18] tests: osmux: validate dummy padding with no voice data interaction
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Make sure that early dummy bandwitch preallocation works fine in the absence of
any kind of voice traffic.
---
tests/osmux/osmux_test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index 09ef9fd..0ad02d3 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -191,6 +191,10 @@ int main(void)
osmux_xfrm_input_open_circuit(&h_input, 2, 1);
osmux_xfrm_input_open_circuit(&h_input, 3, 1);
+ /* Wait 10 times to make sure dummy padding timer works fine */
+ for (i = 0; i < 10; i++)
+ osmo_select_main(0);
+
/* Start pushing voice data to circuits 0 and 1 */
osmux_test_loop(0);
/* ... now start pushing voice data to circuits 2 and 3. This circuits
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 14/18] tests: osmux: test circuit reopening after closure
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Make sure circuit routines works correctly.
---
tests/osmux/osmux_test.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index 0ad02d3..7cc838d 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -205,6 +205,13 @@ int main(void)
for (i = 0; i < 4; i++)
osmux_xfrm_input_close_circuit(&h_input, i);
+ /* Reopen with two circuits and retest */
+ osmux_xfrm_input_open_circuit(&h_input, 0, 0);
+ osmux_xfrm_input_open_circuit(&h_input, 1, 1);
+ osmux_test_loop(0);
+ osmux_xfrm_input_close_circuit(&h_input, 0);
+ osmux_xfrm_input_close_circuit(&h_input, 1);
+
osmux_xfrm_input_fini(&h_input);
fprintf(stdout, "OK: Test passed\n");
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 15/18] osmux: kill osmux_get_hdr()
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Never used, so let's get rid of this function. We can recover it later on in
case we need it.
---
include/osmocom/netif/osmux.h | 2 --
src/osmux.c | 12 ------------
2 files changed, 14 deletions(-)
diff --git a/include/osmocom/netif/osmux.h b/include/osmocom/netif/osmux.h
index c1f527a..83bb2e1 100644
--- a/include/osmocom/netif/osmux.h
+++ b/include/osmocom/netif/osmux.h
@@ -70,8 +70,6 @@ struct osmux_out_handle {
uint32_t rtp_ssrc;
};
-struct osmux_hdr *osmux_get_hdr(struct msgb *msg);
-
static inline uint8_t *osmux_get_payload(struct osmux_hdr *osmuxh)
{
return (uint8_t *)osmuxh + sizeof(struct osmux_hdr);
diff --git a/src/osmux.c b/src/osmux.c
index 74883d9..eedae69 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -39,18 +39,6 @@
static void *osmux_ctx;
-struct osmux_hdr *osmux_get_hdr(struct msgb *msg)
-{
- struct osmux_hdr *osmuxh = (struct osmux_hdr *)msg->data;
-
- if (msg->len < sizeof(struct osmux_hdr)) {
- DEBUGPC(DLMUX, "received OSMUX frame too short (len = %d)\n",
- msg->len);
- return NULL;
- }
- return osmuxh;
-}
-
static uint32_t osmux_get_payload_len(struct osmux_hdr *osmuxh)
{
return osmo_amr_bytes(osmuxh->amr_ft) * (osmuxh->ctr+1);
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 16/18] tests: osmux: fix msgb leaks
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
---
tests/osmux/osmux_test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index 7cc838d..9ceabcd 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -78,6 +78,7 @@ static void tx_cb(struct msgb *msg, void *data)
}
rtp_pkts--;
+ msgb_free(msg);
}
static struct osmux_out_handle h_output;
@@ -98,6 +99,7 @@ static void osmux_deliver(struct msgb *batch_msg, void *data)
osmux_xfrm_output(osmuxh, &h_output, &list);
osmux_tx_sched(&list, tx_cb, NULL);
}
+ msgb_free(batch_msg);
}
struct osmux_in_handle h_input = {
--
1.7.10.4
4:23 p.m.
New subject: [PATCH libosmo-netif 17/18] tests: compile tests with debugging symbols, ie. -g
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Useful when debuggin via valgrind/gdb.
---
tests/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a986940..f99e276 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,4 +1,4 @@
-AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS)
+AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g
AM_LDFLAGS = $(LIBOSMOCORE_LDFLAGS)
check_PROGRAMS = osmux/osmux_test
--
1.7.10.4
8 Aug
8 Aug
9:11 p.m.
New subject: [PATCH libosmo-netif 17/18] tests: compile tests with debugging symbols, ie. -g
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org wrote:
-AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS)
+AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g
I like -ggdb3 :)
In terms of the tests. You run the code but I don’t see many asserts. Could you
please explain how the “no voice traffic” code tests? Would a lack of data end
printing a message that would cause the test to fail?
holger
10 Aug
10 Aug
10:33 a.m.
New subject: [PATCH libosmo-netif 17/18] tests: compile tests with debugging symbols, ie. -g
On Sat, Aug 08, 2015 at 09:11:39PM +0200, Holger Freyther wrote:
OK, will change this.
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org
wrote:
-AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS)
+AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g
I like -ggdb3 :) In terms of the tests. You run the code but I don’t
see many asserts. Could you
please explain how the “no voice traffic” code tests? Would a lack of data end
printing a message that would cause the test to fail?
The test most likely will timeout if it doesn't get any voice traffic.
But I think it's a good idea to add more asserts.
In summary:
1) Will rename dummy to ndummy.
2) Will mangle this patch to replace -g to -ggdb3.
And I will push out this patchset, then will send two follow up
patches:
3) Will revisit the one timer running + circuit gone scenario. This
will come in a follow up patch since this problem since to be there
since the beginning.
4) Will add more asserts to validate osmux outputs from tests.
Fine with this procedure? Let me know if I'm missing anything.
Thanks for reviewing.
10:29 a.m.
New subject: [PATCH libosmo-netif 17/18] tests: compile tests with debugging symbols, ie. -g
On 10 Aug 2015, at 10:33, Pablo Neira Ayuso
<pablo(a)gnumonks.org> wrote:
On Sat, Aug 08, 2015 at 09:11:39PM +0200, Holger Freyther wrote:
OK, will change this.
This was just personal taste. No need to update if -g was good enough for you. I
tend to compile with CFLAGS=“-Og -ggdb3” make
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org
wrote:
-AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS)
+AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g
I like -ggdb3 :) Fine with this procedure? Let me know if I'm
missing anything.
yes, that is fine.
19 Aug
19 Aug
2:52 a.m.
New subject: [PATCH libosmo-netif 17/18] tests: compile tests with debugging symbols, ie. -g
On Mon, Aug 10, 2015 at 10:29:21AM +0200, Holger Freyther wrote:
[...]
Just pushed out this to master, including a patch to rename dummy to
ndummy as you suggested. Will follow up soon by reviewing the other
concerns you spot during review.
Cheers from Seattle!
Fine with this
procedure? Let me know if I'm missing anything.
yes, that is fine.
7:51 a.m.
New subject: [PATCH libosmo-netif 17/18] tests: compile tests with debugging symbols, ie. -g
On 19 Aug 2015, at 02:52, Pablo Neira Ayuso
<pablo(a)gnumonks.org> wrote:
Hi!
Just pushed out this to master, including a patch to
rename dummy to
ndummy as you suggested. Will follow up soon by reviewing the other
concerns you spot during review.
the CI fails because of missing documentation strings in the new command
Documentation error (missing docs):
<command id='osmux dummy (on|off)'>
<param name='off' doc='(null)' />
Documentation error (missing docs):
<command id='osmux dummy (on|off)'>
<param name='off' doc='(null)' />
with the VTY code every parameter needs to be documented in this case it
needs to be:
osmux
dummy
on
off
Given the error message I think everything but off is documented.
enjoy the city!
holger
21 Jul
21 Jul
4:23 p.m.
New subject: [PATCH libosmo-netif 18/18] tests: osmux: iterate 64 times in osmo_test_loop()
From: Pablo Neira Ayuso <pablo(a)soleta.eu>
Instead of 63, this resolves major "definitely lost" remaining leak that
valgrind reports regarding msgb.
---
tests/osmux/osmux_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index 9ceabcd..94b95de 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -122,7 +122,7 @@ static void osmux_test_loop(int ccid)
uint16_t seq;
int i, j, k = 0;
- for (i = 1; i < 64; i++) {
+ for (i = 1; i < 65; i++) {
msg = msgb_alloc(1500, "test");
if (!msg)
exit(EXIT_FAILURE);
--
1.7.10.4
22 Jul
22 Jul
1:04 a.m.
On 21 Jul 2015, at 16:23, pablo(a)gnumonks.org wrote:
Hi!,
If no major concerns, I'll push this initial batch
soon into the repository.
it is a bit late for me right now. The granularity of changes, comments, commit
messages all look great. Please give me until tomorrow evening to have a
second look.
holger
3175
days inactive
3204
days old
31 comments
3 participants
participants (3)
-
Holger Freyther -
Pablo Neira Ayuso -
pablo＠gnumonks.org