From owner-svn-src-head@FreeBSD.ORG Thu Oct 30 14:43:11 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx2.freebsd.org (mx2.freebsd.org [8.8.178.116]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id E7B272E4; Thu, 30 Oct 2014 14:43:11 +0000 (UTC) Received: from butcher-nb.yandex.net (hub.freebsd.org [IPv6:2001:1900:2254:206c::16:88]) by mx2.freebsd.org (Postfix) with ESMTP id C82651900; Thu, 30 Oct 2014 14:43:10 +0000 (UTC) Message-ID: <54524E7A.1020802@FreeBSD.org> Date: Thu, 30 Oct 2014 17:43:06 +0300 From: "Andrey V. Elsukov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Bruce Simpson , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r273855 - head/sys/netinet6 References: <201410301059.s9UAxwAg055812@svn.freebsd.org> <1414668869.979804.185070381.0D4C866C@webmail.messagingengine.com> In-Reply-To: <1414668869.979804.185070381.0D4C866C@webmail.messagingengine.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Oct 2014 14:43:12 -0000 On 30.10.2014 14:34, Bruce Simpson wrote: > Hello, > > This is a really inconvenient time for me (I am up against a deadline) > but I am not 100% comfortable with this change. > > On Thu, 30 Oct 2014, at 10:59, Andrey V. Elsukov wrote: >> Log: >> Fix mbuf leak in IPv6 multicast code. >> When multicast capable interface goes away, it leaves multicast groups, >> this leads to generate MLD reports, but MLD code does deffered send and >> MLD reports are queued in the in6_multi's in6m_scq ifq. The problem is >> that in6_multi structures are freed when interface leaves multicast >> groups >> and thread that does deffered send will not take these queued packets. > > A few comments: > > 1) Stylistic -- a change of this kind should probably be part of > inm_purge() itself because it modifies state which is private to the > group membership. > > 2) Logical -- The patch forces pending (queued) state change record > fragments to be freed when the parent interface is taken down. > Unfortunately, those are pending for a reason; there has been a state > change, and MLD needs to communicate it upstream to on-link routers (and > snooping switches). > > So - there is a risk with this approach that upstream MLD listener (e.g. > router, switch) will be inconsistent, at least until the next General > Query. I'm not quite sure, but I think that the leak happened only when interface disappeared. In case when system just leaves the group, MLD code takes reference to in6_multi and releases it when fasttimo handler dispatches packets. When interface is disappearing, in6_ifdetach() calls in6_purgemaddrs(), where all references to in6_multi released again. Now in6m_release_locked() will drain queue when in6_multi has no more references. -- WBR, Andrey V. Elsukov