[PATCH] pktgen: document 32-bit timestamp overflow

[PATCH] pktgen: document 32-bit timestamp overflow

[Arnd Bergmann]

Timestamps in pktgen are currently retrieved using the deprecated
do_gettimeofday() function that wraps its signed 32-bit seconds in 2038
(on 32-bit architectures) and requires a division operation to calculate
microseconds.

The pktgen header is also defined with the same limitations, hardcoding
to a 32-bit seconds field that can be interpreted as unsigned to produce
times that only wrap in 2106. Whatever code reads the timestamps should
be aware of that problem in general, but probably doesn't care too
much as we are mostly interested in the time passing between packets,
and that is correctly represented.

Using 64-bit nanoseconds would be cheaper and good for 584 years. Using
monotonic times would also make this unambiguous by avoiding the overflow,
but would make it harder to correlate to the times with those on remote
machines. Either approach would require adding a new runtime flag and
implementing the same thing on the remote side, which we probably don't
want to do unless someone sees it as a real problem. Also, this should
be coordinated with other pktgen implementations and might need a new
magic number.

For the moment, I'm documenting the overflow in the source code, and
changing the implementation over to an open-coded ktime_get_real_ts64()
plus division, so we don't have to look at it again while scanning for
deprecated time interfaces.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/core/pktgen.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 6c5763b4411c..f95a15086225 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2711,7 +2711,7 @@ static inline __be16 build_tci(unsigned int id, unsigned int cfi,
 static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
 				int datalen)
 {
-	struct timeval timestamp;
+	struct timespec64 timestamp;
 	struct pktgen_hdr *pgh;
 
 	pgh = skb_put(skb, sizeof(*pgh));
@@ -2773,9 +2773,17 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
 		pgh->tv_sec = 0;
 		pgh->tv_usec = 0;
 	} else {
-		do_gettimeofday(&timestamp);
+		/*
+		 * pgh->tv_sec wraps in y2106 when interpreted as unsigned
+		 * as done by wireshark, or y2038 when interpreted as signed.
+		 * This is probably harmless, but if anyone wants to improve
+		 * it, we could introduce a variant that puts 64-bit nanoseconds
+		 * into the respective header bytes.
+		 * This would also be slightly faster to read.
+		 */
+		ktime_get_real_ts64(&timestamp);
 		pgh->tv_sec = htonl(timestamp.tv_sec);
-		pgh->tv_usec = htonl(timestamp.tv_usec);
+		pgh->tv_usec = htonl(timestamp.tv_nsec / NSEC_PER_USEC);
 	}
 }
 
-- 
2.9.0

Re: [PATCH] pktgen: document 32-bit timestamp overflow

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue,  7 Nov 2017 11:38:32 +0100

> Timestamps in pktgen are currently retrieved using the deprecated
> do_gettimeofday() function that wraps its signed 32-bit seconds in 2038
> (on 32-bit architectures) and requires a division operation to calculate
> microseconds.
> 
> The pktgen header is also defined with the same limitations, hardcoding
> to a 32-bit seconds field that can be interpreted as unsigned to produce
> times that only wrap in 2106. Whatever code reads the timestamps should
> be aware of that problem in general, but probably doesn't care too
> much as we are mostly interested in the time passing between packets,
> and that is correctly represented.
> 
> Using 64-bit nanoseconds would be cheaper and good for 584 years. Using
> monotonic times would also make this unambiguous by avoiding the overflow,
> but would make it harder to correlate to the times with those on remote
> machines. Either approach would require adding a new runtime flag and
> implementing the same thing on the remote side, which we probably don't
> want to do unless someone sees it as a real problem. Also, this should
> be coordinated with other pktgen implementations and might need a new
> magic number.
> 
> For the moment, I'm documenting the overflow in the source code, and
> changing the implementation over to an open-coded ktime_get_real_ts64()
> plus division, so we don't have to look at it again while scanning for
> deprecated time interfaces.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied to net-next, thanks Arnd.