net: plip: slight optimization of addr compare
Subsystems (3)
PATCH 19/20 net: plip: slight optimization of addr compare
Use possibly more efficient ether_addr_equal
to instead of memcmp.
--- drivers/net/plip/plip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/plip/plip.c +1 -1
@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;
Signature · 1 line
1.8.0
Quoted 14 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.Cc "David S. Miller" <[email protected]>Signed-off-by Ding Tianhong <[email protected]>--- drivers/net/plip/plip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/plip/plip.c +0
-0
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 8 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;
What about :
if (is_multicast_ether_addr(eth->h_dest)) {
if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast))
skb->pkt_type = PACKET_BROADCAST;
else
skb->pkt_type = PACKET_MULTICAST;
}Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.
[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0
[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) {
if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast))
skb->pkt_type = PACKET_BROADCAST;
else
skb->pkt_type = PACKET_MULTICAST;
}That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using ether_addr_equal_64bits may be possible to do in other places given other structs too.
Perhaps it's a possible spatch/coccinelle conversion,
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Maybe Julia does. (cc'd)
Quoted 35 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.
Quoted 8 lines
Perhaps it's a possible spatch/coccinelle conversion,
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Maybe Julia does. (cc'd)
I'm not sure to competely understand the issues. Could you explain more?
thanks,
julia
Quoted 39 lines · Julia Lawall
Quoted 37 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.Perhaps it's a possible spatch/coccinelle conversion,
Quoted 8 lines
Quoted 6 lines
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Maybe Julia does. (cc'd)
I'm not sure to competely understand the issues. Could you explain more?
Hi Julia.
Maybe this explanation is helpful?
ethernet addresses are u8[6] (48 bits)
ether_addr_equal_64bits gets passed a pointer to u8[8] and is more efficient on 64 bit architectures than ether_addr_equal because the test can be done with a single compare and shift.
The idea is not to access past the end of the ethernet address as appropriate (think pointer to eeprom or other such end-of-addressable memory conditions)
If a struct containing an ethernet address has additional members after the ethernet address, or the u8[6] address passed to ether_addr_equal is not going to access past the end of memory or the structure, then ether_addr_equal_64bits should be used in lieu of ether_addr_equal.
Joe
PATCH 1/1 i40e: use ether_addr_equal_64bits
All ether_addr_equal tests in i40e can use the
slightly more efficient ether_addr_equal_64bits.
All addresses passed to the various functions that use ether_addr_equal are using structs that have 2 or more bytes of additional data after the mac addr being tested.
struct i40e_mac_filter.macaddr[6] (followed by s16 vlan) struct net_device.dev_addr (pointer to char array of MAX_ADDR_LEN) struct sockaddr.sa_data (array of 14 bytes)
struct netdev_hw_addr.addr (pointer to char array of MAX_ADDR_LEN)
---
Quoted 64 lines · Joe Perches
Quoted 39 lines · Julia Lawall
Quoted 37 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.Perhaps it's a possible spatch/coccinelle conversion,
Quoted 8 lines
Quoted 6 lines
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Maybe Julia does. (cc'd)
I'm not sure to competely understand the issues. Could you explain more?
Hi Julia.
Maybe this explanation is helpful?
ethernet addresses are u8[6] (48 bits)
ether_addr_equal_64bitsgets passed a pointer to u8[8] and is more efficient on 64 bit architectures thanether_addr_equalbecause the test can be done with a single compare and shift.The idea is not to access past the end of the ethernet address as appropriate (think pointer to eeprom or other such end-of-addressable memory conditions)
Quoted 6 lines
If a struct containing an ethernet address has additional members after the ethernet address, or the u8[6] addressa passed to
ether_addr_equalis not going to access past the end of memory or the structure, thenether_addr_equal_64bitsshould be used in lieu ofether_addr_equal.
I believe this is correct, but maybe the
conditions for using ether_addr_equal_64bits
could be documented a bit better.
Jeff/Intel folk? What do you think?
drivers/net/ethernet/intel/i40e/i40e_main.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_main.c +11 -7
@@ -992,7 +992,7 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi,return NULL ;list_for_each_entry (f, &vsi->mac_filter_list, list) { -if ((ether_addr_equal (macaddr, f->macaddr)) && +if (ether_addr_equal_64bits (macaddr, f->macaddr) && (vlan == f->vlan) && (!is_vf || f->is_vf) && (!is_netdev || f->is_netdev))
@@ -1020,7 +1020,7 @@ struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr,return NULL ;list_for_each_entry (f, &vsi->mac_filter_list, list) { -if ((ether_addr_equal (macaddr, f->macaddr)) && +if (ether_addr_equal_64bits (macaddr, f->macaddr) && (!is_vf || f->is_vf) && (!is_netdev || f->is_netdev))return f;
@@ -1209,7 +1209,7 @@ static int i40e_set_mac(struct net_device *netdev, void *p)netdev_info (netdev,"set mac address=%pM\n" , addr->sa_data); -if (ether_addr_equal (netdev->dev_addr, addr->sa_data)) +if (ether_addr_equal_64bits (netdev->dev_addr, addr->sa_data))return 0 ;if (vsi->type ==I40E_VSI_MAIN ) {
@@ -1409,21 +1409,24 @@ static void i40e_set_rx_mode(struct net_device *netdev)if (is_multicast_ether_addr (f->macaddr)) {netdev_for_each_mc_addr (mca, netdev) { -if (ether_addr_equal (mca->addr, f->macaddr)) { +if (ether_addr_equal_64bits (mca->addr, + f->macaddr)) { found = true;break ; } } }else {netdev_for_each_uc_addr (uca, netdev) { -if (ether_addr_equal (uca->addr, f->macaddr)) { +if (ether_addr_equal_64bits (uca->addr, + f->macaddr)) { found = true;break ; } }for_each_dev_addr (netdev, ha) { -if (ether_addr_equal (ha->addr, f->macaddr)) { +if (ether_addr_equal_64bits (ha->addr, + f->macaddr)) { found = true;break ; }
@@ -1889,7 +1892,8 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)list_for_each_entry (f, &vsi->mac_filter_list, list) {if (is_netdev) {if (f->vlan && -ether_addr_equal (netdev->dev_addr, f->macaddr)) +ether_addr_equal_64bits (netdev->dev_addr, + f->macaddr)) filter_count++; }
Re: [PATCH] i40e: use ether_addr_equal_64bits
Quoted 8 lines · Joe Perches
All
ether_addr_equaltests in i40e can use the
slightly more efficientether_addr_equal_64bits.All addresses passed to the various functions that use
ether_addr_equalare using structs that have 2 or more bytes of additional data after the mac addr being tested.
Quoted 6 lines
struct i40e_mac_filter.macaddr[6] (followed by s16 vlan) struct net_device.dev_addr (pointer to char array of
MAX_ADDR_LEN) struct sockaddr.sa_data (array of 14 bytes)
struct netdev_hw_addr.addr (pointer to char array ofMAX_ADDR_LEN)Signed-off-by Joe Perches <[email protected]>
It looks good, but I would like Jesse and Shannon to review it since we just were talking about a similar patch last week.
I will add the patch to my queue for now, thanks Joe.
Quoted 145 lines
---
Quoted 64 lines · Joe Perches
Quoted 39 lines · Julia Lawall
Quoted 37 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.Perhaps it's a possible spatch/coccinelle conversion,
Quoted 8 lines
Quoted 6 lines
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Maybe Julia does. (cc'd)
I'm not sure to competely understand the issues. Could you explain more?
Hi Julia.
Maybe this explanation is helpful?
ethernet addresses are u8[6] (48 bits)
ether_addr_equal_64bitsgets passed a pointer to u8[8] and is more efficient on 64 bit architectures thanether_addr_equalbecause the test can be done with a single compare and shift.The idea is not to access past the end of the ethernet address as appropriate (think pointer to eeprom or other such end-of-addressable memory conditions)
Quoted 6 lines
If a struct containing an ethernet address has additional members after the ethernet address, or the u8[6] addressa passed to
ether_addr_equalis not going to access past the end of memory or the structure, thenether_addr_equal_64bitsshould be used in lieu ofether_addr_equal.I believe this is correct, but maybe the
conditions for usingether_addr_equal_64bits
could be documented a bit better.Jeff/Intel folk? What do you think?
drivers/net/ethernet/intel/i40e/i40e_main.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)drivers/net/ethernet/intel/i40e/i40e_main.c +9
-6@@ -992,7 +992,7 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi,return NULL ;list_for_each_entry (f, &vsi->mac_filter_list, list) { -if ((ether_addr_equal (macaddr, f->macaddr)) && +if (ether_addr_equal_64bits (macaddr, f->macaddr) && (vlan == f->vlan) && (!is_vf || f->is_vf) && (!is_netdev || f->is_netdev))@@ -1020,7 +1020,7 @@ struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr,return NULL ;list_for_each_entry (f, &vsi->mac_filter_list, list) { -if ((ether_addr_equal (macaddr, f->macaddr)) && +if (ether_addr_equal_64bits (macaddr, f->macaddr) && (!is_vf || f->is_vf) && (!is_netdev || f->is_netdev))return f;@@ -1209,7 +1209,7 @@ static int i40e_set_mac(struct net_device *netdev, void *p)netdev_info (netdev,"set mac address=%pM\n" , addr->sa_data); -if (ether_addr_equal (netdev->dev_addr, addr->sa_data)) +if (ether_addr_equal_64bits (netdev->dev_addr, addr->sa_data))return 0 ;if (vsi->type ==I40E_VSI_MAIN ) {@@ -1409,21 +1409,24 @@ static void i40e_set_rx_mode(struct net_device *netdev)if (is_multicast_ether_addr (f->macaddr)) {netdev_for_each_mc_addr (mca, netdev) { -if (ether_addr_equal (mca->addr, f->macaddr)) { +if (ether_addr_equal_64bits (mca->addr, + f->macaddr)) { found = true;break ; } } }else {netdev_for_each_uc_addr (uca, netdev) { -if (ether_addr_equal (uca->addr, f->macaddr)) { +if (ether_addr_equal_64bits (uca->addr, + f->macaddr)) { found = true;break ; } }for_each_dev_addr (netdev, ha) { -if (ether_addr_equal (ha->addr, f->macaddr)) { +if (ether_addr_equal_64bits (ha->addr, + f->macaddr)) { found = true;break ; }
drivers/net/ethernet/intel/i40e/i40e_main.c int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) · line 1899
Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +2
-1@@ -1889,7 +1892,8 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)list_for_each_entry (f, &vsi->mac_filter_list, list) {if (is_netdev) {if (f->vlan && -ether_addr_equal (netdev->dev_addr, f->macaddr)) +ether_addr_equal_64bits (netdev->dev_addr, + f->macaddr)) filter_count++; }
------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
Quoted 15 lines
Hi Julia.
Maybe this explanation is helpful?
ethernet addresses are u8[6] (48 bits)
ether_addr_equal_64bitsgets passed a pointer to u8[8] and is more efficient on 64 bit architectures thanether_addr_equalbecause the test can be done with a single compare and shift.The idea is not to access past the end of the ethernet address as appropriate (think pointer to eeprom or other such end-of-addressable memory conditions)
Quoted 6 lines
If a struct containing an ethernet address has additional members after the ethernet address, or the u8[6] address passed to
ether_addr_equalis not going to access past the end of memory or the structure, thenether_addr_equal_64bitsshould be used in lieu ofether_addr_equal.
I tried the following semantic patch:
@r@
type T;
T *eth;
identifier fld;
type T1;
T1 *eth1;
identifier fld1;
expression e1;
position p;
@@
ether_addr_equal@p(eth->fld, eth1->fld1)
@ok@
type r.T, t1, t2;
identifier r.fld, fld2;
@@
T { ...
t1 fld[...];
t2 fld2;
...};
@ok1@
type r.T1, t1, t2;
identifier r.fld1, fld2;
@@
T1 { ...
t1 fld1[...];
t2 fld2;
...};
@depends on ok && ok1@
position r.p;
@@
*ether_addr_equal@p(...)
The first rule finds an existing call to ether_addr_equal where both arguments are structure field references. It gets the type of the structure in each case, and notes the field name. The next two rules check each of the structure declarations to ensure that the field is declared as an array and it is followed by at least one other field. The last rule generates some output for cases where both fields pass the test.
Note that this is not at all exhaustive, because it is not checking cases where one argument is a parameter that is passed from some call site where the argument is a field that has this property. Thus, it does not find the case in drivers/net/ethernet/intel/i40e/i40e_main.c. Nevertheless, it does find a few occurrences, as shown below. In this output, - indicates an item of interest, not something to be removed. It is not a patch, even though it looks like one.
One can get quite a lot more results if one doesn't require that both arguments satisfy the property, ie allowing one argument to be a function parameter rather than a structure field reference. So perhaps it would be worth making a (more complicated) semantic patch to find such cases.
julia
tmp/nothing/drivers/net/wireless/ipw2x00/libipw_rx.c renamed from var/linuxes/linux-next/drivers/net/wireless/ipw2x00/libipw_rx.c +0 -1
@@ -1468,7 +1468,6 @@ static inline int is_same_network(struct* as one network */ return ((src->ssid_len == dst->ssid_len) && (src->channel == dst->channel) && -ether_addr_equal (src->bssid, dst->bssid) && !memcmp (src->ssid, dst->ssid, src->ssid_len)); }
tmp/nothing/drivers/scsi/fcoe/fcoe_ctlr.c renamed from var/linuxes/linux-next/drivers/scsi/fcoe/fcoe_ctlr.c +0 -1
@@ -339,7 +339,6 @@ static void fcoe_ctlr_announce(struct fcspin_unlock_bh (&fip->ctlr_lock); sel = fip->sel_fcf; -if (sel &ðer_addr_equal (sel->fcf_mac, fip->dest_addr)) goto unlock;if (!is_zero_ether_addr (fip->dest_addr)) { printk(KERN_NOTICE "libfcoe: host%d: "
tmp/nothing/drivers/scsi/fcoe/fcoe_sysfs.c renamed from var/linuxes/linux-next/drivers/scsi/fcoe/fcoe_sysfs.c +0 -1
@@ -657,7 +657,6 @@ static int fcoe_fcf_device_match(structif (new->switch_name == old->switch_name && new->fabric_name == old->fabric_name && new->fc_map == old->fc_map && -ether_addr_equal (new->mac, old->mac)) return 1;return 0 ; }
tmp/nothing/drivers/staging/slicoss/slicoss.c renamed from var/linuxes/linux-next/drivers/staging/slicoss/slicoss.c +0 -2
@@ -787,8 +787,6 @@ static bool slic_mac_filter(struct adaptstruct mcast_address *mcaddr = adapter->mcastaddrs;while (mcaddr) { -if (ether_addr_equal (mcaddr->address, - ether_frame->ether_dhost)) { adapter->rcv_multicasts++; netdev->stats.multicast++;return true;
tmp/nothing/drivers/staging/vt6655/wpactl.c renamed from var/linuxes/linux-next/drivers/staging/vt6655/wpactl.c +0 -1
@@ -394,7 +394,6 @@ int wpa_set_keys(PSDevice pDevice, void }else {// Key Table Full -if (ether_addr_equal (param->addr, pDevice->abyBSSID)) {//DBG_PRN_WLAN03(("return NDIS_STATUS_INVALID_DATA -Key Table Full.2\n")); //spin_unlock_irq(&pDevice->lock); return -EINVAL ;
Quoted 15 lines
Hi Julia.
Maybe this explanation is helpful?
ethernet addresses are u8[6] (48 bits)
ether_addr_equal_64bitsgets passed a pointer to u8[8] and is more efficient on 64 bit architectures thanether_addr_equalbecause the test can be done with a single compare and shift.The idea is not to access past the end of the ethernet address as appropriate (think pointer to eeprom or other such end-of-addressable memory conditions)
Quoted 6 lines
If a struct containing an ethernet address has additional members after the ethernet address, or the u8[6] address passed to
ether_addr_equalis not going to access past the end of memory or the structure, thenether_addr_equal_64bitsshould be used in lieu ofether_addr_equal.
After some work on getting more include files to be taken into account, I get the following list of cases where both arguments have the form of a field reference, the field has array type, and the field is not at the end of the structure. I guess these are all candidates for ether_addr_equal_64bits (assuming that they correctly do satisfy these properties)?
Most of the uses of ether_addr_equal unfortunately take at least one pointer as an argument. If the pointer is a parameter one has to check all of the calls sites. If the function is static, this is perhaps not too bad, but from looking at some examples, it seems that this is often not the case.
There seem to also be quite a number of comparisons that use memcmp rather than ether_addr_equal. It seems that using ether_addr_equal would require knowing about alignment. In some cases, the structure field declaration explicitly mentions alignment. I'm not sure how to be sure that it holds otherwise. In particular, many of the relevant structures are declared as __packed. Perhaps that is also an issue for using ether_addr_equal_64bits?
julia
Quoted 1 line
From linux-next of 20131224.
file drivers/scsi/fcoe/fcoe_ctlr.c on line 1524
file drivers/scsi/fcoe/fcoe_ctlr.c on line 1528
file drivers/scsi/fcoe/fcoe_ctlr.c on line 342
file drivers/scsi/fcoe/fcoe_ctlr.c on line 1042
file drivers/scsi/fcoe/fcoe_sysfs.c on line 660
file drivers/scsi/fcoe/fcoe.c on line 1465
file drivers/net/wireless/rtlwifi/ps.c on line 926 file drivers/net/wireless/rtlwifi/ps.c on line 481 file drivers/net/wireless/rtlwifi/base.c on line 1296 file drivers/net/wireless/rtlwifi/base.c on line 1784 file drivers/net/wireless/p54/txrx.c on line 311
file drivers/net/wireless/rt2x00/rt2x00dev.c on line 568 file drivers/net/wireless/rt2x00/rt2x00dev.c on line 571 file drivers/net/wireless/ipw2x00/libipw_rx.c on line 1471 file drivers/net/wireless/ath/carl9170/rx.c on line 605 file drivers/net/wireless/ath/carl9170/rx.c on line 606 file drivers/net/wireless/ath/carl9170/rx.c on line 539 file drivers/net/wireless/ath/ath9k/htc_drv_txrx.c on line 1080 file drivers/net/wireless/ath/ath9k/recv.c on line 985 file drivers/net/wireless/ath/ath5k/base.c on line 1248 file drivers/net/wireless/ath/ath5k/base.c on line 1312 file drivers/net/wireless/iwlegacy/3945.c on line 472 file drivers/net/wireless/iwlegacy/3945.c on line 469 file drivers/net/wireless/iwlegacy/common.c on line 3749 file drivers/net/wireless/iwlegacy/common.c on line 3750 file drivers/net/wireless/iwlegacy/common.c on line 3751 file drivers/net/wireless/mwl8k.c on line 1261
file drivers/net/wireless/at76c50x-usb.c on line 1724 file drivers/net/wireless/iwlwifi/dvm/rxon.c on line 878 file drivers/net/wireless/iwlwifi/dvm/rxon.c on line 879 file drivers/net/wireless/iwlwifi/dvm/rxon.c on line 880 file drivers/net/wireless/iwlwifi/dvm/rx.c on line 783 file drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c on line 334 file drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c on line 580 file drivers/net/vxlan.c on line 1371
file drivers/staging/slicoss/slicoss.c on line 790 file drivers/staging/vt6656/wpactl.c on line 230
file drivers/staging/vt6656/dpc.c on line 357
file drivers/staging/vt6656/dpc.c on line 369
file net/bluetooth/bnep/core.c on line 407
file net/bluetooth/bnep/core.c on line 410
file net/wireless/scan.c on line 636
file net/wireless/mlme.c on line 561
file net/wireless/mlme.c on line 575
file net/wireless/mlme.c on line 588
file net/hsr/hsr_framereg.c on line 88
file net/hsr/hsr_framereg.c on line 187
file net/hsr/hsr_framereg.c on line 90
file net/hsr/hsr_framereg.c on line 178
file net/802/stp.c on line 49
file net/mac80211/tx.c on line 1693
file net/mac80211/mesh.c on line 1055
file net/mac80211/mesh.c on line 1005
file net/mac80211/scan.c on line 187
file net/mac80211/scan.c on line 188
file net/mac80211/mlme.c on line 2584
file net/mac80211/mlme.c on line 4036
file net/mac80211/mlme.c on line 3827
file net/mac80211/mlme.c on line 2784
file net/mac80211/mlme.c on line 2814
file net/mac80211/mlme.c on line 2217
file net/mac80211/mlme.c on line 2245
file net/mac80211/mlme.c on line 2708
file net/mac80211/mlme.c on line 2712
file net/mac80211/mlme.c on line 2695
file net/mac80211/iface.c on line 288
file net/mac80211/rx.c on line 1910
file net/mac80211/rx.c on line 1641
file net/mac80211/rx.c on line 1642
file net/mac80211/rx.c on line 3104
file net/mac80211/rx.c on line 2341
file net/mac80211/rx.c on line 2521
file net/mac80211/rx.c on line 2340
file net/mac80211/rx.c on line 2335
file net/mac80211/rx.c on line 3093
file net/mac80211/rx.c on line 3111
file net/mac80211/rx.c on line 3127
file net/mac80211/rx.c on line 3137
file net/mac80211/rx.c on line 3147
file net/mac80211/rx.c on line 3168
file net/mac80211/rx.c on line 3103
file net/mac80211/rx.c on line 2146
file net/mac80211/sta_info.c on line 416
file net/mac80211/status.c on line 595
file net/mac80211/ibss.c on line 1126
file net/mac80211/ibss.c on line 1018
file net/mac80211/ibss.c on line 1437
Quoted 6 lines · Julia Lawall
After some work on getting more include files to be taken into account, I get the following list of cases where both arguments have the form of a field reference, the field has array type, and the field is not at the end of the structure. I guess these are all candidates for
ether_addr_equal_64bits(assuming that they correctly do satisfy these properties)?
I believe yes.
Quoted 2 lines
There seem to also be quite a number of comparisons that use memcmp rather than
ether_addr_equal.
Ding's patch series converts all the memcmp(e1, e1, \(ETH_ALEN\|6\)) via spatch to either ether_addr_equal or ether_addr_equal_unaligned.
I believe he's inspecting the sites to determine if the conversion should use the _unaligned version.
Quoted 6 lines
It seems that using
ether_addr_equalwould require
knowing about alignment. In some cases, the structure field declaration explicitly mentions alignment. I'm not sure how to be sure that it holds otherwise. In particular, many of the relevant structures are declared as__packed. Perhaps that is also an issue for usingether_addr_equal_64bits?
No, not really, the packing really doesn't matter, just whether or not there are 2 more addressable bytes after the u8 array[6].
Thanks for the list Julia.
Quoted 89 lines
From linux-next of 20131224.
file
drivers/scsi/fcoe/fcoe_ctlr.con line 1524
filedrivers/scsi/fcoe/fcoe_ctlr.con line 1528
filedrivers/scsi/fcoe/fcoe_ctlr.con line 342
filedrivers/scsi/fcoe/fcoe_ctlr.con line 1042
filedrivers/scsi/fcoe/fcoe_sysfs.con line 660
filedrivers/scsi/fcoe/fcoe.con line 1465
filedrivers/net/wireless/rtlwifi/ps.con line 926 filedrivers/net/wireless/rtlwifi/ps.con line 481 filedrivers/net/wireless/rtlwifi/base.con line 1296 filedrivers/net/wireless/rtlwifi/base.con line 1784 filedrivers/net/wireless/p54/txrx.con line 311
filedrivers/net/wireless/rt2x00/rt2x00dev.con line 568 filedrivers/net/wireless/rt2x00/rt2x00dev.con line 571 filedrivers/net/wireless/ipw2x00/libipw_rx.con line 1471 filedrivers/net/wireless/ath/carl9170/rx.con line 605 filedrivers/net/wireless/ath/carl9170/rx.con line 606 filedrivers/net/wireless/ath/carl9170/rx.con line 539 filedrivers/net/wireless/ath/ath9k/htc_drv_txrx.con line 1080 filedrivers/net/wireless/ath/ath9k/recv.con line 985 filedrivers/net/wireless/ath/ath5k/base.con line 1248 filedrivers/net/wireless/ath/ath5k/base.con line 1312 filedrivers/net/wireless/iwlegacy/3945.con line 472 filedrivers/net/wireless/iwlegacy/3945.con line 469 filedrivers/net/wireless/iwlegacy/common.con line 3749 filedrivers/net/wireless/iwlegacy/common.con line 3750 filedrivers/net/wireless/iwlegacy/common.con line 3751 filedrivers/net/wireless/mwl8k.con line 1261
filedrivers/net/wireless/at76c50x-usb.con line 1724 filedrivers/net/wireless/iwlwifi/dvm/rxon.con line 878 filedrivers/net/wireless/iwlwifi/dvm/rxon.con line 879 filedrivers/net/wireless/iwlwifi/dvm/rxon.con line 880 filedrivers/net/wireless/iwlwifi/dvm/rx.con line 783 filedrivers/net/ethernet/qlogic/qlcnic/qlcnic_io.con line 334 filedrivers/net/ethernet/qlogic/qlcnic/qlcnic_io.con line 580 filedrivers/net/vxlan.con line 1371
filedrivers/staging/slicoss/slicoss.con line 790 filedrivers/staging/vt6656/wpactl.con line 230
filedrivers/staging/vt6656/dpc.con line 357
filedrivers/staging/vt6656/dpc.con line 369
filenet/bluetooth/bnep/core.con line 407
filenet/bluetooth/bnep/core.con line 410
filenet/wireless/scan.con line 636
filenet/wireless/mlme.con line 561
filenet/wireless/mlme.con line 575
filenet/wireless/mlme.con line 588
filenet/hsr/hsr_framereg.con line 88
filenet/hsr/hsr_framereg.con line 187
filenet/hsr/hsr_framereg.con line 90
filenet/hsr/hsr_framereg.con line 178
filenet/802/stp.con line 49
filenet/mac80211/tx.con line 1693
filenet/mac80211/mesh.con line 1055
filenet/mac80211/mesh.con line 1005
filenet/mac80211/scan.con line 187
filenet/mac80211/scan.con line 188
filenet/mac80211/mlme.con line 2584
filenet/mac80211/mlme.con line 4036
filenet/mac80211/mlme.con line 3827
filenet/mac80211/mlme.con line 2784
filenet/mac80211/mlme.con line 2814
filenet/mac80211/mlme.con line 2217
filenet/mac80211/mlme.con line 2245
filenet/mac80211/mlme.con line 2708
filenet/mac80211/mlme.con line 2712
filenet/mac80211/mlme.con line 2695
filenet/mac80211/iface.con line 288
filenet/mac80211/rx.con line 1910
filenet/mac80211/rx.con line 1641
filenet/mac80211/rx.con line 1642
filenet/mac80211/rx.con line 3104
filenet/mac80211/rx.con line 2341
filenet/mac80211/rx.con line 2521
filenet/mac80211/rx.con line 2340
filenet/mac80211/rx.con line 2335
filenet/mac80211/rx.con line 3093
filenet/mac80211/rx.con line 3111
filenet/mac80211/rx.con line 3127
filenet/mac80211/rx.con line 3137
filenet/mac80211/rx.con line 3147
filenet/mac80211/rx.con line 3168
filenet/mac80211/rx.con line 3103
filenet/mac80211/rx.con line 2146
filenet/mac80211/sta_info.con line 416
filenet/mac80211/status.con line 595
filenet/mac80211/ibss.con line 1126
filenet/mac80211/ibss.con line 1018
filenet/mac80211/ibss.con line 1437
Quoted 35 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.
Quoted 10 lines
Perhaps it's a possible spatch/coccinelle conversion,
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Maybe Julia does. (cc'd)
As the below patch said, that a lot of ether_addr_equal could be instead of ether_addr_equal_64bits, and I need to review them and resend.
Regards
Ding
Quoted 1 line
Quoted 43 lines · Ding Tianhong
Quoted 42 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.Perhaps it's a possible spatch/coccinelle conversion,
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Quoted 6 lines
Quoted 3 lines
Maybe Julia does. (cc'd)
As the below patch said, that a lot of
ether_addr_equalcould be instead ofether_addr_equal_64bits, and I need to review them and resend.
I don't think so.
I think what you've done so far is fine.
Any conversions to ether_addr_equal_64bit
can be done later.
Quoted 51 lines · Joe Perches
Quoted 43 lines · Ding Tianhong
Quoted 42 lines · Joe Perches
Quoted 3 lines · Eric Dumazet
Quoted 2 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.[]
Quoted 1 line
drivers/net/plip/plip.c
Quoted 1 line
drivers/net/plip/plip.c +0
-0[]
Quoted 12 lines
drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 9 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 6 lines
if (is_multicast_ether_addr(eth->h_dest)) { if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast)) skb->pkt_type = PACKET_BROADCAST; else skb->pkt_type = PACKET_MULTICAST; }That is better though I wonder how many systems are still using laplink via parallel null-printer cables.
No matter, better is better.
The same optimization using
ether_addr_equal_64bitsmay be possible to do in other places given other structs too.Perhaps it's a possible spatch/coccinelle conversion,
I don't know spatch well enough to know if a mechanism to check if structure members have other fields that follow them in the structure or if the structure member is an array of a minimum size.
Quoted 6 lines
Quoted 3 lines
Maybe Julia does. (cc'd)
As the below patch said, that a lot of
ether_addr_equalcould be instead ofether_addr_equal_64bits, and I need to review them and resend.
Quoted 7 lines
I don't think so.
I think what you've done so far is fine.
Any conversions to
ether_addr_equal_64bit
can be done later.
Ok, thanks.
Regards
Ding
Quoted 3 lines
Quoted 26 lines · Eric Dumazet
Quoted 14 lines · Ding Tianhong
Use possibly more efficient
ether_addr_equal
to instead of memcmp.Cc "David S. Miller" <[email protected]>Signed-off-by Ding Tianhong <[email protected]>--- drivers/net/plip/plip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/plip/plip.c +0
-0drivers/net/plip/plip.c static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev) · line 555
Quoted 8 lines
drivers/net/plip/plip.c +1
-1@@ -549,7 +549,7 @@ static __be16 plip_type_trans(struct sk_buff *skb, struct net_device *dev)if (*eth->h_dest&1 ) { -if (memcmp (eth->h_dest,dev->broadcast,ETH_ALEN )==0 ) +if (ether_addr_equal (eth->h_dest, dev->broadcast)) skb->pkt_type=PACKET_BROADCAST ;else skb->pkt_type=PACKET_MULTICAST ;What about :
Quoted 9 lines
if (is_multicast_ether_addr(eth->h_dest)) {
if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast))
skb->pkt_type = PACKET_BROADCAST;
else
skb->pkt_type = PACKET_MULTICAST;
}more better, thanks.
Regards
Ding