Re: [dpdk-dev] [PATCH v3] eal: fix argument to rte_bsf32_safe
23/07/2021 17:45, Stephen Hemminger:
> The first argument to rte_bsf32_safe was incorrectly declared as
> a 64 bit value. The code only works on 32 bit values and the underlying
> function rte_bsf32 only accepts 32 bit values. This was a mistake
> introduced when the safe version was added and probably cause
> by copy/paste from the 64 bit version.
>
> The bug passed silently under the radar until some other code was
> built with -Wall and -Wextra in C++ and C++ complains about the
> missing cast.
>
> Yes, this is a API signature change, but the original code was wrong.
> It is an inline so not an ABI change.
>
> Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf functions")
> Cc: anatoly.bura...@intel.com
> Signed-off-by: Stephen Hemminger
> Acked-by: Tyler Retzlaff
+Cc: sta...@dpdk.org
Applied, thanks.
I think these functions lack a reference to the name Bit Scan Forward.
Re: [dpdk-dev] [dpdk-stable] [PATCH v3 1/1] power: check freq count before filling the freqs array
23/07/2021 10:37, David Hunt:
> Hi Richael,
>
> On 23/7/2021 3:22 AM, Richael Zhuang wrote:
> > The freqs array size is RTE_MAX_LCORE_FREQS. Before filling the
> > array with num_freqs elements, restrict the total num to
> > RTE_MAX_LCORE_FREQS. This fix aims to fix the coverity scan issue
> > like:
> > Overrunning array "pi->freqs" of 256 bytes by passing it to a
> > function which accesses it at byte offset 464.
> >
> > Coverity issue: 371913
> > Fixes: ef1cc88f1837 ("power: support cppc_cpufreq driver")
> > Cc: richael.zhu...@arm.com
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Richael Zhuang
>
> LGTM to fix the coverity issue.
>
> Acked-by: David Hunt
Removed the space before ":" and applied, thanks.
Re: [dpdk-dev] [PATCH 0/2] bugfix for sched
23/04/2021 13:01, Min Hu (Connor): > This patch set contains two bugfixes for sched. > > Huisong Li (2): > lib/sched: fix return value judgment > lib/sched: optimize exception handling code Acked-by: Cristian Dumitrescu Applied, thanks.
Re: [dpdk-dev] [dpdk-stable] [PATCH v4] mbuf: fix reset on mbuf free
What's the follow-up for this patch? 19/01/2021 15:04, Slava Ovsiienko: > Hi, All > > Could we postpose this patch at least to rc2? We would like to conduct more > investigations? > > With best regards, Slava > > From: Olivier Matz > > On Mon, Jan 18, 2021 at 05:52:32PM +, Ali Alnubani wrote: > > > Hi, > > > (Sorry had to resend this to some recipients due to mail server problems). > > > > > > Just confirming that I can still reproduce the regression with single > > > core and > > 64B frames on other servers. > > > > Many thanks for the feedback. Can you please detail what is the amount of > > performance loss in percent, and confirm the test case? (I suppose it is > > testpmd io forward). > > > > Unfortunatly, I won't be able to spend a lot of time on this soon (sorry for > > that). So I see at least these 2 options: > > > > - postpone the patch again, until I can find more time to analyze > > and optimize > > - apply the patch if the performance loss is acceptable compared to > > the added value of fixing a bug > > > > Regards, > > Olivier > > [...] > > > > Assuming that pw86457 doesn't have an effect on this test, it looks > > > > to me that this patch caused a regression in Intel hardware as well. > > > > > > > > Can someone update the baseline's expected values for the Intel NICs > > > > and rerun the test on this patch? > > > > > > > > Thanks, > > > > Ali
Re: [dpdk-dev] [PATCH v1 0/8] gro: support TCP/IPv6 and UDP/IPv6 for VLAN and VXLAN
24/03/2021 22:22, Thomas Monjalon: > 21/12/2020 04:50, yang_y...@163.com: > > Yi Yang (8): > > gro: support TCP/IPv6 > > gro: support IPv4 VXLAN TCP/IPv6 > > gro: support IPv6 VXLAN TCP/IPv4 > > gro: support IPv6 VXLAN TCP/IPv6 > > gro: support UDP/IPv6 > > gro: support IPv4 VXLAN UDP/IPv6 > > gro: support IPv6 VXLAN UDP/IPv4 > > gro: support IPv6 VXLAN UDP/IPv6 > > There was no review/activity on this thread. > Is it abandoned? No reply. Should I assume it can be dropped?
Re: [dpdk-dev] [EXT] Re: [PATCH] config/arm: add ability to express arch extensions
18/05/2021 15:20, Honnappa Nagarahalli: > Are we concluding there is no solution? Should I mark this patch as rejected?
Re: [dpdk-dev] [PATCH] app/testpmd: fix help string for port reset
23/07/2021 21:36, Andrew Rybchenko:
> On 7/23/21 3:24 PM, Ferruh Yigit wrote:
> > Command help string is missing 'reset' keyword, although description has
> > it. Adding it.
> >
> > Fixes: 97f1e196799f ("app/testpmd: add port reset command")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Ferruh Yigit
>
> Reviewed-by: Andrew Rybchenko
Applied, thanks
Re: [dpdk-dev] [PATCH v1] app/testpmd: fix port MAC address after resetting port
> > MAC address of each port in global variable ports hasn't been updated after
> > resetting. It was the initial one after resetting VF MAC address.
> > This patch gets correct port MAC address when starting port.
> >
> > Fixes: a5279d25616d ("app/testpmd: check status of getting MAC address")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Yuying Zhang
>
> Acked-by: Xiaoyun Li
Applied, thanks
Re: [dpdk-dev] [PATCH] app/testpmd: fix TX checksum calculation for tunnel
Please we need more reviews for this patch.
19/07/2021 10:33, Gregory Etelson:
> TX checksum of a tunnelled packet can be calculated for outer headers
> only or for both outer and inner parts. The calculation method is
> determined by application.
> If TX checksum calculation can be offloaded, hardware ignores
> existing checksum value and replaces it with an updated result.
> If TX checksum is calculated by a software, existing value must be
> zeroed first.
> The testpmd checksum forwarding engine always zeroed inner checksums.
> If inner checksum calculation was offloaded, that header was left
> with 0 checksum value.
> Following outer software checksum calculation produced wrong value.
> The patch zeroes inner IPv4 checksum only before software calculation.
>
> Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
>
> Cc: sta...@dpdk.org
nit: no blank line between Fixes and Cc lines please
>
> Signed-off-by: Gregory Etelson
> Reviewed-by: Dmitry Kozlyuk
> ---
> + } else if (ipv4_hdr->hdr_checksum) {
Please do an explicit comparison with 0 here
as it cannot be considered as a boolean test.
> + ipv4_hdr->hdr_checksum = 0;
> ipv4_hdr->hdr_checksum =
> rte_ipv4_cksum(ipv4_hdr);
> + }
Re: [dpdk-dev] [PATCH v16] app/testpmd: support multi-process
10/07/2021 05:50, Min Hu (Connor): > This patch adds multi-process support for testpmd. > For example the following commands run two testpmd > processes: > > * the primary process: > > ./dpdk-testpmd --proc-type=auto -l 0-1 -- -i \ >--rxq=4 --txq=4 --num-procs=2 --proc-id=0 > > * the secondary process: > > ./dpdk-testpmd --proc-type=auto -l 2-3 -- -i \ >--rxq=4 --txq=4 --num-procs=2 --proc-id=1 > > Signed-off-by: Min Hu (Connor) > Signed-off-by: Lijun Ou > Signed-off-by: Andrew Rybchenko > Acked-by: Xiaoyun Li > Acked-by: Ajit Khaparde > Reviewed-by: Ferruh Yigit > --- > V16: > * revert unrelated changes. > * add some restrictions in doc. I didn't see clear agreement to integrate this feature in DPDK 21.08. BTW, the testpmd maintainer was not Cc'ed.
Re: [dpdk-dev] [PATCH 03/11] ethdev: fix docs of functions getting xstats by IDs
On 7/23/21 5:19 PM, Ferruh Yigit wrote:
On 7/22/2021 10:12 AM, Andrew Rybchenko wrote:
On 7/20/21 7:25 PM, Ferruh Yigit wrote:
On 6/4/2021 3:42 PM, Andrew Rybchenko wrote:
From: Ivan Ilchenko
Document valid combinations of input arguments in accordance with
current implementation in ethdev.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/rte_ethdev.h | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index faf3bd901d..1f63118544 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2873,12 +2873,15 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* The port identifier of the Ethernet device.
* @param xstats_names
* An rte_eth_xstat_name array of at least *size* elements to
- * be filled. If set to NULL, the function returns the required number
- * of elements.
+ * be filled. Must not be NULL if @p ids are specified (not NULL).
Removed part is also valid. If both 'ids' & 'xstats_names' are NULL, API returns
number of all elements.
Yes, but it is an excessive information. The trigger to return number
of all elements is 'ids == NULL'. Here we are talking about
'xstats_names' parameter. If the parameter is NULL, but ids is not
null, it does not trigger number of all elements return. It is an
invalid input parameters. That's what a new description says.
ids xstats_names
a) NULL NULLvalid, return _number_ of all elements
b) NULL !NULL valid, return all elements (if size big enough)
c) !NULLNULLinvalid
Above a) is a valid option, the previous comment tries to describe it via "If
set to NULL, the function returns the required number of elements." and that
part is removed.
Since it is valid option I think we should keep it documented, location or
wording is up to you.
OK, I see. IMHO, it looks better if it is mentioned in ids parameter
description. See v4.
Addition part looks good.
* @param ids
- * IDs array given by app to retrieve specific statistics
+ * IDs array given by app to retrieve specific statistics. May be NULL
+ * to retrieve all available statistics.
ack
* @param size
- * The size of the xstats_names array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in @p xstats_names to put names in. If @p ids is
+ * NULL, number of elements in @p xstats_names to put all available
statistics
+ * names in.
ack
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2886,7 +2889,7 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
ack
The 'eth_dev_get_xstats_count()' API is flexible but it makes API unnecessarily
complex, not for this patch but for future perhaps we can update the API and it
can return error if either 'ids' or 'xstats_names' is NULL. Remove support to
get all elements or getting number of elements support, these already supported
by non _id version of API.
I'm not sure that it is a right direction. The support allows
application to use less number of functions and depend on less
number of function prototypes.
True, but makes API more complex, and creates multiple way to do same thing (get
number of all entries), trade off.
Yes.
And as a note for future, if we ever consider updating these _by_id APIs, we can
consider making the parameter order same for both, currently it is:
"rte_eth_xstats_get_names_by_id(port_id, values, size, ids)"
" rte_eth_xstats_get_by_id(port_id, ids, values, size)"
+1, current difference is terribly bad
*/
int
rte_eth_xstats_get_names_by_id(uint16_t port_id,
@@ -2900,13 +2903,15 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* The port identifier of the Ethernet device.
* @param ids
* A pointer to an ids array passed by application. This tells which
- * statistics values function should retrieve. This parameter
- * can be set to NULL if size is 0. In this case function will retrieve
+ * statistics values function should retrieve. May be NULL to retrieve
* all available statistics.
Update is good. But what do you think to make it exact same in the both APIs
('rte_eth_xstats_get_names_by_id()' & 'rte_eth_xstats_get_by_id()')? Since it is
used for same purpose and exact same way in both APIs, no need to have slightly
different
Re: [dpdk-dev] [PATCH v3 03/11] ethdev: fix docs of functions getting xstats by IDs
On 7/23/21 5:42 PM, Ferruh Yigit wrote:
On 7/23/2021 2:15 PM, Andrew Rybchenko wrote:
From: Ivan Ilchenko
Document valid combinations of input arguments in accordance with
current implementation in ethdev.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/rte_ethdev.h | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..28440c46d3 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2873,12 +2873,15 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* The port identifier of the Ethernet device.
* @param xstats_names
* An rte_eth_xstat_name array of at least *size* elements to
- * be filled. If set to NULL, the function returns the required number
- * of elements.
+ * be filled. Must not be NULL if @p ids are specified (not NULL).
* @param ids
- * IDs array given by app to retrieve specific statistics
+ * IDs array given by app to retrieve specific statistics. May be NULL
+ * to retrieve all available statistics.
* @param size
- * The size of the xstats_names array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in @p xstats_names to put names in. If @p ids is
+ * NULL, number of elements in @p xstats_names to put all available
statistics
+ * names in.
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2886,7 +2889,7 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
*/
int
rte_eth_xstats_get_names_by_id(uint16_t port_id,
@@ -2900,13 +2903,15 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* The port identifier of the Ethernet device.
* @param ids
* A pointer to an ids array passed by application. This tells which
- * statistics values function should retrieve. This parameter
- * can be set to NULL if size is 0. In this case function will retrieve
+ * statistics values function should retrieve. May be NULL to retrieve
* all available statistics.
'ids' parameter in 'rte_eth_xstats_get_names_by_id()' &
'rte_eth_xstats_get_by_id()' are exactly same thing, and description is same but
wording is different.
Do you think does it make sense to use exact same wording, to clarify that there
is no difference in this parameter within APIs?
I thought that I've fixed it, but it looks like I've lost it on the way.
Will fix in v4. IMHO, it looks awkward if description is absolutely the
same, but description structure should be definitely the same.
* @param values
* A pointer to a table to be filled with device statistics values.
+ * Must not be NULL if ids are specified (not NULL).
Similar comment on this one, two different API get 'name' and 'value' part of
key-value pair. The description between APIs can be almost same.
Will fix in v4 similar to above.
* @param size
- * The size of the ids array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in values to put statistics in. If @p ids is NULL,
+ * number of elements in values to put all available statistics in.
And same comment again on using exact same comment on two APIs.
It is almost the same since it refers to other parameters which differ.
Other than that the description is equal.
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2914,7 +2919,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
*/
int rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids,
uint64_t *values, unsigned int size);
[dpdk-dev] [PATCH v4 1/2] ethdev: fix docs of functions getting xstats by IDs
From: Ivan Ilchenko
Document valid combinations of input arguments in accordance with
current implementation in ethdev.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/rte_ethdev.h | 32 +++-
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..b14067fe7e 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2872,13 +2872,17 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* @param port_id
* The port identifier of the Ethernet device.
* @param xstats_names
- * An rte_eth_xstat_name array of at least *size* elements to
- * be filled. If set to NULL, the function returns the required number
- * of elements.
+ * Array to be filled in with names of requested device statistics.
+ * Must not be NULL if @p ids are specified (not NULL).
* @param ids
- * IDs array given by app to retrieve specific statistics
+ * IDs array given by app to retrieve specific statistics. May be NULL to
+ * retrieve names of all available statistics or, if @p xstats_names is
+ * NULL as well, just a number of available statistics.
* @param size
- * The size of the xstats_names array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in @p xstats_names to put names in. If @p ids is
+ * NULL, number of elements in @p xstats_names to put all available
statistics
+ * names in.
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2886,7 +2890,7 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
*/
int
rte_eth_xstats_get_names_by_id(uint16_t port_id,
@@ -2899,14 +2903,16 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* @param port_id
* The port identifier of the Ethernet device.
* @param ids
- * A pointer to an ids array passed by application. This tells which
- * statistics values function should retrieve. This parameter
- * can be set to NULL if size is 0. In this case function will retrieve
- * all available statistics.
+ * IDs array given by app to retrieve specific statistics. May be NULL to
+ * retrieve all available statistics or, if @p values is NULL as well,
+ * just a number of available statistics.
* @param values
- * A pointer to a table to be filled with device statistics values.
+ * Array to be filled in with requested device statistics.
+ * Must not be NULL if ids are specified (not NULL).
* @param size
- * The size of the ids array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in @p values to put statistics in. If @p ids is
NULL,
+ * number of elements in @p values to put all available statistics in.
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2914,7 +2920,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
*/
int rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids,
uint64_t *values, unsigned int size);
--
2.30.2
[dpdk-dev] [PATCH v4 2/2] ethdev: fix docs of drivers callbacks getting xstats by IDs
From: Ivan Ilchenko
Update xstats by IDs callbacks documentation in accordance with
ethdev usage of these callbacks. Document valid combinations of
input arguments to make driver implementation simpler.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/ethdev_driver.h | 43 --
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 40e474aa7e..fd5b7ca550 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -187,11 +187,28 @@ typedef int (*eth_xstats_get_t)(struct rte_eth_dev *dev,
struct rte_eth_xstat *stats, unsigned int n);
/**< @internal Get extended stats of an Ethernet device. */
+/**
+ * @internal
+ * Get extended stats of an Ethernet device.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param ids
+ * IDs array to retrieve specific statistics. Must not be NULL.
+ * @param values
+ * A pointer to a table to be filled with device statistics values.
+ * Must not be NULL.
+ * @param n
+ * Element count in @p ids and @p values
+ *
+ * @return
+ * - A number of filled in stats.
+ * - A negative value on error.
+ */
typedef int (*eth_xstats_get_by_id_t)(struct rte_eth_dev *dev,
const uint64_t *ids,
uint64_t *values,
unsigned int n);
-/**< @internal Get extended stats of an Ethernet device. */
/**
* @internal
@@ -218,10 +235,32 @@ typedef int (*eth_xstats_get_names_t)(struct rte_eth_dev
*dev,
struct rte_eth_xstat_name *xstats_names, unsigned int size);
/**< @internal Get names of extended stats of an Ethernet device. */
+/**
+ * @internal
+ * Get names of extended stats of an Ethernet device.
+ * For name count, set @p xstats_names and @p ids to NULL.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param xstats_names
+ * An rte_eth_xstat_name array of at least *size* elements to
+ * be filled. Can be NULL together with @p ids to retrieve number of
+ * available statistics.
+ * @param ids
+ * IDs array to retrieve specific statistics. Can be NULL together
+ * with @p xstats_names to retrieve number of available statistics.
+ * @param size
+ * Size of ids and xstats_names arrays.
+ * Element count in @p ids and @p xstats_names
+ *
+ * @return
+ * - A number of filled in stats if both xstats_names and ids are not NULL.
+ * - A number of available stats if both xstats_names and ids are NULL.
+ * - A negative value on error.
+ */
typedef int (*eth_xstats_get_names_by_id_t)(struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names, const uint64_t *ids,
unsigned int size);
-/**< @internal Get names of extended stats of an Ethernet device. */
typedef int (*eth_queue_stats_mapping_set_t)(struct rte_eth_dev *dev,
uint16_t queue_id,
--
2.30.2
Re: [dpdk-dev] [PATCH] app/testpmd: remove most uses of rte_eth_devices
15/07/2021 16:52, Ferruh Yigit: > On 7/15/2021 3:20 PM, Paulis Gributs wrote: > > This patch removes most uses of the global variable rte_eth_devices > > from testpmd. This was done to avoid using the object directly which > > applications should not do. > > > > Most uses have been replaced with standard function calls, however > > the use of it in the show_macs function could not be replaced as no > > function call exists to get all mac addresses of a given port. > > > > Signed-off-by: Paulis Gributs > > Reviewed-by: Ferruh Yigit > > +1 to eliminate 'rte_eth_devices' access from application Acked-by: Xiaoyun Li Applied, thanks that's a good step. However, I think we should not expose the rte_device pointer at all as it is done in rte_eth_dev_info.
Re: [dpdk-dev] [PATCH v3] app/testpmd: send failure logs to stderr
> > Running with stdout suppressed or redirected for further processing > > is very confusing in the case of errors. Fix it by logging errors and > > warnings to stderr. > > > > Since lines with log messages are touched anyway concatenate split > > format strings to make it easier to search using grep. > > > > Fix indent of format string arguments. > > > > Signed-off-by: Andrew Rybchenko > > Acked-by: Xiaoyun Li Applied, rebased and added few more stderrs, thanks.
Re: [dpdk-dev] [PATCH] app/acl: add script for automate testing
18/05/2021 13:26, Konstantin Ananyev: > The purpose of this script is to help automate ACL library functional > testing using test-acl app. > Sample input files are also provided. > > Signed-off-by: Konstantin Ananyev The input files make the script devtools/check-spdx-tag.sh failing.
[dpdk-dev] [dpdk-announce] release candidate 21.08-rc2
A new DPDK release candidate is ready for testing: https://git.dpdk.org/dpdk/tag/?id=v21.08-rc2 There are 287 new patches in this snapshot, most of them adding or fixing features in drivers. Release notes: https://doc.dpdk.org/guides/rel_notes/release_21_08.html Highlights of 21.08-rc2: - Wangxun ngbe ethernet driver - Marvell CNXK event Rx/Tx adapters - NVIDIA mlx5 crypto driver supporting AES-XTS - ISAL compress support on Arm Please test and report issues on bugs.dpdk.org. DPDK 21.08-rc3 is expected in one week. Thank you everyone
[dpdk-dev] [PATCH v2] eventdev: fix event port setup in tx adapter
The event port config set by application in
rte_event_eth_tx_adapter_create API is modified in
default configuration callback function. This patch removes
this hardcode to use application provided event port
config value.
Fixes: ("eventdev: fix event port config override in tx adapter")
Signed-off-by: Naga Harish K, S V
---
lib/eventdev/rte_event_eth_tx_adapter.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
b/lib/eventdev/rte_event_eth_tx_adapter.c
index db260bfb68..18c0359db7 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c
@@ -286,7 +286,6 @@ txa_service_conf_cb(uint8_t __rte_unused id, uint8_t dev_id,
return ret;
}
- pc->event_port_cfg = 0;
ret = rte_event_port_setup(dev_id, port_id, pc);
if (ret) {
RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
--
2.25.1
[dpdk-dev] [PATCH v2] eventdev: fix event port setup in tx adapter
The event port config set by application in
rte_event_eth_tx_adapter_create API is modified in
default configuration callback function. This patch removes
this hardcode to use application provided event port
config value.
Fixes: ("eventdev: fix event port config override in tx adapter")
Signed-off-by: Naga Harish K, S V
---
lib/eventdev/rte_event_eth_tx_adapter.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
b/lib/eventdev/rte_event_eth_tx_adapter.c
index db260bfb68..18c0359db7 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c
@@ -286,7 +286,6 @@ txa_service_conf_cb(uint8_t __rte_unused id, uint8_t dev_id,
return ret;
}
- pc->event_port_cfg = 0;
ret = rte_event_port_setup(dev_id, port_id, pc);
if (ret) {
RTE_EDEV_LOG_ERR("failed to setup event port %u\n",
--
2.25.1
Re: [dpdk-dev] [PATCH] app/testpmd: fix TX checksum calculation for tunnel
> Please we need more reviews for this patch.
>
I'll update the patch and post a v2.
> 19/07/2021 10:33, Gregory Etelson:
> > TX checksum of a tunnelled packet can be calculated for outer headers
> > only or for both outer and inner parts. The calculation method is
> > determined by application.
> > If TX checksum calculation can be offloaded, hardware ignores existing
> > checksum value and replaces it with an updated result.
> > If TX checksum is calculated by a software, existing value must be
> > zeroed first.
> > The testpmd checksum forwarding engine always zeroed inner
> checksums.
> > If inner checksum calculation was offloaded, that header was left with
> > 0 checksum value.
> > Following outer software checksum calculation produced wrong value.
> > The patch zeroes inner IPv4 checksum only before software calculation.
> >
> > Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
> >
> > Cc: sta...@dpdk.org
>
> nit: no blank line between Fixes and Cc lines please
>
> >
> > Signed-off-by: Gregory Etelson
> > Reviewed-by: Dmitry Kozlyuk
> > ---
> > + } else if (ipv4_hdr->hdr_checksum) {
>
> Please do an explicit comparison with 0 here as it cannot be considered as a
> boolean test.
>
> > + ipv4_hdr->hdr_checksum = 0;
> > ipv4_hdr->hdr_checksum =
> > rte_ipv4_cksum(ipv4_hdr);
> > + }
>
>
[dpdk-dev] [PATCH] net/mlx5: fix queue leaking in hairpin auto bind checking
During the start up stage, the hairpin auto bind was executed for
each port. All the Tx and Rx queues configured for this port should
be checked to confirm if the auto bind of hairpin is needed.
1. The queue is hairpin queue.
2. The peer port is the same one and the peer queue should also be
with hairpin type.
3. The manual bind attribute is not set for this queue.
If the queue is not a hairpin queue or it doesn't need to be bound
automatically, the reference count should be decreased by 1 since
the count was increased when calling the mlx5_*xq_get().
When the peer port is not the same, it means that no auto bind is
supported and the mlx5_*xq_release() was missed in the current
implementation.
By calling the release function before continue, the count is
correct when calling the device close.
Fixes: aa8bea0e3455 ("net/mlx5: add conditional hairpin auto bind")
Cc: sta...@dpdk.org
Signed-off-by: Bing Zhao
Acked-by: Viacheslav Ovsiienko
---
drivers/net/mlx5/mlx5_trigger.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index a9d5d58fd9..54173bfacb 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -228,12 +228,11 @@ mlx5_hairpin_auto_bind(struct rte_eth_dev *dev)
txq_ctrl = mlx5_txq_get(dev, i);
if (!txq_ctrl)
continue;
- if (txq_ctrl->type != MLX5_TXQ_TYPE_HAIRPIN) {
+ if (txq_ctrl->type != MLX5_TXQ_TYPE_HAIRPIN ||
+ txq_ctrl->hairpin_conf.peers[0].port != self_port) {
mlx5_txq_release(dev, i);
continue;
}
- if (txq_ctrl->hairpin_conf.peers[0].port != self_port)
- continue;
if (txq_ctrl->hairpin_conf.manual_bind) {
mlx5_txq_release(dev, i);
return 0;
@@ -247,13 +246,12 @@ mlx5_hairpin_auto_bind(struct rte_eth_dev *dev)
txq_ctrl = mlx5_txq_get(dev, i);
if (!txq_ctrl)
continue;
- if (txq_ctrl->type != MLX5_TXQ_TYPE_HAIRPIN) {
+ /* Skip hairpin queues with other peer ports. */
+ if (txq_ctrl->type != MLX5_TXQ_TYPE_HAIRPIN ||
+ txq_ctrl->hairpin_conf.peers[0].port != self_port) {
mlx5_txq_release(dev, i);
continue;
}
- /* Skip hairpin queues with other peer ports. */
- if (txq_ctrl->hairpin_conf.peers[0].port != self_port)
- continue;
if (!txq_ctrl->obj) {
rte_errno = ENOMEM;
DRV_LOG(ERR, "port %u no txq object found: %d",
--
2.27.0
Re: [dpdk-dev] [PATCH v3] eal: fix argument to rte_bsf32_safe
On Sat, 24 Jul 2021 09:58:44 +0200
Thomas Monjalon wrote:
> 23/07/2021 17:45, Stephen Hemminger:
> > The first argument to rte_bsf32_safe was incorrectly declared as
> > a 64 bit value. The code only works on 32 bit values and the underlying
> > function rte_bsf32 only accepts 32 bit values. This was a mistake
> > introduced when the safe version was added and probably cause
> > by copy/paste from the 64 bit version.
> >
> > The bug passed silently under the radar until some other code was
> > built with -Wall and -Wextra in C++ and C++ complains about the
> > missing cast.
> >
> > Yes, this is a API signature change, but the original code was wrong.
> > It is an inline so not an ABI change.
> >
> > Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf functions")
> > Cc: anatoly.bura...@intel.com
> > Signed-off-by: Stephen Hemminger
> > Acked-by: Tyler Retzlaff
>
> +Cc: sta...@dpdk.org
>
> Applied, thanks.
>
> I think these functions lack a reference to the name Bit Scan Forward.
>
>
>
>
Tyler wanted to fix a bunch more stuff in these for 21.11 where it will
be a bigger API change.
[dpdk-dev] [PATCH] net/virtio: fix memory leak in dev close
Free the "intr_handle" memory in virtio_user_dev_uninit() to
avoid memory leak.
when virtio user dev closes, the "intr_handle" memory is not freeed
that is alloced in virtio_user_fill_intr_handle().
Fixes: 7f468b2ebfad ("net/virtio: release port upon close")
Signed-off-by: Gaoxiang Liu
---
drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 1cd1e95f4..16c58710d 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -654,6 +654,13 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
*path, int queues,
void
virtio_user_dev_uninit(struct virtio_user_dev *dev)
{
+ struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->hw.port_id];
+
+ if (eth_dev->intr_handle) {
+ free(eth_dev->intr_handle);
+ eth_dev->intr_handle = NULL;
+ }
+
virtio_user_stop_device(dev);
rte_mem_event_callback_unregister(VIRTIO_USER_MEM_EVENT_CLB_NAME, dev);
--
2.32.0
