[RFC] mlx5: add parameter to disable enhanced IPoIB

[RFC] mlx5: add parameter to disable enhanced IPoIB

[Nicolas Morey-Chaisemartin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Recent ConnextX-[45] HCA have enhanced IPoIB enabled which prevents the use of the connected mode.
Although not an issue in a fully compatible setup, it can be an issue in a mixed HW one.

Mellanox OFED uses a ipoib_enhanced flag on the ib_ipoib module to work around the issue.
This patch adds a similarly name flag to the mlx5_ib module to disable enhanced IPoIB for
all mlx5 HCA and allow users to pick datagram/connected the usual way.

Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com>
- ---
 drivers/infiniband/hw/mlx5/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index c2a5780cb394..779a35883494 100644
- --- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -78,6 +78,10 @@ MODULE_AUTHOR("Eli Cohen <eli@mellanox.com>");
 MODULE_DESCRIPTION("Mellanox Connect-IB HCA IB driver");
 MODULE_LICENSE("Dual BSD/GPL");
 
+static int ipoib_enhanced = 1;
+module_param(ipoib_enhanced, int, 0444);
+MODULE_PARM_DESC(ipoib_enhanced, "Enable IPoIB enhanced for capable devices (default = 1) (0-1)");
+
 static char mlx5_version[] =
 	DRIVER_NAME ": Mellanox Connect-IB Infiniband driver v"
 	DRIVER_VERSION "\n";
@@ -6383,6 +6387,7 @@ static int mlx5_ib_stage_caps_init(struct mlx5_ib_dev *dev)
 		(1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW);
 
 	if (MLX5_CAP_GEN(mdev, ipoib_enhanced_offloads) &&
+	    ipoib_enhanced &&
 	    IS_ENABLED(CONFIG_MLX5_CORE_IPOIB))
 		ib_set_device_ops(&dev->ib_dev,
 				  &mlx5_ib_dev_ipoib_enhanced_ops);
- -- 
2.21.0

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEQtJThcGhwCuLGxxvgBvduCWYj2QFAl0/QXMACgkQgBvduCWY
j2TqCQf/YjNVtRd/EbsZUt5bnwIevHPaETwJ7Xa88cjG8kW7THZCokom1fh5UWmV
4ZBGep2b4UEfeJ0u3NMI1ux3G9qFnhhE/hmwMNlsLalc2vQzqX0OIcvcYrpYwEys
ipo78KQjQHFSZAEc+AGq/PfvKp//zi/BWLRAivncF1nM4u3kQ0bO4eGK7NaNiLOK
D43hrAgsLLu+tYxFMX9rLi/RcZkHiMCJwMvp6i9PTKghSow7FbzyK2p8KSzBWzGa
urtYs66ckvyfHSoUKcoMlGfeTBbUYC8yziKqXobRbI7n4CttCxOlrNqLCFF1kzAl
PsnK1EDFTaKHamblNrdQzZTqLiywzQ==
=FENl
-----END PGP SIGNATURE-----

Re: [RFC] mlx5: add parameter to disable enhanced IPoIB

[Doug Ledford]

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

On Mon, 2019-07-29 at 18:57 +0000, Nicolas Morey-Chaisemartin wrote:
> Recent ConnextX-[45] HCA have enhanced IPoIB enabled which prevents
> the use of the connected mode.
> Although not an issue in a fully compatible setup, it can be an issue
> in a mixed HW one.
> 
> Mellanox OFED uses a ipoib_enhanced flag on the ib_ipoib module to
> work around the issue.
> This patch adds a similarly name flag to the mlx5_ib module to disable
> enhanced IPoIB for
> all mlx5 HCA and allow users to pick datagram/connected the usual way.
> 
> Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com
> >
> ---
>  drivers/infiniband/hw/mlx5/main.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/mlx5/main.c
> b/drivers/infiniband/hw/mlx5/main.c
> index c2a5780cb394..779a35883494 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -78,6 +78,10 @@ MODULE_AUTHOR("Eli Cohen <eli@mellanox.com>");
>  MODULE_DESCRIPTION("Mellanox Connect-IB HCA IB driver");
>  MODULE_LICENSE("Dual BSD/GPL");
>  
> +static int ipoib_enhanced = 1;
> +module_param(ipoib_enhanced, int, 0444);
> +MODULE_PARM_DESC(ipoib_enhanced, "Enable IPoIB enhanced for capable
> devices (default = 1) (0-1)");
> +
>  static char mlx5_version[] =
>  	DRIVER_NAME ": Mellanox Connect-IB Infiniband driver v"
>  	DRIVER_VERSION "\n";
> @@ -6383,6 +6387,7 @@ static int mlx5_ib_stage_caps_init(struct
> mlx5_ib_dev *dev)
>  		(1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW);
>  
>  	if (MLX5_CAP_GEN(mdev, ipoib_enhanced_offloads) &&
> +	    ipoib_enhanced &&
>  	    IS_ENABLED(CONFIG_MLX5_CORE_IPOIB))
>  		ib_set_device_ops(&dev->ib_dev,
>  				  &mlx5_ib_dev_ipoib_enhanced_ops);

Module parameters are highly frowned upon in general, and in this
particular instance, I could easily see where if you had a dual port IB
card, with one port plugged into a fully compatible setup, and the other
port plugged into a more heterogeneous setup (say a Lustre backend or
something), that you really want this to be on a per-port basis.  So,
I'm gonna say this is a nak unless Mellanox comes back and says doing
this per-port is strictly not possible (and even if it must be per-card
or driver wide, I would still prefer maybe a netlink control to a module
option, if for no other reason than I don't want to hear the groans from
other kernel folk if I take a module option now a days).

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC] mlx5: add parameter to disable enhanced IPoIB

[Nicolas Morey-Chaisemartin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 7/29/19 10:42 PM, Doug Ledford wrote:
> On Mon, 2019-07-29 at 18:57 +0000, Nicolas Morey-Chaisemartin wrote:
>> Recent ConnextX-[45] HCA have enhanced IPoIB enabled which prevents
>> the use of the connected mode.
>> Although not an issue in a fully compatible setup, it can be an issue
>> in a mixed HW one.
>>
>> Mellanox OFED uses a ipoib_enhanced flag on the ib_ipoib module to
>> work around the issue.
>> This patch adds a similarly name flag to the mlx5_ib module to disable
>> enhanced IPoIB for
>> all mlx5 HCA and allow users to pick datagram/connected the usual way.
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com
>>>
>> ---
>>  drivers/infiniband/hw/mlx5/main.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/main.c
>> b/drivers/infiniband/hw/mlx5/main.c
>> index c2a5780cb394..779a35883494 100644
>> --- a/drivers/infiniband/hw/mlx5/main.c
>> +++ b/drivers/infiniband/hw/mlx5/main.c
>> @@ -78,6 +78,10 @@ MODULE_AUTHOR("Eli Cohen <eli@mellanox.com>");
>>  MODULE_DESCRIPTION("Mellanox Connect-IB HCA IB driver");
>>  MODULE_LICENSE("Dual BSD/GPL");
>>  
>> +static int ipoib_enhanced = 1;
>> +module_param(ipoib_enhanced, int, 0444);
>> +MODULE_PARM_DESC(ipoib_enhanced, "Enable IPoIB enhanced for capable
>> devices (default = 1) (0-1)");
>> +
>>  static char mlx5_version[] =
>>  	DRIVER_NAME ": Mellanox Connect-IB Infiniband driver v"
>>  	DRIVER_VERSION "\n";
>> @@ -6383,6 +6387,7 @@ static int mlx5_ib_stage_caps_init(struct
>> mlx5_ib_dev *dev)
>>  		(1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW);
>>  
>>  	if (MLX5_CAP_GEN(mdev, ipoib_enhanced_offloads) &&
>> +	    ipoib_enhanced &&
>>  	    IS_ENABLED(CONFIG_MLX5_CORE_IPOIB))
>>  		ib_set_device_ops(&dev->ib_dev,
>>  				  &mlx5_ib_dev_ipoib_enhanced_ops);
> 
> Module parameters are highly frowned upon in general, and in this
> particular instance, I could easily see where if you had a dual port IB
> card, with one port plugged into a fully compatible setup, and the other
> port plugged into a more heterogeneous setup (say a Lustre backend or
> something), that you really want this to be on a per-port basis.  So,
> I'm gonna say this is a nak unless Mellanox comes back and says doing
> this per-port is strictly not possible (and even if it must be per-card
> or driver wide, I would still prefer maybe a netlink control to a module
> option, if for no other reason than I don't want to hear the groans from
> other kernel folk if I take a module option now a days).
> 

I feared as much but figured it would ne worth a shot.
I'll have a look at how we can deal with this nicely. But it might not be that easy to change that settings later on as the IPoIB interface is usually created very early so we would have to destroy the existing netdev and recreate a new one.
The easiest solution would be that mstconfig allow to set/clear this flag so we can deal with this in the FW and be fully transparent driver wise but don't know if there is hope for that...

Nicolas

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEQtJThcGhwCuLGxxvgBvduCWYj2QFAl0/3dgACgkQgBvduCWY
j2Rt5QgAifN+J4+8pO9UAt6pHzTGjhwxPSJBOErsb1pD0LLiMtPght6dnKqgxuDm
Q6r8NixmKnqCBRc867NeLWhgphml0v1PR59YZ2fMKpK0wNwCRwp3aVEPCw97mh06
84OjIE6UXBpvehykd55rsKP2cq1dnUmcJejFJThr51INTTbS28LTdDauKrkcl/gc
M5ej04O76L3xNYUubAuLvj3ZSchegMabGzq7wEeAptK0AfbjvYUCvi42WLQuNcZl
PdPMFLVsxAVsc6gaBGXDzXh4zgpL4x3NzPMYK0fHRue+paElKpirdRAr39mIhp6D
K9+pIQKLMB7OnwPbzxB0l/YJ2hrTSQ==
=+Vk2
-----END PGP SIGNATURE-----