On Thu, 2016-09-08 at 07:33 -0400, Hal Rosenstock wrote: > On 9/7/2016 7:42 AM, Knut Omang wrote: >> On Tue, 2016-09-06 at 10:01 -0400, Hal Rosenstock wrote: >>> On 9/1/2016 8:09 PM, Knut Omang wrote: >>>> >>>> From: Dag Moxnes <dag.moxnes@xxxxxxxxxx> >>>> >>>> The process_mad function is an optional IB driver entry point >>>> allows a driver to intercept or modify MAD traffic. >>> >>> process_mad is optional but currently that optionality is based on >>> whether MADs (QP0 and/or QP1) are supported or not. This is reflected in >>> the core_cap_flags and is related to the device type: IB including RoCE >>> v. non IB. The current in tree device drivers that do not support >>> process_mad are i40iw (iWARP) and usnic. >> >> I see - we are introducing a new case here, then. >>>> >>>> This fix allows MAD traffic to flow down to the device also >>>> when MAD traffic is completely handled by the device >>> >>> All MAD traffic is not handled by the device. >> >> Yes, in our case all MAD packet handling is done by the device, > > SMInfo is not handled by device unless you have SM in hardware. > It also depends on management class, method, and direction > (outgoing/incoming) and in case of DR SMP the DR path. Yes, with "handling" here we really mean "deciding what to do with it" - in our case, the driver is not involved in those decisions at all. That includes both inbound and outbound DR and LID routed SMInfo packets. >> the driver's task for MAD packets is just to forward. > > I think that it's a little more complex than that. See above. > > Perhaps some of this is due to not understanding what the effects of > forwarding are with device such as SIF where SMA (and SMI) as well as > PMA are totally in hardware. If MAD is locally handled rather than > "forwarded", I assume that the device generates and sends the response > MAD, right ? Yes, correct. > Do locally handled MADs work correctly ? Yes > Does OpenSM properly run on SIF? The goal is to have OpenSM work seamlessly with SIF, yes. > I saw a post yesterday on issue with running OpenSM on SIF in terms of > obtaining the local PortInfo via DR SMP with hop count of 0. Do you have a link ref to this? >>>> and no process_mad function is provided. >>>> >>>> Signed-off-by: Knut Omang <knut.omang@xxxxxxxxxx> >>>> --- >>>> drivers/infiniband/core/mad.c | 6 ++++++ >>>> drivers/infiniband/core/smi.h | 6 ++---- >>>> 2 files changed, 8 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c >>>> index 2d49228..ece33ec 100644 >>>> --- a/drivers/infiniband/core/mad.c >>>> +++ b/drivers/infiniband/core/mad.c >>>> @@ -813,6 +813,12 @@ static int handle_outgoing_dr_smp(struct ib_mad_agent_private *mad_agent_priv, >>>> goto out; >>>> } >>>> >>>> + /* If device does not define the optional process_mad function it means that >>>> + * everything is handled in hardware: >>>> + */ >>>> + if (!device->process_mad) >>>> + goto out; >>>> + >>> This changes the ib_post_send_mad flow for those devices in that it >>> takes the send rather than error path. >> >> Yes, but no packets will ever be received by this function for the i40iw and usnic >> devices because they have said in their capabilities that they do not support SMI? > > You're right - it shouldn't get here for those devices (it's based on > MAD rather than SMI cap) - umad open would fail for the port GUID. Ok, I see - good! >>>> local = kmalloc(sizeof *local, GFP_ATOMIC); >>>> if (!local) { >>>> ret = -ENOMEM; >>>> diff --git a/drivers/infiniband/core/smi.h b/drivers/infiniband/core/smi.h >>>> index 33c91c8..16a9f9a 100644 >>>> --- a/drivers/infiniband/core/smi.h >>>> +++ b/drivers/infiniband/core/smi.h >>>> @@ -67,8 +67,7 @@ static inline enum smi_action smi_check_local_smp(struct ib_smp *smp, >>>> { >>>> /* C14-9:3 -- We're at the end of the DR segment of path */ >>>> /* C14-9:4 -- Hop Pointer = Hop Count + 1 -> give to SMA/SM */ >>>> - return ((device->process_mad && >>>> - !ib_get_smp_direction(smp) && >>>> + return ((!ib_get_smp_direction(smp) && >>>> (smp->hop_ptr == smp->hop_cnt + 1)) ? >>>> IB_SMI_HANDLE : IB_SMI_DISCARD); >>>> } >>>> @@ -82,8 +81,7 @@ static inline enum smi_action smi_check_local_returning_smp(struct ib_smp *smp, >>>> { >>>> /* C14-13:3 -- We're at the end of the DR segment of path */ >>>> /* C14-13:4 -- Hop Pointer == 0 -> give to SM */ >>>> - return ((device->process_mad && >>>> - ib_get_smp_direction(smp) && >>>> + return ((ib_get_smp_direction(smp) && >>>> !smp->hop_ptr) ? IB_SMI_HANDLE : IB_SMI_DISCARD); >>>> } >>> Also, with this approach of optional process_mad for IB device, will/how >>> will sysfs port counters be supported for this device ? This currently >>> relies on process_mad. >> >> Yes, that is actually a problem for us right now. >> We do however think that the solution of composing a packet to send via process_mad is >> kind of overkill as this doesn't allow devices to optimize the way to retrieve this info. > > Is there a way other than via PMA to get these counters ? Yes, the driver have a work request based model to request such info from the device. This is partly firmware based so it can be extended/adapted if necessary. >>> It should be possible to implement a process_mad routine to return >>> ib_mad_result based on management class and perhaps attribute ID in the >>> case of SMInfo. Can this alternative approach be used for SIF ? >> >> The problem is that the handle_outgoing_dr_smp function has an implicit assumption that >> some packets are handled by the process_mad function itself. There is no way to provide return >> values from the process_mad function that ensures that packets are always forwarded to the device, >> so the only viable solution without breaking the API seems to be to not implement process_mad. > > Are you referring to the difference between returning 0 when no > process_mad function as this patch does and what happens when > process_mad returns 0 inside of the ib_mad_post_send routine ? Yes, that sounds about right, it's been a while since we struggled with this. > Doesn't the send MAD need to be tracked in the MAD layer ? The MAD layer tracking happens above the driver, it creates and manages QP0 and sees all mad packets that gets sent as with Connect-X<n> ? > Is there some problem invoking ib_send_mad (or ib_send_rmpp_mad) ? No, to my knowledge we are able to support the normal management applications just as one would expect. Thanks, Knut -- 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