[ofa-general] [PATCH 4/10] IB/ipoib: Add LSO support to ipoib

[Eli Cohen]

On Tue, 2008-03-25 at 11:56 +0200, Or Gerlitz wrote:
> Eli Cohen wrote:
> > On Sun, 2008-03-23 at 09:33 +0200, Or Gerlitz wrote:
> >> is the 60 bytes being a limitation of the connectX HW, the Linux kernel 
> >> stack or some "lso spec"?
> > It's an implementation decision - I assume that I will never get TSO
> > SKBs where the headers exceed 60 bytes.
> What does this assumption buys you? do you have to allocate/copy the 
> header and you want to be limited to 60 bytes? can you point me to where 
> this limitation comes into play over this patch set? is there an 
> equivalent design/check in one of the Ethernet drivers supporting LSO?
This is a temporary  ConnectX limitation that will be removed in one of
the next FW releases. I should have put this check in mlx4 so I will
modify the patches accordingly and re-send. Note that I have never seen
this assertion on any of the kernels we've been using so I don't know if
the network stack will ever pass a TSO SKB with header length exceeding
60 bytes.

> 
> > It looks like I would get the same effect if I'd used e1000 style though
> > I'm not sure which approach is faster. 
> I think that the e1000 approach is --much-- more readable, I would go on 
> clarity here.
OK, I will change this.

> >> shouldn't the tx_dropped counter be incremented here?
> > I am not sure. Does every erroneous tx packets imply incrementing the
> > drop counter too?
> I think so, but you can look around in ipoib or other driver or get more 
> opinions.
> 
ipoib is does not follow this rule - is it a bug? This is from
ipoib_ib.c:

	if (unlikely(ipoib_dma_map_tx(priv->ca, tx_req))) {
		++dev->stats.tx_errors;
		dev_kfree_skb_any(skb);
		return;
	}



Other drivers do not increment any of these counters. It looks like they
read them from directly from hardware registers or ignore them (for
example both e1000 and bnx2 assume dma mapping operations never fail).