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

Deprecate Collection.Assign(), refactor CollectionSpec.(LoadAnd)Assign() #371

Merged
merged 5 commits into from
Sep 1, 2021

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Aug 11, 2021

Closes #366.

Best reviewed per commit.

tl;dr

  • deprecated Collection.Assign()
  • flattenStruct() is now a discrete function in the form of ebpfFields()
  • assignValues() now takes a validator function that can be used to reject the transaction before the struct's fields are actually Set()
  • interface{} is now used instead of a reflect.Value for tracking assigned object uniqueness

collection.go Show resolved Hide resolved
@ti-mo ti-mo changed the title Deprecate Collection.Assign(), unified assignValues() Deprecate Collection.Assign(), refactor CollectionSpec.(LoadAnd)Assign() Aug 16, 2021
@ti-mo ti-mo force-pushed the lna-prog-arrays branch 2 times, most recently from f6e3d52 to bc48c76 Compare August 17, 2021 20:26
@ti-mo ti-mo marked this pull request as ready for review August 17, 2021 20:29
collection.go Outdated Show resolved Hide resolved
collection.go Outdated Show resolved Hide resolved
collection.go Outdated Show resolved Hide resolved
close() and finalize() are fairly similar in connotation.
Rename .close() to .cleanup() to make it clearer that it's run to back out of
the transaction and not really part of the happy path.

Signed-off-by: Timo Beckers <timo@isovalent.com>
This doesn't fit in with the new collectionLoader, so the time has come
to finally remove it.

Signed-off-by: Timo Beckers <timo@isovalent.com>
collection.go Outdated Show resolved Hide resolved
collection.go Outdated Show resolved Hide resolved
This patch aims to remove a layer of indirection by moving flattenStruct()
into its own discrete function ebpfFields, since it extracts type info from
struct fields tagged with 'ebpf' annotations. This hopefully makes the code
somewhat more understandable, easier to extend and testable.

'valueOf' was renamed to 'getValue'. Its signature was changed and now yields
a value of type interface{}. reflect.ValueOf() is then called once in
assignValues(), simplifying the closure implementations.

Signed-off-by: Timo Beckers <timo@isovalent.com>
…gramArrays

Track assigned maps in LoadAndAssign to be able to reject ProgArrays
that are loaded into the kernel but are not assigned to the struct.
These get closed on the first GC, leading to inconsistent behaviour.

Signed-off-by: Timo Beckers <timo@isovalent.com>
Highlight a few specifics of their functionality, previously their docstrings
were largely copy-pasted.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo merged commit b67689d into cilium:master Sep 1, 2021
@ti-mo ti-mo deleted the lna-prog-arrays branch September 1, 2021 08:16
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.

Require explicit assignment of prog arrays in LoadAndAssign
2 participants