[PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Leon Romanovsky]

From: Daniel Jurgens <danielj@mellanox.com>

For now the only LSM security enforcement mechanism available is
specific to InfiniBand. Bypass enforcement for non-IB link types.
This fixes a regression where modify_qp fails for iWARP because
querying the PKEY returns -EINVAL.

Cc: Paul Moore <paul@paul-moore.com>
Cc: Don Dutile <ddutile@redhat.com>
Cc: stable@vger.kernel.org
Reported-by: Potnuri Bharat Teja <bharat@chelsio.com>
Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
Reviewed-by: Parav Pandit <parav@mellanox.com>
Tested-by: Potnuri Bharat Teja <bharat@chelsio.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/core/security.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
index 23278ed5be45..314bf1137c7b 100644
--- a/drivers/infiniband/core/security.c
+++ b/drivers/infiniband/core/security.c
@@ -417,8 +417,17 @@ void ib_close_shared_qp_security(struct ib_qp_security *sec)
 
 int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev)
 {
+	u8 i = rdma_start_port(dev);
+	bool is_ib = false;
 	int ret;
 
+	while (i <= rdma_end_port(dev) && !is_ib)
+		is_ib = rdma_protocol_ib(dev, i++);
+
+	/* If this isn't an IB device don't create the security context */
+	if (!is_ib)
+		return 0;
+
 	qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL);
 	if (!qp->qp_sec)
 		return -ENOMEM;
-- 
2.15.0

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Mark Bloch]

Hi,

On 21/11/2017 12:26, Leon Romanovsky wrote:
> From: Daniel Jurgens <danielj@mellanox.com>
> 
> For now the only LSM security enforcement mechanism available is
> specific to InfiniBand. Bypass enforcement for non-IB link types.
> This fixes a regression where modify_qp fails for iWARP because
> querying the PKEY returns -EINVAL.
> 
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Don Dutile <ddutile@redhat.com>
> Cc: stable@vger.kernel.org
> Reported-by: Potnuri Bharat Teja <bharat@chelsio.com>
> Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
> Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Tested-by: Potnuri Bharat Teja <bharat@chelsio.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>
> ---
>  drivers/infiniband/core/security.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
> index 23278ed5be45..314bf1137c7b 100644
> --- a/drivers/infiniband/core/security.c
> +++ b/drivers/infiniband/core/security.c
> @@ -417,8 +417,17 @@ void ib_close_shared_qp_security(struct ib_qp_security *sec)
>  
>  int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev)
>  {
> +	u8 i = rdma_start_port(dev);
> +	bool is_ib = false;
>  	int ret;
>  
> +	while (i <= rdma_end_port(dev) && !is_ib)
> +		is_ib = rdma_protocol_ib(dev, i++);
> +

What happens if we have mixed port types?
I believe mlx4 can expose two ports where each port uses a different ll protocol.
Was that changed?

> +	/* If this isn't an IB device don't create the security context */
> +	if (!is_ib)
> +		return 0;
> +
>  	qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL);
>  	if (!qp->qp_sec)
>  		return -ENOMEM;
> 

Mark.

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Leon Romanovsky]

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

On Tue, Nov 21, 2017 at 12:44:10PM +0200, Mark Bloch wrote:
> Hi,
>
> On 21/11/2017 12:26, Leon Romanovsky wrote:
> > From: Daniel Jurgens <danielj@mellanox.com>
> >
> > For now the only LSM security enforcement mechanism available is
> > specific to InfiniBand. Bypass enforcement for non-IB link types.
> > This fixes a regression where modify_qp fails for iWARP because
> > querying the PKEY returns -EINVAL.
> >
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Don Dutile <ddutile@redhat.com>
> > Cc: stable@vger.kernel.org
> > Reported-by: Potnuri Bharat Teja <bharat@chelsio.com>
> > Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
> > Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
> > Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > Tested-by: Potnuri Bharat Teja <bharat@chelsio.com>
> > Signed-off-by: Leon Romanovsky <leon@kernel.org>
> > ---
> >  drivers/infiniband/core/security.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
> > index 23278ed5be45..314bf1137c7b 100644
> > --- a/drivers/infiniband/core/security.c
> > +++ b/drivers/infiniband/core/security.c
> > @@ -417,8 +417,17 @@ void ib_close_shared_qp_security(struct ib_qp_security *sec)
> >
> >  int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev)
> >  {
> > +	u8 i = rdma_start_port(dev);
> > +	bool is_ib = false;
> >  	int ret;
> >
> > +	while (i <= rdma_end_port(dev) && !is_ib)
> > +		is_ib = rdma_protocol_ib(dev, i++);
> > +
>
> What happens if we have mixed port types?

We will have is_ib set and qp_sec will be allocated on device layer and
not on port level, but because pkeys are IB specific term (at least, I
didn't find any mentioning in RoCE spec), the modify_qp won't query
for PKEYS.

> I believe mlx4 can expose two ports where each port uses a different ll protocol.
> Was that changed?

It is still true.

>
> > +	/* If this isn't an IB device don't create the security context */
> > +	if (!is_ib)
> > +		return 0;
> > +
> >  	qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL);
> >  	if (!qp->qp_sec)
> >  		return -ENOMEM;
> >
>
> Mark.
> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Mark Bloch]

