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

[Doug Ledford <dledford@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3081 bytes --]

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: {

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]