Re: [ovs-dev] [PATCH] datapath-windows: Fix bug in OvsTcpGetWscale().

2016-04-27 Thread Daniele Di Proietto
Thanks, pushed this to master!




On 20/04/2016 21:06, "Sairam Venugopal"  wrote:

>Acked-by: Sairam Venugopal 
>
>
>On 4/15/16, 5:04 PM, "Daniele Di Proietto"  wrote:
>
>>The userspace conntrack had a bug in tcp_wscale_get(), where the length
>>of an option would be read from the third octet of the option TLV
>>instead of the second.  This could cause an incorrect wscale value to
>>be returned, and it would at least impact performance.
>>
>>Also use 'int' instead of 'unsigned' for 'len', since the value can be
>>negative.
>>
>>CC: Sairam Venugopal 
>>Signed-off-by: Daniele Di Proietto 
>>---
>>
>>I tested a similar fix on the userspace connection tracker, but I didn't
>>compile this for the windows datapath.
>>
>>---
>> datapath-windows/ovsext/Conntrack-tcp.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>>diff --git a/datapath-windows/ovsext/Conntrack-tcp.c
>>b/datapath-windows/ovsext/Conntrack-tcp.c
>>index 3e25ba5..340c469 100644
>>--- a/datapath-windows/ovsext/Conntrack-tcp.c
>>+++ b/datapath-windows/ovsext/Conntrack-tcp.c
>>@@ -166,7 +166,7 @@ OvsConntrackValidateTcpFlags(const TCPHdr *tcp)
>> static __inline uint8_t
>> OvsTcpGetWscale(const TCPHdr *tcp)
>> {
>>-unsigned len = tcp->doff * 4 - sizeof *tcp;
>>+int len = tcp->doff * 4 - sizeof *tcp;
>> const uint8_t *opt = (const uint8_t *)(tcp + 1);
>> uint8_t wscale = 0;
>> uint8_t optlen;
>>@@ -185,7 +185,7 @@ OvsTcpGetWscale(const TCPHdr *tcp)
>> wscale |= CT_WSCALE_FLAG;
>> /* fall through */
>> default:
>>-optlen = opt[2];
>>+optlen = opt[1];
>> if (optlen < 2) {
>> optlen = 2;
>> }
>>@@ -529,4 +529,4 @@ OvsNewTcpConntrack(const TCPHdr *tcp,
>> OvsConntrackUpdateExpiration(newconn, now, CT_ENTRY_TIMEOUT);
>> 
>> return &newconn->up;
>>-}
>>\ No newline at end of file
>>+}
>>-- 
>>2.1.4
>>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: Fix bug in OvsTcpGetWscale().

2016-04-20 Thread Sairam Venugopal
Acked-by: Sairam Venugopal 


On 4/15/16, 5:04 PM, "Daniele Di Proietto"  wrote:

>The userspace conntrack had a bug in tcp_wscale_get(), where the length
>of an option would be read from the third octet of the option TLV
>instead of the second.  This could cause an incorrect wscale value to
>be returned, and it would at least impact performance.
>
>Also use 'int' instead of 'unsigned' for 'len', since the value can be
>negative.
>
>CC: Sairam Venugopal 
>Signed-off-by: Daniele Di Proietto 
>---
>
>I tested a similar fix on the userspace connection tracker, but I didn't
>compile this for the windows datapath.
>
>---
> datapath-windows/ovsext/Conntrack-tcp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Conntrack-tcp.c
>b/datapath-windows/ovsext/Conntrack-tcp.c
>index 3e25ba5..340c469 100644
>--- a/datapath-windows/ovsext/Conntrack-tcp.c
>+++ b/datapath-windows/ovsext/Conntrack-tcp.c
>@@ -166,7 +166,7 @@ OvsConntrackValidateTcpFlags(const TCPHdr *tcp)
> static __inline uint8_t
> OvsTcpGetWscale(const TCPHdr *tcp)
> {
>-unsigned len = tcp->doff * 4 - sizeof *tcp;
>+int len = tcp->doff * 4 - sizeof *tcp;
> const uint8_t *opt = (const uint8_t *)(tcp + 1);
> uint8_t wscale = 0;
> uint8_t optlen;
>@@ -185,7 +185,7 @@ OvsTcpGetWscale(const TCPHdr *tcp)
> wscale |= CT_WSCALE_FLAG;
> /* fall through */
> default:
>-optlen = opt[2];
>+optlen = opt[1];
> if (optlen < 2) {
> optlen = 2;
> }
>@@ -529,4 +529,4 @@ OvsNewTcpConntrack(const TCPHdr *tcp,
> OvsConntrackUpdateExpiration(newconn, now, CT_ENTRY_TIMEOUT);
> 
> return &newconn->up;
>-}
>\ No newline at end of file
>+}
>-- 
>2.1.4
>

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] datapath-windows: Fix bug in OvsTcpGetWscale().

2016-04-15 Thread Daniele Di Proietto
The userspace conntrack had a bug in tcp_wscale_get(), where the length
of an option would be read from the third octet of the option TLV
instead of the second.  This could cause an incorrect wscale value to
be returned, and it would at least impact performance.

Also use 'int' instead of 'unsigned' for 'len', since the value can be
negative.

CC: Sairam Venugopal 
Signed-off-by: Daniele Di Proietto 
---

I tested a similar fix on the userspace connection tracker, but I didn't
compile this for the windows datapath.

---
 datapath-windows/ovsext/Conntrack-tcp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index 3e25ba5..340c469 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -166,7 +166,7 @@ OvsConntrackValidateTcpFlags(const TCPHdr *tcp)
 static __inline uint8_t
 OvsTcpGetWscale(const TCPHdr *tcp)
 {
-unsigned len = tcp->doff * 4 - sizeof *tcp;
+int len = tcp->doff * 4 - sizeof *tcp;
 const uint8_t *opt = (const uint8_t *)(tcp + 1);
 uint8_t wscale = 0;
 uint8_t optlen;
@@ -185,7 +185,7 @@ OvsTcpGetWscale(const TCPHdr *tcp)
 wscale |= CT_WSCALE_FLAG;
 /* fall through */
 default:
-optlen = opt[2];
+optlen = opt[1];
 if (optlen < 2) {
 optlen = 2;
 }
@@ -529,4 +529,4 @@ OvsNewTcpConntrack(const TCPHdr *tcp,
 OvsConntrackUpdateExpiration(newconn, now, CT_ENTRY_TIMEOUT);
 
 return &newconn->up;
-}
\ No newline at end of file
+}
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev