diff mbox

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

Message ID 1408481635-29639-1-git-send-email-haiyangz@microsoft.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Haiyang Zhang Aug. 19, 2014, 8:53 p.m. UTC 
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 <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

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

Comments

David Miller Aug. 22, 2014, 4:30 a.m. UTC | #1 
From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Tue, 19 Aug 2014 20:53:55 +0000

> @@ -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 netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Haiyang Zhang Aug. 22, 2014, 9:06 p.m. UTC | #2 
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 22, 2014 12:31 AM
> To: Haiyang Zhang
> Cc: netdev@vger.kernel.org; KY Srinivasan; olaf@aepfle.de;
> jasowang@redhat.com; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org
> Subject: Re: [PATCH net-next] hyperv: Add handling of IP header with
> option field in netvsc_set_hash()
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Date: Tue, 19 Aug 2014 20:53:55 +0000
> 
> > @@ -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 netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
 	}