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

[Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.com>]

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