On 1/16/2020 12:05 PM, Ori Kam wrote:
> 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?
>
I think testpmd patch is clear enough on its own and better to send PMD patch
separately when it is complete.
