Re: [PATCH 09/30] IB/core: Modify ib_verbs and cma in order to use roce_gid_cache

Matan Barak
2015-02-19T15:31:02Z

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?