Re: [dpdk-dev] [PATCH] ethdev: modify comment of INTR RESET event

[Singh, Aman Deep]

Hi Chengwen,

On 7/26/2021 12:13 PM, Chengwen Feng wrote:

According to the definition of rte_eth_dev_reset(), the
RTE_ETH_EVENT_INTR_RESET event could also use when PF resets.

This patch modifies the comment of RTE_ETH_EVENT_INTR_RESET event, so
that it could use in all resets.

Signed-off-by: Chengwen Feng
---
  lib/ethdev/rte_ethdev.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c3..e6646a6 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3499,8 +3499,7 @@ enum rte_eth_event_type {
RTE_ETH_EVENT_INTR_LSC, /**< lsc interrupt event */
RTE_ETH_EVENT_QUEUE_STATE,
/**< queue state event (enabled/disabled) */
-   RTE_ETH_EVENT_INTR_RESET,
-   /**< reset interrupt event, sent to VF on PF reset */
+   RTE_ETH_EVENT_INTR_RESET, /**< reset interrupt event */
RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
RTE_ETH_EVENT_MACSEC,   /**< MACsec offload related event */
RTE_ETH_EVENT_INTR_RMV, /**< device removal event */


Acked-by: Aman Deep Singh 

Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue

[Jerin Jacob]

On Mon, Aug 9, 2021 at 7:46 PM Xueming(Steven) Li  wrote:
>
> Hi,
>
> > -Original Message-
> > From: Jerin Jacob 
> > Sent: Monday, August 9, 2021 9:51 PM
> > To: Xueming(Steven) Li 
> > Cc: dpdk-dev ; Ferruh Yigit ; 
> > NBU-Contact-Thomas Monjalon ;
> > Andrew Rybchenko 
> > Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue
> >
> > On Mon, Aug 9, 2021 at 5:18 PM Xueming Li  wrote:
> > >
> > > In current DPDK framework, each RX queue is pre-loaded with mbufs for
> > > incoming packets. When number of representors scale out in a switch
> > > domain, the memory consumption became significant. Most important,
> > > polling all ports leads to high cache miss, high latency and low
> > > throughput.
> > >
> > > This patch introduces shared RX queue. Ports with same configuration
> > > in a switch domain could share RX queue set by specifying sharing group.
> > > Polling any queue using same shared RX queue receives packets from all
> > > member ports. Source port is identified by mbuf->port.
> > >
> > > Port queue number in a shared group should be identical. Queue index
> > > is
> > > 1:1 mapped in shared group.
> > >
> > > Share RX queue is supposed to be polled on same thread.
> > >
> > > Multiple groups is supported by group ID.
> >
> > Is this offload specific to the representor? If so can this name be changed 
> > specifically to representor?
>
> Yes, PF and representor in switch domain could take advantage.
>
> > If it is for a generic case, how the flow ordering will be maintained?
>
> Not quite sure that I understood your question. The control path of is almost 
> same as before,
> PF and representor port still needed, rte flows not impacted.
> Queues still needed for each member port, descriptors(mbuf) will be supplied 
> from shared Rx queue
> in my PMD implementation.

My question was if create a generic RTE_ETH_RX_OFFLOAD_SHARED_RXQ offload,
multiple ethdev receive queues land into the same receive queue, In that case,
how the flow order is maintained for respective receive queues.
If this offload is only useful for representor case, Can we make this
offload specific
to representor the case by changing its name and scope.



>
> >
> > >
> > > Signed-off-by: Xueming Li 
> > > ---
> > >  doc/guides/nics/features.rst| 11 +++
> > >  doc/guides/nics/features/default.ini|  1 +
> > >  doc/guides/prog_guide/switch_representation.rst | 10 ++
> > >  lib/ethdev/rte_ethdev.c |  1 +
> > >  lib/ethdev/rte_ethdev.h |  7 +++
> > >  5 files changed, 30 insertions(+)
> > >
> > > diff --git a/doc/guides/nics/features.rst
> > > b/doc/guides/nics/features.rst index a96e12d155..2e2a9b1554 100644
> > > --- a/doc/guides/nics/features.rst
> > > +++ b/doc/guides/nics/features.rst
> > > @@ -624,6 +624,17 @@ Supports inner packet L4 checksum.
> > >
> > > ``tx_offload_capa,tx_queue_offload_capa:DEV_TX_OFFLOAD_OUTER_UDP_CKSUM``.
> > >
> > >
> > > +.. _nic_features_shared_rx_queue:
> > > +
> > > +Shared Rx queue
> > > +---
> > > +
> > > +Supports shared Rx queue for ports in same switch domain.
> > > +
> > > +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: 
> > > ``offloads:RTE_ETH_RX_OFFLOAD_SHARED_RXQ``.
> > > +* **[provides] mbuf**: ``mbuf.port``.
> > > +
> > > +
> > >  .. _nic_features_packet_type_parsing:
> > >
> > >  Packet type parsing
> > > diff --git a/doc/guides/nics/features/default.ini
> > > b/doc/guides/nics/features/default.ini
> > > index 754184ddd4..ebeb4c1851 100644
> > > --- a/doc/guides/nics/features/default.ini
> > > +++ b/doc/guides/nics/features/default.ini
> > > @@ -19,6 +19,7 @@ Free Tx mbuf on demand =
> > >  Queue start/stop =
> > >  Runtime Rx queue setup =
> > >  Runtime Tx queue setup =
> > > +Shared Rx queue  =
> > >  Burst mode info  =
> > >  Power mgmt address monitor =
> > >  MTU update   =
> > > diff --git a/doc/guides/prog_guide/switch_representation.rst
> > > b/doc/guides/prog_guide/switch_representation.rst
> > > index ff6aa91c80..45bf5a3a10 100644
> > > --- a/doc/guides/prog_guide/switch_representation.rst
> > > +++ b/doc/guides/prog_guide/switch_representation.rst
> > > @@ -123,6 +123,16 @@ thought as a software "patch panel" front-end for 
> > > applications.
> > >  .. [1] `Ethernet switch device driver model (switchdev)
> > >
> > > `_
> > >
> > > +- Memory usage of representors is huge when number of representor
> > > +grows,
> > > +  because PMD always allocate mbuf for each descriptor of Rx queue.
> > > +  Polling the large number of ports brings more CPU load, cache miss
> > > +and
> > > +  latency. Shared Rx queue can be used to share Rx queue between PF
> > > +and
> > > +  representors in same switch domain.
> > > +``RTE_ETH_RX_OFFLOAD_SHARED_RXQ``
> > > +  is present in Rx offloading capability of device info. Setting the
> > > +  offloading flag in device 

Re: [dpdk-dev] [PATCH v2 2/4] cryptodev: change valid dev API

[Akhil Goyal]

> 
> 
> From: Akhil Goyal
> > The API rte_cryptodev_pmd_is_valid_dev, can be used by the application
> as
> > well as PMD to check whether the device is valid or not. Hence, _pmd is
> > removed from the API.
> > The applications and drivers which use this API are also updated.
> >
> > Signed-off-by: Akhil Goyal 
> 
> Agree.
> 
> What's about updating the release notes on this change?
> 
Yes this need to be updated in release notes.
21.11 release notes file is not yet created, will add it once it is available.

Re: [dpdk-dev] [PATCH v2 3/4] examples/fips_validation: remove illegal usage of APIs

[Akhil Goyal]

> 
> From: Akhil Goyal
> > Some of the cryptodev APIs are not allowed to be used by application
> > directly. Hence removing the usage of 1. queue_pair_release: it is not
> > required, as configure
> >of queue pair release the previous queue pairs and the
> >dev is not directly exposed to application, hence cannot
> >use its ops from app.
> > 2. rte_cryptodev_stop: it can be used directly without
> >checking if the device is started or not.
> > 3. rte_cryptodev_pmd_destroy: application should use
> >rte_cryptodev_close instead.
> >
> > Signed-off-by: Akhil Goyal 
> 
> Look's like it should be backported to stable releases with a Fixes reference.
> What do you think?
> 
> Besides,
> Acked-by: Matan Azrad 
> 
Yes, Agreed.
I wanted to get opinion from the Maintainer of fips app first if the changes 
are 
Correct or not.

Re: [dpdk-dev] [PATCH v2 4/4] cryptodev: expose driver interface as internal

[Akhil Goyal]

> From: Akhil Goyal
> > The rte_cryptodev_pmd.* files are for drivers only and should be private to
> > DPDK, and not installed for app use.
> >
> > Signed-off-by: Akhil Goyal 
> 
> Please consider adding to release notes.
> 
> Acked-by: Matan Azrad 
Yes will do so when it is available.

Re: [dpdk-dev] [PATCH] eal/windows: add sys/queue.h.

[Nick Connolly]

What we can do:

1. Introduce `rte_queue.h` (name can be better) that is env-specific:

1.1. For Linux and FreeBSD it just includes 
 and renames a few macros that are used in headers to RTE_xxx.
1.2. For Windows it defines the same RTE_xxx macros in a way
 compatible with the  version used to build DPDK.

2. Add #include  in :
Linux and FreeBSD will include a system header,
Windows will use the bundled one.

This way application are not exposed to non-RTE symbols,
at the same time RTE_xxx are binary-compatible with what DPDK
implementation expects (and outside of Windows there is no change in fact).


+1
Nick

Re: [dpdk-dev] [PATCH v3 3/5] vdpa/mlx5: fix minsize build

[Ruifeng Wang]

> -Original Message-
> From: dev  On Behalf Of Thomas Monjalon
> Sent: Sunday, August 8, 2021 8:52 PM
> To: dev@dpdk.org
> Cc: bruce.richard...@intel.com; david.march...@redhat.com; Matan Azrad
> ; Viacheslav Ovsiienko 
> Subject: [dpdk-dev] [PATCH v3 3/5] vdpa/mlx5: fix minsize build
> 
> Error occurs when configuring meson with --buildtype=minsize with GCC
> 11.1.0:
> 
> drivers/vdpa/mlx5/mlx5_vdpa_mem.c: In function
> ‘mlx5_vdpa_mem_register’:
> drivers/vdpa/mlx5/mlx5_vdpa_mem.c:183:24: error:
> initialization of ‘uint64_t’ {aka ‘long unsigned int’} from ‘void *’
> makes integer from pointer without a cast [-Werror=int-conversion]
> | uint64_t gcd = NULL;
> |^~~~
> drivers/vdpa/mlx5/mlx5_vdpa_mem.c:244:75: error:
> ‘mode’ may be used uninitialized in this function [-Werror=maybe-
> uninitialized]
> | klm_size = mode == MLX5_MKC_ACCESS_MODE_KLM ?
> |~~
> |   KLM_SIZE_MAX_ALIGN(empty_region_sz) : gcd;
> |
> | ^
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  drivers/vdpa/mlx5/mlx5_vdpa_mem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_mem.c
> b/drivers/vdpa/mlx5/mlx5_vdpa_mem.c
> index a13bde5a0b..59ce4e891c 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa_mem.c
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa_mem.c
> @@ -177,10 +177,10 @@ mlx5_vdpa_mem_register(struct mlx5_vdpa_priv
> *priv)
>   struct mlx5_devx_mkey_attr mkey_attr;
>   struct mlx5_vdpa_query_mr *entry = NULL;
>   struct rte_vhost_mem_region *reg = NULL;
> - uint8_t mode;
> + uint8_t mode = 0;
>   uint32_t entries_num = 0;
>   uint32_t i;
> - uint64_t gcd;
> + uint64_t gcd = 0;
>   uint64_t klm_size;
>   uint64_t mem_size;
>   uint64_t k;
> --
> 2.31.1
Reviewed-by: Ruifeng Wang 

Re: [dpdk-dev] [PATCH v3 2/5] regex/mlx5: fix minsize build

[Ruifeng Wang]

> -Original Message-
> From: dev  On Behalf Of Thomas Monjalon
> Sent: Sunday, August 8, 2021 8:52 PM
> To: dev@dpdk.org
> Cc: bruce.richard...@intel.com; david.march...@redhat.com; Ori Kam
> 
> Subject: [dpdk-dev] [PATCH v3 2/5] regex/mlx5: fix minsize build
> 
> Error occurs when configuring meson with --buildtype=minsize with GCC
> 11.1.0:
> 
> drivers/regex/mlx5/mlx5_regex_fastpath.c:398:17: error:
> ‘len’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | complete_umr_wqe(qp, sq, &qp->jobs[mkey_job_id], sq->pi,
> |
> ^~~~
> |  klm_num, len);
> |  ~
> drivers/regex/mlx5/mlx5_regex_fastpath.c:315:31: note: ‘len’ was declared
> here
> | uint32_t klm_num = 0, len;
> |   ^~~
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  drivers/regex/mlx5/mlx5_regex_fastpath.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/regex/mlx5/mlx5_regex_fastpath.c
> b/drivers/regex/mlx5/mlx5_regex_fastpath.c
> index 786718af53..c79445ce7d 100644
> --- a/drivers/regex/mlx5/mlx5_regex_fastpath.c
> +++ b/drivers/regex/mlx5/mlx5_regex_fastpath.c
> @@ -312,7 +312,8 @@ prep_regex_umr_wqe_set(struct mlx5_regex_priv
> *priv, struct mlx5_regex_qp *qp,
>   struct mlx5_regex_job *job = NULL;
>   size_t sqid = sq->sqn, mkey_job_id = 0;
>   size_t left_ops = nb_ops;
> - uint32_t klm_num = 0, len;
> + uint32_t klm_num = 0;
> + uint32_t len = 0;
>   struct mlx5_klm *mkey_klm = NULL;
>   struct mlx5_klm klm;
> 
> --
> 2.31.1
Reviewed-by: Ruifeng Wang 

Re: [dpdk-dev] [PATCH v3 4/5] test/crypto: fix minsize build

[Ruifeng Wang]

> -Original Message-
> From: dev  On Behalf Of Thomas Monjalon
> Sent: Sunday, August 8, 2021 8:52 PM
> To: dev@dpdk.org
> Cc: bruce.richard...@intel.com; david.march...@redhat.com; Akhil Goyal
> ; Declan Doherty 
> Subject: [dpdk-dev] [PATCH v3 4/5] test/crypto: fix minsize build
> 
> Error occurs when configuring meson with --buildtype=minsize with GCC
> 11.1.0:
> 
> app/test/test_cryptodev_blockcipher.c:1133:45: error:
> ‘blk_tcs’ may be used uninitialized in this function [-Werror=maybe-
> uninitialized]
> | const struct blockcipher_test_case *blk_tcs;
> | ^~~
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  app/test/test_cryptodev_blockcipher.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test/test_cryptodev_blockcipher.c
> b/app/test/test_cryptodev_blockcipher.c
> index 53fd4718af..0d5082887e 100644
> --- a/app/test/test_cryptodev_blockcipher.c
> +++ b/app/test/test_cryptodev_blockcipher.c
> @@ -1184,7 +1184,7 @@ build_blockcipher_test_suite(enum
> blockcipher_test_type test_type)
>   ts_setup = authonly_setup;
>   break;
>   default:
> - break;
> + return NULL;
>   }
> 
>   ts = calloc(1, sizeof(struct unit_test_suite) +
> --
> 2.31.1
Reviewed-by: Ruifeng Wang 

[dpdk-dev] [PATCH v2 1/4] test/crypto: add lookaside IPsec tests

[Anoob Joseph]

Added test case for lookaside IPsec. Inbound known vector
tests are added.

Cipher list: AES-GCM 128, 192 & 256

Signed-off-by: Anoob Joseph 
Signed-off-by: Tejasree Kondoj 
---
 app/test/meson.build   |   1 +
 app/test/test.h|   6 +
 app/test/test_cryptodev.c  | 231 +++
 app/test/test_cryptodev_security_ipsec.c   | 212 ++
 app/test/test_cryptodev_security_ipsec.h   |  66 +
 .../test_cryptodev_security_ipsec_test_vectors.h   | 321 +
 6 files changed, 837 insertions(+)
 create mode 100644 app/test/test_cryptodev_security_ipsec.c
 create mode 100644 app/test/test_cryptodev_security_ipsec.h
 create mode 100644 app/test/test_cryptodev_security_ipsec_test_vectors.h

diff --git a/app/test/meson.build b/app/test/meson.build
index a761168..f144d8b 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -38,6 +38,7 @@ test_sources = files(
 'test_cryptodev.c',
 'test_cryptodev_asym.c',
 'test_cryptodev_blockcipher.c',
+'test_cryptodev_security_ipsec.c',
 'test_cryptodev_security_pdcp.c',
 'test_cycles.c',
 'test_debug.c',
diff --git a/app/test/test.h b/app/test/test.h
index c3b2a87..7115edf 100644
--- a/app/test/test.h
+++ b/app/test/test.h
@@ -124,6 +124,12 @@ struct unit_test_case {
 #define TEST_CASE_WITH_DATA(setup, teardown, testcase, data) \
{ setup, teardown, NULL, testcase, #testcase, 1, data }
 
+#define TEST_CASE_NAMED_ST(name, setup, teardown, testcase) \
+   { setup, teardown, NULL, testcase, name, 1, NULL }
+
+#define TEST_CASE_NAMED_WITH_DATA(name, setup, teardown, testcase, data) \
+   { setup, teardown, NULL, testcase, name, 1, data }
+
 #define TEST_CASE_DISABLED(fn) { NULL, NULL, fn, NULL, #fn, 0, NULL }
 
 #define TEST_CASE_ST_DISABLED(setup, teardown, testcase) \
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9ad0b37..73923f1 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifdef RTE_CRYPTO_SCHEDULER
@@ -42,6 +43,8 @@
 #include "test_cryptodev_hmac_test_vectors.h"
 #include "test_cryptodev_mixed_test_vectors.h"
 #ifdef RTE_LIB_SECURITY
+#include "test_cryptodev_security_ipsec.h"
+#include "test_cryptodev_security_ipsec_test_vectors.h"
 #include "test_cryptodev_security_pdcp_test_vectors.h"
 #include "test_cryptodev_security_pdcp_sdap_test_vectors.h"
 #include "test_cryptodev_security_pdcp_test_func.h"
@@ -124,6 +127,13 @@ test_AES_CBC_HMAC_SHA512_decrypt_perform(struct 
rte_cryptodev_sym_session *sess,
const uint8_t *digest,
const uint8_t *iv);
 
+static int
+security_proto_supported(enum rte_security_session_action_type action,
+   enum rte_security_session_protocol proto);
+
+static int
+dev_configure_and_start(uint64_t ff_disable);
+
 static struct rte_mbuf *
 setup_test_string(struct rte_mempool *mpool,
const char *string, size_t len, uint8_t blocksize)
@@ -754,6 +764,42 @@ crypto_gen_testsuite_setup(void)
 
 #ifdef RTE_LIB_SECURITY
 static int
+ipsec_proto_testsuite_setup(void)
+{
+   struct crypto_testsuite_params *ts_params = &testsuite_params;
+   struct crypto_unittest_params *ut_params = &unittest_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_SECURITY)) {
+   RTE_LOG(INFO, USER1, "Feature flag requirements for IPsec Proto 
"
+   "testsuite not met\n");
+   return TEST_SKIPPED;
+   }
+
+   /* Reconfigure to enable security */
+   dev_configure_and_start(RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO |
+   RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO);
+
+   /* Set action type */
+   ut_params->type = RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL;
+
+   if (security_proto_supported(
+   RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL,
+   RTE_SECURITY_PROTOCOL_IPSEC) < 0) {
+   RTE_LOG(INFO, USER1, "Capability requirements for IPsec Proto "
+   "test not met\n");
+   return TEST_SKIPPED;
+   }
+
+   /* Stop the device */
+   rte_cryptodev_stop(ts_params->valid_devs[0]);
+
+   return 0;
+}
+
+static int
 pdcp_proto_testsuite_setup(void)
 {
struct crypto_testsuite_params *ts_params = &testsuite_params;
@@ -8811,6 +8857,170 @@ test_PDCP_SDAP_PROTO_decap_all(void)
 }
 
 static int
+test_ipsec_proto_process(const struct ipsec_test_data td[],
+struct ipsec_test_data res_d[],
+int nb_td,
+bool silent)
+{
+   struct crypto_testsuite_params *ts_params = &testsuite_param

[dpdk-dev] [PATCH v2 0/4] Add lookaside IPsec tests

[Anoob Joseph]

Add lookaside IPsec functional tests. Known vector tests and
combined mode framework is added.

Known vectors are outbound vectors based on
https://datatracker.ietf.org/doc/html/draft-mcgrew-gcm-test-01

The vectors are updated to have sequence number as 1 & L4 checksum
computed correctly. And they have following properties,
1. ESP
2. Tunnel mode
3. IPv4
4. IPv4 tunnel

Known vector tests for inbound operation would generate test vectors by
reversing outbound known vectors. The input_text would become encrypted
packet and output_text would be the plain packet. Tests would then validate
the operation by comparing against plain packet.

Combined mode tests are used to test all IPsec features against all ciphers
supported by the PMD. The framework is introduced to avoid testing
with any specific algo, thereby making it mandatory to be supported. Also,
testing with all supported combinations will help with increasing coverage
as well.

Three test cases use combined mode,
1. Display algo coverage and basic in + out tests
2. Negative test for ICV corruption
3. IV generation

IV generation test case compares IV generated for a batch of packets and returns
failure if IV is repeated.

Upcoming additions,
1. AES-CBC-SHA1-HMAC known vectors & combined mode
2. IPv6
3. UDP encapsulation
4. Transport
5. Mixed mode (IPv4-in-IPv6 etc, all combinations)

Tested with following PMDs
1. crypto_octeontx2
2. crypto_cn10k

Changes in v2
- Dropped outbound known vector tests as lookaside protocol would require IV
  generated by PMD. The tests would be introduced with spec change to allow user
  to specify IV.
- Added IV generation tests
- Minor fixes in combined mode tests to handle multiple packets

Anoob Joseph (2):
  test/crypto: add lookaside IPsec tests
  test/crypto: add combined mode tests

Tejasree Kondoj (2):
  test/crypto: add lookaside IPsec ICV corrupt test case
  test/crypto: add IV gen tests

 app/test/meson.build   |   1 +
 app/test/test.h|   6 +
 app/test/test_cryptodev.c  | 331 ++
 app/test/test_cryptodev_security_ipsec.c   | 373 +
 app/test/test_cryptodev_security_ipsec.h   | 118 +++
 .../test_cryptodev_security_ipsec_test_vectors.h   | 321 ++
 6 files changed, 1150 insertions(+)
 create mode 100644 app/test/test_cryptodev_security_ipsec.c
 create mode 100644 app/test/test_cryptodev_security_ipsec.h
 create mode 100644 app/test/test_cryptodev_security_ipsec_test_vectors.h

-- 
2.7.4

[dpdk-dev] [PATCH v2 2/4] test/crypto: add combined mode tests

[Anoob Joseph]

Add framework to test IPsec features with all supported
combinations of ciphers.

Signed-off-by: Anoob Joseph 
Signed-off-by: Tejasree Kondoj 
---
 app/test/test_cryptodev.c|  73 +++--
 app/test/test_cryptodev_security_ipsec.c | 107 +--
 app/test/test_cryptodev_security_ipsec.h |  52 ++-
 3 files changed, 223 insertions(+), 9 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 73923f1..d89307d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8860,7 +8860,8 @@ static int
 test_ipsec_proto_process(const struct ipsec_test_data td[],
 struct ipsec_test_data res_d[],
 int nb_td,
-bool silent)
+bool silent,
+const struct ipsec_test_flags *flags)
 {
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct crypto_unittest_params *ut_params = &unittest_params;
@@ -8977,7 +8978,7 @@ test_ipsec_proto_process(const struct ipsec_test_data 
td[],
/* Process crypto operation */
process_crypto_request(dev_id, ut_params->op);
 
-   ret = test_ipsec_status_check(ut_params->op, dir);
+   ret = test_ipsec_status_check(ut_params->op, flags, dir);
if (ret != TEST_SUCCESS)
goto crypto_op_free;
 
@@ -8985,7 +8986,7 @@ test_ipsec_proto_process(const struct ipsec_test_data 
td[],
res_d_tmp = &res_d[i];
 
ret = test_ipsec_post_process(ut_params->ibuf, &td[i],
- res_d_tmp, silent);
+ res_d_tmp, silent, flags);
if (ret != TEST_SUCCESS)
goto crypto_op_free;
 
@@ -9013,11 +9014,71 @@ test_ipsec_proto_process(const struct ipsec_test_data 
td[],
 static int
 test_ipsec_proto_known_vec_inb(const void *td_outb)
 {
+   struct ipsec_test_flags flags;
struct ipsec_test_data td_inb;
 
+   memset(&flags, 0, sizeof(flags));
+
test_ipsec_td_in_from_out(td_outb, &td_inb);
 
-   return test_ipsec_proto_process(&td_inb, NULL, 1, false);
+   return test_ipsec_proto_process(&td_inb, NULL, 1, false, &flags);
+}
+
+static int
+test_ipsec_proto_all(const struct ipsec_test_flags *flags)
+{
+   struct ipsec_test_data td_outb[IPSEC_TEST_PACKETS_MAX];
+   struct ipsec_test_data td_inb[IPSEC_TEST_PACKETS_MAX];
+   unsigned int i, nb_pkts = 1, pass_cnt = 0;
+   int ret;
+
+   for (i = 0; i < RTE_DIM(aead_list); i++) {
+   test_ipsec_td_prepare(&aead_list[i],
+ NULL,
+ flags,
+ td_outb,
+ nb_pkts);
+
+   ret = test_ipsec_proto_process(td_outb, td_inb, nb_pkts, true,
+  flags);
+   if (ret == TEST_SKIPPED)
+   continue;
+
+   if (ret == TEST_FAILED)
+   return TEST_FAILED;
+
+   test_ipsec_td_update(td_inb, td_outb, nb_pkts, flags);
+
+   ret = test_ipsec_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_ipsec_display_alg(&aead_list[i], NULL);
+
+   pass_cnt++;
+   }
+
+   if (pass_cnt > 0)
+   return TEST_SUCCESS;
+   else
+   return TEST_SKIPPED;
+}
+
+static int
+test_ipsec_proto_display_list(const void *data __rte_unused)
+{
+   struct ipsec_test_flags flags;
+
+   memset(&flags, 0, sizeof(flags));
+
+   flags.display_alg = true;
+
+   return test_ipsec_proto_all(&flags);
 }
 
 static int
@@ -13926,6 +13987,10 @@ static struct unit_test_suite ipsec_proto_testsuite  = 
{
"Inbound known vector (ESP tunnel mode IPv4 AES-GCM 
256)",
ut_setup_security, ut_teardown,
test_ipsec_proto_known_vec_inb, &pkt_aes_256_gcm),
+   TEST_CASE_NAMED_ST(
+   "Combined test alg list",
+   ut_setup_security, ut_teardown,
+   test_ipsec_proto_display_list),
TEST_CASES_END() /**< NULL terminate unit test array */
}
 };
diff --git a/app/test/test_cryptodev_security_ipsec.c 
b/app/test/test_cryptodev_security_ipsec.c
index 2431fcb..d08e093 100644
--- a/app/test/test_cryptodev_security_ipsec.c
+++ b/app/test/test_cryptodev_security_ipsec.c
@@ -10,6 +10,8 @@
 #include "test.h"
 #include "test_cryptodev_se

[dpdk-dev] [PATCH v2 3/4] test/crypto: add lookaside IPsec ICV corrupt test case

[Anoob Joseph]

From: Tejasree Kondoj 

Adding lookaside IPsec ICV corrupt test case.

Signed-off-by: Anoob Joseph 
Signed-off-by: Tejasree Kondoj 
---
 app/test/test_cryptodev.c| 16 
 app/test/test_cryptodev_security_ipsec.c | 30 --
 app/test/test_cryptodev_security_ipsec.h |  1 +
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index d89307d..488daed 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -9082,6 +9082,18 @@ test_ipsec_proto_display_list(const void *data 
__rte_unused)
 }
 
 static int
+test_ipsec_proto_err_icv_corrupt(const void *data __rte_unused)
+{
+   struct ipsec_test_flags flags;
+
+   memset(&flags, 0, sizeof(flags));
+
+   flags.icv_corrupt = true;
+
+   return test_ipsec_proto_all(&flags);
+}
+
+static int
 test_PDCP_PROTO_all(void)
 {
struct crypto_testsuite_params *ts_params = &testsuite_params;
@@ -13991,6 +14003,10 @@ static struct unit_test_suite ipsec_proto_testsuite  = 
{
"Combined test alg list",
ut_setup_security, ut_teardown,
test_ipsec_proto_display_list),
+   TEST_CASE_NAMED_ST(
+   "Negative test: ICV corruption",
+   ut_setup_security, ut_teardown,
+   test_ipsec_proto_err_icv_corrupt),
TEST_CASES_END() /**< NULL terminate unit test array */
}
 };
diff --git a/app/test/test_cryptodev_security_ipsec.c 
b/app/test/test_cryptodev_security_ipsec.c
index d08e093..aebbe66 100644
--- a/app/test/test_cryptodev_security_ipsec.c
+++ b/app/test/test_cryptodev_security_ipsec.c
@@ -175,9 +175,12 @@ test_ipsec_td_update(struct ipsec_test_data td_inb[],
memcpy(td_inb[i].output_text.data, td_outb[i].input_text.data,
   td_outb[i].input_text.len);
td_inb[i].output_text.len = td_outb->input_text.len;
-   }
 
-   RTE_SET_USED(flags);
+   if (flags->icv_corrupt) {
+   int icv_pos = td_inb[i].input_text.len - 4;
+   td_inb[i].input_text.data[icv_pos] += 1;
+   }
+   }
 }
 
 void
