Re: [dpdk-dev] [PATCH v4 11/11] doc: update mlx5 guide on tunnel offloading
On Tue, Apr 17, 2018 at 11:14:36PM +0800, Xueming Li wrote: > Remove tunnel limitations, add new hardware tunnel offload features. > > Signed-off-by: Xueming Li > --- > doc/guides/nics/features/default.ini | 1 + > doc/guides/nics/features/mlx5.ini| 3 +++ > doc/guides/nics/mlx5.rst | 22 -- > 3 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/nics/features/default.ini > b/doc/guides/nics/features/default.ini > index dae2ad776..49be81450 100644 > --- a/doc/guides/nics/features/default.ini > +++ b/doc/guides/nics/features/default.ini > @@ -29,6 +29,7 @@ Multicast MAC filter = > RSS hash = > RSS key update = > RSS reta update = > +Inner RSS= > VMDq = > SR-IOV = > DCB = > diff --git a/doc/guides/nics/features/mlx5.ini > b/doc/guides/nics/features/mlx5.ini > index f8ce08770..e75b14bdc 100644 > --- a/doc/guides/nics/features/mlx5.ini > +++ b/doc/guides/nics/features/mlx5.ini > @@ -21,6 +21,7 @@ Multicast MAC filter = Y > RSS hash = Y > RSS key update = Y > RSS reta update = Y > +Inner RSS= Y > SR-IOV = Y > VLAN filter = Y > Flow director= Y > @@ -30,6 +31,8 @@ VLAN offload = Y > L3 checksum offload = Y > L4 checksum offload = Y > Timestamp offload= Y > +Inner L3 checksum= Y > +Inner L4 checksum= Y > Packet type parsing = Y > Rx descriptor status = Y > Tx descriptor status = Y > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst > index c28c83278..51590b0a3 100644 > --- a/doc/guides/nics/mlx5.rst > +++ b/doc/guides/nics/mlx5.rst > @@ -74,12 +74,12 @@ Features > - RX interrupts. > - Statistics query including Basic, Extended and per queue. > - Rx HW timestamp. > +- Tunnel types: VXLAN, L3 VXLAN, VXLAN-GPE, GRE, MPLS-in-GRE, MPLS-in-UDP. > +- Tunnel HW offloads: packet type, inner/outer RSS, IP and UDP checksum > verification. > > Limitations > --- > > -- Inner RSS for VXLAN frames is not supported yet. > -- Hardware checksum RX offloads for VXLAN inner header are not supported yet. > - For secondary process: > >- Forked secondary process not supported. > @@ -327,6 +327,24 @@ Run-time configuration > >Enabled by default, valid only on VF devices ignored otherwise. > > +Firmware configuration > +~~ > + > +- L3 VXLAN and VXLAN-GPE destination UDP port > + > + .. code-block:: console > + > + mlxconfig -d set IP_OVER_VXLAN_EN=1 > + mlxconfig -d set IP_OVER_VXLAN_PORT= > + > + Verify configurations are set: > + > + .. code-block:: console > + > + mlxconfig -d query | grep IP_OVER_VXLAN > + IP_OVER_VXLAN_ENTrue(1) > + IP_OVER_VXLAN_PORT 4790 > + > Prerequisites > - > > -- > 2.13.3 The documentation modification related to the L3 VXLAN should be in the same patch as the code in mlx5. Thanks, -- Nélio Laranjeiro 6WIND
Re: [dpdk-dev] [PATCH v2 0/4] Hyper-V Netvsc PMD
I'm facing this on gcc version 5.4.0 on 64bit.
Kindest regards,
Raslan Darawsheh
-Original Message-
From: Stephen Hemminger [mailto:step...@networkplumber.org]
Sent: Tuesday, April 17, 2018 7:22 PM
To: Raslan Darawsheh
Cc: dev@dpdk.org; Stephen Hemminger ; Thomas Monjalon
Subject: Re: [dpdk-dev] [PATCH v2 0/4] Hyper-V Netvsc PMD
On Tue, 17 Apr 2018 09:02:45 +
Raslan Darawsheh wrote:
> Hi Stephan,
>
> I've just tried to apply your patches on top of current master branch and I
> have the following two issues:
> 1- compilation is failing with the following error when compiling with
> RTE_TARGET=x86_64-native-linuxapp-gcc:
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_common.h:122:30: error:
> cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
> #define RTE_PTR_ADD(ptr, x) ((void*)((uintptr_t)(ptr) + (x)))
> ^
> /root/dpdk/buildtools/pmdinfogen/pmdinfogen.c:99:9: note: in expansion of
> macro 'RTE_PTR_ADD'
> return RTE_PTR_ADD(info->hdr,
> ^
> 2- compilation is also failing with the following errors when compiling with
> RTE_TARGET=i686-native-linuxapp-gcc
> CC linux/vmbus_uio.o
> /root/dpdk/drivers/bus/vmbus/linux/vmbus_bus.c: In function
> 'rte_vmbus_map_device':
> /root/dpdk/drivers/bus/vmbus/linux/vmbus_bus.c:194:39: error: passing
> argument 2 of 'eal_parse_sysfs_value' from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>if (eal_parse_sysfs_value(filename, &res->len) < 0) {
>^
> In file included from /root/dpdk/drivers/bus/vmbus/linux/vmbus_bus.c:22:0:
> /root/dpdk/lib/librte_eal/common/eal_filesystem.h:123:5: note: expected 'long
> unsigned int *' but argument is of type 'uint64_t * {aka long long unsigned
> int *}'
> int eal_parse_sysfs_value(const char *filename, unsigned long *val);
>
>
> I noticed one additional new dependency which is uuid-dev and I think you
> might be missing the documentation of that.
>
> Kindest regards,
> Raslan Darawsheh
I rebased against dpdk.org master yesterday.
What version of Gcc? I don't see this on Gcc 7.3.0 and build on 64 bit.
Will update the documentation and fix a couple of typos.
Re: [dpdk-dev] [PATCH v2 2/3] net/szedata2: add support for new NIC
On 13.04.2018 18:29, Ferruh Yigit wrote: On 4/13/2018 4:03 PM, Jan Remeš wrote: It is mentioned on the website with the packages itself. The current version works with the reworked PMD - there is a headline which says: "DPDK from 18.05" (since we expect the patches to land in the upcoming release; we will update it if this does not happen). The older version is still available on the website, with appropriate installation steps and a headline that says "DPDK up to 18.02". Is this form of documentation sufficient for you or would you like to see it mentioned in the DPDK documentation as well? I think it is better to have some level of information in DPDK documentation too. There is already a release note update in this set, do you think does it make sense to mention new dependency there? I will get patches right now, please send doc/release notes updates as incremental patches. Hi Ferruh, we will send the follow-up patches updating both documentation and release notes soon. Thanks, Matej Thanks, Jan Jan Remeš | Software Developer Netcope Technologies, a.s. T: +420 530 510 680 A: Sochorova 3232/34, Brno, 616 00, Czech Republic W: www.netcope.com On Fri, Apr 13, 2018 at 4:26 PM, Ferruh Yigit wrote: On 4/13/2018 9:17 AM, Jan Remeš wrote: Hi Matej, Where szedata_get_area_numa_node() is defined? Is it possible that you are missing a patch? Thanks, ferruh Hi Ferruh, the new PMD requires an updated version of libsze2 and drivers. We have just published the updated packages here: https://www.netcope.com/en/company/community-support/dpdk-libsze2. DPDK documentation mentions about libsze2 dependency but there is no version mentioned. And I have not seen any note that says a new version of library is required, perhaps I missed it, is there a documentation which says which version of DPDK requires which version of the dependencies? What is the way of tracing this dependency? Sorry for the hickup. Best regards, Jan
Re: [dpdk-dev] [PATCH] eal: force gcc to inline rte_movX function
18/04/2018 04:43, Chen, Junjie J: > Thanks to point this out. I agree for the title change. > > Do you want me to send v2 patch? Or you can handle it when committing? > > > > > Sometimes gcc does not inline the function despite keyword *inline*, > > > > we obeserve rte_movX is not inline when doing performance profiling, > > > > so use *always_inline* keyword to force gcc to inline the function. > > > > > > > > Signed-off-by: Chen, Junjie > > > > --- > > > > .../common/include/arch/x86/rte_memcpy.h | 22 > > +++--- > > > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > > > The title should start with "eal/x86:" > > > Something like that: > > > eal/x86: force inlining of memcpy sub-functions > > > > > > Bruce, Konstantin, any review of the content/optimization? > > > > > No objection here. > > > > Acked-by: Bruce Richardson Applied, thanks
[dpdk-dev] [PATCH] examples/performance-thread: convert license headers to SPDX
Convert dual license headers with Intel and Hasan Alayli names to SPDX. Signed-off-by: Pablo de Lara --- examples/performance-thread/common/arch/x86/ctx.c | 62 ++ examples/performance-thread/common/lthread.c | 61 ++--- examples/performance-thread/common/lthread.h | 62 ++ examples/performance-thread/common/lthread_api.h | 62 ++ examples/performance-thread/common/lthread_cond.c | 61 ++--- examples/performance-thread/common/lthread_cond.h | 61 ++--- examples/performance-thread/common/lthread_int.h | 61 ++--- examples/performance-thread/common/lthread_sched.c | 62 ++ examples/performance-thread/common/lthread_sched.h | 61 ++--- 9 files changed, 27 insertions(+), 526 deletions(-) diff --git a/examples/performance-thread/common/arch/x86/ctx.c b/examples/performance-thread/common/arch/x86/ctx.c index 1e8e27177..d63fd9fc0 100644 --- a/examples/performance-thread/common/arch/x86/ctx.c +++ b/examples/performance-thread/common/arch/x86/ctx.c @@ -1,65 +1,9 @@ -/*- - * BSD LICENSE - * - * Copyright(c) 2015 Intel Corporation. All rights reserved. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Intel Corporation nor the names of its - * contributors may be used to endorse or promote products derived - * from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - /* - * https://github.com/halayli/lthread which carries the following license. - * - * Copyright (C) 2012, Hasan Alayli - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - *notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - *notice, this list of conditions and the following disclaimer in the - *documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY AUTHOR AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. + * SPDX-License-Identifier: BSD-3-Clause + * Copyright 2015 Intel Corporation. + * Copyright 2012 Hasan Alayli */ - - #if defined(__x86_64__) __asm__ ( ".text\n" diff --git a/examples/performance-thread/common/lthread.c b/examples/performance-thread/common/lthread.c index 7d76c8c46..c9e0a5297 100644 --- a/examples/performance-thread/common/lthread.c +++ b/examples/performance-thread/common/lthread.c @@ -1,62 +1,7 @@ -/*- - * BSD LICENSE - * - * Copyright(c) 2015 Intel Corporation. All rights reserved. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions -
Re: [dpdk-dev] [PATCH 01/30] crypto/qat: use SPDX license
> -Original Message- > From: Trahe, Fiona > Sent: Friday, April 6, 2018 7:52 PM > To: dev@dpdk.org; De Lara Guarch, Pablo > Cc: Trahe, Fiona ; Jozwiak, TomaszX > > Subject: [PATCH 01/30] crypto/qat: use SPDX license > > Use SPDK license for dual-licensed files and update license date in all files > > Signed-off-by: Fiona Trahe Applied this single patch to dpdk-next-crypto. Thanks, Pablo
Re: [dpdk-dev] [PATCH 2/2] net/mlx4: fix alignment of Memory Region
On Tue, Apr 17, 2018 at 11:39:14AM -0700, Yongseok Koh wrote:
> The memory region is [start, end), so if the memseg of 'end' isn't
> allocated yet, the returned memseg will have zero entries and this will
> make 'end' zero (nil).
>
> Fixes: c2fe5823224a ("net/mlx4: use virt2memseg instead of iteration")
> Cc: Anatoly Burakov
>
> Signed-off-by: Yongseok Koh
Acked-by: Adrien Mazarguil
One nit in case another version is sent, please remove the unnecessary empty
line in both patches, thanks.
> ---
> drivers/net/mlx4/mlx4_mr.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
> index e69b43322..f2dcbcb76 100644
> --- a/drivers/net/mlx4/mlx4_mr.c
> +++ b/drivers/net/mlx4/mlx4_mr.c
> @@ -144,9 +144,7 @@ mlx4_mr_get(struct priv *priv, struct rte_mempool *mp)
> ms = rte_mem_virt2memseg((void *)start, NULL);
> if (ms != NULL)
> start = RTE_ALIGN_FLOOR(start, ms->hugepage_sz);
> - ms = rte_mem_virt2memseg((void *)end, NULL);
> - if (ms != NULL)
> - end = RTE_ALIGN_CEIL(end, ms->hugepage_sz);
> + end = RTE_ALIGN_CEIL(end, ms->hugepage_sz);
>
> DEBUG("mempool %p using start=%p end=%p size=%zu for MR",
> (void *)mp, (void *)start, (void *)end,
> --
> 2.11.0
>
--
Adrien Mazarguil
6WIND
[dpdk-dev] [PATCH v4] drivers/net/i40e: fix missing promiscuous disable at device disable
v4 updates: === - Add some comments for this patch v3 updates: === - Move modification from device close to device disable - i40evf_reset_vf() will cause kernel driver enable all vlan promiscuous, so unicast/multicast promiscuous disable should set before reset. v2 updates: === - Add more comments In scenario of Kernel Driver runs on PF and PMD runs on VF, PMD exit doesn't disable promiscuous mode, this will cause vlan filter set by Kernel Driver will not take effect. This patch will fix it, add promiscuous disable at device disable. Signed-off-by: Rosen Xu --- drivers/net/i40e/i40e_ethdev_vf.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index 031c706..91fbf6c 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -2288,6 +2288,14 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) i40evf_dev_stop(dev); i40e_dev_free_queues(dev); + /* +* disable promiscuous mode before reset vf +* it is a workaround solution when work with kernel driver +* and it is not the normal way +*/ + i40evf_dev_promiscuous_disable(dev); + i40evf_dev_allmulticast_disable(dev); + i40evf_reset_vf(hw); i40e_shutdown_adminq(hw); /* disable uio intr before callback unregister */ -- 1.8.3.1
[dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
"port config all rss all" command will fail on PMD that not support any
of hard coding RSS hash types. This patch changed hard coding hash types
to supported types retrieved from device info.
Signed-off-by: Xueming Li
---
app/test-pmd/cmdline.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 512e3b55e..d357de7e6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result,
{
struct cmd_config_rss *res = parsed_result;
struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
+ struct rte_eth_dev_info dev_info = {0};
int diag;
uint8_t i;
@@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
}
rss_conf.rss_key = NULL;
for (i = 0; i < rte_eth_dev_count(); i++) {
+ if (!strcmp(res->value, "all")) {
+ rte_eth_dev_info_get(i, &dev_info);
+ if (dev_info.flow_type_rss_offloads)
+ rss_conf.rss_hf =
+ dev_info.flow_type_rss_offloads;
+ }
diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
if (diag < 0)
printf("Configuration of RSS hash at ethernet port %d "
--
2.13.3
[dpdk-dev] [PATCH v3 1/2] ethdev: add supported hash function check
Add supported RSS hash function check in device configuration to
have better error verbosity for application developers.
Signed-off-by: Xueming Li
---
lib/librte_ether/rte_ethdev.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 3c049ef43..a6212f146 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1179,6 +1179,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t
nb_rx_q, uint16_t nb_tx_q,
ETHER_MAX_LEN;
}
+ /* Check that device supports requested rss hash functions. */
+ if ((dev_info.flow_type_rss_offloads |
+dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
+ dev_info.flow_type_rss_offloads) {
+ RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: "
+ "0x"PRIx64", valid value: 0x%"PRIx64"\n",
+ port_id,
+ dev_conf->rx_adv_conf.rss_conf.rss_hf,
+ dev_info.flow_type_rss_offloads);
+ return -EINVAL;
+ }
+
/*
* Setup new number of RX/TX queues and reconfigure device.
*/
@@ -2835,9 +2847,20 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
struct rte_eth_rss_conf *rss_conf)
{
struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info = {0};
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
+ rte_eth_dev_info_get(port_id, &dev_info);
+ if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) !=
+ dev_info.flow_type_rss_offloads) {
+ RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: "
+ "0x"PRIx64", valid value: 0x%"PRIx64"\n",
+ port_id,
+ rss_conf->rss_hf,
+ dev_info.flow_type_rss_offloads);
+ return -EINVAL;
+ }
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP);
return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
rss_conf));
--
2.13.3
Re: [dpdk-dev] [PATCH v5 2/4] ethdev: introduce new tunnel VXLAN-GPE
On Tue, Apr 17, 2018 at 10:56:52PM +0200, Thomas Monjalon wrote:
> 17/04/2018 17:04, Xueming Li:
> > +struct rte_flow_item_vxlan_gpe {
> > + uint8_t flags; /**< Normally 0x0c (I and P flags). */
> > + uint8_t rsvd0[2]; /**< Reserved, normally 0x. */
> > + uint8_t protocol; /**< Protocol type. */
> > + uint8_t vni[3]; /**< VXLAN identifier. */
> > + uint8_t rsvd1; /**< Reserved, normally 0x00. */
> > +};
>
> These comments are not aligned.
Regarding this part: it's OK for rte_flow.h since it's the style used in
that file. Besides I favor it as it circumvents the entire alignemnt issue
altogether for subsequent contributions.
> > +struct vxlan_gpe_hdr {
> > + uint8_t vx_flags; /**< flag (8). */
> > + uint8_t reserved[2];/**< Reserved (16). */
> > + uint8_t proto; /**< next-protocol (8). */
> > + uint32_t vx_vni;/**< VNI (24) + Reserved (8). */
> > +} __attribute__((__packed__));
>
> Alignment must be done with spaces.
>
>
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [PATCH v4 01/16] ethdev: add error types to flow API
On Tue, Apr 17, 2018 at 08:37:31PM +0100, Ferruh Yigit wrote: > On 4/16/2018 5:22 PM, Adrien Mazarguil wrote: > > These enable more precise reporting of objects responsible for errors. > > > > This breaks ABI compatibility for the following public functions: > > > > - rte_flow_create() > > - rte_flow_destroy() > > - rte_flow_error_set() > > - rte_flow_flush() > > - rte_flow_isolate() > > - rte_flow_query() > > - rte_flow_validate() > > Is there a deprecation notice for this API break? A notice covering the main changes in this series (most patches have an ABI impact) was sent but not included [1]. This particular patch rides on the announced ABI breakage in order to add a relatively minor feature while there. This ABI change was implicitly needed by upcoming work for 18.05 (Xueming's RSS stuff [2][3], Declan's TEP [4], the rest is summarized by a RFC [5]) due to the necessary changes of behavior in flow rules. Note that Xueming's deprecation notice [3] alone would have triggered such an ABI change because struct rte_flow_action_rss wouldn't have been binary compatible if struct rte_eth_rss_conf was updated. This change would have propagated back to rte_flow functions manipulating them. [1] "doc: announce API changes for flow rules" http://dpdk.org/ml/archives/dev/2018-February/090988.html [2] "MLX5 tunnel Rx offloading" http://dpdk.org/ml/archives/dev/2018-February/091461.html [3] "doc: annouce ABI change for RSS configuraiton structure" http://dpdk.org/ml/archives/dev/2018-February/090127.html [4] "tunnel endpoint hw acceleration enablement" http://dpdk.org/ml/archives/dev/2017-December/084676.html [5] "Switch device offload with DPDK" http://dpdk.org/ml/archives/dev/2018-March/092513.html > > Signed-off-by: Adrien Mazarguil > > Acked-by: Andrew Rybchenko > > <...> > -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] [PATCH 0/4] Fixes for coverity issues in fbarray
On Tue, Apr 17, 2018 at 04:44:04PM +0100, Anatoly Burakov wrote: > This patchset fixes a number of Coverity issues introduced > in recent DPDK memory hotplug patchset. > > Coverity issues fixed: > - 272564 - error condition not checked > - 272579 - dereference before null check > - 272586 - error condition not checked > > There are two additional issues reported by coverity: > - 272598 - error condition not checked > - 272599 - error condition not checked > > However, they are fixed by a separate patch [1]. > > [1] http://dpdk.org/dev/patchwork/patch/38091/ > > Anatoly Burakov (4): > fbarray: use strlcpy instead of snprintf > fbarray: add check for failed file descriptor open > fbarray: fix potential null-dereference > fbarray: handle negative return Given that I recently addressed a similar issue in fbarray as well: Acked-by: Adrien Mazarguil -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] [RFC] checkpatch: don't complain about SPDX tag format
On Wed, Apr 18, 2018 at 12:19:07AM +0200, Thomas Monjalon wrote: > 18/04/2018 00:11, Scott Branden: > > On 18-04-17 03:06 PM, Thomas Monjalon wrote: > > > 17/04/2018 23:49, Stephen Hemminger: > > >> IMHO would have been better to use the kernel SPDX style and > > >> keep the check but that appears to be a minority opinion. > > > > > > I think it is better to work on checkpatch itself. > > > When defining our SPDX style, Linux one was not definitive. > > > Do you think we can ask the Linux community to support our SPDX style? > > > > > I think it better to simply follow the Linux community defacto style > > rather than go your own way. > > But our way is better! :) > And it has been decided in the Technical Board. > As a general issue, I think we could do with having our own checkpatch-like script for performing addition DPDK-specific code-checks *after* Linux checkpatch ones. That is, reuse Linux check patch checks as much as possible, but have other checks too. For example, check for use of strcpy or strncpy (or snprintf with "%s") and suggest replacing with strlcpy. If we did have our own extension script, we could put our own SPDX format check there too. Thoughts, or any volunteers to look into this? /Bruce
Re: [dpdk-dev] [PATCH] net/tap: remove queue specific offload support
On 4/12/2018 5:23 PM, Ferruh Yigit wrote:
> On 4/5/2018 6:49 PM, Thomas Monjalon wrote:
>> Pascal, Moti, Ophir,
>> please comment.
>
> Hi Moti,
>
> Any comment? This has been asked many times now.
Hi Moti, Ophir,
You have not responded why queue specific offload added in other thread.
And you are not responding to this patch...
Hi Pascal,
If you also have no objection, this patch is going in.
Thanks,
ferruh
>
>>
>> 22/03/2018 19:28, Ferruh Yigit:
>>> It is not clear if tap PMD supports queue specific offloads, removing
>>> the related code.
>>>
>>> Fixes: 95ae196ae10b ("net/tap: use new Rx offloads API")
>>> Fixes: 818fe14a9891 ("net/tap: use new Tx offloads API")
>>> Cc: mo...@mellanox.com
>>>
>>> Signed-off-by: Ferruh Yigit
>>
>>
>>
>
Re: [dpdk-dev] [PATCH v3] mem: fix memory leaks of hugedir and replace snprintf
18/04/2018 05:09, Yangchao Zhou:
> The hugedir returned by get_hugepage_dir is allocated by strdup
> but not released. Replace snprintf with a more suitable strlcpy.
>
> Coverity issue: 272585
> Fixes: cb97d93e9d3b ("mem: share hugepage info primary and secondary")
>
> Signed-off-by: Yangchao Zhou
> Acked-by: Anatoly Burakov
There was a misunderstanding.
You replaced some snprintf which were not involved in the leaks.
But I think it's fine.
Applied, thanks
[dpdk-dev] [PATCH] lib/librte_ip_frag:fix ip frag process log
in ip_frag_process,some IP_FRAG_LOG content is wrong Signed-off-by: Li Han --- lib/librte_ip_frag/ip_frag_internal.c | 10 +- ...40e-add-null-point-check-and-fix-mem-leak.patch | 150 +++ ...1000-add-null-point-check-for-rte_zmalloc.patch | 65 ...qat-add-null-point-check-and-fix-mem-leak.patch | 84 +++ ...qat-add-null-point-check-and-fix-mem-leak.patch | 92 ...1000-add-null-point-check-for-rte_zmalloc.patch | 77 ++ ...40e-add-null-point-check-and-fix-mem-leak.patch | 163 + 7 files changed, 636 insertions(+), 5 deletions(-) create mode 100644 v3-0001-net-i40e-add-null-point-check-and-fix-mem-leak.patch create mode 100644 v3-0002-net-e1000-add-null-point-check-for-rte_zmalloc.patch create mode 100644 v3-0003-crypto-qat-add-null-point-check-and-fix-mem-leak.patch create mode 100644 v4-0001-crypto-qat-add-null-point-check-and-fix-mem-leak.patch create mode 100644 v4-0001-net-e1000-add-null-point-check-for-rte_zmalloc.patch create mode 100644 v4-0001-net-i40e-add-null-point-check-and-fix-mem-leak.patch diff --git a/lib/librte_ip_frag/ip_frag_internal.c b/lib/librte_ip_frag/ip_frag_internal.c index 7397aa6..7db5bc6 100644 --- a/lib/librte_ip_frag/ip_frag_internal.c +++ b/lib/librte_ip_frag/ip_frag_internal.c @@ -152,9 +152,9 @@ struct rte_mbuf * fp->frags[IP_LAST_FRAG_IDX].len); else IP_FRAG_LOG(DEBUG, "%s:%d invalid fragmented packet:\n" - "ipv4_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " + "ipv6_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " "total_size: %u, frag_size: %u, last_idx: %u\n" - "first fragment: ofs: %u, len: %u\n" + i "first fragment: ofs: %u, len: %u\n" "last fragment: ofs: %u, len: %u\n\n", __func__, __LINE__, fp, IPv6_KEY_BYTES(fp->key.src_dst), fp->key.id, @@ -210,7 +210,7 @@ struct rte_mbuf * fp->frags[IP_LAST_FRAG_IDX].len); else IP_FRAG_LOG(DEBUG, "%s:%d invalid fragmented packet:\n" - "ipv4_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " + "ipv6_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " "total_size: %u, frag_size: %u, last_idx: %u\n" "first fragment: ofs: %u, len: %u\n" "last fragment: ofs: %u, len: %u\n\n", @@ -331,7 +331,7 @@ struct ip_frag_pkt * if (p1->key.key_len == IPV4_KEYLEN) IP_FRAG_LOG(DEBUG, "%s:%d:\n" "tbl: %p, max_entries: %u, use_entries: %u\n" - "ipv6_frag_pkt line0: %p, index: %u from %u\n" + "ipv4_frag_pkt line0: %p, index: %u from %u\n" "key: <%" PRIx64 ", %#x>, start: %" PRIu64 "\n", __func__, __LINE__, tbl, tbl->max_entries, tbl->use_entries, @@ -357,7 +357,7 @@ struct ip_frag_pkt * if (p2->key.key_len == IPV4_KEYLEN) IP_FRAG_LOG(DEBUG, "%s:%d:\n" "tbl: %p, max_entries: %u, use_entries: %u\n" - "ipv6_frag_pkt line1: %p, index: %u from %u\n" + "ipv4_frag_pkt line1: %p, index: %u from %u\n" "key: <%" PRIx64 ", %#x>, start: %" PRIu64 "\n", __func__, __LINE__, tbl, tbl->max_entries, tbl->use_entries, diff --git a/v3-0001-net-i40e-add-null-point-check-and-fix-mem-leak.patch b/v3-0001-net-i40e-add-null-point-check-and-fix-mem-leak.patch new file mode 100644 index 000..766ca68 --- /dev/null +++ b/v3-0001-net-i40e-add-null-point-check-and-fix-mem-leak.patch @@ -0,0 +1,150 @@ +From b506152e205041f8138e728e135571a024f8a5a6 Mon Sep 17 00:00:00 2001 +From: Yong Wang +Date: Wed, 20 Dec 2017 05:14:28 -0500 +Subject: [PATCH v3 1/3] net/i40e: add null point check and fix mem leak + +Signed-off-by: Yong Wang +--- +v2: +* Fix code style warning. +--- + drivers/net/i40e/i40e_ethdev.c | 32 + drivers/net/i40e/i40e_fdir.c | 7 +++ + 2 files changed, 39 insertions(+) + +diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c +index c4df65d..277c1a8 100644 +--- a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c +@@ -7188,11 +7188,13 @@ struct i40e_tunnel_filter * + node = i40e_sw_tunnel_filter_lookup(tunnel_rule,
Re: [dpdk-dev] reply: [disscussion] A problem about dpdk backup-mode bond switching with mlx4 VF devices
On Wed, Apr 18, 2018 at 01:55:41AM +, chenchanghu wrote: > Hi, Adrien Mazarguil, > > Thanks for your reply very much. > > It means in our tests, when ‘ifconfig eth7 down’, the expect result is the > bond primary netdevice will switch to eth8. > > However, we find the bond primary is not changed to eth8 in 19 times of 20 > time tests., and it means the dpdk bond doesn't receive an LSC interrupt > signal. > > In our test, the netdevice is mlx4 VF in Virtual Machine, which netdevice is > direct through by SR-IOV. I overlooked this part, I'm not sure VFs impact link status on the DPDK side since they can't really bring it down. Regardless, you should have encountered an identical behavior 20 out of 20 times otherwise there could be a bug in the DPDK version you're using. > We also test mlx4 PF in Physical Machine, when ‘ifconfig eth7 down’, the > test esult is the bond primary netdevice switched to eth8. Right, PF has authority over link status, this is expected as documented. > Quesetion: > > Is it related with SR-IOV direct through ? For example, the VF netdevice > status changed, but it will not send an LSC interruput signal. > > Looking forward to your any reply. To summarize, updating the link status of a netdevice associated with a VF shouldn't impact a DPDK application. On the other hand updating it on a netdevice associated with PF will impact all VFs and their applications. Please check again with a more recent DPDK version (e.g. 18.02). If you manage to get a consistent behavior every time, it means a bug is present in in 16.04. > -邮件原件- > 发件人: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > 发送时间: 2018年4月17日 18:08 > 收件人: chenchanghu > 抄送: dev@dpdk.org; nelio.laranje...@6wind.com; Zhoujingbin (Robin, Cloud > Networking) ; Zhoulei (G) ; > yangleyuan > 主题: Re: [disscussion] A problem about dpdk backup-mode bond switching with > mlx4 VF devices > > > > On Tue, Apr 17, 2018 at 06:40:20AM +, chenchanghu wrote: > > > > > > Hi, > > > When I used the mlx4 pmd, I meet a problem about mlx4 VF bond > > switching which bond mod is backup-mode . The detail test is descripted > > below. > > > 1.Test environmemt infomation: > > > a. Linux distribution: CentOS > > > b. dpdk version: dpdk-16.04 > > > c. Ethernet device : mlx4 VF > > > d. pmd info: mlx4 poll-mode-driver > > > > > > 2.Test step: > > > a. we bond the mlx4 VF Ethernet device eth7,eth8 into backup-mode by dpdk > > application. Eth7 and eth8 are both active, and eth7 is the primary device. > > > b. As we know, the device eth7 , eth8 are also visible by kernel driver > > mlx4_en. > > > c. Then we config the Ethernet device eth7 down by the command ' ifconfig > > eth7 down', the expect result is the bond primary device will not switch. > > > d. However we find the dpdk bond primary device switch to eth8 by dpdk > > maintenance interface one time in all 20 test times. > > > > > > 3.Question: > > >Is the VF up or down State of kernel interface has some relations to > > user-space state? For example, when ifconfig eth7 down, and the user-space > > will change to down state too. > > > > Yes, this is expected. Netdevices and the mlx4 DPDK PMD share a common link > status. Bringing a netdevice down causes link status to be down for all its > users. This behavior is documented [1]. > > > > [1] http://dpdk.org/doc/guides/nics/mlx4.html#run-time-configuration > > > > -- > > Adrien Mazarguil > > 6WIND -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] [dpdk-web] [PATCH v2] update stable releases roadmap
On 4/11/2018 12:28 AM, Thomas Monjalon wrote: > - Typically a new stable release version follows a mainline release > - by 1-2 weeks, depending on the test results. > + The first stable release (.1) of a branch should follow > + its mainline release (.0) by at least two months, > + after the first release candidate (-rc1) of the next branch. Hi Thomas, What this change suggest? To be able to backport patches from rc1?
Re: [dpdk-dev] [dpdk-web] [PATCH v2] update stable releases roadmap
18/04/2018 11:05, Ferruh Yigit: > On 4/11/2018 12:28 AM, Thomas Monjalon wrote: > > - Typically a new stable release version follows a mainline release > > - by 1-2 weeks, depending on the test results. > > + The first stable release (.1) of a branch should follow > > + its mainline release (.0) by at least two months, > > + after the first release candidate (-rc1) of the next branch. > > Hi Thomas, > > What this change suggest? To be able to backport patches from rc1? Yes, it is the proposal we discussed earlier. We can wait one week after RC1 to get some validation confirmation. Do you agree?
[dpdk-dev] 答复: reply: [disscussion] A problem about dpdk backup-mode bond switching with mlx4 VF devices
Thanks very much, OK, we will test again with recent dpdk version. -邮件原件- 发件人: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] 发送时间: 2018年4月18日 17:05 收件人: chenchanghu 抄送: dev@dpdk.org; nelio.laranje...@6wind.com; Zhoujingbin (Robin, Cloud Networking) ; Zhoulei (G) ; yangleyuan 主题: Re: reply: [disscussion] A problem about dpdk backup-mode bond switching with mlx4 VF devices On Wed, Apr 18, 2018 at 01:55:41AM +, chenchanghu wrote: > Hi, Adrien Mazarguil, > > Thanks for your reply very much. > > It means in our tests, when ‘ifconfig eth7 down’, the expect result is the > bond primary netdevice will switch to eth8. > > However, we find the bond primary is not changed to eth8 in 19 times of 20 > time tests., and it means the dpdk bond doesn't receive an LSC interrupt > signal. > > In our test, the netdevice is mlx4 VF in Virtual Machine, which netdevice is > direct through by SR-IOV. I overlooked this part, I'm not sure VFs impact link status on the DPDK side since they can't really bring it down. Regardless, you should have encountered an identical behavior 20 out of 20 times otherwise there could be a bug in the DPDK version you're using. > We also test mlx4 PF in Physical Machine, when ‘ifconfig eth7 down’, the > test esult is the bond primary netdevice switched to eth8. Right, PF has authority over link status, this is expected as documented. > Quesetion: > > Is it related with SR-IOV direct through ? For example, the VF netdevice > status changed, but it will not send an LSC interruput signal. > > Looking forward to your any reply. To summarize, updating the link status of a netdevice associated with a VF shouldn't impact a DPDK application. On the other hand updating it on a netdevice associated with PF will impact all VFs and their applications. Please check again with a more recent DPDK version (e.g. 18.02). If you manage to get a consistent behavior every time, it means a bug is present in in 16.04. > -邮件原件- > 发件人: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > 发送时间: 2018年4月17日 18:08 > 收件人: chenchanghu > 抄送: dev@dpdk.org; nelio.laranje...@6wind.com; Zhoujingbin (Robin, > Cloud Networking) ; Zhoulei (G) > ; yangleyuan > 主题: Re: [disscussion] A problem about dpdk backup-mode bond switching > with mlx4 VF devices > > > > On Tue, Apr 17, 2018 at 06:40:20AM +, chenchanghu wrote: > > > > > > Hi, > > > When I used the mlx4 pmd, I meet a problem about mlx4 VF bond > > switching which bond mod is backup-mode . The detail test is descripted > > below. > > > 1.Test environmemt infomation: > > > a. Linux distribution: CentOS > > > b. dpdk version: dpdk-16.04 > > > c. Ethernet device : mlx4 VF > > > d. pmd info: mlx4 poll-mode-driver > > > > > > 2.Test step: > > > a. we bond the mlx4 VF Ethernet device eth7,eth8 into backup-mode by dpdk > > application. Eth7 and eth8 are both active, and eth7 is the primary device. > > > b. As we know, the device eth7 , eth8 are also visible by kernel driver > > mlx4_en. > > > c. Then we config the Ethernet device eth7 down by the command ' ifconfig > > eth7 down', the expect result is the bond primary device will not switch. > > > d. However we find the dpdk bond primary device switch to eth8 by dpdk > > maintenance interface one time in all 20 test times. > > > > > > 3.Question: > > >Is the VF up or down State of kernel interface has some relations to > > user-space state? For example, when ifconfig eth7 down, and the user-space > > will change to down state too. > > > > Yes, this is expected. Netdevices and the mlx4 DPDK PMD share a common link > status. Bringing a netdevice down causes link status to be down for all its > users. This behavior is documented [1]. > > > > [1] http://dpdk.org/doc/guides/nics/mlx4.html#run-time-configuration > > > > -- > > Adrien Mazarguil > > 6WIND -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH] lib/librte_ip_frag:fix ip frag process log
in ip_frag_process,some IP_FRAG_LOG content is wrong Signed-off-by: Li Han --- lib/librte_ip_frag/ip_frag_internal.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/librte_ip_frag/ip_frag_internal.c b/lib/librte_ip_frag/ip_frag_internal.c index 7397aa6..2560c77 100644 --- a/lib/librte_ip_frag/ip_frag_internal.c +++ b/lib/librte_ip_frag/ip_frag_internal.c @@ -152,7 +152,7 @@ struct rte_mbuf * fp->frags[IP_LAST_FRAG_IDX].len); else IP_FRAG_LOG(DEBUG, "%s:%d invalid fragmented packet:\n" - "ipv4_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " + "ipv6_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " "total_size: %u, frag_size: %u, last_idx: %u\n" "first fragment: ofs: %u, len: %u\n" "last fragment: ofs: %u, len: %u\n\n", @@ -210,7 +210,7 @@ struct rte_mbuf * fp->frags[IP_LAST_FRAG_IDX].len); else IP_FRAG_LOG(DEBUG, "%s:%d invalid fragmented packet:\n" - "ipv4_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " + "ipv6_frag_pkt: %p, key: <" IPv6_KEY_BYTES_FMT ", %#x>, " "total_size: %u, frag_size: %u, last_idx: %u\n" "first fragment: ofs: %u, len: %u\n" "last fragment: ofs: %u, len: %u\n\n", @@ -331,7 +331,7 @@ struct ip_frag_pkt * if (p1->key.key_len == IPV4_KEYLEN) IP_FRAG_LOG(DEBUG, "%s:%d:\n" "tbl: %p, max_entries: %u, use_entries: %u\n" - "ipv6_frag_pkt line0: %p, index: %u from %u\n" + "ipv4_frag_pkt line0: %p, index: %u from %u\n" "key: <%" PRIx64 ", %#x>, start: %" PRIu64 "\n", __func__, __LINE__, tbl, tbl->max_entries, tbl->use_entries, @@ -357,7 +357,7 @@ struct ip_frag_pkt * if (p2->key.key_len == IPV4_KEYLEN) IP_FRAG_LOG(DEBUG, "%s:%d:\n" "tbl: %p, max_entries: %u, use_entries: %u\n" - "ipv6_frag_pkt line1: %p, index: %u from %u\n" + "ipv4_frag_pkt line1: %p, index: %u from %u\n" "key: <%" PRIx64 ", %#x>, start: %" PRIu64 "\n", __func__, __LINE__, tbl, tbl->max_entries, tbl->use_entries, -- 1.8.3.1
Re: [dpdk-dev] [PATCH v4 01/16] ethdev: add error types to flow API
On 4/18/2018 9:41 AM, Adrien Mazarguil wrote: > On Tue, Apr 17, 2018 at 08:37:31PM +0100, Ferruh Yigit wrote: >> On 4/16/2018 5:22 PM, Adrien Mazarguil wrote: >>> These enable more precise reporting of objects responsible for errors. >>> >>> This breaks ABI compatibility for the following public functions: >>> >>> - rte_flow_create() >>> - rte_flow_destroy() >>> - rte_flow_error_set() >>> - rte_flow_flush() >>> - rte_flow_isolate() >>> - rte_flow_query() >>> - rte_flow_validate() >> >> Is there a deprecation notice for this API break? > > A notice covering the main changes in this series (most patches have an ABI > impact) was sent but not included [1]. This particular patch rides on the > announced ABI breakage in order to add a relatively minor feature while > there. My take from "announced ABI breakage" is the deprecation notice get three acks and merged into release, so it seems there is no deprecation notice and according process first deprecation notice should go in this release. Hi Thomas, Any comment on issue? > > This ABI change was implicitly needed by upcoming work for 18.05 (Xueming's > RSS stuff [2][3], Declan's TEP [4], the rest is summarized by a RFC [5]) due > to the necessary changes of behavior in flow rules. > > Note that Xueming's deprecation notice [3] alone would have triggered such > an ABI change because struct rte_flow_action_rss wouldn't have been binary > compatible if struct rte_eth_rss_conf was updated. This change would have > propagated back to rte_flow functions manipulating them. To be honest I lost track of Xueming's patches, because of split/merge of patchset, multiple set with multiple versions out. Is it possible to document the dependency graph including your set? > > [1] "doc: announce API changes for flow rules" > http://dpdk.org/ml/archives/dev/2018-February/090988.html > [2] "MLX5 tunnel Rx offloading" > http://dpdk.org/ml/archives/dev/2018-February/091461.html > [3] "doc: annouce ABI change for RSS configuraiton structure" > http://dpdk.org/ml/archives/dev/2018-February/090127.html > [4] "tunnel endpoint hw acceleration enablement" > http://dpdk.org/ml/archives/dev/2017-December/084676.html > [5] "Switch device offload with DPDK" > http://dpdk.org/ml/archives/dev/2018-March/092513.html > >>> Signed-off-by: Adrien Mazarguil >>> Acked-by: Andrew Rybchenko >> >> <...> >> >
Re: [dpdk-dev] [PATCH v3] mem: fix memory leaks of hugedir and replace snprintf
On 18-Apr-18 10:00 AM, Thomas Monjalon wrote:
18/04/2018 05:09, Yangchao Zhou:
The hugedir returned by get_hugepage_dir is allocated by strdup
but not released. Replace snprintf with a more suitable strlcpy.
Coverity issue: 272585
Fixes: cb97d93e9d3b ("mem: share hugepage info primary and secondary")
Signed-off-by: Yangchao Zhou
Acked-by: Anatoly Burakov
There was a misunderstanding.
You replaced some snprintf which were not involved in the leaks.
But I think it's fine.
Applied, thanks
I'll check to see if i have to rebase my strlcpy patchset.
--
Thanks,
Anatoly
[dpdk-dev] [PATCH v2] app/bbdev: remove improper WARNING printouts
Improper WARNING printouts in BBDev Test Application removed
Signed-off-by: KamilX Chalupnik
v2:
- apply patch failure fixed
---
app/test-bbdev/test_bbdev_perf.c | 2 +-
app/test-bbdev/test_bbdev_vector.c | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
index 00f3b08..84a5393 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -138,7 +138,7 @@ check_dev_cap(const struct rte_bbdev_info *dev_info)
!(cap->capability_flags &
RTE_BBDEV_TURBO_SOFT_OUTPUT)) {
printf(
- "WARNING: Device \"%s\" does not
support soft output - soft output flags will be ignored.\n",
+ "INFO: Device \"%s\" does not support
soft output - soft output flags will be ignored.\n",
dev_info->dev_name);
clear_soft_out_cap(
&test_vector.turbo_dec.op_flags);
diff --git a/app/test-bbdev/test_bbdev_vector.c
b/app/test-bbdev/test_bbdev_vector.c
index addef05..0f19912 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -642,7 +642,7 @@ check_decoder_llr_spec(struct test_bbdev_vector *vector)
!(turbo_dec->op_flags &
RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN)) {
printf(
- "WARNING: input LLR sign formalism was not specified
and will be set to negative LLR for '1' bit\n");
+ "INFO: input LLR sign formalism was not specified and
will be set to negative LLR for '1' bit\n");
turbo_dec->op_flags |= RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN;
}
@@ -661,7 +661,7 @@ check_decoder_llr_spec(struct test_bbdev_vector *vector)
!(turbo_dec->op_flags &
RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT)) {
printf(
- "WARNING: soft output LLR sign formalism was not
specified and will be set to negative LLR for '1' bit\n");
+ "INFO: soft output LLR sign formalism was not specified
and will be set to negative LLR for '1' bit\n");
turbo_dec->op_flags |=
RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT;
}
@@ -722,7 +722,7 @@ check_decoder(struct test_bbdev_vector *vector)
}
if (!(mask & TEST_BBDEV_VF_RV_INDEX))
printf(
- "WARNING: rv_index was not specified in vector file and
will be set to 0\n");
+ "INFO: rv_index was not specified in vector file and
will be set to 0\n");
if (!(mask & TEST_BBDEV_VF_ITER_MIN))
printf(
"WARNING: iter_min was not specified in vector file and
will be set to 0\n");
@@ -742,7 +742,7 @@ check_decoder(struct test_bbdev_vector *vector)
} else if (!(turbo_dec->op_flags & RTE_BBDEV_TURBO_MAP_DEC) &&
mask & TEST_BBDEV_VF_NUM_MAPS) {
printf(
- "WARNING: RTE_BBDEV_TURBO_MAP_DEC was not set in vector
file and num_maps will be set to 0\n");
+ "INFO: RTE_BBDEV_TURBO_MAP_DEC was not set in vector
file and num_maps will be set to 0\n");
turbo_dec->num_maps = 0;
}
if (!(mask & TEST_BBDEV_VF_EXPECTED_STATUS))
@@ -827,10 +827,10 @@ check_encoder(struct test_bbdev_vector *vector)
}
if (!(mask & TEST_BBDEV_VF_RV_INDEX))
printf(
- "WARNING: rv_index was not specified in vector file and
will be set to 0\n");
+ "INFO: rv_index was not specified in vector file and
will be set to 0\n");
if (!(mask & TEST_BBDEV_VF_OP_FLAGS))
printf(
- "WARNING: op_flags was not specified in vector file and
capabilities will not be validated\n");
+ "INFO: op_flags was not specified in vector file and
capabilities will not be validated\n");
if (!(mask & TEST_BBDEV_VF_EXPECTED_STATUS))
printf(
"WARNING: expected_status was not specified in vector
file and will be set to 0\n");
--
2.5.5
--
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
Re: [dpdk-dev] [PATCH] net/tap: remove queue specific offload support
Hi Ferruh,
Sorry for the delayed response.
I would like to verify the correctness of this patch by running several
internal tests.
Is a reply by Monday OK with you?
Regards,
Ophir
> -Original Message-
> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
> Sent: Wednesday, April 18, 2018 11:59 AM
> To: Thomas Monjalon ; Pascal Mazon
> ; Mordechay Haimovsky
> ; Ophir Munk
> Cc: dev@dpdk.org; Shahaf Shuler ; Olga Shern
>
> Subject: Re: [dpdk-dev] [PATCH] net/tap: remove queue specific offload
> support
>
> On 4/12/2018 5:23 PM, Ferruh Yigit wrote:
> > On 4/5/2018 6:49 PM, Thomas Monjalon wrote:
> >> Pascal, Moti, Ophir,
> >> please comment.
> >
> > Hi Moti,
> >
> > Any comment? This has been asked many times now.
>
> Hi Moti, Ophir,
>
> You have not responded why queue specific offload added in other thread.
> And you are not responding to this patch...
>
> Hi Pascal,
>
> If you also have no objection, this patch is going in.
>
> Thanks,
> ferruh
>
>
> >
> >>
> >> 22/03/2018 19:28, Ferruh Yigit:
> >>> It is not clear if tap PMD supports queue specific offloads,
> >>> removing the related code.
> >>>
> >>> Fixes: 95ae196ae10b ("net/tap: use new Rx offloads API")
> >>> Fixes: 818fe14a9891 ("net/tap: use new Tx offloads API")
> >>> Cc: mo...@mellanox.com
> >>>
> >>> Signed-off-by: Ferruh Yigit
> >>
> >>
> >>
> >
Re: [dpdk-dev] [PATCH v11 00/10] crypto: add virtio poll mode driver
> -Original Message- > From: Jay Zhou [mailto:jianjay.z...@huawei.com] > Sent: Tuesday, April 17, 2018 10:23 AM > To: dev@dpdk.org > Cc: De Lara Guarch, Pablo ; Zhang, Roy Fan > ; tho...@monjalon.net; > arei.gong...@huawei.com; Zeng, Xin ; > weidong.hu...@huawei.com; wangxinxin.w...@huawei.com; > longpe...@huawei.com; jianjay.z...@huawei.com > Subject: [PATCH v11 00/10] crypto: add virtio poll mode driver > > This patch series introduce virtio crypto poll mode driver. Series applied to dpdk-next-crypto. Thanks for the work! Pablo
Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix usage print
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Radu Nicolau
> Sent: Tuesday, April 17, 2018 10:02 AM
> To: Anoob Joseph ; Akhil Goyal
> ; Doherty, Declan
> Cc: Jerin Jacob ; Narayana Prasad
> ; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix usage print
>
>
> On 4/16/2018 1:23 PM, Anoob Joseph wrote:
> > The usage print was not updated when jumbo frames & crypto_dev mask
> > support was added. Fixing that. Also, the optional arguments were not
> > properly highlighted in the usage header. This is also fixed.
> >
> > General cleanup of the usage print was also done to make it look more
> > cleaner and similar to what is existing in other applications like
> > l3fwd.
> >
> > Fixes: bbabfe6e4ee4 ("examples/ipsec_secgw: support jumbo frames")
> > Fixes: 2c68fe791538 ("examples/ipsec-secgw: add cryptodev mask
> > option")
> > Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample
> > application")
> >
> > Signed-off-by: Anoob Joseph
> > ---
> >
> Acked-by: Radu Nicolau
>
Cc'ing stable list.
Applied to dpdk-next-crypto.
Thanks,
Pablo
[dpdk-dev] [PATCH v2 00/10] Coverity fixes for EAL memory
This patchset fixes a host of coverity issues in memory subsystem introduced with recent DPDK memory hotplug patchset. Coverity issues fixed: - 272601 - leaking fd - 272560 - double close fd - 272568 - leaking fd - 272570 - leaking fd - 272589 - dereference before null check - 272602 - freeing wrong pointer - 272608 - expression does nothing - 272577 - negative return not handled - 272578 - negative return not handled - 272584 - use after free Additionally, also replace all instances of snprintf with strlcpy. v2: - Rebase on top of latest master Anatoly Burakov (10): mem: use strlcpy instead of snprintf mem: fix resource leak mem: fix potential double close mem: fix potential resource leak mem: fix potential resource leak mem: fix comparing pointer to value mem: fix potential bad unmap mem: fix statement having no effect mem: fix negative return value mem: fix possible use-after-free lib/librte_eal/common/eal_common_memalloc.c | 5 +++-- lib/librte_eal/common/eal_common_memory.c | 23 +-- lib/librte_eal/linuxapp/eal/eal_memalloc.c | 22 -- lib/librte_eal/linuxapp/eal/eal_memory.c| 1 + 4 files changed, 37 insertions(+), 14 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v2 04/10] mem: fix potential resource leak
We close fd if we managed to find it in the list of allocated
segment lists (which should always be the case under normal
conditions), but if we didn't, the fd was leaking. Close it if
we couldn't find it in the segment list. This is not an issue
as if the segment is zero length, we're getting rid of it
anyway, so there's no harm in not storing the fd anywhere.
Coverity issue: 272568
Fixes: 2a04139f66b4 ("eal: add single file segments option")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memalloc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 9156f8b..fab5a98 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -569,6 +569,8 @@ free_seg(struct rte_memseg *ms, struct hugepage_info *hi,
if (te != NULL && te->fd >= 0) {
close(te->fd);
te->fd = -1;
+ } else {
+ close(fd);
}
unlink(path);
}
--
2.7.4
[dpdk-dev] [PATCH v2 02/10] mem: fix resource leak
Coverity issue: 272601
Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memory.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
b/lib/librte_eal/linuxapp/eal/eal_memory.c
index fadc1de..9351e84 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1750,6 +1750,7 @@ eal_legacy_hugepage_attach(void)
if (map_addr == MAP_FAILED) {
RTE_LOG(ERR, EAL, "Could not map %s: %s\n",
hf->filepath, strerror(errno));
+ close(fd);
goto error;
}
--
2.7.4
[dpdk-dev] [PATCH v2 07/10] mem: fix potential bad unmap
Previously, if mmap failed to map page address at requested
address, we were attempting to unmap the wrong address. Fix it
by unmapping our actual mapped address, and jump further to
avoid unmapping memory that is not allocated.
Coverity issue: 272602
Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memalloc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 8420a26..6a75e5b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -466,7 +466,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
}
if (va != addr) {
RTE_LOG(DEBUG, EAL, "%s(): wrong mmap() address\n", __func__);
- goto mapped;
+ munmap(va, alloc_sz);
+ goto resized;
}
/* for non-single file segments, we can close fd here */
if (!internal_config.single_file_segments)
--
2.7.4
[dpdk-dev] [PATCH v2 01/10] mem: use strlcpy instead of snprintf
Signed-off-by: Anatoly Burakov --- Notes: One other instance of using snprintf is left alone as it is expected to be addressed by another patch [1]. [1] http://dpdk.org/dev/patchwork/patch/38301/ lib/librte_eal/common/eal_common_memalloc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/common/eal_common_memalloc.c b/lib/librte_eal/common/eal_common_memalloc.c index 49fd53c..e983688 100644 --- a/lib/librte_eal/common/eal_common_memalloc.c +++ b/lib/librte_eal/common/eal_common_memalloc.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "eal_private.h" @@ -179,7 +180,7 @@ eal_memalloc_mem_event_callback_register(const char *name, /* callback successfully created and is valid, add it to the list */ entry->clb = clb; - snprintf(entry->name, RTE_MEM_EVENT_CALLBACK_NAME_LEN, "%s", name); + strlcpy(entry->name, name, RTE_MEM_EVENT_CALLBACK_NAME_LEN); TAILQ_INSERT_TAIL(&mem_event_callback_list, entry, next); ret = 0; @@ -284,7 +285,7 @@ eal_memalloc_mem_alloc_validator_register(const char *name, entry->clb = clb; entry->socket_id = socket_id; entry->limit = limit; - snprintf(entry->name, RTE_MEM_ALLOC_VALIDATOR_NAME_LEN, "%s", name); + strlcpy(entry->name, name, RTE_MEM_ALLOC_VALIDATOR_NAME_LEN); TAILQ_INSERT_TAIL(&mem_alloc_validator_list, entry, next); ret = 0; -- 2.7.4
[dpdk-dev] [PATCH v2 03/10] mem: fix potential double close
We were closing descriptor before checking if mapping has
failed, but if it did, we did a second close afterwards. Fix
it by moving closing descriptor to after we check if mmap has
succeeded.
Coverity issue: 272560
Fixes: 2a04139f66b4 ("eal: add single file segments option")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memalloc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 1f553dd..9156f8b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -458,9 +458,6 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
*/
void *va = mmap(addr, alloc_sz, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_POPULATE | MAP_FIXED, fd, map_offset);
- /* for non-single file segments, we can close fd here */
- if (!internal_config.single_file_segments)
- close(fd);
if (va == MAP_FAILED) {
RTE_LOG(DEBUG, EAL, "%s(): mmap() failed: %s\n", __func__,
@@ -471,6 +468,9 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
RTE_LOG(DEBUG, EAL, "%s(): wrong mmap() address\n", __func__);
goto mapped;
}
+ /* for non-single file segments, we can close fd here */
+ if (!internal_config.single_file_segments)
+ close(fd);
rte_iova_t iova = rte_mem_virt2iova(addr);
if (iova == RTE_BAD_PHYS_ADDR) {
--
2.7.4
[dpdk-dev] [PATCH v2 10/10] mem: fix possible use-after-free
If user has specified a flag to unmap the area right after mapping it,
we were passing an already-unmapped pointer to RTE_LOG. This is not an
issue since RTE_LOG doesn't actually dereference the pointer, but fix
it anyway by moving call to RTE_LOG to before unmap.
Coverity issue: 272584
Fixes: b7cc54187ea4 ("mem: move virtual area function in common directory")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/common/eal_common_memory.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_memory.c
b/lib/librte_eal/common/eal_common_memory.c
index 68fc70e..e979eba 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -113,12 +113,12 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
RTE_LOG(WARNING, EAL, " This may cause issues with mapping
memory into secondary processes\n");
}
- if (unmap)
- munmap(mapped_addr, map_sz);
-
RTE_LOG(DEBUG, EAL, "Virtual area found at %p (size = 0x%zx)\n",
aligned_addr, *size);
+ if (unmap)
+ munmap(mapped_addr, map_sz);
+
baseaddr_offset += *size;
return aligned_addr;
--
2.7.4
[dpdk-dev] [PATCH v2 08/10] mem: fix statement having no effect
Coverity reports these lines as having no effect. Technically, we do
want for those lines to have no effect, however they would've likely
been optimized out. Add volatile qualifiers to ensure the code has
effects.
Coverity issue: 272608
Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memalloc.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 6a75e5b..655c69e 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -503,7 +503,12 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
(unsigned int)(alloc_sz >> 20));
goto mapped;
}
- *(int *)addr = *(int *)addr;
+ /* we need to trigger a write to the page to enforce page fault and
+* ensure that page is accessible to us, but we can't overwrite value
+* that is already there, so read the old value, and write itback.
+* kernel populates the page with zeroes initially.
+*/
+ *(volatile int *)addr = *(volatile int *)addr;
ms->addr = addr;
ms->hugepage_sz = alloc_sz;
--
2.7.4
[dpdk-dev] [PATCH v2 09/10] mem: fix negative return value
Although unlikely during normal operation, rte_socket_id_by_idx()
may return a negative value, which would've caused an out-of-bounds
read. Fix it by making socket ID signed, and check for negative
return.
Coverity issue: 272577
Coverity issue: 272578
Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/common/eal_common_memory.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_memory.c
b/lib/librte_eal/common/eal_common_memory.c
index 24a9ed5..68fc70e 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -205,7 +205,8 @@ memseg_primary_init_32(void)
{
struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
int active_sockets, hpi_idx, msl_idx = 0;
- unsigned int socket_id, i;
+ unsigned int i;
+ int socket_id;
struct rte_memseg_list *msl;
uint64_t extra_mem_per_socket, total_extra_mem, total_requested_mem;
uint64_t max_mem;
@@ -238,6 +239,11 @@ memseg_primary_init_32(void)
uint64_t mem;
socket_id = rte_socket_id_by_idx(i);
+ if (socket_id < 0) {
+ RTE_LOG(ERR, EAL, "Invalid socket index: %u\n",
+ i);
+ continue;
+ }
mem = internal_config.socket_mem[socket_id];
if (mem == 0)
@@ -281,6 +287,10 @@ memseg_primary_init_32(void)
bool skip;
socket_id = rte_socket_id_by_idx(i);
+ if (socket_id < 0) {
+ RTE_LOG(ERR, EAL, "Invalid socket index: %u\n", i);
+ continue;
+ }
#ifndef RTE_EAL_NUMA_AWARE_HUGEPAGES
if (socket_id > 0)
@@ -294,10 +304,11 @@ memseg_primary_init_32(void)
* socket, and this is not master lcore
*/
master_lcore_socket = rte_lcore_to_socket_id(cfg->master_lcore);
- skip |= active_sockets == 0 && socket_id != master_lcore_socket;
+ skip |= active_sockets == 0 &&
+ (unsigned int)socket_id != master_lcore_socket;
if (skip) {
- RTE_LOG(DEBUG, EAL, "Will not preallocate memory on
socket %u\n",
+ RTE_LOG(DEBUG, EAL, "Will not preallocate memory on
socket %i\n",
socket_id);
continue;
}
--
2.7.4
[dpdk-dev] [PATCH v2 06/10] mem: fix comparing pointer to value
Previous code had an old rebase leftover from the time when
oldpolicy was an actual int, instead of a pointer. Fix it to
do comparison with dereferencing the pointer.
Coverity issue: 272589
Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index b02e3a5..8420a26 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -146,7 +146,7 @@ resotre_numa(int *oldpolicy, struct bitmask *oldmask)
{
RTE_LOG(DEBUG, EAL,
"Restoring previous memory policy: %d\n", *oldpolicy);
- if (oldpolicy == MPOL_DEFAULT) {
+ if (*oldpolicy == MPOL_DEFAULT) {
numa_set_localalloc();
} else if (set_mempolicy(*oldpolicy, oldmask->maskp,
oldmask->size + 1) < 0) {
--
2.7.4
[dpdk-dev] [PATCH v2 05/10] mem: fix potential resource leak
Normally, tailq entry should have a valid fd by the time we attempt
to map the segment. However, in case it doesn't, we're leaking fd,
so fix it.
Coverity issue: 272570
Fixes: 2a04139f66b4 ("eal: add single file segments option")
Cc: anatoly.bura...@intel.com
Signed-off-by: Anatoly Burakov
---
lib/librte_eal/linuxapp/eal/eal_memalloc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index fab5a98..b02e3a5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -524,6 +524,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
if (te != NULL && te->fd >= 0) {
close(te->fd);
te->fd = -1;
+ } else {
+ close(fd);
}
/* ignore errors, can't make it any worse */
unlink(path);
--
2.7.4
Re: [dpdk-dev] [PATCH] drivers/net: update link status
On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> Hi Ferruh,
>
>
> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
>> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>> On Tue, Mar 13, 2018 at 06:05:34PM +, Ferruh Yigit wrote:
Update link status related feature document items and minor updates in
some link status related functions.
Signed-off-by: Ferruh Yigit
---
doc/guides/nics/features/fm10k.ini | 2 ++
doc/guides/nics/features/fm10k_vf.ini | 2 ++
doc/guides/nics/features/i40e_vf.ini| 1 +
doc/guides/nics/features/igb_vf.ini | 1 +
doc/guides/nics/features/qede.ini | 1 -
doc/guides/nics/features/qede_vf.ini| 1 -
doc/guides/nics/features/vhost.ini | 2 --
doc/guides/nics/features/virtio_vec.ini | 1 +
drivers/net/e1000/em_ethdev.c | 2 +-
drivers/net/ena/ena_ethdev.c| 2 +-
drivers/net/fm10k/fm10k_ethdev.c| 6 ++
drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
drivers/net/ixgbe/ixgbe_ethdev.c| 2 +-
drivers/net/mlx4/mlx4_ethdev.c | 2 +-
drivers/net/mlx5/mlx5_ethdev.c | 2 +-
15 files changed, 15 insertions(+), 14 deletions(-)
>>> [...]
diff --git a/doc/guides/nics/features/vhost.ini
b/doc/guides/nics/features/vhost.ini
index dffd1f493..31302745a 100644
--- a/doc/guides/nics/features/vhost.ini
+++ b/doc/guides/nics/features/vhost.ini
@@ -4,8 +4,6 @@
; Refer to default.ini for the full list of available PMD features.
;
[Features]
-Link status = Y
-Link status event= Y
>>> I think vhost PMD supports above features.
>> I am not able to find where it is supported.
>>
>> Some virtual PMDs report fixed link, with empty link_update() dev_ops,
>> and they
>> are not reported as supporting Link status, as far as I can see vhost
>> also one
>> of them.
>>
>> And for Link status event, PMD needs to support LSC interrupts and should
>> register interrupt handler for it, which I can't find for vhost.
>>
>> I will send next version without updating above one, please point me
>> where these
>> support added if I missed them.
> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>
> static int
> new_device(int vid)
> {
> ..
>
> eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>
> ..
>
> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>
> ..
> }
>
> static void
> destroy_device(int vid)
> {
> ..
>
> eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>
> ..
>
> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>
> ..
> }
>
> They are the callbacks for vhost library.
>
> When a frontend (e.g. QEMU) is connected to this vhost backend
> and the frontend virtio device becomes ready, new_device() will
> be called by the vhost library, and the link status will be
> updated to UP.
>
> And when e.g. the connection is closed, destroy_device() will be
> called by the vhost library, and the link status will be updated
> to DOWN.
Got it. This behavior is similar for virtual PMDs. Provide static link
information and update link as UP during start and update it as DOWN
during stop.
>>> No, the link status isn't updated during vhost PMD start
>>> and stop. When the vhost PMD has been started, the link
>>> status still may be DOWN. The link status becomes UP only
>>> when the QEMU (it's another virtual machine process which
>>> has a virtio device) connects to this vhost PMD via a UNIX
>>> socket and the virtio driver in the virtual machine has
>>> setup the virtio device of the virtual machine.
>>>
>>> So if vhost PMD reports the link status as DOWN, it means
>>> there is no QEMU (virtual machine) connects to it or the
>>> virtio device in the virtual machine hasn't been setup.
>>> (PS. The frontend can also be virtio-user PMD besides QEMU)
>> I believe announcing link feature reporting on virtual pmds still in gray
>> area,
>> but because of qemu involvement in vhost case, I will keep link feature but
>> will
>> drop link event.
>
> AFAIK, link status means we can get link status through APIs like
> rte_eth_link_get(); while link status event means applications can
> register link status events, and those events get called if link status
> is changed.
>
> If
Re: [dpdk-dev] [RFC] checkpatch: don't complain about SPDX tag format
Hello,
> On Wed, Apr 18, 2018 at 12:19:07AM +0200, Thomas Monjalon wrote:
> > 18/04/2018 00:11, Scott Branden:
> > > On 18-04-17 03:06 PM, Thomas Monjalon wrote:
> > > > 17/04/2018 23:49, Stephen Hemminger:
> > > >> IMHO would have been better to use the kernel SPDX style and keep
> > > >> the check but that appears to be a minority opinion.
> > > >
> > > > I think it is better to work on checkpatch itself.
> > > > When defining our SPDX style, Linux one was not definitive.
> > > > Do you think we can ask the Linux community to support our SPDX style?
> > > >
> > > I think it better to simply follow the Linux community defacto style
> > > rather than go your own way.
> >
> > But our way is better! :)
> > And it has been decided in the Technical Board.
> >
>
> As a general issue, I think we could do with having our own checkpatch-like
> script for performing addition DPDK-specific code-checks *after* Linux
> checkpatch ones. That is, reuse Linux check patch checks as much as possible,
> but have other checks too.
>
> For example, check for use of strcpy or strncpy (or snprintf with "%s") and
> suggest replacing with strlcpy. If we did have our own extension script, we
> could put our own SPDX format check there too.
>
> Thoughts, or any volunteers to look into this?
In addition, the checkpatches.sh could be improved so that it actually checks
that a proper file is found behind the selected env variable. I am planning to
add this check (as it bite me just yesterday).
Speaking of strlcpy, I do think that it has a caveat* that everybody should be
aware of: depending on implementation, it may read unintended memory regions
when the source is not properly null terminated (like in Unix domain sockets,
or just by other mistake). It may be a bad idea just blindly replace everything
with strlcpy, without making sure that copied buffers are really
null-terminated in the first place or making sure the strlcpy version is really
a one that does not have this problem. As it depends on dynamic libraries,
making sure may be difficult.
Some may argue that this is unlikely and thus irrelevant. Why do I know about
it then? :) Needless to say, strncpy or snprintf do not have _this_ problem,
although they have their own issues. Internally without dynamic libs DPDK
rte_strlcpy uses snprintf which should be safe, though.
> /Bruce
--
Juhamatti
* A caveat on some implementations:
...
/* Not enough room in dst, add NUL and traverse rest of src */
if (n == 0) {
if (siz != 0)
*d = '\0'; /* NUL-terminate dst */
while (*s++) <- what happens when s is not null-terminated?
;
}
...
Another one:
...
return n + strlen (src); <- what happens when src is not null-terminated?
...
Re: [dpdk-dev] [RFC 2/2] nfp: allow for non-root user
On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole wrote:
> Alejandro Lucero writes:
>
> > I was just wondering, if device device PCI sysfs resource files or VFIO
> group /dev files require to change
> > permissions for non-root users, does it not make sense to adjust also
> /var/lock in the system?
>
> For the /dev, we use udev rules - so the correct individual vfio device
> files get assigned the correct permissions. No such mechanism exists
> for /var/lock as far as I can tell.
>
> Ex. see:
>
> https://github.com/openvswitch/ovs/blob/master/
> rhel/usr_lib_udev_rules.d_91-vfio.rules
>
> Maybe something similar exists that we could use to generate the lock
> file automatically?
>
What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
Is RH forcing OVS DPDK to only work if the host has IOMMU support?
>
> > On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero <
> alejandro.luc...@netronome.com> wrote:
> >
> > I have seen that VFIO also requires explicitly to set the right
> permissions for non-root users to VFIO
> > groups under /dev/vfio.
> >
> > I assume then that running OVS or other DPDK apps as non-root is
> possible, although requiring
> > those explicit permissions changes, and therefore this patch is
> necessary.
> >
> > Adding stable@ and Thomas for discussing how can this be added to
> stable DPDK versions even if
> > this is not going to be a patch for current DPDK version.
> >
> > Acked-by: Alejandro Lucero
> >
> > On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero <
> alejandro.luc...@netronome.com> wrote:
> >
> > On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole
> wrote:
> >
> > Alejandro Lucero writes:
> >
> > > Again, this patch is correct, but because NFP PMD needs to access
> > > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and
> these files have
> > just
> > > read/write accesses for root, I do not know if this is really
> necessary.
> > >
> > > Being honest, I have not used a DPDK app with NFP PMD and not being
> root. Does it
> > work
> > > with non-root users and other PMDs with same requirements regarding
> sysfs resource
> > files?
> >
> > We do run as non-root user definitely with Intel PMDs.
> >
> > I'm not very sure about other vendors, but I think mlx pmd runs as
> > non-root user (and it was modified to move off of sysfs for that
> > reason[1]).
> >
> > It is possible to not rely on sysfs resource files if device is
> attached to VFIO, but I think that is a
> > must with UIO.
> >
> >
> > I'll continue to push for more information from the testing side to find
> > out though.
> >
> > [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
> >
> > > On Fri, Apr 13, 2018 at 12:22 AM, Aaron Conole
> wrote:
> > >
> > > Currently, the nfp lock files are taken from the global lock file
> > > location, which will work when the user is running as root. However,
> > > some distributions and applications (notably ovs 2.8+ on RHEL/Fedora)
> > > run as a non-root user.
> > >
> > > Signed-off-by: Aaron Conole
> > > ---
> > > drivers/net/nfp/nfp_nfpu.c | 23 ++-
> > > 1 file changed, 18 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/nfp/nfp_nfpu.c b/drivers/net/nfp/nfp_nfpu.c
> > > index 2ed985ff4..ae2e07220 100644
> > > --- a/drivers/net/nfp/nfp_nfpu.c
> > > +++ b/drivers/net/nfp/nfp_nfpu.c
> > > @@ -18,6 +18,22 @@
> > > #define NFP_CFG_EXP_BAR 7
> > >
> > > #define NFP_CFG_EXP_BAR_CFG_BASE 0x3
> > > +#define NFP_LOCKFILE_PATH_FMT "%s/nfp%d"
> > > +
> > > +/* get nfp lock file path (/var/lock if root, $HOME otherwise) */
> > > +static void
> > > +nspu_get_lockfile_path(char *buffer, int bufsz, nfpu_desc_t *desc)
> > > +{
> > > + const char *dir = "/var/lock";
> > > + const char *home_dir = getenv("HOME");
> > > +
> > > + if (getuid() != 0 && home_dir != NULL)
> > > + dir = home_dir;
> > > +
> > > + /* use current prefix as file path */
> > > + snprintf(buffer, bufsz, NFP_LOCKFILE_PATH_FMT, dir,
> > > + desc->nfp);
> > > +}
> > >
> > > /* There could be other NFP userspace tools using the NSP interface.
> > >* Make sure there is no other process using it and locking the
> access for
> > > @@ -30,9 +46,7 @@ nspv_aquire_process_lock(nfpu_desc_t *desc)
> > > struct flock lock;
> > > char lockname[30];
> > >
> > > - memset(&lock, 0, sizeof(lock));
> > > -
> > > - snprintf(lockname, sizeof(lockname), "/var/lock/nfp%d",
> desc->nfp);
> > > + nspu_get_lockfile_path(lockname, sizeof(lockname), desc);
> > >
> > > /* Using S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> S_IWOTH */
> > > desc->lock = open(lockname, O_RDWR | O_CREAT, 0666);
> > > @@ -106,7 +120,6 @@ nfpu_close(nfpu_desc_t *desc)
> > > rte_free(desc->nspu);
> > > close(desc->lock);
> > >
>
Re: [dpdk-dev] [PATCH] net/tap: remove queue specific offload support
On 4/18/2018 10:40 AM, Ophir Munk wrote:
> Hi Ferruh,
> Sorry for the delayed response.
>
> I would like to verify the correctness of this patch by running several
> internal tests.
> Is a reply by Monday OK with you?
Monday can be late to include patch into rc1, any chance to do earlier?
>
> Regards,
> Ophir
>
>> -Original Message-
>> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
>> Sent: Wednesday, April 18, 2018 11:59 AM
>> To: Thomas Monjalon ; Pascal Mazon
>> ; Mordechay Haimovsky
>> ; Ophir Munk
>> Cc: dev@dpdk.org; Shahaf Shuler ; Olga Shern
>>
>> Subject: Re: [dpdk-dev] [PATCH] net/tap: remove queue specific offload
>> support
>>
>> On 4/12/2018 5:23 PM, Ferruh Yigit wrote:
>>> On 4/5/2018 6:49 PM, Thomas Monjalon wrote:
Pascal, Moti, Ophir,
please comment.
>>>
>>> Hi Moti,
>>>
>>> Any comment? This has been asked many times now.
>>
>> Hi Moti, Ophir,
>>
>> You have not responded why queue specific offload added in other thread.
>> And you are not responding to this patch...
>>
>> Hi Pascal,
>>
>> If you also have no objection, this patch is going in.
>>
>> Thanks,
>> ferruh
>>
>>
>>>
22/03/2018 19:28, Ferruh Yigit:
> It is not clear if tap PMD supports queue specific offloads,
> removing the related code.
>
> Fixes: 95ae196ae10b ("net/tap: use new Rx offloads API")
> Fixes: 818fe14a9891 ("net/tap: use new Tx offloads API")
> Cc: mo...@mellanox.com
>
> Signed-off-by: Ferruh Yigit
>>>
>
[dpdk-dev] [PATCH v3 1/2] ethdev: add supported hash function check
Add supported RSS hash function check in device configuration to
have better error verbosity for application developers.
Signed-off-by: Xueming Li
---
lib/librte_ether/rte_ethdev.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 3c049ef43..4ac0fadae 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1179,6 +1179,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t
nb_rx_q, uint16_t nb_tx_q,
ETHER_MAX_LEN;
}
+ /* Check that device supports requested rss hash functions. */
+ if ((dev_info.flow_type_rss_offloads |
+dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
+ dev_info.flow_type_rss_offloads) {
+ RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: "
+ "0x%"PRIx64", valid value: 0x%"PRIx64"\n",
+ port_id,
+ dev_conf->rx_adv_conf.rss_conf.rss_hf,
+ dev_info.flow_type_rss_offloads);
+ return -EINVAL;
+ }
+
/*
* Setup new number of RX/TX queues and reconfigure device.
*/
@@ -2835,9 +2847,20 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
struct rte_eth_rss_conf *rss_conf)
{
struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info = {0};
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
+ rte_eth_dev_info_get(port_id, &dev_info);
+ if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) !=
+ dev_info.flow_type_rss_offloads) {
+ RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: "
+ "0x%"PRIx64", valid value: 0x%"PRIx64"\n",
+ port_id,
+ rss_conf->rss_hf,
+ dev_info.flow_type_rss_offloads);
+ return -EINVAL;
+ }
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP);
return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
rss_conf));
--
2.13.3
[dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
"port config all rss all" command will fail on PMD that not support any
of hard coding RSS hash types. This patch changed hard coding hash types
to supported types retrieved from device info.
Signed-off-by: Xueming Li
---
app/test-pmd/cmdline.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 512e3b55e..d357de7e6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result,
{
struct cmd_config_rss *res = parsed_result;
struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
+ struct rte_eth_dev_info dev_info = {0};
int diag;
uint8_t i;
@@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
}
rss_conf.rss_key = NULL;
for (i = 0; i < rte_eth_dev_count(); i++) {
+ if (!strcmp(res->value, "all")) {
+ rte_eth_dev_info_get(i, &dev_info);
+ if (dev_info.flow_type_rss_offloads)
+ rss_conf.rss_hf =
+ dev_info.flow_type_rss_offloads;
+ }
diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
if (diag < 0)
printf("Configuration of RSS hash at ethernet port %d "
--
2.13.3
[dpdk-dev] [PATCH 0/2] implement set_mc_addr devop
Support eth_dev_ops.set_mc_addr_list(). Nelio Laranjeiro (2): net/mlx5: split MAC address add/remove code net/mlx5: implement multicast add list devop drivers/net/mlx5/mlx5.c| 2 + drivers/net/mlx5/mlx5.h| 2 + drivers/net/mlx5/mlx5_defs.h | 7 +- drivers/net/mlx5/mlx5_ethdev.c | 2 +- drivers/net/mlx5/mlx5_mac.c| 144 + 5 files changed, 139 insertions(+), 18 deletions(-) -- 2.17.0
[dpdk-dev] [PATCH 2/2] net/mlx5: implement multicast add list devop
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5.c | 2 ++
drivers/net/mlx5/mlx5.h | 2 ++
drivers/net/mlx5/mlx5_defs.h | 5 +++-
drivers/net/mlx5/mlx5_mac.c | 52
4 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 68783c3ac..887924d07 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -277,6 +277,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
.mac_addr_remove = mlx5_mac_addr_remove,
.mac_addr_add = mlx5_mac_addr_add,
.mac_addr_set = mlx5_mac_addr_set,
+ .set_mc_addr_list = mlx5_set_mc_addr_list,
.mtu_set = mlx5_dev_set_mtu,
.vlan_strip_queue_set = mlx5_vlan_strip_queue_set,
.vlan_offload_set = mlx5_vlan_offload_set,
@@ -329,6 +330,7 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
.mac_addr_remove = mlx5_mac_addr_remove,
.mac_addr_add = mlx5_mac_addr_add,
.mac_addr_set = mlx5_mac_addr_set,
+ .set_mc_addr_list = mlx5_set_mc_addr_list,
.mtu_set = mlx5_dev_set_mtu,
.vlan_strip_queue_set = mlx5_vlan_strip_queue_set,
.vlan_offload_set = mlx5_vlan_offload_set,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 6ad41390a..64f025d22 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -203,6 +203,8 @@ void mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t
index);
int mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
uint32_t index, uint32_t vmdq);
int mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr);
+int mlx5_set_mc_addr_list(struct rte_eth_dev *dev,
+ struct ether_addr *mc_addr_set, uint32_t nb_mc_addr);
/* mlx5_rss.c */
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index e3aa03bef..5973742a7 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -15,8 +15,11 @@
/* Maximum number of simultaneous unicast MAC addresses. */
#define MLX5_MAX_UC_MAC_ADDRESSES 128
+/* Maximum number of simultaneous Multicast MAC addresses. */
+#define MLX5_MAX_MC_MAC_ADDRESSES 128
/* Maximum number of simultaneous MAC addresses. */
-#define MLX5_MAX_MAC_ADDRESSES MLX5_MAX_UC_MAC_ADDRESSES
+#define MLX5_MAX_MAC_ADDRESSES \
+ (MLX5_MAX_UC_MAC_ADDRESSES + MLX5_MAX_MC_MAC_ADDRESSES)
/* Maximum number of simultaneous VLAN filters. */
#define MLX5_MAX_VLAN_IDS 128
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index 9470520b1..deeed9598 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -209,3 +209,55 @@ mlx5_mac_addr_set(struct rte_eth_dev *dev, struct
ether_addr *mac_addr)
dev->data->port_id);
return mlx5_mac_addr_add(dev, mac_addr, 0, 0);
}
+
+/**
+ * DPDK callback to set Multicast Addresses list.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @param mac_addr_set
+ * Multicast MAC address pointer array.
+ * @param nb_mac_addr
+ * Number of entries in the array.
+ *
+ * @return
+ * 0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+mlx5_set_mc_addr_list(struct rte_eth_dev *dev,
+ struct ether_addr *mc_addr_set, uint32_t nb_mc_addr)
+{
+ uint32_t i;
+ int ret;
+ int idx;
+
+ for (i = MLX5_MAX_UC_MAC_ADDRESSES;
+i != MLX5_MAX_UC_MAC_ADDRESSES + MLX5_MAX_MC_MAC_ADDRESSES;
+++i)
+ if (!is_zero_ether_addr(&dev->data->mac_addrs[i])) {
+ ret = mlx5_internal_mac_addr_remove(dev, i);
+ if (ret < 0)
+ return ret;
+ }
+ for (idx = MLX5_MAX_UC_MAC_ADDRESSES, i = 0;
+i != nb_mc_addr;
+++i) {
+ if (!is_multicast_ether_addr(mc_addr_set)) {
+ rte_errno = EINVAL;
+ goto rollback;
+ }
+ ret = mlx5_internal_mac_addr_add(dev, mc_addr_set++, idx++);
+ if (ret < 0)
+ return ret;
+ }
+ if (!dev->data->promiscuous)
+ return mlx5_traffic_restart(dev);
+ return 0;
+rollback:
+ ret = rte_errno; /* Save rte_errno before cleanup. */
+ for (i = MLX5_MAX_UC_MAC_ADDRESSES; i != MLX5_MAX_MC_MAC_ADDRESSES; ++i)
+ if (!is_zero_ether_addr(&dev->data->mac_addrs[i]))
+ mlx5_internal_mac_addr_remove(dev, i);
+ rte_errno = ret; /* Restore rte_errno. */
+ return -rte_errno;
+}
--
2.17.0
[dpdk-dev] [PATCH 1/2] net/mlx5: split MAC address add/remove code
Move some code in DPDK callbacks to add/remove MAC addresses to internal
function. This modification will be necessary to handle implement the
devop set_mc_addr_list.
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5_defs.h | 4 +-
drivers/net/mlx5/mlx5_ethdev.c | 2 +-
drivers/net/mlx5/mlx5_mac.c| 92 --
3 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index 3fb2f1480..e3aa03bef 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -13,8 +13,10 @@
/* Reported driver name. */
#define MLX5_DRIVER_NAME "net_mlx5"
+/* Maximum number of simultaneous unicast MAC addresses. */
+#define MLX5_MAX_UC_MAC_ADDRESSES 128
/* Maximum number of simultaneous MAC addresses. */
-#define MLX5_MAX_MAC_ADDRESSES 128
+#define MLX5_MAX_MAC_ADDRESSES MLX5_MAX_UC_MAC_ADDRESSES
/* Maximum number of simultaneous VLAN filters. */
#define MLX5_MAX_VLAN_IDS 128
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index ef44cc91f..99ac4cc5c 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -446,7 +446,7 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *info)
max = 65535;
info->max_rx_queues = max;
info->max_tx_queues = max;
- info->max_mac_addrs = RTE_DIM(priv->mac);
+ info->max_mac_addrs = MLX5_MAX_UC_MAC_ADDRESSES;
info->rx_queue_offload_capa = mlx5_get_rx_queue_offloads(dev);
info->rx_offload_capa = (mlx5_get_rx_port_offloads() |
info->rx_queue_offload_capa);
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index edc7a32ae..9470520b1 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -57,35 +57,32 @@ mlx5_get_mac(struct rte_eth_dev *dev, uint8_t
(*mac)[ETHER_ADDR_LEN])
}
/**
- * DPDK callback to remove a MAC address.
+ * Remove a MAC address from the internal array.
*
* @param dev
* Pointer to Ethernet device structure.
* @param index
* MAC address index.
+ *
+ * @return
+ * 0 on success, a negative errno value otherwise and rte_errno is set.
*/
-void
-mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
+static int
+mlx5_internal_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
{
struct priv *priv = dev->data->dev_private;
const int vf = priv->config.vf;
- int ret;
assert(index < MLX5_MAX_MAC_ADDRESSES);
if (vf)
mlx5_nl_mac_addr_remove(dev, &dev->data->mac_addrs[index],
index);
memset(&dev->data->mac_addrs[index], 0, sizeof(struct ether_addr));
- if (!dev->data->promiscuous) {
- ret = mlx5_traffic_restart(dev);
- if (ret)
- DRV_LOG(ERR, "port %u cannot restart traffic: %s",
- dev->data->port_id, strerror(rte_errno));
- }
+ return 0;
}
/**
- * DPDK callback to add a MAC address.
+ * Adds a MAC address to the internal array.
*
* @param dev
* Pointer to Ethernet device structure.
@@ -93,15 +90,13 @@ mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t
index)
* MAC address to register.
* @param index
* MAC address index.
- * @param vmdq
- * VMDq pool index to associate address with (ignored).
*
* @return
* 0 on success, a negative errno value otherwise and rte_errno is set.
*/
-int
-mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
- uint32_t index, uint32_t vmdq __rte_unused)
+static int
+mlx5_internal_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
+ uint32_t index)
{
struct priv *priv = dev->data->dev_private;
const int vf = priv->config.vf;
@@ -126,6 +121,71 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct
ether_addr *mac,
return ret;
}
dev->data->mac_addrs[index] = *mac;
+ return 0;
+}
+
+/**
+ * DPDK callback to remove a MAC address.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @param index
+ * MAC address index.
+ */
+void
+mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
+{
+ int ret;
+
+ if (index >= MLX5_MAX_UC_MAC_ADDRESSES) {
+ rte_errno = ENOMEM;
+ DRV_LOG(ERR,
+ "port %u not enough room to add the MAC address: %s",
+ dev->data->port_id, strerror(rte_errno));
+ return;
+ }
+ ret = mlx5_internal_mac_addr_remove(dev, index);
+ if (ret < 0) {
+ DRV_LOG(ERR,
+ "port %u failed to remove the MAC address: %s",
+ dev->data->port_id, strerror(rte_errno));
+ }
+ if (!dev->data->promiscuous) {
+ ret = mlx5_traff
Re: [dpdk-dev] [PATCH] drivers/net: update link status
On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> > Hi Ferruh,
> >
> >
> > On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
> >> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
> >>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> > On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
> >> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> >>> On Tue, Mar 13, 2018 at 06:05:34PM +, Ferruh Yigit wrote:
> Update link status related feature document items and minor updates
> in
> some link status related functions.
>
> Signed-off-by: Ferruh Yigit
> ---
> doc/guides/nics/features/fm10k.ini | 2 ++
> doc/guides/nics/features/fm10k_vf.ini | 2 ++
> doc/guides/nics/features/i40e_vf.ini| 1 +
> doc/guides/nics/features/igb_vf.ini | 1 +
> doc/guides/nics/features/qede.ini | 1 -
> doc/guides/nics/features/qede_vf.ini| 1 -
> doc/guides/nics/features/vhost.ini | 2 --
> doc/guides/nics/features/virtio_vec.ini | 1 +
> drivers/net/e1000/em_ethdev.c | 2 +-
> drivers/net/ena/ena_ethdev.c| 2 +-
> drivers/net/fm10k/fm10k_ethdev.c| 6 ++
> drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> drivers/net/ixgbe/ixgbe_ethdev.c| 2 +-
> drivers/net/mlx4/mlx4_ethdev.c | 2 +-
> drivers/net/mlx5/mlx5_ethdev.c | 2 +-
> 15 files changed, 15 insertions(+), 14 deletions(-)
> >>> [...]
> diff --git a/doc/guides/nics/features/vhost.ini
> b/doc/guides/nics/features/vhost.ini
> index dffd1f493..31302745a 100644
> --- a/doc/guides/nics/features/vhost.ini
> +++ b/doc/guides/nics/features/vhost.ini
> @@ -4,8 +4,6 @@
> ; Refer to default.ini for the full list of available PMD features.
> ;
> [Features]
> -Link status = Y
> -Link status event= Y
> >>> I think vhost PMD supports above features.
> >> I am not able to find where it is supported.
> >>
> >> Some virtual PMDs report fixed link, with empty link_update() dev_ops,
> >> and they
> >> are not reported as supporting Link status, as far as I can see vhost
> >> also one
> >> of them.
> >>
> >> And for Link status event, PMD needs to support LSC interrupts and
> >> should
> >> register interrupt handler for it, which I can't find for vhost.
> >>
> >> I will send next version without updating above one, please point me
> >> where these
> >> support added if I missed them.
> > In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
> >
> > static int
> > new_device(int vid)
> > {
> > ..
> >
> > eth_dev->data->dev_link.link_status = ETH_LINK_UP;
> >
> > ..
> >
> > _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
> > NULL);
> >
> > ..
> > }
> >
> > static void
> > destroy_device(int vid)
> > {
> > ..
> >
> > eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
> >
> > ..
> >
> > _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
> > NULL);
> >
> > ..
> > }
> >
> > They are the callbacks for vhost library.
> >
> > When a frontend (e.g. QEMU) is connected to this vhost backend
> > and the frontend virtio device becomes ready, new_device() will
> > be called by the vhost library, and the link status will be
> > updated to UP.
> >
> > And when e.g. the connection is closed, destroy_device() will be
> > called by the vhost library, and the link status will be updated
> > to DOWN.
>
> Got it. This behavior is similar for virtual PMDs. Provide static link
> information and update link as UP during start and update it as DOWN
> during stop.
> >>> No, the link status isn't updated during vhost PMD start
> >>> and stop. When the vhost PMD has been started, the link
> >>> status still may be DOWN. The link status becomes UP only
> >>> when the QEMU (it's another virtual machine process which
> >>> has a virtio device) connects to this vhost PMD via a UNIX
> >>> socket and the virtio driver in the virtual machine has
> >>> setup the virtio device of the virtual machine.
> >>>
> >>> So if vhost PMD reports the link status as DOWN, it means
> >>> there is no QEMU (virtual machine) connects to it or the
> >>> virtio device in the virtual machine hasn't been setup.
> >>> (PS. The frontend can also be virtio-user PMD besides QEMU)
> >> I believe announcing link feature rep
Re: [dpdk-dev] [PATCH] drivers/net: update link status
On 4/18/2018 12:36 PM, Tiwei Bie wrote:
> On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
>> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
>>> Hi Ferruh,
>>>
>>>
>>> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
On 4/17/2018 5:54 AM, Tiwei Bie wrote:
> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> On Tue, Mar 13, 2018 at 06:05:34PM +, Ferruh Yigit wrote:
>> Update link status related feature document items and minor updates
>> in
>> some link status related functions.
>>
>> Signed-off-by: Ferruh Yigit
>> ---
>> doc/guides/nics/features/fm10k.ini | 2 ++
>> doc/guides/nics/features/fm10k_vf.ini | 2 ++
>> doc/guides/nics/features/i40e_vf.ini| 1 +
>> doc/guides/nics/features/igb_vf.ini | 1 +
>> doc/guides/nics/features/qede.ini | 1 -
>> doc/guides/nics/features/qede_vf.ini| 1 -
>> doc/guides/nics/features/vhost.ini | 2 --
>> doc/guides/nics/features/virtio_vec.ini | 1 +
>> drivers/net/e1000/em_ethdev.c | 2 +-
>> drivers/net/ena/ena_ethdev.c| 2 +-
>> drivers/net/fm10k/fm10k_ethdev.c| 6 ++
>> drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
>> drivers/net/ixgbe/ixgbe_ethdev.c| 2 +-
>> drivers/net/mlx4/mlx4_ethdev.c | 2 +-
>> drivers/net/mlx5/mlx5_ethdev.c | 2 +-
>> 15 files changed, 15 insertions(+), 14 deletions(-)
> [...]
>> diff --git a/doc/guides/nics/features/vhost.ini
>> b/doc/guides/nics/features/vhost.ini
>> index dffd1f493..31302745a 100644
>> --- a/doc/guides/nics/features/vhost.ini
>> +++ b/doc/guides/nics/features/vhost.ini
>> @@ -4,8 +4,6 @@
>> ; Refer to default.ini for the full list of available PMD features.
>> ;
>> [Features]
>> -Link status = Y
>> -Link status event= Y
> I think vhost PMD supports above features.
I am not able to find where it is supported.
Some virtual PMDs report fixed link, with empty link_update() dev_ops,
and they
are not reported as supporting Link status, as far as I can see vhost
also one
of them.
And for Link status event, PMD needs to support LSC interrupts and
should
register interrupt handler for it, which I can't find for vhost.
I will send next version without updating above one, please point me
where these
support added if I missed them.
>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>
>>> static int
>>> new_device(int vid)
>>> {
>>> ..
>>>
>>> eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>
>>> ..
>>>
>>> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
>>> NULL);
>>>
>>> ..
>>> }
>>>
>>> static void
>>> destroy_device(int vid)
>>> {
>>> ..
>>>
>>> eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>
>>> ..
>>>
>>> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
>>> NULL);
>>>
>>> ..
>>> }
>>>
>>> They are the callbacks for vhost library.
>>>
>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>> and the frontend virtio device becomes ready, new_device() will
>>> be called by the vhost library, and the link status will be
>>> updated to UP.
>>>
>>> And when e.g. the connection is closed, destroy_device() will be
>>> called by the vhost library, and the link status will be updated
>>> to DOWN.
>>
>> Got it. This behavior is similar for virtual PMDs. Provide static link
>> information and update link as UP during start and update it as DOWN
>> during stop.
> No, the link status isn't updated during vhost PMD start
> and stop. When the vhost PMD has been started, the link
> status still may be DOWN. The link status becomes UP only
> when the QEMU (it's another virtual machine process which
> has a virtio device) connects to this vhost PMD via a UNIX
> socket and the virtio driver in the virtual machine has
> setup the virtio device of the virtual machine.
>
> So if vhost PMD reports the link status as DOWN, it means
> there is no QEMU (virtual machine) connects to it or the
> virtio device in the virtual machine hasn't been setup.
> (PS. The frontend can also be virtio-user PMD besides QEMU)
>
Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add supported hash function check
Hi Thomas,
> -Original Message-
> From: Thomas Monjalon
> Sent: Wednesday, April 18, 2018 6:01 AM
> To: Xueming(Steven) Li
> Cc: dev@dpdk.org; Shahaf Shuler ; Nelio Laranjeiro
> ;
> Wenzhuo Lu ; Jingjing Wu
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add supported hash function
> check
>
> 17/04/2018 16:24, Xueming Li:
> > + /* Check that device supports requested rss hash functions. */
> > + if ((dev_info.flow_type_rss_offloads |
> > +dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
> > + dev_info.flow_type_rss_offloads) {
> > + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx,
> > valid value: 0x%lx\n",
> > + port_id,
> > + dev_conf->rx_adv_conf.rss_conf.rss_hf,
> > + dev_info.flow_type_rss_offloads);
> > + return -EINVAL;
> > + }
>
> Please use PRIx64 and test 32-bit compilation.
>
> Reminder from this recent post:
>
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpdk.org%2Fml%2Farchives%2Fdev%2F201
> 8-
> February%2F090882.html&data=02%7C01%7Cxuemingl%40mellanox.com%7C5508bc716aac41b932fb08d5a4aeb241%7Ca65
> 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636595992591300475&sdata=1hirZ1j7VqCMnrzngZFAuQj42OACfxFUgYgzy
> VQyw%2F4%3D&reserved=0
> "
> Most of the times, using %l is wrong (except when printing a long).
> So next time you write %l, please think "I am probably wrong".
> "
Thanks, got following warning from checkpatch when applying this rule to my
other patchset, is it normal?
CHECK:CAMELCASE: Avoid CamelCase:
#49: FILE: drivers/net/mlx5/mlx5_flow.c:2083:
+ " hash:%" PRIx64 "/%u specs:%hhu(%hu), priority:%hu, type:%d,"
Re: [dpdk-dev] [PATCH v5 0/4] introduce new tunnel types
> -Original Message- > From: Iremonger, Bernard > Sent: Wednesday, April 18, 2018 12:05 AM > To: Xueming(Steven) Li ; Lu, Wenzhuo > ; Wu, Jingjing > ; Thomas Monjalon ; Adrien > Mazarguil > > Cc: Nélio Laranjeiro ; Shahaf Shuler > ; dev@dpdk.org; > Olivier Matz > Subject: RE: [dpdk-dev] [PATCH v5 0/4] introduce new tunnel types > > Hi Li, > > > -Original Message- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Xueming Li > > Sent: Tuesday, April 17, 2018 4:04 PM > > To: Lu, Wenzhuo ; Wu, Jingjing > > ; Thomas Monjalon ; Adrien > > Mazarguil > > Cc: Xueming Li ; Nelio Laranjeiro > > ; Shahaf Shuler ; > > dev@dpdk.org; Olivier Matz > > Subject: [dpdk-dev] [PATCH v5 0/4] introduce new tunnel types > > > > v5: > > - Fixed VXLAN-GPE comment alignment > > v4: > > - Update testpmd doc for flow VXLAN-GPE paramter. > > v3: > > - Change VXLAN-GPE definition order to avoid ABI compatibility issue. > > v2: > > - Split patch set into public and mlx5 two series, this one is the first. > > v1: > > - Support new tunnel type MPLS-in-GRE and MPLS-in-UDP > > - Remove deprecation notes of rss level > > > > This patchset introduced new tunnel type and related testpmd code: > > - New tunnel type VXLAN-GPE > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdat > > atracker.ietf.org%2Fdoc%2Fdraft-ietf-nvo3-vxlan-gpe%2F&data=02%7C01%7C > > xuemingl%40mellanox.com%7C2dffef40890b4cf8ff9d08d5a47d0420%7Ca652971c7 > > d2e4d9ba6a4d149256f461b%7C0%7C0%7C636595779231620631&sdata=%2Bvx%2FgVB > > 3e3BHI%2BYxPxOIpqK6CuKvQQ8qej4B1Faxihc%3D&reserved=0 > > - New tunnel type MPLS-in-GRE > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftoo > > ls.ietf.org%2Fhtml%2Frfc4023&data=02%7C01%7Cxuemingl%40mellanox.com%7C > > 2dffef40890b4cf8ff9d08d5a47d0420%7Ca652971c7d2e4d9ba6a4d149256f461b%7C > > 0%7C0%7C636595779231620631&sdata=wk2wvoB9LSbI2LfHZVvWzmtgS0XQbGNMFL4G1 > > kyr77E%3D&reserved=0 > > - New tunnel type MPLS-in-UDP > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftoo > > ls.ietf.org%2Fhtml%2Frfc7510&data=02%7C01%7Cxuemingl%40mellanox.com%7C > > 2dffef40890b4cf8ff9d08d5a47d0420%7Ca652971c7d2e4d9ba6a4d149256f461b%7C > > 0%7C0%7C636595779231620631&sdata=4RXbPD9tV7ArcnKsK8IJy%2B9XbKlzKcNBSv1 > > LXVwwuPs%3D&reserved=0 > > - Support GRE extension in testpmd csum forwarding engine > > > > Xueming Li (4): > > doc: remove RSS configuration change announcement > > ethdev: introduce new tunnel VXLAN-GPE > > app/testpmd: introduce new tunnel VXLAN-GPE > > app/testpmd: add more GRE extension support to csum engine > > > > app/test-pmd/cmdline_flow.c | 24 +++ > > app/test-pmd/config.c | 2 + > > app/test-pmd/csumonly.c | 103 > > +--- > > app/test-pmd/parameters.c | 12 +++- > > app/test-pmd/testpmd.h | 2 + > > doc/guides/prog_guide/rte_flow.rst | 12 > > doc/guides/rel_notes/deprecation.rst| 4 -- > > doc/guides/testpmd_app_ug/run_app.rst | 5 ++ > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 ++ > > lib/librte_ether/rte_eth_ctrl.h | 3 +- > > lib/librte_ether/rte_flow.c | 1 + > > lib/librte_ether/rte_flow.h | 27 > > lib/librte_mbuf/rte_mbuf.c | 3 + > > lib/librte_mbuf/rte_mbuf.h | 1 + > > lib/librte_mbuf/rte_mbuf_ptype.c| 1 + > > lib/librte_mbuf/rte_mbuf_ptype.h| 13 > > lib/librte_net/rte_ether.h | 25 +++ > > 17 files changed, 225 insertions(+), 17 deletions(-) > > > > -- > > 2.13.3 > > Patch 3 of this patch set fails to apply to the latest master, the other > three patches apply ok. > > Regards, > > Bernard. > I tried it with orgin/master branch and it worked for me. Could you please share more information? Best Regards, Xueming Li
Re: [dpdk-dev] [PATCH v4] drivers/net/i40e: fix missing promiscuous disable at device disable
> -Original Message- > From: Xu, Rosen > Sent: Wednesday, April 18, 2018 4:12 PM > To: dev@dpdk.org > Cc: Xing, Beilei ; Zhang, Helin > ; Zhang, Qi Z ; Wu, Jingjing > > Subject: [PATCH v4] drivers/net/i40e: fix missing promiscuous disable at > device disable > > v4 updates: > === > - Add some comments for this patch > > v3 updates: > === > - Move modification from device close to device disable > - i40evf_reset_vf() will cause kernel driver enable all vlan promiscuous, >so unicast/multicast promiscuous disable should set before reset. > > v2 updates: > === > - Add more comments > > In scenario of Kernel Driver runs on PF and PMD runs on VF, PMD exit > doesn't disable promiscuous mode, this will cause vlan filter set by Kernel > Driver will not take effect. > > This patch will fix it, add promiscuous disable at device disable. > > Signed-off-by: Rosen Xu Acked-by: Qi Zhang Change log need be removed when apply
Re: [dpdk-dev] [PATCH] drivers/net: update link status
On Wed, Apr 18, 2018 at 12:44:54PM +0100, Ferruh Yigit wrote:
> On 4/18/2018 12:36 PM, Tiwei Bie wrote:
> > On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
> >> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> >>> Hi Ferruh,
> >>>
> >>>
> >>> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
> > On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
> >> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> >>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> > On Tue, Mar 13, 2018 at 06:05:34PM +, Ferruh Yigit wrote:
> >> Update link status related feature document items and minor
> >> updates in
> >> some link status related functions.
> >>
> >> Signed-off-by: Ferruh Yigit
> >> ---
> >> doc/guides/nics/features/fm10k.ini | 2 ++
> >> doc/guides/nics/features/fm10k_vf.ini | 2 ++
> >> doc/guides/nics/features/i40e_vf.ini| 1 +
> >> doc/guides/nics/features/igb_vf.ini | 1 +
> >> doc/guides/nics/features/qede.ini | 1 -
> >> doc/guides/nics/features/qede_vf.ini| 1 -
> >> doc/guides/nics/features/vhost.ini | 2 --
> >> doc/guides/nics/features/virtio_vec.ini | 1 +
> >> drivers/net/e1000/em_ethdev.c | 2 +-
> >> drivers/net/ena/ena_ethdev.c| 2 +-
> >> drivers/net/fm10k/fm10k_ethdev.c| 6 ++
> >> drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> >> drivers/net/ixgbe/ixgbe_ethdev.c| 2 +-
> >> drivers/net/mlx4/mlx4_ethdev.c | 2 +-
> >> drivers/net/mlx5/mlx5_ethdev.c | 2 +-
> >> 15 files changed, 15 insertions(+), 14 deletions(-)
> > [...]
> >> diff --git a/doc/guides/nics/features/vhost.ini
> >> b/doc/guides/nics/features/vhost.ini
> >> index dffd1f493..31302745a 100644
> >> --- a/doc/guides/nics/features/vhost.ini
> >> +++ b/doc/guides/nics/features/vhost.ini
> >> @@ -4,8 +4,6 @@
> >> ; Refer to default.ini for the full list of available PMD
> >> features.
> >> ;
> >> [Features]
> >> -Link status = Y
> >> -Link status event= Y
> > I think vhost PMD supports above features.
> I am not able to find where it is supported.
>
> Some virtual PMDs report fixed link, with empty link_update()
> dev_ops, and they
> are not reported as supporting Link status, as far as I can see
> vhost also one
> of them.
>
> And for Link status event, PMD needs to support LSC interrupts and
> should
> register interrupt handler for it, which I can't find for vhost.
>
> I will send next version without updating above one, please point me
> where these
> support added if I missed them.
> >>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
> >>>
> >>> static int
> >>> new_device(int vid)
> >>> {
> >>> ..
> >>>
> >>> eth_dev->data->dev_link.link_status = ETH_LINK_UP;
> >>>
> >>> ..
> >>>
> >>> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
> >>> NULL);
> >>>
> >>> ..
> >>> }
> >>>
> >>> static void
> >>> destroy_device(int vid)
> >>> {
> >>> ..
> >>>
> >>> eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
> >>>
> >>> ..
> >>>
> >>> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
> >>> NULL);
> >>>
> >>> ..
> >>> }
> >>>
> >>> They are the callbacks for vhost library.
> >>>
> >>> When a frontend (e.g. QEMU) is connected to this vhost backend
> >>> and the frontend virtio device becomes ready, new_device() will
> >>> be called by the vhost library, and the link status will be
> >>> updated to UP.
> >>>
> >>> And when e.g. the connection is closed, destroy_device() will be
> >>> called by the vhost library, and the link status will be updated
> >>> to DOWN.
> >>
> >> Got it. This behavior is similar for virtual PMDs. Provide static link
> >> information and update link as UP during start and update it as DOWN
> >> during stop.
> > No, the link status isn't updated during vhost PMD start
> > and stop. When the vhost PMD has been started, the link
> > status still may be DOWN. The link status becomes UP only
> > when the QEMU (it's another virtual machine process which
> > has a virtio device) connects to this vhost PMD via a UNIX
> > socket and the virtio driver in the virtual machine has
> > setup the virtio devic
Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add supported hash function check
18/04/2018 13:55, Xueming(Steven) Li:
> From: Thomas Monjalon
> > 17/04/2018 16:24, Xueming Li:
> > > + /* Check that device supports requested rss hash functions. */
> > > + if ((dev_info.flow_type_rss_offloads |
> > > + dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
> > > + dev_info.flow_type_rss_offloads) {
> > > + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx,
> > > valid value: 0x%lx\n",
> > > + port_id,
> > > + dev_conf->rx_adv_conf.rss_conf.rss_hf,
> > > + dev_info.flow_type_rss_offloads);
> > > + return -EINVAL;
> > > + }
> >
> > Please use PRIx64 and test 32-bit compilation.
> >
> > Reminder from this recent post:
> >
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpdk.org%2Fml%2Farchives%2Fdev%2F201
> > 8-
> > February%2F090882.html&data=02%7C01%7Cxuemingl%40mellanox.com%7C5508bc716aac41b932fb08d5a4aeb241%7Ca65
> > 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636595992591300475&sdata=1hirZ1j7VqCMnrzngZFAuQj42OACfxFUgYgzy
> > VQyw%2F4%3D&reserved=0
> > "
> > Most of the times, using %l is wrong (except when printing a long).
> > So next time you write %l, please think "I am probably wrong".
> > "
>
> Thanks, got following warning from checkpatch when applying this rule to my
> other patchset, is it normal?
> CHECK:CAMELCASE: Avoid CamelCase:
> #49: FILE: drivers/net/mlx5/mlx5_flow.c:2083:
> + " hash:%" PRIx64 "/%u specs:%hhu(%hu), priority:%hu,
> type:%d,"
>
Yes it is "normal".
Something we should fix in checkpatch.
Re: [dpdk-dev] [PATCH v2] net/i40e: fix I40e start failing for missing JUMBO_FRAME offload
> -Original Message-
> From: Wu, Yanglong
> Sent: Wednesday, April 18, 2018 9:42 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z ; Dai, Wei ; Yao,
> Lei A ; Wu, Yanglong
> Subject: [PATCH v2] net/i40e: fix I40e start failing for missing JUMBO_FRAME
> offload
>
> JUMBO_FRAME offload should be set as default, missing it will lead to start
> failing for I40e.
>
> Fixes: fbf959fe61e7 ("net/i40e: convert to new Rx offloads API")
> Signed-off-by: Yanglong Wu
Acked-by: Qi Zhang
Title and commit log could be changed to
net/i40e: fix missing jumbo frame offload capability
JUMBO_FRAME offload capability should be exposed since i40e
does support it.
Re: [dpdk-dev] [PATCH] drivers/net: update link status
On 4/18/2018 1:08 PM, Tiwei Bie wrote:
> On Wed, Apr 18, 2018 at 12:44:54PM +0100, Ferruh Yigit wrote:
>> On 4/18/2018 12:36 PM, Tiwei Bie wrote:
>>> On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> Hi Ferruh,
>
>
> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
>> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>> On Tue, Mar 13, 2018 at 06:05:34PM +, Ferruh Yigit wrote:
Update link status related feature document items and minor
updates in
some link status related functions.
Signed-off-by: Ferruh Yigit
---
doc/guides/nics/features/fm10k.ini | 2 ++
doc/guides/nics/features/fm10k_vf.ini | 2 ++
doc/guides/nics/features/i40e_vf.ini| 1 +
doc/guides/nics/features/igb_vf.ini | 1 +
doc/guides/nics/features/qede.ini | 1 -
doc/guides/nics/features/qede_vf.ini| 1 -
doc/guides/nics/features/vhost.ini | 2 --
doc/guides/nics/features/virtio_vec.ini | 1 +
drivers/net/e1000/em_ethdev.c | 2 +-
drivers/net/ena/ena_ethdev.c| 2 +-
drivers/net/fm10k/fm10k_ethdev.c| 6 ++
drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
drivers/net/ixgbe/ixgbe_ethdev.c| 2 +-
drivers/net/mlx4/mlx4_ethdev.c | 2 +-
drivers/net/mlx5/mlx5_ethdev.c | 2 +-
15 files changed, 15 insertions(+), 14 deletions(-)
>>> [...]
diff --git a/doc/guides/nics/features/vhost.ini
b/doc/guides/nics/features/vhost.ini
index dffd1f493..31302745a 100644
--- a/doc/guides/nics/features/vhost.ini
+++ b/doc/guides/nics/features/vhost.ini
@@ -4,8 +4,6 @@
; Refer to default.ini for the full list of available PMD
features.
;
[Features]
-Link status = Y
-Link status event= Y
>>> I think vhost PMD supports above features.
>> I am not able to find where it is supported.
>>
>> Some virtual PMDs report fixed link, with empty link_update()
>> dev_ops, and they
>> are not reported as supporting Link status, as far as I can see
>> vhost also one
>> of them.
>>
>> And for Link status event, PMD needs to support LSC interrupts and
>> should
>> register interrupt handler for it, which I can't find for vhost.
>>
>> I will send next version without updating above one, please point me
>> where these
>> support added if I missed them.
> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>
> static int
> new_device(int vid)
> {
> ..
>
> eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>
> ..
>
> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
> NULL);
>
> ..
> }
>
> static void
> destroy_device(int vid)
> {
> ..
>
> eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>
> ..
>
> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC,
> NULL);
>
> ..
> }
>
> They are the callbacks for vhost library.
>
> When a frontend (e.g. QEMU) is connected to this vhost backend
> and the frontend virtio device becomes ready, new_device() will
> be called by the vhost library, and the link status will be
> updated to UP.
>
> And when e.g. the connection is closed, destroy_device() will be
> called by the vhost library, and the link status will be updated
> to DOWN.
Got it. This behavior is similar for virtual PMDs. Provide static link
information and update link as UP during start and update it as DOWN
during stop.
>>> No, the link status isn't updated during vhost PMD start
>>> and stop. When the vhost PMD has been started, the link
>>> status still may be DOWN. The link status becomes UP only
>>> when the QEMU (it's another virtual machine process which
>>> has a virtio device) connects to this vhost PMD via a UNIX
>>> socket and the virtio driver in the virtual
[dpdk-dev] [PATCH] devtools: improve kernel script execution check
Handle properly a case where DPDK_PATCH_PATH is set to point to a directory. Signed-off-by: Juhamatti Kuusisaari --- devtools/checkpatches.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh index 7676a6b..2bb5458 100755 --- a/devtools/checkpatches.sh +++ b/devtools/checkpatches.sh @@ -75,7 +75,7 @@ while getopts hn:qv ARG ; do done shift $(($OPTIND - 1)) -if [ ! -x "$DPDK_CHECKPATCH_PATH" ] ; then +if [ ! -f "$DPDK_CHECKPATCH_PATH" ] || [ ! -x "$DPDK_CHECKPATCH_PATH" ] ; then print_usage >&2 echo echo 'Cannot execute DPDK_CHECKPATCH_PATH' >&2 -- 2.8.1
Re: [dpdk-dev] [PATCH v4 05/16] ethdev: alter behavior of flow API actions
On 04/16/2018 07:22 PM, Adrien Mazarguil wrote:
This patch makes the following changes to flow rule actions:
- List order now matters, they are redefined as performed first to last
instead of "all simultaneously".
- Repeated actions are now supported (e.g. specifying QUEUE multiple times
now duplicates traffic among them). Previously only the last action of
any given kind was taken into account.
- No more distinction between terminating/non-terminating/meta actions.
Flow rules themselves are now defined as always terminating unless a
PASSTHRU action is specified.
These changes alter the behavior of flow rules in corner cases in order to
prepare the flow API for actions that modify traffic contents or properties
(e.g. encapsulation, compression) and for which order matter when combined.
Previously one would have to do so through multiple flow rules by combining
PASSTRHU with priority levels, however this proved overly complex to
implement at the PMD level, hence this simpler approach.
This breaks ABI compatibility for the following public functions:
- rte_flow_create()
- rte_flow_validate()
PMDs with rte_flow support are modified accordingly:
- bnxt: no change, implementation already forbids multiple actions and does
not support PASSTHRU.
- e1000: no change, same as bnxt.
- enic: modified to forbid redundant actions, no support for default drop.
- failsafe: no change needed.
- i40e: no change, implementation already forbids multiple actions.
- ixgbe: same as i40e.
- mlx4: modified to forbid multiple fate-deciding actions and drop when
unspecified.
- mlx5: same as mlx4, with other redundant actions also forbidden.
- sfc: same as mlx4.
- tap: implementation already complies with the new behavior except for
the default pass-through modified as a default drop.
Signed-off-by: Adrien Mazarguil
Reviewed-by: Andrew Rybchenko
Cc: Ajit Khaparde
Cc: Wenzhuo Lu
Cc: John Daley
Cc: Gaetan Rivet
Cc: Beilei Xing
Cc: Konstantin Ananyev
Cc: Nelio Laranjeiro
Cc: Andrew Rybchenko
Cc: Pascal Mazon
---
doc/guides/prog_guide/rte_flow.rst | 67 +---
drivers/net/enic/enic_flow.c | 25
drivers/net/mlx4/mlx4_flow.c | 21 +++---
drivers/net/mlx5/mlx5_flow.c | 69 ++---
drivers/net/sfc/sfc_flow.c | 22 +++
drivers/net/tap/tap_flow.c | 11 ++
lib/librte_ether/rte_flow.h| 54 +++---
7 files changed, 138 insertions(+), 131 deletions(-)
[...]
diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index b9f36587c..a5c6a1670 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -975,6 +979,10 @@ enic_copy_action_v1(const struct rte_flow_action actions[],
const struct rte_flow_action_queue *queue =
(const struct rte_flow_action_queue *)
actions->conf;
+
+ if (overlap & FATE)
+ return ENOTSUP;
+ overlap |= FATE;
enic_action->rq_idx =
enic_rte_rq_idx_to_sop_idx(queue->index);
break;
@@ -984,6 +992,8 @@ enic_copy_action_v1(const struct rte_flow_action actions[],
break;
}
}
+ if (!overlap & FATE)
Build using clang on Ubuntu 17.10 fails:
/var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: fatal error:
logical not is only applied to
the left hand side of this bitwise operator
[-Wlogical-not-parentheses]
if (!overlap & FATE)
^ ~
/var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add
parentheses after the '!' to
evaluate the bitwise operator first
if (!overlap & FATE)
^
( )
/var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add
parentheses around left hand side
expression to silence this warning
if (!overlap & FATE)
^
( )
1 error generated.
/var/tmp/dpdk-next-net/mk/internal/rte.compile-pre.mk:114: recipe for
target 'enic_flow.o' failed
make[4]: *** [enic_flow.o] Error 1
/var/tmp/dpdk-next-net/mk/rte.subdir.mk:35: recipe for target 'enic' failed
make[3]: *** [enic] Error 2
CC nfp_cpp_pcie_ops.o
make[3]: *** Waiting for unfinished jobs
$ clang --version
clang version 4.0.1-6 (tags/RELEASE_401/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
+ return ENOTSUP;
enic_action->type = FILTER_ACTION_RQ_STEERING;
return 0;
}
@@ -1001,6 +1011,9 @@ static int
enic_copy_action_v2(const struct rte_flow_action actions[],
struct filter_action_v2 *enic_action)
{
+ enum { FATE = 1, MARK = 2, };
+ uint32_t overlap = 0;
+
FLOW_TRACE();
Re: [dpdk-dev] [dpdk-web] [PATCH v2] update stable releases roadmap
On 4/18/2018 10:14 AM, Thomas Monjalon wrote: > 18/04/2018 11:05, Ferruh Yigit: >> On 4/11/2018 12:28 AM, Thomas Monjalon wrote: >>> - Typically a new stable release version follows a mainline release >>> - by 1-2 weeks, depending on the test results. >>> + The first stable release (.1) of a branch should follow >>> + its mainline release (.0) by at least two months, >>> + after the first release candidate (-rc1) of the next branch. >> >> Hi Thomas, >> >> What this change suggest? To be able to backport patches from rc1? > > Yes, it is the proposal we discussed earlier. > We can wait one week after RC1 to get some validation confirmation. > Do you agree? This has been discussed in tech-board, what I remember the decision was to wait the release to backport patches into stable tree.
Re: [dpdk-dev] [RFC 2/2] nfp: allow for non-root user
Alejandro Lucero writes:
> On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole wrote:
>
> Alejandro Lucero writes:
>
> > I was just wondering, if device device PCI sysfs resource files or VFIO
> group /dev files
> require to change
> > permissions for non-root users, does it not make sense to adjust also
> /var/lock in the
> system?
>
> For the /dev, we use udev rules - so the correct individual vfio device
> files get assigned the correct permissions. No such mechanism exists
> for /var/lock as far as I can tell.
>
> Ex. see:
>
>
> https://github.com/openvswitch/ovs/blob/master/rhel/usr_lib_udev_rules.d_91-vfio.rules
>
>
> Maybe something similar exists that we could use to generate the lock
> file automatically?
>
> What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
>
> Is RH forcing OVS DPDK to only work if the host has IOMMU support?
Yes.
> > On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero
> wrote:
> >
> > I have seen that VFIO also requires explicitly to set the right
> permissions for non-root
> users to VFIO
> > groups under /dev/vfio.
> >
> > I assume then that running OVS or other DPDK apps as non-root is possible,
> although requiring
> > those explicit permissions changes, and therefore this patch is necessary.
> >
> > Adding stable@ and Thomas for discussing how can this be added to stable
> DPDK
> versions even if
> > this is not going to be a patch for current DPDK version.
> >
> > Acked-by: Alejandro Lucero
> >
> > On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero
> wrote:
> >
> > On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole wrote:
> >
> > Alejandro Lucero writes:
> >
> > > Again, this patch is correct, but because NFP PMD needs to access
> > > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and these
> files
> have
> > just
> > > read/write accesses for root, I do not know if this is really necessary.
> > >
> > > Being honest, I have not used a DPDK app with NFP PMD and not being
> root. Does
> it
> > work
> > > with non-root users and other PMDs with same requirements regarding
> sysfs
> resource
> > files?
> >
> > We do run as non-root user definitely with Intel PMDs.
> >
> > I'm not very sure about other vendors, but I think mlx pmd runs as
> > non-root user (and it was modified to move off of sysfs for that
> > reason[1]).
> >
> > It is possible to not rely on sysfs resource files if device is attached
> to VFIO, but I
> think that is a
> > must with UIO.
> >
> >
> > I'll continue to push for more information from the testing side to find
> > out though.
> >
> > [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
> >
> > > On Fri, Apr 13, 2018 at 12:22 AM, Aaron Conole
> wrote:
> > >
> > > Currently, the nfp lock files are taken from the global lock file
> > > location, which will work when the user is running as root. However,
> > > some distributions and applications (notably ovs 2.8+ on RHEL/Fedora)
> > > run as a non-root user.
> > >
> > > Signed-off-by: Aaron Conole
> > > ---
> > > drivers/net/nfp/nfp_nfpu.c | 23 ++-
> > > 1 file changed, 18 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/nfp/nfp_nfpu.c b/drivers/net/nfp/nfp_nfpu.c
> > > index 2ed985ff4..ae2e07220 100644
> > > --- a/drivers/net/nfp/nfp_nfpu.c
> > > +++ b/drivers/net/nfp/nfp_nfpu.c
> > > @@ -18,6 +18,22 @@
> > > #define NFP_CFG_EXP_BAR 7
> > >
> > > #define NFP_CFG_EXP_BAR_CFG_BASE 0x3
> > > +#define NFP_LOCKFILE_PATH_FMT "%s/nfp%d"
> > > +
> > > +/* get nfp lock file path (/var/lock if root, $HOME otherwise) */
> > > +static void
> > > +nspu_get_lockfile_path(char *buffer, int bufsz, nfpu_desc_t *desc)
> > > +{
> > > + const char *dir = "/var/lock";
> > > + const char *home_dir = getenv("HOME");
> > > +
> > > + if (getuid() != 0 && home_dir != NULL)
> > > + dir = home_dir;
> > > +
> > > + /* use current prefix as file path */
> > > + snprintf(buffer, bufsz, NFP_LOCKFILE_PATH_FMT, dir,
> > > + desc->nfp);
> > > +}
> > >
> > > /* There could be other NFP userspace tools using the NSP interface.
> > >* Make sure there is no other process using it and locking the
> access for
> > > @@ -30,9 +46,7 @@ nspv_aquire_process_lock(nfpu_desc_t *desc)
> > > struct flock lock;
> > > char lockname[30];
> > >
> > > - memset(&lock, 0, sizeof(lock));
> > > -
> > > - snprintf(lockname, sizeof(lockname), "/var/lock/nfp%d",
> desc->nfp);
> > > + nspu_get_lockfile_path(lockname, sizeof(lockname), desc);
> > >
> > > /* Using S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> S_IWOTH */
> > > desc->lock = open(lockname, O_RDWR | O_CREAT, 0666);
> > > @@ -106,7 +120,
Re: [dpdk-dev] [PATCH v11 00/10] crypto: add virtio poll mode driver
> -Original Message- > From: De Lara Guarch, Pablo [mailto:pablo.de.lara.gua...@intel.com] > Sent: Wednesday, April 18, 2018 6:01 PM > To: Zhoujian (jay) ; dev@dpdk.org > Cc: Zhang, Roy Fan ; tho...@monjalon.net; Gonglei > (Arei) ; Zeng, Xin ; > Huangweidong (C) ; wangxin (U) > ; longpeng > Subject: RE: [PATCH v11 00/10] crypto: add virtio poll mode driver > > > > > -Original Message- > > From: Jay Zhou [mailto:jianjay.z...@huawei.com] > > Sent: Tuesday, April 17, 2018 10:23 AM > > To: dev@dpdk.org > > Cc: De Lara Guarch, Pablo ; Zhang, Roy > > Fan ; tho...@monjalon.net; > > arei.gong...@huawei.com; Zeng, Xin ; > > weidong.hu...@huawei.com; wangxinxin.w...@huawei.com; > > longpe...@huawei.com; jianjay.z...@huawei.com > > Subject: [PATCH v11 00/10] crypto: add virtio poll mode driver > > > > This patch series introduce virtio crypto poll mode driver. > > Series applied to dpdk-next-crypto. > > Thanks for the work! > Pablo Thank for your help and review, Pablo! Regards, Jay
Re: [dpdk-dev] [PATCH v3 1/4] bus/vmbus: add hyper-v virtual bus support
On 4/17/2018 10:53 PM, Stephen Hemminger wrote:
> From: Stephen Hemminger
>
> This patch adds support for an additional bus type Virtual Machine BUS
> (VMBUS) on Microsoft Hyper-V in Windows 10, Windows Server 2016
> and Azure. Most of this code was extracted from FreeBSD and some of
> this is from earlier code donated by Brocade.
>
> Only Linux is supported at present, but the code is split
> to allow future FreeBSD and Windows support.
>
> The bus support relies on the uio_hv_generic driver from Linux
> kernel 4.16. Multiple queue support requires additional sysfs
> interfaces which is in kernel 5.0 (a.k.a 4.17).
>
> Signed-off-by: Stephen Hemminger
> ---
> MAINTAINERS | 3 +
> config/common_base | 5 +
> config/common_linuxapp | 4 +
> drivers/bus/Makefile| 1 +
> drivers/bus/vmbus/Makefile | 36 ++
> drivers/bus/vmbus/linux/Makefile| 3 +
> drivers/bus/vmbus/linux/vmbus_bus.c | 355 +
> drivers/bus/vmbus/linux/vmbus_uio.c | 381 ++
> drivers/bus/vmbus/private.h | 132 +++
> drivers/bus/vmbus/rte_bus_vmbus.h | 400 +++
> drivers/bus/vmbus/rte_bus_vmbus_version.map | 28 ++
> drivers/bus/vmbus/rte_vmbus_reg.h | 344 +
> drivers/bus/vmbus/vmbus_bufring.c | 241
> drivers/bus/vmbus/vmbus_channel.c | 405
> drivers/bus/vmbus/vmbus_common.c| 286 ++
> drivers/bus/vmbus/vmbus_common_uio.c| 232 +++
> mk/rte.app.mk | 1 +
<...>
> @@ -381,6 +381,9 @@ VDEV bus driver
> M: Jianfeng Tan
> F: drivers/bus/vdev/
>
> +VMBUS bus driver
> +M: Stephen Hemminger
> +F: drivers/bus/vmbus/
We have methods to mark APIs as experimental but not for drivers, we have put "-
EXPERIMENTAL" suffix in MAINTAINER file before, same thing can be done here.
<...>
> @@ -37,3 +37,7 @@ CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL=y
> CONFIG_RTE_LIBRTE_DPAA2_PMD=y
> CONFIG_RTE_LIBRTE_PMD_DPAA2_EVENTDEV=y
> CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC=y
> +
> +# Hyper-V Virtual Machine bus and drivers
> +CONFIG_RTE_LIBRTE_VMBUS=y
We tend to disable the component that has external dependency, because of uuid
library dependency I believe vmbus should be disabled by default.
<...>
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
> +LDLIBS += -lrte_ethdev -lrte_vmbus -luuid
-lrte_vmbus is causing build error for shared build, there is no library like
that, looks like can be dropped.
<...>
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +EXPERIMENTAL {
> + global:
> +
> + rte_vmbus_chan_close;
> + rte_vmbus_chan_open;
> + rte_vmbus_chan_recv;
> + rte_vmbus_chan_recv_raw;
> + rte_vmbus_chan_rx_empty;
> + rte_vmbus_chan_send;
> + rte_vmbus_chan_send_sglist;
> + rte_vmbus_chan_signal_tx;
> + rte_vmbus_irq_mask;
> + rte_vmbus_irq_read;
> + rte_vmbus_irq_unmask;
> + rte_vmbus_map_device;
> + rte_vmbus_max_channels;
> + rte_vmbus_probe;
> + rte_vmbus_probe_one;
> + rte_vmbus_register;
> + rte_vmbus_scan;
> + rte_vmbus_sub_channel_index;
> + rte_vmbus_subchan_open;
> + rte_vmbus_unmap_device;
> + rte_vmbus_unregister;
> +
> + local: *;
> +};
Just to confirm, these APIs are for bus drivers, not for applications, right? If
so we already don't guarantee comptiblity in this level, perhaps adding
__experimental flag to these APIs is overkill but no harm.
<...>
Re: [dpdk-dev] [PATCH v3 2/4] net/netvsc: add hyper-v netvsc network device
On 4/17/2018 10:53 PM, Stephen Hemminger wrote:
> From: Stephen Hemminger
>
> The driver supports Hyper-V networking directly like
> virtio for KVM or vmxnet3 for VMware.
>
> This code is based off of the FreeBSD driver. The file and variable
> names are kept the same to help with understanding (with most of the
> BSD style warts removed).
>
> Signed-off-by: Stephen Hemminger
> ---
> MAINTAINERS |7 +
> config/common_base|8 +
> config/common_linuxapp|2 +-
> drivers/net/Makefile |1 +
> drivers/net/netvsc/Makefile | 23 +
> drivers/net/netvsc/hn_ethdev.c| 760 ++
> drivers/net/netvsc/hn_logs.h | 35 +
> drivers/net/netvsc/hn_nvs.c | 533 +++
> drivers/net/netvsc/hn_nvs.h | 243
> drivers/net/netvsc/hn_rndis.c | 1101 +++
> drivers/net/netvsc/hn_rndis.h | 26 +
> drivers/net/netvsc/hn_rxtx.c | 1221 +
> drivers/net/netvsc/hn_var.h | 140 ++
> drivers/net/netvsc/ndis.h | 378 +
> drivers/net/netvsc/rndis.h| 414 ++
> drivers/net/netvsc/rte_pmd_netvsc_version.map |5 +
> mk/rte.app.mk |1 +
Can you please add .ini file (doc/guides/nics/features/*.ini) to document PMD
features?
<...>
> @@ -479,6 +479,13 @@ F: drivers/net/enic/
> F: doc/guides/nics/enic.rst
> F: doc/guides/nics/features/enic.ini
>
> +Hyper-V netvsc
> +M: Stephen Hemminger
> +M: K. Y. Srinivasan"
Is " a typo?
<...>
> +/* XXX Why is this not generic in RTE? */
> +static int
> +hn_dev_atomic_write_link_status(struct rte_eth_dev *dev,
> + struct rte_eth_link *link)
> +{
It is indeed generic now thanks to you :)
Can you switch to your generic functions?
<...>
> +/* enables testpmd to collect per queue stats. */
> +static int
> +hn_queue_stats_mapping_set(__rte_unused struct rte_eth_dev *eth_dev,
> +__rte_unused uint16_t queue_id,
> +__rte_unused uint8_t stat_idx,
> +__rte_unused uint8_t is_rx)
> +{
> + return 0;
> +}
IS this dev_ops needed at all?
<...>
> @@ -0,0 +1,5 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +
> +DPDK_18.02 {
v18.05
Re: [dpdk-dev] [PATCH v3 3/4] net/netvsc: add documentation
On 4/17/2018 10:53 PM, Stephen Hemminger wrote: > Matching documentation for new netvsc device. > > Signed-off-by: Stephen Hemminger > --- > doc/guides/nics/index.rst | 1 + > doc/guides/nics/netvsc.rst | 97 ++ > doc/guides/rel_notes/release_18_05.rst | 6 ++ > 3 files changed, 104 insertions(+) > create mode 100644 doc/guides/nics/netvsc.rst Documentation added for network driver but not for bus driver, does it make sense to add vmbus doc too? <...> > diff --git a/doc/guides/rel_notes/release_18_05.rst > b/doc/guides/rel_notes/release_18_05.rst > index bc9cdda6af7a..0a7b65d49293 100644 > --- a/doc/guides/rel_notes/release_18_05.rst > +++ b/doc/guides/rel_notes/release_18_05.rst > @@ -115,6 +115,12 @@ New Features > >Linux uevent is supported as backend of this device event notification > framework. > > +* **Added experimental support for Hyper-V netvsc PMD.** > + > + The new experimntal ``netvsc`` poll mode driver provides native support for > + networking on Hyper-V. See the :doc:`../nics/netvsc` nic driver guide > + for more details on this new driver. > + Also there is a "Shared Library Versions" section of the release notes, the libraries with public API should be listed here with version information. I can see some bus libraries are there. If any of the libraries added has public API, should be added into that list.
Re: [dpdk-dev] [PATCH v3 0/4] Hyper-V Netvsc PMD
On 4/17/2018 10:53 PM, Stephen Hemminger wrote: > This is a revised version of the Hyper-V native bus (VMBus) > and network device (netvsc) drivers. This virtual device > is used in Microsoft Hyper-V in Windows 10, Windows Server 2016 > and Azure. Most of this code was extracted from FreeBSD and some of > this is from earlier code donated by Brocade. > > Only Linux is supported at present, but the code is split > to allow future FreeBSD and Windows support. > > This version works with upstream kernel (4.16) but in that > mode only a single queue is supported. With additional > patches that are pending for 5.0 kernel, multi-queue > support works as well. > > Device binding is best done via driverctl; this required some > additional fixes to kernel and driverctl to work correctly. > Linux kernel vmbus support needed to support sysfs driver_override > and driverctl needed to handle non-PCI bus from udev. > https://gitlab.com/driverctl/driverctl/merge_requests/3 > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-April/118889.html > > v3 > - fix documentation to format correctly and include more info > about binding > - fix build on 32 bit > - minor white space and indentation fixes > > Stephen Hemminger (4): > bus/vmbus: add hyper-v virtual bus support > net/netvsc: add hyper-v netvsc network device > net/netvsc: add documentation > bus/vmbus and net/netvsc: add meson build support Overall this is a bus driver implementation and a net driver implementation, it is not API that effect other part of the DPDK or end user. As long as it is not breaking anything and documented I am for getting the patch as experimental for this release. There are few minor comments on patches.
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
Hi Xueming,
On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> "port config all rss all" command will fail on PMD that not support any
> of hard coding RSS hash types. This patch changed hard coding hash types
> to supported types retrieved from device info.
>
> Signed-off-by: Xueming Li
Problem is that this patch removes the ability to request "all" RSS types
regardless of PMD support.
Users will expect "all" to behave as documented, however doing so will only
result in the limited set of types reported by PMDs. For instance at least
mlx4 doesn't update the flow_type_rss_offloads field since it has never
implemented configuration support for the legacy RSS API.
You should add an argument with a different name (e.g. "supported" or
"default") for that and keep the original meaning for "all".
Testpmd documentation has to be updated accordingly.
One more nit below.
> ---
> app/test-pmd/cmdline.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 512e3b55e..d357de7e6 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result,
> {
> struct cmd_config_rss *res = parsed_result;
> struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> + struct rte_eth_dev_info dev_info = {0};
This could be declared in the new block where it's used. Also note that
"{0}" is nonstandard syntax, use memset() or initialize a field like
rss_key_len above.
> int diag;
> uint8_t i;
>
> @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> }
> rss_conf.rss_key = NULL;
> for (i = 0; i < rte_eth_dev_count(); i++) {
> + if (!strcmp(res->value, "all")) {
> + rte_eth_dev_info_get(i, &dev_info);
> + if (dev_info.flow_type_rss_offloads)
> + rss_conf.rss_hf =
> + dev_info.flow_type_rss_offloads;
> + }
> diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> if (diag < 0)
> printf("Configuration of RSS hash at ethernet port %d "
> --
> 2.13.3
>
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [RFC] checkpatch: don't complain about SPDX tag format
On Wed, Apr 18, 2018 at 10:49:07AM +, Kuusisaari, Juhamatti wrote:
>
> Hello,
>
> > On Wed, Apr 18, 2018 at 12:19:07AM +0200, Thomas Monjalon wrote:
> > > 18/04/2018 00:11, Scott Branden:
> > > > On 18-04-17 03:06 PM, Thomas Monjalon wrote:
> > > > > 17/04/2018 23:49, Stephen Hemminger:
> > > > >> IMHO would have been better to use the kernel SPDX style and keep
> > > > >> the check but that appears to be a minority opinion.
> > > > >
> > > > > I think it is better to work on checkpatch itself.
> > > > > When defining our SPDX style, Linux one was not definitive.
> > > > > Do you think we can ask the Linux community to support our SPDX style?
> > > > >
> > > > I think it better to simply follow the Linux community defacto style
> > > > rather than go your own way.
> > >
> > > But our way is better! :)
> > > And it has been decided in the Technical Board.
> > >
> >
> > As a general issue, I think we could do with having our own checkpatch-like
> > script for performing addition DPDK-specific code-checks *after* Linux
> > checkpatch ones. That is, reuse Linux check patch checks as much as
> > possible,
> > but have other checks too.
> >
> > For example, check for use of strcpy or strncpy (or snprintf with "%s") and
> > suggest replacing with strlcpy. If we did have our own extension script, we
> > could put our own SPDX format check there too.
> >
> > Thoughts, or any volunteers to look into this?
>
> In addition, the checkpatches.sh could be improved so that it actually checks
> that a proper file is found behind the selected env variable. I am planning
> to add this check (as it bite me just yesterday).
>
> Speaking of strlcpy, I do think that it has a caveat* that everybody should
> be aware of: depending on implementation, it may read unintended memory
> regions when the source is not properly null terminated (like in Unix domain
> sockets, or just by other mistake). It may be a bad idea just blindly replace
> everything with strlcpy, without making sure that copied buffers are really
> null-terminated in the first place or making sure the strlcpy version is
> really a one that does not have this problem. As it depends on dynamic
> libraries, making sure may be difficult.
>
> Some may argue that this is unlikely and thus irrelevant. Why do I know about
> it then? :) Needless to say, strncpy or snprintf do not have _this_ problem,
> although they have their own issues. Internally without dynamic libs DPDK
> rte_strlcpy uses snprintf which should be safe, though.
>
> > /Bruce
>
> --
> Juhamatti
>
> * A caveat on some implementations:
> ...
> /* Not enough room in dst, add NUL and traverse rest of src */
> if (n == 0) {
> if (siz != 0)
> *d = '\0'; /* NUL-terminate dst */
> while (*s++) <- what happens when s is not null-terminated?
> ;
> }
> ...
> Another one:
> ...
> return n + strlen (src); <- what happens when src is not null-terminated?
> ...
Thanks for pointing that out. It's good to be aware of these caveats. I
suspect in most cases the replacement is safe, but we should not blindly
replace one thing with another without checking for possible unintended
side effects.
/Bruce
Re: [dpdk-dev] [dpdk-web] [PATCH v2] update stable releases roadmap
18/04/2018 14:28, Ferruh Yigit: > On 4/18/2018 10:14 AM, Thomas Monjalon wrote: > > 18/04/2018 11:05, Ferruh Yigit: > >> On 4/11/2018 12:28 AM, Thomas Monjalon wrote: > >>> - Typically a new stable release version follows a mainline release > >>> - by 1-2 weeks, depending on the test results. > >>> + The first stable release (.1) of a branch should follow > >>> + its mainline release (.0) by at least two months, > >>> + after the first release candidate (-rc1) of the next branch. > >> > >> Hi Thomas, > >> > >> What this change suggest? To be able to backport patches from rc1? > > > > Yes, it is the proposal we discussed earlier. > > We can wait one week after RC1 to get some validation confirmation. > > Do you agree? > > This has been discussed in tech-board, what I remember the decision was to > wait > the release to backport patches into stable tree. It was not so clear to me. I thought post-rc1 was acceptable. The idea is to speed-up stable releases pace, especially first release of a series.
Re: [dpdk-dev] [PATCH] devtools: improve kernel script execution check
On 4/18/2018 1:21 PM, Juhamatti Kuusisaari wrote: > Handle properly a case where DPDK_PATCH_PATH is set > to point to a directory. > > Signed-off-by: Juhamatti Kuusisaari Acked-by: Ferruh Yigit
[dpdk-dev] [PATCH v6 0/2] support Tx generic tunnel checksum and TSO
V6: - Comments update according to maillist discussion. V5: - Removed duplicated testpmd patch in other pathset. - More comments on PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP V4: - Removed DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM and DEV_TX_OFFLOAD_GENERIC_TNL_TSO - Replaced with DEV_TX_OFFLOAD_IP_TNL_TSO - Removed PKT_TX_OUTER_UDP - Splited PKT_TX_TUNNEL_UNKNOWN into PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP V3: - Add VXLAN-GPE and GRE extention support to testpmd csum forwarding enginee - Split DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM_TSO into DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM and DEV_TX_OFFLOAD_GENERIC_TNL_TSO - Add PKT_TX_TUNNEL_UNKNOWN and PKT_TX_OUTER_UDP http://www.dpdk.org/dev/patchwork/patch/34655/ This patchset introduces new HW capability of generic tunnel cheksum and TSO offloads, HW supporting generic tunnel checksum and TSO could handle new tunnel type offloading w/o upgrading HW. New Generic tunnel checksum and TSO offload support IP tunnel and UDP tunnel. Please note that tunnel type that has length, sequence id or checksum are not considered as generi c tunnel type. Xueming Li (2): ethdev: introduce generic IP/UDP tunnel checksum and TSO app/testpmd: testpmd support Tx generic tunnel offloads app/test-pmd/cmdline.c| 14 -- app/test-pmd/config.c | 17 + lib/librte_ether/rte_ethdev.h | 12 lib/librte_mbuf/rte_mbuf.c| 6 ++ lib/librte_mbuf/rte_mbuf.h| 25 + 5 files changed, 72 insertions(+), 2 deletions(-) -- 2.13.3
[dpdk-dev] [PATCH V20 0/4] add hot plug recovery mechanism
At the prior, device event monitor framework have been introduced, the typical usage of it is for device hot plug. If we want application would not be break down when device hot plug in or out, we still need some measures to do recovery to do preparation for device detach, so that we will not encounter any memory fault after device be hot unplug, that will let application to keep working. This patch set will introduces an API to implement the recovery mechanism to handle hot plug, and also use testpmd to show example how to use the API for process hot plug event, let the process could be smoothly like below: plug out->failure handle->stop forward->stop port->close port->detach port with this mechanism, user such as fail-safe driver or testpmd could be able to develop their own hot plug application. patchset history: v20->v19: clean the code refine the remap logic for multiple device. remove the auto binding v19->18: note for limitation of multiple hotplug,fix some typo, sqeeze patch. v18->v15: add document, add signal bus handler, refine the code to be more clear. the prior patch history please check the patch set "add device event monitor framework" Jeff Guo (4): bus/pci: introduce device hot unplug handle eal: add failure handler mechanism for hot plug igb_uio: fix uio release issue when hot unplug app/testpmd: show example to handler hot unplug app/test-pmd/testpmd.c | 29 ++-- doc/guides/rel_notes/release_18_05.rst | 6 ++ drivers/bus/pci/pci_common.c| 67 + drivers/bus/pci/pci_common_uio.c| 32 + drivers/bus/pci/private.h | 12 kernel/linux/igb_uio/igb_uio.c | 4 ++ lib/librte_eal/common/include/rte_bus.h | 16 + lib/librte_eal/common/include/rte_dev.h | 11 +++ lib/librte_eal/linuxapp/eal/eal_dev.c | 124 +++- lib/librte_eal/rte_eal_version.map | 1 + 10 files changed, 297 insertions(+), 5 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH V20 1/4] bus/pci: introduce device hot unplug handle
As of device hot unplug, we need some preparatory measures so that we will
not encounter memory fault after device be plug out of the system,
and also let we could recover the running data path but not been break.
This patch allows the buses to handle device hot unplug event.
The patch only enable the ops in pci bus, when handle device hot unplug
event, remap a dummy memory to avoid bus read/write error.
Other buses could accordingly implement this ops specific by themselves.
Signed-off-by: Jeff Guo
---
v20->19:
clean the code
---
drivers/bus/pci/pci_common.c| 67 +
drivers/bus/pci/pci_common_uio.c| 32
drivers/bus/pci/private.h | 12 ++
lib/librte_eal/common/include/rte_bus.h | 16
4 files changed, 127 insertions(+)
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 2a00f36..709eaf3 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -474,6 +474,72 @@ pci_find_device(const struct rte_device *start,
rte_dev_cmp_t cmp,
}
static int
+pci_handle_hot_unplug(struct rte_device *dev, void *failure_addr)
+{
+ struct rte_pci_device *pdev = NULL;
+ int ret = 0, i, isfound = 0;
+
+ if (failure_addr != NULL) {
+ FOREACH_DEVICE_ON_PCIBUS(pdev) {
+ for (i = 0; i != sizeof(pdev->mem_resource) /
+ sizeof(pdev->mem_resource[0]); i++) {
+ if ((uint64_t)failure_addr >=
+ (uint64_t)pdev->mem_resource[i].addr &&
+ (uint64_t)failure_addr <=
+ (uint64_t)pdev->mem_resource[i].addr +
+ pdev->mem_resource[i].len) {
+ RTE_LOG(ERR, EAL, "Failure address "
+ "%16.16"PRIx64" is belong to "
+ "resource of device %s!\n",
+ (uint64_t)failure_addr,
+ pdev->device.name);
+ isfound = 1;
+ break;
+ }
+ }
+ if (isfound)
+ break;
+ }
+ } else if (dev != NULL) {
+ pdev = RTE_DEV_TO_PCI(dev);
+ } else {
+ return -EINVAL;
+ }
+
+ if (!pdev)
+ return -1;
+
+ /* remap resources for devices */
+ switch (pdev->kdrv) {
+ case RTE_KDRV_VFIO:
+#ifdef VFIO_PRESENT
+ /* TODO */
+#endif
+ break;
+ case RTE_KDRV_IGB_UIO:
+ case RTE_KDRV_UIO_GENERIC:
+ if (rte_eal_using_phys_addrs()) {
+ /* map resources for devices that use uio */
+ ret = pci_uio_remap_resource(pdev);
+ }
+ break;
+ case RTE_KDRV_NIC_UIO:
+ ret = pci_uio_remap_resource(pdev);
+ break;
+ default:
+ RTE_LOG(DEBUG, EAL,
+ " Not managed by a supported kernel driver,
skipped\n");
+ ret = -1;
+ break;
+ }
+
+ if (ret != 0)
+ RTE_LOG(ERR, EAL, "failed to handle hot unplug of %s",
+ pdev->name);
+ return ret;
+}
+
+static int
pci_plug(struct rte_device *dev)
{
return pci_probe_all_drivers(RTE_DEV_TO_PCI(dev));
@@ -503,6 +569,7 @@ struct rte_pci_bus rte_pci_bus = {
.unplug = pci_unplug,
.parse = pci_parse,
.get_iommu_class = rte_pci_get_iommu_class,
+ .handle_hot_unplug = pci_handle_hot_unplug,
},
.device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
.driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
index 54bc20b..ba2c458 100644
--- a/drivers/bus/pci/pci_common_uio.c
+++ b/drivers/bus/pci/pci_common_uio.c
@@ -146,6 +146,38 @@ pci_uio_unmap(struct mapped_pci_resource *uio_res)
}
}
+/* remap the PCI resource of a PCI device in anonymous virtual memory */
+int
+pci_uio_remap_resource(struct rte_pci_device *dev)
+{
+ int i;
+ void *map_address;
+
+ if (dev == NULL)
+ return -1;
+
+ /* Remap all BARs */
+ for (i = 0; i != PCI_MAX_RESOURCE; i++) {
+ /* skip empty BAR */
+ if (dev->mem_resource[i].phys_addr == 0)
+ continue;
+ pci_unmap_resource(dev->mem_resource[i].addr,
+ (size_t)dev->mem_resource[i].len);
+ map_address = pci_map_resource(
+ dev
[dpdk-dev] [PATCH V20 2/4] eal: add failure handler mechanism for hot plug
This patch introduces a failure handler mechanism to handle device
hot unplug event. When device be hot plug out, the device resource
become invalid, if this resource is still be unexpected read/write,
system will crash. This patch let eal help application to handle
this fault, when sigbus error occur, check the failure address and
accordingly remap the invalid memory for the corresponding device,
that could guaranty the application not to be shut down when hot plug.
Signed-off-by: Jeff Guo
---
v20->v19:
refine the logic of remapping for multiple device.
---
doc/guides/rel_notes/release_18_05.rst | 6 ++
lib/librte_eal/common/include/rte_dev.h | 11 +++
lib/librte_eal/linuxapp/eal/eal_dev.c | 124 +++-
lib/librte_eal/rte_eal_version.map | 1 +
4 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_18_05.rst
b/doc/guides/rel_notes/release_18_05.rst
index a018ef5..a4ea9af 100644
--- a/doc/guides/rel_notes/release_18_05.rst
+++ b/doc/guides/rel_notes/release_18_05.rst
@@ -70,6 +70,12 @@ New Features
Linux uevent is supported as backend of this device event notification
framework.
+* **Added hot plug failure handler.**
+
+ Added a failure handler machenism to handle hot unplug device.
+
+ * ``rte_dev_handle_hot_unplug`` for handle hot unplug device failure.
+
API Changes
---
diff --git a/lib/librte_eal/common/include/rte_dev.h
b/lib/librte_eal/common/include/rte_dev.h
index 0955e9a..9933131 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -360,4 +360,15 @@ rte_dev_event_monitor_start(void);
int __rte_experimental
rte_dev_event_monitor_stop(void);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * It can be used to handle the device signal bus error. when signal bus error
+ * occur, the handler would check the failure address to find the corresponding
+ * device and remap the memory resource of the device, that would guaranty
+ * the system not crash when the device be hot unplug.
+ */
+void __rte_experimental
+rte_dev_handle_hot_unplug(void);
#endif /* _RTE_DEV_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c
b/lib/librte_eal/linuxapp/eal/eal_dev.c
index 9478a39..33e7026 100644
--- a/lib/librte_eal/linuxapp/eal/eal_dev.c
+++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
@@ -4,6 +4,8 @@
#include
#include
+#include
+#include
#include
#include
@@ -13,12 +15,16 @@
#include
#include
#include
+#include
+#include
#include "eal_private.h"
static struct rte_intr_handle intr_handle = {.fd = -1 };
static bool monitor_started;
+extern struct rte_bus_list rte_bus_list;
+
#define EAL_UEV_MSG_LEN 4096
#define EAL_UEV_MSG_ELEM_LEN 128
@@ -33,6 +39,68 @@ enum eal_dev_event_subsystem {
};
static int
+dev_uev_failure_process(struct rte_device *dev, void *dev_addr)
+{
+ struct rte_bus *bus;
+ int ret = 0;
+
+ if (!dev && !dev_addr) {
+ return -EINVAL;
+ } else if (dev) {
+ bus = rte_bus_find_by_device_name(dev->name);
+ if (bus->handle_hot_unplug) {
+ /**
+* call bus ops to handle hot unplug.
+*/
+ ret = bus->handle_hot_unplug(dev, dev_addr);
+ if (ret) {
+ RTE_LOG(ERR, EAL,
+ "It cannot handle hot unplug "
+ "for device (%s) "
+ "on the bus.\n ",
+ dev->name);
+ }
+ }
+ } else {
+ TAILQ_FOREACH(bus, &rte_bus_list, next) {
+ if (bus->handle_hot_unplug) {
+ /**
+* call bus ops to handle hot unplug.
+*/
+ ret = bus->handle_hot_unplug(dev, dev_addr);
+ if (ret) {
+ RTE_LOG(ERR, EAL,
+ "It cannot handle hot unplug "
+ "for the device "
+ "on the bus.\n ");
+ }
+ }
+ }
+ }
+ return ret;
+}
+
+static void sigbus_handler(int signum __rte_unused, siginfo_t *info,
+ void *ctx __rte_unused)
+{
+ int ret;
+
+ RTE_LOG(ERR, EAL, "SIGBUS error, fault address:%p\n", info->si_addr);
+ ret = dev_uev_failure_process(NULL, info->si_addr);
+ if (!ret)
+ RTE_LOG(DEBUG, EAL,
+ "SIGBUS error is because of hot unplug!\n");
+}
+
+static int cmp_dev_name(const struct rte_device *dev,
+ const
[dpdk-dev] [PATCH V20 3/4] igb_uio: fix uio release issue when hot unplug
when device being hot unplug, release a none exist uio resource will
result kernel null pointer error, so this patch will check if device
has been remove before release uio release procedure, if so just return
back.
Signed-off-by: Jeff Guo
---
v20->v19:
split patch independently.
---
kernel/linux/igb_uio/igb_uio.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c
index cbc5ab6..c296332 100644
--- a/kernel/linux/igb_uio/igb_uio.c
+++ b/kernel/linux/igb_uio/igb_uio.c
@@ -344,6 +344,10 @@ igbuio_pci_release(struct uio_info *info, struct inode
*inode)
struct rte_uio_pci_dev *udev = info->priv;
struct pci_dev *dev = udev->pdev;
+ /* check if device has been remove before release */
+ if ((&dev->dev.kobj)->state_remove_uevent_sent == 1)
+ return -1;
+
mutex_lock(&udev->lock);
if (--udev->refcnt > 0) {
mutex_unlock(&udev->lock);
--
2.7.4
[dpdk-dev] [PATCH V20 4/4] app/testpmd: show example to handler hot unplug
Use testpmd for example, to show how an application smoothly handle
failure when device being hot unplug. Once app detect the removal event,
the callback would be called, it first stop the packet forwarding, then
stop the port, close the port and finally detach the port.
Signed-off-by: Jeff Guo
---
v20->v19:
remove the auto binding example.
---
app/test-pmd/testpmd.c | 29 +
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5986ff7..3751901 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1125,6 +1125,9 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t
pkt_fwd)
tics_datum = rte_rdtsc();
tics_per_1sec = rte_get_timer_hz();
#endif
+ if (hot_plug)
+ rte_dev_handle_hot_unplug();
+
fsm = &fwd_streams[fc->stream_idx];
nb_fs = fc->stream_nb;
do {
@@ -2069,6 +2072,26 @@ rmv_event_callback(void *arg)
dev->device->name);
}
+static void
+rmv_dev_event_callback(char *dev_name)
+{
+ uint16_t port_id;
+ int ret;
+
+ ret = rte_eth_dev_get_port_by_name(dev_name, &port_id);
+ if (ret) {
+ printf("can not get port by device %s!\n", dev_name);
+ return;
+ }
+
+ RTE_ETH_VALID_PORTID_OR_RET(port_id);
+ printf("removing port id:%u\n", port_id);
+ stop_packet_forwarding();
+ stop_port(port_id);
+ close_port(port_id);
+ detach_port(port_id);
+}
+
/* This function is used by the interrupt thread */
static int
eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
@@ -2130,9 +2153,7 @@ eth_dev_event_callback(char *device_name, enum
rte_dev_event_type type,
case RTE_DEV_EVENT_REMOVE:
RTE_LOG(ERR, EAL, "The device: %s has been removed!\n",
device_name);
- /* TODO: After finish failure handle, begin to stop
-* packet forward, stop port, close port, detach port.
-*/
+ rmv_dev_event_callback(device_name);
break;
case RTE_DEV_EVENT_ADD:
RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
@@ -2640,7 +2661,7 @@ main(int argc, char** argv)
return -1;
}
eth_dev_event_callback_register();
-
+ rte_dev_handle_hot_unplug();
}
if (start_port(RTE_PORT_ALL) != 0)
--
2.7.4
Re: [dpdk-dev] [PATCH v2 1/2] net/pcap: physical interface MAC support
On 4/18/2018 5:35 AM, Kuusisaari, Juhamatti wrote: > Hello Ferruh, > >> On 4/17/2018 1:53 PM, Juhamatti Kuusisaari wrote: >>> Support for PCAP MAC address using physical interface MAC. >>> Support for getting proper link status, speed and duplex. >>> >>> Signed-off-by: Juhamatti Kuusisaari >>> --- >>> config/common_base | 1 + >>> drivers/net/pcap/rte_eth_pcap.c | 52 >>> - >>> 2 files changed, 52 insertions(+), 1 deletion(-) >>> >>> diff --git a/config/common_base b/config/common_base index >>> c2b0d91..9804585 100644 >>> --- a/config/common_base >>> +++ b/config/common_base >>> @@ -410,6 +410,7 @@ CONFIG_RTE_LIBRTE_PMD_NULL=y # Compile >> software >>> PMD backed by PCAP files # CONFIG_RTE_LIBRTE_PMD_PCAP=n >>> +CONFIG_RTE_LIBRTE_PMD_PCAP_IF_MAC_SUPPORT=n >> >> Hi Juhamatti, >> >> Why a build time config option for this? Can we make it a runtime devarg? > > Sure, we can make it a devarg. Or do we even need that? Are there a lot of > test dependencies that would need to be fixed if we have it enabled by > default? Not test dependencies but this may be overkill for some usecases, I prefer making this dynamically configurable, no strong opinion though. > >> Overall we are trying to reduce config options already and this seems no >> need to be build time option at all. >> >> btw, this is a little late in release cycle, so lets target this patch for >> next >> release. > > The patch is on top of net-next, this should be just fine. Perhaps we should rename the sub-tree :) because this is not happening first time. next-net is not for next release, as it has been Linux, it is for this release but just a sub-tree for net PMDs. > >> Thanks, >> ferruh > > Thanks, > -- > Juhamatti >
[dpdk-dev] [PATCH v2 0/3] net/mlx5: implement set_mc_addr devop
Support eth_dev_ops.set_mc_addr_list(). Changes in v2: Simplify the code and are even more verifications. Nelio Laranjeiro (3): net/mlx5: more checks on MAC addresses net/mlx5: split MAC address add/remove code net/mlx5: implement multicast add list devop drivers/net/mlx5/mlx5.c| 2 + drivers/net/mlx5/mlx5.h| 2 + drivers/net/mlx5/mlx5_defs.h | 7 +- drivers/net/mlx5/mlx5_ethdev.c | 2 +- drivers/net/mlx5/mlx5_mac.c| 117 - 5 files changed, 112 insertions(+), 18 deletions(-) -- 2.17.0
[dpdk-dev] [PATCH v2 2/3] net/mlx5: split MAC address add/remove code
Move some code in DPDK callbacks to add/remove MAC addresses to internal
function. This modification will be necessary to handle implement the
devop set_mc_addr_list.
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5_defs.h | 4 +-
drivers/net/mlx5/mlx5_ethdev.c | 2 +-
drivers/net/mlx5/mlx5_mac.c| 86 +-
3 files changed, 68 insertions(+), 24 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index 3fb2f1480..e3aa03bef 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -13,8 +13,10 @@
/* Reported driver name. */
#define MLX5_DRIVER_NAME "net_mlx5"
+/* Maximum number of simultaneous unicast MAC addresses. */
+#define MLX5_MAX_UC_MAC_ADDRESSES 128
/* Maximum number of simultaneous MAC addresses. */
-#define MLX5_MAX_MAC_ADDRESSES 128
+#define MLX5_MAX_MAC_ADDRESSES MLX5_MAX_UC_MAC_ADDRESSES
/* Maximum number of simultaneous VLAN filters. */
#define MLX5_MAX_VLAN_IDS 128
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index ef44cc91f..99ac4cc5c 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -446,7 +446,7 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *info)
max = 65535;
info->max_rx_queues = max;
info->max_tx_queues = max;
- info->max_mac_addrs = RTE_DIM(priv->mac);
+ info->max_mac_addrs = MLX5_MAX_UC_MAC_ADDRESSES;
info->rx_queue_offload_capa = mlx5_get_rx_queue_offloads(dev);
info->rx_offload_capa = (mlx5_get_rx_port_offloads() |
info->rx_queue_offload_capa);
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index f24b5f759..e9334ea4a 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -57,38 +57,30 @@ mlx5_get_mac(struct rte_eth_dev *dev, uint8_t
(*mac)[ETHER_ADDR_LEN])
}
/**
- * DPDK callback to remove a MAC address.
+ * Remove a MAC address from the internal array.
*
* @param dev
* Pointer to Ethernet device structure.
* @param index
* MAC address index.
*/
-void
-mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
+static void
+mlx5_internal_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
{
struct priv *priv = dev->data->dev_private;
const int vf = priv->config.vf;
- int ret;
- if (index >= MLX5_MAX_MAC_ADDRESSES)
- return;
+ assert(index < MLX5_MAX_MAC_ADDRESSES);
if (is_zero_ether_addr(&dev->data->mac_addrs[index]))
return;
if (vf)
mlx5_nl_mac_addr_remove(dev, &dev->data->mac_addrs[index],
index);
memset(&dev->data->mac_addrs[index], 0, sizeof(struct ether_addr));
- if (!dev->data->promiscuous) {
- ret = mlx5_traffic_restart(dev);
- if (ret)
- DRV_LOG(ERR, "port %u cannot restart traffic: %s",
- dev->data->port_id, strerror(rte_errno));
- }
}
/**
- * DPDK callback to add a MAC address.
+ * Adds a MAC address to the internal array.
*
* @param dev
* Pointer to Ethernet device structure.
@@ -96,24 +88,19 @@ mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t
index)
* MAC address to register.
* @param index
* MAC address index.
- * @param vmdq
- * VMDq pool index to associate address with (ignored).
*
* @return
* 0 on success, a negative errno value otherwise and rte_errno is set.
*/
-int
-mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
- uint32_t index, uint32_t vmdq __rte_unused)
+static int
+mlx5_internal_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
+ uint32_t index)
{
struct priv *priv = dev->data->dev_private;
const int vf = priv->config.vf;
unsigned int i;
- if (index >= MLX5_MAX_MAC_ADDRESSES) {
- rte_errno = EINVAL;
- return -rte_errno;
- }
+ assert(index < MLX5_MAX_MAC_ADDRESSES);
if (is_zero_ether_addr(mac)) {
rte_errno = EINVAL;
return -rte_errno;
@@ -136,6 +123,61 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct
ether_addr *mac,
return ret;
}
dev->data->mac_addrs[index] = *mac;
+ return 0;
+}
+
+/**
+ * DPDK callback to remove a MAC address.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @param index
+ * MAC address index.
+ */
+void
+mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
+{
+ int ret;
+
+ if (index >= MLX5_MAX_UC_MAC_ADDRESSES)
+ return;
+ mlx5_internal_mac_addr_remove(dev, index);
+ if (!dev->data->promiscuous) {
+ ret = mlx5_traffic_restart(dev);
+ if (ret)
+ DR
[dpdk-dev] [PATCH v2 1/3] net/mlx5: more checks on MAC addresses
Verify MAC address before further process.
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5_mac.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index edc7a32ae..f24b5f759 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -71,7 +71,10 @@ mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
const int vf = priv->config.vf;
int ret;
- assert(index < MLX5_MAX_MAC_ADDRESSES);
+ if (index >= MLX5_MAX_MAC_ADDRESSES)
+ return;
+ if (is_zero_ether_addr(&dev->data->mac_addrs[index]))
+ return;
if (vf)
mlx5_nl_mac_addr_remove(dev, &dev->data->mac_addrs[index],
index);
@@ -107,7 +110,14 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct
ether_addr *mac,
const int vf = priv->config.vf;
unsigned int i;
- assert(index < MLX5_MAX_MAC_ADDRESSES);
+ if (index >= MLX5_MAX_MAC_ADDRESSES) {
+ rte_errno = EINVAL;
+ return -rte_errno;
+ }
+ if (is_zero_ether_addr(mac)) {
+ rte_errno = EINVAL;
+ return -rte_errno;
+ }
/* First, make sure this address isn't already configured. */
for (i = 0; (i != MLX5_MAX_MAC_ADDRESSES); ++i) {
/* Skip this index, it's going to be reconfigured. */
--
2.17.0
Re: [dpdk-dev] [RFC] checkpatch: don't complain about SPDX tag format
18/04/2018 10:56, Bruce Richardson: > On Wed, Apr 18, 2018 at 12:19:07AM +0200, Thomas Monjalon wrote: > > 18/04/2018 00:11, Scott Branden: > > > On 18-04-17 03:06 PM, Thomas Monjalon wrote: > > > > 17/04/2018 23:49, Stephen Hemminger: > > > >> IMHO would have been better to use the kernel SPDX style and > > > >> keep the check but that appears to be a minority opinion. > > > > > > > > I think it is better to work on checkpatch itself. > > > > When defining our SPDX style, Linux one was not definitive. > > > > Do you think we can ask the Linux community to support our SPDX style? > > > > > > > I think it better to simply follow the Linux community defacto style > > > rather than go your own way. > > > > But our way is better! :) > > And it has been decided in the Technical Board. > > > > As a general issue, I think we could do with having our own checkpatch-like > script for performing addition DPDK-specific code-checks *after* Linux > checkpatch ones. That is, reuse Linux check patch checks as much as > possible, but have other checks too. +1 to call more scripts in checkpatches.sh. We need to find the right language to do code checks. Coccinelle looks to be a good candidate for some checks. > For example, check for use of strcpy or strncpy (or snprintf with "%s") and > suggest replacing with strlcpy. If we did have our own extension script, we > could put our own SPDX format check there too. > > Thoughts, or any volunteers to look into this? I am not volunteer to start the work but I would be glad to contribute later. Any motivated volunteer?
[dpdk-dev] [PATCH v2 3/3] net/mlx5: implement multicast add list devop
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5.c | 2 ++
drivers/net/mlx5/mlx5.h | 2 ++
drivers/net/mlx5/mlx5_defs.h | 5 -
drivers/net/mlx5/mlx5_mac.c | 33 +
4 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 68783c3ac..887924d07 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -277,6 +277,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
.mac_addr_remove = mlx5_mac_addr_remove,
.mac_addr_add = mlx5_mac_addr_add,
.mac_addr_set = mlx5_mac_addr_set,
+ .set_mc_addr_list = mlx5_set_mc_addr_list,
.mtu_set = mlx5_dev_set_mtu,
.vlan_strip_queue_set = mlx5_vlan_strip_queue_set,
.vlan_offload_set = mlx5_vlan_offload_set,
@@ -329,6 +330,7 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
.mac_addr_remove = mlx5_mac_addr_remove,
.mac_addr_add = mlx5_mac_addr_add,
.mac_addr_set = mlx5_mac_addr_set,
+ .set_mc_addr_list = mlx5_set_mc_addr_list,
.mtu_set = mlx5_dev_set_mtu,
.vlan_strip_queue_set = mlx5_vlan_strip_queue_set,
.vlan_offload_set = mlx5_vlan_offload_set,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 6ad41390a..64f025d22 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -203,6 +203,8 @@ void mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t
index);
int mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
uint32_t index, uint32_t vmdq);
int mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr);
+int mlx5_set_mc_addr_list(struct rte_eth_dev *dev,
+ struct ether_addr *mc_addr_set, uint32_t nb_mc_addr);
/* mlx5_rss.c */
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index e3aa03bef..5973742a7 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -15,8 +15,11 @@
/* Maximum number of simultaneous unicast MAC addresses. */
#define MLX5_MAX_UC_MAC_ADDRESSES 128
+/* Maximum number of simultaneous Multicast MAC addresses. */
+#define MLX5_MAX_MC_MAC_ADDRESSES 128
/* Maximum number of simultaneous MAC addresses. */
-#define MLX5_MAX_MAC_ADDRESSES MLX5_MAX_UC_MAC_ADDRESSES
+#define MLX5_MAX_MAC_ADDRESSES \
+ (MLX5_MAX_UC_MAC_ADDRESSES + MLX5_MAX_MC_MAC_ADDRESSES)
/* Maximum number of simultaneous VLAN filters. */
#define MLX5_MAX_VLAN_IDS 128
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index e9334ea4a..32560d55e 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -201,3 +201,36 @@ mlx5_mac_addr_set(struct rte_eth_dev *dev, struct
ether_addr *mac_addr)
dev->data->port_id);
return mlx5_mac_addr_add(dev, mac_addr, 0, 0);
}
+
+/**
+ * DPDK callback to set multicast addresses list.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @param mac_addr_set
+ * Multicast MAC address pointer array.
+ * @param nb_mac_addr
+ * Number of entries in the array.
+ *
+ * @return
+ * 0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+mlx5_set_mc_addr_list(struct rte_eth_dev *dev,
+ struct ether_addr *mc_addr_set, uint32_t nb_mc_addr)
+{
+ uint32_t i;
+ int ret;
+
+ for (i = MLX5_MAX_UC_MAC_ADDRESSES; i != MLX5_MAX_MAC_ADDRESSES; ++i)
+ mlx5_internal_mac_addr_remove(dev, i);
+ i = MLX5_MAX_UC_MAC_ADDRESSES;
+ while (nb_mc_addr--) {
+ ret = mlx5_internal_mac_addr_add(dev, mc_addr_set++, i++);
+ if (ret)
+ return ret;
+ }
+ if (!dev->data->promiscuous)
+ return mlx5_traffic_restart(dev);
+ return 0;
+}
--
2.17.0
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
> -Original Message-
> From: Adrien Mazarguil
> Sent: Wednesday, April 18, 2018 9:26 PM
> To: Xueming(Steven) Li
> Cc: Shahaf Shuler ; Nelio Laranjeiro
> ; Wenzhuo Lu
> ; Jingjing Wu ; Thomas Monjalon
> ;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS
> hash types
>
> Hi Xueming,
>
> On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > "port config all rss all" command will fail on PMD that not support
> > any of hard coding RSS hash types. This patch changed hard coding hash
> > types to supported types retrieved from device info.
> >
> > Signed-off-by: Xueming Li
>
> Problem is that this patch removes the ability to request "all" RSS types
> regardless of PMD support.
>
> Users will expect "all" to behave as documented, however doing so will only
> result in the limited set
> of types reported by PMDs. For instance at least
> mlx4 doesn't update the flow_type_rss_offloads field since it has never
> implemented configuration
> support for the legacy RSS API.
>
> You should add an argument with a different name (e.g. "supported" or
> "default") for that and keep the original meaning for "all".
>
> Testpmd documentation has to be updated accordingly.
You must want to have a look at first part of this patchset:
http://www.dpdk.org/ml/archives/dev/2018-April/097849.html
>
> One more nit below.
>
> > ---
> > app/test-pmd/cmdline.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 512e3b55e..d357de7e6 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > struct cmd_config_rss *res = parsed_result;
> > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > + struct rte_eth_dev_info dev_info = {0};
>
> This could be declared in the new block where it's used. Also note that "{0}"
> is nonstandard syntax,
> use memset() or initialize a field like rss_key_len above.
>
> > int diag;
> > uint8_t i;
> >
> > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > }
> > rss_conf.rss_key = NULL;
> > for (i = 0; i < rte_eth_dev_count(); i++) {
> > + if (!strcmp(res->value, "all")) {
> > + rte_eth_dev_info_get(i, &dev_info);
> > + if (dev_info.flow_type_rss_offloads)
> > + rss_conf.rss_hf =
> > + dev_info.flow_type_rss_offloads;
> > + }
> > diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> > if (diag < 0)
> > printf("Configuration of RSS hash at ethernet port %d "
> > --
> > 2.13.3
> >
>
>
> --
> Adrien Mazarguil
> 6WIND
[dpdk-dev] [PATCH v6 1/2] ethdev: introduce generic IP/UDP tunnel checksum and TSO
This patch introduce new TX offload flags for device that supports
IP or UDP tunneled packet L3/L4 checksum and TSO offload.
It will be used for non-standard tunnels.
The support from the device is for inner and outer checksums on
IPV4/TCP/UDP and TSO for *any packet with the following format*:
/ [optional IPv4/IPv6] / [optional TCP/UDP] / / [optional inner IPv4/IPv6] / [optional TCP/UDP]
For example the following packets can use this feature:
1. eth / ipv4 / udp / VXLAN / ip / tcp
2. eth / ipv4 / GRE / MPLS / ipv4 / udp
Please note that specific tunnel headers that contain payload length,
sequence id or checksum will not be updated.
The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM.
The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in
later release.
Signed-off-by: Xueming Li
---
lib/librte_ether/rte_ethdev.h | 12
lib/librte_mbuf/rte_mbuf.c| 6 ++
lib/librte_mbuf/rte_mbuf.h| 25 +
3 files changed, 43 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4f417f573..ef152dec6 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -980,6 +980,18 @@ struct rte_eth_conf {
* the same mempool and has refcnt = 1.
*/
#define DEV_TX_OFFLOAD_SECURITY 0x0002
+/**
+ * Device supports generic UDP tunneled packet TSO.
+ * Application must set PKT_TX_TUNNEL_UDP and other mbuf fields required
+ * for tunnel TSO.
+ */
+#define DEV_TX_OFFLOAD_UDP_TNL_TSO 0x0004
+/**
+ * Device supports generic IP tunneled packet TSO.
+ * Application must set PKT_TX_TUNNEL_IP and other mbuf fields required
+ * for tunnel TSO.
+ */
+#define DEV_TX_OFFLOAD_IP_TNL_TSO 0x0008
/*
* If new Tx offload capabilities are defined, they also must be
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 3f4c83305..64e960d4c 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -390,6 +390,8 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
+ case PKT_TX_TUNNEL_IP: return "PKT_TX_TUNNEL_IP";
+ case PKT_TX_TUNNEL_UDP: return "PKT_TX_TUNNEL_UDP";
case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
case PKT_TX_SEC_OFFLOAD: return "PKT_TX_SEC_OFFLOAD";
default: return NULL;
@@ -424,6 +426,10 @@ rte_get_tx_ol_flag_list(uint64_t mask, char *buf, size_t
buflen)
"PKT_TX_TUNNEL_NONE" },
{ PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MASK,
"PKT_TX_TUNNEL_NONE" },
+ { PKT_TX_TUNNEL_IP, PKT_TX_TUNNEL_MASK,
+ "PKT_TX_TUNNEL_NONE" },
+ { PKT_TX_TUNNEL_UDP, PKT_TX_TUNNEL_MASK,
+ "PKT_TX_TUNNEL_NONE" },
{ PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
{ PKT_TX_SEC_OFFLOAD, PKT_TX_SEC_OFFLOAD, NULL },
};
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 06eceba37..a1ee1c3a7 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -210,6 +210,31 @@ extern "C" {
#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
/**< TX packet with MPLS-in-UDP RFC 7510 header. */
#define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
+/**
+ * Generic IP encapsulated tunnel type, used for TSO and checksum offload.
+ * It can be used for tunnels which are not standards or listed above.
+ * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
+ * if possible.
+ * The ethdev must be configured with DEV_TX_OFFLOAD_IP_TNL_TSO.
+ * Outer and inner checksums are done according to the existing flags like
+ * PKT_TX_xxx_CKSUM.
+ * Specific tunnel headers that contain payload length, sequence id
+ * or checksum are not expected to be updated.
+ */
+#define PKT_TX_TUNNEL_IP (0xDULL << 45)
+/**
+ * Generic UDP encapsulated tunnel type, used for TSO and checksum offload.
+ * UDP tunnel type implies outer IP layer.
+ * It can be used for tunnels which are not standards or listed above.
+ * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
+ * if possible.
+ * The ethdev must be configured with DEV_TX_OFFLOAD_UDP_TNL_TSO.
+ * Outer and inner checksums are done according to the existing flags like
+ * PKT_TX_xxx_CKSUM.
+ * Specific tunnel headers that contain payload length, sequence id
+ * or checksum are not expected to be updated.
+ */
+#define PKT_TX_TUNNEL_UDP (0xEULL << 45)
/* add new TX TUNNEL type here */
#define PKT_TX_TUNNEL_MASK(0xFULL << 45)
--
2.13.3
[dpdk-dev] [PATCH v6 2/2] app/testpmd: testpmd support Tx generic tunnel offloads
"show port cap" and "csum parse tunnel" command support TX generic
tunnel offloads
Signed-off-by: Xueming Li
---
app/test-pmd/cmdline.c | 14 --
app/test-pmd/config.c | 17 +
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 512e3b55e..4ec3dab56 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -4157,6 +4157,12 @@ check_tunnel_tso_nic_support(portid_t port_id)
if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GENEVE_TNL_TSO))
printf("Warning: GENEVE TUNNEL TSO not supported therefore "
"not enabled for port %d\n", port_id);
+ if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_IP_TNL_TSO))
+ printf("Warning: IP TUNNEL TSO not supported therefore "
+ "not enabled for port %d\n", port_id);
+ if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_UDP_TNL_TSO))
+ printf("Warning: UDP TUNNEL TSO not supported therefore "
+ "not enabled for port %d\n", port_id);
return dev_info;
}
@@ -4184,13 +4190,17 @@ cmd_tunnel_tso_set_parsed(void *parsed_result,
~(DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
DEV_TX_OFFLOAD_GRE_TNL_TSO |
DEV_TX_OFFLOAD_IPIP_TNL_TSO |
- DEV_TX_OFFLOAD_GENEVE_TNL_TSO);
+ DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
+ DEV_TX_OFFLOAD_IP_TNL_TSO |
+ DEV_TX_OFFLOAD_UDP_TNL_TSO);
printf("TSO for tunneled packets is disabled\n");
} else {
uint64_t tso_offloads = (DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
DEV_TX_OFFLOAD_GRE_TNL_TSO |
DEV_TX_OFFLOAD_IPIP_TNL_TSO |
-DEV_TX_OFFLOAD_GENEVE_TNL_TSO);
+DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
+DEV_TX_OFFLOAD_IP_TNL_TSO |
+DEV_TX_OFFLOAD_UDP_TNL_TSO);
ports[res->port_id].dev_conf.txmode.offloads |=
(tso_offloads & dev_info.tx_offload_capa);
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index dd051f5ca..f329a8810 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -722,6 +722,23 @@ port_offload_cap_display(portid_t port_id)
printf("off\n");
}
+ if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_IP_TNL_TSO) {
+ printf("IP tunnel TSO: ");
+ if (ports[port_id].dev_conf.txmode.offloads &
+ DEV_TX_OFFLOAD_IP_TNL_TSO)
+ printf("on\n");
+ else
+ printf("off\n");
+ }
+
+ if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_UDP_TNL_TSO) {
+ printf("UDP tunnel TSO: ");
+ if (ports[port_id].dev_conf.txmode.offloads &
+ DEV_TX_OFFLOAD_UDP_TNL_TSO)
+ printf("on\n");
+ else
+ printf("off\n");
+ }
}
int
--
2.13.3
Re: [dpdk-dev] [PATCH] examples/ip_pipeline: fix logically dead node
> -Original Message-
> From: Zhang, Roy Fan
> Sent: Monday, April 16, 2018 11:58 AM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian ; Singh, Jasvinder
>
> Subject: [PATCH] examples/ip_pipeline: fix logically dead node
>
> Coverity issue: 272567
> Fixes: d75c371e9b46 ("examples/ip_pipeline: add pipeline object")
>
> Signed-off-by: Fan Zhang
> ---
> examples/ip_pipeline/cli.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c index
> 199a31ff8..6c07ca223 100644
> --- a/examples/ip_pipeline/cli.c
> +++ b/examples/ip_pipeline/cli.c
> @@ -1890,12 +1890,6 @@ cmd_pipeline_table(char **tokens,
>
> t0 += 6;
> } else if (strcmp(tokens[t0], "stub") == 0) {
> - if (n_tokens < t0 + 1) {
> - snprintf(out, out_size, MSG_ARG_MISMATCH,
> - "pipeline table stub");
> - return;
> - }
> -
> p.match_type = TABLE_STUB;
>
> t0 += 1;
> --
> 2.13.6
Reviewed-by: Jasvinder Singh
Re: [dpdk-dev] [PATCH] devtools: improve kernel script execution check
18/04/2018 15:30, Ferruh Yigit: > On 4/18/2018 1:21 PM, Juhamatti Kuusisaari wrote: > > Handle properly a case where DPDK_PATCH_PATH is set > > to point to a directory. > > > > Signed-off-by: Juhamatti Kuusisaari > > Acked-by: Ferruh Yigit Funny bug, I didn't thought that "test -x" is positive for directories. It makes sense. Applied, thanks
Re: [dpdk-dev] [PATCH] devtools: improve kernel script execution check
18/04/2018 16:06, Thomas Monjalon: > 18/04/2018 15:30, Ferruh Yigit: > > On 4/18/2018 1:21 PM, Juhamatti Kuusisaari wrote: > > > Handle properly a case where DPDK_PATCH_PATH is set > > > to point to a directory. > > > > > > Signed-off-by: Juhamatti Kuusisaari > > > > Acked-by: Ferruh Yigit > > Funny bug, I didn't thought that "test -x" is positive for directories. > It makes sense. > > Applied, thanks No, sorry, not applied yet. Please, could you send a v2 fixing devtools/get-maintainer.sh too? Thanks
Re: [dpdk-dev] kernel binding of devices + hotplug
On Sun, Apr 15, 2018 at 01:48:36AM +, Stephen Hemminger wrote: > My vote is to work with udev and not try to replace it. > > Driverctl works well. Just not for bifurcated driver I second that. We also have other system configs to care about like kernel parameters and hugepage configuration which I think follow the same idea that they are system wide configs and should not be managed by DPDK itself. fbl > > On Fri, Apr 13, 2018, 9:31 AM Thomas Monjalon wrote: > > > It's time to think (again) how we bind devices with kernel modules. > > We need to decide how we want to manage hotplugged devices with DPDK. > > > > A bit of history first. > > There was some code in DPDK for bind/unbind, but it has been removed > > in DPDK 1.7 - http://dpdk.org/commit/5d8751b83 > > Copy of the commit message (in 2014): > > " > > The bind/unbind operations should not be handled by the eal. > > These operations should be either done outside of dpdk or > > inside the PMDs themselves as these are their problems. > > " > > > > The question raised at this time (4 years ago) is still under discussion. > > Should we manage binding inside or outside DPDK? > > Should it be controlled in the application or in the OS base? > > > > As you know, we use dpdk-devbind.py. > > This tool lacks two major features: > > - persistent configuration > > - hotplug > > > > If we consider that the DPDK applications should be able to apply its own > > policy to choose the devices to bind, then we need to implement binding > > in the PMD (with EAL helpers). > > > > On the other hand, if we consider that it is the system responsibility, > > then we could choose systemd/udev and driverctl. > > > > The debate is launched! > > > > Please find more details in the references below. > > > > Announce of driverctl: > > http://dpdk.org/ml/archives/dev/2015-December/029500.html > > Repository of driverctl: > > https://gitlab.com/driverctl/driverctl > > > > Discussion about binding script and driverctl: > > http://dpdk.org/ml/archives/dev/2018-April/095687.html > > > > Patch to implement binding in DPDK (for hotplug): > > http://dpdk.org/ml/archives/dev/2018-April/095714.html > > > > Discussion in the same hotplug series: > > http://dpdk.org/ml/archives/dev/2018-April/097058.html > > > > > > > > -- Flavio
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
> -Original Message-
> From: Adrien Mazarguil
> Sent: Wednesday, April 18, 2018 9:26 PM
> To: Xueming(Steven) Li
> Cc: Shahaf Shuler ; Nelio Laranjeiro
> ; Wenzhuo Lu
> ; Jingjing Wu ; Thomas Monjalon
> ;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS
> hash types
>
> Hi Xueming,
>
> On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > "port config all rss all" command will fail on PMD that not support
> > any of hard coding RSS hash types. This patch changed hard coding hash
> > types to supported types retrieved from device info.
> >
> > Signed-off-by: Xueming Li
>
> Problem is that this patch removes the ability to request "all" RSS types
> regardless of PMD support.
>
> Users will expect "all" to behave as documented, however doing so will only
> result in the limited set
> of types reported by PMDs. For instance at least
> mlx4 doesn't update the flow_type_rss_offloads field since it has never
> implemented configuration
> support for the legacy RSS API.
Keeping it "all" as it is will result in error for PMDs not completely support
"all" RSS types and this
always confuse people.
How about adding a check, if flow_type_rss_offloads not set, ignore and set
default RSS "all" types?
>
> You should add an argument with a different name (e.g. "supported" or
> "default") for that and keep the original meaning for "all".
>
> Testpmd documentation has to be updated accordingly.
>
> One more nit below.
>
> > ---
> > app/test-pmd/cmdline.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 512e3b55e..d357de7e6 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > struct cmd_config_rss *res = parsed_result;
> > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > + struct rte_eth_dev_info dev_info = {0};
>
> This could be declared in the new block where it's used. Also note that "{0}"
> is nonstandard syntax,
> use memset() or initialize a field like rss_key_len above.
>
> > int diag;
> > uint8_t i;
> >
> > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > }
> > rss_conf.rss_key = NULL;
> > for (i = 0; i < rte_eth_dev_count(); i++) {
> > + if (!strcmp(res->value, "all")) {
> > + rte_eth_dev_info_get(i, &dev_info);
> > + if (dev_info.flow_type_rss_offloads)
> > + rss_conf.rss_hf =
> > + dev_info.flow_type_rss_offloads;
> > + }
> > diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> > if (diag < 0)
> > printf("Configuration of RSS hash at ethernet port %d "
> > --
> > 2.13.3
> >
>
>
> --
> Adrien Mazarguil
> 6WIND
Re: [dpdk-dev] [PATCH v2] app/bbdev: remove improper WARNING printouts
> -Original Message- > From: Chalupnik, KamilX > Sent: Wednesday 18 April 2018 10:26 > To: dev@dpdk.org > Cc: Mokhtar, Amr ; Chalupnik, KamilX > > Subject: [PATCH v2] app/bbdev: remove improper WARNING printouts > > Improper WARNING printouts in BBDev Test Application removed > > Signed-off-by: KamilX Chalupnik > > v2: > - apply patch failure fixed > Acked-by: Amr Mokhtar
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
On Wed, Apr 18, 2018 at 01:54:20PM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Adrien Mazarguil
> > Sent: Wednesday, April 18, 2018 9:26 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; Nelio Laranjeiro
> > ; Wenzhuo Lu
> > ; Jingjing Wu ; Thomas
> > Monjalon ;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported
> > RSS hash types
> >
> > Hi Xueming,
> >
> > On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > > "port config all rss all" command will fail on PMD that not support
> > > any of hard coding RSS hash types. This patch changed hard coding hash
> > > types to supported types retrieved from device info.
> > >
> > > Signed-off-by: Xueming Li
> >
> > Problem is that this patch removes the ability to request "all" RSS types
> > regardless of PMD support.
> >
> > Users will expect "all" to behave as documented, however doing so will only
> > result in the limited set
> > of types reported by PMDs. For instance at least
> > mlx4 doesn't update the flow_type_rss_offloads field since it has never
> > implemented configuration
> > support for the legacy RSS API.
> >
> > You should add an argument with a different name (e.g. "supported" or
> > "default") for that and keep the original meaning for "all".
> >
> > Testpmd documentation has to be updated accordingly.
>
> You must want to have a look at first part of this patchset:
> http://www.dpdk.org/ml/archives/dev/2018-April/097849.html
I did, however this is does not address my previous comment.
In this patch you're preventing testpmd users from requesting something. If
they want to enable "all" RSS types and some of them happen to be
unsupported on some ports, it means they want to get errors. Otherwise they
might think their request succeeded while it was in fact interpreted in an
undocumented manner.
The point is if users want to enable only "supported" RSS types, they
wouldn't write "all" on the command line right? A new parameter is needed
for that.
>
> >
> > One more nit below.
> >
> > > ---
> > > app/test-pmd/cmdline.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > 512e3b55e..d357de7e6 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > > struct cmd_config_rss *res = parsed_result;
> > > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > > + struct rte_eth_dev_info dev_info = {0};
> >
> > This could be declared in the new block where it's used. Also note that
> > "{0}" is nonstandard syntax,
> > use memset() or initialize a field like rss_key_len above.
> >
> > > int diag;
> > > uint8_t i;
> > >
> > > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > > }
> > > rss_conf.rss_key = NULL;
> > > for (i = 0; i < rte_eth_dev_count(); i++) {
> > > + if (!strcmp(res->value, "all")) {
> > > + rte_eth_dev_info_get(i, &dev_info);
> > > + if (dev_info.flow_type_rss_offloads)
> > > + rss_conf.rss_hf =
> > > + dev_info.flow_type_rss_offloads;
> > > + }
> > > diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> > > if (diag < 0)
> > > printf("Configuration of RSS hash at ethernet port %d "
> > > --
> > > 2.13.3
> > >
> >
> >
> > --
> > Adrien Mazarguil
> > 6WIND
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
+Olivier
> -Original Message-
> From: Adrien Mazarguil
> Sent: Wednesday, April 18, 2018 10:17 PM
> To: Xueming(Steven) Li
> Cc: Shahaf Shuler ; Nelio Laranjeiro
> ; Wenzhuo Lu
> ; Jingjing Wu ; Thomas Monjalon
> ;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS
> hash types
>
> On Wed, Apr 18, 2018 at 01:54:20PM +, Xueming(Steven) Li wrote:
> >
> >
> > > -Original Message-
> > > From: Adrien Mazarguil
> > > Sent: Wednesday, April 18, 2018 9:26 PM
> > > To: Xueming(Steven) Li
> > > Cc: Shahaf Shuler ; Nelio Laranjeiro
> > > ; Wenzhuo Lu ;
> > > Jingjing Wu ; Thomas Monjalon
> > > ; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config
> > > supported RSS hash types
> > >
> > > Hi Xueming,
> > >
> > > On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > > > "port config all rss all" command will fail on PMD that not
> > > > support any of hard coding RSS hash types. This patch changed hard
> > > > coding hash types to supported types retrieved from device info.
> > > >
> > > > Signed-off-by: Xueming Li
> > >
> > > Problem is that this patch removes the ability to request "all" RSS types
> > > regardless of PMD
> support.
> > >
> > > Users will expect "all" to behave as documented, however doing so
> > > will only result in the limited set of types reported by PMDs. For
> > > instance at least
> > > mlx4 doesn't update the flow_type_rss_offloads field since it has
> > > never implemented configuration support for the legacy RSS API.
> > >
> > > You should add an argument with a different name (e.g. "supported"
> > > or
> > > "default") for that and keep the original meaning for "all".
> > >
> > > Testpmd documentation has to be updated accordingly.
> >
> > You must want to have a look at first part of this patchset:
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> > dpdk.org%2Fml%2Farchives%2Fdev%2F2018-April%2F097849.html&data=02%7C01
> > %7Cxuemingl%40mellanox.com%7C497583093d324a6861f708d5a5370a8f%7Ca65297
> > 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636596578186582002&sdata=uNVw2egf
> > 0rcTHazCuoMelNrY44lZVfd6Ado1bJfzg8s%3D&reserved=0
>
> I did, however this is does not address my previous comment.
In first patch, if flow_type_rss_offloads not set, any RSS will not be allowed.
We need to clarify definition of empty flow_type_rss_offloads, not set or no
rss support.
>
> In this patch you're preventing testpmd users from requesting something. If
> they want to enable "all"
> RSS types and some of them happen to be unsupported on some ports, it means
> they want to get errors.
> Otherwise they might think their request succeeded while it was in fact
> interpreted in an undocumented
> manner.
>
> The point is if users want to enable only "supported" RSS types, they
> wouldn't write "all" on the
> command line right? A new parameter is needed for that.
>
> >
> > >
> > > One more nit below.
> > >
> > > > ---
> > > > app/test-pmd/cmdline.c | 7 +++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > > 512e3b55e..d357de7e6 100644
> > > > --- a/app/test-pmd/cmdline.c
> > > > +++ b/app/test-pmd/cmdline.c
> > > > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > > > struct cmd_config_rss *res = parsed_result;
> > > > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > > > + struct rte_eth_dev_info dev_info = {0};
> > >
> > > This could be declared in the new block where it's used. Also note
> > > that "{0}" is nonstandard syntax, use memset() or initialize a field like
> > > rss_key_len above.
> > >
> > > > int diag;
> > > > uint8_t i;
> > > >
> > > > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > > > }
> > > > rss_conf.rss_key = NULL;
> > > > for (i = 0; i < rte_eth_dev_count(); i++) {
> > > > + if (!strcmp(res->value, "all")) {
> > > > + rte_eth_dev_info_get(i, &dev_info);
> > > > + if (dev_info.flow_type_rss_offloads)
> > > > + rss_conf.rss_hf =
> > > > + dev_info.flow_type_rss_offloads;
> > > > + }
> > > > diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> > > > if (diag < 0)
> > > > printf("Configuration of RSS hash at ethernet
> > > > port %d "
> > > > --
> > > > 2.13.3
> > > >
> > >
> > >
> > > --
> > > Adrien Mazarguil
> > > 6WIND
>
> --
> Adrien Mazarguil
> 6WIND
Re: [dpdk-dev] [PATCH v6 1/2] ethdev: introduce generic IP/UDP tunnel checksum and TSO
18/04/2018 15:58, Xueming Li: > This patch introduce new TX offload flags for device that supports > IP or UDP tunneled packet L3/L4 checksum and TSO offload. > It will be used for non-standard tunnels. > > The support from the device is for inner and outer checksums on > IPV4/TCP/UDP and TSO for *any packet with the following format*: > > / [optional IPv4/IPv6] / [optional TCP/UDP] / headers> / [optional inner IPv4/IPv6] / [optional TCP/UDP] > > For example the following packets can use this feature: > > 1. eth / ipv4 / udp / VXLAN / ip / tcp > 2. eth / ipv4 / GRE / MPLS / ipv4 / udp > > Please note that specific tunnel headers that contain payload length, > sequence id or checksum will not be updated. > > The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM. > The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in > later release. > > Signed-off-by: Xueming Li Except a small comment below, it looks OK. Acked-by: Thomas Monjalon Please send a deprecation notice for PKT_TX_OUTER_IP_CKSUM. > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > +/** > + * Generic IP encapsulated tunnel type, used for TSO and checksum offload. > + * It can be used for tunnels which are not standards or listed above. > + * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN > + * if possible. PKT_TX_TUNNEL_GRE or PKT_TX_TUNNEL_IPIP may be a better example than PKT_TX_TUNNEL_VXLAN in IP tunnel case. > + * The ethdev must be configured with DEV_TX_OFFLOAD_IP_TNL_TSO. > + * Outer and inner checksums are done according to the existing flags like > + * PKT_TX_xxx_CKSUM. > + * Specific tunnel headers that contain payload length, sequence id > + * or checksum are not expected to be updated. > + */ > +#define PKT_TX_TUNNEL_IP (0xDULL << 45) > +/** > + * Generic UDP encapsulated tunnel type, used for TSO and checksum offload. > + * UDP tunnel type implies outer IP layer. > + * It can be used for tunnels which are not standards or listed above. > + * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN > + * if possible. > + * The ethdev must be configured with DEV_TX_OFFLOAD_UDP_TNL_TSO. > + * Outer and inner checksums are done according to the existing flags like > + * PKT_TX_xxx_CKSUM. > + * Specific tunnel headers that contain payload length, sequence id > + * or checksum are not expected to be updated. > + */ > +#define PKT_TX_TUNNEL_UDP (0xEULL << 45)
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
On Wed, Apr 18, 2018 at 02:10:45PM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Adrien Mazarguil
> > Sent: Wednesday, April 18, 2018 9:26 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; Nelio Laranjeiro
> > ; Wenzhuo Lu
> > ; Jingjing Wu ; Thomas
> > Monjalon ;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported
> > RSS hash types
> >
> > Hi Xueming,
> >
> > On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > > "port config all rss all" command will fail on PMD that not support
> > > any of hard coding RSS hash types. This patch changed hard coding hash
> > > types to supported types retrieved from device info.
> > >
> > > Signed-off-by: Xueming Li
> >
> > Problem is that this patch removes the ability to request "all" RSS types
> > regardless of PMD support.
> >
> > Users will expect "all" to behave as documented, however doing so will only
> > result in the limited set
> > of types reported by PMDs. For instance at least
> > mlx4 doesn't update the flow_type_rss_offloads field since it has never
> > implemented configuration
> > support for the legacy RSS API.
>
> Keeping it "all" as it is will result in error for PMDs not completely
> support "all" RSS types and this
> always confuse people.
You're redefining the commonly accepted meaning of "all". The first instance
of "all" in the "port config all rss all" command means "all ports", not
"only ports that happen to implement the RSS configuration callback". Same
for all other testpmd commands accepting "all" as a parameter anywhere.
Those that don't will return errors, it's fine.
> How about adding a check, if flow_type_rss_offloads not set, ignore and set
> default RSS "all" types?
Results would be even less predictable. Keep it simple. Testpmd is a testing
tool, if PMD developers want to check error-spitting abilities of their
PMDs, they must be able to. Adding a dedicated parameter is one way to
satisfy everyone, dropping this patch is another.
> > You should add an argument with a different name (e.g. "supported" or
> > "default") for that and keep the original meaning for "all".
> >
> > Testpmd documentation has to be updated accordingly.
> >
> > One more nit below.
> >
> > > ---
> > > app/test-pmd/cmdline.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > 512e3b55e..d357de7e6 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > > struct cmd_config_rss *res = parsed_result;
> > > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > > + struct rte_eth_dev_info dev_info = {0};
> >
> > This could be declared in the new block where it's used. Also note that
> > "{0}" is nonstandard syntax,
> > use memset() or initialize a field like rss_key_len above.
> >
> > > int diag;
> > > uint8_t i;
> > >
> > > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > > }
> > > rss_conf.rss_key = NULL;
> > > for (i = 0; i < rte_eth_dev_count(); i++) {
> > > + if (!strcmp(res->value, "all")) {
> > > + rte_eth_dev_info_get(i, &dev_info);
> > > + if (dev_info.flow_type_rss_offloads)
> > > + rss_conf.rss_hf =
> > > + dev_info.flow_type_rss_offloads;
> > > + }
> > > diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> > > if (diag < 0)
> > > printf("Configuration of RSS hash at ethernet port %d "
> > > --
> > > 2.13.3
> > >
> >
> >
> > --
> > Adrien Mazarguil
> > 6WIND
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [PATCH v4 04/11] net/mlx5: support Rx tunnel type identification
> -Original Message- > From: Nélio Laranjeiro > Sent: Wednesday, April 18, 2018 2:51 PM > To: Xueming(Steven) Li > Cc: Shahaf Shuler ; dev@dpdk.org > Subject: Re: [PATCH v4 04/11] net/mlx5: support Rx tunnel type identification > > On Tue, Apr 17, 2018 at 11:14:29PM +0800, Xueming Li wrote: > > This patch introduced tunnel type identification based on flow rules. > > If flows of multiple tunnel types built on same queue, > > RTE_PTYPE_TUNNEL_MASK will be returned, user application could use > > bits in flow mark as tunnel type identifier. > > As discussed in the previous thread, you cannot set all tunnel bits in the > mbuf, it will break > existing applications. > > This is an non announce API breakage. > > Thanks, > > -- > Nélio Laranjeiro > 6WIND There was another issue in code, please check below comments: http://www.dpdk.org/ml/archives/dev/2018-April/096991.html Thanks, Xueming
Re: [dpdk-dev] [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> -Original Message-
> From: Nélio Laranjeiro
> Sent: Wednesday, April 18, 2018 2:49 PM
> To: Xueming(Steven) Li
> Cc: Shahaf Shuler ; dev@dpdk.org
> Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
>
> On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> > This patch support L3 VXLAN, no inner L2 header comparing to standard
> > VXLAN protocol. L3 VXLAN using specific overlay UDP destination port
> > to discriminate against standard VXLAN, FW has to be configured to
> > support
> > it:
> > sudo mlxconfig -d -y s IP_OVER_VXLAN_EN=1
> > sudo mlxconfig -d -y s IP_OVER_VXLAN_PORT=
> >
> > Signed-off-by: Xueming Li
> > ---
> > drivers/net/mlx5/mlx5_flow.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > b/drivers/net/mlx5/mlx5_flow.c index 771d5f14d..d7a921dff 100644
> > --- a/drivers/net/mlx5/mlx5_flow.c
> > +++ b/drivers/net/mlx5/mlx5_flow.c
> > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] =
> > {
> > .dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > },
> > [RTE_FLOW_ITEM_TYPE_VXLAN] = {
> > - .items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> > + .items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > + RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> > + RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
> > .actions = valid_actions,
> > .mask = &(const struct rte_flow_item_vxlan){
> > .vni = "\xff\xff\xff",
> > --
> > 2.13.3
>
> Such support must be under device parameter has it depends on the
> configuration of the firmware. If
> the firmware is not correctly configured the PMD must refuse such rule.
>
> Thanks,
>
> --
> Nélio Laranjeiro
> 6WIND
Are you suggesting Verbs parameter? I'm afraid we can't have it in short time,
need new patch in later
release when Verbs ready.
Re: [dpdk-dev] [PATCH v2 0/3] net/mlx5: implement set_mc_addr devop
On Wed, Apr 18, 2018 at 03:50:03PM +0200, Nelio Laranjeiro wrote: > Support eth_dev_ops.set_mc_addr_list(). > > Changes in v2: > > Simplify the code and are even more verifications. > > Nelio Laranjeiro (3): > net/mlx5: more checks on MAC addresses > net/mlx5: split MAC address add/remove code > net/mlx5: implement multicast add list devop > > drivers/net/mlx5/mlx5.c| 2 + > drivers/net/mlx5/mlx5.h| 2 + > drivers/net/mlx5/mlx5_defs.h | 7 +- > drivers/net/mlx5/mlx5_ethdev.c | 2 +- > drivers/net/mlx5/mlx5_mac.c| 117 - > 5 files changed, 112 insertions(+), 18 deletions(-) For the series, Acked-by: Adrien Mazarguil -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
On Wed, Apr 18, 2018 at 02:26:30PM +, Xueming(Steven) Li wrote:
> +Olivier
>
> > -Original Message-
> > From: Adrien Mazarguil
> > Sent: Wednesday, April 18, 2018 10:17 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; Nelio Laranjeiro
> > ; Wenzhuo Lu
> > ; Jingjing Wu ; Thomas
> > Monjalon ;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported
> > RSS hash types
> >
> > On Wed, Apr 18, 2018 at 01:54:20PM +, Xueming(Steven) Li wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Adrien Mazarguil
> > > > Sent: Wednesday, April 18, 2018 9:26 PM
> > > > To: Xueming(Steven) Li
> > > > Cc: Shahaf Shuler ; Nelio Laranjeiro
> > > > ; Wenzhuo Lu ;
> > > > Jingjing Wu ; Thomas Monjalon
> > > > ; dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config
> > > > supported RSS hash types
> > > >
> > > > Hi Xueming,
> > > >
> > > > On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > > > > "port config all rss all" command will fail on PMD that not
> > > > > support any of hard coding RSS hash types. This patch changed hard
> > > > > coding hash types to supported types retrieved from device info.
> > > > >
> > > > > Signed-off-by: Xueming Li
> > > >
> > > > Problem is that this patch removes the ability to request "all" RSS
> > > > types regardless of PMD
> > support.
> > > >
> > > > Users will expect "all" to behave as documented, however doing so
> > > > will only result in the limited set of types reported by PMDs. For
> > > > instance at least
> > > > mlx4 doesn't update the flow_type_rss_offloads field since it has
> > > > never implemented configuration support for the legacy RSS API.
> > > >
> > > > You should add an argument with a different name (e.g. "supported"
> > > > or
> > > > "default") for that and keep the original meaning for "all".
> > > >
> > > > Testpmd documentation has to be updated accordingly.
> > >
> > > You must want to have a look at first part of this patchset:
> > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> > > dpdk.org%2Fml%2Farchives%2Fdev%2F2018-April%2F097849.html&data=02%7C01
> > > %7Cxuemingl%40mellanox.com%7C497583093d324a6861f708d5a5370a8f%7Ca65297
> > > 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636596578186582002&sdata=uNVw2egf
> > > 0rcTHazCuoMelNrY44lZVfd6Ado1bJfzg8s%3D&reserved=0
> >
> > I did, however this is does not address my previous comment.
>
> In first patch, if flow_type_rss_offloads not set, any RSS will not be
> allowed.
> We need to clarify definition of empty flow_type_rss_offloads, not set or no
> rss support.
It's fine, mlx4 does not implement rte_eth_dev_rss_hash_update() either
(legacy RSS configuration is not implemented at all).
With mlx4, the command:
port config all rss (all|anything else)
Always returns an error. It's a good thing because it's not implemented and
users must be made aware of that fact. No need to lie about it.
> > In this patch you're preventing testpmd users from requesting something. If
> > they want to enable "all"
> > RSS types and some of them happen to be unsupported on some ports, it means
> > they want to get errors.
> > Otherwise they might think their request succeeded while it was in fact
> > interpreted in an undocumented
> > manner.
> >
> > The point is if users want to enable only "supported" RSS types, they
> > wouldn't write "all" on the
> > command line right? A new parameter is needed for that.
> >
> > >
> > > >
> > > > One more nit below.
> > > >
> > > > > ---
> > > > > app/test-pmd/cmdline.c | 7 +++
> > > > > 1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > > > 512e3b55e..d357de7e6 100644
> > > > > --- a/app/test-pmd/cmdline.c
> > > > > +++ b/app/test-pmd/cmdline.c
> > > > > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > > > > struct cmd_config_rss *res = parsed_result;
> > > > > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > > > > + struct rte_eth_dev_info dev_info = {0};
> > > >
> > > > This could be declared in the new block where it's used. Also note
> > > > that "{0}" is nonstandard syntax, use memset() or initialize a field
> > > > like rss_key_len above.
> > > >
> > > > > int diag;
> > > > > uint8_t i;
> > > > >
> > > > > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > > > > }
> > > > > rss_conf.rss_key = NULL;
> > > > > for (i = 0; i < rte_eth_dev_count(); i++) {
> > > > > + if (!strcmp(res->value, "all")) {
> > > > > + rte_eth_dev_info_get(i, &dev_info);
> > > > > + if (dev_info.flow_type_rss_offloads)
> > > > > + rss_conf.rss_hf =
> > > > > + dev_info.flow_type_rss_offloads;
> > > > > + }
> > > > > diag = rte_eth_dev_
Re: [dpdk-dev] [PATCH v4 05/16] ethdev: alter behavior of flow API actions
On Wed, Apr 18, 2018 at 03:26:00PM +0300, Andrew Rybchenko wrote: > On 04/16/2018 07:22 PM, Adrien Mazarguil wrote: > > This patch makes the following changes to flow rule actions: > > > > - List order now matters, they are redefined as performed first to last > >instead of "all simultaneously". > > > > - Repeated actions are now supported (e.g. specifying QUEUE multiple times > >now duplicates traffic among them). Previously only the last action of > >any given kind was taken into account. > > > > - No more distinction between terminating/non-terminating/meta actions. > >Flow rules themselves are now defined as always terminating unless a > >PASSTHRU action is specified. > > > > These changes alter the behavior of flow rules in corner cases in order to > > prepare the flow API for actions that modify traffic contents or properties > > (e.g. encapsulation, compression) and for which order matter when combined. > > > > Previously one would have to do so through multiple flow rules by combining > > PASSTRHU with priority levels, however this proved overly complex to > > implement at the PMD level, hence this simpler approach. > > > > This breaks ABI compatibility for the following public functions: > > > > - rte_flow_create() > > - rte_flow_validate() > > > > PMDs with rte_flow support are modified accordingly: > > > > - bnxt: no change, implementation already forbids multiple actions and does > >not support PASSTHRU. > > > > - e1000: no change, same as bnxt. > > > > - enic: modified to forbid redundant actions, no support for default drop. > > > > - failsafe: no change needed. > > > > - i40e: no change, implementation already forbids multiple actions. > > > > - ixgbe: same as i40e. > > > > - mlx4: modified to forbid multiple fate-deciding actions and drop when > >unspecified. > > > > - mlx5: same as mlx4, with other redundant actions also forbidden. > > > > - sfc: same as mlx4. > > > > - tap: implementation already complies with the new behavior except for > >the default pass-through modified as a default drop. > > > > Signed-off-by: Adrien Mazarguil > > Reviewed-by: Andrew Rybchenko > > Cc: Ajit Khaparde > > Cc: Wenzhuo Lu > > Cc: John Daley > > Cc: Gaetan Rivet > > Cc: Beilei Xing > > Cc: Konstantin Ananyev > > Cc: Nelio Laranjeiro > > Cc: Andrew Rybchenko > > Cc: Pascal Mazon > > --- > > doc/guides/prog_guide/rte_flow.rst | 67 +--- > > drivers/net/enic/enic_flow.c | 25 > > drivers/net/mlx4/mlx4_flow.c | 21 +++--- > > drivers/net/mlx5/mlx5_flow.c | 69 ++--- > > drivers/net/sfc/sfc_flow.c | 22 +++ > > drivers/net/tap/tap_flow.c | 11 ++ > > lib/librte_ether/rte_flow.h| 54 +++--- > > 7 files changed, 138 insertions(+), 131 deletions(-) > > [...] > > > diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c > > index b9f36587c..a5c6a1670 100644 > > --- a/drivers/net/enic/enic_flow.c > > +++ b/drivers/net/enic/enic_flow.c > > @@ -975,6 +979,10 @@ enic_copy_action_v1(const struct rte_flow_action > > actions[], > > const struct rte_flow_action_queue *queue = > > (const struct rte_flow_action_queue *) > > actions->conf; > > + > > + if (overlap & FATE) > > + return ENOTSUP; > > + overlap |= FATE; > > enic_action->rq_idx = > > enic_rte_rq_idx_to_sop_idx(queue->index); > > break; > > @@ -984,6 +992,8 @@ enic_copy_action_v1(const struct rte_flow_action > > actions[], > > break; > > } > > } > > + if (!overlap & FATE) > > Build using clang on Ubuntu 17.10 fails: > > /var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: fatal error: > logical not is only applied to > the left hand side of this bitwise operator > [-Wlogical-not-parentheses] > if (!overlap & FATE) > ^ ~ > /var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add > parentheses after the '!' to > evaluate the bitwise operator first > if (!overlap & FATE) > ^ > ( ) > /var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add > parentheses around left hand side > expression to silence this warning > if (!overlap & FATE) > ^ > ( ) > 1 error generated. > /var/tmp/dpdk-next-net/mk/internal/rte.compile-pre.mk:114: recipe for target > 'enic_flow.o' failed > make[4]: *** [enic_flow.o] Error 1 > /var/tmp/dpdk-next-net/mk/rte.subdir.mk:35: recipe for target 'enic' failed > make[3]: *** [enic] Error 2 > CC nfp_cpp_pcie_ops.o > make[3]: *** Waiting for unfinished jobs > > $ clang --version > clang version 4.0.1-6 (tags/RELEAS
