snort3: fix deps and CMake options per arch to allow building for more devices

[BKPepe]

CI/CD testing

[graysky2]

Traveling this whole week...

[graysky2]

I pushed to your repo. Check it out.

[Copilot]

Pull Request Overview

This PR refactors the snort3 package build configuration to improve cross-architecture compatibility by making architecture-specific dependencies optional and using conditional CMake options based on the target architecture.

Key changes:

• Replaced hardcoded architecture filtering for dependencies with optional package dependencies
• Moved architecture-specific CMake options into conditional blocks based on CONFIG_ARCH
• Simplified dependency declaration by inlining them directly in the Package/snort3 definition

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[BKPepe]

I think we should merge this as it is to solve the missing snort3 packages for almost any OpenWrt device except aarch64 and x86_64. Further improvements can be done later. This is definitely a huge step forward, and I’d like to backport it to OpenWrt 24.10 as soon as possible.

If @graysky2 finds a better solution, he can always open another pull request. :)

Before merging, I’ll drop the recently added commit related to actions-shared-workflows, since it was only created for testing and submitting a PR against that repo.

[graysky2]

I agree @BKPepe, this is a major step forward, thank you for the collaboration!

[graysky2]

Since you merged our two commits, eg BKPepe@f999d41 I think you add me as a Co-authored-by, no?

[BKPepe]

Since you merged our two commits, eg BKPepe@f999d41 I think you add me as a Co-authored-by, no?

I didn't merge your commits to my branch, because CI/CD in BKPepe#4 still fails.
There are some issues, which you need to take a look.

[graysky2]

I do not have the CI report on my view of that PR. I thought this PR was going to merge it all. Tell me - does this PR force the user to select gperftools-runtime and vectorscan-runtime in addition to selecting snort3? If so, I am not supportive of this due to the undocumented and extra overhead it introduces. I feel that we need this to work within the buildsystem.

[BKPepe]

I’ve spent quite a lot of time looking into the things you added to the packages repository, especially regarding the integration of vectorscan/hyperscan and gperftools for snort3. Being a package maintainer is not just about adding and updating packages — it also means addressing certain issues with the upstream developers, or passing on some of the tasks I asked you to solve (e.g. #21627 (comment)). I have to say I was a bit disappointed that things were not properly finalized, and I had to step in to make sure snort3 could be used on more devices than just aarch64 and x86_64.

Looking at the history, ever since I wrote this comment: #24055 (comment), things have still not been fully polished. Step by step, with the changes I’ve prepared, we’re getting closer, but since last week there’s already a change prepared that works and has passed CI/CD.

Now you’re telling me this needs more time, but how much time is actually needed?
At this point, by merging this change:

• Nothing will break — in fact, snort3 will become available for many more targets in both OpenWrt master and OpenWrt 24.10 than it is today.

• People using full builds (i.e., building all packages) will get snort3 with tmalloc and hyperscan support, if those are available.

• In your use case - downloading the sources and preparing individual packages or you do have a minimal/small build. In those cases, you still need to select snort3 in make menuconfig, or otherwise tell the build system that you require that package (i.e., CONFIG_PACKAGE_snort3=y). But once you’ve already selected the package, that assumes some technical knowledge — and I’m sure selecting an additional package you need won’t be an issue. :)

But we should also ask ourselves an important question: how many people actually use snort3 on OpenWrt, and how many of them really require tmalloc or hyperscan/vectorscan support — or both at the same time? OpenWrt can run even on very minimal devices with as little as 8 MB of flash and 64 MB of RAM.

I understand that these integrations are important to you, but for many users they are simply not. For them, basic snort3 support will be more than enough.

I’m not saying that my solution is the perfect one, but I believe we should merge it as it is for now. If you later come up with a better approach, feel free to adjust and improve it. :)

[graysky2]

Many of the points you raise are valid. Your time and effort to help diagnose and fix the non-trivial dependency chain are also appreciated. OpenWrt is a complex system given the multiple targets/SoCs it offers so I feel that at times, that keeping things updated and working is a team sport.

