* [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 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
* 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
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).