On Wednesday 03 November 2010 11:25:12 Tobias Klauser wrote:
On 2010-11-03 at 11:12:12 +0100, Sven Eckelmann sven.eckelmann@gmx.de
wrote:
On Wednesday 03 November 2010 10:59:02 you wrote:
Replace custom ethernet address check functions by calls to the helpers in linux/etherdevice.h
Have you proven that the addresses are always two bytes aligned in memory? Afaik this function needs it.
I don't think they need to be two bytes aligned, but I might be wrong.
compare_ether_addr uses a two byte pointer to access 3x two bytes. This makes it necessary to have all those 3 bytes aligned to 2 byte boundaries. Otherwise the compiler has to generate special instructions on architectures which don't support loads on non-aligned addresses. Usually he doesn't do it unless he has some indications that it is necessary (__attribute__ ((packed)) for example).
There is also documentation available on that topic in Documentation/unaligned-memory-access.txt
And maybe it is good to use is_broadcast_ether_addr, but leave compare_ether_addr part open (or prove that we always have those two operands correctly aligned).
In one case where the address was tested for broadcast and multicast address, the broadcast address check can be omitted as broadcast is also a multicast address.
We need to distinguish between these two types for different optimizations (research is currently done for multicast over mesh - but currently not part of batman-adv).
I was refering to the following part of the patch:
if (is_bcast(ethhdr->h_dest) || is_mcast(ethhdr->h_dest)) {
if (is_multicast_ether_addr(ethhdr->h_dest)) {I think this change should be legitimate as the same branch is done for multicast and broadcast addresses.
Yes, in that situation it is correct. I was talking more about the complete bcast vs. multicast in batman-adv and didn't look at your patch.
Best regards, Sven