[net-next PATCH 0/3] Backwards-compatible noqueue in virtual interfaces

[net-next PATCH 0/3] Backwards-compatible noqueue in virtual interfaces

[Phil Sutter]

This series is inspired by a patch sent[1] in from Jesper Brouer and the
discussion it started. Basically it tries to provide a solution which is
backwards compatible while still enabling the veth performance
improvement by default.

[1]: http://www.spinics.net/lists/netdev/msg299407.html

Jesper Dangaard Brouer (1):
  veth: don't assign a qdisc to veth

Phil Sutter (2):
  net: make default tx_queue_len configurable
  net: sched: set tx_queue_len to default when changing noqueue device's
    qdisc

 drivers/net/veth.c  |  1 +
 net/Kconfig         | 12 ++++++++++++
 net/ethernet/eth.c  |  2 +-
 net/sched/sch_api.c |  4 ++++
 4 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.1.2

[net-next PATCH 1/3] net: make default tx_queue_len configurable

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/Kconfig        | 12 ++++++++++++
 net/ethernet/eth.c |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/Kconfig b/net/Kconfig
index 7021c1b..21c164f 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -48,6 +48,18 @@ config COMPAT_NETLINK_MESSAGES
 config NET_INGRESS
 	bool
 
+config DEFAULT_TX_QUEUE_LEN
+	prompt "Default TX queue length (in packets)" if EXPERT
+	int
+	default 1000	# Ethernet wants good queues
+	help
+	  Set the default value of tx_queue_len for newly created network
+	  interfaces. It is used by queueing disciplines to determine how many
+	  packets to keep in backlog before starting to drop new ones.
+
+	  The default value of 1000 packets is there for a very long time and
+	  in combination with GSO way too big.
+
 menu "Networking options"
 
 source "net/packet/Kconfig"
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 77e0f0e..b778586 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -355,7 +355,7 @@ void ether_setup(struct net_device *dev)
 	dev->hard_header_len 	= ETH_HLEN;
 	dev->mtu		= ETH_DATA_LEN;
 	dev->addr_len		= ETH_ALEN;
-	dev->tx_queue_len	= 1000;	/* Ethernet wants good queues */
+	dev->tx_queue_len	= CONFIG_DEFAULT_TX_QUEUE_LEN;
 	dev->flags		= IFF_BROADCAST|IFF_MULTICAST;
 	dev->priv_flags		|= IFF_TX_SKB_SHARING;
 
-- 
2.1.2

[net-next PATCH 2/3] net: sched: set tx_queue_len to default when changing noqueue device's qdisc

[Phil Sutter]

Virtual interfaces don't necessarily need a qdisc attached to them. This
is signalled by setting dev->tx_queue_len to zero upon initialisation. The
problems begin when a user still adds a qdisc, as then the special value
is used as a regular one causing massive packet drops as soon as the
device sees a bit of traffic load.

This patch solves the issue by setting tx_queue_len to the global
default for physical devices if it is zero and a qdisc is added. Another
benefit of this is that veth devices can default to noqueue while still
preserving the expected behaviour when adding a qdisc.

The drawback of this patch is that the noqueue state won't be restored
after a user has removed the custom qdisc, as the information about
whether this is legitimate for the given device is lost.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/sched/sch_api.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f06aa01..79b8900 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1315,6 +1315,10 @@ graft:
 			qdisc_destroy(q);
 		return err;
 	}
+	/* circumvent noqueue hack for virtual interfaces if
+	 * user desires to use a qdisc on it */
+	if (!dev->tx_queue_len)
+		dev->tx_queue_len = CONFIG_DEFAULT_TX_QUEUE_LEN;
 
 	return 0;
 }
-- 
2.1.2

[net-next PATCH 3/3] veth: don't assign a qdisc to veth

[Phil Sutter]

From: Jesper Dangaard Brouer <brouer@redhat.com>