Can you please edit the commit msg to mention the need to manually pull in these two dependencies since both give performance increases? Also please mention the intention to fix this in a future PR. Then let's merge this PR.

/edit also did you want to bump the PKG_RELEASE

[BKPepe]

Please keep in mind that others cannot be expected to fix mistakes just because someone “didn’t have the time” to test or check properly. In open-source, people usually dedicate their free time to things they find interesting and want to contribute to, rather than correcting issues caused by someone else’s lack of effort.

Can you please edit the commit msg to mention the need to manually pull in these two dependencies since both give performance increases?

I’m not quite sure what you mean by manually pulling those two dependencies. If you were using an official OpenWrt build on your router and installing packages from the OpenWrt repository, you would simply install snort3. If vectorscan/hyperscan and/or gperftools are available, they would be installed automatically as dependencies, since snort3 was compiled with their support.

It seems like you haven’t tested this thoroughly. You can try it by using the available build artifacts. What I meant is that if gperftools, vectorscan, or hyperscan fail to build due to an error or because the architecture is unsupported, their support will just be disabled, but snort3 will still work without them.

What remains questionable is how important gperftools really is for snort3. You would need a very powerful device to see any real benefit. For regular snort3 usage, you don’t actually need gperftools.

Also please mention the intention to fix this in a future PR.

Well, no — because that would imply I’m committing to doing that, which I’m not. Anyone could do that. :)

[BKPepe]

@neheb Could you also take a look here, please? :)

[graysky2]

Please keep in mind that others cannot be expected to fix mistakes just because someone “didn’t have the time” to test or check properly

This happened not out of laziness, the blind spot is that I am continuing to learning the OpenWrt build system. This is why I am now using this script to help me cover the many targets and subtargets.

I’m not quite sure what you mean by manually pulling those two dependencies.

I am thinking (perhaps incorrectly) that if this is merged, the build bots will indeed compile snort3 but will do so without gperftools and without vectorscan. So the only way to users can have access to snort3 compiled against either or both of those is to compile an image on their own. I based my statement on the output of the aforementioned script after pull in this PR:

Click to expand

