You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/01/02 08:57:26 UTC
[GitHub] [mynewt-nimble] andrzej-kaczmarek opened a new pull request #713:
nimble/ll: Simplify and cleanup PDU RX flow in scanner
andrzej-kaczmarek opened a new pull request #713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
URL: https://github.com/apache/mynewt-nimble/pull/713
(tbd, wip)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] andrzej-kaczmarek merged pull request #713:
nimble/ll: Simplify and cleanup PDU RX flow in scanner
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek merged pull request #713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
URL: https://github.com/apache/mynewt-nimble/pull/713
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
URL: https://github.com/apache/mynewt-nimble/pull/713#discussion_r363746837
##########
File path: nimble/controller/src/ble_ll_scan.c
##########
@@ -2055,338 +1965,458 @@ ble_ll_scan_adv_decode_addr(uint8_t pdu_type, uint8_t *rxbuf,
return 0;
}
-/**
- * Called when a receive PDU has ended.
- *
- * Context: Interrupt
- *
- * @param rxpdu
+static void
+ble_ll_scan_get_addr_data_from_legacy(uint8_t pdu_type, uint8_t *rxbuf,
+ struct ble_ll_scan_addr_data *addrd)
+{
+ BLE_LL_ASSERT(pdu_type < BLE_ADV_PDU_TYPE_ADV_EXT_IND);
+
+ addrd->adva_present = true;
+
+ addrd->adva = rxbuf + BLE_LL_PDU_HDR_LEN;
+ addrd->adva_type = ble_ll_get_addr_type(rxbuf[0] & BLE_ADV_PDU_HDR_TXADD_MASK);
+
+ if (pdu_type == BLE_ADV_PDU_TYPE_ADV_DIRECT_IND) {
+ addrd->targeta = rxbuf + BLE_LL_PDU_HDR_LEN + BLE_DEV_ADDR_LEN;
+ addrd->targeta_type = ble_ll_get_addr_type(rxbuf[0] & BLE_ADV_PDU_HDR_RXADD_MASK);
+ } else {
+ addrd->targeta = NULL;
+ addrd->targeta_type = 0;
+ }
+}
+
+/*
+ * Matches incoming PDU using scan filter policy and whitelist, if applicable.
+ * This will also resolve addresses and update flags/fields in header and
+ * addr_data as needed.
*
- * @return int
- * < 0: Disable the phy after reception.
- * == 0: Success. Do not disable the PHY.
- * > 0: Do not disable PHY as that has already been done.
+ * @return 0 = no match
+ * 1 = match
+ * 2 = match, but do not scan
*/
-int
-ble_ll_scan_rx_isr_end(struct os_mbuf *rxpdu, uint8_t crcok)
+static int
+ble_ll_scan_rx_filter(struct ble_mbuf_hdr *hdr, struct ble_ll_scan_addr_data *addrd)
{
- int rc;
- int chk_send_req;
- int chk_wl;
- int rpa_index;
- int resolved;
- uint8_t pdu_type;
- uint8_t adva_type = 0;
- uint8_t *adva = NULL; /* Original AdvA */
- uint8_t targeta_type = 0;
- uint8_t *targeta = NULL; /* Original TargetA */
- uint8_t adv_addr_type = 0;
- uint8_t *adv_addr = NULL; /* Actual advertiser address (AdvA or identity) */
- uint8_t *rxbuf;
- struct ble_mbuf_hdr *ble_hdr;
- struct ble_ll_scan_sm *scansm;
- struct ble_ll_scan_params *scanp;
- int ext_adv_mode = -1;
+ struct ble_ll_scan_sm *scansm = &g_ble_ll_scan_sm;
+ struct ble_ll_scan_params *scanp = scansm->scanp;
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
- uint8_t phy_mode;
- uint16_t adi;
- struct ble_ll_aux_data *aux_data = NULL;
+ struct ble_ll_aux_data *aux_data = hdr->rxinfo.user_data;
#endif
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY)
+ struct ble_mbuf_hdr_rxinfo *rxinfo = &hdr->rxinfo;
struct ble_ll_resolv_entry *rl = NULL;
- bool resolve_peer = false;
#endif
+ bool scan_req_allowed = true;
+ bool resolved = false;
- /* Get scanning state machine */
- scansm = &g_ble_ll_scan_sm;
- scanp = scansm->scanp;
-
- /*
- * The reason we do something different here (as opposed to failed CRC) is
- * that the received PDU will not be handed up in this case. So we have
- * to restart scanning and handle a failed scan request. Note that we
- * return 0 in this case because we dont want the phy disabled.
- */
- if (rxpdu == NULL) {
- ble_ll_scan_interrupted(scansm);
- return 0;
- }
+ /* Use AdvA as initial advertiser address, we may try to resolve it later */
+ addrd->adv_addr = addrd->adva;
+ addrd->adv_addr_type = addrd->adva_type;
- rc = -1;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY)
+ /* By default, assume AdvA is not resolved */
+ rxinfo->rpa_index = -1;
- ble_hdr = BLE_MBUF_HDR_PTR(rxpdu);
+ switch (ble_ll_addr_subtype(addrd->adva, addrd->adva_type)) {
+ case BLE_LL_ADDR_SUBTYPE_RPA:
+ /*
+ * Only resolve if packet actually contained AdvA.
+ * In extended advertising PDUs we may use RL index from a PDU that
+ * already had AdvA (e.g. ADV_EXT_IND in case of AUX_ADV_IND without
+ * AdvA). In legacy advertising PDUs we always need to resolve AdvA.
+ */
+ if (addrd->adva_present) {
+ rxinfo->rpa_index = ble_hw_resolv_list_match();
+ } else {
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
+ BLE_LL_ASSERT(aux_data);
+ rxinfo->rpa_index = aux_data->rpa_index;
+#else
+ BLE_LL_ASSERT(false);
+ rxinfo->rpa_index = -1;
+#endif
+ }
- /* Get pdu type, pointer to address and address "type" */
- rxbuf = rxpdu->om_data;
- pdu_type = rxbuf[0] & BLE_ADV_PDU_HDR_TYPE_MASK;
+ if (rxinfo->rpa_index < 0) {
+ break;
+ }
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
- if (scansm->cur_aux_data) {
- ble_hdr->rxinfo.user_data = scansm->cur_aux_data;
- scansm->cur_aux_data = NULL;
- /* If we were expecting aux/chain and it not arrived,
- * lets just exit here.
- */
- if (pdu_type != BLE_ADV_PDU_TYPE_ADV_EXT_IND) {
- goto scan_rx_isr_exit;
+ if (aux_data) {
+ aux_data->rpa_index = rxinfo->rpa_index;
}
- }
#endif
- /* Just leave if the CRC is not OK. */
- if (!crcok) {
- goto scan_rx_isr_exit;
+ /* Use resolved identity address as advertiser address */
+ rl = &g_ble_ll_resolv_list[rxinfo->rpa_index];
+ addrd->adv_addr = rl->rl_identity_addr;
+ addrd->adv_addr_type = rl->rl_addr_type;
+ addrd->rl = rl;
+
+ rxinfo->flags |= BLE_MBUF_HDR_F_RESOLVED;
+ resolved = 1;
+ break;
+ case BLE_LL_ADDR_SUBTYPE_IDENTITY:
+ /*
+ * If AdvA is an identity address, we need to check if that device was
+ * added to RL in order to use proper privacy mode.
+ */
+ rl = ble_ll_resolv_list_find(addrd->adva, addrd->adva_type);
+ if (!rl) {
+ break;
+ }
+
+ addrd->rl = rl;
+
+ /* Ignore device if using network privacy mode and it has IRK */
+ if ((rl->rl_priv_mode == BLE_HCI_PRIVACY_NETWORK) && rl->rl_has_peer) {
+ return 0;
+ }
+ break;
+ default:
+ /* NRPA goes through filtering policy directly */
+ break;
}
- switch (pdu_type) {
-#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
- /*
- * For extended advertising events all data are decoded to aux_data and can
- * be retrieved from there. For legacy advertising we need to get addresses
- * from PDU.
- */
- case BLE_ADV_PDU_TYPE_ADV_EXT_IND:
- if (!scansm->ext_scanning) {
- goto scan_rx_isr_exit;
+ if (addrd->targeta) {
+ switch (ble_ll_addr_subtype(addrd->targeta, addrd->targeta_type)) {
+ case BLE_LL_ADDR_SUBTYPE_RPA:
+ /* Check if TargetA can be resolved using the same RL entry as AdvA */
+ if (rl && ble_ll_resolv_rpa(addrd->targeta, rl->rl_local_irk)) {
+ rxinfo->flags |= BLE_MBUF_HDR_F_TARGETA_RESOLVED;
+ break;
+ }
+
+ /* Check if scan filter policy allows unresolved RPAs to be processed */
+ if (!(scanp->scan_filt_policy & 0x02)) {
+ return 0;
+ }
+
+ /* We will notify host, but make sure we do not scan this device */
Review comment:
we are not allowed to send scan requests because at this point we don't know if it is to us.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on issue #713:
nimble/ll: Simplify and cleanup PDU RX flow in scanner
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on issue #713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
URL: https://github.com/apache/mynewt-nimble/pull/713#issuecomment-571749577
fixed small issue pointed out in comment and rebased
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #713: nimble/ll: Simplify and cleanup PDU RX flow in scanner
URL: https://github.com/apache/mynewt-nimble/pull/713#discussion_r363738660
##########
File path: nimble/controller/src/ble_ll_scan.c
##########
@@ -2055,338 +1965,458 @@ ble_ll_scan_adv_decode_addr(uint8_t pdu_type, uint8_t *rxbuf,
return 0;
}
-/**
- * Called when a receive PDU has ended.
- *
- * Context: Interrupt
- *
- * @param rxpdu
+static void
+ble_ll_scan_get_addr_data_from_legacy(uint8_t pdu_type, uint8_t *rxbuf,
+ struct ble_ll_scan_addr_data *addrd)
+{
+ BLE_LL_ASSERT(pdu_type < BLE_ADV_PDU_TYPE_ADV_EXT_IND);
+
+ addrd->adva_present = true;
+
+ addrd->adva = rxbuf + BLE_LL_PDU_HDR_LEN;
+ addrd->adva_type = ble_ll_get_addr_type(rxbuf[0] & BLE_ADV_PDU_HDR_TXADD_MASK);
+
+ if (pdu_type == BLE_ADV_PDU_TYPE_ADV_DIRECT_IND) {
+ addrd->targeta = rxbuf + BLE_LL_PDU_HDR_LEN + BLE_DEV_ADDR_LEN;
+ addrd->targeta_type = ble_ll_get_addr_type(rxbuf[0] & BLE_ADV_PDU_HDR_RXADD_MASK);
+ } else {
+ addrd->targeta = NULL;
+ addrd->targeta_type = 0;
+ }
+}
+
+/*
+ * Matches incoming PDU using scan filter policy and whitelist, if applicable.
+ * This will also resolve addresses and update flags/fields in header and
+ * addr_data as needed.
*
- * @return int
- * < 0: Disable the phy after reception.
- * == 0: Success. Do not disable the PHY.
- * > 0: Do not disable PHY as that has already been done.
+ * @return 0 = no match
+ * 1 = match
+ * 2 = match, but do not scan
*/
-int
-ble_ll_scan_rx_isr_end(struct os_mbuf *rxpdu, uint8_t crcok)
+static int
+ble_ll_scan_rx_filter(struct ble_mbuf_hdr *hdr, struct ble_ll_scan_addr_data *addrd)
{
- int rc;
- int chk_send_req;
- int chk_wl;
- int rpa_index;
- int resolved;
- uint8_t pdu_type;
- uint8_t adva_type = 0;
- uint8_t *adva = NULL; /* Original AdvA */
- uint8_t targeta_type = 0;
- uint8_t *targeta = NULL; /* Original TargetA */
- uint8_t adv_addr_type = 0;
- uint8_t *adv_addr = NULL; /* Actual advertiser address (AdvA or identity) */
- uint8_t *rxbuf;
- struct ble_mbuf_hdr *ble_hdr;
- struct ble_ll_scan_sm *scansm;
- struct ble_ll_scan_params *scanp;
- int ext_adv_mode = -1;
+ struct ble_ll_scan_sm *scansm = &g_ble_ll_scan_sm;
+ struct ble_ll_scan_params *scanp = scansm->scanp;
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
- uint8_t phy_mode;
- uint16_t adi;
- struct ble_ll_aux_data *aux_data = NULL;
+ struct ble_ll_aux_data *aux_data = hdr->rxinfo.user_data;
#endif
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY)
+ struct ble_mbuf_hdr_rxinfo *rxinfo = &hdr->rxinfo;
struct ble_ll_resolv_entry *rl = NULL;
- bool resolve_peer = false;
#endif
+ bool scan_req_allowed = true;
+ bool resolved = false;
- /* Get scanning state machine */
- scansm = &g_ble_ll_scan_sm;
- scanp = scansm->scanp;
-
- /*
- * The reason we do something different here (as opposed to failed CRC) is
- * that the received PDU will not be handed up in this case. So we have
- * to restart scanning and handle a failed scan request. Note that we
- * return 0 in this case because we dont want the phy disabled.
- */
- if (rxpdu == NULL) {
- ble_ll_scan_interrupted(scansm);
- return 0;
- }
+ /* Use AdvA as initial advertiser address, we may try to resolve it later */
+ addrd->adv_addr = addrd->adva;
+ addrd->adv_addr_type = addrd->adva_type;
- rc = -1;
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY)
+ /* By default, assume AdvA is not resolved */
+ rxinfo->rpa_index = -1;
- ble_hdr = BLE_MBUF_HDR_PTR(rxpdu);
+ switch (ble_ll_addr_subtype(addrd->adva, addrd->adva_type)) {
+ case BLE_LL_ADDR_SUBTYPE_RPA:
+ /*
+ * Only resolve if packet actually contained AdvA.
+ * In extended advertising PDUs we may use RL index from a PDU that
+ * already had AdvA (e.g. ADV_EXT_IND in case of AUX_ADV_IND without
+ * AdvA). In legacy advertising PDUs we always need to resolve AdvA.
+ */
+ if (addrd->adva_present) {
+ rxinfo->rpa_index = ble_hw_resolv_list_match();
+ } else {
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
+ BLE_LL_ASSERT(aux_data);
+ rxinfo->rpa_index = aux_data->rpa_index;
+#else
+ BLE_LL_ASSERT(false);
+ rxinfo->rpa_index = -1;
+#endif
+ }
- /* Get pdu type, pointer to address and address "type" */
- rxbuf = rxpdu->om_data;
- pdu_type = rxbuf[0] & BLE_ADV_PDU_HDR_TYPE_MASK;
+ if (rxinfo->rpa_index < 0) {
+ break;
+ }
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
- if (scansm->cur_aux_data) {
- ble_hdr->rxinfo.user_data = scansm->cur_aux_data;
- scansm->cur_aux_data = NULL;
- /* If we were expecting aux/chain and it not arrived,
- * lets just exit here.
- */
- if (pdu_type != BLE_ADV_PDU_TYPE_ADV_EXT_IND) {
- goto scan_rx_isr_exit;
+ if (aux_data) {
+ aux_data->rpa_index = rxinfo->rpa_index;
}
- }
#endif
- /* Just leave if the CRC is not OK. */
- if (!crcok) {
- goto scan_rx_isr_exit;
+ /* Use resolved identity address as advertiser address */
+ rl = &g_ble_ll_resolv_list[rxinfo->rpa_index];
+ addrd->adv_addr = rl->rl_identity_addr;
+ addrd->adv_addr_type = rl->rl_addr_type;
+ addrd->rl = rl;
+
+ rxinfo->flags |= BLE_MBUF_HDR_F_RESOLVED;
+ resolved = 1;
Review comment:
resolved is a bool type.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services