On 27/03/2018 17:40, Jason Gunthorpe wrote:
On Tue, Mar 27, 2018 at 01:39:02PM +0300, Matan Barak wrote:
Why do we need to make all uapi structs u64 aligned in our new uapi?
We don't have the "size in quadwords" like we have in the write() path.
Keeping them 32bit aligned with all u64 fields marked as __aligned_64 seems
to suffice. It should prevent the nasty 32bit-vs-64bit (user-space vs
kernel) bugs.
It is less about alignment, more about not having implicit
padding in the uapi structs at all.
One reason is the 32/64 issue. I think consistent use of __aligned_u64
should result in consistent implicit padding, so hopefully that is
taken care of now.
The other is just understand-ability. If someone sends a patch to add a
new member to this:
+struct ib_uverbs_flow_action_esp_encap {
+ /* This struct represents a list of pointers to flow_xxxx_filter that
+ * encapsulates the payload in ESP tunnel mode.
+ */
+ RDMA_UAPI_PTR(void *, val_ptr); /* pointer to a flow_xxxx_filter */
+ RDMA_UAPI_PTR(struct ib_uverbs_flow_action_esp_encap *, next_ptr);
+ __u16 len; /* Len of the filter struct val_ptr points to */
+ __u16 type; /* Use flow_spec_type enum */
+};
Did it make the struct longer? Is it taking up implicit padding? Did
the padding get zero'd?
If we explicitly align for 32bit and we use __aligned_u64 for 64bit
types, wouldn't it solve the problem?
If you would like to add a field that is < 32bit, you would explicitly
need to change the reserved field. For fields >= 32bit, you would
increase the structure's size.
But.. it sure would be simpler to not pad at all as everyone seems to
get this wrong anyhow.
Yeah, that's exactly why I would like to change that.
Jason
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