@@ -217,6 +220,11 @@ test_ipsec_td_verify(struct rte_mbuf *m, const struct 
ipsec_test_data *td,
uint8_t *output_text = rte_pktmbuf_mtod(m, uint8_t *);
uint32_t skip, len = rte_pktmbuf_pkt_len(m);
 
+   /* For negative tests, no need to do verification */
+   if (flags->icv_corrupt &&
+   td->ipsec_xform.direction == RTE_SECURITY_IPSEC_SA_DIR_INGRESS)
+   return TEST_SUCCESS;
+
if (len != td->output_text.len) {
printf("Output length (%d) not matching with expected (%d)\n",
len, td->output_text.len);
@@ -241,8 +249,6 @@ test_ipsec_td_verify(struct rte_mbuf *m, const struct 
ipsec_test_data *td,
return TEST_FAILED;
}
 
-   RTE_SET_USED(flags);
-
return TEST_SUCCESS;
 }
 
@@ -299,13 +305,17 @@ test_ipsec_status_check(struct rte_crypto_op *op,
 {
int ret = TEST_SUCCESS;
 
-   if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
-   printf("Security op processing failed\n");
-   ret = TEST_FAILED;
+   if (dir == RTE_SECURITY_IPSEC_SA_DIR_INGRESS && flags->icv_corrupt) {
+   if (op->status != RTE_CRYPTO_OP_STATUS_ERROR) {
+   printf("ICV corruption test case failed\n");
+   ret = TEST_FAILED;
+   }
+   } else {
+   if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
+   printf("Security op processing failed\n");
+   ret = TEST_FAILED;
+   }
}
 
-   RTE_SET_USED(flags);
-   RTE_SET_USED(dir);
-
return ret;
 }
diff --git a/app/test/test_cryptodev_security_ipsec.h 
b/app/test/test_cryptodev_security_ipsec.h
index cbb3ee4..134fc3a 100644
--- a/app/test/test_cryptodev_security_ipsec.h
+++ b/app/test/test_cryptodev_security_ipsec.h
@@ -49,6 +49,7 @@ struct ipsec_test_data {
 
 struct ipsec_test_flags {
bool display_alg;
+   bool icv_corrupt;
 };
 
 struct crypto_param {
-- 
2.7.4

[dpdk-dev] [PATCH v2 4/4] test/crypto: add IV gen tests

[Anoob Joseph]

From: Tejasree Kondoj 

Add test cases to verify IV generated by PMD.

Signed-off-by: Anoob Joseph 
Signed-off-by: Tejasree Kondoj 
---
 app/test/test_cryptodev.c| 19 
 app/test/test_cryptodev_security_ipsec.c | 52 
 app/test/test_cryptodev_security_ipsec.h |  1 +
 3 files changed, 72 insertions(+)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 488daed..71e6c1a 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -9032,6 +9032,9 @@ test_ipsec_proto_all(const struct ipsec_test_flags *flags)
unsigned int i, nb_pkts = 1, pass_cnt = 0;
int ret;
 
+   if (flags->iv_gen)
+   nb_pkts = IPSEC_TEST_PACKETS_MAX;
+
for (i = 0; i < RTE_DIM(aead_list); i++) {
test_ipsec_td_prepare(&aead_list[i],
  NULL,
@@ -9082,6 +9085,18 @@ test_ipsec_proto_display_list(const void *data 
__rte_unused)
 }
 
 static int
+test_ipsec_proto_iv_gen(const void *data __rte_unused)
+{
+   struct ipsec_test_flags flags;
+
+   memset(&flags, 0, sizeof(flags));
+
+   flags.iv_gen = true;
+
+   return test_ipsec_proto_all(&flags);
+}
+
+static int
 test_ipsec_proto_err_icv_corrupt(const void *data __rte_unused)
 {
struct ipsec_test_flags flags;
@@ -14004,6 +14019,10 @@ static struct unit_test_suite ipsec_proto_testsuite  = 
{
ut_setup_security, ut_teardown,
test_ipsec_proto_display_list),
TEST_CASE_NAMED_ST(
+   "IV generation",
+   ut_setup_security, ut_teardown,
+   test_ipsec_proto_iv_gen),
+   TEST_CASE_NAMED_ST(
"Negative test: ICV corruption",
ut_setup_security, ut_teardown,
test_ipsec_proto_err_icv_corrupt),
diff --git a/app/test/test_cryptodev_security_ipsec.c 
b/app/test/test_cryptodev_security_ipsec.c
index aebbe66..78c7f3a 100644
--- a/app/test/test_cryptodev_security_ipsec.c
+++ b/app/test/test_cryptodev_security_ipsec.c
@@ -4,12 +4,15 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include "test.h"
 #include "test_cryptodev_security_ipsec.h"
 
+#define IV_LEN_MAX 16
+
 extern struct ipsec_test_data pkt_aes_256_gcm;
 
 int
@@ -214,6 +217,46 @@ test_ipsec_tunnel_hdr_len_get(const struct ipsec_test_data 
*td)
 }
 
 static int
+test_ipsec_iv_verify_push(struct rte_mbuf *m, const struct ipsec_test_data *td)
+{
+   static uint8_t iv_queue[IV_LEN_MAX * IPSEC_TEST_PACKETS_MAX];
+   uint8_t *iv_tmp, *output_text = rte_pktmbuf_mtod(m, uint8_t *);
+   int i, iv_pos, iv_len;
+   static int index;
+
+   if (td->aead)
+   iv_len = td->xform.aead.aead.iv.length - td->salt.len;
+   else
+   iv_len = td->xform.chain.cipher.cipher.iv.length;
+
+   iv_pos = test_ipsec_tunnel_hdr_len_get(td) + sizeof(struct rte_esp_hdr);
+   output_text += iv_pos;
+
+   TEST_ASSERT(iv_len <= IV_LEN_MAX, "IV length greater than supported");
+
+   /* Compare against previous values */
+   for (i = 0; i < index; i++) {
+   iv_tmp = &iv_queue[i * IV_LEN_MAX];
+
+   if (memcmp(output_text, iv_tmp, iv_len) == 0) {
+   printf("IV repeated");
+   return TEST_FAILED;
+   }
+   }
+
+   /* Save IV for future comparisons */
+
+   iv_tmp = &iv_queue[index * IV_LEN_MAX];
+   memcpy(iv_tmp, output_text, iv_len);
+   index++;
+
+   if (index == IPSEC_TEST_PACKETS_MAX)
+   index = 0;
+
+   return TEST_SUCCESS;
+}
+
+static int
 test_ipsec_td_verify(struct rte_mbuf *m, const struct ipsec_test_data *td,
 bool silent, const struct ipsec_test_flags *flags)
 {
@@ -279,6 +322,15 @@ test_ipsec_post_process(struct rte_mbuf *m, const struct 
ipsec_test_data *td,
struct ipsec_test_data *res_d, bool silent,
const struct ipsec_test_flags *flags)
 {
+   int ret;
+
+   if (flags->iv_gen &&
+   td->ipsec_xform.direction == RTE_SECURITY_IPSEC_SA_DIR_EGRESS) {
+   ret = test_ipsec_iv_verify_push(m, td);
+   if (ret != TEST_SUCCESS)
+   return ret;
+   }
+
/*
 * In case of known vector tests & all inbound tests, res_d provided
 * would be NULL and output data need to be validated against expected.
diff --git a/app/test/test_cryptodev_security_ipsec.h 
b/app/test/test_cryptodev_security_ipsec.h
index 134fc3a..d2ec63f 100644
--- a/app/test/test_cryptodev_security_ipsec.h
+++ b/app/test/test_cryptodev_security_ipsec.h
@@ -50,6 +50,7 @@ struct ipsec_test_data {
 struct ipsec_test_flags {
bool display_alg;
bool icv_corrupt;
+   bool iv_gen;
 };
 
 struct crypto_param {
-- 
2.7.4

Re: [dpdk-dev] [PATCH v1] examples/l3fwd: fix jumbo packet drop issue

[Singh, Aman Deep]

Hi Rohit,

On 7/27/2021 2:55 PM, rohit@nxp.com wrote:

From: Rohit Raj 

l3fwd uses mbufs with 2KB data size. If we enable jumbo packets, it is
not able to store packets with size greater than 2KB, hence these
packets are dropped.

This patch fixes this issue by enabling scatter for jumbo packet, if
it is supported by NIC.

If scatter is not supported by NIC and max jumbo packet length is
greater than default mbuf data size, then application exits with
proper error message.

Fixes: f68aad7904f ("examples/l3fwd: update")

Signed-off-by: Rohit Raj 
Signed-off-by: Sachin Saxena 
Signed-off-by: Vanshika Shukla 
---
  examples/l3fwd/main.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 4cb800aa15..68ecb5 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -1035,6 +1035,20 @@ l3fwd_poll_resource_setup(void)
"Error during getting device (port %u) info: 
%s\n",
portid, strerror(-ret));
  
+		/* Enable Receive side SCATTER, if supported by NIC,

+* when jumbo packet is enabled.
+*/
+   if (local_port_conf.rxmode.offloads &
+   DEV_RX_OFFLOAD_JUMBO_FRAME){
+   if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER)
+   local_port_conf.rxmode.offloads |=
+   DEV_RX_OFFLOAD_SCATTER;
+   else if (local_port_conf.rxmode.max_rx_pkt_len >
+   RTE_MBUF_DEFAULT_DATAROOM)
+   rte_exit(EXIT_FAILURE,
+   "Max packet length greater than default MBUF 
size\n");
+   }
+
if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
local_port_conf.txmode.offloads |=
DEV_TX_OFFLOAD_MBUF_FAST_FREE;


Acked-by: Aman Deep Singh 

Re: [dpdk-dev] [External] Re: [PATCH v2] app/testpmd: flowgen support ip and udp fields

[Ferruh Yigit]

On 8/11/2021 3:48 AM, 王志宏 wrote:
> On Tue, Aug 10, 2021 at 5:12 PM Ferruh Yigit  wrote:
>>
>> On 8/10/2021 8:57 AM, 王志宏 wrote:
>>> Thanks for the review Ferruh :)
>>>
>>> On Mon, Aug 9, 2021 at 11:18 PM Ferruh Yigit  wrote:

 On 8/9/2021 7:52 AM, Zhihong Wang wrote:
> This patch aims to:
>  1. Add flexibility by supporting IP & UDP src/dst fields

 What is the reason/"use case" of this flexibility?
>>>
>>> The purpose is to emulate pkt generator behaviors.
>>>
>>
>> 'flowgen' forwarding is already to emulate pkt generator, but it was only
>> changing destination IP.
>>
>> What additional benefit does changing udp ports of the packets brings? What 
>> is
>> your usecase for this change?
> 
> Pkt generators like pktgen/trex/ixia/spirent can change various fields
> including ip/udp src/dst.
> 

But testpmd is not packet generator, it has very simple 'flowgen' forwarding
engine, I would like to understand motivation to make it more complex.

> Keeping the cfg_n_* while setting cfg_n_ip_dst = 1024 and others = 1
> makes the default behavior exactly unchanged. Do you think it makes
> sense?
> 
>>

>  2. Improve multi-core performance by using per-core vars>

 On multi core this also has syncronization problem, OK to make it 
 per-core. Do
 you have any observed performance difference, if so how much is it?
>>>
>>> Huge difference, one example: 8 core flowgen -> rxonly results: 43
>>> Mpps (per-core) vs. 9.3 Mpps (shared), of course the numbers "varies
>>> depending on system configuration".
>>>
>>
>> Thanks for clarification.
>>

 And can you please separate this to its own patch? This can be before 
 ip/udp update.
>>>
>>> Will do.
>>>

> v2: fix assigning ip header cksum
>

 +1 to update, can you please make it as seperate patch?
>>>
>>> Sure.
>>>

 So overall this can be a patchset with 4 patches:
 1- Fix retry logic (nb_rx -> nb_pkt)
 2- Use 'rte_ipv4_cksum()' API (instead of static 'ip_sum()')
 3- User per-core varible (for 'next_flow')
 4- Support ip/udp src/dst variaty of packets

>>>
>>> Great summary. Thanks a lot.
>>>
> Signed-off-by: Zhihong Wang 
> ---
>  app/test-pmd/flowgen.c | 137 
> +++--
>  1 file changed, 86 insertions(+), 51 deletions(-)
>

 <...>

> @@ -185,30 +193,57 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
>   }
>   pkts_burst[nb_pkt] = pkt;
>
> - next_flow = (next_flow + 1) % cfg_n_flows;
> + if (++next_udp_dst < cfg_n_udp_dst)
> + continue;
> + next_udp_dst = 0;
> + if (++next_udp_src < cfg_n_udp_src)
> + continue;
> + next_udp_src = 0;
> + if (++next_ip_dst < cfg_n_ip_dst)
> + continue;
> + next_ip_dst = 0;
> + if (++next_ip_src < cfg_n_ip_src)
> + continue;
> + next_ip_src = 0;

 What is the logic here, can you please clarifiy the packet generation 
 logic both
 in a comment here and in the commit log?
>>>
>>> It's round-robin field by field. Will add the comments.
>>>
>>
>> Thanks. If the receiving end is doing RSS based on IP address, dst address 
>> will
>> change in every 100 packets and src will change in every 1 packets. This 
>> is
>> a slight behavior change.
>>
>> When it was only dst ip, it was simple to just increment it, not sure about 
>> it
>> in this case. I wonder if we should set all randomly for each packet. I don't
>> know what is the better logic here, we can discuss it more in the next 
>> version.
> 
> A more sophisticated pkt generator provides various options among
> "step-by-step" / "random" / etc.
> 
> But supporting multiple fields naturally brings this implicitly. It
> won't be a problem as it can be configured by setting the cfg_n_* as
> we discussed above.
> 
> I think rte_rand() is a good option, anyway this can be tweaked easily
> once the framework becomes shaped.
> 

Can be done, but do we really want to add more packet generator capability to
testpmd?

>>

>   }
>
>   nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, 
> nb_pkt);
>   /*
>* Retry if necessary
>*/
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
> + if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
>   retry = 0;
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> + while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
>   rte_delay_us(burst_tx_delay_time);
>   nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_rx - nb_tx);
> +  

Re: [dpdk-dev] [PATCH v2 1/4] test/crypto: remove illegal header include

[Matan Azrad]

From: Akhil Goyal
> rte_cryptodev_pmd.h is an interface between driver and library and it is
> mentioned in the file that application cannot use it directly.
> Hence, removing the include.
> 
> Signed-off-by: Akhil Goyal 
Acked-by: Matan Azrad 

Re: [dpdk-dev] [PATCH v2 2/4] cryptodev: change valid dev API

[Matan Azrad]

From: Akhil Goyal
> The API rte_cryptodev_pmd_is_valid_dev, can be used by the application as
> well as PMD to check whether the device is valid or not. Hence, _pmd is
> removed from the API.
> The applications and drivers which use this API are also updated.
> 
> Signed-off-by: Akhil Goyal 

Agree.

What's about updating the release notes on this change?

Matan

> ---
>  .../net/softnic/rte_eth_softnic_cryptodev.c   |  2 +-
>  examples/fips_validation/main.c   |  2 +-
>  examples/ip_pipeline/cryptodev.c  |  3 +-
>  lib/cryptodev/rte_cryptodev.c | 50 +--
>  lib/cryptodev/rte_cryptodev.h | 11 
>  lib/cryptodev/rte_cryptodev_pmd.h | 11 
>  lib/cryptodev/version.map |  2 +-
>  lib/eventdev/rte_event_crypto_adapter.c   |  4 +-
>  lib/eventdev/rte_eventdev.c   |  2 +-
>  lib/pipeline/rte_table_action.c   |  2 +-
>  10 files changed, 44 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/net/softnic/rte_eth_softnic_cryptodev.c
> b/drivers/net/softnic/rte_eth_softnic_cryptodev.c
> index a1a4ca5650..8e278801c5 100644
> --- a/drivers/net/softnic/rte_eth_softnic_cryptodev.c
> +++ b/drivers/net/softnic/rte_eth_softnic_cryptodev.c
> @@ -82,7 +82,7 @@ softnic_cryptodev_create(struct pmd_internals *p,
> 
> dev_id = (uint32_t)status;
> } else {
> -   if (rte_cryptodev_pmd_is_valid_dev(params->dev_id) == 0)
> +   if (rte_cryptodev_is_valid_dev(params->dev_id) == 0)
> return NULL;
> 
> dev_id = params->dev_id; diff --git
> a/examples/fips_validation/main.c b/examples/fips_validation/main.c index
> c175fe6ac2..e892078f0e 100644
> --- a/examples/fips_validation/main.c
> +++ b/examples/fips_validation/main.c
> @@ -196,7 +196,7 @@ parse_cryptodev_id_arg(char *arg)
> }
> 
> 
> -   if (!rte_cryptodev_pmd_is_valid_dev(cryptodev_id)) {
> +   if (!rte_cryptodev_is_valid_dev(cryptodev_id)) {
> RTE_LOG(ERR, USER1, "Error %i: invalid cryptodev id %s\n",
> cryptodev_id, arg);
> return -1;
> diff --git a/examples/ip_pipeline/cryptodev.c
> b/examples/ip_pipeline/cryptodev.c
> index b0d9f3d217..9997d97456 100644
> --- a/examples/ip_pipeline/cryptodev.c
> +++ b/examples/ip_pipeline/cryptodev.c
> @@ -6,7 +6,6 @@
>  #include 
> 
>  #include 
> -#include 
>  #include 
> 
>  #include "cryptodev.h"
> @@ -74,7 +73,7 @@ cryptodev_create(const char *name, struct
> cryptodev_params *params)
> 
> dev_id = (uint32_t)status;
> } else {
> -   if (rte_cryptodev_pmd_is_valid_dev(params->dev_id) == 0)
> +   if (rte_cryptodev_is_valid_dev(params->dev_id) == 0)
> return NULL;
> 
> dev_id = params->dev_id; diff --git 
> a/lib/cryptodev/rte_cryptodev.c
> b/lib/cryptodev/rte_cryptodev.c index 447aa9d519..37502b9b3c 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -663,7 +663,7 @@ rte_cryptodev_is_valid_device_data(uint8_t dev_id)
> }
> 
>  unsigned int
> -rte_cryptodev_pmd_is_valid_dev(uint8_t dev_id)
> +rte_cryptodev_is_valid_dev(uint8_t dev_id)
>  {
> struct rte_cryptodev *dev = NULL;
> 
> @@ -761,7 +761,7 @@ rte_cryptodev_socket_id(uint8_t dev_id)  {
> struct rte_cryptodev *dev;
> 
> -   if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
> +   if (!rte_cryptodev_is_valid_dev(dev_id))
> return -1;
> 
> dev = rte_cryptodev_pmd_get_dev(dev_id);
> @@ -1032,7 +1032,7 @@ rte_cryptodev_configure(uint8_t dev_id, struct
> rte_cryptodev_config *config)
> struct rte_cryptodev *dev;
> int diag;
> 
> -   if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +   if (!rte_cryptodev_is_valid_dev(dev_id)) {
> CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> return -EINVAL;
> }
> @@ -1080,7 +1080,7 @@ rte_cryptodev_start(uint8_t dev_id)
> 
> CDEV_LOG_DEBUG("Start dev_id=%" PRIu8, dev_id);
> 
> -   if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +   if (!rte_cryptodev_is_valid_dev(dev_id)) {
> CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> return -EINVAL;
> }
> @@ -1110,7 +1110,7 @@ rte_cryptodev_stop(uint8_t dev_id)  {
> struct rte_cryptodev *dev;
> 
> -   if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +   if (!rte_cryptodev_is_valid_dev(dev_id)) {
> CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
> return;
> }
> @@ -1136,7 +1136,7 @@ rte_cryptodev_close(uint8_t dev_id)
> struct rte_cryptodev *dev;
> int retval;
> 
> -   if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) {
> +   if (!rte_cryptodev_is_valid_dev(dev_id)) {
> CDEV_LOG_ERR("Invalid dev_id=%" PRIu8, dev_id);
>

Re: [dpdk-dev] [PATCH v2 3/4] examples/fips_validation: remove illegal usage of APIs

[Matan Azrad]

From: Akhil Goyal
> Some of the cryptodev APIs are not allowed to be used by application
> directly. Hence removing the usage of 1. queue_pair_release: it is not
> required, as configure
>of queue pair release the previous queue pairs and the
>dev is not directly exposed to application, hence cannot
>use its ops from app.
> 2. rte_cryptodev_stop: it can be used directly without
>checking if the device is started or not.
> 3. rte_cryptodev_pmd_destroy: application should use
>rte_cryptodev_close instead.
> 
> Signed-off-by: Akhil Goyal 

Look's like it should be backported to stable releases with a Fixes reference.
What do you think?

Besides,
Acked-by: Matan Azrad 

> ---
>  examples/fips_validation/fips_dev_self_test.c | 19 ++-
>  examples/fips_validation/main.c   |  7 ++-
>  2 files changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/examples/fips_validation/fips_dev_self_test.c
> b/examples/fips_validation/fips_dev_self_test.c
> index 17e85973c0..b4eab05a98 100644
> --- a/examples/fips_validation/fips_dev_self_test.c
> +++ b/examples/fips_validation/fips_dev_self_test.c
> @@ -3,7 +3,7 @@
>   */
> 
>  #include 
> -#include 
> +#include 
> 
>  #include "fips_dev_self_test.h"
> 
> @@ -1523,12 +1523,6 @@ static void
>  fips_dev_auto_test_uninit(uint8_t dev_id,
> struct fips_dev_auto_test_env *env)  {
> -   struct rte_cryptodev *dev = rte_cryptodev_pmd_get_dev(dev_id);
> -   uint32_t i;
> -
> -   if (!dev)
> -   return;
> -
> if (env->mbuf)
> rte_pktmbuf_free(env->mbuf);
> if (env->op)
> @@ -1542,16 +1536,7 @@ fips_dev_auto_test_uninit(uint8_t dev_id,
> if (env->sess_priv_pool)
> rte_mempool_free(env->sess_priv_pool);
> 
> -   if (dev->data->dev_started)
> -   rte_cryptodev_stop(dev_id);
> -
> -   if (dev->data->nb_queue_pairs) {
> -   for (i = 0; i < dev->data->nb_queue_pairs; i++)
> -   (*dev->dev_ops->queue_pair_release)(dev, i);
> -   dev->data->nb_queue_pairs = 0;
> -   rte_free(dev->data->queue_pairs);
> -   dev->data->queue_pairs = NULL;
> -   }
> +   rte_cryptodev_stop(dev_id);
>  }
> 
>  static int
> diff --git a/examples/fips_validation/main.c
> b/examples/fips_validation/main.c index e892078f0e..a8daad1f48 100644
> --- a/examples/fips_validation/main.c
> +++ b/examples/fips_validation/main.c
> @@ -7,7 +7,7 @@
>  #include 
> 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -73,10 +73,7 @@ cryptodev_fips_validate_app_int(void)
> if (env.self_test) {
> ret = fips_dev_self_test(env.dev_id, env.broken_test_config);
> if (ret < 0) {
> -   struct rte_cryptodev *cryptodev =
> -   rte_cryptodev_pmd_get_dev(env.dev_id);
> -
> -   rte_cryptodev_pmd_destroy(cryptodev);
> +   rte_cryptodev_close(env.dev_id);
> 
> return ret;
> }
> --
> 2.25.1

Re: [dpdk-dev] [PATCH v2 4/4] cryptodev: expose driver interface as internal

[Matan Azrad]

From: Akhil Goyal
> The rte_cryptodev_pmd.* files are for drivers only and should be private to
> DPDK, and not installed for app use.
> 
> Signed-off-by: Akhil Goyal 

Please consider adding to release notes.

Acked-by: Matan Azrad 

Re: [dpdk-dev] [PATCH v1] examples/l3fwd: fix jumbo packet drop issue

[Ferruh Yigit]

On 7/27/2021 10:25 AM, rohit@nxp.com wrote:
> From: Rohit Raj 
> 
> l3fwd uses mbufs with 2KB data size. If we enable jumbo packets, it is
> not able to store packets with size greater than 2KB, hence these
> packets are dropped.
> 
> This patch fixes this issue by enabling scatter for jumbo packet, if
> it is supported by NIC.
> 
> If scatter is not supported by NIC and max jumbo packet length is
> greater than default mbuf data size, then application exits with
> proper error message.
> 
> Fixes: f68aad7904f ("examples/l3fwd: update")
> 
> Signed-off-by: Rohit Raj 
> Signed-off-by: Sachin Saxena 
> Signed-off-by: Vanshika Shukla 
> ---
>  examples/l3fwd/main.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 4cb800aa15..68ecb5 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -1035,6 +1035,20 @@ l3fwd_poll_resource_setup(void)
>   "Error during getting device (port %u) info: 
> %s\n",
>   portid, strerror(-ret));
>  
> + /* Enable Receive side SCATTER, if supported by NIC,
> +  * when jumbo packet is enabled.
> +  */
> + if (local_port_conf.rxmode.offloads &
> + DEV_RX_OFFLOAD_JUMBO_FRAME){
> + if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_SCATTER)
> + local_port_conf.rxmode.offloads |=
> + DEV_RX_OFFLOAD_SCATTER;
> + else if (local_port_conf.rxmode.max_rx_pkt_len >
> + RTE_MBUF_DEFAULT_DATAROOM)
> + rte_exit(EXIT_FAILURE,
> + "Max packet length greater than default 
> MBUF size\n");

This is a configuration set by application. So application is failing itself
because of configuration it sets, seems odd.

I guess the jumbo frame can be enabled when user provides '--enable-jumbo'
argument. What do you think adding above check where that argument is parsed.

Btw, no need to enable scattered Rx if the packets fits into buffer, so above
check can be done slightly different:

if (max_rx_pkt_len > buffer_size)
if (OFFLOAD_SCATTER supported)
enable OFFLOAD_SCATTER
else
fail


> + }
> +
>   if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
>   local_port_conf.txmode.offloads |=
>   DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> 

Re: [dpdk-dev] [RFC] ethdev: change queue release callback

[Ferruh Yigit]

On 8/10/2021 10:07 AM, Xueming(Steven) Li wrote:
> 
> 
>> -Original Message-
>> From: Ferruh Yigit 
>> Sent: Tuesday, August 10, 2021 4:54 PM
>> To: Xueming(Steven) Li ; Singh, Aman Deep 
>> ; Andrew Rybchenko
>> 
>> Cc: dev@dpdk.org; Slava Ovsiienko ; 
>> NBU-Contact-Thomas Monjalon 
>> Subject: Re: [dpdk-dev] [RFC] ethdev: change queue release callback
>>
>> On 8/10/2021 9:03 AM, Xueming(Steven) Li wrote:
>>> Hi Singh and Ferruh,
>>>
 -Original Message-
 From: Ferruh Yigit 
 Sent: Monday, August 9, 2021 11:31 PM
 To: Singh, Aman Deep ; Andrew Rybchenko
 ; Xueming(Steven) Li
 
 Cc: dev@dpdk.org; Slava Ovsiienko ;
 NBU-Contact-Thomas Monjalon 
 Subject: Re: [dpdk-dev] [RFC] ethdev: change queue release callback

 On 8/9/2021 3:39 PM, Singh, Aman Deep wrote:
> Hi Xueming,
>
> On 7/28/2021 1:10 PM, Andrew Rybchenko wrote:
>> On 7/27/21 6:41 AM, Xueming Li wrote:
>>> To align with other eth device queue configuration callbacks,
>>> change RX and TX queue release callback API parameter from queue
>>> object to device and queue index.
>>>
>>> Signed-off-by: Xueming Li 
>>
>> In fact, there is no strong reasons to do it, but I think it is a
>> nice cleanup to use (dev + queue index) on control path.
>>
>> Hopefully it will not result in any regressions.
>
> Combined there are 100+ API's for Rx/Tx queue_release that need to
> be modified for it.
>
> I believe all regression possibilities here will be caught, in
> compilation phase itself.
>

 Same here, it is a good cleanup but there is no strong reason for it.

 Since it is all internal, there is no ABI restriction on the patch,
 and v21.11 will be full ABI break patches, to not cause conflicts with 
 this change, what would you think to have it on v22.02?
>>>
>>> This patch is required by shared-rxq feature which ABI broken, target to 
>>> 21.11.
>>
>> Why it is required?
> 
> In rx burst function, rxq object is used in data path. For best data 
> performance, it's shared-rxq object in case of shared rxq enabled.
> I think eth api defined rxq object for performance as well, specific on data 
> plane. 
> Hardware saves port info received packet descriptor for my case.
> Can't tell which device's queue with this shared rxq object, control path 
> can't use this shared rxq anymore, have to be specific on dev and queue id.
> 

I have seen shared Rx queue patch, but that just introduces the offload and
doesn't have the PMD implementation, so hard to see the dependency, can you
please put the pseudocode for PMDs for shared-rxq?
How a queue will know if it is shared or not, during release?

Btw, shared Rx doesn't mention from this dependency in the patch.

>>
>>> I'll do it carefully, fortunately, the change is straightforward.
>>>
> 

Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue

[Ferruh Yigit]

On 8/11/2021 9:28 AM, Xueming(Steven) Li wrote:
> 
> 
>> -Original Message-
>> From: Jerin Jacob 
>> Sent: Wednesday, August 11, 2021 4:03 PM
>> To: Xueming(Steven) Li 
>> Cc: dpdk-dev ; Ferruh Yigit ; 
>> NBU-Contact-Thomas Monjalon ;
>> Andrew Rybchenko 
>> Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue
>>
>> On Mon, Aug 9, 2021 at 7:46 PM Xueming(Steven) Li  
>> wrote:
>>>
>>> Hi,
>>>
 -Original Message-
 From: Jerin Jacob 
 Sent: Monday, August 9, 2021 9:51 PM
 To: Xueming(Steven) Li 
 Cc: dpdk-dev ; Ferruh Yigit ;
 NBU-Contact-Thomas Monjalon ; Andrew Rybchenko
 
 Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue

 On Mon, Aug 9, 2021 at 5:18 PM Xueming Li  wrote:
>
> In current DPDK framework, each RX queue is pre-loaded with mbufs
> for incoming packets. When number of representors scale out in a
> switch domain, the memory consumption became significant. Most
> important, polling all ports leads to high cache miss, high
> latency and low throughput.
>
> This patch introduces shared RX queue. Ports with same
> configuration in a switch domain could share RX queue set by specifying 
> sharing group.
> Polling any queue using same shared RX queue receives packets from
> all member ports. Source port is identified by mbuf->port.
>
> Port queue number in a shared group should be identical. Queue
> index is
> 1:1 mapped in shared group.
>
> Share RX queue is supposed to be polled on same thread.
>
> Multiple groups is supported by group ID.

 Is this offload specific to the representor? If so can this name be 
 changed specifically to representor?
>>>
>>> Yes, PF and representor in switch domain could take advantage.
>>>
 If it is for a generic case, how the flow ordering will be maintained?
>>>
>>> Not quite sure that I understood your question. The control path of is
>>> almost same as before, PF and representor port still needed, rte flows not 
>>> impacted.
>>> Queues still needed for each member port, descriptors(mbuf) will be
>>> supplied from shared Rx queue in my PMD implementation.
>>
>> My question was if create a generic RTE_ETH_RX_OFFLOAD_SHARED_RXQ offload, 
>> multiple ethdev receive queues land into the same
>> receive queue, In that case, how the flow order is maintained for respective 
>> receive queues.
> 
> I guess the question is testpmd forward stream? The forwarding logic has to 
> be changed slightly in case of shared rxq.
> basically for each packet in rx_burst result, lookup source stream according 
> to mbuf->port, forwarding to target fs.
> Packets from same source port could be grouped as a small burst to process, 
> this will accelerates the performance if traffic come from
> limited ports. I'll introduce some common api to do shard rxq forwarding, 
> call it with packets handling callback, so it suites for
> all forwarding engine. Will sent patches soon.
> 

All ports will put the packets in to the same queue (share queue), right? Does
this means only single core will poll only, what will happen if there are
multiple cores polling, won't it cause problem?

And if this requires specific changes in the application, I am not sure about
the solution, can't this work in a transparent way to the application?

Overall, is this for optimizing memory for the port represontors? If so can't we
have a port representor specific solution, reducing scope can reduce the
complexity it brings?

>> If this offload is only useful for representor case, Can we make this 
>> offload specific to representor the case by changing its name and
>> scope.
> 
> It works for both PF and representors in same switch domain, for application 
> like OVS, few changes to apply.
> 
>>
>>
>>>

>
> Signed-off-by: Xueming Li 
> ---
>  doc/guides/nics/features.rst| 11 +++
>  doc/guides/nics/features/default.ini|  1 +
>  doc/guides/prog_guide/switch_representation.rst | 10 ++
>  lib/ethdev/rte_ethdev.c |  1 +
>  lib/ethdev/rte_ethdev.h |  7 +++
>  5 files changed, 30 insertions(+)
>
> diff --git a/doc/guides/nics/features.rst
> b/doc/guides/nics/features.rst index a96e12d155..2e2a9b1554 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -624,6 +624,17 @@ Supports inner packet L4 checksum.
>
> ``tx_offload_capa,tx_queue_offload_capa:DEV_TX_OFFLOAD_OUTER_UDP_CKSUM``.
>
>
> +.. _nic_features_shared_rx_queue:
> +
> +Shared Rx queue
> +---
> +
> +Supports shared Rx queue for ports in same switch domain.
> +
> +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: 
> ``offloads:RTE_ETH_RX_OFFLOAD_SHARED_RXQ``.
> +* **[provides] mbuf**: ``mbuf.port``.
> +
> +
>  

Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue

[Xueming(Steven) Li]

> -Original Message-
> From: Jerin Jacob 
> Sent: Wednesday, August 11, 2021 4:03 PM
> To: Xueming(Steven) Li 
> Cc: dpdk-dev ; Ferruh Yigit ; 
> NBU-Contact-Thomas Monjalon ;
> Andrew Rybchenko 
> Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue
> 
> On Mon, Aug 9, 2021 at 7:46 PM Xueming(Steven) Li  wrote:
> >
> > Hi,
> >
> > > -Original Message-
> > > From: Jerin Jacob 
> > > Sent: Monday, August 9, 2021 9:51 PM
> > > To: Xueming(Steven) Li 
> > > Cc: dpdk-dev ; Ferruh Yigit ;
> > > NBU-Contact-Thomas Monjalon ; Andrew Rybchenko
> > > 
> > > Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue
> > >
> > > On Mon, Aug 9, 2021 at 5:18 PM Xueming Li  wrote:
> > > >
> > > > In current DPDK framework, each RX queue is pre-loaded with mbufs
> > > > for incoming packets. When number of representors scale out in a
> > > > switch domain, the memory consumption became significant. Most
> > > > important, polling all ports leads to high cache miss, high
> > > > latency and low throughput.
> > > >
> > > > This patch introduces shared RX queue. Ports with same
> > > > configuration in a switch domain could share RX queue set by specifying 
> > > > sharing group.
> > > > Polling any queue using same shared RX queue receives packets from
> > > > all member ports. Source port is identified by mbuf->port.
> > > >
> > > > Port queue number in a shared group should be identical. Queue
> > > > index is
> > > > 1:1 mapped in shared group.
> > > >
> > > > Share RX queue is supposed to be polled on same thread.
> > > >
> > > > Multiple groups is supported by group ID.
> > >
> > > Is this offload specific to the representor? If so can this name be 
> > > changed specifically to representor?
> >
> > Yes, PF and representor in switch domain could take advantage.
> >
> > > If it is for a generic case, how the flow ordering will be maintained?
> >
> > Not quite sure that I understood your question. The control path of is
> > almost same as before, PF and representor port still needed, rte flows not 
> > impacted.
> > Queues still needed for each member port, descriptors(mbuf) will be
> > supplied from shared Rx queue in my PMD implementation.
> 
> My question was if create a generic RTE_ETH_RX_OFFLOAD_SHARED_RXQ offload, 
> multiple ethdev receive queues land into the same
> receive queue, In that case, how the flow order is maintained for respective 
> receive queues.

I guess the question is testpmd forward stream? The forwarding logic has to be 
changed slightly in case of shared rxq.
basically for each packet in rx_burst result, lookup source stream according to 
mbuf->port, forwarding to target fs.
Packets from same source port could be grouped as a small burst to process, 
this will accelerates the performance if traffic come from
limited ports. I'll introduce some common api to do shard rxq forwarding, call 
it with packets handling callback, so it suites for
all forwarding engine. Will sent patches soon.

> If this offload is only useful for representor case, Can we make this offload 
> specific to representor the case by changing its name and
> scope.

It works for both PF and representors in same switch domain, for application 
like OVS, few changes to apply.

> 
> 
> >
> > >
> > > >
> > > > Signed-off-by: Xueming Li 
> > > > ---
> > > >  doc/guides/nics/features.rst| 11 +++
> > > >  doc/guides/nics/features/default.ini|  1 +
> > > >  doc/guides/prog_guide/switch_representation.rst | 10 ++
> > > >  lib/ethdev/rte_ethdev.c |  1 +
> > > >  lib/ethdev/rte_ethdev.h |  7 +++
> > > >  5 files changed, 30 insertions(+)
> > > >
> > > > diff --git a/doc/guides/nics/features.rst
> > > > b/doc/guides/nics/features.rst index a96e12d155..2e2a9b1554 100644
> > > > --- a/doc/guides/nics/features.rst
> > > > +++ b/doc/guides/nics/features.rst
> > > > @@ -624,6 +624,17 @@ Supports inner packet L4 checksum.
> > > >
> > > > ``tx_offload_capa,tx_queue_offload_capa:DEV_TX_OFFLOAD_OUTER_UDP_CKSUM``.
> > > >
> > > >
> > > > +.. _nic_features_shared_rx_queue:
> > > > +
> > > > +Shared Rx queue
> > > > +---
> > > > +
> > > > +Supports shared Rx queue for ports in same switch domain.
> > > > +
> > > > +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: 
> > > > ``offloads:RTE_ETH_RX_OFFLOAD_SHARED_RXQ``.
> > > > +* **[provides] mbuf**: ``mbuf.port``.
> > > > +
> > > > +
> > > >  .. _nic_features_packet_type_parsing:
> > > >
> > > >  Packet type parsing
> > > > diff --git a/doc/guides/nics/features/default.ini
> > > > b/doc/guides/nics/features/default.ini
> > > > index 754184ddd4..ebeb4c1851 100644
> > > > --- a/doc/guides/nics/features/default.ini
> > > > +++ b/doc/guides/nics/features/default.ini
> > > > @@ -19,6 +19,7 @@ Free Tx mbuf on demand =
> > > >  Queue start/stop =
> > > >  Runtime Rx queue setup =
> > > >  Runtime Tx queue setup =
> > > > +Shared Rx queue  =
> > 

[dpdk-dev] Failsafe Secondary Process

[kumaraparameshwaran rathinavel]

Hi All,
In the commit ee27edbe0c10ec8337c4cc4d2935a751d0da605f I see that for the
probe from the secondary process the below check was made, for vdev devices
like tap, af_packet.

+ if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
+   strlen(rte_vdev_device_args(dev)) == 0) {
+   eth_dev = rte_eth_dev_attach_secondary(name);
+   if (!eth_dev) {
+   RTE_LOG(ERR, PMD, "Failed to probe %s\n", name);
+   return -1;
+   }
+   /* TODO: request info from primary to set up Rx and Tx */
+   eth_dev->dev_ops = &ops;
+   return 0;
+   }

After that in the commit 4852aa8f6e2125664698afc43b820bd787b02756 the
strlen(rte_vdev_device_args(dev)) check for the secondary was removed so
that the secondary process could attach to the devices created by the
primary and work on it.
-   if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
-   strlen(rte_vdev_device_args(dev)) == 0) {
+   if (rte_eal_process_type() == RTE_PROC_SECONDARY) {


But I see in the rte_pmd_failsafe_probe looks like the strlen check was not
removed. Because of this when i try to use initialise the failsafe device
from secondary which was created by primary, rather than attaching to the
existing device, a new failsafe device is created. Looks like this is a
miss for the failsafe driver or is it intentionally retained?


commit ee27edbe0c10ec8337c4cc4d2935a751d0da605f
Date:   Tue Apr 24 05:51:24 2018 +

drivers/net: share vdev data to secondary process

dpdk-procinfo, as a secondary process, cannot fetch stats for vdev.

This patch enables that by attaching the port from the shared data.
We also fill the eth dev ops, with only some ops works in secondary
process, for example, stats_get().


commit 4852aa8f6e2125664698afc43b820bd787b02756
Date:   Tue Oct 16 08:16:30 2018 +0800

drivers/net: enable hotplug on secondary process

Attach port from secondary should ignore devargs since the private
device is not necessary to support. Also previously, detach port on
a secondary process will mess primary process and cause the same
device can't be attached back again. A secondary process should use
rte_eth_dev_release_port_secondary to release a port.

Thanks,
Param.

[dpdk-dev] [PATCHv2] include: fix sys/queue.h.

[William Tu]

Currently there are a couple of header files include 'sys/queue.h',
which is a POSIX functionality.  When compiling DPDK with OVS on
Windows, we encountered issues such as, found the missing header.
In file included from ../lib/dpdk.c:27:
C:\temp\dpdk\include\rte_log.h:24:10: fatal error: 'sys/queue.h' file
not found

The patch fixes it by 1) removing the #include  from
a couple of headers, replace it with  #include , and
2) include sys/queue.h in the rte_os.h. As a result, for Linux/BSD,
it is using the sys/queue.h from its POSIX library, and for Windows,
DPDK will use the bundled sys/queue.h.

1) fixes the case that DPDK library shouldn't export POSIX functionality
into the environment (symbols, macros, headers etc), which cause definitions
clashing with the application.
2) fixes the case that DPDK should depend only on the C library and not
require POSIX functionality from the underlying system.

There are still a couple of headers using sys/queue.h, ex:
rte_bus_pci.h. Since it's not been used in Windows yet, we can
fix them later.

Suggested-by: Nick Connolly 
Suggested-by: Dmitry Kozliuk 
Signed-off-by: William Tu 
---
v1->v2:
  - follow the suggestion by Nick and Dmitry 
  - http://mails.dpdk.org/archives/dev/2021-August/216304.html
---
 lib/eal/freebsd/include/rte_os.h| 1 +
 lib/eal/include/rte_bus.h   | 2 +-
 lib/eal/include/rte_class.h | 2 --
 lib/eal/include/rte_dev.h   | 2 +-
 lib/eal/include/rte_devargs.h   | 1 -
 lib/eal/include/rte_log.h   | 1 -
 lib/eal/include/rte_service.h   | 1 -
 lib/eal/include/rte_tailq.h | 2 +-
 lib/eal/linux/include/rte_os.h  | 1 +
 lib/eal/windows/include/meson.build | 4 
 lib/eal/windows/include/rte_os.h| 1 +
 lib/eal/windows/include/sys/meson.build | 6 ++
 lib/mempool/rte_mempool.h   | 1 -
 13 files changed, 16 insertions(+), 9 deletions(-)
 create mode 100644 lib/eal/windows/include/sys/meson.build

diff --git a/lib/eal/freebsd/include/rte_os.h b/lib/eal/freebsd/include/rte_os.h
index 627f0483ab..c0fa9791a1 100644
--- a/lib/eal/freebsd/include/rte_os.h
+++ b/lib/eal/freebsd/include/rte_os.h
@@ -11,6 +11,7 @@
  */
 
 #include 
+#include 
 
 typedef cpuset_t rte_cpuset_t;
 #define RTE_HAS_CPUSET
diff --git a/lib/eal/include/rte_bus.h b/lib/eal/include/rte_bus.h
index 80b154fb98..8e90d768bd 100644
--- a/lib/eal/include/rte_bus.h
+++ b/lib/eal/include/rte_bus.h
@@ -19,8 +19,8 @@ extern "C" {
 #endif
 
 #include 
-#include 
 
+#include 
 #include 
 #include 
 
diff --git a/lib/eal/include/rte_class.h b/lib/eal/include/rte_class.h
index 856d09b22d..1430c62943 100644
--- a/lib/eal/include/rte_class.h
+++ b/lib/eal/include/rte_class.h
@@ -22,8 +22,6 @@
 extern "C" {
 #endif
 
-#include 
-
 #include 
 
 /** Double linked list of classes */
diff --git a/lib/eal/include/rte_dev.h b/lib/eal/include/rte_dev.h
index 6dd72c11a1..7d1bf32a42 100644
--- a/lib/eal/include/rte_dev.h
+++ b/lib/eal/include/rte_dev.h
@@ -18,8 +18,8 @@ extern "C" {
 #endif
 
 #include 
-#include 
 
+#include 
 #include 
 #include 
 #include 
diff --git a/lib/eal/include/rte_devargs.h b/lib/eal/include/rte_devargs.h
index cd90944fe8..9ec2786812 100644
--- a/lib/eal/include/rte_devargs.h
+++ b/lib/eal/include/rte_devargs.h
@@ -21,7 +21,6 @@ extern "C" {
 #endif
 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/lib/eal/include/rte_log.h b/lib/eal/include/rte_log.h
index b706bb8710..bb3523467b 100644
--- a/lib/eal/include/rte_log.h
+++ b/lib/eal/include/rte_log.h
@@ -21,7 +21,6 @@ extern "C" {
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
diff --git a/lib/eal/include/rte_service.h b/lib/eal/include/rte_service.h
index c7d037d862..1c9275c32a 100644
--- a/lib/eal/include/rte_service.h
+++ b/lib/eal/include/rte_service.h
@@ -29,7 +29,6 @@ extern "C" {
 
 #include
 #include 
-#include 
 
 #include 
 #include 
diff --git a/lib/eal/include/rte_tailq.h b/lib/eal/include/rte_tailq.h
index b6fe4e5f78..b948ccd45c 100644
--- a/lib/eal/include/rte_tailq.h
+++ b/lib/eal/include/rte_tailq.h
@@ -15,8 +15,8 @@
 extern "C" {
 #endif
 
-#include 
 #include 
+#include 
 #include 
 
 /** dummy structure type used by the rte_tailq APIs */
diff --git a/lib/eal/linux/include/rte_os.h b/lib/eal/linux/include/rte_os.h
index 1618b4df22..9077e9b614 100644
--- a/lib/eal/linux/include/rte_os.h
+++ b/lib/eal/linux/include/rte_os.h
@@ -11,6 +11,7 @@
  */
 
 #include 
+#include 
 
 #ifdef CPU_SETSIZE /* may require _GNU_SOURCE */
 typedef cpu_set_t rte_cpuset_t;
diff --git a/lib/eal/windows/include/meson.build 
b/lib/eal/windows/include/meson.build
index b3534b025f..875cc1cf0d 100644
--- a/lib/eal/windows/include/meson.build
+++ b/lib/eal/windows/include/meson.build
@@ -8,3 +8,7 @@ headers += files(
 'rte_virt2phys.h',
 'rte_windows.h',
 )
+
+sys_headers = []
+subdir('sys')
+install_headers(sys_headers, subdir: 'sys')
diff --git a/lib/eal/windows/include/rte_os.h b/lib

Re: [dpdk-dev] [PATCH] eal/windows: add sys/queue.h.

[William Tu]

On Wed, Aug 11, 2021 at 1:34 AM Nick Connolly  wrote:
>
>
> > What we can do:
> >
> > 1. Introduce `rte_queue.h` (name can be better) that is env-specific:
> >
> > 1.1. For Linux and FreeBSD it just includes 
> >  and renames a few macros that are used in headers to RTE_xxx.
> > 1.2. For Windows it defines the same RTE_xxx macros in a way
> >  compatible with the  version used to build DPDK.
> >
> > 2. Add #include  in :
> > Linux and FreeBSD will include a system header,
> > Windows will use the bundled one.
> >
> > This way application are not exposed to non-RTE symbols,
> > at the same time RTE_xxx are binary-compatible with what DPDK
> > implementation expects (and outside of Windows there is no change in fact).
>
> +1
> Nick

Hi Nick and Dmitry,
Thanks for such a detailed explanation!
I sent the v2 patch, hopefully I understand you guys correctly.
William

[dpdk-dev] [PATCH v3 1/2] devtools: fix version pattern for fix search

[Xueming Li]

When scanning fixes from current(HEAD) branch, local tags were included
and reported as version. For example:
  $ git tag --contains  --merged
  20.11_backport_202010506   // user tag
  v20.11
  v20.11.1

This patch matches DPDK officail version pattern in search, selects
the most early tag. Official tag pattern: "v."

Fixes: 752d8e097ec1 ("scripts: show fixes with release version of bug")
Cc: Thomas Monjalon 
Cc: sta...@dpdk.org

Signed-off-by: Xueming Li 
Reviewed-by: Christian Ehrhardt 
---
 devtools/git-log-fixes.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/devtools/git-log-fixes.sh b/devtools/git-log-fixes.sh
index 210c8dcf25..153ba5b438 100755
--- a/devtools/git-log-fixes.sh
+++ b/devtools/git-log-fixes.sh
@@ -38,12 +38,13 @@ range="$*"
 # get major release version of a commit
 commit_version () # 
 {
+   local VER="v*.*"
# use current branch as history reference
local refbranch=$(git rev-parse --abbrev-ref HEAD)
-   local tag=$( (git tag -l --contains $1 --merged $refbranch 2>&- ||
+   local tag=$( (git tag -l "$VER" --contains $1 --sort=creatordate 
--merged $refbranch 2>&- ||
# tag --merged option has been introduced in git 2.7.0
# below is a fallback in case of old git version
-   for t in $(git tag -l --contains $1) ; do
+   for t in $(git tag -l "$VER" --contains $1) ; do
git branch $refbranch --contains $t |
sed "s,.\+,$t,"
done) |
-- 
2.25.1

[dpdk-dev] [PATCH v3 2/2] devtools: fix patches missing if range newer than HEAD

[Xueming Li]

Current fix scan scripts used HEAD branch as history reference.
When users ran it in an earlier branch, few patches were scanned
due to the fixes in the range are newer and not merged to current
branch.

This patch introduces optional  argument, default to HEAD
if not specified. Checks the  specified in parameter must
being merged in .

Fixes: 752d8e097ec1 ("scripts: show fixes with release version of bug")
Cc: Thomas Monjalon 
Cc: sta...@dpdk.org
Cc: Christian Ehrhardt 

Signed-off-by: Xueming Li 
---
 devtools/git-log-fixes.sh | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/devtools/git-log-fixes.sh b/devtools/git-log-fixes.sh
index 153ba5b438..dbed4b6419 100755
--- a/devtools/git-log-fixes.sh
+++ b/devtools/git-log-fixes.sh
@@ -4,7 +4,7 @@
 
 print_usage ()
 {
-   echo "usage: $(basename $0) [-h] "
+   echo "usage: $(basename $0) [-h]  []"
 }
 
 print_help ()
@@ -15,6 +15,7 @@ print_help ()
Find fixes to backport on previous versions.
It looks for the word "fix" in the headline or a tag "Fixes" or 
"Reverts".
The oldest bug origin is printed as well as partially fixed versions.
+   It looks into current branch or the branch specified.
END_OF_HELP
 }
 
@@ -33,14 +34,23 @@ while getopts h ARG ; do
 done
 shift $(($OPTIND - 1))
 [ $# -ge 1 ] || usage_error 'range argument required'
-range="$*"
+range="$1"
+branch="$2"
+
+# default to current branch as history reference
+[ -n "$branch" ] || branch="HEAD"
+# get real brnach name
+refbranch=$(git rev-parse --abbrev-ref $branch)
+range_last=$(git rev-parse $range | head -n1)
+if ! git branch -a --contains $range_last | grep -q -e " $refbranch$" -e " 
remotes/$refbranch$"; then
+   echo "range $range not included by branch $refbranch"
+   exit 1
+fi
 
 # get major release version of a commit
 commit_version () # 
 {
local VER="v*.*"
-   # use current branch as history reference
-   local refbranch=$(git rev-parse --abbrev-ref HEAD)
local tag=$( (git tag -l "$VER" --contains $1 --sort=creatordate 
--merged $refbranch 2>&- ||
# tag --merged option has been introduced in git 2.7.0
# below is a fallback in case of old git version
@@ -49,9 +59,11 @@ commit_version () # 
sed "s,.\+,$t,"
done) |
head -n1)
-   if [ -z "$tag" ] ; then
-   # before -rc1 tag of release in progress
-   cat VERSION | cut -d'.' -f-2
+   if [ -z "$tag" ]; then
+   if [ "$branch" = 'HEAD' ]; then
+   # before -rc1 tag of release in progress
+   cat VERSION | cut -d'.' -f-2
+   fi
else
echo $tag | sed 's,^v,,' | sed 's,-rc.*,,'
fi
-- 
2.25.1

Re: [dpdk-dev] [PATCHv2] include: fix sys/queue.h.

[Dmitry Kozlyuk]

Hi William,

2021-08-11 20:46 (UTC+), William Tu:
> Currently there are a couple of header files include 'sys/queue.h',
> which is a POSIX functionality.  When compiling DPDK with OVS on
> Windows, we encountered issues such as, found the missing header.
> In file included from ../lib/dpdk.c:27:
> C:\temp\dpdk\include\rte_log.h:24:10: fatal error: 'sys/queue.h' file
> not found
> 
> The patch fixes it by 1) removing the #include  from
> a couple of headers, replace it with  #include , and
> 2) include sys/queue.h in the rte_os.h. As a result, for Linux/BSD,
> it is using the sys/queue.h from its POSIX library, and for Windows,
> DPDK will use the bundled sys/queue.h.
> 
> 1) fixes the case that DPDK library shouldn't export POSIX functionality
> into the environment (symbols, macros, headers etc), which cause definitions
> clashing with the application.
> 2) fixes the case that DPDK should depend only on the C library and not
> require POSIX functionality from the underlying system.

Sorry, this is not exactly what was suggested here:

http://inbox.dpdk.org/dev/20210811013325.34c36220@sovereign/

The point was not to install . Its full content is not needed
for DPDK interface, only for implementation. Public headers only need a
handful of macros for list/tailq heads and links. Those macros should be
provided by DPDK, with RTE_ prefix. For Linux and FreeBSD it will just be:

#include 
#define RTE_TAILQ_ENTRY(type) TAILQ_ENTRY(type)
/* ... */

For Windows you would have to copy definitions from 
to some public header,  seems OK. All public headers that need
 macros must have them replaced with RTE_ version.
Implementation remains unchanged. No new files need to be installed,
because when DPDK is built on Windows, it uses the bundled ,
and when it is installed, only RTE_ macros you created are visible.

Macro documentation should clearly state that they are compatible with system
 for Linux and FreeBSD, and for Windows they are compatible with
the version used during build.

+Bruce, David, Ray, Tyler to confirm/object the idea.

> There are still a couple of headers using sys/queue.h, ex:
> rte_bus_pci.h. Since it's not been used in Windows yet, we can
> fix them later.

Another item I think of is a checkpatch rule to prohibit TAILQ_xxx, etc
macro in public headers after replacing them all.

> Suggested-by: Nick Connolly 
> Suggested-by: Dmitry Kozliuk 
> Signed-off-by: William Tu 
> ---
> v1->v2:
>   - follow the suggestion by Nick and Dmitry 
>   - http://mails.dpdk.org/archives/dev/2021-August/216304.html

Please send new versions as replies to the previous ones:

git send-email --in-reply-to MSGID ...

https://doc.dpdk.org/guides/contributing/patches.html

Re: [dpdk-dev] [v3, 0/3] common/cnxk: enable npa telemetry

[Power, Ciara]

Hi Gowrishankar,

>-Original Message-
>From: Gowrishankar Muthukrishnan 
>Sent: Tuesday 3 August 2021 09:06
>To: dev@dpdk.org
>Cc: Richardson, Bruce ; Power, Ciara
>; jer...@marvell.com; kirankum...@marvell.com;
>ndabilpu...@marvell.com; sk...@marvell.com; skotesh...@marvell.com;
>Gowrishankar Muthukrishnan 
>Subject: [v3, 0/3] common/cnxk: enable npa telemetry
>
>This patch series enables telemetry in NPA LF of cnxk.
>
>v3:
> - fixed format specifier for uintptr_t
>
>Gowrishankar Muthukrishnan (3):
>  telemetry: enable storing pointer value
>  test/telemetry: add unit tests for pointer value
>  common/cnxk: add telemetry endpoints to npa
>
> app/test/test_telemetry_data.c   | 125 +
> app/test/test_telemetry_json.c   |  29 ++-
> drivers/common/cnxk/cnxk_telemetry.h |  26 +++
> drivers/common/cnxk/cnxk_telemetry_npa.c | 227
>+++
> drivers/common/cnxk/meson.build  |   4 +
> drivers/common/cnxk/roc_platform.h   |   8 +
> lib/telemetry/rte_telemetry.h|  37 +++-
> lib/telemetry/telemetry.c|  21 ++-
> lib/telemetry/telemetry_data.c   |  40 +++-
> lib/telemetry/telemetry_data.h   |   2 +
> lib/telemetry/telemetry_json.h   |  32 
> lib/telemetry/version.map|   2 +
> 12 files changed, 539 insertions(+), 14 deletions(-)  create mode 100644
>drivers/common/cnxk/cnxk_telemetry.h
> create mode 100644 drivers/common/cnxk/cnxk_telemetry_npa.c
>
>--
>2.25.1

I am still unsure exactly what the use case is here - why are we choosing to 
publish the pointer values through telemetry rather than using a debug log for 
example?
Maybe I am missing something here.

Thanks,
Ciara

Re: [dpdk-dev] [RFC] ethdev: change queue release callback

[Xueming(Steven) Li]

> -Original Message-
> From: Ferruh Yigit 
> Sent: Wednesday, August 11, 2021 7:58 PM
> To: Xueming(Steven) Li ; Singh, Aman Deep 
> ; Andrew Rybchenko
> 
> Cc: dev@dpdk.org; Slava Ovsiienko ; 
> NBU-Contact-Thomas Monjalon ;
> jer...@marvell.com
> Subject: Re: [dpdk-dev] [RFC] ethdev: change queue release callback
> 
> On 8/10/2021 10:07 AM, Xueming(Steven) Li wrote:
> >
> >
> >> -Original Message-
> >> From: Ferruh Yigit 
> >> Sent: Tuesday, August 10, 2021 4:54 PM
> >> To: Xueming(Steven) Li ; Singh, Aman Deep
> >> ; Andrew Rybchenko
> >> 
> >> Cc: dev@dpdk.org; Slava Ovsiienko ;
> >> NBU-Contact-Thomas Monjalon 
> >> Subject: Re: [dpdk-dev] [RFC] ethdev: change queue release callback
> >>
> >> On 8/10/2021 9:03 AM, Xueming(Steven) Li wrote:
> >>> Hi Singh and Ferruh,
> >>>
>  -Original Message-
>  From: Ferruh Yigit 
>  Sent: Monday, August 9, 2021 11:31 PM
>  To: Singh, Aman Deep ; Andrew Rybchenko
>  ; Xueming(Steven) Li
>  
>  Cc: dev@dpdk.org; Slava Ovsiienko ;
>  NBU-Contact-Thomas Monjalon 
>  Subject: Re: [dpdk-dev] [RFC] ethdev: change queue release callback
> 
>  On 8/9/2021 3:39 PM, Singh, Aman Deep wrote:
> > Hi Xueming,
> >
> > On 7/28/2021 1:10 PM, Andrew Rybchenko wrote:
> >> On 7/27/21 6:41 AM, Xueming Li wrote:
> >>> To align with other eth device queue configuration callbacks,
> >>> change RX and TX queue release callback API parameter from queue
> >>> object to device and queue index.
> >>>
> >>> Signed-off-by: Xueming Li 
> >>
> >> In fact, there is no strong reasons to do it, but I think it is a
> >> nice cleanup to use (dev + queue index) on control path.
> >>
> >> Hopefully it will not result in any regressions.
> >
> > Combined there are 100+ API's for Rx/Tx queue_release that need to
> > be modified for it.
> >
> > I believe all regression possibilities here will be caught, in
> > compilation phase itself.
> >
> 
>  Same here, it is a good cleanup but there is no strong reason for it.
> 
>  Since it is all internal, there is no ABI restriction on the patch,
>  and v21.11 will be full ABI break patches, to not cause conflicts with 
>  this change, what would you think to have it on v22.02?
> >>>
> >>> This patch is required by shared-rxq feature which ABI broken, target to 
> >>> 21.11.
> >>
> >> Why it is required?
> >
> > In rx burst function, rxq object is used in data path. For best data 
> > performance, it's shared-rxq object in case of shared rxq enabled.
> > I think eth api defined rxq object for performance as well, specific on 
> > data plane.
> > Hardware saves port info received packet descriptor for my case.
> > Can't tell which device's queue with this shared rxq object, control path 
> > can't use this shared rxq anymore, have to be specific on
> dev and queue id.
> >
> 
> I have seen shared Rx queue patch, but that just introduces the offload and 
> doesn't have the PMD implementation, so hard to see the
> dependency, can you please put the pseudocode for PMDs for shared-rxq?

The code is almost ready, I'll upload the PMD part soon.
But firstly, I'll upload v1 patch for this RFC, the make PMD patches depends on 
this v1 patch.

> How a queue will know if it is shared or not, during release?

That's why this RFC want to change callback parameter to device and queue id.
There is an offloading flag during rxq setup, either in device or in queue 
configuration.
PMD driver saves the flag and operate accordingly.
Ethdev api doesn't need to save this, unless a solid reason.

> 
> Btw, shared Rx doesn't mention from this dependency in the patch.

Agree, indeed a strong dependency, thanks!

> 
> >>
> >>> I'll do it carefully, fortunately, the change is straightforward.
> >>>
> >

Re: [dpdk-dev] [v3, 0/3] common/cnxk: enable npa telemetry

[Gowrishankar Muthukrishnan]

Hi Ciara,

> -Original Message-
> From: Power, Ciara 
> Sent: Wednesday, August 11, 2021 9:30 PM
> To: Gowrishankar Muthukrishnan ; dev@dpdk.org
> Cc: Richardson, Bruce ; Jerin Jacob Kollanukkaran
> ; Kiran Kumar Kokkilagadda ;
> Nithin Kumar Dabilpuram ; Sunil Kumar Kori
> ; Satha Koteswara Rao Kottidi
> 
> Subject: [EXT] RE: [v3, 0/3] common/cnxk: enable npa telemetry
> 
> External Email
> 
> --
> Hi Gowrishankar,
> 
> >-Original Message-
> >From: Gowrishankar Muthukrishnan 
> >Sent: Tuesday 3 August 2021 09:06
> >To: dev@dpdk.org
> >Cc: Richardson, Bruce ; Power, Ciara
> >; jer...@marvell.com; kirankum...@marvell.com;
> >ndabilpu...@marvell.com; sk...@marvell.com; skotesh...@marvell.com;
> >Gowrishankar Muthukrishnan 
> >Subject: [v3, 0/3] common/cnxk: enable npa telemetry
> >
> >This patch series enables telemetry in NPA LF of cnxk.
> >
> >v3:
> > - fixed format specifier for uintptr_t
> >
> >Gowrishankar Muthukrishnan (3):
> >  telemetry: enable storing pointer value
> >  test/telemetry: add unit tests for pointer value
> >  common/cnxk: add telemetry endpoints to npa
> >
> > app/test/test_telemetry_data.c   | 125 +
> > app/test/test_telemetry_json.c   |  29 ++-
> > drivers/common/cnxk/cnxk_telemetry.h |  26 +++
> > drivers/common/cnxk/cnxk_telemetry_npa.c | 227
> >+++
> > drivers/common/cnxk/meson.build  |   4 +
> > drivers/common/cnxk/roc_platform.h   |   8 +
> > lib/telemetry/rte_telemetry.h|  37 +++-
> > lib/telemetry/telemetry.c|  21 ++-
> > lib/telemetry/telemetry_data.c   |  40 +++-
> > lib/telemetry/telemetry_data.h   |   2 +
> > lib/telemetry/telemetry_json.h   |  32 
> > lib/telemetry/version.map|   2 +
> > 12 files changed, 539 insertions(+), 14 deletions(-)  create mode
> >100644 drivers/common/cnxk/cnxk_telemetry.h
> > create mode 100644 drivers/common/cnxk/cnxk_telemetry_npa.c
> >
> >--
> >2.25.1
> 
> I am still unsure exactly what the use case is here - why are we choosing to
> publish the pointer values through telemetry rather than using a debug log for
> example?

Pointer values are useful sometimes for more debugging through telemetry, hence 
this proposal.
As I mentioned in v1 thread, this is architecture compliant approach rather 
than assuming pointer 
value is always 64 bit, when there is need to use pointer value in current 
telemetry path.

Thanks,
Gowrishankar

> Maybe I am missing something here.
> 
> Thanks,
> Ciara

Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue

[Xueming(Steven) Li]

> -Original Message-
> From: Ferruh Yigit 
> Sent: Wednesday, August 11, 2021 8:04 PM
> To: Xueming(Steven) Li ; Jerin Jacob 
> 
> Cc: dpdk-dev ; NBU-Contact-Thomas Monjalon 
> ; Andrew Rybchenko
> 
> Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue
> 
> On 8/11/2021 9:28 AM, Xueming(Steven) Li wrote:
> >
> >
> >> -Original Message-
> >> From: Jerin Jacob 
> >> Sent: Wednesday, August 11, 2021 4:03 PM
> >> To: Xueming(Steven) Li 
> >> Cc: dpdk-dev ; Ferruh Yigit ;
> >> NBU-Contact-Thomas Monjalon ; Andrew Rybchenko
> >> 
> >> Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue
> >>
> >> On Mon, Aug 9, 2021 at 7:46 PM Xueming(Steven) Li  
> >> wrote:
> >>>
> >>> Hi,
> >>>
>  -Original Message-
>  From: Jerin Jacob 
>  Sent: Monday, August 9, 2021 9:51 PM
>  To: Xueming(Steven) Li 
>  Cc: dpdk-dev ; Ferruh Yigit ;
>  NBU-Contact-Thomas Monjalon ; Andrew Rybchenko
>  
>  Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx
>  queue
> 
>  On Mon, Aug 9, 2021 at 5:18 PM Xueming Li  wrote:
> >
> > In current DPDK framework, each RX queue is pre-loaded with mbufs
> > for incoming packets. When number of representors scale out in a
> > switch domain, the memory consumption became significant. Most
> > important, polling all ports leads to high cache miss, high
> > latency and low throughput.
> >
> > This patch introduces shared RX queue. Ports with same
> > configuration in a switch domain could share RX queue set by specifying 
> > sharing group.
> > Polling any queue using same shared RX queue receives packets from
> > all member ports. Source port is identified by mbuf->port.
> >
> > Port queue number in a shared group should be identical. Queue
> > index is
> > 1:1 mapped in shared group.
> >
> > Share RX queue is supposed to be polled on same thread.
> >
> > Multiple groups is supported by group ID.
> 
>  Is this offload specific to the representor? If so can this name be 
>  changed specifically to representor?
> >>>
> >>> Yes, PF and representor in switch domain could take advantage.
> >>>
>  If it is for a generic case, how the flow ordering will be maintained?
> >>>
> >>> Not quite sure that I understood your question. The control path of
> >>> is almost same as before, PF and representor port still needed, rte flows 
> >>> not impacted.
> >>> Queues still needed for each member port, descriptors(mbuf) will be
> >>> supplied from shared Rx queue in my PMD implementation.
> >>
> >> My question was if create a generic RTE_ETH_RX_OFFLOAD_SHARED_RXQ
> >> offload, multiple ethdev receive queues land into the same receive queue, 
> >> In that case, how the flow order is maintained for
> respective receive queues.
> >
> > I guess the question is testpmd forward stream? The forwarding logic has to 
> > be changed slightly in case of shared rxq.
> > basically for each packet in rx_burst result, lookup source stream 
> > according to mbuf->port, forwarding to target fs.
> > Packets from same source port could be grouped as a small burst to
> > process, this will accelerates the performance if traffic come from
> > limited ports. I'll introduce some common api to do shard rxq forwarding, 
> > call it with packets handling callback, so it suites for all
> forwarding engine. Will sent patches soon.
> >
> 
> All ports will put the packets in to the same queue (share queue), right? 
> Does this means only single core will poll only, what will
> happen if there are multiple cores polling, won't it cause problem?

This has been mentioned in commit log, the shared rxq is supposed to be polling 
in single thread(core) - I think it should be "MUST".
Result is unexpected if there are multiple cores pooling, that's why I added a 
polling schedule check in testpmd.
Similar for rx/tx burst function, a queue can't be polled on multiple 
thread(core), and for performance concern, no such check in eal api.

If users want to utilize multiple cores to distribute workloads, it's possible 
to define more groups, queues in different group could be
could be polled on multiple cores.

It's possible to poll every member port in group, but not necessary, any port 
in group could be polled to get packets for all ports in group.

If the member port subject to hot plug/remove,  it's possible to create a vdev 
with same queue number, copy rxq object and poll vdev
as a dedicate proxy for the group.

> 
> And if this requires specific changes in the application, I am not sure about 
> the solution, can't this work in a transparent way to the
> application?

Yes, we considered different options in design stage. One possible solution is 
to cache received packets in rings, this can be done on
eth layer, but I'm afraid less benefits, user still has to be a ware of 
multiple core polling. 
This can be done as a wrapper PMD later, more effort

[dpdk-dev] [PATCH v2 04/15] app/testpmd: make sure shared Rx queue polled on same core

[Xueming Li]

Shared rxqs uses one set rx queue internally, queues must be polled from
one core.

Stops forwarding if shared rxq being scheduled on multiple cores.

Signed-off-by: Xueming Li 
---
 app/test-pmd/config.c  | 91 ++
 app/test-pmd/testpmd.c |  4 +-
 app/test-pmd/testpmd.h |  2 +
 3 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index bb882a56a4..51f7d26045 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2885,6 +2885,97 @@ port_rss_hash_key_update(portid_t port_id, char 
rss_type[], uint8_t *hash_key,
}
 }
 
+/*
+ * Check whether a shared rxq scheduled on other lcores.
+ */
+static bool
+fwd_stream_on_other_lcores(uint16_t domain_id, portid_t src_port,
+  queueid_t src_rxq, lcoreid_t src_lc)
+{
+   streamid_t sm_id;
+   streamid_t nb_fs_per_lcore;
+   lcoreid_t  nb_fc;
+   lcoreid_t  lc_id;
+   struct fwd_stream *fs;
+   struct rte_port *port;
+   struct rte_eth_rxconf *rxq_conf;
+
+   nb_fc = cur_fwd_config.nb_fwd_lcores;
+   for (lc_id = src_lc + 1; lc_id < nb_fc; lc_id++) {
+   sm_id = fwd_lcores[lc_id]->stream_idx;
+   nb_fs_per_lcore = fwd_lcores[lc_id]->stream_nb;
+   for (; sm_id < fwd_lcores[lc_id]->stream_idx + nb_fs_per_lcore;
+sm_id++) {
+   fs = fwd_streams[sm_id];
+   port = &ports[fs->rx_port];
+   rxq_conf = &port->rx_conf[fs->rx_queue];
+   if ((rxq_conf->offloads & RTE_ETH_RX_OFFLOAD_SHARED_RXQ)
+   == 0)
+   /* Not shared rxq. */
+   continue;
+   if (domain_id != port->dev_info.switch_info.domain_id)
+   continue;
+   if (fs->rx_queue != src_rxq)
+   continue;
+   printf("Shared RX queue can't be scheduled on different 
cores:\n");
+   printf("  lcore %hhu Port %hu queue %hu\n",
+  src_lc, src_port, src_rxq);
+   printf("  lcore %hhu Port %hu queue %hu\n",
+  lc_id, fs->rx_port, fs->rx_queue);
+   printf("  please use --nb-cores=%hu to limit forwarding 
cores\n",
+  nb_rxq);
+   return true;
+   }
+   }
+   return false;
+}
+
+/*
+ * Check shared rxq configuration.
+ *
+ * Shared group must not being scheduled on different core.
+ */
+bool
+pkt_fwd_shared_rxq_check(void)
+{
+   streamid_t sm_id;
+   streamid_t nb_fs_per_lcore;
+   lcoreid_t  nb_fc;
+   lcoreid_t  lc_id;
+   struct fwd_stream *fs;
+   uint16_t domain_id;
+   struct rte_port *port;
+   struct rte_eth_rxconf *rxq_conf;
+
+   nb_fc = cur_fwd_config.nb_fwd_lcores;
+   /*
+* Check streams on each core, make sure the same switch domain +
+* group + queue doesn't get scheduled on other cores.
+*/
+   for (lc_id = 0; lc_id < nb_fc; lc_id++) {
+   sm_id = fwd_lcores[lc_id]->stream_idx;
+   nb_fs_per_lcore = fwd_lcores[lc_id]->stream_nb;
+   for (; sm_id < fwd_lcores[lc_id]->stream_idx + nb_fs_per_lcore;
+sm_id++) {
+   fs = fwd_streams[sm_id];
+   /* Update lcore info stream being scheduled. */
+   fs->lcore = fwd_lcores[lc_id];
+   port = &ports[fs->rx_port];
+   rxq_conf = &port->rx_conf[fs->rx_queue];
+   if ((rxq_conf->offloads & RTE_ETH_RX_OFFLOAD_SHARED_RXQ)
+   == 0)
+   /* Not shared rxq. */
+   continue;
+   /* Check shared rxq not scheduled on remaining cores. */
+   domain_id = port->dev_info.switch_info.domain_id;
+   if (fwd_stream_on_other_lcores(domain_id, fs->rx_port,
+  fs->rx_queue, lc_id))
+   return false;
+   }
+   }
+   return true;
+}
+
 /*
  * Setup forwarding configuration for each logical core.
  */
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 67fd128862..d941bd982e 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2169,10 +2169,12 @@ start_packet_forwarding(int with_tx_first)
 
fwd_config_setup();
 
+   pkt_fwd_config_display(&cur_fwd_config);
+   if (!pkt_fwd_shared_rxq_check())
+   return;
if(!no_flush_rx)
flush_fwd_rx_queues();
 
-   pkt_fwd_config_display(&cur_fwd_config);
rxtx_config_display();
 
fwd_stats_reset();
diff --git a/

[dpdk-dev] [PATCH v2 05/15] app/testpmd: adds common forwarding for shared Rx queue

[Xueming Li]

By enabling shared Rx queue, received packets come from all member ports
in same shared Rx queue.

This patch adds a common forwarding function for shared Rx queue, groups
source forwarding stream by looking up local streams on current lcore
with packet source port(mbuf->port) and queue, then invokes callback to
handle received packets for source stream.

Signed-off-by: Xueming Li 
---
 app/test-pmd/testpmd.c | 69 ++
 app/test-pmd/testpmd.h |  4 +++
 2 files changed, 73 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d941bd982e..f46bd97948 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2034,6 +2034,75 @@ flush_fwd_rx_queues(void)
}
 }
 
+/**
+ * Get packet source stream by source port and queue.
+ * All streams of same shared Rx queue locates on same core.
+ */
+static struct fwd_stream *
+forward_stream_get(struct fwd_stream *fs, uint16_t port)
+{
+   streamid_t sm_id;
+   struct fwd_lcore *fc;
+   struct fwd_stream **fsm;
+   streamid_t nb_fs;
+
+   fc = fs->lcore;
+   fsm = &fwd_streams[fc->stream_idx];
+   nb_fs = fc->stream_nb;
+   for (sm_id = 0; sm_id < nb_fs; sm_id++) {
+   if (fsm[sm_id]->rx_port == port &&
+   fsm[sm_id]->rx_queue == fs->rx_queue)
+   return fsm[sm_id];
+   }
+   return NULL;
+}
+
+/**
+ * Forward packet by source port and queue.
+ */
+static void
+forward_by_port(struct fwd_stream *src_fs, uint16_t port, uint16_t nb_rx,
+   struct rte_mbuf **pkts, packet_fwd_cb fwd)
+{
+   struct fwd_stream *fs = forward_stream_get(src_fs, port);
+
+   if (fs != NULL) {
+   fs->rx_packets += nb_rx;
+   fwd(fs, nb_rx, pkts);
+   } else {
+   /* Source stream not found, drop all packets. */
+   src_fs->fwd_dropped += nb_rx;
+   while (nb_rx > 0)
+   rte_pktmbuf_free(pkts[--nb_rx]);
+   }
+}
+
+/**
+ * Forward packets from shared Rx queue.
+ *
+ * Source port of packets are identified by mbuf->port.
+ */
+void
+forward_shared_rxq(struct fwd_stream *fs, uint16_t nb_rx,
+  struct rte_mbuf **pkts_burst, packet_fwd_cb fwd)
+{
+   uint16_t i, nb_fs_rx = 1, port;
+
+   /* Locate real source fs according to mbuf->port. */
+   for (i = 0; i < nb_rx; ++i) {
+   rte_prefetch0(pkts_burst[i + 1]);
+   port = pkts_burst[i]->port;
+   if (i + 1 == nb_rx || pkts_burst[i + 1]->port != port) {
+   /* Forward packets with same source port. */
+   forward_by_port(fs, port, nb_fs_rx,
+   &pkts_burst[i + 1 - nb_fs_rx], fwd);
+   nb_fs_rx = 1;
+   } else {
+   nb_fs_rx++;
+   }
+   }
+}
+
 static void
 run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 6497c56359..13141dfed9 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -272,6 +272,8 @@ struct fwd_lcore {
 typedef void (*port_fwd_begin_t)(portid_t pi);
 typedef void (*port_fwd_end_t)(portid_t pi);
 typedef void (*packet_fwd_t)(struct fwd_stream *fs);
+typedef void (*packet_fwd_cb)(struct fwd_stream *fs, uint16_t nb_rx,
+ struct rte_mbuf **pkts);
 
 struct fwd_engine {
const char   *fwd_mode_name; /**< Forwarding mode name. */
@@ -897,6 +899,8 @@ char *list_pkt_forwarding_modes(void);
 char *list_pkt_forwarding_retry_modes(void);
 void set_pkt_forwarding_mode(const char *fwd_mode);
 void start_packet_forwarding(int with_tx_first);
+void forward_shared_rxq(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst, packet_fwd_cb fwd);
 void fwd_stats_display(void);
 void fwd_stats_reset(void);
 void stop_packet_forwarding(void);
-- 
2.25.1

[dpdk-dev] [PATCH v2 01/15] ethdev: introduce shared Rx queue

[Xueming Li]

In current DPDK framework, each RX queue is pre-loaded with mbufs for
incoming packets. When number of representors scale out in a switch
domain, the memory consumption became significant. Most important,
polling all ports leads to high cache miss, high latency and low
throughput.

This patch introduces shared RX queue. Ports with same configuration in
a switch domain could share RX queue set by specifying sharing group.
Polling any queue using same shared RX queue receives packets from all
member ports. Source port is identified by mbuf->port.

Port queue number in a shared group should be identical. Queue index is
1:1 mapped in shared group.

Share RX queue must be polled on single thread or core.

Multiple groups is supported by group ID.

Signed-off-by: Xueming Li 
Cc: Jerin Jacob 
---
Rx queue object could be used as shared Rx queue object, it's important
to clear all queue control callback api that using queue object:
  https://mails.dpdk.org/archives/dev/2021-July/215574.html
---
 doc/guides/nics/features.rst| 11 +++
 doc/guides/nics/features/default.ini|  1 +
 doc/guides/prog_guide/switch_representation.rst | 10 ++
 lib/ethdev/rte_ethdev.c |  1 +
 lib/ethdev/rte_ethdev.h |  7 +++
 5 files changed, 30 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index a96e12d155..2e2a9b1554 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -624,6 +624,17 @@ Supports inner packet L4 checksum.
   ``tx_offload_capa,tx_queue_offload_capa:DEV_TX_OFFLOAD_OUTER_UDP_CKSUM``.
 
 
+.. _nic_features_shared_rx_queue:
+
+Shared Rx queue
+---
+
+Supports shared Rx queue for ports in same switch domain.
+
+* **[uses] rte_eth_rxconf,rte_eth_rxmode**: 
``offloads:RTE_ETH_RX_OFFLOAD_SHARED_RXQ``.
+* **[provides] mbuf**: ``mbuf.port``.
+
+
 .. _nic_features_packet_type_parsing:
 
 Packet type parsing
diff --git a/doc/guides/nics/features/default.ini 
b/doc/guides/nics/features/default.ini
index 754184ddd4..ebeb4c1851 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -19,6 +19,7 @@ Free Tx mbuf on demand =
 Queue start/stop =
 Runtime Rx queue setup =
 Runtime Tx queue setup =
+Shared Rx queue  =
 Burst mode info  =
 Power mgmt address monitor =
 MTU update   =
diff --git a/doc/guides/prog_guide/switch_representation.rst 
b/doc/guides/prog_guide/switch_representation.rst
index ff6aa91c80..45bf5a3a10 100644
--- a/doc/guides/prog_guide/switch_representation.rst
+++ b/doc/guides/prog_guide/switch_representation.rst
@@ -123,6 +123,16 @@ thought as a software "patch panel" front-end for 
applications.
 .. [1] `Ethernet switch device driver model (switchdev)
`_
 
+- Memory usage of representors is huge when number of representor grows,
+  because PMD always allocate mbuf for each descriptor of Rx queue.
+  Polling the large number of ports brings more CPU load, cache miss and
+  latency. Shared Rx queue can be used to share Rx queue between PF and
+  representors in same switch domain. ``RTE_ETH_RX_OFFLOAD_SHARED_RXQ``
+  is present in Rx offloading capability of device info. Setting the
+  offloading flag in device Rx mode or Rx queue configuration to enable
+  shared Rx queue. Polling any member port of shared Rx queue can return
+  packets of all ports in group, port ID is saved in ``mbuf.port``.
+
 Basic SR-IOV
 
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 193f0d8295..058f5c88d9 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -127,6 +127,7 @@ static const struct {
RTE_RX_OFFLOAD_BIT2STR(OUTER_UDP_CKSUM),
RTE_RX_OFFLOAD_BIT2STR(RSS_HASH),
RTE_ETH_RX_OFFLOAD_BIT2STR(BUFFER_SPLIT),
+   RTE_ETH_RX_OFFLOAD_BIT2STR(SHARED_RXQ),
 };
 
 #undef RTE_RX_OFFLOAD_BIT2STR
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..a578c9db9d 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1047,6 +1047,7 @@ struct rte_eth_rxconf {
uint8_t rx_drop_en; /**< Drop packets if no descriptors are available. 
*/
uint8_t rx_deferred_start; /**< Do not start queue with 
rte_eth_dev_start(). */
uint16_t rx_nseg; /**< Number of descriptions in rx_seg array. */
+   uint32_t shared_group; /**< Shared port group index in switch domain. */
/**
 * Per-queue Rx offloads to be set using DEV_RX_OFFLOAD_* flags.
 * Only offloads set on rx_queue_offload_capa or rx_offload_capa
@@ -1373,6 +1374,12 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x0004
 #define DEV_RX_OFFLOAD_RSS_HASH0x0008
 #define RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT 0x0010
+/**
+ * Rx queue is shared among ports in same switch domain to save memory,
+ * avoid p

[dpdk-dev] [PATCH v2 02/15] app/testpmd: dump port and queue info for each packet

[Xueming Li]

In case of shared Rx queue, port number of mbufs returned from one rx
burst could be different.

To support shared Rx queue, this patch dumps mbuf->port and queue for
each packet.

Signed-off-by: Xueming Li 
---
 app/test-pmd/util.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5dd7157947..1733d5e663 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -100,6 +100,7 @@ dump_pkt_burst(uint16_t port_id, uint16_t queue, struct 
rte_mbuf *pkts[],
struct rte_flow_restore_info info = { 0, };
 
mb = pkts[i];
+   MKDUMPSTR(print_buf, buf_size, cur_len, "port %u, ", mb->port);
eth_hdr = rte_pktmbuf_read(mb, 0, sizeof(_eth_hdr), &_eth_hdr);
eth_type = RTE_BE_TO_CPU_16(eth_hdr->ether_type);
packet_type = mb->packet_type;
-- 
2.25.1

[dpdk-dev] [PATCH v2 03/15] app/testpmd: new parameter to enable shared Rx queue

[Xueming Li]

Adds "--rxq-share" parameter to enable shared rxq for each rxq.

Default shared rxq group 0 is used, RX queues in same switch domain
shares same rxq according to queue index.

Shared Rx queue is enabled only if device support offloading flag
RTE_ETH_RX_OFFLOAD_SHARED_RXQ.

Signed-off-by: Xueming Li 
---
 app/test-pmd/config.c |  6 +-
 app/test-pmd/parameters.c |  4 
 app/test-pmd/testpmd.c| 14 ++
 app/test-pmd/testpmd.h|  2 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 +
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 31d8ba1b91..bb882a56a4 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2709,7 +2709,11 @@ rxtx_config_display(void)
printf("  RX threshold registers: pthresh=%d 
hthresh=%d "
" wthresh=%d\n",
pthresh_tmp, hthresh_tmp, wthresh_tmp);
-   printf("  RX Offloads=0x%"PRIx64"\n", offloads_tmp);
+   printf("  RX Offloads=0x%"PRIx64, offloads_tmp);
+   if (rx_conf->offloads & RTE_ETH_RX_OFFLOAD_SHARED_RXQ)
+   printf(" share group=%u",
+  rx_conf->shared_group);
+   printf("\n");
}
 
/* per tx queue config only for first queue to be less verbose 
*/
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7c13210f04..a466a20bfb 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -166,6 +166,7 @@ usage(char* progname)
printf("  --tx-ip=src,dst: IP addresses in Tx-only mode\n");
printf("  --tx-udp=src[,dst]: UDP ports in Tx-only mode\n");
printf("  --eth-link-speed: force link speed.\n");
+   printf("  --rxq-share: share rxq between PF and representors\n");
printf("  --disable-link-check: disable check on link status when "
   "starting/stopping ports.\n");
printf("  --disable-device-start: do not automatically start port\n");
@@ -602,6 +603,7 @@ launch_args_parse(int argc, char** argv)
{ "rxpkts", 1, 0, 0 },
{ "txpkts", 1, 0, 0 },
{ "txonly-multi-flow",  0, 0, 0 },
+   { "rxq-share",  0, 0, 0 },
{ "eth-link-speed", 1, 0, 0 },
{ "disable-link-check", 0, 0, 0 },
{ "disable-device-start",   0, 0, 0 },
@@ -1256,6 +1258,8 @@ launch_args_parse(int argc, char** argv)
}
if (!strcmp(lgopts[opt_idx].name, "txonly-multi-flow"))
txonly_multi_flow = 1;
+   if (!strcmp(lgopts[opt_idx].name, "rxq-share"))
+   rxq_share = 1;
if (!strcmp(lgopts[opt_idx].name, "no-flush-rx"))
no_flush_rx = 1;
if (!strcmp(lgopts[opt_idx].name, "eth-link-speed")) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6cbe9ba3c8..67fd128862 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -223,6 +223,9 @@ uint8_t  rx_pkt_nb_segs; /**< Number of segments to split */
 uint16_t rx_pkt_seg_offsets[MAX_SEGS_BUFFER_SPLIT];
 uint8_t  rx_pkt_nb_offs; /**< Number of specified offsets */
 
+uint8_t rxq_share;
+/**< Create shared rxq for PF and representors. */
+
 /*
  * Configuration of packet segments used by the "txonly" processing engine.
  */
@@ -1441,6 +1444,11 @@ init_config_port_offloads(portid_t pid, uint32_t 
socket_id)
port->dev_conf.txmode.offloads &=
~DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 
+   if (rxq_share &&
+   (port->dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_SHARED_RXQ))
+   port->dev_conf.rxmode.offloads |=
+   RTE_ETH_RX_OFFLOAD_SHARED_RXQ;
+
/* Apply Rx offloads configuration */
for (i = 0; i < port->dev_info.max_rx_queues; i++)
port->rx_conf[i].offloads = port->dev_conf.rxmode.offloads;
@@ -3334,6 +3342,12 @@ rxtx_port_config(struct rte_port *port)
for (qid = 0; qid < nb_rxq; qid++) {
offloads = port->rx_conf[qid].offloads;
port->rx_conf[qid] = port->dev_info.default_rxconf;
+
+   if (rxq_share > 0 &&
+   (port->dev_info.rx_offload_capa &
+RTE_ETH_RX_OFFLOAD_SHARED_RXQ))
+   offloads |= RTE_ETH_RX_OFFLOAD_SHARED_RXQ;
+
if (offloads != 0)
port->rx_conf[qid].offloads = offloads;
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 16a3598e48..f3b1d34e28 100644
--- a/app/test-pmd/testpmd.h
++

[dpdk-dev] [PATCH v2 07/15] app/testpmd: support shared Rx queues for IO forwarding

[Xueming Li]

Supports shared Rx queue.

If shared Rx queue is enabled, group received packets by stream
according to mbuf->port value and then and forward in stream basis as
before.

If shared Rx queue is not enabled, just forward in stream basis.

Signed-off-by: Xueming Li 
---
 app/test-pmd/iofwd.c | 27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/iofwd.c b/app/test-pmd/iofwd.c
index 83d098adcb..316a80d65c 100644
--- a/app/test-pmd/iofwd.c
+++ b/app/test-pmd/iofwd.c
@@ -44,25 +44,11 @@
  * to packets data.
  */
 static void
-pkt_burst_io_forward(struct fwd_stream *fs)
+io_forward_stream(struct fwd_stream *fs, uint16_t nb_rx,
+ struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
-   uint16_t nb_rx;
uint16_t nb_tx;
uint32_t retry;
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /*
-* Receive a burst of packets and forward them.
-*/
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue,
-   pkts_burst, nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
-   fs->rx_packets += nb_rx;
 
nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
pkts_burst, nb_rx);
@@ -85,8 +71,15 @@ pkt_burst_io_forward(struct fwd_stream *fs)
rte_pktmbuf_free(pkts_burst[nb_tx]);
} while (++nb_tx < nb_rx);
}
+}
 
-   get_end_cycles(fs, start_tsc);
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+pkt_burst_io_forward(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, io_forward_stream);
 }
 
 struct fwd_engine io_fwd_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 10/15] app/testpmd: support shared Rx queue for csum fwd

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then fwd it in stream basis (as before).

If shared rxq is not enabled, just fwd it in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/csumonly.c | 28 +++-
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 607c889359..3b7fb35843 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -763,7 +763,7 @@ pkt_copy_split(const struct rte_mbuf *pkt)
 }
 
 /*
- * Receive a burst of packets, and for each packet:
+ * For each packet in received mbuf:
  *  - parse packet, and try to recognize a supported packet type (1)
  *  - if it's not a supported packet type, don't touch the packet, else:
  *  - reprocess the checksum of all supported layers. This is done in SW
@@ -792,9 +792,9 @@ pkt_copy_split(const struct rte_mbuf *pkt)
  * OUTER_IP is only useful for tunnel packets.
  */
 static void
-pkt_burst_checksum_forward(struct fwd_stream *fs)
+checksum_forward_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
struct rte_mbuf *gso_segments[GSO_MAX_PKT_BURST];
struct rte_gso_ctx *gso_ctx;
struct rte_mbuf **tx_pkts_burst;
@@ -805,7 +805,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
void **gro_ctx;
uint16_t gro_pkts_num;
uint8_t gro_enable;
-   uint16_t nb_rx;
uint16_t nb_tx;
uint16_t nb_prep;
uint16_t i;
@@ -820,18 +819,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
uint16_t nb_segments = 0;
int ret;
 
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /* receive a burst of packet */
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
-
-   fs->rx_packets += nb_rx;
rx_bad_ip_csum = 0;
rx_bad_l4_csum = 0;
rx_bad_outer_l4_csum = 0;
@@ -1139,8 +1126,15 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
rte_pktmbuf_free(tx_pkts_burst[nb_tx]);
} while (++nb_tx < nb_rx);
}
+}
 
-   get_end_cycles(fs, start_tsc);
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+pkt_burst_checksum_forward(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, checksum_forward_stream);
 }
 
 struct fwd_engine csum_fwd_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 09/15] app/testpmd: support shared Rx queue for icmpecho fwd

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then fwd it in stream basis (as before).

If shared rxq is not enabled, just fwd it in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/icmpecho.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/icmpecho.c b/app/test-pmd/icmpecho.c
index 8948f28eb5..d6d11a2efb 100644
--- a/app/test-pmd/icmpecho.c
+++ b/app/test-pmd/icmpecho.c
@@ -267,13 +267,13 @@ ipv4_hdr_cksum(struct rte_ipv4_hdr *ip_h)
(((rte_be_to_cpu_32((ipv4_addr)) >> 24) & 0x00FF) == 0xE0)
 
 /*
- * Receive a burst of packets, lookup for ICMP echo requests, and, if any,
- * send back ICMP echo replies.
+ * Lookup for ICMP echo requests in received mbuf and, if any,
+ * send back ICMP echo replies to corresponding Tx port.
  */
 static void
-reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
+reply_to_icmp_echo_rqsts_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
struct rte_mbuf *pkt;
struct rte_ether_hdr *eth_h;
struct rte_vlan_hdr *vlan_h;
@@ -283,7 +283,6 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
struct rte_ether_addr eth_addr;
uint32_t retry;
uint32_t ip_addr;
-   uint16_t nb_rx;
uint16_t nb_tx;
uint16_t nb_replies;
uint16_t eth_type;
@@ -291,22 +290,9 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
uint16_t arp_op;
uint16_t arp_pro;
uint32_t cksum;
-   uint8_t  i;
+   uint16_t  i;
int l2_len;
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /*
-* First, receive a burst of packets.
-*/
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
 
-   fs->rx_packets += nb_rx;
nb_replies = 0;
for (i = 0; i < nb_rx; i++) {
if (likely(i < nb_rx - 1))
@@ -509,8 +495,15 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
} while (++nb_tx < nb_replies);
}
}
+}
 
