Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 5, 2025

  • Explore repository structure and understand the problem
  • Locate relevant files (hook/map.go, libbpf package)
  • Add C wrapper for bpf_program__set_autoload in libbpf_api.h
  • Add Go binding for SetProgramAutoload in libbpf.go
  • Modify loadObj in map.go to retry without calico_tc_skb_ipv4_frag on load failure
  • Fix error handling in SetProgramAutoload
  • Track skipIPDefrag flag to skip program in allocateLayout
  • Address code review feedback (extract helper, remove unnecessary return)
  • Run security scan (no issues found)
  • Add SetProgramAutoload stub for non-CGO builds
  • Change SetProgramAutoload to not return error (always succeeds)

Summary

This PR implements a fallback mechanism for loading BPF programs when the calico_tc_skb_ipv4_frag program causes loading failures on kernels that don't support it.

Changes:

  1. Added bpf_set_program_autoload() C wrapper to expose libbpf's bpf_program__set_autoload()
  2. Added SetProgramAutoload() Go binding in libbpf package (both CGO and stub implementations)
  3. Modified loadObj() to retry without IP defrag program on load failure when at.hasIPDefrag() is true
  4. Updated allocateLayout() to skip the IP defrag program when explicitly disabled
  5. Extracted configureMaps() helper to reduce code duplication

Behavior:
When obj.Load() fails at lines 206-208 and at.hasIPDefrag() is true:

  • Close and reopen the BPF object
  • Reconfigure all maps
  • Disable autoload for calico_tc_skb_ipv4_frag using bpf_program__set_autoload()
  • Retry loading
  • Skip adding the IP defrag program to the jump map during layout allocation

Tail calls into the disabled program will fail, but this is acceptable as per requirements.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@marvin-tigera marvin-tigera added this to the Calico v3.32.0 milestone Nov 5, 2025
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Nov 5, 2025
Copilot AI changed the title [WIP] Update obj.Load logic for IP defragmentation Add fallback to load BPF programs without calico_tc_skb_ipv4_frag on failure Nov 5, 2025
Copilot AI requested a review from tomastigera November 5, 2025 20:08
Copilot finished work on behalf of tomastigera November 5, 2025 20:08
@tomastigera tomastigera added docs-not-required Docs not required for this change and removed docs-pr-required Change is not yet documented labels Nov 5, 2025

// SetProgramAutoload sets whether a program should be automatically loaded.
// When set to false, the program will not be loaded when Load() is called.
func (o *Obj) SetProgramAutoload(progName string, autoload bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot update libbpf_stub.go as well with an empty implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in ad1ca79

Copilot AI requested a review from tomastigera November 5, 2025 23:07
Copilot finished work on behalf of tomastigera November 5, 2025 23:07
@tomastigera tomastigera marked this pull request as ready for review November 5, 2025 23:21
@tomastigera tomastigera requested a review from a team as a code owner November 5, 2025 23:21
Copilot AI review requested due to automatic review settings November 5, 2025 23:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a fallback mechanism to handle BPF program loading failures on kernels that don't support the calico_tc_skb_ipv4_frag program. When the initial load fails, the system retries without the IP defragmentation program.

  • Adds libbpf bindings for controlling program autoload via bpf_program__set_autoload()
  • Implements retry logic in loadObj() to disable and skip the IP defrag program on load failure
  • Refactors map configuration into a reusable helper function

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
felix/bpf/libbpf/libbpf_api.h Adds C wrapper bpf_set_program_autoload() to expose libbpf's autoload control function
felix/bpf/libbpf/libbpf.go Implements Go binding SetProgramAutoload() for CGO builds
felix/bpf/libbpf/libbpf_stub.go Adds stub implementation of SetProgramAutoload() for non-CGO builds
felix/bpf/hook/map.go Implements fallback logic and extracts configureMaps() helper to reduce duplication
_codeql_detected_source_root CodeQL configuration file (not a functional change)


// SetProgramAutoload sets whether a program should be automatically loaded.
// When set to false, the program will not be loaded when Load() is called.
func (o *Obj) SetProgramAutoload(progName string, autoload bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot This function never returns an error. Amend the stub as well and do not handle the error where this function is called

Suggested change
func (o *Obj) SetProgramAutoload(progName string, autoload bool) error {
func (o *Obj) SetProgramAutoload(progName string, autoload bool) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in ccdc113 - changed function signature to not return error in both libbpf.go and libbpf_stub.go, and updated the caller in map.go to not handle the error.

Copilot AI requested a review from tomastigera November 6, 2025 16:51
Copilot finished work on behalf of tomastigera November 6, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick-candidate docs-not-required Docs not required for this change release-note-required Change has user-facing impact (no matter how small)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants