oxidecomputer / oxidecomputer/opte

Some SDT probes are expensive

Open
#259 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
77
Forks
11
Avg merge
9d 20h
Merged PRs (30d)
8

Description

As illumos currently doesn't have any real support for providing CTF data for Rust FBT probes, the job has fallen to relying more heavily on SDT probes. The way this has been done up to this point is as following:

  • Define an extern C symbol that is discoverable by the DTrace SDT module, i.e. __dtrace_probe_some__event(...).
  • Define a rust-side helper function that is named some_event_probe(...).
  • Inside this helper is conditional compilation for in-kernel SDT vs. std USDT (the later is just there to emulate SDT output so you can trace probes when running tests).
  • Arrange the rust-side arguments into types that the DTrace system and user-land consumer can deal with. This can be as simple as grabbing a pointer to something or as complicated as building up a repr(C) struct that can be typedef'd and deconstructed in the user-land DTrace scripts (e.g. flow_id_sdt_arg).

It's this last point where things can get expensive. For example, the port_process_return_probe() passes a stringified version of the result to the SDT probe. This allows a user to easily see the result of processing a given packet. To do this we have code like the following:

                let res_str = match res {
                    Ok(v) => format!("{:?}", v),
                    Err(e) => format!("ERROR: {:?}", e),
                };
                let res_arg = cstr_core::CString::new(res_str).unwrap();
...
                    __dtrace_probe_port__process__return(
                        dir.cstr_raw() as uintptr_t,
                        self.name_cstr.as_ptr() as uintptr_t,
                        &flow_b_arg as *const flow_id_sdt_arg as uintptr_t,
                        &flow_a_arg as *const flow_id_sdt_arg as uintptr_t,
                        epoch as uintptr_t,
                        pkt.mblk_addr(),
                        hp_pkt_ptr,
                        res_arg.as_ptr() as uintptr_t,
                    );

The problem is twofold:

  1. The call to format! allocates a new String. And then another allocation is made to convert this into a CString.
  2. This happens EVERY TIME a packet is processed by the port, regardless if someone is tracing the SDT probe.

The enablement of the SDT probe has no bearing on the rust wrapper around it. Even when no OPTE SDT probes are enabled we end up doing a lot of allocation in the datapath, and for no reason. There are several things we could/should do to improve this situation.

  • Avoid strings in SDT probes when possible. Not only do they get us into this bind, but they also aren't as useful as primitive/structured data types as they can't be easily matched upon in DTrace scripts. This is why I created types like flow_id_sdt_arg, which allows programmatic filtering.
  • If a string can be static, make it static.
  • Consider caching a CString for dynamic but unchanging strings. This is what I did with things like Port::name_cstr.
  • For dynamic strings that we want to show up in the SDT probe: use some statically sized buffer and truncate the message when it's too long.

This problem is no different than that found in the C code. AFAIK, there is no way for a kernel module to query the SDT module to see if a probe site is enabled, and certainly if there is I haven't seen any C code use it. Rather, I think it's understood that you don't go doing expensive things for the sake of an SDT argument (this is even discussed in the DTrace Guide in terms of not dereferencing pointers). But when I was first developing OPTE I didn't really have this in the front of my mind. In any event, I think focusing on eliminating/improving dynamic string usage is the best bang for our buck here.

This issue doesn't have to be fixed all at once. Rather we can iterate. I'd start with the hottest path: any probe that fires when we have a UFT hit. E.g., the port process probes would be perfect candidates.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at port_process_return_probe() and the port process probes that fire on a UFT hit, then inspect the dynamic string construction around the SDT call. Compare this with Port::name_cstr and the structured flow_id_sdt_arg approach; done means reducing unnecessary datapath allocations while retaining useful SDT output.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.