On 1/9/2017 7:02 PM, Jason Gunthorpe wrote:
On Mon, Jan 09, 2017 at 06:46:33PM +0200, Yishai Hadas wrote:
cmd->rate_limit is not being 0'd if it isn't set
The 'cmd' was fully set to 0'd by the provider, see usage in the last patch.
Why are some providers zeroing this and other are not?
ibv_cmd_modify_qp_ex is used only by mlx5 and it uses the = {}
initialization.
In case we want to do it in this layer I would vote on using memset at the
beginning of that function to prevent repeating this for any new field. Your
last patch in that area could do it as well.
Sounds like you should clean this all up and use = {} consistently in
the providers and drop the explicit = 0.
We can add some pre-patch to the series to make the
ibv_cmd_modify/ibv_cmd_modify_ex behaves similarly by cleaning up the
explicit = 0 and use = {} in all the providers.
We can consider adding an extra check in ibv_cmd_query_qp as part of this
series to check whether IBV_QP_RATE_LIMIT or higher bits were asked, if yes
return an error as there is no support for that value without using an
extended command, makes sense ?
Absolutely.
Will add this check as part of V1 and return an error for that attr_mask
value (i.e IBV_QP_RATE_LIMIT) and for higher future bits.
When the extended query_qp command in the kernel will be added we can
re-discuss whether attr_mask which comes from the application as part of
ibv_query_qp can serve as an output indication and we can stay with
current API from application point of view. My personal tendency is go
with that approach and let applications use one API and handle the
choice whether to call the legacy or the extended command internally
based on attr_mask.
--
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
