On 9/15/2017 11:37 AM, Nipun Gupta wrote:
-----Original Message-----
From: Nikhil Rao [mailto:nikhil....@intel.com]
<snip>
+static inline void
+fill_event_buffer(struct rte_event_eth_rx_adapter *rx_adapter,
+ uint8_t dev_id,
+ uint16_t rx_queue_id,
+ struct rte_mbuf **mbufs,
+ uint16_t num)
+{
+ uint32_t i;
+ struct eth_device_info *eth_device_info =
+ &rx_adapter->eth_devices[dev_id];
+ struct eth_rx_queue_info *eth_rx_queue_info =
+ ð_device_info-
rx_queue[rx_queue_id];
+
+ int32_t qid = eth_rx_queue_info->event_queue_id;
+ uint8_t sched_type = eth_rx_queue_info->sched_type;
+ uint8_t priority = eth_rx_queue_info->priority;
+ uint32_t flow_id;
+ struct rte_event events[BATCH_SIZE];
+ struct rte_mbuf *m = mbufs[0];
+ uint32_t rss_mask;
+ uint32_t rss;
+ int do_rss;
+
+ /* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
+ rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
+ do_rss = !rss_mask && !eth_rx_queue_info->flow_id_mask;
+
+ for (i = 0; i < num; i++) {
+ m = mbufs[i];
+ struct rte_event *ev = &events[i];
+
+ rss = do_rss ? do_softrss(m) : m->hash.rss;
+ flow_id =
+ eth_rx_queue_info->flow_id &
+ eth_rx_queue_info->flow_id_mask;
+ flow_id |= rss & ~eth_rx_queue_info->flow_id_mask;
+
+ ev->flow_id = flow_id;
+ ev->op = RTE_EVENT_OP_NEW;
+ ev->sched_type = sched_type;
+ ev->queue_id = qid;
+ ev->event_type = RTE_EVENT_TYPE_ETHDEV;
+ ev->sub_event_type = 0;
+ ev->priority = priority;
+ ev->mbuf = m;
How will the application get to know about the queue and eth_port from where the
packet is received by the eventdev? Is it the user responsibility to have that
information
in the flow_id?
queue: If the application needs to know the queue ID, it would have to
set the flow ID to the
queue ID at the time of adding the queue.
eth port: input port contained in in the mbuf.
+
+ buf_event_enqueue(rx_adapter, ev);
+ }
+}
+
<snip>
+int
+rte_event_eth_rx_adapter_queue_add(uint8_t id,
+ uint8_t eth_dev_id,
+ int32_t rx_queue_id,
+ const struct rte_event_eth_rx_adapter_queue_conf
*queue_conf)
+{
+ int ret;
+ uint32_t rx_adapter_cap;
+ struct rte_event_eth_rx_adapter *rx_adapter;
+ struct rte_eventdev *dev;
+ struct eth_device_info *dev_info;
+ int start_service = 0;
+
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
+
+ rx_adapter = id_to_rx_adapter(id);
+ if (!rx_adapter || !queue_conf)
+ return -EINVAL;
+
+ dev = &rte_eventdevs[rx_adapter->eventdev_id];
+ ret = (*dev->dev_ops->eth_rx_adapter_caps_get)(dev, eth_dev_id,
+ &rx_adapter_cap);
+ if (ret) {
+ RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
+ "eth port %" PRIu8, id, eth_dev_id);
+ return ret;
+ }
+
+ if (!(rx_adapter_cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID)
&&
+ !(queue_conf->rx_queue_flags &
+
RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID)) {
+ RTE_EDEV_LOG_ERR("Flow ID required for configuration,"
+ " eth port: %" PRIu8 " adapter id: %" PRIu8,
+ eth_dev_id, id);
+ return -EINVAL;
+ }
+
+ if ((rx_adapter_cap &
RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ) &&
+ (rx_queue_id != -1)) {
+ RTE_EDEV_LOG_ERR("Rx queues can only be connected to
single "
+ "event queue id %u eth port %u", id, eth_dev_id);
+ return -EINVAL;
+ }
+
+ if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
+ rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
+ RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
+ (uint16_t)rx_queue_id);
+ return -EINVAL;
+ }
+
+ start_service = 0;
+ dev_info = &rx_adapter->eth_devices[eth_dev_id];
+
+ if (rx_adapter_cap &
RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT) {
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
eth_rx_adapter_queue_add,
+ -ENOTSUP);
+ if (!dev_info->rx_queue) {
+ dev_info->rx_queue =
+ rte_zmalloc_socket(rx_adapter->mem_name,
+ dev_info->dev->data->nb_rx_queues *
+ sizeof(struct eth_rx_queue_info), 0,
+ rx_adapter->socket_id);
+ if (!dev_info->rx_queue)
+ return -ENOMEM;
+ }
+
+ ret = (*dev->dev_ops->eth_rx_adapter_queue_add)(dev,
eth_dev_id,
+ rx_queue_id, queue_conf);
IMO, Instead of eth_dev_id rte_eth_dev parameter should be passed to the
underlying driver.
Otherwise the underlying driver needs to use rte_eth_devices[] global variable
to fetch the device.
This does not seems very right to me.
Agree, passing struct rte_eth_dev to the PMD is a cleaner approach.
+ if (!ret) {
+ update_queue_info(rx_adapter,
+ &rx_adapter-
eth_devices[eth_dev_id],
+ rx_queue_id,
+ 1);
+ }
+ } else {
+ rte_spinlock_lock(&rx_adapter->rx_lock);
+ ret = init_service(rx_adapter, id);
+ if (!ret)
+ ret = add_rx_queue(rx_adapter, eth_dev_id,
rx_queue_id,
+ queue_conf);
+ rte_spinlock_unlock(&rx_adapter->rx_lock);
+ if (!ret)
+ start_service =
!!sw_rx_adapter_queue_count(rx_adapter);
+ }
+
+ if (ret)
+ return ret;
+
+ if (start_service)
+ rte_service_component_runstate_set(rx_adapter->service_id,
1);
+
+ return 0;
+}
+
+int
+rte_event_eth_rx_adapter_queue_del(uint8_t id, uint8_t eth_dev_id,
+ int32_t rx_queue_id)
+{
+ int ret = 0;
+ struct rte_eventdev *dev;
+ struct rte_event_eth_rx_adapter *rx_adapter;
+ struct eth_device_info *dev_info;
+ uint32_t rx_adapter_cap;
+ uint16_t i;
+
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_dev_id, -EINVAL);
+
+ rx_adapter = id_to_rx_adapter(id);
+ if (!rx_adapter)
+ return -EINVAL;
+
+ dev = &rte_eventdevs[rx_adapter->eventdev_id];
+ ret = dev->dev_ops->eth_rx_adapter_caps_get(dev, eth_dev_id,
+ &rx_adapter_cap);
+ if (ret)
+ return ret;
+
+ if (rx_queue_id != -1 && (uint16_t)rx_queue_id >=
+ rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
+ RTE_EDEV_LOG_ERR("Invalid rx queue_id %" PRIu16,
+ (uint16_t)rx_queue_id);
+ return -EINVAL;
+ }
+
+ dev_info = &rx_adapter->eth_devices[eth_dev_id];
+
+ if (rx_adapter_cap &
RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT) {
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
eth_rx_adapter_queue_del,
+ -ENOTSUP);
+ ret = (*dev->dev_ops->eth_rx_adapter_queue_del)(dev,
eth_dev_id,
+ rx_queue_id);
Here too rte_eth_device shall be passed.
OK.
+ if (!ret) {
+ update_queue_info(rx_adapter,
+ &rx_adapter-
eth_devices[eth_dev_id],
+ rx_queue_id,
+ 0);
+ if (!dev_info->nb_dev_queues) {
+ rte_free(dev_info->rx_queue);
+ dev_info->rx_queue = NULL;
+ }
+ }
+ } else {
+ int rc;
+ rte_spinlock_lock(&rx_adapter->rx_lock);
+ if (rx_queue_id == -1) {
+ for (i = 0; i < dev_info->dev->data->nb_rx_queues; i++)
+
_rte_event_eth_rx_adapter_queue_del(rx_adapter,
+ dev_info,
+ i);
+ } else {
+ _rte_event_eth_rx_adapter_queue_del(rx_adapter,
+ dev_info,
+
(uint16_t)rx_queue_id);
+ }
+
+ rc = eth_poll_wrr_calc(rx_adapter);
+ if (rc)
+ RTE_EDEV_LOG_ERR("WRR recalculation failed %"
PRId32,
+ rc);
+
+ if (!dev_info->nb_dev_queues) {
+ rte_free(dev_info->rx_queue);
+ dev_info->rx_queue = NULL;
+ }
+
+ rte_spinlock_unlock(&rx_adapter->rx_lock);
+ rte_service_component_runstate_set(rx_adapter->service_id,
+ sw_rx_adapter_queue_count(rx_adapter));
+ }
+
+ return ret;
+}
+
+
+int
+rte_event_eth_rx_adapter_start(uint8_t id)
+{
+ return rx_adapter_ctrl(id, 1);
+}
+
+int
+rte_event_eth_rx_adapter_stop(uint8_t id)
+{
+ return rx_adapter_ctrl(id, 0);
+}
+
+int
+rte_event_eth_rx_adapter_stats_get(uint8_t id,
+ struct rte_event_eth_rx_adapter_stats *stats)
+{
+ struct rte_event_eth_rx_adapter *rx_adapter;
+ struct rte_event_eth_rx_adapter_stats dev_stats_sum = { 0 };
+ struct rte_event_eth_rx_adapter_stats dev_stats;
+ struct rte_eventdev *dev;
+ struct eth_device_info *dev_info;
+ uint32_t i;
+ int ret;
+
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
+
+ rx_adapter = id_to_rx_adapter(id);
+ if (!rx_adapter || !stats)
+ return -EINVAL;
+
+ dev = &rte_eventdevs[rx_adapter->eventdev_id];
+ memset(stats, 0, sizeof(*stats));
+ for (i = 0; i < rte_eth_dev_count(); i++) {
+ dev_info = &rx_adapter->eth_devices[i];
+ if (!dev_info->internal_event_port)
+ continue;
+ ret = (*dev->dev_ops->eth_rx_adapter_stats_get)(dev, i,
+ &dev_stats);
Preferable to check if the adapter stats get and stats set API are registered
from the driver.
OK.
Thanks for the review, I will make the changes and post a v4.
Nikhil
Regards,
Nipun
