From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Abeni Subject: Re: [PATCH net-next 3/3] ipv6: obsolete cached dst when removing them from fib tree Date: Wed, 18 Oct 2017 15:03:02 +0200 Message-ID: <1508331782.2559.5.camel@redhat.com> References: <59304478d82e52c817c92d183ef2105f35c17266.1508261949.git.pabeni@redhat.com> <1508270578.2548.22.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Linux Kernel Network Developers , "David S. Miller" , Hannes Frederic Sowa To: Wei Wang , Eric Dumazet , Martin KaFai Lau , Xin Long Return-path: Received: from mx1.redhat.com ([209.132.183.28]:41072 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753046AbdJRNDF (ORCPT ); Wed, 18 Oct 2017 09:03:05 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 2017-10-17 at 13:48 -0700, Wei Wang wrote: > On Tue, Oct 17, 2017 at 1:02 PM, Paolo Abeni wrote: > > Meanwhile others sockets may grab more references to (and use) the same > > aged-out dst. > > > > I don't think other sockets could grab more reference to this dst > because this dst should already be removed from the fib6 tree. With the current net-next code, the dst is not removed from the fib tree while someone else is holding it and dst_check() does not fail after that the cached dst is aged out. If a socket cache grab a reference to the CACHE dst, it will not release it untill the next sernum change, regardless of the dst aging. > > The commit 1e2ea8ad37be ("ipv6: set dst.obsolete when a cached route > > has expired") was the solution to the above issue prior to the recent > > refactor. > > > > I don't really understand how this commit is solving the above issue. > This commit still only ages out cached route if &rt->dst.__refcnt == > 1. So if socket is holding refcnt to this dst and dst_check() is not > getting called, this cached route still won't get deleted. Setting obsolete to DST_OBSOLETE_KILL forced whoever was holding the dst reference to drop it on the next dst_check(), so that refcnt could go down. Cheers, Paolo