-   get_end_cycles(fs, start_tsc);
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, reply_to_icmp_echo_rqsts_stream);
 }
 
 struct fwd_engine icmp_echo_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 06/15] app/testpmd: add common fwd wrapper function

[Xueming Li]

From: Xiaoyu Min 

Added an inline common wrapper function for all fwd engines
which do the following in common:

1. get_start_cycles
2. rte_eth_rx_burst(...,nb_pkt_per_burst)
3. if rxq_share do forward_shared_rxq(), otherwise do fwd directly
4. get_end_cycle

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/testpmd.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 13141dfed9..b685ac48d6 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1022,6 +1022,30 @@ void add_tx_dynf_callback(portid_t portid);
 void remove_tx_dynf_callback(portid_t portid);
 int update_jumbo_frame_offload(portid_t portid);
 
+static inline void
+do_burst_fwd(struct fwd_stream *fs, packet_fwd_cb fwd)
+{
+   struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
+   uint16_t nb_rx;
+   uint64_t start_tsc = 0;
+
+   get_start_cycles(&start_tsc);
+
+   /*
+* Receive a burst of packets and forward them.
+*/
+   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue,
+   pkts_burst, nb_pkt_per_burst);
+   inc_rx_burst_stats(fs, nb_rx);
+   if (unlikely(nb_rx == 0))
+   return;
+   if (unlikely(rxq_share > 0))
+   forward_shared_rxq(fs, nb_rx, pkts_burst, fwd);
+   else
+   (*fwd)(fs, nb_rx, pkts_burst);
+   get_end_cycles(fs, start_tsc);
+}
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
-- 
2.25.1

