From patchwork Wed Mar 23 23:57:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wei Wang X-Patchwork-Id: 601412 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3qVmhQ6ryrz9s5M for ; Thu, 24 Mar 2016 10:57:26 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=v6l2OBTJ; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752256AbcCWX5Z (ORCPT ); Wed, 23 Mar 2016 19:57:25 -0400 Received: from mail-vk0-f48.google.com ([209.85.213.48]:34562 "EHLO mail-vk0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751477AbcCWX5X (ORCPT ); Wed, 23 Mar 2016 19:57:23 -0400 Received: by mail-vk0-f48.google.com with SMTP id e185so39107388vkb.1 for ; Wed, 23 Mar 2016 16:57:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=Z6dAFWnsR5D6IzIMpS2o+1Lt1qZJYuAxp5A06+t9EX4=; b=v6l2OBTJw+WMnvwKuI/h7f9EIXiscWAlIHtFjJynC/VmGm3FH2EfDLFQ920yy/bGek G75Rq12Y+GxXo5fneaDOdaxJTIesBttETbjjd0+JFibcBb5MUpL5VtO6KRYDLrwxKMxg fKA9e6XuuGZVi6q+CgefHNhKd1qcx9pFzfHXYUf8IZy49Rz7m9o++RF7sW5Ieylp/ErD i58aY09i2gozZrH0dQ/1onkpoF9rMCTOghSQ4Ov7ax4umMTsCOxrtH+CsZYSI9w++RAV Sdh1g77PKCpoy/YxUNjnjXzj3GPbsiHO+6qC5sieE9saQ7zfxqSqhIl0QmIcYRykDc09 QHlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=Z6dAFWnsR5D6IzIMpS2o+1Lt1qZJYuAxp5A06+t9EX4=; b=Vsa4qMd3WIfgOrWn/SHcSGTl3TJXojwX/xlf/WLYwGocGejg+nAOYZbI1ZMpRCDZQQ nsynwC72rqVe4xLKvKIV/o8W8Xkmhv5XB8YLf52PD8mlPaMIzblA2Mrxk/Mj9XA6/cyS QVsCi8PguwRiq8dCvGM9Kkz0oUwx12MdYpuEYFHVK6DfV9ivFDQgXH3ykHN2/6hUaIKM xwJyOgpePFpqKvqW/NntgdEoCKDxd/bxD26jAr1z/66BiL7YFNL8lapqrPIrmf5bA7Z+ otHjiv6g5vmjspWRHI41bfXP0SU49CuJYFBha/n0WMl2IgrQ+0aivMIhNmNukERTu8jf qrbA== X-Gm-Message-State: AD7BkJKVtMuTq//to5BXTkDCczm2g3of0Lek0j12H+fyNLPQ660KAb/gXUPAhJp1R4LhoNLKgMKC0RbnXiCsrg== MIME-Version: 1.0 X-Received: by 10.159.40.168 with SMTP id d37mr2829110uad.83.1458777442336; Wed, 23 Mar 2016 16:57:22 -0700 (PDT) Received: by 10.31.86.132 with HTTP; Wed, 23 Mar 2016 16:57:22 -0700 (PDT) In-Reply-To: <1458689814.10868.29.camel@edumazet-glaptop3.roam.corp.google.com> References: <20160316.223803.240230703927987310.davem@davemloft.net> <1458185652.7353.28.camel@edumazet-glaptop3.roam.corp.google.com> <20160322060220.GA50824@kafai-mba.local> <20160322173939.GA53936@kafai-mba.local> <1458689814.10868.29.camel@edumazet-glaptop3.roam.corp.google.com> Date: Wed, 23 Mar 2016 16:57:22 -0700 Message-ID: Subject: Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket From: Wei Wang To: Eric Dumazet , Martin KaFai Lau , Cong Wang , Eric Dumazet Cc: Wei Wang , David Miller , Linux Kernel Network Developers Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org What about something like this: { @@ -1401,13 +1414,7 @@ void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu, struct dst_entry *dst; struct flowi6 fl6; - memset(&fl6, 0, sizeof(fl6)); - fl6.flowi6_oif = oif; - fl6.flowi6_mark = mark ? mark : IP6_REPLY_MARK(net, skb->mark); - fl6.daddr = iph->daddr; - fl6.saddr = iph->saddr; - fl6.flowlabel = ip6_flowinfo(iph); - + ip6_fill_in_flow(&fl6, net, skb, oif, mark); dst = ip6_route_output(net, NULL, &fl6); if (!dst->error) __ip6_rt_update_pmtu(dst, NULL, iph, ntohl(mtu)); @@ -1417,8 +1424,22 @@ EXPORT_SYMBOL_GPL(ip6_update_pmtu); void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu) { - ip6_update_pmtu(skb, sock_net(sk), mtu, + struct ipv6_pinfo *np = inet6_sk(sk); + struct dst_entry *dst_new; + struct flowi6 fl6; + struct net *net = sock_net(sk); + + ip6_update_pmtu(skb, net, mtu, + sk->sk_bound_dev_if, sk->sk_mark); + + if (sk->sk_state == TCP_ESTABLISHED && + !sk_dst_check(sk, np->dst_cookie)) { + ip6_fill_in_flow(&fl6, net, skb, sk->sk_bound_dev_if, sk->sk_mark); + dst_new = ip6_route_output(net, NULL, &fl6); + if (!IS_ERR(dst_new)) + ip6_dst_store(sk, dst_new, NULL, NULL); + } } EXPORT_SYMBOL_GPL(ip6_sk_update_pmtu); Thanks. Wei On Tue, Mar 22, 2016 at 4:36 PM, Eric Dumazet wrote: > On Tue, 2016-03-22 at 13:13 -0700, Cong Wang wrote: >> On Tue, Mar 22, 2016 at 11:03 AM, Wei Wang wrote: >> > Thanks Martin and Cong. >> > >> > I guess then we are going with the following fix in ip6_sk_update_pmtu(): >> > 1. call ip6_upate_pmtu() as it is >> > 2. do a dst_check() >> > 3. re-lookup() if it is invalid >> > 4. and then do a ip6_dst_store()/dst_set >> >> Exactly, please try the attached patch. Note I did nothing more than a >> compile test. >> >> Does it make sense to you now? > > > Hard to reply on your patch as it was not inlined. > > 1) Lot of code duplication, for some reason I do not yet understand. > > ip6_sk_update_pmtu() and ip6_update_pmtu() will basically do the same > thing... > > 2) > > + if (sk->sk_state == TCP_ESTABLISHED) > + ip6_dst_store(sk, dst, &iph->daddr, &iph->saddr); > +out: > > > ip6_dst_store() will do : > > np->daddr_cache = daddr; (&iph->daddr) > np->saddr_cache = saddr; (&iph->saddr) > > So when skb is freed, daddr_cache & saddr_cache point to freed data. > > > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c index ed44663..21b4102 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1394,6 +1394,19 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk, __ip6_rt_update_pmtu(dst, sk, skb ? ipv6_hdr(skb) : NULL, mtu); } +static void ip6_fill_in_flow(struct flowi6 *fl6, struct net *net, + struct sk_buff *skb, int oif, u32 mark) +{ + const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data; + + memset(fl6, 0, sizeof(fl6)); + fl6->flowi6_oif = oif; + fl6->flowi6_mark = mark ? mark : IP6_REPLY_MARK(net, skb->mark); + fl6->daddr = iph->daddr; + fl6->saddr = iph->saddr; + fl6->flowlabel = ip6_flowinfo(iph); +} + void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu, int oif, u32 mark)