[PATCH for-next 0/3] Raw Packet QP Fixes for 4.8

[PATCH for-next 0/3] Raw Packet QP Fixes for 4.8

[Leon Romanovsky]

Hi Doug,

This patch series includes raw packet QP fixes which are targeted for 4.8.
They apply cleanly on top of your k.o/for-4.8 branch.

These patches are available in the "topic/fix_raw_eth_qos" branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git

Or for browsing:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/fix_raw_eth_qos

Thanks

Talat Batheesh (3):
  IB/core: Add optional AV attribute for Raw Packet QP in INIT to RTR
  IB/core: Fix ib_resolve_eth_dmac for Raw Packet QP
  IB/mlx5: Fix mlx5_set_path for Raw Packet QP

 drivers/infiniband/core/verbs.c | 3 ++-
 drivers/infiniband/hw/mlx5/qp.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 1/3] IB/core: Add optional AV attribute for Raw Packet QP in INIT to RTR

[Leon Romanovsky]

From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In order to configure QoS for Raw Packet QP, the user passes the SL
in the AV attribute when modifying the QP from INIT to RTR state.

This SL is used later on to set the Ethernet Priority of the QP.

Until today, we couldn't pass the AV attribute for Raw Packet QP
because it was blocked in the QP state transition matrix. This
patch adds the AV as optional in INIT to RTR.

Fixes: 8a51866f0810 ('IB: Add ib_modify_qp_is_ok() library function')
Signed-off-by: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/verbs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 2e813ed..ba75780 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -932,6 +932,7 @@ static const struct {
 						 IB_QP_QKEY),
 				 [IB_QPT_GSI] = (IB_QP_PKEY_INDEX		|
 						 IB_QP_QKEY),
+				 [IB_QPT_RAW_PACKET] = IB_QP_AV,
 			 },
 		},
 	},
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 1/3] IB/core: Add optional AV attribute for Raw Packet QP in INIT to RTR

[Or Gerlitz]

On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> In order to configure QoS for Raw Packet QP, the user passes the SL
> in the AV attribute when modifying the QP from INIT to RTR state.
>
> This SL is used later on to set the Ethernet Priority of the QP.
>
> Until today, we couldn't pass the AV attribute for Raw Packet QP
> because it was blocked in the QP state transition matrix. This
> patch adds the AV as optional in INIT to RTR.
>
> Fixes: 8a51866f0810 ('IB: Add ib_modify_qp_is_ok() library function')

This commit is 10y old and was done way before RAW Packet QP was added
to the kernel, please fix your Fixes: line...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 1/3] IB/core: Add optional AV attribute for Raw Packet QP in INIT to RTR

[Jason Gunthorpe]

On Mon, Jul 04, 2016 at 12:51:00AM +0300, Or Gerlitz wrote:
> On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > In order to configure QoS for Raw Packet QP, the user passes the SL
> > in the AV attribute when modifying the QP from INIT to RTR state.
> >
> > This SL is used later on to set the Ethernet Priority of the QP.
> >
> > Until today, we couldn't pass the AV attribute for Raw Packet QP
> > because it was blocked in the QP state transition matrix. This
> > patch adds the AV as optional in INIT to RTR.
> >
> > Fixes: 8a51866f0810 ('IB: Add ib_modify_qp_is_ok() library function')
> 
> This commit is 10y old and was done way before RAW Packet QP was added
> to the kernel, please fix your Fixes: line...

It isn't a fixes if you are adding a new feature.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 1/3] IB/core: Add optional AV attribute for Raw Packet QP in INIT to RTR

[Leon Romanovsky]

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

