On Mon, Jun 12, 2017 at 7:15 PM, Hefty, Sean <sean.hefty@xxxxxxxxx> wrote: >> +static bool is_valid_mcast_lid(struct ib_qp *qp, u16 lid) >> +{ >> + struct ib_qp_init_attr init_attr = {}; >> + struct ib_qp_attr attr = {}; >> + int num_eth_ports = 0; >> + int port; >> + >> + /* If QP state >= init, it is assigned to a port and we can >> check this >> + * port only. >> + */ >> + if (!ib_query_qp(qp, &attr, IB_QP_STATE | IB_QP_PORT, >> &init_attr)) { >> + if (attr.qp_state >= IB_QPS_INIT) { >> + if (qp->device->get_link_layer(qp->device, >> attr.port_num) != >> + IB_LINK_LAYER_INFINIBAND) >> + return true; >> + goto lid_check; >> + } >> + } >> + >> + /* Can't get a quick answer, iterate over all ports */ >> + for (port = 0; port < qp->device->phys_port_cnt; port++) >> + if (qp->device->get_link_layer(qp->device, port) != >> + IB_LINK_LAYER_INFINIBAND) >> + num_eth_ports++; >> + >> + /* If we have at lease one Ethernet port, RoCE annex declares >> that >> + * multicast LID should be ignored. We can't tell at this step >> if the >> + * QP belongs to an IB or Ethernet port. >> + */ >> + if (num_eth_ports) >> + return true; > > Reporting that a lid *might* be valid is a really weak check. Can we require that RoCE QPs be in the INIT state prior to joining, or use the mgid, or something to make the validation check work? Do we even know if the qp is a roce qp? > > This is not a join flow but attach flow. AFAIK, attaching a QP to a mcast group can be when QP is in RESET state. This patch does the best it can to give a valid answer but when QP is in RESET state and at least one of the ports is Eth port then we can't invalidate a MLID. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html