The veth driver is a virtual device, and should not have assigned
the default qdisc.  Verified (ndo_start_xmit) veth_xmit can only
return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Reported-by: Mrunal Patel <mpatel@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 drivers/net/veth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index c8186ff..6b3d822 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -302,6 +302,7 @@ static const struct net_device_ops veth_netdev_ops = {
 static void veth_setup(struct net_device *dev)
 {
 	ether_setup(dev);
+	dev->tx_queue_len = 0;
 
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
-- 
2.1.2

Re: [net-next PATCH 2/3] net: sched: set tx_queue_len to default when changing noqueue device's qdisc

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> Virtual interfaces don't necessarily need a qdisc attached to them. This
> is signalled by setting dev->tx_queue_len to zero upon initialisation. The
> problems begin when a user still adds a qdisc, as then the special value
> is used as a regular one causing massive packet drops as soon as the
> device sees a bit of traffic load.
> 
> This patch solves the issue by setting tx_queue_len to the global
> default for physical devices if it is zero and a qdisc is added. Another
> benefit of this is that veth devices can default to noqueue while still
> preserving the expected behaviour when adding a qdisc.
> 
> The drawback of this patch is that the noqueue state won't be restored
> after a user has removed the custom qdisc, as the information about
> whether this is legitimate for the given device is lost.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/sched/sch_api.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index f06aa01..79b8900 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1315,6 +1315,10 @@ graft:
>  			qdisc_destroy(q);
>  		return err;
>  	}
> +	/* circumvent noqueue hack for virtual interfaces if
> +	 * user desires to use a qdisc on it */
> +	if (!dev->tx_queue_len)
> +		dev->tx_queue_len = CONFIG_DEFAULT_TX_QUEUE_LEN;

The comment should tell you that this isn't pretty :-)

How about making tx_queue_len really the tx_queue_len, and some
not some "yeah, if its zero don't assign a queue"?

IOW, why not specifically annotate virtual devices so they do not get a
queue, ever (unless configured?)

That would allow to leave every default setting just the way they are,
and it would not muck with people assigning e.g. htb to veth since
the txqlen would be untouched.

But aside from that, I think your suggested hack would work.

Re: [net-next PATCH 2/3] net: sched: set tx_queue_len to default when changing noqueue device's qdisc

[Eric Dumazet]

On Wed, 2015-07-29 at 22:51 +0200, Phil Sutter wrote:
> Virtual interfaces don't necessarily need a qdisc attached to them. This
> is signalled by setting dev->tx_queue_len to zero upon initialisation. The
> problems begin when a user still adds a qdisc, as then the special value
> is used as a regular one causing massive packet drops as soon as the
> device sees a bit of traffic load.
> 
> This patch solves the issue by setting tx_queue_len to the global
> default for physical devices if it is zero and a qdisc is added. Another
> benefit of this is that veth devices can default to noqueue while still
> preserving the expected behaviour when adding a qdisc.
> 
> The drawback of this patch is that the noqueue state won't be restored
> after a user has removed the custom qdisc, as the information about
> whether this is legitimate for the given device is lost.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/sched/sch_api.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index f06aa01..79b8900 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1315,6 +1315,10 @@ graft:
>  			qdisc_destroy(q);
>  		return err;
>  	}
> +	/* circumvent noqueue hack for virtual interfaces if
> +	 * user desires to use a qdisc on it */
> +	if (!dev->tx_queue_len)
> +		dev->tx_queue_len = CONFIG_DEFAULT_TX_QUEUE_LEN;
>  
>  	return 0;
>  }

This is still broken for htb : htb_init() would run before this,
and htb_init() does :


q->direct_qlen = qdisc_dev(sch)->tx_queue_len;

Really, I am not sure this can be changed. qdisc scripts are mostly
written by skilled people knowing if txqueuelen needs to be changed or
not...

Change the kernel behavior and scripts will break.

Re: [net-next PATCH 2/3] net: sched: set tx_queue_len to default when changing noqueue device's qdisc

[Phil Sutter]

