- In configurations with a pseudo devices section, move 'device crypto' into that section.
- Use a consistent comment. Note that other things common in kernel configs such as GELI also require 'device crypto', not just IPSEC.
Details
Details
- tinderbox build
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| sys/mips/conf/MALTA64EL | ||
|---|---|---|
| 12 ↗ | (On Diff #46848) | Did this sneak in, this seams to be unrelated and belong in a separate review/commit. |
Comment Actions
If using option IPSEC (and I guess GELI and maybe other things) requires explicitly specifying device crypto, then shouldn't this change also cleanup and simplify sys/conf/files to remove "| ipsec | ipsec_support" from all the files that are optional on "crypto"?
And as long as I'm on about consistancy, if we're going to start specifying devices in std.arm* then shouldn't we comb the existing arm kernel config files to find all the common [pseudo-]devices and move them into std.arm* as well? I don't remember why we decided that the std.arm* files would contain only options, but I do vaguely remember discussing it; it wasn't by accident.
Comment Actions
IMHO this change standsalone as a "bring consistency" to how crypto is optioned in the GENERIC/std.* kernel confs. That being said, I would support another differential/commit that cleans up the sys/conf/files stuff, if infact there is an issue there, but from what I can see it is valid to have crypto without IPSEC, so I dont think much can be done.
Comment Actions
+1 (eventually, does not have to be in this changeset).
| sys/arm/conf/std.armv7 | ||
|---|---|---|
| 12 ↗ | (On Diff #46848) | Should this one be ordered in a separate devices section, like other conf files? I'll admit I don't really understand the distinction we draw between options and devices. |
| sys/sparc64/conf/GENERIC | ||
| 232 ↗ | (On Diff #46848) | (Just picking one.) Maintaining the "Required by IPSEC" makes it convenient to grep for the presence of both options IPSEC and device crypto. I don't feel strongly about this, though. |
Comment Actions
I do think we shouldn't tag 'crypto' on 'IPSec' files nor 'IPSec' on crypto files. Ok, I've looked at sys/conf/files and I do think it needs cleaning up. In particular, crypto algorithms that have a software OCF binding should have 'crypto' in their list so that OCF supports them. Things like IPSec and GELI that use OCF for crypto should not be listed in the list of options for crypto algorithms, only things that directly use the API exported by the file in question.
As far as std.arm*: I don't know why it only has options. It seems to me that 'std.*' files should be fine to have devices as well. Note that 'std.MALTA' in sys/conf/mips has both devices and options. I think moving "standard" devices into std.arm* to reduce duplication in kernel configurations for things that are a truly "standard" thing (which pseudo-devices probably are) would probably be a good thing, but that probably needs to be run by Warner.
The only real difference between 'option' and 'device' is that an 'option' always defines a macro in an "opt_foo.h" header. Devices didn't used to do that but now define a DEV_FOO macro in an opt_foo.h header IFF a DEV_FOO option exists in sys/conf/options.
| sys/mips/conf/MALTA64EL | ||
|---|---|---|
| 12 ↗ | (On Diff #46848) | Yes, it's not part of this change. |
| sys/sparc64/conf/GENERIC | ||
| 232 ↗ | (On Diff #46848) | With the current indentation it doesn't fit in 80 cols otherwise I might have kept it. OTOH, to be accurate it might also need to list other things (ECKD, etc.) depending on the kernel config. We don't generally document those dependencies here, so I stuck with the existing comment from sys/conf/NOTES. |
| sys/sparc64/conf/GENERIC | ||
|---|---|---|
| 232 ↗ | (On Diff #46848) | That's reasonable. |
Comment Actions
I'd love it, in general. I'd started several times in the past, and got too much pushback on the design so never finalized it.
I'd love to see, btw, kernel config files look generally like:
ident FOO
include std.soc
other stuff here.
and then std.soc would include other standard things for that SoC (addresses, nexus bus, etc) and include a std.$arch which includes all the arch specific std things. That should then include std.all which has all the really standard things.
This is a great mental model, but breaks down when you want to do things like 'all cam things' or 'all pci' things since it becomes a Venn diagram. So if we were to go that way, we should look at a good design. though if we're going to put that much work into it, maybe a more-standard format would allow us to leverage more tools to more quickly replace things entirely.
tl;dr: conceptually I'm on board, run the actual changes by me would be really nice.
Comment Actions
Generally, I like what's presented here. There's much we need to cleanup, I think, but this is a good start.