Re: [PATCH net-next-2.6] netdev: tilepro: Use is_multicast_ether_addr helper

On 2011-01-13 at 08:42:50 +0100, David Miller [off-list ref] wrote:

From: Tobias Klauser <tklauser@distanz.ch>
Date: Thu, 13 Jan 2011 07:58:55 +0100

quoted

On 2011-01-13 at 03:45:01 +0100, David Miller [off-list ref] wrote:

quoted

From: Chris Metcalf <redacted>
Date: Wed, 12 Jan 2011 12:49:03 -0500

quoted

On 1/12/2011 4:31 AM, Tobias Klauser wrote:

quoted

Use is_multicast_ether_addr from linux/etherdevice.h instead of a custom
macro. Also remove the broadcast address check, as it is considered a
multicast address too.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/tile/tilepro.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

Thanks, I've taken this into the Tilera tree!

Don't, his transformation is buggy.

Why?

Doesn't the code want to make sure that only unicast addresses get
filtered?

quoted

You can't get rid of the broadcast check, it needs to be there.
Think about it.

If a unicast address is in buf, is_multicast_ether_addr returns false
(and is_broadcast_addr would too) and thus it would get filtered.

Ok, this may be correct, but it makes the code hard to read.

I think the old code, whilst redundant, is easier to understand.

Agreed. Thanks for clarifying.

Why not add a function "is_unicast_ether_addr()" and use that
here instead?

I'll send a patch to add the function to linux/etherdevice.h and an
updated patch for the tilepro driver as a follow up.