Patchwork IP fragmentation broken in 3.6-rc ?

login
register
mail settings
Submitter Eric Dumazet
Date Aug. 21, 2012, 2:42 p.m.
Message ID <1345560131.5158.500.camel@edumazet-glaptop>
Download mbox | patch
Permalink /patch/179075/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - Aug. 21, 2012, 2:42 p.m.
On Tue, 2012-08-21 at 15:22 +0200, Eric Dumazet wrote:
> On Tue, 2012-08-21 at 15:05 +0200, Sylvain Munaut wrote:
> > Hi,
> > 
> > I'm starting to wonder if it's not related to
> > 46517008e1168dc926cf2c47d529efc07eca85c0 "ipv4: Kill
> > ip_rt_frag_needed()."
> > 
> > Cheers,
> > 
> >     Sylvain
> 
> Hmm, most probably problem came with commit
> ceb3320610d6f15ff20dd4c042b36473d77de76f
> (ipv4: Kill routes during PMTU/redirect updates.)
> 
> 

Following patch should help :



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julian Anastasov - Aug. 21, 2012, 4:34 p.m.
Hello,

On Tue, 21 Aug 2012, Eric Dumazet wrote:

> Following patch should help :
> 
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 621e351..a04aa37 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -435,7 +435,7 @@ static inline void dst_set_expires(struct dst_entry *dst, int timeout)
>  	if (expires == 0)
>  		expires = 1;
>  

	In theory, restart of PMTUD should not lead to
fatal problems, we will get new MTUs. But with such
change should be better, not much, because all MTU
events will come at same time, later timer will expire
and we will get again events from routers. The gain
will be an increased period (with milliseconds to seconds)
between PMTUD restarts. Compared to the 600-second timer,
this should be gain below 1% in reduced traffic for PMTUD.
Before now we started timer from first router, now we
will start/update timer period after event from last router.

	But ipv4_link_failure and ip6_link_failure want to stop
this timer by setting it to NOW (0). May be we have to add
also a !timeout check here or to leave the code as before?

> -	if (dst->expires == 0 || time_before(expires, dst->expires))
> +	if (dst->expires == 0 || time_after(expires, dst->expires))
>  		dst->expires = expires;
>  }

	The original problem should be somewhere else, I think.

Regards

--
Julian Anastasov <ja@ssi.bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sylvain Munaut - Aug. 21, 2012, 5:44 p.m.
Hi,

> Following patch should help :
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 621e351..a04aa37 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -435,7 +435,7 @@ static inline void dst_set_expires(struct dst_entry *dst, int timeout)
>         if (expires == 0)
>                 expires = 1;
>
> -       if (dst->expires == 0 || time_before(expires, dst->expires))
> +       if (dst->expires == 0 || time_after(expires, dst->expires))
>                 dst->expires = expires;
>  }

Yes, this fixes it for me too . ( 2h without issues on 2 different machines )

But that line has been in the kernel for _years_ ... so I'm not sure
what's going on there ...


Cheers,

    Sylvain
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/net/dst.h b/include/net/dst.h
index 621e351..a04aa37 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -435,7 +435,7 @@  static inline void dst_set_expires(struct dst_entry *dst, int timeout)
 	if (expires == 0)
 		expires = 1;
 
-	if (dst->expires == 0 || time_before(expires, dst->expires))
+	if (dst->expires == 0 || time_after(expires, dst->expires))
 		dst->expires = expires;
 }