Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1293097 > unrolled thread

[PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

Started byHaiyang Zhang <haiyangz@microsoft.com>
First post2015-12-16 17:40 +0100
Last post2015-12-16 19:50 +0100
Articles 7 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers Haiyang Zhang <haiyangz@microsoft.com> - 2015-12-16 17:40 +0100
    Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6  headers Eric Dumazet <eric.dumazet@gmail.com> - 2015-12-16 18:10 +0100
      RE: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6  headers Haiyang Zhang <haiyangz@microsoft.com> - 2015-12-16 21:00 +0100
        Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6  headers Eric Dumazet <eric.dumazet@gmail.com> - 2015-12-16 22:20 +0100
        Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6  headers David Miller <davem@davemloft.net> - 2015-12-17 00:30 +0100
    Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6  headers Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-16 19:40 +0100
      Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6  headers Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-16 19:50 +0100

#1293097 — [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromHaiyang Zhang <haiyangz@microsoft.com>
Date2015-12-16 17:40 +0100
Subject[PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGnho-4bq-23@gated-at.bofh.it>
To avoid performance overhead when using skb_flow_dissect_flow_keys(),
we switch to the simple parsers to get the IP and port numbers.

Performance comparison: throughput (Gbps):
Number of connections, before patch, after patch
1			8.56		10.18
4			11.17		14.07
16			12.21		21.78
64			18.71		32.08
256			15.92		26.32
1024			8.41		15.49
3000			7.82		11.58

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Tested-by: Simon Xiao <sixiao@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c |   38 +++++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1c8db9a..e28951f 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -237,20 +237,40 @@ static u32 comp_hash(u8 *key, int klen, void *data, int dlen)
 
 static bool netvsc_set_hash(u32 *hash, struct sk_buff *skb)
 {
-	struct flow_keys flow;
+	struct iphdr *iphdr;
+	struct ipv6hdr *ipv6hdr;
+	__be32 dbuf[9];
 	int data_len;
 
-	if (!skb_flow_dissect_flow_keys(skb, &flow, 0) ||
-	    !(flow.basic.n_proto == htons(ETH_P_IP) ||
-	      flow.basic.n_proto == htons(ETH_P_IPV6)))
+	if (eth_hdr(skb)->h_proto != htons(ETH_P_IP) &&
+	    eth_hdr(skb)->h_proto != htons(ETH_P_IPV6))
 		return false;
 
-	if (flow.basic.ip_proto == IPPROTO_TCP)
-		data_len = 12;
-	else
-		data_len = 8;
+	iphdr = ip_hdr(skb);
+	ipv6hdr = ipv6_hdr(skb);
+
+	if (iphdr->version == 4) {
+		dbuf[0] = iphdr->saddr;
+		dbuf[1] = iphdr->daddr;
+		if (iphdr->protocol == IPPROTO_TCP) {
+			dbuf[2] = *(__be32 *)&tcp_hdr(skb)->source;
+			data_len = 12;
+		} else {
+			data_len = 8;
+		}
+	} else if (ipv6hdr->version == 6) {
+		memcpy(dbuf, &ipv6hdr->saddr, 32);
+		if (ipv6hdr->nexthdr == IPPROTO_TCP) {
+			dbuf[8] = *(__be32 *)&tcp_hdr(skb)->source;
+			data_len = 36;
+		} else {
+			data_len = 32;
+		}
+	} else {
+		return false;
+	}
 
-	*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, &flow, data_len);
+	*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, data_len);
 
 	return true;
 }
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1293131 — Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromEric Dumazet <eric.dumazet@gmail.com>
Date2015-12-16 18:10 +0100
SubjectRe: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGnKq-4Bl-33@gated-at.bofh.it>
In reply to#1293097
On Wed, 2015-12-16 at 10:03 -0800, Haiyang Zhang wrote:
> To avoid performance overhead when using skb_flow_dissect_flow_keys(),
> we switch to the simple parsers to get the IP and port numbers.
> 
> Performance comparison: throughput (Gbps):
> Number of connections, before patch, after patch
> 1			8.56		10.18
> 4			11.17		14.07
> 16			12.21		21.78
> 64			18.71		32.08
> 256			15.92		26.32
> 1024			8.41		15.49
> 3000			7.82		11.58
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Tested-by: Simon Xiao <sixiao@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/net/hyperv/netvsc_drv.c |   38 +++++++++++++++++++++++++++++---------
>  1 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1c8db9a..e28951f 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -237,20 +237,40 @@ static u32 comp_hash(u8 *key, int klen, void *data, int dlen)
>  
>  static bool netvsc_set_hash(u32 *hash, struct sk_buff *skb)
>  {
> -	struct flow_keys flow;
> +	struct iphdr *iphdr;
> +	struct ipv6hdr *ipv6hdr;
> +	__be32 dbuf[9];
>  	int data_len;
>  
> -	if (!skb_flow_dissect_flow_keys(skb, &flow, 0) ||
> -	    !(flow.basic.n_proto == htons(ETH_P_IP) ||
> -	      flow.basic.n_proto == htons(ETH_P_IPV6)))
> +	if (eth_hdr(skb)->h_proto != htons(ETH_P_IP) &&
> +	    eth_hdr(skb)->h_proto != htons(ETH_P_IPV6))
>  		return false;
>  
> -	if (flow.basic.ip_proto == IPPROTO_TCP)
> -		data_len = 12;
> -	else
> -		data_len = 8;
> +	iphdr = ip_hdr(skb);
> +	ipv6hdr = ipv6_hdr(skb);
> +
> +	if (iphdr->version == 4) {
> +		dbuf[0] = iphdr->saddr;
> +		dbuf[1] = iphdr->daddr;
> +		if (iphdr->protocol == IPPROTO_TCP) {
> +			dbuf[2] = *(__be32 *)&tcp_hdr(skb)->source;
> +			data_len = 12;
> +		} else {
> +			data_len = 8;
> +		}
> +	} else if (ipv6hdr->version == 6) {
> +		memcpy(dbuf, &ipv6hdr->saddr, 32);
> +		if (ipv6hdr->nexthdr == IPPROTO_TCP) {
> +			dbuf[8] = *(__be32 *)&tcp_hdr(skb)->source;
> +			data_len = 36;
> +		} else {
> +			data_len = 32;
> +		}
> +	} else {
> +		return false;
> +	}
>  
> -	*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, &flow, data_len);
> +	*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, data_len);
>  
>  	return true;
>  }


