All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steffen Klassert <steffen.klassert@secunet.com>
To: Martin Lau <kafai@fb.com>
Cc: shengyong <shengyong1@huawei.com>, <davem@davemloft.net>,
	<netdev@vger.kernel.org>, <yangyingliang@huawei.com>,
	<hannes@redhat.com>, Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH RFC 1/3] ipv6: Fix after pmtu events dissapearing host routes
Date: Wed, 1 Apr 2015 10:09:45 +0200	[thread overview]
Message-ID: <20150401080944.GF20559@secunet.com> (raw)
In-Reply-To: <20150330182433.GC2303816@devbig242.prn2.facebook.com>

On Mon, Mar 30, 2015 at 11:24:33AM -0700, Martin Lau wrote:
> On Mon, Mar 30, 2015 at 12:33:13PM +0200, Steffen Klassert wrote:
> > ---
> >  net/ipv6/route.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 4688bd4..4318626 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -961,7 +961,7 @@ redo_rt6_select:
> >  
> >  	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
> >  		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
> > -	else if (!(rt->dst.flags & DST_HOST))
> > +	else if (!(rt->dst.flags & DST_HOST) || !(rt->rt6i_flags & RTF_LOCAL))
> >  		nrt = rt6_alloc_clone(rt, &fl6->daddr);
> I suspect the nrt and rt will share the same inetpeer.  Hence, after the nrt is
> removed, the MTU (obtained from PMTU update) will stay.

I used a route with default metrics in my tests, therefore the
merics were copied for nrt. But if the route has a mtu set,
the metrics are not copied. So we overwrite the route mtu
with the pmtu value. Maybe we should make sure to always
copy the metrics if we do a pmtu update.

Something like this:

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e0c6ac9..675bc42 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1182,6 +1182,7 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
 		if (mtu < IPV6_MIN_MTU)
 			mtu = IPV6_MIN_MTU;
 
+		dst_cow_metrics_generic(dst, dst->_metrics);
 		dst_metric_set(dst, RTAX_MTU, mtu);
 		rt6_update_expires(rt6, net->ipv6.sysctl.ip6_rt_mtu_expires);
 	}

This could fix the pmtu case, but what are we going to do if
we have multiple routes with different mtu values to the same
host? Adding a second hostroute will overwrite the mtu value
of the first hostroute.

This reminds me at the times when we removed the ipv4 routing cache.
The first approach to handle pmtu updates was to cache the pmtu
values at the inetpeer, but we never got it to work. We swiched
then to use exceptional routes in this case.

  reply	other threads:[~2015-04-01  8:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-02  8:20 Question: should local address be expired when updating PMTU? shengyong
2015-02-02 21:31 ` David Miller
2015-02-03  0:52 ` Alex Gartrell
2015-02-03  0:52   ` Alex Gartrell
2015-02-03  1:28   ` shengyong
2015-02-03  1:28     ` shengyong
2015-02-03  2:10   ` Calvin Owens
2015-02-03  2:10     ` Calvin Owens
2015-02-03  3:21     ` shengyong
2015-02-03  3:21       ` shengyong
2015-02-03  9:28 ` Steffen Klassert
2015-02-03 10:54   ` shengyong
2015-02-03 12:01     ` Steffen Klassert
2015-02-04  1:59       ` shengyong
2015-02-05  7:21         ` Steffen Klassert
2015-02-27  2:37           ` shengyong
2015-02-27 10:32             ` Steffen Klassert
2015-03-30 10:32             ` Steffen Klassert
2015-03-30 10:33               ` [PATCH RFC 1/3] ipv6: Fix after pmtu events dissapearing host routes Steffen Klassert
2015-03-30 11:15                 ` Sheng Yong
2015-03-30 18:24                 ` Martin Lau
2015-04-01  8:09                   ` Steffen Klassert [this message]
2015-03-30 10:33               ` [PATCH RFC 2/3] ipv6: Extend the route lookups to low priority metrics Steffen Klassert
2015-03-30 10:34               ` [PATCH RFC 3/3] ipv6: Don't update pmtu on uncached routes Steffen Klassert
2015-03-30 11:13               ` Question: should local address be expired when updating PMTU? Sheng Yong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150401080944.GF20559@secunet.com \
    --to=steffen.klassert@secunet.com \
    --cc=Kernel-team@fb.com \
    --cc=davem@davemloft.net \
    --cc=hannes@redhat.com \
    --cc=kafai@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=shengyong1@huawei.com \
    --cc=yangyingliang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.