From 2fcbdaf3ba7f7703aa79b518d4a633d594405221 Mon Sep 17 00:00:00 2001 From: Erik Brockhoff Date: Tue, 15 Nov 2022 13:23:31 +0100 Subject: [PATCH] Bluetooth: controller: refactor pdu encode/decode functions Introducing common pdu struct declarations for conn param req/rsp and data length req/rsp to utilize identicality for optimal pdu handling Signed-off-by: Erik Brockhoff --- subsys/bluetooth/controller/ll_sw/pdu.h | 32 +++ .../controller/ll_sw/ull_llcp_internal.h | 1 - .../bluetooth/controller/ll_sw/ull_llcp_pdu.c | 208 ++++++++---------- 3 files changed, 120 insertions(+), 121 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/pdu.h b/subsys/bluetooth/controller/ll_sw/pdu.h index bb5b5b6fb63..008299a9872 100644 --- a/subsys/bluetooth/controller/ll_sw/pdu.h +++ b/subsys/bluetooth/controller/ll_sw/pdu.h @@ -710,6 +710,26 @@ struct pdu_data_llctrl_conn_param_rsp { uint16_t offset5; } __packed; +/* + * According to Spec Core v5.3, section 2.4.2.17 + * LL_CONNECTION_PARAM_RSP and LL_CONNECTION_PARAM_REQ are identical + * This is utilized in pdu encode/decode, and for this is needed a common struct + */ +struct pdu_data_llctrl_conn_param_req_rsp_common { + uint16_t interval_min; + uint16_t interval_max; + uint16_t latency; + uint16_t timeout; + uint8_t preferred_periodicity; + uint16_t reference_conn_event_count; + uint16_t offset0; + uint16_t offset1; + uint16_t offset2; + uint16_t offset3; + uint16_t offset4; + uint16_t offset5; +} __packed; + struct pdu_data_llctrl_reject_ext_ind { uint8_t reject_opcode; uint8_t error_code; @@ -737,6 +757,18 @@ struct pdu_data_llctrl_length_rsp { uint16_t max_tx_time; } __packed; +/* + * According to Spec Core v5.3, section 2.4.2.21 + * LL_LENGTH_REQ and LL_LENGTH_RSP are identical + * This is utilized in pdu encode/decode, and for this is needed a common struct + */ +struct pdu_data_llctrl_length_req_rsp_common { + uint16_t max_rx_octets; + uint16_t max_rx_time; + uint16_t max_tx_octets; + uint16_t max_tx_time; +} __packed; + struct pdu_data_llctrl_phy_req { uint8_t tx_phys; uint8_t rx_phys; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h index 2a888d4617c..ef502fc9754 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h @@ -500,7 +500,6 @@ void llcp_rp_conn_param_req_apm_reply(struct ll_conn *conn, struct proc_ctx *ctx * Terminate Helper */ void llcp_pdu_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu); -void llcp_ntf_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu); void llcp_pdu_decode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu); /* diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c index a162beb9c45..9d8212c6950 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c @@ -233,17 +233,6 @@ void llcp_pdu_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu) p->error_code = ctx->data.term.error_code; } -void llcp_ntf_encode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu) -{ - struct pdu_data_llctrl_terminate_ind *p; - - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(terminate_ind); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_TERMINATE_IND; - p = &pdu->llctrl.terminate_ind; - p->error_code = ctx->data.term.error_code; -} - void llcp_pdu_decode_terminate_ind(struct proc_ctx *ctx, struct pdu_data *pdu) { ctx->data.term.error_code = pdu->llctrl.terminate_ind.error_code; @@ -307,8 +296,8 @@ static int csrand_get(void *buf, size_t len) } } -#if defined(CONFIG_BT_CENTRAL) -void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu) +#if defined(CONFIG_BT_CENTRAL) || defined(CONFIG_BT_PERIPHERAL) +static void encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu) { struct pdu_data_llctrl_enc_req *p; @@ -320,7 +309,18 @@ void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu) memcpy(p->rand, ctx->data.enc.rand, sizeof(p->rand)); p->ediv[0] = ctx->data.enc.ediv[0]; p->ediv[1] = ctx->data.enc.ediv[1]; +} +#endif /* CONFIG_BT_CENTRAL || CONFIG_BT_PERIPHERAL */ + +#if defined(CONFIG_BT_CENTRAL) +void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu) +{ + struct pdu_data_llctrl_enc_req *p; + + encode_enc_req(ctx, pdu); + /* Optimal getting random data, p->ivm is packed right after p->skdm */ + p = &pdu->llctrl.enc_req; BUILD_ASSERT(offsetof(struct pdu_data_llctrl_enc_req, ivm) == offsetof(struct pdu_data_llctrl_enc_req, skdm) + sizeof(p->skdm), "Member IVM must be after member SKDM"); @@ -332,16 +332,7 @@ void llcp_pdu_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu) #if defined(CONFIG_BT_PERIPHERAL) void llcp_ntf_encode_enc_req(struct proc_ctx *ctx, struct pdu_data *pdu) { - struct pdu_data_llctrl_enc_req *p; - - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(enc_req); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_ENC_REQ; - - p = &pdu->llctrl.enc_req; - memcpy(p->rand, ctx->data.enc.rand, sizeof(p->rand)); - p->ediv[0] = ctx->data.enc.ediv[0]; - p->ediv[1] = ctx->data.enc.ediv[1]; + encode_enc_req(ctx, pdu); } void llcp_pdu_encode_enc_rsp(struct pdu_data *pdu) @@ -487,15 +478,15 @@ void llcp_pdu_decode_phy_rsp(struct proc_ctx *ctx, struct pdu_data *pdu) /* * Connection Update Procedure Helper */ -void llcp_pdu_encode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu) +static void encode_conn_param_req_rsp_common(struct proc_ctx *ctx, struct pdu_data *pdu, + struct pdu_data_llctrl_conn_param_req_rsp_common *p, + uint8_t opcode) { - struct pdu_data_llctrl_conn_param_req *p; - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(conn_param_req); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ; + /* The '+ 1U' is to count in opcode octet, the first member of struct pdu_data_llctrl */ + pdu->len = sizeof(struct pdu_data_llctrl_conn_param_req_rsp_common) + 1U; + pdu->llctrl.opcode = opcode; - p = (void *)&pdu->llctrl.conn_param_req; p->interval_min = sys_cpu_to_le16(ctx->data.cu.interval_min); p->interval_max = sys_cpu_to_le16(ctx->data.cu.interval_max); p->latency = sys_cpu_to_le16(ctx->data.cu.latency); @@ -510,65 +501,48 @@ void llcp_pdu_encode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu) p->offset5 = sys_cpu_to_le16(ctx->data.cu.offsets[5]); } + +void llcp_pdu_encode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu) +{ + encode_conn_param_req_rsp_common(ctx, pdu, + (struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_req, + PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ); +} + void llcp_pdu_encode_conn_param_rsp(struct proc_ctx *ctx, struct pdu_data *pdu) { - struct pdu_data_llctrl_conn_param_req *p; + encode_conn_param_req_rsp_common(ctx, pdu, + (struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_rsp, + PDU_DATA_LLCTRL_TYPE_CONN_PARAM_RSP); +} - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(conn_param_rsp); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_RSP; - - p = (void *)&pdu->llctrl.conn_param_rsp; - p->interval_min = sys_cpu_to_le16(ctx->data.cu.interval_min); - p->interval_max = sys_cpu_to_le16(ctx->data.cu.interval_max); - p->latency = sys_cpu_to_le16(ctx->data.cu.latency); - p->timeout = sys_cpu_to_le16(ctx->data.cu.timeout); - p->preferred_periodicity = ctx->data.cu.preferred_periodicity; - p->reference_conn_event_count = sys_cpu_to_le16(ctx->data.cu.reference_conn_event_count); - p->offset0 = sys_cpu_to_le16(ctx->data.cu.offsets[0]); - p->offset1 = sys_cpu_to_le16(ctx->data.cu.offsets[1]); - p->offset2 = sys_cpu_to_le16(ctx->data.cu.offsets[2]); - p->offset3 = sys_cpu_to_le16(ctx->data.cu.offsets[3]); - p->offset4 = sys_cpu_to_le16(ctx->data.cu.offsets[4]); - p->offset5 = sys_cpu_to_le16(ctx->data.cu.offsets[5]); +static void decode_conn_param_req_rsp_common(struct proc_ctx *ctx, + struct pdu_data_llctrl_conn_param_req_rsp_common *p) +{ + ctx->data.cu.interval_min = sys_le16_to_cpu(p->interval_min); + ctx->data.cu.interval_max = sys_le16_to_cpu(p->interval_max); + ctx->data.cu.latency = sys_le16_to_cpu(p->latency); + ctx->data.cu.timeout = sys_le16_to_cpu(p->timeout); + ctx->data.cu.preferred_periodicity = p->preferred_periodicity; + ctx->data.cu.reference_conn_event_count = sys_le16_to_cpu(p->reference_conn_event_count); + ctx->data.cu.offsets[0] = sys_le16_to_cpu(p->offset0); + ctx->data.cu.offsets[1] = sys_le16_to_cpu(p->offset1); + ctx->data.cu.offsets[2] = sys_le16_to_cpu(p->offset2); + ctx->data.cu.offsets[3] = sys_le16_to_cpu(p->offset3); + ctx->data.cu.offsets[4] = sys_le16_to_cpu(p->offset4); + ctx->data.cu.offsets[5] = sys_le16_to_cpu(p->offset5); } void llcp_pdu_decode_conn_param_req(struct proc_ctx *ctx, struct pdu_data *pdu) { - struct pdu_data_llctrl_conn_param_req *p; - - p = (void *)&pdu->llctrl.conn_param_req; - ctx->data.cu.interval_min = sys_le16_to_cpu(p->interval_min); - ctx->data.cu.interval_max = sys_le16_to_cpu(p->interval_max); - ctx->data.cu.latency = sys_le16_to_cpu(p->latency); - ctx->data.cu.timeout = sys_le16_to_cpu(p->timeout); - ctx->data.cu.preferred_periodicity = p->preferred_periodicity; - ctx->data.cu.reference_conn_event_count = sys_le16_to_cpu(p->reference_conn_event_count); - ctx->data.cu.offsets[0] = sys_le16_to_cpu(p->offset0); - ctx->data.cu.offsets[1] = sys_le16_to_cpu(p->offset1); - ctx->data.cu.offsets[2] = sys_le16_to_cpu(p->offset2); - ctx->data.cu.offsets[3] = sys_le16_to_cpu(p->offset3); - ctx->data.cu.offsets[4] = sys_le16_to_cpu(p->offset4); - ctx->data.cu.offsets[5] = sys_le16_to_cpu(p->offset5); + decode_conn_param_req_rsp_common(ctx, + (struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_req); } void llcp_pdu_decode_conn_param_rsp(struct proc_ctx *ctx, struct pdu_data *pdu) { - struct pdu_data_llctrl_conn_param_rsp *p; - - p = (void *)&pdu->llctrl.conn_param_req; - ctx->data.cu.interval_min = sys_le16_to_cpu(p->interval_min); - ctx->data.cu.interval_max = sys_le16_to_cpu(p->interval_max); - ctx->data.cu.latency = sys_le16_to_cpu(p->latency); - ctx->data.cu.timeout = sys_le16_to_cpu(p->timeout); - ctx->data.cu.preferred_periodicity = p->preferred_periodicity; - ctx->data.cu.reference_conn_event_count = sys_le16_to_cpu(p->reference_conn_event_count); - ctx->data.cu.offsets[0] = sys_le16_to_cpu(p->offset0); - ctx->data.cu.offsets[1] = sys_le16_to_cpu(p->offset1); - ctx->data.cu.offsets[2] = sys_le16_to_cpu(p->offset2); - ctx->data.cu.offsets[3] = sys_le16_to_cpu(p->offset3); - ctx->data.cu.offsets[4] = sys_le16_to_cpu(p->offset4); - ctx->data.cu.offsets[5] = sys_le16_to_cpu(p->offset5); + decode_conn_param_req_rsp_common(ctx, + (struct pdu_data_llctrl_conn_param_req_rsp_common *)&pdu->llctrl.conn_param_rsp); } #endif /* defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) */ @@ -627,43 +601,43 @@ void llcp_pdu_decode_chan_map_update_ind(struct proc_ctx *ctx, struct pdu_data * /* * Data Length Update Procedure Helpers */ +static void encode_length_req_rsp_common(struct pdu_data *pdu, + struct pdu_data_llctrl_length_req_rsp_common *p, + const uint8_t opcode, + const struct data_pdu_length *dle) +{ + pdu->ll_id = PDU_DATA_LLID_CTRL; + /* The '+ 1U' is to count in opcode octet, the first member of struct pdu_data_llctrl */ + pdu->len = sizeof(struct pdu_data_llctrl_length_req_rsp_common) + 1U; + pdu->llctrl.opcode = opcode; + p->max_rx_octets = sys_cpu_to_le16(dle->max_rx_octets); + p->max_tx_octets = sys_cpu_to_le16(dle->max_tx_octets); + p->max_rx_time = sys_cpu_to_le16(dle->max_rx_time); + p->max_tx_time = sys_cpu_to_le16(dle->max_tx_time); +} + void llcp_pdu_encode_length_req(struct ll_conn *conn, struct pdu_data *pdu) { - struct pdu_data_llctrl_length_req *p = &pdu->llctrl.length_req; - - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(length_req); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_LENGTH_REQ; - p->max_rx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_rx_octets); - p->max_tx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_tx_octets); - p->max_rx_time = sys_cpu_to_le16(conn->lll.dle.local.max_rx_time); - p->max_tx_time = sys_cpu_to_le16(conn->lll.dle.local.max_tx_time); + encode_length_req_rsp_common(pdu, + (struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_req, + PDU_DATA_LLCTRL_TYPE_LENGTH_REQ, + &conn->lll.dle.local); } void llcp_pdu_encode_length_rsp(struct ll_conn *conn, struct pdu_data *pdu) { - struct pdu_data_llctrl_length_rsp *p = &pdu->llctrl.length_rsp; - - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(length_rsp); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_LENGTH_RSP; - p->max_rx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_rx_octets); - p->max_tx_octets = sys_cpu_to_le16(conn->lll.dle.local.max_tx_octets); - p->max_rx_time = sys_cpu_to_le16(conn->lll.dle.local.max_rx_time); - p->max_tx_time = sys_cpu_to_le16(conn->lll.dle.local.max_tx_time); + encode_length_req_rsp_common(pdu, + (struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_rsp, + PDU_DATA_LLCTRL_TYPE_LENGTH_RSP, + &conn->lll.dle.local); } void llcp_ntf_encode_length_change(struct ll_conn *conn, struct pdu_data *pdu) { - struct pdu_data_llctrl_length_rsp *p = &pdu->llctrl.length_rsp; - - pdu->ll_id = PDU_DATA_LLID_CTRL; - pdu->len = PDU_DATA_LLCTRL_LEN(length_rsp); - pdu->llctrl.opcode = PDU_DATA_LLCTRL_TYPE_LENGTH_RSP; - p->max_rx_octets = sys_cpu_to_le16(conn->lll.dle.eff.max_rx_octets); - p->max_tx_octets = sys_cpu_to_le16(conn->lll.dle.eff.max_tx_octets); - p->max_rx_time = sys_cpu_to_le16(conn->lll.dle.eff.max_rx_time); - p->max_tx_time = sys_cpu_to_le16(conn->lll.dle.eff.max_tx_time); + encode_length_req_rsp_common(pdu, + (struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_rsp, + PDU_DATA_LLCTRL_TYPE_LENGTH_RSP, + &conn->lll.dle.eff); } static bool dle_remote_valid(const struct data_pdu_length *remote) @@ -702,10 +676,9 @@ static bool dle_remote_valid(const struct data_pdu_length *remote) return true; } - -void llcp_pdu_decode_length_req(struct ll_conn *conn, struct pdu_data *pdu) +static void decode_length_req_rsp_common(struct ll_conn *conn, + struct pdu_data_llctrl_length_req_rsp_common *p) { - struct pdu_data_llctrl_length_req *p = &pdu->llctrl.length_req; struct data_pdu_length remote; remote.max_rx_octets = sys_le16_to_cpu(p->max_rx_octets); @@ -720,21 +693,16 @@ void llcp_pdu_decode_length_req(struct ll_conn *conn, struct pdu_data *pdu) conn->lll.dle.remote = remote; } +void llcp_pdu_decode_length_req(struct ll_conn *conn, struct pdu_data *pdu) +{ + decode_length_req_rsp_common(conn, + (struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_req); +} + void llcp_pdu_decode_length_rsp(struct ll_conn *conn, struct pdu_data *pdu) { - struct pdu_data_llctrl_length_rsp *p = &pdu->llctrl.length_rsp; - struct data_pdu_length remote; - - remote.max_rx_octets = sys_le16_to_cpu(p->max_rx_octets); - remote.max_tx_octets = sys_le16_to_cpu(p->max_tx_octets); - remote.max_rx_time = sys_le16_to_cpu(p->max_rx_time); - remote.max_tx_time = sys_le16_to_cpu(p->max_tx_time); - - if (!dle_remote_valid(&remote)) { - return; - } - - conn->lll.dle.remote = remote; + decode_length_req_rsp_common(conn, + (struct pdu_data_llctrl_length_req_rsp_common *)&pdu->llctrl.length_rsp); } #endif /* CONFIG_BT_CTLR_DATA_LENGTH */