[PATCH -next] batadv: Slight optimization of batadv_compare_eth

[PATCH -next] batadv: Slight optimization of batadv_compare_eth

[Joe Perches]

Use CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to check if
this function can be optimized by using the generic
ether_addr_equal.

Remove the unnecessary ?: after the unoptimized memcmp.

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/batman-adv/main.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index f94f287b..74a7a3f 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -266,7 +266,11 @@ static inline void batadv_dbg(int type __always_unused,
  */
 static inline int batadv_compare_eth(const void *data1, const void *data2)
 {
-	return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+	return ether_addr_equal(data1, data2);
+#else
+	return memcmp(data1, data2, ETH_ALEN) == 0;
+#endif
 }
 
 /**

Re: [PATCH -next] batadv: Slight optimization of batadv_compare_eth

[Antonio Quartulli]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 06/12/13 09:18, Joe Perches wrote:
> Use CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to check if this
> function can be optimized by using the generic ether_addr_equal.
> 
> Remove the unnecessary ?: after the unoptimized memcmp.
> 
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

Acked-by: Antonio Quartulli <antonio-x4xJYDvStAgysxA8WJXlww@public.gmane.org>


- -- 
Antonio Quartulli
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJSoaKXAAoJEEKTMo6mOh1VMjoP/3GqGy0TQG1yXRzbrzAbbklJ
8Oa6DHQd2pSN/cJkR501JYwSUnCGh3m7eh7qc1HZYrEIgxIrJDLHWSqBL3FPtgap
gQ92BBQG/pd38ZmmWSeZob44bEMEGAn00B1kgFx+Xb4YW4bP2rDAlvPxjjaqjT3U
tBNbNYxUTBY2JMftI7WwEOwMWgLPVoHR1PhAbBTpbXMXCRrGp/vXN1xXam1xIDNx
kahFmHA1Dc91dwr8ev4bcUjfGO2tXkQOEoqKRkwc1SEzuKTjgJNDxdbFNqG6vMAm
k/aXhMvf3DOUkaCeslRQIkwefbKmzN69c1x2ND7S8mLEMIKyBuhKUFcf6PebD9SK
Dc+KdwIx0lsaS2vGqOt6mUSie1KfY8QNAc74eF6XjDhKyJynYXe6S3L40uT4eLXR
8fwGClJmyYVIiYD65NOtiIx6C2uHFchjDpXjlz0X8N9oQ2Azr4e2RMTGkkoihvOh
94r1dmAaBrnRiAu6AkImhgN+lWgLglHWD5trmmRCxNEJf0iW+x9ZaR4yAQFDUP3k
mrk5AN8JuRZkOeY5QK4AIZ0bFUh8tKW6JaNb8VHQGsLqjhAB/HPS6VljyRgMalyQ
c/kLlEOIIZ9dsjMwKMgyoJ9bMtTYUTxrriKz12mQbx8mWvm4CMFEQglljQiuTyvh
J+UsjAHs/ffW59JIGCfB
=8xL7
-----END PGP SIGNATURE-----

Re: [PATCH -next] batadv: Slight optimization of batadv_compare_eth

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Fri, 06 Dec 2013 00:18:10 -0800

> @@ -266,7 +266,11 @@ static inline void batadv_dbg(int type __always_unused,
>   */
>  static inline int batadv_compare_eth(const void *data1, const void *data2)
>  {
> -	return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
> +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> +	return ether_addr_equal(data1, data2);
> +#else
> +	return memcmp(data1, data2, ETH_ALEN) == 0;
> +#endif
>  }

Let's not crap up implementations with these ifdefs.

What's the specific situation here?  Is it that 'data1' and/or
'data2' my not be u16 aligned?

If so, make a function for that in linux/etherdevice.h and invoke it
in such places.  You can name it something like
"ether_addr_equal_unaligned()" or similar.

Thanks.

Re: [PATCH -next] batadv: Slight optimization of batadv_compare_eth

[Joe Perches]

On Fri, 2013-12-06 at 15:39 -0500, David Miller wrote:
> From: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> Date: Fri, 06 Dec 2013 00:18:10 -0800
> 
> > @@ -266,7 +266,11 @@ static inline void batadv_dbg(int type __always_unused,
> >   */
> >  static inline int batadv_compare_eth(const void *data1, const void *data2)
> >  {
> > -	return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
> > +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> > +	return ether_addr_equal(data1, data2);
> > +#else
> > +	return memcmp(data1, data2, ETH_ALEN) == 0;
> > +#endif
> >  }
> 
> Let's not crap up implementations with these ifdefs.
> 
> What's the specific situation here?  Is it that 'data1' and/or
> 'data2' my not be u16 aligned?

Yes.

> If so, make a function for that in linux/etherdevice.h and invoke it
> in such places.  You can name it something like
> "ether_addr_equal_unaligned()" or similar.

I'll resubmit if/after you apply the
ether_addr_equal/compare_ether_addr removal.

cheers, Joe

Re: [PATCH -next] batadv: Slight optimization of batadv_compare_eth

[David Miller]

From: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
Date: Fri, 06 Dec 2013 13:17:01 -0800

> On Fri, 2013-12-06 at 15:39 -0500, David Miller wrote:
>> If so, make a function for that in linux/etherdevice.h and invoke it
>> in such places.  You can name it something like
>> "ether_addr_equal_unaligned()" or similar.
> 
> I'll resubmit if/after you apply the
> ether_addr_equal/compare_ether_addr removal.

Done.

[PATCH -next] etherdevice: Add ether_addr_equal_unaligned

[Joe Perches]

Add a generic routine to test if possibly unaligned
to u16 Ethernet addresses are equal.

If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set,
this uses the slightly faster generic routine
ether_addr_equal, otherwise this uses memcmp.

Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
---
 include/linux/etherdevice.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 3526e81..eb36845 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -266,6 +266,24 @@ static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
 }
 
 /**
+ * ether_addr_equal_unaligned - Compare two not u16 aligned Ethernet addresses
+ * @addr1: Pointer to a six-byte array containing the Ethernet address
+ * @addr2: Pointer other six-byte array containing the Ethernet address
+ *
+ * Compare two Ethernet addresses, returns true if equal
+ *
+ * Please note: Use only when any Ethernet address may not be u16 aligned.
+ */
+static inline bool ether_addr_equal_unaligned(const u8 *addr1, const u8 *addr2)
+{
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+	return ether_addr_equal(addr1, addr2);
+#else
+	return memcmp(addr1, addr2, ETH_ALEN) == 0;
+#endif
+}
+
+/**
  * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
  * @dev: Pointer to a device structure
  * @addr: Pointer to a six-byte array containing the Ethernet address

[PATCH -next] batadv: Slight optimization of batadv_compare_eth

[Joe Perches]

Use the newly added generic routine ether_addr_equal_unaligned
to test if possibly unaligned to u16 Ethernet addresses are equal.

This slightly improves comparison time for systems with
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.

Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
---
 net/batman-adv/main.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index f94f287b..322dd73 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -266,7 +266,7 @@ static inline void batadv_dbg(int type __always_unused,
  */
 static inline int batadv_compare_eth(const void *data1, const void *data2)
 {
-	return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
+	return ether_addr_equal_unaligned(data1, data2);
 }
 
 /**

Re: [PATCH -next] batadv: Slight optimization of batadv_compare_eth

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Fri, 06 Dec 2013 14:39:46 -0800

> Use the newly added generic routine ether_addr_equal_unaligned
> to test if possibly unaligned to u16 Ethernet addresses are equal.
> 
> This slightly improves comparison time for systems with
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

[PATCH -next] etherdevice: Optimize a few is_<foo>_ether_addr functions

[Joe Perches]

If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set,
several is_<foo>_ether_addr functions can be slightly
improved by using u32 dereferences.

I believe all current uses of is_zero_ether_addr and
is_broadcast_ether_addr are u16 aligned, so always use
u16 references to improve those functions performance.

Document the u16 alignment requirements.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/linux/etherdevice.h | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 3526e81..9518e59 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -61,6 +61,8 @@ static const u8 eth_reserved_addr_base[ETH_ALEN] __aligned(2) =
  *
  * Return true if address is link local reserved addr (01:80:c2:00:00:0X) per
  * IEEE 802.1Q 8.6.3 Frame filtering.
+ *
+ * Please note: addr must be aligned to u16.
  */
 static inline bool is_link_local_ether_addr(const u8 *addr)
 {
@@ -68,7 +70,12 @@ static inline bool is_link_local_ether_addr(const u8 *addr)
 	static const __be16 *b = (const __be16 *)eth_reserved_addr_base;
 	static const __be16 m = cpu_to_be16(0xfff0);
 
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+	return (((*(const u32 *)addr) ^ (*(const u32 *)b)) |
+		((a[2] ^ b[2]) & m)) == 0;
+#else
 	return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | ((a[2] ^ b[2]) & m)) == 0;
+#endif
 }
 
 /**
@@ -76,10 +83,18 @@ static inline bool is_link_local_ether_addr(const u8 *addr)
  * @addr: Pointer to a six-byte array containing the Ethernet address
  *
  * Return true if the address is all zeroes.
+ *
+ * Please note: addr must be aligned to u16.
  */
 static inline bool is_zero_ether_addr(const u8 *addr)
 {
-	return !(addr[0] | addr[1] | addr[2] | addr[3] | addr[4] | addr[5]);
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+	return ((*(const u32 *)addr) | (*(const u16 *)(addr + 4))) == 0;
+#else
+	return (*(const u16 *)(addr + 0) |
+		*(const u16 *)(addr + 2) |
+		*(const u16 *)(addr + 4)) == 0;
+#endif
 }
 
 /**
@@ -110,10 +125,14 @@ static inline bool is_local_ether_addr(const u8 *addr)
  * @addr: Pointer to a six-byte array containing the Ethernet address
  *
  * Return true if the address is the broadcast address.
+ *
+ * Please note: addr must be aligned to u16.
  */
 static inline bool is_broadcast_ether_addr(const u8 *addr)
 {
-	return (addr[0] & addr[1] & addr[2] & addr[3] & addr[4] & addr[5]) == 0xff;
+	return (*(const u16 *)(addr + 0) &
+		*(const u16 *)(addr + 2) &
+		*(const u16 *)(addr + 4)) == 0xffff;
 }
 
 /**
@@ -135,6 +154,8 @@ static inline bool is_unicast_ether_addr(const u8 *addr)
  * a multicast address, and is not FF:FF:FF:FF:FF:FF.
  *
  * Return true if the address is valid.
+ *
+ * Please note: addr must be aligned to u16.
  */
 static inline bool is_valid_ether_addr(const u8 *addr)
 {

Re: [PATCH -next] etherdevice: Optimize a few is_<foo>_ether_addr functions

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Fri, 06 Dec 2013 15:44:21 -0800

> If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set,
> several is_<foo>_ether_addr functions can be slightly
> improved by using u32 dereferences.
> 
> I believe all current uses of is_zero_ether_addr and
> is_broadcast_ether_addr are u16 aligned, so always use
> u16 references to improve those functions performance.
> 
> Document the u16 alignment requirements.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

Re: [PATCH -next] etherdevice: Add ether_addr_equal_unaligned

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Fri, 06 Dec 2013 14:21:01 -0800

> Add a generic routine to test if possibly unaligned
> to u16 Ethernet addresses are equal.
> 
> If CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set,
> this uses the slightly faster generic routine
> ether_addr_equal, otherwise this uses memcmp.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.