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

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

Closed
wants to merge 4 commits into from

Conversation

toonst
Copy link
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
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
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
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

@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
Member

miri64 commented Oct 17, 2016

Fixes #2195 btw.

@LudwigKnuepfer
Copy link
Member

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

@miri64
Copy link
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
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
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
Member Author

toonst commented Nov 4, 2016

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

@miri64
Copy link
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