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 _______________________________________________ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general