[ofa-general] [PATCH 4/16] Add checksum offload support for ipoib

[Or Gerlitz]

Eli Cohen wrote:
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> @@ -231,6 +232,18 @@ static void ipoib_ib_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
>  	skb->dev = dev;
>  	/* XXX get correct PACKET_ type here */
>  	skb->pkt_type = PACKET_HOST;
> +
> +	/* check rx csum */
> +	if (test_bit(IPOIB_FLAG_CSUM, &priv->flags) && likely(wc->csum_ok)) {

First, since the device IB_DEVICE_IP_CSUM capability means that "devices 
which publish this capability must support insertion of UDP and TCP 
checksum on outgoing packets and can verify the validity of checksum for 
incoming packets" the IPOIB_FLAG_CSUM bit is redundant, I suggest to 
remove it. Second, the csum_ok bit is not well defined, etc as I 
commented on patch #4

> @@ -394,6 +407,12 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb,
>  		return;
>  	}
>  
> +	if (priv->ca->flags & IB_DEVICE_IP_CSUM &&
> +	    skb->ip_summed == CHECKSUM_PARTIAL)
> +		priv->tx_wr.send_flags |= IB_SEND_IP_CSUM;
> +	else
> +		priv->tx_wr.send_flags &= IB_SEND_IP_CSUM;

I think that the code would be somehow clearer if you use dev->features 
and not priv->ca->flags

Or