From owner-svn-src-all@FreeBSD.ORG Wed Feb 19 15:32:31 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id E4E8E8B6; Wed, 19 Feb 2014 15:32:30 +0000 (UTC) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 6AEE91307; Wed, 19 Feb 2014 15:32:29 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.8/8.14.8) with ESMTP id s1JFWFW7076483 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 19 Feb 2014 19:32:15 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.8/8.14.8/Submit) id s1JFWFw6076482; Wed, 19 Feb 2014 19:32:15 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Wed, 19 Feb 2014 19:32:15 +0400 From: Gleb Smirnoff To: Martin Matuska Subject: Re: svn commit: r262196 - head/sys/netpfil/pf Message-ID: <20140219153215.GD63039@FreeBSD.org> References: <201402182217.s1IMHCeM077356@svn.freebsd.org> <20140219101736.GX63039@glebius.int.ru> <20140219140123.Horde.zrXx5GqkiiaAfGIetc98KQ1@mail.vx.sk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20140219140123.Horde.zrXx5GqkiiaAfGIetc98KQ1@mail.vx.sk> User-Agent: Mutt/1.5.22 (2013-10-16) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Nikos Vassiliadis , src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Feb 2014 15:32:31 -0000 Martin, M> > On Tue, Feb 18, 2014 at 10:17:12PM +0000, Martin Matuska wrote: M> > M> Author: mm M> > M> Date: Tue Feb 18 22:17:12 2014 M> > M> New Revision: 262196 M> > M> URL: http://svnweb.freebsd.org/changeset/base/262196 M> > M> M> > M> Log: M> > M>   De-virtualize pf_mtag_z [1] M> > M>   Process V_pf_overloadqueue in vnet context [2] M> > M> M> > M>   This fixes two VIMAGE kernel panics and allows to simultaneously M> > run host-pf M> > M>   and vnet jails. pf inside jails remains broken. M> > M> M> > M>   PR:                kern/182964 M> > M>   Submitted by:        glebius@FreeBSD.org [2], myself [1] M> > M>   Tested by:        rodrigc@FreeBSD.org, myself M> > M>   MFC after:        2 weeks M> > M> > I've sent your patch to Nikos, who is working on pf+vimage. He M> > also accumulates his work on pf+vimage in projects/pf branch, M> > planning to do it properly and then merge to head in one go. M> > I was waiting for his review. Yes, he is slow with reviews, M> > but that's not a reason to commit w/o review. On Wed, Feb 19, 2014 at 02:01:23PM +0100, Martin Matuska wrote: M> I understand your point - if anything is broken (or more broken than M> before) I can revert this patch anytime. M> M> FreeNAS and other folks may fork separate branches and we can wait until M> about FreeBSD 12.0 for the patch being reviewed so we can commit it around M> 14.0 - maybe we have switched to a completely different firewall at that M> time and this issue becomes obsolete anyway. No need for sarcasm and top quoting. Since you already got sharp in your reply, let me too. First of all. I did not submitted you [2], right now I just checked my sent mail to ensure that. I submitted you other patch, that later was rejected by zec@, and that patch was very unlike [2]. So statement in commit message is not true. Second, these two changes are absolutely unrelated. They shouldn't been committed as one patch. Third. As you already know, there is projects/pf branch, where Nicos is getting things right wrt pf+VIMAGE. The patches should first go to this branch and tested in it. Committing to head (even a good code), you are creating conflicts for Nicos. You are fixing two particular problems that hurt you, while Nicos tries to get things right in general, for everyones sake. My approach on taskqueue context (that was rejected by Marko), was also an attempt to create a good and generic way of dealing with the problem. Unfortunately, Marko didn't suggest good alternatives. Anyway this is not a reason to plumb problems in place. As you may notice yourself the code you added: if (IS_DEFAULT_VNET(curvnet)) pf_mtag_z = uma_zcreate("pf mtags", sizeof(struct m_tag) + sizeof(struct pf_mtag), NULL, NULL, pf_mtag_init, NULL, UMA_ALIGN_PTR, 0); Is quite not like the rest of the code of the function. That is because in head/ the per-VNET initialization in pf isn't separated from global initialization. This is a generic problem, that Nikos is solving in projects/pf. Making pf mtag zone in projects/pf would be more clean than in head. And of course after your change merge of head to projects/pf would fail. You could join Nikos efforts, but instead you are just putting obstacles on his way. And mine too, since I would do next merge. -- Totus tuus, Glebius.