This looks very very wrong to me.

How many times this is called per second, for the 'one flow' case ?

Don't you use TSO in this driver ?

What about encapsulation ?

I suspect you have a quite different issue here.

You simply could use skb_get_hash() since local TCP flows will provide a
l4 skb->hash and you have no further flow dissection to do.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293332 — RE: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromHaiyang Zhang <haiyangz@microsoft.com>
Date2015-12-16 21:00 +0100
SubjectRE: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGqoV-64C-1@gated-at.bofh.it>
In reply to#1293131
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBFcmljIER1bWF6ZXQgW21haWx0
bzplcmljLmR1bWF6ZXRAZ21haWwuY29tXQ0KPiBTZW50OiBXZWRuZXNkYXksIERlY2VtYmVyIDE2
LCAyMDE1IDEyOjA4IFBNDQo+IA0KPiBUaGlzIGxvb2tzIHZlcnkgdmVyeSB3cm9uZyB0byBtZS4N
Cj4gDQo+IEhvdyBtYW55IHRpbWVzIHRoaXMgaXMgY2FsbGVkIHBlciBzZWNvbmQsIGZvciB0aGUg
J29uZSBmbG93JyBjYXNlID8NCj4gDQo+IERvbid0IHlvdSB1c2UgVFNPIGluIHRoaXMgZHJpdmVy
ID8NCj4gDQo+IFdoYXQgYWJvdXQgZW5jYXBzdWxhdGlvbiA/DQo+IA0KPiBJIHN1c3BlY3QgeW91
IGhhdmUgYSBxdWl0ZSBkaWZmZXJlbnQgaXNzdWUgaGVyZS4NCj4gDQo+IFlvdSBzaW1wbHkgY291
bGQgdXNlIHNrYl9nZXRfaGFzaCgpIHNpbmNlIGxvY2FsIFRDUCBmbG93cyB3aWxsIHByb3ZpZGUg
YQ0KPiBsNCBza2ItPmhhc2ggYW5kIHlvdSBoYXZlIG5vIGZ1cnRoZXIgZmxvdyBkaXNzZWN0aW9u
IHRvIGRvLg0KDQpJbiBvdXIgdGVzdCwgd2UgaGF2ZSBiaXNlY3RlZCBhbmQgZm91bmQgdGhlIGZv
bGxvd2luZyBwYXRjaCBpbnRyb2R1Y2VkIGJpZyANCm92ZXJoZWFkIGludG8gc2tiX2Zsb3dfZGlz
c2VjdF9mbG93X2tleXMoKSwgYW5kIGNhdXNlZCBwZXJmb3JtYW5jZSANCnJlZ3Jlc3Npb246DQpj
b21taXQ6IGQzNGFmODIzDQpuZXQ6IEFkZCBWTEFOIElEIHRvIGZsb3dfa2V5cw0KDQpUaGlzIHBh
dGNoIGRpZG4ndCBhZGQgdG9vIG1hbnkgaW5zdHJ1Y3Rpb25zLCBidXQgd2UgdGhpbmsgdGhlIGNo
YW5nZSB0byANCnRoZSBzaXplIG9mIHN0cnVjdCBmbG93X2tleXMgbWF5IGNhdXNlIGRpZmZlcmVu
dCBjYWNoZSBtaXNzaW5nIHJhdGUuLi4NCg0KVG8gYXZvaWQgYWZmZWN0aW5nIG90aGVyIGRyaXZl
cnMgdXNpbmcgdGhpcyBmdW5jdGlvbiwgb3VyIHBhdGNoIGxpbWl0cyB0aGUgDQpjaGFuZ2UgaW5z
aWRlIG91ciBkcml2ZXIgdG8gZml4IHRoaXMgcGVyZm9ybWFuY2UgcmVncmVzc2lvbi4NCg0KUmVn
YXJkaW5nIHlvdXIgc3VnZ2VzdGlvbiBvbiBza2JfZ2V0X2hhc2goKSwgSSBsb29rZWQgYXQgdGhl
IGNvZGUgYW5kIHJhbiANCnNvbWUgdGVzdHMsIGFuZCBmb3VuZCB0aGUgc2tiLT5sNF9oYXNoIGFu
ZCBza2ItPnN3X2hhc2ggYml0cyBhcmUgbm90IHNldCwgDQpzbyBpdCBjYWxscyBfX3NrYl9nZXRf
aGFzaCgpIHdoaWNoIGV2ZW50dWFsbHkgY2FsbHMgDQpza2JfZmxvd19kaXNzZWN0X2Zsb3dfa2V5
cygpLiBTbyBpdCBzdGlsbCBpbmNsdWRlcyB0aGUgcGVyZm9ybWFuY2UgDQpvdmVyaGVhZCBtZW50
aW9uZWQgYWJvdmUuDQoNCnN0YXRpYyBpbmxpbmUgX191MzIgc2tiX2dldF9oYXNoKHN0cnVjdCBz
a19idWZmICpza2IpDQp7DQogICAgICAgIGlmICghc2tiLT5sNF9oYXNoICYmICFza2ItPnN3X2hh
c2gpDQogICAgICAgICAgICAgICAgX19za2JfZ2V0X2hhc2goc2tiKTsNCg0KICAgICAgICByZXR1
cm4gc2tiLT5oYXNoOw0KfQ0KDQoNCnZvaWQgX19za2JfZ2V0X2hhc2goc3RydWN0IHNrX2J1ZmYg
KnNrYikNCnsNCiAgICAgICAgc3RydWN0IGZsb3dfa2V5cyBrZXlzOw0KDQogICAgICAgIF9fZmxv
d19oYXNoX3NlY3JldF9pbml0KCk7DQoNCiAgICAgICAgX19za2Jfc2V0X3N3X2hhc2goc2tiLCBf
X19za2JfZ2V0X2hhc2goc2tiLCAma2V5cywgaGFzaHJuZCksDQogICAgICAgICAgICAgICAgICAg
ICAgICAgIGZsb3dfa2V5c19oYXZlX2w0KCZrZXlzKSk7DQp9DQoNCg0Kc3RhdGljIGlubGluZSB1
MzIgX19fc2tiX2dldF9oYXNoKGNvbnN0IHN0cnVjdCBza19idWZmICpza2IsDQogICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0IGZsb3dfa2V5cyAqa2V5cywgdTMyIGtleXZh
bCkNCnsNCiAgICAgICAgc2tiX2Zsb3dfZGlzc2VjdF9mbG93X2tleXMoc2tiLCBrZXlzLA0KICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBGTE9XX0RJU1NFQ1RPUl9GX1NUT1BfQVRf
RkxPV19MQUJFTCk7DQoNCiAgICAgICAgcmV0dXJuIF9fZmxvd19oYXNoX2Zyb21fa2V5cyhrZXlz
LCBrZXl2YWwpOw0KfQ0KDQoNClRoYW5rcywNCi0gSGFpeWFuZw0KDQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293349 — Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromEric Dumazet <eric.dumazet@gmail.com>
Date2015-12-16 22:20 +0100
SubjectRe: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGrEl-73J-3@gated-at.bofh.it>
In reply to#1293332
On Wed, 2015-12-16 at 19:20 +0000, Haiyang Zhang wrote:
> > -----Original Message-----
> > From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> > Sent: Wednesday, December 16, 2015 12:08 PM
> > 
> > This looks very very wrong to me.
> > 
> > How many times this is called per second, for the 'one flow' case ?
> > 
> > Don't you use TSO in this driver ?
> > 
> > What about encapsulation ?
> > 
> > I suspect you have a quite different issue here.
> > 
> > You simply could use skb_get_hash() since local TCP flows will provide a
> > l4 skb->hash and you have no further flow dissection to do.
> 
> In our test, we have bisected and found the following patch introduced big 
> overhead into skb_flow_dissect_flow_keys(), and caused performance 
> regression:
> commit: d34af823
> net: Add VLAN ID to flow_keys

