Skip to content

Commit

Permalink
Revert "mvebu: switch default kernel to 5.15"
Browse files Browse the repository at this point in the history 
This reverts commit 5429411 as upstream
in commit e5f31552674e ("ethernet: fix PTP_1588_CLOCK dependencies") has
changed `PTP_1588_CLOCK` dependency handling in 5.15 kernel.

That currently leads to `CONFIG_NET_DSA_MV88E6XXX=m` in images produced
by buildbots due to `CONFIG_ALL_KMODS=y` config option being used in
those builds, which leads to a broken LAN bridge network on several
devices.

References: https://lists.infradead.org/pipermail/openwrt-devel/2022-December/039950.html
Signed-off-by: Petr Štetiar <ynezz@true.cz>
  • Loading branch information
@ynezz
ynezz committed Dec 4, 2022
1 parent 8719f73 commit 97c77ff
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion target/linux/mvebu/Makefile
View file
Expand Up @@ -9,7 +9,8 @@ BOARDNAME:=Marvell EBU Armada
FEATURES:=fpu usb pci pcie gpio nand squashfs ramdisk boot-part rootfs-part legacy-sdcard targz
SUBTARGETS:=cortexa9 cortexa53 cortexa72

KERNEL_PATCHVER:=5.15
KERNEL_PATCHVER:=5.10
KERNEL_TESTING_PATCHVER:=5.15

include $(INCLUDE_DIR)/target.mk

Expand Down

19 comments on commit 97c77ff

@pesa1234
Copy link

@pesa1234 pesa1234 commented on 97c77ff Dec 4, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask why? Thanks

Sorry, it is written on commit, right question is when...

Sorry

@diabl0w
Copy link

@diabl0w diabl0w commented on 97c77ff Dec 5, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to pretend I understand all this, but just in case it helps... divested openwrt builds based on 5.15 run fine on my wrt1900ac, but "official" openwrt december 4th snapshot has the lan port not working issue. so something that the divested builds did fix it:
https://divested.dev/unofficial-openwrt-builds/mvebu-linksys/20221203-00/

@ynezz
Copy link
Member Author

@ynezz ynezz commented on 97c77ff Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so something that the divested builds did fix it:

Nope, looking into their build config they simply don't enable CONFIG_ALL_KMODS=y so they don't hit that issue.

@johnnyslee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

diff --git a/package/kernel/linux/modules/netdevices.mk b/package/kernel/linux/modules/netdevices.mk
index 518d83f9a2..4e364b29e9 100644
--- a/package/kernel/linux/modules/netdevices.mk
+++ b/package/kernel/linux/modules/netdevices.mk
@@ -7,6 +7,18 @@
 
 NETWORK_DEVICES_MENU:=Network Devices
 
+define KernelPackage/mv88e6xxx
+  SUBMENU:=$(NETWORK_DEVICES_MENU)
+  TITLE:=Marvell 88E6xxx Ethernet switch fabric support
+  KCONFIG:= CONFIG_NET_DSA_MV88E6XXX
+  HIDDEN:=1
+  FILES:=$(LINUX_DIR)/drivers/net/dsa/mv88e6xxx/mv88e6xxx.ko@ge5.15
+  AUTOLOAD:=$(call AutoProbe,mv88e6xxx)
+endef
+
+$(eval $(call KernelPackage,mv88e6xxx))
+
+
 define KernelPackage/sis190
   SUBMENU:=$(NETWORK_DEVICES_MENU)
   TITLE:=SiS 190 Fast/Gigabit Ethernet support
diff --git a/target/linux/mvebu/image/cortexa9.mk b/target/linux/mvebu/image/cortexa9.mk
index d9738903fb..ae0bafc2ea 100644
--- a/target/linux/mvebu/image/cortexa9.mk
+++ b/target/linux/mvebu/image/cortexa9.mk
@@ -110,6 +110,7 @@ define Device/linksys
   $(Device/NAND-128K)
   DEVICE_VENDOR := Linksys
   DEVICE_PACKAGES := kmod-mwlwifi wpad-basic-wolfssl
+  DEVICE_PACKAGES += kmod-mv88e6xxx
   IMAGES += factory.img
   IMAGE/factory.img := append-kernel | pad-to $$$$(KERNEL_SIZE) | \
 	append-ubi | pad-to $$$$(PAGESIZE)
@@ -293,6 +294,7 @@ define Device/solidrun_clearfog-pro-a1
   KERNEL_INSTALL := 1
   KERNEL := kernel-bin
   DEVICE_PACKAGES := mkf2fs e2fsprogs partx-utils
+  DEVICE_PACKAGES += kmod-mv88e6xxx
   IMAGES := sdcard.img.gz
   IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip | append-metadata
   DEVICE_DTS := armada-388-clearfog-pro armada-388-clearfog-base
Package: kmod-mv88e6xxx
Version: 5.15.82-1
Depends: kernel (=5.15.82-1-2d0868c7c81909698b88d1222df7ac5c)
License: GPL-2.0
Section: kernel
Architecture: arm_cortex-a9_vfpv3-d16
Installed-Size: 50718
Filename: kmod-mv88e6xxx_5.15.82-1_arm_cortex-a9_vfpv3-d16.ipk
Size: 50850
SHA256sum: 81efbec823d0ffe07b60d97e72b12a59e98f7dda08de28d76fdbe6ff9e10272d
Description:  Marvell 88E6xxx Ethernet switch fabric support

I have this kmod built, but not yet tested, meaning I'm not sure if the AUTOLOAD working or not.

@ynezz
Copy link
Member Author

@ynezz ynezz commented on 97c77ff Dec 12, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the network switch available as a module is going to break rootfs over NFS use case and maybe some others. Having PTP_1588_CLOCK=y is not an option for me either as it bloats the kernel, increases the kernel attack surface. So I'm leaning more towards a hack/patch approach, by reverting that PTP_1588_CLOCK_OPTIONAL tristate dependency.

@johnnyslee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following change seems to meet the requirements

diff --git a/package/kernel/linux/modules/other.mk b/package/kernel/linux/modules/other.mk
index c5f944ed31..94f7584cf2 100644
--- a/package/kernel/linux/modules/other.mk
+++ b/package/kernel/linux/modules/other.mk
@@ -1079,6 +1079,7 @@ define KernelPackage/ptp
   SUBMENU:=$(OTHER_MENU)
   TITLE:=PTP clock support
   DEPENDS:=+kmod-pps
+  DEPENDS+=@!(TARGET_mvebu&&LINUX_5_15)
   KCONFIG:= \
 	CONFIG_PTP_1588_CLOCK \
 	CONFIG_NET_PTP_CLASSIFY=y

grep KMODS .config

CONFIG_ALL_KMODS=y

grep -E '(PTP_1588|MV88E6X)' build_dir/target-arm_cortex-a9+vfpv3-d16_musl_eabi/linux-mvebu_cortexa9/linux-5.15.82/.config

CONFIG_NET_DSA_MV88E6XXX=y
# CONFIG_PTP_1588_CLOCK is not set
CONFIG_PTP_1588_CLOCK_OPTIONAL=y

but impact on other targets/linux-5.10 is unknown.

@msdos03
Copy link
Contributor

@msdos03 msdos03 commented on 97c77ff Dec 13, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this patch is reasonable. Also include kirkwood in the blacklist please.

The following change seems to meet the requirements

diff --git a/package/kernel/linux/modules/other.mk b/package/kernel/linux/modules/other.mk
index c5f944ed31..94f7584cf2 100644
--- a/package/kernel/linux/modules/other.mk
+++ b/package/kernel/linux/modules/other.mk
@@ -1079,6 +1079,7 @@ define KernelPackage/ptp
   SUBMENU:=$(OTHER_MENU)
   TITLE:=PTP clock support
   DEPENDS:=+kmod-pps
+  DEPENDS+=@!(TARGET_mvebu&&LINUX_5_15)
   KCONFIG:= \
 	CONFIG_PTP_1588_CLOCK \
 	CONFIG_NET_PTP_CLASSIFY=y

grep KMODS .config

CONFIG_ALL_KMODS=y

grep -E '(PTP_1588|MV88E6X)' build_dir/target-arm_cortex-a9+vfpv3-d16_musl_eabi/linux-mvebu_cortexa9/linux-5.15.82/.config

CONFIG_NET_DSA_MV88E6XXX=y
# CONFIG_PTP_1588_CLOCK is not set
CONFIG_PTP_1588_CLOCK_OPTIONAL=y

but impact on other targets/linux-5.10 is unknown.

@johnnyslee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find target/linux/ -name config-5.15 -type f | xargs grep MV88E6XXX=y

target/linux/qoriq/config-5.15:CONFIG_NET_DSA_MV88E6XXX=y
target/linux/mvebu/config-5.15:CONFIG_NET_DSA_MV88E6XXX=y
target/linux/kirkwood/config-5.15:CONFIG_NET_DSA_MV88E6XXX=y

diff --git a/package/kernel/linux/modules/other.mk b/package/kernel/linux/modules/other.mk
index c5f944ed31..3a689c9bb9 100644
--- a/package/kernel/linux/modules/other.mk
+++ b/package/kernel/linux/modules/other.mk
@@ -1079,6 +1079,7 @@ define KernelPackage/ptp
   SUBMENU:=$(OTHER_MENU)
   TITLE:=PTP clock support
   DEPENDS:=+kmod-pps
+  DEPENDS+=@!(LINUX_5_15&&(TARGET_kirkwood||TARGET_mvebu||TARGET_qoriq))
   KCONFIG:= \
 	CONFIG_PTP_1588_CLOCK \
 	CONFIG_NET_PTP_CLASSIFY=y

