From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 3A686C75C for ; Thu, 16 Jun 2016 17:00:45 +0200 (CEST) Received: by mail-wm0-f42.google.com with SMTP id k184so32680958wme.1 for ; Thu, 16 Jun 2016 08:00:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=clrWiODkmQQaBxRLZIGUGNf4RQftYGoFzg2kgk5oJ1s=; b=jql8X6+EuMhynzru1baZCegsEKU0x2a0NWjXK6LIb2LrNiPlDBOnjo2NShJh3omCaH 8aUXgv/IVDEnloqMSXdfP2ekExP0xGgTqzk4F+OfzymU39FGtgxTkSVORqG4h/oSV+yT Fcs/k4BDafbaScIFzfAslZwPTJLBFNsTC1O1/adc1VcKCI6rBVH4MSSEwH4siDND/0sz CS+oe68a4Q/ft42xwBtLf+bqahFHio66dPpDgmyDH4e/94uA6XjLBCeDa8heGi3o66lU gk+shV5veHVwZV2X+U68mD9D4K2lYHZ/vVmE8d0rZoLBNo95VG8UW7BvTCDKW7rfYEdv UjvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=clrWiODkmQQaBxRLZIGUGNf4RQftYGoFzg2kgk5oJ1s=; b=Kb1DQHpL+OgYIpY/gRkAfVGeZ6joqESG1moFJmbXu+W5GRmcDes+1PzcaLI94/Aedl gIs4PzNCjYltenBsH+mtMpgLuOhso5MX1F9BlAMVTylPgmVzyfLOgTdnE95TYj+pNhO9 AniMYfbfPseGyCsT8tXEu8ORk+2kyNWSvMEXFnAQC+L2DTxQsJfnhDM1F17AB0q3QIbb V7S9Ff0eukrffIgHQGSF+f1eAALE61xk9NcTErjwoGykcUJZc4YCXDd3O2jlpe28GHlb pLbmaewQ+kTt4RO8041D6DN80dUfM/6OD17ubITc60WpyxA47mzciEJMNDUBMt2AS7Wt jLnw== X-Gm-Message-State: ALyK8tJWlMcuzk+wl6sm5m27ITD33g7O17vfyDbBo+alMCPgJRBrjPyHkqPmiQ0sHITXuR4B X-Received: by 10.28.168.7 with SMTP id r7mr5627972wme.9.1466089244975; Thu, 16 Jun 2016 08:00:44 -0700 (PDT) Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184]) by smtp.gmail.com with ESMTPSA id zg10sm19701779wjb.1.2016.06.16.08.00.44 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 16 Jun 2016 08:00:44 -0700 (PDT) From: Thomas Monjalon To: Bruce Richardson , "Iremonger, Bernard" Cc: dev@dpdk.org, "Doherty, Declan" , "Ananyev, Konstantin" Date: Thu, 16 Jun 2016 17:00:43 +0200 Message-ID: <5839452.IzJ3v2K0cK@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <20160616143210.GB14984@bricha3-MOBL3> References: <1464280727-25752-2-git-send-email-bernard.iremonger@intel.com> <8CEF83825BEC744B83065625E567D7C21A038B2E@IRSMSX108.ger.corp.intel.com> <20160616143210.GB14984@bricha3-MOBL3> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 3/4] bonding: take queue spinlock in rx/tx burst functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Jun 2016 15:00:45 -0000 2016-06-16 15:32, Bruce Richardson: > On Mon, Jun 13, 2016 at 01:28:08PM +0100, Iremonger, Bernard wrote: > > > Why does this particular PMD need spinlocks when doing RX and TX, while > > > other device types do not? How is adding/removing devices from a bonded > > > device different to other control operations that can be done on physical > > > PMDs? Is this not similar to say bringing down or hotplugging out a physical > > > port just before an RX or TX operation takes place? > > > For all other PMDs we rely on the app to synchronise control and data plane > > > operation - why not here? > > > > > > /Bruce > > > > This issue arose during VM live migration testing. > > For VM live migration it is necessary (while traffic is running) to be able to remove a bonded slave device, stop it, close it and detach it. > > It a slave device is removed from a bonded device while traffic is running a segmentation fault may occur in the rx/tx burst function. The spinlock has been added to prevent this occurring. > > > > The bonding device already uses a spinlock to synchronise between the add and remove functionality and the slave_link_status_change_monitor code. > > > > Previously testpmd did not allow, stop, close or detach of PMD while traffic was running. Testpmd has been modified with the following patchset > > > > http://dpdk.org/dev/patchwork/patch/13472/ > > > > It now allows stop, close and detach of a PMD provided in it is not forwarding and is not a slave of bonded PMD. > > > I will admit to not being fully convinced, but if nobody else has any serious > objections, and since this patch has been reviewed and acked, I'm ok to merge it > in. I'll do so shortly. Please hold on. Seeing locks introduced in the Rx/Tx path is an alert. We clearly need a design document to explain where locks can be used and what are the responsibility of the control plane. If everybody agrees in this document that DPDK can have some locks in the fast path, then OK to merge it. So I would say NACK for 16.07 and maybe postpone to 16.11.