On 21/11/2017 15:22, Leon Romanovsky wrote:
> On Tue, Nov 21, 2017 at 12:44:10PM +0200, Mark Bloch wrote:
>> Hi,
>>
>> On 21/11/2017 12:26, Leon Romanovsky wrote:
>>> From: Daniel Jurgens <danielj@mellanox.com>
>>>
>>> For now the only LSM security enforcement mechanism available is
>>> specific to InfiniBand. Bypass enforcement for non-IB link types.
>>> This fixes a regression where modify_qp fails for iWARP because
>>> querying the PKEY returns -EINVAL.
>>>
>>> Cc: Paul Moore <paul@paul-moore.com>
>>> Cc: Don Dutile <ddutile@redhat.com>
>>> Cc: stable@vger.kernel.org
>>> Reported-by: Potnuri Bharat Teja <bharat@chelsio.com>
>>> Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
>>> Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
>>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
>>> Reviewed-by: Parav Pandit <parav@mellanox.com>
>>> Tested-by: Potnuri Bharat Teja <bharat@chelsio.com>
>>> Signed-off-by: Leon Romanovsky <leon@kernel.org>
>>> ---
>>>  drivers/infiniband/core/security.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
>>> index 23278ed5be45..314bf1137c7b 100644
>>> --- a/drivers/infiniband/core/security.c
>>> +++ b/drivers/infiniband/core/security.c
>>> @@ -417,8 +417,17 @@ void ib_close_shared_qp_security(struct ib_qp_security *sec)
>>>
>>>  int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev)
>>>  {
>>> +	u8 i = rdma_start_port(dev);
>>> +	bool is_ib = false;
>>>  	int ret;
>>>
>>> +	while (i <= rdma_end_port(dev) && !is_ib)
>>> +		is_ib = rdma_protocol_ib(dev, i++);
>>> +
>>
>> What happens if we have mixed port types?
> 
> We will have is_ib set and qp_sec will be allocated on device layer and
> not on port level, but because pkeys are IB specific term (at least, I
> didn't find any mentioning in RoCE spec), the modify_qp won't query
> for PKEYS.
> 
What is port level for qp_sec?

I just gave mlx4 as an example, but I was talking more about the ability of the RDMA
subsystem to support mixed port types, so in this case, if in the future a vendor
will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.

>> I believe mlx4 can expose two ports where each port uses a different ll protocol.
>> Was that changed?
> 
> It is still true.
> 
>>
>>> +	/* If this isn't an IB device don't create the security context */
>>> +	if (!is_ib)
>>> +		return 0;
>>> +
>>>  	qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL);
>>>  	if (!qp->qp_sec)
>>>  		return -ENOMEM;
>>>
>>
>> Mark.
>> --
>> 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

Mark

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Jason Gunthorpe]

On Tue, Nov 21, 2017 at 03:56:19PM +0200, Mark Bloch wrote:

> I just gave mlx4 as an example, but I was talking more about the ability of the RDMA
> subsystem to support mixed port types, so in this case, if in the future a vendor
> will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.

That will never be allowed.

Even mixing roce and IB on the same device should be banned, IMHO.

If APM does not work between the ports then they do not belong on the
same device.

Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Mark Bloch]

On 21/11/2017 17:14, Jason Gunthorpe wrote:
> On Tue, Nov 21, 2017 at 03:56:19PM +0200, Mark Bloch wrote:
> 
>> I just gave mlx4 as an example, but I was talking more about the ability of the RDMA
>> subsystem to support mixed port types, so in this case, if in the future a vendor
>> will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.
> 
> That will never be allowed.
> 

You say never be allowed, I say code talks, and in the code we don't have this restriction.
maybe we should add something?

LSM security enforcement should only take place on IB devices, Daniel's comment even says that:

