From 3a4ebe98c39ce19ee12baddfc5f09f2540b5dd3a Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Mon, 30 Sep 2019 14:18:32 +0200 Subject: [PATCH 1/8] gnrc_sixlowpan_frag_rb: get index instead of pointer with _rbuf_get() --- .../frag/rb/gnrc_sixlowpan_frag_rb.c | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c index 1e4022ec64..8e2505fecd 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c +++ b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c @@ -69,20 +69,20 @@ static gnrc_sixlowpan_frag_rb_int_t *_rbuf_int_get_free(void); static bool _rbuf_update_ints(gnrc_sixlowpan_frag_rb_base_t *entry, uint16_t offset, size_t frag_size); /* gets an entry identified by its tupel */ -static gnrc_sixlowpan_frag_rb_t *_rbuf_get(const void *src, size_t src_len, - const void *dst, size_t dst_len, - size_t size, uint16_t tag, - unsigned page); +static int _rbuf_get(const void *src, size_t src_len, + const void *dst, size_t dst_len, + size_t size, uint16_t tag, + unsigned page); /* internal add to repeat add when fragments overlapped */ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, size_t offset, unsigned page); /* status codes for _rbuf_add() */ enum { - RBUF_ADD_SUCCESS, - RBUF_ADD_ERROR, - RBUF_ADD_REPEAT, - RBUF_ADD_DUPLICATE, + RBUF_ADD_SUCCESS = 0, + RBUF_ADD_ERROR = -1, + RBUF_ADD_REPEAT = -2, + RBUF_ADD_DUPLICATE = -3, }; #ifdef MODULE_GNRC_SIXLOWPAN_FRAG_STATS @@ -172,6 +172,7 @@ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, gnrc_sixlowpan_frag_rb_t *entry; uint8_t *data; size_t frag_size; + int res; uint16_t datagram_size; uint16_t datagram_tag; @@ -183,16 +184,16 @@ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, datagram_tag = sixlowpan_frag_datagram_tag(pkt->data); gnrc_sixlowpan_frag_rb_gc(); - entry = _rbuf_get(gnrc_netif_hdr_get_src_addr(netif_hdr), netif_hdr->src_l2addr_len, - gnrc_netif_hdr_get_dst_addr(netif_hdr), netif_hdr->dst_l2addr_len, - datagram_size, datagram_tag, page); + res = _rbuf_get(gnrc_netif_hdr_get_src_addr(netif_hdr), netif_hdr->src_l2addr_len, + gnrc_netif_hdr_get_dst_addr(netif_hdr), netif_hdr->dst_l2addr_len, + datagram_size, datagram_tag, page); - if (entry == NULL) { + if (res < 0) { DEBUG("6lo rbuf: reassembly buffer full.\n"); gnrc_pktbuf_release(pkt); return RBUF_ADD_ERROR; } - + entry = &rbuf[res]; if ((offset + frag_size) > entry->super.datagram_size) { DEBUG("6lo rfrag: fragment too big for resulting datagram, discarding datagram\n"); gnrc_pktbuf_release(entry->pkt); @@ -329,10 +330,10 @@ static inline void _set_rbuf_timeout(void) &_gc_timer_msg, sched_active_pid); } -static gnrc_sixlowpan_frag_rb_t *_rbuf_get(const void *src, size_t src_len, - const void *dst, size_t dst_len, - size_t size, uint16_t tag, - unsigned page) +static int _rbuf_get(const void *src, size_t src_len, + const void *dst, size_t dst_len, + size_t size, uint16_t tag, + unsigned page) { gnrc_sixlowpan_frag_rb_t *res = NULL, *oldest = NULL; uint32_t now_usec = xtimer_now_usec(); @@ -355,7 +356,7 @@ static gnrc_sixlowpan_frag_rb_t *_rbuf_get(const void *src, size_t src_len, (unsigned)rbuf[i].super.datagram_size, rbuf[i].super.tag); rbuf[i].super.arrival = now_usec; _set_rbuf_timeout(); - return &(rbuf[i]); + return i; } /* if there is a free spot: remember it */ @@ -393,7 +394,7 @@ static gnrc_sixlowpan_frag_rb_t *_rbuf_get(const void *src, size_t src_len, #ifdef MODULE_GNRC_SIXLOWPAN_FRAG_STATS _stats.rbuf_full++; #endif - return NULL; + return -1; } } @@ -413,7 +414,7 @@ static gnrc_sixlowpan_frag_rb_t *_rbuf_get(const void *src, size_t src_len, res->pkt = gnrc_pktbuf_add(NULL, NULL, size, reass_type); if (res->pkt == NULL) { DEBUG("6lo rfrag: can not allocate reassembly buffer space.\n"); - return NULL; + return -1; } *((uint64_t *)res->pkt->data) = 0; /* clean first few bytes for later @@ -437,7 +438,7 @@ static gnrc_sixlowpan_frag_rb_t *_rbuf_get(const void *src, size_t src_len, _set_rbuf_timeout(); - return res; + return res - &(rbuf[0]); } #ifdef TEST_SUITES From b0864533470aadb80014c260ef5c1c2f03e18304 Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Mon, 30 Sep 2019 14:26:21 +0200 Subject: [PATCH 2/8] gnrc_sixlowpan_frag_rb: return pointer to entry on add() --- sys/include/net/gnrc/sixlowpan/frag/rb.h | 9 ++++++--- .../sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c | 18 ++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/sys/include/net/gnrc/sixlowpan/frag/rb.h b/sys/include/net/gnrc/sixlowpan/frag/rb.h index 27aa0c3600..f30bb6dc4e 100644 --- a/sys/include/net/gnrc/sixlowpan/frag/rb.h +++ b/sys/include/net/gnrc/sixlowpan/frag/rb.h @@ -105,10 +105,13 @@ typedef struct { * @param[in] frag The fragment to add. * @param[in] offset The fragment's offset. * @param[in] page Current 6Lo dispatch parsing page. + * + * @return The reassembly buffer entry the fragment was added to on success. + * @return NULL on error. */ -void gnrc_sixlowpan_frag_rb_add(gnrc_netif_hdr_t *netif_hdr, - gnrc_pktsnip_t *frag, size_t offset, - unsigned page); +gnrc_sixlowpan_frag_rb_t *gnrc_sixlowpan_frag_rb_add(gnrc_netif_hdr_t *netif_hdr, + gnrc_pktsnip_t *frag, + size_t offset, unsigned page); /** * @brief Checks if a reassembly buffer entry is unset diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c index 8e2505fecd..1448abc5d5 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c +++ b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c @@ -120,13 +120,15 @@ static int _check_fragments(gnrc_sixlowpan_frag_rb_base_t *entry, return RBUF_ADD_SUCCESS; } -void gnrc_sixlowpan_frag_rb_add(gnrc_netif_hdr_t *netif_hdr, - gnrc_pktsnip_t *pkt, size_t offset, - unsigned page) +gnrc_sixlowpan_frag_rb_t *gnrc_sixlowpan_frag_rb_add(gnrc_netif_hdr_t *netif_hdr, + gnrc_pktsnip_t *pkt, + size_t offset, unsigned page) { - if (_rbuf_add(netif_hdr, pkt, offset, page) == RBUF_ADD_REPEAT) { - _rbuf_add(netif_hdr, pkt, offset, page); + int res; + if ((res = _rbuf_add(netif_hdr, pkt, offset, page)) == RBUF_ADD_REPEAT) { + res = _rbuf_add(netif_hdr, pkt, offset, page); } + return (res < 0) ? NULL : &rbuf[res]; } #ifndef NDEBUG @@ -210,7 +212,7 @@ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, return RBUF_ADD_REPEAT; case RBUF_ADD_DUPLICATE: gnrc_pktbuf_release(pkt); - return RBUF_ADD_SUCCESS; + return res; default: break; } @@ -230,7 +232,7 @@ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, return RBUF_ADD_ERROR; } gnrc_sixlowpan_iphc_recv(pkt, entry, 0); - return RBUF_ADD_SUCCESS; + return res; } else #endif @@ -243,7 +245,7 @@ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, } gnrc_sixlowpan_frag_rb_dispatch_when_complete(entry, netif_hdr); gnrc_pktbuf_release(pkt); - return RBUF_ADD_SUCCESS; + return res; } static inline bool _rbuf_int_overlap_partially(gnrc_sixlowpan_frag_rb_int_t *i, From a229755abe258ca6215e9eb1e718e5e16982742b Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Mon, 30 Sep 2019 15:29:38 +0200 Subject: [PATCH 3/8] gnrc_sixlowpan_frag_rb: return status on dispatch_when_complete() --- sys/include/net/gnrc/sixlowpan/frag/rb.h | 9 +++++++-- .../sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c | 9 ++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sys/include/net/gnrc/sixlowpan/frag/rb.h b/sys/include/net/gnrc/sixlowpan/frag/rb.h index f30bb6dc4e..f6f1b7469c 100644 --- a/sys/include/net/gnrc/sixlowpan/frag/rb.h +++ b/sys/include/net/gnrc/sixlowpan/frag/rb.h @@ -187,9 +187,14 @@ static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf) * @param[in] netif Original @ref gnrc_netif_hdr_t of the last received frame. * Used to construct the @ref gnrc_netif_hdr_t of the completed * datagram. Must not be NULL. + * + * @return >0, when the datagram in @p rbuf was complete and dispatched. + * @return 0, when the datagram in @p rbuf is not complete. + * @return -1, if the the reassembled datagram was not dispatched. @p rbuf is + * destroyed either way. */ -void gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf, - gnrc_netif_hdr_t *netif); +int gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf, + gnrc_netif_hdr_t *netif); #else /* NOPs to be used with gnrc_sixlowpan_iphc if gnrc_sixlowpan_frag_rb is not * compiled in */ diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c index 1448abc5d5..a9af53786a 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c +++ b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c @@ -476,12 +476,14 @@ void gnrc_sixlowpan_frag_rb_base_rm(gnrc_sixlowpan_frag_rb_base_t *entry) entry->datagram_size = 0; } -void gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf, +int gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf, gnrc_netif_hdr_t *netif_hdr) { assert(rbuf); assert(netif_hdr); - if (rbuf->super.current_size == rbuf->super.datagram_size) { + int res = (rbuf->super.current_size == rbuf->super.datagram_size); + + if (res) { gnrc_pktsnip_t *netif = gnrc_netif_hdr_build(rbuf->super.src, rbuf->super.src_len, rbuf->super.dst, @@ -491,7 +493,7 @@ void gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbu DEBUG("6lo rbuf: error allocating netif header\n"); gnrc_pktbuf_release(rbuf->pkt); gnrc_sixlowpan_frag_rb_remove(rbuf); - return; + return -1; } /* copy the transmit information of the latest fragment into the newly @@ -507,6 +509,7 @@ void gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbu gnrc_sixlowpan_dispatch_recv(rbuf->pkt, NULL, 0); gnrc_sixlowpan_frag_rb_remove(rbuf); } + return res; } From 709baf8f1fd307ba95191418e79870864b5db348 Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Mon, 30 Sep 2019 15:29:57 +0200 Subject: [PATCH 4/8] gnrc_sixlowpan_frag_rb: make NOPs static inline functions --- sys/include/net/gnrc/sixlowpan/frag/rb.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sys/include/net/gnrc/sixlowpan/frag/rb.h b/sys/include/net/gnrc/sixlowpan/frag/rb.h index f6f1b7469c..0affb6006d 100644 --- a/sys/include/net/gnrc/sixlowpan/frag/rb.h +++ b/sys/include/net/gnrc/sixlowpan/frag/rb.h @@ -198,9 +198,19 @@ int gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf #else /* NOPs to be used with gnrc_sixlowpan_iphc if gnrc_sixlowpan_frag_rb is not * compiled in */ -#define gnrc_sixlowpan_frag_rb_remove(rbuf) (void)(rbuf) -#define gnrc_sixlowpan_frag_rb_dispatch_when_complete(rbuf, netif) \ - (void)(rbuf); (void)(netif) +static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf) +{ + (void)rbuf; + return; +} + +static inline int gnrc_sixlowpan_frag_rb_dispatch_when_complete( + gnrc_sixlowpan_frag_rb_t *rbuf, gnrc_netif_hdr_t *netif) +{ + (void)rbuf; + (void)netif; + return -1; +} #endif #ifdef __cplusplus From df484926a2b4832b04316a0d6afa53dc6195e1d9 Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Mon, 30 Sep 2019 14:50:43 +0200 Subject: [PATCH 5/8] tests/gnrc_sixlowpan_frag: adapt for API change These changes reflect adaptations for the following API changes: - gnrc_sixlowpan_frag_rb: return pointer to entry on add() --- tests/gnrc_sixlowpan_frag/main.c | 97 ++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 43 deletions(-) diff --git a/tests/gnrc_sixlowpan_frag/main.c b/tests/gnrc_sixlowpan_frag/main.c index 6cd5eb385b..b6efe1ff24 100644 --- a/tests/gnrc_sixlowpan_frag/main.c +++ b/tests/gnrc_sixlowpan_frag/main.c @@ -256,9 +256,9 @@ static void test_rbuf_add__success_first_fragment(void) const gnrc_sixlowpan_frag_rb_t *entry; TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); - entry = _first_non_empty_rbuf(); + TEST_ASSERT_NOT_NULL((entry = gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT1_OFFSET, TEST_PAGE + ))); /* current_size must be the offset of fragment 2, not the size of * fragment 1 (fragment dispatch was removed, IPHC was applied etc.). */ _test_entry(entry, TEST_FRAGMENT2_OFFSET, @@ -273,9 +273,9 @@ static void test_rbuf_add__success_subsequent_fragment(void) const gnrc_sixlowpan_frag_rb_t *entry; TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT2_OFFSET, TEST_PAGE); - entry = _first_non_empty_rbuf(); + TEST_ASSERT_NOT_NULL((entry = gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT2_OFFSET, TEST_PAGE + ))); /* current_size must be the offset of fragment 3, not the size of * fragment 2 (fragment dispatch was removed, IPHC was applied etc.). */ _test_entry(entry, TEST_FRAGMENT3_OFFSET - TEST_FRAGMENT2_OFFSET, @@ -292,12 +292,13 @@ static void test_rbuf_add__success_duplicate_fragments(void) const gnrc_sixlowpan_frag_rb_t *entry; TEST_ASSERT_NOT_NULL(pkt1); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt1, - TEST_FRAGMENT3_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt1, TEST_FRAGMENT3_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt2); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt2, - TEST_FRAGMENT3_OFFSET, TEST_PAGE); - entry = _first_non_empty_rbuf(); + TEST_ASSERT_NOT_NULL((entry = gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt2, TEST_FRAGMENT3_OFFSET, TEST_PAGE + ))); /* current_size must be the offset of fragment 4, not the size of * fragment 3 (fragment dispatch was removed, IPHC was applied etc.). */ _test_entry(entry, TEST_FRAGMENT4_OFFSET - TEST_FRAGMENT3_OFFSET, @@ -325,17 +326,21 @@ static void test_rbuf_add__success_complete(void) gnrc_netreg_register(TEST_DATAGRAM_NETTYPE, ®); /* Mixing up things. Order decided by fair dice-rolls ;-) */ TEST_ASSERT_NOT_NULL(pkt2); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt2, - TEST_FRAGMENT2_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt2, TEST_FRAGMENT2_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt4); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt4, - TEST_FRAGMENT4_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt4, TEST_FRAGMENT4_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt1); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt1, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt1, TEST_FRAGMENT1_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt3); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt3, - TEST_FRAGMENT3_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt3, TEST_FRAGMENT3_OFFSET, TEST_PAGE + )); TEST_ASSERT_MESSAGE( xtimer_msg_receive_timeout(&msg, TEST_RECEIVE_TIMEOUT) >= 0, "Receiving reassembled datagram timed out" @@ -362,16 +367,18 @@ static void test_rbuf_add__full_rbuf(void) pkt = gnrc_pktbuf_add(NULL, _fragment1, sizeof(_fragment1), GNRC_NETTYPE_SIXLOWPAN); TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT1_OFFSET, TEST_PAGE + )); _set_fragment_tag(_fragment1, TEST_TAG + i + 1); /* pkt is released in gnrc_sixlowpan_frag_rb_add() */ } pkt = gnrc_pktbuf_add(NULL, _fragment1, sizeof(_fragment1), GNRC_NETTYPE_SIXLOWPAN); TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT1_OFFSET, TEST_PAGE + )); rbuf = gnrc_sixlowpan_frag_rb_array(); for (unsigned i = 0; i < GNRC_SIXLOWPAN_FRAG_RBUF_SIZE; i++) { const gnrc_sixlowpan_frag_rb_t *entry = &rbuf[i]; @@ -404,8 +411,9 @@ static void test_rbuf_add__too_big_fragment(void) GNRC_NETTYPE_SIXLOWPAN); TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); + TEST_ASSERT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT1_OFFSET, TEST_PAGE + )); /* packet buffer is empty*/ TEST_ASSERT_NULL(_first_non_empty_rbuf()); _check_pktbuf(NULL); @@ -424,11 +432,13 @@ static void test_rbuf_add__overlap_lhs(void) pkt2 = gnrc_pktbuf_add(NULL, _fragment2, sizeof(_fragment2), GNRC_NETTYPE_SIXLOWPAN); TEST_ASSERT_NOT_NULL(pkt1); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt1, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt1, TEST_FRAGMENT1_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt2); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt2, pkt2_offset, - TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt2, pkt2_offset, TEST_PAGE + )); rbuf = gnrc_sixlowpan_frag_rb_array(); for (unsigned i = 0; i < GNRC_SIXLOWPAN_FRAG_RBUF_SIZE; i++) { const gnrc_sixlowpan_frag_rb_t *entry = &rbuf[i]; @@ -467,14 +477,17 @@ static void test_rbuf_add__overlap_rhs(void) pkt2 = gnrc_pktbuf_add(NULL, _fragment2, sizeof(_fragment2), GNRC_NETTYPE_SIXLOWPAN); TEST_ASSERT_NOT_NULL(pkt1); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt1, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt1, TEST_FRAGMENT1_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt3); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt3, - TEST_FRAGMENT3_OFFSET, TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt3, TEST_FRAGMENT3_OFFSET, TEST_PAGE + )); TEST_ASSERT_NOT_NULL(pkt2); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt2, pkt2_offset, - TEST_PAGE); + TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt2, pkt2_offset, TEST_PAGE + )); rbuf = gnrc_sixlowpan_frag_rb_array(); for (unsigned i = 0; i < GNRC_SIXLOWPAN_FRAG_RBUF_SIZE; i++) { const gnrc_sixlowpan_frag_rb_t *entry = &rbuf[i]; @@ -521,10 +534,9 @@ static void test_rbuf_gc__manually(void) gnrc_sixlowpan_frag_rb_t *entry; TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); - /* discarding const qualifier intentionally to override `arrival` */ - entry = (gnrc_sixlowpan_frag_rb_t *)_first_non_empty_rbuf(); + TEST_ASSERT_NOT_NULL((entry = gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT1_OFFSET, TEST_PAGE + ))); TEST_ASSERT_NOT_NULL(entry); /* set arrival GNRC_SIXLOWPAN_FRAG_RBUF_TIMEOUT_US into the past */ entry->super.arrival -= GNRC_SIXLOWPAN_FRAG_RBUF_TIMEOUT_US; @@ -542,10 +554,9 @@ static void test_rbuf_gc__timed(void) gnrc_sixlowpan_frag_rb_t *entry; TEST_ASSERT_NOT_NULL(pkt); - gnrc_sixlowpan_frag_rb_add(&_test_netif_hdr.hdr, pkt, - TEST_FRAGMENT1_OFFSET, TEST_PAGE); - /* discarding const qualifier intentionally to override `arrival` */ - entry = (gnrc_sixlowpan_frag_rb_t *)_first_non_empty_rbuf(); + TEST_ASSERT_NOT_NULL((entry = gnrc_sixlowpan_frag_rb_add( + &_test_netif_hdr.hdr, pkt, TEST_FRAGMENT1_OFFSET, TEST_PAGE + ))); TEST_ASSERT_NOT_NULL(entry); TEST_ASSERT_MESSAGE( xtimer_msg_receive_timeout(&msg, TEST_GC_TIMEOUT) >= 0, From f8d75d7add4b3ba0c8abe2944ad7c11869d3d12c Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Mon, 30 Sep 2019 16:26:33 +0200 Subject: [PATCH 6/8] gnrc_sixlowpan_frag_rb: behavioral change to add() Rather than dispatching the packet automatically once it is complete, `gnrc_sixlowpan_frag_rb_add()` now only returns success, and leaves it to the caller to dispatch the packet. --- sys/include/net/gnrc/sixlowpan/frag/rb.h | 47 ++++++++----------- .../sixlowpan/frag/gnrc_sixlowpan_frag.c | 13 ++++- .../frag/rb/gnrc_sixlowpan_frag_rb.c | 18 +++++-- .../sixlowpan/iphc/gnrc_sixlowpan_iphc.c | 1 - tests/gnrc_sixlowpan_frag/main.c | 24 ++++++++-- 5 files changed, 66 insertions(+), 37 deletions(-) diff --git a/sys/include/net/gnrc/sixlowpan/frag/rb.h b/sys/include/net/gnrc/sixlowpan/frag/rb.h index 0affb6006d..996cbbc411 100644 --- a/sys/include/net/gnrc/sixlowpan/frag/rb.h +++ b/sys/include/net/gnrc/sixlowpan/frag/rb.h @@ -102,7 +102,8 @@ typedef struct { * @param[in] netif_hdr The interface header of the fragment, with * gnrc_netif_hdr_t::if_pid and its source and * destination address set. - * @param[in] frag The fragment to add. + * @param[in] frag The fragment to add. Will be released by the + * function. * @param[in] offset The fragment's offset. * @param[in] page Current 6Lo dispatch parsing page. * @@ -158,24 +159,6 @@ void gnrc_sixlowpan_frag_rb_base_rm(gnrc_sixlowpan_frag_rb_base_t *entry); */ void gnrc_sixlowpan_frag_rb_gc(void); -#if defined(MODULE_GNRC_SIXLOWPAN_FRAG_RB) || defined(DOXYGEN) -/** - * @brief Unsets a reassembly buffer entry (but does not free - * rbuf_t::super::pkt) - * - * @pre `rbuf != NULL` - * - * This functions sets rbuf_t::super::pkt to NULL and removes all rbuf::ints. - * - * @param[in] rbuf A reassembly buffer entry. Must not be NULL. - */ -static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf) -{ - assert(rbuf != NULL); - gnrc_sixlowpan_frag_rb_base_rm(&rbuf->super); - rbuf->pkt = NULL; -} - /** * @brief Checks if a reassembly buffer entry is complete and dispatches it * to the next layer if that is the case @@ -195,6 +178,24 @@ static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf) */ int gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf, gnrc_netif_hdr_t *netif); + +#if defined(MODULE_GNRC_SIXLOWPAN_FRAG_RB) || defined(DOXYGEN) +/** + * @brief Unsets a reassembly buffer entry (but does not free + * rbuf_t::super::pkt) + * + * @pre `rbuf != NULL` + * + * This functions sets rbuf_t::super::pkt to NULL and removes all rbuf::ints. + * + * @param[in] rbuf A reassembly buffer entry. Must not be NULL. + */ +static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf) +{ + assert(rbuf != NULL); + gnrc_sixlowpan_frag_rb_base_rm(&rbuf->super); + rbuf->pkt = NULL; +} #else /* NOPs to be used with gnrc_sixlowpan_iphc if gnrc_sixlowpan_frag_rb is not * compiled in */ @@ -203,14 +204,6 @@ static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf) (void)rbuf; return; } - -static inline int gnrc_sixlowpan_frag_rb_dispatch_when_complete( - gnrc_sixlowpan_frag_rb_t *rbuf, gnrc_netif_hdr_t *netif) -{ - (void)rbuf; - (void)netif; - return -1; -} #endif #ifdef __cplusplus diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/gnrc_sixlowpan_frag.c b/sys/net/gnrc/network_layer/sixlowpan/frag/gnrc_sixlowpan_frag.c index 4c418db811..0ade15e56d 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/frag/gnrc_sixlowpan_frag.c +++ b/sys/net/gnrc/network_layer/sixlowpan/frag/gnrc_sixlowpan_frag.c @@ -339,8 +339,10 @@ error: void gnrc_sixlowpan_frag_recv(gnrc_pktsnip_t *pkt, void *ctx, unsigned page) { - gnrc_netif_hdr_t *hdr = pkt->next->data; + gnrc_pktsnip_t *netif_hdr = pkt->next; + gnrc_netif_hdr_t *hdr = netif_hdr->data; sixlowpan_frag_t *frag = pkt->data; + gnrc_sixlowpan_frag_rb_t *rbe; uint16_t offset = 0; (void)ctx; @@ -359,7 +361,14 @@ void gnrc_sixlowpan_frag_recv(gnrc_pktsnip_t *pkt, void *ctx, unsigned page) return; } - gnrc_sixlowpan_frag_rb_add(hdr, pkt, offset, page); + gnrc_pktbuf_hold(netif_hdr, 1); /* hold netif header to use it with + * dispatch_when_complete() + * (rb_add() releases `pkt`) */ + rbe = gnrc_sixlowpan_frag_rb_add(hdr, pkt, offset, page); + if (rbe != NULL) { + gnrc_sixlowpan_frag_rb_dispatch_when_complete(rbe, hdr); + } + gnrc_pktbuf_release(netif_hdr); } uint16_t gnrc_sixlowpan_frag_next_tag(void) diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c index a9af53786a..b7dc27dad1 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c +++ b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c @@ -223,27 +223,39 @@ static int _rbuf_add(gnrc_netif_hdr_t *netif_hdr, gnrc_pktsnip_t *pkt, if (offset == 0) { #ifdef MODULE_GNRC_SIXLOWPAN_IPHC if (sixlowpan_iphc_is(data)) { + DEBUG("6lo rbuf: detected IPHC header.\n"); gnrc_pktsnip_t *frag_hdr = gnrc_pktbuf_mark(pkt, sizeof(sixlowpan_frag_t), GNRC_NETTYPE_SIXLOWPAN); if (frag_hdr == NULL) { + DEBUG("6lo rbuf: unable to mark fragment header. " + "aborting reassembly.\n"); gnrc_pktbuf_release(entry->pkt); gnrc_pktbuf_release(pkt); gnrc_sixlowpan_frag_rb_remove(entry); return RBUF_ADD_ERROR; } - gnrc_sixlowpan_iphc_recv(pkt, entry, 0); - return res; + else { + DEBUG("6lo rbuf: handing over to IPHC reception.\n"); + /* `pkt` released in IPHC */ + gnrc_sixlowpan_iphc_recv(pkt, entry, 0); + /* check if entry was deleted in IPHC (error case) */ + if (gnrc_sixlowpan_frag_rb_entry_empty(entry)) { + res = RBUF_ADD_ERROR; + } + return res; + } } else #endif if (data[0] == SIXLOWPAN_UNCOMP) { + DEBUG("6lo rbuf: detected uncompressed datagram\n"); data++; } } memcpy(((uint8_t *)entry->pkt->data) + offset, data, frag_size); } - gnrc_sixlowpan_frag_rb_dispatch_when_complete(entry, netif_hdr); + /* no errors and not consumed => release packet */ gnrc_pktbuf_release(pkt); return res; } diff --git a/sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c b/sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c index 4f007dead6..ce74a4da91 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c +++ b/sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c @@ -582,7 +582,6 @@ void gnrc_sixlowpan_iphc_recv(gnrc_pktsnip_t *sixlo, void *rbuf_ptr, sixlo->size - payload_offset); if (rbuf != NULL) { rbuf->super.current_size += (uncomp_hdr_len - payload_offset); - gnrc_sixlowpan_frag_rb_dispatch_when_complete(rbuf, netif_hdr); } else { LL_DELETE(sixlo, netif); diff --git a/tests/gnrc_sixlowpan_frag/main.c b/tests/gnrc_sixlowpan_frag/main.c index b6efe1ff24..8d0e3e34ca 100644 --- a/tests/gnrc_sixlowpan_frag/main.c +++ b/tests/gnrc_sixlowpan_frag/main.c @@ -317,6 +317,7 @@ static void test_rbuf_add__success_complete(void) gnrc_pktsnip_t *pkt4 = gnrc_pktbuf_add(NULL, _fragment4, sizeof(_fragment4), GNRC_NETTYPE_SIXLOWPAN); gnrc_pktsnip_t *datagram; + gnrc_sixlowpan_frag_rb_t *entry1, *entry2; msg_t msg = { .type = 0U }; gnrc_netreg_entry_t reg = GNRC_NETREG_ENTRY_INIT_PID( GNRC_NETREG_DEMUX_CTX_ALL, @@ -326,20 +327,35 @@ static void test_rbuf_add__success_complete(void) gnrc_netreg_register(TEST_DATAGRAM_NETTYPE, ®); /* Mixing up things. Order decided by fair dice-rolls ;-) */ TEST_ASSERT_NOT_NULL(pkt2); - TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + TEST_ASSERT_NOT_NULL((entry1 = gnrc_sixlowpan_frag_rb_add( &_test_netif_hdr.hdr, pkt2, TEST_FRAGMENT2_OFFSET, TEST_PAGE + ))); + TEST_ASSERT_EQUAL_INT(0, gnrc_sixlowpan_frag_rb_dispatch_when_complete( + entry1, &_test_netif_hdr.hdr )); TEST_ASSERT_NOT_NULL(pkt4); - TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + TEST_ASSERT_NOT_NULL((entry2 = gnrc_sixlowpan_frag_rb_add( &_test_netif_hdr.hdr, pkt4, TEST_FRAGMENT4_OFFSET, TEST_PAGE + ))); + TEST_ASSERT(entry1 == entry2); + TEST_ASSERT_EQUAL_INT(0, gnrc_sixlowpan_frag_rb_dispatch_when_complete( + entry1, &_test_netif_hdr.hdr )); TEST_ASSERT_NOT_NULL(pkt1); - TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + TEST_ASSERT_NOT_NULL((entry2 = gnrc_sixlowpan_frag_rb_add( &_test_netif_hdr.hdr, pkt1, TEST_FRAGMENT1_OFFSET, TEST_PAGE + ))); + TEST_ASSERT(entry1 == entry2); + TEST_ASSERT_EQUAL_INT(0, gnrc_sixlowpan_frag_rb_dispatch_when_complete( + entry1, &_test_netif_hdr.hdr )); TEST_ASSERT_NOT_NULL(pkt3); - TEST_ASSERT_NOT_NULL(gnrc_sixlowpan_frag_rb_add( + TEST_ASSERT_NOT_NULL((entry2 = gnrc_sixlowpan_frag_rb_add( &_test_netif_hdr.hdr, pkt3, TEST_FRAGMENT3_OFFSET, TEST_PAGE + ))); + TEST_ASSERT(entry1 == entry2); + TEST_ASSERT(0 < gnrc_sixlowpan_frag_rb_dispatch_when_complete( + entry1, &_test_netif_hdr.hdr )); TEST_ASSERT_MESSAGE( xtimer_msg_receive_timeout(&msg, TEST_RECEIVE_TIMEOUT) >= 0, From 184e91780a60400a1370cb1084660618e5c77cc7 Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Fri, 18 Oct 2019 11:14:13 +0200 Subject: [PATCH 7/8] gnrc_sixlowpan_frag_rb: explain second _rbuf_add() call more detailed --- .../sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c index b7dc27dad1..d0c8856fa4 100644 --- a/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c +++ b/sys/net/gnrc/network_layer/sixlowpan/frag/rb/gnrc_sixlowpan_frag_rb.c @@ -126,6 +126,13 @@ gnrc_sixlowpan_frag_rb_t *gnrc_sixlowpan_frag_rb_add(gnrc_netif_hdr_t *netif_hdr { int res; if ((res = _rbuf_add(netif_hdr, pkt, offset, page)) == RBUF_ADD_REPEAT) { + /* there was an overlap with existing fragments detected when trying to + * add the new fragment. + * https://tools.ietf.org/html/rfc4944#section-5.3 states "A fresh + * reassembly may be commenced with the most recently received link + * fragment.", so let's do that. Since the reassembly buffer entry was + * deleted another overlap should not be detected (so _rbuf_add() won't + * return RBUF_ADD_REPEAT again) */ res = _rbuf_add(netif_hdr, pkt, offset, page); } return (res < 0) ? NULL : &rbuf[res]; From ae879bc2bab8eeab8fdf4bab66c1383702553836 Mon Sep 17 00:00:00 2001 From: "Martine S. Lenders" Date: Fri, 18 Oct 2019 12:02:03 +0200 Subject: [PATCH 8/8] gnrc_sixlowpan_frag_rb: add doc on rb_remove() NOP --- sys/include/net/gnrc/sixlowpan/frag/rb.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sys/include/net/gnrc/sixlowpan/frag/rb.h b/sys/include/net/gnrc/sixlowpan/frag/rb.h index 996cbbc411..b66b1d31d1 100644 --- a/sys/include/net/gnrc/sixlowpan/frag/rb.h +++ b/sys/include/net/gnrc/sixlowpan/frag/rb.h @@ -188,6 +188,8 @@ int gnrc_sixlowpan_frag_rb_dispatch_when_complete(gnrc_sixlowpan_frag_rb_t *rbuf * * This functions sets rbuf_t::super::pkt to NULL and removes all rbuf::ints. * + * @note Does nothing if module `gnrc_sixlowpan_frag_rb` is not included. + * * @param[in] rbuf A reassembly buffer entry. Must not be NULL. */ static inline void gnrc_sixlowpan_frag_rb_remove(gnrc_sixlowpan_frag_rb_t *rbuf)