On Mon, Mar 12, 2018 at 08:40:50AM -0600, Parav Pandit wrote:
> > 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
> > > > +++ 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.
Please try not to rely on these long chains of logic to explain this
stuff..
CM is already so complicated, better to explicitly provide the
expected control flow in each function so someone can hope to
understand it.
Jason
--
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
