Always install the pf headers, even when building with WITHOUT_PF=yes.
If the headers are missing the header validation step (test-includes)
will fail.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 44058 Build 40946: arc lint + arc unit
Event Timeline
Comment Actions
I tried this and got inconsistent result, indicating a different bug in the header validation. It would be better to omit the troubling headers from that validation until I can sort that out.
Comment Actions
What sort of results did you see? I've tested this with a make universe, and that all builds as I'd expect it to.
This is also something we're going to have to do anyway, unless we change how we install headers from sys/. Those are installed unconditionally, so we always install sys/pfvar.h, which wants to include netpfil/pf/pf.h, which we currently don't install if WITHOUT_PF=yes. So we either always install pf.h, or teach the build system to not install pfvar.h if WITHOUT_PF=yes.
Comment Actions
I saw it working when it shouldn't, and failing when it shouldn't. That indicated that the understanding of the problem is incorrect and that this solution was not appropriate. to me it points more to testing the wrong thing, or we're installing our staging area incorrectly and this is but one symptom. But this may also point to a bigger issue: if we're only installing some of the dependencies, the dependencies are wrong. I'd like to understand that before hitting it with a huge hammer of always installing everything...
This is also something we're going to have to do anyway, unless we change how we install headers from sys/. Those are installed unconditionally, so we always install sys/pfvar.h, which wants to include netpfil/pf/pf.h, which we currently don't install if WITHOUT_PF=yes. So we either always install pf.h, or teach the build system to not install pfvar.h if WITHOUT_PF=yes.
So why are we always installing pfvar.h? If it's part of pf, it should be omitted WITHOUT_PF...
Comment Actions
Agreed. I've not seen any unexpected failures or successes with this though, which is why I asked.
So why are we always installing pfvar.h? If it's part of pf, it should be omitted WITHOUT_PF...
As far as I can tell that's because we always install all of the headers from /usr/src/sys/net (and everything listed in LDIRS in include/Makefile), which includes pfvar.h (most pf headers live in netpfil/pf, but not pfvar.h. Userspace expectations mean we can't change that.). At least, that's what it looks like to me, but I don't understand our build system.
If my understanding is correct we could also fix this problem by teaching the build system to not install pfvar.h when WITHOUT_PF is set, but that's not something I know how to do.
Comment Actions
My big worry is that there's some contamination from the host or from the source tree (since I couldn't recreate this problem just adding WITHOUT_PF to my build). I also had an indication that we might be getting the list from the wrong place since one of the builds couldn't find a file that should have been there, though that only happened once. Based on that, I decided I needed to understand better...
So why are we always installing pfvar.h? If it's part of pf, it should be omitted WITHOUT_PF...
As far as I can tell that's because we always install all of the headers from /usr/src/sys/net (and everything listed in LDIRS in include/Makefile), which includes pfvar.h (most pf headers live in netpfil/pf, but not pfvar.h. Userspace expectations mean we can't change that.). At least, that's what it looks like to me, but I don't understand our build system.
If my understanding is correct we could also fix this problem by teaching the build system to not install pfvar.h when WITHOUT_PF is set, but that's not something I know how to do.
Yea, I'll look at that sometime this week... The last few weeks have been crazy with weird, demanding bugs from $WORK, but I think I've fixed them well enough (I think I got a pathology to stop happening, and made the code robust enough in the face of tiny races when the pathology hits to fail safe) to have some time for this...Is it OK to ask you to sit on this a day or two until this issue bubbles to the top of my list?
Comment Actions
Oh, and it doesn't help that I have a mix of meta and non-meta machines I tested this on, which has added to the muddiness of my understanding of why this might be needed.
Comment Actions
Yes, that's fine. It's not super urgent. It's been broken for a while now, and I'd forgotten about it until Michael Dexter reminded me.
Comment Actions
Kristof, how far are we from getting rid of any dependency on net/pfvar.h outside of kernel build? I started the process back in 2013, but back then it was very far from being accomplished. If we are able to accomplish that now, let's just do that and move it to netpfil/pf and problem solved.
Comment Actions
Pretty far. It's not really been a priority for me.
If nothing else there's an unknowable number of other projects out there that rely on net/pfvar.h including netpfil/pf/pf.h, so I'd be very hesitant to make that move anyway.