Re: [dpdk-dev] [PATCH] add flow shared action API
Hi, Jerin & Thomas. On Fri, Jul 3, 2020 at 6:21 PM Thomas Monjalon wrote: > 03/07/2020 17:02, Jerin Jacob: > > On Fri, Jul 3, 2020 at 8:07 PM Andrey Vesnovaty > wrote: > > > Signed-off-by: Andrey Vesnovaty > > > Signed-off-by: Andrey Vesnovaty > > > Signed-off-by: Andrey Vesnovaty > > > > Duplicate Signoffs. > > > > # Request to CC all the people who are already giving comments to this > patch. > > Yes you are absolutely right Jerin. > It is said in the contribution docs, but we must repeat it again > and again, especially for newcomers like here. > > The CC list is very important! > Please pay attention to Cc the relevant maintainers > AND the persons who already showed some interest. > Just to clarify my next steps: - remove irrelevant signoffs. - all those who already showed interest should be added to CC field - send PATCH v2 in reply to this email. Thanks & sorry for the mess. > > It may be a blocker for merge, thanks for sharing with co-workers. > > >
Re: [dpdk-dev] [PATCH] add flow shared action API
Thanks,
Andrey Vesnovaty
(+972)*526775512* | *Skype:* andrey775512
On Fri, Jul 3, 2020 at 6:02 PM Jerin Jacob wrote:
> On Fri, Jul 3, 2020 at 8:07 PM Andrey Vesnovaty
> wrote:
> >
> > From: Andrey Vesnovaty
> >
> > This commit introduces extension of DPDK flow action API enabling
> > sharing of single rte_flow_action in multiple flows. The API intended for
> > PMDs where multiple HW offloaded flows can reuse the same HW
> > essence/object representing flow action and modification of such an
> > essence/object effects all the rules using it.
> >
> > Motivation and example
> > ===
> > Adding or removing one or more queues to RSS used by multiple flow rules
> > imposes per rule toll for current DPDK flow API; the scenario requires
> > for each flow sharing cloned RSS action:
> > - call `rte_flow_destroy()`
> > - call `rte_flow_create()` with modified RSS action
> >
> > API for sharing action and its in-place update benefits:
> > - reduce the overhead of multiple RSS flow rules reconfiguration .
> > - optimize resource utilization by sharing action across of of multiple
> > flows
> >
> > Change description
> > ===
> >
> > Shared action
> > ===
> > In order to represent flow action shared by multiple flows new action
> > type RTE_FLOW_ACTION_TYPE_SHARED introduced (see `enum
> > rte_flow_action_type`).
> > Actually the introduced API decouples action from any specific flow and
> > enables sharing of single action by its handle for multiple flows.
> >
> > Shared action create/use/destroy
> > ===
> > Shared action may be reused by some or none flow rules at any given
> > moment, IOW shared action reside outside of the context of any flow.
> > Shared action represent HW resources/objects used for action offloading
> > implementation. For allocation/release of all HW resources and all
> > related initializations/cleanups in PMD space required for shared action
> > implementation added new API
> > rte_flow_shared_action_create()/rte_flow_shared_action_destroy().
> > In addition to the above all preparations needed to maintain shared
> > access to the action resources, configuration and state should be done in
> > rte_flow_shared_action_create().
> >
> > In order to share some flow action reuse the handle of type
> > `struct rte_flow_shared_action` returned by
> > rte_flow_shared_action_create() as a `conf` field of
> > `struct rte_flow_action` (see "example" section).
> >
> > If some shared action not used by any flow rule all resources allocated
> > by the shared action can be released by rte_flow_shared_action_destroy()
> > (see "example" section). The shared action handle passed as argument to
> > destroy API should not be used i.e. result of the usage is undefined.
> >
> > Shared action re-configuration
> > ===
> > Shared action behavior defined by its configuration & and can be updated
> > via rte_flow_shared_action_update() (see "example" section). The shared
> > action update operation modifies HW related resources/objects allocated
> > by the action. The number of operations performed by the update operation
> > should not be dependent on number of flows sharing the related action.
> > On return of shared action updated API action behavior should be
> > according to updated configuration for all flows sharing the action.
> >
> > Shared action query
> > ===
> > Provide separate API to query shared action sate (see
> > rte_flow_shared_action_update()). Taking a counter as an example: query
> > returns value aggregating all counter increments across all flow rules
> > sharing the counter.
> >
> > PMD support
> > ===
> > The support of introduced API is pure PMD specific design and
> > responsibility for each action type (see struct rte_flow_ops).
> >
> > testpmd
> > ===
> > In order to utilize introduced API testpmd cli may implement following
> > extension
> > create/update/destroy/query shared action accordingly
> >
> > flow shared_action create {port_id} [index] {action}
> > flow shared_action update {port_id} {index} {action}
> > flow shared_action destroy {port_id} {index}
> > flow shared_action query {port_id} {index}
> >
> > testpmd example
> > ===
> >
> > configure rss to queues 1 & 2
> >
> > testpmd> flow shared_action create 0 100 rss 1 2
> >
> > create flow rule utilizing shared action
> >
> > testpmd> flow create 0 ingress \
> > pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \
> > actions shared 100 end / end
> >
> > add 2 more queues
> >
> > testpmd> flow shared_action modify 0 100 rss 1 2 3 4
> >
> > example
> > ===
> >
> > struct rte_flow_action actions[2];
> > struct rte_flow_action action;
> > /* skipped: initialize action */
> > struct rte_flow_shared_action *handle = rte_flow_shared_action_create(
> > port_id, &action, &error);
> > actions[0].type = RTE_FLOW_ACTION_TYPE_SHARED;
> > actions[0].conf = handle;
> > actions[1].type = RTE_FLOW_ACTION_TYPE_END;
> > /* skipped: init attr0 & pattern0 args */
> > struct rte_flow *flow0 = r
[dpdk-dev] [PATCH 4/4] doc: update feature list of hns3 rst document
From: Lijun Ou This patch updates the feature list for hns3 PMD driver document. Signed-off-by: Lijun Ou Signed-off-by: Wei Hu (Xavier) --- doc/guides/nics/hns3.rst | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst index ae3c5f6..a62fcfd 100644 --- a/doc/guides/nics/hns3.rst +++ b/doc/guides/nics/hns3.rst @@ -25,7 +25,16 @@ Features of the HNS3 PMD are: - Jumbo frames - Link state information - Interrupt mode for RX -- VLAN stripping +- VLAN stripping and inserting +- QinQ inserting +- DCB +- Scattered and gather for TX and RX +- Flow director +- Dump register +- SR-IOV VF +- Multi-process +- MAC/VLAN filter +- MTU update - NUMA support Prerequisites -- 2.7.4
[dpdk-dev] [PATCH 2/4] net/hns3: cleanup duplicate code when processing TSO in Tx
From: Chengchang Tang
This patch fixes up paylen calculation twice when processing TSO request
in the '.tx_pkt_burst' ops implementation function to avoid performance
loss.
Fixes: 6dca716c9e1d ("net/hns3: support TSO")
Cc: sta...@dpdk.org
Signed-off-by: Chengchang Tang
Signed-off-by: Wei Hu (Xavier)
Signed-off-by: Hongbo Zheng
---
drivers/net/hns3/hns3_rxtx.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 8892fc1..07640db 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -1979,12 +1979,11 @@ hns3_pkt_is_tso(struct rte_mbuf *m)
}
static void
-hns3_set_tso(struct hns3_desc *desc,
-uint64_t ol_flags, struct rte_mbuf *rxm)
+hns3_set_tso(struct hns3_desc *desc, uint64_t ol_flags,
+ uint32_t paylen, struct rte_mbuf *rxm)
{
- uint32_t paylen, hdr_len;
- uint32_t tmp;
uint8_t l2_len = rxm->l2_len;
+ uint32_t tmp;
if (!hns3_pkt_is_tso(rxm))
return;
@@ -1992,10 +1991,6 @@ hns3_set_tso(struct hns3_desc *desc,
if (hns3_tso_proc_tunnel(desc, ol_flags, rxm, &l2_len))
return;
- hdr_len = rxm->l2_len + rxm->l3_len + rxm->l4_len;
- hdr_len += (ol_flags & PKT_TX_TUNNEL_MASK) ?
- rxm->outer_l2_len + rxm->outer_l3_len : 0;
- paylen = rxm->pkt_len - hdr_len;
if (paylen <= rxm->tso_segsz)
return;
@@ -2036,7 +2031,7 @@ fill_desc(struct hns3_tx_queue *txq, uint16_t tx_desc_id,
struct rte_mbuf *rxm,
rxm->outer_l2_len + rxm->outer_l3_len : 0;
paylen = rxm->pkt_len - hdr_len;
desc->tx.paylen = rte_cpu_to_le_32(paylen);
- hns3_set_tso(desc, ol_flags, rxm);
+ hns3_set_tso(desc, ol_flags, paylen, rxm);
}
hns3_set_bit(rrcfv, HNS3_TXD_FE_B, frag_end);
--
2.7.4
[dpdk-dev] [PATCH 0/4] updates for hns3 PMD driver
This series are updates for hns3 PMD driver. Chengchang Tang (1): net/hns3: cleanup duplicate code when processing TSO in Tx Lijun Ou (1): doc: update feature list of hns3 rst document Wei Hu (Xavier) (2): net/hns3: check if registering mp action successfully net/hns3: fix VLAN tag inserted in wrong position in Tx doc/guides/nics/hns3.rst | 11 +++- drivers/net/hns3/hns3_ethdev.c| 22 +++- drivers/net/hns3/hns3_ethdev_vf.c | 20 ++- drivers/net/hns3/hns3_mp.c| 34 ++-- drivers/net/hns3/hns3_mp.h| 4 +- drivers/net/hns3/hns3_rxtx.c | 113 +++--- 6 files changed, 147 insertions(+), 57 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 3/4] net/hns3: fix VLAN tag inserted in wrong position in Tx
Based on hns3 network engine, in order to configure hardware VLAN insert
offload in Tx direction, PMD driver reads the VLAN tags from the
vlan_tci_outer and vlan_tci of the structure rte_mbuf, fills them into the
Tx Buffer Descriptor and sets the related offload flag for every packet.
Currently, there are two VLAN related problems in the 'tx_pkt_burst' ops
implementation function:
1) When setting the related offload flag, PMD driver inserts the VLAN tag
into the position that close to L3 header. So, when upper application
sends a packet with a VLAN tag in the data buffer, the VLAN offloaded
by hardware will be added to the wrong position. It is supposed to add
the VLAN tag from the rte_mbuf to the position close to the MAC header
in the packet when using VLAN insertion.
And when PF PVID is enabled by calling the API function named
rte_eth_dev_set_vlan_pvid or VF PVID is enabled by hns3 PF kernel ether
driver, the VLAN tag from the structure rte_mbuf to enable the VLAN
insertion should be filled into the position that close to L3 header to
avoid to be overwittern by the PVID which will always be inserted in the
position that close to the MAC address.
2) When sending multiple segment packets, VLAN information is required to
be filled into the first Tx Buffer descriptor. However, currently hns3
PMD driver incorrectly placed it in the last Tx Buffer Descriptor. This
results in VLAN insert offload failure when sending multiple segment
packets.
This patch fixed them by filling the VLAN information into the position of
the Tx Buffer Descriptor.
Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: sta...@dpdk.org
Signed-off-by: Chengchang Tang
Signed-off-by: Wei Hu (Xavier)
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_rxtx.c | 102 +++
1 file changed, 65 insertions(+), 37 deletions(-)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 07640db..b0253d5 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2007,51 +2007,58 @@ hns3_set_tso(struct hns3_desc *desc, uint64_t ol_flags,
desc->tx.mss = rte_cpu_to_le_16(rxm->tso_segsz);
}
+static inline void
+hns3_fill_per_desc(struct hns3_desc *desc, struct rte_mbuf *rxm)
+{
+ desc->addr = rte_mbuf_data_iova(rxm);
+ desc->tx.send_size = rte_cpu_to_le_16(rte_pktmbuf_data_len(rxm));
+ desc->tx.tp_fe_sc_vld_ra_ri = rte_cpu_to_le_16(BIT(HNS3_TXD_VLD_B));
+}
+
static void
-fill_desc(struct hns3_tx_queue *txq, uint16_t tx_desc_id, struct rte_mbuf *rxm,
- bool first, int offset)
+hns3_fill_first_desc(struct hns3_tx_queue *txq, struct hns3_desc *desc,
+struct rte_mbuf *rxm)
{
- struct hns3_desc *tx_ring = txq->tx_ring;
- struct hns3_desc *desc = &tx_ring[tx_desc_id];
- uint8_t frag_end = rxm->next == NULL ? 1 : 0;
uint64_t ol_flags = rxm->ol_flags;
- uint16_t size = rxm->data_len;
- uint16_t rrcfv = 0;
uint32_t hdr_len;
uint32_t paylen;
- uint32_t tmp;
- desc->addr = rte_mbuf_data_iova(rxm) + offset;
- desc->tx.send_size = rte_cpu_to_le_16(size);
- hns3_set_bit(rrcfv, HNS3_TXD_VLD_B, 1);
-
- if (first) {
- hdr_len = rxm->l2_len + rxm->l3_len + rxm->l4_len;
- hdr_len += (ol_flags & PKT_TX_TUNNEL_MASK) ?
+ hdr_len = rxm->l2_len + rxm->l3_len + rxm->l4_len;
+ hdr_len += (ol_flags & PKT_TX_TUNNEL_MASK) ?
rxm->outer_l2_len + rxm->outer_l3_len : 0;
- paylen = rxm->pkt_len - hdr_len;
- desc->tx.paylen = rte_cpu_to_le_32(paylen);
- hns3_set_tso(desc, ol_flags, paylen, rxm);
- }
-
- hns3_set_bit(rrcfv, HNS3_TXD_FE_B, frag_end);
- desc->tx.tp_fe_sc_vld_ra_ri = rte_cpu_to_le_16(rrcfv);
-
- if (frag_end) {
- if (ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_QINQ_PKT)) {
- tmp = rte_le_to_cpu_32(desc->tx.type_cs_vlan_tso_len);
- hns3_set_bit(tmp, HNS3_TXD_VLAN_B, 1);
- desc->tx.type_cs_vlan_tso_len = rte_cpu_to_le_32(tmp);
- desc->tx.vlan_tag = rte_cpu_to_le_16(rxm->vlan_tci);
- }
+ paylen = rxm->pkt_len - hdr_len;
+ desc->tx.paylen = rte_cpu_to_le_32(paylen);
+ hns3_set_tso(desc, ol_flags, paylen, rxm);
- if (ol_flags & PKT_TX_QINQ_PKT) {
- tmp = rte_le_to_cpu_32(desc->tx.ol_type_vlan_len_msec);
- hns3_set_bit(tmp, HNS3_TXD_OVLAN_B, 1);
- desc->tx.ol_type_vlan_len_msec = rte_cpu_to_le_32(tmp);
+ /*
+* Currently, hardware doesn't support more than two layers VLAN offload
+* in Tx direction based on hns3 network engine. So when the number of
+* VLANs in the packets represented by rxm plus the number of VLAN
[dpdk-dev] [PATCH 1/4] net/hns3: check if registering mp action successfully
Currently, there is a coverity defect warning about hns3 PMD driver, the
detail information as blow:
CID 289969 (#1 of 1): Unchecked return value (CHECKED_RETURN)
1. check_return: Calling rte_mp_action_register without checking return
value (as is done elsewhere 11 out of 13 times).
The problem is that missing checking the return value of calling the API
rte_mp_action_register during initialization. If regitering an action
function for primary and secondary communication failed, the secondary
process can't work properly.
This patch fixes it by adding check return value of the API function
named rte_mp_action_register in the '.dev_init' implementation function
of hns3 PMD driver.
Coverity issue: 289969
Fixes: 23d4b61fee5d ("net/hns3: support multiple process")
Cc: sta...@dpdk.org
Signed-off-by: Lijun Ou
Signed-off-by: Wei Hu (Xavier)
---
drivers/net/hns3/hns3_ethdev.c| 22 --
drivers/net/hns3/hns3_ethdev_vf.c | 20 ++--
drivers/net/hns3/hns3_mp.c| 34 +-
drivers/net/hns3/hns3_mp.h| 4 ++--
4 files changed, 69 insertions(+), 11 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 00ed3e2..265d620 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5451,12 +5451,25 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
hns3_set_rxtx_function(eth_dev);
eth_dev->dev_ops = &hns3_eth_dev_ops;
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
- hns3_mp_init_secondary();
+ ret = hns3_mp_init_secondary();
+ if (ret) {
+ PMD_INIT_LOG(ERR, "Failed to init for secondary "
+"process, ret = %d", ret);
+ goto err_mp_init_secondary;
+ }
+
hw->secondary_cnt++;
return 0;
}
- hns3_mp_init_primary();
+ ret = hns3_mp_init_primary();
+ if (ret) {
+ PMD_INIT_LOG(ERR,
+"Failed to init for primary process, ret = %d",
+ret);
+ goto err_mp_init_primary;
+ }
+
hw->adapter_state = HNS3_NIC_UNINITIALIZED;
hns->is_vf = false;
hw->data = eth_dev->data;
@@ -5517,7 +5530,12 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
err_init_pf:
rte_free(hw->reset.wait_data);
+
err_init_reset:
+ hns3_mp_uninit_primary();
+
+err_mp_init_primary:
+err_mp_init_secondary:
eth_dev->dev_ops = NULL;
eth_dev->rx_pkt_burst = NULL;
eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
b/drivers/net/hns3/hns3_ethdev_vf.c
index 3c5998a..54e5dac 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2524,12 +2524,24 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
hns3_set_rxtx_function(eth_dev);
eth_dev->dev_ops = &hns3vf_eth_dev_ops;
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
- hns3_mp_init_secondary();
+ ret = hns3_mp_init_secondary();
+ if (ret) {
+ PMD_INIT_LOG(ERR, "Failed to init for secondary "
+ "process, ret = %d", ret);
+ goto err_mp_init_secondary;
+ }
+
hw->secondary_cnt++;
return 0;
}
- hns3_mp_init_primary();
+ ret = hns3_mp_init_primary();
+ if (ret) {
+ PMD_INIT_LOG(ERR,
+"Failed to init for primary process, ret = %d",
+ret);
+ goto err_mp_init_primary;
+ }
hw->adapter_state = HNS3_NIC_UNINITIALIZED;
hns->is_vf = true;
@@ -2586,6 +2598,10 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
rte_free(hw->reset.wait_data);
err_init_reset:
+ hns3_mp_uninit_primary();
+
+err_mp_init_primary:
+err_mp_init_secondary:
eth_dev->dev_ops = NULL;
eth_dev->rx_pkt_burst = NULL;
eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
index 596c310..639f46c 100644
--- a/drivers/net/hns3/hns3_mp.c
+++ b/drivers/net/hns3/hns3_mp.c
@@ -14,6 +14,8 @@
#include "hns3_rxtx.h"
#include "hns3_mp.h"
+static bool hns3_inited;
+
/*
* Initialize IPC message.
*
@@ -192,9 +194,20 @@ void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev)
/*
* Initialize by primary process.
*/
-void hns3_mp_init_primary(void)
+int hns3_mp_init_primary(void)
{
- rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
+ int ret;
+
+ if (!hns3_inited) {
+ /* primary is allowed to not support IPC */
+ ret = rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
+ if (ret && rte_errno != ENOTSUP)
+ return re
[dpdk-dev] [PATCH v3] build: check functionality rather than binutils version
Rather than checking the binutils version number, which can lead to
unnecessary disabling of AVX512 if fixes have been backported to distro
versions, we can instead check the output of "as" from binutils to see if
it is correct.
The check in the script uses the minimal assembly reproduction code posted
to the public bug tracker for gcc/binutils for those issues [1]. If the
binutils bug is present, the instruction parameters - specifically the
displacement parameter - will be different in the disassembled output
compared to the input. Therefore the check involves assembling a single
instruction and disassembling it again, checking that the two match.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90028
Signed-off-by: Bruce Richardson
Tested-by: Harry van Haaren
---
V3:
- Use mktemp to create a temporary file rather than place object file in
build folder. Use trap to ensure temp file deletion on exit.
V2:
- Renamed "as_ok" variable to "binutils_ok" for readability
- Removed one test case from the script because even though two DPDK bugs
were filed, the one binutils bug is the root cause, and a single commit
fixes them both
- Changed message() to warning() in the printout
---
buildtools/binutils-avx512-check.sh | 16
buildtools/meson.build | 3 +--
config/x86/meson.build | 10 +++---
meson.build | 5 -
4 files changed, 24 insertions(+), 10 deletions(-)
create mode 100755 buildtools/binutils-avx512-check.sh
diff --git a/buildtools/binutils-avx512-check.sh
b/buildtools/binutils-avx512-check.sh
new file mode 100755
index 0..a7e068140
--- /dev/null
+++ b/buildtools/binutils-avx512-check.sh
@@ -0,0 +1,16 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+AS=${AS:-as}
+OBJFILE=$(mktemp -t dpdk.binutils-check.XX.o)
+trap 'rm -f "$OBJFILE"' EXIT
+# from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90028
+GATHER_PARAMS='0x8(,%ymm1,1),%ymm0{%k2}'
+
+# assemble vpgather to file and similarly check
+echo "vpgatherqq $GATHER_PARAMS" | $AS --64 -o $OBJFILE -
+objdump -d --no-show-raw-insn $OBJFILE | grep -q $GATHER_PARAMS || {
+ echo "vpgatherqq displacement error with as"
+ exit 1
+}
diff --git a/buildtools/meson.build b/buildtools/meson.build
index f9d2fdf74..cf0048f3c 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -1,13 +1,12 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2017-2019 Intel Corporation
-subdir('pmdinfogen')
-
pkgconf = find_program('pkg-config', 'pkgconf', required: false)
pmdinfo = find_program('gen-pmdinfo-cfile.sh')
list_dir_globs = find_program('list-dir-globs.py')
check_symbols = find_program('check-symbols.sh')
ldflags_ibverbs_static = find_program('options-ibverbs-static.sh')
+binutils_avx512_check = find_program('binutils-avx512-check.sh')
# set up map-to-win script using python, either built-in or external
python3 = import('python').find_installation(required: false)
diff --git a/config/x86/meson.build b/config/x86/meson.build
index adc857ba2..6ec020ef6 100644
--- a/config/x86/meson.build
+++ b/config/x86/meson.build
@@ -3,14 +3,10 @@
# get binutils version for the workaround of Bug 97
if not is_windows
- ldver = run_command('ld', '-v').stdout().strip()
- if ldver.contains('2.30') and cc.has_argument('-mno-avx512f')
+ binutils_ok = run_command(binutils_avx512_check)
+ if binutils_ok.returncode() != 0 and cc.has_argument('-mno-avx512f')
machine_args += '-mno-avx512f'
- message('Binutils 2.30 detected, disabling AVX512 support as
workaround for bug #97')
- endif
- if ldver.contains('2.31') and cc.has_argument('-mno-avx512f')
- machine_args += '-mno-avx512f'
- message('Binutils 2.31 detected, disabling AVX512 support as
workaround for bug #249')
+ warning('Binutils error with AVX512 assembly, disabling AVX512
support')
endif
endif
diff --git a/meson.build b/meson.build
index d21adfd30..7c336df6d 100644
--- a/meson.build
+++ b/meson.build
@@ -40,10 +40,13 @@ global_inc = include_directories('.', 'config',
'lib/librte_eal/@0@/include'.format(host_machine.system()),
'lib/librte_eal/@0@/include'.format(arch_subdir),
)
+
+# do configuration and get tool paths
+subdir('buildtools')
subdir('config')
# build libs and drivers
-subdir('buildtools')
+subdir('buildtools/pmdinfogen')
subdir('lib')
subdir('drivers')
--
2.25.1
Re: [dpdk-dev] [PATCH] add flow shared action API
On Sat, Jul 4, 2020 at 3:40 PM Andrey Vesnovaty
wrote:
>
>
> Thanks,
>
> Andrey Vesnovaty
> (+972)526775512 | Skype: andrey775512
>
>
> On Fri, Jul 3, 2020 at 6:02 PM Jerin Jacob wrote:
>>
>> On Fri, Jul 3, 2020 at 8:07 PM Andrey Vesnovaty wrote:
>> >
>> > From: Andrey Vesnovaty
>> >
>> > This commit introduces extension of DPDK flow action API enabling
>> > sharing of single rte_flow_action in multiple flows. The API intended for
>> > PMDs where multiple HW offloaded flows can reuse the same HW
>> > essence/object representing flow action and modification of such an
>> > essence/object effects all the rules using it.
>> >
>> > Motivation and example
>> > ===
>> > Adding or removing one or more queues to RSS used by multiple flow rules
>> > imposes per rule toll for current DPDK flow API; the scenario requires
>> > for each flow sharing cloned RSS action:
>> > - call `rte_flow_destroy()`
>> > - call `rte_flow_create()` with modified RSS action
>> >
>> > API for sharing action and its in-place update benefits:
>> > - reduce the overhead of multiple RSS flow rules reconfiguration .
>> > - optimize resource utilization by sharing action across of of multiple
>> > flows
>> >
>> > Change description
>> > ===
>> >
>> > Shared action
>> > ===
>> > In order to represent flow action shared by multiple flows new action
>> > type RTE_FLOW_ACTION_TYPE_SHARED introduced (see `enum
>> > rte_flow_action_type`).
>> > Actually the introduced API decouples action from any specific flow and
>> > enables sharing of single action by its handle for multiple flows.
>> >
>> > Shared action create/use/destroy
>> > ===
>> > Shared action may be reused by some or none flow rules at any given
>> > moment, IOW shared action reside outside of the context of any flow.
>> > Shared action represent HW resources/objects used for action offloading
>> > implementation. For allocation/release of all HW resources and all
>> > related initializations/cleanups in PMD space required for shared action
>> > implementation added new API
>> > rte_flow_shared_action_create()/rte_flow_shared_action_destroy().
>> > In addition to the above all preparations needed to maintain shared
>> > access to the action resources, configuration and state should be done in
>> > rte_flow_shared_action_create().
>> >
>> > In order to share some flow action reuse the handle of type
>> > `struct rte_flow_shared_action` returned by
>> > rte_flow_shared_action_create() as a `conf` field of
>> > `struct rte_flow_action` (see "example" section).
>> >
>> > If some shared action not used by any flow rule all resources allocated
>> > by the shared action can be released by rte_flow_shared_action_destroy()
>> > (see "example" section). The shared action handle passed as argument to
>> > destroy API should not be used i.e. result of the usage is undefined.
>> >
>> > Shared action re-configuration
>> > ===
>> > Shared action behavior defined by its configuration & and can be updated
>> > via rte_flow_shared_action_update() (see "example" section). The shared
>> > action update operation modifies HW related resources/objects allocated
>> > by the action. The number of operations performed by the update operation
>> > should not be dependent on number of flows sharing the related action.
>> > On return of shared action updated API action behavior should be
>> > according to updated configuration for all flows sharing the action.
>> >
>> > Shared action query
>> > ===
>> > Provide separate API to query shared action sate (see
>> > rte_flow_shared_action_update()). Taking a counter as an example: query
>> > returns value aggregating all counter increments across all flow rules
>> > sharing the counter.
>> >
>> > PMD support
>> > ===
>> > The support of introduced API is pure PMD specific design and
>> > responsibility for each action type (see struct rte_flow_ops).
>> >
>> > testpmd
>> > ===
>> > In order to utilize introduced API testpmd cli may implement following
>> > extension
>> > create/update/destroy/query shared action accordingly
>> >
>> > flow shared_action create {port_id} [index] {action}
>> > flow shared_action update {port_id} {index} {action}
>> > flow shared_action destroy {port_id} {index}
>> > flow shared_action query {port_id} {index}
>> >
>> > testpmd example
>> > ===
>> >
>> > configure rss to queues 1 & 2
>> >
>> > testpmd> flow shared_action create 0 100 rss 1 2
>> >
>> > create flow rule utilizing shared action
>> >
>> > testpmd> flow create 0 ingress \
>> > pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \
>> > actions shared 100 end / end
>> >
>> > add 2 more queues
>> >
>> > testpmd> flow shared_action modify 0 100 rss 1 2 3 4
>> >
>> > example
>> > ===
>> >
>> > struct rte_flow_action actions[2];
>> > struct rte_flow_action action;
>> > /* skipped: initialize action */
>> > struct rte_flow_shared_action *handle = rte_flow_shared_action_create(
>> > port_id, &action, &error);
>> > actions[0].type =
Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow
On 7/2/20 9:43 PM, Jiawei Wang wrote:
> When using full offload, all traffic will be handled by the HW, and
> directed to the requested vf or wire, the control application loses
vf->VF
> visibility on the traffic.
> So there's a need for an action that will enable the control application
> some visibility.
>
> The solution is introduced a new action that will sample the incoming
> traffic and send a duplicated traffic in some predefined ratio to the
> application, while the original packet will continue to the target
> destination.
>
May be 1 packet per second is a better sampling approach?
Or just different.
> The packets sampled equals is '1/ratio', if the ratio value be set to 1
> , means that the packets would be completely mirrored. The sample packet
Comma on the next line looks bad.
> can be assigned with different set of actions from the original packet.
>
> In order to support the sample packet in rte_flow, new rte_flow action
> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
> will be introduced.
>
> Signed-off-by: Jiawei Wang
> Acked-by: Ori Kam
> ---
> doc/guides/prog_guide/rte_flow.rst | 25 +
> doc/guides/rel_notes/release_20_08.rst | 6 ++
> lib/librte_ethdev/rte_flow.c | 1 +
> lib/librte_ethdev/rte_flow.h | 28
> 4 files changed, 60 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index d5dd18c..50dfe1f 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2645,6 +2645,31 @@ timeout passed without any matching on the flow.
> | ``context`` | user input flow context |
> +--+-+
>
> +Action: ``SAMPLE``
> +^^
> +
> +Adds a sample action to a matched flow.
> +
> +The matching packets will be duplicated to a special queue or vport
what is vport above?
> +with the predefined ``ratio``, the packets sampled equals is '1/ratio'.
> +All the packets continues to the target destination.
continues -> continue (if I'm not mistaken)
> +
> +When the ``ratio`` is set to 1 then the packets will be 100% mirrored.
> +``actions`` represent the different set of actions for the sampled or
> mirrored
> +packets.
> +
> +.. _table_rte_flow_action_sample:
> +
> +.. table:: SAMPLE
> +
> + +--+-+
> + | Field| Value |
> + +==+=+
> + | ``ratio``| 32 bits sample ratio value |
> + +--+-+
> + | ``actions`` | sub-action list for sampling|
> + +--+-+
> +
> Negative types
> ~~
>
> diff --git a/doc/guides/rel_notes/release_20_08.rst
> b/doc/guides/rel_notes/release_20_08.rst
> index 5cbc4ce..313e8d3 100644
> --- a/doc/guides/rel_notes/release_20_08.rst
> +++ b/doc/guides/rel_notes/release_20_08.rst
> @@ -81,6 +81,12 @@ New Features
>* Added support for virtio queue statistics.
>* Added support for MTU update.
>
> +* **Added flow-based traffic sampling support.**
> +
> + Added new action: ``RTE_FLOW_ACTION_TYPE_SAMPLE`` to duplicate the matching
> + packets with given ratio and redirects to vport or queue. The sampled
> packets
What is vport above?
> + also can be assigned with an additional optional actions.
May actions list be empty or NULL? If no, it does not look
optional.
> +
> * **Updated Marvell octeontx2 ethdev PMD.**
>
>Updated Marvell octeontx2 driver with cn98xx support.
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 1685be5..733871d 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -173,6 +173,7 @@ struct rte_flow_desc_data {
> MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct rte_flow_action_set_dscp)),
> MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct rte_flow_action_set_dscp)),
> MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
> + MK_FLOW_ACTION(SAMPLE, sizeof(struct rte_flow_action_sample)),
> };
>
> int
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b0e4199..c9cd80d 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -2099,6 +2099,13 @@ enum rte_flow_action_type {
>* see enum RTE_ETH_EVENT_FLOW_AGED
>*/
> RTE_FLOW_ACTION_TYPE_AGE,
> +
> + /**
> + * Redirects specific ratio of packets to vport or queue.
> + *
> + * See struct rte_flow_action_sample.
> + */
> + RTE_FLOW_ACTION_TYPE_SAMPLE,
> };
>
> /**
> @@ -2709,6 +2716,27 @@ struct rte_flow_action {
> struct rte_flow;
>
> /**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_SAMPLE
> + *
> + * A
Re: [dpdk-dev] [PATCH v4] examples/l2fwd: add cmdline option for forwarding port info
On Mon, May 25, 2020 at 2:59 PM Bruce Richardson
wrote:
>
> On Sun, May 24, 2020 at 06:13:22PM +0200, Thomas Monjalon wrote:
> > Bruce, as maintainer of l2fwd example, any opinion about this change?
> >
> Assuming all previous discussion on it is resolved, I'm fine with this
> patch, though I suspect it will only make 20.08 now.
>
> Acked-by: Bruce Richardson
Ping for merge.
>
> >
> > 11/05/2020 02:23, Pavan Nikhilesh Bhagavatula:
> > > Hi Vipin,
> > >
> > > >Hi Pavan,
> > > >
> > > >snipped
> > > >> >
> > > >> >Should we check & warn the user if
> > > >> >1. port speed mismatch
> > > >> >2. on different NUMA
> > > >> >3. port pairs are physical and vdev like tap, and KNI (performance).
> > > >> >
> > > >>
> > > >> Sure, it can be a separate patch as it will be applicable for multiple
> > > >examples.
> > > >I believe this patch is for example `l2fwd`. But you would like to have
> > > >to
> > > >updated for all `example`. I am ok for this.
> > > >
> > > >snipped
> > > >> >
> > > >> >Should not the check_port_pair be after this? If the port is not
> > > >> >enabled in port_mask will you skip that pair? or skip RX-TX from that
> > > >port?
> > > >>
> > > >> We check every port pair against l2fwd_enabled_port_mask in
> > > >> check_port_pair_config()
> > > >
> > > >
> > > >>
> > > >snipped
> > > >> >
> > > >> >As mentioned above there can ports in mask which might be
> > > >disabled for
> > > >> >port pair. Should not that be skipped rather than setting last port
> > > >> >rx-
> > > >> >tx loopback?
> > > >>
> > > >> There could be scenarios where user might want to test 2x10G and
> > > >1x40G Why
> > > >> force the user to explicitly mention 1x40G as port pair of itself in
> > > >> the
> > > >portpair
> > > >> config?
> > > >I am not sure if I follow your thought, as your current port map only
> > > >allows `1:1` mapping by `struct port_pair_params`. This can be to self
> > > >like `(port0:port0),(port1:port1)` or `(port-0:port-1)`.
> > > >
> > > >1. But current `l2fwd_parse_port_pair_config` does not consider the
> > > >same port mapping as we have hard check for `if (nb_port_pair_params
> > > >>= RTE_MAX_ETHPORTS/2)`.
> > > >
> > > >2. `l2fwd_enabled_port_mask` is global variable of user port mask. This
> > > >can contain both valid and invalid mask. Hence we check
> > > >`l2fwd_enabled_port_mask & ~((1 << nb_ports) - 1)`.
> > > >
> > > >3. can these scenarios are true if we invoke `check_port_pair_config`
> > > >before actual port_mask check.
> > > > a. there are only 4 ports, hence possible mask is `0xf`.
> > > > b. user passes port argument as `0xe`
> > > > c. `check_port_pair_config` gets masks for `(1,3)` as input and
> > > >populates `port_pair_config_mask`.
> > > > d. As per the code, port 2 which is valid port and part of user port
> > > > mask
> > > >will have lastport (which is port 3)? May be I did understand the logic
> > > >correct. Can you help me?
> > >
> > > Here user needs to explicitly mention (2,2) for port 2 to be setup else it
> > > will be skipped.
> > > If you see `check_port_pair_config` below we disable the ports that are
> > > not
> > > Mentioned in portmap.
> > >
> > > "
> > > check_port_pair_config(void)
> > > {
> > >
> > >
> > > port_pair_config_mask |= port_pair_mask;
> > > }
> > >
> > > l2fwd_enabled_port_mask &= port_pair_config_mask;
> > >
> > > return 0;
> > > }
> > > "
> > >
> > >
> > > >
> > > >So my concerns are 1) there is no same port mapping, 2) my
> > > >understanding on lastport logic is not clear and 3) as per the code there
> > > >is 1:N but 1:1.
> > > >
> > > >Hence there should be sufficient warning to user if port are of wrong
> > > >speed and NUMA.
> > >
> > > Unless the user disables stats using -T 0 option all the prints will be
> > > skipped.
> > >
> > > >
> > > >Note: current speed can be fetched only if the port are started too (in
> > > >Fortville).
> > > >
> > > >snipped
> >
> >
> >
> >
Re: [dpdk-dev] [PATCH v2] build: remove special handling for node library
On Thu, Jul 2, 2020 at 9:39 PM Thomas Monjalon wrote:
>
> The node library had a need of being linked as a whole
> to make some constructors effective.
> Now that all libraries are linked with --whole-archive,
> there is no need to have this library separate.
>
> Fixes: e2db26f76673 ("build: always link whole DPDK static libraries")
>
> Signed-off-by: Thomas Monjalon
Tested the change with: echo "node_list_dump" | sudo
./build/app/test/dpdk-test -c 0x3
Tested-by: Jerin Jacob
> ---
> v2: write real commit log
> ---
> app/test/meson.build | 4 +---
> examples/meson.build | 4 +---
> lib/meson.build | 3 ---
> meson.build | 1 -
> 4 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index b224d6f2bb..da5f39f018 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -415,15 +415,13 @@ endforeach
> test_dep_objs += cc.find_library('execinfo', required: false)
>
> link_libs = []
> -link_nodes = []
> if get_option('default_library') == 'static'
> link_libs = dpdk_static_libraries + dpdk_drivers
> - link_nodes = dpdk_graph_nodes
> endif
>
> dpdk_test = executable('dpdk-test',
> test_sources,
> - link_whole: link_libs + link_nodes,
> + link_whole: link_libs,
> dependencies: test_dep_objs,
> c_args: cflags,
> install_rpath: driver_install_path,
> diff --git a/examples/meson.build b/examples/meson.build
> index 120eebf716..eb13e82101 100644
> --- a/examples/meson.build
> +++ b/examples/meson.build
> @@ -2,10 +2,8 @@
> # Copyright(c) 2017-2019 Intel Corporation
>
> link_whole_libs = []
> -node_libs = []
> if get_option('default_library') == 'static'
> link_whole_libs = dpdk_static_libraries + dpdk_drivers
> - node_libs = dpdk_graph_nodes
> endif
>
> execinfo = cc.find_library('execinfo', required: false)
> @@ -101,7 +99,7 @@ foreach example: examples
> endif
> executable('dpdk-' + name, sources,
> include_directories: includes,
> - link_whole: link_whole_libs + node_libs,
> + link_whole: link_whole_libs,
> link_args: dpdk_extra_ldflags,
> c_args: cflags,
> dependencies: dep_objs)
> diff --git a/lib/meson.build b/lib/meson.build
> index c1b9e1633f..8ca25172c3 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -202,9 +202,6 @@ foreach l:libraries
>
> dpdk_libraries = [shared_lib] + dpdk_libraries
> dpdk_static_libraries = [static_lib] +
> dpdk_static_libraries
> - if libname == 'rte_node'
> - dpdk_graph_nodes = [static_lib]
> - endif
> endif # sources.length() > 0
>
> set_variable('shared_rte_' + name, shared_dep)
> diff --git a/meson.build b/meson.build
> index d21adfd303..e8bb9c4c1e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -16,7 +16,6 @@ cc = meson.get_compiler('c')
> dpdk_conf = configuration_data()
> dpdk_libraries = []
> dpdk_static_libraries = []
> -dpdk_graph_nodes = []
> dpdk_driver_classes = []
> dpdk_drivers = []
> dpdk_extra_ldflags = []
> --
> 2.26.2
>
Re: [dpdk-dev] [PATCH 2/8] trace: simplify trace point registration
On Mon, May 4, 2020 at 2:02 AM David Marchand wrote: > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs. > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part. > > Signed-off-by: David Marchand > --- > app/test/test_trace_register.c| 12 +- > doc/guides/prog_guide/trace_lib.rst | 12 +- > lib/librte_cryptodev/cryptodev_trace_points.c | 84 +++ > .../common/eal_common_trace_points.c | 164 ++ > lib/librte_eal/include/rte_eal_trace.h| 122 +-- > lib/librte_eal/include/rte_trace_point.h | 14 +- > .../include/rte_trace_point_register.h| 6 +- > lib/librte_ethdev/ethdev_trace_points.c | 44 ++-- > lib/librte_eventdev/eventdev_trace_points.c | 205 +++--- > lib/librte_mempool/mempool_trace_points.c | 124 --- > 10 files changed, 309 insertions(+), 478 deletions(-) This needs to be rebased to ToT. Please merge to RC1 if possible. Acked-by: Jerin Jacob
[dpdk-dev] [PATCH] service: fix wrong lcore indexes
From: Igor Romanov
The service core list is populated, but not used. Incorrect
lcore states are examined for a service.
Use the populated list to iterate over service cores.
Fixes: e30dd31847d2 ("service: add mechanism for quiescing")
Cc: sta...@dpdk.org
Signed-off-by: Igor Romanov
Signed-off-by: Andrew Rybchenko
---
lib/librte_eal/common/rte_service.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/rte_service.c
b/lib/librte_eal/common/rte_service.c
index 6123a2124d..e2795f857e 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -422,7 +422,7 @@ rte_service_may_be_active(uint32_t id)
return -EINVAL;
for (i = 0; i < lcore_count; i++) {
- if (lcore_states[i].service_active_on_lcore[id])
+ if (lcore_states[ids[i]].service_active_on_lcore[id])
return 1;
}
--
2.17.1
Re: [dpdk-dev] [PATCH v3 6/9] eal: register non-EAL threads as lcores
On Fri, Jul 3, 2020 at 6:40 PM Ananyev, Konstantin wrote: > what are the advantages of current approach (forbid MP support on the fly) > over explicit start-up parameters (either --proc-type=single or > --lcore-allow=...)? > Why do you think it is a better one? I don't want to perpetuate the "please carefully set your command line" habit. This feature is added through a C API, with documentation and flagged as experimental, it should be enough. How about moving the mp disable in rte_thread_register() as a separate API? Then a developer must call rte_mp_disable() before attempting rte_thread_register(). That would be equivalent to --proc-type=single. Why not convert lcore-allow into an API? This would force us to put something in the init so that external users see how the application has been started and adjust the secondary commandline. On the other hand, rte_mp_disable() is easy to do with my current v4 + I am running out of time for rc1. We can still revisit this experimental API. -- David Marchand
Re: [dpdk-dev] [PATCH] service: fix wrong lcore indexes
Hi Andrew/Igor,
A effective test case is missing for this, can you please add a test
case? Otherwise it looks good.
> -Original Message-
> From: dev On Behalf Of Andrew Rybchenko
> Sent: Saturday, July 4, 2020 9:36 AM
> To: dev@dpdk.org
> Cc: Igor Romanov ; sta...@dpdk.org; Harry van
> Haaren
> Subject: [dpdk-dev] [PATCH] service: fix wrong lcore indexes
Nit, 'indices' would be better?
^^^
>
> From: Igor Romanov
>
> The service core list is populated, but not used. Incorrect lcore states are
> examined for a service.
>
> Use the populated list to iterate over service cores.
>
> Fixes: e30dd31847d2 ("service: add mechanism for quiescing")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Igor Romanov
> Signed-off-by: Andrew Rybchenko
> ---
> lib/librte_eal/common/rte_service.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/rte_service.c
> b/lib/librte_eal/common/rte_service.c
> index 6123a2124d..e2795f857e 100644
> --- a/lib/librte_eal/common/rte_service.c
> +++ b/lib/librte_eal/common/rte_service.c
> @@ -422,7 +422,7 @@ rte_service_may_be_active(uint32_t id)
> return -EINVAL;
>
> for (i = 0; i < lcore_count; i++) {
> - if (lcore_states[i].service_active_on_lcore[id])
> + if (lcore_states[ids[i]].service_active_on_lcore[id])
> return 1;
> }
>
> --
> 2.17.1
Re: [dpdk-dev] [PATCH] service: fix wrong lcore indexes
On Sat, Jul 4, 2020 at 5:07 PM Honnappa Nagarahalli wrote: > > Hi Andrew/Igor, > A effective test case is missing for this, can you please add a test > case? Otherwise it looks good. +1, I was about to reply this. -- David Marchand
[dpdk-dev] [PATCH v2] trace: simplify trace point registration
RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
Signed-off-by: David Marchand
Acked-by: Jerin Jacob
---
Changes since v1:
- rebased on main branch,
---
app/test/test_trace_register.c| 12 +-
doc/guides/prog_guide/trace_lib.rst | 12 +-
lib/librte_cryptodev/cryptodev_trace_points.c | 84 +++
.../common/eal_common_trace_points.c | 164 ++
lib/librte_eal/include/rte_eal_trace.h| 122 +--
lib/librte_eal/include/rte_trace_point.h | 14 +-
.../include/rte_trace_point_register.h| 6 +-
lib/librte_ethdev/ethdev_trace_points.c | 44 ++--
lib/librte_eventdev/eventdev_trace_points.c | 205 +++---
lib/librte_mempool/mempool_trace_points.c | 124 ---
10 files changed, 309 insertions(+), 478 deletions(-)
diff --git a/app/test/test_trace_register.c b/app/test/test_trace_register.c
index b9372d3f9b..4891493687 100644
--- a/app/test/test_trace_register.c
+++ b/app/test/test_trace_register.c
@@ -6,13 +6,5 @@
#include "test_trace.h"
-/* Define trace points */
-RTE_TRACE_POINT_DEFINE(app_dpdk_test_tp);
-RTE_TRACE_POINT_DEFINE(app_dpdk_test_fp);
-
-RTE_INIT(register_valid_trace_points)
-{
- RTE_TRACE_POINT_REGISTER(app_dpdk_test_tp, app.dpdk.test.tp);
- RTE_TRACE_POINT_REGISTER(app_dpdk_test_fp, app.dpdk.test.fp);
-}
-
+RTE_TRACE_POINT_REGISTER(app_dpdk_test_tp, app.dpdk.test.tp)
+RTE_TRACE_POINT_REGISTER(app_dpdk_test_fp, app.dpdk.test.fp)
diff --git a/doc/guides/prog_guide/trace_lib.rst
b/doc/guides/prog_guide/trace_lib.rst
index b6c6285779..9bbfd165d8 100644
--- a/doc/guides/prog_guide/trace_lib.rst
+++ b/doc/guides/prog_guide/trace_lib.rst
@@ -100,12 +100,7 @@ Register the tracepoint
#include
- RTE_TRACE_POINT_DEFINE(app_trace_string);
-
- RTE_INIT(app_trace_init)
- {
- RTE_TRACE_POINT_REGISTER(app_trace_string, app.trace.string);
- }
+ RTE_TRACE_POINT_REGISTER(app_trace_string, app.trace.string)
The above code snippet registers the ``app_trace_string`` tracepoint to
trace library. Here, the ``my_tracepoint.h`` is the header file
@@ -118,9 +113,6 @@ There is no requirement for the tracepoint function and its
name to be similar.
However, it is recommended to have a similar name for a better naming
convention.
-The user must register the tracepoint before the ``rte_eal_init`` invocation.
-The user can use the ``RTE_INIT`` construction scheme to achieve this.
-
.. note::
The ``rte_trace_point_register.h`` header must be included before any
@@ -128,7 +120,7 @@ The user can use the ``RTE_INIT`` construction scheme to
achieve this.
.. note::
- The ``RTE_TRACE_POINT_DEFINE`` defines the placeholder for the
+ The ``RTE_TRACE_POINT_REGISTER`` defines the placeholder for the
``rte_trace_point_t`` tracepoint object. The user must export a
``__`` symbol in the library ``.map`` file for this
tracepoint to be used out of the library, in shared builds.
diff --git a/lib/librte_cryptodev/cryptodev_trace_points.c
b/lib/librte_cryptodev/cryptodev_trace_points.c
index 7672c7b99b..5d58951fd5 100644
--- a/lib/librte_cryptodev/cryptodev_trace_points.c
+++ b/lib/librte_cryptodev/cryptodev_trace_points.c
@@ -6,70 +6,50 @@
#include "rte_cryptodev_trace.h"
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_configure);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_start);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_stop);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_close);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_queue_pair_setup);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_pool_create);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_create);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_create);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_free);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_free);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_init);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_init);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_clear);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_clear);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_enqueue_burst);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_dequeue_burst);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_configure,
+ lib.cryptodev.configure)
-RTE_INIT(cryptodev_trace_init)
-{
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_configure,
- lib.cryptodev.configure);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_start,
+ lib.cryptodev.start)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_start,
- lib.cryptodev.start);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_stop,
+ lib.cryptodev.stop)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_stop,
- lib.cryptodev.stop);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_close,
+ l
Re: [dpdk-dev] [PATCH v5 1/3] lib/lpm: integrate RCU QSBR
Hi David,
Sorry, I missed tracking of this thread.
> -Original Message-
> From: David Marchand
> Sent: Monday, June 29, 2020 7:56 PM
> To: Ruifeng Wang ; Vladimir Medvedkin
> ; Bruce Richardson
>
> Cc: John McNamara ; Marko Kovacevic
> ; Ray Kinsella ; Neil Horman
> ; dev ; Ananyev, Konstantin
> ; Honnappa Nagarahalli
> ; nd
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] lib/lpm: integrate RCU QSBR
>
> On Mon, Jun 29, 2020 at 10:03 AM Ruifeng Wang
> wrote:
> >
> > Currently, the tbl8 group is freed even though the readers might be
> > using the tbl8 group entries. The freed tbl8 group can be reallocated
> > quickly. This results in incorrect lookup results.
> >
> > RCU QSBR process is integrated for safe tbl8 group reclaim.
> > Refer to RCU documentation to understand various aspects of
> > integrating RCU library into other libraries.
> >
> > Signed-off-by: Ruifeng Wang
> > Reviewed-by: Honnappa Nagarahalli
> > ---
> > doc/guides/prog_guide/lpm_lib.rst | 32 +++
> > lib/librte_lpm/Makefile| 2 +-
> > lib/librte_lpm/meson.build | 1 +
> > lib/librte_lpm/rte_lpm.c | 129 ++---
> > lib/librte_lpm/rte_lpm.h | 59 +
> > lib/librte_lpm/rte_lpm_version.map | 6 ++
> > 6 files changed, 216 insertions(+), 13 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/lpm_lib.rst
> > b/doc/guides/prog_guide/lpm_lib.rst
> > index 1609a57d0..7cc99044a 100644
> > --- a/doc/guides/prog_guide/lpm_lib.rst
> > +++ b/doc/guides/prog_guide/lpm_lib.rst
> > @@ -145,6 +145,38 @@ depending on whether we need to move to the
> next table or not.
> > Prefix expansion is one of the keys of this algorithm, since it
> > improves the speed dramatically by adding redundancy.
> >
> > +Deletion
> > +
> > +
> > +When deleting a rule, a replacement rule is searched for. Replacement
> > +rule is an existing rule that has the longest prefix match with the rule
> > to be
> deleted, but has smaller depth.
> > +
> > +If a replacement rule is found, target tbl24 and tbl8 entries are
> > +updated to have the same depth and next hop value with the
> replacement rule.
> > +
> > +If no replacement rule can be found, target tbl24 and tbl8 entries will be
> cleared.
> > +
> > +Prefix expansion is performed if the rule's depth is not exactly 24 bits or
> 32 bits.
> > +
> > +After deleting a rule, a group of tbl8s that belongs to the same tbl24
> > entry
> are freed in following cases:
> > +
> > +* All tbl8s in the group are empty .
> > +
> > +* All tbl8s in the group have the same values and with depth no greater
> than 24.
> > +
> > +Free of tbl8s have different behaviors:
> > +
> > +* If RCU is not used, tbl8s are cleared and reclaimed immediately.
> > +
> > +* If RCU is used, tbl8s are reclaimed when readers are in quiescent
> > state.
> > +
> > +When the LPM is not using RCU, tbl8 group can be freed immediately
> > +even though the readers might be using the tbl8 group entries. This might
> result in incorrect lookup results.
> > +
> > +RCU QSBR process is integrated for safe tbl8 group reclaimation.
> > +Application has certain responsibilities while using this feature.
> > +Please refer to resource reclaimation framework of :ref:`RCU library
> ` for more details.
> > +
>
> Would the lpm6 library benefit from the same?
> Asking as I do not see much code shared between lpm and lpm6.
>
Didn't look into lpm6. It may need separate integration with RCU since no
shared code between lpm and lpm6 as you mentioned.
> [...]
>
> > diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index
> > 38ab512a4..41e9c49b8 100644
> > --- a/lib/librte_lpm/rte_lpm.c
> > +++ b/lib/librte_lpm/rte_lpm.c
> > @@ -1,5 +1,6 @@
> > /* SPDX-License-Identifier: BSD-3-Clause
> > * Copyright(c) 2010-2014 Intel Corporation
> > + * Copyright(c) 2020 Arm Limited
> > */
> >
> > #include
> > @@ -245,13 +246,84 @@ rte_lpm_free(struct rte_lpm *lpm)
> > TAILQ_REMOVE(lpm_list, te, next);
> >
> > rte_mcfg_tailq_write_unlock();
> > -
> > +#ifdef ALLOW_EXPERIMENTAL_API
> > + if (lpm->dq)
> > + rte_rcu_qsbr_dq_delete(lpm->dq); #endif
>
> All DPDK code under lib/ is compiled with the ALLOW_EXPERIMENTAL_API
> flag set.
> There is no need to protect against this flag in rte_lpm.c.
>
OK, I see. So DPDK libraries will always be compiled with the
ALLOW_EXPERIMENTAL_API. It is application's
choice to use experimental APIs.
Will update in next version to remove the ALLOW_EXPERIMENTAL_API flag from
rte_lpm.c and only keep the one in rte_lpm.h.
> [...]
>
> > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index
> > b9d49ac87..7889f21b3 100644
> > --- a/lib/librte_lpm/rte_lpm.h
> > +++ b/lib/librte_lpm/rte_lpm.h
>
> > @@ -130,6 +143,28 @@ struct rte_lpm {
> > __rte_cache_aligned; /**< LPM tbl24 table. */
> > struct rte_lpm_tbl_entry *tbl8; /**< LPM tbl8 table. */
>
Re: [dpdk-dev] [dpdk-dev v4 1/4] cryptodev: add symmetric crypto data-path APIs
Hi Fan,
> +
> +/**
> + * Asynchronous operation job descriptor.
> + * Used by HW crypto devices direct API call that supports such activity
> + **/
> +struct rte_crypto_sym_job {
> + union {
> + /**
> + * When RTE_CRYPTO_HW_ENQ_FLAG_IS_SGL bit is set in flags,
> sgl
> + * field is used as input data. Otherwise data_iova is
> + * used.
> + **/
> + rte_iova_t data_iova;
> + struct rte_crypto_sgl *sgl;
> + };
> + union {
> + /**
> + * Different than cryptodev ops, all ofs and len fields have
> + * the unit of bytes (including Snow3G/Kasumi/Zuc.
> + **/
> + struct {
> + uint32_t cipher_ofs;
> + uint32_t cipher_len;
> + } cipher_only;
> + struct {
> + uint32_t auth_ofs;
> + uint32_t auth_len;
> + rte_iova_t digest_iova;
> + } auth_only;
> + struct {
> + uint32_t aead_ofs;
> + uint32_t aead_len;
> + rte_iova_t tag_iova;
> + uint8_t *aad;
> + rte_iova_t aad_iova;
> + } aead;
> + struct {
> + uint32_t cipher_ofs;
> + uint32_t cipher_len;
> + uint32_t auth_ofs;
> + uint32_t auth_len;
> + rte_iova_t digest_iova;
> + } chain;
> + };
> + uint8_t *iv;
> + rte_iova_t iv_iova;
> +};
NACK,
Why do you need this structure definitions again when you have similar ones
(the ones used in CPU crypto) available for the same purpose.
In case of CPU crypto, there were 2 main requirements
- synchronous API instead of enq +deq
- raw buffers.
Now for this patchset, the requirement is
- raw buffers
- asynchronous APIs
The data structure for raw buffers and crypto related offsets are already
defined
So they should be reused.
And I believe with some changes in rte_crypto_op and rte_crypto_sym_op,
We can support raw buffers with the same APIs.
Instead of m_src and m_dst, raw buffer data structures can be combined in a
Union and some of the fields in the rte_crypto_op can be left NULL in case of
raw buffers.
> +/* Struct for user to perform HW specific enqueue/dequeue function calls */
> +struct rte_crypto_hw_ops {
> + /* Driver written queue pair data pointer, should NOT be alterred by
> + * the user.
> + */
> + void *qp;
> + /* Function handler to enqueue AEAD job */
> + rte_crypto_hw_enq_cb_fn enqueue_aead;
> + /* Function handler to enqueue cipher only job */
> + rte_crypto_hw_enq_cb_fn enqueue_cipher;
> + /* Function handler to enqueue auth only job */
> + rte_crypto_hw_enq_cb_fn enqueue_auth;
> + /* Function handler to enqueue cipher + hash chaining job */
> + rte_crypto_hw_enq_cb_fn enqueue_chain;
> + /* Function handler to query processed jobs */
> + rte_crypto_hw_query_processed query_processed;
> + /* Function handler to dequeue one job and return opaque data stored
> */
> + rte_crypto_hw_deq_one_cb_fn dequeue_one;
> + /* Function handler to dequeue many jobs */
> + rte_crypto_hw_deq_many_cb_fn dequeue_many;
> + /* Reserved */
> + void *reserved[8];
> +};
Why do we need such callbacks in the library?
These should be inside the drivers, or else we do the same for
Legacy case as well. The pain of finding the correct enq function for
Appropriate crypto operation is already handled by all the drivers
And we can reuse that or else we modify it there as well.
We should not add a lot of data paths for the user, otherwise the
APIs will become centric to a particular vendor and it will be very difficult
For the user to migrate from one vendor to another and would defeat the
Purpose of DPDK which provide uniform abstraction layer for all the hardware
Vendors.
Adding other vendors to comment.
Regards,
Akhil
Re: [dpdk-dev] [PATCH v2] examples: add multi process crypto application
> > > > > > > This multi process app is only taking care of crypto queues while others > > > are > > > for NICs. > > > Is it not worth to have crypto+NIC multi process app instead of this app? > > [Arek] - initially main purpose was to check PMD behavior when: > > 1) configure cryptodev, sessions, queues and do enqueue/dequeue from > another different processes > > 2) run enqueue/dequeue from different processes on the same queue pair. > > If it can be done with one app I think it is ok. > > > I believe most common usecases of crypto are with network traffic. > > > Can we modify l2fwd-crypto for multi process? > [Fiona] Yes, it would be a good idea to do that sample app too. > However this app allows standalone validation of cryptodev lib and PMDs, > running in multiple processes, without introducing dependencies on > ethdev APIs, traffic generator, NIC, etc. I think this is useful as is. > One consideration is whether it would be better to treat this as a test tool > and move to the test directory - as you're right, it's not showing > a typical complete application, just allowing to play around with the crypto > part. > My preference is to move to under the examples/multi-process, but up to you. > Example applications are also not close to real work application. But they should Have end to end functionality available with some missing processing in between. I would say: - modifying l2fwd-crypto can be preference 1 - moving multiprocess crypto to test app can be preference 2 as it is just a unit test application for multi process. - moving to examples/multi-process will be preference 3.
Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow
On Fri, Jul 3, 2020 at 8:27 AM Thomas Monjalon wrote: > 03/07/2020 17:08, Jerin Jacob: > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad wrote: > > > From: Jerin Jacob: > > > > When adding overlapping API(rte_eth_mirror_rule_set()) in the same > > > > library(ethdev). > > > > Please depreciate the old API. > > > > We should not have two separate paths for the same function in the > same > > > > ethdev library. It is pain for app and driver developers. > > > > > > What are about all the other rte_flow parallel configuration APIs in > ethdev: > > > promiscuous_enable; > > > promiscuous_disable; > > > allmulticast_enable; > > > allmulticast_disable; > > > mac_addr_remove; > > > mac_addr_add; > > > mac_addr_set; > > > set_mc_addr_list; > > > vlan_filter_set; > > > vlan_tpid_set; > > > vlan_strip_queue_set; > > > vlan_offload_set; > > > vlan_pvid_set; > > > udp_tunnel_port_add; > > > udp_tunnel_port_del; > > > ... > > > > > > These APIs can be replaced easily by rte_flow API. > > > Do you think we need to deprecate all? > > > > I think, basic stuff like below can have separate API. > > 1) promiscuous_enable; > > 2) promiscuous_disable; > > 3) allmulticast_enable; > > 4) allmulticast_disable; > > 5) mac_addr_remove; > > 6) mac_addr_add; > > 7) mac_addr_set; > > 8) set_mc_addr_list; > > "Basic" is not a precise definition :) > I would say port-level configuration should remain > out of rte_flow API. > +1 > > > But The VLAN and UDP related should be rte_flow candidates.(IMO) > I do not have a strong opinion on VLAN in port-level or rte_flow list. But isn't the UDP port number for tunnels a port-level setting for HW? > > Yes definitely, tunneling is better managed with rte_flow. > > This is an important discussion, I Cc other ethdev maintainers. > Note: this is an ethdev patch, all ethdev maintainers should be Cc'ed. > Aren't you using --cc-cmd devtools/get-maintainer.sh ? > > >
Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow
On Thu, Jul 2, 2020 at 11:40 PM Jerin Jacob wrote:
> On Fri, Jul 3, 2020 at 12:13 AM Jiawei Wang wrote:
> >
> > When using full offload, all traffic will be handled by the HW, and
> > directed to the requested vf or wire, the control application loses
> > visibility on the traffic.
> > So there's a need for an action that will enable the control application
> > some visibility.
> >
> > The solution is introduced a new action that will sample the incoming
> > traffic and send a duplicated traffic in some predefined ratio to the
> > application, while the original packet will continue to the target
> > destination.
> >
> > The packets sampled equals is '1/ratio', if the ratio value be set to 1
> > , means that the packets would be completely mirrored. The sample packet
> > can be assigned with different set of actions from the original packet.
> >
> > In order to support the sample packet in rte_flow, new rte_flow action
> > definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> rte_flow_action_sample
> > will be introduced.
> >
> > Signed-off-by: Jiawei Wang
> > Acked-by: Ori Kam
>
> When adding overlapping API(rte_eth_mirror_rule_set()) in the same
> library(ethdev).
> Please depreciate the old API.
> We should not have two separate paths for the same function in the
> same ethdev library. It is pain for app and driver developers.
>
> With the above deprecation notice,
> Acked-by: Jerin Jacob
>
I am fine with the proposed RTE_FLOW_ACTION_TYPE_SAMPLE. But..
When rte_eth_mirror_rule_set() is deprecated, are we going to add
RTE_FLOW_ACTION_TYPE_MIRROR for full fledged mirror action?
Or we are proposing to use RTE_FLOW_ACTION_TYPE_SAMPLE with
ratio of 1 to mirror all packets, thereby doing away with the need for
a separate RTE_FLOW_ACTION_TYPE_MIRROR?
>
>
> > ---
> > doc/guides/prog_guide/rte_flow.rst | 25 +
> > doc/guides/rel_notes/release_20_08.rst | 6 ++
> > lib/librte_ethdev/rte_flow.c | 1 +
> > lib/librte_ethdev/rte_flow.h | 28
> > 4 files changed, 60 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> > index d5dd18c..50dfe1f 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2645,6 +2645,31 @@ timeout passed without any matching on the flow.
> > | ``context`` | user input flow context |
> > +--+-+
> >
> > +Action: ``SAMPLE``
> > +^^
> > +
> > +Adds a sample action to a matched flow.
> > +
> > +The matching packets will be duplicated to a special queue or vport
> > +with the predefined ``ratio``, the packets sampled equals is '1/ratio'.
> > +All the packets continues to the target destination.
> > +
> > +When the ``ratio`` is set to 1 then the packets will be 100% mirrored.
> > +``actions`` represent the different set of actions for the sampled or
> mirrored
> > +packets.
> > +
> > +.. _table_rte_flow_action_sample:
> > +
> > +.. table:: SAMPLE
> > +
> > + +--+-+
> > + | Field| Value |
> > + +==+=+
> > + | ``ratio``| 32 bits sample ratio value |
> > + +--+-+
> > + | ``actions`` | sub-action list for sampling|
> > + +--+-+
> > +
> > Negative types
> > ~~
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> b/doc/guides/rel_notes/release_20_08.rst
> > index 5cbc4ce..313e8d3 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -81,6 +81,12 @@ New Features
> >* Added support for virtio queue statistics.
> >* Added support for MTU update.
> >
> > +* **Added flow-based traffic sampling support.**
> > +
> > + Added new action: ``RTE_FLOW_ACTION_TYPE_SAMPLE`` to duplicate the
> matching
> > + packets with given ratio and redirects to vport or queue. The sampled
> packets
> > + also can be assigned with an additional optional actions.
> > +
> > * **Updated Marvell octeontx2 ethdev PMD.**
> >
> >Updated Marvell octeontx2 driver with cn98xx support.
> > diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> > index 1685be5..733871d 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -173,6 +173,7 @@ struct rte_flow_desc_data {
> > MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
> > MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
> > MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
> > + MK_FLOW_ACTION(SAMPLE, sizeof(struct rte_flow_action_sample)),
> > };
> >
> > int
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index b
Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow
Hi all From: Jerin Jacob: > On Fri, Jul 3, 2020 at 8:57 PM Thomas Monjalon > wrote: > > > > 03/07/2020 17:08, Jerin Jacob: > > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad > wrote: > > > > From: Jerin Jacob: > > > > > When adding overlapping API(rte_eth_mirror_rule_set()) in the > > > > > same library(ethdev). > > > > > Please depreciate the old API. > > > > > We should not have two separate paths for the same function in > > > > > the same ethdev library. It is pain for app and driver developers. > > > > > > > > What are about all the other rte_flow parallel configuration APIs in > ethdev: > > > > promiscuous_enable; > > > > promiscuous_disable; > > > > allmulticast_enable; > > > > allmulticast_disable; > > > > mac_addr_remove; > > > > mac_addr_add; > > > > mac_addr_set; > > > > set_mc_addr_list; > > > > vlan_filter_set; > > > > vlan_tpid_set; > > > > vlan_strip_queue_set; > > > > vlan_offload_set; > > > > vlan_pvid_set; > > > > udp_tunnel_port_add; > > > > udp_tunnel_port_del; > > > > ... > > > > > > > > These APIs can be replaced easily by rte_flow API. > > > > Do you think we need to deprecate all? > > > > > > I think, basic stuff like below can have separate API. > > > 1) promiscuous_enable; > > > 2) promiscuous_disable; > > > 3) allmulticast_enable; > > > 4) allmulticast_disable; > > > 5) mac_addr_remove; > > > 6) mac_addr_add; > > > 7) mac_addr_set; > > > 8) set_mc_addr_list; > > > > "Basic" is not a precise definition :) > > Yep. > > > I would say port-level configuration should remain out of rte_flow > > API. Thomas, Can you explain what is port-level? Everything in rte_flow is per port... Also, can you give reasons for your claim? > +1. > In addition that, I would say anything needs to configured at "per-flow" > granularity use rte_flow. Jerin, What do you mean "per-flow" ? Everything in traffic filtering\actions is per flow, for example: Promiscuous: flow create 0 ingress pattern eth / end actions queue index 0 / end Multicast\mac related: flow create 0 ingress pattern eth dst is X /end actions queue 0/ end (in case of legacy RSS queue action will be replaced by rss). Everything here are flows. > > > > > But The VLAN and UDP related should be rte_flow candidates.(IMO) > > > > Yes definitely, tunneling is better managed with rte_flow. Can you give reasons for your claim? Why should Vlan\Tunnel be in rte_flow and promiscuous\Multicast\mac not? > > This is an important discussion, I Cc other ethdev maintainers. Agree, this is a good trigger to open this important discussion. > > Note: this is an ethdev patch, all ethdev maintainers should be Cc'ed. > > Aren't you using --cc-cmd devtools/get-maintainer.sh ?
Re: [dpdk-dev] [PATCH v4 0/7] add support for DOCSIS protocol
> Introduction > > > This patchset adds support for the DOCSIS protocol to the DPDK Security > API (rte_security), to be used by the AESNI-MB and QAT crypto devices to > combine and accelerate Crypto and CRC functions of the DOCSIS protocol > into a single operation. > > Performing these functions in parallel as a single operation can enable a > significant performance improvement in a DPDK-based DOCSIS MAC pipeline. > > > Background > == > > A number of approaches to combine DOCSIS Crypto and CRC functions have > been discussed in the DPDK community to date, namely: > 1) adding a new rte_accelerator API, to provide a generic interface for >combining operations of different types > 2) using rawdev through a multi-function interface, again to provide a >generic interface for combining operations of different types > 3) adding support for DOCSIS Crypto-CRC to rte_security > > The third option above is the preferred approach for the following > reasons: > - it addresses the immediate use case to add DOCSIS Crypto-CRC support to > DPDK so that it can be consumed easily by cable equipment vendors > - it uses an already existing framework in DPDK > - it will mean much less code churn in DOCSIS applications, which already > use rte_cryptodev for encryption/decryption > > > Use Cases > = > > The primary use case for this proposal has already been mentioned, namely > to add DOCSIS Crypto-CRC support to DPDK: > > - DOCSIS MAC: Crypto-CRC > - Order: > - Downstream: CRC, Encrypt > - Upstream: Decrypt, CRC > - Specifications: > - Crypto: 128-bit and 256-bit AES-CFB encryption variant > for DOCSIS as described in section 11.1 of DOCSIS 3.1 > Security Specification > > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.ca > blelabs.com%2Fspecification%2FCM-SP- > SECv3.1&data=02%7C01%7Cakhil.goyal%40nxp.com%7C39c59476749d4f5 > ec88a08d81f5153d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > 37293781444756595&sdata=W4YJl2bu8jsADjsLVDG2vXhYkOmbBhFY%2B4A > a47onVak%3D&reserved=0) > - CRC: Ethernet 32-bit CRC as defined in > Ethernet/[ISO/IEC 8802-3] > > Note that support for these chained operations is already available in > the Intel IPSec Multi-Buffer library. > > However, other DOCSIS protocol functions could be optimized too in the > future using the same rte_security API for DOCSIS (e.g. Header Checksum > (HCS) calculation). > > v4: > * addressed Akhil's comments regarding documentation > * made some code fixes > * fixed possible NULL pointer dereference when allocating security_ctx > in AESNI-MB and QAT PMDs > * freed security_ctx memory when exiting AESNI-MB and QAT PMDs > * added session IOVA verification update when creating security > sessions in QAT PMD > > v3: > * removed rte_security_op definition > * now using rte_crypto_sym_op->auth.data fields for CRC offset and > length as suggested by feedback from Akhil and Konstantin > * addressed Pablo's comments > * removed support for out-of-place for DOCSIS protocol from QAT PMD > * updated dpdk-crypto-perf-test tool for DOCSIS > * updated documentation > > v2: > * added rte_security and rte_cryptodev code changes > * added AESNI MB crypto PMD code changes > * added QAT SYM crypto PMD code changes > * added crypto unit tests > * added security unit tests > > v1: > * added proposed API changes > * added security capabilities to aesni_mb crypto PMD > > David Coyle (7): > security: add support for DOCSIS protocol > cryptodev: add a note regarding DOCSIS protocol support > crypto/aesni_mb: add support for DOCSIS protocol > crypto/qat: add support for DOCSIS protocol > test/crypto: add DOCSIS security test cases > test/security: add DOCSIS capability check tests > app/crypto-perf: add support for DOCSIS protocol > > app/test-crypto-perf/cperf_ops.c | 82 +- > app/test-crypto-perf/cperf_options.h |5 +- > app/test-crypto-perf/cperf_options_parsing.c | 67 +- > app/test-crypto-perf/cperf_test_throughput.c |3 +- > app/test-crypto-perf/cperf_test_vectors.c |3 +- > app/test-crypto-perf/main.c |5 +- > app/test-crypto-perf/meson.build |2 +- > app/test/test_cryptodev.c | 513 ++ > ...t_cryptodev_security_docsis_test_vectors.h | 1544 + > app/test/test_security.c | 88 + > doc/guides/cryptodevs/aesni_mb.rst|8 + > doc/guides/cryptodevs/features/aesni_mb.ini |1 + > doc/guides/cryptodevs/features/qat.ini|1 + > doc/guides/cryptodevs/qat.rst |7 + > doc/guides/prog_guide/rte_security.rst| 114 +- > doc/guides/rel_notes/release_20_08.rst| 21 + > doc/guides/tools/cryptoperf.rst |5 + > drivers/common/qat/Makefile
Re: [dpdk-dev] [PATCH v4 3/7] crypto/aesni_mb: add support for DOCSIS protocol
> +#ifdef RTE_LIBRTE_SECURITYY > + rte_free(cryptodev->security_ctx); > +#endif > + Fixed typo while merging.
Re: [dpdk-dev] [PATCH v2 1/8] common/cpt: fix encryption offset
> In case of gmac auth the encryption offset should be set to zero.
>
> Fixes: b74652f3a91f ("common/cpt: add microcode interface for encryption")
> Fixes: 177b41ceee61 ("common/cpt: add microcode interface for decryption")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ankur Dwivedi
> ---
Series applied to dpdk-next-crypto
Thanks.
Re: [dpdk-dev] [PATCH] crypto/qat: fix AES-XTS capabilities
> > This patch fixes the increment field of the AES-XTS cipher key size.
> >
> > Fixes: 7d5ef3bb32cd ("crypto/qat: support XTS")
> >
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Adam Dybkowski
> Acked-by: Fiona Trahe
Applied to dpdk-next-crypto
Thanks.
Re: [dpdk-dev] [PATCH 2/2] cryptodev: add cryptodev trace in multi process function
> > From: Fiona Trahe > > > > This patch adds traces to some Cryptodev functions that are used > > in primary/secondary context. > > > > Signed-off-by: Fiona Trahe > Acked-by: Fiona Trahe Self Ack is not required and should not be done. Acked-by: Akhil Goyal Applied to dpdk-next-crypto Thanks.
Re: [dpdk-dev] [PATCH v2] cryptodev: add function to check if qp was setup
> > Hi Arek/Fiona, > > > > > > From: Fiona Trahe > > > > > > This patch adds function that can check if queue pair was already > > > setup. This may be useful when dealing with multi process approach in > > > cryptodev. > > > > > > Signed-off-by: Fiona Trahe > > > --- > > > v2: > > > - changed return values > > > - added function to map file > > > > > Please mark the previous versions as superseded in the patchwork. > > I see that previous version had 2 patches one was acked and so you skipped > > that in this version. > > Or you do not want that patch? > > > http://patches.dpdk.org/patch/71212/ > [Arek] - sorry for that, both patches should be included, should I send v2 > with > this one ? Or both with v3? Please take care of this in future. I have applied both. > > > > Those patches could have been sent separately but if you are sending the > > patches in a series, Then next version should have all the patches unless > > you > > want to drop some of the patches. > > > > Acked-by: Akhil Goyal > > > > Applied to dpdk-next-crypto Thanks.
Re: [dpdk-dev] [PATCH 2/2] crypto/octeontx2: add ChaCha20-Poly1305 support
> From: Tejasree Kondoj > > Add ChaCha20-Poly1305 AEAD algorithm support in crypto_octeontx2 PMD > > Signed-off-by: Anoob Joseph > Signed-off-by: Tejasree Kondoj > --- Series Applied to dpdk-next-crypto Thanks.
Re: [dpdk-dev] [PATCH v3 6/9] eal: register non-EAL threads as lcores
> > On Fri, Jul 3, 2020 at 6:40 PM Ananyev, Konstantin > wrote: > > what are the advantages of current approach (forbid MP support on the fly) > > over explicit start-up parameters (either --proc-type=single or > > --lcore-allow=...)? > > Why do you think it is a better one? > > I don't want to perpetuate the "please carefully set your command line" habit. > This feature is added through a C API, with documentation and flagged > as experimental, it should be enough. > > How about moving the mp disable in rte_thread_register() as a separate API? > Then a developer must call rte_mp_disable() before attempting > rte_thread_register(). > That would be equivalent to --proc-type=single. It wouldn't be exactly the same thing, but yes, I agree user can call it as first thing after rte_eal_init() and it should help to prevent non-consistency in behaviour. I think it is a good compromise. > > Why not convert lcore-allow into an API? > This would force us to put something in the init so that external > users see how the application has been started and adjust the > secondary commandline. Not sure I understand you here... If we'll make lcore-allow dynamic it is basically the same as moving lcore_role[] (or some similar structure) into shared memory. I am ok with that, but I thought you stated that it would require quite a lot of work. > On the other hand, rte_mp_disable() is easy to do with my current v4 + > I am running out of time for rc1. Yes, as I said above such approach seems good enough to me (at least for now). Konstantin
Re: [dpdk-dev] [PATCH v3 3/3] doc: update deprecation of CIO barrier APIs
On Sat, Jul 4, 2020 at 12:28 AM Honnappa Nagarahalli wrote: > > rte_cio_*mb APIs will be deprecated in 20.11 release. > > Signed-off-by: Honnappa Nagarahalli Acked-by: Jerin Jacob > --- > doc/guides/rel_notes/deprecation.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > index d1034f60f..59656da3d 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -40,6 +40,12 @@ Deprecation Notices >These wrappers must be used for patches that need to be merged in 20.08 >onwards. This change will not introduce any performance degradation. > > +* rte_cio_*mb: Since the IO barriers for ArmV8-a platforms are relaxed from > DSB > + to DMB, rte_cio_*mb APIs provide the same functionality as rte_io_*mb > + APIs(taking all platforms into consideration). rte_io_*mb APIs should be > used > + in the place of rte_cio_*mb APIs. The rte_cio_*mb APIs will be deprecated > in > + 20.11 release. > + > * igb_uio: In the view of reducing the kernel dependency from the main tree, >as a first step, the Technical Board decided to move ``igb_uio`` >kernel module to the dpdk-kmods repository in the /linux/igb_uio/ directory > -- > 2.17.1 >
Re: [dpdk-dev] [PATCH v3 2/3] doc: update armv8-a IO barrier changes
On Sat, Jul 4, 2020 at 12:28 AM Honnappa Nagarahalli wrote: > > Updated the use of DMB instruction in rte_*mb APIs for Armv8-a. > > Signed-off-by: Honnappa Nagarahalli > --- > doc/guides/rel_notes/release_20_08.rst | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/doc/guides/rel_notes/release_20_08.rst > b/doc/guides/rel_notes/release_20_08.rst > index 5cbc4ce14..15c21996d 100644 > --- a/doc/guides/rel_notes/release_20_08.rst > +++ b/doc/guides/rel_notes/release_20_08.rst > @@ -56,6 +56,13 @@ New Features > Also, make sure to start the actual text at the margin. > = > > +* **rte_*mb APIs are updated to use DMB instruction.** IMO, It is better to change to following as the end user can ignore parsing the description if not interested in arm64. rte_*mb APIs are updated to use DMB instruction for Armv8-a With above change: Acked-by: Jerin Jacob > + > + Armv8-a memory model has been strengthened to require other-multi-copy > + atomicity. This allows for using DMB instruction instead of DSB for IO > + barriers. rte_*mb APIs, for Armv8-a platforms, are changed to use DMB > + instruction to reflect this. > + > * **Updated PCAP driver.** > >Updated PCAP driver with new features and improvements, including: > -- > 2.17.1 >
Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow
On Sun, Jul 5, 2020 at 12:56 AM Matan Azrad wrote: > > Hi all > > From: Jerin Jacob: > > On Fri, Jul 3, 2020 at 8:57 PM Thomas Monjalon > > wrote: > > > > > > 03/07/2020 17:08, Jerin Jacob: > > > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad > > wrote: > > > > > From: Jerin Jacob: > > > > > > When adding overlapping API(rte_eth_mirror_rule_set()) in the > > > > > > same library(ethdev). > > > > > > Please depreciate the old API. > > > > > > We should not have two separate paths for the same function in > > > > > > the same ethdev library. It is pain for app and driver developers. > > > > > > > > > > What are about all the other rte_flow parallel configuration APIs in > > ethdev: > > > > > promiscuous_enable; > > > > > promiscuous_disable; > > > > > allmulticast_enable; > > > > > allmulticast_disable; > > > > > mac_addr_remove; > > > > > mac_addr_add; > > > > > mac_addr_set; > > > > > set_mc_addr_list; > > > > > vlan_filter_set; > > > > > vlan_tpid_set; > > > > > vlan_strip_queue_set; > > > > > vlan_offload_set; > > > > > vlan_pvid_set; > > > > > udp_tunnel_port_add; > > > > > udp_tunnel_port_del; > > > > > ... > > > > > > > > > > These APIs can be replaced easily by rte_flow API. > > > > > Do you think we need to deprecate all? > > > > > > > > I think, basic stuff like below can have separate API. > > > > 1) promiscuous_enable; > > > > 2) promiscuous_disable; > > > > 3) allmulticast_enable; > > > > 4) allmulticast_disable; > > > > 5) mac_addr_remove; > > > > 6) mac_addr_add; > > > > 7) mac_addr_set; > > > > 8) set_mc_addr_list; > > > > > > "Basic" is not a precise definition :) > > > > Yep. > > > > > I would say port-level configuration should remain out of rte_flow > > > API. > > Thomas, Can you explain what is port-level? > Everything in rte_flow is per port... > > Also, can you give reasons for your claim? > > > +1. > > In addition that, I would say anything needs to configured at "per-flow" > > granularity use rte_flow. > > Jerin, What do you mean "per-flow" ? In Terms of NIC HW features, Typical HW will have a) Basic "port" level configuration like - enable/disable promiscuous b) Advance HW's will have CAM based flow filtering. IMO, CAM related stuff should go to rte_flow. This is to enable, The very basic PMD(without advanced features) should work with port level basic APIs(i.e without rte_flow) I have seen promiscuous, mac address handling is part of basic NIC HW(i.e NICs without advanced CAM filters). That's my reasoning for the split. > Everything in traffic filtering\actions is per flow, for example: > Promiscuous: flow create 0 ingress pattern eth / end actions queue index 0 / > end IMO, it is not an accurate representation of promiscuous enable. It needs to send the traffic to all queues and patterns is not just eth. > Multicast\mac related: flow create 0 ingress pattern eth dst is X /end > actions queue 0/ end > (in case of legacy RSS queue action will be replaced by rss). > > > Everything here are flows. > > > > > > > > But The VLAN and UDP related should be rte_flow candidates.(IMO) > > > > > > Yes definitely, tunneling is better managed with rte_flow. > > Can you give reasons for your claim? > Why should Vlan\Tunnel be in rte_flow and promiscuous\Multicast\mac not? > > > > This is an important discussion, I Cc other ethdev maintainers. > > Agree, this is a good trigger to open this important discussion. > > > > Note: this is an ethdev patch, all ethdev maintainers should be Cc'ed. > > > Aren't you using --cc-cmd devtools/get-maintainer.sh ? >
[dpdk-dev] [pull-request] next-eventdev 20.08 RC1
The following changes since commit 9c99878aa1b16de26fcce82c112b401766dd910e: log: introduce logtype register macro (2020-07-03 15:52:51 +0200) are available in the Git repository at: http://dpdk.org/git/next/dpdk-next-eventdev for you to fetch changes up to 50efcc7264847627c7556fada8d860c00cdefffc: eventdev: relax smp barriers with c11 atomics (2020-07-05 08:08:30 +0530) Harman Kalra (1): event/octeontx: fix memory corruption Harry van Haaren (1): examples/eventdev_pipeline: fix 32-bit coremask logic Pavan Nikhilesh (3): event/octeontx2: fix device reconfigure event/octeontx2: fix sub event type violation event/octeontx2: improve datapath memory locality Phil Yang (4): eventdev: fix race condition on timer list counter eventdev: use c11 atomics for lcore timer armed flag eventdev: remove redundant code eventdev: relax smp barriers with c11 atomics drivers/event/octeontx/ssovf_worker.c | 3 +- drivers/event/octeontx2/otx2_evdev.c | 60 +++ drivers/event/octeontx2/otx2_evdev.h | 5 ++ drivers/event/octeontx2/otx2_evdev_adptr.c| 67 - drivers/event/octeontx2/otx2_worker.c | 15 +++-- drivers/event/octeontx2/otx2_worker.h | 21 --- drivers/event/octeontx2/otx2_worker_dual.c| 15 +++-- drivers/event/octeontx2/otx2_worker_dual.h| 7 ++- examples/eventdev_pipeline/main.c | 10 ++-- examples/eventdev_pipeline/pipeline_common.h | 8 +-- lib/librte_eventdev/rte_event_timer_adapter.c | 86 ++- lib/librte_eventdev/rte_event_timer_adapter.h | 2 +- 12 files changed, 229 insertions(+), 70 deletions(-)
Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow
From: Jerin Jacob: > On Sun, Jul 5, 2020 at 12:56 AM Matan Azrad wrote: > > > > Hi all > > > > From: Jerin Jacob: > > > On Fri, Jul 3, 2020 at 8:57 PM Thomas Monjalon > > > wrote: > > > > > > > > 03/07/2020 17:08, Jerin Jacob: > > > > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad > > > wrote: > > > > > > From: Jerin Jacob: > > > > > > > When adding overlapping API(rte_eth_mirror_rule_set()) in > > > > > > > the same library(ethdev). > > > > > > > Please depreciate the old API. > > > > > > > We should not have two separate paths for the same function > > > > > > > in the same ethdev library. It is pain for app and driver > > > > > > > developers. > > > > > > > > > > > > What are about all the other rte_flow parallel configuration > > > > > > APIs in > > > ethdev: > > > > > > promiscuous_enable; > > > > > > promiscuous_disable; > > > > > > allmulticast_enable; > > > > > > allmulticast_disable; > > > > > > mac_addr_remove; > > > > > > mac_addr_add; > > > > > > mac_addr_set; > > > > > > set_mc_addr_list; > > > > > > vlan_filter_set; > > > > > > vlan_tpid_set; > > > > > > vlan_strip_queue_set; > > > > > > vlan_offload_set; > > > > > > vlan_pvid_set; > > > > > > udp_tunnel_port_add; > > > > > > udp_tunnel_port_del; > > > > > > ... > > > > > > > > > > > > These APIs can be replaced easily by rte_flow API. > > > > > > Do you think we need to deprecate all? > > > > > > > > > > I think, basic stuff like below can have separate API. > > > > > 1) promiscuous_enable; > > > > > 2) promiscuous_disable; > > > > > 3) allmulticast_enable; > > > > > 4) allmulticast_disable; > > > > > 5) mac_addr_remove; > > > > > 6) mac_addr_add; > > > > > 7) mac_addr_set; > > > > > 8) set_mc_addr_list; > > > > > > > > "Basic" is not a precise definition :) > > > > > > Yep. > > > > > > > I would say port-level configuration should remain out of rte_flow > > > > API. > > > > Thomas, Can you explain what is port-level? > > Everything in rte_flow is per port... > > > > Also, can you give reasons for your claim? > > > > > +1. > > > In addition that, I would say anything needs to configured at "per-flow" > > > granularity use rte_flow. > > > > Jerin, What do you mean "per-flow" ? > > In Terms of NIC HW features, Typical HW will have > a) Basic "port" level configuration like > - enable/disable promiscuous What is "port level", everything in rte_flow is also per port... > b) Advance HW's will have CAM based flow filtering. IMO, CAM related stuff > should go to rte_flow. It is HW internal, I don't think all HWs use the same logic here. Since rte_flow is generic for all filtering methods, It is good candidate API for all HWs. > This is to enable, The very basic PMD(without advanced features) should > work with port level basic APIs(i.e without rte_flow) What is "basic"? Do you mean simple match and simple action? As I said, Also rte_flow is port level API - so "port level" is not good term here. As you said " When adding overlapping API(rte_eth_mirror_rule_set()) in the same library(ethdev). Please depreciate the old API." Different APIs to do the same thing is not good, especially in packet filtering: What should we do if we have conflicts? For example: legacy filtering APIs say to receive packet A and rte_flow says to drop it. Don't you think it complicates more the user API understanding, also the PMD implementations? > I have seen promiscuous, mac address handling is part of basic NIC HW(i.e > NICs without advanced CAM filters). > That's my reasoning for the split. As I said, the nic HW specific implementation should not affect the API. I don't think we need to split API and to complicate the user because of it. IMO, It is better to have one generic API for packet filtering: It is clearer, simpler, generic and classic. The user and PMD need to understand only one filtering API and that’s it (no need to combine between multiple filtering APIs). I know this is big change but we can do it in modular way. It reminds me the big change that was done in Rx\Tx offload configurations: So, when offload became more popular we modularly forced users to replace the offload API. Also here, flow filtering becomes popular so maybe this is the time(20.08-20.11) to force changes in the old APIs. > > Everything in traffic filtering\actions is per flow, for example: > > Promiscuous: flow create 0 ingress pattern eth / end actions queue > > index 0 / end > > IMO, it is not an accurate representation of promiscuous enable. It needs to > send the traffic to all queues and patterns is not just eth. Yes, if legacy RSS is configured we need to replace the above queue action by rss action as I wrote before.(did you read it just below?) So, we can add legacy RSS APIs to the list above... > > Multicast\mac related: flow create 0 ingress pattern eth dst is X /end > > actions queue 0/ end (in case of legacy RSS queue action will be replaced by > rss). > > > > > > Everything here are flows. > > > > > > > > > > > But The VLAN and U
