gnrc/tcp : Expose configurations to Kconfig by akshaim · Pull Request #14126 · RIOT-OS/RIOT

akshaim · 2020-05-24T04:03:58Z

Contribution description

This PR exposes compile configurations in GNRC: TCP to Kconfig.

Testing procedure

New documentation was built using Doxygen

The build works fine.

Header and macro was introduced to tests/gnrc_tcp/

#include "net/gnrc/tcp/config.h"
#define STR(x)   #x
#define SHOW_DEFINE(x) printf("%s=%s\n", #x, STR(x))

The test file can be found here

Compiled binaries (BOARD=samr21-xpro) were uploaded to FIT IoT Test bed.

Default State:

Firmware Output

~~}!main(): This is RIOT! (Version: 2020.07-devel-737-g0b75d-Kconfig_tcp_tests)
~~}!CONFIG_GNRC_TCP_CONNECTION_TIMEOUT_DURATION=3000000
~~}!CONFIG_GNRC_TCP_MSL=1000000
~~}!CONFIG_GNRC_TCP_MSS_MULTIPLICATOR=(1U)
~~}!CONFIG_GNRC_TCP_RCV_BUFFERS=(1U)
~~}!CONFIG_GNRC_TCP_RTO_LOWER_BOUND=(1U * (1000000LU))
~~}!CONFIG_GNRC_TCP_RTO_UPPER_BOUND=(60U * (1000000LU))
~~}!CONFIG_GNRC_TCP_RTO_GRANULARITY=(10U * (1000LU))
~~}!CONFIG_GNRC_TCP_RTO_A_DIV=(8U)
~~}!CONFIG_GNRC_TCP_RTO_B_DIV=(4U)
~~}!CONFIG_GNRC_TCP_RTO_K=(4U)
~~}!CONFIG_GNRC_TCP_PROBE_LOWER_BOUND=(1U * (1000000LU))
~~}!CONFIG_GNRC_TCP_PROBE_UPPER_BOUND=(60U * (1000000LU))
~~}!RIOT GNRC_TCP test application

Usage with menuconfig [default values]

make menuconfig

Firmware Output

!main(): This is RIOT! (Version: 2020.07-devel-737-g82bda-Kconfig_tcp_tests)
~~}!CONFIG_GNRC_TCP_CONNECTION_TIMEOUT_DURATION=120000000
~~}!CONFIG_GNRC_TCP_MSL=30000000
~~}!CONFIG_GNRC_TCP_MSS_MULTIPLICATOR=1
~~}!CONFIG_GNRC_TCP_RCV_BUFFERS=1
~~}!CONFIG_GNRC_TCP_RTO_LOWER_BOUND=1000000
~~}!CONFIG_GNRC_TCP_RTO_UPPER_BOUND=60000000
~~}!CONFIG_GNRC_TCP_RTO_GRANULARITY=10000
~~}!CONFIG_GNRC_TCP_RTO_A_DIV=8
~~}!CONFIG_GNRC_TCP_RTO_B_DIV=4
~~}!CONFIG_GNRC_TCP_RTO_K=4
~~}!CONFIG_GNRC_TCP_PROBE_LOWER_BOUND=1000000
~~}!CONFIG_GNRC_TCP_PROBE_UPPER_BOUND=60000000
~~}!RIOT GNRC_TCP test application

Usage with menuconfig

make menuconfig

Firmware Output

!main(): This is RIOT! (Version: 2020.07-devel-737-g82bda-Kconfig_tcp_tests)
~~}!CONFIG_GNRC_TCP_CONNECTION_TIMEOUT_DURATION=110000000
~~}!CONFIG_GNRC_TCP_MSL=32000000
~~}!CONFIG_GNRC_TCP_MSS_MULTIPLICATOR=2
~~}!CONFIG_GNRC_TCP_RCV_BUFFERS=2
~~}!CONFIG_GNRC_TCP_RTO_LOWER_BOUND=1200000
~~}!CONFIG_GNRC_TCP_RTO_UPPER_BOUND=62000000
~~}!CONFIG_GNRC_TCP_RTO_GRANULARITY=20000
~~}!CONFIG_GNRC_TCP_RTO_A_DIV=4
~~}!CONFIG_GNRC_TCP_RTO_B_DIV=2
~~}!CONFIG_GNRC_TCP_RTO_K=2
~~}!CONFIG_GNRC_TCP_PROBE_LOWER_BOUND=1200000
~~}!CONFIG_GNRC_TCP_PROBE_UPPER_BOUND=50000000
~~}!RIOT GNRC_TCP test application

