Fix compatibility with Linux kernel 6.15–6.16 ABI/API changes#302
Fix compatibility with Linux kernel 6.15–6.16 ABI/API changes#302sheviks wants to merge 5 commits intoXilinx-CNS:masterfrom
Conversation
… structure Building with 'scripts/onload_build --kernelver <clang-compiled kernel>' would fail due to unsupported Clang extension: 'error: fields must have a constant size: variable length array in structure'. This commit updates the affected #if conditions to use valid macros.
- Rename `page->index` to `page->__folio_index` [acc53a0](torvalds/linux@acc53a0) - `UNIXCB()` usage is now restricted to internal only as per upstream change [84960bf](torvalds/linux@84960bf) - Replace `d_hash_and_lookup()` with `try_lookup_noperm()` [06c5674](torvalds/linux@06c5674) - Fix `EFRM_HAVE_OLD_FAULT`; `vm_operations_struct::fault()` has always been single-parameter, and `EFRM_HAVE_NEW_FAULT` remains valid [1c8f422](torvalds/linux@1c8f422)
|
I've applied @lukester1975's
If needed, I can commit and push the changes so they can be included and await merge. Thanks! |
|
I just realized that when I initially addressed the When copying The function using This indicates that using If
Both methods require the kernel to be built with the appropriate debug configuration options enabled (e.g., If we completely avoid using Does anyone have other ideas or suggestions? |
|
clone the repo. swtiched and tried the branch but on a rocky linux 6.15 i am getting: what am i missing ? |
|
This issue is due to the absence of @lukester1975's If you'd like to test the compilation, you can try: This branch additionally includes:
Thanks!
|
|
nice. compiled and tested. works well ! |
There was a problem hiding this comment.
This file is part of the sfc Linux net driver which is managed in a separate project so we would make changes there. Linux 6.15 build compatibility changes have already been made in the net driver so this particular change is no longer needed since 2b28cf9, thanks!
There was a problem hiding this comment.
This file is part of the sfc Linux net driver which is managed in a separate project so we would make changes there. Linux 6.15 build compatibility changes have already been made in the net driver so this particular change is no longer needed since 2b28cf9, thanks!
kieranm-xilinx
left a comment
There was a problem hiding this comment.
Thanks for the pull request. This is helpful in identifying the areas of change needed. I've left comments for the individual parts, as I think the Onload team needs to make some changes before merging. Likely we'll split this into small changes so we can immediately merge some parts.
| /* verifies compile time expression is 0 or positive - | ||
| * no-op for runtime expressions */ | ||
| #if __GNUC__ * 100 + __GNUC_MINOR__ >= 409 | ||
| #if defined(__clang__) || __GNUC__ * 100 + __GNUC_MINOR__ >= 409 |
There was a problem hiding this comment.
Although we don't officially support clang I'm inclined to recommend taking this fix as it will make life easier for any clang users and seems harmless for everyone else.
There was a problem hiding this comment.
If the Clang-related changes above are accepted, I’d really appreciate it if you could also include this commit: sheviks@9525ca6.
Clang emits more errors than GCC; adding that commit ensures Clang generates the same autocompat.h as GCC.
This is also harmless for users not using Clang.
Thanks!
There was a problem hiding this comment.
I’ve committed the change. Please see this commit for details: sheviks@dc0b741
Thank you.
There was a problem hiding this comment.
After I committed the patch, GitHub reported conflicts. When I resolved them, GitHub automatically added a merge commit.
Because of that, I found that the commit mentioned in issue #293 (2b28cf9) does not include all ccflags-y changes.
For example:
- src/driver/linux_net/scripts/kernel_compat_funcs.sh has been updated to use
ccflags-y, - but src/driver/linux_resource/kernel_compat_funcs.sh still uses
EXTRA_CFLAGS.
A grep for EXTRA_CFLAGS on current master still shows quite a few remaining occurrences.
Could you please take another look and make sure the ccflags-y changes are applied consistently?
Alternatively, if you'd prefer, I can review them and submit the fixes here.
Thank you.
There was a problem hiding this comment.
I reconsidered the approach. Since this PR will be split into smaller merges, I went ahead and committed the ccflags-y changes so they’re ready for review.
Please review and decide on the final merge strategy.
Thanks!
| EFRM_NET_HAS_USER_NS member struct_net user_ns include/net/net_namespace.h | ||
|
|
||
| EFRM_HAVE_OLD_FAULT memtype struct_vm_operations_struct fault include/linux/mm.h int (*)(struct vm_area_struct *vma, struct vm_fault *vmf) | ||
| EFRM_HAVE_OLD_FAULT memtype struct_vm_operations_struct fault include/linux/mm.h int (*)(struct vm_fault *vmf) |
There was a problem hiding this comment.
I'd like us to understand why this was wrong before changing it. It has been this way for a long time. I appreciate that as this repository doesn't have the history pre-2020, it makes it difficult for external contributors to do that kind of archaeology, so I think this is a task for the AMD team. I'm wondering if when this was written, what was "new" then has become "old" now. I.e. there are potentially three signatures we need to interoperate with, although I'm hoping that the original "old" is now so old that we don't need it.
In summary, I think this change is probably correct, but I want to check history to be sure.
| #include "ef100_vdpa.h" | ||
| #include "mcdi_filters.h" | ||
| #endif | ||
| #ifdef EFX_HAVE_TRY_LOOKUP_NOPERM |
There was a problem hiding this comment.
The changes to src/driver/linux_net/* need to go via the AMD net driver team, as we consume the driver unmodified from them. We'll work internally to get this kernel supported with them, and then import a new version of the net driver to the Onload repository.
| EFRM_HAVE_FOLLOW_PTE symbol follow_pte include/linux/mm.h | ||
| EFRM_HAVE_FOLLOW_PTE_VMA symtype follow_pte include/linux/mm.h int(struct vm_area_struct*, unsigned long, pte_t**, spinlock_t**) | ||
|
|
||
| EFRM_PAGE_HAS_FOLIO_INDEX member struct_page __folio_index include/linux/mm_types.h |
There was a problem hiding this comment.
This looks good to me.
| EFRM_HAVE_FOLLOW_PTE_VMA symtype follow_pte include/linux/mm.h int(struct vm_area_struct*, unsigned long, pte_t**, spinlock_t**) | ||
|
|
||
| EFRM_PAGE_HAS_FOLIO_INDEX member struct_page __folio_index include/linux/mm_types.h | ||
| EFRM_NEED_UNIXCB nsymbol UNIXCB include/net/af_unix.h |
There was a problem hiding this comment.
This, and the code that uses it in shrub_socket_kernel.c needs closer inspection. While it might succeed in getting it to compile on recent kernels, we need to be careful about introducing new kernel definitions as they can be fragile to maintain. If there's a way we can avoid that, we'd have a strong preference for that. AMD Onload team to have a look at this.
| #ifdef EFRM_PAGE_HAS_FOLIO_INDEX | ||
| pgoff_t index = page->__folio_index; | ||
| #else | ||
| pgoff_t index = page->index; | ||
| #endif | ||
| #if defined(EFRM_HAS_FILEMAP_LOCK_HUGETLB_FOLIO) && ! defined(EFRM_HAS_HUGETLB_BASEPAGE_INDEX) | ||
| return page->index * PAGE_SIZE; | ||
| return index * PAGE_SIZE; | ||
| #else | ||
| return page->index * OO_HUGEPAGE_SIZE; | ||
| return index * OO_HUGEPAGE_SIZE; |
| #ifdef EFRM_NEED_UNIXCB | ||
| struct unix_skb_parms { | ||
| struct pid *pid; /* skb credentials */ | ||
| kuid_t uid; | ||
| kgid_t gid; | ||
| struct scm_fp_list *fp; /* Passed files */ | ||
| #ifdef CONFIG_SECURITY_NETWORK | ||
| u32 secid; /* Security ID */ | ||
| #endif | ||
| u32 consumed; | ||
| } __randomize_layout; | ||
|
|
||
| #define UNIXCB(skb) (*(struct unix_skb_parms *)&((skb)->cb)) | ||
| #endif | ||
|
|
There was a problem hiding this comment.
See above comment for UNIXCB - Onload team to have a look
|
I have replied above regarding the Clang-related part. Thank you! |
This patch is an extension of issue #293, addressing additional ABI/API changes required to successfully build against kernel 6.15+.
In addition to the
kbuild ccflags-yissue reported in #293, this update resolves several compatibility problems introduced in recent upstream kernel changes:Renamed
page->indextopage->__folio_indexUpstream commit: acc53a0
UNIXCB()usage has been restricted to internal useUpstream commit: 84960bf
Replaced
d_hash_and_lookup()withtry_lookup_noperm()Upstream commit: 06c5674
Corrected
EFRM_HAVE_OLD_FAULTusage:vm_operations_struct::fault()has consistently been a single-parameter function;EFRM_HAVE_NEW_FAULTremains validUpstream commit: 1c8f422
Resolved Clang build error
When building via
env LLVM=1 scripts/onload_build --kernelver <clang-compiled kernel>, compilation would fail due to unsupported Clang extension:error: fields must have a constant size: variable length array in structureThe affected
#ifconditions have now been updated to use valid macros.This patch introduces some new kernel_compact tests and can be merged independently. However, to fully build on Linux 6.15+, the
kbuildadjustments from issue #293 are still required.