Skip to content

Conversation

@mejedi
Copy link
Contributor

@mejedi mejedi commented Jun 18, 2025

Linux syscall program errors on non-nil ctxOut, uses ctxIn for both input and output [1]. We have separate Context and ContextOut fields in RunOptions. It should be possible to capture output from syscall programs which is currently not: non-nil ContextOut triggers EINVAL, while Context is (expectedly) not updated.

Implement the following semantics: either Context, or ContextOut or both can be non-nil. Map to ctxIn internally, complain if Context and ContextOut lengths are different.

[1] https://elixir.bootlin.com/linux/v6.15.2/source/net/bpf/test_run.c#L1530

@mejedi mejedi requested a review from ti-mo June 18, 2025 11:24
@mejedi mejedi requested a review from a team as a code owner June 18, 2025 11:24
@mejedi

This comment was marked as resolved.

@mejedi mejedi force-pushed the syscall-prog-ctx-out branch from f86497d to be7dfb0 Compare June 18, 2025 22:59
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Could you add a test which executes a syscall program or are they too complicated to set up?

@mejedi mejedi force-pushed the syscall-prog-ctx-out branch 5 times, most recently from f381284 to 75af2bc Compare June 22, 2025 12:34
@mejedi mejedi requested a review from lmb June 22, 2025 12:43
prog.go Outdated
// Linux syscall program errors on non-nil ctxOut, uses ctxIn
// for both input and output. Shield the user from this wart.
sysCtxOut = nil
if ctxIn == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this set ctxOut?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ctxOut is unmarshaled from later.

We need it to be the same as the syscall input (ctxIn == ctxOut). We can't ctxIn = ctxOut since it has been possibly marshaled to, therefore ctxOut = ctxIn.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that this should just fail if no input is provided, since that is what upstream does as well. We don't support in == nil && out != nil for other types, no?

Copy link
Contributor Author

@mejedi mejedi Jun 25, 2025

Choose a reason for hiding this comment

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

We don't support in == nil && out != nil for other types, no?

It looks like it is supposed to work — see for instance bpf_ctx_init — but it doesn't, the below program fails with EINVAL (arguments to bpf syscall look reasonable).

I still feel that we shouldn't repurpose Context for output. I will DID simplify by dropping support for in == nil && out != nil.

package main

import (
	"log"

	"github.com/cilium/ebpf"
	"github.com/cilium/ebpf/asm"
)

func main() {
	prog, err := ebpf.NewProgram(&ebpf.ProgramSpec{
		Type: ebpf.XDP,
		Instructions: []asm.Instruction{
			asm.Mov.Imm(asm.R2, -8),
			asm.FnXdpAdjustMeta.Call(),
			asm.Return(),
		},
	})
	if err != nil {
		log.Fatal(err)
	}

	data := make([]byte, 128)
	dataOut := make([]byte, 1024)
	ctx := struct{ Data, DataEnd, DataMeta, I1, I2, I3 uint32 }{DataEnd: 128}

	rc, err := prog.Run(&ebpf.RunOptions{Data: data, DataOut: dataOut, ContextOut: &ctx})
	log.Print(rc, err, ctx)
}

@mejedi mejedi force-pushed the syscall-prog-ctx-out branch from 75af2bc to 1e39130 Compare June 25, 2025 10:00
@mejedi mejedi requested a review from lmb June 25, 2025 10:02
Linux syscall program errors on non-nil ctxOut, uses ctxIn for both
input and output [1]. We have separate Context and ContextOut fields in
RunOptions. It should be possible to capture output from syscall
programs which is currently not: non-nil ContextOut triggers EINVAL,
while Context is (expectedly) not updated.

Implement the following semantics: either Context, or ContextOut or both
can be non-nil. Map to ctxIn internally, complain if Context and
ContextOut lengths are different.

[1] https://elixir.bootlin.com/linux/v6.15.2/source/net/bpf/test_run.c#L1530

Signed-off-by: Nick Zavaritsky <[email protected]>
@mejedi mejedi force-pushed the syscall-prog-ctx-out branch from 1e39130 to 7f98dfa Compare June 25, 2025 10:39
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Thanks!

@lmb lmb merged commit 29f1245 into main Jul 14, 2025
31 of 33 checks passed
@lmb lmb deleted the syscall-prog-ctx-out branch July 14, 2025 15:30
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