[MERGED] osmocom-bb[fixeria/trx]: trxcon: clean up DATA / TRAFFIC indication API

2018-03-11 Thread Vadim Yanitskiy
Vadim Yanitskiy has submitted this change and it was merged.

Change subject: trxcon: clean up DATA / TRAFFIC indication API
..


trxcon: clean up DATA / TRAFFIC indication API

  - change 'l1ctl_tx_data_ind' symbol to 'l1ctl_tx_dt_ind' in
order to indicate that it's used for both DATA and TRAFFIC;

  - introduce a 'traffic' flag, which is used to define either
TRAFFIC or DATA indication type;

  - pass L2 payload and its length separately from the
Downlink info header.

Change-Id: I9fe65ee9b2d772576b86b7bc85d53518530d1579
---
M src/host/trxcon/l1ctl.c
M src/host/trxcon/l1ctl.h
M src/host/trxcon/sched_lchan_common.c
3 files changed, 23 insertions(+), 27 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/host/trxcon/l1ctl.c b/src/host/trxcon/l1ctl.c
index 3702e8a..3de0cf6 100644
--- a/src/host/trxcon/l1ctl.c
+++ b/src/host/trxcon/l1ctl.c
@@ -183,29 +183,30 @@
return l1ctl_link_send(l1l, msg);
 }
 
