[ofa-general] Re: [PATCH] IB/ipoib: fix thinko in packet length checks

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCH] IB/ipoib: fix thinko in packet length checks
> 
> This definitely looks like a problem, but I'm confused by this:
> 
>  > --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>  > +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>  > @@ -452,7 +452,7 @@ void ipoib_cm_send(struct net_device *dev, struct sk_buff *skb, struct ipoib_cm_
>  >  			   skb->len, tx->mtu);
>  >  		++priv->stats.tx_dropped;
>  >  		++priv->stats.tx_errors;
>  > -		ipoib_cm_skb_too_long(dev, skb, tx->mtu - INFINIBAND_ALEN);
>  > +		ipoib_cm_skb_too_long(dev, skb, tx->mtu - IPOIB_ENCAP_LEN);
>  >  		return;
>  >  	}
> 
> After this change, the code looks like:
> 
> 	if (unlikely(skb->len > tx->mtu)) {
> 		ipoib_warn(priv, "packet len %d (> %d) too long to send, dropping\n",
> 			   skb->len, tx->mtu);
> 		++priv->stats.tx_dropped;
> 		++priv->stats.tx_errors;
> 		ipoib_cm_skb_too_long(dev, skb, tx->mtu - IPOIB_ENCAP_LEN);
> 		return;
> 	}
> 
> so why is the test against just tx->mtu, while ipoib_cm_skb_too_long()
> is passed an mtu of tx->mtu - IPOIB_ENCAP_LEN?

I actually copied this from datagram code.

What ipoib_cm_skb_too_long does is set the dest MTU metric.

So look at this logic in datagram mode:
	if (new_mtu > IPOIB_PACKET_SIZE - IPOIB_ENCAP_LEN) {
		return -EINVAL;
	}

Why is the max MTU set to IPOIB_PACKET_SIZE - IPOIB_ENCAP_LEN and
not to IPOIB_PACKET_SIZE?

-- 
MST