IB/ehca: Construct MAD redirect replies from request MAD

The old code used a lot of hardcoded values, which might not be valid in all
environments (especially routed fabrics or partitioned subnets). Copy as much information as possible from the incoming request to prevent that.

Signed-off-by Joachim Fenkes <[email protected]>

---

Hal, Jason -- here's the change I promised. Looks okay to you? Roland -- if Hal and Jason don't object, please queue this up for the next kernel. Thanks!

Joachim

drivers/infiniband/hw/ehca/ehca_sqp.c | 47
++++++++++++++++++++++++++++----

1 files changed, 41 insertions(+), 6 deletions(-)

drivers/infiniband/hw/ehca/ehca_sqp.c +38 -3

index c568b28..8c1213f 100644
@@ -125,14 +125,30 @@ struct ib_perf {
       u8 data[192];
 } __attribute__ ((packed));
 
+/* TC/SL/FL packed into 32 bits, as in ClassPortInfo */
+struct tcslfl {
+       u32 tc:8;
+       u32 sl:4;
+       u32 fl:20;
+} __attribute__ ((packed));
+
+/* IP Version/TC/FL packed into 32 bits, as in GRH */
+struct vertcfl {
+       u32 ver:4;
+       u32 tc:8;
+       u32 fl:20;
+} __attribute__ ((packed));
 
 static int ehca_process_perf(struct ib_device *ibdev, u8 port_num,
+                            struct ib_wc *in_wc, struct ib_grh *in_grh,
                            struct ib_mad *in_mad, struct ib_mad *out_mad)
 {
       struct ib_perf *in_perf = (struct ib_perf *)in_mad;
       struct ib_perf *out_perf = (struct ib_perf *)out_mad;
       struct ib_class_port_info *poi =
               (struct ib_class_port_info *)out_perf->data;
+       struct tcslfl *tcslfl =
+               (struct tcslfl *)&poi->redirect_tcslfl;
       struct ehca_shca *shca =
               container_of(ibdev, struct ehca_shca, ib_device);
       struct ehca_sport *sport = &shca->sport[port_num - 1];

@@ -158,10 +174,29 @@ static int ehca_process_perf(struct ib_device *ibdev,
 u8 port_num,
               poi->base_version = 1;
               poi->class_version = 1;
               poi->resp_time_value = 18;
-               poi->redirect_lid = sport->saved_attr.lid;
-               poi->redirect_qp = sport->pma_qp_nr;
+
+               /* copy local routing information from WC where applicable
 */
+               tcslfl->sl         = in_wc->sl;
+               poi->redirect_lid  =
+                       sport->saved_attr.lid | in_wc->dlid_path_bits;
+               poi->redirect_qp   = sport->pma_qp_nr;
               poi->redirect_qkey = IB_QP1_QKEY;
-               poi->redirect_pkey = IB_DEFAULT_PKEY_FULL;
+
+               ehca_query_pkey(ibdev, port_num, in_wc->pkey_index,
+                               &poi->redirect_pkey);
+
+               /* if request was globally routed, copy route info */
+               if (in_grh) {
+                       struct vertcfl *vertcfl =
+                               (struct vertcfl
 *)&in_grh->version_tclass_flow;
+                       memcpy(poi->redirect_gid, in_grh->dgid.raw,
+                              sizeof(poi->redirect_gid));
+                       tcslfl->tc        = vertcfl->tc;
+                       tcslfl->fl        = vertcfl->fl;
+               } else
+                       /* else only fill in default GID */
+                       ehca_query_gid(ibdev, port_num, 0,
+                                      (union ib_gid *)&poi->redirect_gid);

               ehca_dbg(ibdev, "ehca_pma_lid=%x ehca_pma_qp=%x",
                        sport->saved_attr.lid, sport->pma_qp_nr);

drivers/infiniband/hw/ehca/ehca_sqp.c +3 -3

@@ -183,8 +218,7 @@ perf_reply:
 
 int ehca_process_mad(struct ib_device *ibdev, int 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)
+                    struct ib_mad *in_mad, struct ib_mad *out_mad)
 {
       int ret;
 

@@ -196,7 +230,8 @@ int ehca_process_mad(struct ib_device *ibdev, int
 mad_flags, u8 port_num,
               return IB_MAD_RESULT_SUCCESS;
 
       ehca_dbg(ibdev, "port_num=%x src_qp=%x", port_num, in_wc->src_qp);
-       ret = ehca_process_perf(ibdev, port_num, in_mad, out_mad);
+       ret = ehca_process_perf(ibdev, port_num, in_wc, in_grh,
+                               in_mad, out_mad);
 
       return ret;

}
--
1.6.0.4

_______________________________________________
ewg mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg

Thanks for doing this. It looks sane to me. The only issue I recall that

appears to be remaining is a better setting of

rather than hardcoding. Perhaps using the value from PortInfo is the way

(ideally it would be that value from the port to which the the requester

being redirected to but that might not be so easy to get from this port.

Hal Rosenstock <[email protected]> wrote on 26.08.2009 17:15:03:

Quoted 3 lines

Thanks for doing this. It looks sane to me. The only issue I recall that

appears to be remaining is a better setting of

ClassPortInfo:RespTimeValue

Quoted 1 line

rather than hardcoding. Perhaps using the value from PortInfo is the way

to go

Quoted 1 line

(ideally it would be that value from the port to which the the requester

is

Quoted 1 line

being redirected to but that might not be so easy to get from this port.

I don't think that effort will be necessary or even legal. The requestor will react to the redirection with another Get(ClassPortInfo) to the redirection target, which will reply with its own RespTimeValue, so our driver should speak for itself.

processing and sending of the response is going to be scheduled (we're not running on real-time constraints here), we play it safe and return 18, which amounts to roughly a second.

Make sense?

Regards
Joachim

I don't think it should be hard coded. IMO it would be better to default

and somehow able to be adjusted (via a (dynamic) module parameter ?).

Given that you seem to like the rest of the code and Jason hasn't spoken up yet, I think we can have Roland merge this patch. Roland, what do you think?