[ovs-dev] [PATCH v2 3/3] xlate: call tnl_neigh_snoop() from terminate_native_tunnel()
Zoltán Balogh
zoltan.balogh at ericsson.com
Mon Jan 8 11:43:58 UTC 2018
Hi Ben,
> I don't understand yet the motivation here. What does it mean "to keep
> ARP neighbor cache clean"? Is this a bug fix, a performance fix, a
> cleanup, or something else?
>
> What is the goal of the filtering you mention? Is that a second change,
> that can and should be a separate patch, or is it closely coupled to the
> first change you mention?
Jan gave a very good summary in his mail:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/342662.html
> This gives me the following Clang errors:
>
> ../ofproto/ofproto-dpif-xlate.c:3749:13: error: cast from 'const uint8_t *' (aka 'const unsigned char *') to
> 'const struct in6_addr *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]
> /usr/include/netinet/in.h:454:36: note: expanded from macro 'IN6_ARE_ADDR_EQUAL'
> ../ofproto/ofproto-dpif-xlate.c:3749:13: error: cast from 'const uint8_t *' (aka 'const unsigned char *') to
> 'const struct in6_addr *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]
> /usr/include/netinet/in.h:455:36: note: expanded from macro 'IN6_ARE_ADDR_EQUAL'
Thank you for the review, I'm going to fix this.
> The change to tnl-neigh-cache.c that just adds an #include directive
> seems odd to me.
Sorry, this is a leftover from a previous version. There is no need for the
include.
>
> The treatment of the new xbridge_addr is different from the treatment of
> everything already handled in xlate_ofproto_set(). Why does it need
> special treatment? Is it RCU safe? Why does it need a refcount (none
> of the other structs do)?
>
The xbridge_addr holds the current IP addresses assigned to the bridge. These
can change by time. Xbridge_addr is passed to xlate_xbridge_set(), where it is
set as xbridge->addr. Most of the xbridge members are handled the same way
(counting references etc.) in xlate_xbridge_set().
Maybe it's disturbing to retrieve xbridge_addr in xlate_ofproto_set() and then
pass it to xlate_bridge_set().
Do you think, would it be better to store xbridge_addr in ofproto and update it
when an IP address is added or removed, then pass an ofproto member to
xlate_ofproto_set()?
Best regards,
Zoltan
> Thanks,
>
> Ben.
More information about the dev
mailing list