[ewg] RE: sean_cm_limit_mra_timeout.patch

[Hal Rosenstock]

On Mon, 2007-12-10 at 10:34 -0800, Sean Hefty wrote:
> >but it's currently "solidly" part of both OFED 1.3 and 1.2.5. Should it
> >then be ?
> 
> IMO - any patch which has been rejected for upstream submission or is considered
> experimental should be yanked from OFED.

Relative to rejected patches, that makes sense to me; not sure how older
releases would be handled when the patch is rejected post the release
though.

Also, experimental patches could be provided on some basis but shouldn't
be part of the normal OFED.


In terms of this specific patch, there appears to be some inconsistency
as follows:

The patch has:
+       if (cm_id_priv->timeout_ms > cm_convert_to_ms(max_timeout)) {
+               printk(KERN_WARNING PFX "req timeout_ms %d > %d, decreasing\n",
+                      cm_id_priv->timeout_ms, cm_convert_to_ms(max_timeout));
+               cm_id_priv->timeout_ms = cm_convert_to_ms(max_timeout);
+       }

whereas cm.c has:
        cm_id_priv->timeout_ms = cm_convert_to_ms(
                                    param->primary_path->packet_life_time) * 2 +
                                 cm_convert_to_ms(
                                    param->remote_cm_response_timeout);

This forces max_timeout to be set one higher than really needed. Should
the comparison in the patch also account for the packet life time ?

> >Is there some other approach to the specific problem this patch was
> >attempting to fix ?
> 
> A fix could be provided to the SRP target, which is where the real problem is.

and documented in the OFED release notes.

-- Hal

> - Sean