uverbs message alignment

Hey Roland,

I noticed a an alignment issue, I think, with struct ibv_create_qp.

I was adding a new __u32 to my raw qp create message. Here is the new struct with the new field added (nfids) from my libcxgb4 abi header file:

struct c4iw_create_raw_qp_req {
        struct ibv_create_qp ibv_req;
        __u32 port;
        __u32 vlan_pri;
        __u32 nfids;
};

sizeof sez it is 80 bytes, but when packed like this:

struct c4iw_create_raw_qp_req {
        struct ibv_create_qp ibv_req;
        __u32 port;
        __u32 vlan_pri;
        __u32 nfids;
} __attribute__ ((packed));

sizeof sez its 76 bytes.

I think this means struct ibv_create_qp is somehow not naturally aligned. Shouldn't it be?

Here is struct ibv_create_qp. I'm not sure why it isn't aligned:

struct ibv_create_qp {
        __u32 command;
        __u16 in_words;
        __u16 out_words;
        __u64 response;
        __u64 user_handle;
        __u32 pd_handle;
        __u32 send_cq_handle;
        __u32 recv_cq_handle;
        __u32 srq_handle;
        __u32 max_send_wr;
        __u32 max_recv_wr;
        __u32 max_send_sge;
        __u32 max_recv_sge;
        __u32 max_inline_data;
        __u8  sq_sig_all;
        __u8  qp_type;
        __u8  is_srq;
        __u8  reserved;
        __u64 driver_data[0];
};

Does this have to do with field driver_data being defined as a __u64 and my struct is using __u32s? Am I incorrect in using 3 __u32s?

This causes a problem, because the provider libs use sizeof their cmd struct to determine the amount of data to move into the kernel. The kernel then sets udata->inlen/outlen based on these sizes. Its harmless, until:

I discovered this because I was adding code in my kernel driver to detect a down-level library by looking at udata->inlen like this:

        if (udata->inlen - sizeof(struct ib_uverbs_cmd_hdr) != sizeof ureq)
                return ERR_PTR(-EINVAL);

Here's what ureq is in the driver's ABI header:

struct c4iw_create_raw_qp_req {
    __u32 port;
    __u32 vlan_pri;
    __u32 nfids;
};

However, udata->inlen was 24 due to this packed issue (sizeof struct ib_uverbs_cmd_hdr is 8 by the way). It should have been 20. When I packed the struct, my driver then found the correct size.

Also, I would think when we enter a provider's verb call, that udata->inlen would == the size of the driver's version of the ABI request message, and outlen would == the size of the driver's version of the ABI response message. I think having to subtract the struct ib_uverbs_cmd_hdr is a bug too eh?

Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

struct ibv_create_qp contains a u64, which will force the size of the structure to 64-bit. You should to add an additional 32-bits of padding.

these last two fields force the alignment

- Sean

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Specifically, the padding Sean is talking about is here:

struct c4iw_create_raw_qp_req {
        struct ibv_create_qp ibv_req;
        __u32 port;
        __u32 vlan_pri;
        __u32 nfids;
        __u32 _rsvd0;  // <<<---- pad
};

And similarly in your driver's header. Things are setup so that device_data does not have to be a multple of 8 bytes long, but it is guarenteed to be 8 byte aligned.

However as long as you include ibv_req in your struct you have to be a multiple of 8 bytes long so, you may want to construct your driver_data the same way that userspace does. Remove the ibv_req from the struct and cast from driver_data. This avoids this sort of problem completely.

Also, always ensure structs you are passing between kernel and user space have no implicit padding at all.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html