25 Mar
2015
25 Mar
'15
11:39 p.m.
On Wed, Aug 27, 2014 at 11:54:22PM +0200, Harald Welte wrote:
+ /* don't free, because we forwarded data */
+ return 1;
Maybe we can make that a define for the ownership handling
instead of a magic number.
+static int ph_data_req(struct gsm_bts_trx *trx,
struct msgb *msg,
+ struct osmo_phsap_prim *l1sap)
+{
...
+ } else {
+ LOGP(DL1C, LOGL_NOTICE, "unknown prim %d op %d "
+ "chan_nr %d link_id %d\n", l1sap->oph.primitive,
+ l1sap->oph.operation, chan_nr, link_id);
sure this should not happen in normal operation
msgb_free(l1sap);
+ return -EINVAL;
+ }
+ /* wrap zeroed l1p structure arrount payload
around...
+ * this must be done in three steps, since the
actual
+ * payload is not at the end but inside the l1p structure. */
maybe we can use the pull_to_l3 method hre?
+
+ /* send message to DSP's queue */
+ osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], msg);
The existing code doesn't do this either but wqueue_enqueue is
written in a way that if the queue has reached the max depth the
message will not be added. This was done to let the caller decide
if it should clear the queue or wants to print the msgb. The result
is that the result of enqueue should be checked and if the queuing
was rejected msgb_free must be called.
+int bts_model_l1sap_down(struct gsm_bts_trx *trx,
struct osmo_phsap_prim *l1sap)
+{
+ struct msgb *msg = l1sap->oph.msg;
+ int rc = 0;
+
+ switch (OSMO_PRIM_HDR(&l1sap->oph)) {
+ case OSMO_PRIM(PRIM_PH_DATA, PRIM_OP_REQUEST):
+ rc = ph_data_req(trx, msg, l1sap);
+ break;
+ default:
+ LOGP(DL1C, LOGL_NOTICE, "unknown prim %d op %d\n",
+ l1sap->oph.primitive, l1sap->oph.operation);
+ rc = -EINVAL;
+ }
+
+ if (rc)
+ msgb_free(msg);
did the rc == 1 turn into a rc != 0 here?
@@ -598,6 +740,7 @@ tx:
/* transmit */
osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], resp_msg);
the existing osmo_queue_enquire issue.
@@ -952,8 +1095,8 @@ static int l1if_handle_ind(struct
femtol1_hdl *fl1, struct msgb *msg)
case GsmL1_PrimId_PhConnectInd:
break;
case GsmL1_PrimId_PhReadyToSendInd:
- rc = handle_ph_readytosend_ind(fl1, &l1p->u.phReadyToSendInd);
- break;
+ return handle_ph_readytosend_ind(fl1, &l1p->u.phReadyToSendInd,
+ msg);
maybe it is better to split the switch into two cases to show the new
and old memory semantics?