Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RFC, RFT] mvebu: add 6.6 testing kernel #14912

Merged
merged 7 commits into from Apr 3, 2024
Merged

[RFC, RFT] mvebu: add 6.6 testing kernel #14912

merged 7 commits into from Apr 3, 2024

Conversation

Borromini
Copy link

@Borromini Borromini commented Mar 16, 2024 •
edited

This adds 6.6 support for mvebu.

Compile-tested:

  • mvebu/cortexa9: WRT1200AC
  • mvebu/cortexa72: RB5009UG+S+IN

Run-tested:

  • mvebu/cortexa9: WRT1200AC (myself), WRT3200ACM (@anomeome), Turris Omnia (@drujd)
  • mvebu/cortexa53: GL-MV1000 (@mrkiko)
  • mvebu/cortexa72: RB5009UG+S+IN (myself)

@github-actions github-actions bot added kernel pull request/issue with Linux kernel related changes target/mvebu pull request/issue for mvebu target labels Mar 16, 2024
@Borromini
Copy link
Author

Borromini commented Mar 16, 2024 •
edited

CI is failing on the DTB move, but that's a given since 6.1 has the DTSes still in the main dts/ dir, and CI is compiling 6.1. Not sure how to have the CI compile 6.6 short of switching kernel version already.

I now realise moving the DTSes to marvell/ like upstream does breaks 6.1, but I am not aware of a workaround. Either we move the DTSes to the vendor subdir, and we need to make minimal edits to e.g. Makefiles, or we keep our downstream DTSes in the DTS top dir, but need to edit their includes to point to the marvell/ subdir. Both approaches will break 6.1 support.

@namiltd
Copy link
Contributor

namiltd commented Mar 16, 2024

Take a look at this: #14713
Use kernel_bump.sh to avoid losing file history.

@anomeome
Copy link
Contributor

DNWFM with configdiff, fail is:

arm-openwrt-linux-muslgnueabi-nm -n /home/kc/wrtpac/source/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu_cortexa9/linux-6.6.20/vmlinux.o | awk '/^[0-9a-f]+ [rR] __ksymtab_/ {print substr($3,11)}' > /home/kc/wrtpac/source/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu_cortexa9/kernel_symtab.txt
grep -Ff /home/kc/wrtpac/source/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu_cortexa9/mod_symtab.txt /home/kc/wrtpac/source/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu_cortexa9/kernel_symtab.txt > /home/kc/wrtpac/source/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu_cortexa9/sym_include.txt
make[4]: *** [Makefile:24: /home/kc/wrtpac/source/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu_cortexa9/symtab.h] Error 1

symtab.h extent, but zero length...

@mrkiko
Copy link
Contributor

mrkiko commented Mar 17, 2024

Tested and running successfully on GL-MV1000 (initramfs) altough with some emitted warnings related to DTS
See http://sprunge.us/htgN2k
for dmesg and some other sparse informations. Feel free to contact me on IRC.
The build prompted for some AEAD options - relative to AEAD split I guess.

@Borromini
Copy link
Author

@anomeome I saw that happen too, a make dirclean made it go away though. Haven't been able to reproduce it since.

@Borromini
Copy link
Author

The build prompted for some AEAD options - relative to AEAD split I guess.

Can you share the symbols you had to set? So I can add them to the cortexa53 config.

@anomeome
Copy link
Contributor

@Borromini , Yes, that is where I started but no success.

@mrkiko
Copy link
Contributor

mrkiko commented Mar 18, 2024

CRYPTO_SM4_ARM64_CE_CCM
CONFIG_CRYPTO_SM4_ARM64_CE_GCM

@Borromini
Copy link
Author

CRYPTO_SM4_ARM64_CE_CCM CONFIG_CRYPTO_SM4_ARM64_CE_GCM

I will add them to the A53 config, thanks.

@robimarko Would you mind reviewing this? I'm curious about your take on the DTS vendor subdir issue as well. Thanks.

BKPepe
BKPepe reviewed Mar 18, 2024
View reviewed changes
target/linux/mvebu/patches-6.6/700-mvneta-tx-queue-workaround.patch
Comment on lines +1 to +2
From: Felix Fietkau <nbd@nbd.name>
Subject: mvneta: tx queue workaround
Copy link
Member

Choose a reason for hiding this comment

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

Do we need still this hacky patch for kernel 6.6?

Copy link
Contributor

Choose a reason for hiding this comment

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

