From: Wilson Peng <[email protected]>
The stats(byte_count) is got via function call ofputil_decode_flow_stats_reply()
and for OpenFlow15 it will also call oxs_pull_entry__(). Currently we found on
Windows the byte_count counter is incorrect. It will get the byte_count on
OpenFlow15
handling via ntohll(get_unaligned_be64(payload))
Quote the comments below from Ilya Maximets(thanks for the given soluton and
explanation)
static inline uint64_t get_unaligned_u64__(const uint64_t *p_)
...
return ntohll(((uint64_t) p[0] << 56)
| ((uint64_t) p[1] << 48)
| ((uint64_t) p[2] << 40)
| ((uint64_t) p[3] << 32)
| (p[4] << 24)
| (p[5] << 16)
| (p[6] << 8)
| p[7]);
And indeed the expression above has an issue with data types.
The problem is the (p[4] << 24) part. The p[4] itself has a type
'uint8_t' which is unsigned 8bit value. It is not enough to hold
the result of a left shift, so compiler automatically promotes it
to the 'int' by default. But it is *signed* 32bit value.
In your original report p[4] was equal to 0x81. After the left
shift it became 0x81000000. Looks correct, but the type is 'int'.
The next operation that we do is '|' with the previous shifted
bytes that were explicitly converted to uint64_t before the left
shift. So we have uint64_t | int. In this case compiler needs
to extend the 'int' to 'unit64_t' before performing the operation.
And since the 'int' is signed and the sign bit happens to be set
in the 0x81000000, the sign extension is performed in order to
preserve the value. The result is 0xffffffff81000000. And that
is breaking everything else.
>From the new test below, it is incorrect for the n_bytes counter via OpenFlow15
On CMD: ovs-ofctl dump-flows.
With the patch, get_unaligned_u64__() will return correct value to caller
on Windows.
In the output (Got via original CMD without fix) below n_bytes 2177130813 will
be incorrectly changed to 18446744071591715133 when processing OpenFlow15 which
is equal to 0xFFFFFFFF81C4613D and here the p[4] on Windows is 0x81.
With the fix, new compiled ovs-ofctl1025.exe could dump the correct n_bytes
counter
Via OpenFlow15.
ovs-ofctl.exe -O OpenFlow15 dump-flows nsx-managed | findstr 1516011
cookie=0x1e77082def43e867, duration=2167263.984s, table=4, n_packets=1516011,
n_bytes=18446744071591715133,
cookie=0x2033543ed8e89cc1, duration=2167263.984s, table=4, n_packets=1516011,
n_bytes=18446744071591715133,
ovs-ofctl.exe -O OpenFlow10 dump-flows nsx-managed | findstr 1516011
cookie=0x1e77082def43e867, duration=2167271.690s, table=4, n_packets=1516011,
n_bytes=2177130813,
cookie=0x2033543ed8e89cc1, duration=2167271.690s, table=4, n_packets=1516011,
n_bytes=2177130813,
ovs-ofctl.exe dump-flows nsx-managed | findstr 1516011
cookie=0x1e77082def43e867, duration=2167282.103s, table=4, n_packets=1516011,
n_bytes=2177130813,
cookie=0x2033543ed8e89cc1, duration=2167282.103s, table=4, n_packets=1516011,
n_bytes=2177130813,
With the fix, new compiled ovs-ofctl1025.exe could dump the correct n_bytes
counter
Via OpenFlow15.
ovs-ofctl1025.exe -O OpenFlow15 dump-flows nsx-managed | findstr 1516011
cookie=0x1e77082def43e867, duration=2167239.242s, table=4, n_packets=1516011,
n_bytes=2177130813,
cookie=0x2033543ed8e89cc1, duration=2167239.242s, table=4, n_packets=1516011,
n_bytes=2177130813,
Fixes: afa3a93165f1 ("Add header for access to potentially unaligned data.")
Signed-off-by: Wilson Peng <[email protected]>
---
lib/unaligned.h | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/lib/unaligned.h b/lib/unaligned.h
index f40e4e10d..022e8c976 100644
--- a/lib/unaligned.h
+++ b/lib/unaligned.h
@@ -91,11 +91,13 @@ GCC_UNALIGNED_ACCESSORS(ovs_be32, be32);
GCC_UNALIGNED_ACCESSORS(ovs_be64, be64);
#else
/* Generic implementations. */
+static inline ovs_be64
+get_32aligned_be64(const ovs_32aligned_be64 *x);
static inline uint16_t get_unaligned_u16(const uint16_t *p_)
{
const uint8_t *p = (const uint8_t *) p_;
- return ntohs((p[0] << 8) | p[1]);
+ return ntohs(((uint16_t) p[0] << 8) | (uint16_t) p[1]);
}
static inline void put_unaligned_u16(uint16_t *p_, uint16_t x_)
@@ -110,7 +112,8 @@ static inline void put_unaligned_u16(uint16_t *p_, uint16_t
x_)
static inline uint32_t get_unaligned_u32(const uint32_t *p_)
{
const uint8_t *p = (const uint8_t *) p_;
- return ntohl((p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]);
+ return ntohl(((uint32_t) p[0] << 24) | ((uint32_t) p[1] << 16) |
+ ((uint32_t) p[2] << 8) | (uint32_t) p[3]);
}
static inline void put_unaligned_u32(uint32_t *p_, uint32_t x_)
@@ -131,10 +134,10 @@ static inline uint64_t get_unaligned_u64__(const uint64_t
*p_)
| ((uint64_t) p[1] << 48)
| ((uint64_t) p[2] << 40)
| ((uint64_t) p[3] << 32)
- | (p[4] << 24)
- | (p[5] << 16)
- | (p[6] << 8)
- | p[7]);
+ | ((uint64_t) p[4] << 24)
+ | ((uint64_t) p[5] << 16)
+ | ((uint64_t) p[6] << 8)
+ | (uint64_t) p[7]);
}
static inline void put_unaligned_u64__(uint64_t *p_, uint64_t x_)
--
2.32.1 (Apple Git-133)
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
