All of lore.kernel.org
 help / color / mirror / Atom feed
From: Haiyang Zhang <haiyangz@microsoft.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>,
	Simon Xiao <sixiao@microsoft.com>,
	Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tom Herbert <tom@herbertland.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	KY Srinivasan <kys@microsoft.com>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	David Miller <davem@davemloft.net>
Subject: RE: [PATCH net-next] hv_netvsc: don't make assumptions on struct flow_keys layout
Date: Fri, 8 Jan 2016 21:07:58 +0000	[thread overview]
Message-ID: <DM2PR0301MB0784EBC7C507C17E596D87C0CAF60@DM2PR0301MB0784.namprd03.prod.outlook.com> (raw)
In-Reply-To: <877fjlfrid.fsf@vitty.brq.redhat.com>



> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Thursday, January 7, 2016 8:28 AM
> To: Simon Xiao <sixiao@microsoft.com>; Eric Dumazet
> <eric.dumazet@gmail.com>
> Cc: Tom Herbert <tom@herbertland.com>; netdev@vger.kernel.org; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; David Miller
> <davem@davemloft.net>
> Subject: Re: [PATCH net-next] hv_netvsc: don't make assumptions on
> struct flow_keys layout
> 
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
> > On Thu, 2016-01-07 at 10:33 +0100, Vitaly Kuznetsov wrote:
> >> Recent changes to 'struct flow_keys' (e.g commit d34af823ff40 ("net:
> Add
> >>  VLAN ID to flow_keys")) introduced a performance regression in
> netvsc
> >> driver. Is problem is, however, not the above mentioned commit but
> the
> >> fact that netvsc_set_hash() function did some assumptions on the
> struct
> >> flow_keys data layout and this is wrong. We need to extract the data
> we
> >> need (src/dst addresses and ports) after the dissect.
> >>
> >> The issue could also be solved in a completely different way: as
> suggested
> >> by Eric instead of our own homegrown netvsc_set_hash() we could use
> >> skb_get_hash() which does more or less the same. Unfortunately, the
> >> testing done by Simon showed that Hyper-V hosts are not happy with
> our
> >> Jenkins hash, selecting the output queue with the current algorithm
> based
> >> on Toeplitz hash works significantly better.
> >
> > Were tests done on IPv6 traffic ?
> >
> 
> Simon, could you please test this patch for IPv6 and show us the numbers?
> 
> > Toeplitz hash takes at least 100 ns to hash 12 bytes (one iteration
> per
> > bit : 96 iterations)
> >
> > For IPv6 it is 3 times this, since we have to hash 36 bytes.
> >
> > I do not see how it can compete with skb_get_hash() that directly
> gives
> > skb->hash for local TCP flows.
> >
> 
> My guess is that this is not the bottleneck, something is happening
> behind the scene with out packets in Hyper-V host (e.g. re-distributing
> them to hardware queues?) but I don't know the internals, Microsoft
> folks could probably comment.

The Hyper-V vRSS protocol lets us use the Toeplitz hash algorithm. We are
currently running further tests, including IPv6 too, and will share the 
results when available.

Thanks,
- Haiyang

WARNING: multiple messages have this Message-ID (diff)
From: Haiyang Zhang <haiyangz@microsoft.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>,
	Simon Xiao <sixiao@microsoft.com>,
	Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tom Herbert <tom@herbertland.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	David Miller <davem@davemloft.net>
Subject: RE: [PATCH net-next] hv_netvsc: don't make assumptions on struct flow_keys layout
Date: Fri, 8 Jan 2016 21:07:58 +0000	[thread overview]
Message-ID: <DM2PR0301MB0784EBC7C507C17E596D87C0CAF60@DM2PR0301MB0784.namprd03.prod.outlook.com> (raw)
In-Reply-To: <877fjlfrid.fsf@vitty.brq.redhat.com>



> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Thursday, January 7, 2016 8:28 AM
> To: Simon Xiao <sixiao@microsoft.com>; Eric Dumazet
> <eric.dumazet@gmail.com>
> Cc: Tom Herbert <tom@herbertland.com>; netdev@vger.kernel.org; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; David Miller
> <davem@davemloft.net>
> Subject: Re: [PATCH net-next] hv_netvsc: don't make assumptions on
> struct flow_keys layout
> 
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
> > On Thu, 2016-01-07 at 10:33 +0100, Vitaly Kuznetsov wrote:
> >> Recent changes to 'struct flow_keys' (e.g commit d34af823ff40 ("net:
> Add
> >>  VLAN ID to flow_keys")) introduced a performance regression in
> netvsc
> >> driver. Is problem is, however, not the above mentioned commit but
> the
> >> fact that netvsc_set_hash() function did some assumptions on the
> struct
> >> flow_keys data layout and this is wrong. We need to extract the data
> we
> >> need (src/dst addresses and ports) after the dissect.
> >>
> >> The issue could also be solved in a completely different way: as
> suggested
> >> by Eric instead of our own homegrown netvsc_set_hash() we could use
> >> skb_get_hash() which does more or less the same. Unfortunately, the
> >> testing done by Simon showed that Hyper-V hosts are not happy with
> our
> >> Jenkins hash, selecting the output queue with the current algorithm
> based
> >> on Toeplitz hash works significantly better.
> >
> > Were tests done on IPv6 traffic ?
> >
> 
> Simon, could you please test this patch for IPv6 and show us the numbers?
> 
> > Toeplitz hash takes at least 100 ns to hash 12 bytes (one iteration
> per
> > bit : 96 iterations)
> >
> > For IPv6 it is 3 times this, since we have to hash 36 bytes.
> >
> > I do not see how it can compete with skb_get_hash() that directly
> gives
> > skb->hash for local TCP flows.
> >
> 
> My guess is that this is not the bottleneck, something is happening
> behind the scene with out packets in Hyper-V host (e.g. re-distributing
> them to hardware queues?) but I don't know the internals, Microsoft
> folks could probably comment.

The Hyper-V vRSS protocol lets us use the Toeplitz hash algorithm. We are
currently running further tests, including IPv6 too, and will share the 
results when available.

Thanks,
- Haiyang

  parent reply	other threads:[~2016-01-08 21:08 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-07  9:33 [PATCH net-next] hv_netvsc: don't make assumptions on struct flow_keys layout Vitaly Kuznetsov
2016-01-07  9:33 ` Vitaly Kuznetsov
2016-01-07 12:52 ` Eric Dumazet
2016-01-07 13:28   ` Vitaly Kuznetsov
2016-01-07 13:28     ` Vitaly Kuznetsov
2016-01-08  1:02     ` John Fastabend
2016-01-08  3:49       ` KY Srinivasan
2016-01-08  3:49         ` KY Srinivasan
2016-01-08  6:16         ` John Fastabend
2016-01-08  6:16           ` John Fastabend
2016-01-08 18:01           ` KY Srinivasan
2016-01-08 21:07     ` Haiyang Zhang [this message]
2016-01-08 21:07       ` Haiyang Zhang
2016-01-09  0:17   ` Tom Herbert
2016-01-09  0:17     ` Tom Herbert
2016-01-10 22:25 ` David Miller
2016-01-10 22:25   ` David Miller
2016-01-13 23:10   ` Haiyang Zhang
2016-01-13 23:10     ` Haiyang Zhang
2016-01-14  4:56     ` David Miller
2016-01-14  4:56       ` David Miller
2016-01-14 17:14     ` Tom Herbert
2016-01-14 17:14       ` Tom Herbert
2016-01-14 17:53       ` One Thousand Gnomes
2016-01-14 17:53         ` One Thousand Gnomes
2016-01-14 18:24         ` Eric Dumazet
2016-01-14 18:24           ` Eric Dumazet
2016-01-14 18:35           ` Haiyang Zhang
2016-01-14 18:35             ` Haiyang Zhang
2016-01-14 18:48             ` Tom Herbert
2016-01-14 19:15               ` Haiyang Zhang
2016-01-14 19:15                 ` Haiyang Zhang
2016-01-14 19:41                 ` Tom Herbert
2016-01-14 20:23                   ` Haiyang Zhang
2016-01-14 20:23                     ` Haiyang Zhang
2016-01-14 21:44                     ` Tom Herbert
2016-01-14 21:44                       ` Tom Herbert
2016-01-14 22:06                       ` David Miller
2016-01-14 22:08                     ` Eric Dumazet
2016-01-14 22:08                       ` Eric Dumazet
2016-01-14 22:29                       ` Haiyang Zhang
2016-01-14 22:29                         ` Haiyang Zhang
2016-01-14 17:53     ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM2PR0301MB0784EBC7C507C17E596D87C0CAF60@DM2PR0301MB0784.namprd03.prod.outlook.com \
    --to=haiyangz@microsoft.com \
    --cc=davem@davemloft.net \
    --cc=devel@linuxdriverproject.org \
    --cc=eric.dumazet@gmail.com \
    --cc=kys@microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sixiao@microsoft.com \
    --cc=tom@herbertland.com \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.