On Tue, Nov 10, 2015 at 04:03:13PM -0800, Ben Pfaff wrote:
> On Thu, Oct 22, 2015 at 03:29:04PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > Use IPv4-mapped addresses and use either tnl_arp_lookup or tnl_nd_lookup.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@redhat.com>
>
> ...
>
> > @@ -5348,8 +5349,13 @@ xlate_push_stats(struct xlate_cache *xcache,
> > break;
> > case XC_TNL_ARP:
> > /* Lookup arp to avoid arp timeout. */
> > - tnl_arp_lookup(entry->u.tnl_arp_cache.br_name,
> > - entry->u.tnl_arp_cache.d_ip, &dmac);
> > + d_ip =
> > in6_addr_get_mapped_ipv4(&entry->u.tnl_arp_cache.d_ipv6);
> > + if (d_ip) {
> > + tnl_arp_lookup(entry->u.tnl_arp_cache.br_name, d_ip,
> > &dmac);
> > + } else {
> > + tnl_nd_lookup(entry->u.tnl_arp_cache.br_name,
> > + &entry->u.tnl_arp_cache.d_ipv6, &dmac);
> > + }
>
> This code seems a little silly to me, because it is going to some
> trouble to distinguish IPv4 from IPv6 and pick the correct
> tnl_*_lookup() function, and either function it picks is going to
> convert that right back to do the lookup. I think it would be more
> sensible to export tnl_arp_lookup__() so that the double conversion
> isn't needed.
>
> Thanks,
>
> Ben.
Urgh. Well, I could just use tnl_nd_lookup here with an extra comment about
that. Exporting tnl_arp_lookup__ would require exporting struct tnl_arp_entry.
But I see your point. The only bad point for me in using tnl_nd_lookup directly
would be the naming. But since we already have an IPv6 address in our hands, why
not just add a comment assuring this will work on IPv4-mapped addresses as well?
I will send something like that.
Thanks.
Cascardo.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
