From owner-svn-src-all@FreeBSD.ORG Wed Feb 19 16:06:27 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 C00EF5D5; Wed, 19 Feb 2014 16:06:27 +0000 (UTC) Received: from mail-pd0-x229.google.com (mail-pd0-x229.google.com [IPv6:2607:f8b0:400e:c02::229]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 7B4921699; Wed, 19 Feb 2014 16:06:27 +0000 (UTC) Received: by mail-pd0-f169.google.com with SMTP id v10so556301pde.0 for ; Wed, 19 Feb 2014 08:06:27 -0800 (PST) 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=z+dgbdEA0hz1wuHJx9L/hrBOZvaZGMfKzvMk7Qr5wLg=; b=X3sOGTUzpeiJei8VUHaxMa69yKl5rCqYTrLmBTlNiEcVRQmbyX5/DOSN9m/LuYImKe Nsuq6edB0Nf0tLi/wO/zmFzO4t63+WgLZc2RNsoU0cp5ji0fX9j851qd6hLk76MRpznQ Wvv9YZX2ut/8tWhHXsEs47/AfdSvV0i+TvpY7xDRZSJvH6hIIvLrZeHUkpvrzNrvoPPA ZtzB1VMHBye/zu4m0GMkoKnkVu4I16dWazIvXgPWVTYBym6lClPNfyJAr2mbQqtTWpQU wZXgByobQUARyWq2Z2GrYXf3LQJWhxIfboInzKDOi/6MEJWyL8xHTgI40eUrKPAt1UjA w1dg== MIME-Version: 1.0 X-Received: by 10.68.129.201 with SMTP id ny9mr3178159pbb.70.1392825987096; Wed, 19 Feb 2014 08:06:27 -0800 (PST) Sender: ermal.luci@gmail.com Received: by 10.70.6.36 with HTTP; Wed, 19 Feb 2014 08:06:27 -0800 (PST) In-Reply-To: <20140219153215.GD63039@FreeBSD.org> References: <201402182217.s1IMHCeM077356@svn.freebsd.org> <20140219101736.GX63039@glebius.int.ru> <20140219140123.Horde.zrXx5GqkiiaAfGIetc98KQ1@mail.vx.sk> <20140219153215.GD63039@FreeBSD.org> Date: Wed, 19 Feb 2014 17:06:27 +0100 X-Google-Sender-Auth: 3NjY7VA1iImMZmy5RwrQpV1mtxE Message-ID: Subject: Re: svn commit: r262196 - head/sys/netpfil/pf From: =?ISO-8859-1?Q?Ermal_Lu=E7i?= To: Gleb Smirnoff Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.17 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Nikos Vassiliadis , src-committers@freebsd.org, Martin Matuska 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 16:06:27 -0000 On Wed, Feb 19, 2014 at 4:32 PM, Gleb Smirnoff wrote: > 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. > > Well go do some work instead of runting around. You did not listen to me as well when you started doing work on pf. > -- > Totus tuus, Glebius. > -- Ermal