@ynezz
Copy link
Member Author

@ynezz ynezz commented on 97c77ff Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnnyslee thanks, this one looks reasonable, can you please send a patch to the list (or create a PR?)

@johnnyslee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent #11486

@Ansuel
Copy link
Member

@Ansuel Ansuel commented on 97c77ff Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the network switch available as a module is going to break rootfs over NFS use case and maybe some others. Having PTP_1588_CLOCK=y is not an option for me either as it bloats the kernel, increases the kernel attack surface. So I'm leaning more towards a hack/patch approach, by reverting that PTP_1588_CLOCK_OPTIONAL tristate dependency.

I think the problem here is that kmod will be loaded only after rootfs mount right? If that's the case we already have a way to mount a kmod in preinit as a boot required module. Did we consider that?

@ynezz
Copy link
Member Author

@ynezz ynezz commented on 97c77ff Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem here is that kmod will be loaded only after rootfs mount right?

Yes, you can't mount network filesystem if you don't have LAN switch enabled in the kernel.

Did we consider that?

Nope, do you've such example? I failed to git grep it.

@Ansuel
Copy link
Member

@Ansuel Ansuel commented on 97c77ff Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ynezz https://openwrt.org/docs/guide-developer/packages#make_a_kernel_module_required_for_boot Here from the wiki

@ynezz
Copy link
Member Author

@ynezz ynezz commented on 97c77ff Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but this is not going to work with NFS rootfs, there you're mostly using just a kernel image and everything else is mounted over the network, so chicken and egg problem.

@Ansuel
Copy link
Member

@Ansuel Ansuel commented on 97c77ff Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok... was just an hint. So this doesn't have a squashfs at all... If that's the case yep then it needs to be part of the kernel since it's the only thing present on the device... Sorry for the noise

@hauke
Copy link
Member

@hauke hauke commented on 97c77ff Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the additional Kconfig option CONFIG_NET_DSA_MV88E6XXX_PTP. I think the MV88E6XXX should not use PTP at all even it is not selected.
Can we change the driver to not depend on PTP_1588_CLOCK or PTP_1588_CLOCK_OPTIONAL in case CONFIG_NET_DSA_MV88E6XXX_PTP is not set.

@johnnyslee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After hitting the "Kconfig recursive dependency" wall a dozen times,

--- a/drivers/net/dsa/mv88e6xxx/Kconfig
+++ b/drivers/net/dsa/mv88e6xxx/Kconfig
@@ -2,7 +2,7 @@
 config NET_DSA_MV88E6XXX
 	tristate "Marvell 88E6xxx Ethernet switch fabric support"
 	depends on NET_DSA
-	depends on PTP_1588_CLOCK_OPTIONAL
+	depends on NET_DSA_MV88E6XXX_PTP_OPTIONAL
 	select IRQ_DOMAIN
 	select NET_DSA_TAG_EDSA
 	select NET_DSA_TAG_DSA
@@ -13,7 +13,13 @@ config NET_DSA_MV88E6XXX
 config NET_DSA_MV88E6XXX_PTP
 	bool "PTP support for Marvell 88E6xxx"
 	default n
-	depends on NET_DSA_MV88E6XXX && PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
+	select NET_DSA_MV88E6XXX
 	help
 	  Say Y to enable PTP hardware timestamping on Marvell 88E6xxx switch
 	  chips that support it.
+
+config NET_DSA_MV88E6XXX_PTP_OPTIONAL
+	tristate
+	default y if NET_DSA_MV88E6XXX_PTP=n
+	default PTP_1588_CLOCK_OPTIONAL if NET_DSA_MV88E6XXX_PTP
CONFIG_NET_DSA_MV88E6XXX=y
# CONFIG_NET_DSA_MV88E6XXX_PTP is not set
CONFIG_NET_DSA_MV88E6XXX_PTP_OPTIONAL=y
CONFIG_PTP_1588_CLOCK=m
CONFIG_PTP_1588_CLOCK_OPTIONAL=m

CONFIG_NET_DSA_MV88E6XXX=m
CONFIG_NET_DSA_MV88E6XXX_PTP=y
CONFIG_NET_DSA_MV88E6XXX_PTP_OPTIONAL=m
CONFIG_PTP_1588_CLOCK=m
CONFIG_PTP_1588_CLOCK_OPTIONAL=m

For CONFIG_NET_DSA_MV88E6XXX_PTP=y, we need to define kmod-mv88e6xxxx package.

@johnnyslee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PR #11522 with Kconfig patch only.
I'm done for now, just hope no one will turn on CONFIG_NET_DSA_MV88E6XXX_PTP and complain about kmod missing.

@BKPepe
Copy link
Member

@BKPepe BKPepe commented on 97c77ff Dec 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, something different was done for broadcom. See 5863363

Please sign in to comment.