Hello!
Quoting r. Michael S. Tsirkin ([EMAIL PROTECTED]) "RFC: process_mad extension":
> 
> Hello!
> ib_verbs.h has:
> 
>         int                        (*process_mad)(struct ib_device *device,
>                                                   int process_mad_flags,
>                                                   u8 port_num,
>                                                   u16 source_lid,
>                                                   struct ib_mad *in_mad,
>                                                   struct ib_mad *out_mad);
> 
> This function serves both GSI and SMA MADs. It is used internally by
> the ib core, and I think its normally not used by ULPs.
> 
> process_mad_flags just says whether to enable the MKey check.
> The Mkey check is enabled for incoming mad packets that are being
> returned to the HCA. When enabled, the HCA is expected to generate
> mkey violation trap when appropriate.
> 
> I see some issues with this interface:
> 
> 1. I think for GSI MADs, the HCA (even Tavor) needs more information to
>       generate traps in case of errors.
>       Consider for example Trap 259 - Bkey violation.
>       Per IB spec 1.2, the trap has the following payload.
> 
>       Bad B_Key, <B_Key> from
>       <LIDADDR>/<GIDADDR>/<QP> attempted
>       <METHOD> with <ATTRIBUTEID> and <ATTRIBUTEMODIFIER>
> 
>       However, the GIDADDR/QP is never passed to process_mad and so is not
>       available to the HCA.
> 
> 
> 2. For BMA MADs, it seems an additional flag would be needed to enable/
>    disable the bkey check for the MAD, same as we do for the mkey?
> 
> 3. The Arbel memfree (aka native) mode hardware needs the Grh, and some
>    bits from the completion of the incoming MAD, to process the MAD.
>    Again, this information is not passed to process_mad.
> 
> 
> I think it makes sence to address these issues sooner rather
> than later :)
> 
> I propose extending the interface in the following way:
> 
>         int                        (*process_mad)(struct ib_device *device,
>                                                   int process_mad_flags,
>                                                   u8 port_num,
>                                                 struct ib_wc* in_wc,
>                                                 struct ib_grh* in_grh,
>                                                   struct ib_mad *in_mad,
>                                                   struct ib_mad *out_mad);
> 
> I think its in some sence nicer than passing the slid directly since
> when sm builds packets (with mkey check disabled) slid may not be
> valid at all.
> The new parameters can be NULL in this case, then no trap can be generated.
> 
> If you guys agree, I'll prepare a patch fixing the core and mthca.
> 
> MST

Here is the patch that adds the parameters to the core interface,
and passes them down to mthca, it builds and works for me.

As planned, I added new parameters instead of the slid, and
renamed a flag to reflect we may have bkey and not only mkey check.

I plan to add code to actually pass them to hardware next,
this is under debug, but that will be a separate patch.

I am of two minds whether we want to rename the flag to
something most generic, like IGNORE_CHECK or something.


mst

Signed-off-by: Michael S. Tsirkin <[EMAIL PROTECTED]>


Index: include/ib_verbs.h
===================================================================
--- include/ib_verbs.h  (revision 1498)
+++ include/ib_verbs.h  (working copy)
@@ -684,9 +684,10 @@ struct ib_fmr {
 };
 
 struct ib_mad;
