From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f170.google.com (mail-wi0-f170.google.com [209.85.212.170]) by dpdk.org (Postfix) with ESMTP id A99519AB1 for ; Mon, 23 Feb 2015 16:12:18 +0100 (CET) Received: by mail-wi0-f170.google.com with SMTP id hi2so20453377wib.1 for ; Mon, 23 Feb 2015 07:12:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=IzAu5Yn+xTwb4Qn4stmQQhlqqxhJEw77j8W6Nko3txQ=; b=Xz+efNmOkG6VhcFGVZ8D5YSHfWA6zl2bwNAWzWrlcEZp/Oqg+LJDjE5C+YPJzhWcC9 ULkdBZKiwUY5mQB1JLQqM2+GeOPdjAeMUZwlBz9B5UUnv7ZW2bdk37Jg4SFa9TZ1Qd4G xEx/97IlAJGL+u4A3tcvB8k3RVQjYuEpn5xeX0oEsl+dF5sxNeSaMUB+RyHAf48qQDiZ vqSiWmgnrDsP9hIA+CXy3KxoBEiWA7Rq2Vo7AZTS1nGZWakYzzz59CTR77kJFG0i+fqK vvgI92yH+DhY0UQzicF6UyBfszOCbPCLkAbKv9QVlxCICk202aBVKfhzzjRtwUZphQPL xINA== X-Gm-Message-State: ALoCoQkLxQ0Bm9NcvRMldWacru9RoktQSpxmoqw3nkqdY0q0hg2DztYbgBSi/AmOvcUvuVr7qp15 X-Received: by 10.180.87.169 with SMTP id az9mr21584993wib.72.1424704337871; Mon, 23 Feb 2015 07:12:17 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id m9sm16245447wiz.24.2015.02.23.07.12.15 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 23 Feb 2015 07:12:16 -0800 (PST) From: Thomas Monjalon To: John McNamara Date: Mon, 23 Feb 2015 16:11:45 +0100 Message-ID: <7382517.Dv0U0yYrtA@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <1424451827-32293-3-git-send-email-john.mcnamara@intel.com> References: <1424281343-2994-1-git-send-email-john.mcnamara@intel.com> <1424451827-32293-1-git-send-email-john.mcnamara@intel.com> <1424451827-32293-3-git-send-email-john.mcnamara@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v5 2/3] ethdev: add optional rxtx callback support 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: Mon, 23 Feb 2015 15:12:18 -0000 Hi John, 2015-02-20 17:03, John McNamara: > From: Richardson, Bruce > > Add optional support for inline processing of packets inside the RX > or TX call. For an RX callback, what happens is that we get a set of > packets from the NIC and then pass them to a callback function, if > configured, to allow additional processing to be done on them, e.g. > filling in more mbuf fields, before passing back to the application. > On TX, the packets are similarly post-processed before being handed > to the NIC for transmission. > > Signed-off-by: Bruce Richardson > Signed-off-by: John McNamara [...] > +#ifdef RTE_ETHDEV_RXTX_CALLBACKS > +void * > +rte_eth_add_rx_callback(uint8_t port_id, uint16_t queue_id, > + rte_rxtx_callback_fn fn, void *user_param) > +{ > + /* check input parameters */ > + if (port_id >= nb_ports || fn == NULL || > + queue_id >= rte_eth_devices[port_id].data->nb_rx_queues) { > + rte_errno = EINVAL; > + return NULL; > + } Why not putting #ifdef only here and return an error ENOTSUP? > + struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0); > + > + if (cb == NULL) { > + rte_errno = ENOMEM; > + return NULL; > + } > + > + cb->fn = fn; > + cb->param = user_param; > + cb->next = rte_eth_devices[port_id].post_rx_burst_cbs[queue_id]; > + rte_eth_devices[port_id].post_rx_burst_cbs[queue_id] = cb; > + return cb; > +} [...] > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -1522,6 +1522,49 @@ struct eth_dev_ops { > eth_filter_ctrl_t filter_ctrl; /**< common filter control*/ > }; > > +#ifdef RTE_ETHDEV_RXTX_CALLBACKS > +/** > + * Function type used for callbacks for processing packets on RX and TX > + * > + * If configured for RX, it is called with a burst of packets that have just > + * been received on the given port and queue. On TX, it is called with a burst > + * of packets immediately before those packets are put onto the hardware queue > + * for transmission. > + * > + * @param port > + * The ethernet port on which rx or tx is being performed > + * @param queue > + * The queue on the ethernet port which is being used to receive or transmit > + * the packets. > + * @param pkts > + * The burst of packets on which processing is to be done. On RX, these > + * packets have just been received. On TX, they are about to be transmitted. > + * @param nb_pkts > + * The number of packets in the burst pointed to by "pkts" > + * @param user_param > + * The arbitrary user parameter passed in by the application when the callback > + * was originally configured. > + * @return > + * The number of packets remaining in pkts are processing. > + * * On RX, this will be returned to the user as the return value from > + * rte_eth_rx_burst. > + * * On TX, this will be the number of packets actually written to the NIC. > + */ > +typedef uint16_t (*rte_rxtx_callback_fn)(uint8_t port, uint16_t queue, > + struct rte_mbuf *pkts[], uint16_t nb_pkts, void *user_param); > + > +/** > + * @internal > + * Structure used to hold information about the callbacks to be called for a > + * queue on RX and TX. > + */ > +struct rte_eth_rxtx_callback { > + struct rte_eth_rxtx_callback *next; > + rte_rxtx_callback_fn fn; > + void *param; > +}; > +#endif > + > /** > * @internal > * The generic data structure associated with each ethernet device. > @@ -1539,7 +1582,22 @@ struct rte_eth_dev { > const struct eth_driver *driver;/**< Driver for this device */ > struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */ > struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */ > - struct rte_eth_dev_cb_list link_intr_cbs; /**< User application callbacks on interrupt*/ > + /** User application callbacks for NIC interrupts */ > + struct rte_eth_dev_cb_list link_intr_cbs; > + > +#ifdef RTE_ETHDEV_RXTX_CALLBACKS > + /** > + * User-supplied functions called from rx_burst to post-process > + * received packets before passing them to the user > + */ > + struct rte_eth_rxtx_callback **post_rx_burst_cbs; > + > + /** > + * User-supplied functions called from tx_burst to pre-process > + * received packets before passing them to the driver for transmission. > + */ > + struct rte_eth_rxtx_callback **pre_tx_burst_cbs; > +#endif > }; Generally, I think it's a bad idea to put #ifdef in API (structs or functions). > struct rte_eth_dev_sriov { > @@ -2393,7 +2451,23 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id, > struct rte_eth_dev *dev; > > dev = &rte_eth_devices[port_id]; > + > +#ifndef RTE_ETHDEV_RXTX_CALLBACKS > return (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], rx_pkts, nb_pkts); > +#else > + nb_pkts = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], rx_pkts, > + nb_pkts); Why not #ifdef only from here... > + struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id]; > + > + if (unlikely(cb != NULL)) { > + do { > + nb_pkts = cb->fn(port_id, queue_id, rx_pkts, nb_pkts, > + cb->param); > + cb = cb->next; > + } while (cb != NULL); > + } ... to here? > + return nb_pkts; > +#endif > } > #endif > > @@ -2520,6 +2594,17 @@ rte_eth_tx_burst(uint8_t port_id, uint16_t queue_id, > struct rte_eth_dev *dev; > > dev = &rte_eth_devices[port_id]; > +#ifdef RTE_ETHDEV_RXTX_CALLBACKS > + struct rte_eth_rxtx_callback *cb = dev->pre_tx_burst_cbs[queue_id]; > + > + if (unlikely(cb != NULL)) { > + do { > + nb_pkts = cb->fn(port_id, queue_id, tx_pkts, nb_pkts, > + cb->param); > + cb = cb->next; > + } while (cb != NULL); > + } > +#endif > return (*dev->tx_pkt_burst)(dev->data->tx_queues[queue_id], tx_pkts, nb_pkts); > } > #endif > @@ -3667,6 +3752,123 @@ int rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type filter_ty > int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type filter_type, > enum rte_filter_op filter_op, void *arg); > > +#ifdef RTE_ETHDEV_RXTX_CALLBACKS > +/** > + * Add a callback to be called on packet RX on a given port and queue. > + * > + * This API configures a function to be called for each burst of > + * packets received on a given NIC port queue. The return value is a pointer > + * that can be used to later remove the callback using > + * rte_eth_remove_rx_callback(). > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The queue on the Ethernet device on which the callback is to be added. > + * @param fn > + * The callback function > + * @param user_param > + * A generic pointer parameter which will be passed to each invocation of the > + * callback function on this port and queue. > + * > + * @return > + * NULL on error. > + * On success, a pointer value which can later be used to remove the callback. > + */ > +void *rte_eth_add_rx_callback(uint8_t port_id, uint16_t queue_id, > + rte_rxtx_callback_fn fn, void *user_param); > + > +/** > + * Add a callback to be called on packet TX on a given port and queue. > + * > + * This API configures a function to be called for each burst of > + * packets sent on a given NIC port queue. The return value is a pointer > + * that can be used to later remove the callback using > + * rte_eth_remove_tx_callback(). > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The queue on the Ethernet device on which the callback is to be added. > + * @param fn > + * The callback function > + * @param user_param > + * A generic pointer parameter which will be passed to each invocation of the > + * callback function on this port and queue. > + * > + * @return > + * NULL on error. > + * On success, a pointer value which can later be used to remove the callback. > + */ > +void *rte_eth_add_tx_callback(uint8_t port_id, uint16_t queue_id, > + rte_rxtx_callback_fn fn, void *user_param); > + > +/** > + * Remove an RX packet callback from a given port and queue. > + * > + * This function is used to removed callbacks that were added to a NIC port > + * queue using rte_eth_add_rx_callback(). > + * > + * Note: the callback is removed from the callback list but it isn't freed > + * since the it may still be in use. The memory for the callback can be > + * subsequently freed back by the application by calling rte_free(): > + * > + * - Immediately - if the port is stopped, or the user knows that no > + * callbacks are in flight e.g. if called from the thread doing RX/TX > + * on that queue. > + * > + * - After a short delay - where the delay is sufficient to allow any > + * in-flight callbacks to complete. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The queue on the Ethernet device from which the callback is to be removed. > + * @param user_cb > + * User supplied callback created via rte_eth_add_rx_callback(). > + * > + * @return > + * - 0: Success. Callback was removed. > + * - -EINVAL: The port_id or the queue_id is out of range, or the callback is > + * NULL or not found for the port/queue. > + */ > +int rte_eth_remove_rx_callback(uint8_t port_id, uint16_t queue_id, > + struct rte_eth_rxtx_callback *user_cb); > + > +/** > + * Remove a TX packet callback from a given port and queue. > + * > + * This function is used to removed callbacks that were added to a NIC port > + * queue using rte_eth_add_tx_callback(). > + * > + * Note: the callback is removed from the callback list but it isn't freed > + * since the it may still be in use. The memory for the callback can be > + * subsequently freed back by the application by calling rte_free(): > + * > + * - Immediately - if the port is stopped, or the user knows that no > + * callbacks are in flight e.g. if called from the thread doing RX/TX > + * on that queue. > + * > + * - After a short delay - where the delay is sufficient to allow any > + * in-flight callbacks to complete. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The queue on the Ethernet device from which the callback is to be removed. > + * @param user_cb > + * User supplied callback created via rte_eth_add_tx_callback(). > + * > + * @return > + * - 0: Success. Callback was removed. > + * - -EINVAL: The port_id or the queue_id is out of range, or the callback is > + * NULL or not found for the port/queue. > + */ > +int rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id, > + struct rte_eth_rxtx_callback *user_cb); > + > +#endif /* RTE_ETHDEV_RXTX_CALLBACKS */ Please avoid #ifdef around function declarations. Thanks