> +	/* If this isn't an IB device don't create the security context */
> +	if (!is_ib)
> +		return 0;

but how do we define an ib device with different port types?
also while today we only deal with pkeys (if I remember currently) in the future
we might add other bits, and those bits might not play nicely in the that configuration.

Maybe we should make sure all the ports are IB, and if not, flag it to the user (dmesg?)

> Even mixing roce and IB on the same device should be banned, IMHO.

> If APM does not work between the ports then they do not belong on the
> same device.
> 
> Jason
> 

Mark.

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Jason Gunthorpe]

On Tue, Nov 21, 2017 at 05:33:04PM +0200, Mark Bloch wrote:

> You say never be allowed, I say code talks, and in the code we don't
> have this restriction.  maybe we should add something?

I say that because I will NAK any attempt to merge such a driver.

I'd welcome patches to encforce this restriction in the core.

> Maybe we should make sure all the ports are IB, and if not, flag it
> to the user (dmesg?)

I would welcome this patch too.

Will this break any mellanox drivers?

Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Daniel Jurgens]

On 11/21/2017 9:33 AM, Mark Bloch wrote:
>
> On 21/11/2017 17:14, Jason Gunthorpe wrote:
>> On Tue, Nov 21, 2017 at 03:56:19PM +0200, Mark Bloch wrote:
>>
>>> I just gave mlx4 as an example, but I was talking more about the ability of the RDMA
>>> subsystem to support mixed port types, so in this case, if in the future a vendor
>>> will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.
>> That will never be allowed.
>>
> You say never be allowed, I say code talks, and in the code we don't have this restriction.
> maybe we should add something?
>
> LSM security enforcement should only take place on IB devices, Daniel's comment even says that:
>
>> +	/* If this isn't an IB device don't create the security context */
>> +	if (!is_ib)
>> +		return 0;
> but how do we define an ib device with different port types?
> also while today we only deal with pkeys (if I remember currently) in the future
> we might add other bits, and those bits might not play nicely in the that configuration.
>
> Maybe we should make sure all the ports are IB, and if not, flag it to the user (dmesg?)

The only warning that would make sense is if the mixed ports aren't all IB or RoCE. As you note, CX-3 can mix those two, we don't want to see warnings about that.

>
>> Even mixing roce and IB on the same device should be banned, IMHO.
>> If APM does not work between the ports then they do not belong on the
>> same device.
>>
>> Jason
>>
> Mark.

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Jason Gunthorpe]

On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:

> The only warning that would make sense is if the mixed ports aren't
> all IB or RoCE. As you note, CX-3 can mix those two, we don't want to
> see warnings about that.

I would really like to see cx3 be changed to not do that, then we
could finalize this issue upstream: All device ports must be the same
protocol.

Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Leon Romanovsky]

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

On Tue, Nov 21, 2017 at 09:04:42AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:
>
> > The only warning that would make sense is if the mixed ports aren't
> > all IB or RoCE. As you note, CX-3 can mix those two, we don't want to
> > see warnings about that.
>
> I would really like to see cx3 be changed to not do that, then we
> could finalize this issue upstream: All device ports must be the same
> protocol.

I don't see the point of such artificial limitation, the users who
brought CX-3 have option to work in mixed mode and IMHO it is not right
to deprecate such ability just because it is hard for us to code for it.

Thanks

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Jason Gunthorpe]

On Tue, Nov 21, 2017 at 06:34:54PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 21, 2017 at 09:04:42AM -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:
> >
> > > The only warning that would make sense is if the mixed ports aren't
> > > all IB or RoCE. As you note, CX-3 can mix those two, we don't want to
> > > see warnings about that.
> >
> > I would really like to see cx3 be changed to not do that, then we
> > could finalize this issue upstream: All device ports must be the same
> > protocol.
> 
> I don't see the point of such artificial limitation, the users who
> brought CX-3 have option to work in mixed mode and IMHO it is not right
> to deprecate such ability just because it is hard for us to code for it.

I don't really think it is really too user visible.. Only the device
and port number change, but only if running in mixed mode.

It is not just 'hard for us' it is impossible to reconcile the
differences between ports when enforcing device level things.

This keeps coming up again and again..

Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Leon Romanovsky]

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

