On Thu, May 27, 2010 at 10:32 AM, Mike Heinz <[email protected]> wrote: > There are QLogic customers who have requested the ability to perform RMPP > transaction handling in user space. This was an option in our old > proprietary stack and there are a few customers still using it which need a > way to forward migrate to OFED while containing the scope of their > application changes. While we have developed appropriate "shim" libraries to > allow their applications to migrate, we can't simulate/shim rmpp processing > without some kernel support. > > We also have some management applications which also need these capabilities. > For those applications, the use of application RMPP control allows the > application to perform some pacing of the RMPP transactions, permits some > parts of the RMPP response to be built on the fly and also permits a degree > of sharing of the response data between multiple requestors. > > The use of OFED kernel based RMPP transaction handling was unable to meet all > these requirements. So this simple option provides a minimal change to OFED > while permitting the application to have the control it needs.
The API would need to be extended for this mode (in some backward compatible manner) rather than supplanting the rmpp_version 0 behavior. Changing that behavior breaks backward compatibility/some existing apps. > > -----Original Message----- > From: Hal Rosenstock [mailto:[email protected]] > Sent: Thursday, May 27, 2010 9:07 AM > To: Mike Heinz > Cc: [email protected]; Roland Dreier > Subject: Re: [PATCH] IB/core User RMPP patch > > On Wed, May 26, 2010 at 4:14 PM, Mike Heinz <[email protected]> wrote: >> Currently, if a user application calls umad_register() or >> umad_register_oui() with an rmpp_version of zero, incoming rmpp messages are >> discarded. > > Actually, it's the underlying ib_register_mad_agent call and this drop > behavior is intentional. > >> This patch changes this behavior so that rmpp_version of zero causes >> incoming rmpp packets to be passed through without alteration, instead. > > What's the motivation to make such a behavioral change ? > > -- Hal > >> >> Signed-off-by: Michael Heinz <[email protected]> >> ---- >> >> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c >> index 80282f1..b3457ae 100644 >> --- a/drivers/infiniband/core/mad.c >> +++ b/drivers/infiniband/core/mad.c >> @@ -1830,25 +1830,39 @@ static void ib_mad_complete_recv(struct >> ib_mad_agent_private *mad_agent_priv, >> if (ib_response_mad(mad_recv_wc->recv_buf.mad)) { >> spin_lock_irqsave(&mad_agent_priv->lock, flags); >> mad_send_wr = ib_find_send_mad(mad_agent_priv, mad_recv_wc); >> - if (!mad_send_wr) { >> + if (mad_send_wr) { >> + ib_mark_mad_done(mad_send_wr); >> spin_unlock_irqrestore(&mad_agent_priv->lock, flags); >> - ib_free_recv_mad(mad_recv_wc); >> - deref_mad_agent(mad_agent_priv); >> - return; >> - } >> - ib_mark_mad_done(mad_send_wr); >> - spin_unlock_irqrestore(&mad_agent_priv->lock, flags); >> >> - /* Defined behavior is to complete response before request */ >> - mad_recv_wc->wc->wr_id = (unsigned long) >> &mad_send_wr->send_buf; >> - mad_agent_priv->agent.recv_handler(&mad_agent_priv->agent, >> - mad_recv_wc); >> - atomic_dec(&mad_agent_priv->refcount); >> + /* Defined behavior is to complete response before >> request */ >> + mad_recv_wc->wc->wr_id = (unsigned long) >> &mad_send_wr->send_buf; >> + >> mad_agent_priv->agent.recv_handler(&mad_agent_priv->agent, >> + mad_recv_wc); >> + atomic_dec(&mad_agent_priv->refcount); >> >> - mad_send_wc.status = IB_WC_SUCCESS; >> - mad_send_wc.vendor_err = 0; >> - mad_send_wc.send_buf = &mad_send_wr->send_buf; >> - ib_mad_complete_send_wr(mad_send_wr, &mad_send_wc); >> + mad_send_wc.status = IB_WC_SUCCESS; >> + mad_send_wc.vendor_err = 0; >> + mad_send_wc.send_buf = &mad_send_wr->send_buf; >> + ib_mad_complete_send_wr(mad_send_wr, &mad_send_wc); >> + } else { >> + if ( !mad_agent_priv->agent.rmpp_version >> + && >> ib_is_mad_class_rmpp(mad_recv_wc->recv_buf.mad->mad_hdr.mgmt_class) >> + && (ib_get_rmpp_flags(&((struct ib_rmpp_mad >> *)mad_recv_wc->recv_buf.mad)->rmpp_hdr) & IB_MGMT_RMPP_FLAG_ACTIVE)) { >> + // user rmpp is in effect >> + >> spin_unlock_irqrestore(&mad_agent_priv->lock, flags); >> + >> + mad_recv_wc->wc->wr_id = 0; >> + >> mad_agent_priv->agent.recv_handler(&mad_agent_priv->agent, >> + >> mad_recv_wc); >> + atomic_dec(&mad_agent_priv->refcount); >> + } else { >> + // not user rmpp, revert to normal behavior >> and drop the mad >> + >> spin_unlock_irqrestore(&mad_agent_priv->lock, flags); >> + ib_free_recv_mad(mad_recv_wc); >> + deref_mad_agent(mad_agent_priv); >> + return; >> + } >> + } >> } else { >> mad_agent_priv->agent.recv_handler(&mad_agent_priv->agent, >> mad_recv_wc); >> diff --git a/drivers/infiniband/core/user_mad.c >> b/drivers/infiniband/core/user_mad.c >> index 36f815c..f9cacbc 100644 >> --- a/drivers/infiniband/core/user_mad.c >> +++ b/drivers/infiniband/core/user_mad.c >> @@ -508,7 +508,7 @@ static ssize_t ib_umad_write(struct file *filp, const >> char __user *buf, >> >> rmpp_mad = (struct ib_rmpp_mad *) packet->mad.data; >> hdr_len = ib_get_mad_data_offset(rmpp_mad->mad_hdr.mgmt_class); >> - if (!ib_is_mad_class_rmpp(rmpp_mad->mad_hdr.mgmt_class)) { >> + if (!agent->rmpp_version || >> !ib_is_mad_class_rmpp(rmpp_mad->mad_hdr.mgmt_class)) { >> copy_offset = IB_MGMT_MAD_HDR; >> rmpp_active = 0; >> } else { >> @@ -560,14 +560,22 @@ static ssize_t ib_umad_write(struct file *filp, const >> char __user *buf, >> rmpp_mad->mad_hdr.tid = *tid; >> } >> >> - spin_lock_irq(&file->send_lock); >> - ret = is_duplicate(file, packet); >> - if (!ret) >> + if ( !agent->rmpp_version >> + && ib_is_mad_class_rmpp(rmpp_mad->mad_hdr.mgmt_class) >> + && (ib_get_rmpp_flags(&rmpp_mad->rmpp_hdr) & >> IB_MGMT_RMPP_FLAG_ACTIVE)) { >> + spin_lock_irq(&file->send_lock); >> list_add_tail(&packet->list, &file->send_list); >> - spin_unlock_irq(&file->send_lock); >> - if (ret) { >> - ret = -EINVAL; >> - goto err_msg; >> + spin_unlock_irq(&file->send_lock); >> + } else { >> + spin_lock_irq(&file->send_lock); >> + ret = is_duplicate(file, packet); >> + if (!ret) >> + list_add_tail(&packet->list, &file->send_list); >> + spin_unlock_irq(&file->send_lock); >> + if (ret) { >> + ret = -EINVAL; >> + goto err_msg; >> + } >> } >> >> ret = ib_post_send_mad(packet->msg, NULL); >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in >> the body of a message to [email protected] >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