On Wed, Jul 29, 2015 at 11:10:45PM +0200, Eric Dumazet wrote:
> On Wed, 2015-07-29 at 22:51 +0200, Phil Sutter wrote:
[...]
> > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> > index f06aa01..79b8900 100644
> > --- a/net/sched/sch_api.c
> > +++ b/net/sched/sch_api.c
> > @@ -1315,6 +1315,10 @@ graft:
> >  			qdisc_destroy(q);
> >  		return err;
> >  	}
> > +	/* circumvent noqueue hack for virtual interfaces if
> > +	 * user desires to use a qdisc on it */
> > +	if (!dev->tx_queue_len)
> > +		dev->tx_queue_len = CONFIG_DEFAULT_TX_QUEUE_LEN;
> >  
> >  	return 0;
> >  }
> 
> This is still broken for htb : htb_init() would run before this,
> and htb_init() does :
> 
> 
> q->direct_qlen = qdisc_dev(sch)->tx_queue_len;

Oh, indeed. But the two lines following it read:

| if (q->direct_qlen < 2) /* some devices have zero tx_queue_len */
| 	q->direct_qlen = 2;

So there is a workaround already in place. Scrolling through the other
implementations it looks like they either ignore tx_queue_len altogether
or have fallbacks for when it's zero already in place. Looks like I was
fixing a pfifo-specific problem in a generic way.

> Really, I am not sure this can be changed. qdisc scripts are mostly
> written by skilled people knowing if txqueuelen needs to be changed or
> not...
> 
> Change the kernel behavior and scripts will break.

I understand that, although I personally wouldn't rely upon such
inconsistent defaults if I were aware of them. Anyway, this series tries
to leave the skilled people alone whilst focussing on the potentially
less skilled ones using veth with default settings. In that regard
Florian's (and now also David's) suggestions to have a separate flag for
drivers defaulting to noqueue and leaving tx_queue_len alone might be a
much better choice.

I'll try that path instead and see where it leads me. Thanks a lot for
the quick help!

Cheers, Phil

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/Kconfig        | 12 ++++++++++++
>  net/ethernet/eth.c |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/net/Kconfig b/net/Kconfig
> index 7021c1b..21c164f 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -48,6 +48,18 @@ config COMPAT_NETLINK_MESSAGES
>  config NET_INGRESS
>  	bool
>  
> +config DEFAULT_TX_QUEUE_LEN
> +	prompt "Default TX queue length (in packets)" if EXPERT
> +	int
> +	default 1000	# Ethernet wants good queues
> +	help
> +	  Set the default value of tx_queue_len for newly created network
> +	  interfaces. It is used by queueing disciplines to determine how many
> +	  packets to keep in backlog before starting to drop new ones.
> +
> +	  The default value of 1000 packets is there for a very long time and
> +	  in combination with GSO way too big.
> +

I can't see how this could be used in a meaningful way.

No distro is going to touch this.

I don't think sysctl value would help either.

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Phil Sutter]

On Wed, Jul 29, 2015 at 11:06:18PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
[...]
> > +config DEFAULT_TX_QUEUE_LEN
> > +	prompt "Default TX queue length (in packets)" if EXPERT
> > +	int
> > +	default 1000	# Ethernet wants good queues
> > +	help
> > +	  Set the default value of tx_queue_len for newly created network
> > +	  interfaces. It is used by queueing disciplines to determine how many
> > +	  packets to keep in backlog before starting to drop new ones.
> > +
> > +	  The default value of 1000 packets is there for a very long time and
> > +	  in combination with GSO way too big.
> > +
> 
> I can't see how this could be used in a meaningful way.
> 
> No distro is going to touch this.
> 
> I don't think sysctl value would help either.

I just didn't want to introduce yet another magic value assignment. It's
merely a #define with a little flexibility and a subtle note that the
default should be changed attached.

Cheers, Phil

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[David Miller]

From: Phil Sutter <phil@nwl.cc>
Date: Wed, 29 Jul 2015 23:34:28 +0200

