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