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

[Eli Cohen]

On Sun, 2008-03-23 at 09:33 +0200, Or Gerlitz wrote:
> Eli Cohen wrote:
> > --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> > @@ -418,14 +449,35 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb,
> >  {
> > +	int hlen;
> > +	void *phead;
> > +
> > +	if (!skb_is_gso(skb)) {
> > +		if (unlikely(skb->len > priv->mcast_mtu + IPOIB_ENCAP_LEN)) {
> > +			ipoib_warn(priv, "packet len %d (> %d) too long to send, dropping\n",
> > +				   skb->len, priv->mcast_mtu + IPOIB_ENCAP_LEN);
> > +			++dev->stats.tx_dropped;
> > +			++dev->stats.tx_errors;
> > +			ipoib_cm_skb_too_long(dev, skb, priv->mcast_mtu);
> > +			return;
> > +		}
> > +		phead = 0;
> > +		hlen = 0;
> > +	} else {
> > +		/*
> > +		 * LSO header is limited to max 60 bytes
> > +		 */
> > +		if (unlikely((ip_hdr(skb)->ihl + tcp_hdr(skb)->doff) > 15)) {
> > +			ipoib_warn(priv, "ip(%d) and tcp(%d) headers too long, dropping skb\n",
> > +				   ip_hdr(skb)->ihl << 2, tcp_hdr(skb)->doff << 2);
> > +			goto drop;
> > +		}
> >   
> is the 60 bytes being a limitation of the connectX HW, the Linux kernel 
> stack or some "lso spec"?
> > +		hlen = ((ip_hdr(skb)->ihl + tcp_hdr(skb)->doff) << 2) + IPOIB_ENCAP_LEN;
> > +		phead = skb->data;
It's an implementation decision - I assume that I will never get TSO
SKBs where the headers exceed 60 bytes.

> >   
> Looking in e1000 tso code (eg 
> http://lxr.linux.no/linux/drivers/net/e1000/e1000_main.c#L2884) I see 
> that the header len is gotten by
> skb_transport_offset(skb) + tcp_hdrlen(skb), is that what 
> ((ip_hdr(skb)->ihl + tcp_hdr(skb)->doff) << 2) gives? 
It looks like I would get the same effect if I'd used e1000 style though
I'm not sure which approach is faster. 

> > @@ -470,6 +521,12 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb,
> >  			netif_stop_queue(dev);
> >  		}
> >  	}
> > +	return;
> > +
> > +drop:
> > +	++dev->stats.tx_errors;
> > +	dev_kfree_skb_any(skb);
> > +	return;
> >   
> shouldn't the tx_dropped counter be incremented here?
I am not sure. Does every erroneous tx packets imply incrementing the
drop counter too?