MACROS were successfully configured.

Issues/PRs references

#12888

sys/include/net/gnrc/tcp/config.h

sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c

brummer-simon

The changes did not break any existing tests (tested on "native" and "nucleo-f401re") and the custom configurations are propagated into the binary. From a technical point of view it works good, however there are some descriptions value descriptions that are a bit misleading.

If you fix those the PR is fine by me.

sys/include/net/gnrc/tcp/config.h

sys/net/gnrc/transport_layer/tcp/Kconfig

brummer-simon

I have some small change requests aside from that good work.

sys/include/net/gnrc/tcp/config.h

sys/net/gnrc/transport_layer/tcp/Kconfig

akshaim · 2020-06-01T13:32:22Z

I have some small change requests aside from that good work.

Thank you for the comments. I have updated the file. Introduced a new symbol to make sure that the default configuration remains intact. Directly configuring the CONFIG_GNRC_TCP_DEFAULT_WINDOW via menuconfig does not have a dependency on GNRC_TCP_MSS_MULTIPLICATOR.

brummer-simon

I have tested this PR on "native" and on "nucleo-f401re". The test suite works as expected an all configured values are present in the resulting binary. From my point of view, this PR can be merged, although I don't have the rights to do that.

@akshaim - Thanks for adding the Kconfig support.

akshaim · 2020-06-02T07:20:12Z

I have tested this PR on "native" and on "nucleo-f401re". The test suite works as expected an all configured values are present in the resulting binary. From my point of view, this PR can be merged, although I don't have the rights to do that.

@akshaim - Thanks for adding the Kconfig support.

Thanks @brummer-simon for the review. @leandrolanzieri Should I squash now ?

leandrolanzieri · 2020-06-02T08:24:53Z

I have tested this PR on "native" and on "nucleo-f401re". The test suite works as expected an all configured values are present in the resulting binary. From my point of view, this PR can be merged, although I don't have the rights to do that.
@akshaim - Thanks for adding the Kconfig support.

Thanks @brummer-simon for the review. @leandrolanzieri Should I squash now ?

Yes please

Add compile time parameters to doxygen group 'net_gnrc_conf'

Expose configurations to Kconfig

Set CONFIG_GNRC_TCP_MSL and CONFIG_GNRC_TCP_CONNECTION_TIMEOUT_DURATION if not set via Kconfig

akshaim · 2020-06-02T10:19:59Z

I have tested this PR on "native" and on "nucleo-f401re". The test suite works as expected an all configured values are present in the resulting binary. From my point of view, this PR can be merged, although I don't have the rights to do that.
@akshaim - Thanks for adding the Kconfig support.

Thanks @brummer-simon for the review. @leandrolanzieri Should I squash now ?

Yes please

Done. I have reordered the commit history, to make better sense of, but github displays commits by order of author date.

leandrolanzieri

Changes look good and @brummer-simon confirmed that this works properly. ACK.

akshaim · 2020-06-03T06:47:57Z

Thanks @leandrolanzieri

akshaim requested review from PeterKietzmann, cgundogan, haukepetersen, jia200x, leandrolanzieri and miri64 as code owners May 24, 2020 04:03

akshaim force-pushed the Kconfig_tcp branch 2 times, most recently from 2c8a0ae to 8269a26 Compare May 24, 2020 05:54

benpicco added the Area: Kconfig Area: Kconfig integration label May 24, 2020

leandrolanzieri reviewed May 25, 2020

View reviewed changes

sys/include/net/gnrc/tcp/config.h Outdated Show resolved Hide resolved

sys/include/net/gnrc/tcp/config.h Outdated Show resolved Hide resolved

sys/include/net/gnrc/tcp/config.h Outdated Show resolved Hide resolved

sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c Outdated Show resolved Hide resolved