On Mon, Jul 04, 2016 at 09:33:37AM -0600, Jason Gunthorpe wrote:
> On Mon, Jul 04, 2016 at 12:51:00AM +0300, Or Gerlitz wrote:
> > On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > In order to configure QoS for Raw Packet QP, the user passes the SL
> > > in the AV attribute when modifying the QP from INIT to RTR state.
> > >
> > > This SL is used later on to set the Ethernet Priority of the QP.
> > >
> > > Until today, we couldn't pass the AV attribute for Raw Packet QP
> > > because it was blocked in the QP state transition matrix. This
> > > patch adds the AV as optional in INIT to RTR.
> > >
> > > Fixes: 8a51866f0810 ('IB: Add ib_modify_qp_is_ok() library function')
> > 
> > This commit is 10y old and was done way before RAW Packet QP was added
> > to the kernel, please fix your Fixes: line...
> 
> It isn't a fixes if you are adding a new feature.

You are right, I'll resubmit.

> 
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH for-next 2/3] IB/core: Fix ib_resolve_eth_dmac for Raw Packet QP

[Leon Romanovsky]

From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

For Raw Packet QP, the AV attribute is passed in INIT to RTR in order
to set the Ethernet Priority using the SL. The rest of the fields
aren't necessarily valid, which causes the ib_resolve_eth_dmac to fail.

Since no release address resolution is needed, don't do anything in
ib_resolve_eth_dmac for Raw Packet QP.

Fixes: dbf727de7440 ('IB/core: Use GID table in AH creation and ...')
Signed-off-by: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index ba75780..d41e403 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1171,7 +1171,7 @@ int ib_resolve_eth_dmac(struct ib_qp *qp,
 {
 	int           ret = 0;
 
-	if (*qp_attr_mask & IB_QP_AV) {
+	if ((*qp_attr_mask & IB_QP_AV) && (qp->qp_type != IB_QPT_RAW_PACKET)) {
 		if (qp_attr->ah_attr.port_num < rdma_start_port(qp->device) ||
 		    qp_attr->ah_attr.port_num > rdma_end_port(qp->device))
 			return -EINVAL;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 2/3] IB/core: Fix ib_resolve_eth_dmac for Raw Packet QP

[Or Gerlitz]

On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> For Raw Packet QP, the AV attribute is passed in INIT to RTR in order
> to set the Ethernet Priority using the SL. The rest of the fields
> aren't necessarily valid, which causes the ib_resolve_eth_dmac to fail.
>
> Since no release address resolution is needed, don't do anything in
> ib_resolve_eth_dmac for Raw Packet QP.

What "release address resolution" means?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 2/3] IB/core: Fix ib_resolve_eth_dmac for Raw Packet QP

[Talat Batheesh]

Sent from my iPhone
> On 3 Jul 2016, at 3:52 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
>> On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> 
>> For Raw Packet QP, the AV attribute is passed in INIT to RTR in order
>> to set the Ethernet Priority using the SL. The rest of the fields
>> aren't necessarily valid, which causes the ib_resolve_eth_dmac to fail.
>> 
>> Since no release address resolution is needed, don't do anything in
>> ib_resolve_eth_dmac for Raw Packet QP.
> 
> What "release address resolution" means?
Oops sorry for the typo error, "real" not "release".

Yours,
Talat
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 2/3] IB/core: Fix ib_resolve_eth_dmac for Raw Packet QP

[Or Gerlitz]

On Mon, Jul 4, 2016 at 9:10 AM, Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> On 3 Jul 2016, at 3:52 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>> What "release address resolution" means?

> Oops sorry for the typo error, "real" not "release".

whatever, fix it for V2, unless you really (really?) want that to be
for ever in the kernel log in this confusing way
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Leon Romanovsky]

From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/qp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Or Gerlitz]

On Sun, Jul 3, 2016 at 3:42 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Leon Romanovsky]

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

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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Or Gerlitz]

On Mon, Jul 4, 2016 at 8:14 AM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/3] Raw Packet QP Fixes for 4.8

[Leon Romanovsky]

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

On Sun, Jul 03, 2016 at 03:42:04PM +0300, Leon Romanovsky wrote:
> Hi Doug,
> 
> This patch series includes raw packet QP fixes which are targeted for 4.8.
> They apply cleanly on top of your k.o/for-4.8 branch.

Doug,
Please drop this series, "fixes line" misled me.
Sorry for inconvenience.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]