[dpdk-dev] [PATCH v2 08/15] app/testpmd: support shared Rx queue for rxonly forwarding

[Xueming Li]

Supports shared Rx queue.

If shared Rx queue is enabled, group received packets by stream
according to mbuf->port value and then and forward in stream basis as
before.

If shared Rx queue is not enabled, just forward in stream basis.

Signed-off-by: Xueming Li 
---
 app/test-pmd/rxonly.c | 34 +-
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index c78fc4609a..80ae0ecf93 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -41,32 +41,24 @@
 #include "testpmd.h"
 
 /*
- * Received a burst of packets.
+ * Process a burst of received packets from same stream.
  */
 static void
-pkt_burst_receive(struct fwd_stream *fs)
+rxonly_forward_stream(struct fwd_stream *fs, uint16_t nb_rx,
+ struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
-   uint16_t nb_rx;
-   uint16_t i;
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /*
-* Receive a burst of packets.
-*/
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
+   RTE_SET_USED(fs);
+   rte_pktmbuf_free_bulk(pkts_burst, nb_rx);
+}
 
-   fs->rx_packets += nb_rx;
-   for (i = 0; i < nb_rx; i++)
-   rte_pktmbuf_free(pkts_burst[i]);
 
-   get_end_cycles(fs, start_tsc);
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+pkt_burst_receive(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, rxonly_forward_stream);
 }
 
 struct fwd_engine rx_only_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 11/15] app/testpmd: support shared Rx queue for flowgen

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then do stats in stream basis (as before).

