On 19/02/2015 17:31, Matan Barak wrote:
> On 2/19/2015 5:03 PM, Haggai Eran wrote:
>> On 20/02/2015 00:02, Somnath Kotur wrote:
>>> @@ -502,9 +547,7 @@ EXPORT_SYMBOL(ib_create_qp);
>>> static const struct {
>>> int valid;
>>> enum ib_qp_attr_mask req_param[IB_QPT_MAX];
>>> - enum ib_qp_attr_mask req_param_add_eth[IB_QPT_MAX];
>>> enum ib_qp_attr_mask opt_param[IB_QPT_MAX];
>>> - enum ib_qp_attr_mask opt_param_add_eth[IB_QPT_MAX];
>>> } qp_state_table[IB_QPS_ERR + 1][IB_QPS_ERR + 1] = {
>>> [IB_QPS_RESET] = {
>>> [IB_QPS_RESET] = { .valid = 1 },
>>> @@ -585,12 +628,6 @@ static const struct {
>>> IB_QP_MAX_DEST_RD_ATOMIC |
>>> IB_QP_MIN_RNR_TIMER),
>>> },
>>> - .req_param_add_eth = {
>>> - [IB_QPT_RC] = (IB_QP_SMAC),
>>> - [IB_QPT_UC] = (IB_QP_SMAC),
>>> - [IB_QPT_XRC_INI] = (IB_QP_SMAC),
>>> - [IB_QPT_XRC_TGT] = (IB_QP_SMAC)
>>> - },
>>> .opt_param = {
>>> [IB_QPT_UD] = (IB_QP_PKEY_INDEX |
>>> IB_QP_QKEY),
>>> @@ -611,21 +648,7 @@ static const struct {
>>> [IB_QPT_GSI] = (IB_QP_PKEY_INDEX |
>>> IB_QP_QKEY),
>>> },
>>> - .opt_param_add_eth = {
>>> - [IB_QPT_RC] = (IB_QP_ALT_SMAC |
>>> - IB_QP_VID |
>>> - IB_QP_ALT_VID),
>>> - [IB_QPT_UC] = (IB_QP_ALT_SMAC |
>>> - IB_QP_VID |
>>> - IB_QP_ALT_VID),
>>> - [IB_QPT_XRC_INI] = (IB_QP_ALT_SMAC |
>>> - IB_QP_VID |
>>> - IB_QP_ALT_VID),
>>> - [IB_QPT_XRC_TGT] = (IB_QP_ALT_SMAC |
>>> - IB_QP_VID |
>>> - IB_QP_ALT_VID)
>>> - }
>>> - }
>>> + },
>>> },
>>> [IB_QPS_RTR] = {
>>> [IB_QPS_RESET] = { .valid = 1 },
>>> @@ -847,13 +870,6 @@ int ib_modify_qp_is_ok(enum ib_qp_state
>>> cur_state, enum ib_qp_state next_state,
>>> req_param = qp_state_table[cur_state][next_state].req_param[type];
>>> opt_param = qp_state_table[cur_state][next_state].opt_param[type];
>>>
>>> - if (ll == IB_LINK_LAYER_ETHERNET) {
>>> - req_param |= qp_state_table[cur_state][next_state].
>>> - req_param_add_eth[type];
>>> - opt_param |= qp_state_table[cur_state][next_state].
>>> - opt_param_add_eth[type];
>>> - }
>>> -
>>> if ((mask & req_param) != req_param)
>>> return 0;
>>
>> I understand this patch will remove any kernel reference to these
>> modify_qp attributes. However, what about user-space? Was it previously
>> allowed to pass in these parameters?
>
> There was no libibverbs that declared those flags. It was filled by
> ib_resolve_eth_l2_attrs. If someone wrote a custom libibverbs that
> passed those flags, they would have just been ignored. We could replace
> them as reserved flags. What do you think?
I guess if there's no existing user space it's okay. Perhaps it would be
best to add some explicit input-checking to the ib_uverbs_modify_qp()
verb to prevent such dilemmas in the future.
--
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
