Netdev List
 help / color / mirror / Atom feed
* Re: [net-next-2.6 PATCH] if_link: Add PORT_REQUEST_MAX
From: David Miller @ 2011-03-09 20:49 UTC (permalink / raw)
  To: roprabhu; +Cc: netdev
In-Reply-To: <20110309203431.21853.21864.stgit@savbu-pc100.cisco.com>

From: Roopa Prabhu <roprabhu@cisco.com>
Date: Wed, 09 Mar 2011 12:34:31 -0800

> From: Roopa Prabhu <roprabhu@cisco.com>
> 
> This patch adds __PORT_REQUEST_MAX to port request enumeration. And defines
> PORT_REQUEST_MAX.
> 
> Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
> Signed-off-by: David Wang <dwang2@cisco.com>
> Signed-off-by: Christian Benvenuti <benve@cisco.com>

Why?

If some new request types get added, this max value will increase and
we don't want that to happen for things exposed to userspace.

Userspace should really not depend upon how many requests there are.

^ permalink raw reply

* [net-next-2.6 PATCH] if_link: Add PORT_REQUEST_MAX
From: Roopa Prabhu @ 2011-03-09 20:34 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Roopa Prabhu <roprabhu@cisco.com>

This patch adds __PORT_REQUEST_MAX to port request enumeration. And defines
PORT_REQUEST_MAX.

Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
Signed-off-by: David Wang <dwang2@cisco.com>
Signed-off-by: Christian Benvenuti <benve@cisco.com>
---
 include/linux/if_link.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)


diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index f4a2e6b..b37cc09 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -353,8 +353,11 @@ enum {
 	PORT_REQUEST_PREASSOCIATE_RR,
 	PORT_REQUEST_ASSOCIATE,
 	PORT_REQUEST_DISASSOCIATE,
+	__PORT_REQUEST_MAX,
 };
 
