Re: commit ab118da4c10a70b8437f5c90ab77adae1835963e causes ib_srpt to fail connections served by target LIO

Max Gurtovoy <maxg@xxxxxxxxxxxx> · Sun, 15 Mar 2020 20:20:00 +0200

On 3/15/2020 7:59 PM, Laurence Oberman wrote:
On Sun, 2020-03-15 at 18:47 +0200, Max Gurtovoy wrote:
On 3/14/2020 11:30 PM, Laurence Oberman wrote:
Hello Bart, Leon and Max
Hi Laurence,

thanks for the great analysis and the fast response !!

Max had reached out to me to test a new set of patches for SRQ.
I had not tested upstream ib_srpt on an LIO target for quite a
while,
only ib_srp client tests had been run of late.
During a baseline test before applying Max's patches it was
apparent
that something had broken ib_srpt connections within LIO target
since
5.5.

Note thet ib_srp client connectivity with the commit functions
fine,
it's just the target that breaks with this commit.

After a long bisect this is the commit that seems to break it.
While it's not directly code in ib_srpt, its code in mlx5 vport
ethernet connectivity that then breaks ib_srpt connectivity over
mlx5
IB RDMA with LIO.
I was able to connect in loopback and also from remote initiator
with
this commit.

So I'm not sure that this commit is broken.

I used Bart's scripts to configure the target and to connect to it
in
loopback (after some modifications for the updated
kernel/sysfs/configfs
interface).

I did see an issue to connect from remote initiator, but after
reloading
openibd in the initiator side I was able to connect.

So I suspect you had the same issue - that also should be debugged.

I will let Leon and others decide but reverting the below commit
allows
SRP connectivity to an LIO target to work again.
I added prints to "mlx5_core_modify_hca_vport_context" function and
found that we don't call it in "pure" mlx5 mode with PFs.

Maybe you can try it too...

I was able to check my patches on my system and I'll send them soon.

Thanks again Laurence and Bart.

Max, I will test your new patches once we have a decision on this.

Client
Linux ibclient.lab.eng.bos.redhat.com 5.6.0-rc5+ #1 SMP Thu Mar 12
16:58:19 EDT 2020 x86_64 x86_64 x86_64 GNU/Linux

Server with reverted commit
Linux fedstorage.bos.redhat.com 5.6.0-rc5+ #1 SMP Sat Mar 14
16:39:35
EDT 2020 x86_64 x86_64 x86_64 GNU/Linux

commit ab118da4c10a70b8437f5c90ab77adae1835963e
Author: Leon Romanovsky <leonro@xxxxxxxxxxxx>
Date:   Wed Nov 13 12:03:47 2019 +0200

      net/mlx5: Don't write read-only fields in
MODIFY_HCA_VPORT_CONTEXT
command
      
      The MODIFY_HCA_VPORT_CONTEXT uses field_selector to mask
fields
needed
      to be written, other fields are required to be zero according
to
the
      HW specification. The supported fields are controlled by
bitfield
      and limited to vport state, node and port GUIDs.
      
      Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
      Signed-off-by: Saeed Mahameed <saeedm@xxxxxxxxxxxx>

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
b/drivers/net/ethernet/mellanox/mlx5
index 30f7848..1faac31f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
@@ -1064,26 +1064,13 @@ int
mlx5_core_modify_hca_vport_context(struct
mlx5_core_dev *dev,
   
          ctx = MLX5_ADDR_OF(modify_hca_vport_context_in, in,
hca_vport_context);
          MLX5_SET(hca_vport_context, ctx, field_select, req-
field_select);
-       MLX5_SET(hca_vport_context, ctx, sm_virt_aware, req-
sm_virt_aware);
-       MLX5_SET(hca_vport_context, ctx, has_smi, req->has_smi);
-       MLX5_SET(hca_vport_context, ctx, has_raw, req->has_raw);
-       MLX5_SET(hca_vport_context, ctx, vport_state_policy, req-
policy);
-       MLX5_SET(hca_vport_context, ctx, port_physical_state, req-
phys_state);
-       MLX5_SET(hca_vport_context, ctx, vport_state, req-
vport_state);
-       MLX5_SET64(hca_vport_context, ctx, port_guid, req-
port_guid);
-       MLX5_SET64(hca_vport_context, ctx, node_guid, req-
node_guid);
-       MLX5_SET(hca_vport_context, ctx, cap_mask1, req-
cap_mask1);
-       MLX5_SET(hca_vport_context, ctx, cap_mask1_field_select,
req-
cap_mask1_perm);
-       MLX5_SET(hca_vport_context, ctx, cap_mask2, req-
cap_mask2);
-       MLX5_SET(hca_vport_context, ctx, cap_mask2_field_select,
req-
cap_mask2_perm);
-       MLX5_SET(hca_vport_context, ctx, lid, req->lid);
-       MLX5_SET(hca_vport_context, ctx, init_type_reply, req-
init_type_reply);
-       MLX5_SET(hca_vport_context, ctx, lmc, req->lmc);
-       MLX5_SET(hca_vport_context, ctx, subnet_timeout, req-
subnet_timeout);
-       MLX5_SET(hca_vport_context, ctx, sm_lid, req->sm_lid);
-       MLX5_SET(hca_vport_context, ctx, sm_sl, req->sm_sl);
-       MLX5_SET(hca_vport_context, ctx, qkey_violation_counter,
req-
qkey_violation_counter);
-       MLX5_SET(hca_vport_context, ctx, pkey_violation_counter,
req-
pkey_violation_counter);
+       if (req->field_select & MLX5_HCA_VPORT_SEL_STATE_POLICY)
+               MLX5_SET(hca_vport_context, ctx,
vport_state_policy,
+                        req->policy);
+       if (req->field_select & MLX5_HCA_VPORT_SEL_PORT_GUID)
+               MLX5_SET64(hca_vport_context, ctx, port_guid, req-
port_guid);
+       if (req->field_select & MLX5_HCA_VPORT_SEL_NODE_GUID)
+               MLX5_SET64(hca_vport_context, ctx, node_guid, req-
node_guid);
          err = mlx5_cmd_exec(dev, in, in_sz, out, sizeof(out));
   ex:
          kfree(in);
   



Hi Max
Re:

"
So I'm not sure that this commit is broken.
..
..
I added prints to "mlx5_core_modify_hca_vport_context" function and
found that we don't call it in "pure" mlx5 mode with PFs.

Maybe you can try it too...
"

The thing is without this commit we connect immediately, no delay no
issue and I am changing nothing else other than reverting here.

So this clearly has a bearing directly on the functionality.

I will look at adding more debug, but with this commit in there is nor
evidence even of an attempt to connect and fail.

Its silently faling.

please send me all the configuration steps you run after booting the 
target + steps in the initiator (can be also in attached file).

I'll try to follow this.

Btw, did you try loopback initiator ?

-Max.



Regards
Laurence










