Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 18, 2017

Adaptation of @kaspar030's proposal in #5511.

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jan 18, 2017
@miri64 miri64 force-pushed the netif/api/initial branch from 9a9c9c3 to 81c104e Compare May 16, 2017 11:27
@miri64
Copy link
Member Author

miri64 commented May 16, 2017

@kaspar030 I added the params-structs for the auto-initialization of netif to this PR, since I think it fits in here.

@miri64 miri64 force-pushed the netif/api/initial branch from 81c104e to e032645 Compare May 23, 2017 14:37
@miri64
Copy link
Member Author

miri64 commented May 23, 2017

Rebased to current master and refined documentation

@miri64 miri64 force-pushed the netif/api/initial branch from e032645 to b217fbe Compare May 23, 2017 14:39
Simplify netif_type and start skeleton implementation
*/

/**
* @defgroup Network interfaces
Copy link
Member

Choose a reason for hiding this comment

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

should be @defgroup net_netif Network Interfaces?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right 😄

@miri64
Copy link
Member Author

miri64 commented May 24, 2017

I hit a wall when trying to port the auto-initialization for GNRC... (mainly questions like: "Where is the thread's memory stack stored and how is its size determined?" and "Where is the adapter struct stored?") Need to think about this a bit more :-/. Input is welcome.

@miri64
Copy link
Member Author

miri64 commented Jun 1, 2017

"Where is the adapter struct stored?"

For this question I basically have an answer (and had when I stated that question) already in my back-hand an answer, but this is related to my over-all GNRC overhaul.

*/
const void *dev_params;
netif_type_t type; /**< type of the interface */
const char *name; /**< name of the interface */
Copy link
Member

@smlng smlng Jun 6, 2017

Choose a reason for hiding this comment

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

is it necessary to have the name attached to every device?

I was thinking of something like a common prefix for certain netif_type_t types and let the system number them starting with 0. (Or instead of const char *name have a uint8_t name_prefix.) And then have map from type (or name_prefix ) to a char prefix (without a number), e.g. "eth" or "wpan" or maybe "gnrc"?

I hope you get what I mean 😕 ❓

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi, I think that makes the initialization just more complicated, because then we need to keep track of how many interfaces are already initialized. The auto-initialization is already pretty complicated so let's not make things more complicated for a totally optional thing for most stacks (the only stack actually requiring a name at the moment is lwIP).

@kaspar030
Copy link
Contributor

Can you please elaborate what you intend to do with this interface?
Is this for auto-initialization only?
It seems a little odd to have a package dependent define in pretty central code.

@miri64
Copy link
Member Author

miri64 commented Jun 7, 2017

Can you please elaborate what you intend to do with this interface?
Is this for auto-initialization only?

For now I am only planing auto-initialization, yes. But long-term plans include a generalized ifconfig command (that's why netif_g/set_opt() exist) and maybe even further down the line sock_netif. But before any of that I'd rather want to clean-up GNRC's network interfaces (for which I do not need this necessarily).

It seems a little odd to have a package dependent define in pretty central code.

What define are you talking about?

@kaspar030
Copy link
Contributor

Maybe we should define first what needs to be configurable, find a nice way of representing that, and then code the initialization around that?

It seems a little odd to have a package dependent define in pretty central code.
What define are you talking about?

You've got #if defined(GNRC), #if defined(MODULE_LWIP_ETHERNET). It is certainly possible to do this without those defines.

IMO it would make sense to keep configuration within the stacks, come up with a way to configure which stack uses which network device, and let them handle it as desired.

@miri64
Copy link
Member Author

miri64 commented Jun 7, 2017

You've got #if defined(GNRC), #if defined(MODULE_LWIP_ETHERNET). It is certainly possible to do this without those defines.

I think these can be removed, the number in the enum is all what matters after all.

IMO it would make sense to keep configuration within the stacks, come up with a way to configure which stack uses which network device, and let them handle it as desired.

This is what this PR is about and that's exactly what I do in #7144.

@miri64
Copy link
Member Author

miri64 commented Jun 7, 2017

I think these can be removed, the number in the enum is all what matters after all.

Done

@kaspar030
Copy link
Contributor

I think these can be removed, the number in the enum is all what matters after all.

It's not about the define, it's about the dependency. As is, you'd need to edit netif if you'd like to use a previously unknown stack. That is unnecessary.

For auto-init, a shared initialization function and parameter definition just don't make sense anyways.

@miri64
Copy link
Member Author

miri64 commented Jun 7, 2017

It's not about the define, it's about the dependency. As is, you'd need to edit netif if you'd like to use a previously unknown stack. That is unnecessary.

As is it is, because currently the auto-initialization is per device, not per stack.

For auto-init, a shared initialization function and parameter definition just don't make sense anyways.

Another approach I was thinking about would be put all initialized netdevs into a linked list, and then iterating over that for a stack-specific initialization. But that would mean sizeof(netdev_t *) extra byte for each netdev (currently) just for initialization.

@aabadie aabadie added this to the Release 2017.10 milestone Jun 23, 2017
@aabadie
Copy link
Contributor

aabadie commented Jun 23, 2017

I fear it won't make it for the release, postponed to 2017.10.

@miri64
Copy link
Member Author

miri64 commented Oct 5, 2017

Anyone care to review?

@smlng
Copy link
Member

smlng commented Nov 17, 2017

@miri64 I need some help/recap on this one: is it still on? And if so how does it fit with netif2?

@miri64
Copy link
Member Author

miri64 commented Nov 17, 2017

@miri64 I need some help/recap on this one: is it still on? And if so how does it fit with netif2?

It has nothing to do with gnrc_ netif2! This is a network-stack-independent API to network interfaces. After #7925 was merged this needs to be adapted, but for now: nothing here to see ;-).

@smlng
Copy link
Member

smlng commented Nov 17, 2017

thanks for resetting my mind 😄 So this could (or better should) wait until #7925 is merge anyways?

@miri64
Copy link
Member Author

miri64 commented Nov 17, 2017

thanks for resetting my mind 😄 So this could (or better should) wait until #7925 is merge anyways?

To quote my self "After #7925 was merged this needs to be adapted" ;-)

@smlng
Copy link
Member

smlng commented Nov 17, 2017

okay I think now I got it 😉

@miri64
Copy link
Member Author

miri64 commented Nov 17, 2017

For reference: since that is automated and I add all PRs related to #7925 to that project: https://github.com/RIOT-OS/RIOT/projects/6 gives you all related PRs (but ignore the TODO column, those are mostly notes for myself)

@smlng
Copy link
Member

smlng commented Dec 15, 2017

@miri64 any progress after #7925 is merged for some time now?

@miri64
Copy link
Member Author

miri64 commented Dec 18, 2017

No, but I'd like to redo this in a 2-phase approach.

  1. provide a common API
  2. provide a common initialization API.

@miri64
Copy link
Member Author

miri64 commented Mar 27, 2018

Let's clean this up! I'm going back to the roots in #8841.

@miri64 miri64 closed this Mar 27, 2018
@miri64 miri64 deleted the netif/api/initial branch March 27, 2018 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants