netdev - Re: [PATCH net] xen-netback: bookkeep number of queues in our own module

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: <20140623084831.GV20819@zion.uk.xensource.com>
Date:	Mon, 23 Jun 2014 09:48:31 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	Paul Durrant <Paul.Durrant@...rix.com>
CC:	Wei Liu <wei.liu2@...rix.com>, David Miller <davem@...emloft.net>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
	Ian Campbell <Ian.Campbell@...rix.com>
Subject: Re: [PATCH net] xen-netback: bookkeep number of queues in our own
 module

On Mon, Jun 23, 2014 at 08:59:16AM +0100, Paul Durrant wrote:
[...]
> > > > So we bookkeep the number of queues in xen-netback to solve this
> > > > problem. The usage of real_num_tx_queues in core driver is to cap
> > queue
> > > > index to a valid value. In start_xmit we've already guarded against out
> > > > of range queue index so we should be fine.
> > > >
> > > > This fixes a regression introduced by multiqueue patchset in 3.16-rc1.
> > > >
> > > > Reported-by: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> > > > Signed-off-by: Wei Liu <wei.liu2@...rix.com>
> > >
> > > I say you should have a select queue method at all.
> > >
> > > You're essentially providing a half-assed version of __netdev_pick_tx()
> > > except that:
> > >
> > > 1) You're _completely_ ignoring the socket hash, if any.
> > >
> > > 2) You're not allowing XPS to work, _at all_.
> > >
> > > I think you need to serious reevaluate providing any select queue
> > > method at all, just let netdev_pick_tx() do all the work.
> > >
> > 
> > Looking at the core driver code in more details I think you're right. I
> > will remove the select queue method.
> >
> 
> Bear in mind that the original intention of the multi-queue patches
> was to allow the queue selection algorithm to be negotiated with the
> frontend (see
> http://lists.xen.org/archives/html/xen-devel/2013-06/msg02654.html).
> Particularly, if the frontend is Windows then netback will need to use
> a Toeplitz hash to steer traffic since this is stipulated by
> Microsoft's RSS (Receive Side Scaling) interfaces. So, IMO netback
> should always implement a select queue method, otherwise any
> (theoretical) algorithm change in __netdev_pick_tx() would be
> immediately imposed on frontends, possibly causing them to misbehave.
> 

In that case we can easily add back this select queue routine when the
implementation comes, right?

On the other hand, we can exploit the fact that current select queue
method takes a "fallback" parameter and it points to __netdev_pick_tx,
we can have something like:

static u16 xenvif_select_queue(struct net_device *dev, struct sk_buff *skb,
                               void *accel_priv, select_queue_fallback_t fallback)
{
        return fallback(dev, skb);
}

But that doesn't make much difference and is a bit redundant IMHO.


Wei.

>   Paul
>  
> > > If you have some issue maintaining the release of queue resources,
> > > maintain that privately and keep those details in the queue resource
> > > allocation and freeing code _only_.  Don't make it an issue that
> > > interferes at all with the normal mechanisms for SKB tx queue
> > > selection.
> > >
> > 
> > Sure. This is exactly the main idea of this patch, just that it
> > interfered the queue selection logic. :-(
> > 
> > Will send an updated version soon.
> > 
> > Wei.
> > 
> > > Thanks.
> > --
> > 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
--
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