On 11/6/26 6:23 AM, Dumitru Ceara wrote:
> 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.
Ok true, I guess I was just thinking if we're going to add to the
version string then it should be more robust than something only used by
the benchmark. But you're right, we can just not append to it if
__has_feature isn't defined. So it doesn't matter.
I'll post a v2 later.
> 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
