On Tue, May 24, 2016 at 09:59:46PM +0000, Hefty, Sean wrote:
> > Your patch didn't explore how to encode the arguments to the
> > operations - what did you want to do? (ie the stuff that trails the urdma_ioctl)
>
> I hadn't considered that aspect yet. I planned to look at the
> netlink proposal as a first step, but as an API it looks painful.
I agree.
If we are using SGL for the response then why not use SGL for
everything? We've already lost the advantage of the packet-like
encoding netlink uses.
eg:
struct urdma_ioctl {
u64 flags;
u16 domain;
u8 object_count;
u8 in_arg_count;
u8 out_arg_count;
u8 reserved[3];
// union urdma_object objects[object_count];
// struct arg_sgl in_args[in_arg_count];
// struct arg_sgl out_args[out_arg_count];
};
union urdma_object {
struct urdma_obj_id obj_id;
u64 data;
};
struct urdma_arg_sgl {
void *ptr;
u32 id;
u32 length;
};
(same comment goes for Mellanox's version)
This looks like it would make the decode and validation steps simpler
and avoid some of those ugly memory allocations in the Mellanox patch
as well...
This would also address your concern about hard-wiring the libibverbs
1.0 needs, and meet mellanox's concern to have a 0 copy path for the
compat udata/etc.
The user caller then uses the simplifed on-stack setup I was
suggesting:
int ibv_open_device(...)
{
struct xxx resp;
struct kern_call
{
struct urdma_ioctl hdr;
struct urdma_arg common_out;
struct urdma_arg udata_out;
} call = {
.hdr = {.domain = URDMA_DEVICE, .out_arg_count = 2},
.common_out = {.ptr = &resp, id = CREATE_DEVICE_COMMON_RESP, .length = sizeof(resp)},
.udata_out = {.ptr = &udata_resp, id = CREATE_DEVICE_MLX4_UDATA_RESP, .length = sizeof(udata_resp)},
};
ioctl(fd, URDMA_IOCTL_OPEN, &kern_call);
// Kernel 0's the resp ID when it writes so user space knows it was supported
if (call.common_out.id != 0)
return EINVAL;
}
This outline is a mash up of ideas from both patches.
Jason
--
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
