From owner-svn-src-head@FreeBSD.ORG Thu Oct 30 16:06:00 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D1AE98F5; Thu, 30 Oct 2014 16:06:00 +0000 (UTC) Received: from mail-wg0-x22e.google.com (mail-wg0-x22e.google.com [IPv6:2a00:1450:400c:c00::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F3EC7D72; Thu, 30 Oct 2014 16:05:59 +0000 (UTC) Received: by mail-wg0-f46.google.com with SMTP id x13so6019457wgg.5 for ; Thu, 30 Oct 2014 09:05:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=kFnNidkO9mgcO10DmgkIfIOCEaiQywCHqpedEJSEkvw=; b=FHN1jlXIVtwOCTg1c17XuRJ4NOTfX5RTNraTIKq7QNKoI0vAdJFAQ3uDN6M5NjBzZV MLQcg/M1euEyXvkLsFViu3XxnwKtb+wiIJvVmPcnU2PlB2QH+8SZ6wZncKHge3gLsvbd L1/R8cifmapPUEYdZT+KmMwaWdvYPnGKOgamBc2+gvsZvYezBo1PdIAMLHbUWPw6NfXR mdcFFN55Zt6UhFmVKrZhMcdpnpojikmdsbpnJplEibsE3aO+AYMnluEpvP1FWHM/WusK 5fd9f/jyN4AwlRcm3nm69Ntsn+O9VsaS6wUVAlUkKGFMY6eQYPHxqCl/yTr2uTAml+/p +oCA== MIME-Version: 1.0 X-Received: by 10.180.83.98 with SMTP id p2mr12956404wiy.20.1414685158191; Thu, 30 Oct 2014 09:05:58 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.216.106.136 with HTTP; Thu, 30 Oct 2014 09:05:58 -0700 (PDT) In-Reply-To: <54524E7A.1020802@FreeBSD.org> References: <201410301059.s9UAxwAg055812@svn.freebsd.org> <1414668869.979804.185070381.0D4C866C@webmail.messagingengine.com> <54524E7A.1020802@FreeBSD.org> Date: Thu, 30 Oct 2014 09:05:58 -0700 X-Google-Sender-Auth: 07BV6kgnY5Faqvg5MVqT0cjbQmg Message-ID: Subject: Re: svn commit: r273855 - head/sys/netinet6 From: Adrian Chadd To: "Andrey V. Elsukov" Content-Type: text/plain; charset=UTF-8 Cc: "svn-src-head@freebsd.org" , Bruce Simpson , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" 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 16:06:01 -0000 Btw - this is also a problem in the wifi stack. When you destroy an interface, do you want to make sure the STA "going away" frames go out and are ACKed before you finish tearing down the interface? We don't really have any framework / standards in place for how to put tearing down an interface on hold whilst we notify things and wait for packets to finish transmitting, so we don't bother. (I had the same issue with net80211 / ath and transmit buffers hanging around after I destroyed a VAP. They came from the "disassociate" packet being generated when the interface was being torn down.) -adrian On 30 October 2014 07:43, Andrey V. Elsukov wrote: > 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 >