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

[Eli Cohen]

On Fri, 2008-03-28 at 20:22 -0700, Roland Dreier wrote:
> OK, applied, but:
> 
>  > +	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);
> 
> can this ever really happen?
I saw the above warning a few times.
>   Is it worth having code to check it?
> presumably we'll just get a local length error on the send and find
> out anyway.
Why do you think we would get a local length error? Looks to me like the
message will be transmitted.

When this condition becomes true an ICMP message will be sent to the
other host to reduce the path MTU? Shouldn't we keep the above condition
for this?

> 
>  > +		if (unlikely(!skb_pull(skb, hlen))) {
>  > +			ipoib_warn(priv, "linear data too small\n");
>  > +			goto drop;
>  > +		}
> 
> similarly can this ever trigger?
> 

I was not 100% sure that this won't happen so I put the condition. I
never saw this warning anyway so I think we can just use this:
	__skb_pull(skb, hlen);

What do you think?