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

cpu/native: allow for multiple netdev2_tap devices #6311

Merged
merged 2 commits into from
Jan 19, 2017

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 10, 2017

Adaptation of #5614 with start-up sequence. Tested with gnrc_networking with

PORT="tapX tapY"
CFLAGS="-DNETDEV2_TAP_MAX=2 -DGNRC_NETIF_NUMOF=2"

Currently the initialization of the (absolute) number of devices is fixed to NETDEV2_TAP_MAX, but with a clean-up of the argument parsing in native we could fix that. I leave this to a follow-up PR.

@miri64 miri64 added Area: drivers Area: Device drivers Platform: native Platform: This PR/issue effects the native platform Area: network Area: Networking labels Jan 10, 2017
@miri64 miri64 added this to the Release 2017.01 milestone Jan 10, 2017
@miri64 miri64 force-pushed the native/enh/multi-tap branch 2 times, most recently from 1fd8a77 to 2ebe5ca Compare January 10, 2017 12:48
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 10, 2017
Copy link
Member

@LudwigKnuepfer LudwigKnuepfer left a comment

Choose a reason for hiding this comment

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

Native-wise just one minor issue :-)
I can not comment on netdev related changes.

@@ -0,0 +1,50 @@
/*
* Copyright (C) 2016 Freie Universität Berlin
Copy link
Member

Choose a reason for hiding this comment

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

Year has been updated upstream.

@@ -246,15 +247,15 @@ __attribute__((constructor)) static void startup(int argc, char **argv)

#if defined(MODULE_NETDEV2_TAP)
if (
(argc < 2)
(argc < (NETDEV2_TAP_MAX + 1)) /* one arg per tap + 0 for command */
Copy link
Member

Choose a reason for hiding this comment

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

This seems to apply the online help above needs updating as it currently says:

#if defined(MODULE_NETDEV2_TAP)
    real_printf(" <tap interface>");
#endif

Copy link
Member Author

Choose a reason for hiding this comment

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

(addressed)

bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
@kYc0o
Copy link
Contributor

kYc0o commented Jan 11, 2017

Tested on OS X and works correctly.

bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 11, 2017
@miri64
Copy link
Member Author

miri64 commented Jan 12, 2017

Addressed @LudwigKnuepfer's comments.

@miri64 miri64 added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 18, 2017
With arg added to async_read callback in 7020b7c, we don't need to keep
track of netdev2_tap locally. As a result we can use multiple
netdev2_tap instances.
@miri64
Copy link
Member Author

miri64 commented Jan 18, 2017

Rebased to current master (and squashed because unpicking my stuff and then repicking was annoying)

Copy link
Member

@LudwigKnuepfer LudwigKnuepfer left a comment

Choose a reason for hiding this comment

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

fine for native

@PeterKietzmann PeterKietzmann added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 19, 2017
@miri64
Copy link
Member Author

miri64 commented Jan 19, 2017

Fixed all issues reported by the CIs and squashed immediately

@miri64
Copy link
Member Author

miri64 commented Jan 19, 2017

Both CIs are happy :-)

@kYc0o
Copy link
Contributor

kYc0o commented Jan 19, 2017

Let's go then!

@kYc0o kYc0o merged commit 70fbcbf into RIOT-OS:master Jan 19, 2017
@miri64 miri64 deleted the native/enh/multi-tap branch January 19, 2017 18:04
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 20, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 20, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 20, 2017
bergzand added a commit to bergzand/RIOT that referenced this pull request Jan 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Platform: native Platform: This PR/issue effects the native platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants