ng_pktbuf: new default packet buffer implementation #3496

miri64 · 2015-07-25T00:18:10Z

Implements the API proposed in #3458. It also simplifies a lot in the packet buffer. It still isn't perfect (there is happening way to much copying for my taste), but I think it's a first step in a better direction.

cgundogan · 2015-07-26T11:51:19Z

sys/net/crosslayer/ng_pktbuf_default/ng_pktbuf_default.c

+
+static inline bool _too_small_hole(_unused_t *a, _unused_t *b)
+{
+ return sizeof(_unused_t) > (size_t)(((uint8_t *)b) - (((uint8_t *)a) + a->size));


is it really necessary to add a->size here? I assumed that the address of b is already at least a->size bytes next to a

Yes. It's not about the size between pointer a and pointer b (which would just be the size of chunk a plaus anything in-between), but the size of the space between the chunks represented by a and b. To put this graphically:

(((uint8_t *)b) - ((uint8_t *)a)): |------------| +---------+ +---------+ +---------+ +---------+ a b vs. (size_t)(((uint8_t *)b) - (((uint8_t *)a) + a->size)); |--| +---------+ +---------+ +---------+ +---------+ a b

This is correcting for the fact, that I size-up the chunk to the next word-aligned pointer and the next size that would fit an _unused_t struct in _pktbuf_alloc()

miri64 · 2015-07-26T23:27:09Z

Tested successfully with 6LoWPAN in the iot-lab testbed in the following configurations:

with IPHC with payloads 10, 120, 240, 320 (ping6 100 <ip> <payload> 10)
without IPHC with payloads 10, 120, 240, 320 (ping6 100 <ip> <payload> 10)

You need the following PRs for it (or just use the commit abeb852 in my repository):

cpu: cortexm_common: set pendSV to default priority #3450 (at least when you face the same problems with UART on iot-lab_M3 as I do)
ng_pktbuf: new default packet buffer implementation #3496 (this PR)
ng_ipv6: fix write protect #3498
ng_ipv6: use ng_netapi_dispatch_send to dispatch to 6LoWPAN #3499 (probably not necessary, but the more the merrier)

miri64 · 2015-07-27T10:49:39Z

Rebased to current #3458. Please merge this one and ignore #3458, which is not able to be merged on its own (I don't care to fix the old packet buffer for the new API when this PR deletes it)

haukepetersen · 2015-07-28T12:47:47Z

Just had to think about it: I think it would make sense to rename this new implementation to pktbuf_static (and of course make it the default packet buffer implicitly)

miri64 · 2015-07-28T13:02:48Z

Fine by me.

miri64 · 2015-07-28T14:38:22Z

Renamed and made unittests only available for ng_pktbuf_static

haukepetersen · 2015-07-28T15:46:16Z

Makefile.dep

@@ -218,6 +218,12 @@ ifneq (,$(filter ng_pktdump,$(USEMODULE)))
 USEMODULE += od
 endif

+ifneq (,$(filter ng_pktbuf, $(USEMODULE)))
+ ifeq (,$(filter ng_pktbuf_dynamic, $(USEMODULE)))


I think we can make this more general: ifeq(,$(filter ng_pktbuf_%, $(USEMODULE)))?!

PeterKietzmann · 2015-07-28T16:17:12Z

sys/include/net/ng_pktbuf.h

 */
-void ng_pktbuf_reset(void);


For me that was a nice function...

ng_pktbuf_init() does the same but is now mandatory on first start ;-)

Ahh, cool! You think it's reasonable to spend a small comment there?

Tell me what to write apart from "initializes the packet buffer" and I'll do it :>

PeterKietzmann · 2015-07-28T17:46:25Z

For the protocol: I'm running a test-application on the iot-lab_M3 node which sends 1000 UDP packets for payloads from 10:1:330 bytes/packet without a pause. No faults and the result looks valid.

cgundogan · 2015-07-28T18:45:57Z

sys/net/crosslayer/ng_pktbuf_static/ng_pktbuf_static.c

+ if (size == 0) {
+ DEBUG("pktbuf: size == 0\n");
+ mutex_unlock(&_mutex);
+ return ENOMEM;


PeterKietzmann · 2015-07-28T18:50:46Z

And again, to finish my protocol: I'm running a test-application on the iot-lab_M3 node which "virtually" receives 1000 UDP packets for payloads from 10:1:330 bytes/packet without a pause. No faults and the result looks valid. Of course, IPHC and fragmentation are enabled. Note: That was not possible with the old implementation. Häppi to see that :-)

PeterKietzmann · 2015-07-28T18:51:15Z

and green lights, btw

miri64 · 2015-07-28T18:51:39Z

Cenk found some last minute erroros

PeterKietzmann · 2015-07-28T18:51:52Z

Hups, what did I see? There were no green lights, sry...

miri64 · 2015-07-28T18:52:43Z

No I just pushed something

miri64 · 2015-07-28T19:31:58Z

Added some last minute optimizations. :-)

OlegHahm · 2015-07-28T20:03:47Z

sys/net/crosslayer/ng_pktbuf_static/ng_pktbuf_static.c

+ size_t aligned_size = (size < sizeof(_unused_t)) ?
+ _align(sizeof(_unused_t)) : _align(size);
+ mutex_lock(&_mutex);
+ assert((pkt == NULL) || (pkt->data == NULL) || _pktbuf_contains(pkt->data));


This assert only checks if at least one condition is met, but should check that all of them are met.

Who coded this stuff? Certainly not me :D

OlegHahm · 2015-07-28T20:57:00Z

ACK

