From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx.bisdn.de (mx.bisdn.de [185.27.182.31]) by dpdk.org (Postfix) with ESMTP id A3DB35A40 for ; Fri, 17 Apr 2015 20:49:46 +0200 (CEST) Received: from [192.168.1.102] (x5ce235ba.dyn.telefonica.de [92.226.53.186]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx.bisdn.de (Postfix) with ESMTPSA id 2FC00A3B4A for ; Fri, 17 Apr 2015 20:49:46 +0200 (CEST) Message-ID: <553155C7.3000106@bisdn.de> Date: Fri, 17 Apr 2015 20:49:43 +0200 From: Marc Sune User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: dev@dpdk.org References: <1428954274-26944-1-git-send-email-keith.wiles@intel.com> <1429283804-28087-1-git-send-email-bruce.richardson@intel.com> In-Reply-To: <1429283804-28087-1-git-send-email-bruce.richardson@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [RFC PATCH 0/4] pktdev 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: Fri, 17 Apr 2015 18:49:46 -0000 On 17/04/15 17:16, Bruce Richardson wrote: > Hi all, > > to continue this discussion a bit more, here is my, slightly different, slant > on what a pktdev abstraction may look like. > > The primary objective I had in mind when drafting this is to provide the > minimal abstraction that can be *easily* used as a common device abstraction for > existing (and future) device types to be passed to dataplane code. The patchset > demonstrates this by defining a minimal interface for pktdev - since I firmly > believe the interface should be as small as possible - and then showing how that > common interface can be used to unify rings and ethdevs under a common API for the > datapath. I believe any attempt to unify things much beyond this to the control > plane or setup phase is not worth doing - at least not initially - as at > init time the code always needs to be aware of the underlying resource type in > order to configure it properly for dataplane use. > > The overall objective I look to achieve is illustrated by the final patch in > the series, which is a sample app where the same code is used for all cores, > irrespective of the underlying device type. > > To get to that point, patch 1 defines the minimal API - just RX and TX. The .c > file in the library is empty for simplicity, though I would see some > functionality moving there when/if it makes sense e.g. the callback support > from ethdev, as is done in Keith's patchset. > Patch 2 then makes very minimal changes to ethdev to allow ethdevs to be used > as pktdevs, and to make use of the pktdev functions when appropriate > Patch 3 was, for me, the key test for this implementation - how hard was it to > make an rte_ring usable as a pktdev too. Two single-line functions for RX/TX > and a separate "converter" function proved to be all that was necessary here - > and I believe simpler solutions may be possible too, as the extra structures > allocated on conversion could be merged into the rte_ring structure itself and > initialized on ring creation if we prefer that option. It is hoped/presumed that > wrapping other structures, such as KNI, may prove to be just as easily done. > [Not attempted yet - left as an exercise for the reader :-)]. > > Now, in terms of pktdev vs ethdev, there is nothing in this proposal that > cannot also be done using ethdev AFAIK. However, pktdev as outlined here > should make the process far easier than trying to create a full PMD for something. > All NIC specific functions, including things like stop/start, are stripped out, > as they don't make sense for an rte_ring or other software objects. > Also, the other thing this provides is that we can move away from just using > port ids. Instead in the same way as we now reference rings/mempools/KNIs etc > via pointer, we can do the same with ethernet ports as pktdevs on the data path. > There was discussion previously on moving beyond 8-bit port ids. If we look to > use ethdev as a common abstraction, I feel that change will soon have to be made > causing a large amount of code churn. Hi Richard, First thank you both for taking the time to look at this. I did not not reply to Keith because you Richard summarized most of my concerns. I had a brief look to this second proposal. It is more aligned to what I had in mind. But still I feel it is slightly too complicated. I don't like much the necessary (in your approach) MACRO-like pkt_dev_data struct. It is also slightly inconvenient that the user has to do: + struct rte_pkt_dev *in = rte_eth_get_dev(0); + struct rte_pkt_dev *out = rte_ring_get_dev( + rte_ring_create(name, 4096, rte_socket_id(), 0)); What about something like (~pseudo-code): rte_pkt_dev_data.h: enum rte_pkt_dev_type{ RTE_PKT_DEV_ETH, RTE_PKT_DEV_RING, RTE_PKT_DEV_KNI, //Keep adding as more PMDs are supported }; //This struct may be redundant if there is nothing more struct rte_pkt_dev_data{ enum rte_pkt_dev_type; //Placeholder, maybe we need more... }; //Make RX/TX pktdev APIs more readable, but not really needed typedef void pkt_dev_t; (In all PMDs and e.g. KNI and RINGs): struct rte_eth_dev { struct rte_pkt_dev_data pkt_dev;// <++++++++++++++++++++++++++++++++++ eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */ eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */ struct rte_eth_dev_data *data; /**< Pointer to device data */ /... rte_pkt_dev.h: #include //Include PMD (and non-PMD) TX/RX headers... static inline uint16_t rte_pkt_tx_burst(pkt_dev_t* dev, uint16_t queue_id, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) { switch (((struct rte_pkt_dev_data*)dev)->type){ case RTE_PKT_DEV_ETH: struct rte_eth_dev* eth_dev = (struct rte_eth_dev*)pkt_dev; rte_pkt_tx_burst(eth_dev, queue_id, tx_pkts, nb_pkts); break; case RTE_PKT_DEV_RING: //... } } //... Maybe it is oversimplified? With this approach tough, we would barely touch the PMDs and the functionality can be built on top of the existing PMDs. Thoughts? Marc > > Bruce Richardson (4): > Add example pktdev implementation > Make ethdev explicitly a subclass of pktdev > add support for a ring to be a pktdev > example app showing pktdevs used in a chain > > config/common_bsdapp | 5 + > config/common_linuxapp | 5 + > examples/pktdev/Makefile | 57 +++++++++++ > examples/pktdev/basicfwd.c | 222 +++++++++++++++++++++++++++++++++++++++++ > lib/Makefile | 1 + > lib/librte_ether/rte_ethdev.h | 26 ++--- > lib/librte_pktdev/Makefile | 56 +++++++++++ > lib/librte_pktdev/rte_pktdev.c | 35 +++++++ > lib/librte_pktdev/rte_pktdev.h | 144 ++++++++++++++++++++++++++ > lib/librte_ring/rte_ring.c | 41 ++++++++ > lib/librte_ring/rte_ring.h | 3 + > 11 files changed, 582 insertions(+), 13 deletions(-) > create mode 100644 examples/pktdev/Makefile > create mode 100644 examples/pktdev/basicfwd.c > create mode 100644 lib/librte_pktdev/Makefile > create mode 100644 lib/librte_pktdev/rte_pktdev.c > create mode 100644 lib/librte_pktdev/rte_pktdev.h >