On Wed, May 16, 2018 at 12:27:22PM -0600, Jason Gunthorpe wrote:
> On Tue, May 08, 2018 at 05:25:09PM +0300, Leon Romanovsky wrote:
> > +enum {
> > + MLX4_SUPPORTED_IB_SPEC_PORT_CAP_FLAGS =
> > + IB_PORT_SM |
> > + IB_PORT_NOTICE_SUP |
> > + IB_PORT_TRAP_SUP |
> > + IB_PORT_OPT_IPD_SUP |
> > + IB_PORT_AUTO_MIGR_SUP |
> > + IB_PORT_SL_MAP_SUP |
> > + IB_PORT_MKEY_NVRAM |
> > + IB_PORT_PKEY_NVRAM |
> > + IB_PORT_LED_INFO_SUP |
> > + IB_PORT_SM_DISABLED |
> > + IB_PORT_SYS_IMAGE_GUID_SUP |
> > + IB_PORT_PKEY_SW_EXT_PORT_TRAP_SUP |
> > + IB_PORT_EXTENDED_SPEEDS_SUP |
> > + IB_PORT_CM_SUP |
> > + IB_PORT_SNMP_TUNNEL_SUP |
> > + IB_PORT_REINIT_SUP |
> > + IB_PORT_DEVICE_MGMT_SUP |
> > + IB_PORT_VENDOR_CLASS_SUP |
> > + IB_PORT_DR_NOTICE_SUP |
> > + IB_PORT_CAP_MASK_NOTICE_SUP |
> > + IB_PORT_BOOT_MGMT_SUP |
> > + IB_PORT_LINK_LATENCY_SUP |
> > + IB_PORT_CLIENT_REG_SUP
> > +};
> > +
> > +
> > static void init_query_mad(struct ib_smp *mad)
> > {
> > mad->base_version = 1;
> > @@ -694,6 +722,7 @@ static int ib_link_query_port(struct ib_device *ibdev, u8 port,
> > props->subnet_timeout = out_mad->data[51] & 0x1f;
> > props->max_vl_num = out_mad->data[37] >> 4;
> > props->init_type_reply = out_mad->data[41] >> 4;
> > + props->port_cap_flags &= MLX4_SUPPORTED_IB_SPEC_PORT_CAP_FLAGS;
>
> What is this doing?
>
> props->port_cap_flags at this point comes from the FW, why would we
> want to mask it?
>
> > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > index 44cb1cfba417..aa15af435362 100644
> > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > @@ -72,6 +72,33 @@
> > #define PVRDMA_NUM_RING_PAGES 4
> > #define PVRDMA_QP_NUM_HEADER_PAGES 1
> >
> > +enum {
> > + PVRDMA_SUPPORTED_IB_SPEC_PORT_CAP_FLAGS =
> > + IB_PORT_SM |
> > + IB_PORT_NOTICE_SUP |
> > + IB_PORT_TRAP_SUP |
> > + IB_PORT_OPT_IPD_SUP |
> > + IB_PORT_AUTO_MIGR_SUP |
> > + IB_PORT_SL_MAP_SUP |
> > + IB_PORT_MKEY_NVRAM |
> > + IB_PORT_PKEY_NVRAM |
> > + IB_PORT_LED_INFO_SUP |
> > + IB_PORT_SM_DISABLED |
> > + IB_PORT_SYS_IMAGE_GUID_SUP |
> > + IB_PORT_PKEY_SW_EXT_PORT_TRAP_SUP |
> > + IB_PORT_EXTENDED_SPEEDS_SUP |
> > + IB_PORT_CM_SUP |
> > + IB_PORT_SNMP_TUNNEL_SUP |
> > + IB_PORT_REINIT_SUP |
> > + IB_PORT_DEVICE_MGMT_SUP |
> > + IB_PORT_VENDOR_CLASS_SUP |
> > + IB_PORT_DR_NOTICE_SUP |
> > + IB_PORT_CAP_MASK_NOTICE_SUP |
> > + IB_PORT_BOOT_MGMT_SUP |
> > + IB_PORT_LINK_LATENCY_SUP |
> > + IB_PORT_CLIENT_REG_SUP
> > +};
> > +
> > struct pvrdma_dev;
> >
> > struct pvrdma_page_dir {
> > @@ -351,7 +378,7 @@ static inline int ib_port_cap_flags_to_pvrdma(int flags)
> >
> > static inline int pvrdma_port_cap_flags_to_ib(int flags)
> > {
> > - return flags;
> > + return flags & PVRDMA_SUPPORTED_IB_SPEC_PORT_CAP_FLAGS;
> > }
>
> This too.. Not sure what this is about..
>
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index d84e6b6a8a56..2323a5624161 100644
> > +++ b/include/rdma/ib_verbs.h
> > @@ -455,6 +455,7 @@ enum ib_port_cap_flags {
> > IB_PORT_LINK_LATENCY_SUP = 1 << 24,
> > IB_PORT_CLIENT_REG_SUP = 1 << 25,
> > IB_PORT_IP_BASED_GIDS = 1 << 26,
> > + IB_PORT_GRH_REQUIRED = 1 << 27,
> > };
>
> Ahh.. What?
>
> This bitmap was supposed to be controlled by IBTA, it is
> PortInfo:CapabilityMask, and bit 26 and 27 are already defined by
> IBTA!
>
> So we can't just co-op them!!!
>
> Is this what all the crazy masking is about above??
>
> NAKitty nak nak..
>
> And someone needs to fix IB_PORT_IP_BASED_GIDS :( :(
>
> It overlaps with IsOtherLocalChangesNoticeSupported - not sure how to
> fix it. Sadness. :(
As you wrote, this enum is already commingled by partial commit
f931551bafe1 ("IB/qib: Add new qib driver for QLogic PCIe InfiniBand adapters")
with todo line in 2010 and commit b4a26a27287a ("IB: Report using RoCE
IP based gids in port caps") in 2014.
IB_PORT_IP_BASED_GIDS is already exported by libibverbs and supported by
two drivers.
More on that, IBTA declares those bits as needed for SM entities
(C14-24.1.1) and it doesn't mean that we should limit everything to MADs.
And yes, we need to ensure that those caps bits (26 and 27) are set correctly
in MAD layer.
I don't know why separation between MADs and other interfaces wasn't done before.
Thanks
>
> 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