-int l1ctl_tx_data_ind(struct l1ctl_link *l1l,
-   struct l1ctl_info_dl *data, uint8_t msg_type)
+/**
+ * Handles both L1CTL_DATA_IND and L1CTL_TRAFFIC_IND.
+ */
+int l1ctl_tx_dt_ind(struct l1ctl_link *l1l, struct l1ctl_info_dl *data,
+   uint8_t *l2, size_t l2_len, bool traffic)
 {
struct l1ctl_info_dl *dl;
struct msgb *msg;
-   size_t len;
+   uint8_t *msg_l2;
 
-   if (msg_type != L1CTL_DATA_IND && msg_type != L1CTL_TRAFFIC_IND) {
-   LOGP(DL1D, LOGL_ERROR, "Incorrect indication type\n");
-   return -EINVAL;
-   }
-
-   msg = l1ctl_alloc_msg(msg_type);
+   msg = l1ctl_alloc_msg(traffic ?
+   L1CTL_TRAFFIC_IND : L1CTL_DATA_IND);
if (msg == NULL)
return -ENOMEM;
 
-   /* We store the payload as a flexible array member */
-   len = sizeof(struct l1ctl_info_dl);
-   len += msg_type == L1CTL_DATA_IND ? 23 : TRAFFIC_DATA_LEN;
-   dl = (struct l1ctl_info_dl *) msgb_put(msg, len);
+   /* Copy DL header */
+   dl = (struct l1ctl_info_dl *) msgb_put(msg, sizeof(*dl));
+   memcpy(dl, data, sizeof(*dl));
 
-   /* Copy header and data from source message */
-   memcpy(dl, data, len);
+   /* Copy the L2 payload if preset */
+   if (l2 && l2_len > 0) {
+   msg_l2 = (uint8_t *) msgb_put(msg, l2_len);
+   memcpy(msg_l2, l2, l2_len);
+   }
 
/* Put message to upper layers */
return l1ctl_link_send(l1l, msg);
diff --git a/src/host/trxcon/l1ctl.h b/src/host/trxcon/l1ctl.h
index 290a0f5..ca8c0be 100644
--- a/src/host/trxcon/l1ctl.h
+++ b/src/host/trxcon/l1ctl.h
@@ -18,8 +18,8 @@
 int l1ctl_tx_reset_conf(struct l1ctl_link *l1l, uint8_t type);
 int l1ctl_tx_reset_ind(struct l1ctl_link *l1l, uint8_t type);
 
-int l1ctl_tx_data_ind(struct l1ctl_link *l1l,
-   struct l1ctl_info_dl *data, uint8_t msg_type);
+int l1ctl_tx_dt_ind(struct l1ctl_link *l1l, struct l1ctl_info_dl *data,
+   uint8_t *l2, size_t l2_len, bool traffic);
 int l1ctl_tx_dt_conf(struct l1ctl_link *l1l,
struct l1ctl_info_dl *data, bool traffic);
 int l1ctl_tx_rach_conf(struct l1ctl_link *l1l, uint32_t fn);
diff --git a/src/host/trxcon/sched_lchan_common.c 
b/src/host/trxcon/sched_lchan_common.c
index d946e57..13c8764 100644
--- a/src/host/trxcon/sched_lchan_common.c
+++ b/src/host/trxcon/sched_lchan_common.c
@@ -90,7 +90,7 @@
struct l1ctl_info_dl *data;
 
/* Allocate memory */
-   data = talloc_zero_size(ts, sizeof(struct l1ctl_info_dl) + l2_len);
+   data = talloc_zero_size(ts, sizeof(struct l1ctl_info_dl));
if (data == NULL)
return -ENOMEM;
 
@@ -108,17 +108,12 @@
/* FIXME: set proper values */
data->snr = 0;
 
-   if (dec_failed) {
-   /* Mark frame as broken */
-   data->fire_crc = 2;
-   } else {
-   /* Fill in the payload */
-   memcpy(data->payload, l2, l2_len);
-   }
+   /* Mark frame as broken if so */
+   data->fire_crc = dec_failed ? 2 : 0;
 
/* Put a packet to higher layers */
-   l1ctl_tx_data_ind(trx->l1l, data, l2_len == GSM_MACBLOCK_LEN ?
-   L1CTL_DATA_IND : L1CTL_TRAFFIC_IND);
+   l1ctl_tx_dt_ind(trx->l1l, data, l2, l2_len,
+   l2_len != GSM_MACBLOCK_LEN);
talloc_free(data);
 
return 0;

-- 
To view, visit https://gerrit.osmocom.org/7212
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9fe65ee9b2d772576b86b7bc85d53518530d1579
Gerrit-PatchSet: 2
Gerrit-Project: osmocom-bb
Gerrit-Branch: fixeria/trx
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 


osmocom-bb[fixeria/trx]: trxcon: clean up DATA / TRAFFIC indication API

2018-03-11 Thread Harald Welte

Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/7212
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fe65ee9b2d772576b86b7bc85d53518530d1579
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: fixeria/trx
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No


[PATCH] osmocom-bb[fixeria/trx]: trxcon: clean up DATA / TRAFFIC indication API

2018-03-11 Thread Vadim Yanitskiy

Review at  https://gerrit.osmocom.org/7212

trxcon: clean up DATA / TRAFFIC indication API

  - change 'l1ctl_tx_data_ind' symbol to 'l1ctl_tx_dt_ind' in
order to indicate that it's used for both DATA and TRAFFIC;

  - introduce a 'traffic' flag, which is used to define either
TRAFFIC or DATA indication type;

  - pass L2 payload and its length separately from the
Downlink info header.

Change-Id: I9fe65ee9b2d772576b86b7bc85d53518530d1579
---
M src/host/trxcon/l1ctl.c
M src/host/trxcon/l1ctl.h
M src/host/trxcon/sched_lchan_common.c
3 files changed, 23 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/12/7212/1

diff --git a/src/host/trxcon/l1ctl.c b/src/host/trxcon/l1ctl.c
index 3702e8a..3de0cf6 100644
--- a/src/host/trxcon/l1ctl.c
+++ b/src/host/trxcon/l1ctl.c
@@ -183,29 +183,30 @@
return l1ctl_link_send(l1l, msg);
 }
 
