netdev - Re: [PATCH net-next 0/2] Neighbour table prep for MPLS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87pp8pqpf2.fsf@x220.int.ebiederm.org>
Date:	Tue, 03 Mar 2015 23:53:21 -0600
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 0/2] Neighbour table prep for MPLS

David Miller <davem@...emloft.net> writes:

> From: ebiederm@...ssion.com (Eric W. Biederman)
> Date: Tue, 03 Mar 2015 17:09:35 -0600
>
>> In preparation for using the IPv4 and IPv6 neighbour tables in my mpls
>> code this patchset factors out ___neigh_lookup_noref from
>> __ipv4_neigh_lookup_noref, __ipv6_lookup_noref and neigh_lookup.
>> Allowing the lookup logic to be shared between the different
>> implementations.  At what appears to be no cost. (Aka the same assembly
>> is generated for ip6_finish_output2 and ip_finish_output2).
>> 
>> After that I add a simple function that takes an address family and an
>> address consults the neighbour table and sends the packet to the
>> appropriate location.  The address family argument decoupls callers
>> of neigh_xmit from the addresses families the packets are sent over.
>> (Aka The ipv6 module can be loaded after mpls and a previously
>> configured ipv6 next hop will start working).
>> 
>> The refactoring in ___neigh_lookup_noref may be a bit overkill but it
>> feels like the right thing to do.  Especially since the same code is
>> generated.
>
> Series applied, thanks.
>
> Maybe we can make neigh_table_find() faster by making it a direct
> array demux of some kind instead of some switch statment thing?
> It's the only think I don't like about neigh_xmit().

We could potentially translate the numbers into the enumeration that is
NEIGH_ARP_TABLE, NEIGH_ND_TABLE, and NEIGH_DN_TABLE.  Or waste a little
bit of memory in have a 30 entry array and looking things up by address
protocol number.   The only disadvantage I can see to using AF_NNN as
the index is that it might be a little less cache friendly.

Other issues the hh header cache doesn't work. (How much do we care).

I worry a little that supporting AF_PACKET case might cause problems
in the future.

The cumulus folks are probably going to want to use neigh_xmit so they
can have ipv6 nexthops on ipv4.  Using this for IPv4 and loosing the
header cache worries me a little.

But it seems like a good starting point.  And right now I am very ready
to say good enough for now and move on to the next thing.

Eric

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html