unaligned: Correct the stats of packet_count and byte_count on Windows.

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=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=18446744071591715133,
 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=18446744071591715133,

ovs-ofctl.exe -O OpenFlow10 dump-flows nsx-managed | findstr 1516011
 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813,
 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813,

ovs-ofctl.exe dump-flows nsx-managed | findstr 1516011
 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813,
 cookie=<>, duration=<>s, 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=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813,
 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813,

Fixes: afa3a93165f1 ("Add header for access to potentially unaligned data.")
Signed-off-by: Wilson Peng <pweisong@vmware.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 7 insertions, 6 deletions

diff --git a/lib/unaligned.h b/lib/unaligned.h
index f40e4e10d..15334e3c7 100644
--- a/lib/unaligned.h
+++ b/lib/unaligned.h
@@ -95,7 +95,7 @@ GCC_UNALIGNED_ACCESSORS(ovs_be64, be64);
 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 +110,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 +132,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_)