add bpf2go support and wrappers for ebpf.Variable generation #1543
Conversation
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
from
bbe1aba
to
799fef7
Compare
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
from
799fef7
to
d978138
Compare
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
2 times, most recently
from
fa76e79
to
71e3029
Compare
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
from
71e3029
to
5137cbe
Compare
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
3 times, most recently
from
2cf61df
to
9cf6f9f
Compare
smagnani96
changed the title
|
This PR has been rebased on https://github.com/ti-mo/ebpf/tree/tb/ebpf-variables. |
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
from
9cf6f9f
to
0f69c00
Compare
…ndler Signed-off-by: Timo Beckers <[email protected]>
Signed-off-by: Timo Beckers <[email protected]>
…iables Signed-off-by: Timo Beckers <[email protected]>
Signed-off-by: Timo Beckers <[email protected]>
Signed-off-by: Timo Beckers <[email protected]>
Signed-off-by: Timo Beckers <[email protected]>
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
from
0f69c00
to
cf717a0
Compare
Signed-off-by: Simone Magnani <[email protected]>
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
2 times, most recently
from
8486b44
to
f6a94c5
Compare
This commit introduces support and wrappers around ebpf.Variable.
An ebpf.Variable does not define per-se wrappers method to automatically
infer the underlying data type and handle typed data in Go. In fact, it
relies on parameters passed to the `Variable.Get` and `Variable.Set` APIs.
This commit allows `bpf2go` to emit `VariableSpec` and `Variable` in the
generated code. In addition, a new set of wrapper methods are created
to provide support for typed method around variables. To do so, this
commit enables emitting `Dataspec`, which so far was not emitted. However,
it modifies the current implementation so that for each variable in `Dataspec`
the needed support is provided.
Supposing to have an ebpf program that defines the following two global variables:
```c
__u64 pkt_count = 0;
const __u32 ro_variable = 1;
char var_msg[] = "Hello World!";
```
The steps performed during the Go code generation are as follows:
1. An alias with the variable name is generated in the Go file:
```go
type PktCountType = uint64
type RoVariable = uint32
type VarMsgType = [16]int8
```
2. A new type definition (non-alias, need new methods) for the variables is performed:
```go
type PktCount ebpf.Variable
type RoVariable ebpf.Variable
type VarMsg ebpf.Variable
```
3. The `.Get` methods for all types are generated:
```go
func (v *PktCount) Get() (PktCountType, error) {
var ret PktCountType
return ret, (*ebpf.Variable)(v).Get(&ret)
}
func (v *RoVariable) Get() (RoVariableType, error) {
var ret RoVariableType
return ret, (*ebpf.Variable)(v).Get(&ret)
}
func (v *VarMsg) Get() (VarMsgType, error) {
var ret VarMsgType
return ret, (*ebpf.Variable)(v).Get(&ret)
}
```
4. The `.Set` methods for the non read-only variables are generated:
```go
func (v *PktCount) Set(val PktCountType) error {
return (*ebpf.Variable)(v).Set(val)
}
func (v *VarMsg) set(val VarMsgType) error {
return (*ebpf.Variable)(v).Set(ret)
}
```
4. In case the type alias is supported by the atomic package, and then
also supported by `ebpf.Variable` (int32, uint32, int64, uint64), then
an additional `AtomicRef` method is created for the non read-only
variables to get an reference to the underlying data.
```go
func (v *PktCount) AtomicRef() (*ebpf.Uint64, error) {
ret, _ := (*ebpf.Variable)(v).AtomicUint64()
return ret
}
```
From the user perspective, ignoring the error catching, the following
operations can be performed:
```go
...
objs := bpfObjects{}
err := loadBpfObjects(&objs, nil)
...
// ref kept and can be used multiple times
aPktCount := objs.PktCount.AtomicRef()
err := aPktCount.Load()
aPktCount.Store(rand.Uint64())
...
vRoVariable, _ := objs.RoVariable.Get()
...
varMsg, _ := objs.VarMsg.Get()
varMsg[0] = 'B'
err := objs.VarMsg.Set(varMsg)
```
Signed-off-by: Simone Magnani <[email protected]>
smagnani96
force-pushed
the
pr/relocated_vars_userspace
branch
from
f6a94c5
to
fb547c3
Compare
|
It probably makes sense to land basic
I see huge value in a type-safe Is it possible to devise an approach that works with |
Do you suggest to change this PR to provide basic VariableSpecs support in bpf2go or should I open a new one? Concerning the type-safe API, I can keep exploring solutions. I tried with generics with no luck, but maybe there's still a way...AFAIU CollectionSpec would also need to be a generic holding maps and variables each of different generic types. |
It is probably worthwhile to keep this one around as a RFC and to showcase the end goal. One way to handle it is to structure commits such that basic support is introduced first, followed by type-safe APIs. Once @ti-mo changes are merged, I'd suggest doing a separate PR to introduce basic variable support in
A clever use of It looks like // in ebpf package
type TypedVariable[T any] Variable
func (v *TypedVariable[T]) Set(val T) error { return (*Variable)(v).Set(val) }
...
What I had in mind was |
Perfect, I'll keep this one open just as reference and will track #1572. Once merged, I'll push basic support for
A wrapper would certainly be a good choice, so we can statically define and test methods rather than generating them at runtime. An approach could be as follows, and then in type ROVariable[T any] struct {
Variable
}
type RWStruct[T any] struct {
ROVariable[T]
}
type AtomicVariable[T int32 | int64 | uint64 | uint32] struct {
RWVariable[T]
}
func (a *ROVariable[T]) Get() T {
...
}
func (a *RWVariable[T]) Set(val T) {
...
}
func (a *AtomicVariable[T]) AtomicReference() atomic.T { //somehow
...
}Would you consider such a wrapper also for |
|
@smagnani96 I've pushed up a few changes to #1572 today. Note that all 'atomic' methods are gone. It wasn't a great design to begin with and @mejedi suggested mmapping over the Go heap so we can hand out arbitrary pointers instead. The pointer stuff is not part of the initial proposal, this work is currently in a separate branch here: https://github.com/ti-mo/ebpf/commits/tb/atomic-memory. tl;dr: // VariablePointer returns a pointer to a value of type T that has its
// underlying memory mapped to the memory representing the Variable. Accessing a
// constant Variable is not supported.
//
// T must be a fixed-size type according to [binary.Size]. Types containing Go
// pointers are not valid.
//
// When accessing structs, embedding [structs.HostLayout] may help ensure the
// layout of the Go struct matches the one in the BPF C program.
func VariablePointer[T any](v *Variable) (*T, error)This works for atomics, structs, scalars, etc. Currently no types containing pointers are accepted. Note that currently, the plan is to not populate The alternative is to emit Variables, but disable Set, Get and VariablePointer (make them return errors), which is also far from ideal. If we go down this route, we'd need to introduce a compatibility layer that implements Get() using a map lookup, but then we need to carry a |
Globals landed in 5.2. Call me biased, but my preference would be to inconvenience (a presumably small number of) users requiring at least 5.2 that can't bump the minimum version to 5.5. As of today, |
|
@mejedi That sounds reasonable, thanks for sharing your perspective. The downside is this is kind of a breaking change; rebuilding the same code base with a newer version of b2g and ebpf-go will no longer work as-is on some systems. |
|
@smagnani96
(Not sure how we should name these generics, The overlap is just the type ROVariableT[T any] Variable
type VariableT[T any] VariableIn return, Note: Couple further questions to consider:
var objects struct { foo *VariableT[uint64, atomic.Uint64] `ebpf:"foo"` }
objects.foo.Addr().Load()v.s. var objects struct { foo *VariableT[uint64] `ebpf:"foo"` }
atomic.LoadUint64(objects.foo.Addr())First option guarantees that value is accessed atomically. It also involves less typing. Second option is more consistent. type compound { bar uint64 }
var objects struct { foo *VariableT[compound] `ebpf:"foo"` }
atomic.LoadUint64(objects.foo.Addr().bar)
Should be easier since we have just one flavour of |
|
I've updated #1572 to emit a Variable to the Collection for every VariableSpec in the CollectionSpec. Get(), Set() and VariablePointer() will return ErrNotSupported on kernels without BPF_F_MMAPABLE. This ensures LoadAndAssign will continue to work as usual after upgrades, but users opting in to interactions with |
|
@smagnani96 FYI #1572 has been merged, and #1607 was opened to keep track of the off-heap Go pointers, though keep in mind this might take a while to be resolved upstream. Then it's at least another 6 months of Go releases until ebpf-go can start depending on the behaviour. I suggest we shelve the atomics use case in bpf2go for now, or we special-case |
|
Apologies for the absence. bpf2go basic supportI just opened #1610. Type-safe API -- New Types
Better than inheritance, but we would still need to re-implement all the other methods, such as func (v *ROVariableT[T]) String() string { return (*Variable)(unsafe.Pointer(v).String() }To all those methods who do not need particular changes. This would also allow us to remove methods such as Otherwise, a way to avoid reimplementing things would be to move all these "shared" methods to a base struct that will be inherited by both Variable and the generics: type baseVariable struct{size, mm, ...}
type Variable struct {
baseVariable
}
type ROVariableT[T any] Variable
type VariableT[T any] VariableAt this point, ms := &VariableSpec {
baseVariableSpec: baseVariableSpec{
// all the previous VariableSpec fields moved now to baseVariableSpec
}
}
// or we define constructors to hide the baseVariableSpec
ms := NewVariableSpec(....)That said, in Assign/LoadAndAssign we would just need to change: func (cs *CollectionSpec) Assign(to interface{}) error {
getValue := func(typ reflect.Type, name string) (interface{}, error) {
switch typ {
......
case reflect.TypeOf((*VariableSpec)(nil)):
return handleVariableSpec()
default:
if typ.ConvertibleTo(reflect.TypeOf((*VariableSpec)(nil))) {
return handleVariableSpec()
}
// Same for MapSpec
return nil, fmt.Errorf("unsupported type %s", typ)
}
}
return assignValues(to, getValue)
}and: func assignValues(to interface{},
.....
for _, field := range fields {
......
if !field.value.CanSet() {
return fmt.Errorf("field %s: can't set value", field.Name)
}
t := reflect.ValueOf(value)
if field.Type == t.Type() {
field.value.Set(reflect.ValueOf(value))
assigned[e] = field.Name
continue
}
if t.Type().ConvertibleTo(field.Type) {
field.value.Set(t.Convert(field.Type))
assigned[e] = field.Name
continue
}
return fmt.Errorf("Unable to assign/convert/blabla")
}
return nil
}
I would say, if it was something purely on bpf2go we could skip the error check and return just T to user. But since I think we don't want bpf2go random code but rather an API for the whole library (e.g. Type-safe API -- Inheritance/EmbeddingPlain embedding would not require further changes when declaring untyped type ROVariableT[T any] struct {
Variable
}
func (v *ROVariableT[T]) Get() (T, error) {
var ret T
return ret, v.Variable.Get(&ret)
}
if _, ok := typ.Elem().FieldByName("Variable"); ok {
assignedVars[name] = true
return loader.loadVariable(name)
}and in func assignValues(to interface{},
..........
for _, field := range fields {
........
if _, ok := field.Type.Elem().FieldByName(t.Type().Elem().Name()); !ok {
x := reflect.New(field.Type.Elem())
x.Elem().FieldByName(t.Type().Elem().Name()).Set(t.Elem())
field.value.Set(x)
assigned[e] = field.Name
}
return fmt.Errorf("unable to set/cast/assign")
}
return nil
}MapMemory and Variable Atomic Ref
I wouldn't go for the 1st option. In that case, I would just write our support for all the atomic methods (or a subset): type VariableT[T] Variable
func(v *Variable[int32]) Load() T {}
func(v *Variable[int32]) Store() T {}
func(v *Variable[int32]) Swap() T {}
func(v *Variable[int32]) Add() T {}
func(v *Variable[int32]) CompareAndSwap() T {}But this would require However, I really like your 2nd option, With Timo's |
This commit introduces support and wrappers around ebpf.Variable.
An ebpf.Variable does not define per-se wrappers method to automatically
infer the underlying data type and handle typed data in Go. In fact, it
relies on parameters passed to the
Variable.GetandVariable.SetAPIs.This commit allows
bpf2goto emitVariableSpecandVariablein thegenerated code. In addition, a new set of wrapper methods are created
to provide support for typed method around variables. To do so, this
commit enables emitting
Dataspec, which so far was not emitted. However,it modifies the current implementation so that for each variable in
Dataspecthe needed support is provided.
Supposing to have an ebpf program that defines the following two global variables:
The steps performed during the Go code generation are as follows:
.Getmethods for all types are generated:.Setmethods for the non read-only variables are generated:also supported by
ebpf.Variable(int32, uint32, int64, uint64), thenan additional
AtomicRefmethod is created for the non read-onlyvariables to get an reference to the underlying data.
From the user perspective, ignoring the error catching, the following
operations can be performed:
Suggestions/Comments are very welcome. For general discussion, I've opened the issue (:arrow_down:).
This PR relies and is rebased on https://github.com/ti-mo/ebpf/tree/tb/ebpf-variables, not yet merged but planned. Additionally, I'd need the support introduced in my 1st commit 59808a1.
The only new commit that should be referred in this PR is the latest f6a94c5.
My old Variable API proposal prior to https://github.com/ti-mo/ebpf/tree/tb/ebpf-variables are kept here.
Fixes: #1542