Hey Yamamoto, You're right, this is too fragile. This patch I just posted checks the system's platform, so we don't run into situations like this (defaults to using time.time() if its not Linux, NetBSD or FreeBSD). It also adds CLOCK_MONOTONIC compatibility for NetBSD and FreeBSD, as well as updates the msec() doctoring.
http://patchwork.openvswitch.org/patch/4365/ Thanks for the review! Ryan On 6/2/14 8:04 PM, "YAMAMOTO Takashi" <yamam...@valinux.co.jp> wrote: >hi, > >> Hey Yamamoto, >> >> Sorry for the delay, I finally set up my NetBSD environment and tested >> this script. You're right, but NetBSD will actually return an error on >> clock_gettime because of this argument difference. Thus, NetBSD will >> continue to use time.time(), which is as expected. > >it returns an error because of a different value of CLOCK_MONOTONIC. >(it's 3 on NetBSD) > >however, it's too fragile to pass "random" arguments to a system call >and expect it detect and return errors. >how about having an explicit platform check? > >> >> I can add NetBSD functionality such that clock_gettime with >> CLOCK_MONOTONIC can also be used for NetBSD as well. Lemme know! > >thanks. please if you can. otherwise i can take a look later. > >btw, docstring of msec() needs an update. > >YAMAMOTO Takashi > >> >> Ryan >> >> On 5/31/14 11:14 PM, "YAMAMOTO Takashi" <yamam...@valinux.co.jp> wrote: >> >>>> Python's time.time() function uses the system wall clock. However, >>>> if NTP resets the wall clock to be a time in the past, then this >>>> causes any applications that poll block based on time, such as >>>> ovs-xapi-sync, to poll block indefinitely since the time is >>>> unexpectedly negative. >>>> >>>> This patch fixes the issue by using time.monotonic() if Python's >>>> version >= 3.3. Otherwise, the timeval module calls out to the >>>> librt C shared library and uses the clock_gettime function with >>>> CLOCK_MONOTONIC. >>>> >>>> Note this is only enabled on Linux-based platforms. This has been >>>> tested on Ubuntu 12.04 and Redhat 6.4. >>>> >>>> Bug #1189434 >>>> Signed-off-by: Ryan Wilson <wr...@nicira.com> >>>> >>>> --- >>>> v2: Pre-load librt.so.1 library instead of loading at every function >>>> call. Use only CLOCK_MONOTONIC for consistency with OVS C library. >>>> v3: Edit commit message, remove if-linux check >>>> --- >>>> python/ovs/timeval.py | 34 +++++++++++++++++++++++++++++++++- >>>> 1 file changed, 33 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/python/ovs/timeval.py b/python/ovs/timeval.py >>>> index ba0e54e..f2681ac 100644 >>>> --- a/python/ovs/timeval.py >>>> +++ b/python/ovs/timeval.py >>>> @@ -12,13 +12,45 @@ >>>> # See the License for the specific language governing permissions and >>>> # limitations under the License. >>>> >>>> +import ctypes >>>> +import sys >>>> import time >>>> >>>> +LIBRT = 'librt.so.1' >>>> +CLOCK_MONOTONIC = 1 >>>> + >>>> +class timespec(ctypes.Structure): >>>> + _fields_ = [ >>>> + ('tv_sec', ctypes.c_long), >>> >>>on NetBSD, timespec.tv_sec is int64_t, not long. >>> >>>YAMAMOTO Takashi >>> >>>> + ('tv_nsec', ctypes.c_long), >>>> + ] >>>> + >>>> +try: >>>> + librt = ctypes.CDLL(LIBRT) >>>> + clock_gettime = librt.clock_gettime >>>> + clock_gettime.argtypes = [ctypes.c_int, ctypes.POINTER(timespec)] >>>> +except: >>>> + # Librt shared library could not be loaded >>>> + librt = None >>>> + >>>> +def monotonic(): >>>> + if not librt: >>>> + return time.time() >>>> + >>>> + t = timespec() >>>> + if clock_gettime(CLOCK_MONOTONIC, ctypes.pointer(t)) == 0: >>>> + return t.tv_sec + t.tv_nsec * 1e-9 >>>> + # Kernel does not support CLOCK_MONOTONIC >>>> + return time.time() >>>> + >>>> +# Use time.monotonic() if Python version >= 3.3 >>>> +if not hasattr(time, 'monotonic'): >>>> + time.monotonic = monotonic >>>> >>>> def msec(): >>>> """Returns the current time, as the amount of time since the >>>>epoch, in >>>> milliseconds, as a float.""" >>>> - return time.time() * 1000.0 >>>> + return time.monotonic() * 1000.0 >>>> >>>> >>>> def postfork(): >>>> -- >>>> 1.7.9.5 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> >>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman >>/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbff >>g%3D%3D%0A&m=MJk53HQEXy0BYs2lRyZq%2FxEzIVWI7tmM1JcuHLrWXmA%3D%0A&s=2f4db1 >>f3f3813411f90a41b9023a389d0de5347fd5b7af65b2eafa8849314d18 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
