Make options IFLIB a real configuration option.

[sbruno]

Summary

Since IFLIB is required for e1000, ixgbe, bnxt and ixl, make it
dependant on those drivers. This allows little artisinal builds to exclude
iflib and these drivers, shrinking their builds several 10s of kilobytes.

[rgrimes]

Does this not also make it so that these drivers require IFLIB, so they need to be marked as such in GENERIC/NOTES/etc and man pages?

[sbruno]

In D16164#342938, @rgrimes wrote:

Does this not also make it so that these drivers require IFLIB, so they need to be marked as such in GENERIC/NOTES/etc and man pages?

Yeah, definitely. It also means that out of tree drivers (intel, bnxt) wouldn't work if a kernel is built without IFLIB. Is this ok to do?

[rgrimes]

In D16164#342949, @sbruno wrote:

In D16164#342938, @rgrimes wrote:

Does this not also make it so that these drivers require IFLIB, so they need to be marked as such in GENERIC/NOTES/etc and man pages?

Yeah, definitely. It also means that out of tree drivers (intel, bnxt) wouldn't work if a kernel is built without IFLIB. Is this ok to do?

I should think it is ok to do, just requires some additional documentation. And that brings up the question, can iflib.ko be created and then the driver modules depend on that? This may be getting larger in scope than 1 review/1 commit though.

[erj]

In D16164#342950, @rgrimes wrote:

can iflib.ko be created and then the driver modules depend on that?

That would be really nice. :-P

[cem]

In D16164#342951, @erj wrote:

can iflib.ko be created and then the driver modules depend on that?

That would be really nice. :-P

Driver modules already can and do depend on iflib, without forcing it to be a kld object (.ko) — grep for MODULE_DEPEND(..., iflib, ...).

If the idea is pulling iflib out of the kernel just for space savings, I'm not sure that really makes sense on any platform that supports iflib drivers anyway (i.e., non-embedded).

Please do add the IFLIB option to most/all GENERICs simultaneously with this change, though.

[sbruno]

In D16164#342962, @cem wrote:

In D16164#342951, @erj wrote:

can iflib.ko be created and then the driver modules depend on that?

That would be really nice. :-P

Driver modules already can and do depend on iflib, without forcing it to be a kld object (.ko) — grep for MODULE_DEPEND(..., iflib, ...).

If the idea is pulling iflib out of the kernel just for space savings, I'm not sure that really makes sense on any platform that supports iflib drivers anyway (i.e., non-embedded).

Please do add the IFLIB option to most/all GENERICs simultaneously with this change, though.

I don't have a strong objection to adding options IFLIB to all the kernels, but isn't that redundant? I've declared and explicit dependency here for the device drivers that need it.

[sbruno]

@shurd any objections from the peanut gallery?

[sbruno]

trivial bump to generate email.

[sbruno]

bump to see if phabricator shovels some emails.

[shurd]

No objections, though I do agree with cem@ in principle regarding putting it in the GENERICs. When 3rd-party drivers start relying on it, it would violate POLA removing Intel NICs from the kernel would break a 3rd-party driver. For now it's likely "fine".

[cem]

To elaborate a bit more, the stated commit log:

Since IFLIB is required for e1000, ixgbe, bnxt and ixl, make it
dependant on those drivers.

is sort of the opposite of the actual change. The change made here is to make iflib optional and those drivers depend on the iflib option.

If you enable 'device ix' but don't enable option 'iflib', you won't get an ixgbe driver.

You could instead change the iflib files (net/ifdi_if.m, net/iflib.c, net/iflib_clone.c, net/mp_ring.c) to be optional ether pci ix | ether pci ixv | ether pci ixl | ether pci ixlv | ether pci em, which would do something more like what the commit message suggests, but obviously that is ugly and only becomes more ugly as other iflib drivers are added.

[rgrimes]

In D16164#405230, @cem wrote:

To elaborate a bit more, the stated commit log:

Since IFLIB is required for e1000, ixgbe, bnxt and ixl, make it
dependant on those drivers.

is sort of the opposite of the actual change. The change made here is to make iflib optional and those drivers depend on the iflib option.

...
Ouch, that is going to cause some people a bit of head scratching. What if we go the route that was done for mii, simply group these drivers togeather in GENERIC under a comment #These NICs need option iflib and remove the complex and hard to maintain proper files entry that cem@ describes.

[imp]

I'm struggling to understand here. How does this make it a real option? It seems to say that you must include iflib to get these drivers and there will be no warning if you don't. Maybe I'm misunderstanding something here, but that strikes me as unwise on its surface. I don't think this is a good idea at all, unless I'm missing something.

I'd honestly rather create a new keyword 'requies' and use it here for 'requires iflib' but there's some other keywords that are needed to make that go since this really is more of a 'bring it in regardless of whether the kernel config has iflib' which might be better expressed as 'force' or some other keyword.

[sbruno]

This was superceded by @kib in r343617. The purpose of this revision was to provide a simple way to have module support for the various adapters supported by iflib and to cull iflib out when not needed.