Skip to content

net: Introduction of global protocol handler registry -> netreg (WIP)#2310

Closed
haukepetersen wants to merge 8 commits intoRIOT-OS:masterfrom
haukepetersen:add_netreg
Closed

net: Introduction of global protocol handler registry -> netreg (WIP)#2310
haukepetersen wants to merge 8 commits intoRIOT-OS:masterfrom
haukepetersen:add_netreg

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

This PR is opened for discussion and to demonstrate the concept of a global protocol handler/module registry that takes care or keeping lists of PID's.

This code is not compiling (I havn't even tried) and please don't spend much thought on the doxygen... I just want to get feedback on the overall concept.

The main things shown are:

  • API and implementation of a global module registry, called netreg
  • Introduction of a global list of used protocols, called netmod
  • Simplified netapi by throwing out everything related to (un-)registering
  • Adjusted pkt_t to make use of netmod
  • Simplified nomac by making use of netreg (and integrating some thoughts about netdev -> compare to net: minimization and simplification of netdev (WIP) #2297)

Let me know what you think or where I have more explaining to do!

@haukepetersen haukepetersen added Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Jan 15, 2015
@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 15, 2015

In general I like the idea, but I'm conflicted if

  • overhead might be increased by this
  • making the connection protocol <=> module is such a good idea (think dual-stack with cc3000 e. g.)

@haukepetersen
Copy link
Copy Markdown
Contributor Author

  • overhead might be increased by this

I think the contrary is true, as the network modules do not have to care about storing PID's anymore -> see removed code in netapi and nomac...

making the connection protocol <=> module is such a good idea (think dual-stack with cc3000 e. g.)

You have to think of protocol more as network module, so we could add a module CC3000_UDP or something.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 16, 2015

You have to think of protocol more as network module, so we could add a module CC3000_UDP or something

This will definitely increase code overhead. Instead of: "give me all UDP threads" it would be "give me UDP1, UDP2, UDP3" and ideally (not) with an humongous tree of #ifdefs which make the code just not fun anymore.

In general I see a lot of reintroduction and requirements of #ifdefs in your rework of the model (not only here). I was under the impression (and already stated this multiple times) however, that one of the major goals of the refactoring was to reduce them, since one of the lessons learned in the transceiver API was that they just don't scale and make the code a pain to read...

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 16, 2015

On a more constructive note: how about making the registry a list of (kernel_pid_t, kernel_pid_t, pkt_proto_t) tuples, where the first ist always the lower and the last always the upper layer thread. This way we identify implementations by their PIDs and specifications by an arbitrary number that is independent of the implementation as before.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 16, 2015

There is also the matter of demultipexing identifiers (next header identifiers in IPv6, Ports in UDP/TCP) that is missing from here.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

Hm, I see it completely on the contrary: For one I don't see any #ifdefs popping up, except on one single, central location, namely in netmod.h.

I also don't see the overhead: Why would you do something like: give me all UDP threads?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 17, 2015

Ok, UDP might have been a bad example, but take for example Broadcast in IP: "give me all MAC threads".

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 19, 2015

It's also still unclear to me: If I have a e.g. a cc110x device with a nomac control thread, would I choose NETMOD_CC110x or NETMOD_NOMAC. If the first is the case first, how would I than differentiate between two cc110x that would run in a setup, where they use to different MAC implementations (since we might want to run experiments like this, this might be a realistic scenario)? And if the latter is the case, how would I differentiate the radios, say on a gateway between a cc110x network with nomac and an IEEE 802.15.4 network with nomac?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 19, 2015

And how would I find out which address to use / which header (if we decide upon it) to use if the NETMOD_NOMAC option is true, or if the NETMOD_CC110x option is true, but the intermediate MAC layer has it's own header types.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

It's also still unclear to me: If I have a e.g. a cc110x device with a nomac control thread, would I choose NETMOD_CC110x or NETMOD_NOMAC

You wouldn't use either of them :-) As interfaces are are accessed by there interface identifier, thats what you use. So if you have a standard IP address you want to send to, you ask your neighbor table to give you the interface id for that device. You ask then the netreg for the PID of that interface. In case of multicast/broadcast, the neighbor/forwarding table gives you a list of interface ID's you can lookup.

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.

Would start with something like NETMOD_UNDEF for unidentified data. To save space in the lookup-array and to have the correct behaviour for NETMOD_NUMOF I would suggest:

typedef enum __attribute__((packed)) { // packed to make enum as small as possible
    NETMOD_UNDEF = -1,
#ifdef MODULE_UDP
    NETMOD_UDP,
#endif
/* … */
    NETMOD_NUMOF
}

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.

#ifdef + Typo

@haukepetersen
Copy link
Copy Markdown
Contributor Author

as I said, it's more or less pseudo-code. I will only start to make it work when we really decide to run with this concept...

@OlegHahm OlegHahm added NSTF and removed NSTF labels Feb 6, 2015
@miri64 miri64 modified the milestone: Network Stack Task Force Feb 8, 2015
@OlegHahm
Copy link
Copy Markdown
Member

Can we close this after #2404 got merged or do you plan to use this for a somehow optimized version of netreg?

@haukepetersen
Copy link
Copy Markdown
Contributor Author

this PR is it's obsolete.

@haukepetersen haukepetersen deleted the add_netreg branch February 14, 2015 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR 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.

3 participants