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

[Roland Dreier]

 > > 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.

Right, but this code sets the dest MTU to tx->mtu - IPOIB_ENCAP_LEN if
it sees a packet longer than tx->mtu.  Shouldn't the initial test be
against tx->mtu - IPOIB_ENCAP_LEN too?

 > 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?

Because the largest MTU we can handle is the biggest message we can
receive minus the 4 byte encapsulation overhead.  In other words, if
we can handle 2048 byte UD messages, then we shouldn't allow a
datagram MTU above 2044.  The same reasoning applies to ethernet, so
an ethernet interface has an MTU of 1500, because ethernet can send
1514 byte packets but space has to be left for the 14 bytes of
src/dest/ethertype fields.

Or am I missing something here?

 - R.