[3/4] ipv4: Fix inetpeer expiration handling

2011.10.11 19:11, Steffen Klassert wrote:
> We leak a trigger to check if the learned pmtu information has
> expired, so the learned pmtu informations never expire. This patch
> add such a trigger to ipv4_dst_check().
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  net/ipv4/route.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 9a6623e..cda370c 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1660,6 +1660,10 @@ static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
>  
>  	if (rt_is_expired(rt))
>  		return NULL;
> +
> +	if (rt->peer && peer_pmtu_expired(rt->peer))
> +		dst_metric_set(dst, RTAX_MTU, rt->peer->pmtu_orig);
> +
>  	if (rt->rt_peer_genid != rt_peer_genid()) {
>  		struct inet_peer *peer;
>  

there are serval problem.
1:rt->peer maybe null,we should call rt_bind_peer just like the code below.
2:rt->peer_pmtu_orig is null. if we hasn't send packet before,the func check_peer_pmtu hasn't be called.
  so the peer->pmtu_orig is null.
3:when rt->rt_peer_genid != rt_peer_genid(), we has no need to do dst_metric_set(dst, RTAX_MTU, rt->peer->pmtu_orig),
  because check_peer_pmtu will do this.

what about this patch?


--
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

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 20 Oct 2011 14:24:33 +0800

> there are serval problem.
> 1:rt->peer maybe null,we should call rt_bind_peer just like the code below.
> 2:rt->peer_pmtu_orig is null. if we hasn't send packet before,the func check_peer_pmtu hasn't be called.
>   so the peer->pmtu_orig is null.
> 3:when rt->rt_peer_genid != rt_peer_genid(), we has no need to do dst_metric_set(dst, RTAX_MTU, rt->peer->pmtu_orig),
>   because check_peer_pmtu will do this.
> 
> what about this patch?

In the case where no peer was created and we use default metrics and
other settings (can be common for UDP) I don't think it's wise to make
an inetpeer lookup every time we check the dst.
--
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

On Thu, Oct 20, 2011 at 02:24:33PM +0800, Gao feng wrote:
> 
> there are serval problem.
> 1:rt->peer maybe null,we should call rt_bind_peer just like the code below.

If rt->peer is NULL, rt_bind_peer() sets rt->rt_peer_genid = rt_peer_genid().
So your check for rt->rt_peer_genid != rt_peer_genid() is false then and
creates cases where an unchecked peer is bound to a route.

> 2:rt->peer_pmtu_orig is null. if we hasn't send packet before,the func check_peer_pmtu hasn't be called.
>   so the peer->pmtu_orig is null.

If a peer is bound to a route during slow path route lookup, the peer
should be properly initialized with rt_init_metrics(). So
rt->peer_pmtu_orig should not be null here as far as I can see.

I still think that my original patch fixes the problem.
--
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