From 0126c5f44ba2b93af0eedf8021bfb365d6640df9 Mon Sep 17 00:00:00 2001 From: Sam Kumar Date: Sun, 24 Apr 2022 17:05:44 -0700 Subject: [PATCH] [tcp] modify cbuf to be able to use all bytes provided by the user (#7634) --- include/openthread/instance.h | 2 +- include/openthread/tcp.h | 4 +- third_party/tcplp/lib/bitmap.c | 8 +++- third_party/tcplp/lib/cbuf.c | 80 +++++++++++++++++++--------------- third_party/tcplp/lib/cbuf.h | 12 ++++- 5 files changed, 63 insertions(+), 43 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index bdd09f312..e065d0666 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (236) +#define OPENTHREAD_API_VERSION (237) /** * @addtogroup api-instance diff --git a/include/openthread/tcp.h b/include/openthread/tcp.h index 6cc11f794..1b1d4b8eb 100644 --- a/include/openthread/tcp.h +++ b/include/openthread/tcp.h @@ -294,7 +294,7 @@ typedef struct otTcpEndpointInitializeArgs * select a smaller buffer size. * */ -#define OT_TCP_RECEIVE_BUFFER_SIZE_FEW_HOPS 2599 +#define OT_TCP_RECEIVE_BUFFER_SIZE_FEW_HOPS 2598 /** * @def OT_TCP_RECEIVE_BUFFER_SIZE_MANY_HOPS @@ -306,7 +306,7 @@ typedef struct otTcpEndpointInitializeArgs * so), then it may be advisable to select a large buffer size manually. * */ -#define OT_TCP_RECEIVE_BUFFER_SIZE_MANY_HOPS 4158 +#define OT_TCP_RECEIVE_BUFFER_SIZE_MANY_HOPS 4157 /** * Initializes a TCP endpoint. diff --git a/third_party/tcplp/lib/bitmap.c b/third_party/tcplp/lib/bitmap.c index 70d03c147..6ae4b0e35 100644 --- a/third_party/tcplp/lib/bitmap.c +++ b/third_party/tcplp/lib/bitmap.c @@ -67,7 +67,9 @@ void bmp_setrange(uint8_t* buf, size_t start, size_t len) { } else { *first_byte_set |= first_byte_mask; memset(first_byte_set + 1, 0xFF, (size_t) (last_byte_set - first_byte_set - 1)); - *last_byte_set |= last_byte_mask; + if (last_byte_mask != 0x00) { + *last_byte_set |= last_byte_mask; + } } } @@ -89,7 +91,9 @@ void bmp_clrrange(uint8_t* buf, size_t start, size_t len) { } else { *first_byte_clear &= first_byte_mask; memset(first_byte_clear + 1, 0x00, (size_t) (last_byte_clear - first_byte_clear - 1)); - *last_byte_clear &= last_byte_mask; + if (last_byte_mask != 0xFF) { + *last_byte_clear &= last_byte_mask; + } } } diff --git a/third_party/tcplp/lib/cbuf.c b/third_party/tcplp/lib/cbuf.c index b7793ac65..f5b7e2afc 100644 --- a/third_party/tcplp/lib/cbuf.c +++ b/third_party/tcplp/lib/cbuf.c @@ -73,67 +73,73 @@ void cbuf_copy_from_message(void* arr, size_t arr_offset, const void* buffer, si void cbuf_init(struct cbufhead* chdr, uint8_t* buf, size_t len) { chdr->r_index = 0; - chdr->w_index = 0; + chdr->used = 0; chdr->size = len; chdr->buf = buf; } size_t cbuf_used_space(struct cbufhead* chdr) { - if (chdr->w_index >= chdr->r_index) { - return chdr->w_index - chdr->r_index; - } else { - return chdr->size + chdr->w_index - chdr->r_index; - } + return chdr->used; } -/* There's always one byte of lost space so I can distinguish between a full - buffer and an empty buffer. */ size_t cbuf_free_space(struct cbufhead* chdr) { - return chdr->size - 1 - cbuf_used_space(chdr); + return chdr->size - chdr->used; } size_t cbuf_size(struct cbufhead* chdr) { - return chdr->size - 1; + return chdr->size; } bool cbuf_empty(struct cbufhead* chdr) { - return (chdr->w_index == chdr->r_index); + return chdr->used == 0; +} + +static inline size_t cbuf_get_w_index(const struct cbufhead* chdr) { + size_t until_end = chdr->size - chdr->r_index; + if (chdr->used < until_end) { + return chdr->r_index + chdr->used; + } else { + return chdr->used - until_end; + } } size_t cbuf_write(struct cbufhead* chdr, const void* data, size_t data_offset, size_t data_len, cbuf_copier_t copy_from) { size_t free_space = cbuf_free_space(chdr); uint8_t* buf_data; - size_t fw_index; + size_t w_index; size_t bytes_to_end; if (free_space < data_len) { data_len = free_space; } buf_data = chdr->buf; - fw_index = (chdr->w_index + data_len) % chdr->size; - if (fw_index >= chdr->w_index) { - copy_from(buf_data, chdr->w_index, data, data_offset, data_len); + w_index = cbuf_get_w_index(chdr); + bytes_to_end = chdr->size - w_index; + if (data_len <= bytes_to_end) { + copy_from(buf_data, w_index, data, data_offset, data_len); } else { - bytes_to_end = chdr->size - chdr->w_index; - copy_from(buf_data, chdr->w_index, data, data_offset, bytes_to_end); + copy_from(buf_data, w_index, data, data_offset, bytes_to_end); copy_from(buf_data, 0, data, data_offset + bytes_to_end, data_len - bytes_to_end); } - chdr->w_index = fw_index; + chdr->used += data_len; return data_len; } void cbuf_read_unsafe(struct cbufhead* chdr, void* data, size_t data_offset, size_t numbytes, int pop, cbuf_copier_t copy_into) { uint8_t* buf_data = chdr->buf; - size_t fr_index = (chdr->r_index + numbytes) % chdr->size; - size_t bytes_to_end; - if (fr_index >= chdr->r_index) { + size_t bytes_to_end = chdr->size - chdr->r_index; + if (numbytes < bytes_to_end) { copy_into(data, data_offset, buf_data, chdr->r_index, numbytes); + if (pop) { + chdr->r_index += numbytes; + chdr->used -= numbytes; + } } else { - bytes_to_end = chdr->size - chdr->r_index; copy_into(data, data_offset, buf_data, chdr->r_index, bytes_to_end); copy_into(data, data_offset + bytes_to_end, buf_data, 0, numbytes - bytes_to_end); - } - if (pop) { - chdr->r_index = fr_index; + if (pop) { + chdr->r_index = numbytes - bytes_to_end; + chdr->used -= numbytes; + } } } @@ -167,22 +173,24 @@ size_t cbuf_pop(struct cbufhead* chdr, size_t numbytes) { numbytes = used_space; } chdr->r_index = (chdr->r_index + numbytes) % chdr->size; + chdr->used -= numbytes; return numbytes; } void cbuf_reference(const struct cbufhead* chdr, otLinkedBuffer* first, otLinkedBuffer* second) { - if (chdr->w_index >= chdr->r_index) { + size_t until_end = chdr->size - chdr->r_index; + if (chdr->used <= until_end) { first->mNext = NULL; first->mData = &chdr->buf[chdr->r_index]; - first->mLength = (uint16_t) (chdr->w_index - chdr->r_index); + first->mLength = (uint16_t) chdr->used; } else { first->mNext = second; first->mData = &chdr->buf[chdr->r_index]; - first->mLength = (uint16_t) (chdr->size - chdr->r_index); + first->mLength = (uint16_t) until_end; second->mNext = NULL; second->mData = &chdr->buf[0]; - second->mLength = (uint16_t) chdr->w_index; + second->mLength = (uint16_t) (chdr->used - until_end); } } @@ -197,7 +205,7 @@ size_t cbuf_reass_write(struct cbufhead* chdr, size_t offset, const void* data, } else if (offset + numbytes > free_space) { numbytes = free_space - offset; } - start_index = (chdr->w_index + offset) % chdr->size; + start_index = (cbuf_get_w_index(chdr) + offset) % chdr->size; end_index = (start_index + numbytes) % chdr->size; if (end_index >= start_index) { copy_from(buf_data, start_index, data, data_offset, numbytes); @@ -220,29 +228,29 @@ size_t cbuf_reass_write(struct cbufhead* chdr, size_t offset, const void* data, } size_t cbuf_reass_merge(struct cbufhead* chdr, size_t numbytes, uint8_t* bitmap) { - size_t old_w = chdr->w_index; + size_t old_w = cbuf_get_w_index(chdr); size_t free_space = cbuf_free_space(chdr); size_t bytes_to_end; if (numbytes > free_space) { numbytes = free_space; } - chdr->w_index = (chdr->w_index + numbytes) % chdr->size; if (bitmap) { - if (chdr->w_index >= old_w) { + bytes_to_end = chdr->size - old_w; + if (numbytes <= bytes_to_end) { bmp_clrrange(bitmap, old_w, numbytes); } else { - bytes_to_end = chdr->size - old_w; bmp_clrrange(bitmap, old_w, bytes_to_end); bmp_clrrange(bitmap, 0, numbytes - bytes_to_end); } } + chdr->used += numbytes; return numbytes; } size_t cbuf_reass_count_set(struct cbufhead* chdr, size_t offset, uint8_t* bitmap, size_t limit) { size_t bitmap_size = BITS_TO_BYTES(chdr->size); size_t until_end; - offset = (chdr->w_index + offset) % chdr->size; + offset = (cbuf_get_w_index(chdr) + offset) % chdr->size; until_end = bmp_countset(bitmap, bitmap_size, offset, limit); if (until_end >= limit || until_end < (chdr->size - offset)) { // If we already hit the limit, or if the streak ended before wrapping, then stop here @@ -254,7 +262,7 @@ size_t cbuf_reass_count_set(struct cbufhead* chdr, size_t offset, uint8_t* bitma } int cbuf_reass_within_offset(struct cbufhead* chdr, size_t offset, size_t index) { - size_t range_start = chdr->w_index; + size_t range_start = cbuf_get_w_index(chdr); size_t range_end = (range_start + offset) % chdr->size; if (range_end >= range_start) { return index >= range_start && index < range_end; diff --git a/third_party/tcplp/lib/cbuf.h b/third_party/tcplp/lib/cbuf.h index 7ba6b6d0f..b006b1422 100644 --- a/third_party/tcplp/lib/cbuf.h +++ b/third_party/tcplp/lib/cbuf.h @@ -42,7 +42,7 @@ struct otLinkedBuffer; /* Represents a circular buffer. */ struct cbufhead { size_t r_index; - size_t w_index; + size_t used; size_t size; uint8_t* buf; }; @@ -105,7 +105,15 @@ size_t cbuf_reass_write(struct cbufhead* chdr, size_t offset, const void* data, size_t cbuf_reass_merge(struct cbufhead* chdr, size_t numbytes, uint8_t* bitmap); /* Counts the number of contiguous out-of-sequence bytes at the specified - OFFSET, until the count reaches the specified LIMIT. */ + OFFSET, until the count reaches the specified LIMIT. Note that, for a given, + limit, this function might overcount the length of the continuous + out-of-sequence bytes and return a greater number; the caller is assumed to + handle this appropriately (i.e., treating the limit not as a hard upper + bound on the return value, but rather as, "I don't care if more bits than + this are set"). Just because the function returns something more than + LIMIT, it doesn't necessarily mean that more than LIMIT bits are actually + set. Note that LIMIT should never be set to a value greater than the number + of bytes in the circular buffer. */ size_t cbuf_reass_count_set(struct cbufhead* chdr, size_t offset, uint8_t* bitmap, size_t limit); /* Returns a true value iff INDEX is the index of a byte within OFFSET bytes