If shared rxq is not enabled, just as usual in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/flowgen.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
index 3bf6e1ce97..d74c302b1c 100644
--- a/app/test-pmd/flowgen.c
+++ b/app/test-pmd/flowgen.c
@@ -83,10 +83,10 @@ ip_sum(const alias_int16_t *hdr, int hdr_len)
  * still do so in order to maintain traffic statistics.
  */
 static void
-pkt_burst_flow_gen(struct fwd_stream *fs)
+flow_gen_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst)
 {
unsigned pkt_size = tx_pkt_length - 4;  /* Adjust FCS */
-   struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
struct rte_mempool *mbp;
struct rte_mbuf  *pkt = NULL;
struct rte_ether_hdr *eth_hdr;
@@ -94,23 +94,14 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
struct rte_udp_hdr *udp_hdr;
uint16_t vlan_tci, vlan_tci_outer;
uint64_t ol_flags = 0;
-   uint16_t nb_rx;
uint16_t nb_tx;
uint16_t nb_pkt;
uint16_t nb_clones = nb_pkt_flowgen_clones;
uint16_t i;
uint32_t retry;
uint64_t tx_offloads;
-   uint64_t start_tsc = 0;
static int next_flow = 0;
 
-   get_start_cycles(&start_tsc);
-
-   /* Receive a burst of packets and discard them. */
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   fs->rx_packets += nb_rx;
-
for (i = 0; i < nb_rx; i++)
rte_pktmbuf_free(pkts_burst[i]);
 
@@ -213,8 +204,15 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
rte_pktmbuf_free(pkts_burst[nb_tx]);
} while (++nb_tx < nb_pkt);
}
+}
 