> On Wed, Jul 29, 2015 at 11:06:18PM +0200, Florian Westphal wrote:
>> Phil Sutter <phil@nwl.cc> wrote:
> [...]
>> > +config DEFAULT_TX_QUEUE_LEN
>> > +	prompt "Default TX queue length (in packets)" if EXPERT
>> > +	int
>> > +	default 1000	# Ethernet wants good queues
>> > +	help
>> > +	  Set the default value of tx_queue_len for newly created network
>> > +	  interfaces. It is used by queueing disciplines to determine how many
>> > +	  packets to keep in backlog before starting to drop new ones.
>> > +
>> > +	  The default value of 1000 packets is there for a very long time and
>> > +	  in combination with GSO way too big.
>> > +
>> 
>> I can't see how this could be used in a meaningful way.
>> 
>> No distro is going to touch this.
>> 
>> I don't think sysctl value would help either.
> 
> I just didn't want to introduce yet another magic value assignment. It's
> merely a #define with a little flexibility and a subtle note that the
> default should be changed attached.

Like others have mentioned, fix the _REAL_ issue.

Which is that there are devices (virtual or whatever) which don't want
a qdisc attached no matter what.  Flag those devices as such and
adjust the qdisc attachment logic to check that new flag.

Anything is better than hacking the queue len.

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Jesper Dangaard Brouer]

On Wed, 29 Jul 2015 14:37:31 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
[...]
> Which is that there are devices (virtual or whatever) which don't want
> a qdisc attached no matter what.  Flag those devices as such and
> adjust the qdisc attachment logic to check that new flag.

I agree on the approach DaveM are suggesting.

But virtual devices must support getting a qdisc attached.  I know of
many companies depending on this behavior.   Some times people just get
hit by this "strange" zero len issues when they happen to use
pfifo_fast as leaf node.


> Anything is better than hacking the queue len.

The hole problem comes from the double meaning of the queue len. e.g.
that value 0 have special meaning, but only during assigning the
default qdisc.  And pfifo_fast will use queue len zero if assigned.

(proposed solution:)

As DaveM also suggested, I would likely use a device flag to indicate
the device does not require any qdisc, and not assign any qdisc
(actually "noqueue") in case the default qdisc is chosen for this
device.

This should solve the problem for veth. And then we should cleanup all
the virtual devices, adding this flag and changing the
dev->tx_queue_len to the default value (e.g. remove setting it to zero).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Phil Sutter]

On Tue, Aug 11, 2015 at 05:48:07PM +0200, Jesper Dangaard Brouer wrote:
> 
> On Wed, 29 Jul 2015 14:37:31 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> [...]
> > Which is that there are devices (virtual or whatever) which don't want
> > a qdisc attached no matter what.  Flag those devices as such and
> > adjust the qdisc attachment logic to check that new flag.
> 
> I agree on the approach DaveM are suggesting.
> 
> But virtual devices must support getting a qdisc attached.  I know of
> many companies depending on this behavior.   Some times people just get
> hit by this "strange" zero len issues when they happen to use
> pfifo_fast as leaf node.

That was just how I (purposely mis-)interpreted David's suggestion.

> > Anything is better than hacking the queue len.
> 
> The hole problem comes from the double meaning of the queue len. e.g.
> that value 0 have special meaning, but only during assigning the
> default qdisc.  And pfifo_fast will use queue len zero if assigned.
> 
> (proposed solution:)
> 
> As DaveM also suggested, I would likely use a device flag to indicate
> the device does not require any qdisc, and not assign any qdisc
> (actually "noqueue") in case the default qdisc is chosen for this
> device.
> 
> This should solve the problem for veth. And then we should cleanup all
> the virtual devices, adding this flag and changing the
> dev->tx_queue_len to the default value (e.g. remove setting it to zero).

I have an unfinished solution in the oven, but being kept busy with
other things for now. The action plan is as follows:

