On Tue, Mar 22, 2016 at 4:36 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Tue, 2016-03-22 at 13:13 -0700, Cong Wang wrote: >> On Tue, Mar 22, 2016 at 11:03 AM, Wei Wang <tracyw...@gmail.com> 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.
Sorry about that, it is still not ready to send out formally yet, because I don't do any run-time tests yet. > > 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... Hmm? After my patch ip6_sk_update_pmtu() will take care of sk_dst_cache, but ip6_update_pmtu() doesn't do that since it has no 'sk' parameter. It is pretty much similar to commit 9cb3a50c5f63. > > 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. Ah, good catch, so ip6_dst_store() can only use the addr's stored in socket or NULL. I will send out 2 formal patches shortly.