+#define PORT_REQUEST_MAX (__PORT_REQUEST_MAX - 1)
+
 enum {
 	PORT_VDP_RESPONSE_SUCCESS = 0,
 	PORT_VDP_RESPONSE_INVALID_FORMAT,


^ permalink raw reply related

* Re: Network performance with small packets - continued
From: Tom Lendacky @ 2011-03-09 20:11 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Shirley Ma, Rusty Russell, Krishna Kumar2, David Miller, kvm,
	netdev, steved
In-Reply-To: <20110309071558.GA25757@redhat.com>

Here are the results again with the addition of the interrupt rate that 
occurred on the guest virtio_net device:

Here is the KVM baseline (average of six runs):
  Txn Rate: 87,070.34 Txn/Sec, Pkt Rate: 172,992 Pkts/Sec
  Exits: 148,444.58 Exits/Sec
  TxCPU: 2.40%  RxCPU: 99.35%
  Virtio1-input  Interrupts/Sec (CPU0/CPU1): 5,154/5,222
  Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0

About 42% of baremetal.

Delayed freeing of TX buffers (average of six runs):
  Txn Rate: 90,886.19 Txn/Sec, Pkt Rate: 180,571 Pkts/Sec
  Exits: 142,681.67 Exits/Sec
  TxCPU: 2.78%  RxCPU: 99.36%
  Virtio1-input  Interrupts/Sec (CPU0/CPU1): 4,796/4,908
  Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0

About a 4% increase over baseline and about 44% of baremetal.

Delaying kick_notify (kick every 5 packets -average of six runs):
  Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
  Exits: 102,587.28 Exits/Sec
  TxCPU: 3.03%  RxCPU: 99.33%
  Virtio1-input  Interrupts/Sec (CPU0/CPU1): 4,200/4,293
  Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0

About a 23% increase over baseline and about 52% of baremetal.

Delaying kick_notify and pinning virtio1-input to CPU0 (average of six runs):
  Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
  Exits: 62,603.37 Exits/Sec
  TxCPU: 3.73%  RxCPU: 98.52%
  Virtio1-input  Interrupts/Sec (CPU0/CPU1): 11,564/0
  Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0

About a 77% increase over baseline and about 74% of baremetal.


On Wednesday, March 09, 2011 01:15:58 am Michael S. Tsirkin wrote:
> On Mon, Mar 07, 2011 at 04:31:41PM -0600, Tom Lendacky wrote:
> > We've been doing some more experimenting with the small packet network
> > performance problem in KVM.  I have a different setup than what Steve D.
> > was using so I re-baselined things on the kvm.git kernel on both the
> > host and guest with a 10GbE adapter.  I also made use of the
> > virtio-stats patch.
> > 
> > The virtual machine has 2 vCPUs, 8GB of memory and two virtio network
> > adapters (the first connected to a 1GbE adapter and a LAN, the second
> > connected to a 10GbE adapter that is direct connected to another system
> > with the same 10GbE adapter) running the kvm.git kernel.  The test was a
> > TCP_RR test with 100 connections from a baremetal client to the KVM
> > guest using a 256 byte message size in both directions.
> > 
> > I used the uperf tool to do this after verifying the results against
> > netperf. Uperf allows the specification of the number of connections as
> > a parameter in an XML file as opposed to launching, in this case, 100
> > separate instances of netperf.
> > 
> > Here is the baseline for baremetal using 2 physical CPUs:
> >   Txn Rate: 206,389.59 Txn/Sec, Pkt Rate: 410,048 Pkts/Sec
> >   TxCPU: 7.88%  RxCPU: 99.41%
> > 
> > To be sure to get consistent results with KVM I disabled the
> > hyperthreads, pinned the qemu-kvm process, vCPUs, vhost thread and
> > ethernet adapter interrupts (this resulted in runs that differed by only
> > about 2% from lowest to highest).  The fact that pinning is required to
> > get consistent results is a different problem that we'll have to look
> > into later...
> > 
> > Here is the KVM baseline (average of six runs):
> >   Txn Rate: 87,070.34 Txn/Sec, Pkt Rate: 172,992 Pkts/Sec
> >   Exits: 148,444.58 Exits/Sec
> >   TxCPU: 2.40%  RxCPU: 99.35%
> > 
> > About 42% of baremetal.
> 
> Can you add interrupt stats as well please?
> 
> > empty.  So I coded a quick patch to delay freeing of the used Tx buffers
> > until more than half the ring was used (I did not test this under a
> > stream condition so I don't know if this would have a negative impact). 
> > Here are the results
> > 
> > from delaying the freeing of used Tx buffers (average of six runs):
> >   Txn Rate: 90,886.19 Txn/Sec, Pkt Rate: 180,571 Pkts/Sec
> >   Exits: 142,681.67 Exits/Sec
> >   TxCPU: 2.78%  RxCPU: 99.36%
> > 
> > About a 4% increase over baseline and about 44% of baremetal.
> 
> Hmm, I am not sure what you mean by delaying freeing.
> I think we do have a problem that free_old_xmit_skbs
> tries to flush out the ring aggressively:
> it always polls until the ring is empty,
> so there could be bursts of activity where
> we spend a lot of time flushing the old entries
> before e.g. sending an ack, resulting in
> latency bursts.
> 
> Generally we'll need some smarter logic,
> but with indirect at the moment we can just poll
> a single packet after we post a new one, and be done with it.
> Is your patch something like the patch below?
> Could you try mine as well please?
> 
> > This spread out the kick_notify but still resulted in alot of them.  I
> > decided to build on the delayed Tx buffer freeing and code up an
> > "ethtool" like coalescing patch in order to delay the kick_notify until
> > there were at least 5 packets on the ring or 2000 usecs, whichever
> > occurred first.  Here are the
> > 
> > results of delaying the kick_notify (average of six runs):
> >   Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
> >   Exits: 102,587.28 Exits/Sec
> >   TxCPU: 3.03%  RxCPU: 99.33%
> > 
> > About a 23% increase over baseline and about 52% of baremetal.
> > 
> > Running the perf command against the guest I noticed almost 19% of the
> > time being spent in _raw_spin_lock.  Enabling lockstat in the guest
> > showed alot of contention in the "irq_desc_lock_class". Pinning the
> > virtio1-input interrupt to a single cpu in the guest and re-running the
> > last test resulted in
> > 
> > tremendous gains (average of six runs):
> >   Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
> >   Exits: 62,603.37 Exits/Sec
> >   TxCPU: 3.73%  RxCPU: 98.52%
> > 
> > About a 77% increase over baseline and about 74% of baremetal.
> > 
> > Vhost is receiving a lot of notifications for packets that are to be
> > transmitted (over 60% of the packets generate a kick_notify).  Also, it
> > looks like vhost is sending a lot of notifications for packets it has
> > received before the guest can get scheduled to disable notifications and
> > begin processing the packets
> 
> Hmm, is this really what happens to you?  The effect would be that guest
> gets an interrupt while notifications are disabled in guest, right? Could
> you add a counter and check this please?
> 
> Another possible thing to try would be these old patches to publish used
> index from guest to make sure this double interrupt does not happen:
>  [PATCHv2] virtio: put last seen used index into ring itself
>  [PATCHv2] vhost-net: utilize PUBLISH_USED_IDX feature
> 
> > resulting in some lock contention in the guest (and
> > high interrupt rates).
> > 
> > Some thoughts for the transmit path...  can vhost be enhanced to do some
> > adaptive polling so that the number of kick_notify events are reduced and
> > replaced by kick_no_notify events?
> 
> Worth a try.
> 
> > Comparing the transmit path to the receive path, the guest disables
> > notifications after the first kick and vhost re-enables notifications
> > after completing processing of the tx ring.
> 
> Is this really what happens? I though the host disables notifications
> after the first kick.
> 
> >  Can a similar thing be done for the
> > 
> > receive path?  Once vhost sends the first notification for a received
> > packet it can disable notifications and let the guest re-enable
> > notifications when it has finished processing the receive ring.  Also,
> > can the virtio-net driver do some adaptive polling (or does napi take
> > care of that for the guest)?
> 
> Worth a try. I don't think napi does anything like this.
> 
> > Running the same workload on the same configuration with a different
> > hypervisor results in performance that is almost equivalent to baremetal
> > without doing any pinning.
> > 
> > Thanks,
> > Tom Lendacky
> 
> There's no need to flush out all used buffers
> before we post more for transmit: with indirect,
> just a single one is enough. Without indirect we'll
> need more possibly, but just for testing this should
> be enough.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> Note: untested.
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 82dba5a..ebe3337 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct
> virtnet_info *vi) struct sk_buff *skb;
>  	unsigned int len, tot_sgs = 0;
> 
> -	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> +	if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
>  		pr_debug("Sent skb %p\n", skb);
>  		vi->dev->stats.tx_bytes += skb->len;
>  		vi->dev->stats.tx_packets++;
> -		tot_sgs += skb_vnet_hdr(skb)->num_sg;
> +		tot_sgs = 2+MAX_SKB_FRAGS;
>  		dev_kfree_skb_any(skb);
>  	}
>  	return tot_sgs;
> @@ -576,9 +576,6 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> struct net_device *dev) struct virtnet_info *vi = netdev_priv(dev);
>  	int capacity;
> 
> -	/* Free up any pending old buffers before queueing new ones. */
> -	free_old_xmit_skbs(vi);
> -
>  	/* Try to transmit */
>  	capacity = xmit_skb(vi, skb);
> 
> @@ -605,6 +602,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> struct net_device *dev) skb_orphan(skb);
>  	nf_reset(skb);
> 
> +	/* Free up any old buffers so we can queue new ones. */
> +	if (capacity < 2+MAX_SKB_FRAGS)
> +		capacity += free_old_xmit_skbs(vi);
> +
>  	/* Apparently nice girls don't return TX_BUSY; stop the queue
>  	 * before it gets out of hand.  Naturally, this wastes entries. */
>  	if (capacity < 2+MAX_SKB_FRAGS) {
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Stephen Hemminger @ 2011-03-09 20:01 UTC (permalink / raw)
  To: Lucas Nussbaum
  Cc: Injong Rhee, David Miller, xiyou.wangcong, netdev, sangtae.ha
In-Reply-To: <20110309182505.GA25272@xanadu.blop.info>

On Wed, 9 Mar 2011 19:25:05 +0100
Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:

> On 09/03/11 at 09:56 -0800, Stephen Hemminger wrote:
> > On Wed, 9 Mar 2011 07:53:19 +0100
> > Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> > 
> > > On 08/03/11 at 20:30 -0500, Injong Rhee wrote:
> > > > Now, both tools can be wrong. But that is not catastrophic since
> > > > congestion avoidance can kick in to save the day. In a pipe where no
> > > > other flows are competing, then exiting slow start too early can
> > > > slow things down as the window can be still too small. But that is
> > > > in fact when delays are most reliable. So those tests that say bad
> > > > performance with hystart are in fact, where hystart is supposed to
> > > > perform well.
> > > 
> > > Hi,
> > > 
> > > In my setup, there is no congestion at all (except the buffer bloat).
> > > Without Hystart, transferring 8 Gb of data takes 9s, with CUBIC exiting
> > > slow start at ~2000 packets.
> > > With Hystart, transferring 8 Gb of data takes 19s, with CUBIC exiting
> > > slow start at ~20 packets.
> > > I don't think that this is "hystart performing well". We could just as
> > > well remove slow start completely, and only do congestion avoidance,
> > > then.
> > > 
> > > While I see the value in Hystart, it's clear that there are some flaws
> > > in the current implementation. It probably makes sense to disable
> > > hystart by default until those problems are fixed.
> > 
> > What is the speed and RTT time of your network?
> > I think you maybe blaming hystart for other issues in the network.
> 
> What kind of issues?
> 
> Host1 is connected through a gigabit ethernet LAN to Router1
> Host2 is connected through a gigabit ethernet LAN to Router2
> Router1 and Router2 are connected through an experimentation network at
> 10 Gb/s
> RTT between Host1 and Host2 is 11.3ms.
> The network is not congested.

By my calculations (1G * 11.3ms) gives BDP of 941 packets which means
CUBIC would ideally exit slow start at 900 or so packets. Old CUBIC
slowstrart of 2000 packets means there is huge overshoot which means
large packet loss burst which would cause a large CPU load on receiver
processing SACK.

I assume you haven't done anything that would disable RFC1323
support like turn off window scaling or tcp timestamps.


-- 

^ permalink raw reply

* Re: [PATCHv4 net-next 0/8] Phonet: pipe protocol cleanup
From: David Miller @ 2011-03-09 19:57 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <201103091043.32391.remi.denis-courmont@nokia.com>

From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Wed, 9 Mar 2011 10:43:32 +0200

> 	Hello,
> 
> As "promised", here is the cleaned up and unified Phonet pipe code.
> The same kernel can now support both old (e.g. Nokia N900) and new
> (e.g. ST-Ericsson U8500 SoC) modems.
> 
> The following changes since commit 7b46ac4e77f3224a1befe032c77f1df31d1b42c4:
> 
>   inetpeer: Don't disable BH for initial fast RCU lookup. (2011-03-08 14:59:28 -0800)
> 
> are available in the git repository at:
>   git://git.remlab.net/linux-phonet.git master

Connects to that git server timeout, something is wrong.

sunset:/home/davem# tcpdump -i eth0 host git.remlab.net
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
11:53:55.290558 IP sunset.davemloft.net.davemloft.net.58504 > yop.chewa.net.git: Flags [S], seq 1785921252, win 4380, options [mss 1460,sackOK,TS val 437283616 ecr 0,nop,wscale 7], length 0
11:54:38.550165 IP sunset.davemloft.net.davemloft.net.51169 > yop.chewa.net.git: Flags [S], seq 3171701698, win 4380, options [mss 1460,sackOK,TS val 437287941 ecr 0,nop,wscale 7], length 0
11:54:41.550544 IP sunset.davemloft.net.davemloft.net.51169 > yop.chewa.net.git: Flags [S], seq 3171701698, win 4380, options [mss 1460,sackOK,TS val 437288242 ecr 0,nop,wscale 7], length 0
11:54:47.570532 IP sunset.davemloft.net.davemloft.net.51169 > yop.chewa.net.git: Flags [S], seq 3171701698, win 4380, options [mss 1460,sackOK,TS val 437288844 ecr 0,nop,wscale 7], length 0

Although web access works just fine.

So I'll apply the patches manually.

Thanks.

^ permalink raw reply

* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Stephen Hemminger @ 2011-03-09 19:56 UTC (permalink / raw)
  To: Lucas Nussbaum
  Cc: Injong Rhee, David Miller, xiyou.wangcong, netdev, sangtae.ha
In-Reply-To: <20110309182505.GA25272@xanadu.blop.info>

On Wed, 9 Mar 2011 19:25:05 +0100
Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:

> On 09/03/11 at 09:56 -0800, Stephen Hemminger wrote:
> > On Wed, 9 Mar 2011 07:53:19 +0100
> > Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> > 
> > > On 08/03/11 at 20:30 -0500, Injong Rhee wrote:
> > > > Now, both tools can be wrong. But that is not catastrophic since
> > > > congestion avoidance can kick in to save the day. In a pipe where no
> > > > other flows are competing, then exiting slow start too early can
> > > > slow things down as the window can be still too small. But that is
> > > > in fact when delays are most reliable. So those tests that say bad
> > > > performance with hystart are in fact, where hystart is supposed to
> > > > perform well.
> > > 
> > > Hi,
> > > 
> > > In my setup, there is no congestion at all (except the buffer bloat).
> > > Without Hystart, transferring 8 Gb of data takes 9s, with CUBIC exiting
> > > slow start at ~2000 packets.
> > > With Hystart, transferring 8 Gb of data takes 19s, with CUBIC exiting
> > > slow start at ~20 packets.
> > > I don't think that this is "hystart performing well". We could just as
> > > well remove slow start completely, and only do congestion avoidance,
> > > then.
> > > 
> > > While I see the value in Hystart, it's clear that there are some flaws
> > > in the current implementation. It probably makes sense to disable
> > > hystart by default until those problems are fixed.
> > 
> > What is the speed and RTT time of your network?
> > I think you maybe blaming hystart for other issues in the network.
> 
> What kind of issues?
> 
> Host1 is connected through a gigabit ethernet LAN to Router1
> Host2 is connected through a gigabit ethernet LAN to Router2
> Router1 and Router2 are connected through an experimentation network at
> 10 Gb/s
> RTT between Host1 and Host2 is 11.3ms.
> The network is not congested.
> 
> (I can provide access to the testbed if someone wants to do further
> testing)

Your backbone is faster than the LAN, interesting.
Could you check packet stats to see where packet drop is occuring?
It could be that routers don't have enough buffering to take packet
trains from 10G network and pace them out to 1G network.


-- 

^ permalink raw reply

* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
From: Michał Mirosław @ 2011-03-09 19:51 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, Ben Hutchings
In-Reply-To: <AANLkTiknMqNOQKiKpVaDy=zXCXuOxZaQTjKmmxY_bRC5@mail.gmail.com>

W dniu 9 marca 2011 20:18 użytkownik Tom Herbert <therbert@google.com> napisał:
>> Please don't add new get/set ops for netdev features - G/SFEATURES
>> handle them all.
>>
>> Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
>> and, if needed for now,
>> legacy get/set ops (they will go away, eventually).
> Don't see this in
> git://git.kernel.org/pub/scm/network/ethtool/ethtool.  Has the patch
> been applied?

This is recent work that just went into net-next. For userspace, if
you want to test it, there is my proof-of-concept patch (posted to
netdev some time ago) or Ben's ethtool cleanup patches (also in netdev
archive).

