[ofa-general] Re: [PATCH] IB/mlx4: Add checksum offload support

[Eli Cohen]

On Sun, 2008-03-16 at 10:06 +0200, Or Gerlitz wrote:
> Roland Dreier wrote:
> > thanks, I applied the ipoib and mlx4 patches with some cleanups.
> > One thing I changed is that ipoib doesn't keep a cached copy of the
> > HCA capabilities flags -- I didn't see any reason why it was needed.
> >   
> Such a copy would make the patches for LSO and interrupt moderation 
> support simpler. If we agree on this point, it could be added through 
> the patch set for those two features, which in the light of the 
> integration these two patches, should be on its way (Eli? also the mthca 
> checksum offload patch).
I am not sure what Roland meant but we could take an approach where
instead of saving a copy of the hca capabilities, we will reflect these
capabilities into priv->flags. We can do the same also for LSO. As for
CQ moderation, my original approach was not to define this as a
capability - I just did the modify. Do you think this should be a
capability published by the device?

> > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > @@ -1120,6 +1121,25 @@ static struct net_device *ipoib_add_port(const char *format,
> >  		goto device_init_failed;
> >  	}
> > .... 
> > +	if (device_attr->device_cap_flags & IB_DEVICE_UD_IP_CSUM) {
> > +		set_bit(IPOIB_FLAG_CSUM, &priv->flags);
> > +		priv->dev->features |= NETIF_F_SG | NETIF_F_IP_CSUM;
> > +	}
> >   
> Don't you need to check the mode (connected vs datagram) of the device 
> at this point before advertising these the S/G and CSUM flags to the stack?
> 
> Or.
IPOIB's default mode of operation is UD so this check is important only
when set_mode() is invoked - Roland, is that what you thought about?

>