[ofa-general] Re: [PATCH] IPOIB: use LRO

[Eli Cohen]

On Thu, 2007-12-06 at 15:27 -0800, Roland Dreier wrote:
> > TODO:
>  > add checksum offload support to the core and hw devices.
> 
> Given this I assume this is just an RFC and you don't expect this to
> be merged as-is, right?
Adding the authors of ehea which what I used as an example.
Yes, please send any comments. I believe we need checksum patches to be
re-generated against latest kernel right?

> 
>  > +static int get_skb_hdr(struct sk_buff *skb, void **iphdr,
>  > +		       void **tcph, u64 *hdr_flags, void *priv)
>  > +{
>  > +	unsigned int ip_len;
>  > +	struct iphdr *iph;
>  > +
>  > +	/* FIXME - verify CQE checksum ??? */
>  > +
>  > +	/* non tcp packet */
> 
> I don't understand this comment here.
I meant here that I think this is where I have to check with the HW
whether the chekcsum of the packet is correct. I looked at IBM ehea and
couldn't see any explicit such check. Do you think such a test is
required?

> 
>  > +	skb_reset_network_header(skb);
>  > +	iph = ip_hdr(skb);
>  > +	if (iph->protocol != IPPROTO_TCP)
>  > +		return -1;
>  > +
>  > +	ip_len = ip_hdrlen(skb);
>  > +	skb_set_transport_header(skb, ip_len);
>  > +	*tcph = tcp_hdr(skb);
>  > +
>  > +	/* check if ip header and tcp header are complete */
>  > +	if (iph->tot_len < ip_len + tcp_hdrlen(skb))
>  > +		return -1;
>  > +
>  > +	*hdr_flags = LRO_IPV4 | LRO_TCP;
> 
> I don't see anywhere that you test the ethertype for IPv4 vs. IPv6.
> So how do you know you have an IPv4 packet here?  I guess you need the
> check before you use ip_hdr() above.
Yes looks like I should add this check. Though I did not see ehea do it.

> 
>  > +	*iphdr = iph;
>  > +
>  > +	return 0;