[openib-general] [PATCH] mad: Add RMPP support for DevMgt class

Hal Rosenstock halr at voltaire.com
Thu Mar 16 15:51:18 PST 2006


On Thu, 2006-03-16 at 18:34, Roland Dreier wrote:
> This patch has a chunk:
> 
>  >  	if (mgmt_class == IB_MGMT_CLASS_SUBN_ADM)
>  >  		return IB_MGMT_SA_HDR;
>  > +	else if (mgmt_class == IB_MGMT_CLASS_DEVICE_MGMT)
>  > +		return IB_MGMT_DM_HDR;
>  >  	else if ((mgmt_class >= IB_MGMT_CLASS_VENDOR_RANGE2_START) &&
>  >  		 (mgmt_class <= IB_MGMT_CLASS_VENDOR_RANGE2_END))
>  >  		return IB_MGMT_VENDOR_HDR;
> 
> and...
> 
>  >  	if (mgmt_class == IB_MGMT_CLASS_SUBN_ADM)
>  >  		return IB_MGMT_SA_HDR;
>  > +	else if (mgmt_class == IB_MGMT_CLASS_DEVICE_MGMT)
>  > +		return IB_MGMT_DM_HDR;
>  >  	else if ((mgmt_class >= IB_MGMT_CLASS_VENDOR_RANGE2_START) &&
>  >  		 (mgmt_class <= IB_MGMT_CLASS_VENDOR_RANGE2_END))
>  >  		return IB_MGMT_VENDOR_HDR;
> 
> We should consolidate this identical code into one place.

Yes, I think Sean pointed this out before when this duplication was
originally created.

> Among other
> benefits, this would make this sort of maintenance easier and less
> error prone.

Sure; I can reissue an updated patch for this.

> In fact, there's also the following chunk which maybe could tie in too:
> 
>  > +	} else if (rmpp_mad->mad_hdr.mgmt_class == IB_MGMT_CLASS_DEVICE_MGMT) {
>  > +		hdr_len = IB_MGMT_DM_HDR;

Yes, that might be possible too.

-- Hal




More information about the general mailing list