| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 27 Mar 2015 18:13:27 +0100
From: Yun Wang <yun.wang@...fitbricks.com>
To: "ira.weiny" <ira.weiny@...el.com>
Cc: Roland Dreier <roland@...nel.org>,
Sean Hefty <sean.hefty@...el.com>,
Hal Rosenstock <hal.rosenstock@...il.com>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-nfs@...r.kernel.org, netdev@...r.kernel.org,
"J. Bruce Fields" <bfields@...ldses.org>,
Trond Myklebust <trond.myklebust@...marydata.com>,
"David S. Miller" <davem@...emloft.net>,
Or Gerlitz <ogerlitz@...lanox.com>,
Moni Shoua <monis@...lanox.com>,
PJ Waskiewicz <pj.waskiewicz@...idfire.com>,
Tatyana Nikolova <Tatyana.E.Nikolova@...el.com>,
Yan Burman <yanb@...lanox.com>,
Jack Morgenstein <jackm@....mellanox.co.il>,
Bart Van Assche <bvanassche@....org>,
Yann Droneaud <ydroneaud@...eya.com>,
Colin Ian King <colin.king@...onical.com>,
Majd Dibbiny <majd@...lanox.com>,
Jiri Kosina <jkosina@...e.cz>,
Matan Barak <matanb@...lanox.com>,
Alex Estrin <alex.estrin@...el.com>,
Doug Ledford <dledford@...hat.com>,
Eric Dumazet <edumazet@...gle.com>,
Erez Shitrit <erezsh@...lanox.com>,
Sagi Grimberg <sagig@...lanox.com>,
Haggai Eran <haggaie@...lanox.com>,
Shachar Raindel <raindel@...lanox.com>,
Mike Marciniszyn <mike.marciniszyn@...el.com>,
Steve Wise <swise@...ngridcomputing.com>,
Tom Tucker <tom@....us>, Chuck Lever <chuck.lever@...cle.com>
Subject: Re: [RFC PATCH 06/11] IB/Verbs: Use management helper has_sa() and
cap_sa(), for sa-check
On Fri, Mar 27, 2015 at 5:47 PM, ira.weiny <ira.weiny@...el.com> 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? Do they show up as 2 devices?
I'm not sure about this... but sounds like your opinion is same to
Jason on ipoib stuff, I'll do more investigation on that part, if it's
just for optimization, then we should be able to eliminate has_xx()
stuff :-) after all, init/exit is not a hot path.
>
> Regardless I think we should define the SA access on a per port basis.
>
>>
>> Cc: Jason Gunthorpe <jgunthorpe@...idianresearch.com>
>> Cc: Doug Ledford <dledford@...hat.com>
>> Cc: Ira Weiny <ira.weiny@...el.com>
>> Cc: Sean Hefty <sean.hefty@...el.com>
>> Signed-off-by: Michael Wang <yun.wang@...fitbricks.com>
>> ---
>> 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?
I'll check if we could merge this part to the following iteration :-)
Regards,
Michael Wang
>
> -- 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
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists