On Thu, Jan 11, 2024 at 10:49 PM Ilya Maximets <[email protected]> wrote: > > On 1/10/24 20:29, Frode Nordahl wrote: > > At present the timeval timewarp functionality is tightly coupled > > with the unixctl interface for external operation by test suite. > > > > It is however desirable to make use of the timewarp functionality > > directly in unit tests. > > > > Split unixctl callback interface and the actual timewarp > > functionality into separate functions. > > > > This will be used in a patch that implements unit tests for the > > cooperative multitasking module. > > > > Signed-off-by: Frode Nordahl <[email protected]> > > --- > > lib/timeval.c | 56 ++++++++++++++++++++++++++++++++++++++++----------- > > lib/timeval.h | 4 ++++ > > 2 files changed, 48 insertions(+), 12 deletions(-) > > Hi, Frode. Thanks for the patch!
Thank you for taking the time to review, much appreciated! > I agree that idea is very useful for unit testing. Though, I'm not > sure we need to replicate the exact unixctl interface internally. > > The unixctl interface is designed around a server that calls > poll_block() and we also really want to allow other threads to do > some work within long warps, so we sleep and we have a stepping > interface that adjusts time in small chunks. > > I'm not sure if internal interface needs most of that, and I'm also > not sure if it is actually easy to take advantage of such functionality > from within the process. What I'm trying to say is that the program > that is using this interface is managing the threads, and it likely > knows when exectly it needs the time to be warped without relying on > that to be done by the poll_block(). > > So, I think, a much simpler API might be better here. We can have > timeval_stop(void) that stops the timer, i.e. directs it into slow path, > and timeval_warp(long long int) that directly advances the time, i.e. > just grabs the mutex, converts the argument into timeval and calls > timeval_add(). I agree, in v1 I was mostly concerned about surfacing the feature for the test with as little change and as much reuse as possible. Adding a new function that advances the time directly does not duplicate too much and allows us to do away with the ugly pointer hack. > In a worst case, if someone will actually need to advance time > gradually in the code, they can always just call timeval_warp() > before they call poll_block(). > > The multi-threading support can be simplified down to adding > seq_change(timewarp_seq); into timeval_warp() and adding one more > function timewarp_wait() that waits on it. It seems reasonable to > expect multi-threaded test applications to call that before > poll_block(). But it is not even necessary to have for tests > introduced in this patch set, so may be delayed until such > fucntionality is actually needed. > > What do you think? No immediate need for multi threaded support for this, so it makes sense to me to expose a simpler interface. Thanks! -- Frode Nordahl > Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