On Tue, Nov 21, 2017 at 09:36:48AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 21, 2017 at 06:34:54PM +0200, Leon Romanovsky wrote:
> > On Tue, Nov 21, 2017 at 09:04:42AM -0700, Jason Gunthorpe wrote:
> > > On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:
> > >
> > > > The only warning that would make sense is if the mixed ports aren't
> > > > all IB or RoCE. As you note, CX-3 can mix those two, we don't want to
> > > > see warnings about that.
> > >
> > > I would really like to see cx3 be changed to not do that, then we
> > > could finalize this issue upstream: All device ports must be the same
> > > protocol.
> >
> > I don't see the point of such artificial limitation, the users who
> > brought CX-3 have option to work in mixed mode and IMHO it is not right
> > to deprecate such ability just because it is hard for us to code for it.
>
> I don't really think it is really too user visible.. Only the device
> and port number change, but only if running in mixed mode.

Ahh, correct me if I'm wrong, you are proposing to split mlx4_ib devices
to two devices once it is configured in mixed mode, so everyone will
have one port only. Did I understand you correctly?

>
> It is not just 'hard for us' it is impossible to reconcile the
> differences between ports when enforcing device level things.
>
> This keeps coming up again and again..
>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Jason Gunthorpe]

On Tue, Nov 21, 2017 at 06:48:02PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 21, 2017 at 09:36:48AM -0700, Jason Gunthorpe wrote:

> Ahh, correct me if I'm wrong, you are proposing to split mlx4_ib devices
> to two devices once it is configured in mixed mode, so everyone will
> have one port only. Did I understand you correctly?

Right, but only for mixed mode. dual port roce or dual port ib is not
altered.

Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Don Dutile]

On 11/21/2017 11:48 AM, Leon Romanovsky wrote:
> On Tue, Nov 21, 2017 at 09:36:48AM -0700, Jason Gunthorpe wrote:
>> On Tue, Nov 21, 2017 at 06:34:54PM +0200, Leon Romanovsky wrote:
>>> On Tue, Nov 21, 2017 at 09:04:42AM -0700, Jason Gunthorpe wrote:
>>>> On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:
>>>>
>>>>> The only warning that would make sense is if the mixed ports aren't
>>>>> all IB or RoCE. As you note, CX-3 can mix those two, we don't want to
>>>>> see warnings about that.
>>>>
>>>> I would really like to see cx3 be changed to not do that, then we
>>>> could finalize this issue upstream: All device ports must be the same
>>>> protocol.
>>>
>>> I don't see the point of such artificial limitation, the users who
>>> brought CX-3 have option to work in mixed mode and IMHO it is not right
>>> to deprecate such ability just because it is hard for us to code for it.
>>
>> I don't really think it is really too user visible.. Only the device
>> and port number change, but only if running in mixed mode.
>
> Ahh, correct me if I'm wrong, you are proposing to split mlx4_ib devices
> to two devices once it is configured in mixed mode, so everyone will
> have one port only. Did I understand you correctly?
>
Which would require splitting resources that are shared now? other splitting issue(s)?

>>
>> It is not just 'hard for us' it is impossible to reconcile the
>> differences between ports when enforcing device level things.
>>
>> This keeps coming up again and again..
>>
>> Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Jason Gunthorpe]

On Tue, Nov 21, 2017 at 01:16:43PM -0500, Don Dutile wrote:

> >>I don't really think it is really too user visible.. Only the device
> >>and port number change, but only if running in mixed mode.
> >
> >Ahh, correct me if I'm wrong, you are proposing to split mlx4_ib devices
> >to two devices once it is configured in mixed mode, so everyone will
> >have one port only. Did I understand you correctly?

> Which would require splitting resources that are shared now? other
> splitting issue(s)?

A change like this would be almost transparent to most users. The cx3
card in mixed mode will look like a mlx5 card does: two single port
RDMA devices.

I think only a minimal amount of stuff is actually shared between
ports, and as cx5 already demonstrates there isn't really a notable
downside to having two kernel RDMA devices.

Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Leon Romanovsky]

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

On Tue, Nov 21, 2017 at 11:57:44AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 21, 2017 at 01:16:43PM -0500, Don Dutile wrote:
>
> > >>I don't really think it is really too user visible.. Only the device
> > >>and port number change, but only if running in mixed mode.
> > >
> > >Ahh, correct me if I'm wrong, you are proposing to split mlx4_ib devices
> > >to two devices once it is configured in mixed mode, so everyone will
> > >have one port only. Did I understand you correctly?
>
> > Which would require splitting resources that are shared now? other
> > splitting issue(s)?
>
> A change like this would be almost transparent to most users. The cx3
> card in mixed mode will look like a mlx5 card does: two single port
> RDMA devices.
>
> I think only a minimal amount of stuff is actually shared between
> ports, and as cx5 already demonstrates there isn't really a notable
> downside to having two kernel RDMA devices.

