Gaëtan Rivet <[email protected]> writes:

> 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 <[email protected]>
>> ---
>>  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.

ACK

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to