1) Introduce IFF_NO_QUEUE net_device->priv_flag.
2) Have attach_default_qdiscs() and attach_one_default_qdisc() treat
   IFF_NO_QUEUE as alternative to tx_queue_len == 0.
3) Add warning to register_netdevice() if tx_queue_len == 0.
4) Change virtual NIC drivers to set IFF_NO_QUEUE and leave tx_queue_len
   alone.
5) Eventually drop all special handling for tx_queue_len == 0.

I am currently somewhere in 2) and need to implement 4) for veth as PoC to
check if 2) suffices in all situations we want. Not sure if 3) is
desireable at all or if there are valid cases for a literally zero
length TX queue length.

Cheers, Phil

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Alexei Starovoitov]

On Tue, Aug 11, 2015 at 06:23:35PM +0200, Phil Sutter wrote:
> 
> I have an unfinished solution in the oven, but being kept busy with
> other things for now. The action plan is as follows:
> 
> 1) Introduce IFF_NO_QUEUE net_device->priv_flag.
> 2) Have attach_default_qdiscs() and attach_one_default_qdisc() treat
>    IFF_NO_QUEUE as alternative to tx_queue_len == 0.
> 3) Add warning to register_netdevice() if tx_queue_len == 0.
> 4) Change virtual NIC drivers to set IFF_NO_QUEUE and leave tx_queue_len
>    alone.
> 5) Eventually drop all special handling for tx_queue_len == 0.
> 
> I am currently somewhere in 2) and need to implement 4) for veth as PoC to
> check if 2) suffices in all situations we want. Not sure if 3) is
> desireable at all or if there are valid cases for a literally zero
> length TX queue length.

sounds like you want to change default qdisc from pfifo_fast to noqueue
for veth, right?
In general 'changing the default' may be an acceptable thing, but then
it needs to strongly justified. How much performance does it bring?
Also why introduce the flag? Why not just add 'tx_queue_len = 0;' 
to veth_setup() like the whole bunch of devices do?

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Eric Dumazet]

On Tue, 2015-08-11 at 18:13 -0700, Alexei Starovoitov wrote:
> Also why introduce the flag? Why not just add 'tx_queue_len = 0;' 
> to veth_setup() like the whole bunch of devices do?

Sigh.

Because some people install htb or pfifo on veth, leaving tx_queue_len
unchanged.

If you install htb while tx_queue_len is 0, pfifo created on htb classe
can only queue one packet.


static int fifo_init(struct Qdisc *sch, struct nlattr *opt)
{
        bool bypass;
        bool is_bfifo = sch->ops == &bfifo_qdisc_ops;

        if (opt == NULL) {
                u32 limit = qdisc_dev(sch)->tx_queue_len ? : 1;

                if (is_bfifo)
                        limit *= psched_mtu(qdisc_dev(sch));

                sch->limit = limit;



Changing veth txqueuelen is too late.

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Phil Sutter]

On Tue, Aug 11, 2015 at 06:13:49PM -0700, Alexei Starovoitov wrote:
> On Tue, Aug 11, 2015 at 06:23:35PM +0200, Phil Sutter wrote:
> > 
> > I have an unfinished solution in the oven, but being kept busy with
> > other things for now. The action plan is as follows:
> > 
> > 1) Introduce IFF_NO_QUEUE net_device->priv_flag.
> > 2) Have attach_default_qdiscs() and attach_one_default_qdisc() treat
> >    IFF_NO_QUEUE as alternative to tx_queue_len == 0.
> > 3) Add warning to register_netdevice() if tx_queue_len == 0.
> > 4) Change virtual NIC drivers to set IFF_NO_QUEUE and leave tx_queue_len
> >    alone.
> > 5) Eventually drop all special handling for tx_queue_len == 0.
> > 
> > I am currently somewhere in 2) and need to implement 4) for veth as PoC to
> > check if 2) suffices in all situations we want. Not sure if 3) is
> > desireable at all or if there are valid cases for a literally zero
> > length TX queue length.
> 
> sounds like you want to change default qdisc from pfifo_fast to noqueue
> for veth, right?
> In general 'changing the default' may be an acceptable thing, but then
> it needs to strongly justified. How much performance does it bring?
> Also why introduce the flag? Why not just add 'tx_queue_len = 0;' 
> to veth_setup() like the whole bunch of devices do?