Adding Tom Herbert <tom@herbertland.com>

Your driver was assuming things about "struct flow_keys" layout.
This is not permitted.

Magic numbers like 12 and 8 are really bad...

static bool netvsc_set_hash(u32 *hash, struct sk_buff *skb)
{
        struct flow_keys flow;
        int data_len;

        if (!skb_flow_dissect_flow_keys(skb, &flow, 0) ||
            !(flow.basic.n_proto == htons(ETH_P_IP) ||
              flow.basic.n_proto == htons(ETH_P_IPV6)))
                return false;

        if (flow.basic.ip_proto == IPPROTO_TCP)
                data_len = 12;
        else
                data_len = 8;

        *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, &flow, data_len);

        return true;
}


> This patch didn't add too many instructions, but we think the change to 
> the size of struct flow_keys may cause different cache missing rate...
> 
> To avoid affecting other drivers using this function, our patch limits the 
> change inside our driver to fix this performance regression.
> 
> Regarding your suggestion on skb_get_hash(), I looked at the code and ran 
> some tests, and found the skb->l4_hash and skb->sw_hash bits are not set, 
> so it calls __skb_get_hash() which eventually calls 
> skb_flow_dissect_flow_keys(). So it still includes the performance 
> overhead mentioned above.

Okay, but have you tried this instead of just guessing ?

