[ovs-dev] [PATCH v4 04/28] datapath: backport: net: add dst_cache support

Jesse Gross jesse at kernel.org
Fri Jul 8 01:44:24 UTC 2016


On Thu, Jul 7, 2016 at 6:38 PM, pravin shelar <pshelar at ovn.org> wrote:
> On Thu, Jul 7, 2016 at 6:04 PM, Jesse Gross <jesse at kernel.org> wrote:
>> On Thu, Jul 7, 2016 at 5:23 PM, Pravin B Shelar <pshelar at ovn.org> wrote:
>>> This backports dst-cache implementation from upstream implementation.
>>>
>>>     commit 911362c70df5b766c243dc297fadeaced786ffd8
>>>     Author: Paolo Abeni <pabeni at redhat.com>
>>>
>>>     net: add dst_cache support
>>>     This patch add a generic, lockless dst cache implementation.
>>>     The need for lock is avoided updating the dst cache fields
>>>     only in per cpu scope, and requiring that the cache manipulation
>>>     functions are invoked with the local bh disabled.
>>>
>>>     The refresh_ts and reset_ts fields are used to ensure the cache
>>>     consistency in case of cuncurrent cache update (dst_cache_set*) and
>>>     reset operation (dst_cache_reset).
>>>
>>>     Consider the following scenario:
>>>
>>>     CPU1:                                       CPU2:
>>>       <cache lookup with emtpy cache: it fails>
>>>       <get dst via uncached route lookup>
>>>                                                 <related configuration changes>
>>>                                                 dst_cache_reset()
>>>       dst_cache_set()
>>>
>>>     The dst entry set passed to dst_cache_set() should not be used
>>>     for later dst cache lookup, because it's obtained using old
>>>     configuration values.
>>>
>>>     Since the refresh_ts is updated only on dst_cache lookup, the
>>>     cached value in the above scenario will be discarded on the next
>>>     lookup.
>>>
>>>     Signed-off-by: Paolo Abeni <pabeni at redhat.com>
>>>     Suggested-and-acked-by: Hannes Frederic Sowa <hannes at stressinduktion.org>
>>>     Signed-off-by: David S. Miller <davem at davemloft.net>
>>>
>>> Signed-off-by: Pravin B Shelar <pshelar at ovn.org>
>>> Acked-by: Jesse Gross <jesse at kernel.org>
>>
>> I saw in the big Geneve/VXLAN backport patch that USE_UPSTREAM_TUNNEL
>> becomes dependent on having the dst cache. That's fine but I couldn't
>> find anywhere that prevents the reverse - continuing to use our
>> version of the dst cache when we have upstream tunnels (the functions
>> here look like unconditional replacements). It seems like that will
>> create a problem in a couple weeks when 4.7 is released.
>
> As discussed I am planing on doing it later when I add support for new
> kernel. I can not test it without compiling it with new kernel.

I don't really understand. Isn't this as simple as wrapping these
files in !USE_UPSTREAM_TUNNEL? Leaving it untested in that state seems
better to me than having an untested version that is pretty much sure
to cause a problem.



More information about the dev mailing list