Skip to content

Conversation

tabrisnet
Copy link

@tabrisnet tabrisnet commented Sep 21, 2025

Description

  • don't force various iptables/nftables to be built-in
    • I'm tempted to make this more normal by adding a new function to lib/functions/compilation/armbian-kernel.sh to make them built as modules by default.
  • modify filogic kernel to build more iptables/nftables stuff

learnings: I cannot make CONFIG_BRIDGE=m w/o it affecting CONFIG_NET_DSA and CONFIG_NET_MEDIATEK_SOC - but that shouldn't matter re armbian-kernel.sh

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.

  • the kernel successfully builds

Checklist:

Please delete options that are not relevant.

  • [?] My code follows the style guidelines of this project
  • I have performed a self-review of my own code

@github-actions github-actions bot added 11 Milestone: Fourth quarter release size/medium PR with more then 50 and less then 250 lines Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Framework Framework components labels Sep 21, 2025
@tabrisnet tabrisnet added the Work in progress Unfinished / work in progress label Sep 21, 2025
@tabrisnet

This comment was marked as resolved.

@tabrisnet
Copy link
Author

@coderabbitai review

@tabrisnet tabrisnet marked this pull request as ready for review September 21, 2025 18:11
@tabrisnet tabrisnet requested review from a team and igorpecovnik as code owners September 21, 2025 18:11
@tabrisnet tabrisnet force-pushed the tabrisnet_net_modules branch from 6bdf607 to 6a763e2 Compare September 21, 2025 18:11
@tabrisnet tabrisnet removed the Work in progress Unfinished / work in progress label Sep 21, 2025
Copy link
Contributor

coderabbitai bot commented Sep 21, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This PR updates linux-filogic kernel configuration to modularize extensive netfilter, nftables/xtables, iptables, bridge, and conntrack features (switching many from built-in y to module m) and adds numerous specific matches/targets and flow table options. It also enables bridge conntrack/ebtables and adds PCS_MTK_USXGMII. Additionally, armbian_kernel_config__enable_docker_support in armbian-kernel.sh is updated to set several networking options (BRIDGE, NF_TABLES, IP_NF_FILTER, IP_NF_IPTABLES, IP_NF_SECURITY, NF_CONNTRACK) from y to m.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

Work in progress, Build

Suggested reviewers

  • rpardini
  • chraac
  • igorpecovnik
  • EvilOlaf
  • paolosabatino
  • teknoid

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the primary change: updating Armbian build machinery to allow more netfilter-related components to be built as modules, so it is directly related to the changeset and easy for a reviewer to understand; it does contain a minor spelling error ("marchinery" → "machinery") but that does not make it misleading.
Description Check ✅ Passed The pull request description is on-topic and directly describes the intent to stop forcing iptables/nftables to be built-in, notes changes to the filogic kernel, and documents that the kernel built successfully, so it meets the lenient acceptance criteria for this check.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
lib/functions/compilation/armbian-kernel.sh (1)

205-335: Extract netfilter modularization into a dedicated helper and call it here.

To reuse beyond Docker and keep this function focused, move the BRIDGE/IP_NF_*/NF_TABLES/NF_CONNTRACK prefs into a new helper, then invoke it from armbian_kernel_config__enable_docker_support.

+function armbian_kernel_config__prefer_netfilter_modules() {
+	if [[ -f .config ]]; then
+		kernel_config_set_m BRIDGE
+		kernel_config_set_m BRIDGE_NETFILTER
+		kernel_config_set_m NF_TABLES
+		kernel_config_set_y NF_TABLES_INET
+		kernel_config_set_y NF_TABLES_NETDEV
+		kernel_config_set_m IP_NF_IPTABLES
+		kernel_config_set_m IP_NF_FILTER
+		kernel_config_set_m IP_NF_SECURITY
+		kernel_config_set_m NF_CONNTRACK
+	fi
+}
@@
 function armbian_kernel_config__enable_docker_support() {
@@
-		kernel_config_set_m BRIDGE                          # Enables support for Ethernet bridges
+		armbian_kernel_config__prefer_netfilter_modules
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 684936e and 6a763e2.

📒 Files selected for processing (2)
  • config/kernel/linux-filogic-current.config (4 hunks)
  • lib/functions/compilation/armbian-kernel.sh (4 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-30T04:13:16.457Z
Learning: Armbian kernel configuration files like linux-filogic-current.config are autogenerated overlays on top of arch defconfig. Comments added manually will be lost during future updates by maintainers, and explicit "CONFIG_OPTION is not set" statements aren't needed for mutually exclusive options since these are overlay configs that only specify changes from the base configuration.
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-30T06:56:33.372Z
Learning: In Armbian kernel configuration, the BTRFS configuration logic preserves existing settings (whether built-in 'y' or module 'm') and only sets BTRFS_FS to module when it was previously disabled or not set, achieving "allow but not require" flexibility while maintaining backward compatibility.
Learnt from: lanefu
PR: armbian/build#8377
File: config/kernel/linux-uefi-arm64-cloud.config:165-170
Timestamp: 2025-07-19T00:03:42.995Z
Learning: CONFIG_NETKIT was introduced in Linux kernel 4.14.330 (November 2023) and enables BPF-programmable network devices that can operate in Layer 3 or Layer 2 mode. It's a valid configuration option in modern kernels including 6.12.
📚 Learning: 2025-08-30T06:56:33.372Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-30T06:56:33.372Z
Learning: In Armbian kernel configuration, the BTRFS configuration logic preserves existing settings (whether built-in 'y' or module 'm') and only sets BTRFS_FS to module when it was previously disabled or not set, achieving "allow but not require" flexibility while maintaining backward compatibility.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
  • config/kernel/linux-filogic-current.config
📚 Learning: 2025-08-02T05:46:10.664Z
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-02T05:46:10.664Z
Learning: In the Armbian build system, the modern recommended approach for kernel configuration is to use the kernel-config command via "./compile.sh BOARD=boardname BRANCH=branchname kernel-config" instead of the deprecated KERNEL_CONFIGURE=yes flag. This provides a two-step workflow: configure using menuconfig, then build, with better transparency and control over configuration changes.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
📚 Learning: 2025-08-02T05:46:10.664Z
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-02T05:46:10.664Z
Learning: In the Armbian build system, the KERNEL_CONFIGURE=yes switch is deprecated. The recommended approach for kernel configuration is now to use the `kernel-config` build command, which provides a more structured way to configure kernel options.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
📚 Learning: 2025-07-27T15:53:30.629Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:805-825
Timestamp: 2025-07-27T15:53:30.629Z
Learning: In the Armbian build system, kernel configurations prioritize broad hardware compatibility over optimization concerns. Even when including numerous legacy/obsolete drivers causes technical issues like increased initramfs size and slower enumeration, the project philosophy is to "appease the masses" by ensuring maximum device compatibility rather than optimizing for specific use cases.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
📚 Learning: 2025-08-30T04:13:16.457Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-30T04:13:16.457Z
Learning: Armbian kernel configuration files like linux-filogic-current.config are autogenerated overlays on top of arch defconfig. Comments added manually will be lost during future updates by maintainers, and explicit "CONFIG_OPTION is not set" statements aren't needed for mutually exclusive options since these are overlay configs that only specify changes from the base configuration.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
  • config/kernel/linux-filogic-current.config
📚 Learning: 2025-06-24T10:08:40.313Z
Learnt from: EvilOlaf
PR: armbian/build#8328
File: lib/functions/compilation/patch/drivers_network.sh:542-545
Timestamp: 2025-06-24T10:08:40.313Z
Learning: In the Armbian build system, when a PR removes build support for a specific kernel version, version check issues for that removed version become practically irrelevant even if they appear incorrect in isolation. Context about which kernel versions are being deprecated/removed is important for understanding the impact of version-related code changes.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
📚 Learning: 2025-08-30T06:48:09.091Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-30T06:48:09.091Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the user prefers flexible grep patterns over anchored ones for BTRFS configuration checks, but agrees to use quiet grep (-q) to avoid polluting build logs.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
📚 Learning: 2025-07-19T00:03:42.995Z
Learnt from: lanefu
PR: armbian/build#8377
File: config/kernel/linux-uefi-arm64-cloud.config:165-170
Timestamp: 2025-07-19T00:03:42.995Z
Learning: CONFIG_NETKIT was introduced in Linux kernel 4.14.330 (November 2023) and enables BPF-programmable network devices that can operate in Layer 3 or Layer 2 mode. It's a valid configuration option in modern kernels including 6.12.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
  • config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-27T15:56:34.414Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:438-439
Timestamp: 2025-07-27T15:56:34.414Z
Learning: CONFIG_NET_SCH_DEFAULT is a boolean kernel configuration option (=y or =n) that enables/disables the ability to override the default network queueing discipline at runtime. When set to 'y', it allows setting the actual qdisc name via /proc/sys/net/core/default_qdisc at runtime, not in the kernel config file itself.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
📚 Learning: 2025-07-19T00:03:42.995Z
Learnt from: lanefu
PR: armbian/build#8377
File: config/kernel/linux-uefi-arm64-cloud.config:165-170
Timestamp: 2025-07-19T00:03:42.995Z
Learning: CONFIG_NETKIT is a valid kernel configuration option in Linux kernel 6.12 and later versions, despite not being present in earlier versions like 6.9/6.10-rc.

Applied to files:

  • lib/functions/compilation/armbian-kernel.sh
  • config/kernel/linux-filogic-current.config
🔇 Additional comments (10)
config/kernel/linux-filogic-current.config (6)

157-175: Conntrack core and helpers modularized: OK.

Good balance: core as m with common helpers as modules; PROCFS/events/timeout enabled for observability.


175-205: nftables framework as module with rich feature set: OK.

Parent m, feature booleans y/m are consistent; flow table PROCFS=y is handy for debugging.


280-287: iptables legacy: switch to modules aligns with PR goal.

Keeps legacy available without forcing built‑ins. Looks good.


205-271: Approve — XT/NFT breadth acceptable; monitor build/image size.

Verified: XT modules = 65, NFT modules = 27. Broad module set aligns with Armbian compatibility; no blockers. Monitor compile time and image size and consider trimming rarely-used modules if regressions appear.


290-296: Approve — BRIDGE built-in for DSA/Mediatek verified

CONFIG_BRIDGE=y coexists with CONFIG_NET_DSA=y and CONFIG_NET_MEDIATEK_SOC=y (lines 292, 294, 403); keeping bridge built-in while nf-bridge filtering modules remain modular is correct.


417-417: CONFIG_PCS_MTK_USXGMII=y — confirm hardware need; otherwise consider =m.

Repository search for DT hints ('usxgmii|mtk-usxgmii|pcs.*usxgmii') across .dts/.dtsi returned no matches, so DT-based justification wasn't found.
Location: config/kernel/linux-filogic-current.config: line 417.
If any platform requires USXGMII at boot (fixed PHY in DT), keep =y; otherwise prefer =m to reduce always‑on footprint.

lib/functions/compilation/armbian-kernel.sh (4)

287-287: NF_CONNTRACK as module — OK; verify Docker/container bring‑up on a clean image.
Repo kernel configs contain CONFIG_NF_CONNTRACK=m for many targets (some targets use =y); no explicit modprobe/insmod for nf_conntrack found. Confirm by bringing up a container from a minimal image — if nf_conntrack does not autoload, ensure the module is available to the container or load it (modprobe) during host/container init.


214-214: Safe to prefer module for BRIDGE — confirm DSA/MTK board configs

kernel_config_set_m does not downgrade existing CONFIG_BRIDGE=y. Scan shows filogic keeps CONFIG_BRIDGE=y (config/kernel/linux-filogic-current.config:292; CONFIG_NET_DSA=y, CONFIG_NET_MEDIATEK_SOC=y), while several DSA/MTK-target configs already have CONFIG_BRIDGE=m (e.g. config/kernel/linux-k3-current.config:150 with CONFIG_NET_DSA=m; config/kernel/linux-meson64-current.config:435 with CONFIG_NET_DSA=m). Confirm those BRIDGE=m entries are intentional for those boards.


235-237: Mixed CONFIG_NF_TABLES found — confirm overlays changed by this PR
rg output shows many overlays with CONFIG_NF_TABLES=m and CONFIG_NF_TABLES_INET/NETDEV=y (valid), but several overlays set CONFIG_NF_TABLES=y with the suboptions also y. Confirm the overlays modified by this PR do not unintentionally force NF_TABLES built‑in.


255-263: iptables targets changed to modules — ensure early userspace autoload

lib/functions/compilation/armbian-kernel.sh (lines 255–263) now builds IP_NF_* as modules; if any init/early-boot scripts invoke iptables targets before kmod autoload, add an explicit modules-load.d entry or include these modules in the initramfs. Repository search returned only the kernel file and found no early-boot iptables usage — manual verification required.

@tabrisnet
Copy link
Author

Actionable comments posted: 0
🧹 Nitpick comments (1)

lib/functions/compilation/armbian-kernel.sh (1)> 205-335: Extract netfilter modularization into a dedicated helper and call it here.

To reuse beyond Docker and keep this function focused, move the BRIDGE/IP_NF_*/NF_TABLES/NF_CONNTRACK prefs into a new helper, then invoke it from armbian_kernel_config__enable_docker_support.

+function armbian_kernel_config__prefer_netfilter_modules() {
+	if [[ -f .config ]]; then
+		kernel_config_set_m BRIDGE
+		kernel_config_set_m BRIDGE_NETFILTER
+		kernel_config_set_m NF_TABLES
+		kernel_config_set_y NF_TABLES_INET
+		kernel_config_set_y NF_TABLES_NETDEV
+		kernel_config_set_m IP_NF_IPTABLES
+		kernel_config_set_m IP_NF_FILTER
+		kernel_config_set_m IP_NF_SECURITY
+		kernel_config_set_m NF_CONNTRACK
+	fi
+}
@@
 function armbian_kernel_config__enable_docker_support() {
@@
-		kernel_config_set_m BRIDGE                          # Enables support for Ethernet bridges
+		armbian_kernel_config__prefer_netfilter_modules

📜 Review details

@coderabbitai
yes, but if I did so, as I noted in my PR description, I'd want to merge the concepts from the linux-filogic config overlay [build all of the netfilter modules], but first I'd want some kind of feedback from the maintainers.
The idea in part comes from a conversation with rogandawes on Discord/IRC in #armbian-allwinner. unfortunately the official logs don't have that day.

@tabrisnet tabrisnet force-pushed the tabrisnet_net_modules branch from 6a763e2 to 2b52aa9 Compare September 22, 2025 00:51
@tabrisnet tabrisnet changed the title armbian build marchinery - allow more netfilter and related components to be modules armbian build machinery - allow more netfilter and related components to be modules Sep 22, 2025
@tabrisnet tabrisnet force-pushed the tabrisnet_net_modules branch from 2b52aa9 to 3513f06 Compare September 22, 2025 01:05
@tabrisnet
Copy link
Author

@coderabbitai
#8661 filed as alternative to this one

@tabrisnet tabrisnet force-pushed the tabrisnet_net_modules branch from 3513f06 to ac1d908 Compare September 22, 2025 12:37
@tabrisnet tabrisnet marked this pull request as draft September 22, 2025 15:35
@tabrisnet tabrisnet force-pushed the tabrisnet_net_modules branch from ac1d908 to 65d9239 Compare September 24, 2025 10:09
@tabrisnet tabrisnet force-pushed the tabrisnet_net_modules branch from 65d9239 to 51209f0 Compare September 25, 2025 18:20
@tabrisnet
Copy link
Author

obsoleted by #8661

@tabrisnet tabrisnet closed this Sep 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 Milestone: Fourth quarter release Framework Framework components Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review size/medium PR with more then 50 and less then 250 lines
Development

Successfully merging this pull request may close these issues.

1 participant