Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 19, 2015

Alternative to #2285. Still WIP (tests need to be adapted).

There were two problems in #2285:

  1. To move the pointer pkt->payload_data was not thread-safe.
  2. adding new headers would always require allocate new data (just because of the way the pktbuf was handled internally there).

The solution for both is more or less to keep pktbuf a void pointer oriented buffer and (for (1) specifically) to not put pkt_t structs into it. This way every thread is responsible for their own instance of pkt_t and a moving payload_data is not the problem anymore. The header problem get's solved, since the internal organizing structure does not inherit from pkt_header_t anymore now.

Since through this we lost some requirements for pkt, I simplified it a little:

  • I moved pkt.h to sys/net/include since it has pktbuf as dependency anyway (technically this is not due to a requirement-loss ;-))
  • I removed the packed attribute from pkt_t since it is not stored in the pktbuf anymore
  • I removed the member payload_proto from pkt_t since I don't see any use-case for it and we save this way a few byte. It was introduced, since I needed a relationship between pkt_t and pkt_hlist_t to use a modified version of them as pktbuf's internal organizing structure.

@miri64 miri64 added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Jan 19, 2015
@miri64 miri64 added this to the Release NEXT MAJOR milestone Jan 19, 2015
@miri64 miri64 force-pushed the pktbuf/api/pkt_t-alternative branch from 8541d2f to 5dff0c3 Compare January 19, 2015 03:53
@miri64 miri64 force-pushed the pktbuf/api/pkt_t-alternative branch from 5dff0c3 to e8d340d Compare January 19, 2015 03:57
@haukepetersen
Copy link
Contributor

As said before, I don't like this. I think the problems (1) and (2) can be solved by a good pktbuf implementation and we gain a lot of benefits in terms of code readability and even more important efficiency when implementing network modules aka protocols.


#ifdef DEVELHELP

if (pkt == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-DDEVELHELP must not shadow errors. Use puts("pkt == NULL, expect errors!").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is conflicting with what @OlegHahm said, back when I introduced this in #1129. Also: Did you mean s/puts/DEBUG/?

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2015

Problem solved in #2285

@miri64 miri64 closed this Jan 30, 2015
@miri64 miri64 deleted the pktbuf/api/pkt_t-alternative branch January 30, 2015 21:08
@OlegHahm OlegHahm modified the milestones: Release NEXT MAJOR, Release 2015.06 Apr 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants