[PATCH] [IPOIB] Check for a MTU overflow on the GSO path

[PATCH] [IPOIB] Check for a MTU overflow on the GSO path

[Jason Gunthorpe]

The code to check that the send packet size is less than the MTU
is only invoked in the non-GSO case. This lets packets which are too
large pass through.

Compute the largest packet the GSO operation can produce and check
that against the MTU.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_ib.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index ec6b4fb..aa9f9cf 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -532,6 +532,7 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb,
 	struct ipoib_tx_buf *tx_req;
 	int hlen, rc;
 	void *phead;
+	unsigned int required_mtu;
 
 	if (skb_is_gso(skb)) {
 		hlen = skb_transport_offset(skb) + tcp_hdrlen(skb);
@@ -543,17 +544,21 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb,
 			dev_kfree_skb_any(skb);
 			return;
 		}
+
+		required_mtu = skb_shinfo(skb)->gso_size + hlen;
 	} else {
-		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 = NULL;
 		hlen  = 0;
+		required_mtu = skb->len;
+	}
+
+	if (unlikely(required_mtu > priv->mcast_mtu + IPOIB_ENCAP_LEN)) {
+		ipoib_warn(priv, "packet len %d (> %d) too long to send, dropping\n",
+			   required_mtu, 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;
 	}
 
 	ipoib_dbg_data(priv, "sending packet, length=%d address=%p qpn=0x%06x\n",
-- 
1.5.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [IPOIB] Check for a MTU overflow on the GSO path

[Or Gerlitz]

Jason Gunthorpe wrote:
> The code to check that the send packet size is less than the MTU is only invoked in the non-GSO case. This lets packets which are too large pass through. 
Jason,

Did you get here following code inspection or during debugging? if the 
latter, what does it take to reproduce that?

Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [IPOIB] Check for a MTU overflow on the GSO path

[Jason Gunthorpe]

On Wed, Sep 15, 2010 at 02:27:39PM +0200, Or Gerlitz wrote:
> Jason Gunthorpe wrote:
> >The code to check that the send packet size is less than the MTU is only 
> >invoked in the non-GSO case. This lets packets which are too large pass 
> >through. 
> Jason,
> 
> Did you get here following code inspection or during debugging? if the 
> latter, what does it take to reproduce that?

I noticed it when I was working on the code for the patch after, IPoIB
was still causing MTU discards in my network and that was why. I
didn't try it with CM mode, but I can't see that it would be any
different.

In my testing what I noticed was that the first few packets of a TCP
session triggered the PMTU stuff, but immediately after were a few
GSO packets that tried to go through with the larger MTU. They were
resent fairly quickly with the correct MTU due to the PMTU fixup that
happened.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html