| Message ID | 1445278165-18442-1-git-send-email-haggaie@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Oct 19, 2015 at 09:09:25PM +0300, Haggai Eran wrote: > When discussing the patches to demux ids in rdma_cm instead of ib_cm, it > was decided that it is best to use the P_Key value in the packet headers > [1]. However, some drivers are currently unable to send correct P_Key in > GMP headers. You should explicitly describe the broken drivers in the commit text. I thought mlx5 was fixed for receive already? I'm confused why we need this. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19/10/2015 21:19, Jason Gunthorpe wrote: > On Mon, Oct 19, 2015 at 09:09:25PM +0300, Haggai Eran wrote: >> When discussing the patches to demux ids in rdma_cm instead of ib_cm, it >> was decided that it is best to use the P_Key value in the packet headers >> [1]. However, some drivers are currently unable to send correct P_Key in >> GMP headers. > > You should explicitly describe the broken drivers in the commit text. These are mlx5 and ipath. I'll add them to the commit message. > I thought mlx5 was fixed for receive already? I'm confused why we need > this. mlx5 had two issues related to GSI pkeys. The issue that was fixed was that it treated the pkey value returned by the hardware in receive completions as a pkey_index. The remaining issue is that it doesn't respect the ib_send_wr.ud.pkey_index field when sending. With the current state of things, cma will try to look for an ipoib net dev matching the BTH pkey of the request, but if the sender is mlx5 or ipath, the BTH pkey would be the default pkey. If the request was intended for a different pkey, cma won't find a matching netdev and will throw away the request. Haggai -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 20, 2015 at 09:45:27AM +0300, Haggai Eran wrote: > On 19/10/2015 21:19, Jason Gunthorpe wrote: > > On Mon, Oct 19, 2015 at 09:09:25PM +0300, Haggai Eran wrote: > >> When discussing the patches to demux ids in rdma_cm instead of ib_cm, it > >> was decided that it is best to use the P_Key value in the packet headers > >> [1]. However, some drivers are currently unable to send correct P_Key in > >> GMP headers. > > > > You should explicitly describe the broken drivers in the commit text. > These are mlx5 and ipath. I'll add them to the commit message. And ipath is actually ipath, the obsolete driver being removed, not qib? (ie we don't care about it?) > The remaining issue is that it doesn't respect the > ib_send_wr.ud.pkey_index field when sending. But this is a very serious bug, to the point the mis-labeled packets may not even be delivered in some cases - you really care about the sub case where mis-labeled packets happen to be deliverable but don't parse right? Well, don't forget to undo this patch when the mlx5 driver is fixed.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/10/2015 19:44, Jason Gunthorpe wrote: > On Tue, Oct 20, 2015 at 09:45:27AM +0300, Haggai Eran wrote: >> On 19/10/2015 21:19, Jason Gunthorpe wrote: >>> On Mon, Oct 19, 2015 at 09:09:25PM +0300, Haggai Eran wrote: >>>> When discussing the patches to demux ids in rdma_cm instead of ib_cm, it >>>> was decided that it is best to use the P_Key value in the packet headers >>>> [1]. However, some drivers are currently unable to send correct P_Key in >>>> GMP headers. >>> >>> You should explicitly describe the broken drivers in the commit text. >> These are mlx5 and ipath. I'll add them to the commit message. > > And ipath is actually ipath, the obsolete driver being removed, not > qib? (ie we don't care about it?) Right. qib is fine. >> The remaining issue is that it doesn't respect the >> ib_send_wr.ud.pkey_index field when sending. > > But this is a very serious bug, to the point the mis-labeled packets > may not even be delivered in some cases - you really care about the > sub case where mis-labeled packets happen to be deliverable but don't > parse right? I understand the issue in mlx5 is serious but we've lived with it a long time. In this patch I only wanted to work around it for the purpose of fixing the regression introduced in 4.3. > Well, don't forget to undo this patch when the mlx5 driver is fixed.. Of course. Thanks, Haggai -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 59a2dafc8c57..e8324543e085 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -1067,14 +1067,14 @@ static int cma_save_req_info(const struct ib_cm_event *ib_event, sizeof(req->local_gid)); req->has_gid = true; req->service_id = req_param->primary_path->service_id; - req->pkey = req_param->bth_pkey; + req->pkey = be16_to_cpu(req_param->primary_path->pkey); break; case IB_CM_SIDR_REQ_RECEIVED: req->device = sidr_param->listen_id->device; req->port = sidr_param->port; req->has_gid = false; req->service_id = sidr_param->service_id; - req->pkey = sidr_param->bth_pkey; + req->pkey = sidr_param->pkey; break; default: return -EINVAL;
When discussing the patches to demux ids in rdma_cm instead of ib_cm, it was decided that it is best to use the P_Key value in the packet headers [1]. However, some drivers are currently unable to send correct P_Key in GMP headers. Change the rdma_cm code to look at the P_Key value that is part of the packet payload as a workaround. Once the drivers are fixed this patch can be reverted. Fixes: 4c21b5bcef73 ("IB/cma: Add net_dev and private data checks to RDMA CM") Signed-off-by: Haggai Eran <haggaie@mellanox.com> --- drivers/infiniband/core/cma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)