Target_Subtarget	Snort3	Gperftools	Vectorscan
airoha_an7581	✔	✘	✘
airoha_an7583	✔	✘	✘
airoha_en7523	✔	✘
apm821xx_nand	✔
apm821xx_sata	✔
at91_sam9x	✔	✘
at91_sama5	✔	✘
at91_sama7	✔	✘
ath79_generic	✔	✘
ath79_mikrotik	✔	✘
ath79_nand	✔	✘
ath79_tiny	✔	✘
bcm27xx_bcm2708	✔	✘
bcm27xx_bcm2709	✔	✘
bcm27xx_bcm2710	✔	✘	✘
bcm27xx_bcm2711	✔	✘	✘
bcm27xx_bcm2712	✔	✘	✘
bcm47xx_generic	✔	✘
bcm47xx_legacy	✔	✘
bcm47xx_mips74k	✔	✘
bcm4908_generic	✔	✘	✘
bcm53xx_generic	✔	✘
bmips_bcm6318	✔	✘
bmips_bcm63268	✔	✘
bmips_bcm6328	✔	✘
bmips_bcm6358	✔	✘
bmips_bcm6362	✔	✘
bmips_bcm6368	✔	✘
d1_generic		✘
econet_en751221	✔	✘
gemini_generic	✔	✘
imx_cortexa53	✔	✘	✘
imx_cortexa7	✔	✘
imx_cortexa9	✔	✘
ipq40xx_chromium	✔	✘
ipq40xx_generic	✔	✘
ipq40xx_mikrotik	✔	✘
ipq806x_chromium	✔	✘
ipq806x_generic	✔	✘
ixp4xx_generic	✔	✘
kirkwood_generic	✔	✘
lantiq_ase	✔	✘
lantiq_falcon	✔	✘
lantiq_xrx200	✔	✘
lantiq_xrx200_legacy	✔	✘
lantiq_xway	✔	✘
lantiq_xway_legacy	✔	✘
layerscape_armv7	✔	✘
layerscape_armv8_64b	✔	✘	✘
loongarch64_generic		✘
malta_be	✔	✘
malta_be64	✔	✘
malta_le	✔	✘
malta_le64		✘
mediatek_filogic	✔	✘	✘
mediatek_mt7622	✔	✘	✘
mediatek_mt7623	✔	✘
mediatek_mt7629	✔	✘
mpc85xx_p1010	✔
mpc85xx_p1020	✔
mpc85xx_p2020	✔
mvebu_cortexa53	✔	✘	✘
mvebu_cortexa72	✔	✘	✘
mvebu_cortexa9	✔	✘
mxs_generic	✔	✘
omap_generic	✔	✘
pistachio_generic	✔	✘
qoriq_generic		✘
qualcommax_ipq50xx	✔	✘	✘
qualcommax_ipq60xx	✔	✘	✘
qualcommax_ipq807x	✔	✘	✘
qualcommbe_ipq95xx	✔	✘	✘
ramips_mt7620	✔	✘
ramips_mt7621	✔	✘
ramips_mt76x8	✔	✘
ramips_rt288x	✔	✘
ramips_rt305x	✔	✘
ramips_rt3883	✔	✘
realtek_rtl838x	✔	✘
realtek_rtl839x	✔	✘
realtek_rtl930x	✔	✘
realtek_rtl930x_nand	✔	✘
realtek_rtl931x	✔	✘
realtek_rtl931x_nand	✔	✘
rockchip_armv8	✔	✘	✘
sifiveu_generic		✘
siflower_sf19a2890	✔	✘
siflower_sf21		✘
stm32_stm32mp1	✔	✘
sunxi_cortexa53	✔	✘	✘
sunxi_cortexa7	✔	✘
sunxi_cortexa8	✔	✘
tegra_generic	✔	✘
uml_generic	✔	✘	✘
x86_64	✔	✘	✘
x86_generic	✔	✘
x86_geode	✔	✘
x86_legacy	✔	✘
zynq_generic	✔	✘

✔ means enabled ; ✘ means available but disabled ; blank means unavailable

[BKPepe]

I am thinking (perhaps incorrectly) that if this is merged, the build bots will indeed compile snort3 but will do so without gperftools and without vectorscan

You're thinking in the right direction, but your reasoning isn’t entirely correct.

At this point, the pull request has successfully passed the CI/CD tests, and if you look at the logs, you’ll see that support for vectorscan, hyperscan, and gperftools is enabled, if it is available. CI/CD uses minimal builds.

The condition in the Makefile ensures that these packages are selected and used automatically if they’re available.

OpenWrt buildbots compile all packages. I believe (and hopefully I’m right) that in your make menuconfig, you don’t have vectorscan, hyperscan, or gperftools selected — which means the build system isn’t aware of them.

Simply mark them as M (modules). You can also test this by forcing those dependencies as Y and then recompiling snort3. That way, the build system will recognize them and compile with the proper support.

Oh and dont forget to save in make menuconfig.

[graysky2]

The condition in the Makefile ensures that these packages are selected and used automatically if they’re available.

I see... so the order of the build matters?

For users compiling their own images, I believe they will have to manually select these based on the output of the script I referenced above. Is this a correct statement? When you are back home and can have a look, BKPepe@e7ecbe7 seeks to address this.

[neheb]

libpthread can go away.

[BKPepe]

Okay, let's remove it in different PR as I am thinking that this is not related to this PR.

[BKPepe]

Hmm, mabye packages (you pointed it even in syslog-ng) has this dependency. Shouldn't be there treewide commit removal?

[neheb]

Probably

[neheb]

So this is interesting. Typically tirpc is added for musl and not glibc. See https://github.com/openwrt/packages/blob/master/net/dante/Makefile#L36

[BKPepe]

I am not using glibc, but good catch, let's create PR and let me ask someone to try it. :)

[BKPepe]

@graysky2 is going to address this in #27602

