From: Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
To: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
Cc: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
"linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH 02/06] opensm/perfmgr: clean up: break out redirect processing from pc_recv_process
Date: Tue, 26 Feb 2013 10:34:56 -0500 [thread overview]
Message-ID: <512CD620.80009@dev.mellanox.co.il> (raw)
In-Reply-To: <20130221133341.0f4083019b5b62c4ca5f24a8-i2BcT+NCU+M@public.gmane.org>
On 2/21/2013 4:33 PM, Ira Weiny wrote:
>
>
> Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
> ---
> opensm/osm_perfmgr.c | 200 ++++++++++++++++++++++++--------------------------
> 1 files changed, 96 insertions(+), 104 deletions(-)
>
> diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
> index c71111f..69b3d77 100644
> --- a/opensm/osm_perfmgr.c
> +++ b/opensm/osm_perfmgr.c
> @@ -1263,6 +1263,96 @@ Exit:
> return pkey_ix;
> }
>
> +static void handle_redirect(osm_perfmgr_t *pm,
> + ib_class_port_info_t *cpi,
> + monitored_node_t *p_mon_node,
> + uint8_t port,
> + osm_madw_context_t *mad_context)
> +{
> + char gid_str[INET6_ADDRSTRLEN];
> + ib_api_status_t status;
> + boolean_t valid = TRUE;
> + int16_t pkey_ix = 0;
> +
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Redirection to LID %u GID %s QP 0x%x received\n",
> + cl_ntoh16(cpi->redir_lid),
> + inet_ntop(AF_INET6, cpi->redir_gid.raw, gid_str,
> + sizeof gid_str), cl_ntoh32(cpi->redir_qp));
> +
> + if (!pm->subn->opt.perfmgr_redir) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Redirection requested but disabled\n");
> + valid = FALSE;
> + }
> +
> + /* valid redirection ? */
> + if (cpi->redir_lid == 0) {
> + if (!ib_gid_is_notzero(&cpi->redir_gid)) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Invalid redirection "
> + "(both redirect LID and GID are zero)\n");
> + valid = FALSE;
> + }
> + }
> + if (cpi->redir_qp == 0) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQP\n");
> + valid = FALSE;
> + }
> + if (cpi->redir_pkey == 0) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectP_Key\n");
> + valid = FALSE;
> + }
> + if (cpi->redir_qkey != IB_QP1_WELL_KNOWN_Q_KEY) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQ_Key\n");
> + valid = FALSE;
> + }
> +
> + pkey_ix = validate_redir_pkey(pm, cpi->redir_pkey);
> + if (pkey_ix == -1) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Index for Pkey 0x%x not found\n",
> + cl_ntoh16(cpi->redir_pkey));
> + valid = FALSE;
> + }
> +
> + if (cpi->redir_lid == 0) {
> + /* GID redirection: get PathRecord information */
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "GID redirection not currently supported\n");
> + return;
> + }
> +
Better with:
if (!valid)
return;
here ?
> + /* LID redirection support (easier than GID redirection) */
> + cl_plock_acquire(&pm->osm->lock);
Port number is already validated before handle_redirect is called, right ?
> + p_mon_node->port[port].redirection = TRUE;
> + p_mon_node->port[port].valid = valid;
> + memcpy(&p_mon_node->port[port].gid, &cpi->redir_gid,
> + sizeof(ib_gid_t));
> + p_mon_node->port[port].lid = cpi->redir_lid;
> + p_mon_node->port[port].qp = cpi->redir_qp;
> + p_mon_node->port[port].pkey = cpi->redir_pkey;
> + if (pkey_ix != -1)
> + p_mon_node->port[port].pkey_ix = pkey_ix;
Add:
p_mon_node->port[port].cpi_valid = FALSE;
here
> + cl_plock_release(&pm->osm->lock);
> +
> + if (valid) {
If check for valid above LID redirection comment, don't need if (valid)
clause here.
> + /* Finally, issue a CPI query to the redirected location */
> + cl_plock_acquire(&pm->osm->lock);
> + p_mon_node->port[port].cpi_valid = FALSE;
> + cl_plock_release(&pm->osm->lock);
Eliminate extra lock/unlock and move cpi_valid above where indicated.
-- Hal
> + status = perfmgr_send_cpi_mad(pm, cpi->redir_lid,
> + cpi->redir_qp, pkey_ix,
> + port, mad_context,
> + 0); /* FIXME SL != 0 */
> + if (status != IB_SUCCESS)
> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5414: "
> + "Failed to send redirected CPI MAD "
> + "for node %s (0x%" PRIx64 ") port %d\n",
> + p_mon_node->name, p_mon_node->guid, port);
> + }
> +}
> +
> /**********************************************************************
> * The dispatcher uses a thread pool which will call this function when
> * there is a thread available to process the mad received on the wire.
> @@ -1281,8 +1371,6 @@ static void pc_recv_process(void *context, void *data)
> perfmgr_db_data_cnt_reading_t data_reading;
> cl_map_item_t *p_node;
> monitored_node_t *p_mon_node;
> - int16_t pkey_ix = 0;
> - boolean_t valid = TRUE;
> ib_class_port_info_t *cpi = NULL;
>
> OSM_LOG_ENTER(pm->log);
> @@ -1334,112 +1422,15 @@ static void pc_recv_process(void *context, void *data)
> p_mon_node->port[port].cpi_valid = TRUE;
> }
> cl_plock_release(&pm->osm->lock);
> - }
> -
> - /* Response could also be redirection (IBM eHCA PMA does this) */
> - if (p_mad->status & IB_MAD_STATUS_REDIRECT) {
> - char gid_str[INET6_ADDRSTRLEN];
> - ib_api_status_t status;
>
> - CL_ASSERT(cpi); /* Redirect should have returned CPI
> - (processed in previous block) */
> -
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Redirection to LID %u GID %s QP 0x%x received\n",
> - cl_ntoh16(cpi->redir_lid),
> - inet_ntop(AF_INET6, cpi->redir_gid.raw, gid_str,
> - sizeof gid_str), cl_ntoh32(cpi->redir_qp));
> + /* Response could also be redirection (IBM eHCA PMA does this) */
> + if (p_mad->status & IB_MAD_STATUS_REDIRECT)
> + handle_redirect(pm, cpi, p_mon_node, port,
> + mad_context);
>
> - if (!pm->subn->opt.perfmgr_redir) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Redirection requested but disabled\n");
> - valid = FALSE;
> - }
> -
> - /* valid redirection ? */
> - if (cpi->redir_lid == 0) {
> - if (!ib_gid_is_notzero(&cpi->redir_gid)) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Invalid redirection "
> - "(both redirect LID and GID are zero)\n");
> - valid = FALSE;
> - }
> - }
> - if (cpi->redir_qp == 0) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQP\n");
> - valid = FALSE;
> - }
> - if (cpi->redir_pkey == 0) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectP_Key\n");
> - valid = FALSE;
> - }
> - if (cpi->redir_qkey != IB_QP1_WELL_KNOWN_Q_KEY) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQ_Key\n");
> - valid = FALSE;
> - }
> -
> - pkey_ix = validate_redir_pkey(pm, cpi->redir_pkey);
> - if (pkey_ix == -1) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Index for Pkey 0x%x not found\n",
> - cl_ntoh16(cpi->redir_pkey));
> - valid = FALSE;
> - }
> -
> - if (cpi->redir_lid == 0) {
> - /* GID redirection: get PathRecord information */
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "GID redirection not currently supported\n");
> - goto Exit;
> - }
> -
> - /* LID redirection support (easier than GID redirection) */
> - cl_plock_acquire(&pm->osm->lock);
> - /* Now, validate port number */
> - if (port >= p_mon_node->num_ports) {
> - cl_plock_release(&pm->osm->lock);
> - OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5413: "
> - "Invalid port num %d for GUID 0x%016"
> - PRIx64 " num ports %d\n", port, node_guid,
> - p_mon_node->num_ports);
> - goto Exit;
> - }
> - p_mon_node->port[port].redirection = TRUE;
> - p_mon_node->port[port].valid = valid;
> - memcpy(&p_mon_node->port[port].gid, &cpi->redir_gid,
> - sizeof(ib_gid_t));
> - p_mon_node->port[port].lid = cpi->redir_lid;
> - p_mon_node->port[port].qp = cpi->redir_qp;
> - p_mon_node->port[port].pkey = cpi->redir_pkey;
> - if (pkey_ix != -1)
> - p_mon_node->port[port].pkey_ix = pkey_ix;
> - cl_plock_release(&pm->osm->lock);
> -
> - if (!valid)
> - goto Exit;
> -
> - /* Finally, issue a CPI query to the redirected location */
> - p_mon_node->port[port].cpi_valid = FALSE;
> - status = perfmgr_send_cpi_mad(pm, cpi->redir_lid,
> - cpi->redir_qp, pkey_ix,
> - port, mad_context,
> - 0); /* FIXME SL != 0 */
> - if (status != IB_SUCCESS)
> - OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5414: "
> - "Failed to send redirected MAD "
> - "with method 0x%x for node %s "
> - "(NodeGuid 0x%" PRIx64 ") port %d\n",
> - mad_context->perfmgr_context.mad_method,
> - p_mon_node->name, node_guid, port);
> goto Exit;
> }
>
> - /* ClassPortInfo needed to process optional Redirection
> - * now exit normally
> - */
> - if (p_mad->attr_id == IB_MAD_ATTR_CLASS_PORT_INFO)
> - goto Exit;
> -
> perfmgr_db_fill_err_read(wire_read, &err_reading);
> /* FIXME separate query for extended counters if they are supported
> * on the port.
> @@ -1465,7 +1456,8 @@ static void pc_recv_process(void *context, void *data)
> perfmgr_db_clear_prev_dc(pm->db, node_guid, port);
> }
>
> - perfmgr_check_overflow(pm, p_mon_node, pkey_ix, port, wire_read);
> + perfmgr_check_overflow(pm, p_mon_node, p_mon_node->port[port].pkey_ix,
> + port, wire_read);
>
> #ifdef ENABLE_OSM_PERF_MGR_PROFILE
> do {
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-02-26 15:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-21 21:33 [PATCH 02/06] opensm/perfmgr: clean up: break out redirect processing from pc_recv_process Ira Weiny
[not found] ` <20130221133341.0f4083019b5b62c4ca5f24a8-i2BcT+NCU+M@public.gmane.org>
2013-02-26 15:34 ` Hal Rosenstock [this message]
[not found] ` <512CD620.80009-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-02-26 19:34 ` Ira Weiny
[not found] ` <20130226113455.e1a10ece200032d8cdf703aa-i2BcT+NCU+M@public.gmane.org>
2013-02-26 19:58 ` Hal Rosenstock
[not found] ` <512D13C9.4070309-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-02-26 21:28 ` Ira Weiny
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=512CD620.80009@dev.mellanox.co.il \
--to=hal-ldsdmyg8hgv8yrgs2mwiifqbs+8scbdb@public.gmane.org \
--cc=hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
--cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=weiny2-i2BcT+NCU+M@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.