[PATCH 00/13] net: Add and use ether_addr_equal

[PATCH 00/13] net: Add and use ether_addr_equal

[Joe Perches]

Add a boolean function to test 2 ethernet addresses for equality
Convert compare_ether_addr uses to ether_addr_equal

Joe Perches (13):
  etherdevice.h: Add ether_addr_equal
  802: Convert compare_ether_addr to ether_addr_equal
  8021q: Convert compare_ether_addr to ether_addr_equal
  bridge: netfilter: Convert compare_ether_addr to ether_addr_equal
  bridge: Convert compare_ether_addr to ether_addr_equal
  atm: Convert compare_ether_addr to ether_addr_equal
  bluetooth: Convert compare_ether_addr to ether_addr_equal
  mac80211: Convert compare_ether_addr to ether_addr_equal
  mac80211: Convert compare_ether_addr to ether_addr_equal by hand
  netfilter: Convert compare_ether_addr to ether_addr_equal
  wireless: Convert compare_ether_addr to ether_addr_equal
  wireless: Convert compare_ether_addr to ether_addr_equal by hand
  dsa: Convert compare_ether_addr to ether_addr_equal

 include/linux/etherdevice.h               |   12 +++++++++
 net/802/stp.c                             |    2 +-
 net/8021q/vlan.c                          |   10 +++---
 net/8021q/vlan_core.c                     |    3 +-
 net/8021q/vlan_dev.c                      |   10 +++---
 net/atm/lec.c                             |    6 ++--
 net/atm/mpc.c                             |    3 +-
 net/bluetooth/bnep/core.c                 |    6 ++--
 net/bridge/br_device.c                    |    2 +-
 net/bridge/br_fdb.c                       |   14 +++++-----
 net/bridge/br_input.c                     |    2 +-
 net/bridge/br_stp_bpdu.c                  |    2 +-
 net/bridge/br_stp_if.c                    |   11 +++----
 net/bridge/netfilter/ebt_stp.c            |    4 +-
 net/dsa/slave.c                           |   10 +++---
 net/mac80211/cfg.c                        |    2 +-
 net/mac80211/ibss.c                       |   12 ++++----
 net/mac80211/ieee80211_i.h                |    2 +-
 net/mac80211/iface.c                      |    2 +-
 net/mac80211/mesh.c                       |    4 +-
 net/mac80211/mesh_hwmp.c                  |   14 +++++-----
 net/mac80211/mesh_pathtbl.c               |   12 ++++----
 net/mac80211/mlme.c                       |   29 +++++++++------------
 net/mac80211/rx.c                         |   39 +++++++++++++---------------
 net/mac80211/scan.c                       |    2 +-
 net/mac80211/sta_info.c                   |    8 +++---
 net/mac80211/sta_info.h                   |    2 +-
 net/mac80211/status.c                     |    2 +-
 net/mac80211/tx.c                         |   11 +++----
 net/netfilter/ipset/ip_set_bitmap_ipmac.c |    4 +-
 net/netfilter/xt_mac.c                    |    2 +-
 net/wireless/ibss.c                       |    2 +-
 net/wireless/mlme.c                       |   31 +++++++++++------------
 net/wireless/scan.c                       |    2 +-
 net/wireless/util.c                       |   11 +++----
 net/wireless/wext-sme.c                   |    2 +-
 net/wireless/wext-spy.c                   |    2 +-
 37 files changed, 147 insertions(+), 147 deletions(-)

-- 
1.7.8.111.gad25c.dirty

[PATCH 04/13] bridge: netfilter: Convert compare_ether_addr to ether_addr_equal

[Joe Perches]

Use the new bool function ether_addr_equal to add
some clarity and reduce the likelihood for misuse
of compare_ether_addr for sorting.

Done via cocci script:

$ cat compare_ether_addr.cocci
@@
expression a,b;
@@
-	!compare_ether_addr(a, b)
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	compare_ether_addr(a, b)
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!ether_addr_equal(a, b) == 0
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!ether_addr_equal(a, b) != 0
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	ether_addr_equal(a, b) == 0
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	ether_addr_equal(a, b) != 0
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!!ether_addr_equal(a, b)
+	ether_addr_equal(a, b)

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/bridge/netfilter/ebt_stp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c
index 5b33a2e..071d872 100644
--- a/net/bridge/netfilter/ebt_stp.c
+++ b/net/bridge/netfilter/ebt_stp.c
@@ -164,8 +164,8 @@ static int ebt_stp_mt_check(const struct xt_mtchk_param *par)
 	    !(info->bitmask & EBT_STP_MASK))
 		return -EINVAL;
 	/* Make sure the match only receives stp frames */
-	if (compare_ether_addr(e->destmac, bridge_ula) ||
-	    compare_ether_addr(e->destmsk, msk) || !(e->bitmask & EBT_DESTMAC))
+	if (!ether_addr_equal(e->destmac, bridge_ula) ||
+	    !ether_addr_equal(e->destmsk, msk) || !(e->bitmask & EBT_DESTMAC))
 		return -EINVAL;
 
 	return 0;
-- 
1.7.8.111.gad25c.dirty

[PATCH 10/13] netfilter: Convert compare_ether_addr to ether_addr_equal

[Joe Perches]

Use the new bool function ether_addr_equal to add
some clarity and reduce the likelihood for misuse
of compare_ether_addr for sorting.

Done via cocci script:

$ cat compare_ether_addr.cocci
@@
expression a,b;
@@
-	!compare_ether_addr(a, b)
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	compare_ether_addr(a, b)
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!ether_addr_equal(a, b) == 0
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!ether_addr_equal(a, b) != 0
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	ether_addr_equal(a, b) == 0
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	ether_addr_equal(a, b) != 0
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!!ether_addr_equal(a, b)
+	ether_addr_equal(a, b)

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/netfilter/ipset/ip_set_bitmap_ipmac.c |    4 ++--
 net/netfilter/xt_mac.c                    |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index 0bb16c4..d7eaf10 100644
--- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -111,7 +111,7 @@ bitmap_ipmac_test(struct ip_set *set, void *value, u32 timeout, u32 flags)
 		return -EAGAIN;
 	case MAC_FILLED:
 		return data->ether == NULL ||
-		       compare_ether_addr(data->ether, elem->ether) == 0;
+		       ether_addr_equal(data->ether, elem->ether);
 	}
 	return 0;
 }
@@ -225,7 +225,7 @@ bitmap_ipmac_ttest(struct ip_set *set, void *value, u32 timeout, u32 flags)
 		return -EAGAIN;
 	case MAC_FILLED:
 		return (data->ether == NULL ||
-			compare_ether_addr(data->ether, elem->ether) == 0) &&
+			ether_addr_equal(data->ether, elem->ether)) &&
 		       !bitmap_expired(map, data->id);
 	}
 	return 0;
diff --git a/net/netfilter/xt_mac.c b/net/netfilter/xt_mac.c
index 8160f6b..d5b4fd4 100644
--- a/net/netfilter/xt_mac.c
+++ b/net/netfilter/xt_mac.c
@@ -36,7 +36,7 @@ static bool mac_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		return false;
 	if (skb_mac_header(skb) + ETH_HLEN > skb->data)
 		return false;
-	ret  = compare_ether_addr(eth_hdr(skb)->h_source, info->srcaddr) == 0;
+	ret  = ether_addr_equal(eth_hdr(skb)->h_source, info->srcaddr);
 	ret ^= info->invert;
 	return ret;
 }
-- 
1.7.8.111.gad25c.dirty

Re: [PATCH 04/13] bridge: netfilter: Convert compare_ether_addr to ether_addr_equal

[Stephen Hemminger]

On Tue,  8 May 2012 21:56:48 -0700
Joe Perches <joe@perches.com> wrote:

> Use the new bool function ether_addr_equal to add
> some clarity and reduce the likelihood for misuse
> of compare_ether_addr for sorting.
> 
> Done via cocci script:
> 
> $ cat compare_ether_addr.cocci
> @@
> expression a,b;
> @@
> -	!compare_ether_addr(a, b)
> +	ether_addr_equal(a, b)
> 
> @@
> expression a,b;
> @@
> -	compare_ether_addr(a, b)
> +	!ether_addr_equal(a, b)
> 
> @@
> expression a,b;
> @@
> -	!ether_addr_equal(a, b) == 0
> +	ether_addr_equal(a, b)
> 
> @@
> expression a,b;
> @@
> -	!ether_addr_equal(a, b) != 0
> +	!ether_addr_equal(a, b)
> 
> @@
> expression a,b;
> @@
> -	ether_addr_equal(a, b) == 0
> +	!ether_addr_equal(a, b)
> 
> @@
> expression a,b;
> @@
> -	ether_addr_equal(a, b) != 0
> +	ether_addr_equal(a, b)
> 
> @@
> expression a,b;
> @@
> -	!!ether_addr_equal(a, b)
> +	ether_addr_equal(a, b)
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  net/bridge/netfilter/ebt_stp.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c
> index 5b33a2e..071d872 100644
> --- a/net/bridge/netfilter/ebt_stp.c
> +++ b/net/bridge/netfilter/ebt_stp.c
> @@ -164,8 +164,8 @@ static int ebt_stp_mt_check(const struct xt_mtchk_param *par)
>  	    !(info->bitmask & EBT_STP_MASK))
>  		return -EINVAL;
>  	/* Make sure the match only receives stp frames */
> -	if (compare_ether_addr(e->destmac, bridge_ula) ||
> -	    compare_ether_addr(e->destmsk, msk) || !(e->bitmask & EBT_DESTMAC))
> +	if (!ether_addr_equal(e->destmac, bridge_ula) ||
> +	    !ether_addr_equal(e->destmsk, msk) || !(e->bitmask & EBT_DESTMAC))
>  		return -EINVAL;
>  
>  	return 0;