http://marc.info/?l=linux-netdev&m=129674290420375&w=3
http://marc.info/?l=linux-netdev&m=129830729917334&w=3

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
From: Ben Hutchings @ 2011-03-09 19:38 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Michał Mirosław, davem, netdev
In-Reply-To: <AANLkTiknMqNOQKiKpVaDy=zXCXuOxZaQTjKmmxY_bRC5@mail.gmail.com>

On Wed, 2011-03-09 at 11:18 -0800, Tom Herbert wrote:
> > Please don't add new get/set ops for netdev features - G/SFEATURES
> > handle them all.
> >
> > Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
> > and, if needed for now,
> > legacy get/set ops (they will go away, eventually).
> >
> 
> Don't see this in
> git://git.kernel.org/pub/scm/network/ethtool/ethtool.  Has the patch
> been applied?

I haven't changed ethtool yet.  I put some patches out for discussion
and Michał found some problems for them; I haven't had the time to
revise them yet.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
From: Tom Herbert @ 2011-03-09 19:18 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: davem, netdev
In-Reply-To: <AANLkTikFSRr649_P8V8J7Xj_KY8hSEw23RBAyyV2wUQr@mail.gmail.com>

> Please don't add new get/set ops for netdev features - G/SFEATURES
> handle them all.
>
> Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
> and, if needed for now,
> legacy get/set ops (they will go away, eventually).
>

Don't see this in
git://git.kernel.org/pub/scm/network/ethtool/ethtool.  Has the patch
been applied?

Tom

^ permalink raw reply

* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Lucas Nussbaum @ 2011-03-09 18:25 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Injong Rhee, David Miller, xiyou.wangcong, netdev, sangtae.ha
In-Reply-To: <20110309095659.1373c296@s6510>

On 09/03/11 at 09:56 -0800, Stephen Hemminger wrote:
> On Wed, 9 Mar 2011 07:53:19 +0100
> Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> 
> > On 08/03/11 at 20:30 -0500, Injong Rhee wrote:
> > > Now, both tools can be wrong. But that is not catastrophic since
> > > congestion avoidance can kick in to save the day. In a pipe where no
> > > other flows are competing, then exiting slow start too early can
> > > slow things down as the window can be still too small. But that is
> > > in fact when delays are most reliable. So those tests that say bad
> > > performance with hystart are in fact, where hystart is supposed to
> > > perform well.
> > 
> > Hi,
> > 
> > In my setup, there is no congestion at all (except the buffer bloat).
> > Without Hystart, transferring 8 Gb of data takes 9s, with CUBIC exiting
> > slow start at ~2000 packets.
> > With Hystart, transferring 8 Gb of data takes 19s, with CUBIC exiting
> > slow start at ~20 packets.
> > I don't think that this is "hystart performing well". We could just as
> > well remove slow start completely, and only do congestion avoidance,
> > then.
> > 
> > While I see the value in Hystart, it's clear that there are some flaws
> > in the current implementation. It probably makes sense to disable
> > hystart by default until those problems are fixed.
> 
> What is the speed and RTT time of your network?
> I think you maybe blaming hystart for other issues in the network.

What kind of issues?

Host1 is connected through a gigabit ethernet LAN to Router1
Host2 is connected through a gigabit ethernet LAN to Router2
Router1 and Router2 are connected through an experimentation network at
10 Gb/s
RTT between Host1 and Host2 is 11.3ms.
The network is not congested.

(I can provide access to the testbed if someone wants to do further
testing)
-- 
| Lucas Nussbaum             MCF Université Nancy 2 |
| lucas.nussbaum@loria.fr         LORIA / AlGorille |
| http://www.loria.fr/~lnussbau/  +33 3 54 95 86 19 |

^ permalink raw reply

* Re: [PATCH 1/2] ethtool: Support no-cache copy in config
From: Rick Jones @ 2011-03-09 18:09 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <alpine.DEB.2.00.1103082056010.4814@pokey.mtv.corp.google.com>

On Tue, 2011-03-08 at 21:05 -0800, Tom Herbert wrote:
> Add support for nccopy configuration as an offload option.  This allows
> enabling and disabling no-cache copies in copy-from-user on
> transmission for a device.

Does/should it depend on CKO being enabled on the device?

rick jones


^ permalink raw reply

* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-09 18:16 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Tom Lendacky, Rusty Russell, Krishna Kumar2, David Miller, kvm,
	netdev, steved
In-Reply-To: <20110309171603.GD7165@redhat.com>

