Re: regression: nvme rdma with bnxt_re0 broken

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Parav
The nvme connect still failed[1], I've paste all the dmesg log to[2], pls check it.


[1]
[root@rdma-perf-07 ~]$ nvme connect -t rdma -a 172.31.40.125 -s 4420 -n testnqn
Failed to write to /dev/nvme-fabrics: Connection reset by peer
[2]
https://pastebin.com/ipvak0Sp

Thanks

Yi


On 7/12/19 5:49 PM, Parav Pandit wrote:
Hi Selvin,

-----Original Message-----
From: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx>
Sent: Friday, July 12, 2019 9:16 AM
To: Parav Pandit <parav@xxxxxxxxxxxx>
Cc: Yi Zhang <yi.zhang@xxxxxxxxxx>; linux-nvme@xxxxxxxxxxxxxxxxxxx;
Daniel Jurgens <danielj@xxxxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx;
Devesh Sharma <devesh.sharma@xxxxxxxxxxxx>
Subject: Re: regression: nvme rdma with bnxt_re0 broken

On Fri, Jul 12, 2019 at 8:19 AM Parav Pandit <parav@xxxxxxxxxxxx>
wrote:
GID table looks fine.

The GID table has  fe80:0000:0000:0000:020a:f7ff:fee3:6e32 entry
repeated 6 times. 2 for each interface bnxt_roce, bnxt_roce.43 and
bnxt_roce.45. Is this expected to have same gid entries for vlan and
base interfaces? As you mentioned earlier, driver's assumption that
only 2 GID entries identical (one for RoCE v1 and one for RoCE
v2)   is breaking here.

Yes, this is correct behavior. Each vlan netdev interface is in
different L2 segment.
Vlan netdev has this ipv6 link local address. Hence, it is added to the GID
table.
A given GID table entry is identified uniquely by GID+ndev+type(v1/v2).

Reviewing bnxt_qplib_add_sgid() does the comparison below.
if (!memcmp(&sgid_tbl->tbl[i], gid, sizeof(*gid))) {

This comparison looks incomplete which ignore netdev and type.
But even with that, I would expect GID entry addition failure for
vlans for ipv6 link local entries.

But I am puzzled now, that , with above memcmp() check, how does both
v1 and v2 entries get added in your table and for vlans too?
I expect add_gid() and core/cache.c add_roce_gid () to fail for the
duplicate entry.
But GID table that Yi Zhang dumped has these vlan entries.
I am missing something.

Ah, found it.
bnxt_re_add_gid() checks for -EALREADY and returns 0 to add_gid() callback.
Ok. so you just need to extend bnxt_qplib_add_sgid() for considering vlan too.
Let me see if I can share a patch in few minutes.

Yi Zhang,
Instead of last 15 lines of dmesg, can you please share the whole dmsg
log which should be enabled before creating vlans.
using
echo -n "module ib_core +p" /sys/kernel/debug/dynamic_debug/control

Selvin,
Additionally, driver shouldn't be looking at the duplicate entries.
core already does it.
You might only want to do for v1/v2 case as bnxt driver has some
dependency with it.
Can you please fix this part?

How about below fix?

 From f3f17008d34b5a0c38c190010281a3030a8e5771 Mon Sep 17 00:00:00 2001
From: Parav Pandit <parav@xxxxxxxxxxxx>
Date: Fri, 12 Jul 2019 04:34:52 -0500
Subject: [PATCH] IB/bnxt_re: Honor vlan_id in GID entry comparision

GID entry consist of GID, vlan, netdev and smac.
Extend GID duplicate check comparions to consider vlan_id as well
to support IPv6 VLAN based link local addresses.

Fixes: 823b23da7113 ("IB/core: Allow vlan link local address based RoCE GIDs")
Change-Id: I2e026ec8065c8425ba24fad8525323d112a2f4e4
Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
---
  drivers/infiniband/hw/bnxt_re/qplib_sp.c | 4 +++-
  drivers/infiniband/hw/bnxt_re/qplib_sp.h | 1 +
  2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
index 48793d3512ac..8567b7367669 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
@@ -296,7 +296,8 @@ int bnxt_qplib_add_sgid(struct bnxt_qplib_sgid_tbl *sgid_tbl,
  	}
  	free_idx = sgid_tbl->max;
  	for (i = 0; i < sgid_tbl->max; i++) {
-		if (!memcmp(&sgid_tbl->tbl[i], gid, sizeof(*gid))) {
+		if (!memcmp(&sgid_tbl->tbl[i], gid, sizeof(*gid)) &&
+		    sgid_tbl->tbl[i].vlan_id == vlan_id) {
  			dev_dbg(&res->pdev->dev,
  				"SGID entry already exist in entry %d!\n", i);
  			*index = i;
@@ -351,6 +352,7 @@ int bnxt_qplib_add_sgid(struct bnxt_qplib_sgid_tbl *sgid_tbl,
  	}
  	/* Add GID to the sgid_tbl */
  	memcpy(&sgid_tbl->tbl[free_idx], gid, sizeof(*gid));
+	sgid_tbl->tbl[free_idx].vlan_id = vlan_id;
  	sgid_tbl->active++;
  	if (vlan_id != 0xFFFF)
  		sgid_tbl->vlan[free_idx] = 1;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.h b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
index 0ec3b12b0bcd..7a1957c9dc6f 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
@@ -82,6 +82,7 @@ struct bnxt_qplib_pd {
struct bnxt_qplib_gid {
  	u8				data[16];
+	u16 vlan_id;
  };
struct bnxt_qplib_ah {



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux