From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id ABCDB1B349 for ; Fri, 19 Jan 2018 16:00:30 +0100 (CET) Received: by mail-wm0-f65.google.com with SMTP id b21so4128888wme.4 for ; Fri, 19 Jan 2018 07:00:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=TDRNvw1tXZAeawL5IHVjjguNu7nha5cwvuYS58Q4aZA=; b=iAfEtjcdp1XuHJwq2/kLmI5V90B7Z9cINGLZRXl6S4xC90B0Ch1rM5bhD9C8Ii8IdU kbOcVX+4ryju6/EPrN811wzICUxh19tueGL22e05llbal4xVAZM+IgQDMGB49b0sR7B0 1gyDfoDyGbvSqCrGOe9USfIqte2fICaBFNR41C/RqgjJvHGKBE1Dipc/dOrAnpqijp64 Dh75S5kjgcmKcJSaWA/8tnfGIKR1eLsTGb5htrr1VnM4e3yt+JNDqXR30P71Fzb8WxMe TAFcfRIttn8l5aUG0qMbtRYebiW0reNVatOhTJ4prO6JwCgb2EmJsL/3oWGJhCgsCEyZ M++A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=TDRNvw1tXZAeawL5IHVjjguNu7nha5cwvuYS58Q4aZA=; b=gp/qcQbOwCOxjrZDl03yxueEDYNkxXmi7roaSlt+ApNWxIvbZAttfWdVbxMzSsIaQu /Nx7SBMTGUmgCXzS5pH9DXID8Jd7MVDNgyI7+TxDtUkJVKKVu+S/UpIN+YoiOKXHJKS7 LzN+TSnOgiGLuNoueF+w0NOuVVAIMcgm/rITDkTI2+hdHfEoilW8lge4wQCJlTo+Bpzq sd73uFE5JFRRLjNC6Al3sjTawWdBbTNG3R8KpvmcW2i1l5C8m12dOpz3wZnJTzzaINQJ Clo+yVYdbXjihX7alpUfrbdb42CxjD8FvoPZrb8Ms0Poeu8iRT3bk2/8ajJsLV1YtxZv rfLw== X-Gm-Message-State: AKwxytdyvj0SjMNam7BzmCWGySq8tpvZ4VLx557T2O/zOrrHEnTvVaT3 50Vzlc3s2LrVosVDDV1l7MBrVCTw X-Google-Smtp-Source: ACJfBoscYavVlZxKFoh+BbSXijFRIWOyfin7OIKjxr+gVaK6/+aNClnfNzZi9sMWppZRZCcnqC19qw== X-Received: by 10.80.215.91 with SMTP id i27mr13494312edj.95.1516374030442; Fri, 19 Jan 2018 07:00:30 -0800 (PST) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id e25sm6754976edj.27.2018.01.19.07.00.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 19 Jan 2018 07:00:29 -0800 (PST) Date: Fri, 19 Jan 2018 16:00:17 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Matan Azrad Cc: "Ananyev, Konstantin" , Thomas Monjalon , "Wu, Jingjing" , "dev@dpdk.org" , Neil Horman , "Richardson, Bruce" Message-ID: <20180119150017.mljpcdmldqx32mkq@bidouze.vm.6wind.com> References: <1515318351-4756-1-git-send-email-matan@mellanox.com> <1516293317-30748-1-git-send-email-matan@mellanox.com> <1516293317-30748-8-git-send-email-matan@mellanox.com> <2601191342CEEE43887BDE71AB97725886280A68@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB97725886280AE8@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v3 7/7] app/testpmd: adjust ethdev port ownership X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Jan 2018 15:00:31 -0000 Hi Matan, On Fri, Jan 19, 2018 at 01:35:10PM +0000, Matan Azrad wrote: > Hi Konstantin > > From: Ananyev, Konstantin, Friday, January 19, 2018 3:09 PM > > > -----Original Message----- > > > From: Matan Azrad [mailto:matan@mellanox.com] > > > Sent: Friday, January 19, 2018 12:52 PM > > > To: Ananyev, Konstantin ; Thomas > > > Monjalon ; Gaetan Rivet > > ; > > > Wu, Jingjing > > > Cc: dev@dpdk.org; Neil Horman ; Richardson, > > > Bruce > > > Subject: RE: [PATCH v3 7/7] app/testpmd: adjust ethdev port ownership > > > > > > Hi Konstantin > > > > > > From: Ananyev, Konstantin, Friday, January 19, 2018 2:38 PM > > > > To: Matan Azrad ; Thomas Monjalon > > > > ; Gaetan Rivet ; > > Wu, > > > > Jingjing > > > > Cc: dev@dpdk.org; Neil Horman ; Richardson, > > > > Bruce > > > > Subject: RE: [PATCH v3 7/7] app/testpmd: adjust ethdev port > > > > ownership > > > > > > > > Hi Matan, > > > > > > > > > -----Original Message----- > > > > > From: Matan Azrad [mailto:matan@mellanox.com] > > > > > Sent: Thursday, January 18, 2018 4:35 PM > > > > > To: Thomas Monjalon ; Gaetan Rivet > > > > > ; Wu, Jingjing > > > > > Cc: dev@dpdk.org; Neil Horman ; > > Richardson, > > > > > Bruce ; Ananyev, Konstantin > > > > > > > > > > Subject: [PATCH v3 7/7] app/testpmd: adjust ethdev port ownership > > > > > > > > > > Testpmd should not use ethdev ports which are managed by other > > > > > DPDK entities. > > > > > > > > > > Set Testpmd ownership to each port which is not used by other > > > > > entity and prevent any usage of ethdev ports which are not owned by > > Testpmd. > > > > > > > > > > Signed-off-by: Matan Azrad > > > > > --- > > > > > app/test-pmd/cmdline.c | 89 +++++++++++++++++++----------------- > > ---- > > > > ----- > > > > > app/test-pmd/cmdline_flow.c | 2 +- > > > > > app/test-pmd/config.c | 37 ++++++++++--------- > > > > > app/test-pmd/parameters.c | 4 +- > > > > > app/test-pmd/testpmd.c | 63 ++++++++++++++++++++------------ > > > > > app/test-pmd/testpmd.h | 3 ++ > > > > > 6 files changed, 103 insertions(+), 95 deletions(-) > > > > > > > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > > > > > 31919ba..6199c64 100644 > > > > > --- a/app/test-pmd/cmdline.c > > > > > +++ b/app/test-pmd/cmdline.c > > > > > @@ -1394,7 +1394,7 @@ struct cmd_config_speed_all { > > > > > &link_speed) < 0) > > > > > return; > > > > > > > > > > - RTE_ETH_FOREACH_DEV(pid) { > > > > > + RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) { > > > > > > > > Why do we need all these changes? > > > > As I understand you changed definition of RTE_ETH_FOREACH_DEV(), so > > > > no testpmd should work ok default (no_owner case). > > > > Am I missing something here? > > > > > > Now, After Gaetan suggestion RTE_ETH_FOREACH_DEV(pid) will iterate > > over all valid and ownerless ports. > > > > Yes. > > > > > Here Testpmd wants to iterate over its owned ports. > > > > Why? Why it can't just iterate over all valid and ownerless ports? > > As I understand it would be enough to fix current problems and would allow > > us to avoid any changes in testmpd (which I think is a good thing). > > Yes, I understand that this big change is very daunted, But I think the current a lot of bugs in testpmd(regarding port ownership) even more daunted. > > Look, > Testpmd initiates some of its internal databases depends on specific port iteration, > In some time someone may take ownership of Testpmd ports and testpmd will continue to touch them. > If I look back on the fail-safe, its sole purpose is to have seamless hotplug with existing applications. Port ownership is a genericization of some functions introduced by the fail-safe, that could structure DPDK further. It should allow applications to have a seamless integration with subsystems using port ownership. Without this, port ownership cannot be used. Testpmd should be fixed, but follow the most common design patterns of DPDK applications. Going with port ownership seems like a paradigm shift. > In addition > Using the old iterator in some places in testpmd will cause a race for run-time new ports(can be created by failsafe or any hotplug code): > - testpmd finds an ownerless port(just now created) by the old iterator and start traffic there, > - failsafe takes ownership of this new port and start traffic there. > Problem! Testpmd does not handle detection of new port. If it did, testing fail-safe with it would be wrong. At startup, RTE_ETH_FOREACH_DEV already fixed the issue of registering DEFERRED ports. There are still remaining issues regarding this, but I think they should be fixed. The architecture does not need to be completely moved to port ownership. If anything, this should serve as a test for your API with common applications. I think you'd prefer to know and debug with testpmd instead of firing up VPP or something like that to determine what went wrong with using the fail-safe. > > In addition > As a good example for well-done application (free from ownership bugs) I tried here to adjust Tespmd to the new rules and BTW to fix a lot of bugs. Testpmd has too much cruft, it won't ever be a good example of a well-done application. If you want to demonstrate ownership, I think you should start an example application demonstrating race conditions and their mitigation. I think that would be interesting for many DPDK users. > > > So actually applications which are not aware to the port ownership still are exposed to races, but if there use the old iterator(with the new change) the amount of races decreases. > > Thanks, Matan. > > Konstantin > > > > > > > > I added to Testpmd ability to take an ownership of ports as the new > > > ownership and synchronization rules suggested, Since Tespmd is a DPDK > > > entity which wants that no one will touch its owned ports, It must allocate > > an unique ID, set owner for its ports (see in main function) and recognizes > > them by its owner ID. > > > > > > > Konstantin > > > > Regards, -- Gaëtan Rivet 6WIND