Fix struct timeval units in DCE poll #94
Conversation
tomhenderson
force-pushed
the
fix-dce-poll
branch
from
843e856 to
ebc51b8
Compare
| @@ -36,7 +36,7 @@ int dce_poll (struct pollfd *fds, nfds_t nfds, int timeout) | |||
| } | |||
| else if (timeout > 0) | |||
| { | |||
| endtime = Now () + MilliSeconds (timeout); | |||
| endtime = Now () + MicroSeconds (timeout); | |||
There was a problem hiding this comment.
the manpage does say it should be Milliseconds so I believe this change is incorrect
There was a problem hiding this comment.
Good catch, I will ask Trish to weigh in about this. It would be nice to have some test code..
| @@ -208,7 +208,7 @@ int dce_select (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, | |||
|
|
|||
| if (timeout) | |||
| { | |||
| pollTo = timeout->tv_sec * 1000 + timeout->tv_usec / 1000; | |||
| pollTo = timeout->tv_sec * 1000000 + timeout->tv_usec / 1000; | |||
There was a problem hiding this comment.
I believe we should make the minimum resolution in UtilsAdvanceTime a global one
current->process->manager->Wait (Time (MicroSeconds (1))); and either round or warn if this value is smaller than the minimum resolution. Would that solve the issue ?
There was a problem hiding this comment.
I am now just reading the thread carefully for the first time. I had some off list discussion with Trish and she does not have a small testcase to contribute.
It seems like there are two issues. First, when dce_select() calls dce_poll() with a non-zero timeout but less than 1ms, and second, when some application calls poll() with timeout of zero (and there needs to be a forced timestep to prevent infinite looping).
For the second issue, if I understand correctly, Matt is proposing to pull out this magic value into a global so that it could be tuned by users more easily, and also round up to 1 timestep if the time set in the global is less than the resolution?? (e.g. if simulation timestep is 1ns and user configures the magic value to picoseconds)
For the first issue, it seems like the issue is when dce_select calls dce_poll but wants sub-millisecond resolution. Here, I suggest possibly to create a dce_poll_internal method that replaces 'int timeout' with 'ns3::Time timeout', and have dce_select call this variant directly, and have the existing dce_poll() forward to this internal dce_poll_internal().
I would like to request a merge of this patch from Trish Deutsch, fixing this issue (option 2 in the discussion):
https://groups.google.com/forum/#!msg/ns-3-users/YhAcJh3Fv3U/wN-NlQw4BQAJ;context-place=forum/ns-3-users