Re: [dpdk-dev] [PATCH 5/8] bus/dpaa: fix resource leak
On Thursday 05 April 2018 02:24 PM, Hemant Agrawal wrote:
Fixes: 1459585888b5 ("bus/dpaa: fix memory allocation during scan")
Coverity issue: 268337
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
---
Acked-By: Shreyansh Jain
Re: [dpdk-dev] [PATCH 4/8] net/dpaa: fix the oob access
On Thursday 05 April 2018 02:24 PM, Hemant Agrawal wrote:
Fixes: b21ed3e2a16d ("net/dpaa: support extended statistics")
Coverity issue: 268318
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
---
drivers/net/dpaa/dpaa_ethdev.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 0aad111..cbdc4f2 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -339,6 +339,9 @@ dpaa_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
Definition of this function is:
static int
dpaa_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names,
__rte_unused unsigned int limit)
{
unsigned int i, stat_cnt = RTE_DIM(dpaa_xstats_strings);
+ if (limit < stat_cnt)
+ return stat_cnt;
As this patch is using the 'limit' argument, '__rte_unused' should be
removed from the function arguments.
+
if (xstats_names != NULL)
for (i = 0; i < stat_cnt; i++)
snprintf(xstats_names[i].name,
@@ -366,7 +369,7 @@ dpaa_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
return 0;
fman_if_stats_get_all(dpaa_intf->fif, values_copy,
- sizeof(struct dpaa_if_stats));
+ sizeof(struct dpaa_if_stats) / 8);
for (i = 0; i < stat_cnt; i++)
values[i] =
Once the above is correct, please use:
Acked-By: Shreyansh Jain
Re: [dpdk-dev] [PATCH] net/i40e: fix flow RSS queue index check error
> -Original Message-
> From: Zhao1, Wei
> Sent: Sunday, April 8, 2018 1:37 PM
> To: dev@dpdk.org; sta...@dpdk.org
> Cc: Zhang, Qi Z ; Zhao1, Wei
> Subject: [PATCH] net/i40e: fix flow RSS queue index check error
>
> Ther is a error in queue index check for RSS queue region configuration.
What is the error?
Would you explain more and please add this in commit log?
Regards
Qi
>
> Fixes: ecad87d22383 ("net/i40e: move RSS to flow API")
> Signed-off-by: Wei Zhao
> Tested-by: Peng Yuan
> ---
> drivers/net/i40e/i40e_flow.c | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> f4d08bb..fb7ad51 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -4240,6 +4240,14 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> *dev,
> return -rte_errno;
> }
> }
> +
> + if (rss_info->num < rss->num) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> + act,
> + "no valid queues");
> + return -rte_errno;
> + }
> }
>
> for (n = 0; n < conf_info->queue_region_number; n++) { @@ -4264,17
> +4272,6 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
> return -rte_errno;
> }
>
> - if (rss_info->num < rss->num ||
> - rss->queue[0] < rss_info->queue[0] ||
> - (rss->queue[0] + rss->num >
> - rss_info->num + rss_info->queue[0])) {
> - rte_flow_error_set(error, EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - act,
> - "no valid queues");
> - return -rte_errno;
> - }
> -
> for (i = 0; i < info->queue_region_number; i++) {
> if (info->region[i].queue_num == rss->num &&
> info->region[i].queue_start_index ==
> --
> 2.7.5
Re: [dpdk-dev] [PATCH v2 0/4] NFP PF support based on new CPP interface
On Fri, Apr 6, 2018 at 4:47 PM, Ferruh Yigit wrote: > On 4/6/2018 3:44 PM, Ferruh Yigit wrote: > > On 4/5/2018 3:42 PM, Alejandro Lucero wrote: > >> NFP PMD PF support requires to access the NFP chip for initialization. > >> Current NFP PMD PF support was added based on the NSPU interface. This > >> implies to do initialization through the NSP, a embedded ARM processor > >> which does initialization tasks on demand. The main problem with this > >> approach is it requires to add support for new NSP commands each time > >> a new functionality is required, which does not scale well and it is > >> not really flexible. > >> > >> Using the new CPP user space interface, the PMD can do whatever could > >> be done by the NSP, this is current commands and any new functionality > >> required. This CPP interface allows to access any single chip component > >> facilitating initialization, firmware uploading, firmware debugging or > >> extended stats. > >> > >> The changes just change the PMD PF initialization and do not touch the > >> datapath at all. No performance changes nor PMD functionalities are > affected. > >> > >> The initial impact using the new CPP interface is the way firmware > upload > >> is handled, which helps the PMD detecting the card type and the > firmware file > >> to upload. Future commits will include extended stats and some sort of > debug > >> channel. > >> > >> The specific CPP code is contained in the first patch, which has not > been > >> splitted up because is completely internal to the NFP functionality. The > >> second patch makes the PMD changes required for using the new interface. > >> > >> v2: > >> - removing unused reference to zlib.h > >> - fix build errors > >> - add SPDX tags in new files > >> - rebase changes nfp.rst > >> > >> Alejandro Lucero (4): > >> net/nfp: add NFP CPP support > >> net/nfp: update PMD for using new CPP interface > >> doc: update NFP guide > >> net/nfp: remove files > > > > Series applied to dpdk-next-net/master, thanks. > > btw, some nfp files seems still missing spdx tags, can you please check > them? > Yes, those are not new files. I will send a patch soon. Thanks!
Re: [dpdk-dev] [PATCH v4 10/20] eal/dev: implement device iteration
HI Gaetan
From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> +/* '\0' forbidden in sym */
> +static const char *
> +strfirstof(const char *str,
> +const char *sym)
> +{
> + const char *s;
> +
> + for (s = str; s[0] != '\0'; s++) {
> + const char *c;
> +
> + for (c = sym; c[0] != '\0'; c++) {
> + if (c[0] == s[0])
> + return s;
> + }
> + }
> + return NULL;
> +}
> +
I think you can use strcspn() instead...
Re: [dpdk-dev] [PATCH] bus/fslmc: support for hotplugging of memory
Hello Anatoly,
On Sunday 08 April 2018 10:44 PM, Burakov, Anatoly wrote:
On 05-Apr-18 3:14 PM, Shreyansh Jain wrote:
Restructure VFIO DMA code for handling hotplug memory events
(callbacks) and --legacy case.
Signed-off-by: Shreyansh Jain
---
###
This is based on the 16fbfef04a3 github repository. This is assuming
that changes already exists as done in patch 26/68.
Though, this can be a standalone, replacing 26/88. Though, the Makefile
changes don't exist in this.
Also, this just a first draft. I will push any review changes after this
incrementally over v4.
###
Hi Shreyansh,
I think we can keep the 26/68 as it still works within the context of
the patchset. I would like to add these changes closer to the end, where
we enable support for callbacks in VFIO (this could/should come as the
next patch).
But then it would also mean that dpaa2 would be broken within the memory
hotplug patches?
I think it would be broken once the memseg ceases to be continuous
physical sets.
That said, i took some liberties when integrating this patch, hopefully
for the better. I know you mentioned it's a draft, so you can post any
comments for the inevitable v4 :)
drivers/bus/fslmc/fslmc_bus.c | 15
drivers/bus/fslmc/fslmc_vfio.c | 161
+++
drivers/bus/fslmc/fslmc_vfio.h | 1 +
drivers/net/dpaa2/dpaa2_ethdev.c | 1 -
4 files changed, 163 insertions(+), 15 deletions(-)
diff --git a/drivers/bus/fslmc/fslmc_bus.c
b/drivers/bus/fslmc/fslmc_bus.c
index 5ee0beb85..50884ff3a 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -266,6 +266,21 @@ rte_fslmc_probe(void)
return 0;
}
+ if (rte_log_get_global_level() >= RTE_LOG_DEBUG)
+ rte_dump_physmem_layout(stdout);
Presumably, this is not needed - just debug leftovers?
Yes, and can be removed.
+
+ /* Map existing segments as well as, in case of hotpluggable memory,
+ * install callback handler.
+ */
+ ret = rte_fslmc_vfio_dmamap();
+ if (ret) {
+ FSLMC_BUS_LOG(ERR, "Unable to DMA map existing VAs: (%d)",
+ ret);
+ /* Not continuing ahead */
+ FSLMC_BUS_LOG(ERR, "FSLMC VFIO Mapping failed");
+ return 0;
+ }
+
What happens if there are no devices on the bus that can be used by
DPDK? As far as i can tell, it would return error, which may or may not
be desirable (failing to map anything because there aren't any fslmc
devices is not an error?).
## If no devices found on the bus:
Call wouldn't have reached here. There is a jump out in rte_fslmc_probe
in case no devices were scanned on the bus.
--->8--- rte_fslmc_probe() ---
...
if (TAILQ_EMPTY(&rte_fslmc_bus.device_list))
return 0;
...
--->8---
## Assuming you are pointing to 'return 0':
So, the rte_eal_scan/probe doesn't expect to be interrupted just because
one of the buses had issues (whether no devices found, not a real
issue). It would continue ahead with scan/probe.
But, I think error should be returned so that rte_eal_probe can dump to
logs the error and continue ahead normally. I will fix this.
For "regular" VFIO, the container is an empty shell unless you add
groups into it - does fslmc VFIO support work differently, and container
is already working/initialized by the time we reach this point?
FSLMC VFIO Container is not much different from generic container. But,
at this point in code, the container is already initialized (group is
connected to it, and relevant device fds extracted).
Only thing pending beyond this point is to look into the container and
initialize various fslmc specific devices contained *within* the group.
And, dma mapping of regions which is done using the newly introduced code.
Anyway, i'll leave this as is.
OK.
ret = fslmc_vfio_process_group();
if (ret) {
FSLMC_BUS_LOG(ERR, "Unable to setup devices %d", ret);
diff --git a/drivers/bus/fslmc/fslmc_vfio.c
b/drivers/bus/fslmc/fslmc_vfio.c
index 31831e3ce..60725fb70 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
<...>
+}
+
static int
-fslmc_vfio_map(const struct rte_memseg_list *msl __rte_unused,
- const struct rte_memseg *ms, void *arg)
+#ifdef RTE_LIBRTE_DPAA2_USE_PHYS_IOVA
+fslmc_map_dma(uint64_t vaddr, rte_iova_t iovaddr, size_t len)
+#else
+fslmc_map_dma(uint64_t vaddr, rte_iova_t iovaddr __rte_unused, size_t
len)
+#endif
I think i'll leave this as just "rte_iova_t iovaaddr __rte_unused" :)
Hmm, it is harmless if used without the enclosing #defines.
But, i don't know if any compiler has any side-effects attached to this
- for example, clang reporting this as error, etc.
{
- int *n_segs = arg;
struct fslmc_vfio_group *group;
struct vfio_iommu_type1_dma_map dma_map = {
.argsz = sizeof(struct vfio_iommu_type1_dma_map),
@
Re: [dpdk-dev] [PATCH] net/i40e: add comment for flow RSS parse function
No need separate patch,
please merge this with another patch for queue index check since they are
related.
And please Cc sta...@dpdk.org in commit for a fix for previous release.
Regards
Qi
> -Original Message-
> From: Zhao1, Wei
> Sent: Monday, April 9, 2018 2:19 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Zhang, Qi Z ; Zhao1, Wei
>
> Subject: [PATCH] net/i40e: add comment for flow RSS parse function
>
> This patch add comment for flow rss parse function in order to explain some
> important info.
>
> Fixes: ecad87d22383 ("net/i40e: move RSS to flow API")
> Signed-off-by: Wei Zhao
> ---
> drivers/net/i40e/i40e_flow.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> 2068026..6404168 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -4171,6 +4171,19 @@ i40e_flow_parse_rss_pattern(__rte_unused struct
> rte_eth_dev *dev,
> return 0;
> }
>
> +/**
> + * This function is used to parse rss queue index, total queue number
> +and
> + * hash functions, If the purpose of this configuration is for queue
> +region
> + * configuration, it will set queue_region_conf flag to TRUE, else to FALSE.
> + * In queue region configuration, it also need to parse hardware
> +flowtype
> + * and user_priority from configuration, it will also cheeck the
> +validity
> + * of these parameters. For example, The queue region sizes should
> + * be any of the following values: 1, 2, 4, 8, 16, 32, 64, the
> + * hw_flowtype or PCTYPE max index should be 63, the user priority
> + * max index should be 7, and so on. And also, queue index should be
> + * continuous sequence and queue region index should be part of rss
> + * queue index for this port.
> + */
> static int
> i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
> const struct rte_flow_action *actions,
> --
> 2.7.5
Re: [dpdk-dev] [PATCH v4 18/20] ethdev: register ether layer as a class
Hi Gaetan
From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> Signed-off-by: Gaetan Rivet
> ---
> lib/Makefile | 2 +-
> lib/librte_ether/Makefile| 3 +-
> lib/librte_ether/rte_class_eth.c | 73
>
> 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644
> lib/librte_ether/rte_class_eth.c
>
> diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> librte_cmdline DEPDIRS-librte_cmdline := librte_eal
> DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether DEPDIRS-librte_ether
> := librte_net librte_eal librte_mempool librte_ring -DEPDIRS-librte_ether +=
> librte_mbuf
> +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev DEPDIRS-
> librte_bbdev := librte_eal librte_mempool librte_mbuf
> DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> 3ca5782bb..a1a0393de 100644
> --- a/lib/librte_ether/Makefile
> +++ b/lib/librte_ether/Makefile
> @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API CFLAGS
> += -O3 CFLAGS += $(WERROR_FLAGS) LDLIBS += -lrte_net -lrte_eal -
> lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> +LDLIBS += -lrte_mbuf -lrte_kvargs
>
> EXPORT_MAP := rte_ethdev_version.map
>
> LIBABIVER := 8
>
> SRCS-y += rte_ethdev.c
> +SRCS-y += rte_class_eth.c
> SRCS-y += rte_flow.c
> SRCS-y += rte_tm.c
> SRCS-y += rte_mtr.c
> diff --git a/lib/librte_ether/rte_class_eth.c
> b/lib/librte_ether/rte_class_eth.c
> new file mode 100644
> index 0..97d24781d
> --- /dev/null
> +++ b/lib/librte_ether/rte_class_eth.c
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Gaëtan Rivet
> + */
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "rte_ethdev.h"
> +#include "rte_ethdev_core.h"
> +
> +enum eth_params {
> + RTE_ETH_PARAMS_MAX,
> +};
> +
> +static const char *eth_params_keys[] = {
> + [RTE_ETH_PARAMS_MAX] = NULL,
> +};
> +
> +static int
> +eth_dev_match(struct rte_eth_dev *edev,
> + struct rte_kvargs *kvlist)
> +{
> + (void) kvlist;
> + (void) edev;
> + return 0;
> +}
> +
> +static void *
> +eth_dev_iterate(const void *_start,
> + const char *str,
> + const struct rte_dev_iterator *it)
> +{
> + const struct rte_eth_dev *start = _start;
> + struct rte_device *dev = it->device;
> + struct rte_kvargs *kvargs = NULL;
> + struct rte_eth_dev *edev = NULL;
> + uint16_t p = 0;
> +
> + if (str != NULL) {
> + kvargs = rte_kvargs_parse(str, eth_params_keys);
> + if (kvargs == NULL) {
> + RTE_LOG(ERR, EAL, "cannot parse argument list\n");
> + rte_errno = EINVAL;
> + return NULL;
> + }
> + }
> + if (start)
> + p = start->data->port_id + 1;
> + for (p = rte_eth_find_next(p);
> + p < RTE_MAX_ETHPORTS;
> + p = rte_eth_find_next(p + 1)) {
What are about differed\owned\bonded devices?
What is actually the use cases of this iterator?
DPDK internal management or applications?
> + edev = &rte_eth_devices[p];
> + if (dev != edev->device)
> + goto next_ethdev;
> + if (eth_dev_match(edev, kvargs) == 0)
> + break;
> +next_ethdev:
> + edev = NULL;
> + }
> + rte_kvargs_free(kvargs);
> + return edev;
> +}
> +
> +struct rte_class rte_class_eth = {
> + .dev_iterate = eth_dev_iterate,
> +};
> +
> +RTE_REGISTER_CLASS(eth, rte_class_eth);
> --
> 2.11.0
Re: [dpdk-dev] [PATCH v4 18/20] ethdev: register ether layer as a class
Hi Matan,
On Mon, Apr 09, 2018 at 07:41:58AM +, Matan Azrad wrote:
> Hi Gaetan
>
> From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > Signed-off-by: Gaetan Rivet
> > ---
> > lib/Makefile | 2 +-
> > lib/librte_ether/Makefile| 3 +-
> > lib/librte_ether/rte_class_eth.c | 73
> >
> > 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644
> > lib/librte_ether/rte_class_eth.c
> >
> > diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> > librte_cmdline DEPDIRS-librte_cmdline := librte_eal
> > DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether DEPDIRS-librte_ether
> > := librte_net librte_eal librte_mempool librte_ring -DEPDIRS-librte_ether +=
> > librte_mbuf
> > +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> > DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev DEPDIRS-
> > librte_bbdev := librte_eal librte_mempool librte_mbuf
> > DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> > a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> > 3ca5782bb..a1a0393de 100644
> > --- a/lib/librte_ether/Makefile
> > +++ b/lib/librte_ether/Makefile
> > @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API CFLAGS
> > += -O3 CFLAGS += $(WERROR_FLAGS) LDLIBS += -lrte_net -lrte_eal -
> > lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> > +LDLIBS += -lrte_mbuf -lrte_kvargs
> >
> > EXPORT_MAP := rte_ethdev_version.map
> >
> > LIBABIVER := 8
> >
> > SRCS-y += rte_ethdev.c
> > +SRCS-y += rte_class_eth.c
> > SRCS-y += rte_flow.c
> > SRCS-y += rte_tm.c
> > SRCS-y += rte_mtr.c
> > diff --git a/lib/librte_ether/rte_class_eth.c
> > b/lib/librte_ether/rte_class_eth.c
> > new file mode 100644
> > index 0..97d24781d
> > --- /dev/null
> > +++ b/lib/librte_ether/rte_class_eth.c
> > @@ -0,0 +1,73 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2018 Gaëtan Rivet
> > + */
> > +
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "rte_ethdev.h"
> > +#include "rte_ethdev_core.h"
> > +
> > +enum eth_params {
> > + RTE_ETH_PARAMS_MAX,
> > +};
> > +
> > +static const char *eth_params_keys[] = {
> > + [RTE_ETH_PARAMS_MAX] = NULL,
> > +};
> > +
> > +static int
> > +eth_dev_match(struct rte_eth_dev *edev,
> > + struct rte_kvargs *kvlist)
> > +{
> > + (void) kvlist;
> > + (void) edev;
> > + return 0;
> > +}
> > +
> > +static void *
> > +eth_dev_iterate(const void *_start,
> > + const char *str,
> > + const struct rte_dev_iterator *it)
> > +{
> > + const struct rte_eth_dev *start = _start;
> > + struct rte_device *dev = it->device;
> > + struct rte_kvargs *kvargs = NULL;
> > + struct rte_eth_dev *edev = NULL;
> > + uint16_t p = 0;
> > +
> > + if (str != NULL) {
> > + kvargs = rte_kvargs_parse(str, eth_params_keys);
> > + if (kvargs == NULL) {
> > + RTE_LOG(ERR, EAL, "cannot parse argument list\n");
> > + rte_errno = EINVAL;
> > + return NULL;
> > + }
> > + }
> > + if (start)
> > + p = start->data->port_id + 1;
> > + for (p = rte_eth_find_next(p);
> > +p < RTE_MAX_ETHPORTS;
> > +p = rte_eth_find_next(p + 1)) {
>
> What are about differed\owned\bonded devices?
> What is actually the use cases of this iterator?
> DPDK internal management or applications?
>
This API is public, so anyone can use it.
It should be primarily used by internal systems however.
I do not think Applications would be interested in calling the
dev_iterate ops.
The EAL would provide a way to list interfaces from each layers. Then
DPDK libraries (such as librte_ether) would translate this following
their internal rules. librte_ether would thus for example here translate
an rte_eth_dev handle to a port_id, that an application could use. If
there was a need to filter by owner_id, then it would be the job of
librte_ether.
The EAL cannot keep track of layers specifics, it can only provide
generic APIs.
> > + edev = &rte_eth_devices[p];
> > + if (dev != edev->device)
> > + goto next_ethdev;
> > + if (eth_dev_match(edev, kvargs) == 0)
> > + break;
> > +next_ethdev:
> > + edev = NULL;
> > + }
> > + rte_kvargs_free(kvargs);
> > + return edev;
> > +}
> > +
> > +struct rte_class rte_class_eth = {
> > + .dev_iterate = eth_dev_iterate,
> > +};
> > +
> > +RTE_REGISTER_CLASS(eth, rte_class_eth);
> > --
> > 2.11.0
>
--
Gaëtan Rivet
6WIND
Re: [dpdk-dev] [PATCH v4 18/20] ethdev: register ether layer as a class
Hi Gaetan
From: Gaëtan Rivet, Monday, April 9, 2018 10:47 AM
> Hi Matan,
>
> On Mon, Apr 09, 2018 at 07:41:58AM +, Matan Azrad wrote:
> > Hi Gaetan
> >
> > From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > > Signed-off-by: Gaetan Rivet
> > > ---
> > > lib/Makefile | 2 +-
> > > lib/librte_ether/Makefile| 3 +-
> > > lib/librte_ether/rte_class_eth.c | 73
> > >
> > > 3 files changed, 76 insertions(+), 2 deletions(-) create mode
> > > 100644 lib/librte_ether/rte_class_eth.c
> > >
> > > diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f
> > > 100644
> > > --- a/lib/Makefile
> > > +++ b/lib/Makefile
> > > @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> > > librte_cmdline DEPDIRS-librte_cmdline := librte_eal
> > > DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
> > > DEPDIRS-librte_ether := librte_net librte_eal librte_mempool
> > > librte_ring -DEPDIRS-librte_ether += librte_mbuf
> > > +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> > > DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev DEPDIRS-
> > > librte_bbdev := librte_eal librte_mempool librte_mbuf
> > > DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> > > a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> > > 3ca5782bb..a1a0393de 100644
> > > --- a/lib/librte_ether/Makefile
> > > +++ b/lib/librte_ether/Makefile
> > > @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
> CFLAGS
> > > += -O3 CFLAGS += $(WERROR_FLAGS) LDLIBS += -lrte_net -lrte_eal -
> > > lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> > > +LDLIBS += -lrte_mbuf -lrte_kvargs
> > >
> > > EXPORT_MAP := rte_ethdev_version.map
> > >
> > > LIBABIVER := 8
> > >
> > > SRCS-y += rte_ethdev.c
> > > +SRCS-y += rte_class_eth.c
> > > SRCS-y += rte_flow.c
> > > SRCS-y += rte_tm.c
> > > SRCS-y += rte_mtr.c
> > > diff --git a/lib/librte_ether/rte_class_eth.c
> > > b/lib/librte_ether/rte_class_eth.c
> > > new file mode 100644
> > > index 0..97d24781d
> > > --- /dev/null
> > > +++ b/lib/librte_ether/rte_class_eth.c
> > > @@ -0,0 +1,73 @@
> > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > + * Copyright(c) 2018 Gaëtan Rivet
> > > + */
> > > +
> > > +#include
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#include "rte_ethdev.h"
> > > +#include "rte_ethdev_core.h"
> > > +
> > > +enum eth_params {
> > > + RTE_ETH_PARAMS_MAX,
> > > +};
> > > +
> > > +static const char *eth_params_keys[] = {
> > > + [RTE_ETH_PARAMS_MAX] = NULL,
> > > +};
> > > +
> > > +static int
> > > +eth_dev_match(struct rte_eth_dev *edev,
> > > + struct rte_kvargs *kvlist)
> > > +{
> > > + (void) kvlist;
> > > + (void) edev;
> > > + return 0;
> > > +}
> > > +
> > > +static void *
> > > +eth_dev_iterate(const void *_start,
> > > + const char *str,
> > > + const struct rte_dev_iterator *it) {
> > > + const struct rte_eth_dev *start = _start;
> > > + struct rte_device *dev = it->device;
> > > + struct rte_kvargs *kvargs = NULL;
> > > + struct rte_eth_dev *edev = NULL;
> > > + uint16_t p = 0;
> > > +
> > > + if (str != NULL) {
> > > + kvargs = rte_kvargs_parse(str, eth_params_keys);
> > > + if (kvargs == NULL) {
> > > + RTE_LOG(ERR, EAL, "cannot parse argument list\n");
> > > + rte_errno = EINVAL;
> > > + return NULL;
> > > + }
> > > + }
> > > + if (start)
> > > + p = start->data->port_id + 1;
> > > + for (p = rte_eth_find_next(p);
> > > + p < RTE_MAX_ETHPORTS;
> > > + p = rte_eth_find_next(p + 1)) {
> >
> > What are about differed\owned\bonded devices?
> > What is actually the use cases of this iterator?
> > DPDK internal management or applications?
> >
>
> This API is public, so anyone can use it.
> It should be primarily used by internal systems however.
> I do not think Applications would be interested in calling the dev_iterate
> ops.
>
> The EAL would provide a way to list interfaces from each layers. Then DPDK
> libraries (such as librte_ether) would translate this following their internal
> rules. librte_ether would thus for example here translate an rte_eth_dev
> handle to a port_id, that an application could use. If there was a need to
> filter
> by owner_id, then it would be the job of librte_ether.
>
> The EAL cannot keep track of layers specifics, it can only provide generic
> APIs.
This is exactly what I'm asking, what is the right behavior here?
Now, The code is skipping DEFFERED devices but iterating over owned\bonded
devices.
Looks like any USED port should be iterated...
Moreover, looks like there is chance to iterate over EAL device more than 1
time for loop, Is it OK?
> > > + edev = &rte_eth_devices[p];
> > > + if (dev != edev->device)
> > > + goto next_ethdev;
> > > + if (eth_dev_match(edev, kvargs) == 0)
> > > + brea
Re: [dpdk-dev] [PATCH v4 18/20] ethdev: register ether layer as a class
On Mon, Apr 09, 2018 at 07:58:08AM +, Matan Azrad wrote:
> Hi Gaetan
>
> From: Gaëtan Rivet, Monday, April 9, 2018 10:47 AM
> > Hi Matan,
> >
> > On Mon, Apr 09, 2018 at 07:41:58AM +, Matan Azrad wrote:
> > > Hi Gaetan
> > >
> > > From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > > > Signed-off-by: Gaetan Rivet
> > > > ---
> > > > lib/Makefile | 2 +-
> > > > lib/librte_ether/Makefile| 3 +-
> > > > lib/librte_ether/rte_class_eth.c | 73
> > > >
> > > > 3 files changed, 76 insertions(+), 2 deletions(-) create mode
> > > > 100644 lib/librte_ether/rte_class_eth.c
> > > >
> > > > diff --git a/lib/Makefile b/lib/Makefile index 4206485d3..47513f03f
> > > > 100644
> > > > --- a/lib/Makefile
> > > > +++ b/lib/Makefile
> > > > @@ -23,7 +23,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) +=
> > > > librte_cmdline DEPDIRS-librte_cmdline := librte_eal
> > > > DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
> > > > DEPDIRS-librte_ether := librte_net librte_eal librte_mempool
> > > > librte_ring -DEPDIRS-librte_ether += librte_mbuf
> > > > +DEPDIRS-librte_ether += librte_mbuf librte_kvargs
> > > > DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev DEPDIRS-
> > > > librte_bbdev := librte_eal librte_mempool librte_mbuf
> > > > DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev diff --git
> > > > a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile index
> > > > 3ca5782bb..a1a0393de 100644
> > > > --- a/lib/librte_ether/Makefile
> > > > +++ b/lib/librte_ether/Makefile
> > > > @@ -12,13 +12,14 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
> > CFLAGS
> > > > += -O3 CFLAGS += $(WERROR_FLAGS) LDLIBS += -lrte_net -lrte_eal -
> > > > lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf
> > > > +LDLIBS += -lrte_mbuf -lrte_kvargs
> > > >
> > > > EXPORT_MAP := rte_ethdev_version.map
> > > >
> > > > LIBABIVER := 8
> > > >
> > > > SRCS-y += rte_ethdev.c
> > > > +SRCS-y += rte_class_eth.c
> > > > SRCS-y += rte_flow.c
> > > > SRCS-y += rte_tm.c
> > > > SRCS-y += rte_mtr.c
> > > > diff --git a/lib/librte_ether/rte_class_eth.c
> > > > b/lib/librte_ether/rte_class_eth.c
> > > > new file mode 100644
> > > > index 0..97d24781d
> > > > --- /dev/null
> > > > +++ b/lib/librte_ether/rte_class_eth.c
> > > > @@ -0,0 +1,73 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > + * Copyright(c) 2018 Gaëtan Rivet
> > > > + */
> > > > +
> > > > +#include
> > > > +
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include "rte_ethdev.h"
> > > > +#include "rte_ethdev_core.h"
> > > > +
> > > > +enum eth_params {
> > > > + RTE_ETH_PARAMS_MAX,
> > > > +};
> > > > +
> > > > +static const char *eth_params_keys[] = {
> > > > + [RTE_ETH_PARAMS_MAX] = NULL,
> > > > +};
> > > > +
> > > > +static int
> > > > +eth_dev_match(struct rte_eth_dev *edev,
> > > > + struct rte_kvargs *kvlist)
> > > > +{
> > > > + (void) kvlist;
> > > > + (void) edev;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static void *
> > > > +eth_dev_iterate(const void *_start,
> > > > + const char *str,
> > > > + const struct rte_dev_iterator *it) {
> > > > + const struct rte_eth_dev *start = _start;
> > > > + struct rte_device *dev = it->device;
> > > > + struct rte_kvargs *kvargs = NULL;
> > > > + struct rte_eth_dev *edev = NULL;
> > > > + uint16_t p = 0;
> > > > +
> > > > + if (str != NULL) {
> > > > + kvargs = rte_kvargs_parse(str, eth_params_keys);
> > > > + if (kvargs == NULL) {
> > > > + RTE_LOG(ERR, EAL, "cannot parse argument
> > > > list\n");
> > > > + rte_errno = EINVAL;
> > > > + return NULL;
> > > > + }
> > > > + }
> > > > + if (start)
> > > > + p = start->data->port_id + 1;
> > > > + for (p = rte_eth_find_next(p);
> > > > +p < RTE_MAX_ETHPORTS;
> > > > +p = rte_eth_find_next(p + 1)) {
> > >
> > > What are about differed\owned\bonded devices?
> > > What is actually the use cases of this iterator?
> > > DPDK internal management or applications?
> > >
> >
> > This API is public, so anyone can use it.
> > It should be primarily used by internal systems however.
> > I do not think Applications would be interested in calling the dev_iterate
> > ops.
> >
> > The EAL would provide a way to list interfaces from each layers. Then DPDK
> > libraries (such as librte_ether) would translate this following their
> > internal
> > rules. librte_ether would thus for example here translate an rte_eth_dev
> > handle to a port_id, that an application could use. If there was a need to
> > filter
> > by owner_id, then it would be the job of librte_ether.
> >
> > The EAL cannot keep track of layers specifics, it can only provide generic
> > APIs.
>
> This is exact
Re: [dpdk-dev] [PATCH v4 10/20] eal/dev: implement device iteration
On Mon, Apr 09, 2018 at 07:28:00AM +, Matan Azrad wrote:
> HI Gaetan
>
> From: Gaetan Rivet, Friday, March 30, 2018 12:24 AM
> > +/* '\0' forbidden in sym */
> > +static const char *
> > +strfirstof(const char *str,
> > + const char *sym)
> > +{
> > + const char *s;
> > +
> > + for (s = str; s[0] != '\0'; s++) {
> > + const char *c;
> > +
> > + for (c = sym; c[0] != '\0'; c++) {
> > + if (c[0] == s[0])
> > + return s;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
>
> I think you can use strcspn() instead...
>
Indeed, didn't know about it, I will change it.
--
Gaëtan Rivet
6WIND
Re: [dpdk-dev] [PATCH] net/mlx5: fix link status initialization
On Sun, Apr 08, 2018 at 01:09:27PM +, Shahaf Shuler wrote: > Thursday, April 5, 2018 9:51 AM, Nélio Laranjeiro: > > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > > > On Thu, Apr 05, 2018 at 05:35:57AM +, Shahaf Shuler wrote: > > > Wednesday, April 4, 2018 3:11 PM, Nélio Laranjeiro: > > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > > > > > > > On Wed, Apr 04, 2018 at 09:58:33AM +, Shahaf Shuler wrote: > > > > > Wednesday, April 4, 2018 10:30 AM, Nélio Laranjeiro: > > > > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > > > > > > > > > > > On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote: > > [..] > > > > > > > > > According to your analysis, this is only necessary when the LCS is > > > > configured in the device. Why not adding this call to > > > > mlx5_dev_interrupt_handler_install() which is responsible to install > > > > the LCS callback. > > > > > > I think it is good practice whether or not LSC is set. > > > The link status should be initialized to the correct value after the > > > probe. > > > > There is no guarantee the link will be accurate, at probe time the link may > > be > > up so internal information has a status up with a speed with this patch. > > The application probes a second port, in the mean time the link of the first > > port goes down, the interrupt is still not installed and the internal status > > becomes wrong (still up whereas the port is down). > > > > Finally at start, the device installs the handler, but the link is still > > down > > whereas internally it is up, the application will call > > rte_eth_link_get_nowait() which will directly copy the wrong internal status > > to the application. > > This is not correct. > Using Verbs, the async_fd on which link status interrupts are reported is > created on probe. > Even if the interrupt handler is not installed, interrupts still > trigger on this fd. They will be processed when the interrupt handler > will be installed as part of the port start. > So in fact you have the whole trace on the link status changes waiting > to be processed upon port start. Right, but in such case, this patch still does not solves the issue. Until the dev_start() is called, the link may be inconsistent with the real status. example: pci_probe --> link is up. leaving pci probe, the link goes downs --> internally the PMD has a link up. Until the dev_start() is called any call to rte_eth_link_get_nowait() will copy the internal PMD status which is not accurate. >From this point, the issue seems to fully come from rte_eth_link_get_nowait() which should not make any copy until the port is not started, until then the link may be inconsistent between the driver and the device. Regards, -- Nélio Laranjeiro 6WIND
Re: [dpdk-dev] [PATCH] doc: fix a typo
> Signed-off-by: Jerin Jacob > --- > doc/guides/prog_guide/rawdev.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Marko Kovacevic
[dpdk-dev] [PATCH v2] net/i40e: fix flow RSS queue index check error
There is an error in queue index check for RSS queue region
configuration.If the queue index is not continuous sequence
for RSS, but queue region index is continuous sequence, in
this case we can not use the old method for queue index check.
This patch also add comment for flow rss parse function in order
to explain some important info.
Fixes: ecad87d22383 ("net/i40e: move RSS to flow API")
Signed-off-by: Wei Zhao
Tested-by: Peng Yuan
---
v2:
-merge this with another patch for function comment, add git log info.
---
drivers/net/i40e/i40e_flow.c | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index f4d08bb..40f15c2 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -4171,6 +4171,19 @@ i40e_flow_parse_rss_pattern(__rte_unused struct
rte_eth_dev *dev,
return 0;
}
+/**
+ * This function is used to parse rss queue index, total queue number and
+ * hash functions, If the purpose of this configuration is for queue region
+ * configuration, it will set queue_region_conf flag to TRUE, else to FALSE.
+ * In queue region configuration, it also need to parse hardware flowtype
+ * and user_priority from configuration, it will also cheeck the validity
+ * of these parameters. For example, The queue region sizes should
+ * be any of the following values: 1, 2, 4, 8, 16, 32, 64, the
+ * hw_flowtype or PCTYPE max index should be 63, the user priority
+ * max index should be 7, and so on. And also, queue index should be
+ * continuous sequence and queue region index should be part of rss
+ * queue index for this port.
+ */
static int
i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
const struct rte_flow_action *actions,
@@ -4240,6 +4253,14 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
return -rte_errno;
}
}
+
+ if (rss_info->num < rss->num) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ act,
+ "no valid queues");
+ return -rte_errno;
+ }
}
for (n = 0; n < conf_info->queue_region_number; n++) {
@@ -4264,17 +4285,6 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
return -rte_errno;
}
- if (rss_info->num < rss->num ||
- rss->queue[0] < rss_info->queue[0] ||
- (rss->queue[0] + rss->num >
- rss_info->num + rss_info->queue[0])) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act,
- "no valid queues");
- return -rte_errno;
- }
-
for (i = 0; i < info->queue_region_number; i++) {
if (info->region[i].queue_num == rss->num &&
info->region[i].queue_start_index ==
--
2.7.5
Re: [dpdk-dev] [PATCH] ethdev: return diagnostic when setting MAC address
On Fri, Apr 06, 2018 at 05:21:48PM +0200, Olivier Matz wrote:
> Change the prototype and the behavior of dev_ops->eth_mac_addr_set(): a
> return code is added to notify the caller (librte_ether) if an error
> occurred in the PMD.
>
> The new default MAC address is now copied in dev->data->mac_addrs[0]
> only if the operation is successful.
>
> The patch also updates all the PMDs accordingly.
>
> Signed-off-by: Olivier Matz
> Signed-off-by: Ivan Malov
> Acked-by: Andrew Rybchenko
> Acked-by: Adrien Mazarguil
> Acked-by: Shreyansh Jain
> ---
>
> v3:
> * mlx5: remove empty line as suggested by Adrien
> * mvpp2: remove wrong changes in mrvl_mac_addr_remove() as suggested
> by Ferruh
> * dpaa/dpaa2: fix return value as suggested by Shreyansh
>
> v2:
> * add same change for net/cxgbe
> * mrvl was renamed as mvpp2
> * mvpp2: return success if no ppio as suggested by Tomasz
> * mlx5: update comment as suggested by Adrien
> * sfc: replace by Ivan's patch
>
>
> doc/guides/rel_notes/deprecation.rst| 8
> drivers/net/ark/ark_ethdev.c| 9 ++---
> drivers/net/avf/avf_ethdev.c| 12 +++
> drivers/net/bnxt/bnxt_ethdev.c | 10 ++
> drivers/net/bonding/rte_eth_bond_pmd.c | 8 ++--
> drivers/net/cxgbe/cxgbe_ethdev.c| 5 +++--
> drivers/net/cxgbe/cxgbe_pfvf.h | 2 +-
> drivers/net/dpaa/dpaa_ethdev.c | 4 +++-
> drivers/net/dpaa2/dpaa2_ethdev.c| 6 --
> drivers/net/e1000/igb_ethdev.c | 12 ++-
> drivers/net/failsafe/failsafe_ops.c | 17 +---
> drivers/net/i40e/i40e_ethdev.c | 24 +-
> drivers/net/i40e/i40e_ethdev_vf.c | 12 ++-
> drivers/net/ixgbe/ixgbe_ethdev.c| 13 +++-
> drivers/net/mlx4/mlx4.h | 2 +-
> drivers/net/mlx4/mlx4_ethdev.c | 7 +--
> drivers/net/mlx5/mlx5.h | 2 +-
> drivers/net/mlx5/mlx5_mac.c | 6 +-
> drivers/net/mvpp2/mrvl_ethdev.c | 11 ---
> drivers/net/null/rte_eth_null.c | 3 ++-
> drivers/net/octeontx/octeontx_ethdev.c | 4 +++-
> drivers/net/qede/qede_ethdev.c | 7 +++
> drivers/net/sfc/sfc_ethdev.c| 35
> ++---
> drivers/net/szedata2/rte_eth_szedata2.c | 3 ++-
> drivers/net/tap/rte_eth_tap.c | 34 +---
> drivers/net/virtio/virtio_ethdev.c | 15 +-
> drivers/net/vmxnet3/vmxnet3_ethdev.c| 5 +++--
> lib/librte_ether/rte_ethdev.c | 7 +--
> lib/librte_ether/rte_ethdev_core.h | 2 +-
> test/test/virtual_pmd.c | 3 ++-
> 30 files changed, 184 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index faacfd9d6..738709854 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -195,7 +195,7 @@ int mlx5_get_mac(struct rte_eth_dev *dev, uint8_t
> (*mac)[ETHER_ADDR_LEN]);
> 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);
> -void mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr);
> +int mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr);
>
> /* mlx5_rss.c */
>
> diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
> index 01c7ba17a..6ca1cc4a1 100644
> --- a/drivers/net/mlx5/mlx5_mac.c
> +++ b/drivers/net/mlx5/mlx5_mac.c
> @@ -124,8 +124,11 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct
> ether_addr *mac,
> * Pointer to Ethernet device structure.
> * @param mac_addr
> * MAC address to register.
> + *
> + * @return
> + * 0 on success, a negative errno value otherwise and rte_errno is set.
> */
> -void
> +int
> mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
> {
> int ret;
> @@ -137,4 +140,5 @@ mlx5_mac_addr_set(struct rte_eth_dev *dev, struct
> ether_addr *mac_addr)
> if (ret)
> DRV_LOG(ERR, "port %u cannot set mac address: %s",
> dev->data->port_id, strerror(rte_errno));
> + return ret;
> }
You should also remove the DRV_LOG which become useless, it has been
added as the function did not return anything, now has it returns an
error, the application can fully handle it.
Unless that, for MLX5
Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
[dpdk-dev] [PATCH] net/vmxnet3: change the SPDX tag style
Cc: skh...@vmware.com Signed-off-by: Hemant Agrawal --- drivers/net/vmxnet3/base/upt1_defs.h| 7 ++- drivers/net/vmxnet3/base/vmxnet3_defs.h | 7 ++- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/net/vmxnet3/base/upt1_defs.h b/drivers/net/vmxnet3/base/upt1_defs.h index cf9141b..5fd7a39 100644 --- a/drivers/net/vmxnet3/base/upt1_defs.h +++ b/drivers/net/vmxnet3/base/upt1_defs.h @@ -1,9 +1,6 @@ -/* +/* SPDX-License-Identifier: BSD-3-Clause * Copyright (C) 2007 VMware, Inc. All rights reserved. - * - * SPDX-License-Identifier:BSD-3-Clause - * - */ + */ /* upt1_defs.h * diff --git a/drivers/net/vmxnet3/base/vmxnet3_defs.h b/drivers/net/vmxnet3/base/vmxnet3_defs.h index a455e27..a30b8f2 100644 --- a/drivers/net/vmxnet3/base/vmxnet3_defs.h +++ b/drivers/net/vmxnet3/base/vmxnet3_defs.h @@ -1,9 +1,6 @@ -/* +/* SPDX-License-Identifier: BSD-3-Clause * Copyright (C) 2007 VMware, Inc. All rights reserved. - * - * SPDX-License-Identifier:BSD-3-Clause - * - */ + */ /* * vmxnet3_defs.h -- -- 2.7.4
[dpdk-dev] [PATCH] test/crypto-perf: add missing SPDX identifier
Cc: pablo.de.lara.gua...@intel.com Signed-off-by: Hemant Agrawal --- app/test-crypto-perf/cperf_options.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-perf/cperf_options.h index 54a3ad5..350ad7e 100644 --- a/app/test-crypto-perf/cperf_options.h +++ b/app/test-crypto-perf/cperf_options.h @@ -1,3 +1,6 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2018 Intel Corporation + */ #ifndef _CPERF_OPTIONS_ #define _CPERF_OPTIONS_ -- 2.7.4
[dpdk-dev] [PATCH] eal: add missing SPDX identifiers
Cc: roman.dement...@intel.com Cc: vikto...@rehivetech.com Signed-off-by: Hemant Agrawal --- lib/librte_eal/common/include/arch/arm/rte_rwlock.h| 2 ++ lib/librte_eal/common/include/arch/ppc_64/rte_rwlock.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/librte_eal/common/include/arch/arm/rte_rwlock.h b/lib/librte_eal/common/include/arch/arm/rte_rwlock.h index 664bec8..18bb37b 100644 --- a/lib/librte_eal/common/include/arch/arm/rte_rwlock.h +++ b/lib/librte_eal/common/include/arch/arm/rte_rwlock.h @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: BSD-3-Clause + */ /* copied from ppc_64 */ #ifndef _RTE_RWLOCK_ARM_H_ diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_rwlock.h b/lib/librte_eal/common/include/arch/ppc_64/rte_rwlock.h index de8af19..9fadc04 100644 --- a/lib/librte_eal/common/include/arch/ppc_64/rte_rwlock.h +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_rwlock.h @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: BSD-3-Clause + */ #ifndef _RTE_RWLOCK_PPC_64_H_ #define _RTE_RWLOCK_PPC_64_H_ -- 2.7.4
[dpdk-dev] [PATCH] usertools: add missing SPDX identifier
CC: nhor...@tuxdriver.com Signed-off-by: Hemant Agrawal --- usertools/dpdk-pmdinfo.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/usertools/dpdk-pmdinfo.py b/usertools/dpdk-pmdinfo.py index 46c1be0..03623d5 100755 --- a/usertools/dpdk-pmdinfo.py +++ b/usertools/dpdk-pmdinfo.py @@ -1,4 +1,6 @@ #!/usr/bin/env python +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2016 Neil Horman # - # -- 2.7.4
[dpdk-dev] [PATCH] usertools: change to SPDX license identifier
Cc: jerin.ja...@caviumnetworks.com Signed-off-by: Hemant Agrawal --- usertools/cpu_layout.py | 36 +++- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py index d3c8eba..6f129b1 100755 --- a/usertools/cpu_layout.py +++ b/usertools/cpu_layout.py @@ -1,38 +1,8 @@ #!/usr/bin/env python +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2010-2014 Intel Corporation +# Copyright(c) 2017 Cavium, Inc. All rights reserved. -# -# BSD LICENSE -# -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. -# Copyright(c) 2017 Cavium, Inc. 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. -# from __future__ import print_function import sys try: -- 2.7.4
[dpdk-dev] [PATCH] pkg: change to SPDX license identifier
Cc: olivier.m...@6wind.com Cc: tho...@monjalon.net Signed-off-by: Hemant Agrawal --- pkg/dpdk.spec | 30 +- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/pkg/dpdk.spec b/pkg/dpdk.spec index 4d3b574..23dec98 100644 --- a/pkg/dpdk.spec +++ b/pkg/dpdk.spec @@ -1,33 +1,5 @@ +# SPDX-License-Identifier: BSD-3-Clause # Copyright 2014 6WIND S.A. -# -# 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 6WIND S.A. 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. Name: dpdk Version: 18.02 -- 2.7.4
[dpdk-dev] [PATCH] kernel: add missing SPDX license identifier
Cc: jm...@redhat.com Cc: step...@networkplumber.org Signed-off-by: Hemant Agrawal --- kernel/linux/igb_uio/compat.h | 1 + kernel/linux/kni/compat.h | 1 + 2 files changed, 2 insertions(+) diff --git a/kernel/linux/igb_uio/compat.h b/kernel/linux/igb_uio/compat.h index ce456d4..f9adc7d 100644 --- a/kernel/linux/igb_uio/compat.h +++ b/kernel/linux/igb_uio/compat.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Minimal wrappers to allow compiling igb_uio on older kernels. */ diff --git a/kernel/linux/kni/compat.h b/kernel/linux/kni/compat.h index 6a6968d..5aadebb 100644 --- a/kernel/linux/kni/compat.h +++ b/kernel/linux/kni/compat.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Minimal wrappers to allow compiling kni on older kernels. */ -- 2.7.4
Re: [dpdk-dev] [PATCH] usertools: change to SPDX license identifier
-Original Message- > Date: Mon, 9 Apr 2018 14:28:37 +0530 > From: Hemant Agrawal > To: dev@dpdk.org > CC: jerin.ja...@caviumnetworks.com, Hemant Agrawal > Subject: [PATCH] usertools: change to SPDX license identifier > X-Mailer: git-send-email 2.7.4 > > Cc: jerin.ja...@caviumnetworks.com > > Signed-off-by: Hemant Agrawal Acked-by: Jerin Jacob > --- > usertools/cpu_layout.py | 36 +++- > 1 file changed, 3 insertions(+), 33 deletions(-) > > diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py > index d3c8eba..6f129b1 100755 > --- a/usertools/cpu_layout.py > +++ b/usertools/cpu_layout.py > @@ -1,38 +1,8 @@ > #!/usr/bin/env python > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2010-2014 Intel Corporation > +# Copyright(c) 2017 Cavium, Inc. All rights reserved. > > -# > -# BSD LICENSE > -# > -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > -# Copyright(c) 2017 Cavium, Inc. 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. > -# > from __future__ import print_function > import sys > try: > -- > 2.7.4 >
Re: [dpdk-dev] [PATCH v4 66/70] bus/fslmc: enable support for mem event callbacks for vfio
Hi Anatoly,
On Monday 09 April 2018 01:48 AM, Anatoly Burakov wrote:
VFIO needs to map and unmap segments for DMA whenever they
become available or unavailable, so register a callback for
memory events, and provide map/unmap functions.
Signed-off-by: Shreyansh Jain
Signed-off-by: Anatoly Burakov
---
drivers/bus/fslmc/fslmc_vfio.c | 150 +
1 file changed, 136 insertions(+), 14 deletions(-)
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index db3eb61..dfdd2bc 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include "rte_fslmc.h"
#include "fslmc_vfio.h"
@@ -188,11 +189,57 @@ static int vfio_map_irq_region(struct fslmc_vfio_group
*group)
return -errno;
}
+static int fslmc_map_dma(uint64_t vaddr, rte_iova_t iovaddr, size_t len);
+static int fslmc_unmap_dma(uint64_t vaddr, rte_iova_t iovaddr, size_t len);
+
+static void
+fslmc_memevent_cb(enum rte_mem_event type, const void *addr, size_t len)
+{
+ struct rte_memseg_list *msl;
+ struct rte_memseg *ms;
+ size_t cur_len = 0, map_len = 0;
+ uint64_t virt_addr;
+ rte_iova_t iova_addr;
+ int ret;
+
+ msl = rte_mem_virt2memseg_list(addr);
+
+ while (cur_len < len) {
+ const void *va = RTE_PTR_ADD(addr, cur_len);
+
+ ms = rte_mem_virt2memseg(va, msl);
+ iova_addr = ms->iova;
+ virt_addr = ms->addr_64;
+ map_len = ms->len;
+
+ DPAA2_BUS_DEBUG("Calling with type=%d, va=%p, virt_addr=0x%" PRIx64 ",
iova=0x%" PRIx64 ", map_len=%zu\n",
I would like to correct this message (80char + rewording) - What do you
suggest? Should I send a new patch to you or just convey what should be
changed?
+ type, va, virt_addr, iova_addr, map_len);
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ ret = fslmc_map_dma(virt_addr, iova_addr, map_len);
+ else
+ ret = fslmc_unmap_dma(virt_addr, iova_addr, map_len);
+
+ if (ret != 0) {
+ DPAA2_BUS_ERR("DMA Mapping/Unmapping failed. Map=%d,
addr=%p, len=%zu, err:(%d)",
+ type, va, map_len, ret);
Same as above. 80 Char issue.
+ return;
+ }
+
+ cur_len += map_len;
+ }
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ DPAA2_BUS_DEBUG("Total Mapped: addr=%p, len=%zu\n",
+ addr, len);
+ else
+ DPAA2_BUS_DEBUG("Total Unmapped: addr=%p, len=%zu\n",
+ addr, len);
+}
+
static int
-fslmc_vfio_map(const struct rte_memseg_list *msl __rte_unused,
- const struct rte_memseg *ms, void *arg)
+fslmc_map_dma(uint64_t vaddr, rte_iova_t iovaddr __rte_unused, size_t len)
{
- int *n_segs = arg;
struct fslmc_vfio_group *group;
struct vfio_iommu_type1_dma_map dma_map = {
.argsz = sizeof(struct vfio_iommu_type1_dma_map),
@@ -200,10 +247,11 @@ fslmc_vfio_map(const struct rte_memseg_list *msl
__rte_unused,
};
int ret;
- dma_map.size = ms->len;
- dma_map.vaddr = ms->addr_64;
+ dma_map.size = len;
+ dma_map.vaddr = vaddr;
+
#ifdef RTE_LIBRTE_DPAA2_USE_PHYS_IOVA
- dma_map.iova = ms->iova;
+ dma_map.iova = iovaddr;
#else
dma_map.iova = dma_map.vaddr;
#endif
@@ -216,30 +264,99 @@ fslmc_vfio_map(const struct rte_memseg_list *msl
__rte_unused,
return -1;
}
- DPAA2_BUS_DEBUG("-->Initial SHM Virtual ADDR %llX",
- dma_map.vaddr);
- DPAA2_BUS_DEBUG("-> DMA size 0x%llX", dma_map.size);
- ret = ioctl(group->container->fd, VFIO_IOMMU_MAP_DMA,
- &dma_map);
+ DPAA2_BUS_DEBUG("--> Map address: %llX, size: 0x%llX\n",
+ dma_map.vaddr, dma_map.size);
+ ret = ioctl(group->container->fd, VFIO_IOMMU_MAP_DMA, &dma_map);
if (ret) {
DPAA2_BUS_ERR("VFIO_IOMMU_MAP_DMA API(errno = %d)",
errno);
return -1;
}
- (*n_segs)++;
+
return 0;
}
+static int
+fslmc_unmap_dma(uint64_t vaddr, uint64_t iovaddr __rte_unused, size_t len)
+{
+ struct fslmc_vfio_group *group;
+ struct vfio_iommu_type1_dma_unmap dma_unmap = {
+ .argsz = sizeof(struct vfio_iommu_type1_dma_unmap),
+ .flags = 0,
+ };
+ int ret;
+
+ dma_unmap.size = len;
+ dma_unmap.iova = vaddr;
+
+ /* SET DMA MAP for IOMMU */
+ group = &vfio_group;
+
+ if (!group->container) {
+ DPAA2_BUS_ERR("Container is not connected ");
+ return -1;
+ }
+
+ DPAA2_BUS_DEBUG(
[dpdk-dev] [dpdk-announce] 2018 DPDK PRC Summit --- Beijing Jun 28th----Call for proposal
2018 DPDK PRC Summit will take place in the China National Convention Center, Beijing on June 28th. The agenda will cover the latest developments to the DPDK and other related projects such as FD.io, Lagopus, OVS, DPVS, Tungsten Fabric and SPDK, including plans for future releases, and will provide an opportunity to hear from DPDK users who have used it in their applications. Submissions must be received by 11:59pm on 1 May 2018. There are five tracks for the submissions: Cloud, Comms/NFVi/SDN, SmartNIC/NIC, Platform and Others. You can submit your abstract on the link https://www.surveymonkey.com/r/DZGCXTC, thanks.
[dpdk-dev] [dpdk-announce] Date for DPDK Summit Userspace
This year's Userspace event will take place on September 5th and 6th. The venue will be the same as for the last 2 years - The Clayton Hotel, Ballsbridge, Dublin (https://www.claytonhotelballsbridge.com/). Details on registration and CFP will be provided closer to the time. For now, I just wanted to confirm the date so that those planning to attend can start to make the necessary arrangements.
Re: [dpdk-dev] [PATCH] usertools: add missing SPDX identifier
On Mon, Apr 09, 2018 at 02:28:36PM +0530, Hemant Agrawal wrote: > CC: nhor...@tuxdriver.com > Signed-off-by: Hemant Agrawal > --- > usertools/dpdk-pmdinfo.py | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/usertools/dpdk-pmdinfo.py b/usertools/dpdk-pmdinfo.py > index 46c1be0..03623d5 100755 > --- a/usertools/dpdk-pmdinfo.py > +++ b/usertools/dpdk-pmdinfo.py > @@ -1,4 +1,6 @@ > #!/usr/bin/env python > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2016 Neil Horman > > # - > # > -- > 2.7.4 > > Acked-by: Neil Horman
[dpdk-dev] [PATCH v2 3/8] net/dpaa: fix the array overrun
Fixes: 62f53995caaf ("net/dpaa: add frame count based tail drop with CGR")
Coverity issue: 268342
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
Acked-By: Shreyansh Jain
---
drivers/net/dpaa/dpaa_ethdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index db49364..0aad111 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1105,10 +1105,10 @@ dpaa_dev_init(struct rte_eth_dev *eth_dev)
dpaa_push_mode_max_queue = DPAA_MAX_PUSH_MODE_QUEUE;
}
- /* Each device can not have more than DPAA_PCD_FQID_MULTIPLIER RX
+ /* Each device can not have more than DPAA_MAX_NUM_PCD_QUEUES RX
* queues.
*/
- if (num_rx_fqs <= 0 || num_rx_fqs > DPAA_PCD_FQID_MULTIPLIER) {
+ if (num_rx_fqs <= 0 || num_rx_fqs > DPAA_MAX_NUM_PCD_QUEUES) {
DPAA_PMD_ERR("Invalid number of RX queues\n");
return -EINVAL;
}
--
2.7.4
[dpdk-dev] [PATCH v2 2/8] bus/dpaa: fix the unchecked return value
From: Sunil Kumar Kori
Fixes: 5d944582d028 ("bus/dpaa: check portal presence in the caller function")
Coverity issue: 268323
Cc: sta...@dpdk.org
Signed-off-by: Sunil Kumar Kori
Acked-by: Hemant Agrawal
---
drivers/bus/dpaa/dpaa_bus.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 3535da5..ffc90a7 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -308,9 +308,15 @@ rte_dpaa_portal_fq_init(void *arg, struct qman_fq *fq)
/* Affine above created portal with channel*/
u32 sdqcr;
struct qman_portal *qp;
+ int ret;
- if (unlikely(!RTE_PER_LCORE(dpaa_io)))
- rte_dpaa_portal_init(arg);
+ if (unlikely(!RTE_PER_LCORE(dpaa_io))) {
+ ret = rte_dpaa_portal_init(arg);
+ if (ret < 0) {
+ DPAA_BUS_LOG(ERR, "portal initialization failure");
+ return ret;
+ }
+ }
/* Initialise qman specific portals */
qp = fsl_qman_portal_create();
--
2.7.4
[dpdk-dev] [PATCH v2 1/8] bus/dpaa: fix the resource leak issue
From: Sunil Kumar Kori
Fixes: 9d32ef0f5d61 ("bus/dpaa: support creating dynamic HW portal")
Coverity issue: 268332
Cc: sta...@dpdk.org
Signed-off-by: Sunil Kumar Kori
Acked-by: Hemant Agrawal
---
drivers/bus/dpaa/base/qbman/qman_driver.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/bus/dpaa/base/qbman/qman_driver.c
b/drivers/bus/dpaa/base/qbman/qman_driver.c
index 66838d2..07b29d5 100644
--- a/drivers/bus/dpaa/base/qbman/qman_driver.c
+++ b/drivers/bus/dpaa/base/qbman/qman_driver.c
@@ -160,6 +160,7 @@ struct qman_portal *fsl_qman_portal_create(void)
&cpuset);
if (ret) {
error(0, ret, "pthread_getaffinity_np()");
+ kfree(q_pcfg);
return NULL;
}
@@ -168,12 +169,14 @@ struct qman_portal *fsl_qman_portal_create(void)
if (CPU_ISSET(loop, &cpuset)) {
if (q_pcfg->cpu != -1) {
pr_err("Thread is not affine to 1 cpu\n");
+ kfree(q_pcfg);
return NULL;
}
q_pcfg->cpu = loop;
}
if (q_pcfg->cpu == -1) {
pr_err("Bug in getaffinity handling!\n");
+ kfree(q_pcfg);
return NULL;
}
@@ -183,6 +186,7 @@ struct qman_portal *fsl_qman_portal_create(void)
ret = process_portal_map(&q_map);
if (ret) {
error(0, ret, "process_portal_map()");
+ kfree(q_pcfg);
return NULL;
}
q_pcfg->channel = q_map.channel;
@@ -217,6 +221,7 @@ struct qman_portal *fsl_qman_portal_create(void)
close(q_fd);
err1:
process_portal_unmap(&q_map.addr);
+ kfree(q_pcfg);
return NULL;
}
--
2.7.4
[dpdk-dev] [PATCH v2 5/8] bus/dpaa: fix resource leak
Fixes: 1459585888b5 ("bus/dpaa: fix memory allocation during scan")
Coverity issue: 268337
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
Acked-By: Shreyansh Jain
---
drivers/bus/dpaa/base/fman/fman.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bus/dpaa/base/fman/fman.c
b/drivers/bus/dpaa/base/fman/fman.c
index e6fd5f3..be91da4 100644
--- a/drivers/bus/dpaa/base/fman/fman.c
+++ b/drivers/bus/dpaa/base/fman/fman.c
@@ -442,6 +442,7 @@ fman_if_init(const struct device_node *dpa_node)
if (!pool_node) {
FMAN_ERR(-ENXIO, "%s: bad fsl,bman-buffer-pools\n",
dname);
+ free(bpool);
goto err;
}
pname = pool_node->full_name;
@@ -449,6 +450,7 @@ fman_if_init(const struct device_node *dpa_node)
prop = of_get_property(pool_node, "fsl,bpid", &proplen);
if (!prop) {
FMAN_ERR(-EINVAL, "%s: no fsl,bpid\n", pname);
+ free(bpool);
goto err;
}
assert(proplen == sizeof(*prop));
--
2.7.4
[dpdk-dev] [PATCH v2 7/8] net/dpaa2: fix the implementation of xstats
Fixes: 1d6329b2fc1f ("net/dpaa2: support extra stats")
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
---
drivers/net/dpaa2/dpaa2_ethdev.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 281483d..eed6dc9 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -1115,12 +1115,12 @@ dpaa2_dev_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
union dpni_statistics value[3] = {};
unsigned int i = 0, num = RTE_DIM(dpaa2_xstats_strings);
- if (xstats == NULL)
- return 0;
-
if (n < num)
return num;
+ if (xstats == NULL)
+ return 0;
+
/* Get Counters from page_0*/
retcode = dpni_get_statistics(dpni, CMD_PRI_LOW, priv->token,
0, 0, &value[0]);
@@ -1153,10 +1153,13 @@ dpaa2_dev_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
static int
dpaa2_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names,
- __rte_unused unsigned int limit)
+ unsigned int limit)
{
unsigned int i, stat_cnt = RTE_DIM(dpaa2_xstats_strings);
+ if (limit < stat_cnt)
+ return stat_cnt;
+
if (xstats_names != NULL)
for (i = 0; i < stat_cnt; i++)
snprintf(xstats_names[i].name,
--
2.7.4
[dpdk-dev] [PATCH v2 4/8] net/dpaa: fix the oob access
Fixes: b21ed3e2a16d ("net/dpaa: support extended statistics")
Coverity issue: 268318
Cc: sta...@dpdk.org
Signed-off-by: Hemant Agrawal
Acked-By: Shreyansh Jain
---
drivers/net/dpaa/dpaa_ethdev.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 0aad111..581e3a0 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -335,10 +335,13 @@ dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
static int
dpaa_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names,
- __rte_unused unsigned int limit)
+ unsigned int limit)
{
unsigned int i, stat_cnt = RTE_DIM(dpaa_xstats_strings);
+ if (limit < stat_cnt)
+ return stat_cnt;
+
if (xstats_names != NULL)
for (i = 0; i < stat_cnt; i++)
snprintf(xstats_names[i].name,
@@ -366,7 +369,7 @@ dpaa_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
return 0;
fman_if_stats_get_all(dpaa_intf->fif, values_copy,
- sizeof(struct dpaa_if_stats));
+ sizeof(struct dpaa_if_stats) / 8);
for (i = 0; i < stat_cnt; i++)
values[i] =
--
2.7.4
[dpdk-dev] [PATCH v2 6/8] net/dpaa: update checksum for external pool obj
From: Akhil Goyal Signed-off-by: Akhil Goyal --- drivers/net/dpaa/dpaa_rxtx.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/dpaa/dpaa_rxtx.c b/drivers/net/dpaa/dpaa_rxtx.c index bdb7f66..1316d2a 100644 --- a/drivers/net/dpaa/dpaa_rxtx.c +++ b/drivers/net/dpaa/dpaa_rxtx.c @@ -825,6 +825,8 @@ tx_on_external_pool(struct qman_fq *txq, struct rte_mbuf *mbuf, } DPAA_MBUF_TO_CONTIG_FD(dmable_mbuf, fd_arr, dpaa_intf->bp_info->bpid); + if (mbuf->ol_flags & DPAA_TX_CKSUM_OFFLOAD_MASK) + dpaa_unsegmented_checksum(mbuf, fd_arr); return 0; } -- 2.7.4
[dpdk-dev] [PATCH v2 8/8] bus/fslmc: configure separate portal for Ethernet Rx
From: Nipun Gupta
In case of Receive from Ethernet we add a new pull request (prefetch)
but do not fetch the results from that pull request until next
dequeue operation. This keeps the portal in busy mode.
This patch updates the portals bifurcation to have separate portals
to receive packets for Ethernet and all other devices to use a
common portal.
Signed-off-by: Nipun Gupta
---
drivers/bus/fslmc/portal/dpaa2_hw_dpio.c| 27 ++-
drivers/bus/fslmc/portal/dpaa2_hw_dpio.h| 8
drivers/bus/fslmc/rte_bus_fslmc_version.map | 8 +++-
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 12 ++--
drivers/net/dpaa2/dpaa2_rxtx.c | 29 -
5 files changed, 47 insertions(+), 37 deletions(-)
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 881dd5f..a741626 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -350,7 +350,7 @@ dpaa2_affine_qbman_swp(void)
}
int
-dpaa2_affine_qbman_swp_sec(void)
+dpaa2_affine_qbman_ethrx_swp(void)
{
unsigned int lcore_id = rte_lcore_id();
uint64_t tid = syscall(SYS_gettid);
@@ -361,35 +361,36 @@ dpaa2_affine_qbman_swp_sec(void)
else if (lcore_id >= RTE_MAX_LCORE)
return -1;
- if (dpaa2_io_portal[lcore_id].sec_dpio_dev) {
+ if (dpaa2_io_portal[lcore_id].ethrx_dpio_dev) {
DPAA2_BUS_DP_INFO(
"DPAA Portal=%p (%d) is being shared between thread"
" %" PRIu64 " and current %" PRIu64 "\n",
- dpaa2_io_portal[lcore_id].sec_dpio_dev,
- dpaa2_io_portal[lcore_id].sec_dpio_dev->index,
+ dpaa2_io_portal[lcore_id].ethrx_dpio_dev,
+ dpaa2_io_portal[lcore_id].ethrx_dpio_dev->index,
dpaa2_io_portal[lcore_id].sec_tid,
tid);
- RTE_PER_LCORE(_dpaa2_io).sec_dpio_dev
- = dpaa2_io_portal[lcore_id].sec_dpio_dev;
+ RTE_PER_LCORE(_dpaa2_io).ethrx_dpio_dev
+ = dpaa2_io_portal[lcore_id].ethrx_dpio_dev;
rte_atomic16_inc(&dpaa2_io_portal
-[lcore_id].sec_dpio_dev->ref_count);
+[lcore_id].ethrx_dpio_dev->ref_count);
dpaa2_io_portal[lcore_id].sec_tid = tid;
DPAA2_BUS_DP_DEBUG(
"Old Portal=%p (%d) affined thread"
" - %" PRIu64 "\n",
- dpaa2_io_portal[lcore_id].sec_dpio_dev,
- dpaa2_io_portal[lcore_id].sec_dpio_dev->index,
+ dpaa2_io_portal[lcore_id].ethrx_dpio_dev,
+ dpaa2_io_portal[lcore_id].ethrx_dpio_dev->index,
tid);
return 0;
}
/* Populate the dpaa2_io_portal structure */
- dpaa2_io_portal[lcore_id].sec_dpio_dev = dpaa2_get_qbman_swp(lcore_id);
+ dpaa2_io_portal[lcore_id].ethrx_dpio_dev =
+ dpaa2_get_qbman_swp(lcore_id);
- if (dpaa2_io_portal[lcore_id].sec_dpio_dev) {
- RTE_PER_LCORE(_dpaa2_io).sec_dpio_dev
- = dpaa2_io_portal[lcore_id].sec_dpio_dev;
+ if (dpaa2_io_portal[lcore_id].ethrx_dpio_dev) {
+ RTE_PER_LCORE(_dpaa2_io).ethrx_dpio_dev
+ = dpaa2_io_portal[lcore_id].ethrx_dpio_dev;
dpaa2_io_portal[lcore_id].sec_tid = tid;
return 0;
} else {
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.h
b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.h
index c0bd878..d593eea 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.h
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.h
@@ -13,7 +13,7 @@
struct dpaa2_io_portal_t {
struct dpaa2_dpio_dev *dpio_dev;
- struct dpaa2_dpio_dev *sec_dpio_dev;
+ struct dpaa2_dpio_dev *ethrx_dpio_dev;
uint64_t net_tid;
uint64_t sec_tid;
void *eventdev;
@@ -25,8 +25,8 @@ RTE_DECLARE_PER_LCORE(struct dpaa2_io_portal_t, _dpaa2_io);
#define DPAA2_PER_LCORE_DPIO RTE_PER_LCORE(_dpaa2_io).dpio_dev
#define DPAA2_PER_LCORE_PORTAL DPAA2_PER_LCORE_DPIO->sw_portal
-#define DPAA2_PER_LCORE_SEC_DPIO RTE_PER_LCORE(_dpaa2_io).sec_dpio_dev
-#define DPAA2_PER_LCORE_SEC_PORTAL DPAA2_PER_LCORE_SEC_DPIO->sw_portal
+#define DPAA2_PER_LCORE_ETHRX_DPIO RTE_PER_LCORE(_dpaa2_io).ethrx_dpio_dev
+#define DPAA2_PER_LCORE_ETHRX_PORTAL DPAA2_PER_LCORE_ETHRX_DPIO->sw_portal
/* Variable to store DPAA2 platform type */
extern uint32_t dpaa2_svr_family;
@@ -39,7 +39,7 @@ struct dpaa2_dpio_dev *dpaa2_get_qbman_swp(int cpu_id);
int dpaa2_affine_qbman_swp(void);
/* Affine additional DPIO portal to current crypto processing thread */
-int dpaa2_affine_qbman_swp_sec(void);
+int dpaa2_affine_qbman_ethrx_swp(void);
Re: [dpdk-dev] [PATCH v4 44/70] net/mlx5: use virt2memseg instead of iteration
On Monday 09 April 2018 01:48 AM, Anatoly Burakov wrote:
Reduce dependency on internal details of EAL memory subsystem, and
simplify code.
Signed-off-by: Anatoly Burakov
---
drivers/net/mlx5/mlx5_mr.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index 2bf1f9c..d8c04dc 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -234,7 +234,7 @@ struct mlx5_mr *
mlx5_mr_new(struct rte_eth_dev *dev, struct rte_mempool *mp)
{
struct priv *priv = dev->data->dev_private;
- const struct rte_memseg *ms = rte_eal_get_physmem_layout();
+ const struct rte_memseg *ms;
uintptr_t start;
uintptr_t end;
unsigned int i;
Unused variable 'i' to be removed.
Thanks,
Gowrishankar
@@ -261,17 +261,15 @@ mlx5_mr_new(struct rte_eth_dev *dev, struct rte_mempool
*mp)
/* Save original addresses for exact MR lookup. */
mr->start = start;
mr->end = end;
+
/* Round start and end to page boundary if found in memory segments. */
- for (i = 0; (i < RTE_MAX_MEMSEG) && (ms[i].addr != NULL); ++i) {
- uintptr_t addr = (uintptr_t)ms[i].addr;
- size_t len = ms[i].len;
- unsigned int align = ms[i].hugepage_sz;
+ ms = rte_mem_virt2memseg((void *)start);
+ if (ms != NULL)
+ start = RTE_ALIGN_FLOOR(start, ms->hugepage_sz);
+ ms = rte_mem_virt2memseg((void *)end);
+ if (ms != NULL)
+ end = RTE_ALIGN_CEIL(end, ms->hugepage_sz);
- if ((start > addr) && (start < addr + len))
- start = RTE_ALIGN_FLOOR(start, align);
- if ((end > addr) && (end < addr + len))
- end = RTE_ALIGN_CEIL(end, align);
- }
DRV_LOG(DEBUG,
"port %u mempool %p using start=%p end=%p size=%zu for memory"
" region",
Re: [dpdk-dev] [PATCH v4 66/70] bus/fslmc: enable support for mem event callbacks for vfio
On 09-Apr-18 11:01 AM, Shreyansh Jain wrote:
Hi Anatoly,
On Monday 09 April 2018 01:48 AM, Anatoly Burakov wrote:
VFIO needs to map and unmap segments for DMA whenever they
become available or unavailable, so register a callback for
memory events, and provide map/unmap functions.
Signed-off-by: Shreyansh Jain
Signed-off-by: Anatoly Burakov
---
<...>
+ DPAA2_BUS_DEBUG("Calling with type=%d, va=%p, virt_addr=0x%"
PRIx64 ", iova=0x%" PRIx64 ", map_len=%zu\n",
I would like to correct this message (80char + rewording) - What do you
suggest? Should I send a new patch to you or just convey what should be
changed?
As far as i know, leaving strings on single line is good for grepping.
However, perhaps having PRIx64 etc in there breaks it anyway.
+ type, va, virt_addr, iova_addr, map_len);
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ ret = fslmc_map_dma(virt_addr, iova_addr, map_len);
+ else
+ ret = fslmc_unmap_dma(virt_addr, iova_addr, map_len);
+
+ if (ret != 0) {
+ DPAA2_BUS_ERR("DMA Mapping/Unmapping failed. Map=%d,
addr=%p, len=%zu, err:(%d)",
+ type, va, map_len, ret);
Same as above. 80 Char issue.
Same reasoning - leaving strings unbroken allows for easier grepping the
codebase, but i'm not sure what's our policy on having formatted strings
unbroken.
+ return;
+ }
+
+ cur_len += map_len;
+ }
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ DPAA2_BUS_DEBUG("Total Mapped: addr=%p, len=%zu\n",
+ addr, len);
+ else
<...>
+ ret = rte_mem_event_callback_register("fslmc_memevent_clb",
+ fslmc_memevent_cb);
+ if (ret && rte_errno == ENOTSUP)
+ DPAA2_BUS_DEBUG("Memory event callbacks not supported");
+ else if (ret)
+ DPAA2_BUS_DEBUG("Unable to install memory handler");
+ else
+ DPAA2_BUS_DEBUG("Installed memory callback handler");
/* Verifying that at least single segment is available */
if (i <= 0) {
+ /* TODO: Is this verification required any more? What would
+ * happen to non-legacy case where nothing was preallocated
+ * thus causing i==0?
+ */
And this as well - if call backs are not going to appear in case of
legacy, this needs to be removed.
Callbacks aren't only not going to appear in legacy mode - they will
also not appear on FreeBSD. We check this above, with checking rte_errno
value (if callbacks are not supported, it's set to ENOTSUP, and having
callbacks unsupported is not an error).
let me know how do you want to take these changes.
Now that i think of it, this error condition is wrong. This is called in
both legacy and non-legacy mode. This is bus probe, no? For non-legacy
mode, it is entirely possible to start without any memory whatsoever. It
just so happens that rte_service API allocates some on init, and hence
you always have at least one segment - that may not be the case forever.
So, non-legacy mode, not having memsegs is not an error, it is expected
behavior, so maybe we should remove this error check altogether.
DPAA2_BUS_ERR("No Segments found for VFIO Mapping");
+ rte_rwlock_read_unlock(mem_lock);
return -1;
}
DPAA2_BUS_DEBUG("Total %d segments found.", i);
@@ -250,6 +367,11 @@ int rte_fslmc_vfio_dmamap(void)
*/
vfio_map_irq_region(&vfio_group);
+ /* Existing segments have been mapped and memory callback for
hotplug
+ * has been installed.
+ */
+ rte_rwlock_read_unlock(mem_lock);
+
return 0;
}
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v3 00/10] add framework to load and execute BPF code
Hi Jerin, > > > > BPF is used quite intensively inside Linux (and BSD) kernels > > for various different purposes and proved to be extremely useful. > > > > BPF inside DPDK might also be used in a lot of places > > for a lot of similar things. > > As an example to: > > - packet filtering/tracing (aka tcpdump) > > - packet classification > > - statistics collection > > - HW/PMD live-system debugging/prototyping - trace HW descriptors, > > internal PMD SW state, etc. > > - Comeup with your own idea > > > > All of that in a dynamic, user-defined and extensible manner. > > > > So these series introduce new library - librte_bpf. > > librte_bpf provides API to load and execute BPF bytecode within > > user-space dpdk app. > > It supports basic set of features from eBPF spec. > > Also it introduces basic framework to load/unload BPF-based filters > > on eth devices (right now via SW RX/TX callbacks). > > > > How to try it: > > === > > > > 1) run testpmd as usual and start your favorite forwarding case. > > 2) build bpf program you'd like to load > > (you'll need clang v3.7 or above): > > $ cd test/bpf > > $ clang -O2 -target bpf -c t1.c > > > > 3) load bpf program(s): > > testpmd> bpf-load rx|tx > > > > : [-][J][M] > > J - use JIT generated native code, otherwise BPF interpreter will be used. > > M - assume input parameter is a pointer to rte_mbuf, > > otherwise assume it is a pointer to first segment's data. > > > > Few examples: > > > > # to load (not JITed) dummy.o at TX queue 0, port 0: > > testpmd> bpf-load tx 0 0 - ./dpdk.org/test/bpf/dummy.o > > #to load (and JIT compile) t1.o at RX queue 0, port 1: > > testpmd> bpf-load rx 1 0 J ./dpdk.org/test/bpf/t1.o > > > > #to load and JIT t3.o (note that it expects mbuf as an input): > > testpmd> bpf-load rx 2 0 JM ./dpdk.org/test/bpf/t3.o > > > > 4) observe changed traffic behavior > > Let say with the examples above: > > - dummy.o does literally nothing, so no changes should be here, > > except some possible slowdown. > > - t1.o - should force to drop all packets that doesn't match: > >'dst 1.2.3.4 && udp && dst port 5000' filter. > > - t3.o - should dump to stdout ARP packets. > > > > 5) unload some or all bpf programs: > > testpmd> bpf-unload tx 0 0 > > > > 6) continue with step 3) or exit > > > > Not currently supported features: > > = > > - cBPF > > - tail-pointer call > > - eBPF MAP > > - JIT for non X86_64 targets > > May be for next release, we are planning to add arm64 JIT support. Sounds great :) > Just wondering, How do you test all EBPF opcodes in JIT/Interpreter mode? > Are you planning to add any UT like linux kernel in dpdk ? or it was > similar to https://github.com/iovisor/ubpf/tree/master/tests ? I added UT for it in v3: http://dpdk.org/dev/patchwork/patch/37456/ But it doesn't cover whole ISA yet. In fact - that's what I am working right now - adding more test-cases to it, so hopefully by next release will have much better test coverage. Another thing I plan to add - harden validate() to catch more cases with Invalid code. Konstantin > > Just asking because, when we introduce arm64 JIT support similar > test cases should be required to verify the implementation.
[dpdk-dev] [PATCH 1/2] net/dpaa: Changes to support ethdev offload APIs
Signed-off-by: Sunil Kumar Kori
---
drivers/net/dpaa/dpaa_ethdev.c | 46 ++
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index db49364..22eb070 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -95,6 +95,9 @@ static const struct rte_dpaa_xstats_name_off
dpaa_xstats_strings[] = {
static struct rte_dpaa_driver rte_dpaa_pmd;
+static void
+dpaa_eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info);
+
static inline void
dpaa_poll_queue_default_config(struct qm_mcc_initfq *opts)
{
@@ -134,13 +137,42 @@ dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
}
static int
-dpaa_eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
+dpaa_eth_dev_configure(struct rte_eth_dev *dev)
{
struct dpaa_if *dpaa_intf = dev->data->dev_private;
+ struct rte_eth_conf *eth_conf = &dev->data->dev_conf;
+ struct rte_eth_dev_info dev_info;
+ uint64_t rx_offloads = eth_conf->rxmode.offloads;
+ uint64_t tx_offloads = eth_conf->txmode.offloads;
PMD_INIT_FUNC_TRACE();
- if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
+ dpaa_eth_dev_info(dev, &dev_info);
+ if (dev_info.rx_offload_capa != rx_offloads) {
+ DPAA_PMD_ERR("Some Rx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ rx_offloads, dev_info.rx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (dev_info.tx_offload_capa != tx_offloads) {
+ DPAA_PMD_ERR("Some Tx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ tx_offloads, dev_info.tx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (((rx_offloads & DEV_RX_OFFLOAD_IPV4_CKSUM) == 0) ||
+ ((rx_offloads & DEV_RX_OFFLOAD_UDP_CKSUM) == 0) ||
+ ((rx_offloads & DEV_RX_OFFLOAD_TCP_CKSUM) == 0) ||
+ ((tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM) == 0) ||
+ ((tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) == 0) ||
+ ((tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) == 0)) {
+ DPAA_PMD_ERR("Checksum offloading is enabled by default"
+ "Cannot be disabled. So ignoring this configuration");
+ }
+
+ if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
if (dev->data->dev_conf.rxmode.max_rx_pkt_len <=
DPAA_MAX_RX_PKT_LEN) {
fman_if_set_maxfrm(dpaa_intf->fif,
@@ -259,11 +291,17 @@ static void dpaa_eth_dev_info(struct rte_eth_dev *dev,
dev_info->rx_offload_capa =
(DEV_RX_OFFLOAD_IPV4_CKSUM |
DEV_RX_OFFLOAD_UDP_CKSUM |
- DEV_RX_OFFLOAD_TCP_CKSUM);
+ DEV_RX_OFFLOAD_TCP_CKSUM) |
+ DEV_RX_OFFLOAD_JUMBO_FRAME |
+ DEV_RX_OFFLOAD_SCATTER;
dev_info->tx_offload_capa =
(DEV_TX_OFFLOAD_IPV4_CKSUM |
DEV_TX_OFFLOAD_UDP_CKSUM |
- DEV_TX_OFFLOAD_TCP_CKSUM);
+ DEV_TX_OFFLOAD_TCP_CKSUM) |
+ DEV_TX_OFFLOAD_MBUF_FAST_FREE |
+ DEV_TX_OFFLOAD_MULTI_SEGS;
+
+ dev_info->default_rxconf.rx_drop_en = true;
}
static int dpaa_eth_link_update(struct rte_eth_dev *dev,
--
2.9.3
[dpdk-dev] [PATCH 0/2] Support for new Ethdev offload APIs
Patchset contains changes to support ethdev offload APIs for DPAA and DPAA2 drivers. Offloading support is categoriesed in following logical parts: 1. If requested offloading features is not supported then returned error. 2. If requested offloading feature is supoorted but cannot be disabled then request to disable the offload is silently discarded with a message. 3. Otherwise configuration is succesfully offloaded Sunil Kumar Kori (2): net/dpaa: Changes to support ethdev offload APIs net/dpaa2: Changes to support ethdev offload APIs drivers/net/dpaa/dpaa_ethdev.c | 46 ++--- drivers/net/dpaa2/dpaa2_ethdev.c | 63 +--- drivers/net/dpaa2/dpaa2_rxtx.c | 32 +++- 3 files changed, 105 insertions(+), 36 deletions(-) -- 2.9.3
[dpdk-dev] [PATCH 2/2] net/dpaa2: Changes to support ethdev offload APIs
Signed-off-by: Sunil Kumar Kori
---
drivers/net/dpaa2/dpaa2_ethdev.c | 63 +---
drivers/net/dpaa2/dpaa2_rxtx.c | 32 +++-
2 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 281483d..acf5f1a 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -172,16 +172,24 @@ dpaa2_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
DEV_RX_OFFLOAD_IPV4_CKSUM |
DEV_RX_OFFLOAD_UDP_CKSUM |
DEV_RX_OFFLOAD_TCP_CKSUM |
- DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
+ DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_VLAN_FILTER |
+ DEV_RX_OFFLOAD_VLAN_STRIP |
+ DEV_RX_OFFLOAD_JUMBO_FRAME |
+ DEV_RX_OFFLOAD_SCATTER;
dev_info->tx_offload_capa =
DEV_TX_OFFLOAD_IPV4_CKSUM |
DEV_TX_OFFLOAD_UDP_CKSUM |
DEV_TX_OFFLOAD_TCP_CKSUM |
DEV_TX_OFFLOAD_SCTP_CKSUM |
- DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
+ DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
+ DEV_TX_OFFLOAD_VLAN_INSERT |
+ DEV_TX_OFFLOAD_MBUF_FAST_FREE |
+ DEV_TX_OFFLOAD_MULTI_SEGS;
dev_info->speed_capa = ETH_LINK_SPEED_1G |
ETH_LINK_SPEED_2_5G |
ETH_LINK_SPEED_10G;
+ dev_info->default_rxconf.rx_drop_en = true;
}
static int
@@ -268,12 +276,33 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
struct dpaa2_dev_priv *priv = dev->data->dev_private;
struct fsl_mc_io *dpni = priv->hw;
struct rte_eth_conf *eth_conf = &dev->data->dev_conf;
- int rx_ip_csum_offload = false;
+ struct rte_eth_dev_info dev_info;
+ uint64_t rx_offloads = eth_conf->rxmode.offloads;
+ uint64_t tx_offloads = eth_conf->txmode.offloads;
+ int rx_l3_csum_offload = false;
+ int rx_l4_csum_offload = false;
+ int tx_l3_csum_offload = false;
+ int tx_l4_csum_offload = false;
int ret;
PMD_INIT_FUNC_TRACE();
- if (eth_conf->rxmode.jumbo_frame == 1) {
+ dpaa2_dev_info_get(dev, &dev_info);
+ if (dev_info.rx_offload_capa != rx_offloads) {
+ DPAA2_PMD_ERR("Some Rx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ rx_offloads, dev_info.rx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (dev_info.tx_offload_capa != tx_offloads) {
+ DPAA2_PMD_ERR("Some Tx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ tx_offloads, dev_info.tx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
if (eth_conf->rxmode.max_rx_pkt_len <= DPAA2_MAX_RX_PKT_LEN) {
ret = dpni_set_max_frame_length(dpni, CMD_PRI_LOW,
priv->token, eth_conf->rxmode.max_rx_pkt_len);
@@ -297,32 +326,44 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
}
}
- if (eth_conf->rxmode.hw_ip_checksum)
- rx_ip_csum_offload = true;
+ if (rx_offloads & DEV_RX_OFFLOAD_IPV4_CKSUM)
+ rx_l3_csum_offload = true;
+
+ if ((rx_offloads & DEV_RX_OFFLOAD_UDP_CKSUM) ||
+ (rx_offloads & DEV_RX_OFFLOAD_TCP_CKSUM))
+ rx_l4_csum_offload = true;
ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
- DPNI_OFF_RX_L3_CSUM, rx_ip_csum_offload);
+ DPNI_OFF_RX_L3_CSUM, rx_l3_csum_offload);
if (ret) {
DPAA2_PMD_ERR("Error to set RX l3 csum:Error = %d", ret);
return ret;
}
ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
- DPNI_OFF_RX_L4_CSUM, rx_ip_csum_offload);
+ DPNI_OFF_RX_L4_CSUM, rx_l4_csum_offload);
if (ret) {
DPAA2_PMD_ERR("Error to get RX l4 csum:Error = %d", ret);
return ret;
}
+ if (tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM)
+ tx_l3_csum_offload = true;
+
+ if ((tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) ||
+ (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) ||
+ (tx_offloads & DEV_TX_OFFLOAD_SCTP_CKSUM))
+ tx_l4_csum_offload = true;
+
ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
- DPNI_OFF_TX_L3_CSUM, true);
+ DPNI_OFF_TX_L3_CSUM, tx_l3_csum_offload);
if (ret) {
DPAA2_PMD_ERR("Error to set TX l3 csum:Error = %d", ret);
return ret;
}
Re: [dpdk-dev] [PATCH v4 66/70] bus/fslmc: enable support for mem event callbacks for vfio
On Monday 09 April 2018 04:25 PM, Burakov, Anatoly wrote:
On 09-Apr-18 11:01 AM, Shreyansh Jain wrote:
Hi Anatoly,
On Monday 09 April 2018 01:48 AM, Anatoly Burakov wrote:
VFIO needs to map and unmap segments for DMA whenever they
become available or unavailable, so register a callback for
memory events, and provide map/unmap functions.
Signed-off-by: Shreyansh Jain
Signed-off-by: Anatoly Burakov
---
<...>
+ DPAA2_BUS_DEBUG("Calling with type=%d, va=%p, virt_addr=0x%"
PRIx64 ", iova=0x%" PRIx64 ", map_len=%zu\n",
I would like to correct this message (80char + rewording) - What do
you suggest? Should I send a new patch to you or just convey what
should be changed?
As far as i know, leaving strings on single line is good for grepping.
However, perhaps having PRIx64 etc in there breaks it anyway.
Yes, that and the debug message was not helpful.
This is what I had in mind. (DPAA2_BUS_DEBUG doesn't require an extra \n)
DPAA2_BUS_DEBUG("Request for %s, va=%p, virt_addr=0x%" PRIx64 ","
"iova=0x%" PRIx64 ", map_len=%zu",
type == RTE_MEM_EVENT_ALLOC? "alloc" : "dealloc",
va, virt_addr, iova_addr, map_len);
+ type, va, virt_addr, iova_addr, map_len);
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ ret = fslmc_map_dma(virt_addr, iova_addr, map_len);
+ else
+ ret = fslmc_unmap_dma(virt_addr, iova_addr, map_len);
+
+ if (ret != 0) {
+ DPAA2_BUS_ERR("DMA Mapping/Unmapping failed. Map=%d,
addr=%p, len=%zu, err:(%d)",
+ type, va, map_len, ret);
Same as above. 80 Char issue.
Same reasoning - leaving strings unbroken allows for easier grepping the
codebase, but i'm not sure what's our policy on having formatted strings
unbroken.
My policy is not different, but the various variables being dumped
cannot anyway help in grepping - So, keeping the variables on separate
lines for 80chars is ok. "DMA Mapping/Unmapping failed." is enough for
greps.
+ return;
+ }
+
+ cur_len += map_len;
+ }
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ DPAA2_BUS_DEBUG("Total Mapped: addr=%p, len=%zu\n",
+ addr, len);
+ else
<...>
+ ret = rte_mem_event_callback_register("fslmc_memevent_clb",
+ fslmc_memevent_cb);
+ if (ret && rte_errno == ENOTSUP)
+ DPAA2_BUS_DEBUG("Memory event callbacks not supported");
+ else if (ret)
+ DPAA2_BUS_DEBUG("Unable to install memory handler");
+ else
+ DPAA2_BUS_DEBUG("Installed memory callback handler");
/* Verifying that at least single segment is available */
if (i <= 0) {
+ /* TODO: Is this verification required any more? What would
+ * happen to non-legacy case where nothing was preallocated
+ * thus causing i==0?
+ */
And this as well - if call backs are not going to appear in case of
legacy, this needs to be removed.
Callbacks aren't only not going to appear in legacy mode - they will
also not appear on FreeBSD. We check this above, with checking rte_errno
value (if callbacks are not supported, it's set to ENOTSUP, and having
callbacks unsupported is not an error).
let me know how do you want to take these changes.
Now that i think of it, this error condition is wrong. This is called in
both legacy and non-legacy mode. This is bus probe, no? For non-legacy
mode, it is entirely possible to start without any memory whatsoever. It
just so happens that rte_service API allocates some on init, and hence
you always have at least one segment - that may not be the case forever.
So, non-legacy mode, not having memsegs is not an error, it is expected
behavior, so maybe we should remove this error check altogether.
Agree - that count was only required in the earlier case. It can be removed.
DPAA2_BUS_ERR("No Segments found for VFIO Mapping");
+ rte_rwlock_read_unlock(mem_lock);
return -1;
}
DPAA2_BUS_DEBUG("Total %d segments found.", i);
@@ -250,6 +367,11 @@ int rte_fslmc_vfio_dmamap(void)
*/
vfio_map_irq_region(&vfio_group);
+ /* Existing segments have been mapped and memory callback for
hotplug
+ * has been installed.
+ */
+ rte_rwlock_read_unlock(mem_lock);
+
return 0;
}
I think there are enough changes, even if trivial. Maybe I can rework
this patch and send you. If that is inconvenient, just extract from that
whatever you want.
[dpdk-dev] [PATCH v2 0/3] net/sfc: support loopback mode configuration
v1 -> v2: - add patch to fix clang build error Andrew Rybchenko (3): net/sfc/base: fix comparison always true warning net/sfc: support loopback mode configuration app/testpmd: add commands to set loopback mode app/test-pmd/cmdline.c | 121 doc/guides/nics/sfc_efx.rst | 4 +- drivers/net/sfc/base/efx_port.c | 2 +- drivers/net/sfc/efsys.h | 2 +- drivers/net/sfc/sfc.c | 2 + drivers/net/sfc/sfc_port.c | 40 + 6 files changed, 167 insertions(+), 4 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v2 1/3] net/sfc/base: fix comparison always true warning
Loopback type used as bit index has efx_loopback_type_t type
which is enum. clang complains that it is always true when it
is compared with qword (64 bit) bits number boundary.
Fixes: 9ee64bd404fc ("net/sfc/base: import loopback control")
Cc: sta...@dpdk.org
Signed-off-by: Andrew Rybchenko
---
drivers/net/sfc/base/efx_port.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sfc/base/efx_port.c b/drivers/net/sfc/base/efx_port.c
index 261ac62..33a1a08 100644
--- a/drivers/net/sfc/base/efx_port.c
+++ b/drivers/net/sfc/base/efx_port.c
@@ -120,7 +120,7 @@ efx_port_loopback_set(
EFSYS_ASSERT(link_mode < EFX_LINK_NMODES);
if (EFX_TEST_QWORD_BIT(encp->enc_loopback_types[link_mode],
- loopback_type) == 0) {
+ (int)loopback_type) == 0) {
rc = ENOTSUP;
goto fail1;
}
--
2.7.4
[dpdk-dev] [PATCH v2 3/3] app/testpmd: add commands to set loopback mode
Signed-off-by: Andrew Rybchenko
Reviewed-by: Roman Zhukov
Reviewed-by: Ivan Malov
Reviewed-by: Ferruh Yigit
---
app/test-pmd/cmdline.c | 121 +
1 file changed, 121 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 40b31ad..92ef7e1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -806,6 +806,9 @@ static void cmd_help_long_parsed(void *parsed_result,
" duplex (half|full|auto)\n"
"Set speed and duplex for all ports or port_id\n\n"
+ "port config (port_id|all) loopback (mode)\n"
+ "Set loopback mode for all ports or port_id\n\n"
+
"port config all (rxq|txq|rxd|txd) (value)\n"
"Set number for rxq/txq/rxd/txd.\n\n"
@@ -1487,6 +1490,122 @@ cmdline_parse_inst_t cmd_config_speed_specific = {
},
};
+/* *** configure loopback for all ports *** */
+struct cmd_config_loopback_all {
+ cmdline_fixed_string_t port;
+ cmdline_fixed_string_t keyword;
+ cmdline_fixed_string_t all;
+ cmdline_fixed_string_t item;
+ uint32_t mode;
+};
+
+static void
+cmd_config_loopback_all_parsed(void *parsed_result,
+ __attribute__((unused)) struct cmdline *cl,
+ __attribute__((unused)) void *data)
+{
+ struct cmd_config_loopback_all *res = parsed_result;
+ portid_t pid;
+
+ if (!all_ports_stopped()) {
+ printf("Please stop all ports first\n");
+ return;
+ }
+
+ RTE_ETH_FOREACH_DEV(pid) {
+ ports[pid].dev_conf.lpbk_mode = res->mode;
+ }
+
+ cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
+}
+
+cmdline_parse_token_string_t cmd_config_loopback_all_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_all, port, "port");
+cmdline_parse_token_string_t cmd_config_loopback_all_keyword =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_all, keyword,
+ "config");
+cmdline_parse_token_string_t cmd_config_loopback_all_all =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_all, all, "all");
+cmdline_parse_token_string_t cmd_config_loopback_all_item =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_all, item,
+ "loopback");
+cmdline_parse_token_num_t cmd_config_loopback_all_mode =
+ TOKEN_NUM_INITIALIZER(struct cmd_config_loopback_all, mode, UINT32);
+
+cmdline_parse_inst_t cmd_config_loopback_all = {
+ .f = cmd_config_loopback_all_parsed,
+ .data = NULL,
+ .help_str = "port config all loopback ",
+ .tokens = {
+ (void *)&cmd_config_loopback_all_port,
+ (void *)&cmd_config_loopback_all_keyword,
+ (void *)&cmd_config_loopback_all_all,
+ (void *)&cmd_config_loopback_all_item,
+ (void *)&cmd_config_loopback_all_mode,
+ NULL,
+ },
+};
+
+/* *** configure loopback for specific port *** */
+struct cmd_config_loopback_specific {
+ cmdline_fixed_string_t port;
+ cmdline_fixed_string_t keyword;
+ uint16_t port_id;
+ cmdline_fixed_string_t item;
+ uint32_t mode;
+};
+
+static void
+cmd_config_loopback_specific_parsed(void *parsed_result,
+ __attribute__((unused)) struct cmdline *cl,
+ __attribute__((unused)) void *data)
+{
+ struct cmd_config_loopback_specific *res = parsed_result;
+
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+ return;
+
+ if (!port_is_stopped(res->port_id)) {
+ printf("Please stop port %u first\n", res->port_id);
+ return;
+ }
+
+ ports[res->port_id].dev_conf.lpbk_mode = res->mode;
+
+ cmd_reconfig_device_queue(res->port_id, 1, 1);
+}
+
+
+cmdline_parse_token_string_t cmd_config_loopback_specific_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_specific, port,
+ "port");
+cmdline_parse_token_string_t cmd_config_loopback_specific_keyword =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_specific, keyword,
+ "config");
+cmdline_parse_token_num_t cmd_config_loopback_specific_id =
+ TOKEN_NUM_INITIALIZER(struct cmd_config_loopback_specific, port_id,
+ UINT16);
+cmdline_parse_token_string_t cmd_config_loopback_specific_item =
+ TOKEN_STRING_INITIALIZER(struct cmd_config_loopback_specific, item,
+ "loopback");
+cmdline_parse_token_num_t cmd_config_loopback_specific_mode =
+ TOKEN_NUM_INITIALIZER(s
[dpdk-dev] [PATCH v2 2/3] net/sfc: support loopback mode configuration
All loopback modes are listed in efx_loopback_type_t.
Available loopback modes are listed per link speed in
the enc_loopback_types member of the efx_nic_cfg_t.
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
Reviewed-by: Ivan Malov
---
doc/guides/nics/sfc_efx.rst | 4 ++--
drivers/net/sfc/efsys.h | 2 +-
drivers/net/sfc/sfc.c | 2 ++
drivers/net/sfc/sfc_port.c | 40
4 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index c170e36..abaed67 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -88,6 +88,8 @@ SFC EFX PMD has support for:
- Flow API
+- Loopback
+
Non-supported Features
--
@@ -98,8 +100,6 @@ The features not yet supported include:
- Priority-based flow control
-- Loopback
-
- Configurable RX CRC stripping (always stripped)
- Header split on receive
diff --git a/drivers/net/sfc/efsys.h b/drivers/net/sfc/efsys.h
index 7eb2c3f..12f77dc 100644
--- a/drivers/net/sfc/efsys.h
+++ b/drivers/net/sfc/efsys.h
@@ -166,7 +166,7 @@ prefetch_read_once(const volatile void *addr)
#define EFSYS_OPT_MAC_STATS 1
-#define EFSYS_OPT_LOOPBACK 0
+#define EFSYS_OPT_LOOPBACK 1
#define EFSYS_OPT_MON_MCDI 0
#define EFSYS_OPT_MON_STATS 0
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 69abaff..716683b 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -123,10 +123,12 @@ sfc_check_conf(struct sfc_adapter *sa)
rc = EINVAL;
}
+#if !EFSYS_OPT_LOOPBACK
if (conf->lpbk_mode != 0) {
sfc_err(sa, "Loopback not supported");
rc = EINVAL;
}
+#endif
if (conf->dcb_capability_en != 0) {
sfc_err(sa, "Priority-based flow control not supported");
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index fd267e1..5cc3ad7 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -121,6 +121,28 @@ sfc_port_init_dev_link(struct sfc_adapter *sa)
return 0;
}
+#if EFSYS_OPT_LOOPBACK
+
+static efx_link_mode_t
+sfc_port_phy_caps_to_max_link_speed(uint32_t phy_caps)
+{
+ if (phy_caps & (1u << EFX_PHY_CAP_10FDX))
+ return EFX_LINK_10FDX;
+ if (phy_caps & (1u << EFX_PHY_CAP_5FDX))
+ return EFX_LINK_5FDX;
+ if (phy_caps & (1u << EFX_PHY_CAP_4FDX))
+ return EFX_LINK_4FDX;
+ if (phy_caps & (1u << EFX_PHY_CAP_25000FDX))
+ return EFX_LINK_25000FDX;
+ if (phy_caps & (1u << EFX_PHY_CAP_1FDX))
+ return EFX_LINK_1FDX;
+ if (phy_caps & (1u << EFX_PHY_CAP_1000FDX))
+ return EFX_LINK_1000FDX;
+ return EFX_LINK_UNKNOWN;
+}
+
+#endif
+
int
sfc_port_start(struct sfc_adapter *sa)
{
@@ -143,6 +165,21 @@ sfc_port_start(struct sfc_adapter *sa)
if (rc != 0)
goto fail_port_init;
+#if EFSYS_OPT_LOOPBACK
+ if (sa->eth_dev->data->dev_conf.lpbk_mode != 0) {
+ efx_link_mode_t link_mode;
+
+ link_mode =
+ sfc_port_phy_caps_to_max_link_speed(port->phy_adv_cap);
+ sfc_log_init(sa, "set loopback link_mode=%u type=%u", link_mode,
+sa->eth_dev->data->dev_conf.lpbk_mode);
+ rc = efx_port_loopback_set(sa->nic, link_mode,
+ sa->eth_dev->data->dev_conf.lpbk_mode);
+ if (rc != 0)
+ goto fail_loopback_set;
+ }
+#endif
+
sfc_log_init(sa, "set flow control to %#x autoneg=%u",
port->flow_ctrl, port->flow_ctrl_autoneg);
rc = efx_mac_fcntl_set(sa->nic, port->flow_ctrl,
@@ -268,6 +305,9 @@ sfc_port_start(struct sfc_adapter *sa)
fail_mac_pdu_set:
fail_phy_adv_cap_set:
fail_mac_fcntl_set:
+#if EFSYS_OPT_LOOPBACK
+fail_loopback_set:
+#endif
efx_port_fini(sa->nic);
fail_port_init:
--
2.7.4
Re: [dpdk-dev] [PATCH 1/2] net/sfc: support loopback mode configuration
On 04/06/2018 03:09 PM, Ferruh Yigit wrote: On 4/4/2018 12:10 PM, Andrew Rybchenko wrote: All loopback modes are listed in efx_loopback_type_t. Available loopback modes are listed per link speed in the enc_loopback_types member of the efx_nic_cfg_t. Signed-off-by: Andrew Rybchenko Reviewed-by: Andy Moreton Reviewed-by: Ivan Malov Getting following build error with clang, can you please check? .../dpdk/drivers/net/sfc/base/efx_port.c:122:6: error: comparison of constant 64 with expression of type 'efx_loopback_type_t' (aka 'enum efx_loopback_type_e') is always true [-Werror,-Wtautological-constant-out-of-range-compare] if (EFX_TEST_QWORD_BIT(encp->enc_loopback_types[link_mode], ^~~ .../dpdk/drivers/net/sfc/base/efx_types.h:1590:28: note: expanded from macro 'EFX_TEST_QWORD_BIT' #define EFX_TEST_QWORD_BIT EFX_TEST_QWORD_BIT64 ^ .../dpdk/drivers/net/sfc/base/efx_types.h:1412:22: note: expanded from macro 'EFX_TEST_QWORD_BIT64' __CPU_TO_LE_64(EFX_SHIFT64(_bit, FIX_LINT(0 != 0) ~~~^~~ .../dpdk/drivers/net/sfc/base/efx_types.h:1290:32: note: expanded from macro 'EFX_SHIFT64' (((_bit) >= (_base) && (_bit) < (_base) + 64) ? \ ^ .../dpdk/drivers/net/sfc/base/efx_types.h:269:50: note: expanded from macro '__CPU_TO_LE_64' #define __CPU_TO_LE_64(_x) ((uint64_t)__NOSWAP64(_x)) ~~~^~~ .../dpdk/drivers/net/sfc/base/efx_types.h:238:26: note: expanded from macro '__NOSWAP64' #define __NOSWAP64(_x) (_x) ^~ 1 error generated. Thanks, I've added patch to fix the problem. Andrew.
[dpdk-dev] [PATCH] examples/ip_pipeline: fix freebsd build error
IP_Pipeline app is not supported in FreeBSD environment. Therefore,
skip it while building the sample apps on FreeBSD.
Fixes: 4bbf8e30aa5e ("examples/ip_pipeline: add CLI interface")
Fixes: 2f74ae28e23f ("examples/ip_pipeline: add tap object")
Signed-off-by: Jasvinder Singh
---
examples/ip_pipeline/Makefile | 5 +
1 file changed, 5 insertions(+)
diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
index c936d1e..6ff2abf 100644
--- a/examples/ip_pipeline/Makefile
+++ b/examples/ip_pipeline/Makefile
@@ -67,6 +67,11 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
include $(RTE_SDK)/mk/rte.vars.mk
+ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")
+$(error This application can only operate in a linuxapp environment, \
+please change the definition of the RTE_TARGET environment variable)
+endif
+
INC += $(sort $(wildcard *.h))
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) := $(SRCS-y)
--
2.9.3
[dpdk-dev] [PATCH v6] ethdev: replace bus specific struct with generic dev
Public struct rte_eth_dev_info has a "struct rte_pci_device" field in it
although it is common for all ethdev in all buses.
Replacing pci specific struct with generic device struct and updating
places that are using pci device in a way to get this information from
generic device.
Signed-off-by: Ferruh Yigit
Reviewed-by: David Marchand
Acked-by: Pablo de Lara
---
Cc: Shreyansh Jain
Cc: Allain Legacy
Cc: Tomasz Duszynski
Cc: Santosh Shukla
Cc: David Marchand
v2:
- prevent possible crash while getting bus (Pablo)
- Remove unnecessary __rte_unused
- Some PMD info_dev->device was assigned to NULL, fixed them
v3:
- rebased on latest next-net
v4:
- Move dev_info->device assignment to ethdev layer
v5:
- Document API change in related section in release notes
v6:
- Rebase on latest next-net, ip_pipeline updated
- Update axgbe too
---
app/test-pmd/config.c | 18 +++-
app/test-pmd/testpmd.h | 38 +++--
doc/guides/rel_notes/release_18_05.rst | 3 +++
drivers/net/ark/ark_ethdev.c| 1 -
drivers/net/avf/avf_ethdev.c| 1 -
drivers/net/avp/avp_ethdev.c| 1 -
drivers/net/axgbe/axgbe_ethdev.c| 4 +---
drivers/net/bnx2x/bnx2x_ethdev.c| 1 -
drivers/net/bnxt/bnxt_ethdev.c | 2 --
drivers/net/cxgbe/cxgbe_ethdev.c| 2 --
drivers/net/e1000/em_ethdev.c | 1 -
drivers/net/e1000/igb_ethdev.c | 2 --
drivers/net/ena/ena_ethdev.c| 2 --
drivers/net/enic/enic_ethdev.c | 1 -
drivers/net/fm10k/fm10k_ethdev.c| 1 -
drivers/net/i40e/i40e_ethdev.c | 1 -
drivers/net/i40e/i40e_ethdev_vf.c | 1 -
drivers/net/ixgbe/ixgbe_ethdev.c| 2 --
drivers/net/kni/rte_eth_kni.c | 1 -
drivers/net/liquidio/lio_ethdev.c | 2 --
drivers/net/mlx4/mlx4_ethdev.c | 1 -
drivers/net/mlx5/mlx5_ethdev.c | 1 -
drivers/net/nfp/nfp_net.c | 1 -
drivers/net/octeontx/octeontx_ethdev.c | 1 -
drivers/net/qede/qede_ethdev.c | 1 -
drivers/net/sfc/sfc_ethdev.c| 1 -
drivers/net/szedata2/rte_eth_szedata2.c | 1 -
drivers/net/tap/rte_eth_tap.c | 1 -
drivers/net/thunderx/nicvf_ethdev.c | 2 --
drivers/net/virtio/virtio_ethdev.c | 1 -
drivers/net/vmxnet3/vmxnet3_ethdev.c| 4 +---
examples/ethtool/lib/rte_ethtool.c | 16 --
examples/ip_pipeline/kni.c | 11 --
examples/kni/main.c | 11 +++---
lib/librte_ether/rte_ethdev.c | 1 +
lib/librte_ether/rte_ethdev.h | 2 +-
test/test/test_kni.c| 35 --
37 files changed, 112 insertions(+), 64 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4bb255c62..dd051f5ca 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -754,6 +754,8 @@ vlan_id_is_invalid(uint16_t vlan_id)
static int
port_reg_off_is_invalid(portid_t port_id, uint32_t reg_off)
{
+ const struct rte_pci_device *pci_dev;
+ const struct rte_bus *bus;
uint64_t pci_len;
if (reg_off & 0x3) {
@@ -762,7 +764,21 @@ port_reg_off_is_invalid(portid_t port_id, uint32_t reg_off)
(unsigned)reg_off);
return 1;
}
- pci_len = ports[port_id].dev_info.pci_dev->mem_resource[0].len;
+
+ if (!ports[port_id].dev_info.device) {
+ printf("Invalid device\n");
+ return 0;
+ }
+
+ bus = rte_bus_find_by_device(ports[port_id].dev_info.device);
+ if (bus && !strcmp(bus->name, "pci")) {
+ pci_dev = RTE_DEV_TO_PCI(ports[port_id].dev_info.device);
+ } else {
+ printf("Not a PCI device\n");
+ return 1;
+ }
+
+ pci_len = pci_dev->mem_resource[0].len;
if (reg_off >= pci_len) {
printf("Port %d: register offset %u (0x%X) out of port PCI "
"resource (length=%"PRIu64")\n",
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 153abea05..4d84e7b00 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -500,12 +500,25 @@ mbuf_pool_find(unsigned int sock_id)
static inline uint32_t
port_pci_reg_read(struct rte_port *port, uint32_t reg_off)
{
+ const struct rte_pci_device *pci_dev;
+ const struct rte_bus *bus;
void *reg_addr;
uint32_t reg_v;
- reg_addr = (void *)
- ((char *)port->dev_info.pci_dev->mem_resource[0].addr +
- reg_off);
+ if (!port->dev_info.device) {
+ printf("Invalid device\n");
+ return 0;
+ }
+
+ bus = rte_bus_find_by_device(port->dev_info.device);
+ if (bus && !strcmp(bus->name, "pci")) {
+ pci_dev = RTE_DEV_TO_PCI(port->dev_info.device);
+ } else {
+ printf("Not
Re: [dpdk-dev] [PATCH] net/mlx5: fix link status initialization
Monday, April 9, 2018 11:28 AM, Nélio Laranjeiro: > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > On Sun, Apr 08, 2018 at 01:09:27PM +, Shahaf Shuler wrote: > > Thursday, April 5, 2018 9:51 AM, Nélio Laranjeiro: > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > > > > > On Thu, Apr 05, 2018 at 05:35:57AM +, Shahaf Shuler wrote: > > > > Wednesday, April 4, 2018 3:11 PM, Nélio Laranjeiro: > > > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > > > > > > > > > On Wed, Apr 04, 2018 at 09:58:33AM +, Shahaf Shuler wrote: > > > > > > Wednesday, April 4, 2018 10:30 AM, Nélio Laranjeiro: > > > > > > > Subject: Re: [PATCH] net/mlx5: fix link status > > > > > > > initialization > > > > > > > > > > > > > > On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote: > > > > [..] > > > > > > > > > > > > According to your analysis, this is only necessary when the LCS > > > > > is configured in the device. Why not adding this call to > > > > > mlx5_dev_interrupt_handler_install() which is responsible to > > > > > install the LCS callback. > > > > > > > > I think it is good practice whether or not LSC is set. > > > > The link status should be initialized to the correct value after the > > > > probe. > > > > > > There is no guarantee the link will be accurate, at probe time the > > > link may be up so internal information has a status up with a speed with > this patch. > > > The application probes a second port, in the mean time the link of > > > the first port goes down, the interrupt is still not installed and > > > the internal status becomes wrong (still up whereas the port is down). > > > > > > Finally at start, the device installs the handler, but the link is > > > still down whereas internally it is up, the application will call > > > rte_eth_link_get_nowait() which will directly copy the wrong > > > internal status to the application. > > > > This is not correct. > > Using Verbs, the async_fd on which link status interrupts are reported is > created on probe. > > Even if the interrupt handler is not installed, interrupts still > > trigger on this fd. They will be processed when the interrupt handler > > will be installed as part of the port start. > > So in fact you have the whole trace on the link status changes waiting > > to be processed upon port start. > > Right, but in such case, this patch still does not solves the issue. > Until the dev_start() is called, the link may be inconsistent with the real > status. > > example: > > pci_probe --> link is up. > leaving pci probe, the link goes downs --> internally the PMD has a link up. > > Until the dev_start() is called any call to rte_eth_link_get_nowait() will > copy > the internal PMD status which is not accurate. > > From this point, the issue seems to fully come from > rte_eth_link_get_nowait() which should not make any copy until the port is > not started, until then the link may be inconsistent between the driver and > the device. Right. However fixing it only on the ethdev layer is not enough. Assume the link was up from the beginning. For the case were the call for rte_eth_link_get_nowait happens only after the port start, the link will still be wrong as it will be reported as down (and no interrupt to fix it). So to summarize I plan to have this patch (with modification of the wait_to_complete) along with another fix for ethdev. Do we have an agreement? > > Regards, > > -- > Nélio Laranjeiro > 6WIND
Re: [dpdk-dev] [PATCH v5 08/18] net/axgbe: add transmit and receive queue setup apis
On 4/9/2018 5:49 AM, Rosen, Rami wrote:
> +static void
> +axgbe_dev_info_get(struct rte_eth_dev *dev,
> +struct rte_eth_dev_info *dev_info) {
> + struct axgbe_port *pdata = dev->data->dev_private;
> +
> + dev_info->pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>
>
> [Rami Rosen] Shouldn't it be here:
> dev_info->max_rx_queues = pdata->rx_ring_count;
> dev_info->max_tx_queues = pdata->tx_ring_count;
>
>
> + dev_info->max_rx_queues = pdata->tx_ring_count;
> + dev_info->max_tx_queues = pdata->rx_ring_count;
Yes, good catch. Can either you or Ravi send a fix please? I can squash it.
[dpdk-dev] [PATCH v1 2/2] app/testpmd: config all supported RSS functions
Only configure RSS hash functions supported by the device.
Signed-off-by: Xueming Li
---
app/test-pmd/cmdline.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 40b31ad7e..c41dd71ce 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1879,6 +1879,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;
@@ -1915,6 +1916,11 @@ 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 v1 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 | 21 +
1 file changed, 21 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 209796d46..a8e122781 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1179,6 +1179,17 @@ 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%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;
+ }
+
/*
* Setup new number of RX/TX queues and reconfigure device.
*/
@@ -2832,9 +2843,19 @@ 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%lx,
valid value: 0x%lx\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 v4 66/70] bus/fslmc: enable support for mem event callbacks for vfio
On 09-Apr-18 1:09 PM, Shreyansh Jain wrote:
On Monday 09 April 2018 04:25 PM, Burakov, Anatoly wrote:
On 09-Apr-18 11:01 AM, Shreyansh Jain wrote:
Hi Anatoly,
On Monday 09 April 2018 01:48 AM, Anatoly Burakov wrote:
VFIO needs to map and unmap segments for DMA whenever they
become available or unavailable, so register a callback for
memory events, and provide map/unmap functions.
Signed-off-by: Shreyansh Jain
Signed-off-by: Anatoly Burakov
---
<...>
+ DPAA2_BUS_DEBUG("Calling with type=%d, va=%p,
virt_addr=0x%" PRIx64 ", iova=0x%" PRIx64 ", map_len=%zu\n",
I would like to correct this message (80char + rewording) - What do
you suggest? Should I send a new patch to you or just convey what
should be changed?
As far as i know, leaving strings on single line is good for grepping.
However, perhaps having PRIx64 etc in there breaks it anyway.
Yes, that and the debug message was not helpful.
This is what I had in mind. (DPAA2_BUS_DEBUG doesn't require an extra \n)
DPAA2_BUS_DEBUG("Request for %s, va=%p, virt_addr=0x%" PRIx64 ","
"iova=0x%" PRIx64 ", map_len=%zu",
type == RTE_MEM_EVENT_ALLOC? "alloc" : "dealloc",
va, virt_addr, iova_addr, map_len);
+ type, va, virt_addr, iova_addr, map_len);
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ ret = fslmc_map_dma(virt_addr, iova_addr, map_len);
+ else
+ ret = fslmc_unmap_dma(virt_addr, iova_addr, map_len);
+
+ if (ret != 0) {
+ DPAA2_BUS_ERR("DMA Mapping/Unmapping failed. Map=%d,
addr=%p, len=%zu, err:(%d)",
+ type, va, map_len, ret);
Same as above. 80 Char issue.
Same reasoning - leaving strings unbroken allows for easier grepping
the codebase, but i'm not sure what's our policy on having formatted
strings unbroken.
My policy is not different, but the various variables being dumped
cannot anyway help in grepping - So, keeping the variables on separate
lines for 80chars is ok. "DMA Mapping/Unmapping failed." is enough for
greps.
+ return;
+ }
+
+ cur_len += map_len;
+ }
+
+ if (type == RTE_MEM_EVENT_ALLOC)
+ DPAA2_BUS_DEBUG("Total Mapped: addr=%p, len=%zu\n",
+ addr, len);
+ else
<...>
+ ret = rte_mem_event_callback_register("fslmc_memevent_clb",
+ fslmc_memevent_cb);
+ if (ret && rte_errno == ENOTSUP)
+ DPAA2_BUS_DEBUG("Memory event callbacks not supported");
+ else if (ret)
+ DPAA2_BUS_DEBUG("Unable to install memory handler");
+ else
+ DPAA2_BUS_DEBUG("Installed memory callback handler");
/* Verifying that at least single segment is available */
if (i <= 0) {
+ /* TODO: Is this verification required any more? What would
+ * happen to non-legacy case where nothing was preallocated
+ * thus causing i==0?
+ */
And this as well - if call backs are not going to appear in case of
legacy, this needs to be removed.
Callbacks aren't only not going to appear in legacy mode - they will
also not appear on FreeBSD. We check this above, with checking
rte_errno value (if callbacks are not supported, it's set to ENOTSUP,
and having callbacks unsupported is not an error).
let me know how do you want to take these changes.
Now that i think of it, this error condition is wrong. This is called
in both legacy and non-legacy mode. This is bus probe, no? For
non-legacy mode, it is entirely possible to start without any memory
whatsoever. It just so happens that rte_service API allocates some on
init, and hence you always have at least one segment - that may not be
the case forever. So, non-legacy mode, not having memsegs is not an
error, it is expected behavior, so maybe we should remove this error
check altogether.
Agree - that count was only required in the earlier case. It can be
removed.
DPAA2_BUS_ERR("No Segments found for VFIO Mapping");
+ rte_rwlock_read_unlock(mem_lock);
return -1;
}
DPAA2_BUS_DEBUG("Total %d segments found.", i);
@@ -250,6 +367,11 @@ int rte_fslmc_vfio_dmamap(void)
*/
vfio_map_irq_region(&vfio_group);
+ /* Existing segments have been mapped and memory callback for
hotplug
+ * has been installed.
+ */
+ rte_rwlock_read_unlock(mem_lock);
+
return 0;
}
I think there are enough changes, even if trivial. Maybe I can rework
this patch and send you. If that is inconvenient, just extract from that
whatever you want.
There aren't a lot of changes so i'll respin it myself, addressing the
comments above. Thanks!
--
Thanks,
Anatoly
Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically
Hi, On Fri, Mar 30, 2018 at 02:58:31PM +0800, Junjie Chen wrote: When creating vdev dynamically, vhost pmd driver starts directly without checking TX/RX queues are ready or not, and thus causes segmentation fault when vhost library accesses queues. This patch adds a flag to check whether queues are setup or not, and adds queues setup into dev_start function to allow user to start them after setting up. for me, with this patch vhost enqueue/dequeue code is never called because if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0)) this check in eth_vhost_rx() is always true. When I revert this patch it works as usual. My testpmd cmdline is: gdb --args $RTE_SDK/install/bin/testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 \ --vdev 'net_vhost0,iface=/tmp/vhost-user1' \ --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- \ --portmask=f --rxq=1 --txq=1 \ --nb-cores=4 --forward-mode=io -i After starting testpmd I issue commands "set portlist 0,2,1,3", start my guest and start another testpmd issue in the guest. Another problem I see: Before this patch I could start testpmd, issue the portlist command and type "start". If I do this now I get an infinite loop of "VHOST CONFIG device not found" messages. regards, Jens
Re: [dpdk-dev] [PATCH v5 4/4] testpmd: make use of per-PMD TxRx parameters
On Friday 06 April 2018 08:20 PM, Remy Horton wrote:
The optimal values of several transmission & reception related
parameters, such as burst sizes, descriptor ring sizes, and number
of queues, varies between different network interface devices. This
patch allows testpmd to make use of per-PMD tuned parameter values.
Signed-off-by: Remy Horton
---
app/test-pmd/cmdline.c| 31 ---
app/test-pmd/parameters.c | 38 +-
app/test-pmd/testpmd.c| 5 +++--
3 files changed, 64 insertions(+), 10 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 40b31ad..0914425 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2599,6 +2599,8 @@ cmd_config_burst_parsed(void *parsed_result,
__attribute__((unused)) void *data)
{
struct cmd_config_burst *res = parsed_result;
+ struct rte_eth_dev_info dev_info;
+ uint16_t rec_nb_pkts;
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
@@ -2606,11 +2608,34 @@ cmd_config_burst_parsed(void *parsed_result,
}
if (!strcmp(res->name, "burst")) {
- if (res->value < 1 || res->value > MAX_PKT_BURST) {
+ if (res->value == 0) {
Documentation for burst mode changes to testpmd would need an update.
I guess, only when the user explicitly sets 'set burst 0' would the
driver defaults be picked up - isn't it?
Maybe something like this:
--->8---
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -372,7 +372,9 @@ The commandline options are:
* ``--burst=N``
Set the number of packets per burst to N, where 1 <= N <= 512.
-The default value is 16.
+The default value is 32.
+If set to 0, driver default is used if defined. Else, if driver default
+is not defined, default of 32 is used.
* ``--mbcache=N``
--->8---
In the above, I think the existing documented default value needs to be
changed. It is set to '#define DEF_PKT_BURST 32'
If you add that, please use my ack for next revision.
(For patch 1/4, I had already given my Ack in v2)
+ /* If user gives a value of zero, query the PMD for
+* its recommended Rx burst size. Testpmd uses a single
+* size for all ports, so assume all ports are the same
+* NIC model and use the values from Port 0.
+*/
+ rte_eth_dev_info_get(0, &dev_info);
+ rec_nb_pkts = dev_info.default_rxportconf.burst_size;
+
+ if (rec_nb_pkts == 0) {
+ printf("PMD does not recommend a burst size.\n"
+ "User provided value must be between"
+ " 1 and %d\n", MAX_PKT_BURST);
+ return;
+ } else if (rec_nb_pkts > MAX_PKT_BURST) {
+ printf("PMD recommended burst size of %d"
+ " exceeds maximum value of %d\n",
+ rec_nb_pkts, MAX_PKT_BURST);
+ return;
+ }
+ printf("Using PMD-provided burst value of %d\n",
+ rec_nb_pkts);
+ nb_pkt_per_burst = rec_nb_pkts;
+ } else if (res->value > MAX_PKT_BURST) {
printf("burst must be >= 1 && <= %d\n", MAX_PKT_BURST);
return;
- }
- nb_pkt_per_burst = res->value;
+ } else
+ nb_pkt_per_burst = res->value;
} else {
printf("Unknown parameter\n");
return;
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 2192bdc..cb6a229 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -544,6 +544,8 @@ launch_args_parse(int argc, char** argv)
/* Default offloads for all ports. */
uint64_t rx_offloads = rx_mode.offloads;
uint64_t tx_offloads = tx_mode.offloads;
+ struct rte_eth_dev_info dev_info;
+ uint16_t rec_nb_pkts;
static struct option lgopts[] = {
{ "help", 0, 0, 0 },
@@ -947,12 +949,38 @@ launch_args_parse(int argc, char** argv)
}
if (!strcmp(lgopts[opt_idx].name, "burst")) {
n = atoi(optarg);
- if ((n >= 1) && (n <= MAX_PKT_BURST))
- nb_pkt_per_burst = (uint16_t) n;
- else
+ if (n == 0) {
+ /* A burst size of zero means that the
+
[dpdk-dev] [PATCH] table: fix build error with gcc 8
Fix build error with gcc 8.0 due to cast between function types.
Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
Signed-off-by: Jasvinder Singh
---
lib/librte_table/rte_table_hash_cuckoo.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_table/rte_table_hash_cuckoo.c
b/lib/librte_table/rte_table_hash_cuckoo.c
index dcb4fe9..f7eae27 100644
--- a/lib/librte_table/rte_table_hash_cuckoo.c
+++ b/lib/librte_table/rte_table_hash_cuckoo.c
@@ -103,11 +103,13 @@ rte_table_hash_cuckoo_create(void *params,
return NULL;
}
+ void *hash_func = p->f_hash;
+
/* Create cuckoo hash table */
struct rte_hash_parameters hash_cuckoo_params = {
.entries = p->n_keys,
.key_len = p->key_size,
- .hash_func = (rte_hash_function)(p->f_hash),
+ .hash_func = (rte_hash_function) hash_func,
.hash_func_init_val = p->seed,
.socket_id = socket_id,
.name = p->name
--
2.9.3
Re: [dpdk-dev] [PATCH] net/enic: enable overlay offload for VXLAN and GENEVE
On 4/7/2018 3:40 AM, Hyong Youb Kim wrote: > On Fri, Apr 06, 2018 at 05:15:40PM +0100, Ferruh Yigit wrote: >> On 4/5/2018 12:54 AM, John Daley wrote: >>> From: Hyong Youb Kim >>> >>> Recent NIC models support overlay offload. The overlay offload >>> feature enables the following on the NIC. >>> - Rx/Tx checksum offloads for both inner and outer packets. >>> - Rx inner packet type classification. >>> - TSO. >>> - Inner RSS. >>> >>> TX descriptors do not require any changes, except the header length >>> for TSO. The NIC parses outer/inner packets and performs offloads on >>> them as necessary. The header length for tunneled TSO includes both >>> inner and outer headers. >>> >>> The NIC actually parses and performs the above for NVGRE as well. DPDK >>> currently has no offload flags for NVGRE, and the hardware has no >>> controls to individually enable tunnel types either. So do nothing for >>> now. >>> >>> Add a config flag to enable overlay offload by default. To disable it, >>> the user should set it to 'n'. >>> >>> CONFIG_RTE_LIBRTE_ENIC_ENABLE_OVERLAY_OFFLOAD=y >>> >>> Also update the enic guide doc. >>> >>> Signed-off-by: Hyong Youb Kim >>> Reviewed-by: John Daley >>> --- >>> config/common_base | 1 + >>> doc/guides/nics/enic.rst| 52 ++ >>> drivers/net/enic/base/vnic_dev.c| 33 >>> drivers/net/enic/base/vnic_dev.h| 5 +- >>> drivers/net/enic/base/vnic_devcmd.h | 12 + >>> drivers/net/enic/base/vnic_wq.h | 1 + >>> drivers/net/enic/enic.h | 6 +++ >>> drivers/net/enic/enic_ethdev.c | 21 ++-- >>> drivers/net/enic/enic_main.c| 25 + >>> drivers/net/enic/enic_res.c | 23 >>> drivers/net/enic/enic_rxtx.c| 104 >>> >>> 11 files changed, 241 insertions(+), 42 deletions(-) >>> >>> diff --git a/config/common_base b/config/common_base >>> index c09c7cf88..964e37b6e 100644 >>> --- a/config/common_base >>> +++ b/config/common_base >>> @@ -205,6 +205,7 @@ CONFIG_RTE_LIBRTE_ENA_COM_DEBUG=n >>> # Compile burst-oriented Cisco ENIC PMD driver >>> # >>> CONFIG_RTE_LIBRTE_ENIC_PMD=y >>> +CONFIG_RTE_LIBRTE_ENIC_ENABLE_OVERLAY_OFFLOAD=y >> >> Hi John, >> >> We are trying to reduce config options we have, is overlay offload enabling >> can >> be done via runtime argument or dedicated offload flag? > > Hi, > > Would you accept a devarg? We would enable overlay offload by default, > and disable it when the app specifies that devarg. This overlay > offload is a bunch of things under one roof, and it does not play > nicely with DPDK offload flags. No problem on implementing this as devargs. > > Thanks. > -Hyong >
Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
> -Original Message-
> From: Singh, Jasvinder
> Sent: Monday, April 9, 2018 1:50 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian
> Subject: [PATCH] table: fix build error with gcc 8
>
> Fix build error with gcc 8.0 due to cast between function types.
> Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
>
> Signed-off-by: Jasvinder Singh
> ---
> lib/librte_table/rte_table_hash_cuckoo.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_table/rte_table_hash_cuckoo.c
> b/lib/librte_table/rte_table_hash_cuckoo.c
> index dcb4fe9..f7eae27 100644
> --- a/lib/librte_table/rte_table_hash_cuckoo.c
> +++ b/lib/librte_table/rte_table_hash_cuckoo.c
> @@ -103,11 +103,13 @@ rte_table_hash_cuckoo_create(void *params,
> return NULL;
> }
>
> + void *hash_func = p->f_hash;
> +
> /* Create cuckoo hash table */
> struct rte_hash_parameters hash_cuckoo_params = {
> .entries = p->n_keys,
> .key_len = p->key_size,
> - .hash_func = (rte_hash_function)(p->f_hash),
> + .hash_func = (rte_hash_function) hash_func,
> .hash_func_init_val = p->seed,
> .socket_id = socket_id,
> .name = p->name
> --
> 2.9.3
Acked-by: Cristian Dumitrescu
Re: [dpdk-dev] [PATCH] examples/ip_pipeline: fix freebsd build error
> -Original Message-
> From: Singh, Jasvinder
> Sent: Monday, April 9, 2018 1:07 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian
> Subject: [PATCH] examples/ip_pipeline: fix freebsd build error
>
> IP_Pipeline app is not supported in FreeBSD environment. Therefore,
> skip it while building the sample apps on FreeBSD.
>
> Fixes: 4bbf8e30aa5e ("examples/ip_pipeline: add CLI interface")
> Fixes: 2f74ae28e23f ("examples/ip_pipeline: add tap object")
>
> Signed-off-by: Jasvinder Singh
> ---
> examples/ip_pipeline/Makefile | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
> index c936d1e..6ff2abf 100644
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples/ip_pipeline/Makefile
> @@ -67,6 +67,11 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
>
> include $(RTE_SDK)/mk/rte.vars.mk
>
> +ifneq ($(CONFIG_RTE_EXEC_ENV),"linuxapp")
> +$(error This application can only operate in a linuxapp environment, \
> +please change the definition of the RTE_TARGET environment variable)
> +endif
> +
> INC += $(sort $(wildcard *.h))
>
> SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) := $(SRCS-y)
> --
> 2.9.3
Acked-by: Cristian Dumitrescu
Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
On Mon, Apr 09, 2018 at 01:49:48PM +0100, Jasvinder Singh wrote:
> Fix build error with gcc 8.0 due to cast between function types.
> Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
>
> Signed-off-by: Jasvinder Singh
What's the actual error message? Why do the types not match?
/Bruce
[dpdk-dev] [PATCH v1] doc: add SPDX Licence to doc files
Added SPDX headers to doc files to have them aligned with the other doc files. Signed-off-by: Marko Kovacevic --- doc/guides/contributing/cheatsheet.rst | 3 +++ doc/guides/contributing/coding_style.rst | 3 +++ doc/guides/contributing/design.rst | 3 +++ doc/guides/contributing/documentation.rst| 3 +++ doc/guides/contributing/img/patch_cheatsheet.svg | 3 ++- doc/guides/contributing/index.rst| 3 +++ doc/guides/contributing/patches.rst | 3 +++ doc/guides/contributing/stable.rst | 3 +++ doc/guides/contributing/versioning.rst | 3 +++ doc/guides/linux_gsg/nic_perf_intel_platform.rst | 3 +++ doc/guides/rel_notes/deprecation.rst | 3 +++ doc/guides/rel_notes/release_16_04.rst | 3 +++ doc/guides/rel_notes/release_16_07.rst | 3 +++ doc/guides/rel_notes/release_16_11.rst | 3 +++ doc/guides/rel_notes/release_17_02.rst | 3 +++ doc/guides/rel_notes/release_17_05.rst | 3 +++ doc/guides/rel_notes/release_17_08.rst | 3 +++ doc/guides/rel_notes/release_17_11.rst | 3 +++ doc/guides/rel_notes/release_18_02.rst | 3 +++ doc/guides/rel_notes/release_2_2.rst | 3 +++ 20 files changed, 59 insertions(+), 1 deletion(-) diff --git a/doc/guides/contributing/cheatsheet.rst b/doc/guides/contributing/cheatsheet.rst index 7bc0771..97512d7 100644 --- a/doc/guides/contributing/cheatsheet.rst +++ b/doc/guides/contributing/cheatsheet.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + Patch Cheatsheet diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst index b0f0adb..e285fc8 100644 --- a/doc/guides/contributing/coding_style.rst +++ b/doc/guides/contributing/coding_style.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + .. _coding_style: DPDK Coding Style diff --git a/doc/guides/contributing/design.rst b/doc/guides/contributing/design.rst index 88d3a43..bbe219e 100644 --- a/doc/guides/contributing/design.rst +++ b/doc/guides/contributing/design.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + Design == diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst index 82f2e1b..7ac1b41 100644 --- a/doc/guides/contributing/documentation.rst +++ b/doc/guides/contributing/documentation.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + .. _doc_guidelines: DPDK Documentation Guidelines diff --git a/doc/guides/contributing/img/patch_cheatsheet.svg b/doc/guides/contributing/img/patch_cheatsheet.svg index 8522592..af2473d 100644 --- a/doc/guides/contributing/img/patch_cheatsheet.svg +++ b/doc/guides/contributing/img/patch_cheatsheet.svg @@ -1,5 +1,6 @@ - + + http://purl.org/dc/elements/1.1/"; diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst index 329b678..9fa0076 100644 --- a/doc/guides/contributing/index.rst +++ b/doc/guides/contributing/index.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + Contributor's Guidelines diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst index 2287835..494cdf4 100644 --- a/doc/guides/contributing/patches.rst +++ b/doc/guides/contributing/patches.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + .. submitting_patches: Contributing Code to DPDK diff --git a/doc/guides/contributing/stable.rst b/doc/guides/contributing/stable.rst index 0f2f1f3..16518b0 100644 --- a/doc/guides/contributing/stable.rst +++ b/doc/guides/contributing/stable.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + .. stable_lts_releases: DPDK Stable Releases and Long Term Support diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst index c495294d..6e2f073 100644 --- a/doc/guides/contributing/versioning.rst +++ b/doc/guides/contributing/versioning.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + Managing ABI updates diff --git a/doc/guides/linux_gsg/nic_perf_intel_platform.rst b/doc/guides/linux_gsg/nic_perf_intel_platform.rst index 987cd0a..7bd05b6 100644 --- a/doc/guides/linux_gsg/nic_perf_intel_platform.rst +++ b/doc/guides/linux_gsg/nic_perf_intel_platform.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright(c) 2018 Intel Corporation + How to get best performance with NICs on Intel platforms diff --git a/doc/guides/rel_notes/deprec
[dpdk-dev] [PATCH dpdk-next-net] net/axgbe: fix an assignment error in axgbe_dev_info_get()
This patch fixes a tirvial error in assigning max Rx/Tx queues in axgbe_dev_info_get() of the axgbe PMD driver. Signed-off-by: Rami Rosen --- drivers/net/axgbe/axgbe_ethdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c index 07c1337ac..a9a9fb570 100644 --- a/drivers/net/axgbe/axgbe_ethdev.c +++ b/drivers/net/axgbe/axgbe_ethdev.c @@ -355,8 +355,8 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct axgbe_port *pdata = dev->data->dev_private; dev_info->pci_dev = RTE_ETH_DEV_TO_PCI(dev); - dev_info->max_rx_queues = pdata->tx_ring_count; - dev_info->max_tx_queues = pdata->rx_ring_count; + dev_info->max_rx_queues = pdata->rx_ring_count; + dev_info->max_tx_queues = pdata->tx_ring_count; dev_info->min_rx_bufsize = AXGBE_RX_MIN_BUF_SIZE; dev_info->max_rx_pktlen = AXGBE_RX_MAX_BUF_SIZE; dev_info->max_mac_addrs = AXGBE_MAX_MAC_ADDRS; -- 2.14.3
Re: [dpdk-dev] [PATCH] net/mlx5: fix link status initialization
On Mon, Apr 09, 2018 at 12:28:04PM +, Shahaf Shuler wrote:
> Monday, April 9, 2018 11:28 AM, Nélio Laranjeiro:
> > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> >
> > On Sun, Apr 08, 2018 at 01:09:27PM +, Shahaf Shuler wrote:
> > > Thursday, April 5, 2018 9:51 AM, Nélio Laranjeiro:
> > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> > > >
> > > > On Thu, Apr 05, 2018 at 05:35:57AM +, Shahaf Shuler wrote:
> > > > > Wednesday, April 4, 2018 3:11 PM, Nélio Laranjeiro:
> > > > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> > > > > >
> > > > > > On Wed, Apr 04, 2018 at 09:58:33AM +, Shahaf Shuler wrote:
> > > > > > > Wednesday, April 4, 2018 10:30 AM, Nélio Laranjeiro:
> > > > > > > > Subject: Re: [PATCH] net/mlx5: fix link status
> > > > > > > > initialization
> > > > > > > >
> > > > > > > > On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote:
> > >
> > > [..]
> > >
> > > > > >
> > > > > > According to your analysis, this is only necessary when the LCS
> > > > > > is configured in the device. Why not adding this call to
> > > > > > mlx5_dev_interrupt_handler_install() which is responsible to
> > > > > > install the LCS callback.
> > > > >
> > > > > I think it is good practice whether or not LSC is set.
> > > > > The link status should be initialized to the correct value after the
> > > > > probe.
> > > >
> > > > There is no guarantee the link will be accurate, at probe time the
> > > > link may be up so internal information has a status up with a speed with
> > this patch.
> > > > The application probes a second port, in the mean time the link of
> > > > the first port goes down, the interrupt is still not installed and
> > > > the internal status becomes wrong (still up whereas the port is down).
> > > >
> > > > Finally at start, the device installs the handler, but the link is
> > > > still down whereas internally it is up, the application will call
> > > > rte_eth_link_get_nowait() which will directly copy the wrong
> > > > internal status to the application.
> > >
> > > This is not correct.
> > > Using Verbs, the async_fd on which link status interrupts are reported is
> > created on probe.
> > > Even if the interrupt handler is not installed, interrupts still
> > > trigger on this fd. They will be processed when the interrupt handler
> > > will be installed as part of the port start.
> > > So in fact you have the whole trace on the link status changes waiting
> > > to be processed upon port start.
> >
> > Right, but in such case, this patch still does not solves the issue.
> > Until the dev_start() is called, the link may be inconsistent with the real
> > status.
> >
> > example:
> >
> > pci_probe --> link is up.
> > leaving pci probe, the link goes downs --> internally the PMD has a link
> > up.
> >
> > Until the dev_start() is called any call to rte_eth_link_get_nowait() will
> > copy
> > the internal PMD status which is not accurate.
> >
> > From this point, the issue seems to fully come from
> > rte_eth_link_get_nowait() which should not make any copy until the port is
> > not started, until then the link may be inconsistent between the driver and
> > the device.
>
> Right. However fixing it only on the ethdev layer is not enough.
>
> Assume the link was up from the beginning.
> For the case were the call for rte_eth_link_get_nowait happens only
> after the port start, the link will still be wrong as it will be
> reported as down (and no interrupt to fix it).
>
> So to summarize I plan to have this patch (with modification of the
> wait_to_complete) along with another fix for ethdev.
> Do we have an agreement?
Sorry but no, as MLX5 is not the only PMD impacted by this issue and a
solution can be done for all of them (ixgbe, virtio, MLX5 I did not
check the others).
Your proposition does not solves the following case neither, when the
link goes down and the application stops the port due to that, if the
application waits using the same API to restart the port, it will never
occur as the interrupts handlers are no more installed and thus the
internal link will never be updated remaining indefinitely down. This
also impact the three PMD above.
Commit b77d21cc2364 ("ethdev: add link status get/set helper functions")
has been integrated a little too fast introducing this bug.
All those issues can be fixed by adding another statement in the if of
the function rte_eth_link_get_nowait()
- if (dev->data->dev_conf.intr_conf.lsc)
+ if (dev->data->dev_conf.intr_conf.lsc && dev->data->dev_started)
rte_eth_linkstatus_get(dev, eth_link);
else {
RTE_FUNC_PTR_OR_RET(*dev->dev_ops->link_update);
(*dev->dev_ops->link_update)(dev, 1);
*eth_link = dev->data->dev_link;
}
Doing this fixes all PMD having the same bug, and remove your specific
patch for MLX5 which becomes useless.
Regards,
--
Nélio Laranjeiro
6WIND
Re: [dpdk-dev] OPDL and 18.02 Release Notes
On 05 Mar 17:58, Ferruh Yigit wrote: > On 2/9/2018 12:08 AM, Rosen, Rami wrote: > > Hi all, > > Following the recent announcement of DPDK 18.02-RC4, I went over > > 18.02 release notes and I have this minor query which I am not sure about: > > In the release notes: > > http://dpdk.org/doc/guides/rel_notes/release_18_02.html > > we have the following: > > ... > > The OPDL (Ordered Packet Distribution Library) eventdev > > ... > > > > While in http://dpdk.org/dev/roadmap > > We have: > > > > eventdev optimized packet distribution library (OPDL) driver > > ... > > > > So I am not sure about this inconsistency -should it be "optimized" or > > "ordered" ? > > According driver documentation (doc/guides/eventdevs/opdl.rst) it is: > "Ordered Packet Distribution Library", release notes seems correct. > > cc'ed maintainers. Hi Ferruh, Team agree stay with "Ordered Packet Distribution Library" name so far. the roadmap information might need update. Many thanks Liang > > > > > Regards, > > Rami Rosen > > > > >
Re: [dpdk-dev] [PATCH 0/6] enable easier app compilation testing with meson
On Fri, Apr 06, 2018 at 01:10:54PM +0100, Van Haaren, Harry wrote: > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson > > Sent: Thursday, March 29, 2018 2:55 PM > > To: dev@dpdk.org > > Cc: hemant.agra...@nxp.com; shreyansh.j...@nxp.com; Richardson, Bruce > > > > Subject: [dpdk-dev] [PATCH 0/6] enable easier app compilation testing with > > meson > > > > Summary: > > With this set you can test building all applicable examples by > > calling meson with "-Dexamples=all" > > > > When building DPDK with meson, it's possible to specify a list of sample > > apps to have built along with the main code. However, specifying a full > > list of all apps can be unwieldy, so this set adds support for passing > > "all" as the examples to be built. > > > > With "all", meson just adds all subdirectories of "examples" to the build, > > so the first few patches are ensuring that we don't get an error by > > attempting to build an unsupported application. On linux, only 7 apps were > > unsupported, in that they had not been given a meson.build file. On > > FreeBSD, a few others had to have their meson.build files updated to report > > them as unsupported. > > > > In terms of behaviour, the meson.build file for each app will report if the > > app can be built or not. If "all" is requested, then a message is printed > > and the meson run can continue. If, however, the app is requested by name, > > then an error is reported and the meson run halts. > > > > The final two patches in the series are more cleanup, the former improves > > error reporting, while the last patch is a performance improvement. Meson > > runs quickly enough in the normal case, but with a full set of examples, > > the dependency chain resolution can slow things down. Reducing the lists of > > dependencies makes a noticable difference in this case. [NOTE: this > > slowness and speedup only applies to the meson run; the actual build using > > ninja is as fast as ever!] > > > > Bruce Richardson (6): > > examples: add empty meson files for unsupported examples > > examples/l2fwd-cat: make build dependent on pqos library > > examples: disable unsupported examples on BSD > > examples: allow building all examples as part of meson build > > examples: improve error report for missing meson deps > > drivers/dpaa*: reduce meson dependency lists > > Working fine for me here! > > Series-Tested-by: Harry van Haaren > Series applied to dpdk-next-build. >From testing, there appears to be an issue with building ip_pipeline sample app for a generic armv8 platform, but not for the thunderx one. Ideally, if someone from the arm community can help fix this in the C code , it would be great, otherwise I'll try and do up a patch to selectively disable ip_pipeline for platforms where it can't compile. /Bruce
Re: [dpdk-dev] [PATCH v1] doc: add SPDX Licence to doc files
On 4/9/2018 2:11 PM, Marko Kovacevic wrote: > Added SPDX headers to doc files to have them aligned with > the other doc files. > > Signed-off-by: Marko Kovacevic <...> > doc/guides/rel_notes/release_16_04.rst | 3 +++ > doc/guides/rel_notes/release_16_07.rst | 3 +++ > doc/guides/rel_notes/release_16_11.rst | 3 +++ > doc/guides/rel_notes/release_17_02.rst | 3 +++ > doc/guides/rel_notes/release_17_05.rst | 3 +++ > doc/guides/rel_notes/release_17_08.rst | 3 +++ > doc/guides/rel_notes/release_17_11.rst | 3 +++ > doc/guides/rel_notes/release_18_02.rst | 3 +++ > doc/guides/rel_notes/release_2_2.rst | 3 +++ Hi Hemant, What does it mean to have a license header in release notes? It looks unnecessary from both license and copyright point of view, is there a legal requirement for it? Indeed I was thinking removing the header from the release notes that has it... Thanks, ferruh
Re: [dpdk-dev] [PATCH dpdk-next-net] net/axgbe: fix an assignment error in axgbe_dev_info_get()
>This patch fixes a tirvial error in assigning max Rx/Tx queues in >axgbe_dev_info_get() of the axgbe PMD driver. > >Signed-off-by: Rami Rosen >--- > drivers/net/axgbe/axgbe_ethdev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/drivers/net/axgbe/axgbe_ethdev.c >b/drivers/net/axgbe/axgbe_ethdev.c >index 07c1337ac..a9a9fb570 100644 >--- a/drivers/net/axgbe/axgbe_ethdev.c >+++ b/drivers/net/axgbe/axgbe_ethdev.c >@@ -355,8 +355,8 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, > struct axgbe_port *pdata = dev->data->dev_private; > > dev_info->pci_dev = RTE_ETH_DEV_TO_PCI(dev); >- dev_info->max_rx_queues = pdata->tx_ring_count; >- dev_info->max_tx_queues = pdata->rx_ring_count; >+ dev_info->max_rx_queues = pdata->rx_ring_count; >+ dev_info->max_tx_queues = pdata->tx_ring_count; > dev_info->min_rx_bufsize = AXGBE_RX_MIN_BUF_SIZE; > dev_info->max_rx_pktlen = AXGBE_RX_MAX_BUF_SIZE; > dev_info->max_mac_addrs = AXGBE_MAX_MAC_ADDRS; >-- >2.14.3 > Thanks a lot Remi. Wonderful catch. Regards, Ravi
Re: [dpdk-dev] [PATCH v2] net/i40e: fix flow RSS queue index check error
> -Original Message-
> From: Zhao1, Wei
> Sent: Monday, April 9, 2018 4:38 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Zhang, Qi Z ; Zhao1, Wei
>
> Subject: [PATCH v2] net/i40e: fix flow RSS queue index check error
>
> There is an error in queue index check for RSS queue region configuration.If
> the queue index is not continuous sequence for RSS, but queue region index is
> continuous sequence, in this case we can not use the old method for queue
> index check.
> This patch also add comment for flow rss parse function in order to explain
> some important info.
>
> Fixes: ecad87d22383 ("net/i40e: move RSS to flow API")
> Signed-off-by: Wei Zhao
> Tested-by: Peng Yuan
>
> ---
>
> v2:
> -merge this with another patch for function comment, add git log info.
> ---
> drivers/net/i40e/i40e_flow.c | 32 +---
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> f4d08bb..40f15c2 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -4171,6 +4171,19 @@ i40e_flow_parse_rss_pattern(__rte_unused struct
> rte_eth_dev *dev,
> return 0;
> }
>
> +/**
> + * This function is used to parse rss queue index, total queue number
> +and
> + * hash functions, If the purpose of this configuration is for queue
> +region
> + * configuration, it will set queue_region_conf flag to TRUE, else to FALSE.
> + * In queue region configuration, it also need to parse hardware
> +flowtype
> + * and user_priority from configuration, it will also cheeck the
> +validity
> + * of these parameters. For example, The queue region sizes should
> + * be any of the following values: 1, 2, 4, 8, 16, 32, 64, the
> + * hw_flowtype or PCTYPE max index should be 63, the user priority
> + * max index should be 7, and so on. And also, queue index should be
> + * continuous sequence and queue region index should be part of rss
> + * queue index for this port.
> + */
> static int
> i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
> const struct rte_flow_action *actions, @@ -4240,6
> +4253,14 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
> return -rte_errno;
> }
> }
> +
> + if (rss_info->num < rss->num) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> + act,
> + "no valid queues");
> + return -rte_errno;
> + }
> }
>
> for (n = 0; n < conf_info->queue_region_number; n++) { @@ -4264,17
> +4285,6 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
> return -rte_errno;
> }
>
> - if (rss_info->num < rss->num ||
> - rss->queue[0] < rss_info->queue[0] ||
> - (rss->queue[0] + rss->num >
> - rss_info->num + rss_info->queue[0])) {
> - rte_flow_error_set(error, EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - act,
> - "no valid queues");
> - return -rte_errno;
> - }
Is this a code clean?
I didn't see anything wrong with the code you removed, but they just looks
redundant.
Btw why you still check (rss_info->num < rss->num) ?,
previous, I think you already iterate all rss queue to make sure they are
matched by a rss_info queue
so it already proved rss_info->num >= rss->num.
> for (i = 0; i < info->queue_region_number; i++) {
> if (info->region[i].queue_num == rss->num &&
> info->region[i].queue_start_index ==
> --
> 2.7.5
Re: [dpdk-dev] [PATCH dpdk-next-net] net/axgbe: fix an assignment error in axgbe_dev_info_get()
On 4/9/2018 2:56 PM, Kumar, Ravi1 wrote: >> This patch fixes a tirvial error in assigning max Rx/Tx queues in >> axgbe_dev_info_get() of the axgbe PMD driver. >> >> Signed-off-by: Rami Rosen >> --- >> drivers/net/axgbe/axgbe_ethdev.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/axgbe/axgbe_ethdev.c >> b/drivers/net/axgbe/axgbe_ethdev.c >> index 07c1337ac..a9a9fb570 100644 >> --- a/drivers/net/axgbe/axgbe_ethdev.c >> +++ b/drivers/net/axgbe/axgbe_ethdev.c >> @@ -355,8 +355,8 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, >> struct axgbe_port *pdata = dev->data->dev_private; >> >> dev_info->pci_dev = RTE_ETH_DEV_TO_PCI(dev); >> -dev_info->max_rx_queues = pdata->tx_ring_count; >> -dev_info->max_tx_queues = pdata->rx_ring_count; >> +dev_info->max_rx_queues = pdata->rx_ring_count; >> +dev_info->max_tx_queues = pdata->tx_ring_count; >> dev_info->min_rx_bufsize = AXGBE_RX_MIN_BUF_SIZE; >> dev_info->max_rx_pktlen = AXGBE_RX_MAX_BUF_SIZE; >> dev_info->max_mac_addrs = AXGBE_MAX_MAC_ADDRS; >> -- >> 2.14.3 >> > > Thanks a lot Remi. Wonderful catch. I am adding your explicit ack for the patch: Acked-by: Ravi Kumar > > Regards, > Ravi >
Re: [dpdk-dev] [PATCH] net/mlx5: fix link status initialization
On Mon, Apr 09, 2018 at 03:26:41PM +0200, Nélio Laranjeiro wrote:
> On Mon, Apr 09, 2018 at 12:28:04PM +, Shahaf Shuler wrote:
> > Monday, April 9, 2018 11:28 AM, Nélio Laranjeiro:
> > > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> > >
> > > On Sun, Apr 08, 2018 at 01:09:27PM +, Shahaf Shuler wrote:
> > > > Thursday, April 5, 2018 9:51 AM, Nélio Laranjeiro:
> > > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> > > > >
> > > > > On Thu, Apr 05, 2018 at 05:35:57AM +, Shahaf Shuler wrote:
> > > > > > Wednesday, April 4, 2018 3:11 PM, Nélio Laranjeiro:
> > > > > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> > > > > > >
> > > > > > > On Wed, Apr 04, 2018 at 09:58:33AM +, Shahaf Shuler wrote:
> > > > > > > > Wednesday, April 4, 2018 10:30 AM, Nélio Laranjeiro:
> > > > > > > > > Subject: Re: [PATCH] net/mlx5: fix link status
> > > > > > > > > initialization
> > > > > > > > >
> > > > > > > > > On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote:
> > > >
> > > > [..]
> > > >
> > > > > > >
> > > > > > > According to your analysis, this is only necessary when the LCS
> > > > > > > is configured in the device. Why not adding this call to
> > > > > > > mlx5_dev_interrupt_handler_install() which is responsible to
> > > > > > > install the LCS callback.
> > > > > >
> > > > > > I think it is good practice whether or not LSC is set.
> > > > > > The link status should be initialized to the correct value after
> > > > > > the probe.
> > > > >
> > > > > There is no guarantee the link will be accurate, at probe time the
> > > > > link may be up so internal information has a status up with a speed
> > > > > with
> > > this patch.
> > > > > The application probes a second port, in the mean time the link of
> > > > > the first port goes down, the interrupt is still not installed and
> > > > > the internal status becomes wrong (still up whereas the port is down).
> > > > >
> > > > > Finally at start, the device installs the handler, but the link is
> > > > > still down whereas internally it is up, the application will call
> > > > > rte_eth_link_get_nowait() which will directly copy the wrong
> > > > > internal status to the application.
> > > >
> > > > This is not correct.
> > > > Using Verbs, the async_fd on which link status interrupts are reported
> > > > is
> > > created on probe.
> > > > Even if the interrupt handler is not installed, interrupts still
> > > > trigger on this fd. They will be processed when the interrupt handler
> > > > will be installed as part of the port start.
> > > > So in fact you have the whole trace on the link status changes waiting
> > > > to be processed upon port start.
> > >
> > > Right, but in such case, this patch still does not solves the issue.
> > > Until the dev_start() is called, the link may be inconsistent with the
> > > real
> > > status.
> > >
> > > example:
> > >
> > > pci_probe --> link is up.
> > > leaving pci probe, the link goes downs --> internally the PMD has a link
> > > up.
> > >
> > > Until the dev_start() is called any call to rte_eth_link_get_nowait()
> > > will copy
> > > the internal PMD status which is not accurate.
> > >
> > > From this point, the issue seems to fully come from
> > > rte_eth_link_get_nowait() which should not make any copy until the port is
> > > not started, until then the link may be inconsistent between the driver
> > > and
> > > the device.
> >
> > Right. However fixing it only on the ethdev layer is not enough.
> >
> > Assume the link was up from the beginning.
> > For the case were the call for rte_eth_link_get_nowait happens only
> > after the port start, the link will still be wrong as it will be
> > reported as down (and no interrupt to fix it).
> >
> > So to summarize I plan to have this patch (with modification of the
> > wait_to_complete) along with another fix for ethdev.
> > Do we have an agreement?
>
> Sorry but no, as MLX5 is not the only PMD impacted by this issue and a
> solution can be done for all of them (ixgbe, virtio, MLX5 I did not
> check the others).
>
> Your proposition does not solves the following case neither, when the
> link goes down and the application stops the port due to that, if the
> application waits using the same API to restart the port, it will never
> occur as the interrupts handlers are no more installed and thus the
> internal link will never be updated remaining indefinitely down. This
> also impact the three PMD above.
>
> Commit b77d21cc2364 ("ethdev: add link status get/set helper functions")
> has been integrated a little too fast introducing this bug.
>
> All those issues can be fixed by adding another statement in the if of
> the function rte_eth_link_get_nowait()
>
> - if (dev->data->dev_conf.intr_conf.lsc)
> + if (dev->data->dev_conf.intr_conf.lsc && dev->data->dev_started)
> rte_eth_linkstatus_get(dev, eth_link);
> else {
> RTE_FUNC_PTR_
Re: [dpdk-dev] OPDL and 18.02 Release Notes
On 4/9/2018 2:43 PM, Liang, Ma wrote: > On 05 Mar 17:58, Ferruh Yigit wrote: >> On 2/9/2018 12:08 AM, Rosen, Rami wrote: >>> Hi all, >>> Following the recent announcement of DPDK 18.02-RC4, I went over >>> 18.02 release notes and I have this minor query which I am not sure about: >>> In the release notes: >>> http://dpdk.org/doc/guides/rel_notes/release_18_02.html >>> we have the following: >>> ... >>> The OPDL (Ordered Packet Distribution Library) eventdev >>> ... >>> >>> While in http://dpdk.org/dev/roadmap >>> We have: >>> >>> eventdev optimized packet distribution library (OPDL) driver >>> ... >>> >>> So I am not sure about this inconsistency -should it be "optimized" or >>> "ordered" ? >> >> According driver documentation (doc/guides/eventdevs/opdl.rst) it is: >> "Ordered Packet Distribution Library", release notes seems correct. >> >> cc'ed maintainers. > Hi Ferruh, >Team agree stay with "Ordered Packet Distribution Library" name so far. >the roadmap information might need update. Hi Liang, Thanks for the clarification, as long as documentation is correct nothing needs to be done. Thanks, ferruh
Re: [dpdk-dev] OPDL and 18.02 Release Notes
Hi Liang, > Team agree stay with "Ordered Packet Distribution Library" name so far. > the roadmap information might need update Actually, now there is no need to update the roadmap, as OPDL does not appear there at all. (Back when I originally sent the first mail in this thread, it referred to the roadmap where it appeared as Optimized and not Ordered, but now there is a new roadmap of 18.05). Regards, Rami -Original Message- From: Ma, Liang J Sent: Monday, April 09, 2018 16:43 To: Yigit, Ferruh Cc: Rosen, Rami ; dev@dpdk.org; tho...@monjalon.net; Mccarthy, Peter Subject: Re: [dpdk-dev] OPDL and 18.02 Release Notes On 05 Mar 17:58, Ferruh Yigit wrote: > On 2/9/2018 12:08 AM, Rosen, Rami wrote: > > Hi all, > > Following the recent announcement of DPDK 18.02-RC4, I went over > > 18.02 release notes and I have this minor query which I am not sure about: > > In the release notes: > > http://dpdk.org/doc/guides/rel_notes/release_18_02.html > > we have the following: > > ... > > The OPDL (Ordered Packet Distribution Library) eventdev ... > > > > While in http://dpdk.org/dev/roadmap We have: > > > > eventdev optimized packet distribution library (OPDL) driver ... > > > > So I am not sure about this inconsistency -should it be "optimized" or > > "ordered" ? > > According driver documentation (doc/guides/eventdevs/opdl.rst) it is: > "Ordered Packet Distribution Library", release notes seems correct. > > cc'ed maintainers. Hi Ferruh, Team agree stay with "Ordered Packet Distribution Library" name so far. the roadmap information might need update. Many thanks Liang > > > > > Regards, > > Rami Rosen > > > > >
Re: [dpdk-dev] [PATCH] rawdev: add to meson build
On Wednesday 04 April 2018 03:42 PM, Bruce Richardson wrote:
Add librte_rawdev to the meson build of DPDK.
Signed-off-by: Bruce Richardson
---
config/rte_config.h | 3 +++
lib/librte_rawdev/meson.build | 6 ++
lib/meson.build | 2 +-
3 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_rawdev/meson.build
Tested-by: Shreyansh Jain
diff --git a/config/rte_config.h b/config/rte_config.h
index 72c0aa2d2..83109a65c 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -56,6 +56,9 @@
#define RTE_EVENT_MAX_DEVS 16
#define RTE_EVENT_MAX_QUEUES_PER_DEV 64
+/* rawdev defines */
+#define RTE_RAWDEV_MAX_DEVS 10
+
/* ip_fragmentation defines */
#define RTE_LIBRTE_IP_FRAG_MAX_FRAG 4
#undef RTE_LIBRTE_IP_FRAG_TBL_STAT
diff --git a/lib/librte_rawdev/meson.build b/lib/librte_rawdev/meson.build
new file mode 100644
index 0..dcd37ad49
--- /dev/null
+++ b/lib/librte_rawdev/meson.build
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+allow_experimental_apis = true
+sources = files('rte_rawdev.c')
+headers = files('rte_rawdev.h', 'rte_rawdev_pmd.h')
diff --git a/lib/meson.build b/lib/meson.build
index ef6159170..17edc4da4 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -18,7 +18,7 @@ libraries = [ 'compat', # just a header, used for versioning
'distributor', 'efd', 'eventdev',
'gro', 'gso', 'ip_frag', 'jobstats',
'kni', 'latencystats', 'lpm', 'member',
- 'meter', 'power', 'pdump',
+ 'meter', 'power', 'pdump', 'rawdev',
'reorder', 'sched', 'security', 'timer', 'vhost',
# add pkt framework libs which use other libs from above
'port', 'table', 'pipeline',
Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add group counter support to rte_flow
Hi Adrien,
On 06/04/2018 21:26, Adrien Mazarguil wrote:
On Fri, Apr 06, 2018 at 01:24:00PM +0100, Declan Doherty wrote:
Add new RTE_FLOW_ACTION_TYPE_GROUP_COUNT action type to enable shared
counters across multiple flows on a single port or across multiple
flows on multiple ports within the same switch domain.
Introduce new API rte_flow_query_group_count to allow querying of group
counters.
Signed-off-by: Declan Doherty
Both features are definitely needed, however I suggest to enhance the
existing action type and query function instead, given the rte_flow ABI
won't be maintained for the 18.05 release [1].
Counters and query support were defined as a kind of PoC in preparation for
future requirements back in DPDK 17.02 and so far few PMDs have implemented
the query callback (mlx5 and failsafe, and the latter isn't really a PMD).
Due to the behavior change of action lists [2], providing an action type as
a query parameter is not specific enough anymore, for instance if a list
contains multiple COUNT, the application should be able to tell which needs
to be queried.
Therefore I suggest to redefine the query function as follows:
int
rte_flow_query(uint16_t port_id,
struct rte_flow *flow,
const struct rte_flow_action *action,
void *data,
struct rte_flow_error *error);
Third argument is an action definition with the same configuration (if any)
as previously defined in the action list originally used to create the flow
rule (not necessarily the same pointer, only the contents matter).
It means two perfectly identical actions can't be distinguished, and that's
how group counters will work.
Instead of adding a new action type to distinguish groups, a configuration
structure is added to the existing RTE_FLOW_ACTION_TYPE_COUNT, with
non-shared counters as a default behavior:
struct rte_flow_action_count {
uint32_t shared:1; /**< Share counter ID with other flow rules. */
uint32_t reserved:31; /**< Reserved, must be zero. */
uint32_t id; /**< Counter ID. */
};
Doing so will impact some existing code in mlx5 and librte_flow_classify,
but that shouldn't be much of an issue.
Keep in mind testpmd and its documentation must be updated as well.
Thoughts?
Please correct me if I am wrong but I think we are talking two different
things here.
If I understood you correctly, you are proposing to pass a list of
actions (instead if a action type) in the third parameter to perform
multiple actions in the same query call. Lets take an example for 100
ingress flows. So, if we want to query the counter for all the flows, we
can get them by a single query providing a list (of size 100) of
action_count in the 3rd param.
On the other hand, we are saying that all the flows are belongs to same
tunnel end-point (we are talking only 1 TEP here), then the PMD will be
responsible to increment the counter of TEP for matching all the flows
(100 flows). So, using one group query by passing one action_count in
3rd param, we can get the count of the TEP.
This case is generic enough for sure for simple flows but may not be
suitable for tunnel cases, as application needs to track the counters
for all the flows, and needs to build the list of action each time the
flows added/deleted.
A few nits below for the sake of commenting.
[1] "Flow API overhaul for switch offloads"
http://dpdk.org/ml/archives/dev/2018-April/095774.html
[2] "ethdev: alter behavior of flow API actions"
http://dpdk.org/ml/archives/dev/2018-April/095779.html
---
doc/guides/prog_guide/rte_flow.rst | 35 +
lib/librte_ether/rte_ethdev_version.map | 8 +
lib/librte_ether/rte_flow.c | 21 +
lib/librte_ether/rte_flow.h | 56 -
lib/librte_ether/rte_flow_driver.h | 6
5 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/doc/guides/prog_guide/rte_flow.rst
b/doc/guides/prog_guide/rte_flow.rst
index 961943d..fd33d19 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1698,6 +1698,41 @@ Return values:
- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
Unnecessary empty line.
I will take into account all the comments.
Regards,
Awal.
+Group Count Query
+~
+
+Query group counter which can be associated with multiple flows on a specified
+port.
+
+This function allows retrieving of group counters. A group counter is a
+counter which can be shared among multiple flows on a single port or among
+multiple flows on multiple ports within the same switch domain. Data is
+gathered by special actions which must be present in the flow rule
+definition.
+
+.. code-block:: c
+
+ int
+ rte_flow_query_group_count(uint16_t port_id,
+ uint32_t group_counter_id,
+ struct rte_flow_quer
[dpdk-dev] [PATCH 2/2] net/dpaa2: Changes to support ethdev offload APIs
Signed-off-by: Sunil Kumar Kori
---
drivers/net/dpaa2/dpaa2_ethdev.c | 63 +---
drivers/net/dpaa2/dpaa2_rxtx.c | 32 +++-
2 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 281483d..acf5f1a 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -172,16 +172,24 @@ dpaa2_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
DEV_RX_OFFLOAD_IPV4_CKSUM |
DEV_RX_OFFLOAD_UDP_CKSUM |
DEV_RX_OFFLOAD_TCP_CKSUM |
- DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
+ DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_VLAN_FILTER |
+ DEV_RX_OFFLOAD_VLAN_STRIP |
+ DEV_RX_OFFLOAD_JUMBO_FRAME |
+ DEV_RX_OFFLOAD_SCATTER;
dev_info->tx_offload_capa =
DEV_TX_OFFLOAD_IPV4_CKSUM |
DEV_TX_OFFLOAD_UDP_CKSUM |
DEV_TX_OFFLOAD_TCP_CKSUM |
DEV_TX_OFFLOAD_SCTP_CKSUM |
- DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
+ DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
+ DEV_TX_OFFLOAD_VLAN_INSERT |
+ DEV_TX_OFFLOAD_MBUF_FAST_FREE |
+ DEV_TX_OFFLOAD_MULTI_SEGS;
dev_info->speed_capa = ETH_LINK_SPEED_1G |
ETH_LINK_SPEED_2_5G |
ETH_LINK_SPEED_10G;
+ dev_info->default_rxconf.rx_drop_en = true;
}
static int
@@ -268,12 +276,33 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
struct dpaa2_dev_priv *priv = dev->data->dev_private;
struct fsl_mc_io *dpni = priv->hw;
struct rte_eth_conf *eth_conf = &dev->data->dev_conf;
- int rx_ip_csum_offload = false;
+ struct rte_eth_dev_info dev_info;
+ uint64_t rx_offloads = eth_conf->rxmode.offloads;
+ uint64_t tx_offloads = eth_conf->txmode.offloads;
+ int rx_l3_csum_offload = false;
+ int rx_l4_csum_offload = false;
+ int tx_l3_csum_offload = false;
+ int tx_l4_csum_offload = false;
int ret;
PMD_INIT_FUNC_TRACE();
- if (eth_conf->rxmode.jumbo_frame == 1) {
+ dpaa2_dev_info_get(dev, &dev_info);
+ if (dev_info.rx_offload_capa != rx_offloads) {
+ DPAA2_PMD_ERR("Some Rx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ rx_offloads, dev_info.rx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (dev_info.tx_offload_capa != tx_offloads) {
+ DPAA2_PMD_ERR("Some Tx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ tx_offloads, dev_info.tx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
if (eth_conf->rxmode.max_rx_pkt_len <= DPAA2_MAX_RX_PKT_LEN) {
ret = dpni_set_max_frame_length(dpni, CMD_PRI_LOW,
priv->token, eth_conf->rxmode.max_rx_pkt_len);
@@ -297,32 +326,44 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
}
}
- if (eth_conf->rxmode.hw_ip_checksum)
- rx_ip_csum_offload = true;
+ if (rx_offloads & DEV_RX_OFFLOAD_IPV4_CKSUM)
+ rx_l3_csum_offload = true;
+
+ if ((rx_offloads & DEV_RX_OFFLOAD_UDP_CKSUM) ||
+ (rx_offloads & DEV_RX_OFFLOAD_TCP_CKSUM))
+ rx_l4_csum_offload = true;
ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
- DPNI_OFF_RX_L3_CSUM, rx_ip_csum_offload);
+ DPNI_OFF_RX_L3_CSUM, rx_l3_csum_offload);
if (ret) {
DPAA2_PMD_ERR("Error to set RX l3 csum:Error = %d", ret);
return ret;
}
ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
- DPNI_OFF_RX_L4_CSUM, rx_ip_csum_offload);
+ DPNI_OFF_RX_L4_CSUM, rx_l4_csum_offload);
if (ret) {
DPAA2_PMD_ERR("Error to get RX l4 csum:Error = %d", ret);
return ret;
}
+ if (tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM)
+ tx_l3_csum_offload = true;
+
+ if ((tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) ||
+ (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) ||
+ (tx_offloads & DEV_TX_OFFLOAD_SCTP_CKSUM))
+ tx_l4_csum_offload = true;
+
ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
- DPNI_OFF_TX_L3_CSUM, true);
+ DPNI_OFF_TX_L3_CSUM, tx_l3_csum_offload);
if (ret) {
DPAA2_PMD_ERR("Error to set TX l3 csum:Error = %d", ret);
return ret;
}
[dpdk-dev] [PATCH 1/2] net/dpaa: Changes to support ethdev offload APIs
Signed-off-by: Sunil Kumar Kori
---
drivers/net/dpaa/dpaa_ethdev.c | 46 ++
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index db49364..efef62c 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -95,6 +95,9 @@ static const struct rte_dpaa_xstats_name_off
dpaa_xstats_strings[] = {
static struct rte_dpaa_driver rte_dpaa_pmd;
+static void
+dpaa_eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info);
+
static inline void
dpaa_poll_queue_default_config(struct qm_mcc_initfq *opts)
{
@@ -134,13 +137,42 @@ dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
}
static int
-dpaa_eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
+dpaa_eth_dev_configure(struct rte_eth_dev *dev)
{
struct dpaa_if *dpaa_intf = dev->data->dev_private;
+ struct rte_eth_conf *eth_conf = &dev->data->dev_conf;
+ struct rte_eth_dev_info dev_info;
+ uint64_t rx_offloads = eth_conf->rxmode.offloads;
+ uint64_t tx_offloads = eth_conf->txmode.offloads;
PMD_INIT_FUNC_TRACE();
- if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
+ dpaa_eth_dev_info(dev, &dev_info);
+ if (dev_info.rx_offload_capa != rx_offloads) {
+ DPAA_PMD_ERR("Some Rx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ rx_offloads, dev_info.rx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (dev_info.tx_offload_capa != tx_offloads) {
+ DPAA_PMD_ERR("Some Tx offloads are not supported "
+ "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+ tx_offloads, dev_info.tx_offload_capa);
+ return -ENOTSUP;
+ }
+
+ if (((rx_offloads & DEV_RX_OFFLOAD_IPV4_CKSUM) == 0) ||
+ ((rx_offloads & DEV_RX_OFFLOAD_UDP_CKSUM) == 0) ||
+ ((rx_offloads & DEV_RX_OFFLOAD_TCP_CKSUM) == 0) ||
+ ((tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM) == 0) ||
+ ((tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) == 0) ||
+ ((tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) == 0)) {
+ DPAA_PMD_ERR(" Cksum offloading is enabled by default "
+ " Cannot be disabled. So ignoring this configuration ");
+ }
+
+ if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
if (dev->data->dev_conf.rxmode.max_rx_pkt_len <=
DPAA_MAX_RX_PKT_LEN) {
fman_if_set_maxfrm(dpaa_intf->fif,
@@ -259,11 +291,17 @@ static void dpaa_eth_dev_info(struct rte_eth_dev *dev,
dev_info->rx_offload_capa =
(DEV_RX_OFFLOAD_IPV4_CKSUM |
DEV_RX_OFFLOAD_UDP_CKSUM |
- DEV_RX_OFFLOAD_TCP_CKSUM);
+ DEV_RX_OFFLOAD_TCP_CKSUM) |
+ DEV_RX_OFFLOAD_JUMBO_FRAME |
+ DEV_RX_OFFLOAD_SCATTER;
dev_info->tx_offload_capa =
(DEV_TX_OFFLOAD_IPV4_CKSUM |
DEV_TX_OFFLOAD_UDP_CKSUM |
- DEV_TX_OFFLOAD_TCP_CKSUM);
+ DEV_TX_OFFLOAD_TCP_CKSUM) |
+ DEV_TX_OFFLOAD_MBUF_FAST_FREE |
+ DEV_TX_OFFLOAD_MULTI_SEGS;
+
+ dev_info->default_rxconf.rx_drop_en = true;
}
static int dpaa_eth_link_update(struct rte_eth_dev *dev,
--
2.9.3
Re: [dpdk-dev] [PATCH v5 4/4] testpmd: make use of per-PMD TxRx parameters
On 09/04/2018 13:55, Shreyansh Jain wrote: [..] Documentation for burst mode changes to testpmd would need an update. I guess, only when the user explicitly sets 'set burst 0' would the driver defaults be picked up - isn't it? Yes. Maybe something like this: --->8--- --- a/doc/guides/testpmd_app_ug/run_app.rst +++ b/doc/guides/testpmd_app_ug/run_app.rst @@ -372,7 +372,9 @@ The commandline options are: * ``--burst=N`` Set the number of packets per burst to N, where 1 <= N <= 512. -The default value is 16. +The default value is 32. +If set to 0, driver default is used if defined. Else, if driver default +is not defined, default of 32 is used. * ``--mbcache=N`` --->8--- In the above, I think the existing documented default value needs to be changed. It is set to '#define DEF_PKT_BURST 32' Had a quick look and it looks like that discrepancy has been there since the documentation was converted to .rst in 2014. If you add that, please use my ack for next revision. (For patch 1/4, I had already given my Ack in v2) I'll add in the snippet above. 18.05 integration deadline has I think passed, but documentation changes will still get in.
[dpdk-dev] [PATCH] build: fix default arm64 build instruction level support
The make based build system has crc+crypto instruction
support for the default arm64 build.
http://dpdk.org/browse/dpdk/tree/mk/machine/armv8a/rte.vars.mk#n31
This patch fixes the disparity with meson build flags for armv8.
As a bonus, This patch fixes the following errors with
ip_pipeline example application.
Assembler messages:
Error: selected processor does not support `crc32cx w3,w3,x0'
Fixes: c6e536e38437 ("build: add more implementers IDs and PNs for ARM")
Cc: herbert.g...@arm.com
Cc: pbhagavat...@caviumnetworks.com
Cc: bruce.richard...@intel.com
Cc: hemant.agra...@nxp.com
Cc: sta...@dpdk.org
Signed-off-by: Jerin Jacob
---
config/arm/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config/arm/meson.build b/config/arm/meson.build
index c1ab6ed01..b1d53576d 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -8,7 +8,7 @@ march_opt = '-march=@0@'.format(machine)
arm_force_native_march = false
machine_args_generic = [
- ['default', ['-march=armv8-a']],
+ ['default', ['-march=armv8-a+crc+crypto']],
['native', ['-march=native']],
['0xd03', ['-mcpu=cortex-a53']],
['0xd04', ['-mcpu=cortex-a35']],
--
2.17.0
Re: [dpdk-dev] [PATCH v2 08/15] ethdev: add hash function to RSS flow API action
On Fri, Apr 06, 2018 at 06:41:35PM +0300, Andrew Rybchenko wrote:
> On 04/06/2018 04:25 PM, Adrien Mazarguil wrote:
> > By definition, RSS involves some kind of hash algorithm, usually Toeplitz.
> >
> > Until now it could not be modified on a flow rule basis and PMDs had to
> > always assume RTE_ETH_HASH_FUNCTION_DEFAULT, which remains the default
> > behavior when unspecified (0).
> >
> > This breaks ABI compatibility for the following public functions:
> >
> > - rte_flow_copy()
> > - rte_flow_create()
> > - rte_flow_query()
> > - rte_flow_validate()
> >
> > Signed-off-by: Adrien Mazarguil
> > Cc: Ferruh Yigit
> > Cc: Thomas Monjalon
> > Cc: Wenzhuo Lu
> > Cc: Jingjing Wu
> > Cc: Beilei Xing
> > Cc: Qi Zhang
> > Cc: Konstantin Ananyev
> > Cc: Nelio Laranjeiro
> > Cc: Yongseok Koh
> > Cc: Andrew Rybchenko
> > Cc: Pascal Mazon
> > ---
> > app/test-pmd/cmdline_flow.c | 72
> > app/test-pmd/config.c | 1 +
> > doc/guides/prog_guide/rte_flow.rst | 2 +
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 +
> > drivers/net/e1000/igb_flow.c| 4 ++
> > drivers/net/e1000/igb_rxtx.c| 4 +-
> > drivers/net/i40e/i40e_ethdev.c | 4 +-
> > drivers/net/i40e/i40e_flow.c| 4 ++
> > drivers/net/ixgbe/ixgbe_flow.c | 4 ++
> > drivers/net/ixgbe/ixgbe_rxtx.c | 4 +-
> > drivers/net/mlx4/mlx4_flow.c| 7 +++
> > drivers/net/mlx5/mlx5_flow.c| 13 +
> > drivers/net/sfc/sfc_flow.c | 3 +
> > drivers/net/tap/tap_flow.c | 6 ++
> > lib/librte_ether/rte_flow.c | 1 +
> > lib/librte_ether/rte_flow.h | 2 +
> > 16 files changed, 131 insertions(+), 3 deletions(-)
>
> <...>
>
> > diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
> > index 1a2c0299c..dbe4c2baa 100644
> > --- a/drivers/net/sfc/sfc_flow.c
> > +++ b/drivers/net/sfc/sfc_flow.c
> > @@ -1261,6 +1261,9 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> > rxq_hw_index_max = rxq->hw_index;
> > }
> > + if (rss->func)
>
> May be it is better to compare with RTE_ETH_HASH_FUNCTION_DEFAULT
> explicitly? I think it is more readable. If so, it is applicable to all
> similar checks
> in the patch.
Good suggestion, although RTE_ETH_HASH_FUNCTION_DEFAULT can't be anything
other than 0 for various reasons, I'll clarify (most of) the code in my next
update.
> In the case of sfc, please, allow RTE_ETH_HASH_FUNCTION_TOEPLITZ as well.
> I'd suggest:
> switch (rss->func) {
> case RTE_ETH_HASH_FUNCTION_DEFAULT:
> case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
> break;
> default:
> return -EINVAL;
> }
I'll add it, thanks.
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [PATCH v2 10/15] ethdev: refine TPID handling in flow API
On Fri, Apr 06, 2018 at 08:11:38PM +0300, Andrew Rybchenko wrote: > On 04/06/2018 04:25 PM, Adrien Mazarguil wrote: > > TPID handling in rte_flow VLAN and E_TAG pattern item definitions is not > > consistent with the normal stacking order of pattern items, which is > > confusing to applications. > > > > Problem is that when followed by one of these layers, the EtherType field > > of the preceding layer keeps its "inner" definition, and the "outer" TPID > > is provided by the subsequent layer, the reverse of how a packet looks like > > on the wire: > > > > Wire: [ ETH TPID = A | VLAN EtherType = B | B DATA ] > > rte_flow: [ ETH EtherType = B | VLAN TPID = A | B DATA ] > > > > Worse, when QinQ is involved, the stacking order of VLAN layers is > > unspecified. It is unclear whether it should be reversed (innermost to > > outermost) as well given TPID applies to the previous layer: > > > > Wire: [ ETH TPID = A | VLAN TPID = B | VLAN EtherType = C | C DATA ] > > rte_flow 1: [ ETH EtherType = C | VLAN TPID = B | VLAN TPID = A | C DATA ] > > rte_flow 2: [ ETH EtherType = C | VLAN TPID = A | VLAN TPID = B | C DATA ] > > > > While specifying EtherType/TPID is hopefully rarely necessary, the stacking > > order in case of QinQ and the lack of documentation remain an issue. > > > > This patch replaces TPID in the VLAN pattern item with an inner > > EtherType/TPID as is usually done everywhere else (e.g. struct vlan_hdr), > > clarifies documentation and updates all relevant code. > > > > It breaks ABI compatibility for the following public functions: > > > > - rte_flow_copy() > > - rte_flow_create() > > - rte_flow_query() > > - rte_flow_validate() > > > > Summary of changes for PMDs that implement ETH, VLAN or E_TAG pattern > > items: > > > > - bnxt: EtherType matching is supported, and vlan->inner_type overrides > >eth->type if the latter has standard TPID value 0x8100, otherwise an > >error is triggered. > > > > - e1000: EtherType matching is only supported with the ETHERTYPE filter, > >which does not support VLAN matching, therefore no impact. > > > > - enic: same as bnxt. > > > > - i40e: same as bnxt with a configurable TPID value for the FDIR filter, > >with existing limitations on allowed EtherType values. The remaining > >filter types (VXLAN, NVGRE, QINQ) do not support EtherType matching. > > > > - ixgbe: same as e1000, with additional minor change to rely on the new > >E-Tag macro definition. > > > > - mlx4: EtherType/TPID matching is not supported, no impact. > > > > - mlx5: same as bnxt. > > > > - mrvl: EtherType matching is supported but eth->type cannot be specified > >when a VLAN item is present. However vlan->inner_type is used if > >specified. > > > > - sfc: same as bnxt with QinQ TPID value 0x88a8 additionally supported. > > > > - tap: same as bnxt. > > > > Signed-off-by: Adrien Mazarguil > > Cc: Ferruh Yigit > > Cc: Thomas Monjalon > > Cc: Wenzhuo Lu > > Cc: Jingjing Wu > > Cc: Ajit Khaparde > > Cc: Somnath Kotur > > Cc: John Daley > > Cc: Hyong Youb Kim > > Cc: Beilei Xing > > Cc: Qi Zhang > > Cc: Konstantin Ananyev > > Cc: Nelio Laranjeiro > > Cc: Yongseok Koh > > Cc: Tomasz Duszynski > > Cc: Dmitri Epshtein > > Cc: Natalie Samsonov > > Cc: Jianbo Liu > > Cc: Andrew Rybchenko > > Cc: Pascal Mazon > > > > --- > > > > Hi PMD maintainers, while I'm pretty confident in these changes, I could > > not validate them with all devices. > > > > It would be great if you could apply this patch, run testpmd, create VLAN > > flow rules with/without inner EtherType as described and send matching > > traffic while making sure nothing was broken in the process. > > > > Thanks! > > --- > > app/test-pmd/cmdline_flow.c | 17 +++--- > > doc/guides/nics/tap.rst | 2 +- > > doc/guides/prog_guide/rte_flow.rst | 21 ++-- > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +- > > drivers/net/bnxt/bnxt_filter.c | 39 +++--- > > drivers/net/enic/enic_flow.c| 22 +--- > > drivers/net/i40e/i40e_flow.c| 69 +++- > > drivers/net/ixgbe/ixgbe_ethdev.c| 3 +- > > drivers/net/mlx5/mlx5_flow.c| 16 +- > > drivers/net/mvpp2/mrvl_flow.c | 27 +++--- > > drivers/net/sfc/sfc_flow.c | 28 ++ > > drivers/net/tap/tap_flow.c | 16 -- > > lib/librte_ether/rte_flow.h | 24 ++--- > > lib/librte_net/rte_ether.h | 1 + > > 14 files changed, 229 insertions(+), 60 deletions(-) > > <...> > > > diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c > > index bc4974edf..f61d4ec92 100644 > > --- a/drivers/net/sfc/sfc_flow.c > > +++ b/drivers/net/sfc/sfc_flow.c > > @@ -7,6 +7,7 @@ > >* for Solarflare) and Solarflare Communications, Inc. > >*/ > > +#include > >
Re: [dpdk-dev] [PATCH v2 07/15] ethdev: flatten RSS configuration in flow API
On Sat, Apr 07, 2018 at 12:05:51PM +0300, Andrew Rybchenko wrote:
> On 04/06/2018 04:25 PM, Adrien Mazarguil wrote:
> > Since its inception, the rte_flow RSS action has been relying in part on
> > external struct rte_eth_rss_conf for compatibility with the legacy RSS API.
> > This structure lacks parameters such as the hash algorithm to use, and more
> > recently, a method to tell which layer RSS should be performed on [1].
> >
> > Given struct rte_eth_rss_conf will never be flexible enough to represent a
> > complete RSS configuration (e.g. RETA table), this patch supersedes it by
> > extending the rte_flow RSS action directly.
> >
> > A subsequent patch will add a field to use a non-default RSS hash
> > algorithm. To that end, a field named "types" replaces the field formerly
> > known as "rss_hf" and standing for "RSS hash functions" as it was
> > confusing. Actual RSS hash function types are defined by enum
> > rte_eth_hash_function.
> > This patch updates all PMDs and example applications accordingly.
> >
> > It breaks ABI compatibility for the following public functions:
> >
> > - rte_flow_copy()
> > - rte_flow_create()
> > - rte_flow_query()
> > - rte_flow_validate()
> >
> > [1] commit 676b605182a5 ("doc: announce ethdev API change for RSS
> > configuration")
> >
> > Signed-off-by: Adrien Mazarguil
> > Cc: Xueming Li
> > Cc: Ferruh Yigit
> > Cc: Thomas Monjalon
> > Cc: Wenzhuo Lu
> > Cc: Jingjing Wu
> > Cc: Beilei Xing
> > Cc: Qi Zhang
> > Cc: Konstantin Ananyev
> > Cc: Nelio Laranjeiro
> > Cc: Yongseok Koh
> > Cc: Andrew Rybchenko
> > Cc: Pascal Mazon
> > Cc: Radu Nicolau
> > Cc: Akhil Goyal
> > ---
> > app/test-pmd/cmdline_flow.c| 59 +-
> > app/test-pmd/config.c | 39 +++
> > doc/guides/prog_guide/rte_flow.rst | 22 ++--
> > drivers/net/e1000/e1000_ethdev.h | 13 ++-
> > drivers/net/e1000/igb_ethdev.c | 4 +-
> > drivers/net/e1000/igb_flow.c | 31 ++---
> > drivers/net/e1000/igb_rxtx.c | 51 +++--
> > drivers/net/i40e/i40e_ethdev.c | 53 +++--
> > drivers/net/i40e/i40e_ethdev.h | 15 ++-
> > drivers/net/i40e/i40e_flow.c | 47
> > drivers/net/ixgbe/ixgbe_ethdev.c | 4 +-
> > drivers/net/ixgbe/ixgbe_ethdev.h | 13 ++-
> > drivers/net/ixgbe/ixgbe_flow.c | 30 ++---
> > drivers/net/ixgbe/ixgbe_rxtx.c | 51 +++--
> > drivers/net/mlx4/mlx4.c| 2 +-
> > drivers/net/mlx4/mlx4_flow.c | 61 +-
> > drivers/net/mlx4/mlx4_flow.h | 2 +-
> > drivers/net/mlx4/mlx4_rxq.c| 2 +-
> > drivers/net/mlx4/mlx4_rxtx.h | 2 +-
> > drivers/net/mlx5/mlx5_flow.c | 193 +++-
> > drivers/net/mlx5/mlx5_rxq.c| 22 ++--
> > drivers/net/mlx5/mlx5_rxtx.h | 26 +++--
> > drivers/net/sfc/sfc_flow.c | 21 ++--
> > drivers/net/tap/tap_flow.c | 8 +-
> > examples/ipsec-secgw/ipsec.c | 10 +-
> > lib/librte_ether/rte_flow.c| 39 +++
> > lib/librte_ether/rte_flow.h| 6 +-
> > 27 files changed, 473 insertions(+), 353 deletions(-)
>
> <...>
>
> > diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
> > index 056405515..1a2c0299c 100644
> > --- a/drivers/net/sfc/sfc_flow.c
> > +++ b/drivers/net/sfc/sfc_flow.c
> > @@ -1234,13 +1234,11 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> > struct sfc_rxq *rxq;
> > unsigned int rxq_hw_index_min;
> > unsigned int rxq_hw_index_max;
> > - const struct rte_eth_rss_conf *rss_conf = rss->rss_conf;
> > - uint64_t rss_hf;
> > - uint8_t *rss_key = NULL;
> > + const uint8_t *rss_key;
> > struct sfc_flow_rss *sfc_rss_conf = &flow->rss_conf;
> > unsigned int i;
> > - if (rss->num == 0)
> > + if (rss->queue_num == 0)
> > return -EINVAL;
> > rxq_sw_index = sa->rxq_count - 1;
> > @@ -1248,7 +1246,7 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> > rxq_hw_index_min = rxq->hw_index;
> > rxq_hw_index_max = 0;
> > - for (i = 0; i < rss->num; ++i) {
> > + for (i = 0; i < rss->queue_num; ++i) {
> > rxq_sw_index = rss->queue[i];
> > if (rxq_sw_index >= sa->rxq_count)
> > @@ -1263,15 +1261,14 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> > rxq_hw_index_max = rxq->hw_index;
> > }
> > - rss_hf = (rss_conf != NULL) ? rss_conf->rss_hf : SFC_RSS_OFFLOADS;
>
> Here we had a fallback to default rss_hf (now types) if rss_conf is
> unspecified.
Thing is, rss_action->conf was never supposed to be NULL in the first
place. Crashing on a NULL configuration has always been fine, but until
recently prevented validation with testpmd's broken implementation. This
problem was addressed in a prior series [1][2][3].
Since a value is now always provided, no need for a fallback.
[1] "app/testpmd: fix lack of flow action configuration"
http://dpdk.org/ml/archives/dev/2018-April/095280.html
[
Re: [dpdk-dev] [PATCH v2 12/15] ethdev: update behavior of VF/PF in flow API
On Sat, Apr 07, 2018 at 12:41:17PM +0300, Andrew Rybchenko wrote: > On 04/06/2018 04:25 PM, Adrien Mazarguil wrote: > > Contrary to all other pattern items, these are inconsistently documented as > > affecting traffic instead of simply matching its origin, without provision > > for the latter. > > > > This commit clarifies documentation and updates PMDs since the original > > behavior now has to be explicitly requested using the new transfer > > attribute. > > > > It breaks ABI compatibility for the following public functions: > > > > - rte_flow_create() > > - rte_flow_validate() > > > > Impacted PMDs are bnxt and i40e, for which the VF pattern item is now only > > supported when a transfer attribute is also present. > > > > Signed-off-by: Adrien Mazarguil > > Cc: Ajit Khaparde > > Cc: Somnath Kotur > > Cc: Beilei Xing > > Cc: Qi Zhang > > --- > > app/test-pmd/cmdline_flow.c | 12 +++--- > > doc/guides/prog_guide/rte_flow.rst | 36 +- > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 12 +++--- > > drivers/net/bnxt/bnxt_filter.c | 22 ++- > > drivers/net/i40e/i40e_flow.c| 23 +++- > > lib/librte_ether/rte_flow.h | 47 ++-- > > 6 files changed, 77 insertions(+), 75 deletions(-) > > <...> > > > diff --git a/doc/guides/prog_guide/rte_flow.rst > > b/doc/guides/prog_guide/rte_flow.rst > > index 735ce6323..beedc713b 100644 > > --- a/doc/guides/prog_guide/rte_flow.rst > > +++ b/doc/guides/prog_guide/rte_flow.rst > > @@ -518,15 +518,12 @@ Usage example, matching non-TCPv4 packets only: > > Item: ``PF`` > > > > -Matches packets addressed to the physical function of the device. > > +Matches traffic originating from (ingress) or going to (egress) the > > physical > > +function of the current device. > > Not sure that I understand above. It looks like ingress and egress are > misplaced. > There many similar cases below. In this API, "ingress" and "egress" are always defined as relative to the application creating the flow rule. In that sense they are respectively synonyms for "from" and "to". I agree they are not properly defined in this document, in fact ingress and egress were clarified (with diagrams and all) in the RFC submitted prior to this patch [1]. I will update the "Attribute: Traffic direction" section in my next update. [1] See "Traffic direction" in http://dpdk.org/ml/archives/dev/2018-March/092513.html -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
> -Original Message-
> From: Richardson, Bruce
> Sent: Monday, April 9, 2018 2:09 PM
> To: Singh, Jasvinder
> Cc: dev@dpdk.org; Dumitrescu, Cristian
> Subject: Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
>
> On Mon, Apr 09, 2018 at 01:49:48PM +0100, Jasvinder Singh wrote:
> > Fix build error with gcc 8.0 due to cast between function types.
> > Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
> >
> > Signed-off-by: Jasvinder Singh
>
> What's the actual error message? Why do the types not match?
>
> /Bruce
Error log is captured below;
CC rte_table_hash_cuckoo.o
/librte_table/rte_table_hash_cuckoo.c: In function
'rte_table_hash_cuckoo_create':
/librte_table/rte_table_hash_cuckoo.c:110:16: error: cast between incompatible
function types from 'rte_table_hash_op_hash' {aka 'long unsigned int (*)(void
*, void *, unsigned int, long unsigned int)'}
to 'uint32_t (*)(const void *, uint32_t, uint32_t)' {aka 'unsigned int
(*)(const void *, unsigned int, unsigned int)'} [-Werror=cast-function-type]
.hash_func = (rte_hash_function) p->f_hash,
^
cc1: all warnings being treated as errors
Re: [dpdk-dev] [PATCH v2 14/15] ethdev: add physical port action to flow API
On Sat, Apr 07, 2018 at 12:51:40PM +0300, Andrew Rybchenko wrote: > On 04/06/2018 04:25 PM, Adrien Mazarguil wrote: > > This patch adds the missing action counterpart to the PHY_PORT pattern > > item, that is, the ability to directly inject matching traffic into a > > physical port of the underlying device. > > Does it mean that if it is applied on ingress (incoming packet from network) > it will simply send packets back to network (specified physical port)? Precisely. > And if it is applied on egress (outgoing from device to network) it will > be directed to possibly different physical port and sent to network. Right, note it gives applications the ability to express that wish, the fact PMDs support it is another matter :) In any case, this action is added for API completeness but should be rarely necessary since we chose to go with port representors. Port representors will expose valid DPDK port IDs, therefore applications will simply have to create ingress/egress flow rules on the right DPDK port targeting different port IDs through the PORT_ID action. > > It breaks ABI compatibility for the following public functions: > > > > - rte_flow_copy() > > - rte_flow_create() > > - rte_flow_query() > > - rte_flow_validate() > > > > Signed-off-by: Adrien Mazarguil > > Cc: "Zhang, Qi Z" > > --- > > app/test-pmd/cmdline_flow.c | 35 > > app/test-pmd/config.c | 1 + > > doc/guides/prog_guide/rte_flow.rst | 20 ++ > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 5 > > lib/librte_ether/rte_flow.c | 1 + > > lib/librte_ether/rte_flow.h | 22 +++ > > 6 files changed, 84 insertions(+) > > <...> -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] Question on documentation / Mellanox ConnectX-3
On Sun, Apr 08, 2018 at 06:27:50PM -0300, Marcelo Ricardo Leitner wrote: > On Thu, Apr 05, 2018 at 11:26:21AM +0200, Adrien Mazarguil wrote: > > On Tue, Apr 03, 2018 at 02:59:38PM -0300, Marcelo Ricardo Leitner wrote: > > > Hi, > > > > > > http://docs.openvswitch.org/en/latest/howto/dpdk/ says: > > > > > > Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address > > > associated with multiple ports. Using a PCI device like above won’t > > > work. Instead, below usage is suggested: > > > > > > $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \ > > > options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01" > > > $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \ > > > options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02" > > > > > > But these MACs are 7 bytes long. Seems the idea was to mention the two > > > incremental MAC addresses that the ports have, and thus the ':55' > > > should have been removed from there, right? > > > > > > Reading the code, it doesn't seem prepared to handle the extra byte in > > > any (special) way. > > > > After a quick glance at the original patch [1], I confirm it looks like a > > mistake in the OVS documentation. MAC addresses should be 6 bytes long, the > > 7th byte is not a workaround to identify a physical port like I initially > > thought. > > > > As you pointed out, since default MAC addresses on mlx4 ports are normally > > incremental, documentation should read something like: > > > > $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \ > > options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55" > > $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \ > > options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56" > > > > [1] https://github.com/openvswitch/ovs/commit/5e7588186839 > > Thanks Adrien. Question now is, who can fix the doc? Someone should submit a request or patch pointing to this thread to the documentation maintainer of OVS. Since it's a different project there's not much I can do from the DPDK side. -- Adrien Mazarguil 6WIND
Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
On Mon, 9 Apr 2018 13:49:48 +0100
Jasvinder Singh wrote:
> Fix build error with gcc 8.0 due to cast between function types.
> Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
>
> Signed-off-by: Jasvinder Singh
> ---
> lib/librte_table/rte_table_hash_cuckoo.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_table/rte_table_hash_cuckoo.c
> b/lib/librte_table/rte_table_hash_cuckoo.c
> index dcb4fe9..f7eae27 100644
> --- a/lib/librte_table/rte_table_hash_cuckoo.c
> +++ b/lib/librte_table/rte_table_hash_cuckoo.c
> @@ -103,11 +103,13 @@ rte_table_hash_cuckoo_create(void *params,
> return NULL;
> }
>
> + void *hash_func = p->f_hash;
> +
> /* Create cuckoo hash table */
> struct rte_hash_parameters hash_cuckoo_params = {
> .entries = p->n_keys,
> .key_len = p->key_size,
> - .hash_func = (rte_hash_function)(p->f_hash),
> + .hash_func = (rte_hash_function) hash_func,
> .hash_func_init_val = p->seed,
> .socket_id = socket_id,
> .name = p->name
This is just tricking the compiler into not complaining.
I would really rather see the two hash functions made the same.
Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add group counter support to rte_flow
Hi Mohammad,
On Mon, Apr 09, 2018 at 03:22:45PM +0100, Mohammad Abdul Awal wrote:
> Hi Adrien,
>
>
> On 06/04/2018 21:26, Adrien Mazarguil wrote:
> > On Fri, Apr 06, 2018 at 01:24:00PM +0100, Declan Doherty wrote:
> > > Add new RTE_FLOW_ACTION_TYPE_GROUP_COUNT action type to enable shared
> > > counters across multiple flows on a single port or across multiple
> > > flows on multiple ports within the same switch domain.
> > >
> > > Introduce new API rte_flow_query_group_count to allow querying of group
> > > counters.
> > >
> > > Signed-off-by: Declan Doherty
> > Both features are definitely needed, however I suggest to enhance the
> > existing action type and query function instead, given the rte_flow ABI
> > won't be maintained for the 18.05 release [1].
> >
> > Counters and query support were defined as a kind of PoC in preparation for
> > future requirements back in DPDK 17.02 and so far few PMDs have implemented
> > the query callback (mlx5 and failsafe, and the latter isn't really a PMD).
> >
> > Due to the behavior change of action lists [2], providing an action type as
> > a query parameter is not specific enough anymore, for instance if a list
> > contains multiple COUNT, the application should be able to tell which needs
> > to be queried.
> >
> > Therefore I suggest to redefine the query function as follows:
> >
> > int
> > rte_flow_query(uint16_t port_id,
> > struct rte_flow *flow,
> > const struct rte_flow_action *action,
> > void *data,
> > struct rte_flow_error *error);
> >
> > Third argument is an action definition with the same configuration (if any)
> > as previously defined in the action list originally used to create the flow
> > rule (not necessarily the same pointer, only the contents matter).
> >
> > It means two perfectly identical actions can't be distinguished, and that's
> > how group counters will work.
> > Instead of adding a new action type to distinguish groups, a configuration
> > structure is added to the existing RTE_FLOW_ACTION_TYPE_COUNT, with
> > non-shared counters as a default behavior:
> >
> > struct rte_flow_action_count {
> > uint32_t shared:1; /**< Share counter ID with other flow rules. */
> > uint32_t reserved:31; /**< Reserved, must be zero. */
> > uint32_t id; /**< Counter ID. */
> > };
> >
> > Doing so will impact some existing code in mlx5 and librte_flow_classify,
> > but that shouldn't be much of an issue.
> >
> > Keep in mind testpmd and its documentation must be updated as well.
> >
> > Thoughts?
> Please correct me if I am wrong but I think we are talking two different
> things here.
> If I understood you correctly, you are proposing to pass a list of actions
> (instead if a action type) in the third parameter to perform multiple
> actions in the same query call. Lets take an example for 100 ingress flows.
> So, if we want to query the counter for all the flows, we can get them by a
> single query providing a list (of size 100) of action_count in the 3rd
> param.
Whoa no! I'm only suggesting a pointer to a single action as a replacement
for the basic action type, not a *list* of actions. I hope this addresses
all concerns :)
The fact the action in question would refer to a shared counter (see struct
above) with a given ID would be enough to make all counters with the same ID
refer to the same internal PMD object.
> On the other hand, we are saying that all the flows are belongs to same
> tunnel end-point (we are talking only 1 TEP here), then the PMD will be
> responsible to increment the counter of TEP for matching all the flows (100
> flows). So, using one group query by passing one action_count in 3rd param,
> we can get the count of the TEP.
>
> This case is generic enough for sure for simple flows but may not be
> suitable for tunnel cases, as application needs to track the counters for
> all the flows, and needs to build the list of action each time the flows
> added/deleted.
I think we're on the same page. I'm only suggesting to define a
configuration structure for the COUNT action (which currently doesn't take
any) as a replacement for GROUP_COUNT, and tweak the query callback to be
able to tell a specific counter to query instead of adding a new query
callback and leaving the existing one broken by design.
> > A few nits below for the sake of commenting.
> >
> > [1] "Flow API overhaul for switch offloads"
> > http://dpdk.org/ml/archives/dev/2018-April/095774.html
> > [2] "ethdev: alter behavior of flow API actions"
> > http://dpdk.org/ml/archives/dev/2018-April/095779.html
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
On Mon, Apr 09, 2018 at 03:51:10PM +0100, Singh, Jasvinder wrote:
>
> > -Original Message-
> > From: Richardson, Bruce
> > Sent: Monday, April 9, 2018 2:09 PM
> > To: Singh, Jasvinder
> > Cc: dev@dpdk.org; Dumitrescu, Cristian
> > Subject: Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
> >
> > On Mon, Apr 09, 2018 at 01:49:48PM +0100, Jasvinder Singh wrote:
> > > Fix build error with gcc 8.0 due to cast between function types.
> > > Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
> > >
> > > Signed-off-by: Jasvinder Singh
> >
> > What's the actual error message? Why do the types not match?
> >
> > /Bruce
>
> Error log is captured below;
>
> CC rte_table_hash_cuckoo.o
> /librte_table/rte_table_hash_cuckoo.c: In function
> 'rte_table_hash_cuckoo_create':
> /librte_table/rte_table_hash_cuckoo.c:110:16: error: cast between incompatible
> function types from 'rte_table_hash_op_hash' {aka 'long unsigned int
> (*)(void *, void *, unsigned int, long unsigned int)'}
> to 'uint32_t (*)(const void *, uint32_t, uint32_t)' {aka 'unsigned int
> (*)(const void *, unsigned int, unsigned int)'} [-Werror=cast-function-type]
>.hash_func = (rte_hash_function) p->f_hash,
> ^
> cc1: all warnings being treated as errors
>
Even if the compiler isn't complaining now, how can that cast work? Looking
at the error message given, it appears there are two big issues:
1. The expected function call takes 3 parameters:
(const void *, uint32_t, uint32_t),
but you are giving it a function that takes 4 parameters:
(void *, void *, unsigned int, long unsigned int)
2. The return type expected is "unsigned int", but you are giving a
function returning "long unsigned int". On 32-bit systems, these are
going to be the same size, but on 64-bit, they will be different.
Similarly for the last function argument.
Is the error message correct?
/Bruce
[dpdk-dev] [PATCH] lib/librte_vhost: fix bugs
Fixes: 256b132f41b2 ("vhost/crypto: add session message handler")
Fixes: 7b5ad7beee17 ("vhost/crypto: update makefile")
Fixes: 2ce5bd8c442d ("examples/vhost_crypto: add vhost crypto sample
application")
This patch fixes the bugs introduced in the above patches.
Signed-off-by: Fan Zhang
---
examples/vhost_crypto/main.c| 5 -
lib/librte_vhost/Makefile | 3 ++-
lib/librte_vhost/vhost_crypto.c | 8
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
index bc867240d..860200e29 100644
--- a/examples/vhost_crypto/main.c
+++ b/examples/vhost_crypto/main.c
@@ -95,11 +95,6 @@ parse_cryptodev_id(const char *q_arg)
/* parse decimal string */
pm = strtoul(q_arg, &end, 10);
- if ((pm == '\0') || (end == NULL) || (*end != '\0')) {
- RTE_LOG(ERR, USER1, "Invalid Cryptodev ID %s\n", q_arg);
- return -1;
- }
-
if (pm > rte_cryptodev_count()) {
RTE_LOG(ERR, USER1, "Invalid Cryptodev ID %s\n", q_arg);
return -1;
diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
index 2cc65f95e..92c267475 100644
--- a/lib/librte_vhost/Makefile
+++ b/lib/librte_vhost/Makefile
@@ -18,7 +18,8 @@ LDLIBS += -lpthread
ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
LDLIBS += -lnuma
endif
-LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net
+LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net \
+ -lrte_cryptodev -lrte_hash
# all source are stored in SRCS-y
SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c \
diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index d84513e7f..2acfc908c 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -381,7 +381,7 @@ vhost_crypto_create_sess(struct vhost_crypto *vcrypto,
return;
}
- VC_LOG_DBG("Session (key %lu, session %p) created.",
+ VC_LOG_DBG("Session (key %llu, session %p) created.",
vcrypto->last_session_id, session);
sess_param->session_id = vcrypto->last_session_id;
@@ -399,7 +399,7 @@ vhost_crypto_close_sess(struct vhost_crypto *vcrypto,
uint64_t session_id)
(void **)&session);
if (unlikely(ret < 0)) {
- VC_LOG_ERR("Failed to delete session (key %lu).", session_id);
+ VC_LOG_ERR("Failed to delete session (key %llu).", session_id);
return -VIRTIO_CRYPTO_INVSESS;
}
@@ -418,7 +418,7 @@ vhost_crypto_close_sess(struct vhost_crypto *vcrypto,
uint64_t session_id)
return -VIRTIO_CRYPTO_ERR;
}
- VC_LOG_DBG("Session (key %lu, session %p) deleted.", sess_id,
+ VC_LOG_DBG("Session (key %llu, session %p) deleted.", sess_id,
session);
return 0;
@@ -932,7 +932,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
&session_id, (void **)&session);
if (unlikely(err < 0)) {
err = VIRTIO_CRYPTO_ERR;
- VC_LOG_ERR("Failed to retrieve session id %lu",
+ VC_LOG_ERR("Failed to retrieve session id %llu",
session_id);
goto error_exit;
}
--
2.13.6
[dpdk-dev] [PATCH] lib/librte_vhost: remove packet dump
This patch removes unnecessary packet dump for debugging.
Signed-off-by: Fan Zhang
---
lib/librte_vhost/vhost_crypto.c | 36
1 file changed, 36 deletions(-)
diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index d84513e7f..d86af0532 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -578,21 +578,12 @@ write_back_data(struct rte_crypto_op *op, struct
vhost_crypto_data_req *vc_req)
left -= to_write;
src_data += to_write;
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- printf("desc addr %llu len %u:", desc->addr, desc->len);
- rte_hexdump(stdout, "", dst, to_write);
-#endif
-
while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
desc = &head[desc->next];
rte_prefetch0(&head[desc->next]);
to_write = RTE_MIN(desc->len, (uint32_t)left);
dst = GPA_TO_VVA(uint8_t *, mem, desc->addr);
rte_memcpy(dst, src_data, to_write);
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- printf("desc addr %llu len %u:", desc->addr, desc->len);
- rte_hexdump(stdout, "DST:", dst, to_write);
-#endif
left -= to_write;
src_data += to_write;
}
@@ -626,10 +617,6 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct
rte_crypto_op *op,
goto error_exit;
}
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- rte_hexdump(stdout, "IV:", iv_data, cipher->para.iv_len);
-#endif
-
m_src->data_len = cipher->para.src_data_len;
switch (vcrypto->option) {
@@ -664,11 +651,6 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct
rte_crypto_op *op,
goto error_exit;
}
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- rte_hexdump(stdout, "SRC:", rte_pktmbuf_mtod(m_src, void *),
- cipher->para.src_data_len);
-#endif
-
/* dst */
desc = find_write_desc(head, desc);
if (unlikely(!desc)) {
@@ -746,9 +728,6 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct
rte_crypto_op *op,
goto error_exit;
}
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- rte_hexdump(stdout, "IV:", iv_data, chain->para.iv_len);
-#endif
m_src->data_len = chain->para.src_data_len;
m_dst->data_len = chain->para.dst_data_len;
@@ -782,11 +761,6 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct
rte_crypto_op *op,
goto error_exit;
}
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- rte_hexdump(stdout, "SRC:", rte_pktmbuf_mtod(m_src, void *),
- chain->para.src_data_len);
-#endif
-
/* dst */
desc = find_write_desc(head, desc);
if (unlikely(!desc)) {
@@ -846,11 +820,6 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct
rte_crypto_op *op,
goto error_exit;
}
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- rte_hexdump(stdout, "Digest:", op->sym->auth.digest.data,
- chain->para.hash_result_len);
-#endif
-
/* record inhdr */
vc_req->inhdr = get_data_ptr(head, mem, &desc, INHDR_LEN);
if (unlikely(vc_req->inhdr == NULL)) {
@@ -1016,11 +985,6 @@ vhost_crypto_finalize_one_request(struct rte_crypto_op
*op,
}
}
-#ifdef RTE_LIBRTE_VHOST_DEBUG
- rte_hexdump(stdout, "DST:", rte_pktmbuf_mtod(op->sym->m_dst, uint8_t *),
- m_dst->data_len);
-#endif
-
vc_req->vq->used->ring[desc_idx].id = desc_idx;
vc_req->vq->used->ring[desc_idx].len = vc_req->len;
--
2.13.6
Re: [dpdk-dev] [PATCH] build: fix default arm64 build instruction level support
On Mon, Apr 09, 2018 at 08:09:46PM +0530, Jerin Jacob wrote:
> The make based build system has crc+crypto instruction
> support for the default arm64 build.
> http://dpdk.org/browse/dpdk/tree/mk/machine/armv8a/rte.vars.mk#n31
>
> This patch fixes the disparity with meson build flags for armv8.
> As a bonus, This patch fixes the following errors with
> ip_pipeline example application.
>
> Assembler messages:
> Error: selected processor does not support `crc32cx w3,w3,x0'
>
> Fixes: c6e536e38437 ("build: add more implementers IDs and PNs for ARM")
>
> Cc: herbert.g...@arm.com
> Cc: pbhagavat...@caviumnetworks.com
> Cc: bruce.richard...@intel.com
> Cc: hemant.agra...@nxp.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Jerin Jacob
> ---
For the issue with ip_pipeline builds:
Tested-by: Bruce Richardson
Re: [dpdk-dev] [PATCH] lib/librte_vhost: fix bugs
Hi Fan,
On 04/09/2018 05:34 PM, Fan Zhang wrote:
Fixes: 256b132f41b2 ("vhost/crypto: add session message handler")
Fixes: 7b5ad7beee17 ("vhost/crypto: update makefile")
Fixes: 2ce5bd8c442d ("examples/vhost_crypto: add vhost crypto sample
application")
This patch fixes the bugs introduced in the above patches.
Signed-off-by: Fan Zhang
---
examples/vhost_crypto/main.c| 5 -
lib/librte_vhost/Makefile | 3 ++-
lib/librte_vhost/vhost_crypto.c | 8
3 files changed, 6 insertions(+), 10 deletions(-)
Can you please split the patch in 3 parts, one for each patches it
fixes?
As my branch hasn't been picked yest y Ferruh, I can squash them
directly.
Thanks,
Maxime
diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
index bc867240d..860200e29 100644
--- a/examples/vhost_crypto/main.c
+++ b/examples/vhost_crypto/main.c
@@ -95,11 +95,6 @@ parse_cryptodev_id(const char *q_arg)
/* parse decimal string */
pm = strtoul(q_arg, &end, 10);
- if ((pm == '\0') || (end == NULL) || (*end != '\0')) {
- RTE_LOG(ERR, USER1, "Invalid Cryptodev ID %s\n", q_arg);
- return -1;
- }
-
if (pm > rte_cryptodev_count()) {
RTE_LOG(ERR, USER1, "Invalid Cryptodev ID %s\n", q_arg);
return -1;
diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
index 2cc65f95e..92c267475 100644
--- a/lib/librte_vhost/Makefile
+++ b/lib/librte_vhost/Makefile
@@ -18,7 +18,8 @@ LDLIBS += -lpthread
ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
LDLIBS += -lnuma
endif
-LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net
+LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net \
+ -lrte_cryptodev -lrte_hash
# all source are stored in SRCS-y
SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c \
diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index d84513e7f..2acfc908c 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -381,7 +381,7 @@ vhost_crypto_create_sess(struct vhost_crypto *vcrypto,
return;
}
- VC_LOG_DBG("Session (key %lu, session %p) created.",
+ VC_LOG_DBG("Session (key %llu, session %p) created.",
vcrypto->last_session_id, session);
sess_param->session_id = vcrypto->last_session_id;
@@ -399,7 +399,7 @@ vhost_crypto_close_sess(struct vhost_crypto *vcrypto,
uint64_t session_id)
(void **)&session);
if (unlikely(ret < 0)) {
- VC_LOG_ERR("Failed to delete session (key %lu).", session_id);
+ VC_LOG_ERR("Failed to delete session (key %llu).", session_id);
return -VIRTIO_CRYPTO_INVSESS;
}
@@ -418,7 +418,7 @@ vhost_crypto_close_sess(struct vhost_crypto *vcrypto, uint64_t session_id)
return -VIRTIO_CRYPTO_ERR;
}
- VC_LOG_DBG("Session (key %lu, session %p) deleted.", sess_id,
+ VC_LOG_DBG("Session (key %llu, session %p) deleted.", sess_id,
session);
return 0;
@@ -932,7 +932,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
&session_id, (void **)&session);
if (unlikely(err < 0)) {
err = VIRTIO_CRYPTO_ERR;
- VC_LOG_ERR("Failed to retrieve session id %lu",
+ VC_LOG_ERR("Failed to retrieve session id %llu",
session_id);
goto error_exit;
}
Re: [dpdk-dev] [PATCH] bus/fslmc: support for hotplugging of memory
On 09-Apr-18 8:49 AM, Shreyansh Jain wrote: Hello Anatoly, On Sunday 08 April 2018 10:44 PM, Burakov, Anatoly wrote: On 05-Apr-18 3:14 PM, Shreyansh Jain wrote: Restructure VFIO DMA code for handling hotplug memory events (callbacks) and --legacy case. Signed-off-by: Shreyansh Jain --- ### This is based on the 16fbfef04a3 github repository. This is assuming that changes already exists as done in patch 26/68. Though, this can be a standalone, replacing 26/88. Though, the Makefile changes don't exist in this. Also, this just a first draft. I will push any review changes after this incrementally over v4. ### Hi Shreyansh, I think we can keep the 26/68 as it still works within the context of the patchset. I would like to add these changes closer to the end, where we enable support for callbacks in VFIO (this could/should come as the next patch). But then it would also mean that dpaa2 would be broken within the memory hotplug patches? I think it would be broken once the memseg ceases to be continuous physical sets. Hi Shreyansh, Why would it be broken? Even when memseg change comes into effect, legacy mem is not enabled until much later in the patchset, when all of the callback/multiprocess business is in place. For all intents and purposes, this stays valid for legacy mode, hence not broken. We later enable callbacks etc. on VFIO, but technically they still aren't enabled until 65 (67 in v4), when it becomes possible to actually run DPDK in non-legacy mode. So, for the duration of this patchset, dpaa2 is not broken, as far as i can tell. Keeping in mind that only legacy mode will be available until patch 65/67, what exactly is being broken here? -- Thanks, Anatoly
Re: [dpdk-dev] [PATCH] lib/librte_vhost: fix meson build
On 04/06/2018 02:43 PM, Fan Zhang wrote:
Fixes: 7834b5c82bf3 ("lib/librte_vhost: update makefile")
This patch fixes some meson build bugs.
Signed-off-by: Fan Zhang
---
lib/librte_vhost/meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/librte_vhost/meson.build b/lib/librte_vhost/meson.build
index f01ef67fc..dee154e40 100644
--- a/lib/librte_vhost/meson.build
+++ b/lib/librte_vhost/meson.build
@@ -10,6 +10,6 @@ endif
version = 4
allow_experimental_apis = true
sources = files('fd_man.c', 'iotlb.c', 'socket.c', 'vhost.c', 'vhost_user.c',
- 'virtio_net.c', 'virtio_crypto.c')
+ 'virtio_net.c', 'vhost_crypto.c')
headers = files('rte_vhost.h', 'rte_vhost_crypto.h')
-deps += ['ethdev', 'cryptodev', 'pci']
+deps += ['ethdev', 'cryptodev', 'pci', 'hash']
Squashed into offending commit!
Thanks,
Maxime
Re: [dpdk-dev] [PATCH] lib/librte_vhost: fix bugs
Overall, please rename the commit title prefix to vhost/crypto.
Thanks,
Maxime
On 04/09/2018 05:34 PM, Fan Zhang wrote:
Fixes: 256b132f41b2 ("vhost/crypto: add session message handler")
Fixes: 7b5ad7beee17 ("vhost/crypto: update makefile")
Fixes: 2ce5bd8c442d ("examples/vhost_crypto: add vhost crypto sample
application")
This patch fixes the bugs introduced in the above patches.
Signed-off-by: Fan Zhang
---
examples/vhost_crypto/main.c| 5 -
lib/librte_vhost/Makefile | 3 ++-
lib/librte_vhost/vhost_crypto.c | 8
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/examples/vhost_crypto/main.c b/examples/vhost_crypto/main.c
index bc867240d..860200e29 100644
--- a/examples/vhost_crypto/main.c
+++ b/examples/vhost_crypto/main.c
@@ -95,11 +95,6 @@ parse_cryptodev_id(const char *q_arg)
/* parse decimal string */
pm = strtoul(q_arg, &end, 10);
- if ((pm == '\0') || (end == NULL) || (*end != '\0')) {
- RTE_LOG(ERR, USER1, "Invalid Cryptodev ID %s\n", q_arg);
- return -1;
- }
-
if (pm > rte_cryptodev_count()) {
RTE_LOG(ERR, USER1, "Invalid Cryptodev ID %s\n", q_arg);
return -1;
diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
index 2cc65f95e..92c267475 100644
--- a/lib/librte_vhost/Makefile
+++ b/lib/librte_vhost/Makefile
@@ -18,7 +18,8 @@ LDLIBS += -lpthread
ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
LDLIBS += -lnuma
endif
-LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net
+LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net \
+ -lrte_cryptodev -lrte_hash
# all source are stored in SRCS-y
SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c \
diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index d84513e7f..2acfc908c 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -381,7 +381,7 @@ vhost_crypto_create_sess(struct vhost_crypto *vcrypto,
return;
}
- VC_LOG_DBG("Session (key %lu, session %p) created.",
+ VC_LOG_DBG("Session (key %llu, session %p) created.",
vcrypto->last_session_id, session);
sess_param->session_id = vcrypto->last_session_id;
@@ -399,7 +399,7 @@ vhost_crypto_close_sess(struct vhost_crypto *vcrypto,
uint64_t session_id)
(void **)&session);
if (unlikely(ret < 0)) {
- VC_LOG_ERR("Failed to delete session (key %lu).", session_id);
+ VC_LOG_ERR("Failed to delete session (key %llu).", session_id);
return -VIRTIO_CRYPTO_INVSESS;
}
@@ -418,7 +418,7 @@ vhost_crypto_close_sess(struct vhost_crypto *vcrypto, uint64_t session_id)
return -VIRTIO_CRYPTO_ERR;
}
- VC_LOG_DBG("Session (key %lu, session %p) deleted.", sess_id,
+ VC_LOG_DBG("Session (key %llu, session %p) deleted.", sess_id,
session);
return 0;
@@ -932,7 +932,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
&session_id, (void **)&session);
if (unlikely(err < 0)) {
err = VIRTIO_CRYPTO_ERR;
- VC_LOG_ERR("Failed to retrieve session id %lu",
+ VC_LOG_ERR("Failed to retrieve session id %llu",
session_id);
goto error_exit;
}
Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Monday, April 9, 2018 4:10 PM
> To: Singh, Jasvinder
> Cc: dev@dpdk.org; Dumitrescu, Cristian
> Subject: Re: [dpdk-dev] [PATCH] table: fix build error with gcc 8
>
> On Mon, 9 Apr 2018 13:49:48 +0100
> Jasvinder Singh wrote:
>
> > Fix build error with gcc 8.0 due to cast between function types.
> > Fixes: 5a80bf0ae613 ("table: add cuckoo hash")
> >
> > Signed-off-by: Jasvinder Singh
> > ---
> > lib/librte_table/rte_table_hash_cuckoo.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_table/rte_table_hash_cuckoo.c
> b/lib/librte_table/rte_table_hash_cuckoo.c
> > index dcb4fe9..f7eae27 100644
> > --- a/lib/librte_table/rte_table_hash_cuckoo.c
> > +++ b/lib/librte_table/rte_table_hash_cuckoo.c
> > @@ -103,11 +103,13 @@ rte_table_hash_cuckoo_create(void *params,
> > return NULL;
> > }
> >
> > + void *hash_func = p->f_hash;
> > +
> > /* Create cuckoo hash table */
> > struct rte_hash_parameters hash_cuckoo_params = {
> > .entries = p->n_keys,
> > .key_len = p->key_size,
> > - .hash_func = (rte_hash_function)(p->f_hash),
> > + .hash_func = (rte_hash_function) hash_func,
> > .hash_func_init_val = p->seed,
> > .socket_id = socket_id,
> > .name = p->name
>
> This is just tricking the compiler into not complaining.
> I would really rather see the two hash functions made the same.
(Adding Bruce as well to consolidate all conversations in a single thread.)
What we want to do here is be able to use the librte_hash under the same API as
the several hash table flavors implemented in librte_table.
Both of these libraries allow configuring the hash function per each hash table
instance. Problem is: hash function in librte_hash has only 3 parameters (no
key mask), while hash function in librte_table has 4 parameters (includes key
mask). The key mask helps a lot for practical protocol implementations by
avoiding key copy & pre-process on lookup.
So then: how to plug in librte_hash under the same API as the suite of hash
tables in librte_table? We don't want to re-implement cuckoo hash from
librte_hash, we simply want to invoke it as a low-level primitive, similarly to
how the LPM and ACL tables are plugged into librte_table.
Solution is: as an exception, pass a 3-parameter hash function to cuckoo hash
flavor under the librte_table. Maybe this should be documented better. This
currently triggers a build warning with gcc 8, which is easy to fix, hence this
trivial patch.
Ideally, for every 3-parameter hash function, I would like to generate the
corresponding 4-parameter hash function on-the-fly, but unfortunately this is
not what C language can do.
Of course, IMO the best solution is to add key mask support to librte_hash.
Regards,
Cristian
Re: [dpdk-dev] [PATCH v2 1/6] mbuf: add buffer offset field for flexible indirection
Hi Yongseok, On Tue, Apr 03, 2018 at 05:12:06PM -0700, Yongseok Koh wrote: > On Tue, Apr 03, 2018 at 10:26:15AM +0200, Olivier Matz wrote: > > Hi, > > > > On Mon, Apr 02, 2018 at 11:50:03AM -0700, Yongseok Koh wrote: > > > When attaching a mbuf, indirect mbuf has to point to start of buffer of > > > direct mbuf. By adding buf_off field to rte_mbuf, this becomes more > > > flexible. Indirect mbuf can point to any part of direct mbuf by calling > > > rte_pktmbuf_attach_at(). > > > > > > Possible use-cases could be: > > > - If a packet has multiple layers of encapsulation, multiple indirect > > > buffers can reference different layers of the encapsulated packet. > > > - A large direct mbuf can even contain multiple packets in series and > > > each packet can be referenced by multiple mbuf indirections. > > > > > > Signed-off-by: Yongseok Koh > > > > I think the current API is already able to do what you want. > > > > 1/ Here is a mbuf m with its data > > > >off > ><--> > > len > > ++ <--> > > || > > +-|v--+ > > | |---| > > m | buf |XXX || > > | ---| > > +-+ > > > > > > 2/ clone m: > > > > c = rte_pktmbuf_alloc(pool); > > rte_pktmbuf_attach(c, m); > > > > Note that c has its own offset and length fields. > > > > > >off > ><--> > > len > > ++ <--> > > || > > +-|v--+ > > | |---| > > m | buf |XXX || > > | ---| > > +--^--+ > >| > > ++ > > indirect | > > +-|---+ > > | |---| > > c | buf | || > > | ---| > > +-+ > > > > offlen > > <--><--> > > > > > > 3/ remove some data from c without changing m > > > >rte_pktmbuf_adj(c, 10) // at head > >rte_pktmbuf_trim(c, 10) // at tail > > > > > > Please let me know if it fits your needs. > > No, it doesn't. > > Trimming head and tail with the current APIs removes data and make the space > available. Adjusting packet head means giving more headroom, not shifting the > buffer itself. If m has two indirect mbufs (c1 and c2) and those are pointing > to > difference offsets in m, > > rte_pktmbuf_adj(c1, 10); > rte_pktmbuf_adj(c2, 20); > > then the owner of c2 regard the first (off+20)B as available headroom. If it > wants to attach outer header, it will overwrite the headroom even though the > owner of c1 is still accessing it. Instead, another mbuf (h1) for the outer > header should be linked by h1->next = c2. Yes, after these operations c1, c2 and m should become read-only. So, to prepend headers, another mbuf has to be inserted before as you suggest. It is possible to wrap this in a function rte_pktmbuf_clone_area(m, offset, length) that will: - alloc and attach indirect mbuf for each segment of m that is in the range [offset : length+offset]. - prepend an empty and writable mbuf for the headers > If c1 and c2 are attached with shifting buffer address by adjusting buf_off, > which actually shrink the headroom, this case can be properly handled. What do you mean by properly handled? Yes, prepending data or adding data in the indirect mbuf won't override the direct mbuf. But prepending data or adding data in the direct mbuf m won't be protected. >From an application point of view, indirect mbufs, or direct mbufs that have refcnt != 1, should be both considered as read-only because they may share their data. How an application can know if the data is shared or not? Maybe we need a flag to differentiate mbufs that are read-only (something like SHARED_DATA, or simply READONLY). In your case, if my understanding is correct, you want to have indirect mbufs with RW data. > And another use-case (this is my actual use-case) is to make a large mbuf have > multiple packets in series. AFAIK, this will also be helpful for some FPGA > NICs > because it transfers multiple packets to a single large buffer to reduce PCIe > overhead for small packet traffic like the Multi-Packet Rx of mlx5 does. > Otherwise, packets should be memcpy'd to regular mbufs one by one instead of > indirect referencing. > > Does this make sense? I understand the need. Another option would be to make the mbuf->buffer point to an external buffer (not inside the direct mbuf). This would require to add a mbuf->free_cb. See "Mbuf with external data buffer" (page 19) in [1] for a quick overview. [1] https://dpdksummit.com/Archive/pdf/2016Userspace
Re: [dpdk-dev] [PATCH] bus/fslmc: support for hotplugging of memory
> -Original Message- > From: Burakov, Anatoly [mailto:anatoly.bura...@intel.com] > Sent: Monday, April 9, 2018 9:20 PM > To: Shreyansh Jain > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] bus/fslmc: support for hotplugging of > memory > > On 09-Apr-18 8:49 AM, Shreyansh Jain wrote: > > Hello Anatoly, > > > > On Sunday 08 April 2018 10:44 PM, Burakov, Anatoly wrote: > >> On 05-Apr-18 3:14 PM, Shreyansh Jain wrote: > >>> Restructure VFIO DMA code for handling hotplug memory events > >>> (callbacks) and --legacy case. > >>> > >>> Signed-off-by: Shreyansh Jain > >>> --- > >>> > >>> ### > >>> This is based on the 16fbfef04a3 github repository. This is assuming > >>> that changes already exists as done in patch 26/68. > >>> Though, this can be a standalone, replacing 26/88. Though, the > Makefile > >>> changes don't exist in this. > >>> Also, this just a first draft. I will push any review changes after > this > >>> incrementally over v4. > >>> ### > >> > >> Hi Shreyansh, > >> > >> I think we can keep the 26/68 as it still works within the context of > >> the patchset. I would like to add these changes closer to the end, > >> where we enable support for callbacks in VFIO (this could/should come > >> as the next patch). > > > > But then it would also mean that dpaa2 would be broken within the > memory > > hotplug patches? > > I think it would be broken once the memseg ceases to be continuous > > physical sets. > > > > Hi Shreyansh, > > Why would it be broken? Even when memseg change comes into effect, > legacy mem is not enabled until much later in the patchset, when all of > the callback/multiprocess business is in place. For all intents and > purposes, this stays valid for legacy mode, hence not broken. Ok. Then, I am mistaken. I was under the impression that as soon as memseg lists are introduced, when memseg stop being contiguous blocks, DPAA2 would have stopped working. I just didn't put an effort to check this. > > We later enable callbacks etc. on VFIO, but technically they still > aren't enabled until 65 (67 in v4), when it becomes possible to actually > run DPDK in non-legacy mode. Got it. Thanks. > > So, for the duration of this patchset, dpaa2 is not broken, as far as i > can tell. Keeping in mind that only legacy mode will be available until > patch 65/67, what exactly is being broken here? > Nothing, just a misunderstanding. > -- > Thanks, > Anatoly
