pkt: simplify API by miri64 · Pull Request #2342 · RIOT-OS/RIOT

miri64 · 2015-01-22T14:35:40Z

Simplifies pkt module API by unifying pkt_t and pkt_hlist_t to pktsnip_t and by changing how packets are viewed in the network stack in general.

A concatenated packet (it does not have to be concatenated ;-)) packet in the packet buffer would be annotated as follows:

                                                                  buffer
              +---------------------------+                      +------+
              | size = 14                 | data +-------------->|      |
              | type = PKT_PROTO_ETHERNET |------+               +------+
              +---------------------------+         +----------->|      |
                    | next                          |            |      |
                    v                               |            |      |
              +---------------------------+         |            +------+
              | size = 40                 | data    |  +-------->|      |
              | type = PKT_PROTO_IPV6     |---------+  |         +------+
              +---------------------------+            |  +----->|      |
                    | next                             |  |      +------+
                    v                                  |  |  +-->|      |
              +---------------------------+            |  |  |   |      |
              | size = 8                  | data       |  |  |   .      .
              | type = PKT_PROTO_UDP      |------------+  |  |   .      .
              +---------------------------+               |  |
                    | next                                |  |
                    v                                     |  |
              +---------------------------+               |  |
              | size = 5                  | data          |  |
              | type = PKT_PROTO_COAP     |---------------+  |
              +---------------------------+                  |
                    | next                                   |
                    v                                        |
              +---------------------------+                  |
              | size = 54                 | data             |
              | type = PKT_PROTO_UNKNOWN  |------------------+
              +---------------------------+

miri64 · 2015-01-30T13:48:28Z

Rebased to master

miri64 · 2015-02-07T14:07:31Z

I included the changes discussed in the Network stack task force meeting and based it on #2397 now.

miri64 · 2015-02-07T14:07:45Z

Tests are not adapted, yet.

haukepetersen · 2015-02-07T15:40:05Z

I would strongly vote for renaming the module and header file to pktsnip and pktsnip.h! Also I would actually cut the module down to a single header file - I my opinion the included functions are not really bringing much benefits, so I would keep it simple and drop them.

miri64 · 2015-02-07T17:21:31Z

I'm against renaming the file or the module. The overall concept is abstracting packets. pktsnip_t is just a type to represent a node in the list of headers that is a packet. We don't named clist.h clist_node.h either, though the type is named clist_node_t.

The functions bring the benefit of reducing code duplication in every layer. I was sick of doing the same thing I did already for nomac and sixlowpan in IPv6 by basically copying 20 lines of code or so over every time.

miri64 · 2015-02-07T23:27:00Z

Since I recently discovered utlist the functions where really just wrappers for utlist macros, so I decided to remove most of them. The only remaining one, which is now inlined is the byte-counter function ng_pkt_len().

LudwigKnuepfer · 2015-02-08T10:22:28Z

sys/include/net/ng_pkt.h

.. in the static ..

… packet buffer. As seen in line below ;-)

haukepetersen · 2015-02-10T12:18:07Z

sys/include/net/ng_pkt.h

This module was already included in 2014.

I was once told that I should always update my copyrights to whenever I touched the file last - but I don't really care...

Both, the first and last year of the copyright "application" are important to determine since, and until when it holds.

Well, that would be 2014-15 then, but we don't do this anywhere else…

Sure we do, also why not start now even if?

RIOT/sys/vtimer/vtimer.c

Line 4 in e5e1d0d

* Copyright (C) 2013 - 2015 Freie Universität Berlin

$ git grep -E "201[0-9].* 201[0-9]" | wc -l 33

miri64 · 2015-02-10T14:10:03Z

Addressed comments

miri64 · 2015-02-10T14:12:26Z

Rebased to current master

miri64 · 2015-02-10T14:57:36Z

Disentangled: squashable commits that contained both changes to pkt and pktqueue

haukepetersen · 2015-02-10T20:49:02Z

ACK when squashed.

miri64 · 2015-02-11T09:50:39Z

Squashed

miri64 · 2015-02-11T10:01:57Z

Added fixup to doc

pkt: simplify API

miri64 assigned haukepetersen Jan 22, 2015

miri64 added this to the Release NEXT MAJOR milestone Jan 22, 2015

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

miri64 force-pushed the pkt/api/simplify branch 2 times, most recently from 31a6e67 to afa484a Compare January 30, 2015 13:48

miri64 mentioned this pull request Jan 30, 2015

pktbuf: port to use pkt_t instead of void* #2285

Merged

OlegHahm modified the milestones: Release NEXT MAJOR, Network Stack Task Force Feb 6, 2015

OlegHahm added NSTF and removed NSTF labels Feb 6, 2015

miri64 force-pushed the pkt/api/simplify branch from 6ec76f9 to 5816332 Compare February 7, 2015 14:06

miri64 mentioned this pull request Feb 7, 2015

Network stack refactoring: TODO list #2278

Closed

36 tasks

LudwigKnuepfer reviewed Feb 8, 2015
View reviewed changes

haukepetersen reviewed Feb 10, 2015
View reviewed changes

miri64 force-pushed the pkt/api/simplify branch from 9cdfd70 to f0953fe Compare February 10, 2015 13:08

miri64 force-pushed the pkt/api/simplify branch from 0f72c1f to d75dfe2 Compare February 10, 2015 14:12

miri64 force-pushed the pkt/api/simplify branch from b79078c to 14f891a Compare February 10, 2015 14:57

miri64 force-pushed the pkt/api/simplify branch from 5608e51 to ddf85a5 Compare February 11, 2015 09:50

miri64 force-pushed the pkt/api/simplify branch from ddf85a5 to b36e9b7 Compare February 11, 2015 10:00

miri64 added 2 commits February 11, 2015 11:01

pkt: simplify API

7317097

pktqueue: adapt to change in pkt.h

cb74094

miri64 force-pushed the pkt/api/simplify branch from b36e9b7 to cb74094 Compare February 11, 2015 10:01

miri64 added a commit that referenced this pull request Feb 11, 2015

Merge pull request #2342 from authmillenon/pkt/api/simplify

cfc64ff

pkt: simplify API

miri64 merged commit cfc64ff into RIOT-OS:master Feb 11, 2015

miri64 deleted the pkt/api/simplify branch February 11, 2015 11:41

Conversation

miri64 commented Jan 22, 2015

Uh oh!

miri64 commented Jan 30, 2015

Uh oh!

miri64 commented Feb 7, 2015

Uh oh!

miri64 commented Feb 7, 2015

Uh oh!

haukepetersen commented Feb 7, 2015

Uh oh!

miri64 commented Feb 7, 2015

Uh oh!

miri64 commented Feb 7, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Feb 10, 2015

Uh oh!

miri64 commented Feb 10, 2015

Uh oh!

miri64 commented Feb 10, 2015

Uh oh!

haukepetersen commented Feb 10, 2015

Uh oh!

miri64 commented Feb 11, 2015

Uh oh!

miri64 commented Feb 11, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants