[Deepin-Kernel-SIG] [linux 6.6-y] [Deepin] net: Fix kabi breakage for skbuff by exclude header #1379
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR conditionally excludes inclusion of <net/gro.h> under Deepin’s KABI-reserve configuration by defining a local GRO_MAX_HEAD macro instead, avoiding KABI changes to skbuff-related symbols without altering runtime behavior. Flow diagram for conditional GRO_MAX_HEAD definition based on CONFIG_DEEPIN_KABI_RESERVEflowchart TD
A[skbuff.c compile time] --> B{CONFIG_DEEPIN_KABI_RESERVE enabled?}
B -- no --> C[Include net/gro.h]
C --> D[Use GRO_MAX_HEAD from net/gro.h]
B -- yes --> E[Do not include net/gro.h]
E --> F[Define GRO_MAX_HEAD as MAX_HEADER + 128]
F --> G[Use local GRO_MAX_HEAD without changing KABI]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Cryolitia The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- The preprocessor branch uses
#elif /* !CONFIG_DEEPIN_KABI_RESERVE */without a condition, which is invalid; this should be#else(with the comment kept if desired) to compile correctly. - Defining
GRO_MAX_HEADlocally risks divergence from the value innet/gro.h; consider centralizing this definition or referencing a shared header to avoid future inconsistencies when protocols with larger headers are added.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The preprocessor branch uses `#elif /* !CONFIG_DEEPIN_KABI_RESERVE */` without a condition, which is invalid; this should be `#else` (with the comment kept if desired) to compile correctly.
- Defining `GRO_MAX_HEAD` locally risks divergence from the value in `net/gro.h`; consider centralizing this definition or referencing a shared header to avoid future inconsistencies when protocols with larger headers are added.
## Individual Comments
### Comment 1
<location> `net/core/skbuff.c:72` </location>
<code_context>
#include <net/checksum.h>
+#ifndef CONFIG_DEEPIN_KABI_RESERVE
#include <net/gro.h>
+#elif /* !CONFIG_DEEPIN_KABI_RESERVE */
+/* This should be increased if a protocol with a bigger head is added. */
+#define GRO_MAX_HEAD (MAX_HEADER + 128)
</code_context>
<issue_to_address>
**issue (bug_risk):** Preprocessor `#elif` is missing a condition and should likely be `#else`.
`#elif` without an expression is invalid preprocessor syntax and will fail to compile. Based on the surrounding `#ifndef CONFIG_DEEPIN_KABI_RESERVE` and closing `#endif`, this branch should be the `#else` case. Please change this line to `#else /* CONFIG_DEEPIN_KABI_RESERVE */` (or equivalent).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin inclusion category: kabi No functioncal change. Log: fix kabi white list symbol in skbuff.c: slab_build_skb build_skb __alloc_skb __netdev_alloc_skb __napi_alloc_skb ... Fixes: dae142e ("net: allow small head cache usage with large MAX_SKB_FRAGS values") Signed-off-by: Wentao Guan <[email protected]>
opsiff
force-pushed
the
linux-6.6.y-2025-12-16-kabi-2
branch
from
7cb4c9e to
d72aa28
Compare
deepin pr auto review我来对这个 diff 进行审查：
改进建议： #ifndef CONFIG_DEEPIN_KABI_RESERVE
#include <net/gro.h>
#else /* CONFIG_DEEPIN_KABI_RESERVE */
/*
* GRO_MAX_HEAD defines the maximum header space needed for GRO (Generic Receive Offload)
* operations. This includes MAX_HEADER plus additional space for protocol headers.
* The current value of 128 bytes should be sufficient for most common protocols,
* but may need to be increased when adding support for new protocols with larger headers.
* TODO: Review this value when adding new protocol support.
*/
#define GRO_MAX_HEAD (MAX_HEADER + 128)
/* Compile-time check to ensure minimum header size */
static_assert(GRO_MAX_HEAD >= MAX_HEADER, "GRO_MAX_HEAD must be at least MAX_HEADER");
#endif /* CONFIG_DEEPIN_KABI_RESERVE */这些改进将：
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a Kernel ABI (KABI) compatibility issue in the Deepin kernel by conditionally excluding the <net/gro.h> header when CONFIG_DEEPIN_KABI_RESERVE is defined. The change prevents ABI breakage for exported symbols in skbuff.c without affecting functionality.
Key Changes:
- Conditionally excludes
<net/gro.h>inclusion whenCONFIG_DEEPIN_KABI_RESERVEis enabled - Defines
GRO_MAX_HEADlocally when the header is excluded to maintain the same constant value - Preserves runtime behavior while protecting KABI compatibility
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <net/checksum.h> | ||
| #ifndef CONFIG_DEEPIN_KABI_RESERVE | ||
| #include <net/gro.h> | ||
| #else /* !CONFIG_DEEPIN_KABI_RESERVE */ |
There was a problem hiding this comment.
The comment on this line is incorrect. It says "!CONFIG_DEEPIN_KABI_RESERVE" but should say "CONFIG_DEEPIN_KABI_RESERVE" since this is the else branch that executes when CONFIG_DEEPIN_KABI_RESERVE is defined. The exclamation mark should be removed.
Suggested change
| #else /* !CONFIG_DEEPIN_KABI_RESERVE */ | |
| #else /* CONFIG_DEEPIN_KABI_RESERVE */ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
deepin inclusion
category: kabi
No functioncal change.
Log: fix kabi white list symbol in skbuff.c:
slab_build_skb
build_skb
__alloc_skb
__netdev_alloc_skb
__napi_alloc_skb
...
Fixes: dae142e ("net: allow small head cache usage with large MAX_SKB_FRAGS values")
Summary by Sourcery
Enhancements: