Skip to content

CP: Modified cmake to disable xdp for arm architecture #5357

Merged
guhetier merged 1 commit intorelease/2.5from
guhetier/fix_pipeline_build
Aug 15, 2025
Merged

CP: Modified cmake to disable xdp for arm architecture #5357
guhetier merged 1 commit intorelease/2.5from
guhetier/fix_pipeline_build

Conversation

@guhetier
Copy link
Collaborator

Description

The undock pipeline uses the same cmake flags for all the archs of a target container which means it will throw an error when QUIC_LINUX_XDP_ENABLED is on for arm architectures as its not supported. This PR adds a check to disable the flag for arm archs.

Testing

CI

Documentation

N/A

@guhetier guhetier requested a review from a team as a code owner August 14, 2025 21:24
@guhetier guhetier changed the base branch from main to release/2.5 August 14, 2025 21:25
if(CMAKE_SYSTEM_PROCESSOR MATCHES "^(arm|aarch64)$")
set(QUIC_LINUX_XDP_ENABLED OFF CACHE BOOL "XDP not supported on ARM architectures" FORCE)
else()
option(QUIC_LINUX_XDP_ENABLED "Enables XDP support" OFF)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is a cherry-pick, but this works? It looks like it just turns off Linux XDP always

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed offline: it seems like when setting the option, the user provided value is used. I am not sure why the option needs to be duplicated from the top of the file, but I'll keep what works in main for here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gaurav2699 If you can chime in about this it would be helpful, so we make sure the state in main is clean.

Copy link
Contributor

@gaurav2699 gaurav2699 Aug 15, 2025

Choose a reason for hiding this comment

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

@anrossi As Guillaume mentioned, the user provided value will be used and it wont stay on. @guhetier I agree the else part is useless.

Copy link
Collaborator

@anrossi anrossi left a comment

Choose a reason for hiding this comment

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

I'll approve since it's a cherry-pick, but I'm curious to have my question answered

@guhetier guhetier closed this Aug 14, 2025
@guhetier guhetier reopened this Aug 14, 2025
@codecov
Copy link

codecov bot commented Aug 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.11%. Comparing base (eb8d2e1) to head (b7c09e3).
⚠️ Report is 2 commits behind head on release/2.5.

Additional details and impacted files
@@             Coverage Diff              @@
##           release/2.5    #5357   +/-   ##
============================================
  Coverage        86.11%   86.11%           
============================================
  Files               59       59           
  Lines            18173    18173           
============================================
  Hits             15650    15650           
  Misses            2523     2523           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guhetier guhetier merged commit 879837a into release/2.5 Aug 15, 2025
285 checks passed
@guhetier guhetier deleted the guhetier/fix_pipeline_build branch August 15, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants