On Thu, Oct 07, 2021 at 02:35:28PM +0200, Paolo Valerio wrote:
> In case of native tunnel with bfd enabled, if the MAC address of the
> remote end's interface changes (e.g. because it got rebooted, and the
> MAC address is allocated dinamically), the BFD session will never be
> re-established.
>
> This happens because the local tunnel neigh entry doesn't get updated,
> and the local end keeps sending BFD packets with the old destination
> MAC address. This was not an issue until
> b23ddcc57d41 ("tnl-neigh-cache: tighten arp and nd snooping.")
> because ARP requests were snooped as well avoiding the problem.
>
> Fix this by snooping the incoming packets, and updating the neigh
> cache accordingly.
Can we add a mention that this only affects slow path?
Otherwise it may suggests a performance impact.
> Signed-off-by: Paolo Valerio <pvale...@redhat.com>
> Fixes: b23ddcc57d41 ("tnl-neigh-cache: tighten arp and nd snooping.")
> ---
> lib/tnl-neigh-cache.c | 12 ++++++------
> lib/tnl-neigh-cache.h | 3 +++
> ofproto/ofproto-dpif-xlate.c | 14 ++++++++++++++
> 3 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
> index c8a7b60cd..9d3f00ad9 100644
> --- a/lib/tnl-neigh-cache.c
> +++ b/lib/tnl-neigh-cache.c
> @@ -135,9 +135,9 @@ tnl_neigh_delete(struct tnl_neigh_entry *neigh)
> ovsrcu_postpone(neigh_entry_free, neigh);
> }
>
> -static void
> -tnl_neigh_set__(const char name[IFNAMSIZ], const struct in6_addr *dst,
> - const struct eth_addr mac)
> +void
> +tnl_neigh_set(const char name[IFNAMSIZ], const struct in6_addr *dst,
> + const struct eth_addr mac)
> {
> ovs_mutex_lock(&mutex);
> struct tnl_neigh_entry *neigh = tnl_neigh_lookup__(name, dst);
> @@ -168,7 +168,7 @@ tnl_arp_set(const char name[IFNAMSIZ], ovs_be32 dst,
> const struct eth_addr mac)
> {
> struct in6_addr dst6 = in6_addr_mapped_ipv4(dst);
> - tnl_neigh_set__(name, &dst6, mac);
> + tnl_neigh_set(name, &dst6, mac);
> }
>
> static int
> @@ -208,7 +208,7 @@ tnl_nd_snoop(const struct flow *flow, struct
> flow_wildcards *wc,
> memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst);
> memset(&wc->masks.nd_target, 0xff, sizeof wc->masks.nd_target);
>
> - tnl_neigh_set__(name, &flow->nd_target, flow->arp_tha);
> + tnl_neigh_set(name, &flow->nd_target, flow->arp_tha);
> return 0;
> }
>
> @@ -355,7 +355,7 @@ tnl_neigh_cache_add(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> return;
> }
>
> - tnl_neigh_set__(br_name, &ip6, mac);
> + tnl_neigh_set(br_name, &ip6, mac);
> unixctl_command_reply(conn, "OK");
> }
>
> diff --git a/lib/tnl-neigh-cache.h b/lib/tnl-neigh-cache.h
> index e4b42b059..92fdf5a93 100644
> --- a/lib/tnl-neigh-cache.h
> +++ b/lib/tnl-neigh-cache.h
> @@ -33,6 +33,9 @@
>
> int tnl_neigh_snoop(const struct flow *flow, struct flow_wildcards *wc,
> const char dev_name[IFNAMSIZ]);
> +void
> +tnl_neigh_set(const char name[IFNAMSIZ], const struct in6_addr *dst,
> + const struct eth_addr mac);
> int tnl_neigh_lookup(const char dev_name[IFNAMSIZ], const struct in6_addr
> *dst,
> struct eth_addr *mac);
> void tnl_neigh_cache_init(void);
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 8723cb4e8..4430ac073 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4098,6 +4098,20 @@ terminate_native_tunnel(struct xlate_ctx *ctx, struct
> flow *flow,
> flow->nw_proto == IPPROTO_ICMPV6) &&
> is_neighbor_reply_correct(ctx, flow)) {
> tnl_neigh_snoop(flow, wc, ctx->xbridge->name);
> + } else if (*tnl_port != ODPP_NONE &&
> + ctx->xin->allow_side_effects &&
> + (flow->dl_type == htons(ETH_TYPE_IP) ||
> + flow->dl_type == htons(ETH_TYPE_IPV6))) {
> + struct eth_addr mac = flow->dl_src;
> + struct in6_addr s_ip6;
> +
> + if (flow->nw_src) {
I don't think we will have zeros as valid source IP addr at this
point, but this looks odd. Why not repeating the same check?
if (flow->dl_type == htons(ETH_TYPE_IP)) {
> + in6_addr_set_mapped_ipv4(&s_ip6, flow->nw_src);
> + } else {
> + s_ip6 = flow->ipv6_src;
> + }
> +
> + tnl_neigh_set(ctx->xbridge->name, &s_ip6, mac);
> }
> }
>
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
fbl
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
