[v4,18/29] node: add ethdev Rx and Tx node ctrl API
Checks
Commit Message
From: Nithin Dabilpuram <ndabilpuram@marvell.com>
Add ctrl api to setup ethdev_rx and ethdev_tx node.
This ctrl api clones 'N' number of ethdev_rx and ethdev_tx
nodes with specific (port, queue) pairs updated in their context.
All the ethdev ports and queues are setup before this api
is called.
Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
---
doc/api/doxy-api-index.md | 2 +
lib/librte_node/Makefile | 6 +-
lib/librte_node/ethdev_ctrl.c | 100 +++++++++++++++++++++++++++
lib/librte_node/meson.build | 5 +-
lib/librte_node/node_private.h | 74 ++++++++++++++++++++
lib/librte_node/rte_node_eth_api.h | 70 +++++++++++++++++++
lib/librte_node/rte_node_version.map | 1 +
7 files changed, 255 insertions(+), 3 deletions(-)
create mode 100644 lib/librte_node/ethdev_ctrl.c
create mode 100644 lib/librte_node/rte_node_eth_api.h
Comments
On 4/5/20 10:56 AM, jerinj@marvell.com wrote:
> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>
> Add ctrl api to setup ethdev_rx and ethdev_tx node.
> This ctrl api clones 'N' number of ethdev_rx and ethdev_tx
> nodes with specific (port, queue) pairs updated in their context.
> All the ethdev ports and queues are setup before this api
> is called.
>
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> ---
[...]
> +struct rte_node_mbuf_priv1 {
> + union {
> + /* IP4 rewrite */
> + struct {
> + uint16_t nh;
> + uint16_t ttl;
> + uint32_t cksum;
> + };
> +
> + uint64_t u;
> + };
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Node mbuf private data to store crypto operation.
> + */
> +struct rte_node_mbuf_priv2 {
> + union {
> + /* Sym crypto */
> + struct {
> + struct rte_crypto_op op;
> + };
> + };
> +} __rte_cache_aligned;
Why such definition?
With regards
Andrzej Ostruszka
On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
> On 4/5/20 10:56 AM, jerinj@marvell.com wrote:
> > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >
> > Add ctrl api to setup ethdev_rx and ethdev_tx node.
> > This ctrl api clones 'N' number of ethdev_rx and ethdev_tx
> > nodes with specific (port, queue) pairs updated in their context.
> > All the ethdev ports and queues are setup before this api
> > is called.
> >
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> > ---
> [...]
> > +struct rte_node_mbuf_priv1 {
> > + union {
> > + /* IP4 rewrite */
> > + struct {
> > + uint16_t nh;
> > + uint16_t ttl;
> > + uint32_t cksum;
> > + };
> > +
> > + uint64_t u;
> > + };
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Node mbuf private data to store crypto operation.
> > + */
> > +struct rte_node_mbuf_priv2 {
> > + union {
> > + /* Sym crypto */
> > + struct {
> > + struct rte_crypto_op op;
> > + };
> > + };
> > +} __rte_cache_aligned;
>
> Why such definition?
For communication b/w nodes, we need some per mbuf private space.
We defined it into two halfs for performance reasons as
#1 rte_node_mbuf_priv1(8 bytes) mapped to mbuf->udata64
#2 rte_node_mbuf_priv2(RTE_CACHE_LINE_SIZE bytes) mapped to mbuf private area.
#1 is smaller area and will not have a cache miss when accessed as mbuf
is already in cache.
#2 is larger area and probably good enough for many use cases like ipsec, crypto
etc, and there will be an extra cost of cache miss to access it.
Atleast in OCTEONTX2, we are able to see 27% performance drop, if use single
private area #2 for everything instead.
Since pkt_mbuf pool are created by application, we these structures are defined
here have a check in ctrl api if the pkt_mbuf pool meets the mbuf private area
size requirement.
>
> With regards
> Andrzej Ostruszka
On Fri, Apr 10, 2020 at 10:39:44AM +0530, Nithin Dabilpuram wrote:
> On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
> > On 4/5/20 10:56 AM, jerinj@marvell.com wrote:
> > > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > >
> > > Add ctrl api to setup ethdev_rx and ethdev_tx node.
> > > This ctrl api clones 'N' number of ethdev_rx and ethdev_tx
> > > nodes with specific (port, queue) pairs updated in their context.
> > > All the ethdev ports and queues are setup before this api
> > > is called.
> > >
> > > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> > > ---
> > [...]
> > > +struct rte_node_mbuf_priv1 {
> > > + union {
> > > + /* IP4 rewrite */
> > > + struct {
> > > + uint16_t nh;
> > > + uint16_t ttl;
> > > + uint32_t cksum;
> > > + };
> > > +
> > > + uint64_t u;
> > > + };
> > > +};
> > > +
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > + *
> > > + * Node mbuf private data to store crypto operation.
> > > + */
> > > +struct rte_node_mbuf_priv2 {
> > > + union {
> > > + /* Sym crypto */
> > > + struct {
> > > + struct rte_crypto_op op;
> > > + };
> > > + };
> > > +} __rte_cache_aligned;
> >
> > Why such definition?
>
> For communication b/w nodes, we need some per mbuf private space.
> We defined it into two halfs for performance reasons as
> #1 rte_node_mbuf_priv1(8 bytes) mapped to mbuf->udata64
> #2 rte_node_mbuf_priv2(RTE_CACHE_LINE_SIZE bytes) mapped to mbuf private area.
>
> #1 is smaller area and will not have a cache miss when accessed as mbuf
> is already in cache.
> #2 is larger area and probably good enough for many use cases like ipsec, crypto
> etc, and there will be an extra cost of cache miss to access it.
>
> Atleast in OCTEONTX2, we are able to see 27% performance drop, if use single
> private area #2 for everything instead.
>
> Since pkt_mbuf pool are created by application, we these structures are defined
> here have a check in ctrl api if the pkt_mbuf pool meets the mbuf private area
> size requirement.
Just wanted to update that I'll also rename this structure and
related api's to not start with "rte_" in next version
as they are not visible outside this librte_node library.
>
> >
> > With regards
> > Andrzej Ostruszka
On 4/10/20 7:09 AM, Nithin Dabilpuram wrote:
> On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
[...]
>>> +struct rte_node_mbuf_priv2 {
>>> + union {
>>> + /* Sym crypto */
>>> + struct {
>>> + struct rte_crypto_op op;
>>> + };
>>> + };
>>> +} __rte_cache_aligned;
>>
>> Why such definition?
The question was more on "technicalities" - you have struct with anon
union with anon struct with a struct. Why such deep nesting - I guess
the union is there for the possible future extensions but the next anon
struct - what is it for?
> For communication b/w nodes, we need some per mbuf private space.
> We defined it into two halfs for performance reasons as
> #1 rte_node_mbuf_priv1(8 bytes) mapped to mbuf->udata64
> #2 rte_node_mbuf_priv2(RTE_CACHE_LINE_SIZE bytes) mapped to mbuf private area.
>
> #1 is smaller area and will not have a cache miss when accessed as mbuf
> is already in cache.
> #2 is larger area and probably good enough for many use cases like ipsec, crypto
> etc, and there will be an extra cost of cache miss to access it.
>
> Atleast in OCTEONTX2, we are able to see 27% performance drop, if use single
> private area #2 for everything instead.
>
> Since pkt_mbuf pool are created by application, we these structures are defined
> here have a check in ctrl api if the pkt_mbuf pool meets the mbuf private area
> size requirement.
Thank you for explanations.
With regards
Andrzej Ostruszka
On Fri, Apr 10, 2020 at 02:52:33PM +0200, Andrzej Ostruszka wrote:
> External Email
>
> ----------------------------------------------------------------------
> On 4/10/20 7:09 AM, Nithin Dabilpuram wrote:
> > On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
> [...]
> >>> +struct rte_node_mbuf_priv2 {
> >>> + union {
> >>> + /* Sym crypto */
> >>> + struct {
> >>> + struct rte_crypto_op op;
> >>> + };
> >>> + };
> >>> +} __rte_cache_aligned;
> >>
> >> Why such definition?
>
> The question was more on "technicalities" - you have struct with anon
> union with anon struct with a struct. Why such deep nesting - I guess
> the union is there for the possible future extensions but the next anon
> struct - what is it for?
I think inner struct helps in collecting together a specific node's data like
priv1. For example
struct node_mbuf_priv2 {
union {
/* Sym crypto */
struct {
struct rte_crypto_op op;
uint64_t extra_session_info;
};
/* Reassembly info */
struct {
uint64_t reassembly_info;
uint64_t pad;
};
};
uint8_t data[64];
} __rte_cache_aligned;
Another thing is given that currently there is no crypto support, I'm removing
the current content of struct node_mbuf_priv2 and just leaving a pad field
for future expansion purpose.
>
> > For communication b/w nodes, we need some per mbuf private space.
> > We defined it into two halfs for performance reasons as
> > #1 rte_node_mbuf_priv1(8 bytes) mapped to mbuf->udata64
> > #2 rte_node_mbuf_priv2(RTE_CACHE_LINE_SIZE bytes) mapped to mbuf private area.
> >
> > #1 is smaller area and will not have a cache miss when accessed as mbuf
> > is already in cache.
> > #2 is larger area and probably good enough for many use cases like ipsec, crypto
> > etc, and there will be an extra cost of cache miss to access it.
> >
> > Atleast in OCTEONTX2, we are able to see 27% performance drop, if use single
> > private area #2 for everything instead.
> >
> > Since pkt_mbuf pool are created by application, we these structures are defined
> > here have a check in ctrl api if the pkt_mbuf pool meets the mbuf private area
> > size requirement.
>
> Thank you for explanations.
>
> With regards
> Andrzej Ostruszka
@@ -161,6 +161,8 @@ The public API headers are grouped by topics:
[table_action] (@ref rte_table_action.h)
* [graph] (@ref rte_graph.h):
[graph_worker] (@ref rte_graph_worker.h)
+ * graph_nodes:
+ [eth_node] (@ref rte_node_eth_api.h),
- **basic**:
[approx fraction] (@ref rte_approx.h),
@@ -11,7 +11,7 @@ CFLAGS += -O3 -DALLOW_EXPERIMENTAL_API
CFLAGS += $(WERROR_FLAGS)
# Strict-aliasing rules are violated by uint8_t[] to context size casts.
CFLAGS += -fno-strict-aliasing
-LDLIBS += -lrte_eal -lrte_graph -lrte_mbuf -lrte_ethdev
+LDLIBS += -lrte_eal -lrte_graph -lrte_mbuf -lrte_ethdev -lrte_mempool
EXPORT_MAP := rte_node_version.map
@@ -20,5 +20,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_NODE) += null.c
SRCS-$(CONFIG_RTE_LIBRTE_NODE) += log.c
SRCS-$(CONFIG_RTE_LIBRTE_NODE) += ethdev_rx.c
SRCS-$(CONFIG_RTE_LIBRTE_NODE) += ethdev_tx.c
+SRCS-$(CONFIG_RTE_LIBRTE_NODE) += ethdev_ctrl.c
+
+# install header files
+SYMLINK-$(CONFIG_RTE_LIBRTE_NODE)-include += rte_node_eth_api.h
include $(RTE_SDK)/mk/rte.lib.mk
new file mode 100644
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Marvell International Ltd.
+ */
+
+#include <rte_debug.h>
+#include <rte_ethdev.h>
+#include <rte_ether.h>
+#include <rte_graph.h>
+
+#include "rte_node_eth_api.h"
+
+#include "ethdev_rx_priv.h"
+#include "ethdev_tx_priv.h"
+#include "node_private.h"
+
+static struct ethdev_ctrl {
+ uint16_t nb_graphs;
+} ctrl;
+
+int
+rte_node_eth_config(struct rte_node_ethdev_config *conf, uint16_t nb_confs,
+ uint16_t nb_graphs)
+{
+ struct ethdev_tx_node_main *tx_node_data;
+ uint16_t tx_q_used, rx_q_used, port_id;
+ struct rte_node_register *tx_node;
+ char name[RTE_NODE_NAMESIZE];
+ struct rte_mempool *mp;
+ uint32_t id;
+ int i, j;
+
+ tx_node_data = ethdev_tx_node_data_get();
+ tx_node = ethdev_tx_node_get();
+ for (i = 0; i < nb_confs; i++) {
+ port_id = conf[i].port_id;
+
+ if (!rte_eth_dev_is_valid_port(port_id))
+ return -EINVAL;
+
+ /* Check for mbuf minimum private size requirement */
+ for (j = 0; j < conf[i].mp_count; j++) {
+ mp = conf[i].mp[j];
+ if (!mp)
+ continue;
+ /* Check for minimum private space */
+ if (rte_pktmbuf_priv_size(mp) <
+ RTE_NODE_MBUF_PRIV2_SIZE) {
+ node_err("ethdev",
+ "Minimum mbuf priv size requirement not met by mp %s",
+ mp->name);
+ return -EINVAL;
+ }
+ }
+
+ rx_q_used = conf[i].num_rx_queues;
+ tx_q_used = conf[i].num_tx_queues;
+ /* Check if we have a txq for each worker */
+ if (tx_q_used < nb_graphs)
+ return -EINVAL;
+
+ /* Create node for each rx port queue pair */
+ for (j = 0; j < rx_q_used; j++) {
+ struct ethdev_rx_node_main *rx_node_data;
+ struct rte_node_register *rx_node;
+ ethdev_rx_node_elem_t *elem;
+
+ rx_node_data = ethdev_rx_get_node_data_get();
+ rx_node = ethdev_rx_node_get();
+ snprintf(name, sizeof(name), "%u-%u", port_id, j);
+ /* Clone a new rx node with same edges as parent */
+ id = rte_node_clone(rx_node->id, name);
+ if (id == RTE_NODE_ID_INVALID)
+ return -EIO;
+
+ /* Add it to list of ethdev rx nodes for lookup */
+ elem = malloc(sizeof(ethdev_rx_node_elem_t));
+ memset(elem, 0, sizeof(ethdev_rx_node_elem_t));
+ elem->ctx.port_id = port_id;
+ elem->ctx.queue_id = j;
+ elem->nid = id;
+ elem->next = rx_node_data->head;
+ rx_node_data->head = elem;
+
+ node_dbg("ethdev", "Rx node %s-%s: is at %u",
+ rx_node->name, name, id);
+ }
+
+ /* Create a per port tx node from base node */
+ snprintf(name, sizeof(name), "%u", port_id);
+ /* Clone a new node with same edges as parent */
+ id = rte_node_clone(tx_node->id, name);
+ tx_node_data->nodes[port_id] = id;
+
+ node_dbg("ethdev", "Tx node %s-%s: is at %u", tx_node->name,
+ name, id);
+ }
+
+ ctrl.nb_graphs = nb_graphs;
+ return 0;
+}
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(C) 2020 Marvell International Ltd.
-sources = files('null.c', 'log.c', 'ethdev_rx.c', 'ethdev_tx.c')
+sources = files('null.c', 'log.c', 'ethdev_rx.c', 'ethdev_tx.c', 'ethdev_ctrl.c')
+headers = files('rte_node_eth_api.h')
allow_experimental_apis = true
# Strict-aliasing rules are violated by uint8_t[] to context size casts.
cflags += '-fno-strict-aliasing'
-deps += ['graph', 'mbuf', 'ethdev']
+deps += ['graph', 'mbuf', 'lpm', 'ethdev', 'mempool', 'cryptodev']
@@ -6,7 +6,9 @@
#define __NODE_PRIVATE_H__
#include <rte_common.h>
+#include <rte_crypto.h>
#include <rte_log.h>
+#include <rte_mbuf.h>
extern int rte_node_logtype;
#define NODE_LOG(level, node_name, ...) \
@@ -19,4 +21,76 @@ extern int rte_node_logtype;
#define node_info(node_name, ...) NODE_LOG(INFO, node_name, __VA_ARGS__)
#define node_dbg(node_name, ...) NODE_LOG(DEBUG, node_name, __VA_ARGS__)
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Node mbuf private data to store next hop, ttl and checksum.
+ */
+struct rte_node_mbuf_priv1 {
+ union {
+ /* IP4 rewrite */
+ struct {
+ uint16_t nh;
+ uint16_t ttl;
+ uint32_t cksum;
+ };
+
+ uint64_t u;
+ };
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Node mbuf private data to store crypto operation.
+ */
+struct rte_node_mbuf_priv2 {
+ union {
+ /* Sym crypto */
+ struct {
+ struct rte_crypto_op op;
+ };
+ };
+} __rte_cache_aligned;
+
+#define RTE_NODE_MBUF_PRIV2_SIZE sizeof(struct rte_node_mbuf_priv2)
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get mbuf_priv1 pointer from rte_mbuf.
+ *
+ * @param
+ * Pointer to the rte_mbuf.
+ *
+ * @return
+ * Pointer to the mbuf_priv1.
+ */
+static __rte_always_inline struct rte_node_mbuf_priv1 *
+rte_node_mbuf_priv1(struct rte_mbuf *m)
+{
+ return (struct rte_node_mbuf_priv1 *)&m->udata64;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get mbuf_priv2 pointer from rte_mbuf.
+ *
+ * @param
+ * Pointer to the rte_mbuf.
+ *
+ * @return
+ * Pointer to the mbuf_priv2.
+ */
+static __rte_always_inline struct rte_node_mbuf_priv2 *
+rte_node_mbuf_priv2(struct rte_mbuf *m)
+{
+ return (struct rte_node_mbuf_priv2 *)rte_mbuf_to_priv(m);
+}
+
#endif /* __NODE_PRIVATE_H__ */
new file mode 100644
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Marvell International Ltd.
+ */
+
+#ifndef __INCLUDE_RTE_NODE_ETH_API_H__
+#define __INCLUDE_RTE_NODE_ETH_API_H__
+
+/**
+ * @file rte_node_eth_api.h
+ *
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * This API allows to setup ethdev_rx and ethdev_tx nodes
+ * and its queue associations.
+ *
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <rte_common.h>
+#include <rte_mempool.h>
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Port config for ethdev_rx and ethdev_tx node.
+ */
+struct rte_node_ethdev_config {
+ uint16_t port_id;
+ /**< Port identifier */
+ uint16_t num_rx_queues;
+ /**< Number of Rx queues. */
+ uint16_t num_tx_queues;
+ /**< Number of Tx queues. */
+ struct rte_mempool **mp;
+ /**< Array of mempools associated to Rx queue. */
+ uint16_t mp_count;
+ /**< Size of mp array. */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Initializes ethdev nodes.
+ *
+ * @param cfg
+ * Array of ethdev config that identifies which port's
+ * ethdev_rx and ethdev_tx nodes need to be created
+ * and queue association.
+ * @param cnt
+ * Size of cfg array.
+ * @param nb_graphs
+ * Number of graphs that will be used.
+ *
+ * @return
+ * 0 on successful initialization, negative otherwise.
+ */
+__rte_experimental
+int rte_node_eth_config(struct rte_node_ethdev_config *cfg,
+ uint16_t cnt, uint16_t nb_graphs);
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __INCLUDE_RTE_NODE_ETH_API_H__ */
@@ -1,6 +1,7 @@
EXPERIMENTAL {
global:
+ rte_node_eth_config;
rte_node_logtype;
local: *;
};