Are you forwarding traffic, or is the traffic locally generated ?

TCP stack does set skb->l4_hash for sure in current kernels.

Your 'basic flow dissection' is very buggy and a step backward.

Just call skb_get_hash() : Not only your perf problem will vanish, but
your driver will correctly work with all possible malformed packets
(like pretending to be TCP packets but too small to even contain one
byte of TCP header) and well formed ones, with all encapsulations.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293386 — Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromDavid Miller <davem@davemloft.net>
Date2015-12-17 00:30 +0100
SubjectRe: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGtG9-8hX-1@gated-at.bofh.it>
In reply to#1293332
From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Wed, 16 Dec 2015 19:20:44 +0000

> In our test, we have bisected and found the following patch introduced big 
> overhead into skb_flow_dissect_flow_keys(), and caused performance 
> regression:
> commit: d34af823
> net: Add VLAN ID to flow_keys

NEVER _EVER_ work around this kind of problem by bypassing the code in
question in your driver.

ALWAYS work to fix the actual problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293297 — Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-12-16 19:40 +0100
SubjectRe: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGp9v-5n4-1@gated-at.bofh.it>
In reply to#1293097
Hello.

On 12/16/2015 09:03 PM, Haiyang Zhang wrote:

> To avoid performance overhead when using skb_flow_dissect_flow_keys(),
> we switch to the simple parsers to get the IP and port numbers.
>
> Performance comparison: throughput (Gbps):
> Number of connections, before patch, after patch
> 1			8.56		10.18
> 4			11.17		14.07
> 16			12.21		21.78
> 64			18.71		32.08
> 256			15.92		26.32
> 1024			8.41		15.49
> 3000			7.82		11.58
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Tested-by: Simon Xiao <sixiao@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>   drivers/net/hyperv/netvsc_drv.c |   38 +++++++++++++++++++++++++++++---------
>   1 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1c8db9a..e28951f 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -237,20 +237,40 @@ static u32 comp_hash(u8 *key, int klen, void *data, int dlen)
[...]
> +	if (iphdr->version == 4) {
> +		dbuf[0] = iphdr->saddr;
> +		dbuf[1] = iphdr->daddr;
> +		if (iphdr->protocol == IPPROTO_TCP) {
> +			dbuf[2] = *(__be32 *)&tcp_hdr(skb)->source;
> +			data_len = 12;
> +		} else {
> +			data_len = 8;
> +		}
> +	} else if (ipv6hdr->version == 6) {
> +		memcpy(dbuf, &ipv6hdr->saddr, 32);
> +		if (ipv6hdr->nexthdr == IPPROTO_TCP) {
> +			dbuf[8] = *(__be32 *)&tcp_hdr(skb)->source;
> +			data_len = 36;
> +		} else {
> +			data_len = 32;
> +		}
> +	} else {
> +		return false;
> +	}

    This is asking to be a *switch* statement.

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293305 — Re: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-12-16 19:50 +0100
SubjectRe: [PATCH net-next] hv_netvsc: Use simple parser for IPv4 and v6 headers
Message-ID<qGpjc-5qi-13@gated-at.bofh.it>
In reply to#1293297
On 12/16/2015 09:34 PM, Sergei Shtylyov wrote:

