Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netdev_tap: don't allocate DEBUG_EXTRA_STACKSIZE twice #12699

Merged
merged 1 commit into from
Nov 16, 2019

Conversation

basilfx
Copy link
Member

@basilfx basilfx commented Nov 12, 2019

Contribution description

While browsing some code, I noticed that DEBUG_EXTRA_STACKSIZE is used at two places in the auto initialization of netdev_tap. I presume that the second time it was used incorrectly, because when creating the thread, it specified the stack size TAP_MAC_STACKSIZE.

Testing procedure

I don't have het tap device set up on macOS, but I don't think this change impacts anything, except reducing the actual allocated stack size.

Issues/PRs references

None

@basilfx basilfx added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform labels Nov 12, 2019
@basilfx basilfx changed the title netdev: don't allocate DEBUG_EXTRA_STACKSIZE twice netdev_tap: don't allocate DEBUG_EXTRA_STACKSIZE twice Nov 12, 2019
@MrKevinWeiss MrKevinWeiss requested a review from miri64 November 15, 2019 09:54
@MrKevinWeiss
Copy link
Contributor

Master:

   text	   data	    bss	    dec	    hex	filename
 241512	   1140	 102696	 345348	  54504	/data/riotbuild/riotbase/examples/gnrc_networking/bin/native/gnrc_networking.elf

PR:

   text	   data	    bss	    dec	    hex	filename
 241508	   1140	 102696	 345344	  54500	/data/riotbuild/riotbase/examples/gnrc_networking/bin/native/gnrc_networking.elf

I was able to ping6 the tapbr from native in both cases.

@miri64 Does this make sense?

@MrKevinWeiss MrKevinWeiss added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Nov 15, 2019
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 16, 2019
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

TAP_MAC_STACKSIZE is #define TAP_MAC_STACKSIZE (THREAD_STACKSIZE_DEFAULT + DEBUG_EXTRA_STACKSIZE).

@kaspar030 kaspar030 merged commit 61c6b07 into RIOT-OS:master Nov 16, 2019
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
@basilfx basilfx deleted the feature/netdev_tap_stack branch October 20, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Reviewed: 3-testing The PR was tested according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants