Re: [PATCH 2/4] app/testpmd: fix burst option parsing
On Tue, Mar 12, 2024 at 5:47 PM Ferruh Yigit wrote:
>
> On 3/8/2024 2:48 PM, David Marchand wrote:
> > rte_eth_dev_info_get() is not supposed to fail for a valid port_id, but
> > for the theoretical case when it would fail, raise an error rather than
> > skip subsequent options.
> >
> > Fixes: 6f51deb903b2 ("app/testpmd: check status of getting ethdev info")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: David Marchand
> > ---
> > app/test-pmd/parameters.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index d715750bb8..8c21744009 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -1128,9 +1128,9 @@ launch_args_parse(int argc, char** argv)
> > 0,
> > &dev_info);
> > if (ret != 0)
> > - return;
> > -
> > - rec_nb_pkts = dev_info
> > + rec_nb_pkts = 0;
> > + else
> > + rec_nb_pkts = dev_info
> >
> > .default_rxportconf.burst_size;
> >
> > if (rec_nb_pkts == 0)
>
> 'eth_dev_info_get_print_err()' already fail, but it may not be very
> clear to the user,
> OK to print a failure log, but setting 'rec_nb_pkts = 0;' as above also
> will generate an error message that also may be confusing to the user.
>
> What about print an explicit error message for the
> 'eth_dev_info_get_print_err()' failed case?
rte_exit(EXIT_FAILURE, "Failed to retrieve device info, this is
probably a driver bug. "
"To workaround this issue, please provide a value between 1
and %d\n", MAX_PKT_BURST);
Does it work for you?
--
David Marchand
Re: [EXTERNAL] Re: [v11 0/4] PCI Dev and SG copy support
Hi Gowrishankar, On 2024/3/12 20:24, Gowrishankar Muthukrishnan wrote: > Hi Fengchengwen > >> >> Hi Thomas, >> >> On 2024/3/12 17:15, Thomas Monjalon wrote: >>> 07/03/2024 14:55, Gowrishankar Muthukrishnan: Hi Fengchengwen, >> Waiting for a confirmation that this series is good to go. > > In the discuss of thread [1], I hope this patchset continue take a > step forward (means new version) to support bi-direction test just by >> modify config.ini file. > This patch set already exposes all configuration via config.ini. I didn't follow >> what is missing. For bi-direction, we can better continue discussing on that >> patch. >>> >>> Chengwen, please can you confirm whether you require a new version? >>> Which change exactly is missing? >> >> This patchset is OK with one sub-test only tackle one DMA direction. >> > Thanks for the confirmation. > >> But there is a later patch [1] which will support multiple DMA directions >> within >> one sub-test. >> it will add a entry "xfer_mode", but I think it complicate the test, I >> prefer we do >> more in this patchset to support some like bi-direction just by modify >> config.ini, some like this: >> > I think we should discuss about that in bi-directional patch series. This > series is self-contained and there is no need to add bi-directional as part > of this series. As far as this patch set is concerned, all the options are > exposed via config.ini. Can you comment if there is anything missing, > assuming that we are taking bi-directional support as a separate feature > addition. I have identified some improvements to the dma-perf app, and I plan to do it in 24.07, so if you don't mind, I will incorporate your commits (keeping your signed-off-by) and modify to the one that I described above, and then send to community (also with my improvements commits). Thanks > > Thanks, > Gowrishankar > >> 1. extend lcore_dma: >>current lcore_dma is: lcore10@:00:04.2 >>extend it support: >> lcore10@:00:04.2,dir=m2d,coreid=1,pfid=2,vfid=3,raddr=0x >> 2. to fix one entry can't hold too many dma device, support entrys: >> lcore_dma_1, lcore_dma_2 >>which value is same with lcore_dma. >> >> So for bi-direction, we just define config.ini as: >> lcore_dma=lcore10@:00:04.2,dir=m2d,coreid=1,pfid=2,vfid=3,raddr=0x >> , lcore10@:00:04.2,dir=d2m,coreid=1,pfid=2,vfid=3,raddr=0x >> >> [1] https://urldefense.proofpoint.com/v2/url?u=https- >> 3A__patches.dpdk.org_project_dpdk_patch_20240229141426.4188428- >> 2D1-2Damitprakashs- >> 40marvell.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=EAtr- >> g7yUFhtOio8r2Rtm13Aqe4WVp_S_gHpcu6KFVo&m=DUaL_AJR1zqM0T2yw3 >> aV44EObB90uqw5weFzSm- >> w39citSeGozNdEe4kzicss_KG&s=UTAcoZx5DjSJHyzxyLMxXz1bPqfPXQM7feDx >> ZdC6Jgk&e= >> >>>
Re: [PATCH 3/4] app/testpmd: check queue count for related options
On Tue, Mar 12, 2024 at 5:59 PM Ferruh Yigit wrote:
>
> On 3/8/2024 2:48 PM, David Marchand wrote:
> > Checking the number of rxq/txq in the middle of option parsing is
> > confusing. Move the check where nb_rxq / nb_txq are modified.
> >
> > Signed-off-by: David Marchand
> > ---
> > app/test-pmd/parameters.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 8c21744009..271f0c995a 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -1063,6 +1063,8 @@ launch_args_parse(int argc, char** argv)
> > rte_exit(EXIT_FAILURE, "rxq %d
> > invalid - must be"
> > " >= 0 && <= %u\n", n,
> >
> > get_allowed_max_nb_rxq(&pid));
> > + if (!nb_rxq && !nb_txq)
> > + rte_exit(EXIT_FAILURE, "Either rx or
> > tx queues should be non-zero\n");
> > }
> > if (!strcmp(lgopts[opt_idx].name, "txq")) {
> > n = atoi(optarg);
> > @@ -1072,6 +1074,8 @@ launch_args_parse(int argc, char** argv)
> > rte_exit(EXIT_FAILURE, "txq %d
> > invalid - must be"
> > " >= 0 && <= %u\n", n,
> >
> > get_allowed_max_nb_txq(&pid));
> > + if (!nb_rxq && !nb_txq)
> > + rte_exit(EXIT_FAILURE, "Either rx or
> > tx queues should be non-zero\n");
> > }
> > if (!strcmp(lgopts[opt_idx].name, "hairpinq")) {
> > n = atoi(optarg);
> > @@ -1098,10 +1102,6 @@ launch_args_parse(int argc, char** argv)
> > n + nb_rxq,
> >
> > get_allowed_max_nb_rxq(&pid));
> > }
> > - if (!nb_rxq && !nb_txq) {
> > - rte_exit(EXIT_FAILURE, "Either rx or tx
> > queues should "
> > - "be non-zero\n");
> > - }
> > if (!strcmp(lgopts[opt_idx].name, "hairpin-mode")) {
> > char *end = NULL;
> > unsigned int n;
>
> There is already a runtime check for queues [1], perhaps we can remove
> it altogether from arg parse.
Good catch.
This other check comes after parsing args, so I suspect it is just dead code.
I guess I'll change it into a rte_exit(EXIT_FAILURE..).
Is this what you propose?
>
> Also I think the order of the 'hairpinq' and queue number parameter
> processing depends on order user provided, so this may not be very
> reliable anyway.
Indeed.
--
David Marchand
RE: [PATCH 2/2] net/mlx5: fix IP-in-IP tunnels recognition
Hi,
> -Original Message-
> From: Gregory Etelson
> Sent: Thursday, February 29, 2024 6:05 PM
> To: dev@dpdk.org
> Cc: Gregory Etelson ; Maayan Kashani
> ; Raslan Darawsheh ; Ori Kam
> ; Dariusz Sosnowski ; Slava
> Ovsiienko ; Suanming Mou
> ; Matan Azrad ; Yongseok
> Koh
> Subject: [PATCH 2/2] net/mlx5: fix IP-in-IP tunnels recognition
>
> The patch fixes IP-in-IP tunnel recognition for the following patterns
>
> / [ipv4|ipv6] proto is [ipv4|ipv6] / end
>
> / [ipv4|ipv6] / [ipv4|ipv6] /
>
> Fixes: 3d69434113d1 ("net/mlx5: add Direct Verbs validation function")
> Signed-off-by: Gregory Etelson
> Acked-by: Ori Kam
Series applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
RE: [PATCH] net/mlx5: fix async flow create error handling
Hi,
> -Original Message-
> From: Dariusz Sosnowski
> Sent: Tuesday, March 5, 2024 8:05 PM
> To: Matan Azrad ; Slava Ovsiienko
> ; Ori Kam ; Suanming Mou
>
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix async flow create error handling
>
> Whenever processing of asynchronous flow rule create operation failed, but
> after some dynamic flow actions had already been allocated, these actions
> were not freed during error handling flow.
> That behavior lead to leaks e.g., RSS/QUEUE action objects were leaked which
> triggered assertions during device cleanup.
>
> This patch adds flow rule cleanup handling in case of an error during async
> flow rule creation.
>
> Fixes: 3a2f674b6aa8 ("net/mlx5: add queue and RSS HW steering action")
> Cc: suanmi...@nvidia.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Dariusz Sosnowski
> Acked-by: Ori Kam
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
RE: [PATCH] net/mlx5: fix mlx5dr context release ordering
Hi,
> -Original Message-
> From: Maayan Kashani
> Sent: Wednesday, March 6, 2024 8:02 AM
> To: dev@dpdk.org
> Cc: Maayan Kashani ; Raslan Darawsheh
> ; sta...@dpdk.org; Dariusz Sosnowski
> ; Slava Ovsiienko ; Ori
> Kam ; Suanming Mou ; Matan
> Azrad
> Subject: [PATCH] net/mlx5: fix mlx5dr context release ordering
>
> Creating rules on group >0, creates a jump action on the group table.
> Non template code releases the group data under shared mlx5dr free code,
> And the mlx5dr context was already closed in HWS code.
>
> Remove mlx5dr context release from hws resource release function.
>
> Fixes: b401400db24e ("net/mlx5: add port flow configuration")
> Cc: sta...@dpdk.org
> Signed-off-by: Maayan Kashani
> Acked-by: Dariusz Sosnowski
Patch applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
RE: [PATCH] net/mlx5: fix pattern template size validation
Hi,
> -Original Message-
> From: Gregory Etelson
> Sent: Wednesday, March 6, 2024 9:39 AM
> To: dev@dpdk.org
> Cc: Gregory Etelson ; Maayan Kashani
> ; Raslan Darawsheh ; Dariusz
> Sosnowski ; Slava Ovsiienko
> ; Ori Kam ; Suanming Mou
> ; Matan Azrad
> Subject: [PATCH] net/mlx5: fix pattern template size validation
>
> PMD running in HWS FDB mode can be configured to steer group 0 to FW.
> In that case PMD activates legacy DV pattern processing.
> There are control flows that require HWS pattern processing in group 0.
>
> Pattern template validation tried to create a matcher both in group 0 and HWS
> group.
> As the result, during group 0 validation HWS tuned pattern was processed as
> DV.
>
> The patch removed pattern validation for group 0.
>
> Fixes: f3aadd103358 ("net/mlx5: improve pattern template validation")
> Signed-off-by: Gregory Etelson
> Acked-by: Dariusz Sosnowski
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
RE: [PATCH 1/4] net/mlx5/hws: fix direct index insert on dep wqe
Hi,
> -Original Message-
> From: Dariusz Sosnowski
> Sent: Wednesday, March 6, 2024 10:22 PM
> To: Slava Ovsiienko ; Ori Kam ;
> Suanming Mou ; Matan Azrad
> ; Yevgeny Kliteynik
> Cc: dev@dpdk.org; Alex Vesker ; sta...@dpdk.org
> Subject: [PATCH 1/4] net/mlx5/hws: fix direct index insert on dep wqe
>
> From: Alex Vesker
>
> In case a depend WQE was required and direct index was needed we would
> not set the direct index on the dep_wqe.
> This leads to incorrect insertion to index zero.
>
> Fixes: 38b5bf6452a6 ("net/mlx5/hws: support insert/distribute RTC
> properties")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Alex Vesker
> Acked-by: Dariusz Sosnowski
Series applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
RE: [PATCH] net/mlx5: prevent ioctl failure log from flood
Hi,
> -Original Message-
> From: Bing Zhao
> Sent: Thursday, March 7, 2024 8:14 AM
> To: Slava Ovsiienko ; dev@dpdk.org; Raslan
> Darawsheh
> Cc: Ori Kam ; Dariusz Sosnowski
> ; Suanming Mou ;
> Matan Azrad ; Eli Britstein ; Ophir
> Munk ; sta...@dpdk.org
> Subject: [PATCH] net/mlx5: prevent ioctl failure log from flood
>
> From: Eli Britstein
>
> The following log is printed in WARNING severity:
>
> mlx5_net: port 1 ioctl(SIOCETHTOOL, ETHTOOL_GPAUSEPARAM) failed:
> Operation not supported
>
> Reduce the severity to DEBUG to prevent this log from flooding when there
> are hundreds of ports probed without supporting this flow ctrl query.
>
> Fixes: 1256805dd54d ("net/mlx5: move Linux-specific functions")
> Cc: ophi...@nvidia.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Eli Britstein
Patch applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
RE: [PATCH] net/mlx5: fix age position in hairpin split
Hi,
> -Original Message-
> From: Bing Zhao
> Sent: Thursday, March 7, 2024 10:09 AM
> To: Slava Ovsiienko ; dev@dpdk.org; Raslan
> Darawsheh
> Cc: Ori Kam ; Dariusz Sosnowski
> ; Suanming Mou ;
> Matan Azrad ; Michael Baum ;
> sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix age position in hairpin split
>
> When splitting a hairpin rule implicitly, the count action will be on either
> Tx or
> Rx subflow based on the encapsulation checking.
>
> Once there is a flow rule with both count and age action, one counter will be
> reused. If there is only age action and the ASO flow hit is supported, the
> flow
> hit will be chosen instead of a counter.
>
> In the previous flow splitting, the age would always be in the Rx part, while
> the
> count would be on the Tx part when there is an encap.
>
> Before this commit, 2 issues can be observed with a hairpin split:
> 1. On the root table, one counter was used on both Rx and Tx parts
> for age and count actions. Then one ingress packet will be
> counted twice.
> 2. On the non-root table, an extra ASO flow hit was used on the Rx
> part. This would cause some overhead.
>
> The age and count actions should be in the same subflow instead of 2.
>
> Fixes: daed4b6e3db2 ("net/mlx5: use aging by counter when counter exists")
> Cc: michae...@nvidia.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Bing Zhao
> Acked-by: Ori Kam
Patch applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
RE: [PATCH] net/mlx5: fix the timing of releasing drop action
Hi,
> -Original Message-
> From: Bing Zhao
> Sent: Friday, March 8, 2024 5:23 AM
> To: Suanming Mou ; dev@dpdk.org; Raslan
> Darawsheh
> Cc: Ori Kam ; Dariusz Sosnowski
> ; Slava Ovsiienko ;
> Matan Azrad ; sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix the timing of releasing drop action
>
> When creating the drop action Devx object, the global counter set is also used
> as in the regular or hairpin queue creation.
>
> The drop action should be destroyed before the global counter set release
> procedure. Or else, the counter set object is still referenced and cannot be
> released successfully. This would cause the counter set resources to be
> exhausted after starting and stopping the ports repeatedly.
>
> Fixes: 65b3cd0dc39b ("net/mlx5: create global drop action")
> Cc: suanmi...@nvidia.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Bing Zhao
> Acked-by: Suanming Mou
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
RE: [EXTERNAL] Re: [v11 0/4] PCI Dev and SG copy support
Hi Fengchengwen > Hi Gowrishankar, > > On 2024/3/12 20:24, Gowrishankar Muthukrishnan wrote: > > Hi Fengchengwen > > > >> > >> Hi Thomas, > >> > >> On 2024/3/12 17:15, Thomas Monjalon wrote: > >>> 07/03/2024 14:55, Gowrishankar Muthukrishnan: > Hi Fengchengwen, > > >> Waiting for a confirmation that this series is good to go. > > > > In the discuss of thread [1], I hope this patchset continue take a > > step forward (means new version) to support bi-direction test just > > by > >> modify config.ini file. > > > > This patch set already exposes all configuration via config.ini. I > didn't follow > >> what is missing. For bi-direction, we can better continue discussing > >> on that patch. > >>> > >>> Chengwen, please can you confirm whether you require a new version? > >>> Which change exactly is missing? > >> > >> This patchset is OK with one sub-test only tackle one DMA direction. > >> > > Thanks for the confirmation. > > > >> But there is a later patch [1] which will support multiple DMA > >> directions within one sub-test. > >> it will add a entry "xfer_mode", but I think it complicate the test, > >> I prefer we do more in this patchset to support some like > >> bi-direction just by modify config.ini, some like this: > >> > > I think we should discuss about that in bi-directional patch series. This > > series > is self-contained and there is no need to add bi-directional as part of this > series. As far as this patch set is concerned, all the options are exposed via > config.ini. Can you comment if there is anything missing, assuming that we are > taking bi-directional support as a separate feature addition. > > I have identified some improvements to the dma-perf app, and I plan to do it It is unclear at this point what is the issue that you have with the app or this patch set. This series was first submitted on Aug 10 2023. You had acked v8 on Jan 25 2024. After the patches were acked, there were still review comments on variable renames etc, which were all addressed. The patches had been under review for more than 8 months with very slow progress. > in 24.07, so if you don't mind, I will incorporate your commits (keeping your > signed-off-by) and modify to the one that I described above, and then send to > community (also with my improvements commits). I would like to have this series merged first and not pulled into another series. We do have few other features that we would like to add on top. I would assume that you can also add your changes on top. To make contribution easier, isn't it better to accept at least this patch set (as you acked earlier) and then you can continue working on the improvements? Thanks, Gowrishankar > > Thanks > > > > > Thanks, > > Gowrishankar > > > >> 1. extend lcore_dma: > >>current lcore_dma is: lcore10@:00:04.2 > >>extend it support: > >> lcore10@:00:04.2,dir=m2d,coreid=1,pfid=2,vfid=3,raddr=0x > >> 2. to fix one entry can't hold too many dma device, support entrys: > >> lcore_dma_1, lcore_dma_2 > >>which value is same with lcore_dma. > >> > >> So for bi-direction, we just define config.ini as: > >> > lcore_dma=lcore10@:00:04.2,dir=m2d,coreid=1,pfid=2,vfid=3,raddr=0 > >> x , > >> lcore10@:00:04.2,dir=d2m,coreid=1,pfid=2,vfid=3,raddr=0x > >> > >> [1] https://urldefense.proofpoint.com/v2/url?u=https- > >> 3A__patches.dpdk.org_project_dpdk_patch_20240229141426.4188428- > >> 2D1-2Damitprakashs- > >> 40marvell.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=EAtr- > >> > g7yUFhtOio8r2Rtm13Aqe4WVp_S_gHpcu6KFVo&m=DUaL_AJR1zqM0T2yw3 > >> aV44EObB90uqw5weFzSm- > >> > w39citSeGozNdEe4kzicss_KG&s=UTAcoZx5DjSJHyzxyLMxXz1bPqfPXQM7feDx > >> ZdC6Jgk&e= > >> > >>>
[PATCH v2] app/test: fix rsa tests in qat suite
This commit fixes incorrectly set keys in the
QAT testsuite for the RSA algorithm.
Fixes: 9b5465867fb8 ("test/crypto: add RSA none padding cases")
Cc: sta...@dpdk.org
Signed-off-by: Arkadiusz Kusztal
---
v2:
- removed camel case
app/test/test_cryptodev_asym.c | 102 ++---
app/test/test_cryptodev_rsa_test_vectors.h | 2 +-
2 files changed, 52 insertions(+), 52 deletions(-)
diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index 17daf734e8..2c745a7f7c 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -3292,11 +3292,8 @@ modular_multiplicative_inverse(const void *test_data)
arg.qt.coef.data = coef; \
arg.qt.coef.length = vector->coef.len
-typedef void (*rsa_key_init_t)(struct rte_crypto_asym_xform *,
- const struct rsa_test_data_2 *);
-
static int
-RSA_Encrypt(const struct rsa_test_data_2 *vector, uint8_t *cipher_buf)
+rsa_encrypt(const struct rsa_test_data_2 *vector, uint8_t *cipher_buf)
{
self->result_op = NULL;
/* Compute encryption on the test vector */
@@ -3314,7 +3311,7 @@ RSA_Encrypt(const struct rsa_test_data_2 *vector, uint8_t
*cipher_buf)
}
static int
-RSA_Decrypt(const struct rsa_test_data_2 *vector, uint8_t *plaintext,
+rsa_decrypt(const struct rsa_test_data_2 *vector, uint8_t *plaintext,
const int use_op)
{
uint8_t cipher[TEST_DATA_SIZE] = { 0 };
@@ -3335,41 +3332,14 @@ RSA_Decrypt(const struct rsa_test_data_2 *vector,
uint8_t *plaintext,
return 0;
}
-static void
-RSA_key_init_Exp(struct rte_crypto_asym_xform *xform,
- const struct rsa_test_data_2 *vector)
-{
- SET_RSA_PARAM(xform->rsa, vector, n);
- SET_RSA_PARAM(xform->rsa, vector, e);
- SET_RSA_PARAM(xform->rsa, vector, d);
- xform->rsa.key_type = RTE_RSA_KEY_TYPE_EXP;
-}
-
-static void
-RSA_key_init_CRT(struct rte_crypto_asym_xform *xform,
- const struct rsa_test_data_2 *vector)
-{
- SET_RSA_PARAM(xform->rsa, vector, n);
- SET_RSA_PARAM(xform->rsa, vector, e);
- SET_RSA_PARAM_QT(xform->rsa, vector, p);
- SET_RSA_PARAM_QT(xform->rsa, vector, q);
- SET_RSA_PARAM_QT(xform->rsa, vector, dP);
- SET_RSA_PARAM_QT(xform->rsa, vector, dQ);
- SET_RSA_PARAM_QT(xform->rsa, vector, qInv);
- xform->rsa.key_type = RTE_RSA_KEY_TYPE_QT;
-}
-
static int
-RSA_Init_Session(const struct rsa_test_data_2 *vector,
- rsa_key_init_t key_init)
+rsa_init_session(struct rte_crypto_asym_xform *xform)
{
const uint8_t dev_id = params->valid_devs[0];
struct rte_cryptodev_info dev_info;
- struct rte_crypto_asym_xform xform = { };
int ret = 0;
- key_init(&xform, vector);
- xform.xform_type = RTE_CRYPTO_ASYM_XFORM_RSA;
+ xform->xform_type = RTE_CRYPTO_ASYM_XFORM_RSA;
rte_cryptodev_info_get(dev_id, &dev_info);
if (!(dev_info.feature_flags & RTE_CRYPTODEV_FF_RSA_PRIV_OP_KEY_QT)) {
@@ -3377,7 +3347,7 @@ RSA_Init_Session(const struct rsa_test_data_2 *vector,
"Device doesn't support decrypt op with quintuple key
type. Test skipped\n");
return TEST_SKIPPED;
}
- ret = rte_cryptodev_asym_session_create(dev_id, &xform,
+ ret = rte_cryptodev_asym_session_create(dev_id, xform,
params->session_mpool, &self->sess);
if (ret < 0) {
RTE_LOG(ERR, USER1,
@@ -3388,17 +3358,23 @@ RSA_Init_Session(const struct rsa_test_data_2 *vector,
}
static int
-KAT_RSA_Encrypt(const void *data)
+kat_rsa_encrypt(const void *data)
{
uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
const struct rsa_test_data_2 *vector = data;
- int ret = RSA_Init_Session(vector, RSA_key_init_Exp);
+ struct rte_crypto_asym_xform xform = { };
+
+ SET_RSA_PARAM(xform.rsa, vector, n);
+ SET_RSA_PARAM(xform.rsa, vector, e);
+ SET_RSA_PARAM(xform.rsa, vector, d);
+ xform.rsa.key_type = RTE_RSA_KEY_TYPE_EXP;
+ int ret = rsa_init_session(&xform);
if (ret) {
RTE_LOG(ERR, USER1, "Failed to init session for RSA\n");
return ret;
}
- TEST_ASSERT_SUCCESS(RSA_Encrypt(vector, cipher_buf),
+ TEST_ASSERT_SUCCESS(rsa_encrypt(vector, cipher_buf),
"RSA: Failed to encrypt");
TEST_ASSERT_BUFFERS_ARE_EQUAL(vector->cipher.data,
self->result_op->asym->rsa.cipher.data,
@@ -3408,17 +3384,26 @@ KAT_RSA_Encrypt(const void *data)
}
static int
-KAT_RSA_Encrypt_CRT(const void *data)
+kat_rsa_encrypt_crt(const void *data)
{
uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
const struct rsa_test_data_2 *vector = data;
- int ret = RSA_Init_Session(vector, RSA_key_init_CRT);
+ struct rte_crypto_asym_xform xform = { };
+ SET_RSA_PARAM(xform.rsa, vector, n);
+ SET_RSA_PARAM(xform.rsa, vector, e);
+ SET_RSA_PARAM_
RE: [PATCH 1/6] examples/l3fwd: fix lcore ID restriction
[AMD Official Use Only - General]
Hi,
> -Original Message-
> From: David Marchand
> Sent: Thursday, March 7, 2024 2:04 PM
> To: Tummala, Sivaprasad
> Cc: david.h...@intel.com; anatoly.bura...@intel.com; jer...@marvell.com;
> radu.nico...@intel.com; gak...@marvell.com; cristian.dumitre...@intel.com;
> Yigit,
> Ferruh ; konstantin.anan...@huawei.com;
> dev@dpdk.org; sta...@dpdk.org
> Subject: Re: [PATCH 1/6] examples/l3fwd: fix lcore ID restriction
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Mon, Dec 18, 2023 at 8:49 AM Sivaprasad Tummala
> wrote:
> >
> > Currently the config option allows lcore IDs up to 255, irrespective
> > of RTE_MAX_LCORES and needs to be fixed.
>
> "needs to be fixed" ?
> I disagree on the principle.
> The examples were written with limitations, this is not a bug.
>
> >
> > The patch allows config options based on DPDK config.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: sta...@dpdk.org
>
> Please remove this request for backport in the next revision.
>
> >
> > Signed-off-by: Sivaprasad Tummala
> > ---
> > examples/l3fwd/main.c | 15 +--
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c index
> > 3bf28aec0c..847ded0ad2 100644
> > --- a/examples/l3fwd/main.c
> > +++ b/examples/l3fwd/main.c
> > @@ -99,7 +99,7 @@ struct parm_cfg parm_config; struct lcore_params {
> > uint16_t port_id;
> > uint8_t queue_id;
> > - uint8_t lcore_id;
> > + uint16_t lcore_id;
>
> lcore_id are stored as an unsigned int (so potentially 32bits) in EAL.
> Moving to uint16_t seems not enough.
Will fix this to be aligned to EAL. However, I don't think of a SOC/CPU with
more than
65536 cores/threads.
>
>
> > } __rte_cache_aligned;
> >
> > static struct lcore_params lcore_params_array[MAX_LCORE_PARAMS];
> > @@ -292,8 +292,8 @@ setup_l3fwd_lookup_tables(void) static int
> > check_lcore_params(void)
> > {
> > - uint8_t queue, lcore;
> > - uint16_t i;
> > + uint8_t queue;
> > + uint16_t i, lcore;
> > int socketid;
> >
> > for (i = 0; i < nb_lcore_params; ++i) { @@ -359,7 +359,7 @@
> > static int
> > init_lcore_rx_queues(void)
> > {
> > uint16_t i, nb_rx_queue;
> > - uint8_t lcore;
> > + uint16_t lcore;
> >
> > for (i = 0; i < nb_lcore_params; ++i) {
> > lcore = lcore_params[i].lcore_id; @@ -500,6 +500,8 @@
> > parse_config(const char *q_arg)
> > char *str_fld[_NUM_FLD];
> > int i;
> > unsigned size;
> > + unsigned int max_fld[_NUM_FLD] = {RTE_MAX_ETHPORTS,
>
> This change here is not described in the commitlog and introduces a bug.
OK! Will fix it
>
> In this example, queue_id is stored as a uint8_t.
> queue_id are stored as uint16_t in ethdev.
> Yet RTE_MAX_ETHPORTS can be larger than 255.
queue_id is already modified as uint16_t based on the earlier comment from
Konstantin.
The port_id is already uint16_t even if RTE_MAX_ETHPORTS exceeds 255.
I will fix the max_fld type to "uint32_t" to accommodate lcore.
>
>
> > + 255, RTE_MAX_LCORE};
> >
> > nb_lcore_params = 0;
> >
> > @@ -518,7 +520,8 @@ parse_config(const char *q_arg)
> > for (i = 0; i < _NUM_FLD; i++){
> > errno = 0;
> > int_fld[i] = strtoul(str_fld[i], &end, 0);
> > - if (errno != 0 || end == str_fld[i] || int_fld[i] >
> > 255)
> > + if (errno != 0 || end == str_fld[i] || int_fld[i] >
> > +
> > + max_fld[i])
> > return -1;
> > }
> > if (nb_lcore_params >= MAX_LCORE_PARAMS) { @@ -531,7
> > +534,7 @@ parse_config(const char *q_arg)
> > lcore_params_array[nb_lcore_params].queue_id =
> > (uint8_t)int_fld[FLD_QUEUE];
> > lcore_params_array[nb_lcore_params].lcore_id =
> > - (uint8_t)int_fld[FLD_LCORE];
> > + (uint16_t)int_fld[FLD_LCORE];
> > ++nb_lcore_params;
> > }
> > lcore_params = lcore_params_array;
> > --
> > 2.25.1
> >
>
> I did not check other patches in the series, but I suggest you revisit them
> in the
> light of those comments.
OK
>
>
> --
> David Marchand
Thanks & Regards,
Sivaprasad
RE: [PATCH 1/2] net/mlx5: update speed capabilities parsing on Linux
Hi, > -Original Message- > From: Thomas Monjalon > Sent: Tuesday, March 5, 2024 3:13 PM > To: dev@dpdk.org > Cc: Dariusz Sosnowski ; Slava Ovsiienko > ; Ori Kam ; Suanming Mou > ; Matan Azrad > Subject: [PATCH 1/2] net/mlx5: update speed capabilities parsing on Linux > > Ease maintenance of speed capabilities parsing from ethtool > by using rte_eth_link_speed_g*(). > Functions in ethdev library are simpler, more complete, > and easier to maintain. > > Signed-off-by: Thomas Monjalon Series applied to next-net-mlx, Kindest regards, Raslan Darawsheh
RE: [PATCH v2] app/dma-perf: calrify incorrect NUMA config
>Ok, the naming convention used by me is `CamelCase`. One suggested from your
>end is `snake_case`.
>Does DPDK has a constrain it can not use CamelCase.
Please refer to:
https://doc.dpdk.org/guides/contributing/coding_style.html
In particular:
1.5.4. Variable Declarations
In declarations, do not put any whitespace between asterisks and adjacent
tokens, except for tokens that are identifiers related to types. (These
identifiers are the names of basic types, type qualifiers, and typedef-names
other than the one being declared.) Separate these identifiers from asterisks
using a single space.
For example:
int *x; /* no space after asterisk */
int * const x; /* space after asterisk when using a type qualifier */
· All externally-visible variables should have an rte_ prefix in the
name to avoid namespace collisions.
· Do not use uppercase letters - either in the form of ALL_UPPERCASE,
or CamelCase - in variable names. Lower-case letters and underscores only.
From: Varghese, Vipin
Sent: Tuesday, March 12, 2024 3:48 AM
To: Fengchengwen ; dev@dpdk.org
Cc: ferruh.yi...@amd.com; neerav.par...@amd.com
Subject: Re: [PATCH v2] app/dma-perf: calrify incorrect NUMA config
diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
index 9b1f58c78c..b6d0dbe4c0 100644
--- a/app/test-dma-perf/benchmark.c
+++ b/app/test-dma-perf/benchmark.c
@@ -311,9 +311,14 @@ setup_memory_env(struct test_configure *cfg, struct
rte_mbuf ***srcs,
uint32_t nr_buf = cfg->nr_buf;
nr_sockets = rte_socket_count();
- if (cfg->src_numa_node >= nr_sockets ||
- cfg->dst_numa_node >= nr_sockets) {
- printf("Error: Source or destination numa exceeds the acture numa
nodes.\n");
+
+ bool isSrcNumaIncorrect = (cfg->src_numa_node >= nr_sockets);
+ bool isDstNumaIncorrect = (cfg->dst_numa_node >= nr_sockets);
The naming style needs to be adjusted, how about
bool is_src_numa_exceed, is_dst_numa_exceed;
Ok, the naming convention used by me is `CamelCase`. One suggested from your
end is `snake_case`.
Does DPDK has a constrain it can not use CamelCase.
And predefine the variable at the beginning of function, sort by length, some
like:
bool is_src_numa_exceed, is_dst_numa_exceed;
unsigned int buf_size = cfg->buf_size.cur;
uint32_t nr_buf = cfg->nr_buf;
unsigned int nr_sockets;
nr_sockets = rte_socket_count();
is_src_numa_exceed =
is_dst_numa_exceed =
if (xxx)
...
+
+ if (isSrcNumaIncorrect || isDstNumaIncorrect) {
+ PRINT_ERR("Error: NUMA config exceeds the actual numa nodes for
%s.\n",
+ (isSrcNumaIncorrect && isDstNumaIncorrect) ? "Source &
Destination" :
+ (isSrcNumaIncorrect) ? "Source" : "Destination");
Please don't capitalize the first letter of "Source" and "Destination"
Can you please explain why?
Thanks
return -1;
}
RE: [PATCH v7 3/4] net/mlx5: fix warning about rte_memcpy length
Hi,
> -Original Message-
> From: Morten Brørup
> Sent: Monday, January 16, 2023 3:07 PM
> To: dev@dpdk.org; roret...@linux.microsoft.com; rm...@marvell.com;
> timothy.mcdan...@intel.com; Matan Azrad ; Slava
> Ovsiienko
> Cc: ruifeng.w...@arm.com; zhou...@loongson.cn;
> d...@linux.vnet.ibm.com; k...@semihalf.com; bruce.richard...@intel.com;
> konstantin.v.anan...@yandex.ru; Morten Brørup
> ; Xueming(Steven) Li
> Subject: [PATCH v7 3/4] net/mlx5: fix warning about rte_memcpy length
>
> Use RTE_PTR_ADD where copying to the offset of a field in a structure holding
> multiple fields, to avoid compiler warnings with decorated rte_memcpy.
>
> Fixes: 16a7dbc4f69006cc1c96ca2a2c6d3e3c51a2ff50 ("net/mlx5: make flow
> modify action list thread safe")
> Cc: xuemi...@nvidia.com
> Cc: ma...@nvidia.com
> Cc: viachesl...@nvidia.com
>
> Signed-off-by: Morten Brørup
Patch applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
RE: [EXTERNAL] [PATCH v2] app/test: fix rsa tests in qat suite
> This commit fixes incorrectly set keys in the
> QAT testsuite for the RSA algorithm.
>
> Fixes: 9b5465867fb8 ("test/crypto: add RSA none padding cases")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Arkadiusz Kusztal
> ---
> v2:
> - removed camel case
Removing camel case should be a separate patch
Or update the patch title description appropriately.
>
> app/test/test_cryptodev_asym.c | 102
> ++---
> app/test/test_cryptodev_rsa_test_vectors.h | 2 +-
> 2 files changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
> index 17daf734e8..2c745a7f7c 100644
> --- a/app/test/test_cryptodev_asym.c
> +++ b/app/test/test_cryptodev_asym.c
> @@ -3292,11 +3292,8 @@ modular_multiplicative_inverse(const void
> *test_data)
> arg.qt.coef.data = coef; \
> arg.qt.coef.length = vector->coef.len
>
> -typedef void (*rsa_key_init_t)(struct rte_crypto_asym_xform *,
> - const struct rsa_test_data_2 *);
> -
> static int
> -RSA_Encrypt(const struct rsa_test_data_2 *vector, uint8_t *cipher_buf)
> +rsa_encrypt(const struct rsa_test_data_2 *vector, uint8_t *cipher_buf)
> {
> self->result_op = NULL;
> /* Compute encryption on the test vector */
> @@ -3314,7 +3311,7 @@ RSA_Encrypt(const struct rsa_test_data_2 *vector,
> uint8_t *cipher_buf)
> }
>
> static int
> -RSA_Decrypt(const struct rsa_test_data_2 *vector, uint8_t *plaintext,
> +rsa_decrypt(const struct rsa_test_data_2 *vector, uint8_t *plaintext,
> const int use_op)
> {
> uint8_t cipher[TEST_DATA_SIZE] = { 0 };
> @@ -3335,41 +3332,14 @@ RSA_Decrypt(const struct rsa_test_data_2 *vector,
> uint8_t *plaintext,
> return 0;
> }
>
> -static void
> -RSA_key_init_Exp(struct rte_crypto_asym_xform *xform,
> - const struct rsa_test_data_2 *vector)
> -{
> - SET_RSA_PARAM(xform->rsa, vector, n);
> - SET_RSA_PARAM(xform->rsa, vector, e);
> - SET_RSA_PARAM(xform->rsa, vector, d);
> - xform->rsa.key_type = RTE_RSA_KEY_TYPE_EXP;
> -}
> -
> -static void
> -RSA_key_init_CRT(struct rte_crypto_asym_xform *xform,
> - const struct rsa_test_data_2 *vector)
> -{
> - SET_RSA_PARAM(xform->rsa, vector, n);
> - SET_RSA_PARAM(xform->rsa, vector, e);
> - SET_RSA_PARAM_QT(xform->rsa, vector, p);
> - SET_RSA_PARAM_QT(xform->rsa, vector, q);
> - SET_RSA_PARAM_QT(xform->rsa, vector, dP);
> - SET_RSA_PARAM_QT(xform->rsa, vector, dQ);
> - SET_RSA_PARAM_QT(xform->rsa, vector, qInv);
> - xform->rsa.key_type = RTE_RSA_KEY_TYPE_QT;
> -}
> -
> static int
> -RSA_Init_Session(const struct rsa_test_data_2 *vector,
> - rsa_key_init_t key_init)
> +rsa_init_session(struct rte_crypto_asym_xform *xform)
> {
> const uint8_t dev_id = params->valid_devs[0];
> struct rte_cryptodev_info dev_info;
> - struct rte_crypto_asym_xform xform = { };
> int ret = 0;
>
> - key_init(&xform, vector);
> - xform.xform_type = RTE_CRYPTO_ASYM_XFORM_RSA;
> + xform->xform_type = RTE_CRYPTO_ASYM_XFORM_RSA;
>
> rte_cryptodev_info_get(dev_id, &dev_info);
> if (!(dev_info.feature_flags &
> RTE_CRYPTODEV_FF_RSA_PRIV_OP_KEY_QT)) {
> @@ -3377,7 +3347,7 @@ RSA_Init_Session(const struct rsa_test_data_2
> *vector,
> "Device doesn't support decrypt op with quintuple key
> type. Test skipped\n");
> return TEST_SKIPPED;
> }
> - ret = rte_cryptodev_asym_session_create(dev_id, &xform,
> + ret = rte_cryptodev_asym_session_create(dev_id, xform,
> params->session_mpool, &self->sess);
> if (ret < 0) {
> RTE_LOG(ERR, USER1,
> @@ -3388,17 +3358,23 @@ RSA_Init_Session(const struct rsa_test_data_2
> *vector,
> }
>
> static int
> -KAT_RSA_Encrypt(const void *data)
> +kat_rsa_encrypt(const void *data)
> {
> uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
> const struct rsa_test_data_2 *vector = data;
> - int ret = RSA_Init_Session(vector, RSA_key_init_Exp);
> + struct rte_crypto_asym_xform xform = { };
> +
> + SET_RSA_PARAM(xform.rsa, vector, n);
> + SET_RSA_PARAM(xform.rsa, vector, e);
> + SET_RSA_PARAM(xform.rsa, vector, d);
> + xform.rsa.key_type = RTE_RSA_KEY_TYPE_EXP;
> + int ret = rsa_init_session(&xform);
>
> if (ret) {
> RTE_LOG(ERR, USER1, "Failed to init session for RSA\n");
> return ret;
> }
> - TEST_ASSERT_SUCCESS(RSA_Encrypt(vector, cipher_buf),
> + TEST_ASSERT_SUCCESS(rsa_encrypt(vector, cipher_buf),
> "RSA: Failed to encrypt");
> TEST_ASSERT_BUFFERS_ARE_EQUAL(vector->cipher.data,
> self->result_op->asym->rsa.cipher.data,
> @@ -3408,17 +3384,26 @@ KAT_RSA_Encrypt(const void *data)
> }
>
> static int
> -KAT_RSA_Encrypt_CRT(const void *data)
> +kat_rsa_encrypt_crt(const void *data)
> {
> uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
> const struct rsa_test_d
[PATCH v3 1/1] net/mana: add vlan tagging support
For tx path, use LONG_PACKET_FORMAT if vlan tag is present. For rx,
extract vlan id from oob, put into mbuf and set the vlan flags in
mbuf.
Signed-off-by: Wei Hu
---
v3:
- Adjust the pkt_idx position in the code so it will be executed even when
adding vlan header fails.
v2:
- Use existing vlan tag processing macros.
- Add vlan header back if vlan_strip flag is not set on the receiving path.
drivers/net/mana/mana.c | 3 +++
drivers/net/mana/mana.h | 4
drivers/net/mana/rx.c | 22 ++
drivers/net/mana/tx.c | 21 ++---
4 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 2df2461d2f..68c625258e 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -94,6 +94,9 @@ mana_dev_configure(struct rte_eth_dev *dev)
return -EINVAL;
}
+ priv->vlan_strip = !!(dev_conf->rxmode.offloads &
+ RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
+
priv->num_queues = dev->data->nb_rx_queues;
manadv_set_context_attr(priv->ib_ctx, MANADV_CTX_ATTR_BUF_ALLOCATORS,
diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h
index 3626925871..37f654f0e6 100644
--- a/drivers/net/mana/mana.h
+++ b/drivers/net/mana/mana.h
@@ -21,10 +21,12 @@ struct mana_shared_data {
#define MANA_MAX_MAC_ADDR 1
#define MANA_DEV_RX_OFFLOAD_SUPPORT ( \
+ RTE_ETH_RX_OFFLOAD_VLAN_STRIP | \
RTE_ETH_RX_OFFLOAD_CHECKSUM | \
RTE_ETH_RX_OFFLOAD_RSS_HASH)
#define MANA_DEV_TX_OFFLOAD_SUPPORT ( \
+ RTE_ETH_TX_OFFLOAD_VLAN_INSERT | \
RTE_ETH_TX_OFFLOAD_MULTI_SEGS | \
RTE_ETH_TX_OFFLOAD_IPV4_CKSUM | \
RTE_ETH_TX_OFFLOAD_TCP_CKSUM | \
@@ -345,6 +347,8 @@ struct mana_priv {
/* IB device port */
uint8_t dev_port;
+ uint8_t vlan_strip;
+
struct ibv_context *ib_ctx;
struct ibv_pd *ib_pd;
struct ibv_pd *ib_parent_pd;
diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c
index 16e647baf5..0c26702b73 100644
--- a/drivers/net/mana/rx.c
+++ b/drivers/net/mana/rx.c
@@ -532,10 +532,6 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
uint16_t pkts_n)
mbuf->hash.rss = oob->packet_info[pkt_idx].packet_hash;
}
- pkts[pkt_received++] = mbuf;
- rxq->stats.packets++;
- rxq->stats.bytes += mbuf->data_len;
-
pkt_idx++;
/* Move on the next completion if all packets are processed */
if (pkt_idx >= RX_COM_OOB_NUM_PACKETINFO_SEGMENTS) {
@@ -543,6 +539,24 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
uint16_t pkts_n)
i++;
}
+ if (oob->rx_vlan_tag_present) {
+ mbuf->ol_flags |=
+ RTE_MBUF_F_RX_VLAN |
RTE_MBUF_F_RX_VLAN_STRIPPED;
+ mbuf->vlan_tci = oob->rx_vlan_id;
+
+ if (!priv->vlan_strip && rte_vlan_insert(&mbuf)) {
+ DRV_LOG(ERR, "vlan insert failed");
+ rxq->stats.errors++;
+ rte_pktmbuf_free(mbuf);
+
+ goto drop;
+ }
+ }
+
+ pkts[pkt_received++] = mbuf;
+ rxq->stats.packets++;
+ rxq->stats.bytes += mbuf->data_len;
+
drop:
rxq->desc_ring_tail++;
if (rxq->desc_ring_tail >= rxq->num_desc)
diff --git a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c
index 58c4a1d976..272a28bcba 100644
--- a/drivers/net/mana/tx.c
+++ b/drivers/net/mana/tx.c
@@ -254,7 +254,18 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts)
}
/* Fill in the oob */
- tx_oob.short_oob.packet_format = SHORT_PACKET_FORMAT;
+ if (m_pkt->ol_flags & RTE_MBUF_F_TX_VLAN) {
+ tx_oob.short_oob.packet_format = LONG_PACKET_FORMAT;
+ tx_oob.long_oob.inject_vlan_prior_tag = 1;
+ tx_oob.long_oob.priority_code_point =
+ RTE_VLAN_TCI_PRI(m_pkt->vlan_tci);
+ tx_oob.long_oob.drop_eligible_indicator =
+ RTE_VLAN_TCI_DEI(m_pkt->vlan_tci);
+ tx_oob.long_oob.vlan_identifier =
+ RTE_VLAN_TCI_ID(m_pkt->vlan_tci);
+ } else {
+ tx_oob.short_oob.packet_format = SHORT_PACKET_FORMAT;
+ }
tx_oob.short_oob.tx_is_outer_ipv4 =
m_pkt->ol_flags & RTE_MBUF_F_TX_IPV4 ? 1 : 0;
tx_oob.short_oob.tx_is_outer_ipv6 =
@@ -409,8 +420,12 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts,
uint16_t
RE: flow create with queue range not working
Hi Team, Gentle reminder! Can somebody please help us on this query. We are validating the fdir with test-pmd tool. We are getting below error while trying to add the flow create rule for the same. Syntax: testpmd> flow create 0 ingress pattern eth / ipv4 src is ipv4 dst is / tcp src is dst is / end actions rss queues 2 3 end / end rule: flow create 0 ingress pattern eth / ipv4 src is 20.20.20.2 dst is 20.20.20.4 / tcp src is 80 dst is 1501 / end actions rss queues 0 1 2 3 end / end port_flow_complain(): Caught PMD error type 16 (specific action): cause: 0x7fff9495c648, RSS types must be empty while configuring queue region: Operation not supported Note: this is for i40e Please help us on this. Thanks, Vajith -Original Message- From: Ferruh Yigit Sent: Monday, March 11, 2024 4:38 PM To: Vajith Raghman ; Beilei Xing ; Jeff Guo ; Bruce Richardson Cc: dev@dpdk.org; us...@dpdk.org Subject: Re: flow create with queue range not working CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. On 3/11/2024 10:32 AM, Vajith Raghman wrote: > Hi Team, > > We are validating the fdir with test-pmd tool. We are getting below > error while trying to add the flow create rule for the same. > > Syntax: > > testpmd>flow create 0ingress pattern eth / ipv4 src *is*ipv4 > dst *is*/tcp src *is* dst *is*/end > actions rss queues 23end /end > > rule: > > flow create 0 ingress pattern eth / ipv4 src is 20.20.20.2 dst is > 20.20.20.4 / tcp src is 80 dst is 1501 / end actions rss queues 0 1 2 > 3 end / end > > port_flow_complain(): Caught PMD error type 16 (specific action): cause: > 0x7fff9495c648, RSS types must be empty while configuring queue region: > Operation not supported > > Please help us on this. > I guess the error from i40e, cc'ing relevant maintainers.
Re: [PATCH 2/4] app/testpmd: fix burst option parsing
On 3/13/2024 7:24 AM, David Marchand wrote:
> On Tue, Mar 12, 2024 at 5:47 PM Ferruh Yigit wrote:
>>
>> On 3/8/2024 2:48 PM, David Marchand wrote:
>>> rte_eth_dev_info_get() is not supposed to fail for a valid port_id, but
>>> for the theoretical case when it would fail, raise an error rather than
>>> skip subsequent options.
>>>
>>> Fixes: 6f51deb903b2 ("app/testpmd: check status of getting ethdev info")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: David Marchand
>>> ---
>>> app/test-pmd/parameters.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index d715750bb8..8c21744009 100644
>>> --- a/app/test-pmd/parameters.c
>>> +++ b/app/test-pmd/parameters.c
>>> @@ -1128,9 +1128,9 @@ launch_args_parse(int argc, char** argv)
>>> 0,
>>> &dev_info);
>>> if (ret != 0)
>>> - return;
>>> -
>>> - rec_nb_pkts = dev_info
>>> + rec_nb_pkts = 0;
>>> + else
>>> + rec_nb_pkts = dev_info
>>>
>>> .default_rxportconf.burst_size;
>>>
>>> if (rec_nb_pkts == 0)
>>
>> 'eth_dev_info_get_print_err()' already fail, but it may not be very
>> clear to the user,
>> OK to print a failure log, but setting 'rec_nb_pkts = 0;' as above also
>> will generate an error message that also may be confusing to the user.
>>
>> What about print an explicit error message for the
>> 'eth_dev_info_get_print_err()' failed case?
>
> rte_exit(EXIT_FAILURE, "Failed to retrieve device info, this is
> probably a driver bug. "
> "To workaround this issue, please provide a value between 1
> and %d\n", MAX_PKT_BURST);
>
> Does it work for you?
>
>
'eth_dev_info_get_print_err()' already logs error about getting device
info, but driver recommended 'burst' setting failed information is missing.
What about more direct,
"Failed to get driver recommended burst size, please provide a value
between 1 and MAX_PKT_BURST"
Re: [PATCH 3/4] app/testpmd: check queue count for related options
On 3/13/2024 7:37 AM, David Marchand wrote:
> On Tue, Mar 12, 2024 at 5:59 PM Ferruh Yigit wrote:
>>
>> On 3/8/2024 2:48 PM, David Marchand wrote:
>>> Checking the number of rxq/txq in the middle of option parsing is
>>> confusing. Move the check where nb_rxq / nb_txq are modified.
>>>
>>> Signed-off-by: David Marchand
>>> ---
>>> app/test-pmd/parameters.c | 8
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index 8c21744009..271f0c995a 100644
>>> --- a/app/test-pmd/parameters.c
>>> +++ b/app/test-pmd/parameters.c
>>> @@ -1063,6 +1063,8 @@ launch_args_parse(int argc, char** argv)
>>> rte_exit(EXIT_FAILURE, "rxq %d
>>> invalid - must be"
>>> " >= 0 && <= %u\n", n,
>>>
>>> get_allowed_max_nb_rxq(&pid));
>>> + if (!nb_rxq && !nb_txq)
>>> + rte_exit(EXIT_FAILURE, "Either rx or
>>> tx queues should be non-zero\n");
>>> }
>>> if (!strcmp(lgopts[opt_idx].name, "txq")) {
>>> n = atoi(optarg);
>>> @@ -1072,6 +1074,8 @@ launch_args_parse(int argc, char** argv)
>>> rte_exit(EXIT_FAILURE, "txq %d
>>> invalid - must be"
>>> " >= 0 && <= %u\n", n,
>>>
>>> get_allowed_max_nb_txq(&pid));
>>> + if (!nb_rxq && !nb_txq)
>>> + rte_exit(EXIT_FAILURE, "Either rx or
>>> tx queues should be non-zero\n");
>>> }
>>> if (!strcmp(lgopts[opt_idx].name, "hairpinq")) {
>>> n = atoi(optarg);
>>> @@ -1098,10 +1102,6 @@ launch_args_parse(int argc, char** argv)
>>> n + nb_rxq,
>>>
>>> get_allowed_max_nb_rxq(&pid));
>>> }
>>> - if (!nb_rxq && !nb_txq) {
>>> - rte_exit(EXIT_FAILURE, "Either rx or tx
>>> queues should "
>>> - "be non-zero\n");
>>> - }
>>> if (!strcmp(lgopts[opt_idx].name, "hairpin-mode")) {
>>> char *end = NULL;
>>> unsigned int n;
>>
>> There is already a runtime check for queues [1], perhaps we can remove
>> it altogether from arg parse.
>
> Good catch.
>
> This other check comes after parsing args, so I suspect it is just dead code.
> I guess I'll change it into a rte_exit(EXIT_FAILURE..).
> Is this what you propose?
>
I think that check is the main check for nb_rxq and nb_txq.
The one you removed is for the 'hairpinq' parameter, which is not a very
common usecase.
But nb_rxq and nb_txq requirement is very common and it is protected in
the main after parameter parsing.
I am not suggesting adding 'rte_exit()' for that case, probably it will
fail in some other part and error log can provide the required hint.
And I am worried if it breaks some unexpected usecase with exit.
I was just thinking the check can be removed from 'hairpinq' parameter.
>
>>
>> Also I think the order of the 'hairpinq' and queue number parameter
>> processing depends on order user provided, so this may not be very
>> reliable anyway.
>
> Indeed.
>
>
[PATCH v5 00/21] Improvements and new test cases
v5: * Define TEST_SEC_CIPHERTEXT_MAX_LEN based on existing MBUF_DATAPAYLOAD_SIZE macro. v4: * Set max ciphertext length for data walkthrough tests to 4k. v3: * Set max packet length for data walkthrough tests to 8k. v2: * Rebased. Aakash Sasidharan (7): test/security: enable AES-GCM in combined mode TLS test/security: add TLS 1.2 data walkthrough test test/security: add DTLS 1.2 data walkthrough test test/security: add TLS SG data walkthrough test test/security: add DTLS 1.2 anti-replay tests test/security: add more DTLS anti-replay window sz test/security: add out of place sgl test case for TLS 1.2 Akhil Goyal (2): test/security: add TLS/DTLS 1.2 AES-256-SHA384 vectors test/crypto: add TLS 1.3 vectors Anoob Joseph (1): test/cryptodev: allow zero packet length buffers Vidya Sagar Velumuri (11): test/security: unit test for TLS packet corruption test/security: unit test for custom content verification test/security: unit test to verify zero TLS records test/security: add unit tests for DTLS-1.2 test/crypto: update verification of header test/crypto: update framework to verify tls-1.3 test/crypto: test to verify hdr corruption in TLS test/crypto: test to verify custom content type in TLS test/crypto: test to verify zero len record in TLS test/crypto: unit tests to verify padding in TLS test/crypto: unit tests for padding in DTLS-1.2 app/test/test_cryptodev.c | 975 -- app/test/test_cryptodev.h | 20 +- app/test/test_cryptodev_security_tls_record.c | 203 ++-- app/test/test_cryptodev_security_tls_record.h | 77 +- ...yptodev_security_tls_record_test_vectors.h | 405 app/test/test_security_proto.c| 17 + app/test/test_security_proto.h| 11 + 7 files changed, 1530 insertions(+), 178 deletions(-) -- 2.25.1
[PATCH v5 01/21] test/security: enable AES-GCM in combined mode TLS
Enable AES-GCM AEAD tests in combined mode TLS test suite.
Coverity issue: 414888
Fixes: 9157ccb8f876 ("test/crypto: verify TLS headers")
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev_security_tls_record.c | 10 --
app/test/test_security_proto.h| 3 +++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index bcb2eba4ff..14a7a2511e 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -116,6 +116,7 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
}
} else {
mac_len = td->xform.aead.aead.digest_length;
+ roundup_len = 0;
exp_nonce_len = 8;
}
@@ -123,7 +124,10 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
case RTE_SECURITY_VERSION_TLS_1_2:
case RTE_SECURITY_VERSION_TLS_1_3:
hdr_len = sizeof(struct rte_tls_hdr);
- min_padding = 1;
+ if (td->aead)
+ min_padding = 0;
+ else
+ min_padding = 1;
break;
case RTE_SECURITY_VERSION_DTLS_1_2:
hdr_len = sizeof(struct rte_dtls_hdr);
@@ -139,7 +143,9 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
/* Padding */
tls_pkt_size += min_padding;
- tls_pkt_size = RTE_ALIGN_MUL_CEIL(tls_pkt_size, roundup_len);
+
+ if (roundup_len)
+ tls_pkt_size = RTE_ALIGN_MUL_CEIL(tls_pkt_size, roundup_len);
/* Explicit nonce */
tls_pkt_size += exp_nonce_len;
diff --git a/app/test/test_security_proto.h b/app/test/test_security_proto.h
index efa023b99d..5b92daa810 100644
--- a/app/test/test_security_proto.h
+++ b/app/test/test_security_proto.h
@@ -27,16 +27,19 @@ static const struct crypto_param aead_list[] = {
.type = RTE_CRYPTO_SYM_XFORM_AEAD,
.alg.aead = RTE_CRYPTO_AEAD_AES_GCM,
.key_length = 16,
+ .digest_length = 16,
},
{
.type = RTE_CRYPTO_SYM_XFORM_AEAD,
.alg.aead = RTE_CRYPTO_AEAD_AES_GCM,
.key_length = 24,
+ .digest_length = 16,
},
{
.type = RTE_CRYPTO_SYM_XFORM_AEAD,
.alg.aead = RTE_CRYPTO_AEAD_AES_GCM,
.key_length = 32,
+ .digest_length = 16,
},
{
.type = RTE_CRYPTO_SYM_XFORM_AEAD,
--
2.25.1
[PATCH v5 02/21] test/security: add TLS 1.2 data walkthrough test
Add data walkthrough test for TLS 1.2.
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev.c | 90 +--
app/test/test_cryptodev_security_tls_record.c | 25 --
app/test/test_cryptodev_security_tls_record.h | 41 -
app/test/test_security_proto.c| 17
app/test/test_security_proto.h| 8 ++
5 files changed, 162 insertions(+), 19 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 3b5e784022..c5837ccbdd 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -858,6 +858,8 @@ ipsec_proto_testsuite_setup(void)
static int
tls_record_proto_testsuite_setup(void)
{
+ test_sec_proto_pattern_generate();
+
return sec_proto_testsuite_setup(RTE_SECURITY_PROTOCOL_TLS_RECORD);
}
@@ -11958,14 +11960,30 @@ test_tls_record_proto_known_vec_read(const void
*test_data)
static int
test_tls_record_proto_all(const struct tls_record_test_flags *flags)
{
+ unsigned int i, nb_pkts = 1, pass_cnt = 0, payload_len, max_payload_len;
struct tls_record_test_data td_outb[TEST_SEC_PKTS_MAX];
struct tls_record_test_data td_inb[TEST_SEC_PKTS_MAX];
- unsigned int i, nb_pkts = 1, pass_cnt = 0;
int ret;
+ switch (flags->tls_version) {
+ case RTE_SECURITY_VERSION_TLS_1_2:
+ max_payload_len = TLS_1_2_RECORD_PLAINTEXT_MAX_LEN;
+ break;
+ case RTE_SECURITY_VERSION_TLS_1_3:
+ max_payload_len = TLS_1_3_RECORD_PLAINTEXT_MAX_LEN;
+ break;
+ case RTE_SECURITY_VERSION_DTLS_1_2:
+ max_payload_len = DTLS_1_2_RECORD_PLAINTEXT_MAX_LEN;
+ break;
+ default:
+ max_payload_len = 0;
+ }
+
for (i = 0; i < RTE_DIM(sec_alg_list); i++) {
+ payload_len = TLS_RECORD_PLAINTEXT_MIN_LEN;
+again:
test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2, flags,
- td_outb, nb_pkts);
+ td_outb, nb_pkts, payload_len);
ret = test_tls_record_proto_process(td_outb, td_inb, nb_pkts,
true, flags);
if (ret == TEST_SKIPPED)
@@ -11983,6 +12001,9 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
if (ret == TEST_FAILED)
return TEST_FAILED;
+ if (flags->data_walkthrough && (++payload_len <=
max_payload_len))
+ goto again;
+
if (flags->display_alg)
test_sec_alg_display(sec_alg_list[i].param1,
sec_alg_list[i].param2);
@@ -11996,22 +12017,69 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
}
static int
-test_tls_record_proto_display_list(void)
+test_tls_1_2_record_proto_data_walkthrough(void)
+{
+ struct tls_record_test_flags flags;
+
+ memset(&flags, 0, sizeof(flags));
+
+ flags.data_walkthrough = true;
+ flags.tls_version = RTE_SECURITY_VERSION_TLS_1_2;
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_tls_1_2_record_proto_display_list(void)
{
struct tls_record_test_flags flags;
memset(&flags, 0, sizeof(flags));
flags.display_alg = true;
+ flags.tls_version = RTE_SECURITY_VERSION_TLS_1_2;
return test_tls_record_proto_all(&flags);
}
static int
-test_tls_record_proto_sgl(void)
+test_tls_1_2_record_proto_sgl(void)
{
struct tls_record_test_flags flags = {
- .nb_segs_in_mbuf = 5
+ .nb_segs_in_mbuf = 5,
+ .tls_version = RTE_SECURITY_VERSION_TLS_1_2
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+ if (!(dev_info.feature_flags & RTE_CRYPTODEV_FF_IN_PLACE_SGL)) {
+ printf("Device doesn't support in-place scatter-gather. Test
Skipped.\n");
+ return TEST_SKIPPED;
+ }
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_dtls_1_2_record_proto_display_list(void)
+{
+ struct tls_record_test_flags flags;
+
+ memset(&flags, 0, sizeof(flags));
+
+ flags.display_alg = true;
+ flags.tls_version = RTE_SECURITY_VERSION_DTLS_1_2;
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_dtls_1_2_record_proto_sgl(void)
+{
+ struct tls_record_test_flags flags = {
+ .nb_segs_in_mbuf = 5,
+ .tls_version = RTE_SECURITY_VERSION_DTLS_1_2
};
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct rte_cryptodev_info dev_info;
@@ -17081,11 +17149,15 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
TEST_CASE_NAMED_ST(
"Combi
[PATCH v5 03/21] test/security: add DTLS 1.2 data walkthrough test
Add data walkthrough test for DTLS 1.2
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev.c | 17 +
app/test/test_cryptodev_security_tls_record.c | 5 -
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index c5837ccbdd..e0695e9eb3 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12061,6 +12061,19 @@ test_tls_1_2_record_proto_sgl(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_dtls_1_2_record_proto_data_walkthrough(void)
+{
+ struct tls_record_test_flags flags;
+
+ memset(&flags, 0, sizeof(flags));
+
+ flags.data_walkthrough = true;
+ flags.tls_version = RTE_SECURITY_VERSION_DTLS_1_2;
+
+ return test_tls_record_proto_all(&flags);
+}
+
static int
test_dtls_1_2_record_proto_display_list(void)
{
@@ -17255,6 +17268,10 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
"Combined test alg list",
ut_setup_security, ut_teardown,
test_dtls_1_2_record_proto_display_list),
+ TEST_CASE_NAMED_ST(
+ "Data walkthrough combined test alg list",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_data_walkthrough),
TEST_CASE_NAMED_ST(
"Multi-segmented mode",
ut_setup_security, ut_teardown,
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index 3745c6a0d1..92bcbff842 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -143,7 +143,10 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
break;
case RTE_SECURITY_VERSION_DTLS_1_2:
hdr_len = sizeof(struct rte_dtls_hdr);
- min_padding = 0;
+ if (td->aead)
+ min_padding = 0;
+ else
+ min_padding = 1;
break;
default:
hdr_len = 0;
--
2.25.1
[PATCH v5 04/21] test/security: add TLS SG data walkthrough test
Add multi segment packet data walkthrough test for TLS 1.2
and DTLS 1.2.
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev.c | 42 +++
app/test/test_cryptodev_security_tls_record.h | 2 +-
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index e0695e9eb3..3591c91130 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11981,6 +11981,8 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
for (i = 0; i < RTE_DIM(sec_alg_list); i++) {
payload_len = TLS_RECORD_PLAINTEXT_MIN_LEN;
+ if (flags->nb_segs_in_mbuf)
+ payload_len = RTE_MAX(payload_len,
flags->nb_segs_in_mbuf);
again:
test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2, flags,
td_outb, nb_pkts, payload_len);
@@ -12061,6 +12063,32 @@ test_tls_1_2_record_proto_sgl(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_tls_record_proto_sgl_data_walkthrough(enum rte_security_tls_version
tls_version)
+{
+ struct tls_record_test_flags flags = {
+ .nb_segs_in_mbuf = 5,
+ .tls_version = tls_version,
+ .data_walkthrough = true
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+ if (!(dev_info.feature_flags & RTE_CRYPTODEV_FF_IN_PLACE_SGL)) {
+ printf("Device doesn't support in-place scatter-gather. Test
Skipped.\n");
+ return TEST_SKIPPED;
+ }
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_tls_1_2_record_proto_sgl_data_walkthrough(void)
+{
+ return
test_tls_record_proto_sgl_data_walkthrough(RTE_SECURITY_VERSION_TLS_1_2);
+}
+
static int
test_dtls_1_2_record_proto_data_walkthrough(void)
{
@@ -12106,6 +12134,12 @@ test_dtls_1_2_record_proto_sgl(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_dtls_1_2_record_proto_sgl_data_walkthrough(void)
+{
+ return
test_tls_record_proto_sgl_data_walkthrough(RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
#endif
static int
@@ -17171,6 +17205,10 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"Multi-segmented mode",
ut_setup_security, ut_teardown,
test_tls_1_2_record_proto_sgl),
+ TEST_CASE_NAMED_ST(
+ "Multi-segmented mode data walkthrough",
+ ut_setup_security, ut_teardown,
+ test_tls_1_2_record_proto_sgl_data_walkthrough),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
@@ -17276,6 +17314,10 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
"Multi-segmented mode",
ut_setup_security, ut_teardown,
test_dtls_1_2_record_proto_sgl),
+ TEST_CASE_NAMED_ST(
+ "Multi-segmented mode data walkthrough",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_sgl_data_walkthrough),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
diff --git a/app/test/test_cryptodev_security_tls_record.h
b/app/test/test_cryptodev_security_tls_record.h
index 56d9d11962..a6c537b35a 100644
--- a/app/test/test_cryptodev_security_tls_record.h
+++ b/app/test/test_cryptodev_security_tls_record.h
@@ -85,7 +85,7 @@ struct tls_record_test_data {
struct tls_record_test_flags {
bool display_alg;
- int nb_segs_in_mbuf;
+ uint8_t nb_segs_in_mbuf;
bool data_walkthrough;
enum rte_security_tls_version tls_version;
};
--
2.25.1
[PATCH v5 05/21] test/security: unit test for TLS packet corruption
From: Vidya Sagar Velumuri
Add test to verify the corrupted TLS packet header
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 27 +--
app/test/test_cryptodev_security_tls_record.c | 4 +++
app/test/test_cryptodev_security_tls_record.h | 1 +
3 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 3591c91130..324ef3c276 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12000,8 +12000,13 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
if (ret == TEST_SKIPPED)
continue;
- if (ret == TEST_FAILED)
- return TEST_FAILED;
+ if (flags->pkt_corruption) {
+ if (ret == TEST_SUCCESS)
+ return TEST_FAILED;
+ } else {
+ if (ret == TEST_FAILED)
+ return TEST_FAILED;
+ }
if (flags->data_walkthrough && (++payload_len <=
max_payload_len))
goto again;
@@ -12089,6 +12094,20 @@ test_tls_1_2_record_proto_sgl_data_walkthrough(void)
return
test_tls_record_proto_sgl_data_walkthrough(RTE_SECURITY_VERSION_TLS_1_2);
}
+static int
+test_tls_record_proto_corrupt_pkt(void)
+{
+ struct tls_record_test_flags flags = {
+ .pkt_corruption = 1
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
static int
test_dtls_1_2_record_proto_data_walkthrough(void)
{
@@ -17209,6 +17228,10 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"Multi-segmented mode data walkthrough",
ut_setup_security, ut_teardown,
test_tls_1_2_record_proto_sgl_data_walkthrough),
+ TEST_CASE_NAMED_ST(
+ "TLS packet header corruption",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_corrupt_pkt),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index 92bcbff842..93ff7f36fa 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -185,6 +185,10 @@ test_tls_record_td_update(struct tls_record_test_data
td_inb[],
td_outb[i].input_text.len);
td_inb[i].output_text.len = td_outb->input_text.len;
+ /* Corrupt the content type in the TLS header of encrypted
packet */
+ if (flags->pkt_corruption)
+ td_inb[i].input_text.data[0] =
~td_inb[i].input_text.data[0];
+
/* Clear outbound specific flags */
td_inb[i].tls_record_xform.options.iv_gen_disable = 0;
}
diff --git a/app/test/test_cryptodev_security_tls_record.h
b/app/test/test_cryptodev_security_tls_record.h
index a6c537b35a..a7c0ca87bf 100644
--- a/app/test/test_cryptodev_security_tls_record.h
+++ b/app/test/test_cryptodev_security_tls_record.h
@@ -88,6 +88,7 @@ struct tls_record_test_flags {
uint8_t nb_segs_in_mbuf;
bool data_walkthrough;
enum rte_security_tls_version tls_version;
+ bool pkt_corruption;
};
extern struct tls_record_test_data tls_test_data_aes_128_gcm_v1;
--
2.25.1
[PATCH v5 06/21] test/security: unit test for custom content verification
From: Vidya Sagar Velumuri
Add unit test to verify the TLS header creation with
custom content type
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 19 +++
app/test/test_cryptodev_security_tls_record.c | 3 +++
app/test/test_cryptodev_security_tls_record.h | 9 +
3 files changed, 31 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 324ef3c276..5cb878b9ba 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#ifdef RTE_CRYPTO_SCHEDULER
@@ -12108,6 +12109,20 @@ test_tls_record_proto_corrupt_pkt(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_tls_record_proto_custom_content_type(void)
+{
+ struct tls_record_test_flags flags = {
+ .content_type = TLS_RECORD_TEST_CONTENT_TYPE_CUSTOM
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
static int
test_dtls_1_2_record_proto_data_walkthrough(void)
{
@@ -17232,6 +17247,10 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"TLS packet header corruption",
ut_setup_security, ut_teardown,
test_tls_record_proto_corrupt_pkt),
+ TEST_CASE_NAMED_ST(
+ "Custom content type",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_custom_content_type),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index 93ff7f36fa..9a2af259c9 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -108,6 +108,9 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
td->input_text.len = data_len;
}
+ if (flags->content_type == TLS_RECORD_TEST_CONTENT_TYPE_CUSTOM)
+ td->app_type = RTE_TLS_TYPE_MAX;
+
tls_pkt_size = td->input_text.len;
if (!td->aead) {
diff --git a/app/test/test_cryptodev_security_tls_record.h
b/app/test/test_cryptodev_security_tls_record.h
index a7c0ca87bf..73719063a8 100644
--- a/app/test/test_cryptodev_security_tls_record.h
+++ b/app/test/test_cryptodev_security_tls_record.h
@@ -42,6 +42,14 @@ static_assert(TLS_1_3_RECORD_PLAINTEXT_MAX_LEN <=
TEST_SEC_CLEARTEXT_MAX_LEN,
#define TLS_RECORD_PLAINTEXT_MIN_LEN (1u)
+enum tls_record_test_content_type {
+ TLS_RECORD_TEST_CONTENT_TYPE_APP,
+ /* For verifying zero packet length */
+ TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE,
+ /* For verifying handling of custom content types */
+ TLS_RECORD_TEST_CONTENT_TYPE_CUSTOM,
+};
+
struct tls_record_test_data {
struct {
uint8_t data[32];
@@ -89,6 +97,7 @@ struct tls_record_test_flags {
bool data_walkthrough;
enum rte_security_tls_version tls_version;
bool pkt_corruption;
+ enum tls_record_test_content_type content_type;
};
extern struct tls_record_test_data tls_test_data_aes_128_gcm_v1;
--
2.25.1
[PATCH v5 07/21] test/cryptodev: allow zero packet length buffers
From: Anoob Joseph
The function 'create_segmented_mbuf' is updated to support zero packet
length mbufs. This allows testing of zero packet length payload with TLS
record processing.
Signed-off-by: Anoob Joseph
---
app/test/test_cryptodev.h | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/app/test/test_cryptodev.h b/app/test/test_cryptodev.h
index f27d9697fd..fd9ea0dd81 100644
--- a/app/test/test_cryptodev.h
+++ b/app/test/test_cryptodev.h
@@ -182,15 +182,8 @@ create_segmented_mbuf(struct rte_mempool *mbuf_pool, int
pkt_len,
int nb_segs, uint8_t pattern) {
struct rte_mbuf *m = NULL, *mbuf = NULL;
+ int size, t_len, data_len = 0;
uint8_t *dst;
- int data_len = 0;
- int i, size;
- int t_len;
-
- if (pkt_len < 1) {
- printf("Packet size must be 1 or more (is %d)\n", pkt_len);
- return NULL;
- }
if (nb_segs < 1) {
printf("Number of segments must be 1 or more (is %d)\n",
@@ -202,17 +195,17 @@ create_segmented_mbuf(struct rte_mempool *mbuf_pool, int
pkt_len,
size = pkt_len;
/* Create chained mbuf_src and fill it generated data */
- for (i = 0; size > 0; i++) {
+ do {
m = rte_pktmbuf_alloc(mbuf_pool);
- if (i == 0)
- mbuf = m;
-
if (m == NULL) {
printf("Cannot create segment for source mbuf");
goto fail;
}
+ if (mbuf == NULL)
+ mbuf = m;
+
/* Make sure if tailroom is zeroed */
memset(m->buf_addr, pattern, m->buf_len);
@@ -229,7 +222,8 @@ create_segmented_mbuf(struct rte_mempool *mbuf_pool, int
pkt_len,
size -= data_len;
- }
+ } while (size > 0);
+
return mbuf;
fail:
--
2.25.1
[PATCH v5 08/21] test/security: unit test to verify zero TLS records
From: Vidya Sagar Velumuri
Add unit tests to verify the zero len TLS records. Zero len packets are
allowed when content type is app data while zero packet length with
other content type (such as handshake) would result in an error.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 51 ++-
app/test/test_cryptodev_security_tls_record.c | 5 +-
app/test/test_cryptodev_security_tls_record.h | 2 +-
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 5cb878b9ba..fa63b9743f 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11984,6 +11984,9 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
payload_len = TLS_RECORD_PLAINTEXT_MIN_LEN;
if (flags->nb_segs_in_mbuf)
payload_len = RTE_MAX(payload_len,
flags->nb_segs_in_mbuf);
+
+ if (flags->zero_len)
+ payload_len = 0;
again:
test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2, flags,
td_outb, nb_pkts, payload_len);
@@ -11992,8 +11995,16 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
if (ret == TEST_SKIPPED)
continue;
- if (ret == TEST_FAILED)
+ if (flags->zero_len &&
+ ((flags->content_type ==
TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE) ||
+ (flags->content_type ==
TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE) ||
+ (flags->content_type ==
TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE))) {
+ if (ret == TEST_SUCCESS)
+ return TEST_FAILED;
+ goto skip_decrypt;
+ } else if (ret == TEST_FAILED) {
return TEST_FAILED;
+ }
test_tls_record_td_update(td_inb, td_outb, nb_pkts, flags);
@@ -12009,6 +12020,7 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
return TEST_FAILED;
}
+skip_decrypt:
if (flags->data_walkthrough && (++payload_len <=
max_payload_len))
goto again;
@@ -12123,6 +12135,35 @@ test_tls_record_proto_custom_content_type(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_tls_record_proto_zero_len(void)
+{
+ struct tls_record_test_flags flags = {
+ .zero_len = 1
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_tls_record_proto_zero_len_non_app(void)
+{
+ struct tls_record_test_flags flags = {
+ .zero_len = 1,
+ .content_type = TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE,
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
static int
test_dtls_1_2_record_proto_data_walkthrough(void)
{
@@ -17251,6 +17292,14 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"Custom content type",
ut_setup_security, ut_teardown,
test_tls_record_proto_custom_content_type),
+ TEST_CASE_NAMED_ST(
+ "Zero len TLS record with content type as app",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_zero_len),
+ TEST_CASE_NAMED_ST(
+ "Zero len TLS record with content type as ctrl",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_zero_len_non_app),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index 9a2af259c9..c5410a4c92 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -103,13 +103,15 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
}
}
- if (flags->data_walkthrough) {
+ if (flags->data_walkthrough || flags->zero_len) {
test_sec_proto_pattern_set(td->input_text.data, data_len);
td->input_text.len = data_len;
}
if (flags->content_type == TLS_RECORD_TEST_CONTENT_TYPE_CUSTOM)
td->app_type = RTE_TLS_TYPE_MAX;
+ else if (flags->content_type == TLS_REC
[PATCH v5 09/21] test/security: add unit tests for DTLS-1.2
From: Vidya Sagar Velumuri
Add unit tests to verify
1. DTLS record with zero length
2. DTLS record with header corruption
3. DTLS record with content type as custom
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 77 +++
1 file changed, 77 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index fa63b9743f..72e7fe3769 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12215,6 +12215,67 @@ test_dtls_1_2_record_proto_sgl_data_walkthrough(void)
return
test_tls_record_proto_sgl_data_walkthrough(RTE_SECURITY_VERSION_DTLS_1_2);
}
+static int
+test_dtls_1_2_record_proto_corrupt_pkt(void)
+{
+ struct tls_record_test_flags flags = {
+ .pkt_corruption = 1,
+ .tls_version = RTE_SECURITY_VERSION_DTLS_1_2
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_dtls_1_2_record_proto_custom_content_type(void)
+{
+ struct tls_record_test_flags flags = {
+ .content_type = TLS_RECORD_TEST_CONTENT_TYPE_CUSTOM,
+ .tls_version = RTE_SECURITY_VERSION_DTLS_1_2
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_dtls_1_2_record_proto_zero_len(void)
+{
+ struct tls_record_test_flags flags = {
+ .zero_len = 1,
+ .tls_version = RTE_SECURITY_VERSION_DTLS_1_2
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_dtls_1_2_record_proto_zero_len_non_app(void)
+{
+ struct tls_record_test_flags flags = {
+ .zero_len = 1,
+ .content_type = TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE,
+ .tls_version = RTE_SECURITY_VERSION_DTLS_1_2
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
#endif
static int
@@ -17409,6 +17470,22 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
"Multi-segmented mode data walkthrough",
ut_setup_security, ut_teardown,
test_dtls_1_2_record_proto_sgl_data_walkthrough),
+ TEST_CASE_NAMED_ST(
+ "Packet corruption",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_corrupt_pkt),
+ TEST_CASE_NAMED_ST(
+ "Custom content type",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_custom_content_type),
+ TEST_CASE_NAMED_ST(
+ "Zero len DTLS record with content type as app",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_zero_len),
+ TEST_CASE_NAMED_ST(
+ "Zero len DTLS record with content type as ctrl",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_zero_len_non_app),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
--
2.25.1
[PATCH v5 10/21] test/security: add TLS/DTLS 1.2 AES-256-SHA384 vectors
From: Akhil Goyal
Added vectors for TLS 1.2 and DTLS 1.2 using algos
AES-256-CBC and HMAC-SHA384
Signed-off-by: Akhil Goyal
---
app/test/test_cryptodev.c | 19 ++
app/test/test_cryptodev_security_tls_record.h | 2 +
...yptodev_security_tls_record_test_vectors.h | 200 ++
3 files changed, 221 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 72e7fe3769..95f2377d4d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -17273,6 +17273,10 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"Write record known vector AES-256-CBC-SHA256",
ut_setup_security, ut_teardown,
test_tls_record_proto_known_vec,
&tls_test_data_aes_256_cbc_sha256_hmac),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Write record known vector AES-256-CBC-SHA384",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec,
&tls_test_data_aes_256_cbc_sha384_hmac),
TEST_CASE_NAMED_WITH_DATA(
"Write record known vector 3DES-CBC-SHA1-HMAC",
ut_setup_security, ut_teardown,
@@ -17316,6 +17320,11 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
ut_setup_security, ut_teardown,
test_tls_record_proto_known_vec_read,
&tls_test_data_aes_256_cbc_sha256_hmac),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Read record known vector AES-256-CBC-SHA384",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec_read,
+ &tls_test_data_aes_256_cbc_sha384_hmac),
TEST_CASE_NAMED_WITH_DATA(
"Read record known vector 3DES-CBC-SHA1-HMAC",
ut_setup_security, ut_teardown,
@@ -17397,6 +17406,11 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
ut_setup_security, ut_teardown,
test_tls_record_proto_known_vec,
&dtls_test_data_aes_256_cbc_sha256_hmac),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Write record known vector AES-256-CBC-SHA384",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec,
+ &dtls_test_data_aes_256_cbc_sha384_hmac),
TEST_CASE_NAMED_WITH_DATA(
"Write record known vector 3DES-CBC-SHA1-HMAC",
ut_setup_security, ut_teardown,
@@ -17439,6 +17453,11 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
ut_setup_security, ut_teardown,
test_tls_record_proto_known_vec_read,
&dtls_test_data_aes_256_cbc_sha256_hmac),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Read record known vector AES-256-CBC-SHA384",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec_read,
+ &dtls_test_data_aes_256_cbc_sha384_hmac),
TEST_CASE_NAMED_WITH_DATA(
"Read record known vector 3DES-CBC-SHA1-HMAC",
ut_setup_security, ut_teardown,
diff --git a/app/test/test_cryptodev_security_tls_record.h
b/app/test/test_cryptodev_security_tls_record.h
index 300f3f08b5..68e243b842 100644
--- a/app/test/test_cryptodev_security_tls_record.h
+++ b/app/test/test_cryptodev_security_tls_record.h
@@ -110,6 +110,7 @@ extern struct tls_record_test_data
tls_test_data_aes_128_cbc_sha1_hmac;
extern struct tls_record_test_data tls_test_data_aes_128_cbc_sha256_hmac;
extern struct tls_record_test_data tls_test_data_aes_256_cbc_sha1_hmac;
extern struct tls_record_test_data tls_test_data_aes_256_cbc_sha256_hmac;
+extern struct tls_record_test_data tls_test_data_aes_256_cbc_sha384_hmac;
extern struct tls_record_test_data tls_test_data_3des_cbc_sha1_hmac;
extern struct tls_record_test_data tls_test_data_null_cipher_sha1_hmac;
extern struct tls_record_test_data tls_test_data_chacha20_poly1305;
@@ -118,6 +119,7 @@ extern struct tls_record_test_data
dtls_test_data_aes_128_cbc_sha1_hmac;
extern struct tls_record_test_data dtls_test_data_aes_128_cbc_sha256_hmac;
extern struct tls_record_test_data dtls_test_data_aes_256_cbc_sha1_hmac;
extern struct tls_record_test_data dtls_test_data_aes_256_cbc_sha256_hmac;
+extern struct tls_record_test_data dtls_test_data_aes_256_cbc_sha384_hmac;
extern struct tls_record_test_data dtls_test_data_3des_cbc_sha1_hmac;
extern struct tls_record_test_data dtls_test_data_null_cipher_sha1_hmac;
diff --git a/app/test/test_cryptodev_security_tls_record_test_vectors.h
b/app/test/test_cryptodev_security_tl
[PATCH v5 11/21] test/security: add DTLS 1.2 anti-replay tests
Add anti-replay test for DTLS 1.2.
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev.c | 115 ++-
app/test/test_cryptodev_security_tls_record.c | 132 ++
app/test/test_cryptodev_security_tls_record.h | 11 +-
3 files changed, 188 insertions(+), 70 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 95f2377d4d..904bad39d3 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11827,6 +11827,10 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
.protocol = RTE_SECURITY_PROTOCOL_TLS_RECORD,
};
+ if ((tls_record_xform.ver == RTE_SECURITY_VERSION_DTLS_1_2) &&
+ (sess_type == RTE_SECURITY_TLS_SESS_TYPE_READ))
+ sess_conf.tls_record.dtls_1_2.ar_win_sz = flags->ar_win_size;
+
if (td[0].aead)
test_tls_record_imp_nonce_update(&td[0], &tls_record_xform);
@@ -11851,6 +11855,17 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
return TEST_SKIPPED;
for (i = 0; i < nb_td; i++) {
+ if (flags->ar_win_size &&
+ (sess_type == RTE_SECURITY_TLS_SESS_TYPE_WRITE)) {
+ sess_conf.tls_record.dtls_1_2.seq_no =
+ td[i].tls_record_xform.dtls_1_2.seq_no;
+ ret = rte_security_session_update(ctx,
ut_params->sec_session, &sess_conf);
+ if (ret) {
+ printf("Could not update sequence number in
session\n");
+ return TEST_SKIPPED;
+ }
+ }
+
/* Setup source mbuf payload */
ut_params->ibuf = create_segmented_mbuf(ts_params->mbuf_pool,
td[i].input_text.len,
nb_segs, 0);
@@ -11890,17 +11905,19 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
/* Process crypto operation */
process_crypto_request(dev_id, ut_params->op);
- ret = test_tls_record_status_check(ut_params->op);
+ ret = test_tls_record_status_check(ut_params->op, &td[i]);
if (ret != TEST_SUCCESS)
goto crypto_op_free;
if (res_d != NULL)
res_d_tmp = &res_d[i];
- ret = test_tls_record_post_process(ut_params->ibuf, &td[i],
res_d_tmp, silent);
- if (ret != TEST_SUCCESS)
- goto crypto_op_free;
-
+ if (ut_params->op->status == RTE_CRYPTO_OP_STATUS_SUCCESS) {
+ ret = test_tls_record_post_process(ut_params->ibuf,
&td[i], res_d_tmp,
+ silent);
+ if (ret != TEST_SUCCESS)
+ goto crypto_op_free;
+ }
rte_crypto_op_free(ut_params->op);
ut_params->op = NULL;
@@ -12190,6 +12207,90 @@ test_dtls_1_2_record_proto_display_list(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_dtls_pkt_replay(const uint64_t seq_no[],
+ bool replayed_pkt[], uint32_t nb_pkts,
+ struct tls_record_test_flags *flags)
+{
+ struct tls_record_test_data td_outb[TEST_SEC_PKTS_MAX];
+ struct tls_record_test_data td_inb[TEST_SEC_PKTS_MAX];
+ unsigned int i, idx, pass_cnt = 0;
+ int ret;
+
+ for (i = 0; i < RTE_DIM(sec_alg_list); i++) {
+ test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2, flags,
+ td_outb, nb_pkts, 0);
+
+ for (idx = 0; idx < nb_pkts; idx++)
+ td_outb[idx].tls_record_xform.dtls_1_2.seq_no =
seq_no[idx];
+
+ ret = test_tls_record_proto_process(td_outb, td_inb, nb_pkts,
true, flags);
+ if (ret == TEST_SKIPPED)
+ continue;
+
+ if (ret == TEST_FAILED)
+ return TEST_FAILED;
+
+ test_tls_record_td_update(td_inb, td_outb, nb_pkts, flags);
+
+ for (idx = 0; idx < nb_pkts; idx++) {
+ td_inb[idx].tls_record_xform.dtls_1_2.ar_win_sz =
flags->ar_win_size;
+ /* Set antireplay flag for packets to be dropped */
+ td_inb[idx].ar_packet = replayed_pkt[idx];
+ }
+
+ ret = test_tls_record_proto_process(td_inb, NULL, nb_pkts,
true, flags);
+ if (ret == TEST_SKIPPED)
+ continue;
+
+ if (ret == TEST_FAILED)
+ return TEST_FAILED;
+
+ if (flags->display_alg)
+ test_sec_alg_display(sec_alg_list[i].param1,
sec_alg_list[i].param2);
+
+ pass_cnt+
[PATCH v5 12/21] test/security: add more DTLS anti-replay window sz
Add anti-replay tests for window sizes 128, 256, 512, 1024,
2048 and 4096 window sizes in DTLS 1.2 suite.
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev.c | 72 +--
1 file changed, 69 insertions(+), 3 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 904bad39d3..72d91d23a2 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12259,12 +12259,12 @@ test_dtls_pkt_replay(const uint64_t seq_no[],
}
static int
-test_dtls_1_2_record_proto_antireplay(void)
+test_dtls_1_2_record_proto_antireplay(uint64_t winsz)
{
struct tls_record_test_flags flags;
- uint64_t winsz = 64, seq_no[5];
uint32_t nb_pkts = 5;
bool replayed_pkt[5];
+ uint64_t seq_no[5];
memset(&flags, 0, sizeof(flags));
@@ -12291,6 +12291,48 @@ test_dtls_1_2_record_proto_antireplay(void)
return test_dtls_pkt_replay(seq_no, replayed_pkt, nb_pkts, &flags);
}
+static int
+test_dtls_1_2_record_proto_antireplay64(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(64);
+}
+
+static int
+test_dtls_1_2_record_proto_antireplay128(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(128);
+}
+
+static int
+test_dtls_1_2_record_proto_antireplay256(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(256);
+}
+
+static int
+test_dtls_1_2_record_proto_antireplay512(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(512);
+}
+
+static int
+test_dtls_1_2_record_proto_antireplay1024(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(1024);
+}
+
+static int
+test_dtls_1_2_record_proto_antireplay2048(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(2048);
+}
+
+static int
+test_dtls_1_2_record_proto_antireplay4096(void)
+{
+ return test_dtls_1_2_record_proto_antireplay(4096);
+}
+
static int
test_dtls_1_2_record_proto_sgl(void)
{
@@ -17609,7 +17651,31 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
TEST_CASE_NAMED_ST(
"Antireplay with window size 64",
ut_setup_security, ut_teardown,
- test_dtls_1_2_record_proto_antireplay),
+ test_dtls_1_2_record_proto_antireplay64),
+ TEST_CASE_NAMED_ST(
+ "Antireplay with window size 128",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_antireplay128),
+ TEST_CASE_NAMED_ST(
+ "Antireplay with window size 256",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_antireplay256),
+ TEST_CASE_NAMED_ST(
+ "Antireplay with window size 512",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_antireplay512),
+ TEST_CASE_NAMED_ST(
+ "Antireplay with window size 1024",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_antireplay1024),
+ TEST_CASE_NAMED_ST(
+ "Antireplay with window size 2048",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_antireplay2048),
+ TEST_CASE_NAMED_ST(
+ "Antireplay with window size 4096",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_antireplay4096),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
--
2.25.1
[PATCH v5 13/21] test/crypto: update verification of header
From: Vidya Sagar Velumuri
In TLS 1.3, the version in the header would be TLS 1.2 and the content
type would be APP irrespective of the type of the payload.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev_security_tls_record.c | 20 +--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index 907e043ddd..498c4923e0 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -275,9 +275,9 @@ tls_record_hdr_verify(const struct tls_record_test_data
*td, const uint8_t *outp
hdr_len = sizeof(struct rte_tls_hdr);
} else if (td->tls_record_xform.ver == RTE_SECURITY_VERSION_TLS_1_3) {
const struct rte_tls_hdr *hdr = (const struct rte_tls_hdr
*)output_text;
- if (rte_be_to_cpu_16(hdr->version) != RTE_TLS_VERSION_1_3) {
+ if (rte_be_to_cpu_16(hdr->version) != RTE_TLS_VERSION_1_2) {
printf("Incorrect header version [expected - %4x,
received - %4x]\n",
- RTE_TLS_VERSION_1_3,
rte_be_to_cpu_16(hdr->version));
+ RTE_TLS_VERSION_1_2,
rte_be_to_cpu_16(hdr->version));
return TEST_FAILED;
}
content_type = hdr->type;
@@ -297,10 +297,18 @@ tls_record_hdr_verify(const struct tls_record_test_data
*td, const uint8_t *outp
return TEST_FAILED;
}
- if (content_type != td->app_type) {
- printf("Incorrect content type in packet [expected - %d,
received - %d]\n",
- td->app_type, content_type);
- return TEST_FAILED;
+ if (td->tls_record_xform.ver == RTE_SECURITY_VERSION_TLS_1_3) {
+ if (content_type != RTE_TLS_TYPE_APPDATA) {
+ printf("Incorrect content type in packet [expected -
%d, received - %d]\n",
+ td->app_type, content_type);
+ return TEST_FAILED;
+ }
+ } else {
+ if (content_type != td->app_type) {
+ printf("Incorrect content type in packet [expected -
%d, received - %d]\n",
+ td->app_type, content_type);
+ return TEST_FAILED;
+ }
}
if (length != td->output_text.len - hdr_len) {
--
2.25.1
[PATCH v5 14/21] test/crypto: add TLS 1.3 vectors
From: Akhil Goyal
Added vectors and test suite for TLS 1.3
AES-128-GCM, AES-256-GCM and CHACHA20-POLY1305
vectors. The vectors are generated using gnuTLS
client server application.
Signed-off-by: Akhil Goyal
---
app/test/test_cryptodev.c | 35 +++
app/test/test_cryptodev_security_tls_record.h | 3 +
...yptodev_security_tls_record_test_vectors.h | 205 ++
3 files changed, 243 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 72d91d23a2..aa9fffe50e 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -17680,6 +17680,40 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
}
};
+static struct unit_test_suite tls13_record_proto_testsuite = {
+ .suite_name = "TLS 1.3 Record Protocol Unit Test Suite",
+ .setup = tls_record_proto_testsuite_setup,
+ .unit_test_cases = {
+ TEST_CASE_NAMED_WITH_DATA(
+ "Write record known vector AES-GCM-128",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec,
&tls13_test_data_aes_128_gcm),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Write record known vector AES-GCM-256",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec,
&tls13_test_data_aes_256_gcm),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Write record known vector CHACHA20-POLY1305",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec,
&tls13_test_data_chacha20_poly1305),
+
+ TEST_CASE_NAMED_WITH_DATA(
+ "Read record known vector AES-GCM-128",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec_read,
&tls13_test_data_aes_128_gcm),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Read record known vector AES-GCM-256",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec_read,
&tls13_test_data_aes_256_gcm),
+ TEST_CASE_NAMED_WITH_DATA(
+ "Read record known vector CHACHA20-POLY1305",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_known_vec_read,
&tls13_test_data_chacha20_poly1305),
+
+ TEST_CASES_END() /**< NULL terminate unit test array */
+ }
+};
+
#define ADD_UPLINK_TESTCASE(data)
\
TEST_CASE_NAMED_WITH_DATA(data.test_descr_uplink, ut_setup_security,
\
ut_teardown, test_docsis_proto_uplink, (const void *) &data),
\
@@ -18699,6 +18733,7 @@ run_cryptodev_testsuite(const char *pmd_name)
&docsis_proto_testsuite,
&tls12_record_proto_testsuite,
&dtls12_record_proto_testsuite,
+ &tls13_record_proto_testsuite,
#endif
&end_testsuite
};
diff --git a/app/test/test_cryptodev_security_tls_record.h
b/app/test/test_cryptodev_security_tls_record.h
index efb16aed7d..9fbc64605d 100644
--- a/app/test/test_cryptodev_security_tls_record.h
+++ b/app/test/test_cryptodev_security_tls_record.h
@@ -124,6 +124,9 @@ extern struct tls_record_test_data
dtls_test_data_aes_256_cbc_sha256_hmac;
extern struct tls_record_test_data dtls_test_data_aes_256_cbc_sha384_hmac;
extern struct tls_record_test_data dtls_test_data_3des_cbc_sha1_hmac;
extern struct tls_record_test_data dtls_test_data_null_cipher_sha1_hmac;
+extern struct tls_record_test_data tls13_test_data_aes_128_gcm;
+extern struct tls_record_test_data tls13_test_data_aes_256_gcm;
+extern struct tls_record_test_data tls13_test_data_chacha20_poly1305;
int test_tls_record_status_check(struct rte_crypto_op *op,
const struct tls_record_test_data *td);
diff --git a/app/test/test_cryptodev_security_tls_record_test_vectors.h
b/app/test/test_cryptodev_security_tls_record_test_vectors.h
index 27b07cd54a..8af17b07e5 100644
--- a/app/test/test_cryptodev_security_tls_record_test_vectors.h
+++ b/app/test/test_cryptodev_security_tls_record_test_vectors.h
@@ -1781,4 +1781,209 @@ struct tls_record_test_data
tls_test_data_3des_cbc_sha1_hmac = {
.app_type = 0x17,
};
+/* TLS 1.3 AES-128-GCM */
+struct tls_record_test_data tls13_test_data_aes_128_gcm = {
+ .key = {
+ .data = {
+ 0x03, 0x12, 0xf5, 0x86, 0xe4, 0xd0, 0x27, 0xc7,
+ 0x47, 0x82, 0x44, 0xca, 0xd3, 0xce, 0x06, 0x6c,
+ },
+ },
+ .input_text = {
+ .data = {
+ 0x54, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20,
+ 0x54, 0x4c, 0x53, 0x20, 0x31, 0x2e, 0x33, 0x20,
+ 0x41, 0x45, 0x53, 0x2d, 0x31, 0x32
[PATCH v5 16/21] test/crypto: test to verify hdr corruption in TLS
From: Vidya Sagar Velumuri
Add unit tests to verify TLS-1.3 record with header corruption.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 25777c1b1f..9f0a737913 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12424,6 +12424,20 @@ test_dtls_1_2_record_proto_zero_len_non_app(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_tls_1_3_record_proto_corrupt_pkt(void)
+{
+ struct tls_record_test_flags flags = {
+ .pkt_corruption = 1,
+ .tls_version = RTE_SECURITY_VERSION_TLS_1_3
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
#endif
static int
@@ -17714,7 +17728,10 @@ static struct unit_test_suite
tls13_record_proto_testsuite = {
"Read record known vector CHACHA20-POLY1305",
ut_setup_security, ut_teardown,
test_tls_record_proto_known_vec_read,
&tls13_test_data_chacha20_poly1305),
-
+ TEST_CASE_NAMED_ST(
+ "TLS-1.3 record header corruption",
+ ut_setup_security, ut_teardown,
+ test_tls_1_3_record_proto_corrupt_pkt),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
--
2.25.1
[PATCH v5 17/21] test/crypto: test to verify custom content type in TLS
From: Vidya Sagar Velumuri
Add unit tests to verify TLS-1.3 record with content type as custom.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f0a737913..fe4fcfbfdb 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12438,6 +12438,21 @@ test_tls_1_3_record_proto_corrupt_pkt(void)
return test_tls_record_proto_all(&flags);
}
+
+static int
+test_tls_1_3_record_proto_custom_content_type(void)
+{
+ struct tls_record_test_flags flags = {
+ .content_type = TLS_RECORD_TEST_CONTENT_TYPE_CUSTOM,
+ .tls_version = RTE_SECURITY_VERSION_TLS_1_3
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
#endif
static int
@@ -17732,6 +17747,10 @@ static struct unit_test_suite
tls13_record_proto_testsuite = {
"TLS-1.3 record header corruption",
ut_setup_security, ut_teardown,
test_tls_1_3_record_proto_corrupt_pkt),
+ TEST_CASE_NAMED_ST(
+ "TLS-1.3 record header with custom content type",
+ ut_setup_security, ut_teardown,
+ test_tls_1_3_record_proto_custom_content_type),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
--
2.25.1
[PATCH v5 18/21] test/crypto: test to verify zero len record in TLS
From: Vidya Sagar Velumuri
Add unit tests to verify TLS-1.3 record with zero length.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index fe4fcfbfdb..8ad5033f32 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12453,6 +12453,37 @@ test_tls_1_3_record_proto_custom_content_type(void)
return test_tls_record_proto_all(&flags);
}
+
+static int
+test_tls_1_3_record_proto_zero_len(void)
+{
+ struct tls_record_test_flags flags = {
+ .zero_len = 1,
+ .tls_version = RTE_SECURITY_VERSION_TLS_1_3
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_tls_1_3_record_proto_zero_len_non_app(void)
+{
+ struct tls_record_test_flags flags = {
+ .zero_len = 1,
+ .content_type = TLS_RECORD_TEST_CONTENT_TYPE_HANDSHAKE,
+ .tls_version = RTE_SECURITY_VERSION_TLS_1_3
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
#endif
static int
@@ -17751,6 +17782,14 @@ static struct unit_test_suite
tls13_record_proto_testsuite = {
"TLS-1.3 record header with custom content type",
ut_setup_security, ut_teardown,
test_tls_1_3_record_proto_custom_content_type),
+ TEST_CASE_NAMED_ST(
+ "TLS-1.3 record with zero len and content type as app",
+ ut_setup_security, ut_teardown,
+ test_tls_1_3_record_proto_zero_len),
+ TEST_CASE_NAMED_ST(
+ "TLS-1.3 record with zero len and content type as ctrl",
+ ut_setup_security, ut_teardown,
+ test_tls_1_3_record_proto_zero_len_non_app),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
--
2.25.1
[PATCH v5 19/21] test/crypto: unit tests to verify padding in TLS
From: Vidya Sagar Velumuri
Add unit tests to verify the padding for TLS-1.2.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 85 ++-
app/test/test_cryptodev_security_tls_record.c | 28 --
app/test/test_cryptodev_security_tls_record.h | 5 +-
3 files changed, 109 insertions(+), 9 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 8ad5033f32..a324c1607b 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11834,6 +11834,9 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
if (td[0].aead)
test_tls_record_imp_nonce_update(&td[0], &tls_record_xform);
+ if (flags->opt_padding)
+ tls_record_xform.options.extra_padding_enable = 1;
+
sess_conf.tls_record = tls_record_xform;
if (td[0].aead) {
@@ -11888,6 +11891,9 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
ut_params->op->sym->m_dst = NULL;
ut_params->op->param1.tls_record.content_type = td[i].app_type;
+ if (flags->opt_padding)
+ ut_params->op->aux_flags = flags->opt_padding;
+
/* Copy IV in crypto operation when IV generation is disabled */
if ((sess_type == RTE_SECURITY_TLS_SESS_TYPE_WRITE) &&
(tls_record_xform.ver != RTE_SECURITY_VERSION_TLS_1_3) &&
@@ -11915,7 +11921,7 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
if (ut_params->op->status == RTE_CRYPTO_OP_STATUS_SUCCESS) {
ret = test_tls_record_post_process(ut_params->ibuf,
&td[i], res_d_tmp,
- silent);
+ silent, flags);
if (ret != TEST_SUCCESS)
goto crypto_op_free;
}
@@ -12184,6 +12190,59 @@ test_tls_record_proto_zero_len_non_app(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_tls_record_proto_opt_padding(uint8_t padding, uint8_t num_segs,
+ enum rte_security_tls_version tls_version)
+{
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+ struct tls_record_test_flags flags = {
+ .nb_segs_in_mbuf = num_segs,
+ .tls_version = tls_version,
+ .opt_padding = padding
+ };
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_tls_record_proto_dm_opt_padding(void)
+{
+ return test_tls_record_proto_opt_padding(1, 0,
RTE_SECURITY_VERSION_TLS_1_2);
+}
+
+static int
+test_tls_record_proto_dm_opt_padding_1(void)
+{
+ return test_tls_record_proto_opt_padding(25, 0,
RTE_SECURITY_VERSION_TLS_1_2);
+}
+
+static int
+test_tls_record_proto_sg_opt_padding(void)
+{
+ return test_tls_record_proto_opt_padding(1, 2,
RTE_SECURITY_VERSION_TLS_1_2);
+}
+
+static int
+test_tls_record_proto_sg_opt_padding_1(void)
+{
+ return test_tls_record_proto_opt_padding(8, 4,
RTE_SECURITY_VERSION_TLS_1_2);
+}
+
+static int
+test_tls_record_proto_sg_opt_padding_2(void)
+{
+ return test_tls_record_proto_opt_padding(8, 5,
RTE_SECURITY_VERSION_TLS_1_2);
+}
+
+static int
+test_tls_record_proto_sg_opt_padding_max(void)
+{
+ return test_tls_record_proto_opt_padding(33, 4,
RTE_SECURITY_VERSION_TLS_1_2);
+}
+
static int
test_dtls_1_2_record_proto_data_walkthrough(void)
{
@@ -17578,6 +17637,30 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"Zero len TLS record with content type as ctrl",
ut_setup_security, ut_teardown,
test_tls_record_proto_zero_len_non_app),
+ TEST_CASE_NAMED_ST(
+ "TLS record DM mode with optional padding < 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_dm_opt_padding),
+ TEST_CASE_NAMED_ST(
+ "TLS record DM mode with optional padding > 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_dm_opt_padding_1),
+ TEST_CASE_NAMED_ST(
+ "TLS record SG mode with optional padding < 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_sg_opt_padding),
+ TEST_CASE_NAMED_ST(
+ "TLS record SG mode with optional padding > 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_tls_record_proto_sg_opt_padding_1),
+ TEST_CASE_NAMED_ST(
+ "TLS reco
[PATCH v5 20/21] test/crypto: unit tests for padding in DTLS-1.2
From: Vidya Sagar Velumuri
Add unit tests to verify the padding for DTLS-1.2.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 60 +++
1 file changed, 60 insertions(+)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index a324c1607b..572740cbf9 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12483,6 +12483,42 @@ test_dtls_1_2_record_proto_zero_len_non_app(void)
return test_tls_record_proto_all(&flags);
}
+static int
+test_dtls_1_2_record_proto_dm_opt_padding(void)
+{
+ return test_tls_record_proto_opt_padding(1, 0,
RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
+static int
+test_dtls_1_2_record_proto_dm_opt_padding_1(void)
+{
+ return test_tls_record_proto_opt_padding(25, 0,
RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
+static int
+test_dtls_1_2_record_proto_sg_opt_padding(void)
+{
+ return test_tls_record_proto_opt_padding(1, 5,
RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
+static int
+test_dtls_1_2_record_proto_sg_opt_padding_1(void)
+{
+ return test_tls_record_proto_opt_padding(8, 4,
RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
+static int
+test_dtls_1_2_record_proto_sg_opt_padding_2(void)
+{
+ return test_tls_record_proto_opt_padding(8, 5,
RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
+static int
+test_dtls_1_2_record_proto_sg_opt_padding_max(void)
+{
+ return test_tls_record_proto_opt_padding(33, 4,
RTE_SECURITY_VERSION_DTLS_1_2);
+}
+
static int
test_tls_1_3_record_proto_corrupt_pkt(void)
{
@@ -17824,6 +17860,30 @@ static struct unit_test_suite
dtls12_record_proto_testsuite = {
"Antireplay with window size 4096",
ut_setup_security, ut_teardown,
test_dtls_1_2_record_proto_antireplay4096),
+ TEST_CASE_NAMED_ST(
+ "DTLS record DM mode with optional padding < 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_dm_opt_padding),
+ TEST_CASE_NAMED_ST(
+ "DTLS record DM mode with optional padding > 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_dm_opt_padding_1),
+ TEST_CASE_NAMED_ST(
+ "DTLS record SG mode with optional padding < 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_sg_opt_padding),
+ TEST_CASE_NAMED_ST(
+ "DTLS record SG mode with optional padding > 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_sg_opt_padding_1),
+ TEST_CASE_NAMED_ST(
+ "DTLS record SG mode with optional padding > 2 blocks",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_sg_opt_padding_2),
+ TEST_CASE_NAMED_ST(
+ "DTLS record SG mode with optional padding > max range",
+ ut_setup_security, ut_teardown,
+ test_dtls_1_2_record_proto_sg_opt_padding_max),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
--
2.25.1
[PATCH v5 15/21] test/crypto: update framework to verify tls-1.3
From: Vidya Sagar Velumuri
Update the fields in preparation of test descriptor.
Signed-off-by: Vidya Sagar Velumuri
---
app/test/test_cryptodev.c | 17 +---
app/test/test_cryptodev_security_tls_record.c | 43 ---
app/test/test_cryptodev_security_tls_record.h | 10 ++---
3 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index aa9fffe50e..25777c1b1f 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11889,8 +11889,9 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
ut_params->op->param1.tls_record.content_type = td[i].app_type;
/* Copy IV in crypto operation when IV generation is disabled */
- if (sess_type == RTE_SECURITY_TLS_SESS_TYPE_WRITE &&
- tls_record_xform.options.iv_gen_disable == 1) {
+ if ((sess_type == RTE_SECURITY_TLS_SESS_TYPE_WRITE) &&
+ (tls_record_xform.ver != RTE_SECURITY_VERSION_TLS_1_3) &&
+ (tls_record_xform.options.iv_gen_disable == 1)) {
uint8_t *iv;
int len;
@@ -12005,8 +12006,10 @@ test_tls_record_proto_all(const struct
tls_record_test_flags *flags)
if (flags->zero_len)
payload_len = 0;
again:
- test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2, flags,
- td_outb, nb_pkts, payload_len);
+ ret = test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2,
+flags, td_outb, nb_pkts,
payload_len);
+ if (ret == TEST_SKIPPED)
+ continue;
ret = test_tls_record_proto_process(td_outb, td_inb, nb_pkts,
true, flags);
if (ret == TEST_SKIPPED)
@@ -12218,8 +12221,10 @@ test_dtls_pkt_replay(const uint64_t seq_no[],
int ret;
for (i = 0; i < RTE_DIM(sec_alg_list); i++) {
- test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2, flags,
- td_outb, nb_pkts, 0);
+ ret = test_tls_record_td_prepare(sec_alg_list[i].param1,
sec_alg_list[i].param2,
+flags, td_outb, nb_pkts, 0);
+ if (ret == TEST_SKIPPED)
+ continue;
for (idx = 0; idx < nb_pkts; idx++)
td_outb[idx].tls_record_xform.dtls_1_2.seq_no =
seq_no[idx];
diff --git a/app/test/test_cryptodev_security_tls_record.c
b/app/test/test_cryptodev_security_tls_record.c
index 498c4923e0..96d0a94731 100644
--- a/app/test/test_cryptodev_security_tls_record.c
+++ b/app/test/test_cryptodev_security_tls_record.c
@@ -70,7 +70,7 @@ test_tls_record_td_read_from_write(const struct
tls_record_test_data *td_out,
}
}
-void
+int
test_tls_record_td_prepare(const struct crypto_param *param1, const struct
crypto_param *param2,
const struct tls_record_test_flags *flags,
struct tls_record_test_data *td_array,
@@ -79,6 +79,10 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
int i, min_padding, hdr_len, tls_pkt_size, mac_len = 0, exp_nonce_len =
0, roundup_len = 0;
struct tls_record_test_data *td = NULL;
+ if ((flags->tls_version == RTE_SECURITY_VERSION_TLS_1_3) &&
+ (param1->type != RTE_CRYPTO_SYM_XFORM_AEAD))
+ return TEST_SKIPPED;
+
memset(td_array, 0, nb_td * sizeof(*td));
for (i = 0; i < nb_td; i++) {
@@ -88,10 +92,17 @@ test_tls_record_td_prepare(const struct crypto_param
*param1, const struct crypt
if (param1->type == RTE_CRYPTO_SYM_XFORM_AEAD) {
/* Copy template for packet & key fields */
- if (flags->tls_version == RTE_SECURITY_VERSION_DTLS_1_2)
- memcpy(td, &dtls_test_data_aes_128_gcm,
sizeof(*td));
- else
+ switch (flags->tls_version) {
+ case RTE_SECURITY_VERSION_TLS_1_2:
memcpy(td, &tls_test_data_aes_128_gcm_v1,
sizeof(*td));
+ break;
+ case RTE_SECURITY_VERSION_DTLS_1_2:
+ memcpy(td, &dtls_test_data_aes_128_gcm,
sizeof(*td));
+ break;
+ case RTE_SECURITY_VERSION_TLS_1_3:
+ memcpy(td, &tls13_test_data_aes_128_gcm,
sizeof(*td));
+ break;
+ }
td->aead = true;
td->xform.aead.aead.algo = param1->alg.aead;
@@ -127,6 +138,7 @@ test_tls_
[PATCH v5 21/21] test/security: add out of place sgl test case for TLS 1.2
Add TLS 1.2 out-of-place multi-segmented packet test.
Signed-off-by: Aakash Sasidharan
---
app/test/test_cryptodev.c | 52 ++-
app/test/test_cryptodev_security_tls_record.h | 1 +
2 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 572740cbf9..1703ebccf1 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11873,6 +11873,11 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
ut_params->ibuf = create_segmented_mbuf(ts_params->mbuf_pool,
td[i].input_text.len,
nb_segs, 0);
pktmbuf_write(ut_params->ibuf, 0, td[i].input_text.len,
td[i].input_text.data);
+ if (flags->out_of_place)
+ ut_params->obuf =
create_segmented_mbuf(ts_params->mbuf_pool,
+ td[i].output_text.len, nb_segs, 0);
+ else
+ ut_params->obuf = NULL;
/* Generate crypto op data structure */
ut_params->op = rte_crypto_op_alloc(ts_params->op_mpool,
@@ -11888,7 +11893,7 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
/* Set crypto operation mbufs */
ut_params->op->sym->m_src = ut_params->ibuf;
- ut_params->op->sym->m_dst = NULL;
+ ut_params->op->sym->m_dst = ut_params->obuf;
ut_params->op->param1.tls_record.content_type = td[i].app_type;
if (flags->opt_padding)
@@ -11920,7 +11925,10 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
res_d_tmp = &res_d[i];
if (ut_params->op->status == RTE_CRYPTO_OP_STATUS_SUCCESS) {
- ret = test_tls_record_post_process(ut_params->ibuf,
&td[i], res_d_tmp,
+ struct rte_mbuf *buf = flags->out_of_place ?
ut_params->obuf :
+ ut_params->ibuf;
+
+ ret = test_tls_record_post_process(buf, &td[i],
res_d_tmp,
silent, flags);
if (ret != TEST_SUCCESS)
goto crypto_op_free;
@@ -11929,6 +11937,11 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
rte_crypto_op_free(ut_params->op);
ut_params->op = NULL;
+ if (flags->out_of_place) {
+ rte_pktmbuf_free(ut_params->obuf);
+ ut_params->obuf = NULL;
+ }
+
rte_pktmbuf_free(ut_params->ibuf);
ut_params->ibuf = NULL;
}
@@ -11937,6 +11950,11 @@ test_tls_record_proto_process(const struct
tls_record_test_data td[],
rte_crypto_op_free(ut_params->op);
ut_params->op = NULL;
+ if (flags->out_of_place) {
+ rte_pktmbuf_free(ut_params->obuf);
+ ut_params->obuf = NULL;
+ }
+
rte_pktmbuf_free(ut_params->ibuf);
ut_params->ibuf = NULL;
@@ -12127,6 +12145,32 @@ test_tls_record_proto_sgl_data_walkthrough(enum
rte_security_tls_version tls_ver
return test_tls_record_proto_all(&flags);
}
+static int
+test_tls_record_proto_sgl_oop(enum rte_security_tls_version tls_version)
+{
+ struct tls_record_test_flags flags = {
+ .nb_segs_in_mbuf = 5,
+ .out_of_place = true,
+ .tls_version = tls_version
+ };
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ struct rte_cryptodev_info dev_info;
+
+ rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
+ if (!(dev_info.feature_flags & RTE_CRYPTODEV_FF_IN_PLACE_SGL)) {
+ printf("Device doesn't support in-place scatter-gather. Test
Skipped.\n");
+ return TEST_SKIPPED;
+ }
+
+ return test_tls_record_proto_all(&flags);
+}
+
+static int
+test_tls_1_2_record_proto_sgl_oop(void)
+{
+ return test_tls_record_proto_sgl_oop(RTE_SECURITY_VERSION_TLS_1_2);
+}
+
static int
test_tls_1_2_record_proto_sgl_data_walkthrough(void)
{
@@ -17657,6 +17701,10 @@ static struct unit_test_suite
tls12_record_proto_testsuite = {
"Multi-segmented mode data walkthrough",
ut_setup_security, ut_teardown,
test_tls_1_2_record_proto_sgl_data_walkthrough),
+ TEST_CASE_NAMED_ST(
+ "Multi-segmented mode out of place",
+ ut_setup_security, ut_teardown,
+ test_tls_1_2_record_proto_sgl_oop),
TEST_CASE_NAMED_ST(
"TLS packet header corruption",
ut_setup_security, ut_teardown,
diff --git a/app/test/test_cryptodev_security_tls_record.h
b/app/
Re: [PATCH 3/4] app/testpmd: check queue count for related options
On Wed, Mar 13, 2024 at 11:52 AM Ferruh Yigit wrote:
>
> On 3/13/2024 7:37 AM, David Marchand wrote:
> > On Tue, Mar 12, 2024 at 5:59 PM Ferruh Yigit wrote:
> >>
> >> On 3/8/2024 2:48 PM, David Marchand wrote:
> >>> Checking the number of rxq/txq in the middle of option parsing is
> >>> confusing. Move the check where nb_rxq / nb_txq are modified.
> >>>
> >>> Signed-off-by: David Marchand
> >>> ---
> >>> app/test-pmd/parameters.c | 8
> >>> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> >>> index 8c21744009..271f0c995a 100644
> >>> --- a/app/test-pmd/parameters.c
> >>> +++ b/app/test-pmd/parameters.c
> >>> @@ -1063,6 +1063,8 @@ launch_args_parse(int argc, char** argv)
> >>> rte_exit(EXIT_FAILURE, "rxq %d
> >>> invalid - must be"
> >>> " >= 0 && <= %u\n", n,
> >>>
> >>> get_allowed_max_nb_rxq(&pid));
> >>> + if (!nb_rxq && !nb_txq)
> >>> + rte_exit(EXIT_FAILURE, "Either rx
> >>> or tx queues should be non-zero\n");
> >>> }
> >>> if (!strcmp(lgopts[opt_idx].name, "txq")) {
> >>> n = atoi(optarg);
> >>> @@ -1072,6 +1074,8 @@ launch_args_parse(int argc, char** argv)
> >>> rte_exit(EXIT_FAILURE, "txq %d
> >>> invalid - must be"
> >>> " >= 0 && <= %u\n", n,
> >>>
> >>> get_allowed_max_nb_txq(&pid));
> >>> + if (!nb_rxq && !nb_txq)
> >>> + rte_exit(EXIT_FAILURE, "Either rx
> >>> or tx queues should be non-zero\n");
> >>> }
> >>> if (!strcmp(lgopts[opt_idx].name, "hairpinq")) {
> >>> n = atoi(optarg);
> >>> @@ -1098,10 +1102,6 @@ launch_args_parse(int argc, char** argv)
> >>> n + nb_rxq,
> >>>
> >>> get_allowed_max_nb_rxq(&pid));
> >>> }
> >>> - if (!nb_rxq && !nb_txq) {
> >>> - rte_exit(EXIT_FAILURE, "Either rx or tx
> >>> queues should "
> >>> - "be non-zero\n");
> >>> - }
> >>> if (!strcmp(lgopts[opt_idx].name, "hairpin-mode")) {
> >>> char *end = NULL;
> >>> unsigned int n;
> >>
> >> There is already a runtime check for queues [1], perhaps we can remove
> >> it altogether from arg parse.
> >
> > Good catch.
> >
> > This other check comes after parsing args, so I suspect it is just dead
> > code.
> > I guess I'll change it into a rte_exit(EXIT_FAILURE..).
> > Is this what you propose?
> >
>
> I think that check is the main check for nb_rxq and nb_txq.
>
> The one you removed is for the 'hairpinq' parameter, which is not a very
> common usecase.
This check was present before hairpinq introduction.
https://git.dpdk.org/dpdk/commit/app/test-pmd/parameters.c?id=1c69df45f8c6b727c3b6a78e13f81225c090dde2
This check in parsing args is hit when setting incorrect rxq / txq config.
This has nothing to do with hairpinq parsing.
$ build-mini/app/dpdk-testpmd -c 3 --no-huge -m 40 -a 0:0.0 --vdev
net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia
--rxq 0 --txq 0
EAL: Detected CPU lcores: 16
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
TELEMETRY: No legacy callbacks, legacy socket not created
Interactive-mode selected
Auto-start selected
EAL: Error - exiting with code: 1
Cause: Either rx or tx queues should be non-zero
Port 0 is closed
Port 1 is closed
> But nb_rxq and nb_txq requirement is very common and it is protected in
> the main after parameter parsing.
Sorry, I am not following.
>
> I am not suggesting adding 'rte_exit()' for that case, probably it will
> fail in some other part and error log can provide the required hint.
> And I am worried if it breaks some unexpected usecase with exit.
If we simply remove this check as you suggest:
$ build-mini/app/dpdk-testpmd -c 3 --no-huge -m 40 -a 0:0.0 --vdev
net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia
--rxq 0 --txq 0
EAL: Detected CPU lcores: 16
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
TELEMETRY: No legacy callbacks, legacy socket not created
Interactive-mode selected
Auto-start selected
Warning: Either rx or tx queues should be non-zero
^^^
Pointless log.
Re: [PATCH 2/4] app/testpmd: fix burst option parsing
On Wed, Mar 13, 2024 at 11:37 AM Ferruh Yigit wrote:
>
> On 3/13/2024 7:24 AM, David Marchand wrote:
> > On Tue, Mar 12, 2024 at 5:47 PM Ferruh Yigit wrote:
> >>
> >> On 3/8/2024 2:48 PM, David Marchand wrote:
> >>> rte_eth_dev_info_get() is not supposed to fail for a valid port_id, but
> >>> for the theoretical case when it would fail, raise an error rather than
> >>> skip subsequent options.
> >>>
> >>> Fixes: 6f51deb903b2 ("app/testpmd: check status of getting ethdev info")
> >>> Cc: sta...@dpdk.org
> >>>
> >>> Signed-off-by: David Marchand
> >>> ---
> >>> app/test-pmd/parameters.c | 6 +++---
> >>> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> >>> index d715750bb8..8c21744009 100644
> >>> --- a/app/test-pmd/parameters.c
> >>> +++ b/app/test-pmd/parameters.c
> >>> @@ -1128,9 +1128,9 @@ launch_args_parse(int argc, char** argv)
> >>> 0,
> >>> &dev_info);
> >>> if (ret != 0)
> >>> - return;
> >>> -
> >>> - rec_nb_pkts = dev_info
> >>> + rec_nb_pkts = 0;
> >>> + else
> >>> + rec_nb_pkts = dev_info
> >>>
> >>> .default_rxportconf.burst_size;
> >>>
> >>> if (rec_nb_pkts == 0)
> >>
> >> 'eth_dev_info_get_print_err()' already fail, but it may not be very
> >> clear to the user,
> >> OK to print a failure log, but setting 'rec_nb_pkts = 0;' as above also
> >> will generate an error message that also may be confusing to the user.
> >>
> >> What about print an explicit error message for the
> >> 'eth_dev_info_get_print_err()' failed case?
> >
> > rte_exit(EXIT_FAILURE, "Failed to retrieve device info, this is
> > probably a driver bug. "
> > "To workaround this issue, please provide a value between 1
> > and %d\n", MAX_PKT_BURST);
> >
> > Does it work for you?
> >
> >
>
> 'eth_dev_info_get_print_err()' already logs error about getting device
> info, but driver recommended 'burst' setting failed information is missing.
>
> What about more direct,
> "Failed to get driver recommended burst size, please provide a value
> between 1 and MAX_PKT_BURST"
Which is really close to the existing log message.
if (rec_nb_pkts == 0)
rte_exit(EXIT_FAILURE,
"PMD does not
recommend a burst size. "
"Provided
value must be between "
"1 and %d\n",
MAX_PKT_BURST);
I am unconvinced, but if you think strongly for this, I won't debate more.
--
David Marchand
Re: [PATCH v3 04/33] net/ena: sub-optimal configuration notifications support
On 3/10/2024 2:43 PM, Brandes, Shai wrote:
>
>
>> -Original Message-
>> From: Ferruh Yigit
>> Sent: Friday, March 8, 2024 7:23 PM
>> To: Brandes, Shai
>> Cc: dev@dpdk.org
>> Subject: RE: [EXTERNAL] [PATCH v3 04/33] net/ena: sub-optimal
>> configuration notifications support
>>
>> CAUTION: This email originated from outside of the organization. Do not click
>> links or open attachments unless you can confirm the sender and know the
>> content is safe.
>>
>>
>>
>> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote:
>>> From: Shai Brandes
>>>
>>> ENA device will send asynchronous notifications to the driver in order
>>> to notify users about sub-optimal configurations and refer them to
>>> public AWS documentation for further action.
>>>
>>
>> Hi Shai,
>>
>> This is an interesting feature, I am curious, is there more public detail
>> provided by AWS on how it detects sub-optimal configuration and what are
>> the possible types of the notifications?
>>
> [Brandes, Shai] This is only a framework to allow notifications to the user.
> Currently, the only notification the device supports relate to sub-optimal
> configuration when enabling ena-express feature.
> The public documentation for it was not published yet, but it currently
> contains only two codes, indicating the user that it is better to run with
> normal-llq when working with ena-express and an option to increase the Tx
> queue depth when working with ena-express to double the default size on
> specific hardwares that have a larger bar (known only in run-time)
>
Thanks for the info. When there is a public documentation for the
feature, can you please reference it from driver documentation?
>
>>> Signed-off-by: Shai Brandes
>>> Reviewed-by: Amit Bernstein
>>> ---
>>> doc/guides/rel_notes/release_24_03.rst| 1 +
>>> .../net/ena/base/ena_defs/ena_admin_defs.h| 11 +++-
>>> drivers/net/ena/ena_ethdev.c | 26 +--
>>> 3 files changed, 35 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/guides/rel_notes/release_24_03.rst
>>> b/doc/guides/rel_notes/release_24_03.rst
>>> index fb66d67d32..f47073c7dc 100644
>>> --- a/doc/guides/rel_notes/release_24_03.rst
>>> +++ b/doc/guides/rel_notes/release_24_03.rst
>>> @@ -104,6 +104,7 @@ New Features
>>> * **Updated Amazon ena (Elastic Network Adapter) net driver.**
>>>
>>>* Removed the reporting of `rx_overruns` errors from xstats and instead
>> updated `imissed` stat with its value.
>>> + * Added support for sub-optimal configuration notifications from the
>> device.
>>>
>>> * **Updated Atomic Rules' Arkville driver.**
>>>
>>> diff --git a/drivers/net/ena/base/ena_defs/ena_admin_defs.h
>>> b/drivers/net/ena/base/ena_defs/ena_admin_defs.h
>>> index fa43e22918..4172916551 100644
>>> --- a/drivers/net/ena/base/ena_defs/ena_admin_defs.h
>>> +++ b/drivers/net/ena/base/ena_defs/ena_admin_defs.h
>>> @@ -1214,7 +1214,8 @@ enum ena_admin_aenq_group {
>>> ENA_ADMIN_NOTIFICATION = 3,
>>> ENA_ADMIN_KEEP_ALIVE= 4,
>>> ENA_ADMIN_REFRESH_CAPABILITIES = 5,
>>> - ENA_ADMIN_AENQ_GROUPS_NUM = 6,
>>> + ENA_ADMIN_CONF_NOTIFICATIONS= 6,
>>> + ENA_ADMIN_AENQ_GROUPS_NUM = 7,
>>> };
>>>
>>> enum ena_admin_aenq_notification_syndrome { @@ -1251,6 +1252,14
>> @@
>>> struct ena_admin_aenq_keep_alive_desc {
>>> uint32_t rx_overruns_high;
>>> };
>>>
>>> +struct ena_admin_aenq_conf_notifications_desc {
>>> + struct ena_admin_aenq_common_desc aenq_common_desc;
>>> +
>>> + uint64_t notifications_bitmap;
>>> +
>>> + uint64_t reserved;
>>> +};
>>> +
>>> struct ena_admin_ena_mmio_req_read_less_resp {
>>> uint16_t req_id;
>>>
>>> diff --git a/drivers/net/ena/ena_ethdev.c
>>> b/drivers/net/ena/ena_ethdev.c index d3f395a832..3157237c0d 100644
>>> --- a/drivers/net/ena/ena_ethdev.c
>>> +++ b/drivers/net/ena/ena_ethdev.c
>>> @@ -36,6 +36,10 @@
>>>
>>> #define ENA_MIN_RING_DESC128
>>>
>>> +#define BITS_PER_BYTE 8
>>> +
>>> +#define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
>>> +
>>>
>>
>> 'CHAR_BIT' macro can be used here, but I can see there are multiple drivers
>> defining similar macros.So no need to update this patch, but to record that
>> this is something to address DPDK wide.
>>
>> If ena team volunteers to tackle this update, it is welcomed ;)
> [Brandes, Shai] sure, can be done
>
Thanks, appreciated.
Re: [PATCH v3 07/33] net/ena: restructure the llq policy setting process
On 3/10/2024 2:29 PM, Brandes, Shai wrote: > > >> -Original Message- >> From: Ferruh Yigit >> Sent: Friday, March 8, 2024 7:24 PM >> To: Brandes, Shai >> Cc: dev@dpdk.org >> Subject: RE: [EXTERNAL] [PATCH v3 07/33] net/ena: restructure the llq policy >> setting process >> >> CAUTION: This email originated from outside of the organization. Do not click >> links or open attachments unless you can confirm the sender and know the >> content is safe. >> >> >> >> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote: >>> From: Shai Brandes >>> >>> The driver will set the size of the LLQ header size according to the >>> recommendation from the device. >>> Replaced `enable_llq` and `large_llq_hdr` devargs with a new devarg >>> `llq_policy` that accepts the following values: >>> 0 - Disable LLQ. >>> Use with extreme caution as it leads to a huge performance >>> degradation on AWS instances from 6th generation onwards. >>> 1 - Accept device recommended LLQ policy (Default). >>> Device can recommend normal or large LLQ policy. >>> 2 - Enforce normal LLQ policy. >>> 3 - Enforce large LLQ policy. >>> Required for packets with header that exceed 96 bytes on >>> AWS instances prior to 5th generation. >>> >> >> We had similar discussion before, although dev_args is not part of the ABI, >> it >> is an user interface, and changes in the devargs will impact users directly. >> >> What would you think to either keep backward compatilibity in the devargs >> (like not remove old one but add new one), or do this change in >> 24.11 release? > [Brandes, Shai] understood. > The new devarg replaced the old ones and added option to enforce normal-llq > mode which is critical for our release. > As you suggested, we will keep backward compatibility and add an additional > devarg for enforcing normal-llq policy. > That way, we can easily replace it in future releases with a common devarg > without the need to make major logic changes. > ack.
Re: [PATCH v3 08/33] net/ena/hal: exponential backoff exp limit
On 3/12/2024 4:53 PM, Brandes, Shai wrote:
>
>
>> -Original Message-
>> From: Brandes, Shai
>> Sent: Sunday, March 10, 2024 4:54 PM
>> To: 'Ferruh Yigit'
>> Cc: dev@dpdk.org
>> Subject: RE: [EXTERNAL] [PATCH v3 08/33] net/ena/hal: exponential backoff
>> exp limit
>>
>>
>>
>>> -Original Message-
>>> From: Ferruh Yigit
>>> Sent: Friday, March 8, 2024 7:24 PM
>>> To: Brandes, Shai
>>> Cc: dev@dpdk.org
>>> Subject: RE: [EXTERNAL] [PATCH v3 08/33] net/ena/hal: exponential
>>> backoff exp limit
>>>
>>> CAUTION: This email originated from outside of the organization. Do
>>> not click links or open attachments unless you can confirm the sender
>>> and know the content is safe.
>>>
>>>
>>>
>>> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote:
From: Shai Brandes
limits the exponent in the exponential backoff mechanism in order to
avoid the value overflowing.
>>>
>>> Is this a fix?
> [Brandes, Shai] No, this is originated from a backport from the Linux
> community to our HAL.
> The backoff mechanism is used to delay device reset, command completion
> checks, etc.
> The backoff eventually could cause the delay to become excessive (1<<32).
> So, this patch cap the backoff value of the exponent used for this backoff at
> (1<<16).
> In addition, for uniformity and readability purposes, the min/max parameter
> in the calls of ENA_MIN32 and ENA_MAX32 macros was changed to be first.
>
I think this fixes the device reset.
Without cap in the backoff value, delay can be too long (depending input
to the function) so that can't reset the device.
But anyway, thanks for the clarification.
>
>>>
>>> What was the impact of the overflowing if not limited? And is there a
>>> significance of the value 16, can you please elaborate?
>>>
>> [Brandes, Shai] I will restructure this patch, since this likely hides a fix
>> in hal.
>> It is originated from the HAL release, from which I took the patches one by
>> one, but the commit messages there tend to be (too) concise.
>>
>>>
>>> Also let me remind the patch subject format, (this may look
>>> insignificant but helps to have more unified commit messages for
>>> developers, and if not updated by author, maintainers update it and
>>> this brings more overhead to
>>> maintainers):
>>> "sub-module: verb object"
>>>
>>> And we use verb 'fix' explicitly for all commits fixing something, and
>>> that something can't be referenced as 'error', 'failure', 'issue',
>>> 'problem', etc... but it should be detailed.
>>>
>>> Most of the times better to document NOT from driver internal
>>> perspective, but impact of it, like "net/ena: set chain limit to 16"
>>> is NOT a good one, it explains driver internal perspective (making all
>>> up) but it can be something
>>> like:
>>> "net/ena: support big packets by increasing link limit"
>>>
>>> For this one, I am not sure impact of the change so hard for me to
>>> propose an alternative, but just as example it can be something like:
>>> "net/ena/base: avoid collision by limiting backoff delay"
>>>
Signed-off-by: Shai Brandes
Reviewed-by: Amit Bernstein
---
drivers/net/ena/hal/ena_com.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ena/hal/ena_com.c
b/drivers/net/ena/hal/ena_com.c index 6953a1fa33..31c37b0ab3 100644
--- a/drivers/net/ena/hal/ena_com.c
+++ b/drivers/net/ena/hal/ena_com.c
@@ -34,6 +34,8 @@
#define ENA_REGS_ADMIN_INTR_MASK 1
+#define ENA_MAX_BACKOFF_DELAY_EXP 16U
+
#define ENA_MIN_ADMIN_POLL_US 100
#define ENA_MAX_ADMIN_POLL_US 5000
@@ -545,8 +547,9 @@ static int ena_com_comp_status_to_errno(struct
ena_com_admin_queue *admin_queue,
static void ena_delay_exponential_backoff_us(u32 exp, u32 delay_us)
{
+ exp = ENA_MIN32(ENA_MAX_BACKOFF_DELAY_EXP, exp);
delay_us = ENA_MAX32(ENA_MIN_ADMIN_POLL_US, delay_us);
- delay_us = ENA_MIN32(delay_us * (1U << exp),
>>> ENA_MAX_ADMIN_POLL_US);
+ delay_us = ENA_MIN32(ENA_MAX_ADMIN_POLL_US, delay_us *
>> (1U
>>> <<
+ exp));
ENA_USLEEP(delay_us);
}
>
Re: [PATCH v3 10/33] net/ena/hal: added a bus parameter to ena memcpy macro
On 3/10/2024 3:08 PM, Brandes, Shai wrote: > > >> -Original Message- >> From: Ferruh Yigit >> Sent: Friday, March 8, 2024 7:25 PM >> To: Brandes, Shai >> Cc: dev@dpdk.org >> Subject: RE: [EXTERNAL] [PATCH v3 10/33] net/ena/hal: added a bus >> parameter to ena memcpy macro >> >> CAUTION: This email originated from outside of the organization. Do not click >> links or open attachments unless you can confirm the sender and know the >> content is safe. >> >> >> >> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote: >>> From: Shai Brandes >>> >>> ENA_MEMCPY_TO_DEVICE_64 macro needs pci bus id in order to write to >>> the device memory when using llq. >>> >> >> As far as I can see macro doesn't use 'bus' at all, "(void)(bus);", how/why >> it is >> needed for LLQ? Can you please describe it more? > [Brandes, Shai] I understand the confusion. > This is part of a hal change that concerns mac OS and required to modify the > common ENA_MEMCPY_TO_DEVICE_64. > Since we expose only the DPDK-specific implementation, where this parameter > is unused, it appears as void. > Avoiding this will create differences between the internal and upstream > versions which will make it hard to maintain. > It is OK to have it, as you said to reduce the diff, but please update the commit log with above detail, and it will be OK.
Re: [PATCH v3 00/33] net/ena: v2.9.0 driver release
On 3/10/2024 2:21 PM, Brandes, Shai wrote: > > >> -Original Message- >> From: Brandes, Shai >> Sent: Friday, March 8, 2024 10:27 PM >> To: Ferruh Yigit >> Cc: dev@dpdk.org >> Subject: RE: [PATCH v3 00/33] net/ena: v2.9.0 driver release >> >> Sure, will upload a new seried, thanks! >> >> בתאריך 8 במרץ 2024 19:36, Ferruh Yigit כתב: >> >> CAUTION: This email originated from outside of the organization. Do not click >> links or open attachments unless you can confirm the sender and know the >> content is safe. >> >> >> >> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote: >>> From: Shai Brandes >>> >>> Hi all, the ena v2.9.0 release introduces: >>> 1. HAL upgrade: >>>- renamed the 'base' folder to be 'hal' >>>- separated the HAL patches instead of a bulk update. >>> 2. Restructured ena stats and metrics. >>> 3. Restructured the LLQ configuration: >>>- configurable via devarg. >>>- support device recommendation. >>>- restructure the logic in driver. >>> 4. Added support for the admin queue to work only in poll-mode >>>- configurable via devarg. >>>- allows to bind ports to uio_pci_generic kernel driver. >>> 5. Reworked the device close to exhaust interrupt callbacks and alarms. >>> 6. Fixed a bug in fast mbuf free. >>> Best regards. >>> >>> --- >>> v3: >>> * Fixed missing admin queue missing intialization in patch 0032 >>> >>> v2: >>> * Fixed minor spelling issues from checkpatch >>> >>> >>> Shai Brandes (33): >>> net/ena: rework the metrics multi-process functions >>> net/ena: report new supported link speed capabilities >>> net/ena: update imissed stat with Rx overruns >>> net/ena: sub-optimal configuration notifications support >>> net/ena: fix fast mbuf free >>> net/ena: rename base folder to hal >>> net/ena: restructure the llq policy setting process >>> net/ena/hal: exponential backoff exp limit >>> net/ena/hal: add a new csum offload bit >>> net/ena/hal: added a bus parameter to ena memcpy macro >>> net/ena/hal: optimize Rx ring submission queue >>> net/ena/hal: rename fields in completion descriptors >>> net/ena/hal: use correct read once on u8 field >>> net/ena/hal: add completion descriptor corruption check >>> net/ena/hal: malformed Tx descriptor error reason >>> net/ena/hal: phc feature modifications >>> net/ena/hal: restructure interrupt handling >>> net/ena/hal: add unlikely to error checks >>> net/ena/hal: missing admin interrupt reset reason >>> net/ena/hal: check for existing keep alive notification >>> net/ena/hal: modify memory barrier comment >>> net/ena/hal: rework Rx ring submission queue >>> net/ena/hal: remove operating system type enum >>> net/ena/hal: handle command abort >>> net/ena/hal: add support for device reset request >>> net/ena: cosmetic changes >>> net/ena/hal: modify customer metrics memory management >>> net/ena/hal: cosmetic changes >>> net/ena: update device-preferred size of rings >>> net/ena: exhaust interrupt callbacks in device close >>> net/ena: support max large llq depth from the device >>> net/ena: control path pure polling mode >>> net/ena: upgrade driver version to 2.9.0 >>> >> >> Hi Shai, >> >> I did review only first 10 patches, there are some common patterns to >> address in the perspective and commit logs. >> >> Can you please update whole series according to comments? I will review >> remaining patches in the new version. >> >> Thanks, >> Ferruh >> > [Brandes, Shai] sure, thanks for the comments, we will align accordingly. > Thanks, I can see v4 is out, I will try to review it today for -rc3.
Re: [PATCH v3 05/33] net/ena: fix fast mbuf free
On 3/10/2024 2:58 PM, Brandes, Shai wrote: > > >> -Original Message- >> From: Ferruh Yigit >> Sent: Friday, March 8, 2024 7:23 PM >> To: Brandes, Shai >> Cc: dev@dpdk.org; sta...@dpdk.org >> Subject: RE: [EXTERNAL] [PATCH v3 05/33] net/ena: fix fast mbuf free >> >> CAUTION: This email originated from outside of the organization. Do not click >> links or open attachments unless you can confirm the sender and know the >> content is safe. >> >> >> >> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote: >>> From: Shai Brandes >>> >>> In case the application enables fast mbuf release optimization, the >>> driver releases 256 TX mbufs in bulk upon reaching the TX free >>> threshold. >>> The existing implementation utilizes rte_mempool_put_bulk for bulk >>> freeing TXs, which exclusively supports direct mbufs. >>> In case the application transmits indirect bufs, the driver must also >>> decrement the mbuf reference count and unlink the mbuf segment. >>> For such case, the driver should employ rte_pktmbuf_free_bulk. >>> >> >> Ack. >> >> I wonder if you observe any performance impact from this change, just for >> reference if we encounter similar decision in the future. > [Brandes, Shai] we did not see performance impact in our testing. > It was discovered by a new latency application we crafted that uses the bulk > free option, which transmitted one by one packets copied from a common > buffer, but showed that there are missing packets. > ack.
Re: [PATCH 2/4] app/testpmd: fix burst option parsing
On 3/13/2024 11:13 AM, David Marchand wrote:
> On Wed, Mar 13, 2024 at 11:37 AM Ferruh Yigit wrote:
>>
>> On 3/13/2024 7:24 AM, David Marchand wrote:
>>> On Tue, Mar 12, 2024 at 5:47 PM Ferruh Yigit wrote:
On 3/8/2024 2:48 PM, David Marchand wrote:
> rte_eth_dev_info_get() is not supposed to fail for a valid port_id, but
> for the theoretical case when it would fail, raise an error rather than
> skip subsequent options.
>
> Fixes: 6f51deb903b2 ("app/testpmd: check status of getting ethdev info")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand
> ---
> app/test-pmd/parameters.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index d715750bb8..8c21744009 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -1128,9 +1128,9 @@ launch_args_parse(int argc, char** argv)
> 0,
> &dev_info);
> if (ret != 0)
> - return;
> -
> - rec_nb_pkts = dev_info
> + rec_nb_pkts = 0;
> + else
> + rec_nb_pkts = dev_info
>
> .default_rxportconf.burst_size;
>
> if (rec_nb_pkts == 0)
'eth_dev_info_get_print_err()' already fail, but it may not be very
clear to the user,
OK to print a failure log, but setting 'rec_nb_pkts = 0;' as above also
will generate an error message that also may be confusing to the user.
What about print an explicit error message for the
'eth_dev_info_get_print_err()' failed case?
>>>
>>> rte_exit(EXIT_FAILURE, "Failed to retrieve device info, this is
>>> probably a driver bug. "
>>> "To workaround this issue, please provide a value between 1
>>> and %d\n", MAX_PKT_BURST);
>>>
>>> Does it work for you?
>>>
>>>
>>
>> 'eth_dev_info_get_print_err()' already logs error about getting device
>> info, but driver recommended 'burst' setting failed information is missing.
>>
>> What about more direct,
>> "Failed to get driver recommended burst size, please provide a value
>> between 1 and MAX_PKT_BURST"
>
> Which is really close to the existing log message.
>
> if (rec_nb_pkts == 0)
> rte_exit(EXIT_FAILURE,
> "PMD does not
> recommend a burst size. "
> "Provided
> value must be between "
> "1 and %d\n",
> MAX_PKT_BURST);
>
> I am unconvinced, but if you think strongly for this, I won't debate more.
>
>
Yes it is close, and I don't have strong opinion on this,
But existing message says "PMD does not recommend a burst size.", I
think this may be confusing for user.
If I see that message I would either debug relevant driver code or
communicate with driver maintainer for it. But that is not the case,
just failed to get recommended burst size and problem is in testpmd.
Anyway, my concern is existing message can be misleading for user.
Re: [PATCH] github: Reduce ASLR entropy to be compatible with asan in llvm 14.
On Tue, Mar 12, 2024 at 3:53 PM Aaron Conole wrote: > > GitHub recently started using newer Ubuntu 22.04 LTS container images, > versioned 20240310.1.0 which use 32-bit entropy for ASLR: > > $ sudo sysctl -a | grep vm.mmap.rnd > vm.mmap_rnd_bits = 32 > vm.mmap_rnd_compat_bits = 16 > > This breaks builds (such as the one at > https://github.com/DPDK/dpdk/actions/runs/8234334617/job/22515850325) by > causing a random segfault when ASAN is used, because older ASAN gets > confused by memory mappings and crashes. > > The issue is fixed in newer releases of LLVM: > > https://github.com/llvm/llvm-project/commit/fb77ca05ffb4f8e666878f2f6718a9fb4d686839 > https://reviews.llvm.org/D148280 > > But these are not available in Ubuntu 22.04 image. > > This should be fixed by GitHub, but until new images are available > reducing ASLR entropy manually to 28 bits to make builds work. > > Reported-at: https://github.com/actions/runner-images/issues/9491 > Signed-off-by: Aaron Conole > Suggested-by: Ilya Maximets Thanks Aaron, I applied this workaround for now. Heads up to subtree maintainers. We have some false positive test failures in GHA for the past days. Please rebase to DPDK main repository or pick this fix in your trees. Thanks. -- David Marchand
Re: [PATCH 3/4] app/testpmd: check queue count for related options
On 3/13/2024 11:10 AM, David Marchand wrote:
> On Wed, Mar 13, 2024 at 11:52 AM Ferruh Yigit wrote:
>>
>> On 3/13/2024 7:37 AM, David Marchand wrote:
>>> On Tue, Mar 12, 2024 at 5:59 PM Ferruh Yigit wrote:
On 3/8/2024 2:48 PM, David Marchand wrote:
> Checking the number of rxq/txq in the middle of option parsing is
> confusing. Move the check where nb_rxq / nb_txq are modified.
>
> Signed-off-by: David Marchand
> ---
> app/test-pmd/parameters.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 8c21744009..271f0c995a 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -1063,6 +1063,8 @@ launch_args_parse(int argc, char** argv)
> rte_exit(EXIT_FAILURE, "rxq %d
> invalid - must be"
> " >= 0 && <= %u\n", n,
>
> get_allowed_max_nb_rxq(&pid));
> + if (!nb_rxq && !nb_txq)
> + rte_exit(EXIT_FAILURE, "Either rx
> or tx queues should be non-zero\n");
> }
> if (!strcmp(lgopts[opt_idx].name, "txq")) {
> n = atoi(optarg);
> @@ -1072,6 +1074,8 @@ launch_args_parse(int argc, char** argv)
> rte_exit(EXIT_FAILURE, "txq %d
> invalid - must be"
> " >= 0 && <= %u\n", n,
>
> get_allowed_max_nb_txq(&pid));
> + if (!nb_rxq && !nb_txq)
> + rte_exit(EXIT_FAILURE, "Either rx
> or tx queues should be non-zero\n");
> }
> if (!strcmp(lgopts[opt_idx].name, "hairpinq")) {
> n = atoi(optarg);
> @@ -1098,10 +1102,6 @@ launch_args_parse(int argc, char** argv)
> n + nb_rxq,
>
> get_allowed_max_nb_rxq(&pid));
> }
> - if (!nb_rxq && !nb_txq) {
> - rte_exit(EXIT_FAILURE, "Either rx or tx
> queues should "
> - "be non-zero\n");
> - }
> if (!strcmp(lgopts[opt_idx].name, "hairpin-mode")) {
> char *end = NULL;
> unsigned int n;
There is already a runtime check for queues [1], perhaps we can remove
it altogether from arg parse.
>>>
>>> Good catch.
>>>
>>> This other check comes after parsing args, so I suspect it is just dead
>>> code.
>>> I guess I'll change it into a rte_exit(EXIT_FAILURE..).
>>> Is this what you propose?
>>>
>>
>> I think that check is the main check for nb_rxq and nb_txq.
>>
>> The one you removed is for the 'hairpinq' parameter, which is not a very
>> common usecase.
>
> This check was present before hairpinq introduction.
> https://git.dpdk.org/dpdk/commit/app/test-pmd/parameters.c?id=1c69df45f8c6b727c3b6a78e13f81225c090dde2
>
>
> This check in parsing args is hit when setting incorrect rxq / txq config.
> This has nothing to do with hairpinq parsing.
>
Right. I misread the code.
Probably check was just after rxq/txq parsing but 'hairpinq' parsing get
in the way and left check in even more odd location.
> $ build-mini/app/dpdk-testpmd -c 3 --no-huge -m 40 -a 0:0.0 --vdev
> net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia
> --rxq 0 --txq 0
> EAL: Detected CPU lcores: 16
> EAL: Detected NUMA nodes: 1
> EAL: Detected static linkage of DPDK
> EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> TELEMETRY: No legacy callbacks, legacy socket not created
> Interactive-mode selected
> Auto-start selected
> EAL: Error - exiting with code: 1
> Cause: Either rx or tx queues should be non-zero
> Port 0 is closed
> Port 1 is closed
>
>
>
>> But nb_rxq and nb_txq requirement is very common and it is protected in
>> the main after parameter parsing.
>
> Sorry, I am not following.
>
>>
>> I am not suggesting adding 'rte_exit()' for that case, probably it will
>> fail in some other part and error log can provide the required hint.
>> And I am worried if it breaks some unexpected usecase with exit.
>
> If we simply remove this check as you suggest:
> $ build-mini/app/dpdk-testpmd -c 3 --no-huge -m 40 -a 0:0.0 --vdev
> net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia
> --rxq 0 --txq 0
> EAL: Detected CPU lcores: 16
> EAL: Detected NUMA nodes: 1
> EAL: Detec
[PATCH v2 0/2] net/virtio: vhost-vdpa fixes
While investigating vhost-vdpa initialization issue with mlx5 vDPA, we found two issues fixed by following patches. In this v2, the control queue issue mentioned in v1 is fixed. It turned out to the control queue being enabled only if multiqueue was negotiated. It is fixed by enabling it at device startup, and disabling it at stop time. We still have an issue on one of our setup with mlx5, where the mlx5 device sets VIRTIO_CONFIG_S_FAILED status, it is currently being investigated. Changes in v2: -- - Fix cvq enablement - Fix typo in commit message (David) Maxime Coquelin (2): net/virtio: fix vDPA device init advertising control queue net/virtio: fix notification area initialization .../net/virtio/virtio_user/virtio_user_dev.c | 27 +-- 1 file changed, 19 insertions(+), 8 deletions(-) -- 2.44.0
[PATCH v2 1/2] net/virtio: fix vDPA device init advertising control queue
If the vDPA device advertises control queue support, but
the user neither passes "cq=1" as devarg nor requests
multiple queues, the initialization fails because the
driver tries to setup the control queue without negotiating
related feature.
This patch enables the control queue at driver level as
soon as the device claims to support it, and not only when
multiple queue pairs are requested. Also, enable the
control queue event if multiqueue feature has not been
negotiated and device start time, and disable it at device
stop time.
Fixes: b277308e8868 ("net/virtio-user: advertise control VQ support with vDPA")
Cc: sta...@dpdk.org
Signed-off-by: Maxime Coquelin
---
.../net/virtio/virtio_user/virtio_user_dev.c | 22 ++-
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index d395fc1676..54187fedf5 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -216,6 +216,12 @@ virtio_user_start_device(struct virtio_user_dev *dev)
if (ret < 0)
goto error;
+ if (dev->scvq) {
+ ret = dev->ops->cvq_enable(dev, 1);
+ if (ret < 0)
+ goto error;
+ }
+
dev->started = true;
pthread_mutex_unlock(&dev->mutex);
@@ -248,6 +254,12 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
goto err;
}
+ if (dev->scvq) {
+ ret = dev->ops->cvq_enable(dev, 0);
+ if (ret < 0)
+ goto err;
+ }
+
/* Stop the backend. */
for (i = 0; i < dev->max_queue_pairs * 2; ++i) {
state.index = i;
@@ -752,7 +764,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
*path, uint16_t queues,
if (virtio_user_dev_init_max_queue_pairs(dev, queues))
dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
- if (dev->max_queue_pairs > 1)
+ if (dev->max_queue_pairs > 1 || dev->hw_cvq)
cq = 1;
if (!mrg_rxbuf)
@@ -770,8 +782,9 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
*path, uint16_t queues,
dev->unsupported_features |= (1ull << VIRTIO_NET_F_MAC);
if (cq) {
- /* device does not really need to know anything about CQ,
-* so if necessary, we just claim to support CQ
+ /* Except for vDPA, the device does not really need to know
+* anything about CQ, so if necessary, we just claim to support
+* control queue.
*/
dev->frontend_features |= (1ull << VIRTIO_NET_F_CTRL_VQ);
} else {
@@ -871,9 +884,6 @@ virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t
q_pairs)
for (i = q_pairs; i < dev->max_queue_pairs; ++i)
ret |= dev->ops->enable_qp(dev, i, 0);
- if (dev->scvq)
- ret |= dev->ops->cvq_enable(dev, 1);
-
dev->queue_pairs = q_pairs;
return ret;
--
2.44.0
[PATCH v2 2/2] net/virtio: fix notification area initialization
Notification area is a Virtio feature that requires to be
negotiated because not all devices support it. Currently,
it is tried to be initialized as soon as the backend
implements the callback, so it assumes all Vhost-vDPA
devices support it.
This patch skips calling the notification area map callback
if the device does not advertise its support.
Fixes: 0fd2782660c8 ("net/virtio-user: support notification area mapping")
Reviewed-by: David Marchand
Signed-off-by: Maxime Coquelin
---
drivers/net/virtio/virtio_user/virtio_user_dev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 54187fedf5..4fdfe70e7c 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -445,8 +445,9 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
dev->kickfds[i] = kickfd;
}
- if (dev->ops->map_notification_area)
- if (dev->ops->map_notification_area(dev))
+ if (dev->device_features & (1ULL << VIRTIO_F_NOTIFICATION_DATA))
+ if (dev->ops->map_notification_area &&
+ dev->ops->map_notification_area(dev))
goto err;
return 0;
--
2.44.0
Re: [PATCH v4] app/eventdev: support DMA adapter test
On Tue, Mar 12, 2024 at 8:10 PM Amit Prakash Shukla wrote: > > Added performance test support for DMA adapter. > > Signed-off-by: Amit Prakash Shukla > Acked-by: Pavan Nikhilesh > --- > v4: > - Updated release notes. > > v3: > - Resolved review comments. > > v2: > - Fixed intel compilation error. > +* **Added DMA producer mode in test-eventdev.** > + > + * DMA producer mode helps to measure performance of OP_FORWARD mode of > event DMA > +adapter. Changed to following and applied to dpdk-next-net-eventdev/for-main. Thanks +* **Added DMA producer mode in test-eventdev.** + + * Added DMA producer mode to measure performance of ``OP_FORWARD`` mode of event DMA +adapter. + > + >
RE: [PATCH v3 00/33] net/ena: v2.9.0 driver release
> -Original Message- > From: Ferruh Yigit > Sent: Wednesday, March 13, 2024 1:28 PM > To: Brandes, Shai > Cc: dev@dpdk.org > Subject: RE: [EXTERNAL] [PATCH v3 00/33] net/ena: v2.9.0 driver release > > CAUTION: This email originated from outside of the organization. Do not click > links or open attachments unless you can confirm the sender and know the > content is safe. > > > > On 3/10/2024 2:21 PM, Brandes, Shai wrote: > > > > > >> -Original Message- > >> From: Brandes, Shai > >> Sent: Friday, March 8, 2024 10:27 PM > >> To: Ferruh Yigit > >> Cc: dev@dpdk.org > >> Subject: RE: [PATCH v3 00/33] net/ena: v2.9.0 driver release > >> > >> Sure, will upload a new seried, thanks! > >> > >> בתאריך 8 במרץ 2024 19:36, Ferruh Yigit כתב: > >> > >> CAUTION: This email originated from outside of the organization. Do > >> not click links or open attachments unless you can confirm the sender > >> and know the content is safe. > >> > >> > >> > >> On 3/6/2024 12:24 PM, shaib...@amazon.com wrote: > >>> From: Shai Brandes > >>> > >>> Hi all, the ena v2.9.0 release introduces: > >>> 1. HAL upgrade: > >>>- renamed the 'base' folder to be 'hal' > >>>- separated the HAL patches instead of a bulk update. > >>> 2. Restructured ena stats and metrics. > >>> 3. Restructured the LLQ configuration: > >>>- configurable via devarg. > >>>- support device recommendation. > >>>- restructure the logic in driver. > >>> 4. Added support for the admin queue to work only in poll-mode > >>>- configurable via devarg. > >>>- allows to bind ports to uio_pci_generic kernel driver. > >>> 5. Reworked the device close to exhaust interrupt callbacks and alarms. > >>> 6. Fixed a bug in fast mbuf free. > >>> Best regards. > >>> > >>> --- > >>> v3: > >>> * Fixed missing admin queue missing intialization in patch 0032 > >>> > >>> v2: > >>> * Fixed minor spelling issues from checkpatch > >>> > >>> > >>> Shai Brandes (33): > >>> net/ena: rework the metrics multi-process functions > >>> net/ena: report new supported link speed capabilities > >>> net/ena: update imissed stat with Rx overruns > >>> net/ena: sub-optimal configuration notifications support > >>> net/ena: fix fast mbuf free > >>> net/ena: rename base folder to hal > >>> net/ena: restructure the llq policy setting process > >>> net/ena/hal: exponential backoff exp limit > >>> net/ena/hal: add a new csum offload bit > >>> net/ena/hal: added a bus parameter to ena memcpy macro > >>> net/ena/hal: optimize Rx ring submission queue > >>> net/ena/hal: rename fields in completion descriptors > >>> net/ena/hal: use correct read once on u8 field > >>> net/ena/hal: add completion descriptor corruption check > >>> net/ena/hal: malformed Tx descriptor error reason > >>> net/ena/hal: phc feature modifications > >>> net/ena/hal: restructure interrupt handling > >>> net/ena/hal: add unlikely to error checks > >>> net/ena/hal: missing admin interrupt reset reason > >>> net/ena/hal: check for existing keep alive notification > >>> net/ena/hal: modify memory barrier comment > >>> net/ena/hal: rework Rx ring submission queue > >>> net/ena/hal: remove operating system type enum > >>> net/ena/hal: handle command abort > >>> net/ena/hal: add support for device reset request > >>> net/ena: cosmetic changes > >>> net/ena/hal: modify customer metrics memory management > >>> net/ena/hal: cosmetic changes > >>> net/ena: update device-preferred size of rings > >>> net/ena: exhaust interrupt callbacks in device close > >>> net/ena: support max large llq depth from the device > >>> net/ena: control path pure polling mode > >>> net/ena: upgrade driver version to 2.9.0 > >>> > >> > >> Hi Shai, > >> > >> I did review only first 10 patches, there are some common patterns to > >> address in the perspective and commit logs. > >> > >> Can you please update whole series according to comments? I will > >> review remaining patches in the new version. > >> > >> Thanks, > >> Ferruh > >> > > [Brandes, Shai] sure, thanks for the comments, we will align accordingly. > > > > Thanks, I can see v4 is out, I will try to review it today for -rc3. [Brandes, Shai] Thank you, I appreciate the effort!
Re: [PATCH v2 1/2] net/virtio: fix vDPA device init advertising control queue
On Wed, Mar 13, 2024 at 1:59 PM Maxime Coquelin
wrote:
>
> If the vDPA device advertises control queue support, but
> the user neither passes "cq=1" as devarg nor requests
> multiple queues, the initialization fails because the
> driver tries to setup the control queue without negotiating
> related feature.
>
> This patch enables the control queue at driver level as
> soon as the device claims to support it, and not only when
> multiple queue pairs are requested. Also, enable the
> control queue event if multiqueue feature has not been
> negotiated and device start time, and disable it at device
> stop time.
>
> Fixes: b277308e8868 ("net/virtio-user: advertise control VQ support with
> vDPA")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Maxime Coquelin
Reviewed-by: David Marchand
--
David Marchand
RE: [PATCH v5 00/21] Improvements and new test cases
> Subject: [PATCH v5 00/21] Improvements and new test cases > > v5: > * Define TEST_SEC_CIPHERTEXT_MAX_LEN based on existing > MBUF_DATAPAYLOAD_SIZE macro. > > v4: > * Set max ciphertext length for data walkthrough tests to 4k. > > v3: > * Set max packet length for data walkthrough tests to 8k. > > v2: > * Rebased. > > Aakash Sasidharan (7): > test/security: enable AES-GCM in combined mode TLS > test/security: add TLS 1.2 data walkthrough test > test/security: add DTLS 1.2 data walkthrough test > test/security: add TLS SG data walkthrough test > test/security: add DTLS 1.2 anti-replay tests > test/security: add more DTLS anti-replay window sz > test/security: add out of place sgl test case for TLS 1.2 > > Akhil Goyal (2): > test/security: add TLS/DTLS 1.2 AES-256-SHA384 vectors > test/crypto: add TLS 1.3 vectors > > Anoob Joseph (1): > test/cryptodev: allow zero packet length buffers > > Vidya Sagar Velumuri (11): > test/security: unit test for TLS packet corruption > test/security: unit test for custom content verification > test/security: unit test to verify zero TLS records > test/security: add unit tests for DTLS-1.2 > test/crypto: update verification of header > test/crypto: update framework to verify tls-1.3 > test/crypto: test to verify hdr corruption in TLS > test/crypto: test to verify custom content type in TLS > test/crypto: test to verify zero len record in TLS > test/crypto: unit tests to verify padding in TLS > test/crypto: unit tests for padding in DTLS-1.2 > > app/test/test_cryptodev.c | 975 -- > app/test/test_cryptodev.h | 20 +- > app/test/test_cryptodev_security_tls_record.c | 203 ++-- > app/test/test_cryptodev_security_tls_record.h | 77 +- > ...yptodev_security_tls_record_test_vectors.h | 405 > app/test/test_security_proto.c| 17 + > app/test/test_security_proto.h| 11 + > 7 files changed, 1530 insertions(+), 178 deletions(-) > Series Acked-by: Akhil Goyal Updated patch title to test/crypto for all patches as these tests are part of crypto suite. Applied to dpdk-next-crypto <>
RE: [EXT] [PATCH v4] examples/ipsec-secgw: fix cryptodev to SA mapping
> > There are use cases where a SA should be able to use different cryptodevs on
> > different lcores, for example there can be cryptodevs with just 1 qp per VF.
> > For this purpose this patch relaxes the check in create lookaside session
> > function.
> > Also add a check to verify that a CQP is available for the current lcore.
> >
> > Fixes: a8ade12123c3 ("examples/ipsec-secgw: create lookaside sessions at
> > init")
> > Cc: sta...@dpdk.org
> > Cc: vfia...@marvell.com
> >
> > Signed-off-by: Radu Nicolau
> > Tested-by: Ting-Kai Ku
> > Acked-by: Ciara Power
> > Acked-by: Kai Ji
>
> Acked-by: Anoob Joseph
>
Applied to dpdk-next-crypto
Thanks.
Re: [PATCH v2] net/bnx2x: fix indentation
On Tue, Mar 12, 2024 at 8:55 PM Stephen Hemminger
wrote:
>
> The DPDK style of indentation uses tabs not spaces.
> This file had mix of both. Convert it.
>
> Signed-off-by: Stephen Hemminger
Applied to dpdk-next-net-mrvl/for-main. Thanks
> ---
> v2 - fix resulting checkpatch warnings
>
> drivers/net/bnx2x/bnx2x_stats.c | 1659 +++
> 1 file changed, 818 insertions(+), 841 deletions(-)
>
> diff --git a/drivers/net/bnx2x/bnx2x_stats.c b/drivers/net/bnx2x/bnx2x_stats.c
> index 69132c7c806e..f15b97116b94 100644
> --- a/drivers/net/bnx2x/bnx2x_stats.c
> +++ b/drivers/net/bnx2x/bnx2x_stats.c
> @@ -29,14 +29,12 @@ bnx2x_get_port_stats_dma_len(struct bnx2x_softc *sc)
> /* 'newest' convention - shmem2 contains the size of the port stats */
> if (SHMEM2_HAS(sc, sizeof_port_stats)) {
> size = SHMEM2_RD(sc, sizeof_port_stats);
> - if (size) {
> + if (size)
> res = size;
> - }
>
> /* prevent newer BC from causing buffer overflow */
> - if (res > sizeof(struct host_port_stats)) {
> + if (res > sizeof(struct host_port_stats))
> res = sizeof(struct host_port_stats);
> - }
> }
>
> /*
> @@ -44,7 +42,7 @@ bnx2x_get_port_stats_dma_len(struct bnx2x_softc *sc)
> * the 'not_used' field
> */
> if (!res) {
> - res = (offsetof(struct host_port_stats, not_used) + 4);
> + res = offsetof(struct host_port_stats, not_used) + 4;
>
> /* if PFC stats are supported by the MFW, DMA them as well */
> if (sc->devinfo.bc_ver >= REQ_BC_VER_4_PFC_STATS_SUPPORTED) {
> @@ -75,9 +73,8 @@ bnx2x_storm_stats_post(struct bnx2x_softc *sc)
> int rc;
>
> if (!sc->stats_pending) {
> - if (sc->stats_pending) {
> + if (sc->stats_pending)
> return;
> - }
>
> sc->fw_stats_req->hdr.drv_stats_counter =
> htole16(sc->stats_counter++);
> @@ -93,9 +90,8 @@ bnx2x_storm_stats_post(struct bnx2x_softc *sc)
> U64_HI(sc->fw_stats_req_mapping),
> U64_LO(sc->fw_stats_req_mapping),
> NONE_CONNECTION_TYPE);
> - if (rc == 0) {
> + if (rc == 0)
> sc->stats_pending = 1;
> - }
> }
> }
>
> @@ -108,15 +104,13 @@ bnx2x_hw_stats_post(struct bnx2x_softc *sc)
> uint32_t opcode;
>
> *stats_comp = DMAE_COMP_VAL;
> - if (CHIP_REV_IS_SLOW(sc)) {
> + if (CHIP_REV_IS_SLOW(sc))
> return;
> - }
>
> /* Update MCP's statistics if possible */
> - if (sc->func_stx) {
> + if (sc->func_stx)
> memcpy(BNX2X_SP(sc, func_stats), &sc->func_stats,
> sizeof(sc->func_stats));
> - }
>
> /* loader */
> if (sc->executer_idx) {
> @@ -230,667 +224,664 @@ bnx2x_stats_pmf_update(struct bnx2x_softc *sc)
> static void
> bnx2x_port_stats_init(struct bnx2x_softc *sc)
> {
> -struct dmae_command *dmae;
> -int port = SC_PORT(sc);
> -uint32_t opcode;
> -int loader_idx = PMF_DMAE_C(sc);
> -uint32_t mac_addr;
> -uint32_t *stats_comp = BNX2X_SP(sc, stats_comp);
> -
> -/* sanity */
> -if (!sc->link_vars.link_up || !sc->port.pmf) {
> - PMD_DRV_LOG(ERR, sc, "BUG!");
> - return;
> -}
> -
> -sc->executer_idx = 0;
> -
> -/* MCP */
> -opcode = bnx2x_dmae_opcode(sc, DMAE_SRC_PCI, DMAE_DST_GRC,
> -TRUE, DMAE_COMP_GRC);
> -
> -if (sc->port.port_stx) {
> - dmae = BNX2X_SP(sc, dmae[sc->executer_idx++]);
> - dmae->opcode = opcode;
> - dmae->src_addr_lo = U64_LO(BNX2X_SP_MAPPING(sc, port_stats));
> - dmae->src_addr_hi = U64_HI(BNX2X_SP_MAPPING(sc, port_stats));
> - dmae->dst_addr_lo = sc->port.port_stx >> 2;
> - dmae->dst_addr_hi = 0;
> - dmae->len = bnx2x_get_port_stats_dma_len(sc);
> - dmae->comp_addr_lo = dmae_reg_go_c[loader_idx] >> 2;
> - dmae->comp_addr_hi = 0;
> - dmae->comp_val = 1;
> -}
> + struct dmae_command *dmae;
> + int port = SC_PORT(sc);
> + uint32_t opcode;
> + int loader_idx = PMF_DMAE_C(sc);
> + uint32_t mac_addr;
> + uint32_t *stats_comp = BNX2X_SP(sc, stats_comp);
>
> -if (sc->func_stx) {
> - dmae = BNX2X_SP(sc, dmae[sc->executer_idx++]);
> - dmae->opcode = opcode;
> - dmae->src_addr_lo = U64_LO(BNX2X_SP_MAPPING(sc, func_stats));
> - dmae->src_addr_hi = U64_HI(BNX2X_SP_MAPPING(sc, func_stats));
> - dmae->dst_addr_lo = (sc->func_stx >> 2);
> - dmae->dst_addr_hi = 0;
> - dmae->len = (sizeof(struct host_func_stats) >> 2);
> - dmae->comp_addr_lo
Re: [PATCH] vhost: fix vring addr update with vDPA
On Thu, Mar 7, 2024 at 11:35 AM David Marchand
wrote:
>
> For vDPA devices, vq are not locked once the device has been configured
> at runtime.
>
> On the other hand, we need to hold the vq lock to evaluate vq->access_ok,
> invalidate vring addresses and translate them.
>
> Move vring address update earlier and, when vDPA is configured, skip parts
> which expect lock to be taken.
>
> Bugzilla ID: 1394
> Fixes: 741dc052eaf9 ("vhost: annotate virtqueue access checks")
>
> Signed-off-by: David Marchand
Applied, thanks.
--
David Marchand
Re: [PATCH] vhost: fix virtqueue access lock check for handlers
On Thu, Mar 7, 2024 at 11:36 AM David Marchand
wrote:
>
> As the vhost library may receive a request for an unsupported request,
> it is necessary to check msg_handler before checking if locking queue
> pairs is requested.
>
> Coverity issue: 415049
> Fixes: 5e8fcc60b59d ("vhost: enhance virtqueue access lock asserts")
>
> Signed-off-by: David Marchand
Applied, thanks.
--
David Marchand
Re: [PATCH v2 0/2] net/virtio: vhost-vdpa fixes
On Wed, Mar 13, 2024 at 1:59 PM Maxime Coquelin wrote: > > While investigating vhost-vdpa initialization issue with mlx5 > vDPA, we found two issues fixed by following patches. > > In this v2, the control queue issue mentioned in v1 is > fixed. It turned out to the control queue being enabled > only if multiqueue was negotiated. It is fixed by enabling > it at device startup, and disabling it at stop time. > > We still have an issue on one of our setup with mlx5, where > the mlx5 device sets VIRTIO_CONFIG_S_FAILED status, it is > currently being investigated. v2 is working fine on my system, what else matters? :-) The current fixes in this series make sense. We may do followup fixes in the next release. > > Changes in v2: > -- > - Fix cvq enablement > - Fix typo in commit message (David) > > > Maxime Coquelin (2): > net/virtio: fix vDPA device init advertising control queue > net/virtio: fix notification area initialization > > .../net/virtio/virtio_user/virtio_user_dev.c | 27 +-- > 1 file changed, 19 insertions(+), 8 deletions(-) > Series applied, thanks. -- David Marchand
RE: [PATCH] compress/nitrox: fix dereference after null check
> Subject: [PATCH] compress/nitrox: fix dereference after null check
>
> In nitrox_check_comp_req() while updating the last byte during FINAL
> flush there is possibility of accessing null mbuf in two rare cases.
> First case is when the application changes the dst mbuf between
> enqueue and dequeue. Second case is when data length reported by
> hardware is greater than the mbuf length. Fix this issue by adding
> mbuf null checks.
>
> Coverity issue: 415046
> Fixes: f008628a6d08 ("compress/nitrox: support stateless request")
> Signed-off-by: Nagadheeraj Rottela
Applied to dpdk-next-crypto
Thanks.
RE: [PATCH] common/qat: fix null dereference in release function
> > Subject: [PATCH] common/qat: fix null dereference in release function
> >
> > This commit fixes NULL dereference in the release function, and three
> additional
> > coverity issues related to NULL check.
> >
> > Coverity issue: 415038
> > Coverity issue: 415050
> > Coverity issue: 415052
> > Coverity issue: 415053
> > Fixes: 477d7d051211 ("common/qat: decouple drivers from common code")
> >
> > Signed-off-by: Arkadiusz Kusztal
> > ---
> > drivers/common/qat/qat_device.c | 14 +++---
> > drivers/compress/qat/qat_comp_pmd.c | 6 --
> > drivers/crypto/qat/qat_asym.c | 5 ++---
> > drivers/crypto/qat/qat_sym.c| 4 ++--
> > 4 files changed, 15 insertions(+), 14 deletions(-)
> >
>
> Acked-by: Ciara Power
Applied to dpdk-next-crypto
Thanks.
RE: [PATCH] crypto/qat: fix ccm null aad pointer segfault
> > Subject: [PATCH] crypto/qat: fix ccm null aad pointer segfault
> >
> > This commit fixes a segfault, that occurs when NULL pointer is being set to
> > the
> > aad pointer field.
> >
> > Fixes: a815a04cea05 ("crypto/qat: support symmetric build op request")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Arkadiusz Kusztal
> > ---
> > drivers/crypto/qat/dev/qat_crypto_pmd_gens.h | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
>
>
> The CI fail looks to be unrelated to this patch:
>
> 70/112 DPDK:fast-tests / pcapng_autotestFAIL
> 6.39s (exit status
> 255 or signal 127 SIGinvalid)
>
> Ok: 101
> Expected Fail: 0
> Fail: 1
>
>
> Acked-by: Ciara Power
Applied to dpdk-next-crypto
Thanks.
[PATCH] app/crypto-perf-test: fix unset crc algorithm
Because net crc api is not thread-safe, setting crc algorithm
by the application will prevent race condition in the calc function.
Race condition still may occur when any of the threads will call this
function again. Function is called with the highest possible SIMD
extension, which is AVX512, but if this is not found, CRC API will
pick the other highest possible extension, or scalar if no SIMD
available.
Fixes: f8be1786b1b8 ("app/crypto-perf: introduce performance test application")
Signed-off-by: Arkadiusz Kusztal
---
app/test-crypto-perf/main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
index 40c0b4b54f..58496797d7 100644
--- a/app/test-crypto-perf/main.c
+++ b/app/test-crypto-perf/main.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#ifdef RTE_CRYPTO_SCHEDULER
#include
#endif
@@ -599,6 +600,8 @@ main(int argc, char **argv)
goto err;
}
+ rte_net_crc_set_alg(RTE_NET_CRC_AVX512);
+
ret = cperf_verify_devices_capabilities(&opts, enabled_cdevs,
nb_cryptodevs);
if (ret) {
--
2.13.6
Re: [PATCH 2/2] net/nfp: fix data endianness problem
On 3/11/2024 2:49 AM, Chaoyong He wrote:
> From: Shihong Wang
>
> The algorithm key of the security framework is stored in the u8
> array according to big-endian, and the driver algorithm key is
> CPU-endian of u32, so it maybe need to convert the endianness order
> to ensure that the value assigned to the driver is CPU-endian.
>
> This patch removes the operation of converting IPsec Tx metadata
> to big-endian to ensure that IPsec Tx metadata is CPU-endian.
>
> Fixes: 547137405be7 ("net/nfp: initialize IPsec related content")
> Fixes: 3d21da66c06b ("net/nfp: create security session")
> Fixes: 310a1780581e ("net/nfp: support IPsec Rx and Tx offload")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Shihong Wang
> Reviewed-by: Chaoyong He
>
Applied to dpdk-next-net/main, thanks.
(not the set, just this patch)
[PATCH v1 1/1] iavf: document limitation on MTU
When configuring a port, the configured MTU will not include VLAN tag size, but the physical function driver will add it automatically if the port has VLAN filtering configured, which may result in seemingly valid MTU to be rejected by the PF. Document the limitation. Signed-off-by: Anatoly Burakov --- doc/guides/nics/intel_vf.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst index ce2bd88cbe..78fd25bad4 100644 --- a/doc/guides/nics/intel_vf.rst +++ b/doc/guides/nics/intel_vf.rst @@ -730,3 +730,13 @@ and has this issue. Set the parameter `--force-max-simd-bitwidth` as 64/128/256 to avoid selecting AVX-512 Tx path. + +ice: VLAN tag length not included in MTU + + +When configuring MTU for a VF, MTU must not include VLAN tag length. In +practice, when kernel driver configures VLAN filtering for a VF, the VLAN +header tag length will be automatically added to MTU when configuring queues. +As a consequence, when attempting to configure a VF port with MTU that, together +with a VLAN tag header, exceeds maximum supported MTU, port configuration will +fail if kernel driver has configured VLAN filtering on that VF. -- 2.43.0
[DPDK/ethdev Bug 1400] net/ena: Failed to initialize ENA device
https://bugs.dpdk.org/show_bug.cgi?id=1400 Bug ID: 1400 Summary: net/ena: Failed to initialize ENA device Product: DPDK Version: 23.11 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: major Priority: Normal Component: ethdev Assignee: dev@dpdk.org Reporter: madhuker.myt...@oracle.com Target Milestone: --- Hi, With DPDK-23.11 on AWS cloud with ENA network devices, DPDK initialization failed with 4GB memory for 2 ports. While DPDK rte_eal_init() call on ENA devices, in the ena_com_allocate_customer_metrics_buffer() DPDK memory zone with "ena_p0_mz0" is taking lots of memory in GB's just for one zone, why ? As part this call "na_com_allocate_customer_metrics_buffer()" API memory size was passed as '0' (customer_metrics->buffer_len = 0) to this "ena_mem_alloc_coherent()" API, which call's --> rte_memzone_reserve_aligned() with RTE_MEMZONE_IOVA_CONTIG option. Thus, when memory size is '0' to allocate, this memzone allocated maximum available contiguous memory in GB's. So, for two DPDK ENA ports, this two zones takes around 2GB Or more of memory in DPDK-23.11. For other Mbuf pool's could not allocate memory, as memory exhausted. Why the "customer_metrics->buffer_len = 0" size is '0' passing to this ena_mem_alloc_coherent() ? Regards, Madhuker. -- You are receiving this mail because: You are the assignee for the bug.
Re: [PATCH v4 13/31] net/ena/base: malformed Tx descriptor error reason
On 3/12/2024 6:06 PM, shaib...@amazon.com wrote: > From: Shai Brandes > > Adding ENA_REGS_RESET_TX_DESCRIPTOR_MALFORMED to identify > cases where the returned TX completion descriptors are > corrupted. > "error reason"? Or "reset reason"? Updating title as: "net/ena/base: add malformed Tx descriptor reset reason"
Re: [PATCH v4 15/31] net/ena/base: restructure interrupt handling
On 3/12/2024 6:07 PM, shaib...@amazon.com wrote:
> From: Shai Brandes
>
> When invoking an admin command, in interrupt mode, if the interrupt
> is received after timeout and also after the calling function finished
> running, the response will be written into a memory that is no longer
> valid.
>
> Signed-off-by: Shai Brandes
> Reviewed-by: Amit Bernstein
>
Fixes: 99ecfbf845b3 ("ena: import communication layer")
Cc: sta...@dpdk.org
Re: [PATCH v4 05/31] net/ena: fix fast mbuf free
On 3/12/2024 6:06 PM, shaib...@amazon.com wrote:
> From: Shai Brandes
>
> In case the application enables fast mbuf release optimization,
> the driver releases 256 TX mbufs in bulk upon reaching the
> TX free threshold.
> The existing implementation utilizes rte_mempool_put_bulk for bulk
> freeing TXs, which exclusively supports direct mbufs.
> In case the application transmits indirect bufs, the driver must
> also decrement the mbuf reference count and unlink the mbuf segment.
> For such case, the driver should employ rte_pktmbuf_free_bulk.
>
> Fixes: c339f53823f3 ("net/ena: support fast mbuf free")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Shai Brandes
> Reviewed-by: Amit Bernstein
>
Dropped release notes update while merging, as user impact of the change
is low, or no more than fixing a defect.
Re: [PATCH v4 24/31] net/ena: cosmetic changes
On 3/12/2024 6:07 PM, shaib...@amazon.com wrote: > From: Shai Brandes > > This patch makes several changes to improve > the style and readability of the code. > > Signed-off-by: Shai Brandes > Reviewed-by: Amit Bernstein Updated patch title while merging, as: net/ena: improve style and readability
Re: [PATCH v4 07/31] net/ena/base: limit exponential backoff exp
On 3/12/2024 6:06 PM, shaib...@amazon.com wrote:
> From: Shai Brandes
>
> Limit the value of the exponent used for this backoff
> at (1<<16) to prevent it from reaching to an excessive
> value (1<<32) or potentially even overflowing.
> In addition, for uniformity and readability purposes,
> the min/max parameter in the calls of ENA_MIN32 and
> ENA_MAX32 macros was changed to be first.
>
> Signed-off-by: Shai Brandes
> Reviewed-by: Amit Bernstein
>
Fixes: 0c84e04824db ("net/ena/base: make delay exponential in polling
functions")
Cc: sta...@dpdk.org
Re: [PATCH v4 08/31] net/ena/base: add a new csum offload bit
On 3/12/2024 6:06 PM, shaib...@amazon.com wrote: > From: Shai Brandes > > Add a new driver supported feature bit for TX IPv6 checksum offload. > > Signed-off-by: Shai Brandes > Reviewed-by: Amit Bernstein > updated patch title as: net/ena/base: add IPv6 checksum offload feature
Re: [PATCH v4 03/31] net/ena: update imissed stat with Rx overruns
On 3/12/2024 6:06 PM, shaib...@amazon.com wrote: > From: Shai Brandes > > Depending on its acceleration support, the device updates > a different statistic when an ingress packet is dropped > because no buffers are available to hold it. > - In AWS instance types from later generations > 'rx_overruns' is updated. > - Otherwise, in legacy instance types, > 'rx_dropped_cnt' is updated. > > That is, there is no need to report rx_overruns separately > as an xstat and the driver can simply sum up the two > self-contained counters as the 'imissed' statistic. > > Signed-off-by: Shai Brandes > Reviewed-by: Amit Bernstein > Formatted release notes update while merging.
Re: [PATCH v4 00/31] net/ena: v2.9.0 driver release
On 3/12/2024 6:06 PM, shaib...@amazon.com wrote: > From: Shai Brandes > > Hi all, the ena v2.9.0 release introduces: > 1. HAL upgrade: >- renamed the 'base' folder to be 'hal' >- separated the HAL patches instead of a bulk update. > 2. Restructured ena stats and metrics. > 3. Restructured the LLQ configuration: >- configurable via devarg. >- support device recommendation. >- restructure the logic in driver. > 4. Added support for the admin queue to work only in poll-mode >- configurable via devarg. >- allows to bind ports to uio_pci_generic kernel driver. > 5. Reworked the device close to exhaust interrupt callbacks and alarms. > 6. Fixed a bug in fast mbuf free. > Best regards. > > --- > v4: > * deleted patch "[06/33] net/ena: rename base folder to hal". > * reworked patch "[07/33] net/ena: restructure the llq policy setting > process" so the code maintain backward compatibility in user devargs. > * modified the title of "[09/33] net/ena/hal: add a new csum offload > bit" > * deleted patch "[10/33] net/ena/hal: added a bus parameter to ena > memcpy macro" > * reworked the commit message of "[08/33] net/ena/hal: exponential > backoff exp limit" to better describe the reasoning for the change. > * reworked the wording of "[28/33] net/ena/hal: cosmetic changes" > * fixed "[26/33] net/ena: cosmetic changes" which broke patch by patch > build. > > v3: > * Fixed missing admin queue missing intialization in patch 0032 > > v2: > * Fixed minor spelling issues from checkpatch > > > Shai Brandes (31): > net/ena: rework the metrics multi-process functions > net/ena: report new supported link speed capabilities > net/ena: update imissed stat with Rx overruns > net/ena: sub-optimal configuration notifications support > net/ena: fix fast mbuf free > net/ena: restructure the llq policy setting process > net/ena/base: limit exponential backoff exp > net/ena/base: add a new csum offload bit > net/ena/base: optimize Rx ring submission queue > net/ena/base: rename fields in completion descriptors > net/ena/base: use correct read once on u8 field > net/ena/base: add completion descriptor corruption check > net/ena/base: malformed Tx descriptor error reason > net/ena/base: phc feature modifications > net/ena/base: restructure interrupt handling > net/ena/base: add unlikely to error checks > net/ena/base: missing admin interrupt reset reason > net/ena/base: check for existing keep alive notification > net/ena/base: modify memory barrier comment > net/ena/base: rework Rx ring submission queue > net/ena/base: remove operating system type enum > net/ena/base: handle command abort > net/ena/base: add support for device reset request > net/ena: cosmetic changes > net/ena/base: modify customer metrics memory management > net/ena/base: modify logs to use unsigned format specifier > net/ena: update device-preferred size of rings > net/ena: exhaust interrupt callbacks in device close > net/ena: support max large llq depth from the device > net/ena: control path pure polling mode > net/ena: upgrade driver version to 2.9.0 > Series applied to dpdk-next-net/main, thanks.
RE: [PATCH v2] app/test: fix rsa tests in qat suite
> -Original Message-
> From: Kusztal, ArkadiuszX
> Sent: Wednesday, March 13, 2024 9:14 AM
> To: dev@dpdk.org
> Cc: gak...@marvell.com; Power, Ciara ; Kusztal,
> ArkadiuszX ; sta...@dpdk.org
> Subject: [PATCH v2] app/test: fix rsa tests in qat suite
>
> This commit fixes incorrectly set keys in the QAT testsuite for the RSA
> algorithm.
>
> Fixes: 9b5465867fb8 ("test/crypto: add RSA none padding cases")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Arkadiusz Kusztal
> ---
> v2:
> - removed camel case
>
Acked-by: Ciara Power
Re: [PATCH v2 1/5] app/test-mp: add multiprocess test
On 3/7/2024 8:01 AM, Artemy Kovalyov wrote: This commit adds a test scenario that initiates multiple processes concurrently. These processes attach to the same shared heap, with an automatic detection mechanism to identify the primary process. Signed-off-by: Artemy Kovalyov --- Acked-by: Anatoly Burakov -- Thanks, Anatoly
Re: [PATCH v2 2/5] eal: fix multiprocess hotplug race
On 3/7/2024 8:01 AM, Artemy Kovalyov wrote:
There exists a time gap between the creation of the multiprocess channel
and the registration of request action handlers. Within this window, a
secondary process that receives an eal_dev_mp_request broadcast
notification might respond with ENOTSUP. This, in turn, causes the
rte_dev_probe() operation to fail in another secondary process.
To avoid this, disregarding ENOTSUP responses to attach notifications.
Fixes: 244d5130719c ("eal: enable hotplug on multi-process")
Cc: sta...@dpdk.org
Signed-off-by: Artemy Kovalyov
---
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [PATCH v2 3/5] ipc: fix mp channel closure to prevent message loss
On 3/7/2024 8:01 AM, Artemy Kovalyov wrote:
This commit addresses an issue related to the cleanup of the
multiprocess channel. Previously, when closing the channel, there was a
risk of losing trailing messages. This issue was particularly noticeable
when broadcast message from primary to secondary processes was sent
while a secondary process was closing it's mp channel. In this fix, we
delete mp socket file before stopping mp receive thread.
Fixes: e7885281ded1 ("ipc: stop mp control thread on cleanup")
Cc: sta...@dpdk.org
Signed-off-by: Artemy Kovalyov
---
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [PATCH v2 4/5] eal: fix first time primary autodetect
On 3/7/2024 8:01 AM, Artemy Kovalyov wrote:
If the configuration file is absent, the autodetection function should
generate and secure it. Otherwise, multiple simultaneous openings could
erroneously identify themselves as primary instances.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Artemy Kovalyov
---
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [PATCH v2 5/5] eal: fix memzone fbarray cleanup
On 3/7/2024 8:01 AM, Artemy Kovalyov wrote:
The initialization of the Memzone file-backed array ensures its
uniqueness by employing an exclusive lock. This is crucial because only
one primary process can exist per specific shm_id, which is further
protected by the exclusive EAL runtime configuration lock.
I think you meant to say "prefix", not "shm_id".
However, during the process closure, the exclusive lock on both the
fbarray and the configuration is not explicitly released. The
responsibility of releasing these locks is left to the generic quit
procedure. This can lead to a potential race condition when the
configuration is released before the fbarray.
To address this, we propose explicitly closing the memzone fbarray. This
ensures proper order of operations during process closure and prevents
any potential race conditions arising from the mismatched lock release
timings.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Artemy Kovalyov
---
I would suggest having a different Fixes: ID, because fbarrays were only
added in 18.05 when we added dynamic memory support. I propose using
this commit ID instead:
49df3db84883 ("memzone: replace memzone array with fbarray")
This is the first commit where memzones used fbarrays.
+void
+rte_eal_memzone_cleanup(void)
+{
+ struct rte_mem_config *mcfg;
+
+ mcfg = rte_eal_get_configuration()->mem_config;
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ rte_fbarray_destroy(&mcfg->memzones);
+ }
Nitpick: extraneous brackets, this is a one liner so they're not needed.
With changes above,
Acked-by: Anatoly Burakov
--
Thanks,
Anatoly
Re: [PATCH 2/4] app/testpmd: fix burst option parsing
On Wed, 13 Mar 2024 12:09:01 + Ferruh Yigit wrote: > > Which is really close to the existing log message. > > > > if (rec_nb_pkts == 0) > > rte_exit(EXIT_FAILURE, > > "PMD does not > > recommend a burst size. " > > "Provided > > value must be between " > > "1 and %d\n", > > MAX_PKT_BURST); > > > > I am unconvinced, but if you think strongly for this, I won't debate more. > > > > > > Yes it is close, and I don't have strong opinion on this, > > But existing message says "PMD does not recommend a burst size.", I > think this may be confusing for user. > > If I see that message I would either debug relevant driver code or > communicate with driver maintainer for it. But that is not the case, > just failed to get recommended burst size and problem is in testpmd. > > Anyway, my concern is existing message can be misleading for user. Maybe shorter: "PMD does not provide required burst size\n"
Re: [PATCH 4/4] app/testpmd: enhance getopt_long usage
On Tue, Mar 12, 2024 at 6:03 PM Ferruh Yigit wrote:
>
> On 3/8/2024 2:48 PM, David Marchand wrote:
> > This is a cleanup similar to previous ones in EAL and examples.
> > Instead of using strcmp for every long options while getopt_long already
> > did such parsing, rely on getopt_long return value.
> >
> > Note for reviewers: this patch is best reviewed once applied locally and
> > displayed with git show -w.
> >
>
> Thanks for the cleanup, it was needed.
Thanks for the in depth review.
>
>
> > Signed-off-by: David Marchand
> > ---
> > app/test-pmd/parameters.c | 1928 +
> > 1 file changed, 1084 insertions(+), 844 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 271f0c995a..58f43cb5a8 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -40,6 +40,346 @@
> >
> > #include "testpmd.h"
> >
> > +enum {
> > + /* long options mapped to a short option */
> > +#define TESTPMD_OPT_AUTO_START "auto-start"
> > + TESTPMD_OPT_AUTO_START_NUM = 'a',
> > +#define TESTPMD_OPT_HELP "help"
> > + TESTPMD_OPT_HELP_NUM = 'h',
> > +#define TESTPMD_OPT_INTERACTIVE "interactive"
> > + TESTPMD_OPT_INTERACTIVE_NUM = 'i',
> > +
> > + /* first long only option value must be >= 256 */
> > + TESTPMD_OPT_LONG_MIN_NUM = 256,
> >
>
> This is to be able to use all short options, not sure if this requires a
> comment.
Yes, those comments do not help.
>
> <...>
>
> > +};
> > +
> > +static struct option long_options[] = {
> >
>
> Can it be constant?
Yes.
>
> > + { TESTPMD_OPT_AUTO_START, 0, NULL, TESTPMD_OPT_AUTO_START_NUM },
> >
>
> In original version "auto-start" long version is enabled only if
> 'RTE_LIB_CMDLINE' enabled, not sure why, but not for short version, I
> think there is a confusion.
I guess one reason why the --auto-start option is under
RTE_LIB_CMDLINE is because forwarding is always started when in non
interactive mode.
https://git.dpdk.org/dpdk/tree/app/test-pmd/testpmd.c#n4722
-a is available without RTE_LIB_CMDLINE, but it was probably not intentional.
In both cases, -a / --auto-start are a nop when testpmd is compiled
without RTE_LIB_CMDLINE.
So moving it out of the RTE_LIB_CMDLINE is not an issue.
>
> OK to remove RTE_LIB_CMDLINE requirement for 'auto-start' but perhaps
> better to have it in separate patch, what do you think?
Yes, it is worth a separate patch but see below my next comment about
RTE_LIB_CMDLINE check.
>
>
> > + { TESTPMD_OPT_HELP, 0, NULL, TESTPMD_OPT_HELP_NUM },
> > +#ifdef RTE_LIB_CMDLINE
> > + { TESTPMD_OPT_INTERACTIVE, 0, NULL, TESTPMD_OPT_INTERACTIVE_NUM },
> > + { TESTPMD_OPT_CMDLINE_FILE, 1, NULL, TESTPMD_OPT_CMDLINE_FILE},
> >
>
> What about using 'no_argument' & 'required_argument' instead of '0' &
> '1', but I guess it will make lines too big,
Line length is the reason why I used the 0/1 but your suggestion looks
good to me.
> as flag will be always NULL in this approach, what about defining macros
> for arg and no_arg, like:
> ```
> #define ARG(name) (name), required_argument, NULL, name##_NUM
> #define NOARG(name) (name), no_argument, NULL, name##_NUM
> #define OPTARG(name) (name), optional_argument, NULL, name##_NUM
>
> { NOARG(TESTPMD_OPT_INTERACTIVE) },
> { ARG(TESTPMD_OPT_CMDLINE_FILE) },
> ```
It lgtm.
>
> > + { TESTPMD_OPT_ETH_PEERS_CONFIGFILE, 1, NULL,
> > TESTPMD_OPT_ETH_PEERS_CONFIGFILE_NUM },
> > + { TESTPMD_OPT_ETH_PEER, 1, NULL, TESTPMD_OPT_ETH_PEER_NUM },
> >
>
> These long options are within "#ifdef RTE_LIB_CMDLINE" block, I don't
> know why, I guess above two can work without cmdline library.
Looking again at the options list, we have a nice mess in there... I
had not realised.
I think the only options that are conditional to RTE_LIB_CMDLINE should be:
-i, -a, --cmdline-file.
As I wrote above, leaving the -a/--auto-start option parsing out of
RTE_LIB_CMDLINE is not an issue.
So it would only leave -i/--interactive and --cmdline-file under
RTE_LIB_CMDLINE.
The rest can be moved out of RTE_LIB_CMDLINE.
And reading your next comment about RTE_LIB_CMDLINE, I think we are in sync.
>
> <...>
>
> > -
> > argvopt = argv;
> >
>
> Why 'argvopt' is required, why not use 'argv' directly?
I'll have to double check.
>
> <...>
>
> > #ifdef RTE_LIB_CMDLINE
> > ...
> > + case TESTPMD_OPT_ETH_PEER_NUM: {
> > + char *port_end;
> >
> > - if (rte_ether_unformat_addr(port_end,
> > - &peer_eth_addrs[n]) < 0)
> > - rte_exit(EXIT_FAILURE,
> > - "Invalid ethernet address:
> > %s\n",
> > - port_end);
> > - nb_peer_eth_addrs++;
> > - }
> > + errno = 0;
> > + n = strtoul(optarg, &port_end, 1
[PATCH] drivers/net/gve: add IPv4 checksum offloading capability
Gvnic's DQO format allows offloading IPv4 checksum.
Made changes to Tx and Rx path to translate DPDK flags
to descriptor for offloading (and vice-versa).
Add ptype adminq support to only add this flags for
supported L3/L4 packet-types.
Signed-off-by: Rushil Gupta
Reviewed-by: Joshua Washington
---
drivers/net/gve/gve_ethdev.c | 34 +++---
drivers/net/gve/gve_ethdev.h | 5 +
drivers/net/gve/gve_rx_dqo.c | 38 --
drivers/net/gve/gve_tx_dqo.c | 2 +-
4 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3b8ec58..475745b 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -434,8 +434,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_SCTP_CKSUM |
RTE_ETH_TX_OFFLOAD_TCP_TSO;
- if (priv->queue_format == GVE_DQO_RDA_FORMAT)
- dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
+ if (!gve_is_gqi(priv)) {
+ dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+ dev_info->rx_offload_capa |=
+ RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |
+ RTE_ETH_RX_OFFLOAD_UDP_CKSUM|
+ RTE_ETH_RX_OFFLOAD_TCP_CKSUM|
+ RTE_ETH_RX_OFFLOAD_TCP_LRO;
+ }
dev_info->default_rxconf = (struct rte_eth_rxconf) {
.rx_free_thresh = GVE_DEFAULT_RX_FREE_THRESH,
@@ -938,6 +944,11 @@ gve_teardown_device_resources(struct gve_priv *priv)
if (err)
PMD_DRV_LOG(ERR, "Could not deconfigure device
resources: err=%d", err);
}
+
+ if (!gve_is_gqi(priv)) {
+ rte_free(priv->ptype_lut_dqo);
+ priv->ptype_lut_dqo = NULL;
+ }
gve_free_counter_array(priv);
gve_free_irq_db(priv);
gve_clear_device_resources_ok(priv);
@@ -997,8 +1008,25 @@ gve_setup_device_resources(struct gve_priv *priv)
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d",
err);
goto free_irq_dbs;
}
- return 0;
+ if (!gve_is_gqi(priv)) {
+ priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
+ sizeof(struct gve_ptype_lut), 0);
+ if (priv->ptype_lut_dqo == NULL) {
+ PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
+ err = -ENOMEM;
+ goto free_irq_dbs;
+ }
+ err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+ if (unlikely(err)) {
+ PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d",
err);
+ goto free_ptype_lut;
+ }
+ }
+ return 0;
+free_ptype_lut:
+ rte_free(priv->ptype_lut_dqo);
+ priv->ptype_lut_dqo = NULL;
free_irq_dbs:
gve_free_irq_db(priv);
free_cnt_array:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d713657..9b19fc5 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -36,6 +36,10 @@
RTE_MBUF_F_TX_L4_MASK |\
RTE_MBUF_F_TX_TCP_SEG)
+#define GVE_TX_CKSUM_OFFLOAD_MASK_DQO (\
+ GVE_TX_CKSUM_OFFLOAD_MASK | \
+ RTE_MBUF_F_TX_IP_CKSUM)
+
#define GVE_RTE_RSS_OFFLOAD_ALL ( \
RTE_ETH_RSS_IPV4 | \
RTE_ETH_RSS_NONFRAG_IPV4_TCP | \
@@ -295,6 +299,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
struct gve_rss_config rss_config;
+ struct gve_ptype_lut *ptype_lut_dqo;
};
static inline bool
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 7c7a8c4..1c37c54 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -75,6 +75,40 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
rxq->bufq_tail = next_avail;
}
+static inline uint16_t
+gve_parse_csum_ol_flags(volatile struct gve_rx_compl_desc_dqo *rx_desc,
+ struct gve_priv *priv) {
+ uint64_t ol_flags = 0;
+ struct gve_ptype ptype =
+ priv->ptype_lut_dqo->ptypes[rx_desc->packet_type];
+
+ if (!rx_desc->l3_l4_processed)
+ return ol_flags;
+
+ if (ptype.l3_type == GVE_L3_TYPE_IPV4) {
+ if (rx_desc->csum_ip_err)
+ ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
+ else
+ ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+ }
+
+ if (rx_desc->csum_l4_err) {
+ ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+ return ol_flags;
+ }
+ switch (ptype.l4_type) {
+ case GVE_L4_TYPE_TCP:
+
[PATCH] drivers/net/gve: add IPv4 checksum offloading capability
Gvnic's DQO format allows offloading IPv4 checksum.
Made changes to Tx and Rx path to translate DPDK flags
to descriptor for offloading (and vice-versa).
Add ptype adminq support to only add this flags for
supported L3/L4 packet-types.
Signed-off-by: Rushil Gupta
Reviewed-by: Joshua Washington
---
drivers/net/gve/gve_ethdev.c | 34 +++---
drivers/net/gve/gve_ethdev.h | 5 +
drivers/net/gve/gve_rx_dqo.c | 38 --
drivers/net/gve/gve_tx_dqo.c | 2 +-
4 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3b8ec58..475745b 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -434,8 +434,14 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_SCTP_CKSUM |
RTE_ETH_TX_OFFLOAD_TCP_TSO;
- if (priv->queue_format == GVE_DQO_RDA_FORMAT)
- dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
+ if (!gve_is_gqi(priv)) {
+ dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+ dev_info->rx_offload_capa |=
+ RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |
+ RTE_ETH_RX_OFFLOAD_UDP_CKSUM|
+ RTE_ETH_RX_OFFLOAD_TCP_CKSUM|
+ RTE_ETH_RX_OFFLOAD_TCP_LRO;
+ }
dev_info->default_rxconf = (struct rte_eth_rxconf) {
.rx_free_thresh = GVE_DEFAULT_RX_FREE_THRESH,
@@ -938,6 +944,11 @@ gve_teardown_device_resources(struct gve_priv *priv)
if (err)
PMD_DRV_LOG(ERR, "Could not deconfigure device
resources: err=%d", err);
}
+
+ if (!gve_is_gqi(priv)) {
+ rte_free(priv->ptype_lut_dqo);
+ priv->ptype_lut_dqo = NULL;
+ }
gve_free_counter_array(priv);
gve_free_irq_db(priv);
gve_clear_device_resources_ok(priv);
@@ -997,8 +1008,25 @@ gve_setup_device_resources(struct gve_priv *priv)
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d",
err);
goto free_irq_dbs;
}
- return 0;
+ if (!gve_is_gqi(priv)) {
+ priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
+ sizeof(struct gve_ptype_lut), 0);
+ if (priv->ptype_lut_dqo == NULL) {
+ PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
+ err = -ENOMEM;
+ goto free_irq_dbs;
+ }
+ err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+ if (unlikely(err)) {
+ PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d",
err);
+ goto free_ptype_lut;
+ }
+ }
+ return 0;
+free_ptype_lut:
+ rte_free(priv->ptype_lut_dqo);
+ priv->ptype_lut_dqo = NULL;
free_irq_dbs:
gve_free_irq_db(priv);
free_cnt_array:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d713657..9b19fc5 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -36,6 +36,10 @@
RTE_MBUF_F_TX_L4_MASK |\
RTE_MBUF_F_TX_TCP_SEG)
+#define GVE_TX_CKSUM_OFFLOAD_MASK_DQO (\
+ GVE_TX_CKSUM_OFFLOAD_MASK | \
+ RTE_MBUF_F_TX_IP_CKSUM)
+
#define GVE_RTE_RSS_OFFLOAD_ALL ( \
RTE_ETH_RSS_IPV4 | \
RTE_ETH_RSS_NONFRAG_IPV4_TCP | \
@@ -295,6 +299,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
struct gve_rss_config rss_config;
+ struct gve_ptype_lut *ptype_lut_dqo;
};
static inline bool
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 7c7a8c4..1c37c54 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -75,6 +75,40 @@ gve_rx_refill_dqo(struct gve_rx_queue *rxq)
rxq->bufq_tail = next_avail;
}
+static inline uint16_t
+gve_parse_csum_ol_flags(volatile struct gve_rx_compl_desc_dqo *rx_desc,
+ struct gve_priv *priv) {
+ uint64_t ol_flags = 0;
+ struct gve_ptype ptype =
+ priv->ptype_lut_dqo->ptypes[rx_desc->packet_type];
+
+ if (!rx_desc->l3_l4_processed)
+ return ol_flags;
+
+ if (ptype.l3_type == GVE_L3_TYPE_IPV4) {
+ if (rx_desc->csum_ip_err)
+ ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
+ else
+ ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+ }
+
+ if (rx_desc->csum_l4_err) {
+ ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+ return ol_flags;
+ }
+ switch (ptype.l4_type) {
+ case GVE_L4_TYPE_TCP:
+
RE: [PATCH v4 00/31] net/ena: v2.9.0 driver release
Thank you Ferruh, I know it was last minute patchset. We appreciate your effort! All the best, Shai בתאריך 13 במרץ 2024 18:00, Ferruh Yigit כתב: CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. On 3/12/2024 6:06 PM, shaib...@amazon.com wrote: > From: Shai Brandes > > Hi all, the ena v2.9.0 release introduces: > 1. HAL upgrade: >- renamed the 'base' folder to be 'hal' >- separated the HAL patches instead of a bulk update. > 2. Restructured ena stats and metrics. > 3. Restructured the LLQ configuration: >- configurable via devarg. >- support device recommendation. >- restructure the logic in driver. > 4. Added support for the admin queue to work only in poll-mode >- configurable via devarg. >- allows to bind ports to uio_pci_generic kernel driver. > 5. Reworked the device close to exhaust interrupt callbacks and alarms. > 6. Fixed a bug in fast mbuf free. > Best regards. > > --- > v4: > * deleted patch "[06/33] net/ena: rename base folder to hal". > * reworked patch "[07/33] net/ena: restructure the llq policy setting > process" so the code maintain backward compatibility in user devargs. > * modified the title of "[09/33] net/ena/hal: add a new csum offload > bit" > * deleted patch "[10/33] net/ena/hal: added a bus parameter to ena > memcpy macro" > * reworked the commit message of "[08/33] net/ena/hal: exponential > backoff exp limit" to better describe the reasoning for the change. > * reworked the wording of "[28/33] net/ena/hal: cosmetic changes" > * fixed "[26/33] net/ena: cosmetic changes" which broke patch by patch > build. > > v3: > * Fixed missing admin queue missing intialization in patch 0032 > > v2: > * Fixed minor spelling issues from checkpatch > > > Shai Brandes (31): > net/ena: rework the metrics multi-process functions > net/ena: report new supported link speed capabilities > net/ena: update imissed stat with Rx overruns > net/ena: sub-optimal configuration notifications support > net/ena: fix fast mbuf free > net/ena: restructure the llq policy setting process > net/ena/base: limit exponential backoff exp > net/ena/base: add a new csum offload bit > net/ena/base: optimize Rx ring submission queue > net/ena/base: rename fields in completion descriptors > net/ena/base: use correct read once on u8 field > net/ena/base: add completion descriptor corruption check > net/ena/base: malformed Tx descriptor error reason > net/ena/base: phc feature modifications > net/ena/base: restructure interrupt handling > net/ena/base: add unlikely to error checks > net/ena/base: missing admin interrupt reset reason > net/ena/base: check for existing keep alive notification > net/ena/base: modify memory barrier comment > net/ena/base: rework Rx ring submission queue > net/ena/base: remove operating system type enum > net/ena/base: handle command abort > net/ena/base: add support for device reset request > net/ena: cosmetic changes > net/ena/base: modify customer metrics memory management > net/ena/base: modify logs to use unsigned format specifier > net/ena: update device-preferred size of rings > net/ena: exhaust interrupt callbacks in device close > net/ena: support max large llq depth from the device > net/ena: control path pure polling mode > net/ena: upgrade driver version to 2.9.0 > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH 4/4] app/testpmd: enhance getopt_long usage
On Wed, Mar 13, 2024 at 5:51 PM David Marchand
wrote:
> >
> > > + { TESTPMD_OPT_ETH_PEERS_CONFIGFILE, 1, NULL,
> > > TESTPMD_OPT_ETH_PEERS_CONFIGFILE_NUM },
> > > + { TESTPMD_OPT_ETH_PEER, 1, NULL, TESTPMD_OPT_ETH_PEER_NUM },
> > >
> >
> > These long options are within "#ifdef RTE_LIB_CMDLINE" block, I don't
> > know why, I guess above two can work without cmdline library.
>
> Looking again at the options list, we have a nice mess in there... I
> had not realised.
>
> I think the only options that are conditional to RTE_LIB_CMDLINE should be:
> -i, -a, --cmdline-file.
>
> As I wrote above, leaving the -a/--auto-start option parsing out of
> RTE_LIB_CMDLINE is not an issue.
> So it would only leave -i/--interactive and --cmdline-file under
> RTE_LIB_CMDLINE.
> The rest can be moved out of RTE_LIB_CMDLINE.
>
> And reading your next comment about RTE_LIB_CMDLINE, I think we are in sync.
Actually, the solution is even simpler... it is not possible to
disable the cmdline library.
testpmd won't compile without it (missing #ifdef RTE_LIB_CMDLINE in
different places).
I'll drop all the #ifdef RTE_LIB_CMDLINE checks in a dedicated patch.
--
David Marchand
RE: [PATCH v3 1/1] net/mana: add vlan tagging support
> Subject: [PATCH v3 1/1] net/mana: add vlan tagging support
>
> For tx path, use LONG_PACKET_FORMAT if vlan tag is present. For rx, extract
> vlan
> id from oob, put into mbuf and set the vlan flags in mbuf.
>
> Signed-off-by: Wei Hu
Acked-by: Long Li
> ---
>
> v3:
> - Adjust the pkt_idx position in the code so it will be executed even when
> adding
> vlan header fails.
>
> v2:
> - Use existing vlan tag processing macros.
> - Add vlan header back if vlan_strip flag is not set on the receiving path.
>
> drivers/net/mana/mana.c | 3 +++
> drivers/net/mana/mana.h | 4
> drivers/net/mana/rx.c | 22 ++
> drivers/net/mana/tx.c | 21 ++---
> 4 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c index
> 2df2461d2f..68c625258e 100644
> --- a/drivers/net/mana/mana.c
> +++ b/drivers/net/mana/mana.c
> @@ -94,6 +94,9 @@ mana_dev_configure(struct rte_eth_dev *dev)
> return -EINVAL;
> }
>
> + priv->vlan_strip = !!(dev_conf->rxmode.offloads &
> + RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
> +
> priv->num_queues = dev->data->nb_rx_queues;
>
> manadv_set_context_attr(priv->ib_ctx,
> MANADV_CTX_ATTR_BUF_ALLOCATORS, diff --git a/drivers/net/mana/mana.h
> b/drivers/net/mana/mana.h index 3626925871..37f654f0e6 100644
> --- a/drivers/net/mana/mana.h
> +++ b/drivers/net/mana/mana.h
> @@ -21,10 +21,12 @@ struct mana_shared_data { #define
> MANA_MAX_MAC_ADDR 1
>
> #define MANA_DEV_RX_OFFLOAD_SUPPORT ( \
> + RTE_ETH_RX_OFFLOAD_VLAN_STRIP | \
> RTE_ETH_RX_OFFLOAD_CHECKSUM | \
> RTE_ETH_RX_OFFLOAD_RSS_HASH)
>
> #define MANA_DEV_TX_OFFLOAD_SUPPORT ( \
> + RTE_ETH_TX_OFFLOAD_VLAN_INSERT | \
> RTE_ETH_TX_OFFLOAD_MULTI_SEGS | \
> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM | \
> RTE_ETH_TX_OFFLOAD_TCP_CKSUM | \
> @@ -345,6 +347,8 @@ struct mana_priv {
> /* IB device port */
> uint8_t dev_port;
>
> + uint8_t vlan_strip;
> +
> struct ibv_context *ib_ctx;
> struct ibv_pd *ib_pd;
> struct ibv_pd *ib_parent_pd;
> diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c index
> 16e647baf5..0c26702b73 100644
> --- a/drivers/net/mana/rx.c
> +++ b/drivers/net/mana/rx.c
> @@ -532,10 +532,6 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
> uint16_t pkts_n)
> mbuf->hash.rss = oob-
> >packet_info[pkt_idx].packet_hash;
> }
>
> - pkts[pkt_received++] = mbuf;
> - rxq->stats.packets++;
> - rxq->stats.bytes += mbuf->data_len;
> -
> pkt_idx++;
> /* Move on the next completion if all packets are processed */
> if (pkt_idx >= RX_COM_OOB_NUM_PACKETINFO_SEGMENTS)
> { @@ -543,6 +539,24 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf
> **pkts, uint16_t pkts_n)
> i++;
> }
>
> + if (oob->rx_vlan_tag_present) {
> + mbuf->ol_flags |=
> + RTE_MBUF_F_RX_VLAN |
> RTE_MBUF_F_RX_VLAN_STRIPPED;
> + mbuf->vlan_tci = oob->rx_vlan_id;
> +
> + if (!priv->vlan_strip && rte_vlan_insert(&mbuf)) {
> + DRV_LOG(ERR, "vlan insert failed");
> + rxq->stats.errors++;
> + rte_pktmbuf_free(mbuf);
> +
> + goto drop;
> + }
> + }
> +
> + pkts[pkt_received++] = mbuf;
> + rxq->stats.packets++;
> + rxq->stats.bytes += mbuf->data_len;
> +
> drop:
> rxq->desc_ring_tail++;
> if (rxq->desc_ring_tail >= rxq->num_desc) diff --git
> a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c index
> 58c4a1d976..272a28bcba 100644
> --- a/drivers/net/mana/tx.c
> +++ b/drivers/net/mana/tx.c
> @@ -254,7 +254,18 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf
> **tx_pkts, uint16_t nb_pkts)
> }
>
> /* Fill in the oob */
> - tx_oob.short_oob.packet_format = SHORT_PACKET_FORMAT;
> + if (m_pkt->ol_flags & RTE_MBUF_F_TX_VLAN) {
> + tx_oob.short_oob.packet_format =
> LONG_PACKET_FORMAT;
> + tx_oob.long_oob.inject_vlan_prior_tag = 1;
> + tx_oob.long_oob.priority_code_point =
> + RTE_VLAN_TCI_PRI(m_pkt->vlan_tci);
> + tx_oob.long_oob.drop_eligible_indicator =
> + RTE_VLAN_TCI_DEI(m_pkt->vlan_tci);
> + tx_oob.long_oob.vlan_identifier =
> + RTE_VLAN_TCI_ID(m_pkt->vlan_tci);
> + } else {
> + tx_oob.short_oob.packet_format =
> SHORT_PACKET_FORMAT;
> + }
> tx_oob.short_oob.tx_is_outer_
RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific
Hi Ganapati, >> Is it not possible to use rte_event_crypto_adapter_enqueue >> if you want to send the event context to cryptodev? > [Ganapati] No, event crypto adapter sends only ev::event_ptr as rte_crypto_op > to cryptodev and not event context. >> While using rte_cryptodev_enqueue() all previous stage event context is >> meant to be lost and >> It would send a new crypto request to cryptodev and is not supposed to be >> aware of event context. > [Ganapati] Yes, proposal is for sending implementation specific value from > eventdev to crypodev and vice versa As discussed in a separate mail thread, impl_opaque in rte_crypto_op is not generic and is specific to your use case. The crypto dev wont be able to make difference whether to alter it (for any other usecase) or not(for event case). As per definition of impl_opaque is meant to be consumed by driver. Hence this will contradict the usage. Since impl_opaque in rte_crypto_op is exposed to driver. Drivers are free to use it. You may consider using mbuf dynamic fields for setting some userdata for your use case for each packet. See rte_security_dynfield and add a fastpath cryptodev API to set pkt userdata which is opaque to driver (similar to rte_security_set_pkt_metadata). Or else add a similar schema to introduce dynamic fields in rte_crypto_op. Regards, Akhil
Re: [PATCH 0/2] fix problems about flow steering
On 3/12/2024 8:02 AM, Chaoyong He wrote: > This patch series fix some problems exist in the flow steering logic. > > Long Wu (2): > net/nfp: fix IP flow rule failed > net/nfp: fix incorrect queue index > Series applied to dpdk-next-net/main, thanks.
RE: [EXTERNAL] [PATCH] app/crypto-perf-test: fix unset crc algorithm
> Because net crc api is not thread-safe, setting crc algorithm
> by the application will prevent race condition in the calc function.
> Race condition still may occur when any of the threads will call this
> function again. Function is called with the highest possible SIMD
> extension, which is AVX512, but if this is not found, CRC API will
> pick the other highest possible extension, or scalar if no SIMD
> available.
>
> Fixes: f8be1786b1b8 ("app/crypto-perf: introduce performance test
> application")
>
> Signed-off-by: Arkadiusz Kusztal
> ---
> app/test-crypto-perf/main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
> index 40c0b4b54f..58496797d7 100644
> --- a/app/test-crypto-perf/main.c
> +++ b/app/test-crypto-perf/main.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
> #ifdef RTE_CRYPTO_SCHEDULER
> #include
> #endif
> @@ -599,6 +600,8 @@ main(int argc, char **argv)
> goto err;
> }
>
> + rte_net_crc_set_alg(RTE_NET_CRC_AVX512);
> +
> ret = cperf_verify_devices_capabilities(&opts, enabled_cdevs,
> nb_cryptodevs);
> if (ret) {
> --
Not sure what is the use of this API here.
Which calc function is it fixing.
Also will it ever pick neon handler?
RE: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem
> Subject: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness
> problem
> From: Shihong Wang
>
> The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
> value is stored in an array of encryption or authentication keys
> according to big-endian. So it maybe need to convert the endianness
> order to ensure that the value assigned to the SA salt is CPU-endian.
>
> Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
> Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional algorithms")
> Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Shihong Wang
> Reviewed-by: Chaoyong He
> ---
> examples/ipsec-secgw/sa.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index c4bac17cd7..4018b0558a 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -374,6 +374,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> uint32_t ti; /*token index*/
> uint32_t *ri /*rule index*/;
> struct ipsec_sa_cnt *sa_cnt;
> + rte_be32_t salt; /*big-endian salt*/
> uint32_t cipher_algo_p = 0;
> uint32_t auth_algo_p = 0;
> uint32_t aead_algo_p = 0;
> @@ -508,8 +509,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> if (algo->algo == RTE_CRYPTO_CIPHER_AES_CTR) {
> key_len -= 4;
> rule->cipher_key_len = key_len;
> - memcpy(&rule->salt,
> + memcpy(&salt,
> &rule->cipher_key[key_len], 4);
> + rule->salt = rte_be_to_cpu_32(salt);
> }
>
> cipher_algo_p = 1;
> @@ -573,8 +575,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> key_len -= 4;
> rule->auth_key_len = key_len;
> rule->iv_len = algo->iv_len;
> - memcpy(&rule->salt,
> + memcpy(&salt,
> &rule->auth_key[key_len], 4);
> + rule->salt = rte_be_to_cpu_32(salt);
> }
>
> auth_algo_p = 1;
> @@ -632,8 +635,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>
> key_len -= 4;
> rule->cipher_key_len = key_len;
> - memcpy(&rule->salt,
> + memcpy(&salt,
> &rule->cipher_key[key_len], 4);
Can you put the memcpy call in a single line?
> + rule->salt = rte_be_to_cpu_32(salt);
Re: [PATCH v3 1/1] net/mana: add vlan tagging support
Recheck-request: iol-broadcom-Performance I am seeing some increased throughput variance on this NIC recently (may be related to upgrading device firmware). The Community Lab team will look. This fail can be ignored of course, and should be overwritten once the retest is picked up.
