On Tue, Mar 22, 2016 at 09:53:35AM -0700, Cong Wang wrote: > On Mon, Mar 21, 2016 at 11:02 PM, Martin KaFai Lau <ka...@fb.com> wrote: > > In term of difference, AFAICT, the current patch is an optimization in the > > sense that the update_pmtu() code path does not have to do a dst_check to > > discover its sk->sk_dst_cache is invalid, and then do a relookup to find out > > that the just created RTF_CACHE clone should be used. To get this, it may > > make more sense to remove all the relookup code together during > > update_pmtu(). > > Even if this slow path was to be optimized, should it be put in a > > separate patch where net-next is a better candidate? > > > > Speaking of RTF_CACHE, I am curious why you didn't use FIB next hop exception > as what ipv4 does to cache exceptions? This makes IPv6 has more gap with IPv4. > This is (almost) irrelevant to this patch. There are a few differences between IPv6 and IPv4. Both in terms of data structure and functionality. The last 'RTF_CACHE on exception' patchset is one step toward this direction. More patches are needed and are welcomed ;)
> > > > I think fixing it in __udp6_lib_err() or what Cong Wang is suggesting makes > > more sense for a net branch fix. If there is logic specific to > > connected-udp, > > I would do it in the __udp6_lib_err() instead. After looking at > > udpv6_sendmsg() and how it calls ip6_dst_store(), may also need to be > > careful > > what daddr and saddr should be passed to ip6_dst_store(), or at least a > > commit > > message. The first patch is essentially passing NULL to daddr and saddr > > while the second patch seems passing something else. > > Raw socket needs to fix too, we can't just fix __udp6_lib_err(), this is also > why fixing ip6_sk_update_pmtu() is better, its call path is better. I don't see rawv6 socket is storing the dst. I probably have overlooked it. Can you point it out? Having said that, I don't feel strongly on any of the two places. I think only implementation can tell.
