Re: [PATCH] staging: batman-adv: Use linux/etherdevice.h address helper functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday 03 November 2010 11:25:12 Tobias Klauser wrote:
> On 2010-11-03 at 11:12:12 +0100, Sven Eckelmann <sven.eckelmann@xxxxxx> 
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

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux