From mboxrd@z Thu Jan 1 00:00:00 1970 From: ANNIE LI Subject: Re: xennet: skb rides the rocket: 20 slots Date: Thu, 10 Jan 2013 19:22:22 +0800 Message-ID: <50EEA46E.7000604@oracle.com> References: <72958707.20130104172854@eikelenboom.it> <1357556115.7989.13.camel@zakaz.uk.xensource.com> <50EB8091.90705@oracle.com> <323202711.20130108215503@eikelenboom.it> <50ED1800.1080208@oracle.com> <20130109150850.GI18395@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130109150850.GI18395@phenom.dumpdata.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: Sander Eikelenboom , Ian Campbell , xen-devel List-Id: xen-devel@lists.xenproject.org On 2013-1-9 23:08, Konrad Rzeszutek Wilk wrote: > On Wed, Jan 09, 2013 at 03:10:56PM +0800, ANNIE LI wrote: >> >> On 2013-1-9 4:55, Sander Eikelenboom wrote: >>>> if (unlikely(frags>= MAX_SKB_FRAGS)) { >>>> netdev_dbg(vif->dev, "Too many frags\n"); >>>> return -frags; >>>> } >>> I have added some rate limited warns in this function. However none seems to be triggered while the pv-guest reports the "skb rides the rocket" .. >> Oh, yes, "skb rides the rocket" is a protect mechanism in netfront, >> and it is not caused by netback checking code, but they all concern >> about the same thing(frags>= MAX_SKB_FRAGS ). I thought those >> packets were dropped by backend check, sorry for the confusion. >> >> In netfront, following code would check whether required slots >> exceed MAX_SKB_FRAGS, and drop skbs which does not meet this >> requirement directly. >> >> if (unlikely(slots> MAX_SKB_FRAGS + 1)) { >> net_alert_ratelimited( >> "xennet: skb rides the rocket: %d slots\n", slots); >> goto drop; >> } >> >> In netback, following code also compared frags with MAX_SKB_FRAGS, >> and create error response for netfront which does not meet this >> requirment. In this case, netfront will also drop corresponding >> skbs. >> >> if (unlikely(frags>= MAX_SKB_FRAGS)) { >> netdev_dbg(vif->dev, "Too many frags\n"); >> return -frags; >> } >> >> So it is correct that netback log was not print out because those >> packets are drops directly by frontend check, not by backend check. >> Without the frontend check, it is likely that netback check would >> block these skbs and create error response for netfront. >> >> So two ways are available: workaround in netfront for those packets, >> doing re-fragment copying, but not sure how copying hurt >> performance. Another is to implement in netback, as discussed in > There is already some copying done (the copying of the socket data > from userspace to the kernel) - so the extra copy might not be that > bad as the data can be in the cache. This would probably be a way > to deal with old backends that cannot deal with this new feature-flag. I am thinking to do re-fragment in netfront for these skbs like following, Create a new skb, copy linear data and frag data from original skb into this one, and make every frags data size is PAGE_SIZE except for the last fragment. It is possible that the last fragment length is less than PAGE_SIZE, then free the original skb. The skb packet is large, and there will be lots of copys. struct skbuff *xennet_refrag_skb(skb) { create newskb copying data and doing fragmentation return newskb } ....... if (unlikely(slots> MAX_SKB_FRAGS + 1)) { net_alert_ratelimited( "xennet: skb rides the rocket: %d slots\n", slots); skb = xennet_refrag_skb(skb); } ..... Thanks Annie > >> "netchannel vs MAX_SKB_FRAGS". Maybe these two mechanism are all >> necessary? > Lets see first if this is indeed the problem. Perhaps a simple debug > patch that just does: > > s/MAX_SKB_FRAGS/DEBUG_MAX_FRAGS/ > #define DEBUG_MAX_FRAGS 21 > > in both netback and netfront to set the maximum number of frags we can > handle to 21? If that works with Sander test - then yes, it looks like > we really need to get this 'feature-max-skb-frags' done. >