[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406226827
##
File path: nimble/host/src/ble_store_util.c
##
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
-case BLE_STORE_OBJ_TYPE_OUR_SEC:
-case BLE_STORE_OBJ_TYPE_PEER_SEC:
-case BLE_STORE_OBJ_TYPE_CCCD:
-return ble_gap_unpair_oldest_peer();
-
-default:
-return BLE_HS_EUNKNOWN;
+case BLE_STORE_OBJ_TYPE_OUR_SEC:
+case BLE_STORE_OBJ_TYPE_PEER_SEC:
+return ble_gap_unpair_oldest_peer();
+case BLE_STORE_OBJ_TYPE_CCCD:
+/* Try to remove unbonded CCCDs first */
+if ((rc = ble_store_clean_old_cccds((void *)
&event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
Hi @rymanluk Please let me know if I am missing something. As per my
understanding CCCDs for unbonded devices can get stored as
`ble_gatts_clt_cfg_access --> ble_store_write_cccd -->
ble_store_write(BLE_STORE_OBJ_TYPE_CCCD, store_value)`.
Edit:
Please disregard my earlier comment, missed on `cccd_value.flags == 0` check.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406226827
##
File path: nimble/host/src/ble_store_util.c
##
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
-case BLE_STORE_OBJ_TYPE_OUR_SEC:
-case BLE_STORE_OBJ_TYPE_PEER_SEC:
-case BLE_STORE_OBJ_TYPE_CCCD:
-return ble_gap_unpair_oldest_peer();
-
-default:
-return BLE_HS_EUNKNOWN;
+case BLE_STORE_OBJ_TYPE_OUR_SEC:
+case BLE_STORE_OBJ_TYPE_PEER_SEC:
+return ble_gap_unpair_oldest_peer();
+case BLE_STORE_OBJ_TYPE_CCCD:
+/* Try to remove unbonded CCCDs first */
+if ((rc = ble_store_clean_old_cccds((void *)
&event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
Hi @rymanluk Please let me know if I am missing something. As per my
understanding CCCDs for unbonded devices can get stored as
`ble_gatts_clt_cfg_access --> ble_store_write_cccd -->
ble_store_write(BLE_STORE_OBJ_TYPE_CCCD, store_value)`.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406222887 ## File path: nimble/host/include/host/ble_gap.h ## @@ -1896,6 +1896,20 @@ int ble_gap_unpair(const ble_addr_t *peer_addr); */ int ble_gap_unpair_oldest_peer(void); +/** + * Similar to `ble_gap_unpair_oldest_peer()`, except it makes sure that current + * peer is not deleted. + * + * @param peer_addr Address of the current peer (not to be deleted) + * + * @return 0 on success; + * A BLE host HCI return code if the controller + * rejected the request; + * A BLE host core return code on unexpected + * error. + */ +int ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer); Review comment: Agreed !! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406038133
##
File path: nimble/host/src/ble_gap.c
##
@@ -5605,6 +5605,41 @@ ble_gap_unpair_oldest_peer(void)
return 0;
}
+int
+ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer)
+{
+ble_addr_t oldest_peer_id_addr[MYNEWT_VAL(BLE_STORE_MAX_BONDS)];
+int num_peers;
+int rc, i;
+
+rc = ble_store_util_bonded_peers(
+&oldest_peer_id_addr[0], &num_peers,
MYNEWT_VAL(BLE_STORE_MAX_BONDS));
+if (rc != 0) {
+return rc;
+}
+
+if (num_peers == 0) {
+return BLE_HS_ENOENT;
+}
+
+for (i = 0; i < num_peers; i++) {
+if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) !=
0) {
Review comment:
Sure !!
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r405530825
##
File path: nimble/host/src/ble_store_util.c
##
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+int rc = BLE_HS_EUNKNOWN;
+ble_addr_t peer_id_addr;
+int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
-case BLE_STORE_OBJ_TYPE_OUR_SEC:
-case BLE_STORE_OBJ_TYPE_PEER_SEC:
-case BLE_STORE_OBJ_TYPE_CCCD:
-return ble_gap_unpair_oldest_peer();
-
-default:
-return BLE_HS_EUNKNOWN;
+case BLE_STORE_OBJ_TYPE_OUR_SEC:
+case BLE_STORE_OBJ_TYPE_PEER_SEC:
+return ble_gap_unpair_oldest_peer();
+case BLE_STORE_OBJ_TYPE_CCCD:
+if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
Hi @rymanluk I think for some peers `CCCD` will exist but `peer_sec` and
`our_sec` won't exist (Ref: [subscribe-event
ble_gatts_clt_cfg_access](https://github.com/apache/mynewt-nimble/blob/9e26fbe97b3d0bb3a22f15ed9c6035be648d1104/nimble/host/src/ble_gatts.c#L782)).
So we need mechanism to clean these stored CCCDs, that is why I have
introduced `ble_store_clean_old_cccd()` function.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r404738151
##
File path: nimble/host/src/ble_store_util.c
##
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+int rc = BLE_HS_EUNKNOWN;
+ble_addr_t peer_id_addr;
+int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
-case BLE_STORE_OBJ_TYPE_OUR_SEC:
-case BLE_STORE_OBJ_TYPE_PEER_SEC:
-case BLE_STORE_OBJ_TYPE_CCCD:
-return ble_gap_unpair_oldest_peer();
-
-default:
-return BLE_HS_EUNKNOWN;
+case BLE_STORE_OBJ_TYPE_OUR_SEC:
+case BLE_STORE_OBJ_TYPE_PEER_SEC:
+return ble_gap_unpair_oldest_peer();
+case BLE_STORE_OBJ_TYPE_CCCD:
+if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
@rymanluk @h2zero , disregard my earlier comment, yes current peer will be
unpaired, and we should simply return error in that case.
I was thinking of adding utility function (`ble_store_clean_old_cccd`) which
will delete CCCDs of all unbonded peers. This can be called first in case of
`CCCD overflow` and if it fails to delete peer then we can continue with
modified `ble_gap_unpair_oldest_peer()`. Please let me know your views on it.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403840472
##
File path: nimble/host/src/ble_store_util.c
##
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+int rc = BLE_HS_EUNKNOWN;
+ble_addr_t peer_id_addr;
+int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
-case BLE_STORE_OBJ_TYPE_OUR_SEC:
-case BLE_STORE_OBJ_TYPE_PEER_SEC:
-case BLE_STORE_OBJ_TYPE_CCCD:
-return ble_gap_unpair_oldest_peer();
-
-default:
-return BLE_HS_EUNKNOWN;
+case BLE_STORE_OBJ_TYPE_OUR_SEC:
+case BLE_STORE_OBJ_TYPE_PEER_SEC:
+return ble_gap_unpair_oldest_peer();
+case BLE_STORE_OBJ_TYPE_CCCD:
+if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
@rymanluk That is what I have done actually in
`ble_store_util_subscribed_cccds` !! What I think is
`ble_gap_unpair_oldest_peer` is not anyway going to unpair current peer (the
one for which we want to make space), so introducing similar function with
`ble_addr_t` as input won't be really that helpful. Please let me know your
thoughts on this.
The issue I believe is only confined to CCCDs, so
`ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT` will make sure that we do not
have any bonded devices and still we are out of storage space, which can be
used to conclude that we need to clean old CCCDs (except the current one).
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403012231
##
File path: nimble/host/src/ble_store_util.c
##
@@ -230,13 +299,35 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+int rc = BLE_HS_EUNKNOWN;
+ble_addr_t peer_id_addr;
+int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
case BLE_STORE_OBJ_TYPE_OUR_SEC:
case BLE_STORE_OBJ_TYPE_PEER_SEC:
case BLE_STORE_OBJ_TYPE_CCCD:
-return ble_gap_unpair_oldest_peer();
Review comment:
@rymanluk agreed !! I have made relevant changes. For CCCD, if
`ble_gap_unpair_oldest_peer` is unable to delete any peer, we continue to find
other peers occupying CCCDs in storage. I have also modified callback function
(`ble_store_util_iter_peer_cccd`) to skip the current (i.e. for which CCCDs
need to be stored) peer entry.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403009539
##
File path: nimble/host/src/ble_gap.c
##
@@ -5594,7 +5594,7 @@ ble_gap_unpair_oldest_peer(void)
}
if (num_peers == 0) {
-return 0;
+return BLE_HS_ENOENT;
Review comment:
Done !!
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:
us...@infra.apache.org
With regards,
Apache Git Services
