On Fri, 2015-03-27 at 12:47 -0400, ira.weiny wrote:
> On Fri, Mar 27, 2015 at 04:46:11PM +0100, Michael Wang wrote:
> >
> > Introduce helper has_sa() and cap_sa() to help us check if an IB device
> > or it's port support Subnet Administrator.
>
> I think these 2 should be combined. The question is if a port requires the use
> of the SA depending on the network it is connected to.
>
> Aren't some dual port Mellanox cards capable of doing IB on 1 port and Eth on
> the other?
Yes.
> Do they show up as 2 devices?
No. They are two ports of the same device with different link layers.
See here:
hca_id: mlx4_1
transport: InfiniBand (0)
fw_ver: 2.31.5050
node_guid: f452:1403:007b:cba0
sys_image_guid: f452:1403:007b:cba3
vendor_id: 0x02c9
vendor_part_id: 4099
hw_ver: 0x0
board_id: MT_1090120019
phys_port_cnt: 2
port: 1
state: PORT_ACTIVE (4)
max_mtu: 2048 (4)
active_mtu: 2048 (4)
sm_lid: 2
port_lid: 2
port_lmc: 0x01
link_layer: InfiniBand
port: 2
state: PORT_ACTIVE (4)
max_mtu: 4096 (5)
active_mtu: 4096 (5)
sm_lid: 0
port_lid: 0
port_lmc: 0x00
link_layer: Ethernet
> Regardless I think we should define the SA access on a per port basis.
Yes.
> >
> > Cc: Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Doug Ledford <dledford@xxxxxxxxxx>
> > Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
> > Cc: Sean Hefty <sean.hefty@xxxxxxxxx>
> > Signed-off-by: Michael Wang <yun.wang@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/infiniband/core/sa_query.c | 12 ++++++------
> > include/rdma/ib_verbs.h | 28 ++++++++++++++++++++++++++++
> > 2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
> > index d95d25f..89c27da 100644
> > --- a/drivers/infiniband/core/sa_query.c
> > +++ b/drivers/infiniband/core/sa_query.c
> > @@ -450,7 +450,7 @@ static void ib_sa_event(struct ib_event_handler *handler, struct ib_event *event
> > struct ib_sa_port *port =
> > &sa_dev->port[event->element.port_num - sa_dev->start_port];
> >
> > - if (!rdma_port_ll_is_ib(handler->device, port->port_num))
> > + if (!cap_sa(handler->device, port->port_num))
> > return;
> >
> > spin_lock_irqsave(&port->ah_lock, flags);
> > @@ -1154,7 +1154,7 @@ static void ib_sa_add_one(struct ib_device *device)
> > struct ib_sa_device *sa_dev;
> > int s, e, i;
> >
> > - if (!rdma_transport_is_ib(device))
> > + if (!has_sa(device))
>
> The logic here should be:
>
> if (no ports of this device need sa access)
> return;
>
> So why not eliminate this check and allow the cap_sa(s) to handle the support?
>
> -- Ira
>
> > return;
> >
> > if (device->node_type == RDMA_NODE_IB_SWITCH)
> > @@ -1175,7 +1175,7 @@ static void ib_sa_add_one(struct ib_device *device)
> >
> > for (i = 0; i <= e - s; ++i) {
> > spin_lock_init(&sa_dev->port[i].ah_lock);
> > - if (!rdma_port_ll_is_ib(device, i + 1))
> > + if (!cap_sa(device, i + 1))
> > continue;
> >
> > sa_dev->port[i].sm_ah = NULL;
> > @@ -1205,14 +1205,14 @@ static void ib_sa_add_one(struct ib_device *device)
> > goto err;
> >
> > for (i = 0; i <= e - s; ++i)
> > - if (rdma_port_ll_is_ib(device, i + 1))
> > + if (cap_sa(device, i + 1))
> > update_sm_ah(&sa_dev->port[i].update_task);
> >
> > return;
> >
> > err:
> > while (--i >= 0)
> > - if (rdma_port_ll_is_ib(device, i + 1))
> > + if (cap_sa(device, i + 1))
> > ib_unregister_mad_agent(sa_dev->port[i].agent);
> >
> > kfree(sa_dev);
> > @@ -1233,7 +1233,7 @@ static void ib_sa_remove_one(struct ib_device *device)
> > flush_workqueue(ib_wq);
> >
> > for (i = 0; i <= sa_dev->end_port - sa_dev->start_port; ++i) {
> > - if (rdma_port_ll_is_ib(device, i + 1)) {
> > + if (cap_sa(device, i + 1)) {
> > ib_unregister_mad_agent(sa_dev->port[i].agent);
> > if (sa_dev->port[i].sm_ah)
> > kref_put(&sa_dev->port[i].sm_ah->ref, free_sm_ah);
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index c0a63f8..fa8ffa3 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -1810,6 +1810,19 @@ static inline int has_cm(struct ib_device *device)
> > }
> >
> > /**
> > + * has_sa - Check if a device support Subnet Administrator.
> > + *
> > + * @device: Device to be checked
> > + *
> > + * Return 0 when a device has none port to support
> > + * Subnet Administrator.
> > + */
> > +static inline int has_sa(struct ib_device *device)
> > +{
> > + return rdma_transport_is_ib(device);
> > +}
> > +
> > +/**
> > * cap_smi - Check if the port of device has the capability
> > * Subnet Management Interface.
> > *
> > @@ -1824,6 +1837,21 @@ static inline int cap_smi(struct ib_device *device, u8 port_num)
> > return rdma_port_ll_is_ib(device, port_num);
> > }
> >
> > +/**
> > + * cap_sa - Check if the port of device has the capability
> > + * Subnet Administrator.
> > + *
> > + * @device: Device to be checked
> > + * @port_num: Port number of the device
> > + *
> > + * Return 0 when port of the device don't support
> > + * Subnet Administrator.
> > + */
> > +static inline int cap_sa(struct ib_device *device, u8 port_num)
> > +{
> > + return rdma_port_ll_is_ib(device, port_num);
> > +}
> > +
> > int ib_query_gid(struct ib_device *device,
> > u8 port_num, int index, union ib_gid *gid);
> >
> > --
> > 2.1.0
> >
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: 0E572FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
