> -----Original Message----- > From: Leon Romanovsky [mailto:leon@xxxxxxxxxx] > Sent: Monday, March 12, 2018 9:01 AM > To: Hal Rosenstock <hal@xxxxxxxxxxxxxxxxxx> > Cc: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe > <jgg@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel > Jurgens <danielj@xxxxxxxxxxxx>; Mark Bloch <markb@xxxxxxxxxxxx>; Parav > Pandit <parav@xxxxxxxxxxxx> > Subject: Re: [PATCH rdma-next 04/13] IB/core: Honor port_num while resolving > GID for IB link layer > > On Mon, Mar 12, 2018 at 08:47:24AM -0400, Hal Rosenstock wrote: > > On 3/11/2018 4:03 AM, Leon Romanovsky wrote: > > > From: Parav Pandit <parav@xxxxxxxxxxxx> > > > > > > Currently initialized ah_attr contains the port number to which > > > cm_id is bound. However, while searching for GID table for matching > > > GID entry, port number is ignored. > > > It is better to honor the port number of the cm_id while searching > > > GID table for IB link layer as well. > > > > > > Reviewed-by: Daniel Jurgens <danielj@xxxxxxxxxxxx> > > > Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx> > > > Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx> > > > --- > > > drivers/infiniband/core/multicast.c | 24 ++++++++++-------------- > > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/infiniband/core/multicast.c > > > b/drivers/infiniband/core/multicast.c > > > index 45f2f095f793..29bd3f1aff12 100644 > > > --- a/drivers/infiniband/core/multicast.c > > > +++ b/drivers/infiniband/core/multicast.c > > > @@ -724,21 +724,17 @@ int ib_init_ah_from_mcmember(struct ib_device > > > *device, u8 port_num, { > > > int ret; > > > u16 gid_index; > > > - u8 p; > > > - > > > - if (rdma_protocol_roce(device, port_num)) { > > > - ret = ib_find_cached_gid_by_port(device, &rec->port_gid, > > > - gid_type, port_num, > > > - ndev, > > > - &gid_index); > > > - } else if (rdma_protocol_ib(device, port_num)) { > > > - ret = ib_find_cached_gid(device, &rec->port_gid, > > > - IB_GID_TYPE_IB, NULL, &p, > > > - &gid_index); > > > - } else { > > > - ret = -EINVAL; > > > - } > > > > > > + /* GID table is not based on the netdevice for IB link layer, > > > + * so ignore ndev during search. > > > + */ > > > + if (rdma_protocol_ib(device, port_num)) > > > + ndev = NULL; > > > > To be equivalent to what is being replaced, shouldn't there be: > > else if (!rdma_protocol_roce(device, port_num)) > > return -EINVAL; > > here (for iWarp) ? > > I'm sure that you are right. > ib_init_ah_from_mcmember() is called from multicast handler based on SA path record entry. Multicast registration can only happen if the HCA advertises capability via rdma_cap_ib_mcast(). RDMA_CORE_CAP_IB_SA capability is defined as RDMA_CORE_PORT_IBA_IB. RDMA_CORE_PORT_IWARP doesn't have RDMA_CORE_CAP_IB_SA. So I don't see a way that this code path can be executed for iWarp port until RDMA_CORE_CAP_IB_SA gets added to RDMA_CORE_PORT_IWARP. > Thanks > > > > > -- Hal > > > > > + > > > + ret = ib_find_cached_gid_by_port(device, &rec->port_gid, > > > + gid_type, port_num, > > > + ndev, > > > + &gid_index); > > > if (ret) > > > return ret; > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html