tinyos / tinyos-main

Main development repository for TinyOS (an OS for embedded, wireless devices).
1.41k stars 519 forks source link

Random.rand* should not be called in Init.init() #225

Open minderdl opened 11 years ago

minderdl commented 11 years ago

The initial seed of the random number generator (both RandomLfsrC and RandomMlcgC) is set in their Init.init() command. However, it cannot be assured that this init happens before the init of other components.

For example, when I compile RadioCountToLeds (with TinyOS 2.1.1 and nesC 1.3.4) I see the following in the resulting app.c:

inline static error_t RealMainP__SoftwareInit__init(void ){
...
  __nesc_result = ecombine(__nesc_result, UniqueSendP__Init__init());
  __nesc_result = ecombine(__nesc_result, RandomMlcgC__Init__init());

However, chips/cc2420/unique/UniqueSendP Init.init() sets the starting DSN by calling Random.rand16(), which is, therefore, always 0! This causes problems especially with simulations.

In the case of UniqueSendP this can be changed to

localSendId = TOS_NODE_ID << 4;

as in rfxlink.

However, I think this is a more general problem that needs to be analyzed further.

phil-levis commented 11 years ago

There does seem to be a problem of sloppy use of Init. TEP 107 reads (unfortunately, in Section 4.2, which is about Interrupts, although the statement is more general than that):

"If a component A depends on another component, B, which needs to be initialized, then A SHOULD wire B's Init directly to the boot sequence, unless there is a temporal ordering requirement to the initialization. The purpose of this convention is to simplify component initialization and the initialization sequence."

In this case, if UniqueSendP is calling Random, then it should have explicitly initialized Random to make sure it is ready.

Phil


Philip Levis Associate Professor Computer Science and Electrical Engineering Stanford University http://csl.stanford.edu/~pal

On Jul 25, 2013, at 2:57 AM, minderdl notifications@github.com wrote:

The initial seed of the random number generator (both RandomLfsrC and RandomMlcgC) is set in their Init.init() command. However, it cannot be assured that this init happens before the init of other components.

For example, when I compile RadioCountToLeds (with TinyOS 2.1.1 and nesC 1.3.4) I see the following in the resulting app.c:

inline static error_t RealMainPSoftwareInitinit(void ){ ... nesc_result = ecombine(__nesc_result, UniqueSendPInitinit()); nesc_result = ecombine(__nesc_result, RandomMlcgCInitinit());

However, UniqueSendP Init.init() sets the starting DSN by calling Random.rand16(), which is, therefore, always 0! This causes problems especially with simulations.

In the case of UniqueSendP this can be changed to

localSendId = TOS_NODE_ID << 4;

as in rfxlink.

However, I think this is a more general problem that needs to be analyzed further.

— Reply to this email directly or view it on GitHub.

phil-levis commented 11 years ago

That being said, given that Random is kind of special (and how often it's used in initialization), I don't think it would be unreasonable to wire it through HardwareInit. I tried to think through simple solutions for something like UniqueSend to handle the ordering, and all of them have issues. For example, having UniqueSend initialize Random itself has problems when multiple components do so -- they will all generate the same root random value. Trying to conditionally invoke random requires more RAM (or the possibility of repeated values).

cire831 commented 11 years ago

On Thu, Aug 8, 2013 at 9:22 PM, phil-levis notifications@github.com wrote:

That being said, given that Random is kind of special (and how often it's used in initialization), I don't think it would be unreasonable to wire it through HardwareInit.

HardwareInit doesn't appear to currently exist. And strictly speaking initilizing Random is a software issue. It isn't very clean or straight forward initializing a software thing from a freshly created thing that appears to be about h/w.

Are you thinking that HardwareInit would be invoked after PlatformInit and prior to SoftwareInit in RealMain?

Perhaps calling it SystemInit and its usage is restricted (ie. you really need to know what you are doing to hook something into it).

It would be invoked after PlatformInit prior to SoftwareInit.

We can initilize Random there as well as hooking in the arbiter initilization if we deem that is a reasonable thing.

In general a two level Software initilization mechanism seems like a reasonable thing. Its what other operating systems need to do. There are seperate initilization sequences for system level services and then initilization for stuff that use those services.

Yes I realize no matter what we select there are potential sequencing problems.

But going to a two level, first SystemInit, then SoftwareInit for other stuff, seems like a movement in the right direction.

I tried to think through simple solutions for something like UniqueSend to

handle the ordering, and all of them have issues. For example, having UniqueSend initialize Random itself has problems when multiple components do so -- they will all generate the same root random value. Trying to conditionally invoke random requires more RAM (or the possibility of repeated values).

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/tinyos-main/issues/225#issuecomment-22374678 .

Eric B. Decker Senior (over 50 :-) Researcher

phil-levis commented 11 years ago

Sorry, by HardwareInit I meant PlatformInit. I don't think adding more init levels is a good solution to solve this narrow problem.

Phil


Philip Levis Associate Professor Computer Science and Electrical Engineering Stanford University http://csl.stanford.edu/~pal

On Aug 9, 2013, at 9:26 AM, Eric Decker notifications@github.com wrote:

On Thu, Aug 8, 2013 at 9:22 PM, phil-levis notifications@github.com wrote:

That being said, given that Random is kind of special (and how often it's used in initialization), I don't think it would be unreasonable to wire it through HardwareInit.

HardwareInit doesn't appear to currently exist. And strictly speaking initilizing Random is a software issue. It isn't very clean or straight forward initializing a software thing from a freshly created thing that appears to be about h/w.

Are you thinking that HardwareInit would be invoked after PlatformInit and prior to SoftwareInit in RealMain?

Perhaps calling it SystemInit and its usage is restricted (ie. you really need to know what you are doing to hook something into it).

It would be invoked after PlatformInit prior to SoftwareInit.

We can initilize Random there as well as hooking in the arbiter initilization if we deem that is a reasonable thing.

In general a two level Software initilization mechanism seems like a reasonable thing. Its what other operating systems need to do. There are seperate initilization sequences for system level services and then initilization for stuff that use those services.

Yes I realize no matter what we select there are potential sequencing problems.

But going to a two level, first SystemInit, then SoftwareInit for other stuff, seems like a movement in the right direction.

I tried to think through simple solutions for something like UniqueSend to

handle the ordering, and all of them have issues. For example, having UniqueSend initialize Random itself has problems when multiple components do so -- they will all generate the same root random value. Trying to conditionally invoke random requires more RAM (or the possibility of repeated values).

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/tinyos-main/issues/225#issuecomment-22374678 .

Eric B. Decker Senior (over 50 :-) Researcher — Reply to this email directly or view it on GitHub.

phil-levis commented 10 years ago

I'd like to suggest moving Random's init into platform init. Any objections?

cire831 commented 10 years ago

+1

On Mon, Jun 16, 2014 at 7:32 PM, Philip Levis notifications@github.com wrote:

I'd like to suggest moving Random's init into platform init. Any objections?

— Reply to this email directly or view it on GitHub https://github.com/tinyos/tinyos-main/issues/225#issuecomment-46261222.

Eric B. Decker Senior (over 50 :-) Researcher

vlahan commented 10 years ago

+1

--Vlado On Jun 17, 2014 6:54 AM, "Eric Decker" notifications@github.com wrote:

+1

On Mon, Jun 16, 2014 at 7:32 PM, Philip Levis notifications@github.com wrote:

I'd like to suggest moving Random's init into platform init. Any objections?

— Reply to this email directly or view it on GitHub https://github.com/tinyos/tinyos-main/issues/225#issuecomment-46261222.

Eric B. Decker Senior (over 50 :-) Researcher

— Reply to this email directly or view it on GitHub https://github.com/tinyos/tinyos-main/issues/225#issuecomment-46267159.