Date: Wed, 19 Feb 2014 17:06:27 +0100 From: =?ISO-8859-1?Q?Ermal_Lu=E7i?= <eri@freebsd.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Nikos Vassiliadis <nvass@gmx.com>, src-committers@freebsd.org, Martin Matuska <mm@freebsd.org> Subject: Re: svn commit: r262196 - head/sys/netpfil/pf Message-ID: <CAPBZQG3aZAbnOw5x8TFx8sGVNST2avLkkV8-YSz17YE_aEYdDQ@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Feb 19, 2014 at 4:32 PM, Gleb Smirnoff <glebius@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPBZQG3aZAbnOw5x8TFx8sGVNST2avLkkV8-YSz17YE_aEYdDQ>