[graysky2]

Yes, the CI completed successfully in that PR. Thank you @neheb for pointing this out.

[BKPepe]

For users compiling their own images, I believe they will have to manually select these based on the output of the script I referenced above. Is this a correct statement?

In these both cases (=Y, =M) will be gperftools used.

Packages marked as =M are known to the build system and are ready to be compiled either individually or as part of a full build.

---

In this case package gperftools will not be used, because it is marked as =N

It really depends on how you configured your .config and whether you selected all the packages to be included and compiled. Based on your script, I think those packages were excluded from your build system and therefore are not detected.

[graysky2]

It really depends on how you configured your .config and whether you selected all the packages to be included and compiled. Based on your script, I think those packages were excluded from your build system and therefore are not detected.

Yes, this is my point. A user should not need to know that in addition to selecting snort3, he or she must also select (either as a module or compiled in) vectorscan-runtime and gperftools-runtime. The build system should do this.

For example, use this is your .config:

CONFIG_TARGET_x86=y
CONFIG_TARGET_x86_64=y
CONFIG_TARGET_x86_64_DEVICE_generic=y
CONFIG_DEVEL=y
CONFIG_TOOLCHAINOPTS=y
# CONFIG_BINUTILS_USE_VERSION_2_44 is not set
CONFIG_BINUTILS_USE_VERSION_2_45=y
CONFIG_BINUTILS_VERSION="2.45"
CONFIG_BINUTILS_VERSION_2_45=y
# CONFIG_GCC_USE_VERSION_14 is not set
CONFIG_GCC_USE_VERSION_15=y
CONFIG_GCC_VERSION="15.2.0"
CONFIG_GCC_VERSION_15=y
# CONFIG_KERNEL_WERROR is not set
CONFIG_LIBC="glibc"
CONFIG_LIBC_USE_GLIBC=y
# CONFIG_LIBC_USE_MUSL is not set
CONFIG_PACKAGE_intel-microcode=y
CONFIG_PACKAGE_libcrypt-compat=y
CONFIG_STRIP_ARGS="--strip-all"
CONFIG_TARGET_SUFFIX="gnu"
CONFIG_USE_GLIBC=y
CONFIG_USE_STRIP=y

Now make nconfig and enable snort3 and save (or just echo "CONFIG_PACKAGE_snort3=y" >> .config ; make defconfig).
Now grep for what is enabled.

% grep --color=auto -E '(gperftools-run|snort3|vectorscan-run)' .config
# CONFIG_PACKAGE_gperftools-runtime is not set
# CONFIG_PACKAGE_vectorscan-runtime is not set
CONFIG_PACKAGE_snort3=y

I am trying to fix this in BKPepe#4 as you know but hitting snags. @neheb - if you have any suggestions, I am grateful to hear them.

[BKPepe]

You have to look at it from another perspective. Hyperscan, Vectorscan, and possibly gperftools are not mandatory dependencies. So why should someone be forced to use them if they don’t want to?

For the average OpenWrt user who simply downloads an official image and installs packages built by the buildbot, everything will already be feature-complete — just as you intend it to be. But what if someone doesn’t want that? :)

Many technically skilled users prefer to build their own OpenWrt images with a minimal set of packages. They first need to check whether their router even has enough capacity, and then explicitly choose which optional dependencies to include during compilation. That’s a conscious design choice.

From that perspective, the build system behaves exactly as it should. It can’t — and shouldn’t — “know” about packages that aren’t part of the build. Optional really means optional, and that flexibility is one of the core strengths of the OpenWrt ecosystem.

[BKPepe]

For now, I am merging this as it is. Discussion can be done as follow-up or you can finish the PR, which you referenced.
Later today, I will address @neheb's suggestions.

[graysky2]

@BKPepe - I can appreciate the minimalist's perspective. I wonder if the best solution here is to simply add some context/instruction to snort3's description to flag to users who might not know, that there are optionally dependencies that can and do improve snort's performance. I will create a new PR with this change. Thanks for explaining the user-case.

EDIT: #27601