On Wed, 2011-03-09 at 19:16 +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 09, 2011 at 08:51:33AM -0800, Shirley Ma wrote:
> > On Wed, 2011-03-09 at 10:09 -0600, Tom Lendacky wrote:
> > > > > Vhost is receiving a lot of notifications for packets that are
> to
> > > be
> > > > > transmitted (over 60% of the packets generate a kick_notify). 
> > 
> > This is guest TX send notification when vhost enables notification.
> > 
> > In TCP_STREAM test, vhost exits from reaching NAPI WEIGHT,
> 
> 
> You mean virtio?

Sorry, I messed up NAPI WEIGHT and VHOST NET WEIGHT.

I meant VHOST_NET_WEIGH, vhost exit handdle_tx() from VHOST NET WEIGHT
w/o enabling notification.

> 
> > it rarely
> > enables the notification, vhost re-enters handle_tx from NAPI poll,
> 
> Does NAPI really call handle_tx? Not rx? 

I meant for TX/RX, vhost re-enter handle_tx from vhost_poll_queue() not
from kick_notify.

Shirley


^ permalink raw reply

* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Stephen Hemminger @ 2011-03-09 17:56 UTC (permalink / raw)
  To: Lucas Nussbaum
  Cc: Injong Rhee, David Miller, xiyou.wangcong, netdev, sangtae.ha
In-Reply-To: <20110309065319.GA23740@xanadu.blop.info>

On Wed, 9 Mar 2011 07:53:19 +0100
Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:

> On 08/03/11 at 20:30 -0500, Injong Rhee wrote:
> > Now, both tools can be wrong. But that is not catastrophic since
> > congestion avoidance can kick in to save the day. In a pipe where no
> > other flows are competing, then exiting slow start too early can
> > slow things down as the window can be still too small. But that is
> > in fact when delays are most reliable. So those tests that say bad
> > performance with hystart are in fact, where hystart is supposed to
> > perform well.
> 
> Hi,
> 
> In my setup, there is no congestion at all (except the buffer bloat).
> Without Hystart, transferring 8 Gb of data takes 9s, with CUBIC exiting
> slow start at ~2000 packets.
> With Hystart, transferring 8 Gb of data takes 19s, with CUBIC exiting
> slow start at ~20 packets.
> I don't think that this is "hystart performing well". We could just as
> well remove slow start completely, and only do congestion avoidance,
> then.
> 
> While I see the value in Hystart, it's clear that there are some flaws
> in the current implementation. It probably makes sense to disable
> hystart by default until those problems are fixed.


What is the speed and RTT time of your network?
I think you maybe blaming hystart for other issues in the network.

^ permalink raw reply

* sky2, vlan and nat/masquerading
From: Christian Hesse @ 2011-03-09 17:15 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hello everybody,

I have a Samsung NF310, running kernel 2.6.37.3 with a patch to make my
ethernet controller work for vlans. It was discussed with the subject "sky2:
convert to new VLAN model (v0.2)" and made it to to kernel tree with commit
86aa77854f47ab6f5f9c687507af1f57d2b89004.
However it does not work properly, here are the details:

* Switch with one trunk port and several port in corresponding vlan ports
* Host connected to one of the vlan ports
* Samsung Netbook (see above) connected to the trunk port.

I get an IP address 192.168.x.x/24 via DHCP on interface connected to vlan 1.
The interface connected to vlan 2 has 172.16.0.1/24 and serves addresses via
DHCP. The system is set up to masquerade from 172.16.0.1/24.

I can access my netbook from the host in vlan 2, however I can not access
anything behind. The packets contain a broken vlan tag and the host does not
recognize them.
I've attached a tcpdump log. Please take a look at the icmp echo request and
reply packets, especially the last one.

I've tested with an usb-ethernet-adapter and everything works fine, so my
setup is ok.
-- 
Regards,
Chris

[-- Attachment #2: tcpdump.log --]
[-- Type: application/octet-stream, Size: 688 bytes --]

^ permalink raw reply

* Re: Network performance with small packets - continued
From: Michael S. Tsirkin @ 2011-03-09 17:16 UTC (permalink / raw)
  To: Shirley Ma
  Cc: Tom Lendacky, Rusty Russell, Krishna Kumar2, David Miller, kvm,
	netdev, steved
In-Reply-To: <1299689493.25664.120.camel@localhost.localdomain>

On Wed, Mar 09, 2011 at 08:51:33AM -0800, Shirley Ma wrote:
> On Wed, 2011-03-09 at 10:09 -0600, Tom Lendacky wrote:
> > > > Vhost is receiving a lot of notifications for packets that are to
> > be
> > > > transmitted (over 60% of the packets generate a kick_notify). 
> 
> This is guest TX send notification when vhost enables notification.
> 
> In TCP_STREAM test, vhost exits from reaching NAPI WEIGHT,


You mean virtio?

> it rarely
> enables the notification, vhost re-enters handle_tx from NAPI poll,

Does NAPI really call handle_tx? Not rx?

> so
> guest doesn't do much kick_notify.
> 
> In multiple TCP_RR test, seems vhost exits from nothing to send in TX vq
> very often, so it enables notification most of the time.
> 
> Shirley

^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Jiri Pirko @ 2011-03-09 17:11 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Andy Gospodarek, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet
In-Reply-To: <4D779CA2.1050302@gmail.com>

Wed, Mar 09, 2011 at 04:28:34PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 09/03/2011 16:09, Jiri Pirko a écrit :
>>Wed, Mar 09, 2011 at 03:49:49PM CET, nicolas.2p.debian@gmail.com wrote:
>>>Le 09/03/2011 08:45, Jiri Pirko a écrit :
>>>>Tue, Mar 08, 2011 at 10:44:37PM CET, nicolas.2p.debian@gmail.com wrote:
>>>>>Le 08/03/2011 14:42, Andy Gospodarek a écrit :
>>>>>>I'm pretty sure this patch will have the same catastrophic problem your
>>>>>>last one did.  By cloning and setting skb2->dev = orig_dev you just
>>>>>>inserted a frame identical to the one we received right back into the
>>>>>>stack.  It only took a few minutes for my box to melt as one frame on
>>>>>>the wire will cause an infinite number of frames to be received by the
>>>>>>stack.
>>>>>
>>>>>I agree with Andy. We still keep one reinject (netif_rx), which is
>>>>>probably better that two (__netif_receive_skb), but not enough.
>>>>>
>>>>>I really think we need a general framework for late delivery of final
>>>>>packets to packet handler registered somewhere in the rx_handler
>>>>>path.
>>>>>
>>>>>Jiri, is this patch the one you announced as "I have some kind nice
>>>>>solution in mind and I'm going to submit that as a patch later (too
>>>>>many patches are in the wind atm)" ?
>>>>
>>>>
>>>>I did not had time to verify my thought yet but I think that the only
>>>>think needed against my original patch (bonding: move processing of recv
>>>>handlers into handle_frame()) is ro remove vlan_on_bond_hook, period.
>>>>
>>>>Because all incoming arps are seen by bond_handle_frame =>
>>>>bond->recv_probe , even vlan ones - that would make eth0-bond0-bond0.5
>>>>work and eth0-bond0-br0-bond0.5 as well. But again, need to verify this.
>>>
>>>All incoming ARPs are seen by bond_handle_frame, even vlan ones, definitely true.
>>>
>>>But as some of them *are* vlan ones, you will have to untag those "by hand" inside bond_handle_frame.
>>
>
>>Hmm. For hw vlan accel this would not be needed.
>
>Agreed.
>
>>But for non-hw-vlan-accel the frame is wrapped when going thru handle_frame. And yes, in that
>>case untagging would be necessary. Unless the vlan untagging happening now in ptype_base handler
>>is moved in rx path somewhere before __netif_receive_skb.
>
>Can't it me moved not before, but inside __netif_receive_skb, as a rx_handler?

I don't think so - rx_handler is not right place to untag. rx_handler is
per device. untaging should happen idealy on realdev skb injection. That
way the rx path for hw-vlan-accel and non-he-vlan-accel would be very
similar.

>
>At the time we setup eth0.100, we can arrange for a vlan_untag
>rx_handler to be registered on eth0. This is exactly the way it works
>now for macvlan. Should it be possible (and desirable) for vlan too?
>
>This might re-open the discussion about the need for several
>rx_handlers per interface, but that is another story.
>
>Also, moving it before __netif_receive_skb would cause every protocol
>handlers to receive the frame untagged. At least protocol sniffers
>registered at ptype_all may want to receive the tagged frame, for
>diagnostic purpose.

Thats how its done for hw-vlan-accel. No problem here.

>
>That would result in two things:
>>
>>1) Bond would be able to scope vlan packets
>>2) The rx path for hw-vlan-accel and non-hw-vlan-accel would be the same
>>    (should be desirable + one reinjection would be avoided for
>>     non-hw-vlan-accel)
>
>Agreed, but moving it inside __netif_receive_skb should have the same
>effects. If we move non-hw-accel-vlan to a rx_handler, the skb would
>be COW before being untagged only if shared. This sounds good to me.
>
>	Nicolas.

^ permalink raw reply

* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-09 16:51 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: Michael S. Tsirkin, Rusty Russell, Krishna Kumar2, David Miller,
	kvm, netdev, steved
In-Reply-To: <201103091009.27932.tahm@linux.vnet.ibm.com>

On Wed, 2011-03-09 at 10:09 -0600, Tom Lendacky wrote:
> > > Vhost is receiving a lot of notifications for packets that are to
> be
> > > transmitted (over 60% of the packets generate a kick_notify). 

This is guest TX send notification when vhost enables notification.

In TCP_STREAM test, vhost exits from reaching NAPI WEIGHT, it rarely
enables the notification, vhost re-enters handle_tx from NAPI poll, so
guest doesn't do much kick_notify.

In multiple TCP_RR test, seems vhost exits from nothing to send in TX vq
very often, so it enables notification most of the time.

Shirley


^ permalink raw reply

* Re: [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state
From: Adam Majer @ 2011-03-09 16:44 UTC (permalink / raw)
  To: Américo Wang; +Cc: netdev, Stephen Hemminger, bridge, linux-kernel
In-Reply-To: <20110309150949.GE16951@cr0.redhat.com>

On Wed, Mar 09, 2011 at 11:09:49PM +0800, Américo Wang wrote:
> On Sat, Mar 05, 2011 at 11:18:33PM -0600, Adam Majer wrote:
> >
> >IPv6 address autoconfiguration relies on an UP interface to processes
> >traffic normally. A bridge that is UP enters LEARNING state and this
> >state "eats" any Router Advertising packets sent to the
> >bridge. Issuing a NETDEV_CHANGE notification on the bridge interface
> >when it changes state allows autoconfiguration code to retry
> >querying router information.
> >
> ...
> 
> >+static void br_port_change_notifier_handler(struct work_struct *work)
> >+{
> >+	struct net_bridge *br = container_of(work,
> >+					     struct net_bridge,
> >+					     change_notification_worker);
> >+
> >+	rtnl_lock();
> >+	netdev_state_change(br->dev);
> >+	rtnl_unlock();
> >+}
> >+
> 
> Do you really want user-space to get this notification too?
> Why do you put it into a workqueue? Maybe it has to be called in
> process-context?

Stephen Hemminger's patch (posted as a reply in this thread but only
on the netdev mailing list) is much better as it only brings the link
up on the bridge interface when the bridge enters forwarding mode.

Effectively it does similar thing via NETDEV_CHANGE but requires no
other hacks in other code. It should make userland much happier too.


As for my patch, I've put that in the workqueue because the
notification cannot be called in interrupt context. Is there a better
way of calling code that can sleep from interrupt context?

- Adam

-- 
Adam Majer
adamm@zombino.com

^ permalink raw reply

* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-09 16:38 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, Krishna Kumar2, David Miller, kvm, netdev, steved,
	Tom Lendacky
In-Reply-To: <20110309163201.GC7165@redhat.com>

On Wed, 2011-03-09 at 18:32 +0200, Michael S. Tsirkin wrote:
> I think your issues are with TX overrun.
> Besides delaying IRQ on TX, I don't have many ideas.
> 
> The one interesting thing is that you see better speed
> if you drop packets. netdev crowd says this should not happen,
> so could be an indicator of a problem somewhere.

Yes, I am looking at why guest didn't see see used_buffers on time from
vhost send TX completion I am trying to collect some data on vhost.

I also wonder whether it's a scheduler issue.

Thanks
Shirley


^ permalink raw reply

* Re: Network performance with small packets - continued
From: Michael S. Tsirkin @ 2011-03-09 16:32 UTC (permalink / raw)
  To: Shirley Ma
  Cc: Rusty Russell, Krishna Kumar2, David Miller, kvm, netdev, steved,
	Tom Lendacky
In-Reply-To: <1299687934.25664.108.camel@localhost.localdomain>

On Wed, Mar 09, 2011 at 08:25:34AM -0800, Shirley Ma wrote:
> On Wed, 2011-03-09 at 18:10 +0200, Michael S. Tsirkin wrote:
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 82dba5a..4477b9a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct
> > virtnet_info *vi)
> >         struct sk_buff *skb;
> >         unsigned int len, tot_sgs = 0;
> > 
> > -       while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> > +       if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> >                 pr_debug("Sent skb %p\n", skb);
> >                 vi->dev->stats.tx_bytes += skb->len;
> >                 vi->dev->stats.tx_packets++;
> > -               tot_sgs += skb_vnet_hdr(skb)->num_sg;
> > +               tot_sgs = 2+MAX_SKB_FRAGS;
> >                 dev_kfree_skb_any(skb);
> >         }
> >         return tot_sgs;
> > @@ -576,7 +576,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> > struct net_device *dev)
> >         struct virtnet_info *vi = netdev_priv(dev);
> >         int capacity;
> > 
> > -       /* Free up any pending old buffers before queueing new ones.
> > */
> > +       /* Free up any old buffers so we can queue new ones. */
> >         free_old_xmit_skbs(vi);
> > 
> >         /* Try to transmit */
> > @@ -605,6 +605,10 @@ static netdev_tx_t start_xmit(struct sk_buff
> > *skb, struct net_device *dev)
> >         skb_orphan(skb);
> >         nf_reset(skb);
> > 
> > +       /* Free up any old buffers so we can queue new ones. */
> > +       if (capacity < 2+MAX_SKB_FRAGS)
> > +               capacity += free_old_xmit_skbs(vi);
> > +
> >         /* Apparently nice girls don't return TX_BUSY; stop the queue
> >          * before it gets out of hand.  Naturally, this wastes
> > entries. */
> >         if (capacity < 2+MAX_SKB_FRAGS) {
> > -- 
> 
> I tried this one as well. It might improve TCP_RR performance but not
> TCP_STREAM. :) Let's wait for Tom's TCP_RR resutls.
> 
> Thanks
> Shirley

I think your issues are with TX overrun.
Besides delaying IRQ on TX, I don't have many ideas.

The one interesting thing is that you see better speed
if you drop packets. netdev crowd says this should not happen,
so could be an indicator of a problem somewhere.


-- 
MST

^ permalink raw reply

* Re: Network performance with small packets - continued
From: Michael S. Tsirkin @ 2011-03-09 16:28 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: Shirley Ma, Rusty Russell, Krishna Kumar2, David Miller, kvm,
	netdev, steved
In-Reply-To: <201103091009.27932.tahm@linux.vnet.ibm.com>

On Wed, Mar 09, 2011 at 10:09:26AM -0600, Tom Lendacky wrote:
> On Wednesday, March 09, 2011 01:15:58 am Michael S. Tsirkin wrote:
> > On Mon, Mar 07, 2011 at 04:31:41PM -0600, Tom Lendacky wrote:
> > > We've been doing some more experimenting with the small packet network
> > > performance problem in KVM.  I have a different setup than what Steve D.
> > > was using so I re-baselined things on the kvm.git kernel on both the
> > > host and guest with a 10GbE adapter.  I also made use of the
> > > virtio-stats patch.
> > > 
> > > The virtual machine has 2 vCPUs, 8GB of memory and two virtio network
> > > adapters (the first connected to a 1GbE adapter and a LAN, the second
> > > connected to a 10GbE adapter that is direct connected to another system
> > > with the same 10GbE adapter) running the kvm.git kernel.  The test was a
> > > TCP_RR test with 100 connections from a baremetal client to the KVM
> > > guest using a 256 byte message size in both directions.

