[PATCH 1/3] gre: used time_before for comparing jiffies

[PATCH 1/3] gre: used time_before for comparing jiffies

[Wei Yongjun]

The functions time_before is more robust for comparing
jiffies against other values.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 net/ipv4/ip_gre.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 0101521..811bef7 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -432,7 +432,7 @@ static void ipgre_err(struct sk_buff *skb, u32 info)
 	if (t->parms.iph.ttl == 0 && type == ICMP_TIME_EXCEEDED)
 		goto out;
 
-	if (jiffies - t->err_time < IPTUNNEL_ERR_TIMEO)
+	if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO))
 		t->err_count++;
 	else
 		t->err_count = 1;
@@ -744,7 +744,8 @@ static int ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 #endif
 
 	if (tunnel->err_count > 0) {
-		if (jiffies - tunnel->err_time < IPTUNNEL_ERR_TIMEO) {
+		if (time_before(jiffies,
+				tunnel->err_time + IPTUNNEL_ERR_TIMEO)) {
 			tunnel->err_count--;
 
 			dst_link_failure(skb);
-- 
1.5.3.8

Re: [PATCH 1/3] gre: used time_before for comparing jiffies

[David Miller]

From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Fri, 20 Feb 2009 12:06:44 +0800

> The functions time_before is more robust for comparing
> jiffies against other values.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

There is some history of this in the networking code.

If you inspect carefully, the open-coded versions of
time comparisons in the networking have a larger window
of acceptance.

So actually, it is a regression fo use time_*() helpers
in these cases.

If these things have been like this for so long, it is
very likely there is a good reason :-)

Re: [PATCH 1/3] gre: used time_before for comparing jiffies

[Herbert Xu]

David Miller <davem@davemloft.net> wrote:
>
> There is some history of this in the networking code.
> 
> If you inspect carefully, the open-coded versions of
> time comparisons in the networking have a larger window
> of acceptance.

Hmm, it looks like the macro version is actually better in this
case.

-	if (jiffies - t->err_time < IPTUNNEL_ERR_TIMEO)

This wraps at jiffies = LONG_MAX + t->err_time + 1

+	if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO))

This expands to

	(long)jiffies - (long)(t->err_time + IPTUNNEL_ERR_TIMEO) < 0

which wraps at jiffies = LONG_MAX + t->err_time + IPTUNNEL_ERR_TIMEO + 1

So assuming that IPTUNNEL_ERR_TIMEO > 0, then the macro version
wraps around after the open-coded version, which would seem to
mean that it's better, no?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/3] gre: used time_before for comparing jiffies

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 20 Feb 2009 16:39:35 +0800

> So assuming that IPTUNNEL_ERR_TIMEO > 0, then the macro version
> wraps around after the open-coded version, which would seem to
> mean that it's better, no?

Seems that way...

Let me see if I can find Alexey's email where he explains
all of this.  I really don't want to touch these things
without being able to read that again.

If anyone else can find his posting about this stuff from
years ago, please post the link.

Thanks.

Re: [PATCH 1/3] gre: used time_before for comparing jiffies

[Herbert Xu]

Wei Yongjun <yjwei@cn.fujitsu.com> wrote:
> The functions time_before is more robust for comparing
> jiffies against other values.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

Please resubmit these patches so that they can be reviewed again.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/3] gre: used time_before for comparing jiffies

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 24 Feb 2009 15:13:52 +0800

> Wei Yongjun <yjwei@cn.fujitsu.com> wrote:
> > The functions time_before is more robust for comparing
> > jiffies against other values.
> > 
> > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> 
> Please resubmit these patches so that they can be reviewed again.

Indeed, I think my original objections have been cleared up
after some research.

[PATCH 1/3] gre: used time_before for comparing jiffies

[Wei Yongjun]

The functions time_before is more robust for comparing
jiffies against other values.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 net/ipv4/ip_gre.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 0101521..811bef7 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -432,7 +432,7 @@ static void ipgre_err(struct sk_buff *skb, u32 info)
 	if (t->parms.iph.ttl == 0 && type == ICMP_TIME_EXCEEDED)
 		goto out;

-	if (jiffies - t->err_time < IPTUNNEL_ERR_TIMEO)
+	if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO))
 		t->err_count++;
 	else
 		t->err_count = 1;
@@ -744,7 +744,8 @@ static int ipgre_tunnel_xmit(struct sk_buff *skb, struct
net_device *dev)
 #endif

 	if (tunnel->err_count > 0) {
-		if (jiffies - tunnel->err_time < IPTUNNEL_ERR_TIMEO) {
+		if (time_before(jiffies,
+				tunnel->err_time + IPTUNNEL_ERR_TIMEO)) {
 			tunnel->err_count--;

 			dst_link_failure(skb);
-- 
1.5.3.8

Re: [PATCH 1/3] gre: used time_before for comparing jiffies

[David Miller]

From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Tue, 24 Feb 2009 15:54:42 +0800

> The functions time_before is more robust for comparing
> jiffies against other values.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

Applied.