-   get_end_cycles(fs, start_tsc);
+/*
+ * Wrapper of real fwd engine
+ */
+static void
+pkt_burst_flow_gen(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, flow_gen_stream);
 }
 
 struct fwd_engine flow_gen_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 12/15] app/testpmd: support shared Rx queue for MAC fwd

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then fwd it in stream basis (as before).

If shared rxq is not enabled, just fwd it as usual in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/macfwd.c | 27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c
index 0568ea794d..75fbea16d4 100644
--- a/app/test-pmd/macfwd.c
+++ b/app/test-pmd/macfwd.c
@@ -44,32 +44,18 @@
  * before forwarding them.
  */
 static void
-pkt_burst_mac_forward(struct fwd_stream *fs)
+mac_forward_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
struct rte_port  *txp;
struct rte_mbuf  *mb;
struct rte_ether_hdr *eth_hdr;
uint32_t retry;
-   uint16_t nb_rx;
uint16_t nb_tx;
uint16_t i;
uint64_t ol_flags = 0;
uint64_t tx_offloads;
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /*
-* Receive a burst of packets and forward them.
-*/
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
 
-   fs->rx_packets += nb_rx;
txp = &ports[fs->tx_port];
tx_offloads = txp->dev_conf.txmode.offloads;
if (tx_offloads & DEV_TX_OFFLOAD_VLAN_INSERT)
@@ -116,8 +102,15 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
rte_pktmbuf_free(pkts_burst[nb_tx]);
} while (++nb_tx < nb_rx);
}
+}
 
-   get_end_cycles(fs, start_tsc);
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+pkt_burst_mac_forward(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, mac_forward_stream);
 }
 
 struct fwd_engine mac_fwd_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 13/15] app/testpmd: support shared Rx queue for macswap fwd

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then fwd it in stream basis (as before).

If shared rxq is not enabled, just fwd it as usual in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/macswap.c | 28 +++-
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
index 310bca06af..daf7170092 100644
--- a/app/test-pmd/macswap.c
+++ b/app/test-pmd/macswap.c
@@ -50,27 +50,13 @@
  * addresses of packets before forwarding them.
  */
 static void
-pkt_burst_mac_swap(struct fwd_stream *fs)
+mac_swap_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
struct rte_port  *txp;
-   uint16_t nb_rx;
uint16_t nb_tx;
uint32_t retry;
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /*
-* Receive a burst of packets and forward them.
-*/
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
 
-   fs->rx_packets += nb_rx;
txp = &ports[fs->tx_port];
 
do_macswap(pkts_burst, nb_rx, txp);
@@ -95,7 +81,15 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
rte_pktmbuf_free(pkts_burst[nb_tx]);
} while (++nb_tx < nb_rx);
}
-   get_end_cycles(fs, start_tsc);
+}
+
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+pkt_burst_mac_swap(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, mac_swap_stream);
 }
 
 struct fwd_engine mac_swap_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 14/15] app/testpmd: support shared Rx queue for 5tuple fwd

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then fwd it in stream basis (as before).

If shared rxq is not enabled, just fwd it as usual in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/5tswap.c | 30 +++---
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/app/test-pmd/5tswap.c b/app/test-pmd/5tswap.c
index e8cef9623b..236a117ee3 100644
--- a/app/test-pmd/5tswap.c
+++ b/app/test-pmd/5tswap.c
@@ -82,18 +82,16 @@ swap_udp(struct rte_udp_hdr *udp_hdr)
  * Parses each layer and swaps it. When the next layer doesn't match it stops.
  */
 static void
-pkt_burst_5tuple_swap(struct fwd_stream *fs)
+_5tuple_swap_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkts_burst)
 {
-   struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
struct rte_port  *txp;
struct rte_mbuf *mb;
uint16_t next_proto;
uint64_t ol_flags;
uint16_t proto;
-   uint16_t nb_rx;
uint16_t nb_tx;
uint32_t retry;
-
int i;
union {
struct rte_ether_hdr *eth;
@@ -105,20 +103,6 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
uint8_t *byte;
} h;
 
-   uint64_t start_tsc = 0;
-
-   get_start_cycles(&start_tsc);
-
-   /*
-* Receive a burst of packets and forward them.
-*/
-   nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
-nb_pkt_per_burst);
-   inc_rx_burst_stats(fs, nb_rx);
-   if (unlikely(nb_rx == 0))
-   return;
-
-   fs->rx_packets += nb_rx;
txp = &ports[fs->tx_port];
ol_flags = ol_flags_init(txp->dev_conf.txmode.offloads);
vlan_qinq_set(pkts_burst, nb_rx, ol_flags,
@@ -182,7 +166,15 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
rte_pktmbuf_free(pkts_burst[nb_tx]);
} while (++nb_tx < nb_rx);
}
-   get_end_cycles(fs, start_tsc);
+}
+
+/*
+ * Wrapper of real fwd engine.
+ */
+static void
+pkt_burst_5tuple_swap(struct fwd_stream *fs)
+{
+   return do_burst_fwd(fs, _5tuple_swap_stream);
 }
 
 struct fwd_engine five_tuple_swap_fwd_engine = {
-- 
2.25.1

[dpdk-dev] [PATCH v2 15/15] app/testpmd: support shared Rx queue for ieee1588 fwd

[Xueming Li]

From: Xiaoyu Min 

Add support of shared rxq.
If shared rxq is enabled, filter packet by stream according
to mbuf->port value and then fwd it in stream basis (as before).

If shared rxq is not enabled, just fwd it as usual in stream basis.

Signed-off-by: Xiaoyu Min 
---
 app/test-pmd/ieee1588fwd.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/app/test-pmd/ieee1588fwd.c b/app/test-pmd/ieee1588fwd.c
index 034f238c34..dc6bf0e39d 100644
--- a/app/test-pmd/ieee1588fwd.c
+++ b/app/test-pmd/ieee1588fwd.c
@@ -90,23 +90,17 @@ port_ieee1588_tx_timestamp_check(portid_t pi)
 }
 
 static void
-ieee1588_packet_fwd(struct fwd_stream *fs)
+ieee1588_fwd_stream(struct fwd_stream *fs, uint16_t nb_rx,
+   struct rte_mbuf **pkt)
 {
-   struct rte_mbuf  *mb;
+   struct rte_mbuf *mb = (*pkt);
struct rte_ether_hdr *eth_hdr;
struct rte_ether_addr addr;
struct ptpv2_msg *ptp_hdr;
uint16_t eth_type;
uint32_t timesync_index;
 
-   /*
-* Receive 1 packet at a time.
-*/
-   if (rte_eth_rx_burst(fs->rx_port, fs->rx_queue, &mb, 1) == 0)
-   return;
-
-   fs->rx_packets += 1;
-
+   RTE_SET_USED(nb_rx);
/*
 * Check that the received packet is a PTP packet that was detected
 * by the hardware.
@@ -198,6 +192,22 @@ ieee1588_packet_fwd(struct fwd_stream *fs)
port_ieee1588_tx_timestamp_check(fs->rx_port);
 }
 
+/*
+ * Wrapper of real fwd ingine.
+ */
+static void
+ieee1588_packet_fwd(struct fwd_stream *fs)
+{
+   struct rte_mbuf *mb;
+
+   if (rte_eth_rx_burst(fs->rx_port, fs->rx_queue, &mb, 1) == 0)
+   return;
+   if (unlikely(rxq_share > 0))
+   forward_shared_rxq(fs, 1, &mb, ieee1588_fwd_stream);
+   else
+   ieee1588_fwd_stream(fs, 1, &mb);
+}
+
 static void
 port_ieee1588_fwd_begin(portid_t pi)
 {
-- 
2.25.1

Re: [dpdk-dev] [PATCHv2] include: fix sys/queue.h.

[William Tu]

On Wed, Aug 11, 2021 at 8:50 AM Dmitry Kozlyuk  wrote:
>
> Hi William,
>
> 2021-08-11 20:46 (UTC+), William Tu:
> > Currently there are a couple of header files include 'sys/queue.h',
> > which is a POSIX functionality.  When compiling DPDK with OVS on
> > Windows, we encountered issues such as, found the missing header.
> > In file included from ../lib/dpdk.c:27:
> > C:\temp\dpdk\include\rte_log.h:24:10: fatal error: 'sys/queue.h' file
> > not found
> >
> > The patch fixes it by 1) removing the #include  from
> > a couple of headers, replace it with  #include , and
> > 2) include sys/queue.h in the rte_os.h. As a result, for Linux/BSD,
> > it is using the sys/queue.h from its POSIX library, and for Windows,
> > DPDK will use the bundled sys/queue.h.
> >
> > 1) fixes the case that DPDK library shouldn't export POSIX functionality
> > into the environment (symbols, macros, headers etc), which cause definitions
> > clashing with the application.
> > 2) fixes the case that DPDK should depend only on the C library and not
> > require POSIX functionality from the underlying system.
>
> Sorry, this is not exactly what was suggested here:
>
> http://inbox.dpdk.org/dev/20210811013325.34c36220@sovereign/
>
> The point was not to install . Its full content is not needed
> for DPDK interface, only for implementation. Public headers only need a
> handful of macros for list/tailq heads and links. Those macros should be
> provided by DPDK, with RTE_ prefix. For Linux and FreeBSD it will just be:
>
> #include 
> #define RTE_TAILQ_ENTRY(type) TAILQ_ENTRY(type)
> /* ... */
>
> For Windows you would have to copy definitions from 
> to some public header,  seems OK. All public headers that need
>  macros must have them replaced with RTE_ version.
> Implementation remains unchanged. No new files need to be installed,
> because when DPDK is built on Windows, it uses the bundled ,
> and when it is installed, only RTE_ macros you created are visible.
>
> Macro documentation should clearly state that they are compatible with system
>  for Linux and FreeBSD, and for Windows they are compatible with
> the version used during build.
>
> +Bruce, David, Ray, Tyler to confirm/object the idea.
>
> > There are still a couple of headers using sys/queue.h, ex:
> > rte_bus_pci.h. Since it's not been used in Windows yet, we can
> > fix them later.
>
> Another item I think of is a checkpatch rule to prohibit TAILQ_xxx, etc
> macro in public headers after replacing them all.
>
> > Suggested-by: Nick Connolly 
> > Suggested-by: Dmitry Kozliuk 
> > Signed-off-by: William Tu 
> > ---
> > v1->v2:
> >   - follow the suggestion by Nick and Dmitry
> >   - http://mails.dpdk.org/archives/dev/2021-August/216304.html
>
> Please send new versions as replies to the previous ones:
>
> git send-email --in-reply-to MSGID ...
>
> https://doc.dpdk.org/guides/contributing/patches.html