One thing that might be happening is that we are out of
atomic memory poll in guest, so indirect allocations
start failing, and this is slow path.
Could you check this please?


> > > I used the uperf tool to do this after verifying the results against
> > > netperf. Uperf allows the specification of the number of connections as
> > > a parameter in an XML file as opposed to launching, in this case, 100
> > > separate instances of netperf.
> > > 
> > > Here is the baseline for baremetal using 2 physical CPUs:
> > >   Txn Rate: 206,389.59 Txn/Sec, Pkt Rate: 410,048 Pkts/Sec
> > >   TxCPU: 7.88%  RxCPU: 99.41%
> > > 
> > > To be sure to get consistent results with KVM I disabled the
> > > hyperthreads, pinned the qemu-kvm process, vCPUs, vhost thread and
> > > ethernet adapter interrupts (this resulted in runs that differed by only
> > > about 2% from lowest to highest).  The fact that pinning is required to
> > > get consistent results is a different problem that we'll have to look
> > > into later...
> > > 
> > > Here is the KVM baseline (average of six runs):
> > >   Txn Rate: 87,070.34 Txn/Sec, Pkt Rate: 172,992 Pkts/Sec
> > >   Exits: 148,444.58 Exits/Sec
> > >   TxCPU: 2.40%  RxCPU: 99.35%
> > > 
> > > About 42% of baremetal.
> > 
> > Can you add interrupt stats as well please?
> 
> Yes I can.  Just the guest interrupts for the virtio device?

Guess so: tx and rx.

> > 
> > > empty.  So I coded a quick patch to delay freeing of the used Tx buffers
> > > until more than half the ring was used (I did not test this under a
> > > stream condition so I don't know if this would have a negative impact). 
> > > Here are the results
> > > 
> > > from delaying the freeing of used Tx buffers (average of six runs):
> > >   Txn Rate: 90,886.19 Txn/Sec, Pkt Rate: 180,571 Pkts/Sec
> > >   Exits: 142,681.67 Exits/Sec
> > >   TxCPU: 2.78%  RxCPU: 99.36%
> > > 
> > > About a 4% increase over baseline and about 44% of baremetal.
> > 
> > Hmm, I am not sure what you mean by delaying freeing.
> 
> In the start_xmit function of virtio_net.c the first thing done is to free any 
> used entries from the ring.  I patched the code to track the number of used tx 
> ring entries and only free the used entries when they are greater than half 
> the capacity of the ring (similar to the way the rx ring is re-filled).

We don't even need than: just max skb frag + 2.
Also don't need to free them all: just enough to get
place for  max skb frag + 2 entries.

> > I think we do have a problem that free_old_xmit_skbs
> > tries to flush out the ring aggressively:
> > it always polls until the ring is empty,
> > so there could be bursts of activity where
> > we spend a lot of time flushing the old entries
> > before e.g. sending an ack, resulting in
> > latency bursts.
> > 
> > Generally we'll need some smarter logic,
> > but with indirect at the moment we can just poll
> > a single packet after we post a new one, and be done with it.
> > Is your patch something like the patch below?
> > Could you try mine as well please?
> 
> Yes, I'll try the patch and post the results.
> 
> > 
> > > This spread out the kick_notify but still resulted in alot of them.  I
> > > decided to build on the delayed Tx buffer freeing and code up an
> > > "ethtool" like coalescing patch in order to delay the kick_notify until
> > > there were at least 5 packets on the ring or 2000 usecs, whichever
> > > occurred first.  Here are the
> > > 
> > > results of delaying the kick_notify (average of six runs):
> > >   Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
> > >   Exits: 102,587.28 Exits/Sec
> > >   TxCPU: 3.03%  RxCPU: 99.33%
> > > 
> > > About a 23% increase over baseline and about 52% of baremetal.
> > > 
> > > Running the perf command against the guest I noticed almost 19% of the
> > > time being spent in _raw_spin_lock.  Enabling lockstat in the guest
> > > showed alot of contention in the "irq_desc_lock_class". Pinning the
> > > virtio1-input interrupt to a single cpu in the guest and re-running the
> > > last test resulted in
> > > 
> > > tremendous gains (average of six runs):
> > >   Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
> > >   Exits: 62,603.37 Exits/Sec
> > >   TxCPU: 3.73%  RxCPU: 98.52%
> > > 
> > > About a 77% increase over baseline and about 74% of baremetal.
> > > 
> > > Vhost is receiving a lot of notifications for packets that are to be
> > > transmitted (over 60% of the packets generate a kick_notify).  Also, it
> > > looks like vhost is sending a lot of notifications for packets it has
> > > received before the guest can get scheduled to disable notifications and
> > > begin processing the packets
> > 
> > Hmm, is this really what happens to you?  The effect would be that guest
> > gets an interrupt while notifications are disabled in guest, right? Could
> > you add a counter and check this please?
> 
> The disabling of the interrupt/notifications is done by the guest.  So the 
> guest has to get scheduled and handle the notification before it disables 
> them.  The vhost_signal routine will keep injecting an interrupt until this 
> happens causing the contention in the guest.  I'll try the patches you specify 
> below and post the results.  They look like they should take care of this 
> issue.
> 
> > 
> > Another possible thing to try would be these old patches to publish used
> > index from guest to make sure this double interrupt does not happen:
> >  [PATCHv2] virtio: put last seen used index into ring itself
> >  [PATCHv2] vhost-net: utilize PUBLISH_USED_IDX feature
> > 
> > > resulting in some lock contention in the guest (and
> > > high interrupt rates).
> > > 
> > > Some thoughts for the transmit path...  can vhost be enhanced to do some
> > > adaptive polling so that the number of kick_notify events are reduced and
> > > replaced by kick_no_notify events?
> > 
> > Worth a try.
> > 
> > > Comparing the transmit path to the receive path, the guest disables
> > > notifications after the first kick and vhost re-enables notifications
> > > after completing processing of the tx ring.
> > 
> > Is this really what happens? I though the host disables notifications
> > after the first kick.
> 
> Yup, sorry for the confusion.  The kick is done by the guest and then vhost 
> disables notifications.  Maybe a similar approach to the above patches of 
> checking the used index in the virtio_net driver could also help here?

If this happens there will be many kicks that find an empty ring.
Could you add a counter in vhost and check please?

-- 
MST

^ permalink raw reply

* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-09 16:25 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, Krishna Kumar2, David Miller, kvm, netdev, steved,
	Tom Lendacky
In-Reply-To: <20110309161013.GA7165@redhat.com>

On Wed, 2011-03-09 at 18:10 +0200, Michael S. Tsirkin wrote:
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 82dba5a..4477b9a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct
> virtnet_info *vi)
>         struct sk_buff *skb;
>         unsigned int len, tot_sgs = 0;
> 
> -       while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> +       if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
>                 pr_debug("Sent skb %p\n", skb);
>                 vi->dev->stats.tx_bytes += skb->len;
>                 vi->dev->stats.tx_packets++;
> -               tot_sgs += skb_vnet_hdr(skb)->num_sg;
> +               tot_sgs = 2+MAX_SKB_FRAGS;
>                 dev_kfree_skb_any(skb);
>         }
>         return tot_sgs;
> @@ -576,7 +576,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> struct net_device *dev)
>         struct virtnet_info *vi = netdev_priv(dev);
>         int capacity;
> 
> -       /* Free up any pending old buffers before queueing new ones.
> */
> +       /* Free up any old buffers so we can queue new ones. */
>         free_old_xmit_skbs(vi);
> 
>         /* Try to transmit */
> @@ -605,6 +605,10 @@ static netdev_tx_t start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>         skb_orphan(skb);
>         nf_reset(skb);
> 
> +       /* Free up any old buffers so we can queue new ones. */
> +       if (capacity < 2+MAX_SKB_FRAGS)
> +               capacity += free_old_xmit_skbs(vi);
> +
>         /* Apparently nice girls don't return TX_BUSY; stop the queue
>          * before it gets out of hand.  Naturally, this wastes
> entries. */
>         if (capacity < 2+MAX_SKB_FRAGS) {
> -- 

I tried this one as well. It might improve TCP_RR performance but not
TCP_STREAM. :) Let's wait for Tom's TCP_RR resutls.

