netdev - Re: IP fragmentation broken in 3.6-rc ?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.1208211954380.2149@ja.ssi.bg>
Date:	Tue, 21 Aug 2012 20:18:25 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Eric Dumazet <eric.dumazet@...il.com>
cc:	Sylvain Munaut <s.munaut@...tever-company.com>,
	netdev@...r.kernel.org
Subject: Re: IP fragmentation broken in 3.6-rc ?


	Hello,

On Tue, 21 Aug 2012, Eric Dumazet wrote:

> On Tue, 2012-08-21 at 19:34 +0300, Julian Anastasov wrote:
> > 	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.
> > 
> 
> Sorry I dont really understand what you mean

	This timer is used only for PMTU, right?
RFC 1191 6.3. Purging stale PMTU information.

> > 	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.
> 
> This patch fixed the problem for me.

	OK, then I'll wait for the patch description
to understand what this change actually does. For me,
it just updates the timer on every MTU event, while
before the change, PMTU timer was started only once and
during the 600-second period it was not updated and also
there was the ability to invalidate the PMTU on link failure
by setting timer to jiffies, i.e. to stop this period
and to start new one with default MTU and possibly
new discovery procedure.

> My first patch was :
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index e4ba974..9858714 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -956,6 +956,7 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
>  		dst->obsolete = DST_OBSOLETE_KILL;
>  	} else {
>  		rt->rt_pmtu = mtu;
> +		rt->dst.expires = 0;
>  		dst_set_expires(&rt->dst, ip_rt_mtu_expires);

	This is better, does not break ipv4_link_failure.
There is a little race some ipv4_mtu() user to see
rt_pmtu != 0 and dst.expires = 0 and to fail in time_after_eq
test. May be that is why dst.expires is never set to 0.
But I still don't understand what both changes fix.

Regards

--
Julian Anastasov <ja@....bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html