bpf: Enable per-CPU map support for BatchOps

[nathanjsweet]

BatchOps have already been enabled for this library.
However, per-CPU map support was deferred for the purposes
of having a distinct debate about what the interface for
the per-CPU maps should look like.

This commit adds support for per-CPU maps but does
fundamentally very little to add the support, taking
the libbpf approach of having the user of the library
add dittographic lists (in golang, slices) as the
"values" argument of Lookup (in that case empty)
or Update.

This commit also make a small change to the stats
tests so that they don't panic the tests when they fail on
a local machine that doesn't support them, but rather
just fail.

Signed-off-by: Nate Sweet [email protected]

[nathanjsweet]

@lmb I am very hesitant about changing the interface for per-CPU maps. The only changes needed would be to the Lookup calls and/or to the Update call. I think we should stick to the libbpf approach and have the add long lists. That will provide the most amount of flexibility to the end-user.

[lmb]

The current API to interact with per-CPU maps looks like this:

Lookup(key, values *[]T)
Update (key, values []T)

Lookup takes a pointer instead of just a plain slice because it means that the function can be used by code that is completely oblivious to the number of CPUs, like so:

var values []uint64
m.Lookup(uint32(0), &values)

for i, v := range values {}

As a consequence internal.PossibleCPUs is not exposed to the user because I didn't have a use case for updating per CPU maps. It turns out that was a poor choice since some people try to use runtime.NumCPU for this, which breaks when GOMAXPROCS is changed. It also means that you can't pre-allocate the slice to look up into, though I'm not sure how much of a problem that is in practice.

Point 1: I'd like to use this opportunity to allow Lookup(key, values []T) which means adding a function like ebpf.PossibleCPUs() int. That function is also required to make use of per-CPU batch look ups as you've proposed them, so we can kill two birds with one stone.

Next, your proposed batch API looks like this:

BatchLookup(keys []T, values []T) // len(values) / possibleCPUs == len(keys)
BatchUpdate(keys []T, values []T) // len(values) / possibleCPUs == len(keys)

The "natural" API would be BatchLookup([]T, [][]T) since this is consistent with regular per-CPU lookups.

I think we should stick to the libbpf approach and have the add long lists. That will provide the most amount of flexibility to the end-user.

I think in this case libbpf doesn't really have any other option besides taking a long list (which needs additional magic like BPF_DECLARE_PERCPU) so that isn't a strong argument for me. I'd much rather know more about your use-cases and what is important for them.

In the meantime, here is a snippet that shows the two hypothetical APIs:

func nestedSlices(m *Map) {
	keys := make([]uint64, batchSize)
	values := make([][]uint32, batchSize)
	for i := range values {
		values[i] = make([]uint32, PossibleCPUs())
	}

	var next uint64
	cnt, _ := m.BatchLookup(nil, &next, keys, values, nil)

	for i, key := range keys[:cnt] {
		row := values[i]
		fmt.Printf("%s: %v\n", key, row)
	}
}

func unrolledSlice(m *Map) {
	cpus := PossibleCPUs()
	keys := make([]uint64, batchSize)
	values := make([]uint64, batchSize*cpus)

	var next uint64
	cnt, _ := m.BatchLookup(nil, &next, keys, values, nil)

	for i, key := range keys[:cnt] {
		row := values[i*cpus:(i+1)*cpus]
		fmt.Printf("%s: %v\n", key, row)
	}
}

nestedSlices is marginally nicer to use but also creates a bunch more allocations plus overhead for slice headers. unrolledSlice isn't that difficult either, brownie points for a lot less allocations. Is there some sugar that could make unrolledSlice nicer to use?

Regardless of what API we end up on, do you mind adding an example that shows how to use BatchLookup with per CPU maps? And the documentation needs updating as well.

[lmb]

Is this on purpose?

[lmb]

Good catch, can you move this to a separate PR?

[lmb]

I think it would be nicer if this took an int instead, which allows removing some duplication from lookup and update. Functions taking bool are also hard to read.

func marshalValues(data interface{}, n int) (...) {
    if perCPU {
        marshalPerCPU(data, valueSize, n)
    }

   if *Map && n != 1 {
      return an error
  }
   ...
   return marshalPtr(data, n*int(m.valueSize))
}

The batch methods then call m.marshalValues(data, keysValue.Len()) and the count != n * possibleCPUs check is done in marshalPerCPUValue.

[nathanjsweet]

@lorenz I'm going to close this PR. We don't need per-CPU maps yet. I do favor the rawest approach possible, but we can defer until someone else wants it.