On 4 Apr 2024, at 14:09, Ilya Maximets wrote:
> ukey_install() returns boolean signaling if the ukey was installed
> or not. Installation may fail for a few reasons:
>
> 1. Conflicting ukey.
> 2. Mutex contention while trying to replace existing ukey.
> 3. The same ukey already exists and active.
>
> Only the first case here signals an actual problem. Third one is
> a little odd for userspace datapath, but harmless. Second is the
> most common one that can easily happen during normal operation
> since other threads like revalidators may be currently working on
> this ukey preventing an immediate access.
>
> Since only the first case is actually worth logging and it already
> has its own log message, removing the 'upcall installation fails'
> warning from the upcall_cb(). This should fix most of the random
> failures of userspace system tests in CI.
>
> While at it, also fixing coverage counters. Mutex contention was
> mistakenly counted as a duplicate upcall. ukey contention for
> revalidators was counted only in one of two places.
>
> New counter added for the ukey contention on replace. We should
> not re-use existing upcall_ukey_contention counter for this, since
> it may lead to double counting.
>
> Fixes: 67f08985d769 ("upcall: Replace ukeys for deleted flows.")
> Fixes: 9cec8274ed9a ("ofproto-dpif-upcall: Add VLOG_WARN_RL logs for
> upcall_cb() error.")
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
Thanks for looking into this, and the patch looks good to me.
Maybe we should have another patch fixing some of the namings?
upcall_ukey_replace -> ukey_replace
handler_duplicate_upcall -> duplicate_upcall
upcall_ukey_contention -> revalidate_ukey_contention -> ukey_contention
Cheers,
Eelco
Acked-by: Eelco Chaudron <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
