Skip to content

Comments

Add Map and Program#2

Merged
lmb merged 235 commits intomasterfrom
step1
Sep 23, 2019
Merged

Add Map and Program#2
lmb merged 235 commits intomasterfrom
step1

Conversation

@lmb
Copy link
Contributor

@lmb lmb commented Sep 6, 2019

Pull in newtools/ebpf Map and Program, and preserve history by creating a merge commit.

This includes the asm subpackage to be able to test Program and Map without requiring an ELF loader.

I made the following changes from newtools/ebpf master, based on our previous discussion:

map: add test for PerfEventArray
program: add test for Benchmark
asm: rename XAdd to StoreXAdd
asm: update and rename built-in functions
map: add benchmarks
readme: update resources section
program: rename receiver
map: remove Marshaler in favour of documentation
doc: make package documentation more concise
abi: improve documentation
program: expose IsNotSupported instead of ErrNotSupported
map: expose Update and remove Create and Replace
map: return a NotExist error from NextKey
map: return error from Delete if key doesn't exist
map: rename Get to Lookup and return not found error
types: rename ProgType to ProgramType
remove helper flags
remove ELF loader
remove perf reader
remove non-self contained examples
remove binaries
remove GitHub templates

Fixes #1

@@ -0,0 +1,7 @@
Copyright (c) 2017 Nathan Sweet
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll have to merge this with LICENSE in a follow up commit.

lmb added 22 commits September 12, 2019 10:43
There is no need to have helper flags in the main package. Arguably they
should live in asm if they are really needed.
Also rename Unrecognized to UnspecifiedProgram to match UnspecifiedMap.
Follow the kernel naming of map_lookup_elem more closely. Also return
an error when a key is not found, instead of the awkward boolean.
Use the IsNotExist infrastructure for Delete calls and remove DeleteStrict.
Let the user specify flags instead of adding a new function for every
flag of map_update_elem. This reduces the API surface, and makes the
library more versatile. It also provides a clear way to support
BPF_F_LOCK which wasn't possible previously.
This gives us flexibility to add more detailed errors later.
Clarify the use case of MapABI and ProgramABI, and document the behaviour
of Check on the function instead of on the struct.
Marshaler isn't really part of the API, it's just there for
documentation purposes. Let's improve the documentation instead of
cluttering the API.
The current receive is called bpf, which doesn't make much sense in
the context of the package.
Lookup, Update, Delete and NextKey are probably the most performance
critical functions. Add a small benchmark for them. On my machine
I get the following:

    name           time/op
    Map/Lookup-8   622ns ± 1%
    Map/Update-8   650ns ± 1%
    Map/NextKey-8  540ns ± 0%
    Map/Delete-8   640ns ± 3%

    name           alloc/op
    Map/Lookup-8   0.00B
    Map/Update-8   0.00B
    Map/NextKey-8  0.00B
    Map/Delete-8   40.0B ± 0%

    name           allocs/op
    Map/Lookup-8    0.00
    Map/Update-8    0.00
    Map/NextKey-8   0.00
    Map/Delete-8    2.00 ± 0%

Delete allocates because it returns a not found error when deleting
a non-existing key.
eBPF helpers are added frequently, so we should make updating the list
simple. Remove the (duplicate, probably incorrect) documentation and
re-generate the new list of functions.
XAdd is just another mode of the Store class, so keep the
naming consistent.
The test was used to show the difference between old and new
syntax. Remove it since it's not needed anymore.
@lmb
Copy link
Contributor Author

lmb commented Sep 12, 2019

@joestringer I've made the fixups, this is good to go from my side.

@lmb
Copy link
Contributor Author

lmb commented Sep 20, 2019

@brb @joestringer can either of you approve the PR?

@lmb lmb merged commit 3bca05c into master Sep 23, 2019
@lmb lmb deleted the step1 branch August 7, 2020 08:41
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.

Upstream Map and Program from newtools/ebpf

10 participants