-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nanosec accurate packet processing #796
Nanosec accurate packet processing #796
Conversation
…me_timefunction Use clock_nanosleep with TIMER_ABSTIME, decide time function in use i…
Read pcap files with nanosec precision, set nano_sleep as default sle…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Will target for next major release.
src/common/timer.c
Outdated
} | ||
|
||
int get_current_time(struct timespec *ts){ | ||
#if defined _POSIX_C_SOURCE && _POSIX_C_SOURCE >= 199309L | ||
return clock_gettime(CLOCK_REALTIME, ts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't looked at this closely, but would this work with CLOCK_MONOTONIC
? It would not be susceptible to NTP, but would not work if real-time clocks are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using CLOCK_MONOTONIC is also possible, I fixed it in a new commit. I also had to modify gettimeofday_sleep function to work properly with CLOCK_MONOTONIC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the PR also using absolute time semantics for more precise sleeping, both CLOCK_MONOTONIC
and CLOCK_REALTIME
would work. However AFAIK POSIX only defines CLOCK_REALTIME
mandatory. Probably every sane OS implements both clocksource, but I think realtime chosen here because of portability concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll put it through it's paces when I start work on versin 4.5. I'll see if I can get my confidence up that it won't affect IoT and oldert stuff.
struct timeval tv; | ||
int success = gettimeofday(&tv, NULL); | ||
TIMEVAL_TO_TIMESPEC(&tv, ts); | ||
return success; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice that this supports older OS's. How much testing has there been on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comprehensive test on this is definitely missing, I made some user tests with basic setups + all the tests pass when make test is executed. However there are many include guards, with if, elseif etc... branches, they all need to be tested separately. Just to mention one, it wasn't tested how it works with netmap
…eep to work with CLOCK_MONOTONIC
* | ||
* ensure there is no overflow in cases where bits_sent is very high | ||
*/ | ||
if (bits_sent > COUNTER_OVERFLOW_RISK && bps > 500000) | ||
next_tx_us = (bits_sent * 1000) / bps * 1000; | ||
next_tx_ns = (bits_sent * 1000) / bps * 1000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will take some amount of testing. Some overflows take > 1 week to occur.
…_accurate_packet_processing
Merging to working branch |
d5c8596
into
appneta:Feature_#796_nanosecond_packet_processing
format commit with `git-clang-format HEAD~1`
* macOS was not reading nanosecond timer * get time called too often, causing overhead * fine tune gettimeofday_sleep() * adjusted overflow value to prevent issues with very long running instances * fix build/merge issues
* macOS was not reading nanosecond timer * get time called too often, causing overhead * fine tune gettimeofday_sleep() * adjusted overflow value to prevent issues with very long running instances * fix build/merge issues
* macOS was not reading nanosecond timer * get time called too often, causing overhead * fine tune gettimeofday_sleep() * adjusted overflow value to prevent issues with very long running instances * fix build/merge issues
* macOS was not reading nanosecond timer * get time called too often, causing overhead * fine tune gettimeofday_sleep() * adjusted overflow value to prevent issues with very long running instances * fix build/merge issues
…rocessing Feature #796 - nanosecond packet processing
Recently new version of tcpreplay was released[1]. This version now uses nanosecond timestamp precision[2][3] instead of microsecond precision. However, function datetime.datetime.fromisoformat() in Python up to 3.10 doesn't accept this format and fails with following error: tools/ft-orchestration/src/generator/tcpreplay.py:262: in stats start_time = datetime.datetime.fromisoformat(start_time) E ValueError: Invalid isoformat string: '1970-01-01 01:16:52.837054314' This commit fixes the issue by cropping nanosecond precision back to microsecond precision. Another solution would be to upgrade to Python 3.11, where fromisoformat() accepts nanosecond precision (even though it internally crops it back to microseconds). [1] https://github.com/appneta/tcpreplay/releases/tag/v4.5.1 [2] appneta/tcpreplay#796 [3] appneta/tcpreplay#863
The aim of this change is to make packet processing possible with nanosecond accuracy and to handle sub sleep times by only nanosleep()'ing.