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

[Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html