[RFC] HEADS UP: s/net_nbuf/net_pkt and separation from net_buf


Tomasz Bursztyka
 

Hello,

After Linaro Connect, and previous talk also about optimizing struct net_buf as well,
I am sending a patch-set (a preliminary one) that:

- Rename net_nbuf to net_pkt: net_nbuf has always been a network buffer metadata placeholder,
which in the end is about describing the actual packet found in the buffer fragments, so
net_pkt as the metadata holder sounded a logical name.

- Separate net_pkt (the former net_nbuf) from net_buf: net_pkt was a specialized type of
net_buf, stored in user data side of it, and thus brought along some unused attributes.
It made it easy for all the reference counting part and buffer looping, but not so much
for understanding what is holding where: all nbuf functions were asking for a net_buf
pointer which confused users. Now, it's quite clearer as you have to specifically pass
a net_pkt for the whole packet, or a net_buf if you work on actual packet buffer fragment.
Along to this separation, net_pkt is now allocated through k_mem_slab kernel object.

(Note: All net_pkt functions are prefixed net_pkt_* BUT the ones dealing only with
net_buf fragments which are then prefixed net_frag_*, I am not yet entirely sure I'll
keep this name. net_pkt_frag_* could be a possibility)

- Various changes, and optimizations such as TCP sent list moved to net_pkt, some net_pkt
attributes removed/changed etc...

I obviously could not test all the existing samples. Our unit tests are rather limited, but
the seem to pass.

I do have an issue with tcp on echo apps, which I am going to fix.

So go fetch it, test it and give feedback. I most probably broke some network drivers or
use-cases.

This is just the beginning of various changes and optimizations.

Anyway, patch-set can be found here:

https://gerrit.zephyrproject.org/r/#/q/status:open+project:zephyr+branch:net+topic:net_pkt


Thanks,

Tomasz


Tomasz Bursztyka
 

Hi,

As a small notice, I quickly checked on the new tests/net/all app (it compiles the net stack with
as much as possible of its options).

This patch-set gives a bit more than 1Ko of ROM saved, and about 1.5Ko of RAM saved.
The gain can be more visible if some apps was having bigger data buffer pools etc...

Overall is not a tremendous gain, but it's a gain. So far so good then.

More changes on net_buf will help to make it better.

Tomasz

Hello,

After Linaro Connect, and previous talk also about optimizing struct net_buf as well,
I am sending a patch-set (a preliminary one) that:

- Rename net_nbuf to net_pkt: net_nbuf has always been a network buffer metadata placeholder,
which in the end is about describing the actual packet found in the buffer fragments, so
net_pkt as the metadata holder sounded a logical name.

- Separate net_pkt (the former net_nbuf) from net_buf: net_pkt was a specialized type of
net_buf, stored in user data side of it, and thus brought along some unused attributes.
It made it easy for all the reference counting part and buffer looping, but not so much
for understanding what is holding where: all nbuf functions were asking for a net_buf
pointer which confused users. Now, it's quite clearer as you have to specifically pass
a net_pkt for the whole packet, or a net_buf if you work on actual packet buffer fragment.
Along to this separation, net_pkt is now allocated through k_mem_slab kernel object.

(Note: All net_pkt functions are prefixed net_pkt_* BUT the ones dealing only with
net_buf fragments which are then prefixed net_frag_*, I am not yet entirely sure I'll
keep this name. net_pkt_frag_* could be a possibility)

- Various changes, and optimizations such as TCP sent list moved to net_pkt, some net_pkt
attributes removed/changed etc...

I obviously could not test all the existing samples. Our unit tests are rather limited, but
the seem to pass.

I do have an issue with tcp on echo apps, which I am going to fix.

So go fetch it, test it and give feedback. I most probably broke some network drivers or
use-cases.

This is just the beginning of various changes and optimizations.

Anyway, patch-set can be found here:

https://gerrit.zephyrproject.org/r/#/q/status:open+project:zephyr+branch:net+topic:net_pkt


Thanks,

Tomasz

_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@lists.zephyrproject.org
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel


Paul Sokolovsky
 

Hello Tomasz,

Thanks for coming forward with this patchset. I eyeballed first 5-6
patches in the set, and just did a smoke test of applying them one by
one and making sure echo_server works. I understand it's just a smoke
test, but well, at least I can say that I tried your patches ;-).

The patchset starts with a couple of truly massive patches, with first
one, https://gerrit.zephyrproject.org/r/12844, doing nothing but
symbolic renaming, i.e. doesn't affect binary code. At the same time,
it's really huge, so would be constantly broken due to conflicts with
concurrent changes made. Given that further patches show real benefits
of this refactor, like optimizing layouts to save memory, and some were
already reviewed and OKed by Jukka, it would be nice to discuss plans
on getting these changes landed, starting with the big renaming patch
above.

On Wed, 12 Apr 2017 12:02:54 +0200
Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com> wrote:

As a small notice, I quickly checked on the new tests/net/all app (it
compiles the net stack with
as much as possible of its options).

This patch-set gives a bit more than 1Ko of ROM saved, and about
1.5Ko of RAM saved.
The gain can be more visible if some apps was having bigger data
buffer pools etc...
I think 1.5KB of RAM saved is already a very good result. That's a
whole IPv6 or Ethernet packet more, which may mean extra network
connection more, and as we'd usually count them as single digits
anyway, having an extra one isn't too bad. I'm sure that the patchset
you've been working on will show the real benefits of the native Zephyr
IP stack, so keeping fingers crossed that the landing process will be
smooth and soon we'll be able to benefit from it!

[]

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog


Paul Sokolovsky
 

Hello,

So, this patchset was merged today to the "net" branch, thanks Tomasz
and Jukka!

I updated my BSD Sockets prototype in MicroPython
(https://github.com/pfalcon/micropython/commits/zephyr-socket2), and
tested it to work as expected with the basic manual tests and few
automated tests (whole 4 of them, which I was able to implement so far).

This removed few hacks even in the basic implementation, and I hope to
perform packet queue optimization without gross hacks which were
required previously.

Anyway, my immediate plan is to take renaming opportunity and rework
net_pkt_append() to return length of added data instead of just boolean
status (https://jira.zephyrproject.org/browse/ZEP-1984), hope to do
that tomorrow.

Thanks!


On Sun, 16 Apr 2017 16:48:09 +0300
Paul Sokolovsky <Paul.Sokolovsky@linaro.org> wrote:

Hello Tomasz,

Thanks for coming forward with this patchset. I eyeballed first 5-6
patches in the set, and just did a smoke test of applying them one by
one and making sure echo_server works. I understand it's just a smoke
test, but well, at least I can say that I tried your patches ;-).

The patchset starts with a couple of truly massive patches, with first
one, https://gerrit.zephyrproject.org/r/12844, doing nothing but
symbolic renaming, i.e. doesn't affect binary code. At the same time,
it's really huge, so would be constantly broken due to conflicts with
concurrent changes made. Given that further patches show real benefits
of this refactor, like optimizing layouts to save memory, and some
were already reviewed and OKed by Jukka, it would be nice to discuss
plans on getting these changes landed, starting with the big renaming
patch above.

On Wed, 12 Apr 2017 12:02:54 +0200
Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com> wrote:

As a small notice, I quickly checked on the new tests/net/all app
(it compiles the net stack with
as much as possible of its options).

This patch-set gives a bit more than 1Ko of ROM saved, and about
1.5Ko of RAM saved.
The gain can be more visible if some apps was having bigger data
buffer pools etc...
I think 1.5KB of RAM saved is already a very good result. That's a
whole IPv6 or Ethernet packet more, which may mean extra network
connection more, and as we'd usually count them as single digits
anyway, having an extra one isn't too bad. I'm sure that the patchset
you've been working on will show the real benefits of the native
Zephyr IP stack, so keeping fingers crossed that the landing process
will be smooth and soon we'll be able to benefit from it!

[]


--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog