[ofa-general] Re: [PATCH 6/16 v4] IB/mthca: Add checksum offload support

[Eli Cohen]

Or Gerlitz wrote:
> Eli Cohen wrote:
>> --- a/drivers/infiniband/hw/mthca/mthca_main.c
>> +++ b/drivers/infiniband/hw/mthca/mthca_main.c
>> @@ -267,6 +267,10 @@ static int mthca_dev_lim(struct mthca_dev *mdev, 
>> struct mthca_dev_lim *dev_lim)
>>      if (dev_lim->flags & DEV_LIM_FLAG_SRQ)
>>          mdev->mthca_flags |= MTHCA_FLAG_SRQ;
>>  
>> +    if (mthca_is_memfree(mdev))
>> +        if (dev_lim->flags & DEV_LIM_FLAG_IPOIB_CSUM)
>> +            mdev->device_cap_flags |= IB_DEVICE_IP_CSUM;
>> +
> ...
>> @@ -1109,6 +1113,8 @@ static int __mthca_init_one(struct pci_dev 
>> *pdev, int hca_type)
>>      if (err)
>>          goto err_cmd;
>>  
>> +    mdev->ib_dev.flags = mdev->device_cap_flags;
> 
>> Roland Dreier wrote:
>> I don't see any place that ca->flags ever gets set; in fact if I
>> delete the flags member of struct ib_device from my tree, it all still
>> compiles fine.
> 
> OK, Roland, correct, in my review I failed to catch the fact that the 
> way to go by the HW driver is set the device_cap_flags and by the ULP to 
> issue device query and not rely on the flags field.
> 
>> So have you actually tested any of these checksum offload code paths?
> 
> yes, although the vast majority of the testing here is under the ofed 
> form of these patches (a point to fix!) where Eli updated the ofed ones 
> after getting feedback/comments from me and others.
> 
> To remove doubt "[PATCH 4/16 v4] IB/ipoib: Add checksum offload support" 
> works well over Mellanox device b/c mthca and mlx4 sets the flags field, 
>  over other devices the behavior is not defined.
> 
> So, Eli, this must be fixed for rc5. Also, and more important, lets no 
> let the missing of 2.6.25 stop the merging of the stateless offloads, 
> submitting the fixes early will ensure its goes into 2.6.26
> 

Or,
I think you're right and that's how it should have been done. However querying
device cap will cause quite a few changes in other patches so I think we should
better leave this as it is right now. I will however re-generate the patches for
2.6.25 .