Skip to content

cpu/native: allow for multiple netdev2_tap devices#6311

Merged
kYc0o merged 2 commits intoRIOT-OS:masterfrom
miri64:native/enh/multi-tap
Jan 19, 2017
Merged

cpu/native: allow for multiple netdev2_tap devices#6311
kYc0o merged 2 commits intoRIOT-OS:masterfrom
miri64:native/enh/multi-tap

Conversation

@miri64
Copy link
Copy Markdown
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 requested a review from LudwigKnuepfer January 10, 2017 12:47
@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
Copy Markdown
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
Copy Markdown
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.

#if defined(MODULE_NETDEV2_TAP)
if (
(argc < 2)
(argc < (NETDEV2_TAP_MAX + 1)) /* one arg per tap + 0 for command */
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 miri64 force-pushed the native/enh/multi-tap branch from 423174c to 5001ba2 Compare January 18, 2017 22:08
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 18, 2017

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

Copy link
Copy Markdown
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 miri64 force-pushed the native/enh/multi-tap branch from 5001ba2 to 92466e7 Compare January 19, 2017 10:56
@miri64 miri64 force-pushed the native/enh/multi-tap branch from 92466e7 to 1f6f02e Compare January 19, 2017 11:07
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 19, 2017

Fixed all issues reported by the CIs and squashed immediately

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 19, 2017

Both CIs are happy :-)

@kYc0o
Copy link
Copy Markdown
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