On 18/08/2017 00:52, Joe Stringer wrote:
On 17 August 2017 at 02:18, Paul Blakey <[email protected]> wrote:
On 15/08/2017 20:04, Joe Stringer wrote:
On 15 August 2017 at 01:19, Paul Blakey <[email protected]> wrote:
On 15/08/2017 00:56, Joe Stringer wrote:
On 8 August 2017 at 07:21, Roi Dayan <[email protected]> wrote:
From: Paul Blakey <[email protected]>
@@ -117,6 +118,17 @@ struct tc_flower {
uint64_t lastused;
struct {
+ bool rewrite;
+ uint8_t pad1[3];
+ struct tc_flower_key key;
+ uint8_t pad2[3];
+ struct tc_flower_key mask;
+ uint8_t pad3[3];
+ } rewrite;
Now that I get why the pads are here.. ;)
Is there an existing macro we can use to ensure that these pad out to
32-bit boundaries?
I'm not sure if that's possible, the size is a minimum of extra 24bits, so
its can't overflow with writing on any address below it. The compiler
might add some extra padding but that shouldn't matter.
My thought was that the main concern here is to align the fields to
32-bit boundaries, so if it already does this then I wouldn't expect
to need any padding at all? For instance, on my system with these
patches I see with pahole that 'struct tc_flower_key' has size 84, and
the 'pad2' and 'pad3' appear to be unnecessary:
struct {
_Bool rewrite; /* 216 1 */
uint8_t pad1[0]; /* 217 0 */
struct tc_flower_key key; /* 220 84 */
/* --- cacheline 1 boundary (64 bytes) was 21 bytes ago --- */
uint8_t pad2[0]; /* 304 0 */
struct tc_flower_key mask; /* 308 84 */
/* --- cacheline 2 boundary (128 bytes) was 41 bytes ago --- */
uint8_t pad3[0]; /* 392 0 */
} rewrite; /* 216 180 */
However, if in future someone adds a 1-byte member to tc_flower_key
then I'm not sure that this alignment will hold. On my system, it
seems like that would end up padding tc_flower_key out to 88B so these
extra padding would still be unnecessary (although pahole says that it
has a brain fart, so I'm not sure exactly how much confidence I should
have in this).
What I was thinking about was whether we could use something like
PADDED_MEMBERS() in this case.
Are you talking about alignment in regards to performance?
Because the padding is there for overflowing.
If someone adds a new member to struct key/mask that is smaller than a
32bits to the end of the struct, setting it via a pointer might overflow
the struct. I don't think PADDED_MEMBERS will work for this type of padding.
We do mask the write to the write size, and it should still be in memory
of owned by struct flower and since the compiler can't reorder the
struct we actually only need the last padding to cover the above case.
Maybe we can add alignment when we measure the performance of the code?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
