datapath: add tail call hooks for custom metrics, bytecounter example

[qmonnet]

Custom Metrics

Byte counters! Let's count how much data is received and sent for each endpoint. Related PR: #13173

But more than byte counters, this PR introduces hooks in the datapath so that users can tail call into custom programs, and collect all kind of metrics they want - Provided they implement it themselves in eBPF. Here is an overview:

• First commit introduces the tail call hooks in the datapath (IPv4 only), plus management for the per-endpoint prog_array maps used to reference the custom programs. Those maps have two entries, one for ingress and one for egress.

• Second commit makes these tail calls optional, and opt-in, through the introduction of a dedicated Cilium option.

• Third commit adds a sample application: a simple byte counter which can be attached to keep track of the amount of data received or sent for a given endpoint.

• Fourth commit adds the tail call hooks to the IPv6 datapath, on the same model as for IPv4.

To use it, once Cilium is deployed:

• Compile the custom program

  make -C bpf/custom

• Load it

  bpftool prog load bpf/custom/bpf_custom.o /sys/fs/bpf/tc/globals/bytecounter type classifier \
          pinmaps /sys/fs/bpf/tc/globals/bytecounter_maps

• Reference it from the prog_array map

  EP=<endpoint_id>
  bpftool map update pinned /sys/fs/bpf/tc/globals/cilium_calls_custom_$(printf '%05d' ${EP}) \
          key 0 0 0 0 value pinned /sys/fs/bpf/tc/globals/bytecounter              # or key `1 0 0 0` for egress

• For now, dump counters' value manually

  bpftool map dump pinned /sys/fs/bpf/tc/globals/bytecounter_maps/bytecount_map

PR Status

Here are some points of interest:

• I am still trying to finalise the location for the ingress hook. In the current version, the tail call added to tail_ipv4_policy() is not always executed, as handle_policy() (which calls it) is skipped in ipv4_local_delivery() if USE_BPF_PROG_FOR_INGRESS_POLICY is defined i.e. if enable-endpoint-routes is set to true. This is the case for GKE, for example. So my current plan is to add the tail call to the different branches in ipv4_local_delivery(), possibly by replacing returns with goto in that function to write the tail call just once, and also possibly with preprocessor guards to make sure it is only called from bpf_lxc.

• The custom program needs to return with the same value the original program would have used, to preserve the datapath logics. This means that we must pass the desired value from bpf_lxc to the custom program we hook into. I'm using cb[CB_CT_STATE] for that because it did not seem to be used at that location, but I might be wrong. Feedback on this approach (or an alternatives) would be welcome.

• I also pass the current direction (ingress or egress) and the source identity to the custom program. Given that direction needs 1 bit, identity needs 24, and existing return values are low and needs less than 7 bits, I make it all fit on the same cb cell.

• Once we have the counters in an eBPF map, we would need to extract and process it. It could be a Cilium metric, but I'm not sure how to implement custom, user-defined metrics. It could be just from the command line, but similarly this requires to give a way to users to specify what kind of metrics they want. At the moment, the values from the counters can be retrieved from bpftool.

[pchaigno]

Thanks for all the comments and explanations! That makes it very easy to review :-)

A few nits below, I haven't commented on the position of hooks within bpf_lxc since we discussed that during #sig-datapath meeting already.

My main remaining concern is around the API we expose. I'm assuming we won't be able to change what we expose to custom program after the fact because it may break custom programs in the wild. I've added a couple comments below in that regard.

[aanm]

Reviewed the agent side changes and they seem pretty small to me

[joestringer]

This looks pretty clean to me, nice work.

The main questions that come to mind reviewing this PR are:

• Custom metrics on packets that are subject to L7 forwarding seems a little inconsistent
• Seems like there are two approaches for handling ingress/egress hinted in the PR, either having two programs (one ingress / one egress) or passing the direction bit in the CB. Is this just left over from a previous implementation or did you intend to support more than one custom program?
• Some helpers could be called by a custom program and influence the handling of the packet, such as bpf_redirect(). What kind of guard rails do we want for cases like this? (One simple answer can be just leave it up to the implementer not to mess up; or if we're more concerned we could put compile-time warnings in place to discourage such behaviour)
• Is IPv6 support intended? How will we handle this?

I've added comments on each of these topics below so we can have threaded discussions.

Beyond that, there's just a scattering of minor nits, none of which are overly consequential.

[qmonnet]

Rebased, following merge conflicts
test-me-please

[qmonnet]

test-gke

[pchaigno]

All required CI jobs are green. A review from @cilium/kubernetes is missing but I think we can skip it as it was only requested for install/kubernetes/ and several person familiar with the agent and Helm reviewed. Marking as ready to merge.

[Thearas]

Hi @qmonnet, great job! I'm just looking for a traffic byte-counter, and I have a few question:

1. The byte-counter is implemented using BPF_MAP_TYPE_HASH, would it be more efficient to implement it using BPF_MAP_TYPE_PERCPU_HASH? Just like cilium_metrics.
2. How to determine if the __ctx_buff is on ingress path or on egress path?

Looking forward to your reply, much appreciated!

[qmonnet]

Hi, thanks for using the feature!

• The byte-counter is implemented using BPF_MAP_TYPE_HASH, would it be more efficient to implement it using BPF_MAP_TYPE_PERCPU_HASH? Just like cilium_metrics.

I'm not sure what gain you expect here. The updates on the counter value are atomic operations, so it shouldn't be an issue if multiple CPUs work with the same map. This being said, the point of these hooks is to have custom programs, so of course you're free to experiment :)

• How to determine if the __ctx_buff is on ingress path or on egress path?

Please have a look at the related README.rst, and in particular at step 3 in the instructions for the byte-counter program. There are several hooks where you can attach the program, some on ingress, some on egress. The current program does not make the distinction between ingress and egress. If you want it, you can edit the program to e.g. use another map for egress (distinct from the one for ingress), and attach this new program to the hooks on egress.

[Thearas]

Thanks!
One more question, how to get byte-counter program to be attached automatically when the cilium_calls_custom_* map is created?

[qmonnet]

There's nothing of the sort implemented in the agent currently, so I suppose you'd have to create some watcher to check for the presence of the maps and attach the programs when necessary.