A quick test on my local VM with veth and netperf (netserver and veth
peer in different netns) I see an increase of about 5% of throughput
when using noqueue instead of the default pfifo_fast.

Cheers, Phil

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Jesper Dangaard Brouer]

On Thu, 13 Aug 2015 03:13:40 +0200 Phil Sutter <phil@nwl.cc> wrote:

> On Tue, Aug 11, 2015 at 06:13:49PM -0700, Alexei Starovoitov wrote:

> > In general 'changing the default' may be an acceptable thing, but then
> > it needs to strongly justified. How much performance does it bring?
> 
> A quick test on my local VM with veth and netperf (netserver and veth
> peer in different netns) I see an increase of about 5% of throughput
> when using noqueue instead of the default pfifo_fast.

Good that you can show 5% improvement with a single netperf flow.  We
are saving approx 6 atomic operations avoiding the qdisc code path.

This fixes a scalability issue with veth. Thus, the real performance
boost will happen with multiple flows and multiple CPU cores in
action.  You can try with a multi core VM and use super_netperf.

https://github.com/borkmann/stuff/blob/master/super_netperf

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [net-next PATCH 1/3] net: make default tx_queue_len configurable

[Phil Sutter]

On Thu, Aug 13, 2015 at 03:10:33PM +0200, Jesper Dangaard Brouer wrote:
> 
> On Thu, 13 Aug 2015 03:13:40 +0200 Phil Sutter <phil@nwl.cc> wrote:
> 
> > On Tue, Aug 11, 2015 at 06:13:49PM -0700, Alexei Starovoitov wrote:
> 
> > > In general 'changing the default' may be an acceptable thing, but then
> > > it needs to strongly justified. How much performance does it bring?
> > 
> > A quick test on my local VM with veth and netperf (netserver and veth
> > peer in different netns) I see an increase of about 5% of throughput
> > when using noqueue instead of the default pfifo_fast.
> 
> Good that you can show 5% improvement with a single netperf flow.  We
> are saving approx 6 atomic operations avoiding the qdisc code path.
> 
> This fixes a scalability issue with veth. Thus, the real performance
> boost will happen with multiple flows and multiple CPU cores in
> action.  You can try with a multi core VM and use super_netperf.
> 
> https://github.com/borkmann/stuff/blob/master/super_netperf

I actually used that on my VM as well, but the difference between a
single and ten streams in parallel was negligible. In order to avoid
tampering the results, I tested again on a physical system with four
cores, ran each benchmark ten times and built an average over the
results. This showed an increase in throughput of about 35% with a
single stream and about 10% with ten streams in parallel. Not sure
though why the improvement is bigger in the first case if there really
is a scalability problem as you say.

Cheers, Phil

Re: [net-next PATCH 0/3] Backwards-compatible noqueue in virtual interfaces

[Cong Wang]

On Wed, Jul 29, 2015 at 1:51 PM, Phil Sutter <phil@nwl.cc> wrote:
> This series is inspired by a patch sent[1] in from Jesper Brouer and the
> discussion it started. Basically it tries to provide a solution which is
> backwards compatible while still enabling the veth performance
> improvement by default.

You can specify the tx_queue_len when you create the veth device,
if that can be your "default".

tx_queue_len ties too much with qdisc probably due to historical
reasons, ideally we should only specify the queue len via qdisc
interface since at least a per-device tx_queue_len doesn't make
much sense for those multiqueue or hierarchical qdisc's. It's hard
to change without breaking existing scripts/code.

Like David said, maybe you can add a new netdev flag saying
not to have any qdisc even when tx_queue_len !=0, or let each
device choose its default qdisc...