On Tue, Jun 2, 2015 at 8:07 PM, Jason Gunthorpe <[email protected]> wrote: > On Tue, Jun 02, 2015 at 10:21:47AM +0300, Matan Barak wrote: >> On Mon, Jun 1, 2015 at 7:56 PM, Jason Gunthorpe >> <[email protected]> wrote: >> > On Sun, May 31, 2015 at 03:14:16PM +0300, Or Gerlitz wrote: >> >> From: Matan Barak <[email protected]> >> >> >> >> Add support for ib_uverbs_ex_create_cq and ib_uverbs_ex_query_device >> >> by setting the appropriate bit in uverbs_ex_cmd_mask. >> > >> > Why is this a seperate patch? Surely the bits should be or'd in the patches >> > that actually include the code to do the new commands? >> > >> >> Although this patch can stand on its own (requesting unsupported flags >> will just fail and the >> comp_mask of uhw's will be disabled), there's no real value for not >> squashing it into the other mlx4 patches. > > Then there is something wrong with this 'uverbs_ex_cmd_mask' stuff - > the bit should be clear if the driver cannot handle the ex inputs, but > these patches make all drivers handle the ex style (by checking > flags), so all drivers should have some of the bits set.. >
That's a general comment regarding the extension mechanism. Since by nature the extended verbs as extendible, one consumer could support A and B while the other only supports A, but they both indicate they support this extension verb. You could argue that if the "flags" field wasn't tested, we would have need this uverbs_ex_cmd_mask - but because it could be also used by kernel consumers, this check is necessary. > An extended command that doesn't use any extended features should > transparently degrade to the normal command as often as possible. > That means that uverbs_ex_cmd_mask should only be used on extended commands that are user-space specific. Anyway, we could add these IB_USER_VERBS_EX_CMD flags to all vendors, but IMHO this general problem doesn't relate to this series, which is only about adding timestamp support. > Jason Thanks for your comments. Matan -- 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