All look good.
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Tue,  8 May 2012 21:56:44 -0700

> Add a boolean function to test 2 ethernet addresses for equality
> Convert compare_ether_addr uses to ether_addr_equal

This series looks great, I'll apply all of it.

Thanks Joe.

That case you didn't convert in mac80211 is probably the
bug Johannes was talking about which started this whole
discussion.

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[Joe Perches]

On Wed, 2012-05-09 at 21:21 -0400, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Tue,  8 May 2012 21:56:44 -0700
> 
> > Add a boolean function to test 2 ethernet addresses for equality
> > Convert compare_ether_addr uses to ether_addr_equal
> 
> This series looks great, I'll apply all of it.

coccinelle is a nifty, better sed, tool.  Thanks Julia et al.

> That case you didn't convert in mac80211 is probably the
> bug Johannes was talking about which started this whole
> discussion.

Looks like it.

Do you want a similar patch/patches for drivers/net?

If I break it out by nominal driver/maintainer,
it'll be a highish number of low density patches.

$ git grep -w compare_ether_addr drivers/net | wc -l
59
$ git grep -w -l compare_ether_addr drivers/net | wc -l
31

Maybe just a single patch?

Also the compare_ether_addr_64bits function is
still used a couple dozen times.  Maybe another
patch for those?  ether_addr_equal_64bits?

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Wed, 09 May 2012 18:48:04 -0700

> Maybe just a single patch?

Yes, I think high density on this one, it's extremely
mechanical.

> Also the compare_ether_addr_64bits function is
> still used a couple dozen times.  Maybe another
> patch for those?  ether_addr_equal_64bits?

Use your best judgment.

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[Johannes Berg]

On Wed, 2012-05-09 at 21:21 -0400, David Miller wrote:

> That case you didn't convert in mac80211 is probably the
> bug Johannes was talking about which started this whole
> discussion.

The bug case that started it all is in net/wireless/scan.c and Emmanuel
has since changed it back to memcmp(). Not sure if that's the one you
were referring to or not :-)

johannes

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[Joe Perches]

On Thu, 2012-05-10 at 08:48 +0200, Johannes Berg wrote:
> On Wed, 2012-05-09 at 21:21 -0400, David Miller wrote:
> 
> > That case you didn't convert in mac80211 is probably the
> > bug Johannes was talking about which started this whole
> > discussion.
> 
> The bug case that started it all is in net/wireless/scan.c and Emmanuel
> has since changed it back to memcmp(). Not sure if that's the one you
> were referring to or not :-)

That's the one that I left alone.

Post patch:
$ git grep -n -w compare_ether_addr net
net/batman-adv/main.h:198: * note: can't use compare_ether_addr() as it requires aligned memory
net/wireless/scan.c:381:        return compare_ether_addr(a->bssid, b->bssid);

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[Johannes Berg]

On Wed, 2012-05-09 at 23:54 -0700, Joe Perches wrote:
> On Thu, 2012-05-10 at 08:48 +0200, Johannes Berg wrote:
> > On Wed, 2012-05-09 at 21:21 -0400, David Miller wrote:
> > 
> > > That case you didn't convert in mac80211 is probably the
> > > bug Johannes was talking about which started this whole
> > > discussion.
> > 
> > The bug case that started it all is in net/wireless/scan.c and Emmanuel
> > has since changed it back to memcmp(). Not sure if that's the one you
> > were referring to or not :-)
> 
> That's the one that I left alone.
> 
> Post patch:
> $ git grep -n -w compare_ether_addr net
> net/batman-adv/main.h:198: * note: can't use compare_ether_addr() as it requires aligned memory
> net/wireless/scan.c:381:        return compare_ether_addr(a->bssid, b->bssid);

Ok, great, then that means the fix from Emmanuel won't conflict when it
gets in.

johannes

Re: [PATCH 00/13] net: Add and use ether_addr_equal

[Emmanuel Grumbach]

>> >
>> > > That case you didn't convert in mac80211 is probably the
>> > > bug Johannes was talking about which started this whole
>> > > discussion.
>> >
>> > The bug case that started it all is in net/wireless/scan.c and Emmanuel
>> > has since changed it back to memcmp(). Not sure if that's the one you
>> > were referring to or not :-)
>>
>> That's the one that I left alone.
>>
>> Post patch:
>> $ git grep -n -w compare_ether_addr net
>> net/batman-adv/main.h:198: * note: can't use compare_ether_addr() as it requires aligned memory
>> net/wireless/scan.c:381:        return compare_ether_addr(a->bssid, b->bssid);
>
> Ok, great, then that means the fix from Emmanuel won't conflict when it
> gets in.
>

Thanks Joe - as Johannes said this won't conflict with my patch. And
yes the code is now more clear.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html