netdev - Re: [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether

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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 13 Jan 2011 11:38:42 -0500
From:	Chris Metcalf <cmetcalf@...era.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Joe Perches <joe@...ches.com>,
	Tobias Klauser <tklauser@...tanz.ch>,
	David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next-2.6] etherdevice.h: Add is_unicast_ether_addr
 function

On 1/13/2011 3:55 AM, Eric Dumazet wrote:
> Le jeudi 13 janvier 2011 à 00:24 -0800, Joe Perches a écrit :
>> On Thu, 2011-01-13 at 09:14 +0100, Tobias Klauser wrote:
>>> >From a check for !is_multicast_ether_addr it is not always obvious that
>>> we're checking for a unicast address. So add this helper function to
>>> make those code paths easier to read.
>>>  include/linux/etherdevice.h |   11 +++++++++++
>> []
>>>  /**
>>> + * is_unicast_ether_addr - Determine if the Ethernet address is unicast
>>> + * @addr: Pointer to a six-byte array containing the Ethernet address
>>> + *
>>> + * Return true if the address is a unicast address.
>>> + */
>>> +static inline int is_unicast_ether_addr(const u8 *addr)
>>> +{
>>> +	return !is_multicast_ether_addr(addr);
>>> +}
>> Can't you simply use is_valid_ether_addr?
>>
>> I can't think of much reason that a new function for
>> !multicast without the !is_zero is needed.
>>
> performance ?
>
> is_valid_ether_addr() is used at device init time, not when receiving
> packets.
>
> I am not sure we _need_ to check for is_zero_ether_addr() each time we
> receive a packet.
>
> Either a MAC is unicast or multicast.
>
> A zero address is not multicast for sure.

I agree - the is_zero_ether_addr() check seems pointless in the context of
the running interface.

Also, I think a static inline is better form than a #define, all things
being equal.

So, I like Tobias' reworked patches.  I can take them into the Tilera tree,
but I'd prefer David Miller take them into the net tree if he is agreeable,
since it now includes changes to generic networking code.  If you take the
latter approach you can include my:

Acked-by: Chris Metcalf <cmetcalf@...era.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
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