Re: [PATCH rdma-next 05/15] IB/uverbs: Add flow_action create and destroy verbs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)))
#endif
#elif UINTPTR_MAX != UINT64_MAX
#error "Pointer size not supported"
#endif

struct ib_uverbs_flow_action_esp_encap {
        union {
                struct {
                        void *     val_ptr;              /*   0     4 */
                        __u32      val_ptr_reserved;     /*   4     4 */
                };                                       /*         8 */
                __u64              val_ptr_data_u64;     /*         8 */
        };                                               /*   0     8 */
        union {
                struct {
struct ib_uverbs_flow_action_esp_encap * next_ptr; /* 8 4 */
                        __u32      next_ptr_reserved;    /*  12     4 */
                };                                       /*         8 */
                __u64              next_ptr_data_u64;    /*         8 */
        };                                               /*   8     8 */
        __u16                      len;                  /*  16     2 */
        __u16                      type;                 /*  18     2 */

        /* size: 24, cachelines: 1, members: 4 */
        /* padding: 4 */
        /* last cacheline: 24 bytes */
};

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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux