Conversation
This comment was marked as resolved.
This comment was marked as resolved.
f86497d to
be7dfb0
Compare
lmb
left a comment
There was a problem hiding this comment.
Could you add a test which executes a syscall program or are they too complicated to set up?
f381284 to
75af2bc
Compare
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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
}75af2bc to
1e39130
Compare
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]>
1e39130 to
7f98dfa
Compare
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