On 11/6/26 8:18 AM, Ales Musil wrote: > On Thu, Nov 5, 2026 at 7:28 PM Rosemarie O'Riorden <[email protected]> > wrote: > >> On 11/5/26 12:05 PM, Rosemarie O'Riorden wrote: >>> On 11/5/26 2:05 AM, Ales Musil wrote: >>>> >>>> >>>> On Sat, Oct 31, 2026 at 2:14 AM Rosemarie O'Riorden via dev <ovs- >>>> [email protected] <mailto:[email protected]>> wrote: >>>> >>>> Compiling with ASan will completely distort the VmPeak memory usage >>>> statistics the benchmark provides, and it is incompatible with >> Valgrind. >>>> In addition, the timing statistics with either sanitizer will be >>>> distorted too. So exit immediately if compiled with either. >>>> >>>> Reported-at: https://patchwork.ozlabs.org/project/ovn/ >>>> patch/[email protected]/ <https:// >>>> patchwork.ozlabs.org/project/ovn/patch/20261023051521.325497-1- >>>> [email protected]/> >>>> Fixes: f4a66b7871d6 ("tutorial: Add developer performance >> benchmark.") >>>> Signed-off-by: Rosemarie O'Riorden <[email protected] >>>> <mailto:[email protected]>> >>>> --- >>>> >>>> >>>> Hi Rosemarie, >>>> >>>> I'm not a huge fan of this approach, it seems a bit fragile. >>>> We could utilize some macro "magic" instead to make it >>>> more reliable. We could use __has_feature(address_sanitizer) >>>> and __has_feature(undefined_behavior_sanitizer) to append string >>>> to OVN_PACKAGE_VERSION, with that any binary compiled with >>>> sanitizers could have "ASAN" and/or "UBSAN" in the --version string. >>>> It doesn't affect normal operation and it might be quite useful in >> general >>>> so you can very easily tell that the binary was compiled with any >>>> sanitizer type WDYT? >>> >>> Hi Ales, thanks for the review. >>>
Hi Rosemarie, Ales, >>> Hm, that is an interesting idea that does sound useful in general. I'll >>> try this approach. >> >> I should note that I just saw that __has_feature() is only supported in >> gcc 14 and on, because it was originally just in clang, so we'll have >> accommodate for older gcc versions. >> >> I found a stack overflow thread [0] with a good workaround: >> >> #if defined(__has_feature) >> # if __has_feature(address_sanitizer) && >> !defined(__SANITIZE_ADDRESS__) // for clang >> # define __SANITIZE_ADDRESS__ // GCC already sets this >> # endif >> #endif >> >> #if defined(__SANITIZE_ADDRESS__) >> // ASAN is enabled . . . >> #endif >> >> However there's no __SANITIZE_ADDRESS__ equivalent for the undefined >> behavior sanitizer in gcc so that complicates things a bit... There was >> actually a rejected gcc patch to add __SANITIZE_UNDEFINED__ :D and the >> maintainer's recommendation was: >> "Those that want such macros to say provide libubsan definitions on >> their own can do that in their build system, next to >> -fsanitize=undefined provide their own -Dwhatever that will make that >> happen." >> >> So I guess I can try this route. It feels kinda hacky and definitely >> asymmetric but I don't see another way. >> >> [0] >> >> https://stackoverflow.com/questions/19627332/how-to-specify-gcc-version-at-runtime >> [1] https://gcc.gnu.org/legacy-ml/gcc-patches/2019-09/msg01291.html > > > Well I wouldn't complicate it, we can just skip those strings if > __has_feature is not defined that is completely fine. Our current > CI has GCC 15 on Ubuntu 26.04 and GCC 16 on Fedora. For > testing purposes we should be good. > > As discussed on the original thread, when reviewing the ovn-benchmark addition, we can also just decide to _not do anything about it_. So I'd keep it as simple as possible. That said, I'm OK with what Ales is suggesting above, I would definitely not try to figure out all edge cases. FWIW, I was also OK with just using nm. Regards, Dumitru >> >>> >>>> >>>> >>>> tutorial/ovn-benchmark.sh | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/tutorial/ovn-benchmark.sh b/tutorial/ovn-benchmark.sh >>>> index 01c090e84..9de23be1d 100755 >>>> --- a/tutorial/ovn-benchmark.sh >>>> +++ b/tutorial/ovn-benchmark.sh >>>> @@ -161,6 +161,11 @@ if [ ! -d "$PWD/sandbox" ]; then >>>> exit 1 >>>> fi >>>> >>>> +if [ "$(nm -an $PWD/../controller/ovn-controller | grep "asan\| >>>> ubsan")" ]; then >>>> + echo "Compiled with address sanitizer, which is incompatible. >>>> Exiting." >>>> + exit 1 >>>> +fi >>>> + >>>> # Apply defaults if not set by user. >>>> NODES=${NODES:-$DEFAULT_NODES} >>>> >>>> -- >>>> 2.56.0 >>>> >>>> _______________________________________________ >>>> dev mailing list >>>> [email protected] <mailto:[email protected]> >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https:// >>>> mail.openvswitch.org/mailman/listinfo/ovs-dev> >>>> >>>> >>>> Regards, >>>> Ales >>> >> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
