Re: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation: Enabling IP checksum offload in mbuf
Hi,
Regards
Sunil Kumar Kori
>-Original Message-
>From: Ananyev, Konstantin
>Sent: Saturday, May 18, 2019 8:51 PM
>To: Sunil Kumar Kori ; dev@dpdk.org
>Subject: [EXT] RE: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation:
>Enabling IP checksum offload in mbuf
>
>External Email
>
>--
>
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Sunil Kumar Kori
>> Sent: Thursday, May 16, 2019 12:42 PM
>> To: dev@dpdk.org
>> Cc: sk...@marvell.com
>> Subject: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation: Enabling IP
>checksum offload in mbuf
>>
>> As per the documentation to use any IP offload features, application
>> must set required offload flags into mbuf->ol_flags.
>>
>> Signed-off-by: Sunil Kumar Kori
>> ---
>> examples/ip_fragmentation/main.c | 7 +--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/examples/ip_fragmentation/main.c
>b/examples/ip_fragmentation/main.c
>> index e90a61e..d821967 100644
>> --- a/examples/ip_fragmentation/main.c
>> +++ b/examples/ip_fragmentation/main.c
>> @@ -354,10 +354,13 @@ struct rte_lpm6_config lpm6_config = {
>>
>> /* src addr */
>> ether_addr_copy(&ports_eth_addr[port_out], ð_hdr-
>>s_addr);
>> -if (ipv6)
>> +if (ipv6) {
>> eth_hdr->ether_type =
>rte_be_to_cpu_16(ETHER_TYPE_IPv6);
>> -else
>> +m->ol_flags |= PKT_TX_IPV6;
>
>Is there is any point to do that?
>This sample app, as I remember doesn't request any extra HW offloads.
>Same comment for patch #3.
>Konstantin
>
>
Is this comment valid for IPv6 only ?
Otherwise for IPv4, it is required as IP checksum is neither being updated by
library nor
by the application.
>> +} else {
>> eth_hdr->ether_type =
>rte_be_to_cpu_16(ETHER_TYPE_IPv4);
>> +m->ol_flags |= (PKT_TX_IPV4 | PKT_TX_IP_CKSUM);
>> +}
>> }
>>
>> len += len2;
>> --
>> 1.8.3.1
[dpdk-dev] [PATCH] net/mlx5: fix memory free on queue create error
In function mlx5_rxq_ibv_new(), pointer *tmpl allocation is attempted
at the start, but not validated or freed in case of error.
In function mlx5_txq_ibv_new(), pointer *txq_ibv allocation is
attempted at the start, but not freed in case of error.
This patch adds pointers initialization, validation and freeing.
Fixes: 09cb5b581762 ("net/mlx5: separate DPDK from verbs Rx queue objects")
Fixes: faf2667fe8d5 ("net/mlx5: separate DPDK from verbs Tx queue objects")
Cc: sta...@dpdk.org
Signed-off-by: Dekel Peled
---
drivers/net/mlx5/mlx5_rxq.c | 22 +-
drivers/net/mlx5/mlx5_txq.c | 4 +++-
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index a00cb12..8cd6157 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -776,7 +776,7 @@ struct mlx5_rxq_ibv *
} attr;
unsigned int cqe_n;
unsigned int wqe_n = 1 << rxq_data->elts_n;
- struct mlx5_rxq_ibv *tmpl;
+ struct mlx5_rxq_ibv *tmpl = NULL;
struct mlx5dv_cq cq_info;
struct mlx5dv_rwq rwq;
unsigned int i;
@@ -1017,15 +1017,19 @@ struct mlx5_rxq_ibv *
priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
return tmpl;
error:
- ret = rte_errno; /* Save rte_errno before cleanup. */
- if (tmpl->wq)
- claim_zero(mlx5_glue->destroy_wq(tmpl->wq));
- if (tmpl->cq)
- claim_zero(mlx5_glue->destroy_cq(tmpl->cq));
- if (tmpl->channel)
- claim_zero(mlx5_glue->destroy_comp_channel(tmpl->channel));
+ if (tmpl) {
+ ret = rte_errno; /* Save rte_errno before cleanup. */
+ if (tmpl->wq)
+ claim_zero(mlx5_glue->destroy_wq(tmpl->wq));
+ if (tmpl->cq)
+ claim_zero(mlx5_glue->destroy_cq(tmpl->cq));
+ if (tmpl->channel)
+ claim_zero(mlx5_glue->destroy_comp_channel
+ (tmpl->channel));
+ rte_free(tmpl);
+ rte_errno = ret; /* Restore rte_errno. */
+ }
priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
- rte_errno = ret; /* Restore rte_errno. */
return NULL;
}
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index b281c45..9efbab6 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -401,7 +401,7 @@ struct mlx5_txq_ibv *
struct mlx5_txq_ctrl *txq_ctrl =
container_of(txq_data, struct mlx5_txq_ctrl, txq);
struct mlx5_txq_ibv tmpl;
- struct mlx5_txq_ibv *txq_ibv;
+ struct mlx5_txq_ibv *txq_ibv = NULL;
union {
struct ibv_qp_init_attr_ex init;
struct ibv_cq_init_attr_ex cq;
@@ -586,6 +586,8 @@ struct mlx5_txq_ibv *
claim_zero(mlx5_glue->destroy_cq(tmpl.cq));
if (tmpl.qp)
claim_zero(mlx5_glue->destroy_qp(tmpl.qp));
+ if (txq_ibv)
+ rte_free(txq_ibv);
priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
rte_errno = ret; /* Restore rte_errno. */
return NULL;
--
1.8.3.1
Re: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation: Enabling IP checksum offload in mbuf
> -Original Message-
> From: Sunil Kumar Kori [mailto:sk...@marvell.com]
> Sent: Monday, May 20, 2019 9:09 AM
> To: Ananyev, Konstantin ; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation: Enabling IP
> checksum offload in mbuf
>
> Hi,
>
> Regards
> Sunil Kumar Kori
>
> >-Original Message-
> >From: Ananyev, Konstantin
> >Sent: Saturday, May 18, 2019 8:51 PM
> >To: Sunil Kumar Kori ; dev@dpdk.org
> >Subject: [EXT] RE: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation:
> >Enabling IP checksum offload in mbuf
> >
> >External Email
> >
> >--
> >
> >
> >> -Original Message-
> >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Sunil Kumar Kori
> >> Sent: Thursday, May 16, 2019 12:42 PM
> >> To: dev@dpdk.org
> >> Cc: sk...@marvell.com
> >> Subject: [dpdk-dev] [PATCH v2 2/3] examples/ip_fragmentation: Enabling IP
> >checksum offload in mbuf
> >>
> >> As per the documentation to use any IP offload features, application
> >> must set required offload flags into mbuf->ol_flags.
> >>
> >> Signed-off-by: Sunil Kumar Kori
> >> ---
> >> examples/ip_fragmentation/main.c | 7 +--
> >> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/examples/ip_fragmentation/main.c
> >b/examples/ip_fragmentation/main.c
> >> index e90a61e..d821967 100644
> >> --- a/examples/ip_fragmentation/main.c
> >> +++ b/examples/ip_fragmentation/main.c
> >> @@ -354,10 +354,13 @@ struct rte_lpm6_config lpm6_config = {
> >>
> >>/* src addr */
> >>ether_addr_copy(&ports_eth_addr[port_out], ð_hdr-
> >>s_addr);
> >> - if (ipv6)
> >> + if (ipv6) {
> >>eth_hdr->ether_type =
> >rte_be_to_cpu_16(ETHER_TYPE_IPv6);
> >> - else
> >> + m->ol_flags |= PKT_TX_IPV6;
> >
> >Is there is any point to do that?
> >This sample app, as I remember doesn't request any extra HW offloads.
> >Same comment for patch #3.
> >Konstantin
> >
> >
> Is this comment valid for IPv6 only ?
Yes, my comment is about the following line of code:
m->ol_flags |= PKT_TX_IPV6;
> Otherwise for IPv4, it is required as IP checksum is neither being updated by
> library nor
> by the application.
> >> + } else {
> >>eth_hdr->ether_type =
> >rte_be_to_cpu_16(ETHER_TYPE_IPv4);
> >> + m->ol_flags |= (PKT_TX_IPV4 | PKT_TX_IP_CKSUM);
> >> + }
> >>}
> >>
> >>len += len2;
> >> --
> >> 1.8.3.1
[dpdk-dev] [PATCH v1] net/ice: update the RSS RETA size with support values
Since ice can support 128, 512, 2K RSS RETA size value, change the
update API to set it to resize the RSS RETA table. And by default,
use 512 to sync with ETH_RSS_RETA_SIZE_x maximum value definition.
Also the flag ICE_FLAG_RSS_AQ_CAPABLE is missed to set.
Fixes: 690175ee51bf ("net/ice: support getting device information")
Fixes: ff963bfa7cb1 ("net/ice: support RSS")
Signed-off-by: Haiyue Wang
---
drivers/net/ice/ice_ethdev.c | 41 +++--
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index bbaa7cf..c4ea09f 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -1149,6 +1149,12 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
TAILQ_INIT(&vsi->mac_list);
TAILQ_INIT(&vsi->vlan_list);
+ /* Be sync with ETH_RSS_RETA_SIZE_x maximum value definition */
+ pf->hash_lut_size = hw->func_caps.common_cap.rss_table_size >
+ ETH_RSS_RETA_SIZE_512 ? ETH_RSS_RETA_SIZE_512 :
+ hw->func_caps.common_cap.rss_table_size;
+ pf->flags |= ICE_FLAG_RSS_AQ_CAPABLE;
+
memset(&vsi_ctx, 0, sizeof(vsi_ctx));
/* base_queue in used in queue mapping of VSI add/update command.
* Suppose vsi->base_queue is 0 now, don't consider SRIOV, VMDQ
@@ -1627,7 +1633,7 @@ static int ice_init_rss(struct ice_pf *pf)
rss_conf = &dev->data->dev_conf.rx_adv_conf.rss_conf;
nb_q = dev->data->nb_rx_queues;
vsi->rss_key_size = ICE_AQC_GET_SET_RSS_KEY_DATA_RSS_KEY_SIZE;
- vsi->rss_lut_size = hw->func_caps.common_cap.rss_table_size;
+ vsi->rss_lut_size = pf->hash_lut_size;
if (is_safe_mode) {
PMD_DRV_LOG(WARNING, "RSS is not supported in safe mode\n");
@@ -2033,7 +2039,7 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
dev_info->rx_queue_offload_capa = 0;
dev_info->tx_queue_offload_capa = 0;
- dev_info->reta_size = hw->func_caps.common_cap.rss_table_size;
+ dev_info->reta_size = pf->hash_lut_size;
dev_info->hash_key_size = (VSIQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);
dev_info->default_rxconf = (struct rte_eth_rxconf) {
@@ -2605,28 +2611,31 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
uint16_t reta_size)
{
struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- uint16_t i, lut_size = hw->func_caps.common_cap.rss_table_size;
+ uint16_t i, lut_size = pf->hash_lut_size;
uint16_t idx, shift;
uint8_t *lut;
int ret;
- if (reta_size != lut_size ||
- reta_size > ETH_RSS_RETA_SIZE_512) {
+ if (reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128 &&
+ reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512 &&
+ reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K) {
PMD_DRV_LOG(ERR,
"The size of hash lookup table configured (%d)"
"doesn't match the number hardware can "
- "supported (%d)",
- reta_size, lut_size);
+ "supported (128, 512, 2048)",
+ reta_size);
return -EINVAL;
}
- lut = rte_zmalloc(NULL, reta_size, 0);
+ /* It MUST use the current LUT size to get the RSS lookup table,
+* otherwise if will fail with -100 error code.
+*/
+ lut = rte_zmalloc(NULL, RTE_MAX(reta_size, lut_size), 0);
if (!lut) {
PMD_DRV_LOG(ERR, "No memory can be allocated");
return -ENOMEM;
}
- ret = ice_get_rss_lut(pf->main_vsi, lut, reta_size);
+ ret = ice_get_rss_lut(pf->main_vsi, lut, lut_size);
if (ret)
goto out;
@@ -2637,6 +2646,12 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
lut[i] = reta_conf[idx].reta[shift];
}
ret = ice_set_rss_lut(pf->main_vsi, lut, reta_size);
+ if (ret == 0 && lut_size != reta_size) {
+ PMD_DRV_LOG(INFO,
+ "The size of hash lookup table is changed from (%d)
to (%d)",
+ lut_size, reta_size);
+ pf->hash_lut_size = reta_size;
+ }
out:
rte_free(lut);
@@ -2650,14 +2665,12 @@ ice_rss_reta_query(struct rte_eth_dev *dev,
uint16_t reta_size)
{
struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- uint16_t i, lut_size = hw->func_caps.common_cap.rss_table_size;
+ uint16_t i, lut_size = pf->hash_lut_size;
uint16_t idx, shift;
uint8_t *lut;
int ret;
- if (reta_size != lut_size ||
-
Re: [dpdk-dev] Instability of port ids
On 18/05/2019 14:33, Stephen Hemminger wrote: > On Sat, 18 May 2019 06:03:22 + > "Wang, Haiyue" wrote: > >>> -Original Message- >>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Stephen Hemminger >>> Sent: Saturday, May 18, 2019 02:47 >>> To: dev@dpdk.org >>> Subject: [dpdk-dev] Instability of port ids >>> >>> Several customers have reported similar issues with how the owned/stack >>> device model >>> works in DPDK. With failsafe/tap and VF or netvsc and VF there are DPDK >>> ports which >>> are marked as owned and therefore not visible. >>> >>> The problem is the application has to guess and workaround these port >>> values in >>> the port mask that gets passed in on command line. This means a working >>> application >>> has to modify its startup script to run on Azure. Worse the actual port >>> values >>> change based on the number of NIC's configured. >>> >>> Overall this is a nuisance for users. The whole DPDK port index concept is >>> a bad >>> design. In Linux/BSD there is ifindex, but few applications care, they all >>> use names >>> which is better. Very very few application care that eth1 is ifindex 4. >>> >>> The whole assignment of ports is a mess as well since it is based on probe >>> order >>> and that is based on PCI order, and not anything dependable. It gets worse >>> with >>> command line arguments, vdev, owned devices etc. >>> >>> All I can think of is that: >>> * DPDK network devices need to have human readable names. current PCI is >>> not good. >>> * The names need to be repeatable/persistent. udev names are probably >>> better than anything so far. >>> Or bsd style names but they end up being device dependent. >>> * The API to get from name to port needs to easy to use and the preferred >>> method. >>> * All examples and documentation should avoid using port index directly. >>> You need port for fast rx/tx but setup should be by name. >> >> idea from system like enp24s0f0 ? >> https://github.com/systemd/systemd/blob/master/docs/PREDICTABLE_INTERFACE_NAMES.md >> https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c >> > > The other suggestion is to use an algorithm like VPP which generates > TenGigabitEthernet0/2/2 > That was going to be my suggestion also ... :-) Either/or I would be fine with, as you say indexes are unreliable, and that only get's worse as you look at ephemeral interfaces like veth. One comment though. What I really do like about uDev/SystemD is that it is highly user configurable, that the end user can set a udev to influence the naming scheme. Thanks, Ray K Ray K
[dpdk-dev] [PATCH] doc: fix helloworld build on Windows
From: Adham Masarwah
The option -Dexamples=helloworld is missing.
The helloworld binary name was wrong.
Forcing clang may be required in some environments.
Fixes: 196c650b8b63 ("doc: add guide for Windows")
Signed-off-by: Adham Masarwah
---
doc/guides/windows_gsg/build_dpdk.rst | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/doc/guides/windows_gsg/build_dpdk.rst
b/doc/guides/windows_gsg/build_dpdk.rst
index f48805236..6711e07e2 100644
--- a/doc/guides/windows_gsg/build_dpdk.rst
+++ b/doc/guides/windows_gsg/build_dpdk.rst
@@ -59,22 +59,30 @@ default.
Using the ninja backend
+Specifying the compiler might be required to complete the meson command.
+
+.. code-block:: console
+
+set CC=clang
+
+To compile the examples, the flag ``-Dexamples`` is required.
+
.. code-block:: console
cd C:\Users\me\dpdk
-meson build
+meson -Dexamples=helloworld build
cd build
ninja
Run the helloworld example
==
-Navigate to the build directory and run `dpdk-helloworld.exe`.
+Navigate to the examples in the build directory and run `dpdk-helloworld.exe`.
.. code-block:: console
-cd C:\Users\me\dpdk\build
-helloworld.exe
+cd C:\Users\me\dpdk\build\examples
+dpdk-helloworld.exe
hello from core 1
hello from core 3
hello from core 0
--
2.21.0.windows.1
Re: [dpdk-dev] [PATCH 2/3] net/i40e: add runtime option to disable vector rx
Hi
>
> Vector RX functions are not at feature parity with non-vector ones and
> currently the vector RX path is enabled by default. Hence, the only
> option to force selection of non-vector variants and be able to retain
> functionality is to disable vector PMD globally at compile time via the
> config file option.
>
> This patch introduces a new runtime device config option named
> disable-vec-rx to allow users to limit the driver to make a choice among
> non-vector RX functions, on a per device basis. This runtime option
> defaults to a value of zero, allowing vector RX functions to be
> selected (current behavior). When disable-vec-rx=1 is specified for a
> device, even if all the other requirements to select a vector RX
> function are satisfied, the driver will still pick one out of the
> appropriate non-vector RX functions.
Not sure I understand the logic of that patch.
If i40e RX PMD selects wrong RX function that doesn't provide
requested by user functionality (which one?) -
then it is a bug in i40e RX function selection code that needs to be fixed.
No point to introduce some new config options for that.
Konstantin
>
> Signed-off-by: Mesut Ali Ergin
> ---
> doc/guides/nics/i40e.rst | 14 +
> drivers/net/i40e/i40e_ethdev.c | 70
> +-
> drivers/net/i40e/i40e_ethdev.h | 1 +
> drivers/net/i40e/i40e_rxtx.c | 4 +++
> 4 files changed, 88 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
> index a97484c..532cc64 100644
> --- a/doc/guides/nics/i40e.rst
> +++ b/doc/guides/nics/i40e.rst
> @@ -183,6 +183,20 @@ Runtime Config Options
>
>-w 84:00.0,use-latest-supported-vec=1
>
> +- ``Disable RX Vector Functions `` (default ``vector RX enabled``)
> +
> + When config file option for the vector PMD is enabled, vector RX functions
> may
> + be the default choice of the driver at device initialization time, if
> certain
> + conditions apply. However, vector RX functions may not always be at feature
> + parity with non-vector ones. In order to allow users to override vector RX
> + function selection behavior at runtime, the ``devargs`` parameter
> + ``disable-vec-rx`` is introduced, such that
> +
> + -w DBDF,disable-vec-rx=1
> +
> + would force driver to limit its choices to non-vector RX function variants
> for
> + the device specified by the DBDF.
> +
> Vector RX Pre-conditions
>
> For Vector RX it is assumed that the number of descriptor rings will be a
> power
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index cab440f..19fbd23 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -44,6 +44,7 @@
> #define ETH_I40E_SUPPORT_MULTI_DRIVER"support-multi-driver"
> #define ETH_I40E_QUEUE_NUM_PER_VF_ARG"queue-num-per-vf"
> #define ETH_I40E_USE_LATEST_VEC "use-latest-supported-vec"
> +#define ETH_I40E_DISABLE_VEC_RX "disable-vec-rx"
>
> #define I40E_CLEAR_PXE_WAIT_MS 200
>
> @@ -410,6 +411,7 @@ static const char *const valid_keys[] = {
> ETH_I40E_SUPPORT_MULTI_DRIVER,
> ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> ETH_I40E_USE_LATEST_VEC,
> + ETH_I40E_DISABLE_VEC_RX,
> NULL};
>
> static const struct rte_pci_id pci_id_i40e_map[] = {
> @@ -1263,6 +1265,68 @@ i40e_use_latest_vec(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +i40e_parse_disable_vec_rx_handler(__rte_unused const char *key,
> + const char *value,
> + void *opaque)
> +{
> + struct i40e_adapter *ad;
> +
> + ad = (struct i40e_adapter *)opaque;
> +
> + switch (atoi(value)) {
> + case 0:
> + /* Selection of RX vector functions left untouched*/
> + break;
> + case 1:
> + /* Disable RX vector functions as requested*/
> + ad->rx_vec_allowed = false;
> + break;
> + default:
> + PMD_DRV_LOG(WARNING, "Value should be 0 or 1, set it as 1!");
> + break;
> + }
> +
> + return 0;
> +}
> +
> +int
> +i40e_disable_vec_rx(struct rte_eth_dev *dev)
> +{
> + struct i40e_adapter *ad =
> + I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> + struct rte_kvargs *kvlist;
> + int kvargs_count;
> +
> +
> + if (!dev->device->devargs)
> + return 0;
> +
> + kvlist = rte_kvargs_parse(dev->device->devargs->args, valid_keys);
> + if (!kvlist)
> + return -EINVAL;
> +
> + kvargs_count = rte_kvargs_count(kvlist, ETH_I40E_DISABLE_VEC_RX);
> + if (!kvargs_count) {
> + rte_kvargs_free(kvlist);
> + return 0;
> + }
> +
> + if (kvargs_count > 1)
> + PMD_DRV_LOG(WARNING, "More than one argument \"%s\" and only "
> + "the first invalid or last valid one is used !",
> + ETH_I40E_DISABLE
Re: [dpdk-dev] [PATCH v2 5/5] eal/x86: force inlining of all memcpy and mov helpers
On Fri, May 17, 2019 at 5:14 PM Maxime Coquelin wrote: > Some helpers in the header file are forced inlined other are > only inlined, this patch forces inline for all. > > It will avoid it to be embedded as functions when called multiple > times in the same object file. For example, when we added packed > ring support in vhost-user library, rte_memcpy_generic got no > more inlined. > Weird that we have only some functions marked as always inlined in commit: https://git.dpdk.org/dpdk/commit/?id=1c9467a6efd8d85b5bbbf7004a4407cae2d09431 Bruce, is there a reason for this? -- David Marchand
Re: [dpdk-dev] [PATCH] doc: fix helloworld build on Windows
On Mon, May 20, 2019 at 11:19:52AM +0300, ad...@mellanox.com wrote:
> From: Adham Masarwah
>
> The option -Dexamples=helloworld is missing.
> The helloworld binary name was wrong.
> Forcing clang may be required in some environments.
>
> Fixes: 196c650b8b63 ("doc: add guide for Windows")
>
> Signed-off-by: Adham Masarwah
> ---
Acked-by: Bruce Richardson
Re: [dpdk-dev] [RFC v8] /net: memory interface (memif)
> -Original Message-
> From: Stephen Hemminger
> Sent: Thursday, May 16, 2019 5:22 PM
> To: Jakub Grajciar
>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC v8] /net: memory interface (memif)
>
> On Thu, 16 May 2019 13:46:58 +0200
> Jakub Grajciar wrote:
>
> > +enum memif_role_t {
> > + MEMIF_ROLE_MASTER,
> > + MEMIF_ROLE_SLAVE,
> > +};
>
> Because master/slave terminology is potentially culturally offensive it is
> flagged by many corporate source scanning tools.
>
> Could you use primary/secondary in memif instead?
Other implementations also use master/slave terminology, so changing it would
be confusing. However, we will consider using primary/secondary in next
protocol version.
[dpdk-dev] [RFC v9] /net: memory interface (memif)
Memory interface (memif), provides high performance packet transfer over shared memory. Signed-off-by: Jakub Grajciar --- MAINTAINERS |6 + config/common_base |5 + config/common_linux |1 + doc/guides/nics/features/memif.ini | 14 + doc/guides/nics/index.rst |1 + doc/guides/nics/memif.rst | 234 doc/guides/rel_notes/release_19_08.rst |5 + drivers/net/Makefile|1 + drivers/net/memif/Makefile | 30 + drivers/net/memif/memif.h | 179 +++ drivers/net/memif/memif_socket.c| 1124 + drivers/net/memif/memif_socket.h| 105 ++ drivers/net/memif/meson.build | 13 + drivers/net/memif/rte_eth_memif.c | 1198 +++ drivers/net/memif/rte_eth_memif.h | 212 drivers/net/memif/rte_pmd_memif_version.map |4 + drivers/net/meson.build |1 + mk/rte.app.mk |1 + 18 files changed, 3134 insertions(+) create mode 100644 doc/guides/nics/features/memif.ini create mode 100644 doc/guides/nics/memif.rst create mode 100644 drivers/net/memif/Makefile create mode 100644 drivers/net/memif/memif.h create mode 100644 drivers/net/memif/memif_socket.c create mode 100644 drivers/net/memif/memif_socket.h create mode 100644 drivers/net/memif/meson.build create mode 100644 drivers/net/memif/rte_eth_memif.c create mode 100644 drivers/net/memif/rte_eth_memif.h create mode 100644 drivers/net/memif/rte_pmd_memif_version.map requires patch: http://patchwork.dpdk.org/patch/51511/ Memif uses a unix domain socket as control channel (cc). rte_interrupts.h is used to handle interrupts for this cc. This patch is required, because the message received can be a reason for disconnecting. v3: - coding style fixes - documentation - doxygen comments - use strlcpy() instead of strncpy() - use RTE_BUILD_BUG_ON instead of _Static_assert() - fix meson build deps v4: - coding style fixes - doc update (desc format, messaging, features, ...) - pointer arithmetic fix - __rte_packed and __rte_aligned instead of __attribute__() - fixed multi-queue - support additional archs (memfd_create syscall) v5: - coding style fixes - add release notes - remove unused dependencies - doc update v6: - remove socket on termination of testpmd application - disallow memif probing in secondary process (return error), multi-process support will be implemented in separate patch - fix chained buffer packet length - use single region for rings and buffers (non-zero-copy) - doc update v7: - coding style fixes v8: - release notes: 19.05 -> 19.08 v9: - rearrange elements in structures to remove holes - default socket file in /run - #define name sizes - implement dev_close op diff --git a/MAINTAINERS b/MAINTAINERS index 15d0829c5..3698c146b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -841,6 +841,12 @@ F: drivers/net/softnic/ F: doc/guides/nics/features/softnic.ini F: doc/guides/nics/softnic.rst +Memif PMD +M: Jakub Grajciar +F: drivers/net/memif/ +F: doc/guides/nics/memif.rst +F: doc/guides/nics/features/memif.ini + Crypto Drivers -- diff --git a/config/common_base b/config/common_base index 6b96e0e80..dca119a65 100644 --- a/config/common_base +++ b/config/common_base @@ -444,6 +444,11 @@ CONFIG_RTE_LIBRTE_PMD_AF_PACKET=n # CONFIG_RTE_LIBRTE_PMD_AF_XDP=n +# +# Compile Memory Interface PMD driver (Linux only) +# +CONFIG_RTE_LIBRTE_PMD_MEMIF=n + # # Compile link bonding PMD library # diff --git a/config/common_linux b/config/common_linux index 75334273d..87514fe4f 100644 --- a/config/common_linux +++ b/config/common_linux @@ -19,6 +19,7 @@ CONFIG_RTE_LIBRTE_VHOST_POSTCOPY=n CONFIG_RTE_LIBRTE_PMD_VHOST=y CONFIG_RTE_LIBRTE_IFC_PMD=y CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y +CONFIG_RTE_LIBRTE_PMD_MEMIF=y CONFIG_RTE_LIBRTE_PMD_SOFTNIC=y CONFIG_RTE_LIBRTE_PMD_TAP=y CONFIG_RTE_LIBRTE_AVP_PMD=y diff --git a/doc/guides/nics/features/memif.ini b/doc/guides/nics/features/memif.ini new file mode 100644 index 0..807d9ecdc --- /dev/null +++ b/doc/guides/nics/features/memif.ini @@ -0,0 +1,14 @@ +; +; Supported features of the 'memif' network poll mode driver. +; +; Refer to default.ini for the full list of available PMD features. +; +[Features] +Link status = Y +Basic stats = Y +Jumbo frame = Y +ARMv8= Y +Power8 = Y +x86-32 = Y +x86-64 = Y +Usage doc= Y diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst index 2221c35f2..691e7209f 100644 --- a/doc/guides/nics/index.rst +++ b/doc/guides/nics/index.rst @@ -36,6 +36,7 @@ Network Interface Controller Drivers intel_vf kni liquidio +memif mlx4 mlx5 mvneta diff --git a/doc/guides/nics/memif.rs
[dpdk-dev] Generating Debug information in Windows using Clang (PDB files)
Hi, In development we use WinDbg for debugging, so we need to create PDB files when compiling the DPDK, so we used ``-g`` in the CFLGAS and the PDB files are being created. But when running the helloworld with WinDbg, we can see only function names, we can't see code neither variables, we get the following error: "Private symbols (symbols.pri) are required for locals." We tried to append some flags like -Z7 and -gcodeview, but the compiler ignores them. Do you have any idea what we miss here ? What is the debug mechanism is being used in Windows ? Thanks
Re: [dpdk-dev] Hugepages not being deleted
On 17-May-19 3:06 PM, David Marchand wrote: Hello Anatoly, Not sure what the issue is at the moment. I too have an error on the eal_flags_autotest but what is worrying me is that it works fine in 18.11.1 and 19.02. Is there anything that rings a bell? Thanks. -- David Marchand Not to me. Last i checked they were passing. -- Thanks, Anatoly
[dpdk-dev] [PATCH v3 1/2] examples/ip_fragmentation: Enabling IP checksum offload in mbuf
As per the documentation to use any IP offload features, application
must set required offload flags into mbuf->ol_flags.
Signed-off-by: Sunil Kumar Kori
---
examples/ip_fragmentation/main.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
index e90a61e..719d6f4 100644
--- a/examples/ip_fragmentation/main.c
+++ b/examples/ip_fragmentation/main.c
@@ -354,10 +354,12 @@ struct rte_lpm6_config lpm6_config = {
/* src addr */
ether_addr_copy(&ports_eth_addr[port_out], ð_hdr->s_addr);
- if (ipv6)
+ if (ipv6) {
eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv6);
- else
+ } else {
eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv4);
+ m->ol_flags |= (PKT_TX_IPV4 | PKT_TX_IP_CKSUM);
+ }
}
len += len2;
--
1.8.3.1
[dpdk-dev] [PATCH v3 2/2] examples/ip_reassembly: Enabling IP checksum offload in mbuf
As per the documentation to use any IP offload features, application
must set required offload flags into mbuf->ol_flags.
Signed-off-by: Sunil Kumar Kori
---
examples/ip_reassembly/main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index 17b55d4..0ad526c 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -353,6 +353,9 @@ struct rte_lpm6_config lpm6_config = {
struct ether_hdr *);
ip_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
}
+
+ /* update offloading flags */
+ m->ol_flags |= (PKT_TX_IPV4 | PKT_TX_IP_CKSUM);
}
ip_dst = rte_be_to_cpu_32(ip_hdr->dst_addr);
--
1.8.3.1
Re: [dpdk-dev] Hugepages not being deleted
On Mon, May 20, 2019 at 12:43 PM Burakov, Anatoly wrote: > On 17-May-19 3:06 PM, David Marchand wrote: > > Hello Anatoly, > > > > Not sure what the issue is at the moment. > > I too have an error on the eal_flags_autotest but what is worrying me is > > that it works fine in 18.11.1 and 19.02. > > Is there anything that rings a bell? > > > > Thanks. > > > > -- > > David Marchand > > Not to me. Last i checked they were passing. > Bisected it, I will send a fix. -- David Marchand
Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
Hi Konstantin,
Thank you for the review.
I will send a revised patch which addresses your comments.
Thanks,
Lukasz
On 19.05.2019 16:47, Ananyev, Konstantin wrote:
>
> Hi Lukasz,
> Thanks for clarifications.
> Looks good in general.
> Few small comments below.
> Konstantin
>
>> When esn is used then high-order 32 bits are included in ICV
>> calculation however are not transmitted. Update packet length
>> to be consistent with auth data offset and length before crypto
>> operation. High-order 32 bits of esn will be removed from packet
>> length in crypto post processing.
>>
>> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
>> Signed-off-by: Lukasz Bartosik
>> ---
>> lib/librte_ipsec/esp_inb.c | 56
>> +
>> lib/librte_ipsec/esp_outb.c | 31 +
>> lib/librte_ipsec/sa.c | 4 ++--
>> lib/librte_ipsec/sa.h | 8 +++
>> 4 files changed, 87 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
>> index 4e0e12a..eb899e3 100644
>> --- a/lib/librte_ipsec/esp_inb.c
>> +++ b/lib/librte_ipsec/esp_inb.c
>> @@ -16,7 +16,8 @@
>> #include "pad.h"
>>
>> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
>> -struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
>> +struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
>> +uint8_t is_inline);
>>
>> /*
>> * helper function to fill crypto_sym op for cipher+auth algorithms.
>> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const
>> struct replay_sqn *rsn,
>> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
>> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>>
>> +/*
>> + * if esn is used then high-order 32 bits are also used in ICV
>> + * calculation but are not transmitted, update packet length
>> + * to be consistent with auth data length and offset, this will
>> + * be subtracted from packet length in post crypto processing
>
> Here and in several other comments below - you repeat basically the same
> thing.
> Seems a bit excessive. I suppose just to put in in one place, or probably even
> in patch description will be enough.
>
>> + */
>> +mb->pkt_len += sa->sqh_len;
>> +ml->data_len += sa->sqh_len;
>> +
>> inb_pkt_xprepare(sa, sqn, icv);
>> return plen;
>> }
>> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t
>> txof_msk, uint64_t txof_val)
>> */
>> static inline uint16_t
>> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> -uint32_t sqn[], uint32_t dr[], uint16_t num)
>> +uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> uint32_t adj, i, k, tl;
>> uint32_t hl[num];
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> -const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> +/*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>
> This comment seems a bit misleading, as we have remove not only sqh,
> but also icv, espt, padding.
>
>> +const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> +(is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct
>> rte_mbuf *mb[],
>> */
>> static inline uint16_t
>> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> -uint32_t sqn[], uint32_t dr[], uint16_t num)
>> +uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> char *np;
>> uint32_t i, k, l2, tl;
>> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct
>> rte_mbuf *mb[],
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> -const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> +/*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>> +const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> +(is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const
>> uint32_t sqn[],
>> * process group of ESP inbound packets.
>> */
>> static inline uint16_t
>> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> -struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
>> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf
>> *mb[],
>> +uint16_t num, uint8_t is_inline, esp_inb_process_t process)
>> {
>> uint32_t k, n;
>> struct rte_ipsec_sa *sa;
>> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const stru
Re: [dpdk-dev] [PATCH v3 1/2] examples/ip_fragmentation: Enabling IP checksum offload in mbuf
> As per the documentation to use any IP offload features, application
> must set required offload flags into mbuf->ol_flags.
>
> Signed-off-by: Sunil Kumar Kori
> ---
> examples/ip_fragmentation/main.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/examples/ip_fragmentation/main.c
> b/examples/ip_fragmentation/main.c
> index e90a61e..719d6f4 100644
> --- a/examples/ip_fragmentation/main.c
> +++ b/examples/ip_fragmentation/main.c
> @@ -354,10 +354,12 @@ struct rte_lpm6_config lpm6_config = {
>
> /* src addr */
> ether_addr_copy(&ports_eth_addr[port_out], ð_hdr->s_addr);
> - if (ipv6)
> + if (ipv6) {
> eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv6);
> - else
> + } else {
> eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv4);
> + m->ol_flags |= (PKT_TX_IPV4 | PKT_TX_IP_CKSUM);
> + }
> }
>
> len += len2;
> --
Acked-by: Konstantin Ananyev
> 1.8.3.1
Re: [dpdk-dev] [PATCH v3 2/2] examples/ip_reassembly: Enabling IP checksum offload in mbuf
> As per the documentation to use any IP offload features, application
> must set required offload flags into mbuf->ol_flags.
>
> Signed-off-by: Sunil Kumar Kori
> ---
> examples/ip_reassembly/main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
> index 17b55d4..0ad526c 100644
> --- a/examples/ip_reassembly/main.c
> +++ b/examples/ip_reassembly/main.c
> @@ -353,6 +353,9 @@ struct rte_lpm6_config lpm6_config = {
> struct ether_hdr *);
> ip_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
> }
> +
> + /* update offloading flags */
> + m->ol_flags |= (PKT_TX_IPV4 | PKT_TX_IP_CKSUM);
> }
> ip_dst = rte_be_to_cpu_32(ip_hdr->dst_addr);
>
> --
Acked-by: Konstantin Ananyev
> 1.8.3.1
Re: [dpdk-dev] [PATCH 01/11] Add HW registers
On 5/20/2019 9:31 AM, Ziyang Xuan wrote: > --- > drivers/net/hinic/base/hinic_csr.h | 172 > + > 1 file changed, 172 insertions(+) > create mode 100644 drivers/net/hinic/base/hinic_csr.h Hi Xuan, Your mails are not reached to the mail list because of "Post by non-member to a members-only list", technically we can release those mails but I prefer not to do, because it can be better to fix a few things and try again: 1- It is better to register to the mail list, we will have many iterations after this point and it is good to clarify that communication is not blocked. 2- Mail is not threaded, each patch sent as individual patch, this makes it hard to follow. Can you please use following git send-email options while sending the patches: sendemail.smtpserver=smtp.intel.com sendemail.chainreplyto=false 3- Patches are missing sign-off, can you please add them before sending? And this may mean './devtools/checkpatches.sh' & './devtools/check-git-log.sh' scripts not run before sending the patches, can you please verify they pass before sending the patches? Thanks, ferruh
Re: [dpdk-dev] Hugepages not being deleted
On 20-May-19 12:09 PM, David Marchand wrote: On Mon, May 20, 2019 at 12:43 PM Burakov, Anatoly mailto:anatoly.bura...@intel.com>> wrote: On 17-May-19 3:06 PM, David Marchand wrote: > Hello Anatoly, > > Not sure what the issue is at the moment. > I too have an error on the eal_flags_autotest but what is worrying me is > that it works fine in 18.11.1 and 19.02. > Is there anything that rings a bell? > > Thanks. > > -- > David Marchand Not to me. Last i checked they were passing. Bisected it, I will send a fix. -- David Marchand I've bisected it too. I think Erik (author of original patch) already is working on the problem. I don't think it's easily (and safely) solvable. You can find more info in below threads: http://patches.dpdk.org/patch/53191/ http://patches.dpdk.org/patch/53268/ -- Thanks, Anatoly
Re: [dpdk-dev] Hugepages not being deleted
On Mon, May 20, 2019 at 1:45 PM Burakov, Anatoly wrote: > On 20-May-19 12:09 PM, David Marchand wrote: > > On Mon, May 20, 2019 at 12:43 PM Burakov, Anatoly > > mailto:anatoly.bura...@intel.com>> wrote: > > > > On 17-May-19 3:06 PM, David Marchand wrote: > > > Hello Anatoly, > > > > > > Not sure what the issue is at the moment. > > > I too have an error on the eal_flags_autotest but what is > > worrying me is > > > that it works fine in 18.11.1 and 19.02. > > > Is there anything that rings a bell? > > > > > > Thanks. > > > > > > -- > > > David Marchand > > > > Not to me. Last i checked they were passing. > > > > > > Bisected it, I will send a fix. > > > > > > -- > > David Marchand > > I've bisected it too. I think Erik (author of original patch) already is > working on the problem. I don't think it's easily (and safely) solvable. > You can find more info in below threads: > > http://patches.dpdk.org/patch/53191/ > http://patches.dpdk.org/patch/53268/ Thanks, indeed I was on this issue. -- David Marchand
Re: [dpdk-dev] [PATCH] app/testpmd: change port detach interface
On Fri, May 17, 2019 at 10:59:38AM +0200, Thomas Monjalon wrote: > 17/05/2019 10:55, Nithin Dabilpuram: > > On Wed, May 15, 2019 at 09:27:22AM +0200, Thomas Monjalon wrote: > > > 15/05/2019 08:52, Nithin Dabilpuram: > > > > Hi Thomas, > > > > On Tue, May 14, 2019 at 05:39:30PM +0200, Thomas Monjalon wrote: > > > > > Hi, > > > > > > > > > > 13/05/2019 13:21, Nithin Dabilpuram: > > > > > > With the latest published interface of > > > > > > rte_eal_hotplug_[add,remove](), and rte_eth_dev_close(), > > > > > > rte_eth_dev_close() would cleanup all the data structures of > > > > > > port's eth dev leaving the device common resource intact > > > > > > if RTE_ETH_DEV_CLOSE_REMOVE is set in dev flags. > > > > > > So "port detach" (~hotplug remove) should be able to work, > > > > > > with device identifier like "port attach" as eth_dev could have > > > > > > been closed already and rte_eth_devices[port_id] reused. > > > > > > > > > > "port attach" uses devargs as identifier because there > > > > > is no port id before creating it. But "detach port" uses > > > > > logically the port id to close. > > > > > > > > But if "port close" was already called on that port, > > > > eth_dev->state would be set as RTE_ETH_DEV_UNUSED and > > > > that port id could be reused. > > > > So after "port close" if we call "port detach", isn't it > > > > incorrect to use the same port id ? > > > > > > Yes it is incorrect to close a port which is already closed :) > > > > > > > > > This change alters "port detach" cmdline interface to > > > > > > work with device identifier like "port attach". > > > > > > > > > > The word "port" means an ethdev port, so it should be > > > > > referenced with a port id. > > > > > If you want to close an EAL rte_device, then you should > > > > > rename the command. > > > > > But testpmd purpose should be to work with ethdev ports only. > > > > > > > > Renaming the command to "detach " ? > > > > > > Yes something like that. > > > But why do you want to manage rte_device in testpmd? > > > Being able to close ports in not enough? > > > Please describe a scenario. > > > > > > > We just want to support testing hotplug detach along with > > hotplug attach from testpmd. Currently there is no way to detach > > if we close the port first. > > OK So can I send next revision with command renamed to "detach " ? > > > Another reason is that in our new PMD, for detaching one specific port, > > we need more than one try as the PMD might return -EAGAIN. > > So with the current "port detach" implementation, after closing the port, > > if PMD returns -EAGAIN for rte_dev_remove() call, there is no way to > > try it again. > > This is a bug. > Should we catch -EAGAIN somewhere? It is already caught in local_dev_remove() and rte_dev_remove() fails. Only problem as I said below is in testpmd if first call to detach_port_device() i.e handler of "port detach", rte_dev_remove() returns -EAGAIN and PMD cleaned up the resources partially like eth_dev resources, the second time call cannot work port_id will not be valid anymore. > >
Re: [dpdk-dev] [PATCH v1] examples/l3fwd-power: add telemetry mode support
On 17-May-19 7:17 PM, Reshma Pattan wrote:
Add new telemetry mode support for l3fwd-power.
This is a standalone mode, in this mode l3fwd-power
does simple l3fwding along with calculating
empty polls, full polls, and busy percentage for
each forwarding core. The aggregation of these
values of all cores is reported as application
level telemetry to metric library for every 10ms from the
master core.
The busy percentage is calculated by recording the poll_count
and when the count reaches a defined value the total
cycles it took is measured and compared with minimum and maximum
reference cycles and busy rate is set according to either 0% or
50% or 100%.
Signed-off-by: Reshma Pattan
---
+ poll_count = 0;
+ prev_tel_tsc = cur_tsc;
+ /* update stats for telemetry */
+ rte_spinlock_lock(&stats[lcore_id].telemetry_lock);
+ stats[lcore_id].ep_nep[0] = ep_nep[0];
+ stats[lcore_id].ep_nep[1] = ep_nep[1];
+ stats[lcore_id].fp_nfp[0] = fp_nfp[0];
+ stats[lcore_id].fp_nfp[1] = fp_nfp[1];
+ stats[lcore_id].br = br;
+ rte_spinlock_unlock(&stats[lcore_id].telemetry_lock);
Locking here seems relatively rare (per-lcore and once every N polls),
but any locking on a hotpath makes me nervous. What is the current
performance impact of this? Should we bother improving?
+ }
+ }
+
+ return 0;
+}
+/* main processing loop */
+static int
main_empty_poll_loop(__attribute__((unused)) void *dummy)
{
struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
@@ -1274,7 +1455,9 @@ print_usage(const char *prgname)
" which max packet len is PKTLEN in decimal (64-9600)\n"
" --parse-ptype: parse packet type by software\n"
" --empty-poll: enable empty poll detection"
- " follow (training_flag, high_threshold, med_threshold)\n",
+ " follow (training_flag, high_threshold, med_threshold)\n"
+ " --telemetry: enable telemetry mode, to upadte"
Update :)
+ " empty polls, full polls, and core busyness to telemetry\n",
I don't particularly like the term "busyness". Load? Workload? I'm OK
with keeping as is though, just a personal preference :)
prgname);
}
@@ -1417,6 +1600,7 @@ parse_ep_config(const char *q_arg)
}
#define CMD_LINE_OPT_PARSE_PTYPE "parse-ptype"
+#define CMD_LINE_OPT_TELEMETRY "telemetry"
if (!strncmp(lgopts[option_index].name,
@@ -1869,6 +2068,52 @@ init_power_library(void)
return ret;
}
static void
+update_telemetry(__attribute__((unused)) struct rte_timer *tim,
+ __attribute__((unused)) void *arg)
+{
I would question the need to put telemetry on a high precision 10ms
timer. Is there any reason why we cannot gather telemetry, say, once
every 100ms, and why we cannot do so from interrupt thread using alarm
API? Using high-precision timer API here seems like an overkill.
+ unsigned int lcore_id = rte_lcore_id();
+ struct lcore_conf *qconf;
+ uint64_t app_eps = 0, app_fps = 0, app_br = 0;
+ uint64_t values[3] = {0};
+ int ret;
+ uint64_t count = 0;
+
+ RTE_LCORE_FOREACH_SLAVE(lcore_id) {
+ qconf = &lcore_conf[lcore_id];
+ if (qconf->n_rx_queue != 0) {
Perhaps just to an early exit? It will avoid having extra indentation
level :)
e.g.
if (qconf->n_rx_queue == 0)
continue;
+ count++;
+ rte_spinlock_lock(&stats[lcore_id].telemetry_lock);
+ app_eps += stats[lcore_id].ep_nep[1];
+ app_fps += stats[lcore_id].fp_nfp[1];
+ app_br += stats[lcore_id].br;
+ rte_spinlock_unlock(&stats[lcore_id].telemetry_lock);
+ }
/* launch per-lcore init on every lcore */
- if (empty_poll_on == false) {
+ if (app_mode == APP_MODE_LEGACY) {
rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
- } else {
+ } else if (app_mode == APP_MODE_EMPTY_POLL) {
empty_poll_stop = false;
rte_eal_mp_remote_launch(main_empty_poll_loop, NULL,
SKIP_MASTER);
+ } else {
+ /* Init metrics library */
+ rte_metrics_init(rte_socket_id());
+ /** Register latency stats with stats library */
Latency? Probably a copy-paste error?
--
Thanks,
Anatoly
[dpdk-dev] [PATCH] app/test-compress-perf: fix improper use of negative value
This patch fixes coverity issue: Improper use of negative value.
test_data->input_data_sz is passed to a parameter that
cannot be negative.
Coverity issue: 328504
Fixes: b68a82425da4 ("app/compress-perf: add performance measurement")
Cc: sta...@dpdk.org
Signed-off-by: Tomasz Jozwiak
---
app/test-compress-perf/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
index c2a45d1..5a579ea 100644
--- a/app/test-compress-perf/main.c
+++ b/app/test-compress-perf/main.c
@@ -244,7 +244,8 @@ comp_perf_dump_input_data(struct comp_test_data *test_data)
if (test_data->input_data_sz == 0)
test_data->input_data_sz = actual_file_sz;
- if (fseek(f, 0, SEEK_SET) != 0) {
+ if (test_data->input_data_sz <= 0 || actual_file_sz <= 0 ||
+ fseek(f, 0, SEEK_SET) != 0) {
RTE_LOG(ERR, USER1, "Size of input could not be calculated\n");
goto end;
}
--
2.7.4
[dpdk-dev] [PATCH] app/test-compress-perf: fix reliance on integer endianness
This patch fixes coverity issue:
Reliance on integer endianness (INCOMPATIBLE_CAST)
in parse_window_sz function.
Coverity issue: 328524
Fixes: e0b6287c035d ("app/compress-perf: add parser")
Cc: sta...@dpdk.org
Signed-off-by: Tomasz Jozwiak
---
app/test-compress-perf/comp_perf_options_parse.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/app/test-compress-perf/comp_perf_options_parse.c
b/app/test-compress-perf/comp_perf_options_parse.c
index 2fb6fb4..56ca580 100644
--- a/app/test-compress-perf/comp_perf_options_parse.c
+++ b/app/test-compress-perf/comp_perf_options_parse.c
@@ -364,13 +364,15 @@ parse_max_num_sgl_segs(struct comp_test_data *test_data,
const char *arg)
static int
parse_window_sz(struct comp_test_data *test_data, const char *arg)
{
- int ret = parse_uint16_t((uint16_t *)&test_data->window_sz, arg);
+ uint16_t tmp;
+ int ret = parse_uint16_t(&tmp, arg);
if (ret) {
RTE_LOG(ERR, USER1, "Failed to parse window size\n");
return -1;
}
+ test_data->window_sz = (int)tmp;
return 0;
}
--
2.7.4
Re: [dpdk-dev] [PATCH] app/test-compress-perf: fix improper use of negative value
> -Original Message-
> From: Jozwiak, TomaszX
> Sent: Monday, May 20, 2019 2:26 PM
> To: dev@dpdk.org; Trahe, Fiona ; Jozwiak, TomaszX
> ; shal...@marvell.com; sta...@dpdk.org
> Subject: [PATCH] app/test-compress-perf: fix improper use of negative value
>
> This patch fixes coverity issue: Improper use of negative value.
> test_data->input_data_sz is passed to a parameter that
> cannot be negative.
>
> Coverity issue: 328504
> Fixes: b68a82425da4 ("app/compress-perf: add performance measurement")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Tomasz Jozwiak
Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH v2 1/5] baseband/fpga_lte_fec: adding driver for FEC on FPGA
On 5/15/2019 9:28 AM, Thomas Monjalon wrote: > 14/05/2019 21:45, Nicolas Chautru: >> +Installation >> +-- >> + >> +Section 3 of the DPDK manual provides instuctions on installing and >> compiling DPDK. The >> +default set of bbdev compile flags may be found in config/common_base, >> where for example >> +the flag to build the FPGA LTE FEC device, >> ``CONFIG_RTE_LIBRTE_PMD_FPGA_LTE_FEC``, is already >> +set. It is assumed DPDK has been compiled for an icc 64-bit target using: >> + >> +.. code-block:: console >> + >> + make install T=x86_64-native-linuxapp-icc > > I already said (2 months ago) that requiring icc is not acceptable, > but I got no reply: > http://mails.dpdk.org/archives/dev/2019-March/126161.html > This PMD doesn't have any dependency to ICC, gcc or clang is as good as any.
Re: [dpdk-dev] [PATCH v2 1/5] baseband/fpga_lte_fec: adding driver for FEC on FPGA
20/05/2019 15:44, Ferruh Yigit: > On 5/15/2019 9:28 AM, Thomas Monjalon wrote: > > 14/05/2019 21:45, Nicolas Chautru: > >> +Installation > >> +-- > >> + > >> +Section 3 of the DPDK manual provides instuctions on installing and > >> compiling DPDK. The > >> +default set of bbdev compile flags may be found in config/common_base, > >> where for example > >> +the flag to build the FPGA LTE FEC device, > >> ``CONFIG_RTE_LIBRTE_PMD_FPGA_LTE_FEC``, is already > >> +set. It is assumed DPDK has been compiled for an icc 64-bit target using: > >> + > >> +.. code-block:: console > >> + > >> + make install T=x86_64-native-linuxapp-icc > > > > I already said (2 months ago) that requiring icc is not acceptable, > > but I got no reply: > > http://mails.dpdk.org/archives/dev/2019-March/126161.html > > > > This PMD doesn't have any dependency to ICC, gcc or clang is as good as any. Good news, so please don't use icc in your example. And you should remove "It is assumed DPDK has been compiled for an icc 64-bit target"
Re: [dpdk-dev] [PATCH] app/test-compress-perf: fix reliance on integer endianness
HI Tomasz,
> -Original Message-
> From: Jozwiak, TomaszX
> Sent: Monday, May 20, 2019 2:26 PM
> To: dev@dpdk.org; Trahe, Fiona ; Jozwiak, TomaszX
> ; shal...@marvell.com; sta...@dpdk.org
> Subject: [PATCH] app/test-compress-perf: fix reliance on integer endianness
>
> This patch fixes coverity issue:
> Reliance on integer endianness (INCOMPATIBLE_CAST)
> in parse_window_sz function.
>
> Coverity issue: 328524
> Fixes: e0b6287c035d ("app/compress-perf: add parser")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Tomasz Jozwiak
> ---
> app/test-compress-perf/comp_perf_options_parse.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-compress-perf/comp_perf_options_parse.c
> b/app/test-compress-
> perf/comp_perf_options_parse.c
> index 2fb6fb4..56ca580 100644
> --- a/app/test-compress-perf/comp_perf_options_parse.c
> +++ b/app/test-compress-perf/comp_perf_options_parse.c
> @@ -364,13 +364,15 @@ parse_max_num_sgl_segs(struct comp_test_data
> *test_data, const char *arg)
> static int
> parse_window_sz(struct comp_test_data *test_data, const char *arg)
> {
> - int ret = parse_uint16_t((uint16_t *)&test_data->window_sz, arg);
> + uint16_t tmp;
> + int ret = parse_uint16_t(&tmp, arg);
>
> if (ret) {
> RTE_LOG(ERR, USER1, "Failed to parse window size\n");
> return -1;
> }
>
> + test_data->window_sz = (int)tmp;
> return 0;
> }
[Fiona] I expect this fixes this coverity issue - but will it result in another
one?
window_sz on the xform is uint8_t - so this int will get truncated later, and
there's no cast done at that point.
Would it be better to add a new parse_uint8_t fn and change
test-data->window_sz to a unit8_t?
Or add that cast?
Re: [dpdk-dev] [PATCH] app/test-compress-perf: fix reliance on integer endianness
Hi Fiona,
> -Original Message-
> From: Trahe, Fiona
> Sent: Monday, May 20, 2019 4:06 PM
> To: Jozwiak, TomaszX ; dev@dpdk.org;
> shal...@marvell.com; sta...@dpdk.org
> Cc: Trahe, Fiona ; Trybula, ArturX
>
> Subject: RE: [PATCH] app/test-compress-perf: fix reliance on integer
> endianness
>
> HI Tomasz,
>
> > -Original Message-
> > From: Jozwiak, TomaszX
> > Sent: Monday, May 20, 2019 2:26 PM
> > To: dev@dpdk.org; Trahe, Fiona ; Jozwiak,
> > TomaszX ; shal...@marvell.com;
> > sta...@dpdk.org
> > Subject: [PATCH] app/test-compress-perf: fix reliance on integer
> > endianness
> >
> > This patch fixes coverity issue:
> > Reliance on integer endianness (INCOMPATIBLE_CAST) in
> parse_window_sz
> > function.
> >
> > Coverity issue: 328524
> > Fixes: e0b6287c035d ("app/compress-perf: add parser")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tomasz Jozwiak
> > ---
> > app/test-compress-perf/comp_perf_options_parse.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-compress-perf/comp_perf_options_parse.c
> > b/app/test-compress- perf/comp_perf_options_parse.c index
> > 2fb6fb4..56ca580 100644
> > --- a/app/test-compress-perf/comp_perf_options_parse.c
> > +++ b/app/test-compress-perf/comp_perf_options_parse.c
> > @@ -364,13 +364,15 @@ parse_max_num_sgl_segs(struct
> comp_test_data
> > *test_data, const char *arg) static int parse_window_sz(struct
> > comp_test_data *test_data, const char *arg) {
> > - int ret = parse_uint16_t((uint16_t *)&test_data->window_sz, arg);
> > + uint16_t tmp;
> > + int ret = parse_uint16_t(&tmp, arg);
> >
> > if (ret) {
> > RTE_LOG(ERR, USER1, "Failed to parse window size\n");
> > return -1;
> > }
> >
> > + test_data->window_sz = (int)tmp;
> > return 0;
> > }
> [Fiona] I expect this fixes this coverity issue - but will it result in
> another one?
> window_sz on the xform is uint8_t - so this int will get truncated later, and
> there's no cast done at that point.
> Would it be better to add a new parse_uint8_t fn and change test-data-
> >window_sz to a unit8_t?
> Or add that cast?
> [Tomek] I measn it's ok. There's a check inside comp_perf_check_capabilities
> function. If the value from test_data->window_sz > cap->window_size we have a
> fail. Also during parsing there's a check is value from command line is
> between 0 and UINT16_MAX, so in my opinion all cases are tested. The point is
> there's only one place where we're parsing uint8_t value. parse_uint8_t
> function will be especially for that.
[dpdk-dev] [PATCH] maintainers: update Marvell PMDs
From: Liron Himi Alan is no longer involved in PMDs maintenance hence update the list. Also append new active maintainer to the list. Signed-off-by: Liron Himi --- MAINTAINERS | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 15d0829..d0bf259 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -644,7 +644,6 @@ F: doc/guides/nics/features/ipn3ke.ini Marvell mvpp2 M: Tomasz Duszynski M: Liron Himi -M: Alan Winkowski F: drivers/common/mvep/ F: drivers/net/mvpp2/ F: doc/guides/nics/mvpp2.rst @@ -653,7 +652,6 @@ F: doc/guides/nics/features/mvpp2.ini Marvell mvneta M: Zyta Szpak M: Liron Himi -M: Alan Winkowski F: drivers/net/mvneta/ F: doc/guides/nics/mvneta.rst F: doc/guides/nics/features/mvneta.ini @@ -901,8 +899,8 @@ F: doc/guides/cryptodevs/features/kasumi.ini Marvell mvsam M: Tomasz Duszynski +M: Michael Shamis M: Liron Himi -M: Alan Winkowski F: drivers/crypto/mvsam/ F: doc/guides/cryptodevs/mvsam.rst F: doc/guides/cryptodevs/features/mvsam.ini -- 2.7.4
[dpdk-dev] [Bug 281] BPF: Linking error in librte_bpf
https://bugs.dpdk.org/show_bug.cgi?id=281 Michel Machado (mic...@digirati.com.br) changed: What|Removed |Added Status|CONFIRMED |RESOLVED Resolution|--- |INVALID --- Comment #2 from Michel Machado (mic...@digirati.com.br) --- I've followed your steps and it worked. I don't know what I did wrong when I was testing it. I'm sorry for bothering you. Thank you for your help. -- You are receiving this mail because: You are the assignee for the bug.
Re: [dpdk-dev] Hugepages not being deleted
> -Original Message- > From: Burakov, Anatoly > Sent: Monday, May 20, 2019 6:45 AM > To: David Marchand > Cc: Michael Santana ; dev ; > Carrillo, Erik G > Subject: Re: [dpdk-dev] Hugepages not being deleted > > On 20-May-19 12:09 PM, David Marchand wrote: > > On Mon, May 20, 2019 at 12:43 PM Burakov, Anatoly > > mailto:anatoly.bura...@intel.com>> wrote: > > > > On 17-May-19 3:06 PM, David Marchand wrote: > > > Hello Anatoly, > > > > > > Not sure what the issue is at the moment. > > > I too have an error on the eal_flags_autotest but what is > > worrying me is > > > that it works fine in 18.11.1 and 19.02. > > > Is there anything that rings a bell? > > > > > > Thanks. > > > > > > -- > > > David Marchand > > > > Not to me. Last i checked they were passing. > > > > > > Bisected it, I will send a fix. > > > > > > -- > > David Marchand > > I've bisected it too. I think Erik (author of original patch) already is > working on > the problem. I don't think it's easily (and safely) solvable. > You can find more info in below threads: > > http://patches.dpdk.org/patch/53191/ > http://patches.dpdk.org/patch/53268/ > Yes, there a memzone that needs to be properly freed at finalize/cleanup time in the timer library. One way to free it safely was explored in the following: http://patchwork.dpdk.org/patch/53334/ but that solution has ABI implications that were undesirable. Still looking for a better way. Thanks, Erik > -- > Thanks, > Anatoly
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by default configuration
On 5/14/2019 2:56 AM, Zhao1, Wei wrote:
> Hi, Ferruh
>
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Tuesday, May 14, 2019 12:36 AM
>> To: Zhao1, Wei ; dev@dpdk.org
>> Cc: sta...@dpdk.org; Peng, Yuan ; Lu, Wenzhuo
>>
>> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by
>> default configuration
>>
>> On 5/9/2019 8:20 AM, Wei Zhao wrote:
>>> There is an error in function rxtx_port_config(), which may overwrite
>>> offloads configuration get from function launch_args_parse() when run
>>> testpmd app. So rxtx_port_config() should do "or" for port offloads.
>>>
>>> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
>>> cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Wei Zhao
>>> ---
>>> app/test-pmd/testpmd.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> 6fbfd29..f0061d9 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -2809,9 +2809,12 @@ static void
>>> rxtx_port_config(struct rte_port *port) {
>>> uint16_t qid;
>>> + uint64_t offloads;
>>>
>>> for (qid = 0; qid < nb_rxq; qid++) {
>>> + offloads = port->rx_conf[qid].offloads;
>>> port->rx_conf[qid] = port->dev_info.default_rxconf;
>>> + port->rx_conf[qid].offloads |= offloads;
>>
>> OK to this changes as a fix for this release.
>>
>> But I think intention is, if no offload information is provided by user to
>> use use
>> the driver provided defaults, if user explicitly provided some values to use
>> them,
>> instead of OR these two.
>>
>> With this approach it is not possible to disable a driver default value, so
>> it
>> becomes mandatory offload instead of default offload values.
>>
>> Wei, what do you think, does it make sense?
>
>
> I agree with you, but it is sure that the original code has offloads
> overwrite issue.
> What is your suggestion for code implement?
> I find that Thomas has apply it, if you has other idea, maybe you has to
> commit patch base to this patch.
Hi Wei,
Yes this needs to be incremental fix to existing code.
Queue specific offload can be altered either by providing Rx/Tx offload as
command line argument [1] (port configs set to each queues) or via testpmd
commands [2].
Does it make sense to set a global flag when one of above occurs and use default
config only if it is not set?
[1]
Tx
tx-offloads
Rx
disable-crc-strip
enable-lro
enable-scatter
enable-rx-cksum
enable-rx-timestamp
enable-hw-vlan
enable-hw-vlan-filter
enable-hw-vlan-strip
enable-hw-vlan-extend
[2]
"port config rx_offload ..."
"port rxq rx_offload ..."
"port config tx_offload ..."
"port txq tx_offload ..."
Re: [dpdk-dev] [RFC v2 00/14] prefix network structures
On 4/10/2019 9:32 AM, Olivier Matz wrote: > This RFC targets 19.08. > > The rte_net headers conflict with the libc headers, because > some definitions are duplicated, sometimes with few differences. > > This RFC adds the rte_ (or RTE_) prefix to all structures, functions > and defines in rte_net library. This is a big changeset, that will > break the API of many functions, but not the ABI. > > This was discussed in [1], and recently requested by the techboard [2]. > > v2: > * rebase on top of v19.05-rc1 > > [1] http://mails.dpdk.org/archives/dev/2018-January/087384.html > [2] http://mails.dpdk.org/archives/dev/2019-February/125033.html > > Olivier Matz (14): > net: add rte prefix to arp structures > net: add rte prefix to arp defines > net: add rte prefix to ether structures > net: add rte prefix to ether functions > net: add rte prefix to ether defines > net: add rte prefix to esp structure > net: add rte prefix to gre structure > net: add rte prefix to icmp structure > net: add rte prefix to icmp defines > net: add rte prefix to ip structure > net: add rte prefix to ip defines > net: add rte prefix to sctp structure > net: add rte prefix to tcp structure > net: add rte prefix to udp structure Hi Oliver, This was planned as one of the early step for the 19.08. Should we continue with this patchset or are you planning to send a new version on top of latest head? Thanks, ferruh
Re: [dpdk-dev] [PATCH] net/mlx5: fix memory free on queue create error
> -Original Message-
> From: dev On Behalf Of Dekel Peled
> Sent: Monday, May 20, 2019 11:07 AM
> To: Yongseok Koh ; Shahaf Shuler
>
> Cc: dev@dpdk.org; Ori Kam ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix memory free on queue create error
>
> In function mlx5_rxq_ibv_new(), pointer *tmpl allocation is attempted
> at the start, but not validated or freed in case of error.
> In function mlx5_txq_ibv_new(), pointer *txq_ibv allocation is
> attempted at the start, but not freed in case of error.
>
> This patch adds pointers initialization, validation and freeing.
>
> Fixes: 09cb5b581762 ("net/mlx5: separate DPDK from verbs Rx queue objects")
> Fixes: faf2667fe8d5 ("net/mlx5: separate DPDK from verbs Tx queue objects")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Dekel Peled
> ---
> drivers/net/mlx5/mlx5_rxq.c | 22 +-
> drivers/net/mlx5/mlx5_txq.c | 4 +++-
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index a00cb12..8cd6157 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -776,7 +776,7 @@ struct mlx5_rxq_ibv *
> } attr;
> unsigned int cqe_n;
> unsigned int wqe_n = 1 << rxq_data->elts_n;
> - struct mlx5_rxq_ibv *tmpl;
> + struct mlx5_rxq_ibv *tmpl = NULL;
> struct mlx5dv_cq cq_info;
> struct mlx5dv_rwq rwq;
> unsigned int i;
> @@ -1017,15 +1017,19 @@ struct mlx5_rxq_ibv *
> priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
> return tmpl;
> error:
> - ret = rte_errno; /* Save rte_errno before cleanup. */
> - if (tmpl->wq)
> - claim_zero(mlx5_glue->destroy_wq(tmpl->wq));
> - if (tmpl->cq)
> - claim_zero(mlx5_glue->destroy_cq(tmpl->cq));
> - if (tmpl->channel)
> - claim_zero(mlx5_glue->destroy_comp_channel(tmpl-
> >channel));
> + if (tmpl) {
> + ret = rte_errno; /* Save rte_errno before cleanup. */
> + if (tmpl->wq)
> + claim_zero(mlx5_glue->destroy_wq(tmpl->wq));
> + if (tmpl->cq)
> + claim_zero(mlx5_glue->destroy_cq(tmpl->cq));
> + if (tmpl->channel)
> + claim_zero(mlx5_glue->destroy_comp_channel
> + (tmpl->channel));
> + rte_free(tmpl);
> + rte_errno = ret; /* Restore rte_errno. */
> + }
> priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
> - rte_errno = ret; /* Restore rte_errno. */
> return NULL;
> }
>
> diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
> index b281c45..9efbab6 100644
> --- a/drivers/net/mlx5/mlx5_txq.c
> +++ b/drivers/net/mlx5/mlx5_txq.c
> @@ -401,7 +401,7 @@ struct mlx5_txq_ibv *
> struct mlx5_txq_ctrl *txq_ctrl =
> container_of(txq_data, struct mlx5_txq_ctrl, txq);
> struct mlx5_txq_ibv tmpl;
> - struct mlx5_txq_ibv *txq_ibv;
> + struct mlx5_txq_ibv *txq_ibv = NULL;
> union {
> struct ibv_qp_init_attr_ex init;
> struct ibv_cq_init_attr_ex cq;
> @@ -586,6 +586,8 @@ struct mlx5_txq_ibv *
> claim_zero(mlx5_glue->destroy_cq(tmpl.cq));
> if (tmpl.qp)
> claim_zero(mlx5_glue->destroy_qp(tmpl.qp));
> + if (txq_ibv)
> + rte_free(txq_ibv);
> priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
> rte_errno = ret; /* Restore rte_errno. */
> return NULL;
> --
> 1.8.3.1
Acked-by: Ori Kam
Thanks,
Ori Kam
Re: [dpdk-dev] [PATCH] net/vmxnet3: uninitialized variable fix
-Original Message-
From: Yogev Chaimovich
Date: Thursday, May 16, 2019 at 12:10 AM
To: Yong Wang
Cc: "dev@dpdk.org" , Yogev Chaimovich
, "sta...@dpdk.org"
Subject: [PATCH] net/vmxnet3: uninitialized variable fix
Coverity issue: 323479
Fixes: 30f77abe net/vmxnet3: support stats reset
Cc: sta...@dpdk.org
Signed-off-by: yogev ch
---
Acked-by: Yong Wang
drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c
b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index f54536b..c465996 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -1132,8 +1132,8 @@ static int eth_vmxnet3_pci_remove(struct
rte_pci_device *pci_dev)
{
unsigned int i;
struct vmxnet3_hw *hw = dev->data->dev_private;
- struct UPT1_TxStats txStats;
- struct UPT1_RxStats rxStats;
+ struct UPT1_TxStats txStats = {0};
+ struct UPT1_RxStats rxStats = {0};
VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS);
--
1.9.1
Re: [dpdk-dev] [PATCH 2/3] net/i40e: add runtime option to disable vector rx
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Monday, May 20, 2019 1:29 AM
> To: Ergin, Mesut A ; Xing, Beilei
> ; Zhang, Qi Z
> Cc: dev@dpdk.org; Ergin, Mesut A
> Subject: RE: [dpdk-dev] [PATCH 2/3] net/i40e: add runtime option to disable
> vector rx
>
>
> Hi
>
> >
> > Vector RX functions are not at feature parity with non-vector ones and
> > currently the vector RX path is enabled by default. Hence, the only
> > option to force selection of non-vector variants and be able to retain
> > functionality is to disable vector PMD globally at compile time via the
> > config file option.
> >
> > This patch introduces a new runtime device config option named
> > disable-vec-rx to allow users to limit the driver to make a choice among
> > non-vector RX functions, on a per device basis. This runtime option
> > defaults to a value of zero, allowing vector RX functions to be
> > selected (current behavior). When disable-vec-rx=1 is specified for a
> > device, even if all the other requirements to select a vector RX
> > function are satisfied, the driver will still pick one out of the
> > appropriate non-vector RX functions.
>
> Not sure I understand the logic of that patch.
> If i40e RX PMD selects wrong RX function that doesn't provide
> requested by user functionality (which one?) -
> then it is a bug in i40e RX function selection code that needs to be fixed.
> No point to introduce some new config options for that.
> Konstantin
>
Current design of RX function selection for the PMDs make it an
initialization time deal. However, the first rte_flow request (thus the need
to enable FD / RTE_FDIR_MODE_PERFECT) may come in at any point in
time, well after the RX function was selected (e.g., OVS does this when the
first packet that can be offloaded arrives). The design in this patch gives
such applications a choice to restrict possible RX functions to non-vector
paths proactively.
> >
> > Signed-off-by: Mesut Ali Ergin
> > ---
> > doc/guides/nics/i40e.rst | 14 +
> > drivers/net/i40e/i40e_ethdev.c | 70
> +-
> > drivers/net/i40e/i40e_ethdev.h | 1 +
> > drivers/net/i40e/i40e_rxtx.c | 4 +++
> > 4 files changed, 88 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
> > index a97484c..532cc64 100644
> > --- a/doc/guides/nics/i40e.rst
> > +++ b/doc/guides/nics/i40e.rst
> > @@ -183,6 +183,20 @@ Runtime Config Options
> >
> >-w 84:00.0,use-latest-supported-vec=1
> >
> > +- ``Disable RX Vector Functions `` (default ``vector RX enabled``)
> > +
> > + When config file option for the vector PMD is enabled, vector RX
> > functions
> may
> > + be the default choice of the driver at device initialization time, if
> > certain
> > + conditions apply. However, vector RX functions may not always be at
> feature
> > + parity with non-vector ones. In order to allow users to override vector
> > RX
> > + function selection behavior at runtime, the ``devargs`` parameter
> > + ``disable-vec-rx`` is introduced, such that
> > +
> > + -w DBDF,disable-vec-rx=1
> > +
> > + would force driver to limit its choices to non-vector RX function
> > variants for
> > + the device specified by the DBDF.
> > +
> > Vector RX Pre-conditions
> >
> > For Vector RX it is assumed that the number of descriptor rings will be a
> > power
> > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> > index cab440f..19fbd23 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -44,6 +44,7 @@
> > #define ETH_I40E_SUPPORT_MULTI_DRIVER "support-multi-driver"
> > #define ETH_I40E_QUEUE_NUM_PER_VF_ARG "queue-num-per-vf"
> > #define ETH_I40E_USE_LATEST_VEC"use-latest-supported-vec"
> > +#define ETH_I40E_DISABLE_VEC_RX"disable-vec-rx"
> >
> > #define I40E_CLEAR_PXE_WAIT_MS 200
> >
> > @@ -410,6 +411,7 @@ static const char *const valid_keys[] = {
> > ETH_I40E_SUPPORT_MULTI_DRIVER,
> > ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> > ETH_I40E_USE_LATEST_VEC,
> > + ETH_I40E_DISABLE_VEC_RX,
> > NULL};
> >
> > static const struct rte_pci_id pci_id_i40e_map[] = {
> > @@ -1263,6 +1265,68 @@ i40e_use_latest_vec(struct rte_eth_dev *dev)
> > return 0;
> > }
> >
> > +static int
> > +i40e_parse_disable_vec_rx_handler(__rte_unused const char *key,
> > + const char *value,
> > + void *opaque)
> > +{
> > + struct i40e_adapter *ad;
> > +
> > + ad = (struct i40e_adapter *)opaque;
> > +
> > + switch (atoi(value)) {
> > + case 0:
> > + /* Selection of RX vector functions left untouched*/
> > + break;
> > + case 1:
> > + /* Disable RX vector functions as requested*/
> > + ad->rx_vec_allowed = false;
> > + break;
> > + default:
> > + PMD_DRV_LOG(WARNING, "Value should be 0 or 1, set it as
> 1!");
Re: [dpdk-dev] Generating Debug information in Windows using Clang (PDB files)
Adham... I don't think we debugged using clang compiled code for Hello world - mainly because it was only a "helloworld" application and we didn't quite need any debugging!:-) I found this link: http://blog.llvm.org/2017/08/llvm-on-windows-now-supports-pdb-debug.html ...which says to use the following: Here are two simple ways to test out this new functionality: 1. Have clang-cl invoke lld automatically clang-cl -fuse-ld=lld -Z7 -MTd hello.cpp 1. Invoke clang-cl and lld separately. clang-cl -c -Z7 -MTd -o hello.obj hello.cpp lld-link -debug hello.obj Can you try it with just the -Z7 option? I'm also adding some persons from Microsoft to this thread, hoping they can help... +Harini +Omar +Jeffrey ranjit m. From: Adham Masarwah Sent: Monday, May 20, 2019 3:27 AM To: dev@dpdk.org Cc: Menon, Ranjit ; Kadam, Pallavi ; Yohad Tor ; Rani Sharoni ; Tal Shnaiderman ; Richardson, Bruce Subject: Generating Debug information in Windows using Clang (PDB files) Hi, In development we use WinDbg for debugging, so we need to create PDB files when compiling the DPDK, so we used ``-g`` in the CFLGAS and the PDB files are being created. But when running the helloworld with WinDbg, we can see only function names, we can't see code neither variables, we get the following error: "Private symbols (symbols.pri) are required for locals." We tried to append some flags like -Z7 and -gcodeview, but the compiler ignores them. Do you have any idea what we miss here ? What is the debug mechanism is being used in Windows ? Thanks
Re: [dpdk-dev] [PATCH v3 0/3] Fixes for building examples
17/05/2019 15:12, Luca Boccassi: > On Fri, 2019-05-17 at 13:02 +0100, Bruce Richardson wrote: > > A small set of fixes for building examples, that was previously part > > of > > the larger set, but pulled out separately for easier apply. > > > > V3: changed patch 1, to properly quit processing examples/meson.build > > if the examples list was empty. > > > > Bruce Richardson (3): > > examples: fix install of sample apps if setting not provided > > examples: remove auto-generation of examples list > > examples: fix make clean when using pkg-config for building > > Series-acked-by: Luca Boccassi Applied, thanks
[dpdk-dev] [PATCH] rte_flow: mark rte_flow_error_set as cold
A minor optimization that save a few cycles during flow setup. Use the GCC cold attribute for the rte_flow_error_set function. This attribute implicitly marks all code paths that arrive at this function as unlikely. Signed-off-by: Stephen Hemminger --- lib/librte_ethdev/rte_flow.h | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h index 63f84fca65c4..dc821be43f19 100644 --- a/lib/librte_ethdev/rte_flow.h +++ b/lib/librte_ethdev/rte_flow.h @@ -2591,7 +2591,13 @@ rte_flow_error_set(struct rte_flow_error *error, int code, enum rte_flow_error_type type, const void *cause, - const char *message); + const char *message) +#ifdef __GNUC__ +#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2)) + __attribute__((cold)) +#endif +#endif + ; /** * @deprecated -- 2.20.1
Re: [dpdk-dev] [PATCH 2/3] net/i40e: add runtime option to disable vector rx
> >
> > Hi
> >
> > >
> > > Vector RX functions are not at feature parity with non-vector ones and
> > > currently the vector RX path is enabled by default. Hence, the only
> > > option to force selection of non-vector variants and be able to retain
> > > functionality is to disable vector PMD globally at compile time via the
> > > config file option.
> > >
> > > This patch introduces a new runtime device config option named
> > > disable-vec-rx to allow users to limit the driver to make a choice among
> > > non-vector RX functions, on a per device basis. This runtime option
> > > defaults to a value of zero, allowing vector RX functions to be
> > > selected (current behavior). When disable-vec-rx=1 is specified for a
> > > device, even if all the other requirements to select a vector RX
> > > function are satisfied, the driver will still pick one out of the
> > > appropriate non-vector RX functions.
> >
> > Not sure I understand the logic of that patch.
> > If i40e RX PMD selects wrong RX function that doesn't provide
> > requested by user functionality (which one?) -
> > then it is a bug in i40e RX function selection code that needs to be fixed.
> > No point to introduce some new config options for that.
> > Konstantin
> >
> Current design of RX function selection for the PMDs make it an
> initialization time deal. However, the first rte_flow request (thus the need
> to enable FD / RTE_FDIR_MODE_PERFECT) may come in at any point in
> time, well after the RX function was selected (e.g., OVS does this when the
> first packet that can be offloaded arrives). The design in this patch gives
> such applications a choice to restrict possible RX functions to non-vector
> paths proactively.
If you plan to use FD mode on your device, why not enable it
at setup phase via rte_eth_dev_configure()?
Then proper rx function would be selected.
>
> > >
> > > Signed-off-by: Mesut Ali Ergin
> > > ---
> > > doc/guides/nics/i40e.rst | 14 +
> > > drivers/net/i40e/i40e_ethdev.c | 70
> > +-
> > > drivers/net/i40e/i40e_ethdev.h | 1 +
> > > drivers/net/i40e/i40e_rxtx.c | 4 +++
> > > 4 files changed, 88 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
> > > index a97484c..532cc64 100644
> > > --- a/doc/guides/nics/i40e.rst
> > > +++ b/doc/guides/nics/i40e.rst
> > > @@ -183,6 +183,20 @@ Runtime Config Options
> > >
> > >-w 84:00.0,use-latest-supported-vec=1
> > >
> > > +- ``Disable RX Vector Functions `` (default ``vector RX enabled``)
> > > +
> > > + When config file option for the vector PMD is enabled, vector RX
> > > functions
> > may
> > > + be the default choice of the driver at device initialization time, if
> > > certain
> > > + conditions apply. However, vector RX functions may not always be at
> > feature
> > > + parity with non-vector ones. In order to allow users to override
> > > vector RX
> > > + function selection behavior at runtime, the ``devargs`` parameter
> > > + ``disable-vec-rx`` is introduced, such that
> > > +
> > > + -w DBDF,disable-vec-rx=1
> > > +
> > > + would force driver to limit its choices to non-vector RX function
> > > variants for
> > > + the device specified by the DBDF.
> > > +
> > > Vector RX Pre-conditions
> > >
> > > For Vector RX it is assumed that the number of descriptor rings will be
> > > a power
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c
> > > index cab440f..19fbd23 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -44,6 +44,7 @@
> > > #define ETH_I40E_SUPPORT_MULTI_DRIVER"support-multi-driver"
> > > #define ETH_I40E_QUEUE_NUM_PER_VF_ARG"queue-num-per-vf"
> > > #define ETH_I40E_USE_LATEST_VEC "use-latest-supported-vec"
> > > +#define ETH_I40E_DISABLE_VEC_RX "disable-vec-rx"
> > >
> > > #define I40E_CLEAR_PXE_WAIT_MS 200
> > >
> > > @@ -410,6 +411,7 @@ static const char *const valid_keys[] = {
> > > ETH_I40E_SUPPORT_MULTI_DRIVER,
> > > ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> > > ETH_I40E_USE_LATEST_VEC,
> > > + ETH_I40E_DISABLE_VEC_RX,
> > > NULL};
> > >
> > > static const struct rte_pci_id pci_id_i40e_map[] = {
> > > @@ -1263,6 +1265,68 @@ i40e_use_latest_vec(struct rte_eth_dev *dev)
> > > return 0;
> > > }
> > >
> > > +static int
> > > +i40e_parse_disable_vec_rx_handler(__rte_unused const char *key,
> > > + const char *value,
> > > + void *opaque)
> > > +{
> > > + struct i40e_adapter *ad;
> > > +
> > > + ad = (struct i40e_adapter *)opaque;
> > > +
> > > + switch (atoi(value)) {
> > > + case 0:
> > > + /* Selection of RX vector functions left untouched*/
> > > + break;
> > > + case 1:
> > > + /* Disable RX vector functions as requested*/
> > > + ad->rx_vec_allowed = false;
> > > + break;
> > > + default:
> > > +
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by default configuration
Hi, Ferruh
> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, May 20, 2019 11:23 PM
> To: Zhao1, Wei ; dev@dpdk.org
> Cc: sta...@dpdk.org; Peng, Yuan ; Lu, Wenzhuo
>
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by
> default configuration
>
> On 5/14/2019 2:56 AM, Zhao1, Wei wrote:
> > Hi, Ferruh
> >
> >> -Original Message-
> >> From: Yigit, Ferruh
> >> Sent: Tuesday, May 14, 2019 12:36 AM
> >> To: Zhao1, Wei ; dev@dpdk.org
> >> Cc: sta...@dpdk.org; Peng, Yuan ; Lu, Wenzhuo
> >>
> >> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads
> >> overwrite by default configuration
> >>
> >> On 5/9/2019 8:20 AM, Wei Zhao wrote:
> >>> There is an error in function rxtx_port_config(), which may
> >>> overwrite offloads configuration get from function
> >>> launch_args_parse() when run testpmd app. So rxtx_port_config() should
> do "or" for port offloads.
> >>>
> >>> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
> >>> cc: sta...@dpdk.org
> >>>
> >>> Signed-off-by: Wei Zhao
> >>> ---
> >>> app/test-pmd/testpmd.c | 5 +
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>> 6fbfd29..f0061d9 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -2809,9 +2809,12 @@ static void
> >>> rxtx_port_config(struct rte_port *port) {
> >>> uint16_t qid;
> >>> + uint64_t offloads;
> >>>
> >>> for (qid = 0; qid < nb_rxq; qid++) {
> >>> + offloads = port->rx_conf[qid].offloads;
> >>> port->rx_conf[qid] = port->dev_info.default_rxconf;
> >>> + port->rx_conf[qid].offloads |= offloads;
> >>
> >> OK to this changes as a fix for this release.
> >>
> >> But I think intention is, if no offload information is provided by
> >> user to use use the driver provided defaults, if user explicitly
> >> provided some values to use them, instead of OR these two.
> >>
> >> With this approach it is not possible to disable a driver default
> >> value, so it becomes mandatory offload instead of default offload values.
> >>
> >> Wei, what do you think, does it make sense?
> >
> >
> > I agree with you, but it is sure that the original code has offloads
> > overwrite
> issue.
> > What is your suggestion for code implement?
> > I find that Thomas has apply it, if you has other idea, maybe you has to
> commit patch base to this patch.
>
> Hi Wei,
>
> Yes this needs to be incremental fix to existing code.
>
> Queue specific offload can be altered either by providing Rx/Tx offload as
> command line argument [1] (port configs set to each queues) or via testpmd
> commands [2].
> Does it make sense to set a global flag when one of above occurs and use
> default config only if it is not set?
I AGREE with you to submit an incremental fix, and it make sense to set a
global flag when one of above occurs and use
default config only if it is not set when implement code, but I do not have
time to prepare such a patch by now, so maybe later or some else.
>
> [1]
> Tx
> tx-offloads
> Rx
> disable-crc-strip
> enable-lro
> enable-scatter
> enable-rx-cksum
> enable-rx-timestamp
> enable-hw-vlan
> enable-hw-vlan-filter
> enable-hw-vlan-strip
> enable-hw-vlan-extend
>
> [2]
> "port config rx_offload ..."
> "port rxq rx_offload ..."
> "port config tx_offload ..."
> "port txq tx_offload ..."
>
[dpdk-dev] [Bug 282] Fix missing headers in FreeBSD CURRENT build
https://bugs.dpdk.org/show_bug.cgi?id=282 Bug ID: 282 Summary: Fix missing headers in FreeBSD CURRENT build Product: DPDK Version: 18.05 Hardware: All OS: FreeBSD Status: CONFIRMED Severity: normal Priority: Normal Component: core Assignee: dev@dpdk.org Reporter: c...@freebsd.org Target Milestone: --- After header pollution cleanup, some files in DPDK 18.05.1 no longer compile. The fixes are straightforward. I don't know if the problem is still present in newer DPDK but I wouldn't be surprised if it were. Feel free to consider these patches in the public domain, or CC0 if you're European: --- kernel/freebsd/contigmem/contigmem.c.orig 2018-09-05 14:29:02 UTC +++ kernel/freebsd/contigmem/contigmem.c @@ -9,9 +9,12 @@ #include #include #include +#include #include +#include #include #include +#include #include #include #include --- kernel/freebsd/nic_uio/nic_uio.c.orig 2018-09-05 14:29:02 UTC +++ kernel/freebsd/nic_uio/nic_uio.c @@ -9,6 +9,7 @@ #include /* types used in module initialization */ #include /* cdevsw struct */ #include /* structs, prototypes for pci bus stuff and DEVMETHOD */ +#include /* used by vm_pager.h => MPASS() */ #include #include #include -- You are receiving this mail because: You are the assignee for the bug.
Re: [dpdk-dev] [Bug 282] Fix missing headers in FreeBSD CURRENT build
On Tue, 21 May 2019 01:54:36 + bugzi...@dpdk.org wrote: > https://bugs.dpdk.org/show_bug.cgi?id=282 > > Bug ID: 282 >Summary: Fix missing headers in FreeBSD CURRENT build >Product: DPDK >Version: 18.05 > Hardware: All > OS: FreeBSD > Status: CONFIRMED > Severity: normal > Priority: Normal > Component: core > Assignee: dev@dpdk.org > Reporter: c...@freebsd.org > Target Milestone: --- > > After header pollution cleanup, some files in DPDK 18.05.1 no longer compile. > The fixes are straightforward. I don't know if the problem is still present > in > newer DPDK but I wouldn't be surprised if it were. Feel free to consider > these > patches in the public domain, or CC0 if you're European: > All patches must be licensed by the same license as the original code. That is BSD-3 in this case. Please don't send with other licenses.
Re: [dpdk-dev] [PATCH v1] net/ice: update the RSS RETA size with support values
Little comments.
> -Original Message-
> From: Wang, Haiyue
> Sent: Monday, May 20, 2019 4:15 PM
> To: dev@dpdk.org; Zhang, Qi Z ; Yang, Qiming
> ; Wu, Jingjing ; Lu,
> Wenzhuo
> Cc: Wang, Haiyue
> Subject: [PATCH v1] net/ice: update the RSS RETA size with support values
>
> Since ice can support 128, 512, 2K RSS RETA size value, change the update
> API to set it to resize the RSS RETA table. And by default, use 512 to sync
> with ETH_RSS_RETA_SIZE_x maximum value definition.
> Also the flag ICE_FLAG_RSS_AQ_CAPABLE is missed to set.
>
> Fixes: 690175ee51bf ("net/ice: support getting device information")
> Fixes: ff963bfa7cb1 ("net/ice: support RSS")
>
> Signed-off-by: Haiyue Wang
> ---
> drivers/net/ice/ice_ethdev.c | 41 +++--
> 1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
> bbaa7cf..c4ea09f 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -1149,6 +1149,12 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type
> type)
> TAILQ_INIT(&vsi->mac_list);
> TAILQ_INIT(&vsi->vlan_list);
>
> + /* Be sync with ETH_RSS_RETA_SIZE_x maximum value definition */
I think this stats is not clear, can't understand what's the
ETH_RSS_RETA_SIZE_x maximum value, why don't say the default max
Value is 512?
> + pf->hash_lut_size = hw->func_caps.common_cap.rss_table_size >
> + ETH_RSS_RETA_SIZE_512 ? ETH_RSS_RETA_SIZE_512 :
> + hw->func_caps.common_cap.rss_table_size;
> + pf->flags |= ICE_FLAG_RSS_AQ_CAPABLE;
> +
> memset(&vsi_ctx, 0, sizeof(vsi_ctx));
> /* base_queue in used in queue mapping of VSI add/update
> command.
>* Suppose vsi->base_queue is 0 now, don't consider SRIOV, VMDQ
> @@ -1627,7 +1633,7 @@ static int ice_init_rss(struct ice_pf *pf)
> rss_conf = &dev->data->dev_conf.rx_adv_conf.rss_conf;
> nb_q = dev->data->nb_rx_queues;
> vsi->rss_key_size = ICE_AQC_GET_SET_RSS_KEY_DATA_RSS_KEY_SIZE;
> - vsi->rss_lut_size = hw->func_caps.common_cap.rss_table_size;
> + vsi->rss_lut_size = pf->hash_lut_size;
>
> if (is_safe_mode) {
> PMD_DRV_LOG(WARNING, "RSS is not supported in safe
> mode\n"); @@ -2033,7 +2039,7 @@ ice_dev_info_get(struct rte_eth_dev
> *dev, struct rte_eth_dev_info *dev_info)
> dev_info->rx_queue_offload_capa = 0;
> dev_info->tx_queue_offload_capa = 0;
>
> - dev_info->reta_size = hw->func_caps.common_cap.rss_table_size;
> + dev_info->reta_size = pf->hash_lut_size;
> dev_info->hash_key_size = (VSIQF_HKEY_MAX_INDEX + 1) *
> sizeof(uint32_t);
>
> dev_info->default_rxconf = (struct rte_eth_rxconf) { @@ -2605,28
> +2611,31 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
> uint16_t reta_size)
> {
> struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> - struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> - uint16_t i, lut_size = hw->func_caps.common_cap.rss_table_size;
> + uint16_t i, lut_size = pf->hash_lut_size;
> uint16_t idx, shift;
> uint8_t *lut;
> int ret;
>
> - if (reta_size != lut_size ||
> - reta_size > ETH_RSS_RETA_SIZE_512) {
> + if (reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128 &&
> + reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512 &&
> + reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K) {
> PMD_DRV_LOG(ERR,
> "The size of hash lookup table configured (%d)"
> "doesn't match the number hardware can "
> - "supported (%d)",
> - reta_size, lut_size);
> + "supported (128, 512, 2048)",
> + reta_size);
> return -EINVAL;
> }
>
> - lut = rte_zmalloc(NULL, reta_size, 0);
> + /* It MUST use the current LUT size to get the RSS lookup table,
> + * otherwise if will fail with -100 error code.
> + */
I think it's no need too detail explain. /* use current size to create lookup
table*/
> + lut = rte_zmalloc(NULL, RTE_MAX(reta_size, lut_size), 0);
> if (!lut) {
> PMD_DRV_LOG(ERR, "No memory can be allocated");
> return -ENOMEM;
> }
> - ret = ice_get_rss_lut(pf->main_vsi, lut, reta_size);
> + ret = ice_get_rss_lut(pf->main_vsi, lut, lut_size);
> if (ret)
> goto out;
>
> @@ -2637,6 +2646,12 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
> lut[i] = reta_conf[idx].reta[shift];
> }
> ret = ice_set_rss_lut(pf->main_vsi, lut, reta_size);
> + if (ret == 0 && lut_size != reta_size) {
> + PMD_DRV_LOG(INFO,
> + "The size of hash lookup table is changed from (%d)
> to (%d)",
> + lut_size, reta_size);
>
Re: [dpdk-dev] [PATCH v1] net/ice: update the RSS RETA size with support values
> -Original Message-
> From: Yang, Qiming
> Sent: Tuesday, May 21, 2019 13:50
> To: Wang, Haiyue ; dev@dpdk.org; Zhang, Qi Z
> ; Wu,
> Jingjing ; Lu, Wenzhuo
> Subject: RE: [PATCH v1] net/ice: update the RSS RETA size with support values
>
> Little comments.
>
> > -Original Message-
> > From: Wang, Haiyue
> > Sent: Monday, May 20, 2019 4:15 PM
> > To: dev@dpdk.org; Zhang, Qi Z ; Yang, Qiming
> > ; Wu, Jingjing ; Lu,
> > Wenzhuo
> > Cc: Wang, Haiyue
> > Subject: [PATCH v1] net/ice: update the RSS RETA size with support values
> >
> > Since ice can support 128, 512, 2K RSS RETA size value, change the update
> > API to set it to resize the RSS RETA table. And by default, use 512 to sync
> > with ETH_RSS_RETA_SIZE_x maximum value definition.
> > Also the flag ICE_FLAG_RSS_AQ_CAPABLE is missed to set.
> >
> > Fixes: 690175ee51bf ("net/ice: support getting device information")
> > Fixes: ff963bfa7cb1 ("net/ice: support RSS")
> >
> > Signed-off-by: Haiyue Wang
> > ---
> > drivers/net/ice/ice_ethdev.c | 41 +++--
> > 1 file changed, 27 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> > index
> > bbaa7cf..c4ea09f 100644
> > --- a/drivers/net/ice/ice_ethdev.c
> > +++ b/drivers/net/ice/ice_ethdev.c
> > @@ -1149,6 +1149,12 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type
> > type)
> > TAILQ_INIT(&vsi->mac_list);
> > TAILQ_INIT(&vsi->vlan_list);
> >
> > + /* Be sync with ETH_RSS_RETA_SIZE_x maximum value definition */
>
> I think this stats is not clear, can't understand what's the
> ETH_RSS_RETA_SIZE_x maximum value, why
> don't say the default max
> Value is 512?
>
The original plan was to add ETH_RSS_RETA_SIZE_2048, but found that testpmd use
512 as maximum in many
places, so just write down some comment here to sync with testpmd. And it
doesn't say 512 is default,
just because it is used in many places. If ETH_RSS_RETA_SIZE_2048 is added, we
can sync with it later.
> > + pf->hash_lut_size = hw->func_caps.common_cap.rss_table_size >
> > + ETH_RSS_RETA_SIZE_512 ? ETH_RSS_RETA_SIZE_512 :
> > + hw->func_caps.common_cap.rss_table_size;
> > + pf->flags |= ICE_FLAG_RSS_AQ_CAPABLE;
> > +
> > memset(&vsi_ctx, 0, sizeof(vsi_ctx));
> > /* base_queue in used in queue mapping of VSI add/update
> > command.
> > * Suppose vsi->base_queue is 0 now, don't consider SRIOV, VMDQ
> > @@ -1627,7 +1633,7 @@ static int ice_init_rss(struct ice_pf *pf)
> > rss_conf = &dev->data->dev_conf.rx_adv_conf.rss_conf;
> > nb_q = dev->data->nb_rx_queues;
> > vsi->rss_key_size = ICE_AQC_GET_SET_RSS_KEY_DATA_RSS_KEY_SIZE;
> > - vsi->rss_lut_size = hw->func_caps.common_cap.rss_table_size;
> > + vsi->rss_lut_size = pf->hash_lut_size;
> >
> > if (is_safe_mode) {
> > PMD_DRV_LOG(WARNING, "RSS is not supported in safe
> > mode\n"); @@ -2033,7 +2039,7 @@ ice_dev_info_get(struct rte_eth_dev
> > *dev, struct rte_eth_dev_info *dev_info)
> > dev_info->rx_queue_offload_capa = 0;
> > dev_info->tx_queue_offload_capa = 0;
> >
> > - dev_info->reta_size = hw->func_caps.common_cap.rss_table_size;
> > + dev_info->reta_size = pf->hash_lut_size;
> > dev_info->hash_key_size = (VSIQF_HKEY_MAX_INDEX + 1) *
> > sizeof(uint32_t);
> >
> > dev_info->default_rxconf = (struct rte_eth_rxconf) { @@ -2605,28
> > +2611,31 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
> > uint16_t reta_size)
> > {
> > struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > - struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > - uint16_t i, lut_size = hw->func_caps.common_cap.rss_table_size;
> > + uint16_t i, lut_size = pf->hash_lut_size;
> > uint16_t idx, shift;
> > uint8_t *lut;
> > int ret;
> >
> > - if (reta_size != lut_size ||
> > - reta_size > ETH_RSS_RETA_SIZE_512) {
> > + if (reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128 &&
> > + reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512 &&
> > + reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K) {
> > PMD_DRV_LOG(ERR,
> > "The size of hash lookup table configured (%d)"
> > "doesn't match the number hardware can "
> > - "supported (%d)",
> > - reta_size, lut_size);
> > + "supported (128, 512, 2048)",
> > + reta_size);
> > return -EINVAL;
> > }
> >
> > - lut = rte_zmalloc(NULL, reta_size, 0);
> > + /* It MUST use the current LUT size to get the RSS lookup table,
> > +* otherwise if will fail with -100 error code.
> > +*/
>
> I think it's no need too detail explain. /* use current size to create lookup
> table*/
>
In fact, this RTE_MAX 'lut' memory content is used to get the original lookup
table
firstly with original size, then
Re: [dpdk-dev] [PATCH v1] net/ice: update the RSS RETA size with support values
-Original Message-
From: Wang, Haiyue
Sent: Monday, May 20, 2019 4:15 PM
To: dev@dpdk.org; Zhang, Qi Z ; Yang, Qiming
; Wu, Jingjing ; Lu, Wenzhuo
Cc: Wang, Haiyue
Subject: [PATCH v1] net/ice: update the RSS RETA size with support values
Since ice can support 128, 512, 2K RSS RETA size value, change the update API
to set it to resize the RSS RETA table. And by default, use 512 to sync with
ETH_RSS_RETA_SIZE_x maximum value definition.
Also the flag ICE_FLAG_RSS_AQ_CAPABLE is missed to set.
Fixes: 690175ee51bf ("net/ice: support getting device information")
Fixes: ff963bfa7cb1 ("net/ice: support RSS")
Signed-off-by: Haiyue Wang
---
drivers/net/ice/ice_ethdev.c | 41 +++--
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
bbaa7cf..c4ea09f 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -1149,6 +1149,12 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
TAILQ_INIT(&vsi->mac_list);
TAILQ_INIT(&vsi->vlan_list);
+ /* Be sync with ETH_RSS_RETA_SIZE_x maximum value definition */
+ pf->hash_lut_size = hw->func_caps.common_cap.rss_table_size >
+ ETH_RSS_RETA_SIZE_512 ? ETH_RSS_RETA_SIZE_512 :
+ hw->func_caps.common_cap.rss_table_size;
+ pf->flags |= ICE_FLAG_RSS_AQ_CAPABLE;
+
memset(&vsi_ctx, 0, sizeof(vsi_ctx));
/* base_queue in used in queue mapping of VSI add/update command.
* Suppose vsi->base_queue is 0 now, don't consider SRIOV, VMDQ @@
-1627,7 +1633,7 @@ static int ice_init_rss(struct ice_pf *pf)
rss_conf = &dev->data->dev_conf.rx_adv_conf.rss_conf;
nb_q = dev->data->nb_rx_queues;
vsi->rss_key_size = ICE_AQC_GET_SET_RSS_KEY_DATA_RSS_KEY_SIZE;
- vsi->rss_lut_size = hw->func_caps.common_cap.rss_table_size;
+ vsi->rss_lut_size = pf->hash_lut_size;
if (is_safe_mode) {
PMD_DRV_LOG(WARNING, "RSS is not supported in safe mode\n"); @@
-2033,7 +2039,7 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
dev_info->rx_queue_offload_capa = 0;
dev_info->tx_queue_offload_capa = 0;
- dev_info->reta_size = hw->func_caps.common_cap.rss_table_size;
+ dev_info->reta_size = pf->hash_lut_size;
dev_info->hash_key_size = (VSIQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);
dev_info->default_rxconf = (struct rte_eth_rxconf) { @@ -2605,28
+2611,31 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
uint16_t reta_size)
{
struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- uint16_t i, lut_size = hw->func_caps.common_cap.rss_table_size;
+ uint16_t i, lut_size = pf->hash_lut_size;
uint16_t idx, shift;
uint8_t *lut;
int ret;
- if (reta_size != lut_size ||
- reta_size > ETH_RSS_RETA_SIZE_512) {
+ if (reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128 &&
+ reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512 &&
+ reta_size != ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K) {
PMD_DRV_LOG(ERR,
"The size of hash lookup table configured (%d)"
"doesn't match the number hardware can "
- "supported (%d)",
- reta_size, lut_size);
+ "supported (128, 512, 2048)",
+ reta_size);
return -EINVAL;
}
- lut = rte_zmalloc(NULL, reta_size, 0);
+ /* It MUST use the current LUT size to get the RSS lookup table,
+* otherwise if will fail with -100 error code.
+*/
+ lut = rte_zmalloc(NULL, RTE_MAX(reta_size, lut_size), 0);
if (!lut) {
PMD_DRV_LOG(ERR, "No memory can be allocated");
return -ENOMEM;
}
- ret = ice_get_rss_lut(pf->main_vsi, lut, reta_size);
+ ret = ice_get_rss_lut(pf->main_vsi, lut, lut_size);
if (ret)
goto out;
@@ -2637,6 +2646,12 @@ ice_rss_reta_update(struct rte_eth_dev *dev,
lut[i] = reta_conf[idx].reta[shift];
}
ret = ice_set_rss_lut(pf->main_vsi, lut, reta_size);
+ if (ret == 0 && lut_size != reta_size) {
+ PMD_DRV_LOG(INFO,
+ "The size of hash lookup table is changed from (%d)
to (%d)",
+ lut_size, reta_size);
+ pf->hash_lut_size = reta_size;
+ }
out:
rte_free(lut);
@@ -2650,14 +2665,12 @@ ice_rss_reta_query(struct rte_eth_dev *dev,
uint16_t reta_size)
{
struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(d
Re: [dpdk-dev] [PATCH] app/test-compress-perf: fix reliance on integer endianness
Hi Fiona,
Outlook issue :D , so once again
> -Original Message-
> From: Trahe, Fiona
> Sent: Monday, May 20, 2019 4:06 PM
> To: Jozwiak, TomaszX ; dev@dpdk.org;
> shal...@marvell.com; sta...@dpdk.org
> Cc: Trahe, Fiona ; Trybula, ArturX
>
> Subject: RE: [PATCH] app/test-compress-perf: fix reliance on integer
> endianness
>
> HI Tomasz,
>
> > -Original Message-
> > From: Jozwiak, TomaszX
> > Sent: Monday, May 20, 2019 2:26 PM
> > To: dev@dpdk.org; Trahe, Fiona ; Jozwiak,
> > TomaszX ; shal...@marvell.com;
> > sta...@dpdk.org
> > Subject: [PATCH] app/test-compress-perf: fix reliance on integer
> > endianness
> >
> > This patch fixes coverity issue:
> > Reliance on integer endianness (INCOMPATIBLE_CAST) in
> parse_window_sz
> > function.
> >
> > Coverity issue: 328524
> > Fixes: e0b6287c035d ("app/compress-perf: add parser")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tomasz Jozwiak
> > ---
> > app/test-compress-perf/comp_perf_options_parse.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-compress-perf/comp_perf_options_parse.c
> > b/app/test-compress- perf/comp_perf_options_parse.c index
> > 2fb6fb4..56ca580 100644
> > --- a/app/test-compress-perf/comp_perf_options_parse.c
> > +++ b/app/test-compress-perf/comp_perf_options_parse.c
> > @@ -364,13 +364,15 @@ parse_max_num_sgl_segs(struct
> comp_test_data
> > *test_data, const char *arg) static int parse_window_sz(struct
> > comp_test_data *test_data, const char *arg) {
> > - int ret = parse_uint16_t((uint16_t *)&test_data->window_sz, arg);
> > + uint16_t tmp;
> > + int ret = parse_uint16_t(&tmp, arg);
> >
> > if (ret) {
> > RTE_LOG(ERR, USER1, "Failed to parse window size\n");
> > return -1;
> > }
> >
> > + test_data->window_sz = (int)tmp;
> > return 0;
> > }
> [Fiona] I expect this fixes this coverity issue - but will it result in
> another one?
> window_sz on the xform is uint8_t - so this int will get truncated later, and
> there's no cast done at that point.
> Would it be better to add a new parse_uint8_t fn and change test-data-
> >window_sz to a unit8_t?
> Or add that cast?
[Tomek] I measn it's ok. There's a check inside comp_perf_check_capabilities
function.
If the value from test_data->window_sz > cap->window_size we have a fail.
Also during parsing there's a check is value from command line between 0 and
UINT16_MAX,
so in my opinion all cases are tested. The point is there's only one place where
we're parsing uint8_t value. parse_uint8_t function will be especially for
that.
