elf_reader: add struct_ops support #1869

shun159 · 2025-09-29T16:01:58Z

This commit adds struct_ops support to the ELF reader: it classifies non-executable PROGBITS sections, parses their BTF Datasec to build MapSpecs, associates relocs with func-pointer members to set ps.AttachTo, and adds TestStructOps.

Related: #1845

elf_reader.go

lmb

Please refuse loading bare .struct_ops and re-enable the LibBPFCompat test of StructOps.

lmb · 2025-10-10T16:33:58Z

elf_reader.go

+		}
+
+		for _, vsi := range dataSec.Vars {
+			varType, ok := btf.As[*btf.Var](vsi.Type)


Why mix btf.As and btf.UnderlyingType here?

removed such code now.

lmb · 2025-10-10T16:35:37Z

elf_reader.go

+
+			// Retrieve raw data from the ELF section.
+			// This data contains the initial values for the struct_ops map.
+			userData, err := sec.Data()


Why do this for each iteration? Can be done once.

elf_reader.go

lmb · 2025-10-16T15:52:15Z

elf_reader.go

+		case sec.Type == elf.SHT_PROGBITS:
+			if (sec.Flags&elf.SHF_EXECINSTR) != 0 && sec.Size > 0 {
+				sections[idx] = newElfSection(sec, programSection)
+			} else if sec.Name == ".struct_ops" || sec.Name == ".struct_ops.link" {


Refuse ".struct_ops" here.

Would it make sense to still allow .struct_ops parsing to Spec, but then not allow loading unless you manually add BPF_F_LINK to the map spec?

If you block it in ELF parsing, then you force users to modify the section, and take away the ability to patch this in userspace.

The reason for not using BPF_F_LINK is to get compat with older kernels, no? Adding BPF_F_LINK manually then kind of defeats the purpose?

I agree that it's probably nicer to refuse loading a StructOpsMap without BPF_F_LINK instead, then we can test more of the libbpf ELFs.

The reason for not using BPF_F_LINK is to get compat with older kernels, no? Adding BPF_F_LINK manually then kind of defeats the purpose?

Yea, BPF_F_LINK got added in 6.4, and struct ops originally got added in 5.6. In the end, its all about the attachment, there is not difference in the how you write the programs. Its only really tcp_congestion_ops that existed before BPF_F_LINK so its examples and selftests use .struct_ops. Schedext and HIDBPF both have macros for defining the struct which do use .struct_ops.link, since those features came after 6.4.

ack. let's refuse ".struct_ops" here.

lmb · 2025-10-16T15:53:54Z

elf_reader.go

 			continue
 		}

+		if !(sec.Type == elf.SHT_PROGBITS && (sec.Flags&elf.SHF_EXECINSTR) != 0) {


Why is this necessary? if kind == programSection then this should always be true.

@shun159 still TODO.

@lmb yes, right. it is unnecessary guard.

lmb · 2025-10-16T15:55:36Z

elf_reader.go

+		}
+
+		// Process the struct_ops section to create the map
+		dataType, err := ec.btf.AnyTypeByName(sec.Name)


This should use TypeByName.

this was addressed

lmb · 2025-10-16T15:57:28Z

elf_reader.go

+					Type:       StructOpsMap,
+					Key:        &btf.Int{Size: 4},
+					KeySize:    4,
+					Value:      userType,


Maybe also set ValueSize for completeness sake?

@lmb At this point, the size of the value type is not yet determined, so my understanding is that it’s difficult to specify this field for now.

Size should be btf.Sizeof(userType) no?

@lmb this is the size of the section data of the user-side struct, which is different from the size of the kernel-side value type.

we specified the length of the user-type as the value type now

lmb · 2025-10-16T16:03:44Z

elf_reader.go

 	Name    string
 }

+type structOpsSpec struct {


I don't think this is necessary. elfSection.relocations already carries this information in the form of an elf.Symbol.

right. this was removed.

lmb · 2025-10-16T16:04:41Z

elf_reader.go


+// loadStructOpsMapsFromSections creates StructOps MapSpecs from DataSec sections
+// ".struct_ops" and ".struct_ops.link" found in the object BTF.
+func (ec *elfCode) loadStructOpsMaps() error {


This should be merged with associateStructOpsRelocs unless there is a reason why they need to be separate which I don't understand.

lmb · 2025-10-16T16:06:19Z

elf_reader.go

+	relSecs map[elf.SectionIndex]*elf.Section,
+	symbols []elf.Symbol,
+) error {
+	for _, sec := range relSecs {


This just duplicates the loop in loadRelocations? You can iterate sec.relocations instead.

outdated, but I will take a look this.

we can use sec.relocations. fixed now.

shun159 · 2025-10-18T08:35:53Z

re-enable the LibBPFCompat test of StructOps

@lmb @dylandreimerink

It seems that in the test cases for programs that include "autocreate", the program fails when load because it cannot determine which map it belongs to.
I believe at this point such cases should be treated as NotSupoorted?

test case: https://github.com/cilium/ebpf/actions/runs/18612775255/job/53073227432?pr=1869#step:8:1224
test data : https://elixir.bootlin.com/linux/v6.17.1/source/tools/testing/selftests/bpf/progs/struct_ops_autocreate2.c#L25

elf_reader.go

lmb · 2025-10-20T10:33:03Z

I believe at this point such cases should be treated as NotSupoorted?

That failure just tells me that AttachTo is not populated. Can you explain why that is?

shun159 · 2025-10-20T10:54:13Z

That failure just tells me that AttachTo is not populated. Can you explain why that is?

@lmb when given the code below, (for now) we cannot derive which struct_ops map the standalone program foo belongs to, so handling AttachTo for foo fails in this case.

#include "common.h"

char _license[] __section("license") = "GPL";

struct bpf_testmod_ops {
	int (*test_1)(void);
	int data;
};

__section("?struct_ops/test_1") int foo(void) {
	return 0;
}

__section("?struct_ops/test_1") int bar(void) {
	return 0;
}

__section(".struct_ops.link") struct bpf_testmod_ops testmod_ops = {
	.test_1 = (void *)bar,
};

lmb · 2025-10-20T11:25:00Z

(for now) we cannot derive which struct_ops map the standalone program foo belongs to, so handling AttachTo for foo fails in this case.

How will we derive that in the future then? IMO the correct semantics here is to set AttachTo == "" in the ELF reader if a structops program is not referenced by a structops section.

shun159 · 2025-10-27T14:43:13Z

rebased to main

lmb

Please take a look at elfSection.relocations, that should contain what you need. Thanks for adding a thorough test case again!

elf_reader.go

struct_ops.go

elf_sections.go

elf_reader.go

lmb · 2025-10-27T15:53:49Z

elf_reader.go

 			continue
 		}

+		if !(sec.Type == elf.SHT_PROGBITS && (sec.Flags&elf.SHF_EXECINSTR) != 0) {


@shun159 still TODO.

shun159 · 2025-10-29T02:58:43Z

@lmb could you please review at your leisure?

lmb

Some nits, I think this is good to go. We're going to cut a release and merge this afterwards so that we get some more time to iterate.

elf_reader.go

shun159 · 2025-10-29T14:53:38Z

@lmb I've addressed your feedback! 😄

This commit adds struct_ops support to the ELF reader: it classifies non-executable PROGBITS sections, parses their BTF Datasec to build MapSpecs, associates relocs with func-pointer members to set ps.AttachTo, and adds TestStructOps. Related: cilium#1845 Signed-off-by: shun159 <[email protected]>

shun159 · 2025-11-01T07:53:49Z

@lmb thx for merging this, and #1844 is the last PR for StructOps feature.

shun159 requested a review from a team as a code owner September 29, 2025 16:01

shun159 mentioned this pull request Oct 3, 2025

link: add StructOpsMap support #1844

Open

shun159 commented Oct 15, 2025

View reviewed changes

elf_reader.go Outdated Show resolved Hide resolved

dylandreimerink mentioned this pull request Oct 16, 2025

add StructOpsMap support #1845

Merged

shun159 force-pushed the feature/struct-ops-3 branch from 6e099b2 to ef7d704 Compare October 16, 2025 15:53

lmb requested changes Oct 16, 2025

View reviewed changes

shun159 commented Oct 19, 2025

View reviewed changes

elf_reader.go Show resolved Hide resolved

shun159 force-pushed the feature/struct-ops-3 branch from d96bed6 to 79cc8a4 Compare October 27, 2025 14:42

lmb requested changes Oct 27, 2025

View reviewed changes

shun159 requested a review from lmb October 29, 2025 11:32

lmb reviewed Oct 29, 2025

View reviewed changes

elf_reader.go Show resolved Hide resolved

elf_reader.go Outdated Show resolved Hide resolved

elf_reader.go Outdated Show resolved Hide resolved

shun159 requested review from dylandreimerink and lmb October 29, 2025 15:15

lmb force-pushed the feature/struct-ops-3 branch from 992536f to f34a9d8 Compare October 31, 2025 14:45

lmb approved these changes Oct 31, 2025

View reviewed changes

lmb merged commit 19b4163 into cilium:main Oct 31, 2025
31 of 33 checks passed

shun159 deleted the feature/struct-ops-3 branch November 1, 2025 07:16

elf_reader: add struct_ops support #1869

elf_reader: add struct_ops support #1869

Conversation

shun159 commented Sep 29, 2025

Uh oh!

Uh oh!

lmb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shun159 commented Oct 18, 2025

Uh oh!

Uh oh!

lmb commented Oct 20, 2025

Uh oh!

shun159 commented Oct 20, 2025

Uh oh!

lmb commented Oct 20, 2025

Uh oh!

shun159 commented Oct 27, 2025

Uh oh!

lmb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!