search

dropbox / goebpf

Library to work with eBPF programs from Go
Other
1.14k stars 85 forks source link

removing get_next_key function from kernel-side code #39

Closed bersoare closed 4 years ago

bersoare commented 4 years ago

this removes get_next_key function from kernel code (bpf_helpers.h), as this functionality is supposed to be used only in userland code. 

src/goebpf/itest# make test
././itest_test -test.v
=== RUN   TestKprobeSuite
=== RUN   TestKprobeSuite/TestElfLoad
=== RUN   TestKprobeSuite/TestKprobeEvents
--- PASS: TestKprobeSuite (0.51s)
    --- PASS: TestKprobeSuite/TestElfLoad (0.29s)
    --- PASS: TestKprobeSuite/TestKprobeEvents (0.23s)
=== RUN   TestMapSuite
=== RUN   TestMapSuite/TestArrayOfMaps
=== RUN   TestMapSuite/TestGetNextKeyInt
=== RUN   TestMapSuite/TestGetNextKeyString
=== RUN   TestMapSuite/TestHashOfMaps
=== RUN   TestMapSuite/TestMapArrayInt
=== RUN   TestMapSuite/TestMapArrayInt16
=== RUN   TestMapSuite/TestMapArrayUInt64
=== RUN   TestMapSuite/TestMapDoubleClose
=== RUN   TestMapSuite/TestMapFromExistingByFd
=== RUN   TestMapSuite/TestMapHash
=== RUN   TestMapSuite/TestMapLPMTrieIPv4
=== RUN   TestMapSuite/TestMapLPMTrieIPv6
=== RUN   TestMapSuite/TestMapPersistent
=== RUN   TestMapSuite/TestMapProgArray
--- PASS: TestMapSuite (0.21s)
    --- PASS: TestMapSuite/TestArrayOfMaps (0.11s)
    --- PASS: TestMapSuite/TestGetNextKeyInt (0.00s)
    --- PASS: TestMapSuite/TestGetNextKeyString (0.00s)
    --- PASS: TestMapSuite/TestHashOfMaps (0.10s)
    --- PASS: TestMapSuite/TestMapArrayInt (0.00s)
    --- PASS: TestMapSuite/TestMapArrayInt16 (0.00s)
    --- PASS: TestMapSuite/TestMapArrayUInt64 (0.00s)
    --- PASS: TestMapSuite/TestMapDoubleClose (0.00s)
    --- PASS: TestMapSuite/TestMapFromExistingByFd (0.00s)
    --- PASS: TestMapSuite/TestMapHash (0.00s)
    --- PASS: TestMapSuite/TestMapLPMTrieIPv4 (0.00s)
    --- PASS: TestMapSuite/TestMapLPMTrieIPv6 (0.00s)
    --- PASS: TestMapSuite/TestMapPersistent (0.00s)
    --- PASS: TestMapSuite/TestMapProgArray (0.00s)
=== RUN   TestPerfEvents
--- PASS: TestPerfEvents (0.01s)
=== RUN   TestGetNumOfPossibleCpus
--- PASS: TestGetNumOfPossibleCpus (0.00s)
=== RUN   TestXdpSuite
=== RUN   TestXdpSuite/TestElfLoad
=== RUN   TestXdpSuite/TestProgramInfo
--- PASS: TestXdpSuite (0.03s)
    --- PASS: TestXdpSuite/TestElfLoad (0.03s)
    --- PASS: TestXdpSuite/TestProgramInfo (0.00s)
PASS

https://travis-ci.org/github/dropbox/goebpf/builds/728180572

belyalov commented 4 years ago

as this functionality is supposed to be used only in userland code.

Are you sure? Is BPF program is not allowed to iterate over hash map?

bersoare commented 4 years ago

i wouldn't say im sure, but this is what i could infer after noticing there's not a single example kernel code implementing such function (https://elixir.bootlin.com/linux/latest/ident/bpf_map_get_next_key) and the kernel prototype documentation seem to say that this is specific to userspace (https://prototype-kernel.readthedocs.io/en/latest/bpf/ebpf_maps.html - although not explicitly).

bersoare commented 4 years ago

btw - i have also found this: https://elixir.bootlin.com/linux/latest/source/include/linux/bpf.h#L41

documentation/code doesn't say that "it's not allowed" with these words exactly, but after looking at that piece of code i believe the intention is to have get_next_key used in userspace only, via syscall.

belyalov commented 4 years ago

btw - i have also found this: https://elixir.bootlin.com/linux/latest/source/include/linux/bpf.h#L41

documentation/code doesn't say that "it's not allowed" with these words exactly, but after looking at that piece of code i believe the intention is to have get_next_key used in userspace only, via syscall.

Sounds good, thanks for your research.

Anyway, if we're wrong here we can always return it.