From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 4C29EB4B6 for ; Wed, 18 Feb 2015 14:49:13 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 18 Feb 2015 05:49:11 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,602,1418112000"; d="scan'208";a="668074679" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.37]) by fmsmga001.fm.intel.com with SMTP; 18 Feb 2015 05:49:09 -0800 Received: by (sSMTP sendmail emulation); Wed, 18 Feb 2015 13:49:08 +0025 Date: Wed, 18 Feb 2015 13:49:08 +0000 From: Bruce Richardson To: Marc Sune Message-ID: <20150218134908.GD5960@bricha3-MOBL3> References: <1424257369-19130-1-git-send-email-mukawa@igel.co.jp> <20150218123006.GB5960@bricha3-MOBL3> <20150218123159.GC5960@bricha3-MOBL3> <54E48F58.6080909@bisdn.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54E48F58.6080909@bisdn.de> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [RFC PATCH] lib/librte_ethdev: Expand port identifier 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: Wed, 18 Feb 2015 13:49:13 -0000 On Wed, Feb 18, 2015 at 02:10:48PM +0100, Marc Sune wrote: > > On 18/02/15 13:31, Bruce Richardson wrote: > >On Wed, Feb 18, 2015 at 12:30:07PM +0000, Bruce Richardson wrote: > >>On Wed, Feb 18, 2015 at 08:02:49PM +0900, Tetsuya Mukawa wrote: > >>>Currently uint8_t is used for port identifier. This patch changes it, > >>>and use uint16_t as port identifier. > >>>This patch only changes ethdev library. ABI of the library will be > >>>kept even after applying it. > >>> > >>>Also, this patch involves following fixes. > >>>- Use "port_id" as variable name instead of "port". > >>> > >>> > >>>Signed-off-by: Tetsuya Mukawa > >>>--- > >>> lib/librte_ether/rte_ethdev.c | 212 +- > >>> lib/librte_ether/rte_ethdev_internal.h | 3672 ++++++++++++++++++++++++++++++++ > >>> 2 files changed, 3778 insertions(+), 106 deletions(-) > >>> create mode 100644 lib/librte_ether/rte_ethdev_internal.h > >>> > >>I'm not sure I follow why we need a new header file for this. > >>Also, thinking about this change, a more fundamental problem is going to be > >>the mbuf structure, which stores a port id inside it in an 8-bit value. > >>Upgrading that to a 16-bit value requires some thought, and verification to > >>ensure any adjustment of fields does not lead to serious performance issues. > >> > >>Therefore, I suggest we leave the port id values as 8-bits until such time > >>as we need greater than 255 port values in a real-world use case. > >>Out of interest - anyone have a DPDK app where they use >16 port id values? If > >>so, how high does the port id value get? > > Just a though on port_id in general; I wouldn't see why other type of ports > could fall into the same abstraction of using port_ids as we do for PHY > ports, if eventually we could create a unified API TX/RX routines he same > regardless of the port (I know KNI deprecated this approach in the past). Of > course initialization routines should be different for each type of port. > > I see quite a bit of code duplicity, basically in TX/RX routines for PHY > ports, KNI ports, SHMEM (ring) ports like ivshmem etc.., which are very > similar, and we put into the shoulders of all users of DPDK to have to do > the "switch() - case" based on the type of port (which is state that they > have to store themselves too). This seems to me it could be improved from a > DPDK user's point of view. > > By no means I am saying lower level APIs should not be exposed (current > APIs)... There is the need to, since users using one type of ports only > should be able to by-pass that (small?) extra overhead of this higher level > APIs. > > If the implementation would eventually would go into this direction, there > would be more pressure in the port_id identifier; e.g. KNI interfaces and > other SW like interfaces can be created and destroyed quite frequently (e.g. > VMs), so more than 8 bits for addressing would probably be needed. > > I know it not helping in the short-term, but let's see if someone thinks > this makes any sense at all. > > Marc > Yes, this makes complete sense, Marc, and it's something I (and some others here in Intel ) have certainly been thinking about - and go on thinking about. The ability to have multiple objects of different types accessible under a generic rx_burst/tx_burst interface is a very powerful one. [I actually tried something a bit like this before, with patches to allow a form of type-casting from rings to ethdevs, but it didn't go further as it was felt to overlap too much with the rings pmd. http://dpdk.org/ml/archives/dev/2014-May/002505.html ]. Overall, at this point we believe that the ethdev itself is a bit of overkill for a common API, since it's got a lot of NIC specific baggage in its APIs. We are thinking that perhaps a "higher-level", more minimal abstraction, which just has rx_burst or tx_burst functions and not a lot else, might be more useful, as it can then be "sub-classed" [to use object-oriented terminology] into device-type specific versions such as ethdev. At this stage, we're just thinking about what such a thing might look like, and trying a few things out to see if such an idea is workable. If we think it's something worth pursuing, we hope to have something to share very soon, to get input from the whole community to see if it's worth doing for future releases. However, it appears your thinking closely aligns with what we are thinking, which is good to see! Regards, /Bruce > >> > >>Regards, > >>/Bruce > >> > >Resending with correct email addr for Neil. > > > >/Bruce >