diff mbox series

Unnecessary extension of default packeges for x86 generic

Message ID bba393157c85fc7671b5b707b82e74dc@dev.tdt.de
State Not Applicable
Delegated to: Petr Štetiar
Headers show
Series Unnecessary extension of default packeges for x86 generic | expand

Commit Message

Florian Eckert March 1, 2021, 10:40 a.m. UTC
Hello,

Is it really necessary that we keep expanding the default package here 
[1]?
The problem is, for example, that I don't need the whole AWS stuff.
But now the whole package gets installed.

If this is the case, then we should also install the vmware needed 
driver as default package.
Maybe someone (like me) would like to run the image in a vmware 
environment.
But I do not want to install this per default.

I think this are all special image.
Therefore it would be a good idea to create individual images.
For example
* define Device/aws
* define Device/vmware


Also, binary blobs are currently already get installed as a dependency 
that the default drivers work.
Therefore I have changed the line before this change as following.
So no binary blob dependency gets installed.
I am also not clear why we need kmod-bnx2, kmod-forcedeth and kmod-r8169 
as default, which have the binary blob dependency.

  TARGET_DEVICES += generic
--

 From my point of view, these are the minimum packets needed to make the 
generic image to work.


Kind Regards

Florian

[1] 
https://github.com/openwrt/openwrt/commit/788ec9a7cfc843c8e46451331521cdc7d81866bd

Comments

Alberto Bursi March 1, 2021, 7:43 p.m. UTC | #1
On 01/03/21 11:40, Florian Eckert wrote:
> Hello,
> 
> Is it really necessary that we keep expanding the default package here [1]?
> The problem is, for example, that I don't need the whole AWS stuff.
> But now the whole package gets installed.
> 
> If this is the case, then we should also install the vmware needed 
> driver as default package.
> Maybe someone (like me) would like to run the image in a vmware 
> environment.

Please look at the kernel config. x86_64 kernel config has *all* drivers 
for KVM, for Xen and VMware and Hyper-V (and some AWS) compiled in 
already and always did for a long while.

I had to add the AWS driver as a module so that people that don't want 
it can remove it with ImageBuilder (which is not something you cannot do 
with drivers compiled in  by the kernel config)

> But I do not want to install this per default.
> 
> I think this are all special image.

No it's not, unless somehow AWS virtualization is treated differently 
from KVM, Xen, VMWare and Hyper-V, which as I said are already included 
by default in the x86_64 kernel.

I personally don't see the point for making multiple images for x86_64 
due to the fact that space and resource usage for leaving all in is 
negligible at best on this target.

> Therefore it would be a good idea to create individual images.
> For example
> * define Device/aws
> * define Device/vmware
> 
> 
> Also, binary blobs are currently already get installed as a dependency 
> that the default drivers work.
> Therefore I have changed the line before this change as following.
> So no binary blob dependency gets installed.
> I am also not clear why we need kmod-bnx2, kmod-forcedeth and kmod-r8169 
> as default, which have the binary blob dependency.

because there are boards and network cards with broadcomm integrated 
ethernet, boards with NVIDiA chipsets need this to make the onboard 
ethernet work and pretty much all cheap motherboards have a Realtek 
ethernet port so we need that included.

If you don't like the default package selection, just use the Image 
Builder to assemble your own image from pre-compiled packages in the 
repos where you can choose what packages you want and what you don't want.

> 
> --- a/target/linux/x86/image/64.mk
> +++ b/target/linux/x86/image/64.mk
> @@ -1,7 +1,6 @@
>   define Device/generic
>     DEVICE_TITLE := Generic x86/64
> -  DEVICE_PACKAGES += kmod-bnx2 kmod-e1000e kmod-e1000 kmod-forcedeth 
> kmod-igb \
> -       kmod-ixgbe kmod-r8169
> +  DEVICE_PACKAGES += kmod-e1000e kmod-e1000 kmod-igb kmod-ixgbe
>     GRUB2_VARIANT := generic
>   endef
>   TARGET_DEVICES += generic
> -- 
> 
>  From my point of view, these are the minimum packets needed to make the 
> generic image to work.
> 

As mentioned above, this cuts out all non-premium consumer motherboards 
that use realtek chipsets and need kmod-r8169 (which is A LOT), plus the 
less common boards with broadcomm ethernet and the even less common ones 
with NVIDIA chipset.

The generic image is supposed to work on most if not all possible 
hardware, because it's built for the less tech-savyy users that can't 
just recompile or use the image builder, and having no working network 
interface is a problem.


> 
> Kind Regards
> 
> Florian
> 
> [1] 
> https://github.com/openwrt/openwrt/commit/788ec9a7cfc843c8e46451331521cdc7d81866bd 
>
Florian Eckert March 2, 2021, 7:27 a.m. UTC | #2
Hello Alberto,

Thank you for the explanation.
If that is so then we should also remove the vmware driver as a kmod 
package [1].
That makes no sense if this is then already in the kernel enabled [2].

Kind Regards

Florian

[1] 
https://github.com/openwrt/openwrt/blob/master/package/kernel/linux/modules/netdevices.mk#L1007
[2] 
https://github.com/openwrt/openwrt/blob/1ca5de13a153061feae260864d73d96f7c463785/target/linux/x86/64/config-5.4#L440
Alberto Bursi March 2, 2021, 3:48 p.m. UTC | #3
On 02/03/21 08:27, Florian Eckert wrote:
> Hello Alberto,
> 
> Thank you for the explanation.
> If that is so then we should also remove the vmware driver as a kmod 
> package [1].
> That makes no sense if this is then already in the kernel enabled [2].
> 
> Kind Regards
> 
> Florian
> 
> [1] 
> https://github.com/openwrt/openwrt/blob/master/package/kernel/linux/modules/netdevices.mk#L1007 
> 
> [2] 
> https://github.com/openwrt/openwrt/blob/1ca5de13a153061feae260864d73d96f7c463785/target/linux/x86/64/config-5.4#L440 
> 

Good catch, given the feedback I received when I added the AWS support, 
if we touch this we should drop the vmware driver from the kernel and 
add the kmod to default package list of x86_64. So that people can use 
ImageBuilder to make images without this package if they want.

Also the same idea applies to all other non-critical VM drivers which 
should be moved to a kmod and added to default package list unless they 
are required to mount the rootfs (the current AWS Elastic something, and 
the KVM VirtIO drivers for example) or cause breakage otherwise.

I'm personally not very interested in doing this conversion because as I 
mentioned before I don't care much about this being compiled in and I 
would not remove it.

But maybe you are interested in doing this or at least a part of it for 
vmware or kvm or whatever hypervisor you are most familiar with.

See the discussion on the github PR here [1]

[1] https://github.com/openwrt/openwrt/pull/3799#discussion_r559839567


-Alberto
diff mbox series

Patch

--- a/target/linux/x86/image/64.mk
+++ b/target/linux/x86/image/64.mk
@@ -1,7 +1,6 @@ 
  define Device/generic
    DEVICE_TITLE := Generic x86/64
-  DEVICE_PACKAGES += kmod-bnx2 kmod-e1000e kmod-e1000 kmod-forcedeth 
kmod-igb \
-       kmod-ixgbe kmod-r8169
+  DEVICE_PACKAGES += kmod-e1000e kmod-e1000 kmod-igb kmod-ixgbe
    GRUB2_VARIANT := generic
  endef