* [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal
@ 2014-03-03 1:14 Ding Tianhong
2014-03-03 1:37 ` Joe Perches
2014-03-03 1:49 ` Joe Perches
0 siblings, 2 replies; 6+ messages in thread
From: Ding Tianhong @ 2014-03-03 1:14 UTC (permalink / raw)
To: Patrick McHardy, David S. Miller, Joe Perches, Julia Lawall, Netdev
Ether_addr_equal_64bits is more efficient than ether_addr_equal, and
can be used when each argument is an array within a structure that
contains at least two bytes of data beyond the array, so it is safe
to use it for vlan.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
net/8021q/vlan.c | 10 +++++-----
net/8021q/vlan_core.c | 2 +-
net/8021q/vlan_dev.c | 10 +++++-----
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index ec99099..16fb0f4 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -286,19 +286,19 @@ static void vlan_sync_address(struct net_device *dev,
struct vlan_dev_priv *vlan = vlan_dev_priv(vlandev);
/* May be called without an actual change */
- if (ether_addr_equal(vlan->real_dev_addr, dev->dev_addr))
+ if (ether_addr_equal_64bits(vlan->real_dev_addr, dev->dev_addr))
return;
/* vlan address was different from the old address and is equal to
* the new address */
- if (!ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
- ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
+ if (!ether_addr_equal_64bits(vlandev->dev_addr, vlan->real_dev_addr) &&
+ ether_addr_equal_64bits(vlandev->dev_addr, dev->dev_addr))
dev_uc_del(dev, vlandev->dev_addr);
/* vlan address was equal to the old address and is different from
* the new address */
- if (ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
- !ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
+ if (ether_addr_equal_64bits(vlandev->dev_addr, vlan->real_dev_addr) &&
+ !ether_addr_equal_64bits(vlandev->dev_addr, dev->dev_addr))
dev_uc_add(dev, vlandev->dev_addr);
ether_addr_copy(vlan->real_dev_addr, dev->dev_addr);
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 6ee48aa..39cbb46 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -26,7 +26,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
/* Our lower layer thinks this is not local, let's make sure.
* This allows the VLAN to have a different MAC than the
* underlying device, and still route correctly. */
- if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
+ if (ether_addr_equal_64bits(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
skb->pkt_type = PACKET_HOST;
}
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 76d8fab..5e1edd6 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -286,7 +286,7 @@ static int vlan_dev_open(struct net_device *dev)
!(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
return -ENETDOWN;
- if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr)) {
+ if (!ether_addr_equal_64bits(dev->dev_addr, real_dev->dev_addr)) {
err = dev_uc_add(real_dev, dev->dev_addr);
if (err < 0)
goto out;
@@ -319,7 +319,7 @@ clear_allmulti:
if (dev->flags & IFF_ALLMULTI)
dev_set_allmulti(real_dev, -1);
del_unicast:
- if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
+ if (!ether_addr_equal_64bits(dev->dev_addr, real_dev->dev_addr))
dev_uc_del(real_dev, dev->dev_addr);
out:
netif_carrier_off(dev);
@@ -338,7 +338,7 @@ static int vlan_dev_stop(struct net_device *dev)
if (dev->flags & IFF_PROMISC)
dev_set_promiscuity(real_dev, -1);
- if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
+ if (!ether_addr_equal_64bits(dev->dev_addr, real_dev->dev_addr))
dev_uc_del(real_dev, dev->dev_addr);
netif_carrier_off(dev);
@@ -357,13 +357,13 @@ static int vlan_dev_set_mac_address(struct net_device *dev, void *p)
if (!(dev->flags & IFF_UP))
goto out;
- if (!ether_addr_equal(addr->sa_data, real_dev->dev_addr)) {
+ if (!ether_addr_equal_64bits(addr->sa_data, real_dev->dev_addr)) {
err = dev_uc_add(real_dev, addr->sa_data);
if (err < 0)
return err;
}
- if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
+ if (!ether_addr_equal_64bits(dev->dev_addr, real_dev->dev_addr))
dev_uc_del(real_dev, dev->dev_addr);
out:
--
1.8.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal
2014-03-03 1:14 [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal Ding Tianhong
@ 2014-03-03 1:37 ` Joe Perches
2014-03-03 1:49 ` Joe Perches
1 sibling, 0 replies; 6+ messages in thread
From: Joe Perches @ 2014-03-03 1:37 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Patrick McHardy, David S. Miller, Julia Lawall, Netdev
On Mon, 2014-03-03 at 09:14 +0800, Ding Tianhong wrote:
> Ether_addr_equal_64bits is more efficient than ether_addr_equal, and
> can be used when each argument is an array within a structure that
> contains at least two bytes of data beyond the array, so it is safe
> to use it for vlan.
It's true that ether_addr_equal_64bits is faster for
64 bit architectures. It's 1 64 bit compare and 1
48 bit mask instead of a 32 bit and a 16 bit compare.
I think unless the code is _extremely_ performance or
fast-path sensitive, ether_addr_equal should be used.
I believe the vlan_do_receive use qualifies.
Are any of the others on the fast path?
Do you have any performance numbers?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal
2014-03-03 1:14 [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal Ding Tianhong
2014-03-03 1:37 ` Joe Perches
@ 2014-03-03 1:49 ` Joe Perches
2014-03-03 2:49 ` Ding Tianhong
1 sibling, 1 reply; 6+ messages in thread
From: Joe Perches @ 2014-03-03 1:49 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Patrick McHardy, David S. Miller, Julia Lawall, Netdev
On Mon, 2014-03-03 at 09:14 +0800, Ding Tianhong wrote:
> Ether_addr_equal_64bits is more efficient than ether_addr_equal, and
> can be used when each argument is an array within a structure that
> contains at least two bytes of data beyond the array, so it is safe
> to use it for vlan.
[]
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
[]
> @@ -26,7 +26,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
> /* Our lower layer thinks this is not local, let's make sure.
> * This allows the VLAN to have a different MAC than the
> * underlying device, and still route correctly. */
> - if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
> + if (ether_addr_equal_64bits(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
> skb->pkt_type = PACKET_HOST;
> }
Hi again Ding
If you do have performance numbers:
The lines above this ether_addr_equal_64 are:
if (skb->pkt_type == PACKET_OTHERHOST) {
/* Our lower layer thinks this is not local, let's make sure.
* This allows the VLAN to have a different MAC than the
* underlying device, and still route correctly. */
if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
skb->pkt_type = PACKET_HOST;
}
Maybe it'd be faster overall to add an unlikely
to the == test
if (unlikely(skb->pkt_type == PACKET_OTHERHOST)) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal
2014-03-03 1:49 ` Joe Perches
@ 2014-03-03 2:49 ` Ding Tianhong
2014-03-03 4:36 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Ding Tianhong @ 2014-03-03 2:49 UTC (permalink / raw)
To: Joe Perches; +Cc: Patrick McHardy, David S. Miller, Julia Lawall, Netdev
On 2014/3/3 9:49, Joe Perches wrote:
> On Mon, 2014-03-03 at 09:14 +0800, Ding Tianhong wrote:
>> Ether_addr_equal_64bits is more efficient than ether_addr_equal, and
>> can be used when each argument is an array within a structure that
>> contains at least two bytes of data beyond the array, so it is safe
>> to use it for vlan.
> []
>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> []
>> @@ -26,7 +26,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
>> /* Our lower layer thinks this is not local, let's make sure.
>> * This allows the VLAN to have a different MAC than the
>> * underlying device, and still route correctly. */
>> - if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
>> + if (ether_addr_equal_64bits(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
>> skb->pkt_type = PACKET_HOST;
>> }
>
> Hi again Ding
>
> If you do have performance numbers:
>
> The lines above this ether_addr_equal_64 are:
>
> if (skb->pkt_type == PACKET_OTHERHOST) {
> /* Our lower layer thinks this is not local, let's make sure.
> * This allows the VLAN to have a different MAC than the
> * underlying device, and still route correctly. */
> if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
> skb->pkt_type = PACKET_HOST;
> }
>
> Maybe it'd be faster overall to add an unlikely
> to the == test
>
> if (unlikely(skb->pkt_type == PACKET_OTHERHOST)) {
>
>
>
Sorry, I can't understand it clearly, do you mean that if the skb deliver to the vlan dev,
it is impossible that the pkt_type is PACKET_OTHERHOST at most time?
Regards
Ding
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal
2014-03-03 2:49 ` Ding Tianhong
@ 2014-03-03 4:36 ` Joe Perches
2014-03-03 6:05 ` Ding Tianhong
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2014-03-03 4:36 UTC (permalink / raw)
To: Ding Tianhong
Cc: Patrick McHardy, David S. Miller, Julia Lawall, Netdev, Jiri Pirko
(adding Jiri Pirko to cc's)
On Mon, 2014-03-03 at 10:49 +0800, Ding Tianhong wrote:
> On 2014/3/3 9:49, Joe Perches wrote:
> > On Mon, 2014-03-03 at 09:14 +0800, Ding Tianhong wrote:
> >> Ether_addr_equal_64bits is more efficient than ether_addr_equal, and
> >> can be used when each argument is an array within a structure that
> >> contains at least two bytes of data beyond the array, so it is safe
> >> to use it for vlan.
> > []
> >> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> > []
> >> @@ -26,7 +26,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
> >> /* Our lower layer thinks this is not local, let's make sure.
> >> * This allows the VLAN to have a different MAC than the
> >> * underlying device, and still route correctly. */
> >> - if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
> >> + if (ether_addr_equal_64bits(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
> >> skb->pkt_type = PACKET_HOST;
> >> }
> >
> > Hi again Ding
> >
> > If you do have performance numbers:
> >
> > The lines above this ether_addr_equal_64 are:
> >
> > if (skb->pkt_type == PACKET_OTHERHOST) {
> > /* Our lower layer thinks this is not local, let's make sure.
> > * This allows the VLAN to have a different MAC than the
> > * underlying device, and still route correctly. */
> > if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
> > skb->pkt_type = PACKET_HOST;
> > }
> >
> > Maybe it'd be faster overall to add an unlikely
> > to the == test
> >
> > if (unlikely(skb->pkt_type == PACKET_OTHERHOST)) {
>
> Sorry, I can't understand it clearly, do you mean that if the skb deliver to the vlan dev,
> it is impossible that the pkt_type is PACKET_OTHERHOST at most time?
No, just that it's maybe better to add an unlikely
to the test.
Jiri Pirko's commit 0b5c9db1b modified the code
from a switch/case to a specific test.
I don't know how likely it is in normal uses for
a packet to be delivered as PACKET_OTHERHOST.
I just hoped that if you in fact had performance
numbers for your suggested ether_addr_equal_64bits
test, you might be able to run those same tests
with if (unlikely(skb->pkt_type == PACKET_OTHERHOST))
and see if that was better or worse for performance.
cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal
2014-03-03 4:36 ` Joe Perches
@ 2014-03-03 6:05 ` Ding Tianhong
0 siblings, 0 replies; 6+ messages in thread
From: Ding Tianhong @ 2014-03-03 6:05 UTC (permalink / raw)
To: Joe Perches
Cc: Patrick McHardy, David S. Miller, Julia Lawall, Netdev, Jiri Pirko
On 2014/3/3 12:36, Joe Perches wrote:
> (adding Jiri Pirko to cc's)
>
> On Mon, 2014-03-03 at 10:49 +0800, Ding Tianhong wrote:
>> On 2014/3/3 9:49, Joe Perches wrote:
>>> On Mon, 2014-03-03 at 09:14 +0800, Ding Tianhong wrote:
>>>> Ether_addr_equal_64bits is more efficient than ether_addr_equal, and
>>>> can be used when each argument is an array within a structure that
>>>> contains at least two bytes of data beyond the array, so it is safe
>>>> to use it for vlan.
>>> []
>>>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>>> []
>>>> @@ -26,7 +26,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
>>>> /* Our lower layer thinks this is not local, let's make sure.
>>>> * This allows the VLAN to have a different MAC than the
>>>> * underlying device, and still route correctly. */
>>>> - if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
>>>> + if (ether_addr_equal_64bits(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
>>>> skb->pkt_type = PACKET_HOST;
>>>> }
>>>
>>> Hi again Ding
>>>
>>> If you do have performance numbers:
>>>
>>> The lines above this ether_addr_equal_64 are:
>>>
>>> if (skb->pkt_type == PACKET_OTHERHOST) {
>>> /* Our lower layer thinks this is not local, let's make sure.
>>> * This allows the VLAN to have a different MAC than the
>>> * underlying device, and still route correctly. */
>>> if (ether_addr_equal(eth_hdr(skb)->h_dest, vlan_dev->dev_addr))
>>> skb->pkt_type = PACKET_HOST;
>>> }
>>>
>>> Maybe it'd be faster overall to add an unlikely
>>> to the == test
>>>
>>> if (unlikely(skb->pkt_type == PACKET_OTHERHOST)) {
>>
>> Sorry, I can't understand it clearly, do you mean that if the skb deliver to the vlan dev,
>> it is impossible that the pkt_type is PACKET_OTHERHOST at most time?
>
> No, just that it's maybe better to add an unlikely
> to the test.
>
> Jiri Pirko's commit 0b5c9db1b modified the code
> from a switch/case to a specific test.
>
> I don't know how likely it is in normal uses for
> a packet to be delivered as PACKET_OTHERHOST.
>
> I just hoped that if you in fact had performance
> numbers for your suggested ether_addr_equal_64bits
> test, you might be able to run those same tests
> with if (unlikely(skb->pkt_type == PACKET_OTHERHOST))
> and see if that was better or worse for performance.
>
> cheers, Joe
>
OK, I got it and I will review the commit 0b5c9db1b, try to
see the performance.
Regards
Ding
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-03 6:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-03 1:14 [PATCH net-next] vlan: use ether_addr_equal_64bits to instead of ether_addr_equal Ding Tianhong
2014-03-03 1:37 ` Joe Perches
2014-03-03 1:49 ` Joe Perches
2014-03-03 2:49 ` Ding Tianhong
2014-03-03 4:36 ` Joe Perches
2014-03-03 6:05 ` Ding Tianhong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.