netdev - RE: [PATCH] bnx2x: add support for receive hashing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 11 Jul 2010 10:22:15 -0700
From:	"Vladislav Zolotarov" <vladz@...adcom.com>
To:	"Eric Dumazet" <eric.dumazet@...il.com>
cc:	"David Miller" <davem@...emloft.net>,
	"therbert@...gle.com" <therbert@...gle.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH] bnx2x: add support for receive hashing



> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
> Behalf Of Eric Dumazet
> Sent: Sunday, July 11, 2010 7:45 PM
> To: Vladislav Zolotarov
> Cc: David Miller; therbert@...gle.com; netdev@...r.kernel.org
> Subject: RE: [PATCH] bnx2x: add support for receive hashing
> 
> Le dimanche 11 juillet 2010 à 06:16 -0700, Vladislav Zolotarov a écrit :
> > Dave, it's obvious that there a demand for a new HW/FW configuration
> > from our side - "rx hash enable" which is currently tightly coupled
> > with the RSS capability. As long as RSS flow in our FW includes a few
> > more things apart from just creating an RSS hash and as long as there
> > are flows (even hypothetical) that demand the RSS hash regardless the
> > RSS itself we started to work on separation of these two features from
> > FW perspective. This of course will demand a new FW version but once
> > we have it we'll be able to be more specific in HW configuration and
> > have a cleaner code.
> >
> > Technically, our FW may provide the Rx HASH always and in a current
> > driver configuration this is what it does.
> > I wonder if the driver always provides the HW RX HASH in the
> > skb->rxhash regardless the value of NETIF_F_RXHASH bit in a
> > netdev->features will it cause any harm? If not we can get rid of two
> > extra conditionals in the bnx2x_rx_int().
> 
> Hi
> 
> Please dont top-post on netdev, thanks.

This discussion is directly related to Tom's patch that's why I'm posting on this thread. 

> 
> NETIF_RX_HASH bit is needed so that we can disable skb->rxhash from a
> particular NIC if we know the hardware provided rxhash is not fulfilling
> our needs (We prefer spend some cpu cycles to recompute a software
> rxhash).
> 
> Software one for example deliver same rxhash values for both ways of a
> TCP flow, it can help conntracking for example.

I understand that, in that case the proper implementation would be to check the netdev->features when u decide to calculate the SW rxhash, isn't it?

> 
> The conditional in driver rx is cheap, since the condition is the same
> for all packets and CPU can predicts the branch.

Not exactly. Our FW/HW won't provide the rxhash for none-IP packets setting the hash CQE field to zero and clearing the ETH_FAST_PATH_RX_CQE_RSS_HASH_FLG in the CQE statsu_flags (for ARPs for instance). Branch prediction is nice but considering my previous remark why do we need this branch at all?

> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html