Hi Dmitry,
Thanks! I see your point now. I will work on the next patch.
William

[dpdk-dev] DTS Workgroup: MoM 08/11/2021

[Honnappa Nagarahalli]

Attendees:
Brandon Lo
Honnappa Nagarahalli
Juraj Linkes
Lincoln Lavoie
Lijuan Tu
Owen Hilyard

The meeting announcements are sent to dev@dpdk.org.

Minutes:
1) Close to completing the triaging/discussions. Hopefully, we will be able to 
complete it in the next couple of weeks.
2) The work item related discussions are captured in [1]

Action Items:
1) Send a patch addressing black formatter issues - Owen
2) Inform the community of the tools for static analysis the DTS workgroup has 
chosen - Owen
3) Add a config for Pylama so that everyone uses the same configurations - Owen
4) Generate a patch to fix the errors from Pylama - Juraj
5) Rename the HEAD of the DTS repo to main from master - Lijuan

[1] 
https://docs.google.com/document/d/1c5S0_mZzFvzZfYkqyORLT2-qNvUb-fBdjA6DGusy4yM/edit?usp=sharing

Re: [dpdk-dev] [PATCH] eal/windows: expose symbol rte_version

[Dmitry Kozlyuk]

2021-08-05 17:48 (UTC+), William Tu:
> When OVS inits, it calls rte_version to get the DPDK's version.
> The patch fixes the error below by exposing rte_version symbol.
> libopenvswitch.a(dpdk.c.obj) : error LNK2019: unresolved external symbol
> rte_version referenced in function dpdk_init

Fixes: 5b637a848195 ("eal: fix querying DPDK version at runtime")
Cc: bruce.richard...@intel.com
Cc: sta...@dpdk.org

> Signed-off-by: William Tu 

Acked-by: Dmitry Kozlyuk 

> ---
>  lib/eal/version.map | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index 887012d02a..3de59910cd 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -198,7 +198,7 @@ DPDK_21 {
>   rte_uuid_is_null; # WINDOWS_NO_EXPORT
>   rte_uuid_parse; # WINDOWS_NO_EXPORT
>   rte_uuid_unparse; # WINDOWS_NO_EXPORT
> - rte_version; # WINDOWS_NO_EXPORT
> + rte_version;
>   rte_vfio_clear_group; # WINDOWS_NO_EXPORT
>   rte_vfio_container_create; # WINDOWS_NO_EXPORT
>   rte_vfio_container_destroy; # WINDOWS_NO_EXPORT

I can see many functions not exported on Windows for no reason.
This includes the rest of the version API, but it's experimental,
so it's better to have this standalone patch for backporting
(since the patch it fixes is backported).
I'm going to restore other exports in a bulk patchset.

[dpdk-dev] [PATCH 0/3] Fixes for txgbe

[Jiawen Wu]

These patches fix link status, module info and flow director.

Jiawen Wu (3):
  net/txgbe: fix link status when device stopped
  net/txgbe: fix to read SFP module's SFF-8472 data
  net/txgbe: fix L4 port mask in FDIR

 drivers/net/txgbe/base/txgbe_phy.c  | 18 +++---
 drivers/net/txgbe/base/txgbe_type.h |  1 +
 drivers/net/txgbe/txgbe_ethdev.c|  4 
 drivers/net/txgbe/txgbe_ethdev_vf.c |  2 ++
 drivers/net/txgbe/txgbe_fdir.c  | 18 +++---
 5 files changed, 17 insertions(+), 26 deletions(-)

-- 
2.21.0.windows.1

[dpdk-dev] [PATCH 1/3] net/txgbe: fix link status when device stopped

[Jiawen Wu]

When device is stopped, the port status is not changed and only the Tx
laser is turned off by hardware design.

Fixes: 0c061eadec59 ("net/txgbe: add link status change")
Cc: sta...@dpdk.org

Signed-off-by: Jiawen Wu 
---
 drivers/net/txgbe/base/txgbe_type.h | 1 +
 drivers/net/txgbe/txgbe_ethdev.c| 4 
 drivers/net/txgbe/txgbe_ethdev_vf.c | 2 ++
 3 files changed, 7 insertions(+)

diff --git a/drivers/net/txgbe/base/txgbe_type.h 
b/drivers/net/txgbe/base/txgbe_type.h
index 823c6756e7..d95467f9f8 100644
--- a/drivers/net/txgbe/base/txgbe_type.h
+++ b/drivers/net/txgbe/base/txgbe_type.h
@@ -781,6 +781,7 @@ struct txgbe_hw {
int api_version;
bool allow_unsupported_sfp;
bool need_crosstalk_fix;
+   bool dev_start;
struct txgbe_devargs devarg;
 
uint64_t isb_dma;
diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
index e62675520a..0063994688 100644
--- a/drivers/net/txgbe/txgbe_ethdev.c
+++ b/drivers/net/txgbe/txgbe_ethdev.c
@@ -1664,6 +1664,7 @@ txgbe_dev_start(struct rte_eth_dev *dev)
return -1;
hw->mac.start_hw(hw);
hw->mac.get_link_status = true;
+   hw->dev_start = true;
 
/* configure PF module if SRIOV enabled */
txgbe_pf_host_configure(dev);
@@ -1933,6 +1934,7 @@ txgbe_dev_stop(struct rte_eth_dev *dev)
 
hw->adapter_stopped = true;
dev->data->dev_started = 0;
+   hw->dev_start = false;
 
return 0;
 }
@@ -2735,6 +2737,8 @@ txgbe_dev_link_update_share(struct rte_eth_dev *dev,
txgbe_dev_setup_link_alarm_handler, dev);
}
return rte_eth_linkstatus_set(dev, &link);
+   } else if (!hw->dev_start) {
+   return rte_eth_linkstatus_set(dev, &link);
}
 
intr->flags &= ~TXGBE_FLAG_NEED_LINK_CONFIG;
diff --git a/drivers/net/txgbe/txgbe_ethdev_vf.c 
b/drivers/net/txgbe/txgbe_ethdev_vf.c
index 0bae6ffd1f..18ed94bd27 100644
--- a/drivers/net/txgbe/txgbe_ethdev_vf.c
+++ b/drivers/net/txgbe/txgbe_ethdev_vf.c
@@ -628,6 +628,7 @@ txgbevf_dev_start(struct rte_eth_dev *dev)
return err;
}
hw->mac.get_link_status = true;
+   hw->dev_start = true;
 
/* negotiate mailbox API version to use with the PF. */
txgbevf_negotiate_api(hw);
@@ -749,6 +750,7 @@ txgbevf_dev_stop(struct rte_eth_dev *dev)
}
 
adapter->rss_reta_updated = 0;
+   hw->dev_start = false;
 
return 0;
 }
-- 
2.21.0.windows.1

[dpdk-dev] [PATCH 2/3] net/txgbe: fix to read SFP module's SFF-8472 data

[Jiawen Wu]

Fix the I2C target address selection to read SFP module's SFF-8472 data.

Fixes: 8f09fb4642fa ("net/txgbe: add module identify")
Cc: sta...@dpdk.org

Signed-off-by: Jiawen Wu 
---
 drivers/net/txgbe/base/txgbe_phy.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/txgbe/base/txgbe_phy.c 
b/drivers/net/txgbe/base/txgbe_phy.c
index 3e9f507212..2db87ae0c5 100644
--- a/drivers/net/txgbe/base/txgbe_phy.c
+++ b/drivers/net/txgbe/base/txgbe_phy.c
@@ -8,7 +8,7 @@
 #include "txgbe_mng.h"
 #include "txgbe_phy.h"
 
-static void txgbe_i2c_start(struct txgbe_hw *hw);
+static void txgbe_i2c_start(struct txgbe_hw *hw, u8 dev_addr);
 static void txgbe_i2c_stop(struct txgbe_hw *hw);
 static s32 txgbe_handle_bp_flow(u32 link_mode, struct txgbe_hw *hw);
 static void txgbe_get_bp_ability(struct txgbe_backplane_ability *ability,
@@ -1248,11 +1248,9 @@ s32 txgbe_write_i2c_eeprom(struct txgbe_hw *hw, u8 
byte_offset,
 s32 txgbe_read_i2c_byte_unlocked(struct txgbe_hw *hw, u8 byte_offset,
   u8 dev_addr, u8 *data)
 {
-   UNREFERENCED_PARAMETER(dev_addr);
-
DEBUGFUNC("txgbe_read_i2c_byte");
 
-   txgbe_i2c_start(hw);
+   txgbe_i2c_start(hw, dev_addr);
 
/* wait tx empty */
if (!po32m(hw, TXGBE_I2CICR, TXGBE_I2CICR_TXEMPTY,
@@ -1314,11 +1312,9 @@ s32 txgbe_read_i2c_byte(struct txgbe_hw *hw, u8 
byte_offset,
 s32 txgbe_write_i2c_byte_unlocked(struct txgbe_hw *hw, u8 byte_offset,
u8 dev_addr, u8 data)
 {
-   UNREFERENCED_PARAMETER(dev_addr);
-
DEBUGFUNC("txgbe_write_i2c_byte");
 
-   txgbe_i2c_start(hw);
+   txgbe_i2c_start(hw, dev_addr);
 
/* wait tx empty */
if (!po32m(hw, TXGBE_I2CICR, TXGBE_I2CICR_TXEMPTY,
@@ -1369,7 +1365,7 @@ s32 txgbe_write_i2c_byte(struct txgbe_hw *hw, u8 
byte_offset,
  *
  *  Sets I2C start condition (High -> Low on SDA while SCL is High)
  **/
-static void txgbe_i2c_start(struct txgbe_hw *hw)
+static void txgbe_i2c_start(struct txgbe_hw *hw, u8 dev_addr)
 {
DEBUGFUNC("txgbe_i2c_start");
 
@@ -1380,9 +1376,9 @@ static void txgbe_i2c_start(struct txgbe_hw *hw)
TXGBE_I2CCON_SPEED(1) |
TXGBE_I2CCON_RESTART |
TXGBE_I2CCON_SDIA));
-   wr32(hw, TXGBE_I2CTAR, TXGBE_I2C_SLAVEADDR);
-   wr32(hw, TXGBE_I2CSSSCLHCNT, 600);
-   wr32(hw, TXGBE_I2CSSSCLLCNT, 600);
+   wr32(hw, TXGBE_I2CTAR, dev_addr >> 1);
+   wr32(hw, TXGBE_I2CSSSCLHCNT, 200);
+   wr32(hw, TXGBE_I2CSSSCLLCNT, 200);
wr32(hw, TXGBE_I2CRXTL, 0); /* 1byte for rx full signal */
wr32(hw, TXGBE_I2CTXTL, 4);
wr32(hw, TXGBE_I2CSCLTMOUT, 0xFF);
-- 
2.21.0.windows.1

[dpdk-dev] [PATCH 3/3] net/txgbe: fix L4 port mask in FDIR

[Jiawen Wu]

Remove bit reverse for TCP/UDP port mask, since it causes the flows with
some TCP/UDP ports to disobey the flow director rules.

Fixes: ea230dda16ad ("net/txgbe: configure flow director filter")
Cc: sta...@dpdk.org

Signed-off-by: Jiawen Wu 
---
 drivers/net/txgbe/txgbe_fdir.c | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_fdir.c b/drivers/net/txgbe/txgbe_fdir.c
index c8ff4b142c..8abb862286 100644
--- a/drivers/net/txgbe/txgbe_fdir.c
+++ b/drivers/net/txgbe/txgbe_fdir.c
@@ -165,18 +165,6 @@ configure_fdir_flags(const struct rte_fdir_conf *conf,
return 0;
 }
 
-static inline uint32_t
-reverse_fdir_bmks(uint16_t hi_dword, uint16_t lo_dword)
-{
-   uint32_t mask = hi_dword << 16;
-
-   mask |= lo_dword;
-   mask = ((mask & 0x) << 1) | ((mask & 0x) >> 1);
-   mask = ((mask & 0x) << 2) | ((mask & 0x) >> 2);
-   mask = ((mask & 0x0F0F0F0F) << 4) | ((mask & 0xF0F0F0F0) >> 4);
-   return ((mask & 0x00FF00FF) << 8) | ((mask & 0xFF00FF00) >> 8);
-}
-
 int
 txgbe_fdir_set_input_mask(struct rte_eth_dev *dev)
 {
@@ -213,9 +201,9 @@ txgbe_fdir_set_input_mask(struct rte_eth_dev *dev)
/* TBD: don't support encapsulation yet */
wr32(hw, TXGBE_FDIRMSK, fdirm);
 
-   /* store the TCP/UDP port masks, bit reversed from port layout */
-   fdirtcpm = reverse_fdir_bmks(rte_be_to_cpu_16(info->mask.dst_port_mask),
-   rte_be_to_cpu_16(info->mask.src_port_mask));
+   /* store the TCP/UDP port masks */
+   fdirtcpm = rte_be_to_cpu_16(info->mask.dst_port_mask) << 16;
+   fdirtcpm |= rte_be_to_cpu_16(info->mask.src_port_mask);
 
/* write all the same so that UDP, TCP and SCTP use the same mask
 * (little-endian)
-- 
2.21.0.windows.1

Re: [dpdk-dev] [PATCH v2] bus/vmbus: Fix crash when handling packets in secondary process

[Long Li]

I think the code is on the right track.

Instead of using vmbus_uio_get_num_subchan() and calling 
vmbus_uio_get_subchan() on each channel, you can just create a new function 
vmbus_uio_get_secondary_subchan(). This function goes through all subchannels 
and map ring buffers to the same addresses used by the primary process.

In the original code, vmbus_uio_map_secondary_subchan() has a check for "if 
(mapaddr == ring_buf)". If the address is mapped to somewhere else, the address 
won't work for the secondary process. So we need to keep this check.

From: Stephen Hemminger 
Sent: Wednesday, August 11, 2021 8:26 AM
To: Jonathan Erb ; Long Li 
Cc: dev@dpdk.org; sta...@dpdk.org
Subject: RE: [PATCH v2] bus/vmbus: Fix crash when handling packets in secondary 
process

Looks fine, only comments would be to keep to original style and add check if 
primary channel not found?

From: Jonathan Erb 
mailto:jonathan@banduracyber.com>>
Sent: Monday, August 9, 2021 9:07 AM
To: Long Li mailto:lon...@microsoft.com>>; Stephen 
Hemminger mailto:sthem...@microsoft.com>>
Cc: dev@dpdk.org; sta...@dpdk.org
Subject: Re: [PATCH v2] bus/vmbus: Fix crash when handling packets in secondary 
process

You don't often get email from 
jonathan@banduracyber.com. Learn why 
this is important

Would it be possible to resolve the lack of subchannels in secondary processes 
as follows:



1. Create the following function in vmbus_uio.c to obtain the count of 
subchannels created by the primary:
int vmbus_uio_get_num_subchan(struct vmbus_channel *primary)
{

const struct rte_vmbus_device *dev = primary->device;
char chan_path[PATH_MAX];
struct dirent *ent;
DIR *chan_dir;
int err;
int subchan_cnt = 0;

snprintf(chan_path, sizeof(chan_path),
 "%s/%s/channels",
 SYSFS_VMBUS_DEVICES, dev->device.name);

chan_dir = opendir(chan_path);
if (!chan_dir) {
VMBUS_LOG(ERR, "cannot open %s: %s",
  chan_path, strerror(errno));
return 0;
}

while ((ent = readdir(chan_dir))) {
unsigned long relid, subid, monid;
char *endp;

if (ent->d_name[0] == '.')
continue;

errno = 0;
relid = strtoul(ent->d_name, &endp, 0);
if (*endp || errno != 0 || relid > UINT16_MAX) {
VMBUS_LOG(NOTICE, "not a valid channel relid: %s",
  ent->d_name);
continue;
}
subchan_cnt++;
}

closedir(chan_dir);
//Less one for primary channel
return subchan_cnt - 1;

}



2. Then change the bottom of vmbus_uio_map_secondary() to be simply:
/* fd is not needed in secondary process, close it */
close(fd);

if (vmbus_chan_create(dev, dev->relid, 0,
dev->monitor_id, &dev->primary)) {
VMBUS_LOG(ERR, "cannot create primary channel");
return -1;
}

int subchannels_cnt = vmbus_uio_get_num_subchan(dev->primary);
for (i = 0; i < subchannels_cnt; i++) {
if(vmbus_uio_get_subchan(dev->primary, &chan))
return -1;
STAILQ_INSERT_TAIL(&dev->primary->subchannel_list, chan, next);
}
return 0;
}



In this way the secondary processes detect the number of subchannels created by 
the primary, then perform their own mappings as needed. All this can occur 
before rte_eal_init has completed.



Jonathan




On 7/26/21 6:16 PM, Long Li wrote:

Subject: [PATCH v2] bus/vmbus: Fix crash when handling packets in

secondary process



Have secondary processes construct their own copy of primary channel with

own mappings.



Remove vmbus_channel primary ptr from struct mapped_vmbus_resource

as its not used.



Populate virtual memory address "addr" in struct rte_mem_resource for

secondary processes as netvsc will attempt to reference it thus causing a

crash. It was initialized for primary processes but not for secondary.

Cc: sta...@dpdk.org



Signed-off-by: Jonathan Erb 


---

v2:

* Remove unnecessary check for NULL pointer before call to rte_free() per

reviwer comment.



 drivers/bus/vmbus/private.h  |  1 -

 drivers/bus/vmbus/vmbus_channel.c|  4 +---

 drivers/bus/vmbus/vmbus_common_uio.c | 14 +-

 3 files changed, 10 insertions(+), 9 deletions(-)



diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h

index 528d60a42f..746212bd5f 100644

--- a/drivers/bus/vmbus/private.h

+++ b/drivers/bus/vmbus/private.h

@@ -42,7 +42,6 @@ struct mapped_vmbus_resource {



   rte_uuid_t id;

   int nb_maps;

-  struct vmbus_channel *primary;

   struct vmbus_map maps[VMBUS_MAX_RESOURCE];

   char path[PATH_MAX];

 };

diff --git a/drivers/bus/vmbus/vmbus_channel.c

b/drivers/bus/vmbus/vmbus_channel.c

index f67f1c438a..119b9b367e 100644

--- a/drivers/bus/vmbus/vmbus_channel.c

+++ b/