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

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]>

``` Section Headers: [Nr] Name Type Address Off Size ES Flg Lk Inf Al [ 0] NULL 0000000000000000 000000 000000 00 0 0 0 [ 1] .strtab STRTAB 0000000000000000 00036b 000077 00 0 0 1 [ 2] .text PROGBITS 0000000000000000 000040 000000 00 AX 0 0 4 [ 3] struct_ops/test_1 PROGBITS 0000000000000000 000040 000010 00 AX 0 0 8 [ 4] license PROGBITS 0000000000000000 000050 000004 00 WA 0 0 1 [ 5] .struct_ops.link PROGBITS 0000000000000000 000058 000018 00 WA 0 0 8 [ 6] .rel.struct_ops.link REL 0000000000000000 000318 000010 10 I 12 5 8 [ 7] .BTF PROGBITS 0000000000000000 000070 0001e0 00 0 0 4 [ 8] .rel.BTF REL 0000000000000000 000328 000020 10 I 12 7 8 [ 9] .BTF.ext PROGBITS 0000000000000000 000250 000050 00 0 0 4 [10] .rel.BTF.ext REL 0000000000000000 000348 000020 10 I 12 9 8 [11] .llvm_addrsig LLVM_ADDRSIG 0000000000000000 000368 000003 00 E 0 0 1 [12] .symtab SYMTAB 0000000000000000 0002a0 000078 18 1 2 8 ``` Signed-off-by: shun159 <[email protected]>

Signed-off-by: shun159 <[email protected]>

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?

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.

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.

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.

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.

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.

Signed-off-by: shun159 <[email protected]>

Co-authored-by: Lorenz Bauer <[email protected]> Signed-off-by: Eishun Kondoh <[email protected]>

Signed-off-by: shun159 <[email protected]>

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

Signed-off-by: shun159 <[email protected]>

…ctOpsRelocs Signed-off-by: shun159 <[email protected]>

shun159 · 2025-10-19T09:03:22Z

elf_reader.go

 	return nil
 }

+// associateStructOpsRelocs handles `.struct_ops.link`


TODO: need to be refactored.

Signed-off-by: shun159 <[email protected]>

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

Draft

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 added 4 commits October 17, 2025 00:53

elf: move logic which finds fp member into a function

025f0f2

Signed-off-by: shun159 <[email protected]>

elf: KeySize of a struct_ops map should always be 4

ef7d704

Signed-off-by: shun159 <[email protected]>

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 and others added 8 commits October 17, 2025 19:26

testing: reenable TestLibBPFCompat

40e9734

Signed-off-by: shun159 <[email protected]>

elf: keep the sec.Size condition

0be509e

Co-authored-by: Lorenz Bauer <[email protected]> Signed-off-by: Eishun Kondoh <[email protected]>

elf: fix to refuse ".struct_ops" section explicitly

fb27620

Signed-off-by: shun159 <[email protected]>

elf: reading user data from sec.Data only needs to be done onc.

9921ee7

Signed-off-by: shun159 <[email protected]>

elf: refactored to use TypeByName/UnderlyingType

8d2dc60

Signed-off-by: shun159 <[email protected]>

elf, struct_ops: move helper function and section name

80c5463

Signed-off-by: shun159 <[email protected]>

elf,struct_ops: fix error message in ".struct_ops" guard

3ab9db7

Signed-off-by: shun159 <[email protected]>

add handle unknown struct_ops target

b2d4a33

Signed-off-by: shun159 <[email protected]>

shun159 added 2 commits October 19, 2025 15:44

elf: reject standalone struct_ops prog

558dc8b

Signed-off-by: shun159 <[email protected]>

remove structOpsSpec, and merged loadStructOpsMaps into associateStru…

a3ac2c8

…ctOpsRelocs Signed-off-by: shun159 <[email protected]>

shun159 commented Oct 19, 2025

View reviewed changes

elf_reader.go

return nil

}

// associateStructOpsRelocs handles `.struct_ops.link`

Copy link

Contributor Author

shun159 Oct 19, 2025

Choose a reason for hiding this comment

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

TODO: need to be refactored.

testing: fix typo in struct_ops_autocreate

0743fe4

Signed-off-by: shun159 <[email protected]>

elf_reader: add struct_ops support #1869

Are you sure you want to change the base?

elf_reader: add struct_ops support #1869

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants