From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 3EA5719F5 for ; Mon, 18 Jun 2018 14:10:10 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jun 2018 05:10:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,239,1526367600"; d="scan'208";a="58525987" Received: from abaagwal-mobl1.gar.corp.intel.com (HELO [10.252.76.57]) ([10.252.76.57]) by fmsmga002.fm.intel.com with ESMTP; 18 Jun 2018 05:10:05 -0700 To: Jerin Jacob Cc: olivier.matz@6wind.com, hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com, abhinandan.gujjar@intel.com, gage.eads@intel.com, jia.guo@intel.com, cristian.dumitrescu@intel.com References: <1527260924-86922-1-git-send-email-nikhil.rao@intel.com> <1528839163-15048-1-git-send-email-nikhil.rao@intel.com> <20180617110953.GA3359@jerin> From: "Rao, Nikhil" Message-ID: Date: Mon, 18 Jun 2018 17:40:03 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180617110953.GA3359@jerin> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [RFC v2] eventdev: event tx adapter APIs 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: Mon, 18 Jun 2018 12:10:12 -0000 On 6/17/2018 4:39 PM, Jerin Jacob wrote: > -----Original Message----- >> Date: Wed, 13 Jun 2018 03:02:43 +0530 >> From: Nikhil Rao >> To: jerin.jacob@caviumnetworks.com, olivier.matz@6wind.com >> CC: hemant.agrawal@nxp.com, dev@dpdk.org, narender.vangati@intel.com, >> abhinandan.gujjar@intel.com, gage.eads@intel.com, jia.guo@intel.com, >> cristian.dumitrescu@intel.com, Nikhil Rao >> Subject: [RFC v2] eventdev: event tx adapter APIs >> X-Mailer: git-send-email 1.8.3.1 >> >> Add common APIs for the transmit stage of an event driven >> DPDK application. Also add a transmit queue field to the mbuf >> that is used by the adapter to transmit mbufs. >> >> Signed-off-by: Nikhil Rao >> --- >> >> Changelog >> ========= >> >> v1->v2: >> * Add the tx_adapter_enqueue function to struct rte_eventdev. >> It is set to the common Tx adapter function when creating the adapter >> if the eventdev PMD does not support it or if the >> DEV_TX_OFFLOAD_MT_LOCKFREE flag is NOT set on all ethernet devices. >> * Add the rte_event_eth_tx_adapter_enqueue() API. >> * Add the txq_id field to struct rte_mbuf. >> > Overall it looks good. Some comments inline. I think you can change it > from RFC and send the implementation. > >> lib/librte_eventdev/rte_event_eth_tx_adapter.h | 380 +++++++++++++++++++++++++ >> lib/librte_eventdev/rte_eventdev.h | 7 +- >> lib/librte_mbuf/rte_mbuf.h | 20 +- > Please split mbuf changes in a separate patch. > > >> 3 files changed, 405 insertions(+), 2 deletions(-) >> create mode 100644 lib/librte_eventdev/rte_event_eth_tx_adapter.h >> >> diff --git a/lib/librte_eventdev/rte_event_eth_tx_adapter.h b/lib/librte_eventdev/rte_event_eth_tx_adapter.h >> new file mode 100644 >> index 0000000..a0e8505 >> --- /dev/null >> +++ b/lib/librte_eventdev/rte_event_eth_tx_adapter.h >> @@ -0,0 +1,380 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2017 Intel Corporation. > 2018 > >> + */ >> + >> +#ifndef _RTE_EVENT_ETH_TX_ADAPTER_ >> +#define _RTE_EVENT_ETH_TX_ADAPTER_ >> + >> +/** >> + * @file >> + * >> + * RTE Event Ethernet Tx Adapter >> + * >> + * The event ethernet Tx adapter provides configuration and data path APIs >> + * for the transmit stage of an event driven packet processing application. >> + * These APIs abstract the implementation of the transmit stage and allow the > s/transmit stage/ethernet transmit stage/ > >> + * the application to use eventdev PMD support or a common implementation. >> + * >> + * In the common implementation, the application uses the adapter API to >> + * enqueue mbufs to the adapter which runs as a rte_service function. The >> + * service function deqeueues events from its event port and transmits the > s/deqeueues/dequeues > >> + * mbufs referenced by these events. >> + * >> + * The ethernet Tx event adapter APIs are: >> + * >> + * - rte_event_eth_tx_adapter_create() >> + * - rte_event_eth_tx_adapter_create_ext() >> + * - rte_event_eth_tx_adapter_free() >> + * - rte_event_eth_tx_adapter_start() >> + * - rte_event_eth_tx_adapter_stop() >> + * - rte_event_eth_tx_adapter_queue_start() >> + * - rte_event_eth_tx_adapter_queue_stop() >> + * - rte_event_eth_tx_adapter_enqueue() >> + * - rte_event_eth_tx_adapter_stats_get() >> + * - rte_event_eth_tx_adapter_stats_reset() >> + * >> + * The application creates the adapter using >> + * rte_event_eth_tx_adapter_create(). The adapter may internally create an event >> + * port using the port configuration parameter. >> + * The adapter is responsible for linking the queue as per its implementation, >> + * for example in the case of the service function, the adapter links this queue >> + * to the event port it will dequeue events from. >> + * >> + * The application uses rte_event_eth_tx_adapter_enqueue() to send mbufs to the >> + * adaptervia this event queue. The ethernet port and transmit queue index to > s/adaptervia/adapter via > >> + * transmit the mbuf on are specified in the mbuf. >> + * >> + * The application can start and stop the adapter using the >> + * rte_event_eth_tx_adapter_start/stop() calls. >> + * >> + * To support dynamic reconfiguration of Tx queues, the application can >> + * call rte_event_eth_tx_adapter_queue_start()/stop() to synchronize >> + * access to the Tx queue with the adapter. For example, if the application >> + * wants to reconfigure a Tx queue that could be concurrently >> + * being accessed by the adapter, it calls rte_event_eth_tx_adapter_queue_stop() >> + * first, reconfigures the queue and then calls >> + * rte_event_eth_tx_adapter_queue_start() which signals to the adapter >> + * that it is safe to resume access to the Tx queue. >> + * >> + * The common adapter implementation uses an EAL service function as described >> + * before and its execution is controlled using the rte_service APIs. The >> + * rte_event_eth_tx_adapter_service_id_get() >> + * function can be used to retrieve the adapter's service function ID. >> + */ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +#include >> + >> +#include "rte_eventdev.h" >> + >> +#define RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32 >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Adapter configuration structure >> + */ >> +struct rte_event_eth_tx_adapter_conf { >> + uint8_t event_port_id; >> + /**< Event port identifier, the adapter service function dequeues mbuf >> + * events from this port. >> + */ >> + uint32_t max_nb_tx; >> + /**< The adapter can return early if it has processed at least >> + * max_nb_tx mbufs. This isn't treated as a requirement; batching may >> + * cause the adapter to process more than max_nb_tx mbufs. >> + */ >> +}; >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Function type used for adapter configuration callback. The callback is >> + * used to fill in members of the struct rte_event_eth_tx_adapter_conf, this >> + * callback is invoked when creating a SW service to transmit packets. >> + * >> + * @param id >> + * Adapter identifier. >> + * @param dev_id >> + * Event device identifier. >> + * @param [out] conf >> + * Structure that needs to be populated by this callback. >> + * @param arg >> + * Argument to the callback. This is the same as the conf_arg passed to the >> + * rte_event_eth_tx_adapter_create_ext(). >> + * >> + * @return >> + * - 0: Success >> + * - <0: Error code on failure >> + */ >> +typedef int (*rte_event_eth_tx_adapter_conf_cb) (uint8_t id, uint8_t dev_id, >> + struct rte_event_eth_tx_adapter_conf *conf, >> + void *arg); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * A structure used to retrieve statistics for an eth tx adapter instance. >> + */ >> +struct rte_event_eth_tx_adapter_stats { >> + uint64_t event_poll_count; >> + /*< Event port poll count */ >> + uint64_t tx_packets; >> + /*< Number of packets transmitted */ >> + uint64_t tx_dropped; >> + /*< Number of packets dropped */ >> +}; >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Create a new event ethernet Tx adapter with the specified identifier. >> + * >> + * @param id >> + * The identifier of the event ethernet Tx adapter. >> + * @param dev_id >> + * The event device identifier. >> + * @param queue_id >> + * The event queue identifier. >> + * @param port_config >> + * Event port configuration, the adapter uses this configuration to >> + * create an event port if needed. It uses this port to dequeue >> + * events that are sent to it by rte_event_eth_tx_adapter_enqueue() >> + * >> + * @return >> + * - 0: Success >> + * - <0: Error code on failure >> + */ > __rte_experimental missing in all the functions. > >> +int rte_event_eth_tx_adapter_create(uint8_t id, uint8_t dev_id, >> + uint8_t queue_id, >> + struct rte_event_port_conf *port_config); >> + > > # Missing the rte_event_eth_tx_adapter_create_ext() prototype? > > #I assume you will have rte_event_eth_tx_adapter_caps_get() with the > RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT capability Yes. >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Free an event adapter >> + * >> + * @param id >> + * Adapter identifier. >> + * @return >> + * - 0: Success >> + * - <0: Error code on failure, If the adapter still has Tx queues >> + * added to it, the function returns -EBUSY. >> + */ >> +int rte_event_eth_tx_adapter_free(uint8_t id); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Start ethernet Tx event adapter >> + * >> + * @param id >> + * Adapter identifier. >> + * @return >> + * - 0: Success, Adapter started correctly. >> + * - <0: Error code on failure. >> + */ >> +int rte_event_eth_tx_adapter_start(uint8_t id); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Stop ethernet Tx event adapter >> + * >> + * @param id >> + * Adapter identifier. >> + * @return >> + * - 0: Success, Adapter started correctly. >> + * - <0: Error code on failure. >> + */ >> +int rte_event_eth_tx_adapter_stop(uint8_t id); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Signal the Tx adapter to start processing mbufs for a >> + * Tx queue. A queue value of -1 is used to indicate all >> + * queues within the device. >> + * >> + * @param id >> + * Adapter identifier. >> + * @param eth_dev_id >> + * Ethernet Port Identifier. >> + * @param queue >> + * Tx queue index. >> + * >> + * @return >> + * - 0: Success, Adapter started correctly. >> + * - <0: Error code on failure. >> + */ >> +int rte_event_eth_tx_adapter_queue_start(uint8_t id, >> + uint16_t eth_dev_id, >> + int32_t queue); >> + > How about changing to to rte_event_eth_tx_adapter_queue_add to inline > with Rx adapter? OK. >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Signal the Tx adapter to stop processing mbufs for a >> + * Tx queue. A queue value of -1 is used to indicate all >> + * queues within the device. >> + * >> + * @param id >> + * Adapter identifier. >> + * @param eth_dev_id >> + * Ethernet Port Identifier. >> + * @param queue >> + * Tx queue index. >> + * >> + * @return >> + * - 0: Success, Adapter started correctly. >> + * - <0: Error code on failure. >> + */ >> +int rte_event_eth_tx_adapter_queue_stop(uint8_t id, >> + uint16_t eth_dev_id, >> + int32_t queue); >> + I think the queue_del() function  should have to take id. >> +static __rte_always_inline uint16_t >> +__rte_event_eth_tx_adapter_enqueue(uint8_t id, uint8_t dev_id, uint8_t port_id, >> + const struct rte_event ev[], >> + uint16_t nb_events, >> + const event_tx_adapter_enqueue fn) >> +{ >> + const struct rte_eventdev *dev = &rte_eventdevs[dev_id]; >> + >> +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG >> + if (id >= RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE || >> + dev_id >= RTE_EVENT_MAX_DEVS || >> + !rte_eventdevs[dev_id].attached) { >> + rte_errno = -EINVAL; >> + return 0; >> + } >> + >> + if (port_id >= dev->data->nb_ports) { >> + rte_errno = -EINVAL; >> + return 0; >> + } >> +#endif >> + /* >> + * TODO: Do we need a special case for nb_events == 1 >> + */ > I think, we don't need the special case for nb_events == 1 > >> + return fn(id, dev, dev->data->ports[port_id], ev, nb_events); >> +} >> + >> +/** >> + * Enqueue a burst of events objects or an event object supplied in *rte_event* >> + * structure on an event device designated by its *dev_id* through the event >> + * port specified by *port_id*. The the event queue on which it will be enqueued >> + * id derived from the adapter id parameter. >> + * >> + * The *nb_events* parameter is the number of event objects to enqueue which are >> + * supplied in the *ev* array of *rte_event* structure. >> + * >> + * The rte_event_eth_tx_adapter_enqueue() function returns the number of >> + * events objects it actually enqueued. A return value equal to *nb_events* >> + * means that all event objects have been enqueued. >> + * >> + * @param id >> + * The identifier of the tx adapter. >> + * @param dev_id >> + * The identifier of the device. >> + * @param port_id >> + * The identifier of the event port. >> + * @param ev >> + * Points to an array of *nb_events* objects of type *rte_event* structure >> + * which contain the event object enqueue operations to be processed. >> + * @param nb_events >> + * The number of event objects to enqueue, typically number of >> + * rte_event_port_enqueue_depth() available for this port. >> + * >> + * @return >> + * The number of event objects actually enqueued on the event device. The >> + * return value can be less than the value of the *nb_events* parameter when >> + * the event devices queue is full or if invalid parameters are specified in a >> + * *rte_event*. If the return value is less than *nb_events*, the remaining >> + * events at the end of ev[] are not consumed and the caller has to take care >> + * of them, and rte_errno is set accordingly. Possible errno values include: >> + * - -EINVAL The port ID is invalid, device ID is invalid, an event's queue >> + * ID is invalid, or an event's sched type doesn't match the >> + * capabilities of the destination queue. >> + * - -ENOSPC The event port was backpressured and unable to enqueue >> + * one or more events. This error code is only applicable to >> + * closed systems. >> + */ >> +static inline uint16_t >> +rte_event_eth_tx_adapter_enqueue(uint8_t id, uint8_t dev_id, >> + uint8_t port_id, >> + const struct rte_event ev[], >> + uint16_t nb_events) >> +{ >> + const struct rte_eventdev *dev = &rte_eventdevs[dev_id]; >> + >> + return __rte_event_eth_tx_adapter_enqueue(id, dev_id, port_id, ev, >> + nb_events, >> + dev->tx_adapter_enqueue); >> +} >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Retrieve statistics for an adapter >> + * >> + * @param id >> + * Adapter identifier. >> + * @param [out] stats >> + * A pointer to structure used to retrieve statistics for an adapter. >> + * @return >> + * - 0: Success, retrieved successfully. >> + * - <0: Error code on failure. >> + */ >> +int rte_event_eth_tx_adapter_stats_get(uint8_t id, >> + struct rte_event_eth_tx_adapter_stats *stats); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Reset statistics for an adapter. >> + * >> + * @param id >> + * Adapter identifier. >> + * @return >> + * - 0: Success, statistics reset successfully. >> + * - <0: Error code on failure. >> + */ >> +int rte_event_eth_tx_adapter_stats_reset(uint8_t id); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Retrieve the service ID of an adapter. If the adapter doesn't use >> + * a rte_service function, this function returns -ESRCH. >> + * >> + * @param id >> + * Adapter identifier. >> + * @param [out] service_id >> + * A pointer to a uint32_t, to be filled in with the service id. >> + * @return >> + * - 0: Success >> + * - <0: Error code on failure, if the adapter doesn't use a rte_service >> + * function, this function returns -ESRCH. >> + */ >> +int rte_event_eth_tx_adapter_service_id_get(uint8_t id, uint32_t *service_id); >> + >> +#ifdef __cplusplus >> +} >> +#endif >> +#endif /* _RTE_EVENT_ETH_TX_ADAPTER_ */ >> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h >> index b6fd6ee..1bf28a0 100644 >> --- a/lib/librte_eventdev/rte_eventdev.h >> +++ b/lib/librte_eventdev/rte_eventdev.h >> @@ -1203,6 +1203,10 @@ typedef uint16_t (*event_dequeue_t)(void *port, struct rte_event *ev, >> typedef uint16_t (*event_dequeue_burst_t)(void *port, struct rte_event ev[], >> uint16_t nb_events, uint64_t timeout_ticks); >> /**< @internal Dequeue burst of events from port of a device */ >> +typedef uint16_t (*event_tx_adapter_enqueue)(uint8_t id, >> + const struct rte_eventdev *dev, void *port, >> + const struct rte_event ev[], uint16_t nb_events); >> +/**< @internal Enqueue burst of events on port of a device */ >> >> #define RTE_EVENTDEV_NAME_MAX_LEN (64) >> /**< @internal Max length of name of event PMD */ >> @@ -1266,7 +1270,8 @@ struct rte_eventdev { >> /**< Pointer to PMD dequeue function. */ >> event_dequeue_burst_t dequeue_burst; >> /**< Pointer to PMD dequeue burst function. */ >> - >> + event_tx_adapter_enqueue tx_adapter_enqueue; >> + /**< Pointer to PMD tx adapter enqueue function. */ >> struct rte_eventdev_data *data; >> /**< Pointer to device data */ >> struct rte_eventdev_ops *dev_ops; >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h >> index 4fd9a0d..c59389c 100644 >> --- a/lib/librte_mbuf/rte_mbuf.h >> +++ b/lib/librte_mbuf/rte_mbuf.h >> @@ -464,7 +464,9 @@ struct rte_mbuf { >> }; >> uint16_t nb_segs; /**< Number of segments. */ >> >> - /** Input port (16 bits to support more than 256 virtual ports). */ >> + /** Input port (16 bits to support more than 256 virtual ports). >> + * The event eth Tx adapter uses this field to specify the output port. > See rte_event_eth_tx_adapter_enqueue() > >> + */ >> uint16_t port; >> >> uint64_t ol_flags; /**< Offload features. */ >> @@ -511,6 +513,7 @@ struct rte_mbuf { >> /** VLAN TCI (CPU order), valid if PKT_RX_VLAN is set. */ >> uint16_t vlan_tci; >> >> + RTE_STD_C11 >> union { >> uint32_t rss; /**< RSS hash result if RSS enabled */ >> struct { >> @@ -531,6 +534,11 @@ struct rte_mbuf { >> uint32_t lo; >> uint32_t hi; >> } sched; /**< Hierarchical scheduler */ >> + struct { >> + uint32_t resvd1; /* overlaps with rte_sched_port_hierarchy::color */ > s/overlaps/Overlaps/ > >> + uint16_t resvd2; >> + uint16_t txq_id; > /* The event eth Tx adapter transmit queue. See rte_event_eth_tx_adapter_enqueue() */ > >> + }; >> uint32_t usr; /**< User defined tags. See rte_distributor_process() */ >> } hash; /**< hash information */ >> >> @@ -1880,6 +1888,16 @@ static inline struct rte_mbuf *rte_pktmbuf_lastseg(struct rte_mbuf *m) >> #define rte_pktmbuf_data_len(m) ((m)->data_len) >> >> /** >> + * A macro that returns the txq field of the mbuf >> + * >> + * The value can be read or assigned. >> + * >> + * @param m >> + * The packet mbuf. >> + */ >> +#define rte_pktmbuf_tx_queue(m) ((m)->txq_id) > Should we change to rte_pktmbuf_tx_adapter_txq to make it explicit that > it is related to Tx adapter? I am fine with the suggestion. Thanks for the review, Nikhil