Thanks
Shirley


^ permalink raw reply

* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-09 16:21 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: Michael S. Tsirkin, Rusty Russell, Krishna Kumar2, David Miller,
	kvm, netdev, steved
In-Reply-To: <201103091009.27932.tahm@linux.vnet.ibm.com>

On Wed, 2011-03-09 at 10:09 -0600, Tom Lendacky wrote:
> > 
> > > This spread out the kick_notify but still resulted in alot of
> them.  I
> > > decided to build on the delayed Tx buffer freeing and code up an
> > > "ethtool" like coalescing patch in order to delay the kick_notify
> until
> > > there were at least 5 packets on the ring or 2000 usecs, whichever
> > > occurred first.  Here are the
> > > 
> > > results of delaying the kick_notify (average of six runs):
> > >   Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
> > >   Exits: 102,587.28 Exits/Sec
> > >   TxCPU: 3.03%  RxCPU: 99.33%
> > > 
> > > About a 23% increase over baseline and about 52% of baremetal.
> > > 
> > > Running the perf command against the guest I noticed almost 19% of
> the
> > > time being spent in _raw_spin_lock.  Enabling lockstat in the
> guest
> > > showed alot of contention in the "irq_desc_lock_class". Pinning
> the
> > > virtio1-input interrupt to a single cpu in the guest and
> re-running the
> > > last test resulted in
> > > 
> > > tremendous gains (average of six runs):
> > >   Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
> > >   Exits: 62,603.37 Exits/Sec
> > >   TxCPU: 3.73%  RxCPU: 98.52%
> > > 
> > > About a 77% increase over baseline and about 74% of baremetal.
> > > 
> > > Vhost is receiving a lot of notifications for packets that are to
> be
> > > transmitted (over 60% of the packets generate a kick_notify).
> Also, it
> > > looks like vhost is sending a lot of notifications for packets it
> has
> > > received before the guest can get scheduled to disable
> notifications and
> > > begin processing the packets
> > 
> > Hmm, is this really what happens to you?  The effect would be that
> guest
> > gets an interrupt while notifications are disabled in guest, right?
> Could
> > you add a counter and check this please?
> 
> The disabling of the interrupt/notifications is done by the guest.  So
> the 
> guest has to get scheduled and handle the notification before it
> disables 
> them.  The vhost_signal routine will keep injecting an interrupt until
> this 
> happens causing the contention in the guest.  I'll try the patches you
> specify 
> below and post the results.  They look like they should take care of
> this 
> issue.

In guest TX path, the guest interrupt should be disabled in the start
since it free_old_xmit_skbs in start_xmit call, it's not necessary to
receive any send completion interrupts to handle free old skbs. Then the
interrupt is only enabled when the netif queue is full. For multiple
streams TCP_RR test, we never hit netif queue full situation, the
cat /proc/interrupts/ send completion interrupts rate is 0, right?

Shirley


^ permalink raw reply

* Re: Network performance with small packets - continued
From: Michael S. Tsirkin @ 2011-03-09 16:10 UTC (permalink / raw)
  To: Shirley Ma
  Cc: Rusty Russell, Krishna Kumar2, David Miller, kvm, netdev, steved,
	Tom Lendacky
In-Reply-To: <1299685543.25664.97.camel@localhost.localdomain>

On Wed, Mar 09, 2011 at 07:45:43AM -0800, Shirley Ma wrote:
> On Wed, 2011-03-09 at 09:15 +0200, Michael S. Tsirkin wrote:
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 82dba5a..ebe3337 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct
> > virtnet_info *vi)
> >         struct sk_buff *skb;
> >         unsigned int len, tot_sgs = 0;
> > 
> > -       while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> > +       if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> >                 pr_debug("Sent skb %p\n", skb);
> >                 vi->dev->stats.tx_bytes += skb->len;
> >                 vi->dev->stats.tx_packets++;
> > -               tot_sgs += skb_vnet_hdr(skb)->num_sg;
> > +               tot_sgs = 2+MAX_SKB_FRAGS;
> >                 dev_kfree_skb_any(skb);
> >         }
> >         return tot_sgs;
> 
> Return value should be different based on indirect or direct buffers
> here?

Something like that. Or we can assume no indirect, worst-case.
But just for testing, I think it should work as an estimation.

> > @@ -576,9 +576,6 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> > struct net_device *dev)
> >         struct virtnet_info *vi = netdev_priv(dev);
> >         int capacity;
> > 
> > -       /* Free up any pending old buffers before queueing new ones.
> > */
> > -       free_old_xmit_skbs(vi);
> > -
> >         /* Try to transmit */
> >         capacity = xmit_skb(vi, skb);
> > 
> > @@ -605,6 +602,10 @@ static netdev_tx_t start_xmit(struct sk_buff
> > *skb, struct net_device *dev)
> >         skb_orphan(skb);
> >         nf_reset(skb);
> > 
> > +       /* Free up any old buffers so we can queue new ones. */
> > +       if (capacity < 2+MAX_SKB_FRAGS)
> > +               capacity += free_old_xmit_skbs(vi);
> > +
> >         /* Apparently nice girls don't return TX_BUSY; stop the queue
> >          * before it gets out of hand.  Naturally, this wastes
> > entries. */
> >         if (capacity < 2+MAX_SKB_FRAGS) { 
> 
> I tried a similar patch before, it didn't help much on TCP stream
> performance. But I didn't try multiple stream TCP_RR.
> 
> Shirley

There's a bug in myh patch by the way. Pls try the following
instead (still untested).

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 82dba5a..4477b9a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
 	struct sk_buff *skb;
 	unsigned int len, tot_sgs = 0;
 
-	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+	if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
-		tot_sgs += skb_vnet_hdr(skb)->num_sg;
+		tot_sgs = 2+MAX_SKB_FRAGS;
 		dev_kfree_skb_any(skb);
 	}
 	return tot_sgs;
@@ -576,7 +576,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct virtnet_info *vi = netdev_priv(dev);
 	int capacity;
 
-	/* Free up any pending old buffers before queueing new ones. */
+	/* Free up any old buffers so we can queue new ones. */
 	free_old_xmit_skbs(vi);
 
 	/* Try to transmit */
@@ -605,6 +605,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	skb_orphan(skb);
 	nf_reset(skb);
 
