Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

btf: move Spec.Add to dedicated type #1069

Merged
merged 6 commits into from
Jun 22, 2023
Merged

btf: move Spec.Add to dedicated type #1069

merged 6 commits into from
Jun 22, 2023

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Jun 20, 2023

internal: add NewBuffer

Share a single pool of bytes.Buffer across the whole library.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: add marshalOptions.Order

Allow specifying which ByteOrder to encode BTF into via a new field in
marshalOptions.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: use number of types to pre-size string table

Remove the dependency on the ELF string table to pre-size the string table
builder for large Specs. Instead we use the heuristic that for vmlinux the
number of strings is roughly equal to the number of types.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: refactor to Builder

Spec allows both querying Types as well as constructing new BTF blobs via
Add. This has some unfortunate consequences: we need to do extra work so
that Added types can be queried and query code needs to be aware that the
set of types can change. The query API also allows observing side-effects of
adding types which we'd rather hide, for example when working around datasec
bugs. It's also difficult to make the implementation efficient for both use
cases at once.

Split the code for constructing new BTF blobs into a separate struct.

NewHandle now takes a Builder instead of a Spec which is a breaking change.
Other API changes are to functions which have not appeared in a release yet.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: unexport NewSpec

NewSpec was added to be able to create BTF from scratch using Spec.Add. This
usecase was migrated to Builder and Spec.Add doesn't exist anymore.

Unexport NewSpec so we can still use it for testing but users only
experience API breakage once.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: generalise datasecWorkaround to Builder.Add

Extend the datasec validation workaround to all Datasec. This is safe to do
because there are no methods on Builder which allow going from TypeID to
Type.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

@lmb lmb requested review from ti-mo and rgo3 June 20, 2023 11:20
@lmb lmb marked this pull request as ready for review June 20, 2023 14:30
btf/handle.go Outdated Show resolved Hide resolved
btf/marshal.go Show resolved Hide resolved
btf/marshal.go Outdated Show resolved Hide resolved
lmb added 3 commits June 22, 2023 11:38
Share a single pool of bytes.Buffer across the whole library.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Allow specifying which ByteOrder to encode BTF into via a new
field in marshalOptions.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Remove the dependency on the ELF string table to pre-size the
string table builder for large Specs. Instead we use the heuristic
that for vmlinux the number of strings is roughly equal to the
number of types.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

Only nits, human linter goes brrr. Otherwise LGTM.

btf/btf.go Outdated Show resolved Hide resolved
btf/marshal_test.go Outdated Show resolved Hide resolved
btf/marshal_test.go Outdated Show resolved Hide resolved
lmb added 3 commits June 22, 2023 14:57
Spec allows both querying Types as well as constructing new BTF
blobs via Add. This has some unfortunate consequences: we need
to do extra work so that Added types can be queried and
query code needs to be aware that the set of types can change.
The query API also allows observing side-effects of adding
types which we'd rather hide, for example when working around
datasec bugs. It's also difficult to make the implementation
efficient for both use cases at once.

Split the code for constructing new BTF blobs into a separate
struct.

NewHandle now takes a Builder instead of a Spec which is a
breaking change. Other API changes are to functions which
have not appeared in a release yet.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
NewSpec was added to be able to create BTF from scratch using
Spec.Add. This usecase was migrated to Builder and Spec.Add
doesn't exist anymore.

Unexport NewSpec so we can still use it for testing but
users only experience API breakage once.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Extend the datasec validation workaround to all Datasec. This is
safe to do because there are no methods on Builder which allow
going from TypeID to Type.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
@lmb lmb merged commit cd851a8 into cilium:master Jun 22, 2023
@lmb lmb deleted the btf-builder branch June 26, 2023 08:55
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.

2 participants