From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 5A2E62C8 for ; Thu, 29 Jun 2017 18:13:12 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga104.jf.intel.com with ESMTP; 29 Jun 2017 09:13:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,281,1496127600"; d="scan'208";a="102807946" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.91]) ([10.237.220.91]) by orsmga004.jf.intel.com with ESMTP; 29 Jun 2017 09:13:10 -0700 To: Bruce Richardson Cc: dev@dpdk.org, anatoly.burakov@intel.com References: <20170526165228.96919-1-ferruh.yigit@intel.com> <20170621110651.75299-1-ferruh.yigit@intel.com> <20170626113909.GD102672@bricha3-MOBL3.ger.corp.intel.com> From: Ferruh Yigit Message-ID: <58c4e676-a9c6-d5d0-c462-f4f96f7182cb@intel.com> Date: Thu, 29 Jun 2017 17:13:10 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170626113909.GD102672@bricha3-MOBL3.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 0/4] Userspace Network Control Interface (UNCI) 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: Thu, 29 Jun 2017 16:13:13 -0000 On 6/26/2017 12:39 PM, Bruce Richardson wrote: > On Wed, Jun 21, 2017 at 12:06:47PM +0100, Ferruh Yigit wrote: >> Userspace Network Control Interface (UNCI), (formerly KCP). >> >> When a NIC bound to the DPDK, it can't be controlled by Linux tools. >> >> This patch creates a virtual network interface for each DPDK port, >> initial target is to get some data from those interfaces, in next >> step target is to control DPDK ports using virtual interfaces. >> > > I've tried out this set to see how it goes, and apart from a few > compile/link errors with individual patches which I've flagged, I think > the result is great. Running testpmd and seeing the dpdk interfaces > listed in "ifconfig" for example, is a great improvement for usability. > Let's hope we can get this into DPDK as soon as possible. Thanks for trying this and reviews. > > Some thoughts about the patchset: > > 1. Do we really need to use a whole new library for the ethtool > functions? Can the UNCI userspace code not just call into ethdev > directly without going through another library? We don't have to, but ethtool library in sample folder is wrapper around ethdev APIs for ethtool functions, that fits well here, and we can re-use that code. > > 2. Right now the interfaces are only created on application start, but > are not removed on app close. My first thought is that we should correct > his imbalance, but perhaps it might be better to always have the > interfaces displayed even before/after an app has run. We obviously need > some way of detecting if a process is controlling the ports or not, but > I'm hopeful that should be doable without too much work. Yes, currently interfaces should be removed manually via "ip", but this is because PMD destroy code paths not called during app exit. I am not sure keeping virtual interfaces always, it looks me better if they disappear when app exits. > > 3. From a patchset split point of view, could this set be split up to be > a bit more granular. There are a lot of functions to be performed on > NICs called out in the code, e.g. start/stop, get stats, etc. etc. To > make review easier, should we initially add the kernel module and > userspace parts with just one function supported, and then add in each > additional function in a new patchset, so that we can clearly see the > code for each function isolated from the rest. This is the approach - > adding feature by feature - that is recommended for NIC drivers, and it > might make sense here too. Let me try to split patches more. > > Otherwise, great work. I think this is a huge improvement in usability > for DPDK, especially if we add in future support for controlling DPDK > interfaces in a (not interfering with the app) safe manner too. > > /Bruce >