[openib-general] patches to 2.6.19.1 kernel for switch Operation

[Hal Rosenstock]

Suri,

On Mon, 2007-02-05 at 12:31, Suresh Shelvapille wrote:
> Hal:
> 
> We are upgrading to 2.6.19.1 kernel and I finally ported the changes
> required for Switch operation from my current kernel (2.6.12) version. 
> 
> I have tested these changes for a switch with different SM(s). But I need
> the community's help to test the changes on different HCAs to make sure I
> have not broken anything.
> 
> Please see if the changes look OK.

Here are my initial comments on these patches based only on code
inspection:

mad.c:
@@ -1871,24 +1877,49 @@
...
        if (recv->mad.mad.mad_hdr.mgmt_class ==
            IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE) {
-               if (!smi_handle_dr_smp_recv(&recv->mad.smp,
-                                          
port_priv->device->node_type,
-                                           port_priv->port_num,
-                                          
port_priv->device->phys_port_cnt))
-                       goto out;
-               if (!smi_check_forward_dr_smp(&recv->mad.smp))
-                       goto local;
-               if (!smi_handle_dr_smp_send(&recv->mad.smp,
-                                          
port_priv->device->node_type,
-                                           port_priv->port_num))
+
+               int retsmi;
+
+               retsmi = smi_handle_dr_smp_recv(&recv->mad.smp,
+                                               port_priv->device->node_type,
+                                               port_num,
+                                               port_priv->device->phys_port_cnt);
+               if (!retsmi)
                        goto out;
-               if (!smi_check_local_smp(&recv->mad.smp,
port_priv->device))
+               else if (retsmi == 2) {
+                       if (!response) {
+                               printk(KERN_ERR PFX "No memory for
forwarded MAD\n");
+                               goto out;
+                       }
+                       memcpy(response, recv, sizeof(*response));
+                       response->header.recv_wc.wc =
&response->header.wc;
+                       response->header.recv_wc.recv_buf.mad =
&response->mad.mad;
+                       response->header.recv_wc.recv_buf.grh =
&response->grh;
+
+                       /* in case of forward, output port should be the
one
+                        * in either the Initial path(for outgoing) or
return_path(return)
+                        */
+                       if (!ib_get_smp_direction(&recv->mad.smp))
+                               port_num =
recv->mad.smp.initial_path[recv->mad.smp.hop_ptr+1];
+                       else
+                               port_num =
recv->mad.smp.return_path[recv->mad.smp.hop_ptr-1];
+
+                       if (!agent_send_response(&response->mad.mad, 
+                                                &response->grh, wc, 
+                                                port_priv->device,
+                                                port_num,
+                                                qp_info->qp->qp_num))
+                               response = NULL;

Per the above change, it appears that smi_check_forward_dr_smp and
smi_handle_dr_smp_send are no longer used at least here
(smi_check_forward_dr_smp is not used at all with this change). Couldn't
these be fixed to do the right thing for this case (as well as existing
cases) ? I'm not sure your changes work for end ports (CA and router
ports).

Also, based on smi comments below, there might also be changes to
following:
+                       if (!ib_get_smp_direction(&recv->mad.smp))
+                               port_num =
recv->mad.smp.initial_path[recv->mad.smp.hop_ptr+1];
+                       else
+                               port_num =
recv->mad.smp.return_path[recv->mad.smp.hop_ptr-1];
+

smi.c:
@@ -147,13 +147,18 @@
...
 
                /* C14-9:3 -- We're at the end of the DR segment of path
*/
                if (hop_ptr == hop_cnt) {
                        if (hop_cnt)
                                smp->return_path[hop_ptr] = port_num;
+                       smp->hop_ptr++;
+
                        /* smp->hop_ptr updated when sending */
The comment indicates the hop_ptr should be updated when sending not
here. Can't this be done ?

@@ -168,8 +173,8 @@
 
                /* C14-13:1 */
                if (hop_cnt && hop_ptr == hop_cnt + 1) {
-                       smp->hop_ptr--;
-                       return (smp->return_path[smp->hop_ptr] ==
+                       /* smp->hop_ptr--;*/
+                       return (smp->return_path[smp->hop_ptr-1] ==
                                port_num);
                }
This change affects more than switches as now the hop_ptr is not correct
per SMI. I think this also should be handled differently.

agent.c: 
@@ -113,6 +119,11 @@
 
        memcpy(send_buf->mad, mad, sizeof *mad);
        send_buf->ah = ah;
+       mad_send_wr = container_of(send_buf,
+                                  struct ib_mad_send_wr_private,
+                                  send_buf);
+       mad_send_wr->send_wr.wr.ud.port_num = port_num;
+       
        if ((ret = ib_post_send_mad(send_buf, NULL))) {

Shouldn't this only be for switches ? Not sure it causes a problem for
other than switches, but I think would be more consistent with the
current code. So I think this change should be surrounded by:
if (device->node_type == RDMA_NODE_IB_SWITCH) {
...
}

-- Hal

> Thanks,
> Suri