On 21/03/2018 16:57, Jason Gunthorpe wrote:
On Wed, Mar 21, 2018 at 04:38:45PM +0200, Matan Barak wrote:
On 21/03/2018 03:44, Jason Gunthorpe wrote:
On Tue, Mar 20, 2018 at 03:01:36PM +0200, Leon Romanovsky wrote:
+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 you test this Matan? It doesn't work the way you think:
#include <linux/types.h>
#include <assert.h>
#define RDMA_UAPI_PTR(_type, _name) _type __attribute__((aligned(8))) _name
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 */
};
static_assert(sizeof(struct ib_uverbs_flow_action_esp_encap) == 8*3, "Bad size");
static_assert(__alignof__(struct ib_uverbs_flow_action_esp_encap) == 8, "Bad Align");
$ gcc -std=c11 -Wall -c /tmp/t.c -m32
In file included from /tmp/t.c:2:0:
/tmp/t.c:16:1: error: static assertion failed: "Bad size"
static_assert(sizeof(struct ib_uverbs_flow_action_esp_encap) == 8*3, "Bad size");
$ pahole32 t.o
struct ib_uverbs_flow_action_esp_encap {
void * val_ptr; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
struct ib_uverbs_flow_action_esp_encap * next_ptr; /* 8 4 */
__u16 len; /* 12 2 */
__u16 type; /* 14 2 */
/* size: 16, cachelines: 1, members: 4 */
/* sum members: 12, holes: 1, sum holes: 4 */
/* last cacheline: 16 bytes */
};
And why is a pointer in a user struct?
The kernel default for RDMA_UAPI_PTR() should simply be
#define RDMA_UAPI_PTR(_type, _name) __aligned_u64 _name
Because we always have to use u64_to_user_ptr() anyhow.
Userspace needs to redefine it to
#define RDMA_UAPI_PTR(_type, _name) union {_type _name; __aligned_u64 _dummy_#_name;}
or
#define RDMA_UAPI_PTR(_type, _name) union {struct {__u32 _dummy2_##_name; _type _name; __aligned_u64 _dummy_##_name;}
Depending on endian
This is redefined in the user-space code at libibverbs/verbs_api.h by:
#if UINTPTR_MAX == UINT32_MAX
#if __BYTE_ORDER == __LITTLE_ENDIAN
#define RDMA_UAPI_PTR(_type, _name) union { \
struct {_type _name; \
__u32 _name ##_reserved;\
}; \
__u64 _name ## _data_u64; } \
__attribute__((aligned(8)))
#else
#define RDMA_UAPI_PTR(_type, _name) union { \
struct {__u32 _name ##_reserved;\
_type _name; \
}; \
__u64 _name ## _data_u64; } \
__attribute__((aligned(8)))
Should be __aligned_u64, but yes, that is fine.
The 64 bit version should be
union {_type _name; __aligned_u64 _name ## _data_u64; }
Though, just to make sure the alignment is right.
So only the kernel macro is wrong, please fix it to
#define RDMA_UAPI_PTR(_type, _name) __aligned_u64 _name
By doing this, you're loosing the type information.
Why not defining it both in user-space and kernel as
union {_type _name; __aligned_u64 _name ## _data_u64; }
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