leandrolanzieri added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels May 25, 2020

leandrolanzieri mentioned this pull request May 25, 2020

[CTF-tracker] Kconfig: Configuration exposure list #12888

Open

71 tasks

miri64 requested a review from brummer-simon May 25, 2020 11:44

brummer-simon suggested changes May 28, 2020

View reviewed changes

brummer-simon suggested changes May 29, 2020

View reviewed changes

sys/include/net/gnrc/tcp/config.h Show resolved Hide resolved

sys/net/gnrc/transport_layer/tcp/Kconfig Show resolved Hide resolved

brummer-simon approved these changes Jun 2, 2020

View reviewed changes

akshaim added 10 commits June 2, 2020 15:46

gnrc/tcp : Update documentation

b5e95f6

Add compile time parameters to doxygen group 'net_gnrc_conf'

gnrc/tcp : Move GNRC_TCP_CONNECTION_TIMEOUT_DURATION to 'CONFIG_'

01dd8e4

gnrc/tcp : Move GNRC_TCP_MSL to 'CONFIG_'

0b3e0de

gnrc/tcp : Move GNRC_TCP_MSS_MULTIPLICATOR to 'CONFIG_'

d014529

gnrc/tcp : Move GNRC_TCP_MSS to 'CONFIG_'

e8b13ab

gnrc/tcp : Move GNRC_TCP_RCV_BUFFERS to 'CONFIG_'

6482475

gnrc/tcp : Move GNRC_TCP_RTO_LOWER_BOUND to 'CONFIG_'

ea79c71

gnrc/tcp : Move GNRC_TCP_RTO_UPPER_BOUND to 'CONFIG_'

c599c65

gnrc/tcp : Move GNRC_TCP_RTO_GRANULARITY to 'CONFIG_'

f25c219

gnrc/tcp : Move GNRC_TCP_RTO_A_DIV to 'CONFIG_'

f704a1e

akshaim added 7 commits June 2, 2020 15:46

gnrc/tcp : Move GNRC_TCP_RTO_B_DIV to 'CONFIG_'

e42cc62

gnrc/tcp : Move GNRC_TCP_RTO_K to 'CONFIG_'

27f9d2d

gnrc/tcp : Move CONFIG_GNRC_TCP_PROBE_LOWER_BOUND to 'CONFIG_'

d690434

gnrc/tcp : Move GNRC_TCP_PROBE_UPPER_BOUND to 'CONFIG_'

49df225

gnrc/tcp : Move GNRC_TCP_DEFAULT_WINDOW to 'CONFIG_'

badeb74

gnrc/tcp : Expose to Kconfig

c191b00

Expose configurations to Kconfig

tests/gnrc_tcp/ : Set 'CONFIG_' if not Kconfig

611231c

Set CONFIG_GNRC_TCP_MSL and CONFIG_GNRC_TCP_CONNECTION_TIMEOUT_DURATION if not set via Kconfig

akshaim force-pushed the Kconfig_tcp branch from 629020e to 611231c Compare June 2, 2020 10:17

leandrolanzieri approved these changes Jun 2, 2020

View reviewed changes

leandrolanzieri added Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jun 2, 2020

leandrolanzieri added this to the Release 2020.07 milestone Jun 2, 2020

leandrolanzieri merged commit 7262b1d into RIOT-OS:master Jun 3, 2020

akshaim deleted the Kconfig_tcp branch June 3, 2020 06:48

Conversation

akshaim commented May 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Default State:

Firmware Output

Usage with menuconfig [default values]

Firmware Output

Usage with menuconfig

Firmware Output

Issues/PRs references

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brummer-simon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brummer-simon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

akshaim commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brummer-simon left a comment

Choose a reason for hiding this comment

Uh oh!

akshaim commented Jun 2, 2020

Uh oh!

leandrolanzieri commented Jun 2, 2020

Uh oh!

akshaim commented Jun 2, 2020

Uh oh!

leandrolanzieri left a comment

Choose a reason for hiding this comment

Uh oh!

akshaim commented Jun 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

akshaim commented May 24, 2020 •

edited

Loading

akshaim commented Jun 1, 2020 •

edited

Loading