reference 5411

Copy link
Member

Choose a reason for hiding this comment

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

That reference is in the patch description, but it does not answer my question. :-)

Copy link
Contributor

@stklcode stklcode Mar 23, 2024 •
edited

Choose a reason for hiding this comment

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

Let's get some empirical data. I repeated my tests from 6.1 bump (#12938 (comment)).

Test device:
Linksys WRT1900ACS (mvebu/cortexa9)
OpenWrt SNAPSHOT, r0+25621-231d84e7a8 (f7c732b with this PR on top)
Kernel 6.6.22
netperf 2.7.0-r3

Test client:
x86_64 Fedora 39 workstation
intel I219-V GBit Ethernet directly connected
netperf 2.7.1

Test commands

netperf -H 172.16.101.1 -l 60 -t TCP_MAERTS &
netperf -H 172.16.101.1 -l 60 -t TCP_MAERTS &
netperf -H 172.16.101.1 -l 60 -t TCP_MAERTS &
netperf -H 172.16.101.1 -l 60 -t TCP_MAERTS &

Results with patch 700

Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec
131072  16384  16384    60.00     234.15
131072  16384  16384    60.00     234.14
131072  16384  16384    60.00     234.14
131072  16384  16384    60.00     234.14
                                  936,57   (total)

Results without patch 700

Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec
131072  16384  16384    60.00     888.79
131072  16384  16384    60.01      15.95
131072  16384  16384    60.01      15.95
131072  16384  16384    60.01      15.92
                                  936,61   (total)

Results without patch 700, and packet steering enabled

config globals 'globals'
	option packet_steering '1'

Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec
131072  16384  16384    60.00     920.67
131072  16384  16384    60.01       8.01
131072  16384  16384    60.01       8.01
131072  16384  16384    60.01       7.92
                                  944,61   (total)

Results without patch 700, and hardware flow offloading

config defaults
        option flow_offloading '1'
        option flow_offloading_hw '1'

Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec
131072  16384  16384    60.02       0.00
131072  16384  16384    60.00     936.53
131072  16384  16384    60.02       0.01
131072  16384  16384    60.02       0.01
                                  936,55   (total)

Results without patch 700, and software flow offloading

config defaults
        option flow_offloading '1'

Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec
131072  16384  16384    60.00     339.62
131072  16384  16384    60.01       0.02
131072  16384  16384    60.00     209.14
131072  16384  16384    60.01     387.74
                                  936,52   (total)

(all results repeatable with minor deviations)

Conclusion

Sadly the answer is "Yes", patch seems to be still necessary.

Results with sw flow offloading are interesting though 🤔

Btw. build & run tested on WRT1900ACS 😉

Copy link
Member

@BKPepe BKPepe Mar 23, 2024 •
edited

Choose a reason for hiding this comment

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

Hmm, it would be interesting to see if it is even broken by using vanilla kernel, but I think that upstream Linux kernel developers would notice that. Does anyone try to reach them? Isn't this caused for us by another hacky patch in this repository?

BTW: Thanks for testing this! :) Appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is software flow offload is working with 6.6.x?

Copy link

@drujd drujd Mar 26, 2024 •
edited

Choose a reason for hiding this comment

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

Is software flow offload is working with 6.6.x?

From what I've gathered, it does not on devices that use the switch for both LAN and WAN, but it does for e.g. Turris Omnia which has a separate WAN connection. It is working on my Turris Omnia - with the usual quirks.

EDIT: But if it IS working on WRT1900ACS, then I don't know...

robimarko
robimarko reviewed Mar 18, 2024
View reviewed changes
target/linux/mvebu/cortexa72/config-6.6 Outdated
@@ -42,6 +42,9 @@ CONFIG_CC_HAVE_SHADOW_CALL_STACK=y
CONFIG_CC_HAVE_STACKPROTECTOR_SYSREG=y
CONFIG_CPU_LITTLE_ENDIAN=y
CONFIG_CRC_CCITT=y
CONFIG_CRYPTO_AES_ARM64_CE_CCM=y
Copy link
Contributor

Choose a reason for hiding this comment

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

These should probably be moved to their respective kmod-crypto

@robimarko
Copy link
Contributor

@Borromini For the DTS path change, see how mediatek solved it
7d1145e

@drujd
Copy link

drujd commented Mar 19, 2024

So far seems to work well on Turris Omnia (cortexa9/armv7).

@Borromini
Copy link
Author

