diff mbox

[for-next,3/3] IB/mlx5: Fix mlx5_set_path for Raw Packet QP

Message ID 1467549727-23479-4-git-send-email-leon@kernel.org (mailing list archive)
State Changes Requested
Headers show

Commit Message

Leon Romanovsky July 3, 2016, 12:42 p.m. UTC 
From: Talat Batheesh <talatb@mellanox.com>

mlx5_set_path returns error when called without GRH in Ethernet
link layer. Since this is not mandatory for Raw Packet QP, it
shouldn't fail in this case.

Added additional check of QP type to make the decision.

Fixes: 2811ba51b049 ('IB/mlx5: Add RoCE fields to Address Vector')
Signed-off-by: Talat Batheesh <talatb@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/hw/mlx5/qp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Or Gerlitz July 3, 2016, 9:55 p.m. UTC | #1 
On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon@kernel.org> wrote:

> --- a/drivers/infiniband/hw/mlx5/qp.c
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -2175,7 +2175,8 @@ static int mlx5_set_path(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
>         }
>
>         if (ll == IB_LINK_LAYER_ETHERNET) {
> -               if (!(ah->ah_flags & IB_AH_GRH))
> +               if (!(ah->ah_flags & IB_AH_GRH) &&
> +                   (qp->ibqp.qp_type != IB_QPT_RAW_PACKET))
>                         return -EINVAL;
>                 memcpy(path->rmac, ah->dmac, sizeof(ah->dmac));
>                 path->udp_sport = mlx5_get_roce_udp_sport(dev, port,

As you can see here, that code was written with RoCE state of mind,
e.g they set the remote mac and udp source port, with both being
irrelevant for RAW packet QPs. So in that respect, your fix is wrong.
Make the code clear and correct in what they set to RoCE QPs, to RAW
Packet QPs and to both.
--
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
Leon Romanovsky July 4, 2016, 5:14 a.m. UTC | #2 
On Mon, Jul 04, 2016 at 12:55:36AM +0300, Or Gerlitz wrote:
> On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon@kernel.org> wrote:
> 
> > --- a/drivers/infiniband/hw/mlx5/qp.c
> > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > @@ -2175,7 +2175,8 @@ static int mlx5_set_path(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
> >         }
> >
> >         if (ll == IB_LINK_LAYER_ETHERNET) {
> > -               if (!(ah->ah_flags & IB_AH_GRH))
> > +               if (!(ah->ah_flags & IB_AH_GRH) &&
> > +                   (qp->ibqp.qp_type != IB_QPT_RAW_PACKET))
> >                         return -EINVAL;
> >                 memcpy(path->rmac, ah->dmac, sizeof(ah->dmac));
> >                 path->udp_sport = mlx5_get_roce_udp_sport(dev, port,
> 
> As you can see here, that code was written with RoCE state of mind,
> e.g they set the remote mac and udp source port, with both being
> irrelevant for RAW packet QPs. So in that respect, your fix is wrong.
> Make the code clear and correct in what they set to RoCE QPs, to RAW
> Packet QPs and to both.

If I read your response and code correctly, we can safely drop this
patch. Am I right?

> --
> 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
Or Gerlitz July 4, 2016, 12:29 p.m. UTC | #3 
On Mon, Jul 4, 2016 at 8:14 AM, Leon Romanovsky <leonro@mellanox.com> wrote:
> On Mon, Jul 04, 2016 at 12:55:36AM +0300, Or Gerlitz wrote:

> If I read your response and code correctly, we can safely drop this
> patch. Am I right?

Of course no. You are fixing a bug and I pointed you that you need to
extend the fix a bit.

Or.
--
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/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index e9b3a1f..fbd94a9 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -2175,7 +2175,8 @@  static int mlx5_set_path(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
 	}
 
 	if (ll == IB_LINK_LAYER_ETHERNET) {
-		if (!(ah->ah_flags & IB_AH_GRH))
+		if (!(ah->ah_flags & IB_AH_GRH) &&
+		    (qp->ibqp.qp_type != IB_QPT_RAW_PACKET))
 			return -EINVAL;
 		memcpy(path->rmac, ah->dmac, sizeof(ah->dmac));
 		path->udp_sport = mlx5_get_roce_udp_sport(dev, port,