This simplifies the `ng_pktbuf` API by adding a new function `ng_pktbuf_mark()` which takes over some functionality of `ng_pktbuf_add()`. `size == 0` for `ng_pktbuf_add()` is now illegal.

miri64 · 2015-07-28T22:58:21Z

Some unittests were still failing… now everything should be alright (hopefully)

miri64 · 2015-07-28T22:58:58Z

Unittests succeeded \o/

ng_pktbuf: new default packet buffer implementation

miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. NSTF labels Jul 25, 2015

miri64 assigned haukepetersen Jul 25, 2015

miri64 added this to the Release 2015.06 milestone Jul 25, 2015

miri64 force-pushed the ng_pktbuf/enh/new_default branch 3 times, most recently from e655e98 to a442bd4 Compare July 25, 2015 20:02

cgundogan reviewed Jul 26, 2015
View reviewed changes

miri64 force-pushed the ng_pktbuf/enh/new_default branch from 1481534 to 74c8cf8 Compare July 27, 2015 10:48

This was referenced Jul 27, 2015

ng_sixlowpan: Fixes #3494

Merged

core: add ng_pktbuf_stats() to core panic #3500

Closed

[RFC] gnrc: initial import of an asynchronous error reporting API #3362

Merged

PeterKietzmann mentioned this pull request Jul 28, 2015

ng_pktbuf: simplify API by extension #3458

Merged

haukepetersen reviewed Jul 28, 2015
View reviewed changes

OlegHahm added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jul 28, 2015

PeterKietzmann reviewed Jul 28, 2015
View reviewed changes

cgundogan reviewed Jul 28, 2015
View reviewed changes

miri64 force-pushed the ng_pktbuf/enh/new_default branch from 04d7154 to d85a7bf Compare July 28, 2015 18:51

miri64 force-pushed the ng_pktbuf/enh/new_default branch 2 times, most recently from b32067e to cbc97a4 Compare July 28, 2015 19:31

miri64 force-pushed the ng_pktbuf/enh/new_default branch 2 times, most recently from 2ae9b11 to 1d3abfa Compare July 28, 2015 19:46

OlegHahm reviewed Jul 28, 2015
View reviewed changes

miri64 force-pushed the ng_pktbuf/enh/new_default branch 4 times, most recently from 3f4b39a to 9578ab9 Compare July 28, 2015 20:42

miri64 and others added 4 commits July 29, 2015 00:47

ng_pktbuf: simplify API by extension

41b0a26

This simplifies the `ng_pktbuf` API by adding a new function `ng_pktbuf_mark()` which takes over some functionality of `ng_pktbuf_add()`. `size == 0` for `ng_pktbuf_add()` is now illegal.

unittests: adapt ng_pktbuf tests for new API

f747b07

ng_pktbuf: new default packet buffer implementation

9e82307

gnrc: replace ng_pktbuf_add() with ng_pktbuf_mark() where it's needed

377f5cc

miri64 force-pushed the ng_pktbuf/enh/new_default branch from 9578ab9 to 377f5cc Compare July 28, 2015 22:48

miri64 added a commit that referenced this pull request Jul 29, 2015

Merge pull request #3496 from authmillenon/ng_pktbuf/enh/new_default

3d99456

ng_pktbuf: new default packet buffer implementation

miri64 merged commit 3d99456 into RIOT-OS:master Jul 29, 2015

miri64 deleted the ng_pktbuf/enh/new_default branch July 29, 2015 00:08

miri64 added the Area: network Area: Networking label Sep 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ng_pktbuf: new default packet buffer implementation #3496

ng_pktbuf: new default packet buffer implementation #3496

miri64 commented Jul 25, 2015

cgundogan Jul 26, 2015

miri64 Jul 26, 2015

miri64 commented Jul 26, 2015

miri64 commented Jul 27, 2015

haukepetersen commented Jul 28, 2015

miri64 commented Jul 28, 2015

miri64 commented Jul 28, 2015

haukepetersen Jul 28, 2015

miri64 Jul 28, 2015

PeterKietzmann Jul 28, 2015

miri64 Jul 28, 2015

PeterKietzmann Jul 28, 2015

miri64 Jul 28, 2015

PeterKietzmann commented Jul 28, 2015

cgundogan Jul 28, 2015

PeterKietzmann commented Jul 28, 2015

PeterKietzmann commented Jul 28, 2015

miri64 commented Jul 28, 2015

PeterKietzmann commented Jul 28, 2015

miri64 commented Jul 28, 2015

miri64 commented Jul 28, 2015

OlegHahm Jul 28, 2015

miri64 Jul 28, 2015

OlegHahm commented Jul 28, 2015

miri64 commented Jul 28, 2015

miri64 commented Jul 28, 2015

		*/
		void ng_pktbuf_reset(void);

ng_pktbuf: new default packet buffer implementation #3496

ng_pktbuf: new default packet buffer implementation #3496

Conversation

miri64 commented Jul 25, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miri64 commented Jul 26, 2015

miri64 commented Jul 27, 2015

haukepetersen commented Jul 28, 2015

miri64 commented Jul 28, 2015

miri64 commented Jul 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterKietzmann commented Jul 28, 2015

Choose a reason for hiding this comment

PeterKietzmann commented Jul 28, 2015

PeterKietzmann commented Jul 28, 2015

miri64 commented Jul 28, 2015

PeterKietzmann commented Jul 28, 2015

miri64 commented Jul 28, 2015

miri64 commented Jul 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlegHahm commented Jul 28, 2015

miri64 commented Jul 28, 2015

miri64 commented Jul 28, 2015