Re: [RFC PATCH V2 for-next] RDMA/hns: Add UD support for hip08

[Weihang Li <liweihang@hisilicon.com>]

On 2019/10/22 0:45, Doug Ledford wrote:
> On Mon, 2019-10-21 at 10:58 -0400, Doug Ledford wrote:
>> On Mon, 2019-10-21 at 22:20 +0800, oulijun wrote:
>>> 在 2019/10/21 22:13, Jason Gunthorpe 写道:
>>>> On Sat, Oct 19, 2019 at 04:46:12PM +0800, Lijun Ou wrote:
>>>>> index bd78ff9..722cc5f 100644
>>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
>>>>> @@ -377,6 +377,10 @@ static int hns_roce_set_user_sq_size(struct
>>>>> hns_roce_dev *hr_dev,
>>>>>  		hr_qp->sge.sge_cnt = roundup_pow_of_two(hr_qp-
>>>>>> sq.wqe_cnt *
>>>>>  							(hr_qp-
>>>>>> sq.max_gs - 2));
>>>>>  
>>>>> +	if (hr_qp->ibqp.qp_type == IB_QPT_UD)
>>>>> +		hr_qp->sge.sge_cnt = roundup_pow_of_two(hr_qp-
>>>>>> sq.wqe_cnt *
>>>>> +						       hr_qp-
>>>>>> sq.max_gs);
>>>>> +
>>>>>  	if ((hr_qp->sq.max_gs > 2) && (hr_dev->pci_dev->revision 
>>>>> ==
>>>>> 0x20)) {
>>>>>  		if (hr_qp->sge.sge_cnt > hr_dev-
>>>>>> caps.max_extend_sg) {
>>>>>  			dev_err(hr_dev->dev,
>>>>> @@ -1022,6 +1026,9 @@ struct ib_qp *hns_roce_create_qp(struct
>>>>> ib_pd *pd,
>>>>>  	int ret;
>>>>>  
>>>>>  	switch (init_attr->qp_type) {
>>>>> +	case IB_QPT_UD:
>>>>> +		if (!capable(CAP_NET_RAW))
>>>>> +			return -EPERM;
>>>> This needs a big comment explaining why this HW requires it.
>>>>
>>>> Jason
>>>>
>>> Add the detail comments for HW limit?
>>
>> I can add those comments while taking the pactch.  Plus we need to add
>> a
>> fallthrough annotation at the same place.  I'll fix it up and unfreeze
>> the hns queue.
>>
> 
> Does this meet people's approval?
> 
>         switch (init_attr->qp_type) {
>         case IB_QPT_UD:
>                 /*
>                  * DO NOT REMOVE!
>                  * The HNS RoCE hardware has a security vulnerability.
>                  * Normally, UD packet routing is achieved using nothing
>                  * but the ib_ah struct, which contains the src gid in the
>                  * sgid_attr element.  Th src gid is sufficient for the
>                  * hardware to know if any vlan tag is needed, as well as
>                  * any priority tag.  In the case of HNS RoCE, the vlan
>                  * tag is passed to the hardware along with the src gid.
>                  * This allows a situation where a malicious user could
>                  * intentionally send packets with a gid that belongs to
>                  * vlan A, but direct the packets to go out to vlan B
>                  * instead.
>                  * Because the ability to send out packets with arbitrary
>                  * headers is reserved for CAP_NET_RAW, and because UD
>                  * queue pairs can be tricked into doing that, make all
>                  * UD queue pairs on this hardware require CAP_NET_RAW.
>                  */
>                 if (!capable(CAP_NET_RAW))
>                         return -EPERM;
>                 /* fallthrough */
>         case IB_QPT_RC: {
> 

Hi Doug,

To avoid the potential risk of vlan config issue, we decide to remove this patch.
Thanks a lot for the work of you, Jason and Leon on this issue.

Weihang