capsule icon indicating copy to clipboard operation
capsule copied to clipboard

panic/unreachable from udp.data function in benchmarks using udp4 proptest strategy

Open sbuzzard opened this issue 4 years ago • 1 comments

Describe the bug?

Using the udp4 proptest strategy to generate UDP4 packets in bench mark test, I noticed when making a call to that packet's data() function I'd get an unreachable panic. In adding some instrumentation, it is panicking because the payload_offset is 42 and the data buffer length itself is also 42. I did notice in adding println to the function that the issue would go away (memory/concurrency issue maybe?). I don't see it with my own strategy that uses the udp4 call but then extends the packet with a payload, which is doesn't have in the proptest generation. But then the payload may be masking it. Will know if that's true with some more testing.

I made a slight change to udp4 in the data() function as part of this repoduction to get the specific error.

Steps to reproduce?

this branch in my fork demonstrates it with this change

Capsule version?

I am using the djin/borrow-mut-parse branch (0.0.2 pre-alpha) - my branch above was made from that.

OS?

Using the capsule sandbox

Docker / VM / Bare?

Using the capsule sandbox with vagrant but not with the docker image in vagrant.

Stack trace or error log output

Benchmarking packets::single_peek_vs_parse_on_udp/packets::single_parse_udp: Warming up for 3.0000 sError in data buffer: Offset 42 exceeds the buffer length 42.
thread 'main' panicked at 'internal error: entered unreachable code', /opt/sources/dpdk/capsule/capsule/core/src/packets/udp.rs:161:17
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:116:14
   2: core::panicking::panic
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:48:5
   3: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
   4: criterion::bencher::Bencher<M>::iter_custom
   5: <criterion::routine::Function<M,F,T> as criterion::routine::Routine<M,T>>::warm_up
   6: criterion::routine::Routine::sample
   7: criterion::analysis::common
   8: criterion::benchmark_group::BenchmarkGroup<M>::bench_function
   9: packets::main

sbuzzard avatar Mar 03 '22 15:03 sbuzzard

My guess would be just that if payload_len is zero, data should be an empty slice (as a special case) since the offset in that case is bogus). I added a check in our app so that we don't call data if payload_len is 0.

sbuzzard avatar Mar 03 '22 15:03 sbuzzard