Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 15 Feb 2015 13:33:19 -0800
From:      Craig Rodrigues <rodrigc@FreeBSD.org>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, Nikos Vassiliadis <nvass@gmx.com>, "src-committers@freebsd.org" <src-committers@freebsd.org>
Subject:   Re: svn commit: r276747 - head/sys/netpfil/pf
Message-ID:  <CAG=rPVd8VkPLHDQqBZPWmFA93%2BR26qUAwQGT2YUpxYJ8hZFJHA@mail.gmail.com>
In-Reply-To: <CAG=rPVfK-Qmh7E_%2B-Mmo8kTrDWq=%2B%2ByVCzHNaQ=5pQh7aSaESQ@mail.gmail.com>
References:  <201501060903.t06934qp081875@svn.freebsd.org> <20150122012709.GM15484@FreeBSD.org> <54C16715.6060701@gmx.com> <20150122222314.GO15484@FreeBSD.org> <CAG=rPVc2YLB-3ZyxDZTxDpkt6R8E_Sf1U%2BWWwKrFB2dxTXGENQ@mail.gmail.com> <20150215190100.GQ15484@FreeBSD.org> <CAG=rPVfK-Qmh7E_%2B-Mmo8kTrDWq=%2B%2ByVCzHNaQ=5pQh7aSaESQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Feb 15, 2015 at 1:26 PM, Craig Rodrigues <rodrigc@freebsd.org>
wrote:

> On Sun, Feb 15, 2015 at 11:01 AM, Gleb Smirnoff <glebius@freebsd.org>
> wrote:
>
>>   Craig,
>>
>> On Sat, Feb 14, 2015 at 07:35:53PM -0800, Craig Rodrigues wrote:
>> C> > On Thu, Jan 22, 2015 at 10:09:41PM +0100, Nikos Vassiliadis wrote:
>> C> > N> > Sorry guys, I backed this out due to broken kldunload of pf
>> module,
>> C> > which
>> C> > N> > is critical when you are working with pf bugs.
>> C> > N>
>> C> > N> For sure. 100% understood.
>> C> > N>
>> C> > N> > I had to backout r276746 as well, since it has numerous build
>> C> > breakages,
>> C> > N> > that are addressed by later revisions.
>> C> > N> >
>> C> > N> > That's my fault that I don't review in time, and I will try to
>> improve
>> C> > N> > the situation.
>> C> > N> >
>> C> > N> > Can you please replay r276746 again, addressing all the build
>> problems
>> C> > N> > and send the patch to me? You can user reviews.freebsd.org if
>> you
>> C> > want.
>> C> > N> >
>> C> > N> > I'd like to get this in, but in a better quality.
>> C> > N>
>> C> > N> I'd like to get involved again and help you fixing pf. Craig
>> could you
>> C> > N> replay 276746?
>> C>
>> C> I wish you could have fixed the pf unload problem without backing out
>> C> all these changes.  I took all these changes from your projects/pf
>> branch,
>> C> which was starting to bitrot because it was not being sync'd with head.
>> C>
>> C> I got confirmation from several people that the fixes as they were
>> (after
>> C> the build break fixes),
>> C> actually fixed their issues with PF and VIMAGE, which have been
>> pending for
>> C> several
>> C> years now with no visible progress made.
>> C>
>> C> Most regular users of PF don't really kldunload it once it is used.
>> C> For development use, I've been testing inside bhyve VM's, which doesn't
>> C> solve the kldunload problem but allows testing and forward progress.
>> C>
>> C> Why do you want me to replay 276746 and give you a patch?
>> C>
>> C> Why don't you just do it yourself?
>>
>> Because it is responsibility of the committer, who broke something, to
>> fix it, not mine.
>>
>> Nevertheless, I tried to do that, and it took more than couple of hours,
>> but
>> I failed to untangle all the problems that r276746 brought and proceeded
>> with
>> checkout.
>>
>
>
> What are the problems in 276746 that you were unhappy with
> besides being unable to kldunload PF?
>
> With 276746, without the follow-on fixes, if you kldunload PF
> while VIMAGE is unabled, the kernel panics.  That seems worse
> than being unable to kldunload PF.
>
> If I submit another patch, I am just going to commit it,
> but I don't know if you will be unhappy and revert it.
>


By the way, it would be helpful if you could provide feedback in
Phabricator.  When I created those Phabricator reviews, I added
you as a reviewer to all of them, so you can't say that you didn't see
the patches.
You did not provide feedback on any of them:

https://reviews.freebsd.org/D1309
https://reviews.freebsd.org/D1312
https://reviews.freebsd.org/D1313
https://reviews.freebsd.org/D1315

Please take some time to go and provide feedback in those
reviews, so that a better patch can be made that makes you happy.

--
Craig



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG=rPVd8VkPLHDQqBZPWmFA93%2BR26qUAwQGT2YUpxYJ8hZFJHA>