* [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
@ 2013-12-28 6:17 Ding Tianhong
2013-12-28 13:58 ` Sergei Shtylyov
0 siblings, 1 reply; 8+ messages in thread
From: Ding Tianhong @ 2013-12-28 6:17 UTC (permalink / raw)
To: David S. Miller, Netdev, linux-kernel@vger.kernel.org
Use possibly more efficient ether_addr_equal
to instead of memcmp.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/ethernet/packetengines/yellowfin.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
index d28593b..b83ac0e 100644
--- a/drivers/net/ethernet/packetengines/yellowfin.c
+++ b/drivers/net/ethernet/packetengines/yellowfin.c
@@ -1097,12 +1097,12 @@ static int yellowfin_rx(struct net_device *dev)
if (status2 & 0x80) dev->stats.rx_dropped++;
#ifdef YF_PROTOTYPE /* Support for prototype hardware errata. */
} else if ((yp->flags & HasMACAddrBug) &&
- memcmp(le32_to_cpu(yp->rx_ring_dma +
- entry*sizeof(struct yellowfin_desc)),
- dev->dev_addr, 6) != 0 &&
- memcmp(le32_to_cpu(yp->rx_ring_dma +
- entry*sizeof(struct yellowfin_desc)),
- "\377\377\377\377\377\377", 6) != 0) {
+ !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
+ entry * sizeof(struct yellowfin_desc)),
+ dev->dev_addr) &&
+ !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
+ entry * sizeof(struct yellowfin_desc)),
+ "\377\377\377\377\377\377")) {
if (bogus_rx++ == 0)
netdev_warn(dev, "Bad frame to %pM\n",
buf_addr);
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-28 6:17 [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr Ding Tianhong
@ 2013-12-28 13:58 ` Sergei Shtylyov
2013-12-28 15:18 ` Ding Tianhong
0 siblings, 1 reply; 8+ messages in thread
From: Sergei Shtylyov @ 2013-12-28 13:58 UTC (permalink / raw)
To: Ding Tianhong, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
Hello.
On 28-12-2013 10:17, Ding Tianhong wrote:
> Use possibly more efficient ether_addr_equal
> to instead of memcmp.
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
> drivers/net/ethernet/packetengines/yellowfin.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
> diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
> index d28593b..b83ac0e 100644
> --- a/drivers/net/ethernet/packetengines/yellowfin.c
> +++ b/drivers/net/ethernet/packetengines/yellowfin.c
> @@ -1097,12 +1097,12 @@ static int yellowfin_rx(struct net_device *dev)
> if (status2 & 0x80) dev->stats.rx_dropped++;
> #ifdef YF_PROTOTYPE /* Support for prototype hardware errata. */
> } else if ((yp->flags & HasMACAddrBug) &&
> - memcmp(le32_to_cpu(yp->rx_ring_dma +
> - entry*sizeof(struct yellowfin_desc)),
> - dev->dev_addr, 6) != 0 &&
> - memcmp(le32_to_cpu(yp->rx_ring_dma +
> - entry*sizeof(struct yellowfin_desc)),
> - "\377\377\377\377\377\377", 6) != 0) {
> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> + entry * sizeof(struct yellowfin_desc)),
> + dev->dev_addr) &&
Previous line was aligned correctly, the above line should start under
le32_to_cpu.
> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> + entry * sizeof(struct yellowfin_desc)),
Start the continuation lines under 'yp', please.
> + "\377\377\377\377\377\377")) {
This line should start under le32_to_cpu.
WBR, Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-28 13:58 ` Sergei Shtylyov
@ 2013-12-28 15:18 ` Ding Tianhong
2013-12-28 17:23 ` Joe Perches
2013-12-28 23:25 ` Sergei Shtylyov
0 siblings, 2 replies; 8+ messages in thread
From: Ding Tianhong @ 2013-12-28 15:18 UTC (permalink / raw)
To: Sergei Shtylyov, Ding Tianhong, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
于 2013/12/28 21:58, Sergei Shtylyov 写道:
> Hello.
>
> On 28-12-2013 10:17, Ding Tianhong wrote:
>
>> Use possibly more efficient ether_addr_equal
>> to instead of memcmp.
>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/ethernet/packetengines/yellowfin.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>
>> diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
>> index d28593b..b83ac0e 100644
>> --- a/drivers/net/ethernet/packetengines/yellowfin.c
>> +++ b/drivers/net/ethernet/packetengines/yellowfin.c
>> @@ -1097,12 +1097,12 @@ static int yellowfin_rx(struct net_device *dev)
>> if (status2 & 0x80) dev->stats.rx_dropped++;
>> #ifdef YF_PROTOTYPE /* Support for prototype hardware errata. */
>> } else if ((yp->flags & HasMACAddrBug) &&
>> - memcmp(le32_to_cpu(yp->rx_ring_dma +
>> - entry*sizeof(struct yellowfin_desc)),
>> - dev->dev_addr, 6) != 0 &&
>> - memcmp(le32_to_cpu(yp->rx_ring_dma +
>> - entry*sizeof(struct yellowfin_desc)),
>> - "\377\377\377\377\377\377", 6) != 0) {
>> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>> + entry * sizeof(struct yellowfin_desc)),
>> + dev->dev_addr) &&
>
> Previous line was aligned correctly, the above line should start under le32_to_cpu.
>
>> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>> + entry * sizeof(struct yellowfin_desc)),
>
> Start the continuation lines under 'yp', please.
>
>> + "\377\377\377\377\377\377")) {
>
> This line should start under le32_to_cpu.
>
> WBR, Sergei
>
Hi sergei:
you mean this way?
!ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
entry * sizeof(struct yellowfin_desc)),
dev->dev_addr) &&
!ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
entry * sizeof(struct yellowfin_desc)),
"\377\377\377\377\377\377")) {
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] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-28 15:18 ` Ding Tianhong
@ 2013-12-28 17:23 ` Joe Perches
2013-12-30 2:39 ` Ding Tianhong
2013-12-28 23:25 ` Sergei Shtylyov
1 sibling, 1 reply; 8+ messages in thread
From: Joe Perches @ 2013-12-28 17:23 UTC (permalink / raw)
To: Ding Tianhong
Cc: Sergei Shtylyov, Ding Tianhong, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
On Sat, 2013-12-28 at 23:18 +0800, Ding Tianhong wrote:
> 于 2013/12/28 21:58, Sergei Shtylyov 写道:
> > Hello.
> >
> > On 28-12-2013 10:17, Ding Tianhong wrote:
> >
> >> Use possibly more efficient ether_addr_equal
> >> to instead of memcmp.
> >
> >> Cc: "David S. Miller" <davem@davemloft.net>
> >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> >> ---
> >> drivers/net/ethernet/packetengines/yellowfin.c | 12 ++++++------
> >> 1 files changed, 6 insertions(+), 6 deletions(-)
> >
> >> diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
> >> index d28593b..b83ac0e 100644
> >> --- a/drivers/net/ethernet/packetengines/yellowfin.c
> >> +++ b/drivers/net/ethernet/packetengines/yellowfin.c
> >> @@ -1097,12 +1097,12 @@ static int yellowfin_rx(struct net_device *dev)
> >> if (status2 & 0x80) dev->stats.rx_dropped++;
> >> #ifdef YF_PROTOTYPE /* Support for prototype hardware errata. */
> >> } else if ((yp->flags & HasMACAddrBug) &&
> >> - memcmp(le32_to_cpu(yp->rx_ring_dma +
> >> - entry*sizeof(struct yellowfin_desc)),
> >> - dev->dev_addr, 6) != 0 &&
> >> - memcmp(le32_to_cpu(yp->rx_ring_dma +
> >> - entry*sizeof(struct yellowfin_desc)),
> >> - "\377\377\377\377\377\377", 6) != 0) {
> >> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> >> + entry * sizeof(struct yellowfin_desc)),
> >> + dev->dev_addr) &&
> >
> > Previous line was aligned correctly, the above line should start under le32_to_cpu.
> >
> >> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> >> + entry * sizeof(struct yellowfin_desc)),
> >
> > Start the continuation lines under 'yp', please.
> >
> >> + "\377\377\377\377\377\377")) {
> >
> > This line should start under le32_to_cpu.
> >
> > WBR, Sergei
> >
>
> Hi sergei:
> you mean this way?
> !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> entry * sizeof(struct yellowfin_desc)),
> dev->dev_addr) &&
>
> !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> entry * sizeof(struct yellowfin_desc)),
> "\377\377\377\377\377\377")) {
Does this really matter?
Does anyone have a packetengine NIC anymore?
\377 octal is 0xff, so this is matching a broadcast address.
is_broadcast_ether_addr(addr) would be appropriate.
So would using a temporary address.
u8 *addr = (u8 *)(unsigned long)le32_to_cpu(etc)
but the whole thing looks very suspect as an le32
value could not be added to correctly on a
big-endian arch anyway.
My guess is this was tested only on an x86 and
it should be:
u8 *addr = (u8 *)(unsigned long)(le32_to_cpu(yp->rx_ring_dma) +
entry * sizeof(struct yellowfin_desc));
It maybe better just to leave these two alone.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-28 15:18 ` Ding Tianhong
2013-12-28 17:23 ` Joe Perches
@ 2013-12-28 23:25 ` Sergei Shtylyov
1 sibling, 0 replies; 8+ messages in thread
From: Sergei Shtylyov @ 2013-12-28 23:25 UTC (permalink / raw)
To: Ding Tianhong, Ding Tianhong, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
Hello.
On 12/28/2013 06:18 PM, Ding Tianhong wrote:
>>> Use possibly more efficient ether_addr_equal
>>> to instead of memcmp.
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>>> ---
>>> drivers/net/ethernet/packetengines/yellowfin.c | 12 ++++++------
>>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>> diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
>>> index d28593b..b83ac0e 100644
>>> --- a/drivers/net/ethernet/packetengines/yellowfin.c
>>> +++ b/drivers/net/ethernet/packetengines/yellowfin.c
>>> @@ -1097,12 +1097,12 @@ static int yellowfin_rx(struct net_device *dev)
>>> if (status2 & 0x80) dev->stats.rx_dropped++;
>>> #ifdef YF_PROTOTYPE /* Support for prototype hardware errata. */
>>> } else if ((yp->flags & HasMACAddrBug) &&
>>> - memcmp(le32_to_cpu(yp->rx_ring_dma +
>>> - entry*sizeof(struct yellowfin_desc)),
>>> - dev->dev_addr, 6) != 0 &&
>>> - memcmp(le32_to_cpu(yp->rx_ring_dma +
>>> - entry*sizeof(struct yellowfin_desc)),
>>> - "\377\377\377\377\377\377", 6) != 0) {
>>> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>>> + entry * sizeof(struct yellowfin_desc)),
>>> + dev->dev_addr) &&
>> Previous line was aligned correctly, the above line should start under le32_to_cpu.
>>> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>>> + entry * sizeof(struct yellowfin_desc)),
>> Start the continuation lines under 'yp', please.
>>> + "\377\377\377\377\377\377")) {
>> This line should start under le32_to_cpu.
>> WBR, Sergei
> Hi sergei:
> you mean this way?
> !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> entry * sizeof(struct yellowfin_desc)),
> dev->dev_addr) &&
>
> !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
> entry * sizeof(struct yellowfin_desc)),
> "\377\377\377\377\377\377")) {
Yes, exactly.
> Regards
> Ding
WBR, Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-28 17:23 ` Joe Perches
@ 2013-12-30 2:39 ` Ding Tianhong
2013-12-30 6:05 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Ding Tianhong @ 2013-12-30 2:39 UTC (permalink / raw)
To: Joe Perches, Ding Tianhong
Cc: Sergei Shtylyov, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
On 2013/12/29 1:23, Joe Perches wrote:
> On Sat, 2013-12-28 at 23:18 +0800, Ding Tianhong wrote:
>> 于 2013/12/28 21:58, Sergei Shtylyov 写道:
>>> Hello.
>>>
>>> On 28-12-2013 10:17, Ding Tianhong wrote:
>>>
>>>> Use possibly more efficient ether_addr_equal
>>>> to instead of memcmp.
>>>
>>>> Cc: "David S. Miller" <davem@davemloft.net>
>>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>>>> ---
>>>> drivers/net/ethernet/packetengines/yellowfin.c | 12 ++++++------
>>>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>>> diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
>>>> index d28593b..b83ac0e 100644
>>>> --- a/drivers/net/ethernet/packetengines/yellowfin.c
>>>> +++ b/drivers/net/ethernet/packetengines/yellowfin.c
>>>> @@ -1097,12 +1097,12 @@ static int yellowfin_rx(struct net_device *dev)
>>>> if (status2 & 0x80) dev->stats.rx_dropped++;
>>>> #ifdef YF_PROTOTYPE /* Support for prototype hardware errata. */
>>>> } else if ((yp->flags & HasMACAddrBug) &&
>>>> - memcmp(le32_to_cpu(yp->rx_ring_dma +
>>>> - entry*sizeof(struct yellowfin_desc)),
>>>> - dev->dev_addr, 6) != 0 &&
>>>> - memcmp(le32_to_cpu(yp->rx_ring_dma +
>>>> - entry*sizeof(struct yellowfin_desc)),
>>>> - "\377\377\377\377\377\377", 6) != 0) {
>>>> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>>>> + entry * sizeof(struct yellowfin_desc)),
>>>> + dev->dev_addr) &&
>>>
>>> Previous line was aligned correctly, the above line should start under le32_to_cpu.
>>>
>>>> + !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>>>> + entry * sizeof(struct yellowfin_desc)),
>>>
>>> Start the continuation lines under 'yp', please.
>>>
>>>> + "\377\377\377\377\377\377")) {
>>>
>>> This line should start under le32_to_cpu.
>>>
>>> WBR, Sergei
>>>
>>
>> Hi sergei:
>> you mean this way?
>> !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>> entry * sizeof(struct yellowfin_desc)),
>> dev->dev_addr) &&
>>
>> !ether_addr_equal(le32_to_cpu(yp->rx_ring_dma +
>> entry * sizeof(struct yellowfin_desc)),
>> "\377\377\377\377\377\377")) {
>
> Does this really matter?
> Does anyone have a packetengine NIC anymore?
>
> \377 octal is 0xff, so this is matching a broadcast address.
> is_broadcast_ether_addr(addr) would be appropriate.
>
> So would using a temporary address.
>
> u8 *addr = (u8 *)(unsigned long)le32_to_cpu(etc)
>
> but the whole thing looks very suspect as an le32
> value could not be added to correctly on a
> big-endian arch anyway.
>
> My guess is this was tested only on an x86 and
> it should be:
>
> u8 *addr = (u8 *)(unsigned long)(le32_to_cpu(yp->rx_ring_dma) +
> entry * sizeof(struct yellowfin_desc));
>
> It maybe better just to leave these two alone.
>
Hi Joe:
I don't understand packetengine NIC anymore, But I think the change is clearly,
as your said, the broadcast check is enough here, did you mean that?
!is_broadcast_ether_addr((u8 *)(le32_to_cpu(yp->rx_ring_dma) +
entry * sizeof(struct yellowfin_desc)))
Thanks
Regards
Ding
>
> .
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-30 2:39 ` Ding Tianhong
@ 2013-12-30 6:05 ` Joe Perches
2013-12-30 6:19 ` Ding Tianhong
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2013-12-30 6:05 UTC (permalink / raw)
To: Ding Tianhong
Cc: Ding Tianhong, Sergei Shtylyov, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
On Mon, 2013-12-30 at 10:39 +0800, Ding Tianhong wrote:
> I don't understand packetengine NIC anymore, But I think the change is clearly,
> as your said, the broadcast check is enough here, did you mean that?
>
> !is_broadcast_ether_addr((u8 *)(le32_to_cpu(yp->rx_ring_dma) +
> entry * sizeof(struct yellowfin_desc)))
Not quite. I meant this could be:
u8 *addr = (u8 *)(unsigned long)le32_to_cpu(yp->rx_ring_dma) +
entry * sizeof(struct yellowfin_desc);
if (!ether_addr_equal(addr, dev->dev_addr) &&
!is_broadcast_ether_addr(addr)) {
etc...
but again, I think thus hardly matters and could just as well
be left alone.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr
2013-12-30 6:05 ` Joe Perches
@ 2013-12-30 6:19 ` Ding Tianhong
0 siblings, 0 replies; 8+ messages in thread
From: Ding Tianhong @ 2013-12-30 6:19 UTC (permalink / raw)
To: Joe Perches
Cc: Ding Tianhong, Sergei Shtylyov, David S. Miller, Netdev,
linux-kernel@vger.kernel.org
On 2013/12/30 14:05, Joe Perches wrote:
> On Mon, 2013-12-30 at 10:39 +0800, Ding Tianhong wrote:
>> I don't understand packetengine NIC anymore, But I think the change is clearly,
>> as your said, the broadcast check is enough here, did you mean that?
>>
>> !is_broadcast_ether_addr((u8 *)(le32_to_cpu(yp->rx_ring_dma) +
>> entry * sizeof(struct yellowfin_desc)))
>
> Not quite. I meant this could be:
>
> u8 *addr = (u8 *)(unsigned long)le32_to_cpu(yp->rx_ring_dma) +
> entry * sizeof(struct yellowfin_desc);
>
> if (!ether_addr_equal(addr, dev->dev_addr) &&
> !is_broadcast_ether_addr(addr)) {
> etc...
>
> but again, I think thus hardly matters and could just as well
> be left alone.
>
>
Ok, I will focus on ether_addr_equal in this patch, anymore will left alone. Thanks
Regards
Ding
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-12-30 6:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-28 6:17 [PATCH net-next v2 11/20] net: packetengines: slight optimization of addr Ding Tianhong
2013-12-28 13:58 ` Sergei Shtylyov
2013-12-28 15:18 ` Ding Tianhong
2013-12-28 17:23 ` Joe Perches
2013-12-30 2:39 ` Ding Tianhong
2013-12-30 6:05 ` Joe Perches
2013-12-30 6:19 ` Ding Tianhong
2013-12-28 23:25 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).