On Thu, Nov 12, 2015 at 02:38:48AM -0200, Flavio Leitner wrote:
> On Wed, Nov 11, 2015 at 08:29:32AM -0800, Ben Pfaff wrote:
> > On Wed, Nov 11, 2015 at 11:57:04AM -0200, Thadeu Lima de Souza Cascardo
> > wrote:
> > > 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?
> >
> > If the naming is an issue, you could invent a more generic name,
> > e.g. "tnl_lookup_mac_to_ip_binding". But I don't think it's a big deal.
>
> maybe tnl_neigh_lookup() ?
>
> fbl
>
I thought of that, but since we are giving an IPv6 address to tnl_nd_lookup, we
could claim that tnl_neigh_lookup works with both IPv4 and IPv6 addresses. But I
would implement that using IPv4-mapped addresses anyway. And tnl_nd_lookup
already supports that. If there were two different databases, that could make
sense, but we have only one for both mappings. I think commenting that is good
enough.
Cascardo.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
