Skip to contentSkip to conversation

/dev/random: Fix latency in rekeying sequence number

PATCH In review Theodore Ts'o 3 participants 3 messages Aug 20, 2004 – Aug 22, 2004

lore
Share & tools
lore
#0 lore raw

PATCH 1/4 /dev/random: Fix latency in rekeying sequence number

Based on reports from Ingo's Latency Tracer that the TCP sequence number rekey code is causing latency problems, I've moved the sequence number rekey to be done out of a workqueue.

patch-random-1-rekey-workqueue

random.c +16 -11
@@ -2246,30 +2246,35 @@
 static spinlock_t ip_lock = SPIN_LOCK_UNLOCKED;
 static unsigned int ip_cnt;
 
-static struct keydata *__check_and_rekey(time_t time)
+static void rekey_seq_generator(void *private_)
 {
 	struct keydata *keyptr;
+	struct timeval 	tv;
+
+	do_gettimeofday(&tv);
+
 	spin_lock_bh(&ip_lock);
 	keyptr = &ip_keydata[ip_cnt&1];
-	if (!keyptr->rekey_time || (time - keyptr->rekey_time) > REKEY_INTERVAL) {
-		keyptr = &ip_keydata[1^(ip_cnt&1)];
-		keyptr->rekey_time = time;
-		get_random_bytes(keyptr->secret, sizeof(keyptr->secret));
-		keyptr->count = (ip_cnt&COUNT_MASK)<<HASH_BITS;
-		mb();
-		ip_cnt++;
-	}
+
+	keyptr = &ip_keydata[1^(ip_cnt&1)];
+	keyptr->rekey_time = tv.tv_sec;
+	get_random_bytes(keyptr->secret, sizeof(keyptr->secret));
+	keyptr->count = (ip_cnt&COUNT_MASK)<<HASH_BITS;
+	mb();
+	ip_cnt++;
+
 	spin_unlock_bh(&ip_lock);
-	return keyptr;
 }
 
+static DECLARE_WORK(rekey_work, rekey_seq_generator, NULL);
+
 static inline struct keydata *check_and_rekey(time_t time)
 {
 	struct keydata *keyptr = &ip_keydata[ip_cnt&1];
 
 	rmb();
 	if (!keyptr->rekey_time || (time - keyptr->rekey_time) > REKEY_INTERVAL) {
-		keyptr = __check_and_rekey(time);
+		schedule_work(&rekey_work);
 	}
 
 	return keyptr;
#1 lore raw
Quoted 4 lines · Theodore Ts'o

Based on reports from Ingo's Latency Tracer that the TCP sequence number rekey code is causing latency problems, I've moved the sequence number rekey to be done out of a workqueue.

This patch does not actually fix the problem, as 3-700usecs is still spent in the spinlocked region, this just causes it to be done out of a workqueue. It reduces the incidence of problems, but for latency sensitive applications the worst-case is the important one.

Lee

#2 lore raw
Quoted 3 lines · Lee Revell

This patch does not actually fix the problem, as 3-700usecs is still spent in the spinlocked region, this just causes it to be done out of a workqueue.

May I suggest that get_random_bytes() be called *outside* of the spinlocked region on a temporary buffer, assuming that the SHA algorithm is the problem here?

Something like this (untested):

  u32 tmp[sizeof(keyptr->secret)/4];      /* on stack */
  get_random_bytes(tmp, sizeof(tmp));
  spin_lock_bh(&ip_lock);
  memcpy(keyptr->secret, tmp, sizeof(keyptr->secret));
  /* ... other housekeeping ... */
  spin_unlock_bh(&ip_lock);

Jens Maurer