netdev - RE: [PATCH net-next] hyperv: Add support for Virtual Receive Side Scaling (vRSS)

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ff7f7541d0164d408b3a4278569c593a@DFM-DB3MBX15-06.exchange.corp.microsoft.com>
Date:	Thu, 19 Dec 2013 18:36:27 +0000
From:	Haiyang Zhang <haiyangz@...rosoft.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	KY Srinivasan <kys@...rosoft.com>,
	"olaf@...fle.de" <olaf@...fle.de>,
	"jasowang@...hat.com" <jasowang@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"driverdev-devel@...uxdriverproject.org" 
	<driverdev-devel@...uxdriverproject.org>
Subject: RE: [PATCH net-next] hyperv: Add support for Virtual Receive Side
 Scaling (vRSS)



> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@...arflare.com]
> Sent: Thursday, December 19, 2013 12:46 PM
> To: Haiyang Zhang
> Cc: davem@...emloft.net; netdev@...r.kernel.org; KY Srinivasan;
> olaf@...fle.de; jasowang@...hat.com; linux-kernel@...r.kernel.org;
> driverdev-devel@...uxdriverproject.org
> Subject: Re: [PATCH net-next] hyperv: Add support for Virtual Receive Side
> Scaling (vRSS)
> 
> On Wed, 2013-12-18 at 14:21 -0800, Haiyang Zhang wrote:
> > This feature allows multiple channels to be used by each virtual NIC.
> > It is available on Hyper-V host 2012 R2.
> >  	} else if (ret == -EAGAIN) {
> > -		netif_stop_queue(ndev);
> > +		netif_tx_stop_all_queues(ndev);
> >  		if (atomic_read(&net_device->num_outstanding_sends) < 1) {
> > -			netif_wake_queue(ndev);
> > +			netif_tx_wake_all_queues(ndev);
> >  			ret = -ENOSPC;
> >  		}
> >  	} else {
> 
> This doesn't makes any sense to me.  How can you safely share the same
> channels between all TX queues?
> 
> I think you need to associate TX queues and channels 1-1.  If you are required to
> map packets to TX queues using the Toeplitz hash, you should implement
> ndo_select_queue and do the mapping there.  Then in
> netvsc_send() you would use the queue number from the skb to find which
> channel to use and which queue may need to be stopped/woken.
> 
> [...]
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> [...]
> > +/* Toeplitz hash function
> > + * data: network byte order
> > + * return: host byte order
> > + */
> 
> This looks incredibly slow.  I've seen software implementations that are likely to
> be more efficient, e.g.
> <http://thread.gmane.org/gmane.linux.network/284612/>
> 
> [...]
> > int netvsc_probe(struct hv_device *dev,
> >  		return ret;
> >  	}
> >  	memcpy(net->dev_addr, device_info.mac_adr, ETH_ALEN);
> > +	nvdev = hv_get_drvdata(dev);
> > +	rtnl_lock();
> > +	netif_set_real_num_tx_queues(net, nvdev->num_chn);
> > +	netif_set_real_num_rx_queues(net, nvdev->num_chn);
> [...]
> 
> These functions can fail if called after registering the net device, so you should
> either call them with the final values earlier or handle failure here.
> 
> Also, I notice that dev_addr is only set after registering; that should be fixed.
> 

Thank you for the suggestions! I will re-write the send queue selection, enhance
the hash calculation, also fix the initialization sequence.

Thanks,
- Haiyang