RE: [PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()

2014-08-22 Thread Haiyang Zhang 


> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Friday, August 22, 2014 12:31 AM
> To: Haiyang Zhang
> Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de;
> jasow...@redhat.com; linux-kernel@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org
> Subject: Re: [PATCH net-next] hyperv: Add handling of IP header with
> option field in netvsc_set_hash()
> 
> From: Haiyang Zhang 
> Date: Tue, 19 Aug 2014 20:53:55 +
> 
> > @@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct
> sk_buff *skb)
> > iphdr = ip_hdr(skb);
> >
> > if (iphdr->version == 4) {
> > -   if (iphdr->protocol == IPPROTO_TCP)
> > +   data = (u8 *)>saddr;
> > +   if (iphdr->protocol == IPPROTO_TCP) {
> > data_len = 12;
> > -   else
> > +   if (iphdr->ihl > 5) {
> > +   memcpy(dbuf, >saddr, 8);
> > +   memcpy([8], _hdr(skb)->source, 4);
> 
> This is rediculous.
> 
> Make hash_comp() take a void pointer for the buffer.
> 
> Then your code is simply:
> 
>   be32 dbuf[2];
> 
>   dbuf[1] = iph->saddr;
>   dbuf[2] = iph->daddr;
>   dbuf[3] = *(be32 *)tcph->source;
> 
>   *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, 12);
> 
> No special cases for IP options or any garbage like that.

I have submitted a revised patch with the suggested changes.

Thanks,
- Haiyang

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

RE: [PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()

2014-08-22 Thread Haiyang Zhang 


 -Original Message-
 From: David Miller [mailto:da...@davemloft.net]
 Sent: Friday, August 22, 2014 12:31 AM
 To: Haiyang Zhang
 Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de;
 jasow...@redhat.com; linux-kernel@vger.kernel.org; driverdev-
 de...@linuxdriverproject.org
 Subject: Re: [PATCH net-next] hyperv: Add handling of IP header with
 option field in netvsc_set_hash()
 
 From: Haiyang Zhang haiya...@microsoft.com
 Date: Tue, 19 Aug 2014 20:53:55 +
 
  @@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct
 sk_buff *skb)
  iphdr = ip_hdr(skb);
 
  if (iphdr-version == 4) {
  -   if (iphdr-protocol == IPPROTO_TCP)
  +   data = (u8 *)iphdr-saddr;
  +   if (iphdr-protocol == IPPROTO_TCP) {
  data_len = 12;
  -   else
  +   if (iphdr-ihl  5) {
  +   memcpy(dbuf, iphdr-saddr, 8);
  +   memcpy(dbuf[8], tcp_hdr(skb)-source, 4);
 
 This is rediculous.
 
 Make hash_comp() take a void pointer for the buffer.
 
 Then your code is simply:
 
   be32 dbuf[2];
 
   dbuf[1] = iph-saddr;
   dbuf[2] = iph-daddr;
   dbuf[3] = *(be32 *)tcph-source;
 
   *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, 12);
 
 No special cases for IP options or any garbage like that.

I have submitted a revised patch with the suggested changes.

Thanks,
- Haiyang

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

Re: [PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()

2014-08-21 Thread David Miller 
From: Haiyang Zhang 
Date: Tue, 19 Aug 2014 20:53:55 +

> @@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct sk_buff 
> *skb)
>   iphdr = ip_hdr(skb);
>  
>   if (iphdr->version == 4) {
> - if (iphdr->protocol == IPPROTO_TCP)
> + data = (u8 *)>saddr;
> + if (iphdr->protocol == IPPROTO_TCP) {
>   data_len = 12;
> - else
> + if (iphdr->ihl > 5) {
> + memcpy(dbuf, >saddr, 8);
> + memcpy([8], _hdr(skb)->source, 4);

This is rediculous.

Make hash_comp() take a void pointer for the buffer.

Then your code is simply:

be32 dbuf[2];

dbuf[1] = iph->saddr;
dbuf[2] = iph->daddr;
dbuf[3] = *(be32 *)tcph->source;

*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, 12);

No special cases for IP options or any garbage like that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()

2014-08-21 Thread David Miller 
From: Haiyang Zhang haiya...@microsoft.com
Date: Tue, 19 Aug 2014 20:53:55 +

 @@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct sk_buff 
 *skb)
   iphdr = ip_hdr(skb);
  
   if (iphdr-version == 4) {
 - if (iphdr-protocol == IPPROTO_TCP)
 + data = (u8 *)iphdr-saddr;
 + if (iphdr-protocol == IPPROTO_TCP) {
   data_len = 12;
 - else
 + if (iphdr-ihl  5) {
 + memcpy(dbuf, iphdr-saddr, 8);
 + memcpy(dbuf[8], tcp_hdr(skb)-source, 4);

This is rediculous.

Make hash_comp() take a void pointer for the buffer.

Then your code is simply:

be32 dbuf[2];

dbuf[1] = iph-saddr;
dbuf[2] = iph-daddr;
dbuf[3] = *(be32 *)tcph-source;

*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, 12);

No special cases for IP options or any garbage like that.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()

2014-08-19 Thread Haiyang Zhang 
In case that the IP header has optional field at the end, this patch will 
get the port numbers after that field, and compute the hash.

Signed-off-by: Haiyang Zhang 
Reviewed-by: K. Y. Srinivasan 

---
 drivers/net/hyperv/netvsc_drv.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a9c5eaa..b12dcd9 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -191,6 +191,8 @@ static u32 comp_hash(u8 *key, int klen, u8 *data, int dlen)
 static bool netvsc_set_hash(u32 *hash, struct sk_buff *skb)
 {
struct iphdr *iphdr;
+   u8 dbuf[12];
+   u8 *data;
int data_len;
bool ret = false;
 
@@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct sk_buff 
*skb)
iphdr = ip_hdr(skb);
 
if (iphdr->version == 4) {
-   if (iphdr->protocol == IPPROTO_TCP)
+   data = (u8 *)>saddr;
+   if (iphdr->protocol == IPPROTO_TCP) {
data_len = 12;
-   else
+   if (iphdr->ihl > 5) {
+   memcpy(dbuf, >saddr, 8);
+   memcpy([8], _hdr(skb)->source, 4);
+   data = dbuf;
+   }
+   } else {
data_len = 8;
-   *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN,
- (u8 *)>saddr, data_len);
+   }
+   *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, data, data_len);
ret = true;
}
 
-- 
1.7.1

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

[PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()

2014-08-19 Thread Haiyang Zhang 
In case that the IP header has optional field at the end, this patch will 
get the port numbers after that field, and compute the hash.

Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Reviewed-by: K. Y. Srinivasan k...@microsoft.com

---
 drivers/net/hyperv/netvsc_drv.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a9c5eaa..b12dcd9 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -191,6 +191,8 @@ static u32 comp_hash(u8 *key, int klen, u8 *data, int dlen)
 static bool netvsc_set_hash(u32 *hash, struct sk_buff *skb)
 {
struct iphdr *iphdr;
+   u8 dbuf[12];
+   u8 *data;
int data_len;
bool ret = false;
 
@@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct sk_buff 
*skb)
iphdr = ip_hdr(skb);
 
if (iphdr-version == 4) {
-   if (iphdr-protocol == IPPROTO_TCP)
+   data = (u8 *)iphdr-saddr;
+   if (iphdr-protocol == IPPROTO_TCP) {
data_len = 12;
-   else
+   if (iphdr-ihl  5) {
+   memcpy(dbuf, iphdr-saddr, 8);
+   memcpy(dbuf[8], tcp_hdr(skb)-source, 4);
+   data = dbuf;
+   }
+   } else {
data_len = 8;
-   *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN,
- (u8 *)iphdr-saddr, data_len);
+   }
+   *hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, data, data_len);
ret = true;
}
 
-- 
1.7.1

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