Re: [ovs-dev] [PATCH] datapath-windows: Fix bug in OvsTcpGetWscale().
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().
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().
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