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 <rte_ethdev.h>
//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
>
