Skip to content

WIP cpu/native: allow for multiple netdev2_tap devices#5614

Closed
toonst wants to merge 4 commits intoRIOT-OS:masterfrom
OTAkeys:pr/multiple_netdev2_tap
Closed

WIP cpu/native: allow for multiple netdev2_tap devices#5614
toonst wants to merge 4 commits intoRIOT-OS:masterfrom
OTAkeys:pr/multiple_netdev2_tap

Conversation

@toonst
Copy link
Copy Markdown
Member

@toonst toonst commented Jul 7, 2016

While implementing some changes in #5613, I saw that the netdev2_tap driver could make use of this change to allow multiple tap devices.
TODO: find some way to have a pointer to the right netdev2_tap device in startup.c

I'm not sure if this makes sense to go through with this, so comments very welcome.

Toon Stegen added 3 commits July 7, 2016 17:40
this makes it possible to pass some generic pointer that's given back as
an argument when the callback is called.
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.
no specific cleanup functions for uart and netdev_tap are needed
anymore, so reboot.c doesn't need a pointer to the netdev2_tap.
@miri64
Copy link
Copy Markdown
Member

miri64 commented Jul 7, 2016

It definitely makes sense! I wanted to do this month ago, but never found the time. Please go ahead!

@LudwigKnuepfer
Copy link
Copy Markdown
Member

I would like to encourage you to take a look at #5582 as it most likely has conflicts / opportunities for positive synergy effects ;-)
Maybe you and @alexaring can work together to combine your respective needs into one pull request. (Remember, you can create pull requests against a pull request..)

@toonst
Copy link
Copy Markdown
Member Author

toonst commented Jul 7, 2016

@LudwigKnuepfer Good idea, seems really similar to the netdev2_tap implementation indeed.

I'm wondering where this netdev2_tap should be defined. It should probably either be in the startup.c or in the auto_init_netdev2_tap.c. How do make sure it is accessible in both places? @miri64 What's the use case for multiple tap devices and how should we handle that?

@alexaring
Copy link
Copy Markdown

@toonst can you provide somehow that the tap interface can be optional? Or is this already included in this pull-request? So starting native riot without any tap device argument is possible?

Thanks. If your pull-request is fine, I will try to adapt your changes to #5582

@OlegHahm OlegHahm added Platform: native Platform: This PR/issue effects the native platform Area: network Area: Networking Area: drivers Area: Device drivers labels Aug 5, 2016
@miri64
Copy link
Copy Markdown
Member

miri64 commented Oct 17, 2016

Fixes #2195 btw.

@LudwigKnuepfer
Copy link
Copy Markdown
Member

Sorry, I don't have time to review this properly at the moment - removed my assignment.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Oct 28, 2016

I get

/home/mlenders/Repositories/RIOT-OS/RIOT/examples/gnrc_networking/bin/native/cpu/startup.o: In function `startup':
startup.c:(.text.startup+0x372): undefined reference to `netdev2_tap'
collect2: error: ld returned 1 exit status
/home/mlenders/Repositories/RIOT-OS/RIOT/examples/gnrc_networking/../../Makefile.include:259: recipe for target 'all' failed
make: *** [all] Error 1
make: Leaving directory '/home/mlenders/Repositories/RIOT-OS/RIOT/examples/gnrc_networking'

if I try to compile. Or is that what you mean by

TODO: find some way to have a pointer to the right netdev2_tap device in startup.c

@OlegHahm OlegHahm added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Oct 30, 2016
@toonst
Copy link
Copy Markdown
Member Author

toonst commented Nov 4, 2016

Sorry for not getting back earlier on this. I'm currently travelling without developer pc and will be for the next 10 months, so I won't be able to finish this timely.

@miri64 Yes, exactly. Before, netdev2_tap_t netdev2_tap; was defined in cpu/native/netdev2_tap/netdev2_tap.c +L 66. But since we want to be able to use multiple interfaces this doesn't seem to make sense anymore. What I did was put it in sys/auto_init/netif/auto_init_netdev2_tap.c, which is similar to other auto_init files, but then there is the problem that startup.c needs access to that variable (the linking problem you showed in previous port).
I hope it's more clear now.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 4, 2016

What I did was put it in sys/auto_init/netif/auto_init_netdev2_tap.c, which is similar to other auto_init files, but then there is the problem that startup.c needs access to that variable (the linking problem you showed in previous port).

I'm currently working on another networking device for native and came up with a solution I think, that would also make netdev2_tap more similar to the other devices: do not use the device itself in startup.c at all, just set the params struct.

Given that you said you won't be able to work on this the next year (or less ;-)): do you mind if I take over this one?

@toonst
Copy link
Copy Markdown
Member Author

toonst commented Nov 4, 2016

Hey @miri64, of course not, go ahead! Seems a clean solution to me.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 10, 2017

Adapted in #6311

@miri64 miri64 closed this Jan 10, 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 Platform: native Platform: This PR/issue effects the native platform State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants