diff mbox

[rdma-next,5/6] RDMA/cma: Fix setting RoCE specific path record fields

Message ID 20180111050453.GB16668@ziepe.ca (mailing list archive)
State Not Applicable
Headers show

Commit Message

Jason Gunthorpe Jan. 11, 2018, 5:04 a.m. UTC
On Mon, Jan 08, 2018 at 05:04:47PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav@mellanox.com>
> 
> rdma_set_ib_path() missed setting path record fields for RoCE
> transport when RoCE transport support was added.
> 
> This results into setting incorrect ndev, destination mac address,
> incorrect GID type etc errors. This patch uses recently introduced
> helper function to setup such RoCE specific path record fields.
> 
> Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>
>  drivers/infiniband/core/cma.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 68223bd56b53..f4c6c2cbc585 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -2523,6 +2523,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  {
>  	struct rdma_id_private *id_priv;
>  	struct sa_path_rec *in_path_rec;
> +	struct net_device *ndev;
>  	struct sa_path_rec opa;
>  	int ret;
>  
> @@ -2545,6 +2546,18 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		goto err;
>  	}
>  
> +	if (rdma_protocol_roce(id->device, id->port_num)) {
> +		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
> +		if (!ndev) {
> +			ret = -ENODEV;
> +			kfree(id->route.path_rec);
> +			id->route.path_rec = NULL;
> +			goto err;
> +		} else {
> +			dev_put(ndev);
> +		}
> +	}

I revised this slightly, I didn't like mixing goto error unwind and
in-lined error unwind in the same function, not the pointless else
block.

Please check my work:

Author: Parav Pandit <parav@mellanox.com>
Date:   Mon Jan 8 17:04:47 2018 +0200

    RDMA/cma: Fix rdma_cm raw IB path setting for RoCE
    
    rdma_set_ib_path() missed setting path record fields for RoCE
    transport when RoCE support was added.
    
    This results in setting incorrect ndev, destination mac address,
    incorrect GID type etc errors when user space attempts to set a raw
    IB path using the roce IB path compatibility mapping from userspace.
    
    Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
    Signed-off-by: Parav Pandit <parav@mellanox.com>
    Reviewed-by: Mark Bloch <markb@mellanox.com>
    Signed-off-by: Leon Romanovsky <leon@kernel.org>
    Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>

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

Comments

Parav Pandit Jan. 11, 2018, 1:57 p.m. UTC | #1
> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg@ziepe.ca]
> Sent: Wednesday, January 10, 2018 11:05 PM
> To: Leon Romanovsky <leon@kernel.org>
> Cc: Doug Ledford <dledford@redhat.com>; RDMA mailing list <linux-
> rdma@vger.kernel.org>; Mark Bloch <markb@mellanox.com>; Parav Pandit
> <parav@mellanox.com>
> Subject: Re: [PATCH rdma-next 5/6] RDMA/cma: Fix setting RoCE specific path
> record fields
> 
> On Mon, Jan 08, 2018 at 05:04:47PM +0200, Leon Romanovsky wrote:
> > From: Parav Pandit <parav@mellanox.com>
> >
> > rdma_set_ib_path() missed setting path record fields for RoCE
> > transport when RoCE transport support was added.
> >
> > This results into setting incorrect ndev, destination mac address,
> > incorrect GID type etc errors. This patch uses recently introduced
> > helper function to setup such RoCE specific path record fields.
> >
> > Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > Reviewed-by: Mark Bloch <markb@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leon@kernel.org>
> > drivers/infiniband/core/cma.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/cma.c
> > b/drivers/infiniband/core/cma.c index 68223bd56b53..f4c6c2cbc585
> > 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -2523,6 +2523,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,  {
> >  	struct rdma_id_private *id_priv;
> >  	struct sa_path_rec *in_path_rec;
> > +	struct net_device *ndev;
> >  	struct sa_path_rec opa;
> >  	int ret;
> >
> > @@ -2545,6 +2546,18 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
> >  		goto err;
> >  	}
> >
> > +	if (rdma_protocol_roce(id->device, id->port_num)) {
> > +		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
> > +		if (!ndev) {
> > +			ret = -ENODEV;
> > +			kfree(id->route.path_rec);
> > +			id->route.path_rec = NULL;
> > +			goto err;
> > +		} else {
> > +			dev_put(ndev);
> > +		}
> > +	}
> 
> I revised this slightly, I didn't like mixing goto error unwind and in-lined error
> unwind in the same function, not the pointless else block.
> 
> Please check my work:
> 
> Author: Parav Pandit <parav@mellanox.com>
> Date:   Mon Jan 8 17:04:47 2018 +0200
> 
>     RDMA/cma: Fix rdma_cm raw IB path setting for RoCE
> 
>     rdma_set_ib_path() missed setting path record fields for RoCE
>     transport when RoCE support was added.
> 
>     This results in setting incorrect ndev, destination mac address,
>     incorrect GID type etc errors when user space attempts to set a raw
>     IB path using the roce IB path compatibility mapping from userspace.
> 
>     Fixes: 3c86aa70bf67 ("RDMA/cm: Add RDMA CM support for IBoE devices")
>     Signed-off-by: Parav Pandit <parav@mellanox.com>
>     Reviewed-by: Mark Bloch <markb@mellanox.com>
>     Signed-off-by: Leon Romanovsky <leon@kernel.org>
>     Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index
> 5c158cda08e31d..30d1c32a816f91 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -2521,6 +2521,7 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		     struct sa_path_rec *path_rec)
>  {
>  	struct rdma_id_private *id_priv;
> +	struct net_device *ndev;
>  	int ret;
> 
>  	id_priv = container_of(id, struct rdma_id_private, id); @@ -2535,8
> +2536,21 @@ int rdma_set_ib_path(struct rdma_cm_id *id,
>  		goto err;
>  	}
> 
> +	if (rdma_protocol_roce(id->device, id->port_num)) {
> +		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
> +		if (!ndev) {
> +			ret = -ENODEV;
> +			goto err_free;
> +		}
> +		dev_put(ndev);
> +	}
> +
>  	id->route.num_paths = 1;
>  	return 0;
> +
> +err_free:
> +	kfree(id->route.path_rec);
> +	id->route.path_rec = NULL;
>  err:
>  	cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED,
> RDMA_CM_ADDR_RESOLVED);
>  	return ret;
Looks good.
--
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 mbox

Patch

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 5c158cda08e31d..30d1c32a816f91 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2521,6 +2521,7 @@  int rdma_set_ib_path(struct rdma_cm_id *id,
 		     struct sa_path_rec *path_rec)
 {
 	struct rdma_id_private *id_priv;
+	struct net_device *ndev;
 	int ret;
 
 	id_priv = container_of(id, struct rdma_id_private, id);
@@ -2535,8 +2536,21 @@  int rdma_set_ib_path(struct rdma_cm_id *id,
 		goto err;
 	}
 
+	if (rdma_protocol_roce(id->device, id->port_num)) {
+		ndev = cma_iboe_set_path_rec_l2_fields(id_priv);
+		if (!ndev) {
+			ret = -ENODEV;
+			goto err_free;
+		}
+		dev_put(ndev);
+	}
+
 	id->route.num_paths = 1;
 	return 0;
+
+err_free:
+	kfree(id->route.path_rec);
+	id->route.path_rec = NULL;
 err:
 	cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED, RDMA_CM_ADDR_RESOLVED);
 	return ret;