CX4+ cards have completely different HW architecture in opposite to CX3,
and mlx5 implementation doesn't mean that your suggestion is doable for
mlx4. It worth to check it.

Thanks

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Don Dutile]

On 11/21/2017 11:34 AM, Leon Romanovsky wrote:
> On Tue, Nov 21, 2017 at 09:04:42AM -0700, Jason Gunthorpe wrote:
>> On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:
>>
>>> The only warning that would make sense is if the mixed ports aren't
>>> all IB or RoCE. As you note, CX-3 can mix those two, we don't want to
>>> see warnings about that.
>>
>> I would really like to see cx3 be changed to not do that, then we
>> could finalize this issue upstream: All device ports must be the same
>> protocol.
>
> I don't see the point of such artificial limitation, the users who
> brought CX-3 have option to work in mixed mode and IMHO it is not right
> to deprecate such ability just because it is hard for us to code for it.
>
We use cx-3 (& cx-4 & cx-5) in mixed mode all over our RDMA cluster.
Loosing that feature is not an option.

> Thanks
>
>>
>> Jason

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Daniel Jurgens]

On 11/21/2017 4:44 AM, Mark Bloch wrote:
> Hi,
>
> On 21/11/2017 12:26, Leon Romanovsky wrote:
>> From: Daniel Jurgens <danielj@mellanox.com>
>>
>> For now the only LSM security enforcement mechanism available is
>> specific to InfiniBand. Bypass enforcement for non-IB link types.
>> This fixes a regression where modify_qp fails for iWARP because
>> querying the PKEY returns -EINVAL.
>>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Cc: Don Dutile <ddutile@redhat.com>
>> Cc: stable@vger.kernel.org
>> Reported-by: Potnuri Bharat Teja <bharat@chelsio.com>
>> Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
>> Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
>> Reviewed-by: Parav Pandit <parav@mellanox.com>
>> Tested-by: Potnuri Bharat Teja <bharat@chelsio.com>
>> Signed-off-by: Leon Romanovsky <leon@kernel.org>
>> ---
>>  drivers/infiniband/core/security.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
>> index 23278ed5be45..314bf1137c7b 100644
>> --- a/drivers/infiniband/core/security.c
>> +++ b/drivers/infiniband/core/security.c
>> @@ -417,8 +417,17 @@ void ib_close_shared_qp_security(struct ib_qp_security *sec)
>>  
>>  int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev)
>>  {
>> +	u8 i = rdma_start_port(dev);
>> +	bool is_ib = false;
>>  	int ret;
>>  
>> +	while (i <= rdma_end_port(dev) && !is_ib)
>> +		is_ib = rdma_protocol_ib(dev, i++);
>> +
> What happens if we have mixed port types?
> I believe mlx4 can expose two ports where each port uses a different ll protocol.
> Was that changed?

If any port on the device is IB security is enforced on all QPs because a QP could eventually be modified to an IB port. The primary motivation was to fix a regression in iWARP, which has no concept of PKeys, queries of the PKey cache always returned -EINVAL.  There is a valid PKey cache for RoCE, so it's doesn't suffer the same issue, but this will benefit an all RoCE port device by eliminating the overhead of security enforcement.  If there is ever a multiport device that mixes iWARP and IB this would be a problem, but I don't see that happening.

>
>> +	/* If this isn't an IB device don't create the security context */
>> +	if (!is_ib)
>> +		return 0;
>> +
>>  	qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL);
>>  	if (!qp->qp_sec)
>>  		return -ENOMEM;
>>
> Mark.

Re: [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand

[Leon Romanovsky]

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

On Tue, Nov 21, 2017 at 12:26:17PM +0200, Leon Romanovsky wrote:
> From: Daniel Jurgens <danielj@mellanox.com>
>
> For now the only LSM security enforcement mechanism available is
> specific to InfiniBand. Bypass enforcement for non-IB link types.
> This fixes a regression where modify_qp fails for iWARP because
> querying the PKEY returns -EINVAL.
>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Don Dutile <ddutile@redhat.com>
> Cc: stable@vger.kernel.org
> Reported-by: Potnuri Bharat Teja <bharat@chelsio.com>
> Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
> Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Tested-by: Potnuri Bharat Teja <bharat@chelsio.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>
> ---
>  drivers/infiniband/core/security.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Please drop this patch, I mistakenly cut it.
It doesn't contain the whole fix now, I'll respin.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]