+	/* Free up any old buffers so we can queue new ones. */
+	if (capacity < 2+MAX_SKB_FRAGS)
+		capacity += free_old_xmit_skbs(vi);
+
 	/* Apparently nice girls don't return TX_BUSY; stop the queue
 	 * before it gets out of hand.  Naturally, this wastes entries. */
 	if (capacity < 2+MAX_SKB_FRAGS) {

^ permalink raw reply related

* Re: Network performance with small packets - continued
From: Tom Lendacky @ 2011-03-09 16:09 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Shirley Ma, Rusty Russell, Krishna Kumar2, David Miller, kvm,
	netdev, steved
In-Reply-To: <20110309071558.GA25757@redhat.com>

On Wednesday, March 09, 2011 01:15:58 am Michael S. Tsirkin wrote:
> On Mon, Mar 07, 2011 at 04:31:41PM -0600, Tom Lendacky wrote:
> > We've been doing some more experimenting with the small packet network
> > performance problem in KVM.  I have a different setup than what Steve D.
> > was using so I re-baselined things on the kvm.git kernel on both the
> > host and guest with a 10GbE adapter.  I also made use of the
> > virtio-stats patch.
> > 
> > The virtual machine has 2 vCPUs, 8GB of memory and two virtio network
> > adapters (the first connected to a 1GbE adapter and a LAN, the second
> > connected to a 10GbE adapter that is direct connected to another system
> > with the same 10GbE adapter) running the kvm.git kernel.  The test was a
> > TCP_RR test with 100 connections from a baremetal client to the KVM
> > guest using a 256 byte message size in both directions.
> > 
> > I used the uperf tool to do this after verifying the results against
> > netperf. Uperf allows the specification of the number of connections as
> > a parameter in an XML file as opposed to launching, in this case, 100
> > separate instances of netperf.
> > 
> > Here is the baseline for baremetal using 2 physical CPUs:
> >   Txn Rate: 206,389.59 Txn/Sec, Pkt Rate: 410,048 Pkts/Sec
> >   TxCPU: 7.88%  RxCPU: 99.41%
> > 
> > To be sure to get consistent results with KVM I disabled the
> > hyperthreads, pinned the qemu-kvm process, vCPUs, vhost thread and
> > ethernet adapter interrupts (this resulted in runs that differed by only
> > about 2% from lowest to highest).  The fact that pinning is required to
> > get consistent results is a different problem that we'll have to look
> > into later...
> > 
> > Here is the KVM baseline (average of six runs):
> >   Txn Rate: 87,070.34 Txn/Sec, Pkt Rate: 172,992 Pkts/Sec
> >   Exits: 148,444.58 Exits/Sec
> >   TxCPU: 2.40%  RxCPU: 99.35%
> > 
> > About 42% of baremetal.
> 
> Can you add interrupt stats as well please?

Yes I can.  Just the guest interrupts for the virtio device?

> 
> > empty.  So I coded a quick patch to delay freeing of the used Tx buffers
> > until more than half the ring was used (I did not test this under a
> > stream condition so I don't know if this would have a negative impact). 
> > Here are the results
> > 
> > from delaying the freeing of used Tx buffers (average of six runs):
> >   Txn Rate: 90,886.19 Txn/Sec, Pkt Rate: 180,571 Pkts/Sec
> >   Exits: 142,681.67 Exits/Sec
> >   TxCPU: 2.78%  RxCPU: 99.36%
> > 
> > About a 4% increase over baseline and about 44% of baremetal.
> 
> Hmm, I am not sure what you mean by delaying freeing.

In the start_xmit function of virtio_net.c the first thing done is to free any 
used entries from the ring.  I patched the code to track the number of used tx 
ring entries and only free the used entries when they are greater than half 
the capacity of the ring (similar to the way the rx ring is re-filled).

> I think we do have a problem that free_old_xmit_skbs
> tries to flush out the ring aggressively:
> it always polls until the ring is empty,
> so there could be bursts of activity where
> we spend a lot of time flushing the old entries
> before e.g. sending an ack, resulting in
> latency bursts.
> 
> Generally we'll need some smarter logic,
> but with indirect at the moment we can just poll
> a single packet after we post a new one, and be done with it.
> Is your patch something like the patch below?
> Could you try mine as well please?

Yes, I'll try the patch and post the results.

> 
> > This spread out the kick_notify but still resulted in alot of them.  I
> > decided to build on the delayed Tx buffer freeing and code up an
> > "ethtool" like coalescing patch in order to delay the kick_notify until
> > there were at least 5 packets on the ring or 2000 usecs, whichever
> > occurred first.  Here are the
> > 
> > results of delaying the kick_notify (average of six runs):
> >   Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
> >   Exits: 102,587.28 Exits/Sec
> >   TxCPU: 3.03%  RxCPU: 99.33%
> > 
> > About a 23% increase over baseline and about 52% of baremetal.
> > 
> > Running the perf command against the guest I noticed almost 19% of the
> > time being spent in _raw_spin_lock.  Enabling lockstat in the guest
> > showed alot of contention in the "irq_desc_lock_class". Pinning the
> > virtio1-input interrupt to a single cpu in the guest and re-running the
> > last test resulted in
> > 
> > tremendous gains (average of six runs):
> >   Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
> >   Exits: 62,603.37 Exits/Sec
> >   TxCPU: 3.73%  RxCPU: 98.52%
> > 
> > About a 77% increase over baseline and about 74% of baremetal.
> > 
> > Vhost is receiving a lot of notifications for packets that are to be
> > transmitted (over 60% of the packets generate a kick_notify).  Also, it
> > looks like vhost is sending a lot of notifications for packets it has
> > received before the guest can get scheduled to disable notifications and
> > begin processing the packets
> 
> Hmm, is this really what happens to you?  The effect would be that guest
> gets an interrupt while notifications are disabled in guest, right? Could
> you add a counter and check this please?

The disabling of the interrupt/notifications is done by the guest.  So the 
guest has to get scheduled and handle the notification before it disables 
them.  The vhost_signal routine will keep injecting an interrupt until this 
happens causing the contention in the guest.  I'll try the patches you specify 
below and post the results.  They look like they should take care of this 
issue.

> 
> Another possible thing to try would be these old patches to publish used
> index from guest to make sure this double interrupt does not happen:
>  [PATCHv2] virtio: put last seen used index into ring itself
>  [PATCHv2] vhost-net: utilize PUBLISH_USED_IDX feature
> 
> > resulting in some lock contention in the guest (and
> > high interrupt rates).
> > 
> > Some thoughts for the transmit path...  can vhost be enhanced to do some
> > adaptive polling so that the number of kick_notify events are reduced and
> > replaced by kick_no_notify events?
> 
> Worth a try.
> 
> > Comparing the transmit path to the receive path, the guest disables
> > notifications after the first kick and vhost re-enables notifications
> > after completing processing of the tx ring.
> 
> Is this really what happens? I though the host disables notifications
> after the first kick.

Yup, sorry for the confusion.  The kick is done by the guest and then vhost 
disables notifications.  Maybe a similar approach to the above patches of 
checking the used index in the virtio_net driver could also help here?

> 
> >  Can a similar thing be done for the
> > 
> > receive path?  Once vhost sends the first notification for a received
> > packet it can disable notifications and let the guest re-enable
> > notifications when it has finished processing the receive ring.  Also,
> > can the virtio-net driver do some adaptive polling (or does napi take
> > care of that for the guest)?
> 
> Worth a try. I don't think napi does anything like this.
> 
> > Running the same workload on the same configuration with a different
> > hypervisor results in performance that is almost equivalent to baremetal
> > without doing any pinning.
> > 
> > Thanks,
> > Tom Lendacky
> 
> There's no need to flush out all used buffers
> before we post more for transmit: with indirect,
> just a single one is enough. Without indirect we'll
> need more possibly, but just for testing this should
> be enough.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> Note: untested.
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 82dba5a..ebe3337 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct
> virtnet_info *vi) struct sk_buff *skb;
>  	unsigned int len, tot_sgs = 0;
> 
> -	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> +	if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
>  		pr_debug("Sent skb %p\n", skb);
>  		vi->dev->stats.tx_bytes += skb->len;
>  		vi->dev->stats.tx_packets++;
> -		tot_sgs += skb_vnet_hdr(skb)->num_sg;
> +		tot_sgs = 2+MAX_SKB_FRAGS;
>  		dev_kfree_skb_any(skb);
>  	}
>  	return tot_sgs;
> @@ -576,9 +576,6 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> struct net_device *dev) struct virtnet_info *vi = netdev_priv(dev);
>  	int capacity;
> 
> -	/* Free up any pending old buffers before queueing new ones. */
> -	free_old_xmit_skbs(vi);
> -
>  	/* Try to transmit */
>  	capacity = xmit_skb(vi, skb);
> 
> @@ -605,6 +602,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> struct net_device *dev) skb_orphan(skb);
>  	nf_reset(skb);
> 
> +	/* Free up any old buffers so we can queue new ones. */
> +	if (capacity < 2+MAX_SKB_FRAGS)
> +		capacity += free_old_xmit_skbs(vi);
> +
>  	/* Apparently nice girls don't return TX_BUSY; stop the queue
>  	 * before it gets out of hand.  Naturally, this wastes entries. */
>  	if (capacity < 2+MAX_SKB_FRAGS) {
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox