Add protection against packet drift. #268
Conversation
- Compare timestamp delta with time of day delta to avoid drifting. - Do not update current time and do not sleep when we are late.
|
Thanks for the PR. I had a quick glance but will dig deeper when I get a chance. Have you tested at 10GigE + rates? My only concern is whether |
|
The change actually call gettimeofday() less often than before. So, it will result in a performance boost. I don't have a 10GigE NIC available to test, but I tested with a 1G NIC and small VoIP packets. Before the PR, tcpreplay was struggling to output ~350+Mbps in realtime. Now, tcpreplay is able to output ~600Mbps in realtime without any issue. I have not tested with higher bandwidth, but it should be able to drive at line rate if the processor running tcpreplay is strong enough. |
| @@ -69,7 +69,7 @@ extern tcpedit_t *tcpedit; | |||
| extern int debug; | |||
| #endif | |||
|
|
|||
| static void calc_sleep_time(tcpreplay_t *ctx, struct timeval *pkt_time, | |||
| static bool calc_sleep_time(tcpreplay_t *ctx, struct timeval *pkt_time, | |||
There was a problem hiding this comment.
There is no return value, so this should be void
There was a problem hiding this comment.
I added a return value to bypass the update of 'now' to reduce the number of call to gettimeofday()
There was a problem hiding this comment.
Ah, I see that now. Makes sense.
|
|
||
| /* | ||
| * Only sleep if we're not in top speed mode (-t) | ||
| * Only sleep if we're not in top speed mode (-t) or late |
There was a problem hiding this comment.
Yes, I rephrased the comment.
| timersub(&pkthdr.ts, &first_pkt_ts, &pkt_ts_delta); | ||
| if (calc_sleep_time(ctx, &pkt_ts_delta, &ctx->stats.last_time, pktlen, sp, packetnum, | ||
| &ctx->stats.end_time, &start_us, &skip_length)) | ||
| { |
There was a problem hiding this comment.
Slight request for style change to be consistent with project: brace moved up a line.
| @@ -806,35 +820,54 @@ send_dual_packets(tcpreplay_t *ctx, pcap_t *pcap1, int cache_file_idx1, pcap_t * | |||
| if (options->flow_stats && !options->file_cache[cache_file_idx].cached) | |||
| update_flow_stats(ctx, sp, pkthdr_ptr, pktdata, datalink); | |||
|
|
|||
| if (ctx->first_time) | |||
| { | |||
| timersub(&pkthdr_ptr->ts, &first_pkt_ts, &pkt_ts_delta); | ||
| if (calc_sleep_time(ctx, &pkt_ts_delta, &ctx->stats.last_time, pktlen, sp, packetnum, | ||
| &ctx->stats.end_time, &start_us, &skip_length)) | ||
| { |
There was a problem hiding this comment.
Same style change: move brace up a line
Hi,
Currently, the next packet sent time is based on the previous packet. When a lot of packets are sent, a small jitter may be inserted and all subsequent packet will have the same added jitter; tcpreplay will not recover from this jitter.
The proposed improvement add a comparison with the system time to recover from those timestamp drift. It also avoid updating the system time and sleeping when there is a jitter to recover from.
Regards.