Re: tun mq failure

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jan 23, 2013 at 08:10:35PM +0800, Jason Wang wrote:
> On 01/23/2013 07:41 PM, Michael S. Tsirkin wrote:
> > On Wed, Jan 23, 2013 at 12:06:40PM +0100, Hannes Frederic Sowa wrote:
> >> On Wed, Jan 23, 2013 at 12:05:16PM +0200, Michael S. Tsirkin wrote:
> >>> This is when trying to start a VPN using some old openvpn binary so MQ
> >>> is not set.
> >>>
> >>> So
> >>> 1. I think we should limit allocation of MQ to when MQ flag is set in SETIFF.
> >>> 2. order 7 allocation is 2^^7 pages - about half a megabyte of contigious
> >>>    memory. This is quite likely to fail.
> >>>    Let's start with a small limit on number of queues, like 8?
> >>>    Then we know it will succeed.
> >>>    Longer term we might want to solve it differently.
> >> This has been come up before:
> >> http://thread.gmane.org/gmane.linux.network/255647/focus=255902
> >>
> >> I think a solution to this problem is still outstanding.
> > Right. What (at least I) missed is that it's the
> > queue array allocation that fails here.
> > So I think something like the following will sort the first issue
> > (compiled only):
> >
> > For the second, for 3.8 maybe the prudent thing to do is
> > to set MAX_TAP_QUEUES to a small value, like 8, to avoid
> > userspace relying on a large number of queues being available,
> > and look at a better way to do this longer term, like
> > using an array of pointers.
> 
> Sure, this is just the method I reply in that thread. Not sure 8 is the
> best, but since it fit into one page, should be ok. Maybe we can use
> flex array to avoid high order memory allocation in the longer term.

Right so let's for now use  DEFAULT_MAX_NUM_RSS_QUEUES as Eric suggested.

> >
> > --->
> >
> > tun: don't waste memory on unused queues
> >
> > If MQ flag is off, we never attach more than 1 queue.
> > So let's not allocate memory for the unused ones.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ---
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index af372d0..813d303 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1577,6 +1577,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >  	else {
> >  		char *name;
> >  		unsigned long flags = 0;
> > +		unsigned int max_tap_queues;
> >  
> >  		if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> >  			return -EPERM;
> > @@ -1599,9 +1600,13 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >  		if (*ifr->ifr_name)
> >  			name = ifr->ifr_name;
> >  
> > +		if (ifr->ifr_flags & IFF_MULTI_QUEUE)
> > +			max_tap_queues = MAX_TAP_QUEUES;
> > +		else
> > +			max_tap_queues = 1;
> >  		dev = alloc_netdev_mqs(sizeof(struct tun_struct), name,
> >  				       tun_setup,
> > -				       MAX_TAP_QUEUES, MAX_TAP_QUEUES);
> > +				       max_tap_queues, max_tap_queues);
> >  		if (!dev)
> >  			return -ENOMEM;
> >