Fwd: Re: xennet: skb rides the rocket: 20 slots

[Sander Eikelenboom <linux@eikelenboom.it>]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

This is a forwarded message
From: ANNIE LI <annie.li@oracle.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Date: Monday, January 14, 2013, 10:39:53 AM
Subject: [Xen-devel] xennet: skb rides the rocket: 20 slots


Resend because xen-devel wasn't copied on the original ...


===8<==============Original message text===============
Hi

I created a patch for this, but I failed to reproduce this issue and 
verify it. The patch was attached,

Thanks
Annie

On 2013-1-11 18:16, Ian Campbell wrote:
> On Fri, 2013-01-11 at 10:09 +0000, Paul Durrant wrote:
>>> -----Original Message-----
>>> Without GSO I don't think you should be seeing packets larger than the MTU,
>>> which would normally be either ~1500 or ~9000 and fit easily within any
>>> sensible negotiation for the max frags. I don't think you should worry unduly
>>> about this case.
>>>
>> A stack could still send down a packet with one byte per frag though,
>> right? A copy-and-coalesce path would still be needed in this case.
> True. In that case skb_linearise would probably do the job on Linux.
>
> Ian
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

===8<===========End of original message text===========

[-- Attachment #2: Message01.eml --]
[-- Type: message/rfc822, Size: 6355 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 876 bytes --]

Hi

I created a patch for this, but I failed to reproduce this issue and 
verify it. The patch was attached,

Thanks
Annie

On 2013-1-11 18:16, Ian Campbell wrote:
> On Fri, 2013-01-11 at 10:09 +0000, Paul Durrant wrote:
>>> -----Original Message-----
>>> Without GSO I don't think you should be seeing packets larger than the MTU,
>>> which would normally be either ~1500 or ~9000 and fit easily within any
>>> sensible negotiation for the max frags. I don't think you should worry unduly
>>> about this case.
>>>
>> A stack could still send down a packet with one byte per frag though,
>> right? A copy-and-coalesce path would still be needed in this case.
> True. In that case skb_linearise would probably do the job on Linux.
>
> Ian
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

[-- Attachment #2.1.2: 0001-This-patch-implements-protect-mechanism-to-avoid-dro.patch --]
[-- Type: text/plain, Size: 2211 bytes --]

>From 73932eea3cd48ec03f4f9eef66aae57afad74eb7 Mon Sep 17 00:00:00 2001
From: root <root@localhost.(none)>
Date: Tue, 15 Jan 2013 01:06:59 +0800
Subject: [PATCH 1/1] This patch implements protect mechanism to avoid dropping packets when
 the required slot of skb is larger than 19.

---
 drivers/net/xen-netfront.c |   45 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 7ffa43b..574c2d3 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -642,6 +642,49 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static int xennet_xmit_skb(struct sk_buff *skb, struct net_device *dev)
+{
+	char *data = skb->data;
+	int slots;
+	unsigned int offset = offset_in_page(data);
+	unsigned int len = skb_headlen(skb);
+	struct sk_buff *segs, *nskb;
+
+	slots = DIV_ROUND_UP(offset + len, PAGE_SIZE) +
+		xennet_count_skb_frag_slots(skb);
+	if (unlikely(slots > MAX_SKB_FRAGS + 1)) {
+		net_alert_ratelimited(
+			"xennet: skb rides the rocket: %d slots\n", slots);
+		if (skb_is_gso(skb)) {
+			segs = skb_gso_segment(skb, dev->features);
+			if (IS_ERR(segs))
+				goto err_end;
+
+			do {
+				nskb = segs;
+				segs = segs->next;
+				nskb->next = NULL;
+				xennet_start_xmit(nskb, dev);
+			} while (segs);
+
+			dev_kfree_skb(skb);
+		} else
+			if (skb_linearize(skb) == 0)
+				xennet_start_xmit(skb, dev);
+			else
+				goto err_end;
+
+	} else
+		xennet_start_xmit(skb, dev);
+
+	return NETDEV_TX_OK;
+
+err_end:
+	dev_kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return NETDEV_TX_OK;
+}
+
 static int xennet_close(struct net_device *dev)
 {
 	struct netfront_info *np = netdev_priv(dev);
@@ -1280,7 +1323,7 @@ static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_open            = xennet_open,
 	.ndo_uninit          = xennet_uninit,
 	.ndo_stop            = xennet_close,
-	.ndo_start_xmit      = xennet_start_xmit,
+	.ndo_start_xmit      = xennet_xmit_skb,
 	.ndo_change_mtu	     = xennet_change_mtu,
 	.ndo_get_stats64     = xennet_get_stats64,
 	.ndo_set_mac_address = eth_mac_addr,
-- 
1.7.3.4


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel