Date: Thu, 30 Oct 2014 17:43:06 +0300 From: "Andrey V. Elsukov" <ae@FreeBSD.org> To: Bruce Simpson <bms@fastmail.net>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r273855 - head/sys/netinet6 Message-ID: <54524E7A.1020802@FreeBSD.org> In-Reply-To: <1414668869.979804.185070381.0D4C866C@webmail.messagingengine.com> References: <201410301059.s9UAxwAg055812@svn.freebsd.org> <1414668869.979804.185070381.0D4C866C@webmail.messagingengine.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54524E7A.1020802>