<p>Vadim Yanitskiy <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11395">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">l1ctl_proto.h: use flexible array member for traffic messages<br><br>Unlike the DATA messages, traffic frames may have different length.<br>Instead of having fixed payload (i.e. TCH frame) length, let's<br>introduce a flexible array member. This would allow one to<br>calculate the frame length using the MSGB API.<br><br>Change-Id: I119fa36c84e95c3003d57c19e25f8146ed45c3c6<br>---<br>M include/l1ctl_proto.h<br>M src/host/layer23/src/common/l1ctl.c<br>M src/host/virt_phy/src/virt_prim_traffic.c<br>M src/target/firmware/layer1/prim_tch.c<br>4 files changed, 34 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/l1ctl_proto.h b/include/l1ctl_proto.h</span><br><span>index f1bff86..c6156f5 100644</span><br><span>--- a/include/l1ctl_proto.h</span><br><span>+++ b/include/l1ctl_proto.h</span><br><span>@@ -96,8 +96,6 @@</span><br><span>    L1CTL_MCS9,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#define TRAFFIC_DATA_LEN   40</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /*</span><br><span>  * NOTE: struct size. We do add manual padding out of the believe</span><br><span>  * that it will avoid some unaligned access.</span><br><span>@@ -162,7 +160,7 @@</span><br><span> </span><br><span> /* traffic from the network */</span><br><span> struct l1ctl_traffic_ind {</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t data[TRAFFIC_DATA_LEN];</span><br><span style="color: hsl(120, 100%, 40%);">+       uint8_t data[0];</span><br><span> } __attribute__((packed));</span><br><span> </span><br><span> /*</span><br><span>@@ -346,7 +344,7 @@</span><br><span> </span><br><span> /* traffic data to network */</span><br><span> struct l1ctl_traffic_req {</span><br><span style="color: hsl(0, 100%, 40%);">-       uint8_t data[TRAFFIC_DATA_LEN];</span><br><span style="color: hsl(120, 100%, 40%);">+       uint8_t data[0];</span><br><span> } __attribute__((packed));</span><br><span> </span><br><span> struct l1ctl_tbf_cfg_req {</span><br><span>diff --git a/src/host/layer23/src/common/l1ctl.c b/src/host/layer23/src/common/l1ctl.c</span><br><span>index 96db52f..5d6d9c0 100644</span><br><span>--- a/src/host/layer23/src/common/l1ctl.c</span><br><span>+++ b/src/host/layer23/src/common/l1ctl.c</span><br><span>@@ -762,6 +762,8 @@</span><br><span> {</span><br><span>         struct l1ctl_info_dl *dl;</span><br><span>    struct l1ctl_traffic_ind *ti;</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t frame_len;</span><br><span style="color: hsl(120, 100%, 40%);">+     uint8_t *frame;</span><br><span> </span><br><span>  if (msgb_l1len(msg) < sizeof(*dl)) {</span><br><span>              LOGP(DL1C, LOGL_ERROR, "TRAFFIC IND MSG too short "</span><br><span>@@ -771,10 +773,17 @@</span><br><span> </span><br><span>    /* Header handling */</span><br><span>        dl = (struct l1ctl_info_dl *) msg->l1h;</span><br><span style="color: hsl(0, 100%, 40%);">-      msg->l2h = dl->payload;</span><br><span style="color: hsl(0, 100%, 40%);">-   ti = (struct l1ctl_traffic_ind *) msg->l2h;</span><br><span style="color: hsl(120, 100%, 40%);">+        ti = (struct l1ctl_traffic_ind *) dl->payload;</span><br><span style="color: hsl(120, 100%, 40%);">+     frame = (uint8_t *) ti->data;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    DEBUGP(DL1C, "TRAFFIC IND (%s)\n", osmo_hexdump(ti->data, 33));</span><br><span style="color: hsl(120, 100%, 40%);">+  msg->l2h = dl->payload;</span><br><span style="color: hsl(120, 100%, 40%);">+ msg->l3h = frame;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        /* Calculate the frame length */</span><br><span style="color: hsl(120, 100%, 40%);">+      frame_len = msgb_l3len(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        DEBUGP(DL1C, "TRAFFIC IND len=%zu (%s)\n", frame_len,</span><br><span style="color: hsl(120, 100%, 40%);">+               osmo_hexdump(frame, frame_len));</span><br><span> </span><br><span>         /* distribute or drop */</span><br><span>     if (ms->l1_entity.l1_traffic_ind)</span><br><span>@@ -791,28 +800,28 @@</span><br><span>         struct l1ctl_hdr *l1h;</span><br><span>       struct l1ctl_info_ul *l1i_ul;</span><br><span>        struct l1ctl_traffic_req *tr;</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t frame_len;</span><br><span style="color: hsl(120, 100%, 40%);">+     uint8_t *frame;</span><br><span> </span><br><span>  /* Header handling */</span><br><span>        tr = (struct l1ctl_traffic_req *) msg->l2h;</span><br><span style="color: hsl(120, 100%, 40%);">+        frame = (uint8_t *) tr->data;</span><br><span style="color: hsl(120, 100%, 40%);">+      msg->l3h = frame;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        DEBUGP(DL1C, "TRAFFIC REQ (%s)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-            osmo_hexdump(msg->l2h, msgb_l2len(msg)));</span><br><span style="color: hsl(120, 100%, 40%);">+  /* Calculate the frame length */</span><br><span style="color: hsl(120, 100%, 40%);">+      frame_len = msgb_l3len(msg);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        if (msgb_l2len(msg) != 33) {</span><br><span style="color: hsl(0, 100%, 40%);">-            LOGP(DL1C, LOGL_ERROR, "Traffic Request has incorrect length "</span><br><span style="color: hsl(0, 100%, 40%);">-                        "(%u != 33)\n", msgb_l2len(msg));</span><br><span style="color: hsl(0, 100%, 40%);">-             msgb_free(msg);</span><br><span style="color: hsl(0, 100%, 40%);">-         return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+     DEBUGP(DL1C, "TRAFFIC REQ len=%zu (%s)\n", frame_len,</span><br><span style="color: hsl(120, 100%, 40%);">+               osmo_hexdump(frame, frame_len));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    if ((tr->data[0] >> 4) != 0xd) {</span><br><span style="color: hsl(120, 100%, 40%);">+     if ((frame[0] >> 4) != 0xd) {</span><br><span>          LOGP(DL1C, LOGL_ERROR, "Traffic Request has incorrect magic "</span><br><span style="color: hsl(0, 100%, 40%);">-                 "(%u != 0xd)\n", tr->data[0] >> 4);</span><br><span style="color: hsl(120, 100%, 40%);">+                        "(%u != 0xd)\n", frame[0] >> 4);</span><br><span>             msgb_free(msg);</span><br><span>              return -EINVAL;</span><br><span>      }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-// printf("TX %s\n", osmo_hexdump(tr->data, 33));</span><br><span style="color: hsl(120, 100%, 40%);">+// printf("TX %s\n", osmo_hexdump(frame, frame_len));</span><br><span> </span><br><span>     /* prepend uplink info header */</span><br><span>     l1i_ul = (struct l1ctl_info_ul *) msgb_push(msg, sizeof(*l1i_ul));</span><br><span>diff --git a/src/host/virt_phy/src/virt_prim_traffic.c b/src/host/virt_phy/src/virt_prim_traffic.c</span><br><span>index 4e58de6..5f6b273 100644</span><br><span>--- a/src/host/virt_phy/src/virt_prim_traffic.c</span><br><span>+++ b/src/host/virt_phy/src/virt_prim_traffic.c</span><br><span>@@ -84,6 +84,7 @@</span><br><span>      struct msgb *l1ctl_msg = NULL;</span><br><span>       struct l1ctl_traffic_ind * l1ti;</span><br><span>     struct l1ctl_info_dl * l1dl;</span><br><span style="color: hsl(120, 100%, 40%);">+  uint8_t *frame, frame_len;</span><br><span>   uint8_t rsl_chan_type, subchan, timeslot;</span><br><span>    l1ctl_msg = l1ctl_msgb_alloc(L1CTL_TRAFFIC_IND);</span><br><span>     l1dl = (struct l1ctl_info_dl *) msgb_put(l1ctl_msg, sizeof(*l1dl));</span><br><span>@@ -102,7 +103,10 @@</span><br><span> </span><br><span>       /* TODO: traffic decoding and decryption */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(l1ti->data, msgb_data(msg), msgb_length(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+      frame_len = msgb_length(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = (uint8_t *) msgb_put(l1ctl_msg, frame_len);</span><br><span style="color: hsl(120, 100%, 40%);">+   memcpy(frame, msgb_data(msg), frame_len);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  DEBUGPMS(DL1P, ms, "Tx L1CTL_TRAFFIC_IND (chan_nr=0x%02x, link_id=0x%02x)\n", chan_nr, link_id);</span><br><span>   l1ctl_sap_tx_to_l23_inst(ms, l1ctl_msg);</span><br><span> }</span><br><span>diff --git a/src/target/firmware/layer1/prim_tch.c b/src/target/firmware/layer1/prim_tch.c</span><br><span>index 2dadaaf..a8036d2 100644</span><br><span>--- a/src/target/firmware/layer1/prim_tch.c</span><br><span>+++ b/src/target/firmware/layer1/prim_tch.c</span><br><span>@@ -338,6 +338,7 @@</span><br><span>                                 struct msgb *msg;</span><br><span>                            struct l1ctl_info_dl *dl;</span><br><span>                            struct l1ctl_traffic_ind *ti;</span><br><span style="color: hsl(120, 100%, 40%);">+                         uint8_t *payload;</span><br><span> </span><br><span>                                /* Allocate msgb */</span><br><span>                          /* FIXME: we actually want all allocation out of L1S! */</span><br><span>@@ -349,15 +350,16 @@</span><br><span> </span><br><span>                                 dl = (struct l1ctl_info_dl *) msgb_put(msg, sizeof(*dl));</span><br><span>                            ti = (struct l1ctl_traffic_ind *) msgb_put(msg, sizeof(*ti));</span><br><span style="color: hsl(120, 100%, 40%);">+                         payload = (uint8_t *) msgb_put(msg, 33);</span><br><span> </span><br><span>                                 /* Copy actual data, skipping the information block [0,1,2] */</span><br><span style="color: hsl(0, 100%, 40%);">-                          dsp_memcpy_from_api(ti->data, &traffic_buf[3], 33, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+                         dsp_memcpy_from_api(payload, &traffic_buf[3], 33, 1);</span><br><span> </span><br><span>                                /**</span><br><span>                           * Perform some bit conversations</span><br><span>                             * FIXME: what about other (than FR) codecs?</span><br><span>                                  */</span><br><span style="color: hsl(0, 100%, 40%);">-                             tch_fr_bit_magic(ti, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+                              tch_fr_bit_magic(payload, 1);</span><br><span> </span><br><span>                            /* Give message to up layer */</span><br><span>                               l1_queue_for_l2(msg);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11395">change 11395</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11395"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmocom-bb </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I119fa36c84e95c3003d57c19e25f8146ed45c3c6 </div>
<div style="display:none"> Gerrit-Change-Number: 11395 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>