libbpf-rs: Add suport for repeat and duration in Program{Input,Output} #1159
Conversation
|
looking at the contribution guidelines:
let me know if you think it is worthy to add a note there. |
libbpf-rs/src/program.rs
Outdated
| @@ -604,6 +605,8 @@ pub struct Input<'dat> { | |||
| pub cpu: u32, | |||
| /// The 'flags' value passed to the kernel. | |||
| pub flags: u32, | |||
| /// How many times to repeat the test run. | |||
| pub repeat: c_int, | |||
There was a problem hiding this comment.
Do negative values have a meaning of sorts? We try to only non-FFI types in the public interface. Let's go with u32/i32/usize/isize or something along those lines?
Also, what would happen if it is zero? Would the test not run at all? If so (and probably even if not), that seems like a questionable default (and yet it is the current default). If zero is garbage, then let's perhaps use some NonZero{U,I}* thingy?
There was a problem hiding this comment.
ok, so this is, down the stack, a u32: https://elixir.bootlin.com/linux/v6.2.11/source/include/uapi/linux/bpf.h#L1430
but not in its interface: https://elixir.bootlin.com/linux/v6.2.11/source/tools/lib/bpf/bpf.h#L446
Also, what would happen if it is zero? Would the test not run at all? If so (and probably even if not), that seems like a questionable default (and yet it is the current default). If zero is garbage, then let's perhaps use some NonZero{U,I}* thingy?
A value of 0 is forced to 1: https://elixir.bootlin.com/linux/v6.2.11/source/net/bpf/test_run.c#L352 at every single test_run_* entrypoints.
What do you want to do here? Let 0 slip through? which could be the default value? If using NonZeroU32, my understanding is that this would become a Option<NonZero<u32>> but we would still need to handle it and default it to 0 when passing down to libbpf?
There was a problem hiding this comment.
Thanks for checking and the references. I don't think it would need to be an Option<NonZero<...>>. Just NonZero would be sufficient and cleaner, in my opinion. You'd have to manually implement Default from what I can tell. I have a slight preference for the NonZero variant, but acknowledge that UX is a bit degraded in that case if you want to change the member, so u32 + a comment saying that 0 will still result in a run is okay as well.
There was a problem hiding this comment.
ok, pushed the u32 and Duration change. I am not psyched about what using the NonZero interface would look like.
https://gist.github.com/chantra/8c662df93036953f622bc1e8ce93e196
I may be missing something that would make it simpler, but given that 0 is a "fine" default, it feels the use of NonZero will be an eyesore.
chantra
force-pushed
the
libbpf_rs_repeat
branch
2 times, most recently
from
2dfcea0 to
26383eb
Compare
…gramOutput The former, when non-zero, is used to run the program `repeat` times. The latter is the number of ns a run took on average. A test was added to confirm the behaviour. The program bumps a counter, said counter is verified to match `repeat`. Signed-off-by: Manu Bretelle <[email protected]>
The former, when non-zero, is used to run the program
repeattimes. The latter is the number of ns a run took on average.A test was added to confirm the behaviour. The program bumps a counter, said counter is verified to match
repeat.