Date: Wed, 19 Feb 2014 19:32:15 +0400 From: Gleb Smirnoff <glebius@FreeBSD.org> To: Martin Matuska <mm@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Nikos Vassiliadis <nvass@gmx.com>, src-committers@freebsd.org Subject: Re: svn commit: r262196 - head/sys/netpfil/pf Message-ID: <20140219153215.GD63039@FreeBSD.org> In-Reply-To: <20140219140123.Horde.zrXx5GqkiiaAfGIetc98KQ1@mail.vx.sk> References: <201402182217.s1IMHCeM077356@svn.freebsd.org> <20140219101736.GX63039@glebius.int.ru> <20140219140123.Horde.zrXx5GqkiiaAfGIetc98KQ1@mail.vx.sk>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140219153215.GD63039>