diff mbox

[libosmo-netif,08/18] osmux: introduce osmux_xfrm_input_close_circuit()

Message ID 1437488613-3943-9-git-send-email-pablo@gnumonks.org
State Accepted
Headers show

Commit Message

Pablo Neira Ayuso July 21, 2015, 2:23 p.m. UTC 
From: Pablo Neira Ayuso <pablo@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(-)

Comments

Holger Freyther Aug. 8, 2015, 7:04 p.m. UTC | #1 
> On 21 Jul 2015, at 16:23, pablo@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?
Pablo Neira Ayuso Aug. 10, 2015, 8:26 a.m. UTC | #2 
On Sat, Aug 08, 2015 at 09:04:12PM +0200, Holger Freyther wrote:
> 
> > On 21 Jul 2015, at 16:23, pablo@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?

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.
diff mbox

Patch

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;