-int l1ctl_tx_data_ind(struct l1ctl_link *l1l,
-   struct l1ctl_info_dl *data, uint8_t msg_type)
+/**
+ * Handles both L1CTL_DATA_IND and L1CTL_TRAFFIC_IND.
+ */
+int l1ctl_tx_dt_ind(struct l1ctl_link *l1l, struct l1ctl_info_dl *data,
+   uint8_t *l2, size_t l2_len, bool traffic)
 {
struct l1ctl_info_dl *dl;
struct msgb *msg;
-   size_t len;
+   uint8_t *msg_l2;
 
-   if (msg_type != L1CTL_DATA_IND && msg_type != L1CTL_TRAFFIC_IND) {
-   LOGP(DL1D, LOGL_ERROR, "Incorrect indication type\n");
-   return -EINVAL;
-   }
-
-   msg = l1ctl_alloc_msg(msg_type);
+   msg = l1ctl_alloc_msg(traffic ?
+   L1CTL_TRAFFIC_IND : L1CTL_DATA_IND);
if (msg == NULL)
return -ENOMEM;
 
-   /* We store the payload as a flexible array member */
-   len = sizeof(struct l1ctl_info_dl);
-   len += msg_type == L1CTL_DATA_IND ? 23 : TRAFFIC_DATA_LEN;
-   dl = (struct l1ctl_info_dl *) msgb_put(msg, len);
+   /* Copy DL header */
+   dl = (struct l1ctl_info_dl *) msgb_put(msg, sizeof(*dl));
+   memcpy(dl, data, sizeof(*dl));
 
-   /* Copy header and data from source message */
-   memcpy(dl, data, len);
+   /* Copy the L2 payload if preset */
+   if (l2 && l2_len > 0) {
+   msg_l2 = (uint8_t *) msgb_put(msg, l2_len);
+   memcpy(msg_l2, l2, l2_len);
+   }
 
/* Put message to upper layers */
return l1ctl_link_send(l1l, msg);
diff --git a/src/host/trxcon/l1ctl.h b/src/host/trxcon/l1ctl.h
index 290a0f5..ca8c0be 100644
--- a/src/host/trxcon/l1ctl.h
+++ b/src/host/trxcon/l1ctl.h
@@ -18,8 +18,8 @@
 int l1ctl_tx_reset_conf(struct l1ctl_link *l1l, uint8_t type);
 int l1ctl_tx_reset_ind(struct l1ctl_link *l1l, uint8_t type);
 
-int l1ctl_tx_data_ind(struct l1ctl_link *l1l,
-   struct l1ctl_info_dl *data, uint8_t msg_type);
+int l1ctl_tx_dt_ind(struct l1ctl_link *l1l, struct l1ctl_info_dl *data,
+   uint8_t *l2, size_t l2_len, bool traffic);
 int l1ctl_tx_dt_conf(struct l1ctl_link *l1l,
struct l1ctl_info_dl *data, bool traffic);
 int l1ctl_tx_rach_conf(struct l1ctl_link *l1l, uint32_t fn);
diff --git a/src/host/trxcon/sched_lchan_common.c 
b/src/host/trxcon/sched_lchan_common.c
index d946e57..13c8764 100644
--- a/src/host/trxcon/sched_lchan_common.c
+++ b/src/host/trxcon/sched_lchan_common.c
@@ -90,7 +90,7 @@
struct l1ctl_info_dl *data;
 
/* Allocate memory */
-   data = talloc_zero_size(ts, sizeof(struct l1ctl_info_dl) + l2_len);
+   data = talloc_zero_size(ts, sizeof(struct l1ctl_info_dl));
if (data == NULL)
return -ENOMEM;
 
@@ -108,17 +108,12 @@
/* FIXME: set proper values */
data->snr = 0;
 
-   if (dec_failed) {
-   /* Mark frame as broken */
-   data->fire_crc = 2;
-   } else {
-   /* Fill in the payload */
-   memcpy(data->payload, l2, l2_len);
-   }
+   /* Mark frame as broken if so */
+   data->fire_crc = dec_failed ? 2 : 0;
 
/* Put a packet to higher layers */
-   l1ctl_tx_data_ind(trx->l1l, data, l2_len == GSM_MACBLOCK_LEN ?
-   L1CTL_DATA_IND : L1CTL_TRAFFIC_IND);
+   l1ctl_tx_dt_ind(trx->l1l, data, l2, l2_len,
+   l2_len != GSM_MACBLOCK_LEN);
talloc_free(data);
 
return 0;

-- 
To view, visit https://gerrit.osmocom.org/7212
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9fe65ee9b2d772576b86b7bc85d53518530d1579
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: fixeria/trx
Gerrit-Owner: Vadim Yanitskiy