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/05/25 11:07:58 UTC
[GitHub] [mynewt-nimble] andrzej-kaczmarek opened a new pull request #821: nimble/hs: Fix flow control
andrzej-kaczmarek opened a new pull request #821:
URL: https://github.com/apache/mynewt-nimble/pull/821
Current implementation of hs flow control uses mbuf user header to
associate mbuf with a connection handle. This unfortunately does not
work well in most cases.
One major problem is that host can reassemble data. This is done by
os_mbuf_concat() which strips packet header from 2nd mbuf and this
means we basically lose connection handle information.
While this problem can be worked around by some extra mbuf counting,
another way to lose connection handle information is to basically pass
received buffer to application. For example, OIC uses user header for
own purposes so it will either overwrite connection handle information
(if our user header is large enough for OIC) or strip our packet header
if it decides to use own mbuf for packet header.
So it's better not to rely on user header to track store connection
handle information. This patch uses a simple private array to store
connection handle for each mbuf - array is indexed by mbuf index in a
mempool. This is a bit of a hack since index of an mbuf in a mempool
has to be calculated using mempool buffer address and block size, but
I do not really see any simple alternative here.
----------------------------------------------------------------
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
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on pull request #821: nimble/hs: Fix flow control
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #821:
URL: https://github.com/apache/mynewt-nimble/pull/821#issuecomment-633518823
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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
[GitHub] [mynewt-nimble] ccollins476ad commented on a change in pull request #821: nimble/hs: Fix flow control
Posted by GitBox <gi...@apache.org>.
ccollins476ad commented on a change in pull request #821:
URL: https://github.com/apache/mynewt-nimble/pull/821#discussion_r430039571
##########
File path: nimble/include/nimble/ble.h
##########
@@ -177,8 +177,8 @@ struct ble_mbuf_hdr
#define BLE_MBUF_MEMBLOCK_OVERHEAD \
(sizeof(struct os_mbuf) + BLE_MBUF_PKTHDR_OVERHEAD)
-/* Length of host user header. Only contains the peer's connection handle. */
-#define BLE_MBUF_HS_HDR_LEN (2)
+#define BLE_MBUF_HS_HDR_LEN (0)
Review comment:
This comment is definitely in "nitpick territory" so feel free to ignore it. IMO, this constant reduces clarity without any benefit. It would be more consistent and easily understood to just hardcode 0 at the allocation site.
##########
File path: nimble/host/src/ble_hs_flow.c
##########
@@ -40,6 +40,23 @@ static ble_npl_event_fn ble_hs_flow_event_cb;
static struct ble_npl_event ble_hs_flow_ev;
+/* Connection handle associated with each mbuf in ACL pool */
+static uint16_t ble_hs_flow_mbuf_conn_handle[ MYNEWT_VAL(BLE_ACL_BUF_COUNT) ];
+
+static inline int
+ble_hs_flow_mbuf_index(const struct os_mbuf *om)
+{
+ const struct os_mempool *mp = om->om_omp->omp_pool;
+ uint32_t addr = (uint32_t)om;
Review comment:
I would use `uintptr_t` here.
----------------------------------------------------------------
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
[GitHub] [mynewt-nimble] andrzej-kaczmarek merged pull request #821: nimble/hs: Fix flow control
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek merged pull request #821:
URL: https://github.com/apache/mynewt-nimble/pull/821
----------------------------------------------------------------
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