Hi Ferruh,
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@intel.com>
> Sent: Wednesday, January 15, 2020 4:02 PM
> To: Ori Kam <or...@mellanox.com>; Matan Azrad <ma...@mellanox.com>;
> Shahaf Shuler <shah...@mellanox.com>; Slava Ovsiienko
> <viachesl...@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
>
> On 1/13/2020 9:29 AM, Ori Kam wrote:
> > The inline feature is designed to save PCI bandwidth by copying some
> > of the data to the wqe. This feature if enabled works for all packets.
> >
> > In some cases when using external memory, the PCI bandwidth is not
> > relevant since the memory can be accessed by other means.
> >
> > This commit introduce the ability to control the inline with mbuf
> > granularity.
> >
> > In order to use this feature the application should register the field
> > name, and restart the port.
> >
> > Signed-off-by: Ori Kam <or...@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> > ---
> > drivers/net/mlx5/mlx5.c | 15 +++++++++++++++
> > drivers/net/mlx5/mlx5_rxtx.c | 2 ++
> > drivers/net/mlx5/mlx5_rxtx.h | 3 +++
> > drivers/net/mlx5/mlx5_trigger.c | 8 ++++++++
> > drivers/net/mlx5/rte_pmd_mlx5.h | 32
> +++++++++++++++++++++++++++++++
> > drivers/net/mlx5/rte_pmd_mlx5_version.map | 7 +++++++
> > 6 files changed, 67 insertions(+)
> > create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h
> >
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> > index 50960c9..27dbe27 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -46,6 +46,7 @@
> > #include "mlx5_glue.h"
> > #include "mlx5_mr.h"
> > #include "mlx5_flow.h"
> > +#include "rte_pmd_mlx5.h"
> >
> > /* Device parameter to enable RX completion queue compression. */
> > #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
> > @@ -1988,6 +1989,20 @@ struct mlx5_flow_id_pool *
> > return ret;
> > }
> >
> > +int
> > +rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n)
> > +{
>
> Now this is a public API, it should validate the user input.
>
Will add validation to make sure names != NULL,
> > + static const char *const dynf_names[] = {
> > + RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
> > + };
> > + int num = RTE_MIN(n, RTE_DIM(dynf_names));
> > + int i;
> > +
> > + for (i = 0; i < num; i++)
> > + strcpy(names[i], dynf_names[i]);
> > + return num;
> > +}
> > +
> > /**
> > * Check sibling device configurations.
> > *
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> > index 67cafd1..aa6aa22 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.c
> > +++ b/drivers/net/mlx5/mlx5_rxtx.c
> > @@ -126,6 +126,8 @@ enum mlx5_txcmp_code {
> > uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
> > uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
> >
> > +uint64_t rte_net_mlx5_dynf_inline_mask;
> > +
> > /**
> > * Build a table to translate Rx completion flags to packet type.
> > *
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> > index e362b4a..7c38c57 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.h
> > +++ b/drivers/net/mlx5/mlx5_rxtx.h
> > @@ -42,6 +42,9 @@
> > /* Support tunnel matching. */
> > #define MLX5_FLOW_TUNNEL 9
> >
> > +/* Mbuf dynamic flag offset for inline. */
> > +extern uint64_t rte_net_mlx5_dynf_inline_mask;
> > +
> > struct mlx5_rxq_stats {
> > #ifdef MLX5_PMD_SOFT_COUNTERS
> > uint64_t ipackets; /**< Total of successfully received packets. */
> > diff --git a/drivers/net/mlx5/mlx5_trigger.c
> b/drivers/net/mlx5/mlx5_trigger.c
> > index ab6937a..ab253b2 100644
> > --- a/drivers/net/mlx5/mlx5_trigger.c
> > +++ b/drivers/net/mlx5/mlx5_trigger.c
> > @@ -13,6 +13,7 @@
> > #include "mlx5.h"
> > #include "mlx5_rxtx.h"
> > #include "mlx5_utils.h"
> > +#include "rte_pmd_mlx5.h"
> >
> > /**
> > * Stop traffic on Tx queues.
> > @@ -270,8 +271,15 @@
> > {
> > struct mlx5_priv *priv = dev->data->dev_private;
> > int ret;
> > + int fine_inline;
> >
> > DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
> > + fine_inline = rte_mbuf_dynflag_lookup
> > + (RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
> > + if (fine_inline > 0)
> > + rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
> > + else
> > + rte_net_mlx5_dynf_inline_mask = 0;
> > ret = mlx5_dev_configure_rss_reta(dev);
> > if (ret) {
> > DRV_LOG(ERR, "port %u reta config failed: %s",
> > diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h
> b/drivers/net/mlx5/rte_pmd_mlx5.h
> > new file mode 100644
> > index 0000000..12e18ca
> > --- /dev/null
> > +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2020 Mellanox Technologies, Ltd
> > + */
> > +
> > +#ifndef RTE_PMD_PRIVATE_MLX5_H_
> > +#define RTE_PMD_PRIVATE_MLX5_H_
> > +
> > +/**
> > + * @file
> > + * MLX5 public header.
> > + *
> > + * This interface provides the ability to support private PMD
> > + * dynamic flags.
> > + */
> > +
> > +#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE
> "mlx5_fine_granularity_inline"
> > +
> > +/**
> > + * Returns the dynamic flags name, that are supported.
> > + *
> > + * @param[out] names
> > + * Array that is used to return the supported dynamic flags names.
> > + * @param[in] n
> > + * The number of elements in the names array.
> > + *
> > + * @return
> > + * The number of dynamic flags that were copied.
> > + */
> > +__rte_experimental
> > +int rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n);
>
> Can you please add this header to the API documentation index,
> doc/api/doxy-api-index.md, so it will be part of API document.
>
Will add.
> > +
> > +#endif
> > diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> > index f9f17e4..c8b1031 100644
> > --- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> > +++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> > @@ -1,3 +1,10 @@
> > DPDK_20.0 {
> > local: *;
> > };
> > +
> > +EXPERIMENTAL {
> > + global:
> > +
> > + # added in 20.02
> > + rte_pmd_mlx5_get_dyn_flag_names;
> > +};
> >
>
> Isn't the datapath implementation missing? Where this new mbuf dynamic
> flag set
> or checked?
The data path implementation will be done in different patch.
The flags is set for example using the testpmd new API or by the application.
The reason that I added this patch is to show usage for the testpmd patch, I
can remove this patch and
re send it when sending the datapath patch.
What do you think?
