bluetooth: Controller: Refactor node_rx footer to avoid ptr arithmetic

The old footer was appended after PDU using pointer arithmetic. Now
the footer fields have been moved to the header struct, the
footer fields are now statically located in the data structure,
this is type safe and fields can be referred to by their actual
names rather than indirectly through reference to other members,
thus avoiding pointer arithmetic. Secondly, this change will pave
the way for adding other meta data in the future.

Signed-off-by: Asger Munk Nielsen <[email protected]>

Author: asmk-ot

subsys/bluetooth/controller/hci/hci.c

@@ -35,6 +35,10 @@
 #include "hci_internal.h"
 #include "hci_vendor.h"
 
+#if (!defined(CONFIG_BT_LL_SW_SPLIT))
+#include "ll_sw/ctrl.h"
+#endif /* CONFIG_BT_LL_SW_SPLIT */
+
 #if defined(CONFIG_BT_HCI_MESH_EXT)
 #include "ll_sw/ll_mesh.h"
 #endif /* CONFIG_BT_HCI_MESH_EXT */
@@ -1309,7 +1313,7 @@ static void le_read_max_data_len(struct net_buf *buf, struct net_buf **evt)
 #if defined(CONFIG_BT_CTLR_PHY)
 static void le_read_phy(struct net_buf *buf, struct net_buf **evt)
 {
-	struct bt_hci_cp_le_read_phy *cmd = (void *) buf->data;
+	struct bt_hci_cp_le_read_phy *cmd = (void *)buf->data;
 	struct bt_hci_rp_le_read_phy *rp;
 	u16_t handle;
 	u8_t status;
@@ -1427,6 +1431,7 @@ static void le_rem_dev_from_rl(struct net_buf *buf, struct net_buf **evt)
 static void le_clear_rl(struct net_buf *buf, struct net_buf **evt)
 {
 	struct bt_hci_evt_cc_status *ccst;
+
 	ccst = hci_cmd_complete(evt, sizeof(*ccst));
 
 	ccst->status = ll_rl_clear();
@@ -2499,8 +2504,15 @@ static void le_advertising_report(struct pdu_data *pdu_data, u8_t *b,
 #endif /* CONFIG_BT_CTLR_EXT_SCAN_FP */
 	s8_t *prssi;
 
-	extra = &b[offsetof(struct node_rx_pdu, pdu) +
-		   offsetof(struct pdu_adv, payload) + adv->len];
+#if defined(CONFIG_BT_LL_SW_SPLIT)
+	struct node_rx_pdu *node_rx;
+
+	node_rx = (struct node_rx_pdu *)b;
+	extra = (u8_t *)&(node_rx->hdr.rx_ftr.param);
+#else
+	extra = (u8_t *)&b[offsetof(struct radio_pdu_node_rx, pdu_data) +
+			   offsetof(struct pdu_adv, payload) + adv->len];
+#endif /* CONFIG_BT_LL_SW_SPLIT */
 
 	/* The Link Layer currently returns RSSI as an absolute value */
 	rssi = -(*extra);
@@ -2592,10 +2604,19 @@ static void le_adv_ext_report(struct pdu_data *pdu_data, u8_t *b,
 {
 	struct pdu_adv *adv = (void *)pdu_data;
 	s8_t rssi;
+	u8_t *extra;
+
+#if defined(CONFIG_BT_LL_SW_SPLIT)
+	struct node_rx_pdu *node_rx;
 
+	node_rx = (struct node_rx_pdu *)b;
+	extra = (u8_t *)&(node_rx->hdr.rx_ftr.param);
+#else
+	extra = (u8_t *)&b[offsetof(struct radio_pdu_node_rx, pdu_data) +
+			   offsetof(struct pdu_adv, payload) + adv->len];
+#endif /* CONFIG_BT_LL_SW_SPLIT */
 	/* The Link Layer currently returns RSSI as an absolute value */
-	rssi = -b[offsetof(struct node_rx_pdu, pdu) +
-		  offsetof(struct pdu_adv, payload) + adv->len];
+	rssi = -(*extra);
 
 	BT_DBG("phy= 0x%x, type= 0x%x, len= %u, tat= %u, rat= %u, rssi=%d dB",
 	       phy, adv->type, adv->len, adv->tx_addr, adv->rx_addr, rssi);
@@ -2674,15 +2695,28 @@ static void le_scan_req_received(struct pdu_data *pdu_data, u8_t *b,
 		char addr_str[BT_ADDR_LE_STR_LEN];
 		bt_addr_le_t addr;
 		u8_t handle;
+		u8_t *extra;
 		s8_t rssi;
 
 		handle = 0U;
 		addr.type = adv->tx_addr;
 		memcpy(&addr.a.val[0], &adv->scan_req.scan_addr[0],
 		       sizeof(bt_addr_t));
+
+#if defined(CONFIG_BT_LL_SW_SPLIT)
+		struct node_rx_pdu *node_rx;
+
+		node_rx = (struct node_rx_pdu *)b;
+		extra = (u8_t *)&(node_rx->hdr.rx_ftr.param);
+#else
+		extra = (u8_t *)&b[offsetof(struct radio_pdu_node_rx,
+					    pdu_data) +
+				   offsetof(struct pdu_adv, payload) +
+				   adv->len];
+#endif /* CONFIG_BT_LL_SW_SPLIT */
+
 		/* The Link Layer currently returns RSSI as an absolute value */
-		rssi = -b[offsetof(struct node_rx_pdu, pdu) +
-			  offsetof(struct pdu_adv, payload) + adv->len];
+		rssi = -(*extra);
 
 		bt_addr_le_to_str(&addr, addr_str, sizeof(addr_str));
 
@@ -2754,7 +2788,8 @@ static void le_conn_complete(struct pdu_data *pdu_data, u16_t handle,
 
 		/* Note: this could be an RPA set as the random address by
 		 * the Host instead of generated by the controller. That said,
-		 * this should make no difference. */
+		 * this should make no difference.
+		 */
 		if ((node_rx->own_addr_type) &&
 		    ((node_rx->own_addr[5] & 0xc0) == 0x40)) {
 			memcpy(&leecc->local_rpa.val[0], &node_rx->own_addr[0],
@@ -3262,9 +3297,16 @@ void hci_acl_encode(struct node_rx_pdu *node_rx, struct net_buf *buf)
 	u16_t handle;
 	u8_t *data;
 
-	pdu_data = (void *)node_rx->pdu;
 	handle = node_rx->hdr.handle;
 
+#if defined(CONFIG_BT_LL_SW_SPLIT)
+	pdu_data = (void *)node_rx->pdu;
+#else
+	u8_t *b = (u8_t *)node_rx;
+
+	pdu_data = (void *)&b[offsetof(struct radio_pdu_node_rx, pdu_data)];
+#endif /* CONFIG_BT_LL_SW_SPLIT */
+
 	switch (pdu_data->ll_id) {
 	case PDU_DATA_LLID_DATA_CONTINUE:
 	case PDU_DATA_LLID_DATA_START:
@@ -3304,7 +3346,13 @@ void hci_evt_encode(struct node_rx_pdu *node_rx, struct net_buf *buf)
 {
 	struct pdu_data *pdu_data;
 
+#if defined(CONFIG_BT_LL_SW_SPLIT)
 	pdu_data = (void *)node_rx->pdu;
+#else
+	u8_t *b = (u8_t *)node_rx;
+
+	pdu_data = (void *)&b[offsetof(struct radio_pdu_node_rx, pdu_data)];
+#endif /* CONFIG_BT_LL_SW_SPLIT */
 
 	if (node_rx->hdr.type != NODE_RX_TYPE_DC_PDU) {
 		encode_control(node_rx, pdu_data, buf);
@@ -3336,7 +3384,13 @@ s8_t hci_get_class(struct node_rx_pdu *node_rx)
 {
 	struct pdu_data *pdu_data;
 
+#if defined(CONFIG_BT_LL_SW_SPLIT)
 	pdu_data = (void *)node_rx->pdu;
+#else
+	u8_t *b = (u8_t *)node_rx;
+
+	pdu_data = (void *)&b[offsetof(struct radio_pdu_node_rx, pdu_data)];
+#endif /* CONFIG_BT_LL_SW_SPLIT */
 
 	if (node_rx->hdr.type != NODE_RX_TYPE_DC_PDU) {
 

subsys/bluetooth/controller/ll_sw/lll.h

@@ -175,6 +175,17 @@ enum node_rx_type {
 #endif /* CONFIG_BT_HCI_MESH_EXT */
 };
 
+
+/* Footer of node_rx_hdr */
+struct node_rx_ftr {
+	void  *param;
+	void  *extra;
+	u32_t ticks_anchor;
+	u32_t us_radio_end;
+	u32_t us_radio_rdy;
+};
+
+
 /* Header of node_rx_pdu */
 struct node_rx_hdr {
 	union {
@@ -185,26 +196,13 @@ struct node_rx_hdr {
 
 	enum node_rx_type   type;
 	u16_t               handle;
-};
 
-/* Footer of node_rx_pdu.
- * TODO: Eliminate footer (move contents to header) to avoid pointer arithmetic
- */
-struct node_rx_ftr {
-	void  *param;
-	void  *extra;
-	u32_t ticks_anchor;
-	u32_t us_radio_end;
-	u32_t us_radio_rdy;
+	struct node_rx_ftr  rx_ftr;
 };
 
 struct node_rx_pdu {
 	struct node_rx_hdr hdr;
 	u8_t               pdu[0];
-	/*
-	 * Footer follows here, but can not be part of this struct due to
-	 * flexible pdu member. Footer obtained by pointer arithmetic
-	 */
 };
 
 enum {

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c

@@ -710,10 +710,7 @@ static inline int isr_rx_pdu(struct lll_adv *lll,
 		memcpy(rx->pdu, pdu_rx, (offsetof(struct pdu_adv, connect_ind) +
 					 sizeof(struct pdu_adv_connect_ind)));
 
-		ftr = (void *)((u8_t *)rx->pdu +
-			       (offsetof(struct pdu_adv, connect_ind) +
-			       sizeof(struct pdu_adv_connect_ind)));
-
+		ftr = &(rx->hdr.rx_ftr);
 		ftr->param = lll;
 		ftr->ticks_anchor = radio_tmr_start_get();
 		ftr->us_radio_end = radio_tmr_end_get() -

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c

@@ -768,10 +768,7 @@ static inline u32_t isr_rx_pdu(struct lll_scan *lll, u8_t devmatch_ok,
 		rx->hdr.type = NODE_RX_TYPE_CONNECTION;
 		rx->hdr.handle = 0xffff;
 
-		ftr = (void *)((u8_t *)rx->pdu +
-			       (offsetof(struct pdu_adv, connect_ind) +
-			       sizeof(struct pdu_adv_connect_ind)));
-
+		ftr = &(rx->hdr.rx_ftr);
 		ftr->param = lll;
 		ftr->ticks_anchor = radio_tmr_start_get();
 		ftr->us_radio_end = conn_space_us -
@@ -1035,8 +1032,8 @@ static u32_t isr_rx_scan_report(struct lll_scan *lll, u8_t rssi_ready,
 	}
 
 	pdu_adv_rx = (void *)node_rx->pdu;
-	extra = &((u8_t *)pdu_adv_rx)[offsetof(struct pdu_adv, payload) +
-				      pdu_adv_rx->len];
+	extra = (u8_t *)&(node_rx->hdr.rx_ftr.param);
+
 	/* save the RSSI value */
 	*extra = (rssi_ready) ? (radio_rssi_get() & 0x7f) : 0x7f;
 	extra += PDU_AC_SIZE_RSSI;

subsys/bluetooth/controller/ll_sw/ull.c

@@ -146,8 +146,7 @@ static MFIFO_DEFINE(pdu_rx_free, sizeof(void *), PDU_RX_CNT);
 static MFIFO_DEFINE(ll_pdu_rx_free, sizeof(void *), LL_PDU_RX_CNT);
 
 #define NODE_RX_HEADER_SIZE      (offsetof(struct node_rx_pdu, pdu))
-#define NODE_RX_FOOTER_SIZE      (sizeof(struct node_rx_ftr))
-#define NODE_RX_STRUCT_OVERHEAD  (NODE_RX_HEADER_SIZE + NODE_RX_FOOTER_SIZE)
+#define NODE_RX_STRUCT_OVERHEAD  (NODE_RX_HEADER_SIZE)
 
 #define PDU_ADVERTIZE_SIZE (PDU_AC_SIZE_MAX + PDU_AC_SIZE_EXTRA)
 #define PDU_DATA_SIZE      (PDU_DC_LL_HEADER_SIZE + LL_LENGTH_OCTETS_RX_MAX)
@@ -553,9 +552,7 @@ void ll_rx_dequeue(void)
 	} else if (rx->type == NODE_RX_TYPE_CONNECTION) {
 		struct node_rx_ftr *ftr;
 
-		ftr = (void *)((u8_t *)((struct node_rx_pdu *)rx)->pdu +
-			       (offsetof(struct pdu_adv, connect_ind) +
-			       sizeof(struct pdu_adv_connect_ind)));
+		ftr = &(rx->rx_ftr);
 
 		if (0) {
 #if defined(CONFIG_BT_PERIPHERAL)

subsys/bluetooth/controller/ll_sw/ull_adv.c

@@ -1140,10 +1140,7 @@ static void disabled_cb(void *param)
 	memset(cc, 0x00, sizeof(struct node_rx_cc));
 	cc->status = 0x3c;
 
-	ftr = (void *)((u8_t *)rx->pdu +
-		       (offsetof(struct pdu_adv, connect_ind) +
-		       sizeof(struct pdu_adv_connect_ind)));
-
+	ftr = &(rx->hdr.rx_ftr);
 	ftr->param = param;
 
 	ll_rx_put(link, rx);

subsys/bluetooth/controller/ll_sw/ull_conn.c

@@ -629,9 +629,7 @@ void ull_conn_setup(memq_link_t *link, struct node_rx_hdr *rx)
 	struct node_rx_ftr *ftr;
 	struct lll_conn *lll;
 
-	ftr = (void *)((u8_t *)((struct node_rx_pdu *)rx)->pdu +
-		       (offsetof(struct pdu_adv, connect_ind) +
-		       sizeof(struct pdu_adv_connect_ind)));
+	ftr = &(rx->rx_ftr);
 
 	lll = *((struct lll_conn **)((u8_t *)ftr->param +
 				     sizeof(struct lll_hdr)));