From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: xennet: skb rides the rocket: 20 slots Date: Wed, 9 Jan 2013 10:08:50 -0500 Message-ID: <20130109150850.GI18395@phenom.dumpdata.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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <50ED1800.1080208@oracle.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: ANNIE LI Cc: Sander Eikelenboom , Ian Campbell , xen-devel List-Id: xen-devel@lists.xenproject.org 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. > "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.