libosmocore: change the memory management of NS2

Until now NS2 always free'd it's own memory. Even when the msg
was sent as primitive to the upper layer.
Change the memory ownership when sending a primitive to the upper layer.
The upper layer has to free the msg buffer.

Merge together with: I180433735bfbb3375c41318d7a7709d5845199ba (osmo-pcu)

Change-Id: Id844d7acbcab102a7dc472d608a5e97a748ecb43

3 files changed, 34 insertions, 16 deletions

diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index ffa31353..403b8b77 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -1001,7 +1001,7 @@ void ns2_nse_notify_unblocked(struct gprs_ns2_vc *nsvc, bool unblocked)
 
 /*! Create a new GPRS NS instance
  *  \param[in] ctx a talloc context to allocate NS instance from
- *  \param[in] cb Call-back function for dispatching primitives to the user
+ *  \param[in] cb Call-back function for dispatching primitives to the user. The Call-back must free all msgb* given in the primitive.
  *  \param[in] cb_data transparent user data passed to Call-back
  *  \returns dynamically allocated gprs_ns_inst; NULL on error */
 struct gprs_ns2_inst *gprs_ns2_instantiate(void *ctx, osmo_prim_cb cb, void *cb_data)
diff --git a/src/gb/gprs_ns2_udp.c b/src/gb/gprs_ns2_udp.c
index 2cc10064..0676314e 100644
--- a/src/gb/gprs_ns2_udp.c
+++ b/src/gb/gprs_ns2_udp.c
@@ -206,7 +206,7 @@ static struct priv_vc *ns2_driver_alloc_vc(struct gprs_ns2_vc_bind *bind, struct
 
 static int handle_nsip_read(struct osmo_fd *bfd)
 {
-	int rc;
+	int rc = 0;
 	int error = 0;
 	struct gprs_ns2_vc_bind *bind = bfd->data;
 	struct osmo_sockaddr saddr;
@@ -240,10 +240,10 @@ static int handle_nsip_read(struct osmo_fd *bfd)
 		}
 	}
 
-	rc = ns2_recv_vc(nsvc, msg);
+	return ns2_recv_vc(nsvc, msg);
+
 out:
 	msgb_free(msg);
-
 	return rc;
 }
 
diff --git a/src/gb/gprs_ns2_vc_fsm.c b/src/gb/gprs_ns2_vc_fsm.c
index 451437f2..4fa53c19 100644
--- a/src/gb/gprs_ns2_vc_fsm.c
+++ b/src/gb/gprs_ns2_vc_fsm.c
@@ -459,8 +459,10 @@ static void gprs_ns2_recv_unitdata(struct osmo_fsm_inst *fi,
 	struct osmo_gprs_ns2_prim nsp = {};
 	uint16_t bvci;
 
-	if (msgb_l2len(msg) < sizeof(*nsh) + 3)
+	if (msgb_l2len(msg) < sizeof(*nsh) + 3) {
+		msgb_free(msg);
 		return;
+	}
 
 	/* TODO: 7.1: For an IP sub-network, an NS-UNITDATA PDU
 	 * for a PTP BVC may indicate a request to change the IP endpoint
@@ -488,6 +490,7 @@ static void gprs_ns2_vc_fsm_allstate_action(struct osmo_fsm_inst *fi,
 {
 	struct gprs_ns2_vc_priv *priv = fi->priv;
 	struct gprs_ns2_inst *nsi = ns_inst_from_fi(fi);
+	struct msgb *msg = data;
 
 	switch (event) {
 	case GPRS_NS2_EV_RESET:
@@ -520,22 +523,31 @@ static void gprs_ns2_vc_fsm_allstate_action(struct osmo_fsm_inst *fi,
 			recv_test_procedure(fi);
 		break;
 	case GPRS_NS2_EV_UNITDATA:
+		/* UNITDATA has to handle the release of msg.
+		 * If send upwards (gprs_ns2_recv_unitdata) it must NOT free
+		 * the msg, the upper layer has to do it.
+		 * Otherwise the msg must be freed.
+		 */
 		switch (fi->state) {
 		case GPRS_NS2_ST_BLOCKED:
 			/* 7.2.1: the BLOCKED_ACK might be lost */
-			if (priv->initiater)
-				gprs_ns2_recv_unitdata(fi, data);
-			else
-				ns2_tx_status(priv->nsvc,
-					      NS_CAUSE_NSVC_BLOCKED,
-					      0, data);
+			if (priv->initiater) {
+				gprs_ns2_recv_unitdata(fi, msg);
+				return;
+			}
+
+			ns2_tx_status(priv->nsvc,
+				      NS_CAUSE_NSVC_BLOCKED,
+				      0, msg);
 			break;
 		/* ALIVE can receive UNITDATA if the ALIVE_ACK is lost */
 		case GPRS_NS2_ST_ALIVE:
 		case GPRS_NS2_ST_UNBLOCKED:
-			gprs_ns2_recv_unitdata(fi, data);
-			break;
+			gprs_ns2_recv_unitdata(fi, msg);
+			return;
 		}
+
+		msgb_free(msg);
 		break;
 	}
 }
@@ -613,6 +625,7 @@ int gprs_ns2_vc_rx(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed
 {
 	struct gprs_ns_hdr *nsh = (struct gprs_ns_hdr *) msg->l2h;
 	struct osmo_fsm_inst *fi = nsvc->fi;
+	int rc = 0;
 	uint8_t cause;
 
 	/* TODO: 7.2: on UNBLOCK/BLOCK: check if NS-VCI is correct,
@@ -622,7 +635,8 @@ int gprs_ns2_vc_rx(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed
 
 	if (gprs_ns2_validate(nsvc, nsh->pdu_type, msg, tp, &cause)) {
 		if (nsh->pdu_type != NS_PDUT_STATUS) {
-			return ns2_tx_status(nsvc, cause, 0, msg);
+			rc = ns2_tx_status(nsvc, cause, 0, msg);
+			goto out;
 		}
 	}
 
@@ -652,15 +666,19 @@ int gprs_ns2_vc_rx(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed
 		osmo_fsm_inst_dispatch(fi, GPRS_NS2_EV_ALIVE_ACK, tp);
 		break;
 	case NS_PDUT_UNITDATA:
+		/* UNITDATA have to free msg because it might send the msg layer upwards */
 		osmo_fsm_inst_dispatch(fi, GPRS_NS2_EV_UNITDATA, msg);
-		break;
+		return 0;
 	default:
 		LOGP(DLNS, LOGL_ERROR, "NSEI=%u Rx unknown NS PDU type %s\n", nsvc->nse->nsei,
 			get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
 		return -EINVAL;
 	}
 
-	return 0;
+out:
+	msgb_free(msg);
+
+	return rc;
 }
 
 /*! is the given NS-VC unblocked? */