+struct ib_grh;
 
 enum ib_process_mad_flags {
-       IB_MAD_IGNORE_MKEY      = 1
+       IB_MAD_IGNORE_MKEY_BKEY = 1,
 };
 
 enum ib_mad_result {
@@ -812,7 +813,8 @@ struct ib_device {
        int                        (*process_mad)(struct ib_device *device,
                                                  int process_mad_flags,
                                                  u8 port_num,
-                                                 u16 source_lid,
+                                                 struct ib_wc* in_wc,
+                                                 struct ib_grh* in_grh,
                                                  struct ib_mad *in_mad,
                                                  struct ib_mad *out_mad);
 
Index: core/mad.c
===================================================================
--- core/mad.c  (revision 1498)
+++ core/mad.c  (working copy)
@@ -614,6 +614,26 @@ static void snoop_recv(struct ib_mad_qp_
        spin_unlock_irqrestore(&qp_info->snoop_lock, flags);
 }
 
+/* build a fake wc for process_mad */
+
+static void build_smp_wc(struct ib_smp *smp,
+               struct ib_send_wr *send_wr,
+               struct ib_wc* wc)
+{
+       memset(wc,0,sizeof *wc);
+       wc->wr_id=send_wr->wr_id;
+       wc->status=IB_WC_SUCCESS;
+       wc->opcode=IB_WC_RECV;
+       wc->pkey_index=send_wr->wr.ud.pkey_index;
+       wc->byte_len = sizeof(struct ib_mad);
+       wc->src_qp = IB_QP0;
+       wc->qp_num = IB_QP0;
+       wc->slid = smp->dr_slid;
+       wc->sl = 0;
+       wc->dlid_path_bits=0;
+       wc->port_num=send_wr->wr.ud.port_num;
+}
+
 /*
  * Return 0 if SMP is to be sent
  * Return 1 if SMP was consumed locally (whether or not solicited)
@@ -629,6 +649,7 @@ static int handle_outgoing_dr_smp(struct
        struct ib_mad_private *mad_priv;
        struct ib_device *device = mad_agent_priv->agent.device;
        u8 port_num = mad_agent_priv->agent.port_num;
+       struct ib_wc mad_wc;
 
        if (!smi_handle_dr_smp_send(smp, device->node_type, port_num)) {
                ret = -EINVAL;
@@ -658,7 +679,11 @@ static int handle_outgoing_dr_smp(struct
                kfree(local);
                goto out;
        }
-       ret = device->process_mad(device, 0, port_num, smp->dr_slid,
+       
+       build_smp_wc(smp, send_wr, &mad_wc);
+
+       /* No grh for dr smp. */
+       ret = device->process_mad(device, 0, port_num, &mad_wc, NULL,
                                  (struct ib_mad *)smp,
                                  (struct ib_mad *)&mad_priv->mad);
        switch (ret)
@@ -1594,7 +1619,7 @@ local:
 
                ret = port_priv->device->process_mad(port_priv->device, 0,
                                                     port_priv->port_num,
-                                                    wc->slid,
+                                                    wc, &recv->grh,
                                                     &recv->mad.mad,
                                                     &response->mad.mad);
                if (ret & IB_MAD_RESULT_SUCCESS) {
Index: core/sysfs.c
===================================================================
--- core/sysfs.c        (revision 1498)
+++ core/sysfs.c        (working copy)
@@ -315,8 +315,9 @@ static ssize_t show_pma_counter(struct i
 
        in_mad->data[41] = p->port_num; /* PortSelect field */
 
-       if ((p->ibdev->process_mad(p->ibdev, IB_MAD_IGNORE_MKEY, p->port_num, 
0xffff,
-                                  in_mad, out_mad) &
+       if ((p->ibdev->process_mad(p->ibdev, 
+               IB_MAD_IGNORE_MKEY_BKEY,
+                p->port_num, NULL, NULL, in_mad, out_mad) &
             (IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_REPLY)) !=
            (IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_REPLY)) {
                ret = -EINVAL;
Index: hw/mthca/mthca_dev.h
===================================================================
--- hw/mthca/mthca_dev.h        (revision 1498)
+++ hw/mthca/mthca_dev.h        (working copy)
@@ -381,7 +381,8 @@ int mthca_multicast_detach(struct ib_qp 
 int mthca_process_mad(struct ib_device *ibdev,
                      int mad_flags,
                      u8 port_num,
-                     u16 slid,
+                     struct ib_wc* in_wc,
+                     struct ib_grh* in_grh,
                      struct ib_mad *in_mad,
                      struct ib_mad *out_mad);
 int mthca_create_agents(struct mthca_dev *dev);
Index: hw/mthca/mthca_mad.c
===================================================================
--- hw/mthca/mthca_mad.c        (revision 1498)
+++ hw/mthca/mthca_mad.c        (working copy)
@@ -185,12 +185,14 @@ static void forward_trap(struct mthca_de
 int mthca_process_mad(struct ib_device *ibdev,
                      int mad_flags,
                      u8 port_num,
-                     u16 slid,
+                     struct ib_wc* in_wc,
+                     struct ib_grh* in_grh,
                      struct ib_mad *in_mad,
                      struct ib_mad *out_mad)
 {
        int err;
        u8 status;
+       u16 slid = in_wc ? in_wc->slid : IB_LID_PERMISSIVE;
 
        /* Forward locally generated traps to the SM */
        if (in_mad->mad_hdr.method == IB_MGMT_METHOD_TRAP &&
@@ -230,7 +232,7 @@ int mthca_process_mad(struct ib_device *
                return IB_MAD_RESULT_SUCCESS;
 
        err = mthca_MAD_IFC(to_mdev(ibdev),
-                           !!(mad_flags & IB_MAD_IGNORE_MKEY),
+                           !!(mad_flags & IB_MAD_IGNORE_MKEY_BKEY),
                            port_num, in_mad, out_mad,
                            &status);
        if (err) {
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to