Backport upstream patch to fix downstream patch collision in RPi 6.6 kernel #3856
Conversation
…kernel Because of refactoring/code quality improvements in upstream, IPv6 reachability patch no longer applied on 6.12 kernel. We added two versions of the patch to address this initially, however, this requires updating of the patch directory name on every kernel bump. Backport the patch causing collision instead to RPi kernel, so we can carry only one version of the patch. This also requires swapping of the patching order - now we first apply board-specific patches, then the global ones. Unless there are collisions, these operations should be idempontent, so at this point it shouldn't have any side-effects.
sairon
added
board/raspberrypi
📝 WalkthroughWalkthroughThe changes introduce a new compile‐time IPv6 reachability probing option by adding the configuration variable Changes
Sequence Diagram(s)sequenceDiagram
participant NP as Network Packet
participant RL as Route Lookup
participant CF as Config Checker
NP->>RL: Receive IPv6 packet
RL->>CF: Check IPv6 forwarding & IPV6_REACHABILITY_PROBE
alt Forwarding disabled or probe enabled
CF-->>RL: Condition met
RL->>RL: Set RT6_LOOKUP_F_REACHABLE flag
else
CF-->>RL: Condition not met
end
RL->>NP: Continue with routing decision
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
buildroot-external/board/raspberrypi/patches/linux/0004-ipv6-annotate-data-races-around-cnf.forwarding.patch (1)
194-195: Mixed usage of READ_ONCE and direct field access.While
READ_ONCE(in6_dev->cnf.forwarding)is correct, consider whetherin6_dev->cnf.accept_redirectsneeds a similar concurrency annotation if it can be updated in other threads. If so, switching toREAD_ONCE(in6_dev->cnf.accept_redirects)could further reduce data race risks.buildroot-external/configs/rpi3_defconfig (1)
9-9: LGTM! Patch directory order change is consistent.The change follows the same pattern as other board configurations, ensuring board-specific patches take precedence.
This consistent reordering across all board configurations suggests a deliberate architectural decision to:
- Prioritize board-specific patches
- Allow global patches as fallback
- Maintain a clear hierarchy in patch application
This approach should make it easier to:
- Override global patches with board-specific ones when needed
- Maintain board-specific customizations without affecting other boards
- Reduce patch collisions by having a clear precedence order
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
buildroot-external/board/raspberrypi/patches/linux/0004-ipv6-add-option-to-explicitly-enable-reachability-te.patch(0 hunks)buildroot-external/board/raspberrypi/patches/linux/0004-ipv6-annotate-data-races-around-cnf.forwarding.patch(1 hunks)buildroot-external/configs/rpi2_defconfig(1 hunks)buildroot-external/configs/rpi3_64_defconfig(1 hunks)buildroot-external/configs/rpi3_defconfig(1 hunks)buildroot-external/configs/rpi4_64_defconfig(1 hunks)buildroot-external/configs/rpi4_defconfig(1 hunks)buildroot-external/configs/rpi5_64_defconfig(1 hunks)
💤 Files with no reviewable changes (1)
- buildroot-external/board/raspberrypi/patches/linux/0004-ipv6-add-option-to-explicitly-enable-reachability-te.patch
✅ Files skipped from review due to trivial changes (1)
- buildroot-external/configs/rpi4_defconfig
🔇 Additional comments (22)
buildroot-external/configs/rpi5_64_defconfig (1)
8-8: LGTM! The patch directory order change aligns with the PR objectives.The modification to prioritize board-specific patches (
board/raspberrypi/patches) before global patches is consistent across all RPi defconfigs. This change supports the PR's goal of fixing the patch collision by ensuring board-specific patches take precedence, while maintaining idempotency as mentioned in the PR objectives.Run the following script to verify the patch application order:
Also applies to: 9-9, 9-9, 9-9
✅ Verification successful
Patch ordering and collision check verified
The board-specific patch directory is correctly prioritized, and our checks didn’t detect any patch collisions. This aligns with the PR objectives.
- Verified the configuration in multiple defconfigs (rpi2, rpi3, rpi4, and rpi5).
- Confirmed that the ordering keeps board-specific patches (
board/raspberrypi/patches) ahead of global patches.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that patches are applied in the correct order with board-specific patches taking precedence. # Test: Check if there are any patches with the same name in both directories echo "Checking for potential patch collisions..." for config in $(fd -t f "rpi.*_defconfig" buildroot-external/configs/); do board_patches_dir=$(grep "BR2_GLOBAL_PATCH_DIR" "$config" | grep -o 'board/raspberrypi/patches') global_patches_dir=$(grep "BR2_GLOBAL_PATCH_DIR" "$config" | grep -o '/patches"' | tr -d '"') if [ -d "$board_patches_dir" ] && [ -d "$global_patches_dir" ]; then echo "Checking patches in $config..." for patch in "$global_patches_dir"/*; do if [ -f "$board_patches_dir/$(basename "$patch")" ]; then echo "Found potential collision: $(basename "$patch")" echo "- Board-specific: $board_patches_dir/$(basename "$patch")" echo "- Global: $patch" fi done fi doneLength of output: 2587
buildroot-external/configs/rpi4_64_defconfig (1)
8-8: LGTM! The patch directory reordering aligns with the PR objectives.The change prioritizes board-specific patches by placing them before global patches in
BR2_GLOBAL_PATCH_DIR. This reordering helps resolve the patch collision issue mentioned in the PR objectives while maintaining an idempotent patch application process.To ensure consistency across all RPi board configurations, run:
✅ Verification successful
Final Verification: Patch Directory Reordering Confirmed
All checked RPi board configuration files correctly list the board-specific patches before the global patches. No defconfig was found with the old (reversed) order. This verifies that the changes align with the PR objectives.
- Verified in files:
rpi2_defconfig,rpi3_defconfig,rpi3_64_defconfig,rpi4_defconfig,rpi4_64_defconfig, andrpi5_64_defconfig.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all RPi board configs have the same patch directory order # Test: Check BR2_GLOBAL_PATCH_DIR in all RPi defconfigs # Expect: All configs should have board patches before global patches rg -l 'BR2_GLOBAL_PATCH_DIR.*board/raspberrypi/patches.*patches' buildroot-external/configs/rpi*_defconfig # Test: Check for any RPi defconfigs with different order # Expect: No matches (old order should be completely replaced) rg -l 'BR2_GLOBAL_PATCH_DIR.*patches.*board/raspberrypi/patches' buildroot-external/configs/rpi*_defconfigLength of output: 916
buildroot-external/board/raspberrypi/patches/linux/0004-ipv6-annotate-data-races-around-cnf.forwarding.patch (18)
1-5: Commit metadata looks good.The commit message and metadata are consistent and clear, detailing the rationale for annotating data races around
cnf.forwarding.
6-9: Good clarification of the concurrency rationale.The note that
idev->cnf.forwardingandnet->ipv6.devconf_all->forwardingmay be accessed locklessly is a helpful pointer for developers.
10-13: Sign-offs are proper.No issues in the signing sequence; standard kernel practice.
14-21: File change summary is accurate.The list of impacted files and net lines changed is correct, providing easy navigation for reviewers.
31-32: Correct use of READ_ONCE in cdc_mbim.c.Switching to
READ_ONCE(in6_dev->cnf.forwarding)is appropriate to protect against concurrent writes.
44-56: Const correctness and atomic reads in ipv6_accept_ra.
- Updating the function signature to
const struct inet6_dev *idevis correct since the function body does not modifyidev.- Using
READ_ONCEfor bothidev->cnf.accept_raandidev->cnf.forwardingensures data-race avoidance.
67-68: Consistent use of READ_ONCE in bpf_ipv6_fib_lookup.Replacing the direct read with
READ_ONCE(idev->cnf.forwarding)maintains concurrency safety.
79-83: Safe read for reporting devconf->forwarding.Using
READ_ONCE(devconf->forwarding)before callingnla_put_s32prevents race conditions ifforwardingchanges mid-read.
89-93: WRITE_ONCE used correctly for idev->cnf.forwarding assignment.Ensuring atomicity when changing the
forwardingfield is essential for concurrency correctness.
100-101: Pointer assignment wrapped with WRITE_ONCE.Applying
WRITE_ONCE(*p, newf)avoids potential data races on the pointer-based assignment.
109-110: Default forwarding assignment properly updated with WRITE_ONCE.This enforces atomic writes on the
devconf_dflt->forwardingfield, matching the rest of the patch’s conventions.
122-123: Forwarding check now protected by READ_ONCE.Ensuring consistent forward-check logic in
ip6_output.cis aligned with the concurrency safeguards introduced here.
135-136: NDP code uses READ_ONCE correctly.Preventing data races when determining if the device is forwarding packets addresses concurrency concerns in neighbor discovery logic.
145-145: Synchronous read when setting is_router.Properly uses
READ_ONCE(idev->cnf.forwarding)to keep the read coherent under parallel updates.
154-154: Router condition guarded by READ_ONCE.Again, ensuring safe concurrent usage of
forwardingin NA message processing.
163-164: Transition to READ_ONCE for checking devconf_all->forwarding.Matches the rest of the patch’s approach to concurrency on
forwarding.
173-174: Consistent concurrency check for RS acceptance.
READ_ONCE(idev->cnf.forwarding)ensures correctness for router solicitation handling.
185-186: Conditional forwarding check uses READ_ONCE.Maintains concurrency guarantees for
net->ipv6.devconf_all->forwarding.buildroot-external/configs/rpi2_defconfig (1)
9-9: LGTM! Patch directory order change aligns with PR objectives.The change prioritizes board-specific patches over global patches, which helps resolve the patch collision issue mentioned in the PR objectives.
Let's verify the patch directories exist and understand their contents:
✅ Verification successful
Below is an updated verification script that explicitly sets the environment variable (assuming that the repository’s base directory for external patches is
"buildroot-external") before checking for the patch directories and listing their contents. This should help us verify that the board‐specific and global patch directories exist as expected.
Patch directory order verified:
- The board-specific patch directory exists at
buildroot-external/board/raspberrypi/patches.- The global patch directory exists at
buildroot-external/patches.Both directories were detected in the repository, and the board-specific directory is prioritized in the configuration as expected. This aligns with the PR objectives to resolve patch collisions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify patch directories and their contents # Check if both patch directories exist echo "Checking patch directories..." fd -t d -g "*patches" "$(BR2_EXTERNAL_HASSOS_PATH)" # List patches in both directories echo -e "\nListing patches in board-specific directory..." fd -t f -e patch "$(BR2_EXTERNAL_HASSOS_PATH)/board/raspberrypi/patches" echo -e "\nListing patches in global directory..." fd -t f -e patch "$(BR2_EXTERNAL_HASSOS_PATH)/patches"Length of output: 908
Script:
#!/bin/bash # Set the BR2_EXTERNAL_HASSOS_PATH to simulate the external configuration environment. export BR2_EXTERNAL_HASSOS_PATH="buildroot-external" echo "Using BR2_EXTERNAL_HASSOS_PATH = ${BR2_EXTERNAL_HASSOS_PATH}" echo "Checking for patch directories..." fd -t d -g "*patches" "${BR2_EXTERNAL_HASSOS_PATH}" --color=never echo -e "\nListing patches in board-specific directory..." if [ -d "${BR2_EXTERNAL_HASSOS_PATH}/board/raspberrypi/patches" ]; then fd -t f -e patch "${BR2_EXTERNAL_HASSOS_PATH}/board/raspberrypi/patches" --color=never else echo "Directory ${BR2_EXTERNAL_HASSOS_PATH}/board/raspberrypi/patches does not exist." fi echo -e "\nListing patches in global directory..." if [ -d "${BR2_EXTERNAL_HASSOS_PATH}/patches" ]; then fd -t f -e patch "${BR2_EXTERNAL_HASSOS_PATH}/patches" --color=never else echo "Directory ${BR2_EXTERNAL_HASSOS_PATH}/patches does not exist." fiLength of output: 2109
buildroot-external/configs/rpi3_64_defconfig (1)
8-8: LGTM! Consistent patch directory ordering.The change maintains consistency with other board configurations by prioritizing board-specific patches.
After #3856, we need to change the patch order also for Yellow.
After #3856, we need to change the patch order also for Yellow.
Because of refactoring/code quality improvements in upstream, IPv6 reachability patch no longer applied on 6.12 kernel. We added two versions of the patch to address this initially, however, this requires updating of the patch directory name on every kernel bump. Backport the patch causing collision instead to RPi kernel, so we can carry only one version of the patch.
This also requires swapping of the patching order - now we first apply board-specific patches, then the global ones. Unless there are collisions, these operations should be idempontent, so at this point it shouldn't have any side-effects.
Summary by CodeRabbit