@Borromini For the DTS path change, see how mediatek solved it 7d1145e

@robimarko Thanks for the hint. I have added the if clause, which works for the upstream DTSes, but that would make the buildroot think our OpenWrt specific DTSes were under $(DTS_DIR)/marvell as well, while they're statically at files/arch/arm/boot/dts/, no matter which kernel, 6.1 or 6.6. So I moved our downstream Cortex A9 DTSes into a dts/ subdir just like the mediatek target. I hope that's acceptable. Tested with both kernels, works like it should.

If that's okay, I'll push those changes to the PR. Haven't had the time to look into the kmod-crypto stuff yet.

@anomeome While testing I ran into the symtab.h issue again as well, it looks like some config option is causing it. Using default buildroot options (ie wipe config, just select a single or multiple device(s) and don't touch anything else besides ticking the testing kernel), it compiles just fine. Hopefully that can help troubleshoot.

@robimarko
Copy link
Contributor

@Borromini No, that would break 6.1 kernel, just make files-6.6 and put the DTS-es in the new path there, while adding files-6.1 with the old path

@Borromini
Copy link
Author

I compile-tested both 6.1 and 6.6 and it won't. I can link the patch this evening for testing. A DTS dir seems cleaner to me than files-6.1 and files-6.6, IMHO?

@robimarko
Copy link
Contributor

dts was traditionally used when there was no upstream dts directory for that vendor/platform, files is more standard

@Borromini
Copy link
Author

Got it. Will do the files-6.1/6.6 split then.

@Borromini
Copy link
Author

@anomeome As per #14860, can you try compiling without CONFIG_STRIP_KERNEL_EXPORTS?

@anomeome
Copy link
Contributor

anomeome commented Mar 21, 2024 •
edited

Yep. Compiled all the wrtpac targets, flashed and running on a rango.; I saw a post on the forum where someone said they had to go back to OEM to move forward with their 6.6.x build, I did not see this issue.

Edit: re. mamba kernel partition 4MB limit: 4019971 Mar 21 11:43 linksys_wrt1900ac-v1-kernel.bin

@mrkiko
Copy link
Contributor

mrkiko commented Mar 22, 2024

PR #14952 by @robimarko adds missing symbols.

@Borromini
Copy link
Author

Borromini commented Mar 22, 2024 •
edited

Thanks, tree did show indeed lots of other ARM targets already manipulated those symbols in their config files. PR is refreshed, using files-6.{1,6} as requested.

@anomeome Not sure how to tackle that (and if it should be part of this PR), does the bootloader support compression? Can we compress the kernel? A cursory look at the makefile seems to suggest the kernel is not compressed (or at least not with LZMA)

@dannil
Copy link

dannil commented Mar 22, 2024 •
edited

Run-tested on WRT1900ACS ( luci-ssl + default packages), it's a spare unit so can test any additional changes if needed.

@anomeome
Copy link
Contributor

@Borromini , just a data point that mamba is again close to limit (~72K), I would guess it will last until next major kernel push.

@Borromini
Copy link
Author

I've come to realise all changes thrown together in one single commit is a bit annoying if troubleshooting is needed, any objections if I refresh this PR with the changes split out?

@mrkiko
Copy link
Contributor

mrkiko commented Mar 25, 2024 via email

@robimarko
Copy link
Contributor

Those warnings are quite clear, that device DTS was not done according to the bindings and they just added validation

@mrkiko
Copy link
Contributor

mrkiko commented Mar 25, 2024 via email

@robimarko
Copy link
Contributor

Please no commits without the accompanying description

@Borromini
Copy link
Author

Sorry, should be okay now.

@stklcode
Copy link
Contributor

stklcode commented Mar 28, 2024 •
edited

Looks like there are still a few commits without description, e.g.

mvebu: 6.6: update Linksys status LED patch

Signed-off-by: ...

If those update ... patch commits are just manual refreshs, I personally prefer to squash all the refreshs together and add just one message instead of adding no additional information like "update for 6.6" to every single one

mvebu: 6.6: refresh patches

manually refreshed:
* 309-linksys-status-led.patch
* 310-linksys-use-eth0-as-cpu-port.patch
* 320-arm-dts-armada-370-synology-ds213j-mtd-parts.patch
* 701-mvpp2-read-mac-address-from-nvmem.patch
* 902-drivers-mfd-Add-a-driver-for-IEI-WT61P803-PUZZLE-MCU.patch

all other patches automatically refreshed

Signed-off-by: ...

But that's more or less personal style, feel free to write a descriptive line for every one.

@Borromini
Copy link
Author

Thanks, that makes more sense. Done.

@robimarko
Copy link
Contributor

You still have a commit without desciption, CI is falling as well on it due to formal issues

@Borromini Borromini force-pushed the mvebu-6.6 branch 2 times, most recently from e38d8c9 to 028dfb2 Compare March 29, 2024 09:45
@Borromini
Copy link
Author

Well that's the sleep deprivation I guess :|. Should be good now.

@mrkiko
Copy link
Contributor

mrkiko commented Mar 29, 2024

Tested PR in it's current state in the GL-MV1000 after rebasing in main (GCC 13). Works fine.

@stklcode
Copy link
Contributor

stklcode commented Mar 30, 2024 •
edited

Build and run tested again on WRT1900ACS and Turris Omnia (both mvebu/cortexa9) with some custom configuraiton, dual MT7915e WiFi, EP06 LTE modem and WireGuard. Rebase onto 0c96d20. No regressions so far.

@Borromini
Copy link
Author

@robimarko I think the comments have been addressed, can this be merged?

Stijn Segers added 7 commits April 3, 2024 17:49
@robimarko
mvebu: move files to files-6.1
9abfdc3 
DTS paths for 32 bit ARM devices changed with 6.6, move files/ to
files-6.1 to prep for kernel 6.6 introduction.

Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@robimarko
mvebu: 6.6: copy files, patches & configs from 6.1
4671a92 
Copy all mvebu 6.1 specific files, patches and configs to 6.1.

Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@robimarko
mvebu: 6.6: adjust 32 bit ARM DTS path
20024ab 
As of 6.6, all upstream DTSes are moved to their respective vendor subdir.
OpenWrt already followed this practice for ARM64, but not yet for 32 bit
ARM (Armada 37x/38x).

Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@robimarko
mvebu: 6.6: remove upstreamed patches
ad5e73f 
000-cpufreq-armada-8k-add-ap807-support.patch was upstreamed.

Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@robimarko
mvebu: 6.6: refresh patches
2260ec4 
Manually refreshed:
 * 309-linksys-status-led.patch
 * 310-linksys-use-eth0-as-cpu-port.patch
 * 320-arm-dts-armada-370-synology-ds213j-mtd-parts.patch
 * 701-mvpp2-read-mac-address-from-nvmem.patch
 * 902-drivers-mfd-Add-a-driver-for-IEI-WT61P803-PUZZLE-MCU.patch

All other patches automatically refreshed.

Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@robimarko
mvebu: cortexa9: set DTS dir for 6.6
ab2fe43 
With 6.6, all DTSes were moved to their vendor subdirectories. ARM64
DTSes already used this scheme, but 32 bit Cortex A9 did not, prior
to 6.6. Introduce a kernel version check to keep backward compatibility
with 6.1.

Suggested-by: Robert Marko <robimarko@gmail.com>
Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@robimarko
mvebu: add 6.6 testing kernel
ed893a3 
Add 6.6 testing kernel for mvebu target.

Signed-off-by: Stijn Segers <foss@volatilesystems.org>
@openwrt-bot openwrt-bot merged commit ed893a3 into openwrt:main Apr 3, 2024
2 checks passed
@robimarko
Copy link
Contributor

Thanks! Rebased on top of main and merged!

@namiltd
Copy link
Contributor

namiltd commented Apr 3, 2024

Some files (e.g. target/linux/mvebu/patches-6.6/300-mvebu-Mangle-bootloader-s-kernel-arguments.patch) require updating to kernel 6.6.23.
Please refresh patches.

@robimarko
Copy link
Contributor

Some files (e.g. target/linux/mvebu/patches-6.6/300-mvebu-Mangle-bootloader-s-kernel-arguments.patch) require updating to kernel 6.6.23. Please refresh patches.

Yeah, already done.

@namiltd namiltd mentioned this pull request Apr 17, 2024
Open
42 tasks
@stklcode stklcode mentioned this pull request May 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robimarko robimarko robimarko left review comments

@BKPepe BKPepe BKPepe left review comments

@drujd drujd drujd left review comments

@stklcode stklcode stklcode left review comments

@anomeome anomeome anomeome left review comments

Assignees
No one assigned
Labels
kernel pull request/issue with Linux kernel related changes target/mvebu pull request/issue for mvebu target
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet