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

netdev: introduce additional flags #3632

Closed
wants to merge 2 commits into from

Conversation

OlegHahm
Copy link
Member

A first use of these flags is introduced to indicate the interface as being wireless which might be a useful information for upper layers.

A first use of these flags is introduced to indicate the interface as
being wireless which might be a useful information for upper layers.
@OlegHahm OlegHahm added Area: network Area: Networking Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Aug 14, 2015
@OlegHahm
Copy link
Member Author

I'm not happy to introduce an additional field in netdev_t, but I couldn't think of another solution. The information if an interface is wireless or not is static and using NETOPT seemed pointless to me.

@kaspar030
Copy link
Contributor

Do you need this for something right now? I'd rather integrate this into netdev2.

@OlegHahm
Copy link
Member Author

It's needed for a new version of #3622, i.e. doing NDP properly for wireless medium,

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

I'd prefer to stay with netopt to keep the interface clean.

@OlegHahm
Copy link
Member Author

And the change itself is (as you can see) super small. So, integrating this in netdev2 should be easy.

@OlegHahm
Copy link
Member Author

IMO using netopt is completely overkill here.

@kaspar030
Copy link
Contributor

@OlegHahm agreed, the change is small, so I don't mind.

@OlegHahm
Copy link
Member Author

(And it would be a misuse, since being wireless is not an option or anything configurable.)

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

I'm still somewhat confused about why we need that however. WiFi is on IP level not seen as wireless and the rest of the radios we aim to support use 6LoWPAN. 6LoWPAN uses its own ND (which is not merged yet) so why do we need it for #3622

@OlegHahm
Copy link
Member Author

WiFi is on IP level not seen as wireless

Why not?

@kaspar030
Copy link
Contributor

@OlegHahm because it does magic to behave like a wire.

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

Then how do you get the information to the upper layer, when not using netopt? netapi has no other means to carry information to other layers.

@OlegHahm
Copy link
Member Author

But in reality it does not behave like Ethernet and RFC5889 applies (actually, I guess that it has been written with specifically 802.11 in mind).

@OlegHahm
Copy link
Member Author

Then how do you get the information to the upper layer, when not using netopt? netapi has no other means to carry information to other layers.

Will introduce this in a follow-up PR as an additional flag in netif.

@OlegHahm
Copy link
Member Author

Oops, forgot to commit ng_netdev.h

@OlegHahm
Copy link
Member Author

Hm, actually turned out to be more complicated since getting static information from the interface seems to be not foreseen in the stack...

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

Yes, that's what I mean't. We really need to think if we make such exceptions to the API because it is 'overkill'. IMHO your flag solution is the right way to store the information on the device (I would however use the device's private flags and not introduce a new field to netdev). But we should use netopt regardless to get the information to the upper layer.

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

Otherwise, we get quickly into situations we had in the old stack, where every layer does it's own thing and nobody has any clue how to fix stuff.

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

(Because it is overkill to have a netopt for every flag, I argued month ago to have a NETOPT_FLAGS value for that and add the operations add/remove to the interfaces, when the interface was still in development, but that was voted down, so I guess we have to live with this situation now).

@OlegHahm
Copy link
Member Author

I partly agree, but I'm still reluctant to go through netopt only to retrieve static information from the device.

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

We do it for the frame length too. So the harm is already done ;-P

@miri64
Copy link
Member

miri64 commented Aug 14, 2015

Oh I was mistaken, you don't need to store it dev internally if you go over netopt. You can just return NETOPT_ENABLE/NETOPT_DISABLE when using it.

@@ -40,6 +40,11 @@ extern "C" {
#define NG_NETDEV_MSG_TYPE_EVENT (0x0100)

/**
* @brief Flag for indicating a wireless network interface
Copy link
Member

Choose a reason for hiding this comment

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

Could you also use @name with an appropriate description and wrap the #define in @{ .. @}?

@thomaseichinger
Copy link
Member

I support @authmillenon on this, it should be done using netapi. The interface wont most probably not change so this shouldn't get fetched too often.

Anyway, the flag is not set for the xbee driver.

@OlegHahm
Copy link
Member Author

Ok, I try to come up with a better approach - in the worst case misusing netopt.

@OlegHahm OlegHahm closed this Aug 19, 2015
@OlegHahm OlegHahm mentioned this pull request Aug 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants