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

[Or Gerlitz]

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.

Or.