All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds
@ 2013-09-20 20:32 Florian Westphal
  2013-09-20 20:32 ` [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values Florian Westphal
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Florian Westphal @ 2013-09-20 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal

We currently accept cookies that were created less than 4 minutes ago
(ie, cookies with counter delta 0-3).  Combined with the 8 mss table
values, this yields 32 possible values (out of 2**32) that will be valid.

Reducing the lifetime to < 2 minutes halves the guessing chance while
still providing a large enough period.

While at it, get rid of jiffies value -- they overflow too quickly on
32 bit platforms.

getnstimeofday is used to create a counter that increments every 64s.
perf shows getnstimeofday cost is negible compared to sha_transform;
normal tcp initial sequence number generation uses getnstimeofday, too.

Reported-by: Jakob Lell <jakob@jakoblell.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 No changes since v2.
 Changes since v1:
  - add comment explaining MAX_SYNCOOKIE_AGE value
  - add sentence about 'getnstimeofday' cost to commit message
  - rebase on net-next master

 include/net/tcp.h     | 18 ++++++++++++++++++
 net/ipv4/syncookies.c | 31 ++++++++++---------------------
 net/ipv6/syncookies.c | 24 +++++++-----------------
 3 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index b1aa324..ebb98e3 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -482,6 +482,24 @@ extern int __cookie_v4_check(const struct iphdr *iph, const struct tcphdr *th,
 extern struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb, 
 				    struct ip_options *opt);
 #ifdef CONFIG_SYN_COOKIES
+#include <linux/ktime.h>
+
+/* Syncookies use a monotonic timer which increments every 64 seconds.
+ * This counter is used both as a hash input and partially encoded into
+ * the cookie value.  A cookie is only validated further if the delta
+ * between the current counter value and the encoded one is less than this,
+ * i.e. a sent cookie is valid only at most for 128 seconds (or less if
+ * the counter advances immediately after a cookie is generated).
+ */
+#define MAX_SYNCOOKIE_AGE 2
+
+static inline u32 tcp_cookie_time(void)
+{
+	struct timespec now;
+	getnstimeofday(&now);
+	return now.tv_sec >> 6; /* 64 seconds granularity */
+}
+
 extern u32 __cookie_v4_init_sequence(const struct iphdr *iph,
 				     const struct tcphdr *th, u16 *mssp);
 extern __u32 cookie_v4_init_sequence(struct sock *sk, struct sk_buff *skb, 
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 14a15c4..b6ea297 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -89,8 +89,7 @@ __u32 cookie_init_timestamp(struct request_sock *req)
 
 
 static __u32 secure_tcp_syn_cookie(__be32 saddr, __be32 daddr, __be16 sport,
-				   __be16 dport, __u32 sseq, __u32 count,
-				   __u32 data)
+				   __be16 dport, __u32 sseq, __u32 data)
 {
 	/*
 	 * Compute the secure sequence number.
@@ -102,7 +101,7 @@ static __u32 secure_tcp_syn_cookie(__be32 saddr, __be32 daddr, __be16 sport,
 	 * As an extra hack, we add a small "data" value that encodes the
 	 * MSS into the second hash value.
 	 */
-
+	u32 count = tcp_cookie_time();
 	return (cookie_hash(saddr, daddr, sport, dport, 0, 0) +
 		sseq + (count << COOKIEBITS) +
 		((cookie_hash(saddr, daddr, sport, dport, count, 1) + data)
@@ -114,22 +113,21 @@ static __u32 secure_tcp_syn_cookie(__be32 saddr, __be32 daddr, __be16 sport,
  * If the syncookie is bad, the data returned will be out of
  * range.  This must be checked by the caller.
  *
- * The count value used to generate the cookie must be within
- * "maxdiff" if the current (passed-in) "count".  The return value
- * is (__u32)-1 if this test fails.
+ * The count value used to generate the cookie must be less than
+ * MAX_SYNCOOKIE_AGE minutes in the past.
+ * The return value (__u32)-1 if this test fails.
  */
 static __u32 check_tcp_syn_cookie(__u32 cookie, __be32 saddr, __be32 daddr,
-				  __be16 sport, __be16 dport, __u32 sseq,
-				  __u32 count, __u32 maxdiff)
+				  __be16 sport, __be16 dport, __u32 sseq)
 {
-	__u32 diff;
+	u32 diff, count = tcp_cookie_time();
 
 	/* Strip away the layers from the cookie */
 	cookie -= cookie_hash(saddr, daddr, sport, dport, 0, 0) + sseq;
 
 	/* Cookie is now reduced to (count * 2^24) ^ (hash % 2^24) */
 	diff = (count - (cookie >> COOKIEBITS)) & ((__u32) - 1 >> COOKIEBITS);
-	if (diff >= maxdiff)
+	if (diff >= MAX_SYNCOOKIE_AGE)
 		return (__u32)-1;
 
 	return (cookie -
@@ -173,7 +171,7 @@ u32 __cookie_v4_init_sequence(const struct iphdr *iph, const struct tcphdr *th,
 
 	return secure_tcp_syn_cookie(iph->saddr, iph->daddr,
 				     th->source, th->dest, ntohl(th->seq),
-				     jiffies / (HZ * 60), mssind);
+				     mssind);
 }
 EXPORT_SYMBOL_GPL(__cookie_v4_init_sequence);
 
@@ -189,13 +187,6 @@ __u32 cookie_v4_init_sequence(struct sock *sk, struct sk_buff *skb, __u16 *mssp)
 }
 
 /*
- * This (misnamed) value is the age of syncookie which is permitted.
- * Its ideal value should be dependent on TCP_TIMEOUT_INIT and
- * sysctl_tcp_retries1. It's a rather complicated formula (exponential
- * backoff) to compute at runtime so it's currently hardcoded here.
- */
-#define COUNTER_TRIES 4
-/*
  * Check if a ack sequence number is a valid syncookie.
  * Return the decoded mss if it is, or 0 if not.
  */
@@ -204,9 +195,7 @@ int __cookie_v4_check(const struct iphdr *iph, const struct tcphdr *th,
 {
 	__u32 seq = ntohl(th->seq) - 1;
 	__u32 mssind = check_tcp_syn_cookie(cookie, iph->saddr, iph->daddr,
-					    th->source, th->dest, seq,
-					    jiffies / (HZ * 60),
-					    COUNTER_TRIES);
+					    th->source, th->dest, seq);
 
 	return mssind < ARRAY_SIZE(msstab) ? msstab[mssind] : 0;
 }
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index bf63ac8..13ca0a0 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -36,14 +36,6 @@ static __u16 const msstab[] = {
 	9000 - 60,
 };
 
-/*
- * This (misnamed) value is the age of syncookie which is permitted.
- * Its ideal value should be dependent on TCP_TIMEOUT_INIT and
- * sysctl_tcp_retries1. It's a rather complicated formula (exponential
- * backoff) to compute at runtime so it's currently hardcoded here.
- */
-#define COUNTER_TRIES 4
-
 static inline struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb,
 					   struct request_sock *req,
 					   struct dst_entry *dst)
@@ -86,8 +78,9 @@ static u32 cookie_hash(const struct in6_addr *saddr, const struct in6_addr *dadd
 static __u32 secure_tcp_syn_cookie(const struct in6_addr *saddr,
 				   const struct in6_addr *daddr,
 				   __be16 sport, __be16 dport, __u32 sseq,
-				   __u32 count, __u32 data)
+				   __u32 data)
 {
+	u32 count = tcp_cookie_time();
 	return (cookie_hash(saddr, daddr, sport, dport, 0, 0) +
 		sseq + (count << COOKIEBITS) +
 		((cookie_hash(saddr, daddr, sport, dport, count, 1) + data)
@@ -96,15 +89,14 @@ static __u32 secure_tcp_syn_cookie(const struct in6_addr *saddr,
 
 static __u32 check_tcp_syn_cookie(__u32 cookie, const struct in6_addr *saddr,
 				  const struct in6_addr *daddr, __be16 sport,
-				  __be16 dport, __u32 sseq, __u32 count,
-				  __u32 maxdiff)
+				  __be16 dport, __u32 sseq)
 {
-	__u32 diff;
+	__u32 diff, count = tcp_cookie_time();
 
 	cookie -= cookie_hash(saddr, daddr, sport, dport, 0, 0) + sseq;
 
 	diff = (count - (cookie >> COOKIEBITS)) & ((__u32) -1 >> COOKIEBITS);
-	if (diff >= maxdiff)
+	if (diff >= MAX_SYNCOOKIE_AGE)
 		return (__u32)-1;
 
 	return (cookie -
@@ -125,8 +117,7 @@ u32 __cookie_v6_init_sequence(const struct ipv6hdr *iph,
 	*mssp = msstab[mssind];
 
 	return secure_tcp_syn_cookie(&iph->saddr, &iph->daddr, th->source,
-				     th->dest, ntohl(th->seq),
-				     jiffies / (HZ * 60), mssind);
+				     th->dest, ntohl(th->seq), mssind);
 }
 EXPORT_SYMBOL_GPL(__cookie_v6_init_sequence);
 
@@ -146,8 +137,7 @@ int __cookie_v6_check(const struct ipv6hdr *iph, const struct tcphdr *th,
 {
 	__u32 seq = ntohl(th->seq) - 1;
 	__u32 mssind = check_tcp_syn_cookie(cookie, &iph->saddr, &iph->daddr,
-					    th->source, th->dest, seq,
-					    jiffies / (HZ * 60), COUNTER_TRIES);
+					    th->source, th->dest, seq);
 
 	return mssind < ARRAY_SIZE(msstab) ? msstab[mssind] : 0;
 }
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values
  2013-09-20 20:32 [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds Florian Westphal
@ 2013-09-20 20:32 ` Florian Westphal
  2013-09-24 14:40   ` David Miller
  2013-09-24 14:40 ` [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds David Miller
  2014-03-19 20:38 ` Eric Dumazet
  2 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2013-09-20 20:32 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal

Halve mss table size to make blind cookie guessing more difficult.
This is sad since the tables were already small, but there
is little alternative except perhaps adding more precise mss information
in the tcp timestamp.  Timestamps are unfortunately not ubiquitous.

Guessing all possible cookie values still has 8-in 2**32 chance.

Reported-by: Jakob Lell <jakob@jakoblell.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Changes since v2:
 - add comment explaining mss choices for tcp6 cookies, too
 Changes since v1:
 - add comment explaining mss choices

 net/ipv4/syncookies.c | 22 +++++++++++-----------
 net/ipv6/syncookies.c | 15 +++++++++------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index b6ea297..15e0241 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -136,22 +136,22 @@ static __u32 check_tcp_syn_cookie(__u32 cookie, __be32 saddr, __be32 daddr,
 }
 
 /*
- * MSS Values are taken from the 2009 paper
- * 'Measuring TCP Maximum Segment Size' by S. Alcock and R. Nelson:
- *  - values 1440 to 1460 accounted for 80% of observed mss values
- *  - values outside the 536-1460 range are rare (<0.2%).
+ * MSS Values are chosen based on the 2011 paper
+ * 'An Analysis of TCP Maximum Segement Sizes' by S. Alcock and R. Nelson.
+ * Values ..
+ *  .. lower than 536 are rare (< 0.2%)
+ *  .. between 537 and 1299 account for less than < 1.5% of observed values
+ *  .. in the 1300-1349 range account for about 15 to 20% of observed mss values
+ *  .. exceeding 1460 are very rare (< 0.04%)
  *
- * Table must be sorted.
+ *  1460 is the single most frequently announced mss value (30 to 46% depending
+ *  on monitor location).  Table must be sorted.
  */
 static __u16 const msstab[] = {
-	64,
-	512,
 	536,
-	1024,
-	1440,
+	1300,
+	1440,	/* 1440, 1452: PPPoE */
 	1460,
-	4312,
-	8960,
 };
 
 /*
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index 13ca0a0..d703218 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -24,15 +24,18 @@
 #define COOKIEBITS 24	/* Upper bits store count */
 #define COOKIEMASK (((__u32)1 << COOKIEBITS) - 1)
 
-/* Table must be sorted. */
+/* RFC 2460, Section 8.3:
+ * [ipv6 tcp] MSS must be computed as the maximum packet size minus 60 [..]
+ *
+ * Due to IPV6_MIN_MTU=1280 the lowest possible MSS is 1220, which allows
+ * using higher values than ipv4 tcp syncookies.
+ * The other values are chosen based on ethernet (1500 and 9k MTU), plus
+ * one that accounts for common encap (PPPoe) overhead. Table must be sorted.
+ */
 static __u16 const msstab[] = {
-	64,
-	512,
-	536,
-	1280 - 60,
+	1280 - 60, /* IPV6_MIN_MTU - 60 */
 	1480 - 60,
 	1500 - 60,
-	4460 - 60,
 	9000 - 60,
 };
 
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds
  2013-09-20 20:32 [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds Florian Westphal
  2013-09-20 20:32 ` [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values Florian Westphal
@ 2013-09-24 14:40 ` David Miller
  2014-03-19 20:38 ` Eric Dumazet
  2 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2013-09-24 14:40 UTC (permalink / raw)
  To: fw; +Cc: netdev

From: Florian Westphal <fw@strlen.de>
Date: Fri, 20 Sep 2013 22:32:55 +0200

> We currently accept cookies that were created less than 4 minutes ago
> (ie, cookies with counter delta 0-3).  Combined with the 8 mss table
> values, this yields 32 possible values (out of 2**32) that will be valid.
> 
> Reducing the lifetime to < 2 minutes halves the guessing chance while
> still providing a large enough period.
> 
> While at it, get rid of jiffies value -- they overflow too quickly on
> 32 bit platforms.
> 
> getnstimeofday is used to create a counter that increments every 64s.
> perf shows getnstimeofday cost is negible compared to sha_transform;
> normal tcp initial sequence number generation uses getnstimeofday, too.
> 
> Reported-by: Jakob Lell <jakob@jakoblell.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values
  2013-09-20 20:32 ` [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values Florian Westphal
@ 2013-09-24 14:40   ` David Miller
  0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2013-09-24 14:40 UTC (permalink / raw)
  To: fw; +Cc: netdev

From: Florian Westphal <fw@strlen.de>
Date: Fri, 20 Sep 2013 22:32:56 +0200

> Halve mss table size to make blind cookie guessing more difficult.
> This is sad since the tables were already small, but there
> is little alternative except perhaps adding more precise mss information
> in the tcp timestamp.  Timestamps are unfortunately not ubiquitous.
> 
> Guessing all possible cookie values still has 8-in 2**32 chance.
> 
> Reported-by: Jakob Lell <jakob@jakoblell.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied.

Thanks for following up on all of my feedback.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds
  2013-09-20 20:32 [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds Florian Westphal
  2013-09-20 20:32 ` [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values Florian Westphal
  2013-09-24 14:40 ` [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds David Miller
@ 2014-03-19 20:38 ` Eric Dumazet
  2014-03-19 21:06   ` [PATCH] tcp: syncookies: do not use getnstimeofday() Eric Dumazet
  2 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2014-03-19 20:38 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev

On Fri, 2013-09-20 at 22:32 +0200, Florian Westphal wrote:
> We currently accept cookies that were created less than 4 minutes ago
> (ie, cookies with counter delta 0-3).  Combined with the 8 mss table
> values, this yields 32 possible values (out of 2**32) that will be valid.
> 
> Reducing the lifetime to < 2 minutes halves the guessing chance while
> still providing a large enough period.
> 
> While at it, get rid of jiffies value -- they overflow too quickly on
> 32 bit platforms.
> 
> getnstimeofday is used to create a counter that increments every 64s.
> perf shows getnstimeofday cost is negible compared to sha_transform;
> normal tcp initial sequence number generation uses getnstimeofday, too.

The getnstimeofday() cost is about 40 cycles if TSC is used, but if this
source is not available and hpet is used, it is more 1600 cycles ...

I'll send a patch to use local_clock() instead (26 to 30 cycles)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] tcp: syncookies: do not use getnstimeofday()
  2014-03-19 20:38 ` Eric Dumazet
@ 2014-03-19 21:06   ` Eric Dumazet
  2014-03-19 22:56     ` Florian Westphal
  2014-03-20  4:02     ` [PATCH v2] " Eric Dumazet
  0 siblings, 2 replies; 16+ messages in thread
From: Eric Dumazet @ 2014-03-19 21:06 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

While it is true that getnstimeofday() uses about 40 cycles if TSC
is available, it can use 1600 cycles if hpet is the clocksource.

Switch to local_clock(). This one consumes 26 cycles and is not
impacted by a date/time change.

Fixes: 8c27bd75f04f ("tcp: syncookies: reduce cookie lifetime to 128 seconds")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
---
 include/net/tcp.h |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index bb253b9b2f06..b7e524bb297e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -484,16 +484,15 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
  * This counter is used both as a hash input and partially encoded into
  * the cookie value.  A cookie is only validated further if the delta
  * between the current counter value and the encoded one is less than this,
- * i.e. a sent cookie is valid only at most for 128 seconds (or less if
+ * i.e. a sent cookie is valid only at most for 2*68 seconds (or less if
  * the counter advances immediately after a cookie is generated).
  */
 #define MAX_SYNCOOKIE_AGE 2
 
 static inline u32 tcp_cookie_time(void)
 {
-	struct timespec now;
-	getnstimeofday(&now);
-	return now.tv_sec >> 6; /* 64 seconds granularity */
+	/* 2^36 nsec = ~68.7 sec */
+	return local_clock() >> 36;
 }
 
 u32 __cookie_v4_init_sequence(const struct iphdr *iph, const struct tcphdr *th,

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] tcp: syncookies: do not use getnstimeofday()
  2014-03-19 21:06   ` [PATCH] tcp: syncookies: do not use getnstimeofday() Eric Dumazet
@ 2014-03-19 22:56     ` Florian Westphal
  2014-03-20  2:26       ` Eric Dumazet
  2014-03-20  4:02     ` [PATCH v2] " Eric Dumazet
  1 sibling, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2014-03-19 22:56 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Florian Westphal, netdev

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> While it is true that getnstimeofday() uses about 40 cycles if TSC
> is available, it can use 1600 cycles if hpet is the clocksource.

Ouch.  Why is secure_tcp_sequence_number also using
ktime_get_real/getnstimeofday?

clock drift when other cpu is used?

[ Just curious; I have no objections to this patch ]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] tcp: syncookies: do not use getnstimeofday()
  2014-03-19 22:56     ` Florian Westphal
@ 2014-03-20  2:26       ` Eric Dumazet
  2014-03-20  2:37         ` Eric Dumazet
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2014-03-20  2:26 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev

On Wed, 2014-03-19 at 23:56 +0100, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > While it is true that getnstimeofday() uses about 40 cycles if TSC
> > is available, it can use 1600 cycles if hpet is the clocksource.
> 
> Ouch.  Why is secure_tcp_sequence_number also using
> ktime_get_real/getnstimeofday?
> 
> clock drift when other cpu is used?

Yeah, I guess we cant really avoid ktime_get() in this sequence
generation.

Alternative patch would be to use get_jiffies_64(), as it would
'solve' the issue you had on 32bit arches, and would be faster
than local_clock() :

diff --git a/include/net/tcp.h b/include/net/tcp.h
index bb253b9b2f06..06f948a3fad1 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -484,16 +484,14 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
  * This counter is used both as a hash input and partially encoded into
  * the cookie value.  A cookie is only validated further if the delta
  * between the current counter value and the encoded one is less than this,
- * i.e. a sent cookie is valid only at most for 128 seconds (or less if
+ * i.e. a sent cookie is valid only at most for 2*64 seconds (or less if
  * the counter advances immediately after a cookie is generated).
  */
 #define MAX_SYNCOOKIE_AGE 2
 
 static inline u32 tcp_cookie_time(void)
 {
-	struct timespec now;
-	getnstimeofday(&now);
-	return now.tv_sec >> 6; /* 64 seconds granularity */
+	return get_jiffies_64() / (64 * HZ);
 }
 
 u32 __cookie_v4_init_sequence(const struct iphdr *iph, const struct tcphdr *th,

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] tcp: syncookies: do not use getnstimeofday()
  2014-03-20  2:26       ` Eric Dumazet
@ 2014-03-20  2:37         ` Eric Dumazet
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2014-03-20  2:37 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev

On Wed, 2014-03-19 at 19:26 -0700, Eric Dumazet wrote:
>  
>  static inline u32 tcp_cookie_time(void)
>  {
> -	struct timespec now;
> -	getnstimeofday(&now);
> -	return now.tv_sec >> 6; /* 64 seconds granularity */
> +	return get_jiffies_64() / (64 * HZ);
>  }
>  

Well, it would need a do_div() to please 32bit arches of course.

diff --git a/include/net/tcp.h b/include/net/tcp.h
index bb253b9b2f06..87d877408188 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -480,20 +480,21 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 			     struct ip_options *opt);
 #ifdef CONFIG_SYN_COOKIES
 
-/* Syncookies use a monotonic timer which increments every 64 seconds.
+/* Syncookies use a monotonic timer which increments every 60 seconds.
  * This counter is used both as a hash input and partially encoded into
  * the cookie value.  A cookie is only validated further if the delta
  * between the current counter value and the encoded one is less than this,
- * i.e. a sent cookie is valid only at most for 128 seconds (or less if
+ * i.e. a sent cookie is valid only at most for 2*60 seconds (or less if
  * the counter advances immediately after a cookie is generated).
  */
 #define MAX_SYNCOOKIE_AGE 2
 
 static inline u32 tcp_cookie_time(void)
 {
-	struct timespec now;
-	getnstimeofday(&now);
-	return now.tv_sec >> 6; /* 64 seconds granularity */
+	u64 val = get_jiffies_64();
+
+	do_div(val, 60 * HZ);
+	return val;
 }
 
 u32 __cookie_v4_init_sequence(const struct iphdr *iph, const struct tcphdr *th,

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-19 21:06   ` [PATCH] tcp: syncookies: do not use getnstimeofday() Eric Dumazet
  2014-03-19 22:56     ` Florian Westphal
@ 2014-03-20  4:02     ` Eric Dumazet
  2014-03-20  8:45       ` Florian Westphal
                         ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Eric Dumazet @ 2014-03-20  4:02 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

While it is true that getnstimeofday() uses about 40 cycles if TSC
is available, it can use 1600 cycles if hpet is the clocksource.

Switch to get_jiffies_64(), as this is more than enough, and
go back to 60 seconds periods.

Fixes: 8c27bd75f04f ("tcp: syncookies: reduce cookie lifetime to 128 seconds")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
---
 include/net/tcp.h |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index bb253b9b2f06..87d877408188 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -480,20 +480,21 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 			     struct ip_options *opt);
 #ifdef CONFIG_SYN_COOKIES
 
-/* Syncookies use a monotonic timer which increments every 64 seconds.
+/* Syncookies use a monotonic timer which increments every 60 seconds.
  * This counter is used both as a hash input and partially encoded into
  * the cookie value.  A cookie is only validated further if the delta
  * between the current counter value and the encoded one is less than this,
- * i.e. a sent cookie is valid only at most for 128 seconds (or less if
+ * i.e. a sent cookie is valid only at most for 2*60 seconds (or less if
  * the counter advances immediately after a cookie is generated).
  */
 #define MAX_SYNCOOKIE_AGE 2
 
 static inline u32 tcp_cookie_time(void)
 {
-	struct timespec now;
-	getnstimeofday(&now);
-	return now.tv_sec >> 6; /* 64 seconds granularity */
+	u64 val = get_jiffies_64();
+
+	do_div(val, 60 * HZ);
+	return val;
 }
 
 u32 __cookie_v4_init_sequence(const struct iphdr *iph, const struct tcphdr *th,

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-20  4:02     ` [PATCH v2] " Eric Dumazet
@ 2014-03-20  8:45       ` Florian Westphal
  2014-03-20 19:36       ` David Miller
  2014-03-20 20:23       ` David Miller
  2 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2014-03-20  8:45 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> While it is true that getnstimeofday() uses about 40 cycles if TSC
> is available, it can use 1600 cycles if hpet is the clocksource.
> 
> Switch to get_jiffies_64(), as this is more than enough, and
> go back to 60 seconds periods.
> 
> Fixes: 8c27bd75f04f ("tcp: syncookies: reduce cookie lifetime to 128 seconds")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Florian Westphal <fw@strlen.de>

Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-20  4:02     ` [PATCH v2] " Eric Dumazet
  2014-03-20  8:45       ` Florian Westphal
@ 2014-03-20 19:36       ` David Miller
  2014-03-20 20:07         ` Eric Dumazet
  2014-03-20 20:23       ` David Miller
  2 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2014-03-20 19:36 UTC (permalink / raw)
  To: eric.dumazet; +Cc: fw, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 19 Mar 2014 21:02:21 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> While it is true that getnstimeofday() uses about 40 cycles if TSC
> is available, it can use 1600 cycles if hpet is the clocksource.
> 
> Switch to get_jiffies_64(), as this is more than enough, and
> go back to 60 seconds periods.
> 
> Fixes: 8c27bd75f04f ("tcp: syncookies: reduce cookie lifetime to 128 seconds")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Florian Westphal <fw@strlen.de>

This applies cleanly to net-next, but with this Fixes: tag I sort of
suspect you are targetting this at net instead?

Please be explicit as to what the target tree is when submitting
patches in the future.

Thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-20 19:36       ` David Miller
@ 2014-03-20 20:07         ` Eric Dumazet
  2014-03-20 20:16           ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2014-03-20 20:07 UTC (permalink / raw)
  To: David Miller; +Cc: fw, netdev

On Thu, 2014-03-20 at 15:36 -0400, David Miller wrote:

> This applies cleanly to net-next, but with this Fixes: tag I sort of
> suspect you are targetting this at net instead?
> 
> Please be explicit as to what the target tree is when submitting
> patches in the future.

Well, I've always tagged net-next patches with net-next, and used no tag
for fixes.

If you want me to add net tag, I am totally fine.

So yes, this was a fix for net tree, but there is no hurry if you
applied to net-next its also fine.

Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-20 20:07         ` Eric Dumazet
@ 2014-03-20 20:16           ` David Miller
  2014-03-20 20:37             ` Eric Dumazet
  0 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2014-03-20 20:16 UTC (permalink / raw)
  To: eric.dumazet; +Cc: fw, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 20 Mar 2014 13:07:39 -0700

> On Thu, 2014-03-20 at 15:36 -0400, David Miller wrote:
> 
>> This applies cleanly to net-next, but with this Fixes: tag I sort of
>> suspect you are targetting this at net instead?
>> 
>> Please be explicit as to what the target tree is when submitting
>> patches in the future.
> 
> Well, I've always tagged net-next patches with net-next, and used no tag
> for fixes.
> 
> If you want me to add net tag, I am totally fine.
> 
> So yes, this was a fix for net tree, but there is no hurry if you
> applied to net-next its also fine.

The confusion came mostly because it applied perfectly to net-next, but
with fuzz to plain net. :)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-20  4:02     ` [PATCH v2] " Eric Dumazet
  2014-03-20  8:45       ` Florian Westphal
  2014-03-20 19:36       ` David Miller
@ 2014-03-20 20:23       ` David Miller
  2 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2014-03-20 20:23 UTC (permalink / raw)
  To: eric.dumazet; +Cc: fw, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 19 Mar 2014 21:02:21 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> While it is true that getnstimeofday() uses about 40 cycles if TSC
> is available, it can use 1600 cycles if hpet is the clocksource.
> 
> Switch to get_jiffies_64(), as this is more than enough, and
> go back to 60 seconds periods.
> 
> Fixes: 8c27bd75f04f ("tcp: syncookies: reduce cookie lifetime to 128 seconds")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] tcp: syncookies: do not use getnstimeofday()
  2014-03-20 20:16           ` David Miller
@ 2014-03-20 20:37             ` Eric Dumazet
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2014-03-20 20:37 UTC (permalink / raw)
  To: David Miller; +Cc: fw, netdev

On Thu, 2014-03-20 at 16:16 -0400, David Miller wrote:
> The confusion came mostly because it applied perfectly to net-next, but
> with fuzz to plain net. :)

My bad, you are totally right, I forgot to double check.

Sorry for the confusion.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-03-20 20:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-20 20:32 [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds Florian Westphal
2013-09-20 20:32 ` [PATCH v3 -next 2/2] tcp: syncookies: reduce mss table to four values Florian Westphal
2013-09-24 14:40   ` David Miller
2013-09-24 14:40 ` [PATCH v3 -next 1/2] tcp: syncookies: reduce cookie lifetime to 128 seconds David Miller
2014-03-19 20:38 ` Eric Dumazet
2014-03-19 21:06   ` [PATCH] tcp: syncookies: do not use getnstimeofday() Eric Dumazet
2014-03-19 22:56     ` Florian Westphal
2014-03-20  2:26       ` Eric Dumazet
2014-03-20  2:37         ` Eric Dumazet
2014-03-20  4:02     ` [PATCH v2] " Eric Dumazet
2014-03-20  8:45       ` Florian Westphal
2014-03-20 19:36       ` David Miller
2014-03-20 20:07         ` Eric Dumazet
2014-03-20 20:16           ` David Miller
2014-03-20 20:37             ` Eric Dumazet
2014-03-20 20:23       ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.