>> To avoid performance overhead when using skb_flow_dissect_flow_keys(),
>> we switch to the simple parsers to get the IP and port numbers.
>>
>> Performance comparison: throughput (Gbps):
>> Number of connections, before patch, after patch
>> 1            8.56        10.18
>> 4            11.17        14.07
>> 16            12.21        21.78
>> 64            18.71        32.08
>> 256            15.92        26.32
>> 1024            8.41        15.49
>> 3000            7.82        11.58
>>
>> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
>> Tested-by: Simon Xiao <sixiao@microsoft.com>
>> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
>> ---
>>   drivers/net/hyperv/netvsc_drv.c |   38 +++++++++++++++++++++++++++++---------
>>   1 files changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
>> index 1c8db9a..e28951f 100644
>> --- a/drivers/net/hyperv/netvsc_drv.c
>> +++ b/drivers/net/hyperv/netvsc_drv.c
>> @@ -237,20 +237,40 @@ static u32 comp_hash(u8 *key, int klen, void *data,
>> int dlen)
> [...]
>> +    if (iphdr->version == 4) {
>> +        dbuf[0] = iphdr->saddr;
>> +        dbuf[1] = iphdr->daddr;
>> +        if (iphdr->protocol == IPPROTO_TCP) {
>> +            dbuf[2] = *(__be32 *)&tcp_hdr(skb)->source;
>> +            data_len = 12;
>> +        } else {
>> +            data_len = 8;
>> +        }
>> +    } else if (ipv6hdr->version == 6) {
>> +        memcpy(dbuf, &ipv6hdr->saddr, 32);
>> +        if (ipv6hdr->nexthdr == IPPROTO_TCP) {
>> +            dbuf[8] = *(__be32 *)&tcp_hdr(skb)->source;
>> +            data_len = 36;
>> +        } else {
>> +            data_len = 32;
>> +        }
>> +    } else {
>> +        return false;
>> +    }
>
>     This is asking to be a *switch* statement.

    Oops, nevermind. I'd misread the code.

> [...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web