netdev - Re: [net-next 2/8] bnx2x: add select queue callback

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Sun, 12 Dec 2010 12:25:03 +0200
From:	"Eilon Greenstein" <eilong@...adcom.com>
To:	"David Miller" <davem@...emloft.net>
cc:	"Dmitry Kravkov" <dmitry@...adcom.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	vladz@...adcom.com
Subject: Re: [net-next 2/8] bnx2x: add select queue callback

On Fri, 2010-12-10 at 14:11 -0800, David Miller wrote:
> From: "Dmitry Kravkov" <dmitry@...adcom.com>
> Date: Thu, 9 Dec 2010 14:09:16 +0200
> 
> > +#endif
> > +	/* Select queue (if defined) adjust for fcoe */
> > +	fp_index = skb_tx_hash(dev, skb) - FCOE_CONTEXT_USE;
> > +
> > +	return (fp_index >= 0 ? fp_index : 0);
> 
> This doesn't make any sense, and it's one of the reasons I really
> hate the fact that drivers sometimes need to override the
> select_queue method.
> 
> Because 9 times out of 10 they get it wrong.
> 
> You're mapping queues 0 --> FCOE_CONTEXT_USE to zero.
> 
> But that's not what you actually want.
> 
> You're actually trying to make non-FCOE traffic hash only amongst the
> non-FCOE queues.
> 
> So make your code actually do that instead of screwing up the
> distribution of the hash result.
> 

Indeed, the current suggestion is giving queue 0 double weight compared
to all other queues just so the skb_tx_hash will be used as is to
benefit from future enhancement changes to it. We will re-spin this
patch and move the content of the skb_tx_hash to __skb_tx_hash that will
receive the netdev->skb_real_num_tx_queues as a parameter - this way, we
can call the __skb_tx_hash with a lower number.

Thanks,
Eilon




--
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