[ovs-dev] [PATCH 3/4] Native tunnel: Do not refresh the entry while revalidating.

[Gaëtan Rivet]

On Tue, Nov 2, 2021, at 18:12, Paolo Valerio wrote:
> This is a minor issue but visible e.g. when you try to flush the neigh
> cache while the ARP flow is still present in the datapath, triggering
> the revalidation of the datapath flows which subsequently
> refreshes/adds the entry in the cache.
>

Hi, the change makes sense I think. Just a nit below.

> Signed-off-by: Paolo Valerio <pvalerio at redhat.com>
> ---
>  lib/tnl-neigh-cache.c        |   20 +++++++++++++-------
>  lib/tnl-neigh-cache.h        |    2 +-
>  ofproto/ofproto-dpif-xlate.c |    3 ++-
>  3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index 8eacaccd8..58dba5e5c 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -174,7 +174,7 @@ tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst,
> 
>  static int
>  tnl_arp_snoop(const struct flow *flow, struct flow_wildcards *wc,
> -              const char name[IFNAMSIZ])
> +              const char name[IFNAMSIZ], bool update)

I did not understand the intention / meaning of 'update' until reading the
last lines of this commit, so until I was able to see 'ctx->xin->allow_side_effects'.
I think renaming this variable 'allow_update' would be clearer about how it's meant
to be used.

Same for other occurrences.

-- 
Gaetan Rivet