[PATCH] net/iavf: remove error logs for vlan offloading
This was reported by RH QE.
When a vlan is enforced on a VF via an administrative configuration on
the PF side, the net/iavf driver logs two error messages.
Those error messages have no consequence on the rest of the port
initialisation and packet processing works fine.
[root@toto ~] # ip l set enp94s0 vf 0 vlan 2
[root@toto ~] # dpdk-testpmd -a :5e:02.0 -- -i
...
Configuring Port 0 (socket 0)
iavf_dev_init_vlan(): Failed to update vlan offload
iavf_dev_configure(): configure VLAN failed: -95
iavf_set_rx_function(): request RXDID[1] in Queue[0] is legacy, set
rx_pkt_burst as legacy for all queues
The first change is to remove the error log in iavf_dev_init_vlan().
This log is unneeded since all error path are covered by a dedicated log
message already.
Then, in iavf_dev_init_vlan(), requesting all possible VLAN offloading
must not trigger an ERROR level log message. This is simply confusing,
as the application may not have requested such vlan offloading.
The reason why the driver requests all offloading is unclear so keep it
as is. Instead, rephrase the log message and lower its level to INFO.
Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
---
drivers/net/iavf/iavf_ethdev.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 1fb876e827..fc92cdf146 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -631,7 +631,7 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
RTE_ETH_VLAN_FILTER_MASK |
RTE_ETH_VLAN_EXTEND_MASK);
if (err) {
- PMD_DRV_LOG(ERR, "Failed to update vlan offload");
+ PMD_DRV_LOG(INFO, "There is no support or the PF refused VLAN
offloading");
return err;
}
@@ -707,9 +707,7 @@ iavf_dev_configure(struct rte_eth_dev *dev)
vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
}
- ret = iavf_dev_init_vlan(dev);
- if (ret)
- PMD_DRV_LOG(ERR, "configure VLAN failed: %d", ret);
+ iavf_dev_init_vlan(dev);
if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
if (iavf_init_rss(ad) != 0) {
--
2.43.0
Re: [PATCH] net/iavf: remove error logs for vlan offloading
On Tue, Feb 06, 2024 at 10:56:07AM +0100, David Marchand wrote:
> This was reported by RH QE.
>
> When a vlan is enforced on a VF via an administrative configuration on
> the PF side, the net/iavf driver logs two error messages.
> Those error messages have no consequence on the rest of the port
> initialisation and packet processing works fine.
>
> [root@toto ~] # ip l set enp94s0 vf 0 vlan 2
> [root@toto ~] # dpdk-testpmd -a :5e:02.0 -- -i
> ...
> Configuring Port 0 (socket 0)
> iavf_dev_init_vlan(): Failed to update vlan offload
> iavf_dev_configure(): configure VLAN failed: -95
> iavf_set_rx_function(): request RXDID[1] in Queue[0] is legacy, set
> rx_pkt_burst as legacy for all queues
>
> The first change is to remove the error log in iavf_dev_init_vlan().
> This log is unneeded since all error path are covered by a dedicated log
> message already.
>
> Then, in iavf_dev_init_vlan(), requesting all possible VLAN offloading
> must not trigger an ERROR level log message. This is simply confusing,
> as the application may not have requested such vlan offloading.
> The reason why the driver requests all offloading is unclear so keep it
> as is. Instead, rephrase the log message and lower its level to INFO.
>
> Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand
Acked-by: Bruce Richardson
One small suggestion inline below.
> ---
> drivers/net/iavf/iavf_ethdev.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index 1fb876e827..fc92cdf146 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -631,7 +631,7 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
> RTE_ETH_VLAN_FILTER_MASK |
> RTE_ETH_VLAN_EXTEND_MASK);
> if (err) {
> - PMD_DRV_LOG(ERR, "Failed to update vlan offload");
> + PMD_DRV_LOG(INFO, "There is no support or the PF refused VLAN
> offloading");
Minor nit, the phrase "no support" seems ambiguous on first reading, since
it's not completely clear that the no support refers to vlan offloading.
How about:
"VLAN offloading is not supported, or offloading was refused by the PF"
> return err;
> }
>
> @@ -707,9 +707,7 @@ iavf_dev_configure(struct rte_eth_dev *dev)
> vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
> }
>
> - ret = iavf_dev_init_vlan(dev);
> - if (ret)
> - PMD_DRV_LOG(ERR, "configure VLAN failed: %d", ret);
> + iavf_dev_init_vlan(dev);
>
> if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
> if (iavf_init_rss(ad) != 0) {
> --
> 2.43.0
>
RE: [EXT] [PATCH v9 08/23] examples: remove term sanity
> Do not use non-inclusive terms. > > Signed-off-by: Stephen Hemminger Acked-by: Akhil Goyal
Re: [PATCH] net/iavf: remove error logs for vlan offloading
On Tue, Feb 6, 2024 at 11:04 AM Bruce Richardson
wrote:
> > @@ -631,7 +631,7 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
> > RTE_ETH_VLAN_FILTER_MASK |
> > RTE_ETH_VLAN_EXTEND_MASK);
> > if (err) {
> > - PMD_DRV_LOG(ERR, "Failed to update vlan offload");
> > + PMD_DRV_LOG(INFO, "There is no support or the PF refused VLAN
> > offloading");
>
> Minor nit, the phrase "no support" seems ambiguous on first reading, since
> it's not completely clear that the no support refers to vlan offloading.
>
> How about:
> "VLAN offloading is not supported, or offloading was refused by the PF"
It reads better, I'll send a v2.
Thanks Bruce.
--
David Marchand
RE: [PATCH v3 1/3] config/arm: avoid mcpu and march conflicts
> > On Feb 5, 2024, at 10:10 PM, Wathsala Wathawana Vithanage
> wrote:
> >
> > Hi Pavan,
> >
> >> The compiler options march and mtune are a subset of mcpu and will lead
> to
> >> conflicts if improper march is chosen for a given mcpu.
> >> To avoid conflicts, force part number march when mcpu is available and is
> >> supported by the compiler.
> >
> > Why would one force the march specified in the part number when mcpu for
> > that part number is also available and supported by the compiler?
> >
> It would be good to explain the use case or the problem being faced.
>
The idea of this patchset is to avoid mcpu and march conflicts that can happen
with the current build flow.
#aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c
cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a'
In some versions of GCC mcpu=neoverse-n2 is supported but -march=armv9-a is not
so, current build flow
will choose the next supported march which is armv8.6-a and report a conflict.
> >>
> >> Example:
> >> march = armv9-a
> >> mcpu = neoverse-n2
> >>
> >> mcpu supported, march supported
> >> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a']
> >
> > -march restricts the compiler to baseline architecture of the -mcpu.
> > For instance, Neoverse-n1's baseline architecture is armv8.2-a,
> > but it has some extensions from armv8.3-a, armv8.4-a, and armv8.5-a.
> > By setting -march to armv8.2-a the compiler will strictly omit extensions
> > from 8.3, 8.4 and 8.5 resulting in a suboptimal outcome.
What if compiler only supports armv8.2-a?
Are you suggesting we don’t use march at all when mcpu is supported?
If so how do you express extensions that the SoC supports?
Neoverse-n2 has optional support for crypto and can only be enabled by
expressing it through march='armv9-a+crypto'
> >
> >>
> >> mcpu supported, march not supported
> >> machine_args = ['-mcpu=neoverse-n2']
> >
> > This will result in the best outcome.
Isn't -mcpu=neoverse-n2 -march=armv9-a+sve2+crypto also the best outcome?
> >
> >>
> >> mcpu not supported, march supported
> >> machine_args = ['-march=armv9-a']
> >
> > This too may result in a suboptimal outcome as optimization space
> > is limited to the given march (not using extensions from later
> > architectures when available).
> >
What if compiler doesn’t support mcpu=neoverse-n2 and only supports
march=armv9-a
> >>
> >> mcpu not supported, march not supported
> >> machine_args = ['-march=armv8.6-a']
> >
> > Compiler knows nothing about the target CPU or the architecture.
> > I think it's better to exit the build process with an error.
> >
Then we would need to mark all old GCC versions as not supported by a newer SoC
I don’t think that’s needed since the binaries still run but not optimally,
currently we have
a warning in place for march mismatch.
> >>
> >> Signed-off-by: Pavan Nikhilesh
> >> ---
> >> v2 Changes:
> >> - Cleanup march inconsistencies. (Juraj Linkes)
> >> - Unify fallback march selection. (Juraj Linkes)
> >> - Tag along ARM WFE patch.
> >> v3 Changes:
> >> - Fix missing 'fallback_march' key check.
> >>
> >> config/arm/meson.build | 108 +-
> --
> >> -
> >> 1 file changed, 66 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >> 36f21d22599a..ba859bd060b5 100644
> >> --- a/config/arm/meson.build
> >> +++ b/config/arm/meson.build
> >> @@ -58,18 +58,18 @@ implementer_generic = { }
> >>
> >> part_number_config_arm = {
> >> -'0xd03': {'compiler_options': ['-mcpu=cortex-a53']},
> >> -'0xd04': {'compiler_options': ['-mcpu=cortex-a35']},
> >> -'0xd05': {'compiler_options': ['-mcpu=cortex-a55']},
> >> -'0xd07': {'compiler_options': ['-mcpu=cortex-a57']},
> >> -'0xd08': {'compiler_options': ['-mcpu=cortex-a72']},
> >> -'0xd09': {'compiler_options': ['-mcpu=cortex-a73']},
> >> -'0xd0a': {'compiler_options': ['-mcpu=cortex-a75']},
> >> -'0xd0b': {'compiler_options': ['-mcpu=cortex-a76']},
> >> +'0xd03': {'mcpu': 'cortex-a53'},
> >> +'0xd04': {'mcpu': 'cortex-a35'},
> >> +'0xd05': {'mcpu': 'cortex-a55'},
> >> +'0xd07': {'mcpu': 'cortex-a57'},
> >> +'0xd08': {'mcpu': 'cortex-a72'},
> >> +'0xd09': {'mcpu': 'cortex-a73'},
> >> +'0xd0a': {'mcpu': 'cortex-a75'},
> >> +'0xd0b': {'mcpu': 'cortex-a76'},
> >> '0xd0c': {
> >> 'march': 'armv8.2-a',
> >> 'march_features': ['crypto', 'rcpc'],
> >> -'compiler_options': ['-mcpu=neoverse-n1'],
> >> +'mcpu': 'neoverse-n1',
> >> 'flags': [
> >> ['RTE_MACHINE', '"neoverse-n1"'],
> >> ['RTE_ARM_FEATURE_ATOMICS', true], @@ -81,7 +81,7 @@
> >> part_number_config_arm = {
> >> '0xd40': {
> >> 'march': 'armv8.4-a',
> >> 'march_features': ['sve'],
> >> -'compiler_options': ['-mcpu=neoverse-v1'],
> >> +'mcpu': 'neoverse-v1',
> >> 'flags': [
> >> [
Re: [PATCH v2 1/2] vhost: fix memory leak in Virtio Tx split path
On Wed, Jan 31, 2024 at 8:53 PM Maxime Coquelin
wrote:
>
> When vIOMMU is enabled and Virtio device is bound to kernel
> driver in guest, rte_vhost_dequeue_burst() will often return
> early because of IOTLB misses.
In theory, we can hit this issue with a dpdk pmd too, as long as the
vIOMMU is in use.
But the consequence would be a "really small" leak which does not have
the same impact as what was seen with the kernel driver which
maps/unmaps pages associated with virtio-net skb way more often :-).
So maybe rephrase this part emphasizing on the kernel case like:
"""
When vIOMMU is enabled, rte_vhost_dequeue_burst() can return early
because of IOTLB misses.
Such IOTLB misses are especially frequent when a Virtio device is
bound to a kernel driver in guest.
"""
>
> This patch fixes a mbuf leak occurring in this case.
>
> Fixes: 242695f6122a ("vhost: allocate and free packets in bulk in Tx split")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Maxime Coquelin
> Signed-off-by: David Marchand
Reviewed-by: David Marchand
--
David Marchand
Re: [PATCH v2 2/2] vhost: add new mbuf allocation failure statistic
On Thu, Feb 1, 2024 at 9:29 AM Maxime Coquelin
wrote:
> On 2/1/24 09:10, David Marchand wrote:
> > On Wed, Jan 31, 2024 at 8:53 PM Maxime Coquelin
> > wrote:
> >>
> >> This patch introduces a new, per virtqueue, mbuf allocation
> >> failure statistic. It can be useful to troubleshoot packets
> >> drops due to insufficient mempool size or memory leaks.
> >>
> >> Signed-off-by: Maxime Coquelin
> >
> > Having a stat for such situation will be useful.
> >
> > I just have one comment, though it is not really related to this change
> > itself.
> >
> > [snip]
> >
> >> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> >> index 9951842b9f..1359c5fb1f 100644
> >> --- a/lib/vhost/virtio_net.c
> >> +++ b/lib/vhost/virtio_net.c
> >> @@ -2996,6 +2996,7 @@ desc_to_mbuf(struct virtio_net *dev, struct
> >> vhost_virtqueue *vq,
> >> if (mbuf_avail == 0) {
> >> cur = rte_pktmbuf_alloc(mbuf_pool);
> >> if (unlikely(cur == NULL)) {
> >> + vq->stats.mbuf_alloc_failed++;
> >> VHOST_DATA_LOG(dev->ifname, ERR,
> >> "failed to allocate memory for
> >> mbuf.");
> >
> > This error log here is scary as it means the datapath can be slowed
> > down for each multisegment mbuf in the event of a mbuf (maybe
> > temporary) shortage.
> > Besides no other mbuf allocation in the vhost library datapath
> > generates such log.
> >
> > I would remove it, probably in a separate patch.
> > WDYT?
>
> Agree, we should not have such log in the datapath.
> And now that we have the stat, it is even less useful.
Ok, nevertheless, you can add my:
Reviewed-by: David Marchand
--
David Marchand
[PATCH v2] net/iavf: remove error logs for vlan offloading
This was reported by RH QE.
When a vlan is enforced on a VF via an administrative configuration on
the PF side, the net/iavf driver logs two error messages.
Those error messages have no consequence on the rest of the port
initialisation and packet processing works fine.
[root@toto ~] # ip l set enp94s0 vf 0 vlan 2
[root@toto ~] # dpdk-testpmd -a :5e:02.0 -- -i
...
Configuring Port 0 (socket 0)
iavf_dev_init_vlan(): Failed to update vlan offload
iavf_dev_configure(): configure VLAN failed: -95
iavf_set_rx_function(): request RXDID[1] in Queue[0] is legacy, set
rx_pkt_burst as legacy for all queues
The first change is to remove the error log in iavf_dev_init_vlan().
This log is unneeded since all error path are covered by a dedicated log
message already.
Then, in iavf_dev_init_vlan(), requesting all possible VLAN offloading
must not trigger an ERROR level log message. This is simply confusing,
as the application may not have requested such vlan offloading.
The reason why the driver requests all offloading is unclear so keep it
as is. Instead, rephrase the log message and lower its level to INFO.
Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
Changes since v1:
- updated log message,
---
drivers/net/iavf/iavf_ethdev.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 1fb876e827..3532e379a1 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -631,7 +631,8 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
RTE_ETH_VLAN_FILTER_MASK |
RTE_ETH_VLAN_EXTEND_MASK);
if (err) {
- PMD_DRV_LOG(ERR, "Failed to update vlan offload");
+ PMD_DRV_LOG(INFO,
+ "VLAN offloading is not supported, or offloading was
refused by the PF");
return err;
}
@@ -707,9 +708,7 @@ iavf_dev_configure(struct rte_eth_dev *dev)
vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
}
- ret = iavf_dev_init_vlan(dev);
- if (ret)
- PMD_DRV_LOG(ERR, "configure VLAN failed: %d", ret);
+ iavf_dev_init_vlan(dev);
if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
if (iavf_init_rss(ad) != 0) {
--
2.43.0
[PATCH 1/6] ethdev: add modify IPv4 next protocol field
Add IPv4 next protocol modify field definition.
Signed-off-by: Viacheslav Ovsiienko
---
doc/guides/rel_notes/release_24_03.rst | 1 +
lib/ethdev/rte_flow.h | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 2b91217943..33e9303ae1 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -64,6 +64,7 @@ New Features
* Added ``RTE_FLOW_ITEM_TYPE_RANDOM`` to match random value.
* Added ``RTE_FLOW_FIELD_RANDOM`` to represent it in field ID struct.
+ * Added ``RTE_FLOW_FIELD_IPV4_PROTO`` to represent it in field ID struct.
* ** Support for getting the number of used descriptors of a Tx queue. **
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 1267c146e5..84af730dc7 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3933,7 +3933,8 @@ enum rte_flow_field_id {
RTE_FLOW_FIELD_IPV4_IHL,/**< IPv4 IHL. */
RTE_FLOW_FIELD_IPV4_TOTAL_LEN, /**< IPv4 total length. */
RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN,/**< IPv6 payload length. */
- RTE_FLOW_FIELD_RANDOM /**< Random value. */
+ RTE_FLOW_FIELD_RANDOM, /**< Random value. */
+ RTE_FLOW_FIELD_IPV4_PROTO /**< IPv4 next protocol. */
};
/**
--
2.18.1
[PATCH 2/6] app/testpmd: add modify IPv4 next protocol command line
Add new modify field action type string: "ipv4_proto".
Signed-off-by: Viacheslav Ovsiienko
---
app/test-pmd/cmdline_flow.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 4062879552..03b418a5d8 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -962,6 +962,7 @@ static const char *const modify_field_ids[] = {
"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
"random",
+ "ipv4_proto",
NULL
};
--
2.18.1
[PATCH 3/6] net/mlx5: add modify IPv4 protocol implementation
Add modify IPv4 protocol implementation for mlx5 PMD.
Signed-off-by: Viacheslav Ovsiienko
---
doc/guides/rel_notes/release_24_03.rst | 1 +
drivers/common/mlx5/mlx5_prm.h | 1 +
drivers/net/mlx5/mlx5_flow_dv.c| 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 33e9303ae1..3e33ff2d86 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -91,6 +91,7 @@ New Features
* Added HW steering support for modify field
``RTE_FLOW_FIELD_GENEVE_OPT_TYPE`` flow action.
* Added HW steering support for modify field
``RTE_FLOW_FIELD_GENEVE_OPT_CLASS`` flow action.
* Added HW steering support for modify field
``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` flow action.
+ * Added HW steering support for modify field ``RTE_FLOW_FIELD_IPV4_PROTO``
flow action.
Removed Items
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index f64f25dbb7..44413517d0 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -839,6 +839,7 @@ enum mlx5_modification_field {
MLX5_MODI_IN_MPLS_LABEL_2,
MLX5_MODI_IN_MPLS_LABEL_3,
MLX5_MODI_IN_MPLS_LABEL_4,
+ MLX5_MODI_OUT_IP_PROTOCOL = 0x4A,
MLX5_MODI_OUT_IPV6_NEXT_HDR = 0x4A,
MLX5_MODI_META_REG_C_8 = 0x8F,
MLX5_MODI_META_REG_C_9 = 0x90,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6998be107f..764940b700 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1384,6 +1384,7 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
case RTE_FLOW_FIELD_IPV4_DSCP:
return 6;
case RTE_FLOW_FIELD_IPV4_TTL:
+ case RTE_FLOW_FIELD_IPV4_PROTO:
return 8;
case RTE_FLOW_FIELD_IPV4_SRC:
case RTE_FLOW_FIELD_IPV4_DST:
@@ -2194,10 +2195,11 @@ mlx5_flow_field_id_to_modify_info
info[idx].offset = data->offset;
}
break;
+ case RTE_FLOW_FIELD_IPV4_PROTO: /* Fall-through. */
case RTE_FLOW_FIELD_IPV6_PROTO:
MLX5_ASSERT(data->offset + width <= 8);
off_be = 8 - (data->offset + width);
- info[idx] = (struct field_modify_info){1, 0,
MLX5_MODI_OUT_IPV6_NEXT_HDR};
+ info[idx] = (struct field_modify_info){1, 0,
MLX5_MODI_OUT_IP_PROTOCOL};
if (mask)
mask[idx] = flow_modify_info_mask_8(width, off_be);
else
--
2.18.1
[PATCH 4/6] ethdev: add modify action support for IPsec fields
The following IPsec related field definitions added:
- RTE_FLOW_FIELD_ESP_SPI - SPI value in IPsec header
- RTE_FLOW_FIELD_ESP_SEQ_NUM - sequence number in header
- RTE_FLOW_FIELD_ESP_PROTO - next protocol value in trailer
Signed-off-by: Viacheslav Ovsiienko
---
doc/guides/rel_notes/release_24_03.rst | 4
lib/ethdev/rte_flow.h | 5 -
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 3e33ff2d86..2f78009dd8 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -65,6 +65,10 @@ New Features
* Added ``RTE_FLOW_ITEM_TYPE_RANDOM`` to match random value.
* Added ``RTE_FLOW_FIELD_RANDOM`` to represent it in field ID struct.
* Added ``RTE_FLOW_FIELD_IPV4_PROTO`` to represent it in field ID struct.
+ * Added ``RTE_FLOW_FIELD_ESP_SPI`` to represent it in field ID struct.
+ * Added ``RTE_FLOW_FIELD_ESP_SEQ_NUM`` to represent it in field ID struct.
+ * Added ``RTE_FLOW_FIELD_ESP_PROTO`` to represent it in field ID struct.
+
* ** Support for getting the number of used descriptors of a Tx queue. **
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 84af730dc7..6efba67f12 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3934,7 +3934,10 @@ enum rte_flow_field_id {
RTE_FLOW_FIELD_IPV4_TOTAL_LEN, /**< IPv4 total length. */
RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN,/**< IPv6 payload length. */
RTE_FLOW_FIELD_RANDOM, /**< Random value. */
- RTE_FLOW_FIELD_IPV4_PROTO /**< IPv4 next protocol. */
+ RTE_FLOW_FIELD_IPV4_PROTO, /**< IPv4 next protocol. */
+ RTE_FLOW_FIELD_ESP_SPI, /**< ESP SPI. */
+ RTE_FLOW_FIELD_ESP_SEQ_NUM, /**< ESP Sequence Number. */
+ RTE_FLOW_FIELD_ESP_PROTO/**< ESP next protocol value. */
};
/**
--
2.18.1
[PATCH 5/6] app/testpmd: add modify ESP related fields command line
Add new modify field destination type strings:
- "esp_spi", to modify Security Parameter Index field
- "esp_seq_num", to modify Sequence Number field
- "esp_proto", to modify next protocol field in ESP trailer
Signed-off-by: Viacheslav Ovsiienko
---
app/test-pmd/cmdline_flow.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 03b418a5d8..9e1048f945 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -963,6 +963,7 @@ static const char *const modify_field_ids[] = {
"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
"random",
"ipv4_proto",
+ "esp_spi", "esp_seq_num", "esp_proto",
NULL
};
--
2.18.1
[PATCH 6/6] net/mlx5: add modify field action IPsec support
Add mlx5 PMD support for the IPsec fields:
- RTE_FLOW_FIELD_ESP_SPI - SPI value in IPsec header
- RTE_FLOW_FIELD_ESP_SEQ_NUM - sequence number in header
- RTE_FLOW_FIELD_ESP_PROTO - next protocol value in trailer
Signed-off-by: Viacheslav Ovsiienko
---
doc/guides/rel_notes/release_24_03.rst | 3 +++
drivers/common/mlx5/mlx5_prm.h | 3 +++
drivers/net/mlx5/mlx5_flow_dv.c| 31 ++
3 files changed, 37 insertions(+)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 2f78009dd8..50e8476e59 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -96,6 +96,9 @@ New Features
* Added HW steering support for modify field
``RTE_FLOW_FIELD_GENEVE_OPT_CLASS`` flow action.
* Added HW steering support for modify field
``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` flow action.
* Added HW steering support for modify field ``RTE_FLOW_FIELD_IPV4_PROTO``
flow action.
+ * Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_SPI`` flow
action.
+ * Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_SEQ_NUM``
flow action.
+ * Added HW steering support for modify field
``RTE_FLOW_FIELD_ESP_PROTO`` flow action.
Removed Items
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index 44413517d0..3150412580 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -854,6 +854,9 @@ enum mlx5_modification_field {
MLX5_MODI_OUT_IPV6_PAYLOAD_LEN = 0x11E,
MLX5_MODI_OUT_IPV4_IHL = 0x11F,
MLX5_MODI_OUT_TCP_DATA_OFFSET = 0x120,
+ MLX5_MODI_OUT_ESP_SPI = 0x5E,
+ MLX5_MODI_OUT_ESP_SEQ_NUM = 0x82,
+ MLX5_MODI_OUT_IPSEC_NEXT_HDR = 0x126,
MLX5_MODI_INVALID = INT_MAX,
};
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 764940b700..90413f4a38 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1414,7 +1414,11 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
case RTE_FLOW_FIELD_GTP_TEID:
case RTE_FLOW_FIELD_MPLS:
case RTE_FLOW_FIELD_TAG:
+ case RTE_FLOW_FIELD_ESP_SPI:
+ case RTE_FLOW_FIELD_ESP_SEQ_NUM:
return 32;
+ case RTE_FLOW_FIELD_ESP_PROTO:
+ return 8;
case RTE_FLOW_FIELD_MARK:
return rte_popcount32(priv->sh->dv_mark_mask);
case RTE_FLOW_FIELD_META:
@@ -2205,6 +2209,33 @@ mlx5_flow_field_id_to_modify_info
else
info[idx].offset = off_be;
break;
+ case RTE_FLOW_FIELD_ESP_PROTO:
+ MLX5_ASSERT(data->offset + width <= 8);
+ off_be = 8 - (data->offset + width);
+ info[idx] = (struct field_modify_info){1, 0,
MLX5_MODI_OUT_IPSEC_NEXT_HDR};
+ if (mask)
+ mask[idx] = flow_modify_info_mask_8(width, off_be);
+ else
+ info[idx].offset = off_be;
+ break;
+ case RTE_FLOW_FIELD_ESP_SPI:
+ MLX5_ASSERT(data->offset + width <= 32);
+ off_be = 32 - (data->offset + width);
+ info[idx] = (struct field_modify_info){4, 0,
MLX5_MODI_OUT_ESP_SPI};
+ if (mask)
+ mask[idx] = flow_modify_info_mask_32(width, off_be);
+ else
+ info[idx].offset = off_be;
+ break;
+ case RTE_FLOW_FIELD_ESP_SEQ_NUM:
+ MLX5_ASSERT(data->offset + width <= 32);
+ off_be = 32 - (data->offset + width);
+ info[idx] = (struct field_modify_info){4, 0,
MLX5_MODI_OUT_ESP_SEQ_NUM};
+ if (mask)
+ mask[idx] = flow_modify_info_mask_32(width, off_be);
+ else
+ info[idx].offset = off_be;
+ break;
case RTE_FLOW_FIELD_FLEX_ITEM:
MLX5_ASSERT(data->flex_handle != NULL && !(data->offset & 0x7));
mlx5_modify_flex_item(dev, (const struct mlx5_flex_item
*)data->flex_handle,
--
2.18.1
RE: [PATCH 2/2] net/mlx5: improve pattern template validation
Hi Gregory,
> -Original Message-
> From: Gregory Etelson
> Sent: Friday, February 2, 2024 16:06
> To: dev@dpdk.org
> Cc: Gregory Etelson ; Maayan Kashani
> ; Dariusz Sosnowski ;
> Slava Ovsiienko ; Ori Kam ;
> Suanming Mou ; Matan Azrad
>
> Subject: [PATCH 2/2] net/mlx5: improve pattern template validation
>
> Current PMD implementation validates pattern templates that will always be
> rejected during table template creation.
>
> The patch adds basic HWS verifications to pattern validation to ensure that
> the
> pattern can be used in table template.
>
> PMD updates `rte_errno` if pattern template validation failed:
>
> E2BIG - pattern too big for PMD
> ENOTSUP - pattern not supported by PMD
> ENOMEM - PMD allocation failure
>
> Signed-off-by: Gregory Etelson
>
> [snip]
>
> --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
> index da873ae2e2..443aa5fcf0 100644
> --- a/drivers/net/mlx5/mlx5_flow_hw.c
> +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> @@ -6840,6 +6840,46 @@ flow_hw_pattern_has_sq_match(const struct
> rte_flow_item *items)
> return false;
> }
>
> +static int
> +pattern_template_validate(struct rte_eth_dev *dev,
> + struct rte_flow_pattern_template *pt[], uint32_t
> pt_num) {
> + uint32_t group = 0;
> + struct rte_flow_error error;
> + struct rte_flow_template_table_attr tbl_attr = {
> + .nb_flows = 64,
> + .insertion_type =
> RTE_FLOW_TABLE_INSERTION_TYPE_PATTERN,
> + .hash_func = RTE_FLOW_TABLE_HASH_FUNC_DEFAULT,
> + .flow_attr = {
> + .ingress = pt[0]->attr.ingress,
> + .egress = pt[0]->attr.egress,
> + .transfer = pt[0]->attr.transfer
> + }
> + };
> + struct mlx5_priv *priv = dev->data->dev_private;
> + struct rte_flow_actions_template *action_template;
> +
> + if (pt[0]->attr.ingress)
> + action_template = priv-
> >action_template_drop[MLX5DR_TABLE_TYPE_NIC_RX];
> + else if (pt[0]->attr.egress)
> + action_template = priv-
> >action_template_drop[MLX5DR_TABLE_TYPE_NIC_TX];
> + else if (pt[0]->attr.transfer)
> + action_template = priv-
> >action_template_drop[MLX5DR_TABLE_TYPE_FDB];
> + else
> + return EINVAL;
> + do {
> + struct rte_flow_template_table *tmpl_tbl;
> +
> + tbl_attr.flow_attr.group = group;
> + tmpl_tbl = flow_hw_template_table_create(dev, &tbl_attr, pt,
> pt_num,
flow_hw_table_create() should be called here.
If E-Switch is enabled, flow_hw_template_table_create() will perform internal
group translation for FDB and TX domain,
so group 0 will be untested.
> + &action_template, 1,
> NULL);
> + if (!tmpl_tbl)
> + return rte_errno;
> + flow_hw_table_destroy(dev, tmpl_tbl, &error);
I don't think that passing error struct is needed here, since this error is not
propagated up.
> [snip]
>
> @@ -9184,6 +9235,66 @@ flow_hw_compare_config(const struct
> mlx5_flow_hw_attr *hw_attr,
> return true;
> }
>
> +/*
> + * No need to explicitly release drop action templates on port stop.
> + * Drop action templates release with other action templates during
> + * mlx5_dev_close -> flow_hw_resource_release ->
> +flow_hw_actions_template_destroy */ static void
> +action_template_drop_release(struct rte_eth_dev *dev) {
> + int i;
> + struct mlx5_priv *priv = dev->data->dev_private;
> +
> + for (i = 0; i < MLX5DR_TABLE_TYPE_MAX; i++) {
> + if (!priv->action_template_drop[i])
> + continue;
> + flow_hw_actions_template_destroy(dev,
> + priv-
> >action_template_drop[i],
> + NULL);
> + }
I'd suggest adding zeroing action_template_drop array entries here.
In case of failure inside rte_flow_configure(), rollback code called on error
must
reset the affected fields in private data to allow safe call to
rte_flow_configure() again.
> [snip]
>
> @@ -9621,10 +9735,7 @@ flow_hw_resource_release(struct rte_eth_dev
> *dev)
> flow_hw_flush_all_ctrl_flows(dev);
> flow_hw_cleanup_tx_repr_tagging(dev);
> flow_hw_cleanup_ctrl_rx_tables(dev);
> - while (!LIST_EMPTY(&priv->flow_hw_grp)) {
> - grp = LIST_FIRST(&priv->flow_hw_grp);
> - flow_hw_group_unset_miss_group(dev, grp, NULL);
> - }
> + action_template_drop_release(dev);
Why is the miss actions cleanup code removed? It does not seem related to the
patch.
Best regards,
Dariusz Sosnowski
RE: [PATCH 1/2] net/mlx5/hws: definer, update pattern validations
Hi Gregory, > -Original Message- > From: Gregory Etelson > Sent: Friday, February 2, 2024 16:06 > To: dev@dpdk.org > Cc: Gregory Etelson ; Maayan Kashani > ; Dariusz Sosnowski ; > Slava Ovsiienko ; Ori Kam ; > Suanming Mou ; Matan Azrad > > Subject: [PATCH 1/2] net/mlx5/hws: definer, update pattern validations > > The patch updates HWS code for upcoming extended PMD pattern template > verification: > Support VOID flow item type. > Return E2BUG error code when pattern is too large for definer. There's a typo here: s/E2BUG/E2BIG/ Best regards, Dariusz Sosnowski
[PATCH] examples/ipsec-secgw: fix IPsec performance drop
Single packet free using rte_pktmbuf_free_bulk() is dropping the
performance. On cn10k, maximum of ~4% drop observed for IPsec
event mode single SA outbound case.
To fix this issue, single packet free will use rte_pktmbuf_free
API.
Fixes: bd7c063561b3 ("examples/ipsec-secgw: use bulk free")
Signed-off-by: Rahul Bhansali
---
examples/ipsec-secgw/ipsec-secgw.h | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec-secgw.h
b/examples/ipsec-secgw/ipsec-secgw.h
index 8baab44ee7..ec33a982df 100644
--- a/examples/ipsec-secgw/ipsec-secgw.h
+++ b/examples/ipsec-secgw/ipsec-secgw.h
@@ -229,11 +229,10 @@ free_reassembly_fail_pkt(struct rte_mbuf *mb)
}
/* helper routine to free bulk of packets */
-static inline void
-free_pkts(struct rte_mbuf *mb[], uint32_t n)
+static __rte_always_inline void
+free_pkts(struct rte_mbuf *mb[], const uint32_t n)
{
- rte_pktmbuf_free_bulk(mb, n);
-
+ n == 1 ? rte_pktmbuf_free(mb[0]) : rte_pktmbuf_free_bulk(mb, n);
core_stats_update_drop(n);
}
--
2.25.1
Re: [PATCH v2] net/iavf: remove error logs for vlan offloading
On Tue, Feb 6, 2024 at 1:44 PM Amiya Ranjan Mohakud wrote: > I also see a similar error with iavf using dpdk-22.11.1. But in my case, I > see 2 more additional error logs showing up during iavf_dev_configure(). > It's with ESX-8.0U1 with an inbox driver. Firmware upgrade did not resolve > the issue though. > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_execute_vf_cmd(): Return failure -4 for cmd 27 > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_enable_vlan_strip(): Failed to execute command of > OP_ENABLE_VLAN_STRIPPING > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_dev_init_vlan(): Failed to update vlan offload > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_dev_configure(): configure VLAN failed: -5 > 2023-12-08T09:58:01.156 |9322| MSG [NET] dpdk_log_write:107 > iavf_execute_vf_cmd(): Return failure -5 for cmd 14 > > I was currently following up with KaiwenX on this. Attached the mail thread > for reference. > > Could you please let me know if this patch would fix the issue or any more > additional fixes are needed? Those logs above are about actually requesting vlan offloading. While in my case, I was not requesting anything. -- David Marchand
Re: [PATCH v2] net/iavf: remove error logs for vlan offloading
On Tue, Feb 6, 2024 at 1:46 PM David Marchand wrote: > > On Tue, Feb 6, 2024 at 1:44 PM Amiya Ranjan Mohakud > wrote: > > I also see a similar error with iavf using dpdk-22.11.1. But in my case, I > > see 2 more additional error logs showing up during iavf_dev_configure(). > > It's with ESX-8.0U1 with an inbox driver. Firmware upgrade did not resolve > > the issue though. > > > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > > iavf_execute_vf_cmd(): Return failure -4 for cmd 27 > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > > iavf_enable_vlan_strip(): Failed to execute command of > > OP_ENABLE_VLAN_STRIPPING > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > > iavf_dev_init_vlan(): Failed to update vlan offload > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > > iavf_dev_configure(): configure VLAN failed: -5 > > 2023-12-08T09:58:01.156 |9322| MSG [NET] dpdk_log_write:107 > > iavf_execute_vf_cmd(): Return failure -5 for cmd 14 > > > > I was currently following up with KaiwenX on this. Attached the mail thread > > for reference. > > > > Could you please let me know if this patch would fix the issue or any more > > additional fixes are needed? > > Those logs above are about actually requesting vlan offloading. > While in my case, I was not requesting anything. Ah sorry, I read too quickly and missed the iavf_dev_init_vlan(). Well, it may be related, though I can't produce the mailbox errors in my case, which seems specific to the ESX PF driver. And for this, I have no clue what is wrong. -- David Marchand
RE: [PATCH 1/6] ethdev: add modify IPv4 next protocol field
Hi Slava
> -Original Message-
> From: Slava Ovsiienko
> Sent: Tuesday, February 6, 2024 2:18 PM
>
> Subject: [PATCH 1/6] ethdev: add modify IPv4 next protocol field
>
> Add IPv4 next protocol modify field definition.
>
> Signed-off-by: Viacheslav Ovsiienko
> ---
> doc/guides/rel_notes/release_24_03.rst | 1 +
> lib/ethdev/rte_flow.h | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guides/rel_notes/release_24_03.rst
> b/doc/guides/rel_notes/release_24_03.rst
> index 2b91217943..33e9303ae1 100644
> --- a/doc/guides/rel_notes/release_24_03.rst
> +++ b/doc/guides/rel_notes/release_24_03.rst
> @@ -64,6 +64,7 @@ New Features
>
>* Added ``RTE_FLOW_ITEM_TYPE_RANDOM`` to match random value.
>* Added ``RTE_FLOW_FIELD_RANDOM`` to represent it in field ID struct.
> + * Added ``RTE_FLOW_FIELD_IPV4_PROTO`` to represent it in field ID
> struct.
>
> * ** Support for getting the number of used descriptors of a Tx queue. **
>
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 1267c146e5..84af730dc7 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3933,7 +3933,8 @@ enum rte_flow_field_id {
> RTE_FLOW_FIELD_IPV4_IHL,/**< IPv4 IHL. */
> RTE_FLOW_FIELD_IPV4_TOTAL_LEN, /**< IPv4 total length. */
> RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN,/**< IPv6 payload length. */
> - RTE_FLOW_FIELD_RANDOM /**< Random value. */
> + RTE_FLOW_FIELD_RANDOM, /**< Random value. */
> + RTE_FLOW_FIELD_IPV4_PROTO /**< IPv4 next protocol. */
> };
>
> /**
> --
> 2.18.1
Acked-by: Ori Kam
Best,
Ori
RE: [PATCH 2/6] app/testpmd: add modify IPv4 next protocol command line
Hi Slava
> -Original Message-
> From: Slava Ovsiienko
> Sent: Tuesday, February 6, 2024 2:18 PM
> Subject: [PATCH 2/6] app/testpmd: add modify IPv4 next protocol command
> line
>
> Add new modify field action type string: "ipv4_proto".
>
> Signed-off-by: Viacheslav Ovsiienko
> ---
> app/test-pmd/cmdline_flow.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 4062879552..03b418a5d8 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -962,6 +962,7 @@ static const char *const modify_field_ids[] = {
> "geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
> "mpls",
> "tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
> "random",
> + "ipv4_proto",
> NULL
> };
>
> --
> 2.18.1
Acked-by: Ori Kam
Best,
Ori
Re: [PATCH v2] net/iavf: remove error logs for vlan offloading
Hi David,
Thanks for the patch.
I also see a similar error with iavf using dpdk-22.11.1. But in my case, I
see 2 more additional error logs showing up during iavf_dev_configure().
It's with ESX-8.0U1 with an inbox driver. Firmware upgrade did not
resolve the issue though.
2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107
iavf_execute_vf_cmd(): Return failure -4 for cmd 27
2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107
iavf_enable_vlan_strip(): Failed to execute command of
OP_ENABLE_VLAN_STRIPPING
2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107
iavf_dev_init_vlan(): Failed to update vlan offload
2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107
iavf_dev_configure(): configure VLAN failed: -5
2023-12-08T09:58:01.156 |9322| MSG [NET] dpdk_log_write:107
iavf_execute_vf_cmd(): Return failure -5 for cmd 14
I was currently following up with KaiwenX on this. Attached the mail thread
for reference.
Could you please let me know if this patch would fix the issue or any more
additional fixes are needed?
Regards,
Amiya
On Tue, Feb 6, 2024 at 4:04 PM David Marchand
wrote:
> This was reported by RH QE.
>
> When a vlan is enforced on a VF via an administrative configuration on
> the PF side, the net/iavf driver logs two error messages.
> Those error messages have no consequence on the rest of the port
> initialisation and packet processing works fine.
>
> [root@toto ~] # ip l set enp94s0 vf 0 vlan 2
> [root@toto ~] # dpdk-testpmd -a :5e:02.0 -- -i
> ...
> Configuring Port 0 (socket 0)
> iavf_dev_init_vlan(): Failed to update vlan offload
> iavf_dev_configure(): configure VLAN failed: -95
> iavf_set_rx_function(): request RXDID[1] in Queue[0] is legacy, set
> rx_pkt_burst as legacy for all queues
>
> The first change is to remove the error log in iavf_dev_init_vlan().
> This log is unneeded since all error path are covered by a dedicated log
> message already.
>
> Then, in iavf_dev_init_vlan(), requesting all possible VLAN offloading
> must not trigger an ERROR level log message. This is simply confusing,
> as the application may not have requested such vlan offloading.
> The reason why the driver requests all offloading is unclear so keep it
> as is. Instead, rephrase the log message and lower its level to INFO.
>
> Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand
> Acked-by: Bruce Richardson
> ---
> Changes since v1:
> - updated log message,
>
> ---
> drivers/net/iavf/iavf_ethdev.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/iavf/iavf_ethdev.c
> b/drivers/net/iavf/iavf_ethdev.c
> index 1fb876e827..3532e379a1 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -631,7 +631,8 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
> RTE_ETH_VLAN_FILTER_MASK |
> RTE_ETH_VLAN_EXTEND_MASK);
> if (err) {
> - PMD_DRV_LOG(ERR, "Failed to update vlan offload");
> + PMD_DRV_LOG(INFO,
> + "VLAN offloading is not supported, or offloading
> was refused by the PF");
> return err;
> }
>
> @@ -707,9 +708,7 @@ iavf_dev_configure(struct rte_eth_dev *dev)
> vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
> }
>
> - ret = iavf_dev_init_vlan(dev);
> - if (ret)
> - PMD_DRV_LOG(ERR, "configure VLAN failed: %d", ret);
> + iavf_dev_init_vlan(dev);
>
> if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
> if (iavf_init_rss(ad) != 0) {
> --
> 2.43.0
>
>
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
--- Begin Message ---
Hi Kaiwenx,
There are no specific parameters except "-m 1024 -l 1,2". Just that the
guest has 2 SR IOV X710 interfaces.
Just for an additional update -
ESX version: 8.0 U1
i40e PF driver version: 1.11.3.5
Firmware version:7.10 0x800075da 19.5.12
I tried updating the firmware version to 9.00 0x8000cadc 21.5.9 based on
the compatibility matrix
https://www.vmware.com/resources/compatibility/detail.php?deviceCategory=io&p
Re: [PATCH v2] net/iavf: remove error logs for vlan offloading
Thanks @David Marchand for the quick response. Regards, Amiya On Tue, Feb 6, 2024 at 6:18 PM David Marchand wrote: > On Tue, Feb 6, 2024 at 1:46 PM David Marchand > wrote: > > > > On Tue, Feb 6, 2024 at 1:44 PM Amiya Ranjan Mohakud > > wrote: > > > I also see a similar error with iavf using dpdk-22.11.1. But in my > case, I see 2 more additional error logs showing up during > iavf_dev_configure(). It's with ESX-8.0U1 with an inbox driver. Firmware > upgrade did not resolve the issue though. > > > > > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_execute_vf_cmd(): Return failure -4 for cmd 27 > > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_enable_vlan_strip(): Failed to execute command of > OP_ENABLE_VLAN_STRIPPING > > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_dev_init_vlan(): Failed to update vlan offload > > > 2023-12-08T09:58:00.906 |9322| MSG [NET] dpdk_log_write:107 > iavf_dev_configure(): configure VLAN failed: -5 > > > 2023-12-08T09:58:01.156 |9322| MSG [NET] dpdk_log_write:107 > iavf_execute_vf_cmd(): Return failure -5 for cmd 14 > > > > > > I was currently following up with KaiwenX on this. Attached the mail > thread for reference. > > > > > > Could you please let me know if this patch would fix the issue or any > more additional fixes are needed? > > > > Those logs above are about actually requesting vlan offloading. > > While in my case, I was not requesting anything. > > Ah sorry, I read too quickly and missed the iavf_dev_init_vlan(). > Well, it may be related, though I can't produce the mailbox errors in > my case, which seems specific to the ESX PF driver. > And for this, I have no clue what is wrong. > > > -- > David Marchand > > -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it.
Re: [PATCH] vdpa/mlx5: fix queue enable drain CQ
On 1/25/24 04:17, Yajun Wu wrote:
For the case: `ethtool -L eth0 combined xxx` in VM, VQ will disable
and enable without calling device close. In such case, need add
drain CQ before reuse/reset event QP.
Fixes: 24969c7b62 ("vdpa/mlx5: reuse event queues")
Cc: sta...@dpdk.org
Signed-off-by: Yajun Wu
Acked-by: Matan Azrad
---
drivers/vdpa/mlx5/mlx5_vdpa_event.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
Reviewed-by: Maxime Coquelin
Thanks,
Maxime
Re: [PATCH v2 1/2] vhost: fix memory leak in Virtio Tx split path
On 2/6/24 11:29, David Marchand wrote:
On Wed, Jan 31, 2024 at 8:53 PM Maxime Coquelin
wrote:
When vIOMMU is enabled and Virtio device is bound to kernel
driver in guest, rte_vhost_dequeue_burst() will often return
early because of IOTLB misses.
In theory, we can hit this issue with a dpdk pmd too, as long as the
vIOMMU is in use.
But the consequence would be a "really small" leak which does not have
the same impact as what was seen with the kernel driver which
maps/unmaps pages associated with virtio-net skb way more often :-).
So maybe rephrase this part emphasizing on the kernel case like:
"""
When vIOMMU is enabled, rte_vhost_dequeue_burst() can return early
because of IOTLB misses.
Such IOTLB misses are especially frequent when a Virtio device is
bound to a kernel driver in guest.
"""
Thanks, I agree with your suggestion, Virtio PMD is indeed also
impacted.
This patch fixes a mbuf leak occurring in this case.
Fixes: 242695f6122a ("vhost: allocate and free packets in bulk in Tx split")
Cc: sta...@dpdk.org
Signed-off-by: Maxime Coquelin
Signed-off-by: David Marchand
Reviewed-by: David Marchand
Re: [PATCH] vdpa/mlx5: fix queue enable drain CQ
On 2/6/24 14:17, Maxime Coquelin wrote:
On 1/25/24 04:17, Yajun Wu wrote:
For the case: `ethtool -L eth0 combined xxx` in VM, VQ will disable
and enable without calling device close. In such case, need add
drain CQ before reuse/reset event QP.
Fixes: 24969c7b62 ("vdpa/mlx5: reuse event queues")
No need to resend, but the Fixes SHA1 should be 12 chars long, not 10.
As a helper, you can below alias in your .gitconfig:
[alias]
fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'
Then, you can just use it like this:
$ git fixline 24969c7b6224afc48751d94fc0152fca8b6645b1
Fixes: 24969c7b6224 ("vdpa/mlx5: reuse event queues")
Cc: yaj...@nvidia.com
Cc: sta...@dpdk.org
Signed-off-by: Yajun Wu
Acked-by: Matan Azrad
---
drivers/vdpa/mlx5/mlx5_vdpa_event.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
Reviewed-by: Maxime Coquelin
Thanks,
Maxime
PMD for non PCI device
Hi, I have a usecase where I have to evaluate writing a DPDK PMD for a non PCI/e device doing the ethernet packet i/o. Wanted to know if the above usecase is supported by DPDK infra and any pointers on how one should go about writing a PMD for such a usecase if supported. Would appreciate any inputs. Regards -Prashant
Re: PMD for non PCI device
On Tue, Feb 06, 2024 at 07:36:16PM +0530, Prashant Upadhyaya wrote: > Hi, > > I have a usecase where I have to evaluate writing a DPDK PMD for a non > PCI/e device doing the ethernet packet i/o. > > Wanted to know if the above usecase is supported by DPDK infra and any > pointers on how one should go about writing a PMD for such a usecase > if supported. Would appreciate any inputs. > Hi, yes, such a usecase is supported, but the specifics of how to go about it will vary depending on the type of PMD it is. DPDK already supports a range of other types of PMD, for emulated, or SW backed PMDs, e.g. net/pcap driver, and drivers for various SoCs which don't use PCI. For the case where the PMD is backed by real hardware (or an emulated device that appears to a VM as a piece of hardware), you may want to consider writing a "bus" driver for DPDK to support probing of the device. For non-HW devices, the "vdev" bus may be what you want to use, where probing is not done and devices are created in response to cmdline arguments on init, or via C APIs later in the app. Regards, /Bruce
RE: [EXT] [PATCH] cryptodev: add a simple mapping cache to speed up ops pool create
> Cache the most recent VA -> PA mapping found so that we can skip
> most of the system calls. With 4K pages this reduces pool create
> time by about 90%.
>
> Signed-off-by: Andrew Boyer
I believe there should be a generic solution for this in mempool
if it is not there already.
Here, you are adding cache in mempool priv
which does not seem to be a correct place.
This optimization would be needed across all types of mempools.
Adding more people for comments.
> ---
> lib/cryptodev/rte_crypto.h| 5 +
> lib/cryptodev/rte_cryptodev.c | 23 ++-
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
> index dbc2700da5..ee6aa1e40e 100644
> --- a/lib/cryptodev/rte_crypto.h
> +++ b/lib/cryptodev/rte_crypto.h
> @@ -220,6 +220,11 @@ struct rte_crypto_op_pool_private {
> /**< Crypto op pool type operation. */
> uint16_t priv_size;
> /**< Size of private area in each crypto operation. */
> +
> + unsigned long vp_cache;
> + /* Virtual page address of previous op. */
> + rte_iova_t iovp_cache;
> + /* I/O virtual page address of previous op. */
> };
>
>
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index b233c0ecd7..d596f85a57 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -2568,12 +2569,32 @@ rte_crypto_op_init(struct rte_mempool *mempool,
> {
> struct rte_crypto_op *op = _op_data;
> enum rte_crypto_op_type type = *(enum rte_crypto_op_type
> *)opaque_arg;
> + struct rte_crypto_op_pool_private *priv;
> + unsigned long virt_addr = (unsigned long)(uintptr_t)_op_data;
> +#ifdef RTE_EXEC_ENV_WINDOWS
> + unsigned long page_mask = 4095;
> +#else
> + unsigned long page_mask = sysconf(_SC_PAGESIZE) - 1;
> +#endif
> + unsigned long virt_page = virt_addr & ~page_mask;
>
> memset(_op_data, 0, mempool->elt_size);
>
> __rte_crypto_op_reset(op, type);
>
> - op->phys_addr = rte_mem_virt2iova(_op_data);
> + priv = (struct rte_crypto_op_pool_private *)
> + rte_mempool_get_priv(mempool);
> +
> + if (virt_page == priv->vp_cache) {
> + op->phys_addr = priv->iovp_cache + (virt_addr & page_mask);
> + } else {
> + op->phys_addr = rte_mem_virt2iova(_op_data);
> +
> + /* Update cached values */
> + priv->vp_cache = virt_page;
> + priv->iovp_cache = op->phys_addr & ~page_mask;
> + }
> +
> op->mempool = mempool;
> }
>
> --
> 2.17.1
[PATCH 0/1] ethdev: add IPv6 field identifiers
Add new field identifiers for IPv6 traffic class and flow label.
Depends-on: series-31008 ("ethdev: add modify IPv4 next protocol field")
Michael Baum (1):
ethdev: add IPv6 FL and TC field identifiers
app/test-pmd/cmdline_flow.c| 1 +
doc/guides/rel_notes/release_24_03.rst | 2 ++
lib/ethdev/rte_flow.h | 4 +++-
3 files changed, 6 insertions(+), 1 deletion(-)
--
2.25.1
[PATCH 1/1] ethdev: add IPv6 FL and TC field identifiers
Add new "rte_flow_field_id" enumeration values to describe both IPv6
traffic class and IPv6 flow label fields.
The TC value is "RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS" in flow API and
"ipv6_traffic_class" in testpmd command.
The FL value is "RTE_FLOW_FIELD_IPV6_FLOW_LABEL" in flow API and
"ipv6_flow_label" in testpmd command.
Signed-off-by: Michael Baum
---
app/test-pmd/cmdline_flow.c| 1 +
doc/guides/rel_notes/release_24_03.rst | 2 ++
lib/ethdev/rte_flow.h | 4 +++-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 9e1048f945..a6ae4c0d29 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -964,6 +964,7 @@ static const char *const modify_field_ids[] = {
"random",
"ipv4_proto",
"esp_spi", "esp_seq_num", "esp_proto",
+ "ipv6_flow_label", "ipv6_traffic_class",
NULL
};
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index f08bf09e7f..10268b7879 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -68,6 +68,8 @@ New Features
* Added ``RTE_FLOW_FIELD_ESP_SPI`` to represent it in field ID struct.
* Added ``RTE_FLOW_FIELD_ESP_SEQ_NUM`` to represent it in field ID struct.
* Added ``RTE_FLOW_FIELD_ESP_PROTO`` to represent it in field ID struct.
+ * Added ``RTE_FLOW_FIELD_IPV6_FLOW_LABEL`` to represent it in field ID
struct.
+ * Added ``RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS`` to represent it in field ID
struct.
* ** Support for getting the number of used descriptors of a Tx queue. **
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6efba67f12..e66c247e7f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3937,7 +3937,9 @@ enum rte_flow_field_id {
RTE_FLOW_FIELD_IPV4_PROTO, /**< IPv4 next protocol. */
RTE_FLOW_FIELD_ESP_SPI, /**< ESP SPI. */
RTE_FLOW_FIELD_ESP_SEQ_NUM, /**< ESP Sequence Number. */
- RTE_FLOW_FIELD_ESP_PROTO/**< ESP next protocol value. */
+ RTE_FLOW_FIELD_ESP_PROTO, /**< ESP next protocol value. */
+ RTE_FLOW_FIELD_IPV6_FLOW_LABEL, /**< IPv6 flow label. */
+ RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS/**< IPv6 traffic class. */
};
/**
--
2.25.1
[PATCH v1 2/7] common/mlx5: reorder modification field PRM list
Reorder modification field PRM list according to values from lowest to
highest.
This patch also removes value specification from all fields which their
value is one more than previous one.
Signed-off-by: Michael Baum
---
drivers/common/mlx5/mlx5_prm.h | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index 1f04a35683..a13b5790b0 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -816,6 +816,7 @@ enum mlx5_modification_field {
MLX5_MODI_OUT_IPV6_HOPLIMIT,
MLX5_MODI_IN_IPV6_HOPLIMIT,
MLX5_MODI_META_DATA_REG_A,
+ MLX5_MODI_OUT_IP_PROTOCOL,
MLX5_MODI_META_DATA_REG_B = 0x50,
MLX5_MODI_META_REG_C_0,
MLX5_MODI_META_REG_C_1,
@@ -829,32 +830,31 @@ enum mlx5_modification_field {
MLX5_MODI_IN_TCP_SEQ_NUM,
MLX5_MODI_OUT_TCP_ACK_NUM,
MLX5_MODI_IN_TCP_ACK_NUM = 0x5C,
+ MLX5_MODI_OUT_ESP_SPI = 0x5E,
MLX5_MODI_GTP_TEID = 0x6E,
MLX5_MODI_OUT_IP_ECN = 0x73,
MLX5_MODI_TUNNEL_HDR_DW_1 = 0x75,
- MLX5_MODI_GTPU_FIRST_EXT_DW_0 = 0x76,
+ MLX5_MODI_GTPU_FIRST_EXT_DW_0,
MLX5_MODI_HASH_RESULT = 0x81,
+ MLX5_MODI_OUT_ESP_SEQ_NUM,
MLX5_MODI_IN_MPLS_LABEL_0 = 0x8a,
MLX5_MODI_IN_MPLS_LABEL_1,
MLX5_MODI_IN_MPLS_LABEL_2,
MLX5_MODI_IN_MPLS_LABEL_3,
MLX5_MODI_IN_MPLS_LABEL_4,
- MLX5_MODI_OUT_IP_PROTOCOL = 0x4A,
- MLX5_MODI_META_REG_C_8 = 0x8F,
- MLX5_MODI_META_REG_C_9 = 0x90,
- MLX5_MODI_META_REG_C_10 = 0x91,
- MLX5_MODI_META_REG_C_11 = 0x92,
- MLX5_MODI_META_REG_C_12 = 0x93,
- MLX5_MODI_META_REG_C_13 = 0x94,
- MLX5_MODI_META_REG_C_14 = 0x95,
- MLX5_MODI_META_REG_C_15 = 0x96,
+ MLX5_MODI_META_REG_C_8,
+ MLX5_MODI_META_REG_C_9,
+ MLX5_MODI_META_REG_C_10,
+ MLX5_MODI_META_REG_C_11,
+ MLX5_MODI_META_REG_C_12,
+ MLX5_MODI_META_REG_C_13,
+ MLX5_MODI_META_REG_C_14,
+ MLX5_MODI_META_REG_C_15,
MLX5_MODI_OUT_IPV6_TRAFFIC_CLASS = 0x11C,
- MLX5_MODI_OUT_IPV4_TOTAL_LEN = 0x11D,
- MLX5_MODI_OUT_IPV6_PAYLOAD_LEN = 0x11E,
- MLX5_MODI_OUT_IPV4_IHL = 0x11F,
- MLX5_MODI_OUT_TCP_DATA_OFFSET = 0x120,
- MLX5_MODI_OUT_ESP_SPI = 0x5E,
- MLX5_MODI_OUT_ESP_SEQ_NUM = 0x82,
+ MLX5_MODI_OUT_IPV4_TOTAL_LEN,
+ MLX5_MODI_OUT_IPV6_PAYLOAD_LEN,
+ MLX5_MODI_OUT_IPV4_IHL,
+ MLX5_MODI_OUT_TCP_DATA_OFFSET,
MLX5_MODI_OUT_IPSEC_NEXT_HDR = 0x126,
MLX5_MODI_INVALID = INT_MAX,
};
--
2.25.1
[PATCH v1 0/7] net/mlx5: support copy from inner fields
This patch-set adds support of encapsulation level for HWS modify field
in MLX5 PMD.
Outermost is represented by 0,1 and inner is represented by 2.
In addition, modify inner/outer us added for both IPv6 flow label and
IPv6 traffic class.
Depends-on: series-31008 ("ethdev: add modify IPv4 next protocol field")
Depends-on: series-31010 ("ethdev: add IPv6 field identifiers")
Michael Baum (7):
common/mlx5: remove enum value duplication
common/mlx5: reorder modification field PRM list
common/mlx5: add inner PRM fields
common/mlx5: add IPv6 flow label PRM field
net/mlx5: add support for modify inner fields
net/mlx5: support modify IPv6 traffic class field
net/mlx5: support modify IPv6 flow label field
doc/guides/nics/mlx5.rst | 28 -
doc/guides/rel_notes/release_24_03.rst | 2 +
drivers/common/mlx5/mlx5_prm.h | 49 +
drivers/net/mlx5/hws/mlx5dr_action.c | 4 +-
drivers/net/mlx5/hws/mlx5dr_pat_arg.c | 2 +-
drivers/net/mlx5/mlx5_flow.c | 12 ++-
drivers/net/mlx5/mlx5_flow_dv.c| 136 +++--
drivers/net/mlx5/mlx5_flow_hw.c| 134 +++-
8 files changed, 282 insertions(+), 85 deletions(-)
--
2.25.1
[PATCH v1 1/7] common/mlx5: remove enum value duplication
The "mlx5_modification_field" enumeration has 2 different fields
representing the same value 0x4A.
1. "MLX5_MODI_OUT_IPV6_NEXT_HDR" - specific for IPv6.
2. "MLX5_MODI_OUT_IP_PROTOCOL" - for both IPv4 and IPv6.
This patch removes "MLX5_MODI_OUT_IPV6_NEXT_HDR" and replaces all its
usages with "MLX5_MODI_OUT_IP_PROTOCOL".
Signed-off-by: Michael Baum
---
drivers/common/mlx5/mlx5_prm.h| 1 -
drivers/net/mlx5/hws/mlx5dr_action.c | 4 ++--
drivers/net/mlx5/hws/mlx5dr_pat_arg.c | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index 3150412580..1f04a35683 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -840,7 +840,6 @@ enum mlx5_modification_field {
MLX5_MODI_IN_MPLS_LABEL_3,
MLX5_MODI_IN_MPLS_LABEL_4,
MLX5_MODI_OUT_IP_PROTOCOL = 0x4A,
- MLX5_MODI_OUT_IPV6_NEXT_HDR = 0x4A,
MLX5_MODI_META_REG_C_8 = 0x8F,
MLX5_MODI_META_REG_C_9 = 0x90,
MLX5_MODI_META_REG_C_10 = 0x91,
diff --git a/drivers/net/mlx5/hws/mlx5dr_action.c
b/drivers/net/mlx5/hws/mlx5dr_action.c
index 862ee3e332..2828a82d5b 100644
--- a/drivers/net/mlx5/hws/mlx5dr_action.c
+++ b/drivers/net/mlx5/hws/mlx5dr_action.c
@@ -2287,7 +2287,7 @@ mlx5dr_action_create_pop_ipv6_route_ext_mhdr3(struct
mlx5dr_action *action)
MLX5_SET(copy_action_in, cmd, length, 8);
MLX5_SET(copy_action_in, cmd, src_offset, 24);
MLX5_SET(copy_action_in, cmd, src_field, mod_id);
- MLX5_SET(copy_action_in, cmd, dst_field, MLX5_MODI_OUT_IPV6_NEXT_HDR);
+ MLX5_SET(copy_action_in, cmd, dst_field, MLX5_MODI_OUT_IP_PROTOCOL);
pattern.data = (__be64 *)cmd;
pattern.sz = sizeof(cmd);
@@ -2348,7 +2348,7 @@ mlx5dr_action_create_push_ipv6_route_ext_mhdr1(struct
mlx5dr_action *action)
/* Set ipv6.protocol to IPPROTO_ROUTING */
MLX5_SET(set_action_in, cmd, action_type, MLX5_MODIFICATION_TYPE_SET);
MLX5_SET(set_action_in, cmd, length, 8);
- MLX5_SET(set_action_in, cmd, field, MLX5_MODI_OUT_IPV6_NEXT_HDR);
+ MLX5_SET(set_action_in, cmd, field, MLX5_MODI_OUT_IP_PROTOCOL);
MLX5_SET(set_action_in, cmd, data, IPPROTO_ROUTING);
pattern.data = (__be64 *)cmd;
diff --git a/drivers/net/mlx5/hws/mlx5dr_pat_arg.c
b/drivers/net/mlx5/hws/mlx5dr_pat_arg.c
index a949844d24..513549ff3c 100644
--- a/drivers/net/mlx5/hws/mlx5dr_pat_arg.c
+++ b/drivers/net/mlx5/hws/mlx5dr_pat_arg.c
@@ -67,7 +67,7 @@ bool mlx5dr_pat_require_reparse(__be64 *actions, uint16_t
num_of_actions)
/* Below fields can change packet structure require a reparse */
if (field == MLX5_MODI_OUT_ETHERTYPE ||
- field == MLX5_MODI_OUT_IPV6_NEXT_HDR)
+ field == MLX5_MODI_OUT_IP_PROTOCOL)
return true;
}
--
2.25.1
[PATCH v1 3/7] common/mlx5: add inner PRM fields
This patch adds inner values into PRM modify field list for each
existing outer field.
Signed-off-by: Michael Baum
---
drivers/common/mlx5/mlx5_prm.h | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index a13b5790b0..7ac43f42b8 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -829,14 +829,17 @@ enum mlx5_modification_field {
MLX5_MODI_OUT_TCP_SEQ_NUM,
MLX5_MODI_IN_TCP_SEQ_NUM,
MLX5_MODI_OUT_TCP_ACK_NUM,
- MLX5_MODI_IN_TCP_ACK_NUM = 0x5C,
+ MLX5_MODI_IN_TCP_ACK_NUM,
MLX5_MODI_OUT_ESP_SPI = 0x5E,
+ MLX5_MODI_IN_ESP_SPI,
MLX5_MODI_GTP_TEID = 0x6E,
MLX5_MODI_OUT_IP_ECN = 0x73,
- MLX5_MODI_TUNNEL_HDR_DW_1 = 0x75,
+ MLX5_MODI_IN_IP_ECN,
+ MLX5_MODI_TUNNEL_HDR_DW_1,
MLX5_MODI_GTPU_FIRST_EXT_DW_0,
MLX5_MODI_HASH_RESULT = 0x81,
MLX5_MODI_OUT_ESP_SEQ_NUM,
+ MLX5_MODI_IN_ESP_SEQ_NUM,
MLX5_MODI_IN_MPLS_LABEL_0 = 0x8a,
MLX5_MODI_IN_MPLS_LABEL_1,
MLX5_MODI_IN_MPLS_LABEL_2,
@@ -855,7 +858,12 @@ enum mlx5_modification_field {
MLX5_MODI_OUT_IPV6_PAYLOAD_LEN,
MLX5_MODI_OUT_IPV4_IHL,
MLX5_MODI_OUT_TCP_DATA_OFFSET,
- MLX5_MODI_OUT_IPSEC_NEXT_HDR = 0x126,
+ MLX5_MODI_IN_IPV6_TRAFFIC_CLASS,
+ MLX5_MODI_IN_IPV4_TOTAL_LEN,
+ MLX5_MODI_IN_IPV6_PAYLOAD_LEN,
+ MLX5_MODI_IN_IPV4_IHL,
+ MLX5_MODI_IN_TCP_DATA_OFFSET,
+ MLX5_MODI_OUT_IPSEC_NEXT_HDR,
MLX5_MODI_INVALID = INT_MAX,
};
--
2.25.1
[PATCH v1 4/7] common/mlx5: add IPv6 flow label PRM field
Add IPv6 flow label field into PRM modify field list.
The new values are "MLX5_MODI_OUT_IPV6_FLOW_LABEL" and
"MLX5_MODI_IN_IPV6_FLOW_LABEL".
Signed-off-by: Michael Baum
---
drivers/common/mlx5/mlx5_prm.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index 7ac43f42b8..afd0001e8f 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -864,6 +864,8 @@ enum mlx5_modification_field {
MLX5_MODI_IN_IPV4_IHL,
MLX5_MODI_IN_TCP_DATA_OFFSET,
MLX5_MODI_OUT_IPSEC_NEXT_HDR,
+ MLX5_MODI_OUT_IPV6_FLOW_LABEL,
+ MLX5_MODI_IN_IPV6_FLOW_LABEL,
MLX5_MODI_INVALID = INT_MAX,
};
--
2.25.1
[PATCH v1 5/7] net/mlx5: add support for modify inner fields
This patch adds support for copying from inner fields using "level" 2.
Signed-off-by: Michael Baum
---
doc/guides/nics/mlx5.rst| 28 ++-
drivers/net/mlx5/mlx5_flow.c| 12 ++-
drivers/net/mlx5/mlx5_flow_dv.c | 113 ++-
drivers/net/mlx5/mlx5_flow_hw.c | 130 +++-
4 files changed, 222 insertions(+), 61 deletions(-)
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 6e1e2df79a..030bbe664f 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -595,7 +595,6 @@ Limitations
Only DWs configured in :ref:`parser creation ` can be
modified,
'type' and 'class' fields can be modified when ``match_on_class_mode=2``.
- Modification of GENEVE TLV option data supports one DW per action.
- - Encapsulation levels are not supported, can modify outermost header fields
only.
- Offsets cannot skip past the boundary of a field.
- If the field type is ``RTE_FLOW_FIELD_MAC_TYPE``
and packet contains one or more VLAN headers,
@@ -609,6 +608,33 @@ Limitations
- For flow metadata fields (e.g. META or TAG)
offset specifies the number of bits to skip from field's start,
starting from LSB in the least significant byte, in the host order.
+ - Modification of the MPLS header is supported with some limitations:
+
+- Only in HW steering.
+- Only in ``src`` field.
+- Only for outermost tunnel header (``level=2``).
+ For ``RTE_FLOW_FIELD_MPLS``,
+ the default encapsulation level ``0`` describes the outermost tunnel
header.
+
+ .. note::
+
+ the default encapsulation level ``0`` describes the "outermost that
match is supported",
+ currently it is first tunnel, but it can be changed to outer when it
is supported.
+
+ - Default encapsulation level ``0`` describes outermost.
+ - Encapsulation level ``1`` is supported.
+ - Encapsulation level ``2`` is supported with some limitations:
+
+- Only in HW steering.
+- Only in ``src`` field.
+- ``RTE_FLOW_FIELD_VLAN_ID`` is not supported.
+- ``RTE_FLOW_FIELD_IPV4_PROTO`` is not supported.
+- ``RTE_FLOW_FIELD_IPV6_PROTO/DSCP/ECN`` are not supported.
+- ``RTE_FLOW_FIELD_ESP_PROTO/SPI/SEQ_NUM`` are not supported.
+- ``RTE_FLOW_FIELD_TCP_SEQ/ACK_NUM`` are not supported.
+- Second tunnel fields are not supported.
+
+ - Encapsulation levels greater than ``2`` are not supported.
- Age action:
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 5159e8e773..fa85089364 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2507,10 +2507,14 @@ flow_validate_modify_field_level(const struct
rte_flow_action_modify_data *data,
if (data->level == 0)
return 0;
if (data->field != RTE_FLOW_FIELD_TAG &&
- data->field != (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION, NULL,
- "inner header fields modification is
not supported");
+ data->field != (enum
rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG) {
+ if (data->level > 1)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL,
+ "inner header fields
modification is not supported");
+ return 0;
+ }
if (data->tag_index != 0)
return rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 90413f4a38..8454d00d4f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -82,6 +82,9 @@
} \
} while (0)
+#define CALC_MODI_ID(field, level) \
+ (((level) > 1) ? MLX5_MODI_IN_##field : MLX5_MODI_OUT_##field)
+
union flow_dv_attr {
struct {
uint32_t valid:1;
@@ -1638,8 +1641,8 @@ mlx5_flow_field_id_to_modify_info
MLX5_ASSERT(data->offset + width <= 48);
off_be = 48 - (data->offset + width);
if (off_be < 16) {
- info[idx] = (struct field_modify_info){2, 4,
- MLX5_MODI_OUT_DMAC_15_0};
+ modi_id = CALC_MODI_ID(DMAC_15_0, data->level);
+ info[idx] = (struct field_modify_info){2, 4, modi_id};
length = off_be + width <= 16 ? width : 16 - off_be;
if (mask)
mask[1] = flow_modify_info_mask_16(length,
@@ -1654,8 +1657,8 @@ mlx5_flow_field_id_to_modify_info
} else {
[PATCH v1 6/7] net/mlx5: support modify IPv6 traffic class field
Add HW steering support for IPv6 traffic class field modification.
Copy from inner IPv6 traffic class field is also supported using
"level=2".
Signed-off-by: Michael Baum
---
doc/guides/rel_notes/release_24_03.rst | 1 +
drivers/net/mlx5/mlx5_flow_dv.c| 11 +++
drivers/net/mlx5/mlx5_flow_hw.c| 3 ++-
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 10268b7879..272bc9d056 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -100,6 +100,7 @@ New Features
* Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_SPI`` flow
action.
* Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_SEQ_NUM``
flow action.
* Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_PROTO``
flow action.
+ * Added HW steering support for modify field
``RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS`` flow action.
Removed Items
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 8454d00d4f..90ef21d75b 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1394,6 +1394,7 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
return 32;
case RTE_FLOW_FIELD_IPV6_DSCP:
return 6;
+ case RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS:
case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
case RTE_FLOW_FIELD_IPV6_PROTO:
return 8;
@@ -1795,6 +1796,16 @@ mlx5_flow_field_id_to_modify_info
else
info[idx].offset = off_be;
break;
+ case RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS:
+ MLX5_ASSERT(data->offset + width <= 8);
+ off_be = 8 - (data->offset + width);
+ modi_id = CALC_MODI_ID(IPV6_TRAFFIC_CLASS, data->level);
+ info[idx] = (struct field_modify_info){1, 0, modi_id};
+ if (mask)
+ mask[idx] = flow_modify_info_mask_8(width, off_be);
+ else
+ info[idx].offset = off_be;
+ break;
case RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN:
MLX5_ASSERT(data->offset + width <= 16);
off_be = 16 - (data->offset + width);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index fc0f9dabb3..90e3cf2555 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2871,7 +2871,7 @@ flow_hw_modify_field_construct(struct mlx5_hw_q_job *job,
* bits left. Shift the data left for IPV6 DSCP
*/
if (field->id == MLX5_MODI_OUT_IPV6_TRAFFIC_CLASS &&
- !(mask & MLX5_IPV6_HDR_ECN_MASK))
+ mhdr_action->dst.field == RTE_FLOW_FIELD_IPV6_DSCP)
data <<= MLX5_IPV6_HDR_DSCP_SHIFT;
data = (data & mask) >> off_b;
job->mhdr_cmd[i++].data1 = rte_cpu_to_be_32(data);
@@ -5058,6 +5058,7 @@ flow_hw_validate_modify_field_level(const struct
rte_flow_action_modify_data *da
case RTE_FLOW_FIELD_IPV4_TTL:
case RTE_FLOW_FIELD_IPV4_SRC:
case RTE_FLOW_FIELD_IPV4_DST:
+ case RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS:
case RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN:
case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
case RTE_FLOW_FIELD_IPV6_SRC:
--
2.25.1
[PATCH v1 7/7] net/mlx5: support modify IPv6 flow label field
Add HW steering support for IPv6 flow label field modification.
Copy from inner IPv6 flow label field is also supported using "level=2".
Signed-off-by: Michael Baum
---
doc/guides/rel_notes/release_24_03.rst | 1 +
drivers/net/mlx5/mlx5_flow_dv.c| 12
drivers/net/mlx5/mlx5_flow_hw.c| 1 +
3 files changed, 14 insertions(+)
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 272bc9d056..4cb45ba439 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -101,6 +101,7 @@ New Features
* Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_SEQ_NUM``
flow action.
* Added HW steering support for modify field ``RTE_FLOW_FIELD_ESP_PROTO``
flow action.
* Added HW steering support for modify field
``RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS`` flow action.
+ * Added HW steering support for modify field
``RTE_FLOW_FIELD_IPV6_FLOW_LABEL`` flow action.
Removed Items
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 90ef21d75b..418e0c0998 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1394,6 +1394,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
return 32;
case RTE_FLOW_FIELD_IPV6_DSCP:
return 6;
+ case RTE_FLOW_FIELD_IPV6_FLOW_LABEL:
+ return 20;
case RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS:
case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
case RTE_FLOW_FIELD_IPV6_PROTO:
@@ -1806,6 +1808,16 @@ mlx5_flow_field_id_to_modify_info
else
info[idx].offset = off_be;
break;
+ case RTE_FLOW_FIELD_IPV6_FLOW_LABEL:
+ MLX5_ASSERT(data->offset + width <= 20);
+ off_be = 20 - (data->offset + width);
+ modi_id = CALC_MODI_ID(IPV6_FLOW_LABEL, data->level);
+ info[idx] = (struct field_modify_info){4, 0, modi_id};
+ if (mask)
+ mask[idx] = flow_modify_info_mask_32(width, off_be);
+ else
+ info[idx].offset = off_be;
+ break;
case RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN:
MLX5_ASSERT(data->offset + width <= 16);
off_be = 16 - (data->offset + width);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 90e3cf2555..90a06e7b44 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -5059,6 +5059,7 @@ flow_hw_validate_modify_field_level(const struct
rte_flow_action_modify_data *da
case RTE_FLOW_FIELD_IPV4_SRC:
case RTE_FLOW_FIELD_IPV4_DST:
case RTE_FLOW_FIELD_IPV6_TRAFFIC_CLASS:
+ case RTE_FLOW_FIELD_IPV6_FLOW_LABEL:
case RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN:
case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
case RTE_FLOW_FIELD_IPV6_SRC:
--
2.25.1
Re: [PATCH] net/virtio: fix duplicated rxq xstats
On 11/24/23 14:52, edwin.brosse...@6wind.com wrote:
From: Edwin Brossette
The flag RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS was temporarily set while
moving queue stats from 'struct rte_eth_stats' to the individual pmds,
as explained in commit f30e69b41f94 ("ethdev: add device flag to bypass
auto-filled queue xstats").
This flag was added so every pmd would keep its original behavior until
the change was implemented. However, this flag was not removed
afterwards in the virtio pmd and as a result, some queue stats are
displayed twice when trying to get them: once in lib_rte_ethdev, and a
second time in the virtio pmd.
Remove this flag so stats are printed only once.
Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue
xstats")
Cc: sta...@dpdk.org
Signed-off-by: Edwin Brossette
---
drivers/net/virtio/virtio_ethdev.c | 2 --
1 file changed, 2 deletions(-)
Applied to next-virtio tree.
Thanks,
Maxime
[PATCH v2 0/7] test case blocking and logging
We currently don't record test case results that couldn't be executed because of a previous failure, such as when a test suite setup failed, resulting in no executed test cases. In order to record the test cases that couldn't be executed, we must know the lists of test suites and test cases ahead of the actual test suite execution, as an error could occur before we even start executing test suites. In addition, the patch series contains two refactors and one improvement. The first refactor is closely related. The dts.py was renamed to runner.py and given a clear purpose - running the test suites and all other orchestration needed to run test suites. The logic for this was not all in the original dts.py module and it was brought there. The runner is also responsible for recording results, which is the blocked test cases are recorded. The other refactor, logging, is related to the first refactor. The logging module was simplified while extending capabilities. Each test suite logs into its own log file in addition to the main log file which the runner must handle (as it knows when we start executing particular test suites). The runner also handles the switching between execution stages for the purposes of logging. The one aforementioned improvement is in unifying how we specify test cases in the conf.yaml file and in the environment variable/command line argument. v2: Rebase and update of the whole patch. There are changes in all parts of the code, mainly improving the design and logic. Also added the last patch which improves test suite specification on the cmdline. Juraj Linkeš (7): dts: convert dts.py methods to class dts: move test suite execution logic to DTSRunner dts: filter test suites in executions dts: reorganize test result dts: block all test cases when earlier setup fails dts: refactor logging configuration dts: improve test suite and case filtering doc/guides/tools/dts.rst | 14 +- dts/framework/config/__init__.py | 36 +- dts/framework/config/conf_yaml_schema.json| 2 +- dts/framework/dts.py | 338 - dts/framework/logger.py | 235 +++--- dts/framework/remote_session/__init__.py | 6 +- .../interactive_remote_session.py | 6 +- .../remote_session/interactive_shell.py | 6 +- .../remote_session/remote_session.py | 8 +- dts/framework/runner.py | 701 ++ dts/framework/settings.py | 188 +++-- dts/framework/test_result.py | 565 -- dts/framework/test_suite.py | 239 +- dts/framework/testbed_model/node.py | 11 +- dts/framework/testbed_model/os_session.py | 7 +- .../traffic_generator/traffic_generator.py| 6 +- dts/main.py | 9 +- dts/pyproject.toml| 3 + dts/tests/TestSuite_smoke_tests.py| 2 +- 19 files changed, 1354 insertions(+), 1028 deletions(-) delete mode 100644 dts/framework/dts.py create mode 100644 dts/framework/runner.py -- 2.34.1
[PATCH v2 1/7] dts: convert dts.py methods to class
The dts.py module deviates from the rest of the code without a clear
reason. Converting it into a class and using better naming will improve
organization and code readability.
Signed-off-by: Juraj Linkeš
---
dts/framework/dts.py| 338
dts/framework/runner.py | 333 +++
dts/main.py | 6 +-
3 files changed, 337 insertions(+), 340 deletions(-)
delete mode 100644 dts/framework/dts.py
create mode 100644 dts/framework/runner.py
diff --git a/dts/framework/dts.py b/dts/framework/dts.py
deleted file mode 100644
index e16d4578a0..00
--- a/dts/framework/dts.py
+++ /dev/null
@@ -1,338 +0,0 @@
-# SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2010-2019 Intel Corporation
-# Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
-# Copyright(c) 2022-2023 University of New Hampshire
-
-r"""Test suite runner module.
-
-A DTS run is split into stages:
-
-#. Execution stage,
-#. Build target stage,
-#. Test suite stage,
-#. Test case stage.
-
-The module is responsible for running tests on testbeds defined in the test
run configuration.
-Each setup or teardown of each stage is recorded in a
:class:`~.test_result.DTSResult` or
-one of its subclasses. The test case results are also recorded.
-
-If an error occurs, the current stage is aborted, the error is recorded and
the run continues in
-the next iteration of the same stage. The return code is the highest
`severity` of all
-:class:`~.exception.DTSError`\s.
-
-Example:
-An error occurs in a build target setup. The current build target is
aborted and the run
-continues with the next build target. If the errored build target was the
last one in the given
-execution, the next execution begins.
-
-Attributes:
-dts_logger: The logger instance used in this module.
-result: The top level result used in the module.
-"""
-
-import sys
-
-from .config import (
-BuildTargetConfiguration,
-ExecutionConfiguration,
-TestSuiteConfig,
-load_config,
-)
-from .exception import BlockingTestSuiteError
-from .logger import DTSLOG, getLogger
-from .test_result import BuildTargetResult, DTSResult, ExecutionResult, Result
-from .test_suite import get_test_suites
-from .testbed_model import SutNode, TGNode
-
-# dummy defaults to satisfy linters
-dts_logger: DTSLOG = None # type: ignore[assignment]
-result: DTSResult = DTSResult(dts_logger)
-
-
-def run_all() -> None:
-"""Run all build targets in all executions from the test run configuration.
-
-Before running test suites, executions and build targets are first set up.
-The executions and build targets defined in the test run configuration are
iterated over.
-The executions define which tests to run and where to run them and build
targets define
-the DPDK build setup.
-
-The tests suites are set up for each execution/build target tuple and each
scheduled
-test case within the test suite is set up, executed and torn down. After
all test cases
-have been executed, the test suite is torn down and the next build target
will be tested.
-
-All the nested steps look like this:
-
-#. Execution setup
-
-#. Build target setup
-
-#. Test suite setup
-
-#. Test case setup
-#. Test case logic
-#. Test case teardown
-
-#. Test suite teardown
-
-#. Build target teardown
-
-#. Execution teardown
-
-The test cases are filtered according to the specification in the test run
configuration and
-the :option:`--test-cases` command line argument or
-the :envvar:`DTS_TESTCASES` environment variable.
-"""
-global dts_logger
-global result
-
-# create a regular DTS logger and create a new result with it
-dts_logger = getLogger("DTSRunner")
-result = DTSResult(dts_logger)
-
-# check the python version of the server that run dts
-_check_dts_python_version()
-
-sut_nodes: dict[str, SutNode] = {}
-tg_nodes: dict[str, TGNode] = {}
-try:
-# for all Execution sections
-for execution in load_config().executions:
-sut_node = sut_nodes.get(execution.system_under_test_node.name)
-tg_node = tg_nodes.get(execution.traffic_generator_node.name)
-
-try:
-if not sut_node:
-sut_node = SutNode(execution.system_under_test_node)
-sut_nodes[sut_node.name] = sut_node
-if not tg_node:
-tg_node = TGNode(execution.traffic_generator_node)
-tg_nodes[tg_node.name] = tg_node
-result.update_setup(Result.PASS)
-except Exception as e:
-failed_node = execution.system_under_test_node.name
-if sut_node:
-failed_node = execution.traffic_generator_node.name
-dts_logger.e
[PATCH v2 2/7] dts: move test suite execution logic to DTSRunner
Move the code responsible for running the test suite from the
TestSuite class to the DTSRunner class. This restructuring decision
was made to consolidate and unify the related logic into a single unit.
Signed-off-by: Juraj Linkeš
---
dts/framework/runner.py | 175
dts/framework/test_suite.py | 152 ++-
2 files changed, 169 insertions(+), 158 deletions(-)
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index acc1c4d6db..933685d638 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -19,6 +19,7 @@
import logging
import sys
+from types import MethodType
from .config import (
BuildTargetConfiguration,
@@ -26,10 +27,18 @@
TestSuiteConfig,
load_config,
)
-from .exception import BlockingTestSuiteError
+from .exception import BlockingTestSuiteError, SSHTimeoutError,
TestCaseVerifyError
from .logger import DTSLOG, getLogger
-from .test_result import BuildTargetResult, DTSResult, ExecutionResult, Result
-from .test_suite import get_test_suites
+from .settings import SETTINGS
+from .test_result import (
+BuildTargetResult,
+DTSResult,
+ExecutionResult,
+Result,
+TestCaseResult,
+TestSuiteResult,
+)
+from .test_suite import TestSuite, get_test_suites
from .testbed_model import SutNode, TGNode
@@ -227,7 +236,7 @@ def _run_build_target(
build_target_result.update_setup(Result.FAIL, e)
else:
-self._run_all_suites(sut_node, tg_node, execution,
build_target_result)
+self._run_test_suites(sut_node, tg_node, execution,
build_target_result)
finally:
try:
@@ -237,7 +246,7 @@ def _run_build_target(
self._logger.exception("Build target teardown failed.")
build_target_result.update_teardown(Result.FAIL, e)
-def _run_all_suites(
+def _run_test_suites(
self,
sut_node: SutNode,
tg_node: TGNode,
@@ -249,6 +258,9 @@ def _run_all_suites(
The method assumes the build target we're testing has already been
built on the SUT node.
The current build target thus corresponds to the current DPDK build
present on the SUT node.
+If a blocking test suite (such as the smoke test suite) fails, the
rest of the test suites
+in the current build target won't be executed.
+
Args:
sut_node: The execution's SUT node.
tg_node: The execution's TG node.
@@ -262,7 +274,7 @@ def _run_all_suites(
execution.test_suites[:0] =
[TestSuiteConfig.from_dict("smoke_tests")]
for test_suite_config in execution.test_suites:
try:
-self._run_single_suite(
+self._run_test_suite_module(
sut_node, tg_node, execution, build_target_result,
test_suite_config
)
except BlockingTestSuiteError as e:
@@ -276,7 +288,7 @@ def _run_all_suites(
if end_build_target:
break
-def _run_single_suite(
+def _run_test_suite_module(
self,
sut_node: SutNode,
tg_node: TGNode,
@@ -284,11 +296,18 @@ def _run_single_suite(
build_target_result: BuildTargetResult,
test_suite_config: TestSuiteConfig,
) -> None:
-"""Run all test suites in a single test suite module.
+"""Set up, execute and tear down all test suites in a single test
suite module.
The method assumes the build target we're testing has already been
built on the SUT node.
The current build target thus corresponds to the current DPDK build
present on the SUT node.
+Test suite execution consists of running the discovered test cases.
+A test case run consists of setup, execution and teardown of said test
case.
+
+Record the setup and the teardown and handle failures.
+
+The test cases to execute are discovered when creating the
:class:`TestSuite` object.
+
Args:
sut_node: The execution's SUT node.
tg_node: The execution's TG node.
@@ -313,14 +332,140 @@ def _run_single_suite(
else:
for test_suite_class in test_suite_classes:
-test_suite = test_suite_class(
-sut_node,
-tg_node,
-test_suite_config.test_cases,
-execution.func,
-build_target_result,
+test_suite = test_suite_class(sut_node, tg_node,
test_suite_config.test_cases)
+
+test_suite_name = test_suite.__class__.__name__
+test_suite_result =
build_target_result.add_test_suite(test_suite_name)
+try:
+self._logger.info(f"Starting test suite setup:
{test_suite_name}")
+test_suite.set_up_suite()
+test_suite_result.update_setup(Result.PASS)
+
[PATCH v2 3/7] dts: filter test suites in executions
We're currently filtering which test cases to run after some setup
steps, such as DPDK build, have already been taken. This prohibits us to
mark the test suites and cases that were supposed to be run as blocked
when an earlier setup fails, as that information is not available at
that time.
To remedy this, move the filtering to the beginning of each execution.
This is the first action taken in each execution and if we can't filter
the test cases, such as due to invalid inputs, we abort the whole
execution. No test suites nor cases will be marked as blocked as we
don't know which were supposed to be run.
On top of that, the filtering takes place in the TestSuite class, which
should only concern itself with test suite and test case logic, not the
processing behind the scenes. The logic has been moved to DTSRunner
which should do all the processing needed to run test suites.
The filtering itself introduces a few changes/assumptions which are more
sensible than before:
1. Assumption: There is just one TestSuite child class in each test
suite module. This was an implicit assumption before as we couldn't
specify the TestSuite classes in the test run configuration, just the
modules. The name of the TestSuite child class starts with "Test" and
then corresponds to the name of the module with CamelCase naming.
2. Unknown test cases specified both in the test run configuration and
the environment variable/command line argument are no longer silently
ignored. This is a quality of life improvement for users, as they
could easily be not aware of the silent ignoration.
Also, a change in the code results in pycodestyle warning and error:
[E] E203 whitespace before ':'
[W] W503 line break before binary operator
These two are not PEP8 compliant, so they're disabled.
Signed-off-by: Juraj Linkeš
---
dts/framework/config/__init__.py | 24 +-
dts/framework/config/conf_yaml_schema.json | 2 +-
dts/framework/runner.py| 426 +++--
dts/framework/settings.py | 3 +-
dts/framework/test_result.py | 34 ++
dts/framework/test_suite.py| 85 +---
dts/pyproject.toml | 3 +
dts/tests/TestSuite_smoke_tests.py | 2 +-
8 files changed, 382 insertions(+), 197 deletions(-)
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 62eded7f04..c6a93b3b89 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -36,7 +36,7 @@
import json
import os.path
import pathlib
-from dataclasses import dataclass
+from dataclasses import dataclass, fields
from enum import auto, unique
from typing import Union
@@ -506,6 +506,28 @@ def from_dict(
vdevs=vdevs,
)
+def copy_and_modify(self, **kwargs) -> "ExecutionConfiguration":
+"""Create a shallow copy with any of the fields modified.
+
+The only new data are those passed to this method.
+The rest are copied from the object's fields calling the method.
+
+Args:
+**kwargs: The names and types of keyword arguments are defined
+by the fields of the :class:`ExecutionConfiguration` class.
+
+Returns:
+The copied and modified execution configuration.
+"""
+new_config = {}
+for field in fields(self):
+if field.name in kwargs:
+new_config[field.name] = kwargs[field.name]
+else:
+new_config[field.name] = getattr(self, field.name)
+
+return ExecutionConfiguration(**new_config)
+
@dataclass(slots=True, frozen=True)
class Configuration:
diff --git a/dts/framework/config/conf_yaml_schema.json
b/dts/framework/config/conf_yaml_schema.json
index 84e45fe3c2..051b079fe4 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -197,7 +197,7 @@
},
"cases": {
"type": "array",
- "description": "If specified, only this subset of test suite's test
cases will be run. Unknown test cases will be silently ignored.",
+ "description": "If specified, only this subset of test suite's test
cases will be run.",
"items": {
"type": "string"
},
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 933685d638..3e95cf9e26 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -17,17 +17,27 @@
and the test case stage runs test cases individually.
"""
+import importlib
+import inspect
import logging
+import re
import sys
from types import MethodType
+from typing import Iterable
from .config import (
BuildTargetConfiguration,
+Configuration,
ExecutionConfiguration,
TestSuiteConfig,
load_config,
)
-from .exception import BlockingTestSuiteError, SSHTimeoutError,
TestCaseVerifyError
+from .exception import (
+BlockingTestSuiteError
[PATCH v2 4/7] dts: reorganize test result
The current order of Result classes in the test_suite.py module is
guided by the needs of type hints, which is not as intuitively readable
as ordering them by the occurrences in code. The order goes from the
topmost level to lowermost:
BaseResult
DTSResult
ExecutionResult
BuildTargetResult
TestSuiteResult
TestCaseResult
This is the same order as they're used in the runner module and they're
also used in the same order between themselves in the test_result
module.
Signed-off-by: Juraj Linkeš
---
dts/framework/test_result.py | 411 ++-
1 file changed, 206 insertions(+), 205 deletions(-)
diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index 075195fd5b..abdbafab10 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -28,6 +28,7 @@
from dataclasses import dataclass
from enum import Enum, auto
from types import MethodType
+from typing import Union
from .config import (
OS,
@@ -129,58 +130,6 @@ def __bool__(self) -> bool:
return bool(self.result)
-class Statistics(dict):
-"""How many test cases ended in which result state along some other basic
information.
-
-Subclassing :class:`dict` provides a convenient way to format the data.
-
-The data are stored in the following keys:
-
-* **PASS RATE** (:class:`int`) -- The FAIL/PASS ratio of all test cases.
-* **DPDK VERSION** (:class:`str`) -- The tested DPDK version.
-"""
-
-def __init__(self, dpdk_version: str | None):
-"""Extend the constructor with keys in which the data are stored.
-
-Args:
-dpdk_version: The version of tested DPDK.
-"""
-super(Statistics, self).__init__()
-for result in Result:
-self[result.name] = 0
-self["PASS RATE"] = 0.0
-self["DPDK VERSION"] = dpdk_version
-
-def __iadd__(self, other: Result) -> "Statistics":
-"""Add a Result to the final count.
-
-Example:
-stats: Statistics = Statistics() # empty Statistics
-stats += Result.PASS # add a Result to `stats`
-
-Args:
-other: The Result to add to this statistics object.
-
-Returns:
-The modified statistics object.
-"""
-self[other.name] += 1
-self["PASS RATE"] = (
-float(self[Result.PASS.name]) * 100 / sum(self[result.name] for
result in Result)
-)
-return self
-
-def __str__(self) -> str:
-"""Each line contains the formatted key = value pair."""
-stats_str = ""
-for key, value in self.items():
-stats_str += f"{key:<12} = {value}\n"
-# according to docs, we should use \n when writing to text files
-# on all platforms
-return stats_str
-
-
class BaseResult(object):
"""Common data and behavior of DTS results.
@@ -245,7 +194,7 @@ def get_errors(self) -> list[Exception]:
"""
return self._get_setup_teardown_errors() + self._get_inner_errors()
-def add_stats(self, statistics: Statistics) -> None:
+def add_stats(self, statistics: "Statistics") -> None:
"""Collate stats from the whole result hierarchy.
Args:
@@ -255,91 +204,149 @@ def add_stats(self, statistics: Statistics) -> None:
inner_result.add_stats(statistics)
-class TestCaseResult(BaseResult, FixtureResult):
-r"""The test case specific result.
+class DTSResult(BaseResult):
+"""Stores environment information and test results from a DTS run.
-Stores the result of the actual test case. This is done by adding an extra
superclass
-in :class:`FixtureResult`. The setup and teardown results are
:class:`FixtureResult`\s and
-the class is itself a record of the test case.
+* Execution level information, such as testbed and the test suite list,
+* Build target level information, such as compiler, target OS and cpu,
+* Test suite and test case results,
+* All errors that are caught and recorded during DTS execution.
+
+The information is stored hierarchically. This is the first level of the
hierarchy
+and as such is where the data form the whole hierarchy is collated or
processed.
+
+The internal list stores the results of all executions.
Attributes:
-test_case_name: The test case name.
+dpdk_version: The DPDK version to record.
"""
-test_case_name: str
+dpdk_version: str | None
+_logger: DTSLOG
+_errors: list[Exception]
+_return_code: ErrorSeverity
+_stats_result: Union["Statistics", None]
+_stats_filename: str
-def __init__(self, test_case_name: str):
-"""Extend the constructor with `test_case_name`.
+def __init__(self, logger: DTSLOG):
+"""Extend the constructor with top-level specifics.
Args:
-test_case_name: The test case's name.
+logger: The logger instance the whole r
[PATCH v2 5/7] dts: block all test cases when earlier setup fails
In case of a failure before a test suite, the child results will be
recursively recorded as blocked, giving us a full report which was
missing previously.
Signed-off-by: Juraj Linkeš
---
dts/framework/runner.py | 21 ++--
dts/framework/test_result.py | 186 +--
2 files changed, 148 insertions(+), 59 deletions(-)
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 3e95cf9e26..f58b0adc13 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -60,13 +60,15 @@ class DTSRunner:
Each setup or teardown of each stage is recorded in a
:class:`~framework.test_result.DTSResult`
or one of its subclasses. The test case results are also recorded.
-If an error occurs, the current stage is aborted, the error is recorded
and the run continues in
-the next iteration of the same stage. The return code is the highest
`severity` of all
+If an error occurs, the current stage is aborted, the error is recorded,
everything in
+the inner stages is marked as blocked and the run continues in the next
iteration
+of the same stage. The return code is the highest `severity` of all
:class:`~.framework.exception.DTSError`\s.
Example:
-An error occurs in a build target setup. The current build target is
aborted and the run
-continues with the next build target. If the errored build target was
the last one in the
+An error occurs in a build target setup. The current build target is
aborted,
+all test suites and their test cases are marked as blocked and the run
continues
+with the next build target. If the errored build target was the last
one in the
given execution, the next execution begins.
"""
@@ -100,6 +102,10 @@ def run(self):
test case within the test suite is set up, executed and torn down.
After all test cases
have been executed, the test suite is torn down and the next build
target will be tested.
+In order to properly mark test suites and test cases as blocked in
case of a failure,
+we need to have discovered which test suites and test cases to run
before any failures
+happen. The discovery happens at the earliest point at the start of
each execution.
+
All the nested steps look like this:
#. Execution setup
@@ -134,11 +140,12 @@ def run(self):
self._logger.info(
f"Running execution with SUT
'{execution.system_under_test_node.name}'."
)
-execution_result =
self._result.add_execution(execution.system_under_test_node)
+execution_result = self._result.add_execution(execution)
try:
test_suites_with_cases = self._get_test_suites_with_cases(
execution.test_suites, execution.func, execution.perf
)
+execution_result.test_suites_with_cases =
test_suites_with_cases
except Exception as e:
self._logger.exception(
f"Invalid test suite configuration found: "
f"{execution.test_suites}."
@@ -486,9 +493,7 @@ def _run_test_suites(
"""
end_build_target = False
for test_suite_with_cases in test_suites_with_cases:
-test_suite_result = build_target_result.add_test_suite(
-test_suite_with_cases.test_suite_class.__name__
-)
+test_suite_result =
build_target_result.add_test_suite(test_suite_with_cases)
try:
self._run_test_suite(sut_node, tg_node, test_suite_result,
test_suite_with_cases)
except BlockingTestSuiteError as e:
diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index abdbafab10..eedb2d20ee 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -37,7 +37,7 @@
BuildTargetInfo,
Compiler,
CPUType,
-NodeConfiguration,
+ExecutionConfiguration,
NodeInfo,
TestSuiteConfig,
)
@@ -88,6 +88,8 @@ class Result(Enum):
ERROR = auto()
#:
SKIP = auto()
+#:
+BLOCK = auto()
def __bool__(self) -> bool:
"""Only PASS is True."""
@@ -141,21 +143,26 @@ class BaseResult(object):
Attributes:
setup_result: The result of the setup of the particular stage.
teardown_result: The results of the teardown of the particular stage.
+child_results: The results of the descendants in the results hierarchy.
"""
setup_result: FixtureResult
teardown_result: FixtureResult
-_inner_results: MutableSequence["BaseResult"]
+child_results: MutableSequence["BaseResult"]
def __init__(self):
"""Initialize the constructor."""
self.setup_result = FixtureResult()
self.teardown_result = FixtureResult()
-self._inner_results = []
+self.child_results =
[PATCH v2 6/7] dts: refactor logging configuration
Remove unused parts of the code and add useful features: 1. Add DTS execution stages such as execution and test suite to better identify where in the DTS lifecycle we are when investigating logs, 2. Logging to separate files in specific stages, which is mainly useful for having test suite logs in additional separate files. 3. Remove the dependence on the settings module which enhances the usefulness of the logger module, as it can now be imported in more modules. The execution stages and the files to log to are the same for all DTS loggers. To achieve this, we have one DTS root logger which should be used for handling stage switching and all other loggers are children of this DTS root logger. The DTS root logger is the one where we change the behavior of all loggers (the stage and which files to log to) and the child loggers just log messages under a different name. Signed-off-by: Juraj Linkeš --- dts/framework/logger.py | 235 +++--- dts/framework/remote_session/__init__.py | 6 +- .../interactive_remote_session.py | 6 +- .../remote_session/interactive_shell.py | 6 +- .../remote_session/remote_session.py | 8 +- dts/framework/runner.py | 19 +- dts/framework/test_result.py | 6 +- dts/framework/test_suite.py | 6 +- dts/framework/testbed_model/node.py | 11 +- dts/framework/testbed_model/os_session.py | 7 +- .../traffic_generator/traffic_generator.py| 6 +- dts/main.py | 1 - 12 files changed, 183 insertions(+), 134 deletions(-) diff --git a/dts/framework/logger.py b/dts/framework/logger.py index cfa6e8cd72..568edad82d 100644 --- a/dts/framework/logger.py +++ b/dts/framework/logger.py @@ -5,141 +5,186 @@ """DTS logger module. -DTS framework and TestSuite logs are saved in different log files. +The module provides several additional features: + +* The storage of DTS execution stages, +* Logging to console, a human-readable log file and a machine-readable log file, +* Optional log files for specific stages. """ import logging -import os.path -from typing import TypedDict +from enum import auto +from logging import FileHandler, StreamHandler +from pathlib import Path +from typing import ClassVar -from .settings import SETTINGS +from .utils import StrEnum date_fmt = "%Y/%m/%d %H:%M:%S" -stream_fmt = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" +stream_fmt = "%(asctime)s - %(stage)s - %(name)s - %(levelname)s - %(message)s" +dts_root_logger_name = "dts" + + +class DtsStage(StrEnum): +"""The DTS execution stage.""" +#: +pre_execution = auto() +#: +execution = auto() +#: +build_target = auto() +#: +suite = auto() +#: +post_execution = auto() -class DTSLOG(logging.LoggerAdapter): -"""DTS logger adapter class for framework and testsuites. -The :option:`--verbose` command line argument and the :envvar:`DTS_VERBOSE` environment -variable control the verbosity of output. If enabled, all messages will be emitted to the -console. +class DTSLogger(logging.Logger): +"""The DTS logger class. -The :option:`--output` command line argument and the :envvar:`DTS_OUTPUT_DIR` environment -variable modify the directory where the logs will be stored. +The class extends the :class:`~logging.Logger` class to add the DTS execution stage information +to log records. The stage is common to all loggers, so it's stored in a class variable. -Attributes: -node: The additional identifier. Currently unused. -sh: The handler which emits logs to console. -fh: The handler which emits logs to a file. -verbose_fh: Just as fh, but logs with a different, more verbose, format. +Any time we switch to a new stage, we have the ability to log to an additional log file along +with a supplementary log file with machine-readable format. These two log files are used until +a new stage switch occurs. This is useful mainly for logging per test suite. """ -_logger: logging.Logger -node: str -sh: logging.StreamHandler -fh: logging.FileHandler -verbose_fh: logging.FileHandler +_stage: ClassVar[DtsStage] = DtsStage.pre_execution +_extra_file_handlers: list[FileHandler] = [] -def __init__(self, logger: logging.Logger, node: str = "suite"): -"""Extend the constructor with additional handlers. +def __init__(self, *args, **kwargs): +"""Extend the constructor with extra file handlers.""" +self._extra_file_handlers = [] +super().__init__(*args, **kwargs) -One handler logs to the console, the other one to a file, with either a regular or verbose -format. +def makeRecord(self, *args, **kwargs): +"""Generates a record with additional stage information. -Args: -
[PATCH v2 7/7] dts: improve test suite and case filtering
The two places where we specify which test suite and test cases to run are complimentary and not that intuitive to use. A unified way provides a better user experience. The syntax in test run configuration file has not changed, but the environment variable and the command line arguments was changed to match the config file syntax. This required changes in the settings module which greatly simplified the parsing of the environment variables while retaining the same functionality. Signed-off-by: Juraj Linkeš --- doc/guides/tools/dts.rst | 14 ++- dts/framework/config/__init__.py | 12 +- dts/framework/runner.py | 21 ++-- dts/framework/settings.py| 187 ++- dts/framework/test_suite.py | 2 +- dts/main.py | 2 - 6 files changed, 116 insertions(+), 122 deletions(-) diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst index f686ca487c..d1c3c2af7a 100644 --- a/doc/guides/tools/dts.rst +++ b/doc/guides/tools/dts.rst @@ -215,28 +215,30 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet .. code-block:: console (dts-py3.10) $ ./main.py --help - usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT] [--test-cases TEST_CASES] [--re-run RE_RUN] + usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL] [--compile-timeout COMPILE_TIMEOUT] [--test-suite TEST_SUITE [TEST_CASES ...]] [--re-run RE_RUN] Run DPDK test suites. All options may be specified with the environment variables provided in brackets. Command line arguments have higher priority. options: -h, --helpshow this help message and exit --config-file CONFIG_FILE - [DTS_CFG_FILE] configuration file that describes the test cases, SUTs and targets. (default: ./conf.yaml) + [DTS_CFG_FILE] The configuration file that describes the test cases, SUTs and targets. (default: ./conf.yaml) --output-dir OUTPUT_DIR, --output OUTPUT_DIR [DTS_OUTPUT_DIR] Output directory where DTS logs and results are saved. (default: output) -t TIMEOUT, --timeout TIMEOUT [DTS_TIMEOUT] The default timeout for all DTS operations except for compiling DPDK. (default: 15) -v, --verbose [DTS_VERBOSE] Specify to enable verbose output, logging all messages to the console. (default: False) - -s, --skip-setup [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: None) + -s, --skip-setup [DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes. (default: False) --tarball TARBALL, --snapshot TARBALL, --git-ref TARBALL [DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, tag ID or tree ID to test. To test local changes, first commit them, then use the commit ID with this option. (default: dpdk.tar.xz) --compile-timeout COMPILE_TIMEOUT [DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK. (default: 1200) - --test-cases TEST_CASES - [DTS_TESTCASES] Comma-separated list of test cases to execute. Unknown test cases will be silently ignored. (default: ) + --test-suite TEST_SUITE [TEST_CASES ...] + [DTS_TEST_SUITES] A list containing a test suite with test cases. The first parameter is the test suite name, and the rest are test case names, which are optional. May be specified multiple times. To specify multiple test suites in the environment + variable, join the lists with a comma. Examples: --test-suite suite case case --test-suite suite case ... | DTS_TEST_SUITES='suite case case, suite case, ...' | --test-suite suite --test-suite suite case ... | DTS_TEST_SUITES='suite, suite case, ...' + (default: []) --re-run RE_RUN, --re_run RE_RUN - [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs (default: 0) + [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0) The brackets contain the names of environment variables that set the same thing. diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py index c6a93b3b89..4cb5c74059 100644 --- a/dts/framework/config/__init__.py +++ b/dts/framework/config/__init__.py @@ -35,9 +35,9 @@ import json import os.path -import pathlib from dataclasses import dataclass, fields from enum import auto, unique +from pathlib import Path from typing import Union import warlock # type: ignore[import] @@ -53,7 +53,6 @@ TrafficGeneratorConfigDict, ) from framework.exception import ConfigurationError -from fram
Re: [PATCH v2] vhost: fix deadlock during software live migration of VDPA in a nested virtualization environment
On 1/22/24 04:27, Hao Chen wrote:
In a nested virtualization environment, running dpdk-vdpa in QEMU-L1 for
software live migration will result in a deadlock between dpdke-vdpa and
QEMU-L2 processes.
'rte_vdpa_relay_vring_used'->
'__vhost_iova_to_vva'->
'vhost_user_iotlb_rd_unlock(vq)'->
'vhost_user_iotlb_miss'-> send vhost message 'VHOST_USER_SLAVE_IOTLB_MSG'
to QEMU-L2's vdpa socket,
then call 'vhost_user_iotlb_rd_lock(vq)' to hold the read lock `iotlb_lock`.
But there is no place to release this read lock.
QEMU-L2 get the 'VHOST_USER_SLAVE_IOTLB_MSG',
then call 'vhost_user_send_device_iotlb_msg' to send 'VHOST_USER_IOTLB_MSG'
messages to dpdk-vdpa.
Dpdk-vdpa will call vhost_user_iotlb_msg->
vhost_user_iotlb_cache_insert, here, will obtain the write lock
`iotlb_lock`, but the read lock `iotlb_lock` has not been released and
will block here.
This patch add lock and unlock function to fix the deadlock.
Fixes: b13ad2decc83 ("vhost: provide helpers for virtio ring relay")
Cc: sta...@dpdk.org
Signed-off-by: Hao Chen
---
Changes v1 ... v2:
- protect the vhost_alloc_copy_ind_table() call too.
Applied to next-virtio tree with reworked commit message.
Thanks,
Maxime
Re: [PATCH v2 1/2] net/virtio-user: improve kick performance with notification area mapping
On 1/23/24 11:55, Srujana Challa wrote: This patch introduces new virtio-user callbacks to map the vq notification area and implements it for the vhost-vDPA backend. This is simply done by using mmap()/munmap() for the vhost-vDPA fd. And also adds code to write to queue notify address in notify callback. This will help in increasing the kick performance. Signed-off-by: Srujana Challa --- drivers/net/virtio/virtio_user/vhost.h| 2 + drivers/net/virtio/virtio_user/vhost_vdpa.c | 68 +++ .../net/virtio/virtio_user/virtio_user_dev.c | 43 ++-- .../net/virtio/virtio_user/virtio_user_dev.h | 2 + drivers/net/virtio/virtio_user_ethdev.c | 37 -- 5 files changed, 143 insertions(+), 9 deletions(-) Applied to next-virtio tree. Thanks, Maxime
Re: [PATCH v2 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features
On 1/23/24 11:55, Srujana Challa wrote: This patch introduces new function to get rss device config and adds code to forward the RSS control command to backend through hw control queue if RSS feature is negotiated. This patch will help to negotiate VIRTIO_NET_F_RSS feature if vhost-vdpa backend supports RSS in HW. Signed-off-by: Srujana Challa --- .../net/virtio/virtio_user/virtio_user_dev.c | 31 ++- .../net/virtio/virtio_user/virtio_user_dev.h | 2 ++ drivers/net/virtio/virtio_user_ethdev.c | 3 ++ 3 files changed, 35 insertions(+), 1 deletion(-) Applied to next-virtio tree. Thanks, Maxime
Re: [PATCH] vdpa/mlx5: fix queue enable drain CQ
On 1/25/24 04:17, Yajun Wu wrote:
For the case: `ethtool -L eth0 combined xxx` in VM, VQ will disable
and enable without calling device close. In such case, need add
drain CQ before reuse/reset event QP.
Fixes: 24969c7b62 ("vdpa/mlx5: reuse event queues")
Cc: sta...@dpdk.org
Signed-off-by: Yajun Wu
Acked-by: Matan Azrad
---
drivers/vdpa/mlx5/mlx5_vdpa_event.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
Applied to next-virtio tree.
Thanks,
Maxime
Re: [PATCH v2 1/2] vhost: fix memory leak in Virtio Tx split path
On 1/31/24 20:53, Maxime Coquelin wrote:
When vIOMMU is enabled and Virtio device is bound to kernel
driver in guest, rte_vhost_dequeue_burst() will often return
early because of IOTLB misses.
This patch fixes a mbuf leak occurring in this case.
Fixes: 242695f6122a ("vhost: allocate and free packets in bulk in Tx split")
Cc: sta...@dpdk.org
Signed-off-by: Maxime Coquelin
Signed-off-by: David Marchand
---
Changes in v2:
==
- Fix descriptors leak (David)
- Rebased on top of next-virtio
---
lib/vhost/virtio_net.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
Applied to next-virtio tree.
Thanks,
Maxime
Re: [PATCH v2 2/2] vhost: add new mbuf allocation failure statistic
On 1/31/24 20:53, Maxime Coquelin wrote: This patch introduces a new, per virtqueue, mbuf allocation failure statistic. It can be useful to troubleshoot packets drops due to insufficient mempool size or memory leaks. Signed-off-by: Maxime Coquelin --- lib/vhost/vhost.c | 1 + lib/vhost/vhost.h | 1 + lib/vhost/virtio_net.c | 17 + 3 files changed, 15 insertions(+), 4 deletions(-) Applied to next-virtio tree. Thanks, Maxime
Re: [PATCH v3] doc: update guideline for fix commit messages
On 2/1/2024 1:48 PM, Sivaramakrishnan Venkat wrote:
> Maintainers remove the Cc author line when merging the patch.
> So, the guidelines is updated with a suggestion for the placement
> of Cc lines in a commit message for easy merging.
>
> Signed-off-by: Sivaramakrishnan Venkat
> ---
> v3:
> - Other samples updated to the desired format for the "Cc:" line in the
> commit message
> v2:
> - Samples updated to the desired format for the "Cc:" line in the commit
> message
> ---
> doc/guides/contributing/patches.rst | 12
> doc/guides/contributing/stable.rst | 3 ++-
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/doc/guides/contributing/patches.rst
> b/doc/guides/contributing/patches.rst
> index e286d9e6d5..4e1768025e 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -271,9 +271,10 @@ Here are some guidelines for the body of a commit
> message:
> Update the docs, fixing description of some parameter.
>
> Fixes: abcdefgh1234 ("doc: add some parameter")
> - Cc: aut...@example.com
>
> Signed-off-by: Alex Smith
> + ---
> + Cc: aut...@example.com
>
> * When fixing an error or warning it is useful to add the error message and
> instructions on how to reproduce it.
>
> @@ -300,9 +301,10 @@ in the body of the commit message. For example::
>
> Coverity issue: 12345
> Fixes: abcdefgh1234 ("doc: add some parameter")
> - Cc: aut...@example.com
>
> Signed-off-by: Alex Smith
> + ---
> + Cc: aut...@example.com
>
>
> `Bugzilla `_
> @@ -319,9 +321,10 @@ For example::
>
> Bugzilla ID: 12345
> Fixes: abcdefgh1234 ("doc: add some parameter")
> -Cc: aut...@example.com
>
> Signed-off-by: Alex Smith
> +---
> +Cc: aut...@example.com
>
> Patch for Stable Releases
> ~
> @@ -336,9 +339,10 @@ In the commit message body the Cc: sta...@dpdk.org
> should be inserted as follows
> Update the docs, fixing description of some parameter.
>
> Fixes: abcdefgh1234 ("doc: add some parameter")
> - Cc: sta...@dpdk.org
>
> Signed-off-by: Alex Smith
> + ---
> + Cc: sta...@dpdk.org
>
We want to keep "Cc: sta...@dpdk.org" in the commit log, so above it wrong.
Please only update "Cc: aut...@example.com" cases.
Re: [PATCH v5 1/5] doc: fix fpga 5gnr configuration values
On 1/23/24 17:54, Hernan Vargas wrote:
flr_timeout was removed from the code a while ago, updating doc.
Fix minor typo in 5GNR example.
Fixes: 2d4306438c92 ("baseband/fpga_5gnr_fec: add configure function")
Cc: sta...@dpdk.org
Signed-off-by: Hernan Vargas
---
doc/guides/bbdevs/fpga_5gnr_fec.rst | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/doc/guides/bbdevs/fpga_5gnr_fec.rst
b/doc/guides/bbdevs/fpga_5gnr_fec.rst
index 956dd6bed560..99fc936829a8 100644
--- a/doc/guides/bbdevs/fpga_5gnr_fec.rst
+++ b/doc/guides/bbdevs/fpga_5gnr_fec.rst
@@ -100,7 +100,6 @@ parameters defined in ``rte_fpga_5gnr_fec_conf`` structure:
uint8_t dl_bandwidth;
uint8_t ul_load_balance;
uint8_t dl_load_balance;
- uint16_t flr_time_out;
};
- ``pf_mode_en``: identifies whether only PF is to be used, or the VFs. PF and
@@ -126,10 +125,6 @@ parameters defined in ``rte_fpga_5gnr_fec_conf`` structure:
If all hardware queues exceeds the watermark, no code blocks will be
streamed in from UL/DL code block FIFO.
-- ``flr_time_out``: specifies how many 16.384us to be FLR time out. The
- time_out = flr_time_out x 16.384us. For instance, if you want to set 10ms for
- the FLR time out then set this setting to 0x262=610.
-
An example configuration code calling the function ``rte_fpga_5gnr_fec_configure()`` is shown
below:
@@ -154,7 +149,7 @@ below:
/* setup FPGA PF */
ret = rte_fpga_5gnr_fec_configure(info->dev_name, &conf);
TEST_ASSERT_SUCCESS(ret,
- "Failed to configure 4G FPGA PF for bbdev %s",
+ "Failed to configure 5GNR FPGA PF for bbdev %s",
info->dev_name);
Reviewed-by: Maxime Coquelin
Thanks,
Maxime
Re: [PATCH v2 0/3] enhance NFP service framework
On 2/2/2024 3:04 AM, Chaoyong He wrote: > Make multiple devices can use single core to run services for > flower firmware. > Also add synchronize module and service module to support it. > > --- > v2: > * Merge the last commit of v1 into the second commit. > Ask was to move the fix as first patch to make is backportable. But by merging last (fix) commit into another one, you are completely loosing the capability to backport it to LTS releases. I just want to double check if this is intentionally and you don't want to backport a fix? > --- > > Long Wu (3): > net/nfp: add synchronize module > net/nfp: create new service code related files > net/nfp: flower driver uses one service core > > drivers/net/nfp/flower/nfp_flower.c | 56 +-- > drivers/net/nfp/flower/nfp_flower_ctrl.c | 20 +- > drivers/net/nfp/flower/nfp_flower_ctrl.h | 2 +- > .../net/nfp/flower/nfp_flower_representor.c | 7 + > drivers/net/nfp/flower/nfp_flower_service.c | 196 + > drivers/net/nfp/flower/nfp_flower_service.h | 17 + > drivers/net/nfp/meson.build | 3 + > drivers/net/nfp/nfp_cpp_bridge.c | 91 + > drivers/net/nfp/nfp_cpp_bridge.h | 1 - > drivers/net/nfp/nfp_ethdev.c | 31 +- > drivers/net/nfp/nfp_net_common.h | 15 +- > drivers/net/nfp/nfp_service.c | 117 ++ > drivers/net/nfp/nfp_service.h | 20 + > drivers/net/nfp/nfpcore/nfp_sync.c| 382 ++ > drivers/net/nfp/nfpcore/nfp_sync.h| 31 ++ > 15 files changed, 854 insertions(+), 135 deletions(-) > create mode 100644 drivers/net/nfp/flower/nfp_flower_service.c > create mode 100644 drivers/net/nfp/flower/nfp_flower_service.h > create mode 100644 drivers/net/nfp/nfp_service.c > create mode 100644 drivers/net/nfp/nfp_service.h > create mode 100644 drivers/net/nfp/nfpcore/nfp_sync.c > create mode 100644 drivers/net/nfp/nfpcore/nfp_sync.h >
Re: [PATCH v5 4/5] baseband/fpga_5gnr_fec: add AGX100 support
On 1/23/24 17:54, Hernan Vargas wrote:
Add support for new FPGA variant AGX100 (on Arrow Creek N6000).
Signed-off-by: Hernan Vargas
---
doc/guides/bbdevs/fpga_5gnr_fec.rst | 69 +-
drivers/baseband/fpga_5gnr_fec/agx100_pmd.h | 273
.../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h| 12 +-
.../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 1230 +++--
drivers/baseband/fpga_5gnr_fec/vc_5gnr_pmd.h |1 -
5 files changed, 1458 insertions(+), 127 deletions(-)
create mode 100644 drivers/baseband/fpga_5gnr_fec/agx100_pmd.h
...
+#endif /* _AGX100_H_ */
diff --git a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
index 982e956dc819..224684902569 100644
--- a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
+++ b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
@@ -8,6 +8,7 @@
#include
#include
+#include "agx100_pmd.h"
#include "vc_5gnr_pmd.h"
/* Helper macro for logging */
@@ -131,12 +132,21 @@ struct fpga_5gnr_fec_device {
uint64_t q_assigned_bit_map;
/** True if this is a PF FPGA 5GNR device. */
bool pf_device;
+ /** Maximum number of possible queues for this device. */
+ uint8_t total_num_queues;
You missed below comment on v4 review:
"
Introduction of total_num_queues should be in a dedicated patch as a
preliminary rework.
"
+ /** FPGA Variant. VC_5GNR_FPGA_VARIANT = 0; AGX100_FPGA_VARIANT = 1. */
+ uint8_t fpga_variant;
};
Re: [PATCH v5 5/5] baseband/fpga_5gnr_fec: cosmetic comment changes
On 1/23/24 17:54, Hernan Vargas wrote: Cosmetic changes for comments. No functional impact. Signed-off-by: Hernan Vargas --- drivers/baseband/fpga_5gnr_fec/agx100_pmd.h | 4 +- .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h| 49 ++-- .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 258 +- .../fpga_5gnr_fec/rte_pmd_fpga_5gnr_fec.h | 16 +- 4 files changed, 162 insertions(+), 165 deletions(-) Reviewed-by: Maxime Coquelin Thanks, Maxime
Re: PMD for non PCI device
On Tue, 6 Feb 2024 at 19:43, Bruce Richardson wrote: > > On Tue, Feb 06, 2024 at 07:36:16PM +0530, Prashant Upadhyaya wrote: > > Hi, > > > > I have a usecase where I have to evaluate writing a DPDK PMD for a non > > PCI/e device doing the ethernet packet i/o. > > > > Wanted to know if the above usecase is supported by DPDK infra and any > > pointers on how one should go about writing a PMD for such a usecase > > if supported. Would appreciate any inputs. > > > Hi, > > yes, such a usecase is supported, but the specifics of how to go about it > will vary depending on the type of PMD it is. DPDK already supports a range > of other types of PMD, for emulated, or SW backed PMDs, e.g. net/pcap > driver, and drivers for various SoCs which don't use PCI. For the case > where the PMD is backed by real hardware (or an emulated device that > appears to a VM as a piece of hardware), you may want to consider writing a > "bus" driver for DPDK to support probing of the device. For non-HW > devices, the "vdev" bus may be what you want to use, where probing is not > done and devices are created in response to cmdline arguments on init, or > via C APIs later in the app. > > Regards, > /Bruce Thanks Bruce, this is helpful. Is there any PMD in DPDK code that you can refer me to for any SoC which does not use PCI (the usecase of backing by real hardware), that would be great to follow. Regards -Prashant
Re: PMD for non PCI device
On Tue, Feb 6, 2024 at 9:20 PM Prashant Upadhyaya wrote: > > On Tue, 6 Feb 2024 at 19:43, Bruce Richardson > wrote: > > > > On Tue, Feb 06, 2024 at 07:36:16PM +0530, Prashant Upadhyaya wrote: > > > Hi, > > > > > > I have a usecase where I have to evaluate writing a DPDK PMD for a non > > > PCI/e device doing the ethernet packet i/o. > > > > > > Wanted to know if the above usecase is supported by DPDK infra and any > > > pointers on how one should go about writing a PMD for such a usecase > > > if supported. Would appreciate any inputs. > > > > > Hi, > > > > yes, such a usecase is supported, but the specifics of how to go about it > > will vary depending on the type of PMD it is. DPDK already supports a range > > of other types of PMD, for emulated, or SW backed PMDs, e.g. net/pcap > > driver, and drivers for various SoCs which don't use PCI. For the case > > where the PMD is backed by real hardware (or an emulated device that > > appears to a VM as a piece of hardware), you may want to consider writing a > > "bus" driver for DPDK to support probing of the device. For non-HW > > devices, the "vdev" bus may be what you want to use, where probing is not > > done and devices are created in response to cmdline arguments on init, or > > via C APIs later in the app. > > > > Regards, > > /Bruce > > Thanks Bruce, this is helpful. Is there any PMD in DPDK code that you > can refer me to for any SoC which does not use PCI (the usecase of > backing by real hardware), that would be great to follow. See drivers/bus/* Based on your description, vfio-platorm will be the closest match(driver/bus/platform) > > Regards > -Prashant
rte_malloc() and alignment
The rte_malloc() API documentation has the following to say about the align parameter: "If 0, the return is a pointer that is suitably aligned for any kind of variable (in the same manner as malloc()). Otherwise, the return is a pointer that is a multiple of align. In this case, it must be a power of two. (Minimum alignment is the cacheline size, i.e. 64-bytes)" After reading this, one might be left with the impression that the parenthesis refers to only the "otherwise" (non-zero-align) case, since surely, cache line alignment should be sufficient for any kind of variable and it semantics would be "in the same manner as malloc()". However, in the actual RTE malloc implementation, any align parameter value less than RTE_CACHE_LINE_SIZE results in an alignment of RTE_CACHE_LINE_SIZE, unless I'm missing something. Is there any conceivable scenario where passing a non-zero align parameter is useful? Would it be an improvement to rephrase the documentation to: "The alignment of the allocated memory meets all of the following criteria: 1) able to hold any built-in type. 2) be at least as large as the align parameter. 3) be at least as large as RTE_CACHE_LINE_SIZE. The align parameter must be a power-of-2 or 0. " ...so it actually describes what is implemented? And also adds the theoretical (?) case of a built-in type requiring > RTE_CACHE_LINE_SIZE amount of alignment.
[PATCH v3] ethdev: fast path async flow API
This patch reworks the async flow API functions called in data path,
to reduce the overhead during flow operations at the library level.
Main source of the overhead was indirection and checks done while
ethdev library was fetching rte_flow_ops from a given driver.
This patch introduces rte_flow_fp_ops struct which holds callbacks
to driver's implementation of fast path async flow API functions.
Each driver implementing these functions must populate flow_fp_ops
field inside rte_eth_dev structure with a reference to
its own implementation.
By default, ethdev library provides dummy callbacks with
implementations returning ENOSYS.
Such design provides a few assumptions:
- rte_flow_fp_ops struct for given port is always available.
- Each callback is either:
- Default provided by library.
- Set up by driver.
As a result, no checks for availability of the implementation
are needed at library level in data path.
Any library-level validation checks in async flow API are compiled
if and only if RTE_FLOW_DEBUG macro is defined.
This design was based on changes in ethdev library introduced in [1].
These changes apply only to the following API functions:
- rte_flow_async_create()
- rte_flow_async_create_by_index()
- rte_flow_async_actions_update()
- rte_flow_async_destroy()
- rte_flow_push()
- rte_flow_pull()
- rte_flow_async_action_handle_create()
- rte_flow_async_action_handle_destroy()
- rte_flow_async_action_handle_update()
- rte_flow_async_action_handle_query()
- rte_flow_async_action_handle_query_update()
- rte_flow_async_action_list_handle_create()
- rte_flow_async_action_list_handle_destroy()
- rte_flow_async_action_list_handle_query_update()
This patch also adjusts the mlx5 PMD to the introduced flow API changes.
[1]
commit c87d435a4d79 ("ethdev: copy fast-path API into separate structure")
Signed-off-by: Dariusz Sosnowski
Acked-by: Ori Kam
---
v3:
- Documented RTE_FLOW_DEBUG build option.
- Enabled RTE_FLOW_DEBUG automatically on debug builds.
- Fixed pointer checks to compare against NULL explicitly.
v2:
- Fixed mlx5 PMD build issue with older versions of rdma-core.
---
doc/guides/nics/build_and_test.rst | 9 +-
doc/guides/rel_notes/release_24_03.rst | 37 ++
drivers/net/mlx5/mlx5_flow.c | 608 +
drivers/net/mlx5/mlx5_flow_hw.c| 25 +
lib/ethdev/ethdev_driver.c | 4 +
lib/ethdev/ethdev_driver.h | 4 +
lib/ethdev/meson.build | 4 +
lib/ethdev/rte_flow.c | 519 -
lib/ethdev/rte_flow_driver.h | 277 ++-
lib/ethdev/version.map | 2 +
10 files changed, 647 insertions(+), 842 deletions(-)
diff --git a/doc/guides/nics/build_and_test.rst
b/doc/guides/nics/build_and_test.rst
index e8b29c2277..453fa74b39 100644
--- a/doc/guides/nics/build_and_test.rst
+++ b/doc/guides/nics/build_and_test.rst
@@ -36,11 +36,16 @@ The ethdev layer supports below build options for debug
purpose:
Build with debug code on Tx path.
+- ``RTE_FLOW_DEBUG`` (default **disabled**; enabled automatically on debug
builds)
+
+ Build with debug code in asynchronous flow APIs.
+
.. Note::
- The ethdev library use above options to wrap debug code to trace invalid
parameters
+ The ethdev library uses above options to wrap debug code to trace invalid
parameters
on data path APIs, so performance downgrade is expected when enabling those
options.
- Each PMD can decide to reuse them to wrap their own debug code in the Rx/Tx
path.
+ Each PMD can decide to reuse them to wrap their own debug code in the Rx/Tx
path
+ and in asynchronous flow APIs implementation.
Running testpmd in Linux
diff --git a/doc/guides/rel_notes/release_24_03.rst
b/doc/guides/rel_notes/release_24_03.rst
index 6f8ad27808..b62330b8b1 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -86,6 +86,43 @@ API Changes
* gso: ``rte_gso_segment`` now returns -ENOTSUP for unknown protocols.
+* ethdev: PMDs implementing asynchronous flow operations are required to
provide relevant functions
+ implementation through ``rte_flow_fp_ops`` struct, instead of
``rte_flow_ops`` struct.
+ Pointer to device-dependent ``rte_flow_fp_ops`` should be provided to
``rte_eth_dev.flow_fp_ops``.
+ This change applies to the following API functions:
+
+ * ``rte_flow_async_create``
+ * ``rte_flow_async_create_by_index``
+ * ``rte_flow_async_actions_update``
+ * ``rte_flow_async_destroy``
+ * ``rte_flow_push``
+ * ``rte_flow_pull``
+ * ``rte_flow_async_action_handle_create``
+ * ``rte_flow_async_action_handle_destroy``
+ * ``rte_flow_async_action_handle_update``
+ * ``rte_flow_async_action_handle_query``
+ * ``rte_flow_async_action_handle_query_update``
+ * ``rte_flow_async_action_list_handle_create``
+ * ``rte_flow_async_action_list_handle_destroy``
+ * ``rte_flow_async_action_list_handle_qu
RE: [PATCH v2] ethdev: fast path async flow API
> -Original Message- > From: Thomas Monjalon > Sent: Monday, February 5, 2024 15:03 > To: Dariusz Sosnowski > Cc: Slava Ovsiienko ; Ori Kam ; > Suanming Mou ; Matan Azrad > ; Ferruh Yigit ; Andrew > Rybchenko ; dev@dpdk.org > Subject: Re: [PATCH v2] ethdev: fast path async flow API > > External email: Use caution opening links or attachments > > > 05/02/2024 14:14, Dariusz Sosnowski: > > From: Thomas Monjalon > > > 31/01/2024 10:35, Dariusz Sosnowski: > > > > As a result, no checks for availability of the implementation are > > > > needed at library level in data path. > > > > Any library-level validation checks in async flow API are compiled > > > > if and only if RTE_FLOW_DEBUG macro is defined. > > > > > > How are we supposed to enable RTE_FLOW_DEBUG? > > > > I should document it, but the idea was that it must be explicitly > > enabled during build, by adding -c_args=-DRTE_FLOW_DEBUG to meson > options. > > > > Do you think doc/guides/nics/build_and_test.rst is a good place to > document this option? > > Yes > > > It would be documented alongside RTE_ETHDEV_DEBUG_RX and > RTE_ETHDEV_DEBUG_TX. > > > > > May it be enabled automatically if other debug option is globally enabled? > > > > Do you mean that if buildtype is defined as debug, then RTE_FLOW_DEBUG > is defined automatically? > > Yes > > > I think that's a good idea. > > Another way of enabling it is to check > #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG I think that defining RTE_FLOW_DEBUG based on buildtype would be more appropriate, since the code under RTE_FLOW_DEBUG is not responsible for any additional logging or tracing. It rather serves as basic checks for all async flow API functions. Best regards, Dariusz Sosnowski
Re: [Patch v4] net/mana: use rte_pktmbuf_alloc_bulk for allocating RX mbufs
On 2/2/2024 1:19 AM, lon...@linuxonhyperv.com wrote:
> From: Long Li
>
> Instead of allocating mbufs one by one during RX, use
> rte_pktmbuf_alloc_bulk() to allocate them in a batch.
>
> With this patch, there are no measurable performance improvements in
> benchmarks. However, this patch should improve CPU cycles and reduce
> potential locking conflicts in real-world applications.
>
> Signed-off-by: Long Li
<...>
> @@ -120,20 +113,33 @@ mana_alloc_and_post_rx_wqe(struct mana_rxq *rxq)
> /*
> * Post work requests for a Rx queue.
> */
> +#define MANA_MBUF_BULK 32u
> static int
> -mana_alloc_and_post_rx_wqes(struct mana_rxq *rxq)
> +mana_alloc_and_post_rx_wqes(struct mana_rxq *rxq, uint32_t count)
> {
> int ret;
> - uint32_t i;
> + uint32_t i, batch_count;
> + struct rte_mbuf *mbufs[MANA_MBUF_BULK];
> +
> +more_mbufs:
> + batch_count = RTE_MIN(count, MANA_MBUF_BULK);
> + ret = rte_pktmbuf_alloc_bulk(rxq->mp, mbufs, batch_count);
> + if (ret) {
> + DP_LOG(ERR, "failed to allocate mbufs for RX");
> + rxq->stats.nombuf += count;
> +
> + /* Bail out to ring doorbell for posted packets */
> + goto out;
> + }
>
> #ifdef RTE_ARCH_32
> rxq->wqe_cnt_to_short_db = 0;
> #endif
> - for (i = 0; i < rxq->num_desc; i++) {
> - ret = mana_alloc_and_post_rx_wqe(rxq);
> + for (i = 0; i < batch_count; i++) {
> + ret = mana_post_rx_wqe(rxq, mbufs[i]);
> if (ret) {
> DP_LOG(ERR, "failed to post RX ret = %d", ret);
> - return ret;
> + break;
>
Hi Long,
Assume that if "count > MANA_MBUF_BULK", and int the first iteration of
the loop 'mana_post_rx_wqe()' failed, but in second iteration it is
successful, this will cause function to return success in spite of
failure in first iteration.
As mbufs posted Rx queue, it may be OK to consider above case as
success, but since 'count' number not posted this may be misleading.
I just want to double check if this is done intentionally.
With the limitation of VLA code become more complex, and if there is no
performance benefit of the allocating 'mbufs' array from stack, you may
prefer to switch back to allocating dynamic memory, up to you.
Re: [PATCH] examples/ipsec-secgw: fix IPsec performance drop
On 2/6/2024 12:38 PM, Rahul Bhansali wrote:
> Single packet free using rte_pktmbuf_free_bulk() is dropping the
> performance. On cn10k, maximum of ~4% drop observed for IPsec
> event mode single SA outbound case.
>
> To fix this issue, single packet free will use rte_pktmbuf_free
> API.
>
> Fixes: bd7c063561b3 ("examples/ipsec-secgw: use bulk free")
>
> Signed-off-by: Rahul Bhansali
> ---
> examples/ipsec-secgw/ipsec-secgw.h | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.h
> b/examples/ipsec-secgw/ipsec-secgw.h
> index 8baab44ee7..ec33a982df 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.h
> +++ b/examples/ipsec-secgw/ipsec-secgw.h
> @@ -229,11 +229,10 @@ free_reassembly_fail_pkt(struct rte_mbuf *mb)
> }
>
> /* helper routine to free bulk of packets */
> -static inline void
> -free_pkts(struct rte_mbuf *mb[], uint32_t n)
> +static __rte_always_inline void
> +free_pkts(struct rte_mbuf *mb[], const uint32_t n)
> {
> - rte_pktmbuf_free_bulk(mb, n);
> -
> + n == 1 ? rte_pktmbuf_free(mb[0]) : rte_pktmbuf_free_bulk(mb, n);
> core_stats_update_drop(n);
> }
>
Hi Rahul,
Do you think the 'rte_pktmbuf_free_bulk()' API performance can be
improved by similar change?
Re: [PATCH] ethdev: recommend against using locks in event callbacks
On 2/1/2024 8:43 AM, David Marchand wrote:
> As described in a recent bugzilla opened against the net/iavf driver,
> a driver may call a event callback from other calls of the ethdev API.
>
> Nothing guarantees in the ethdev API against such behavior.
>
> Add a notice against using locks in those callbacks.
>
> Bugzilla ID: 1337
>
> Signed-off-by: David Marchand
> ---
> lib/ethdev/rte_ethdev.h | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 21e3a21903..5c6b104fb4 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -4090,7 +4090,19 @@ enum rte_eth_event_type {
> RTE_ETH_EVENT_MAX /**< max value of this enum */
> };
>
> -/** User application callback to be registered for interrupts. */
> +/**
> + * User application callback to be registered for interrupts.
> + *
> + * Note: there is no guarantee in the DPDK drivers that a callback won't be
> + * called in the middle of other parts of the ethdev API. For example,
> + * imagine that thread A calls rte_eth_dev_start() and as part of this
> + * call, a RTE_ETH_EVENT_INTR_RESET event gets generated and the
> + * associated callback is ran on thread A. In that example, if the
> + * application protects its internal data using locks before calling
> + * rte_eth_dev_start(), and the callback takes a same lock, a deadlock
> + * occurs. Because of this, it is highly recommended NOT to take locks
> in
> + * those callbacks.
> + */
> typedef int (*rte_eth_dev_cb_fn)(uint16_t port_id,
> enum rte_eth_event_type event, void *cb_arg, void *ret_param);
>
Acked-by: Ferruh Yigit
Re: [PATCH] ethdev: recommend against using locks in event callbacks
On 2/1/2024 10:08 AM, Kevin Traynor wrote:
> On 01/02/2024 08:43, David Marchand wrote:
>> As described in a recent bugzilla opened against the net/iavf driver,
>> a driver may call a event callback from other calls of the ethdev API.
>>
>> Nothing guarantees in the ethdev API against such behavior.
>>
>> Add a notice against using locks in those callbacks.
>>
>> Bugzilla ID: 1337
>>
>> Signed-off-by: David Marchand
>> ---
>> lib/ethdev/rte_ethdev.h | 14 +-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 21e3a21903..5c6b104fb4 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -4090,7 +4090,19 @@ enum rte_eth_event_type {
>> RTE_ETH_EVENT_MAX /**< max value of this enum */
>> };
>>
>> -/** User application callback to be registered for interrupts. */
>> +/**
>> + * User application callback to be registered for interrupts.
>> + *
>> + * Note: there is no guarantee in the DPDK drivers that a callback won't be
>> + * called in the middle of other parts of the ethdev API. For example,
>> + * imagine that thread A calls rte_eth_dev_start() and as part of this
>> + * call, a RTE_ETH_EVENT_INTR_RESET event gets generated and the
>> + * associated callback is ran on thread A. In that example, if the
>> + * application protects its internal data using locks before calling
>> + * rte_eth_dev_start(), and the callback takes a same lock, a deadlock
>> + * occurs. Because of this, it is highly recommended NOT to take
>> locks in
>> + * those callbacks.
>> + */
>
> That is a good practical recommendation for an application developer.
>
> I wonder if it should taken further so that the API formally states the
> callback MUST be non-blocking?
>
Application still can manage the locks in a safe way, but needs to be
aware of above condition and possible deadlock.
I think above note is sufficient instead of forbidding locks in
callbacks completely.
Re: [PATCH v7 1/4] ethdev: rename action modify field data structure
On 2/6/2024 2:06 AM, Suanming Mou wrote: > Current rte_flow_action_modify_data struct describes the pkt > field perfectly and is used only in action. > > It is planned to be used for item as well. This commit renames > it to "rte_flow_field_data" making it compatible to be used by item. > > Signed-off-by: Suanming Mou > Acked-by: Ori Kam > Acked-by: Andrew Rybchenko > Acked-by: Ferruh Yigit
Re: [PATCH v7 2/4] ethdev: move flow field data structures
On 2/6/2024 2:06 AM, Suanming Mou wrote: > As flow field relevant data structures will be used by both actions and > items, this commit moves the relevant data structures up to item parts. > > Signed-off-by: Suanming Mou > Acked-by: Ferruh Yigit
Re: [PATCH v7 4/4] net/mlx5: add compare item support
On 2/6/2024 2:06 AM, Suanming Mou wrote: > @@ -80,6 +80,8 @@ New Features >* Added support for Atomic Rules' TK242 packet-capture family of devices > with PCI IDs: ``0x1024, 0x1025, 0x1026``. > > + * Added support for comparing result between packet fields or value. > + > Comment in v5 seems not addressed, above is still under wrong block. Adding "* **Updated NVIDIA mlx5 driver.**" while merging.
Re: [PATCH v7 0/4] ethdev: add RTE_FLOW_ITEM_TYPE_COMPARE
On 2/6/2024 2:06 AM, Suanming Mou wrote: > The new item type is added for the case user wants to match traffic > based on packet field compare result with other fields or immediate > value. > > e.g. take advantage the compare item user will be able to accumulate > a IPv4/TCP packet's TCP data_offset and IPv4 IHL field to a tag > register, then compare the tag register with IPv4 header total length > to understand the packet has payload or not. > > The supported operations can be as below: > - RTE_FLOW_ITEM_COMPARE_EQ (equal) > - RTE_FLOW_ITEM_COMPARE_NE (not equal) > - RTE_FLOW_ITEM_COMPARE_LT (less than) > - RTE_FLOW_ITEM_COMPARE_LE (less than or equal) > - RTE_FLOW_ITEM_COMPARE_GT (great than) > - RTE_FLOW_ITEM_COMPARE_GE (great than or equal) > > V7: > - Moved release notes to API. > - Optimize comment descriptions. > > V6: > - fix typo and style issue. > - adjust flow_field description. > > V5: > - rebase on top of next-net > - add sample detail for rte_flow_field. > > V4: > - rebase on top of the latest version. > - move ACTION_MODIFY_PATTERN_SIZE and modify_field_ids rename >to first patch. > - add comparison flow create sample in testpmd_funcs.rst. > > V3: > - fix code style missing empty line in rte_flow.rst. > - fix missing the ABI change release notes. > > V2: > - Since modify field data struct is experiment, rename modify >field data directly instead of adding new flow field struct. > > > Suanming Mou (4): > ethdev: rename action modify field data structure > ethdev: move flow field data structures > ethdev: add compare item > net/mlx5: add compare item support > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v7 3/4] ethdev: add compare item
On 2/6/2024 2:06 AM, Suanming Mou wrote: > The new item type is added for the case user wants to match traffic > based on packet field compare result with other fields or immediate > value. > > e.g. take advantage the compare item user will be able to accumulate > a IPv4/TCP packet's TCP data_offset and IPv4 IHL field to a tag > register, then compare the tag register with IPv4 header total length > to understand the packet has payload or not. > > The supported operations can be as below: > - RTE_FLOW_ITEM_COMPARE_EQ (equal) > - RTE_FLOW_ITEM_COMPARE_NE (not equal) > - RTE_FLOW_ITEM_COMPARE_LT (less than) > - RTE_FLOW_ITEM_COMPARE_LE (less than or equal) > - RTE_FLOW_ITEM_COMPARE_GT (great than) > - RTE_FLOW_ITEM_COMPARE_GE (great than or equal) > > A sample for create the comparison flow: > flow pattern_template 0 create ingress pattern_template_id 1 template \ > compare op mask le a_type mask tag a_tag_index mask 1 b_type \ > mask tag b_tag_index mask 2 width mask 0x / end > flow actions_template 0 create ingress actions_template_id 1 template \ > count / drop / end mask count / drop / end > flow template_table 0 create table_id 1 group 2 priority 1 ingress \ > rules_number 1 pattern_template 1 actions_template 1 > flow queue 0 create 0 template_table 1 pattern_template 0 \ > actions_template 0 postpone no pattern compare op is le \ > a_type is tag a_tag_index is 1 b_type is tag b_tag_index is 2 \ > width is 32 / end actions count / drop / end > > Signed-off-by: Suanming Mou > Acked-by: Ori Kam > Acked-by: Andrew Rybchenko > Acked-by: Ferruh Yigit
Re: [PATCH v3] ethdev: fast path async flow API
06/02/2024 18:36, Dariusz Sosnowski:
> --- a/doc/guides/nics/build_and_test.rst
> +++ b/doc/guides/nics/build_and_test.rst
> +- ``RTE_FLOW_DEBUG`` (default **disabled**; enabled automatically on debug
> builds)
> +
> + Build with debug code in asynchronous flow APIs.
> +
> .. Note::
>
> - The ethdev library use above options to wrap debug code to trace invalid
> parameters
> + The ethdev library uses above options to wrap debug code to trace invalid
> parameters
> on data path APIs, so performance downgrade is expected when enabling
> those options.
> - Each PMD can decide to reuse them to wrap their own debug code in the
> Rx/Tx path.
> + Each PMD can decide to reuse them to wrap their own debug code in the
> Rx/Tx path
> + and in asynchronous flow APIs implementation.
Good
> --- a/doc/guides/rel_notes/release_24_03.rst
> +++ b/doc/guides/rel_notes/release_24_03.rst
> +* ethdev: PMDs implementing asynchronous flow operations are required to
> provide relevant functions
> + implementation through ``rte_flow_fp_ops`` struct, instead of
> ``rte_flow_ops`` struct.
> + Pointer to device-dependent ``rte_flow_fp_ops`` should be provided to
> ``rte_eth_dev.flow_fp_ops``.
That's a change only for the driver.
If there is no change for the application, it should not appear in the release
notes.
BTW, API means Application Programming Interface :)
> + This change applies to the following API functions:
> +
> + * ``rte_flow_async_create``
> + * ``rte_flow_async_create_by_index``
> + * ``rte_flow_async_actions_update``
> + * ``rte_flow_async_destroy``
> + * ``rte_flow_push``
> + * ``rte_flow_pull``
> + * ``rte_flow_async_action_handle_create``
> + * ``rte_flow_async_action_handle_destroy``
> + * ``rte_flow_async_action_handle_update``
> + * ``rte_flow_async_action_handle_query``
> + * ``rte_flow_async_action_handle_query_update``
> + * ``rte_flow_async_action_list_handle_create``
> + * ``rte_flow_async_action_list_handle_destroy``
> + * ``rte_flow_async_action_list_handle_query_update``
> +
> +* ethdev: Removed the following fields from ``rte_flow_ops`` struct:
> +
> + * ``async_create``
> + * ``async_create_by_index``
> + * ``async_actions_update``
> + * ``async_destroy``
> + * ``push``
> + * ``pull``
> + * ``async_action_handle_create``
> + * ``async_action_handle_destroy``
> + * ``async_action_handle_update``
> + * ``async_action_handle_query``
> + * ``async_action_handle_query_update``
> + * ``async_action_list_handle_create``
> + * ``async_action_list_handle_destroy``
> + * ``async_action_list_handle_query_update``
[...]
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -71,6 +71,10 @@ struct rte_eth_dev {
> struct rte_eth_dev_data *data;
> void *process_private; /**< Pointer to per-process device data */
> const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
> + /**
> + * Fast path flow API functions exported by PMD.
> + */
This comment may be on one single line.
> + const struct rte_flow_fp_ops *flow_fp_ops;
> struct rte_device *device; /**< Backing device */
> struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
> --- a/lib/ethdev/meson.build
> +++ b/lib/ethdev/meson.build
> +if get_option('buildtype').contains('debug')
> +cflags += ['-DRTE_FLOW_DEBUG']
> +endif
This looks OK.
Acked-by: Thomas Monjalon
Re: [PATCH v2] ethdev: add template table resize API
31/01/2024 10:59, Gregory Etelson:
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> #define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG RTE_BIT32(1)
> +/**
> + * Specialize table for resize.
> + */
> +#define RTE_FLOW_TABLE_SPECIALIZE_RESIZABLE_TABLE RTE_BIT32(2)
I'm not sure about the repeating "TABLE" at the end of this flag name.
[...]
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Query if a table can be resized
s/if/whether/
A dot is missing.
> + *
> + * @param port_id
> + *Port identifier of Ethernet device.
> + * @param tbl_attr
> + *Template table
Every lines should end with a dot in Doxygen for consistency.
> + *
> + * @return
> + * True if the table can be resized.
> + */
> +static __rte_always_inline bool
Why is it inline?
In general we avoid inline except for few performance sensitive ones.
> +rte_flow_table_resizable(__rte_unused uint16_t port_id,
> + const struct rte_flow_template_table_attr *tbl_attr)
> +{
> + return (tbl_attr->specialize &
> + RTE_FLOW_TABLE_SPECIALIZE_RESIZABLE_TABLE) != 0;
> +}
[...]
> + # added in 24.03
> + rte_flow_template_table_resize;
> + rte_flow_async_update_resized;
> + rte_flow_template_table_resize_complete;
I like the idea of these 3 new functions.
The scenario should be described in doc/guides/prog_guide/rte_flow.rst
Re: [PATCH 1/4] ethdev: introduce encap hash calculation
28/01/2024 10:39, Ori Kam: > During the encapsulation of a packet, it is expected to calculate the > hash value which is based on the original packet (the outer values, > which will become the inner values). It is not clear what the hash is for. > The tunnel protocol defines which tunnel field should hold this hash, > but it doesn't define the hash calculation algorithm. If the hash is stored in the packet header, I expect it to be reproducible when being checked. How the algorithm may be undefined? > An application that uses flow offloads gets the first few packets > and then decides to offload the flow. As a result, there are two > different paths that a packet from a given flow may take. > SW for the first few packets or HW for the rest. > When the packet goes through the SW, the SW encapsulates the packet > and must use the same hash calculation as the HW will do for > the rest of the packets in this flow. > > This patch gives the SW a way to query the hash value > for a given packet as if the packet was passed through the HW. > > Signed-off-by: Ori Kam > --- > +Calculate encap hash > + > + > +Calculating hash of a packet in SW as it would be calculated in HW for the > encap action We should give the real full name of the flow action. > + > +When the HW execute an encapsulation action, it may calculate an hash value > which is based > +on the original packet. This hash is stored depending on the encapsulation > protocol, in one > +of the outer fields. Give an example of such encapsulation protocol? > +This function allows the application to calculate the hash for a given > packet as if the > +encapsulation was done in HW. > + > +.. code-block:: c > + > + int > + rte_flow_calc_encap_hash(uint16_t port_id, > +const struct rte_flow_item pattern[], > +enum rte_flow_encap_hash_field > dest_field, > +uint8_t hash_len, > +uint8_t *hash, > +struct rte_flow_error *error); I don't think we should add the complete prototype in this guide. [...] > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Simulates HW hash calculation that is done when encap action is being > used. s/Simulates/Simulate/ > + * > + * @param[in] port_id > + * Port identifier of Ethernet device. > + * @param[in] pattern > + * The values to be used in the hash calculation. > + * @param[in] dest_field > + * Type of destination field for hash calculation. > + * @param[in] hash_len > + * The length of the hash pointer in bytes. Should be according to > encap_hash_field. > + * @param[out] hash > + * Used to return the calculated hash. It will be written in network order, > + * so hash[0] is the MSB. > + * The number of bytes is based on the destination field type. > + * @param[out] error > + * Perform verbose error reporting if not NULL. > + * PMDs initialize this structure in case of error only. > + * > + * @return > + * - (0) if success. > + * - (-ENODEV) if *port_id* invalid. > + * - (-ENOTSUP) if underlying device does not support this functionality. > + * - (-EINVAL) if *pattern* doesn't hold enough information to calculate > the hash > + * or the dest is not supported. > + */ > +__rte_experimental > +int > +rte_flow_calc_encap_hash(uint16_t port_id, const struct rte_flow_item > pattern[], > + enum rte_flow_encap_hash_field dest_field, uint8_t > hash_len, > + uint8_t *hash, struct rte_flow_error *error);
Re: [RFC V1 1/1] net: extend VXLAN header to support more extensions
30/01/2024 12:25, Gavin Li:
> In this patch, all the VXLAN extension header will be merged with VXLAN as
> union if the overlapped field has different format among protocols. The
> existing VXLAN-GPE will be marked as deprecated and new extensions of
> VXLAN should be added to VXLAN instead of a new RTE item.
So VXLAN GPE, GBP, and original ones will all use the same struct.
Asking confirmation to other reviewers:
- do we want to deprecate specific VXLAN GPE?
- do we want to plan for VXLAN GPE removal?
[...]
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with
> ``RTE_FLOW_ITEM_TYPE_VXLAN``.
> + The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct
> ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
> + and the header struct ``rte_vxlan_gpe_hdr`` with the macro
> ``RTE_ETHER_VXLAN_GPE_HLEN``
> + will be removed in DPDK 25.11.
[...]
> @@ -38,8 +38,65 @@ struct rte_vxlan_hdr {
> rte_be32_t vx_vni; /**< VNI (24) + Reserved (8). */
> };
> struct {
> - uint8_tflags;/**< Should be 8 (I flag). */
> - uint8_trsvd0[3]; /**< Reserved. */
> + union {
> + uint8_tflags;/**< Should be 8 (I flag).
> */
> + /* Flag bits defined by GPE */
> + struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> + uint8_t flag_o:1,
> + flag_b:1,
> + flag_p:1,
> + flag_i_gpe:1,
> + flag_ver:2,
> + rsvd_gpe:2;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> + uint8_t rsvd_gpe:2,
> + flag_ver:2,
> + flag_i_gpe:1,
> + flag_p:1,
> + flag_b:1,
> + flag_o:1;
> +#endif
> + };
> + /* Flag bits defined by GBP */
> + struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> + uint8_t rsvd_gbp1:3,
> + flag_i_gbp:1,
> + rsvd_gbp2:3,
> + flag_g:1;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> + uint8_t flag_g:1,
> + rsvd_gbp1:3,
> + flag_i_gbp:1,
> + rsvd_gbp2:3;
> +#endif
> + };
> + };
> + union {
> + uint8_trsvd0[3]; /**< Reserved. */
> + /* Overlap with rte_vxlan_gpe_hdr which is
> deprecated.*/
> + struct {
> + uint8_t rsvd0_gpe[2]; /**< Reserved. */
> + uint8_t proto; /**< Next protocol.
> */
> + };
> + struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> + uint8_t rsvd0_gbp1:3,
> + policy_applied:1,
> + rsvd0_gbp2:2,
> + dont_learn:1,
> + rsvd0_gbp3:1;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> + uint8_t rsvd0_gbp1:1,
> + dont_learn:1,
> + rsvd0_gbp2:2,
> + policy_applied:1,
> + rsvd0_gbp3:3;
> +#endif
> + uint16_t policy_id;
> + };
> + };
> uint8_tvni[3]; /**< VXLAN identifier. */
> uint8_trsvd1;/**< Reserved. */
> };
Naming looks OK.
Any different opinion?
Re: [PATCH v2] app/testpmd: command to get descriptor used count
On 2/1/2024 1:52 PM, skotesh...@marvell.com wrote:
> From: Satha Rao
>
> Existing Rx desc used count command extended to get Tx queue
> used count.
> testpmd> show port 0 rxq 0 desc used count
> testpmd> show port 0 txq 0 desc used count
>
> Signed-off-by: Satha Rao
> ---
> Depends-on: series-30833 ("ethdev: support Tx queue used count")
>
> v2:
> extended rx_queue_desc_used_count command to support Tx
> updated testpmd_app_ug with new command
>
> app/test-pmd/cmdline.c | 184 +++-
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 +-
> 2 files changed, 104 insertions(+), 90 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index f704319771..c8c88f3236 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -12638,6 +12638,104 @@ static cmdline_parse_inst_t
> cmd_show_port_supported_ptypes = {
> },
> };
>
> +/* *** display rx/tx queue descriptor used count *** */
> +struct cmd_show_rx_tx_queue_desc_used_count_result {
> + cmdline_fixed_string_t cmd_show;
> + cmdline_fixed_string_t cmd_port;
> + cmdline_fixed_string_t cmd_dir;
> + cmdline_fixed_string_t cmd_desc;
> + cmdline_fixed_string_t cmd_used;
> + cmdline_fixed_string_t cmd_count;
> + portid_t cmd_pid;
> + portid_t cmd_qid;
> +};
> +
> +static void
> +cmd_show_rx_tx_queue_desc_used_count_parsed(void *parsed_result,
> __rte_unused struct cmdline *cl,
> + __rte_unused void *data)
> +{
> + struct cmd_show_rx_tx_queue_desc_used_count_result *res = parsed_result;
> + int rc;
> +
> + if (!strcmp(res->cmd_dir, "rxq")) {
> + if (rte_eth_rx_queue_is_valid(res->cmd_pid, res->cmd_qid) != 0)
> {
> + fprintf(stderr, "Invalid input: port id = %d, queue id
> = %d\n",
> + res->cmd_pid, res->cmd_qid);
> + return;
> + }
> +
> + rc = rte_eth_rx_queue_count(res->cmd_pid, res->cmd_qid);
> + if (rc < 0) {
> + fprintf(stderr, "Invalid queueid = %d\n", res->cmd_qid);
>
Can you please unify the error message for Rx and Tx?
> + return;
> + }
> + printf("RxQ %d used desc count = %d\n", res->cmd_qid, rc);
> + } else if (!strcmp(res->cmd_dir, "txq")) {
> + if (rte_eth_tx_queue_is_valid(res->cmd_pid, res->cmd_qid) != 0)
> {
> + fprintf(stderr, "Invalid input: port id = %d, queue id
> = %d\n",
> + res->cmd_pid, res->cmd_qid);
> + return;
> + }
> +
> + rc = rte_eth_tx_queue_count(res->cmd_pid, res->cmd_qid);
> + if (rc < 0) {
> + fprintf(stderr, "Tx queue count get failed rc=%d
> queue_id=%d\n", rc,
> + res->cmd_qid);
> + return;
> + }
> + printf("TxQ %d used desc count = %d\n", res->cmd_qid, rc);
> + }
> +}
> +
> +static cmdline_parse_token_string_t
> cmd_show_rx_tx_queue_desc_used_count_show =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_show, "show");
> +static cmdline_parse_token_string_t
> cmd_show_rx_tx_queue_desc_used_count_port =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_port, "port");
> +static cmdline_parse_token_num_t cmd_show_rx_tx_queue_desc_used_count_pid =
> + TOKEN_NUM_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_pid, RTE_UINT16);
> +static cmdline_parse_token_string_t cmd_show_rx_tx_queue_desc_used_count_dir
> =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_dir, "rxq#txq");
> +static cmdline_parse_token_num_t cmd_show_rx_tx_queue_desc_used_count_qid =
> + TOKEN_NUM_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_qid, RTE_UINT16);
> +static cmdline_parse_token_string_t
> cmd_show_rx_tx_queue_desc_used_count_desc =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_desc, "desc");
> +static cmdline_parse_token_string_t
> cmd_show_rx_tx_queue_desc_used_count_used =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_count, "used");
> +static cmdline_parse_token_string_t
> cmd_show_rx_tx_queue_desc_used_count_count =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_show_rx_tx_queue_desc_used_count_result,
> + cmd_count, "count");
> +static cmdline_parse_inst_t cmd_show_rx_tx_queue_desc_used_count = {
> + .f = cmd_show_rx_tx_queue_desc_used_count_parsed,
> +
RE: [PATCH v3 1/3] config/arm: avoid mcpu and march conflicts
> > wrote: > > > > > > Hi Pavan, > > > > > >> The compiler options march and mtune are a subset of mcpu and will > > >> lead > > to > > >> conflicts if improper march is chosen for a given mcpu. > > >> To avoid conflicts, force part number march when mcpu is available > > >> and is supported by the compiler. > > > > > > Why would one force the march specified in the part number when mcpu > > > for that part number is also available and supported by the compiler? > > > > > It would be good to explain the use case or the problem being faced. > > > > The idea of this patchset is to avoid mcpu and march conflicts that can happen > with the current build flow. > > #aarch64-linux-gnu-gcc -mcpu=neoverse-n2 -march=armv8.6-a shrn.c > cc1: warning: switch '-mcpu=neoverse-n2' conflicts with '-march=armv8.6-a' > > In some versions of GCC mcpu=neoverse-n2 is supported but -march=armv9- > a is not so, current build flow will choose the next supported march which is > armv8.6-a and report a conflict. > If compiler support is available for a certain CPU, then it is safe to assume that the Compiler knows the best architecture to use. Therefore, in such cases the best practice is to not provide -march. > > >> > > >> Example: > > >> march = armv9-a > > >> mcpu = neoverse-n2 > > >> > > >> mcpu supported, march supported > > >> machine_args = ['-mcpu=neoverse-n2', '-march=armv9-a'] > > > > > > -march restricts the compiler to baseline architecture of the -mcpu. > > > For instance, Neoverse-n1's baseline architecture is armv8.2-a, but > > > it has some extensions from armv8.3-a, armv8.4-a, and armv8.5-a. > > > By setting -march to armv8.2-a the compiler will strictly omit > > > extensions from 8.3, 8.4 and 8.5 resulting in a suboptimal outcome. > > What if compiler only supports armv8.2-a? > Are you suggesting we don’t use march at all when mcpu is supported? > If so how do you express extensions that the SoC supports? > Neoverse-n2 has optional support for crypto and can only be enabled by > expressing it through march='armv9-a+crypto' > March extensions also works with mcpu, use mcpu=neoverse-n2+crypto instead of march. It's documented in "-march and -mcpu Feature Modifiers" section in gcc manual. > > > > > >> > > >> mcpu supported, march not supported machine_args = > > >> ['-mcpu=neoverse-n2'] > > > > > > This will result in the best outcome. > > Isn't -mcpu=neoverse-n2 -march=armv9-a+sve2+crypto also the best > outcome? > Here also we can append feature modifiers like sve2 and crypto to CPU in -mcpu and drop -march arg. If the compiler supports neoverse-n2 but not armv9-a it will pick the next best architecture. -mcpu=neoverse-n2+sve2+crypto can replace - march=armv9-a+sve2+crypto > > > > > >> > > >> mcpu not supported, march supported machine_args = > > >> ['-march=armv9-a'] > > > > > > This too may result in a suboptimal outcome as optimization space is > > > limited to the given march (not using extensions from later > > > architectures when available). > > > > > What if compiler doesn’t support mcpu=neoverse-n2 and only supports > march=armv9-a > I agree there can be such corner cases where CPU enablement isn't done. Such cases can be handled with a new meson build parameter like -Dplatform=generic-armv9 to build armv9-a binaries (similar to -Dplatform=generic that builds armv8-a binaries today). Having such parameter forces the user to make a conscious decision rather than build system doing it for them. It also comes with the added benefit of having a simpler build system. > > >> > > >> mcpu not supported, march not supported machine_args = > > >> ['-march=armv8.6-a'] > > > > > > Compiler knows nothing about the target CPU or the architecture. > > > I think it's better to exit the build process with an error. > > > > > Then we would need to mark all old GCC versions as not supported by a newer > SoC I don’t think that’s needed since the binaries still run but not > optimally, > currently we have a warning in place for march mismatch. > We don't have to deprecate older versions of GCC. I'm suggesting two options to let the user have greater autonomy on the kind of the binary they want rather than ending up with a binary the build system forced on them. Today -Dplatform=generic does something similar to this with armv8, it simply directs the build system to output an armv8 without any extras. First suggestion is that we simply have a -Dplatform=generic-armv9 that does the same but for armv9. The second suggestion is to empower a sophisticated user even further to override everything in the build system including generics via two parameters -Dmarch and -Dmcpu to set an arbitrary architecture and a cpu. Second option works as a catch-all for every unorthodox request that may come our way. Both these features can be suggested when build exits due to compiler not knowing the target CPU or the architecture. I think these parameters keep user in charge with a simpler build system. > > >> > > >> Sig
Re: [PATCH v3] ethdev: fast path async flow API
On 2/6/2024 10:21 PM, Thomas Monjalon wrote:
> 06/02/2024 18:36, Dariusz Sosnowski:
>> --- a/doc/guides/nics/build_and_test.rst
>> +++ b/doc/guides/nics/build_and_test.rst
>> +- ``RTE_FLOW_DEBUG`` (default **disabled**; enabled automatically on debug
>> builds)
>> +
>> + Build with debug code in asynchronous flow APIs.
>> +
>> .. Note::
>>
>> - The ethdev library use above options to wrap debug code to trace invalid
>> parameters
>> + The ethdev library uses above options to wrap debug code to trace
>> invalid parameters
>> on data path APIs, so performance downgrade is expected when enabling
>> those options.
>> - Each PMD can decide to reuse them to wrap their own debug code in the
>> Rx/Tx path.
>> + Each PMD can decide to reuse them to wrap their own debug code in the
>> Rx/Tx path
>> + and in asynchronous flow APIs implementation.
>
> Good
>
>> --- a/doc/guides/rel_notes/release_24_03.rst
>> +++ b/doc/guides/rel_notes/release_24_03.rst
>> +* ethdev: PMDs implementing asynchronous flow operations are required to
>> provide relevant functions
>> + implementation through ``rte_flow_fp_ops`` struct, instead of
>> ``rte_flow_ops`` struct.
>> + Pointer to device-dependent ``rte_flow_fp_ops`` should be provided to
>> ``rte_eth_dev.flow_fp_ops``.
>
> That's a change only for the driver.
> If there is no change for the application, it should not appear in the
> release notes.
> BTW, API means Application Programming Interface :)
>
>> + This change applies to the following API functions:
>> +
>> + * ``rte_flow_async_create``
>> + * ``rte_flow_async_create_by_index``
>> + * ``rte_flow_async_actions_update``
>> + * ``rte_flow_async_destroy``
>> + * ``rte_flow_push``
>> + * ``rte_flow_pull``
>> + * ``rte_flow_async_action_handle_create``
>> + * ``rte_flow_async_action_handle_destroy``
>> + * ``rte_flow_async_action_handle_update``
>> + * ``rte_flow_async_action_handle_query``
>> + * ``rte_flow_async_action_handle_query_update``
>> + * ``rte_flow_async_action_list_handle_create``
>> + * ``rte_flow_async_action_list_handle_destroy``
>> + * ``rte_flow_async_action_list_handle_query_update``
>> +
>> +* ethdev: Removed the following fields from ``rte_flow_ops`` struct:
>> +
>> + * ``async_create``
>> + * ``async_create_by_index``
>> + * ``async_actions_update``
>> + * ``async_destroy``
>> + * ``push``
>> + * ``pull``
>> + * ``async_action_handle_create``
>> + * ``async_action_handle_destroy``
>> + * ``async_action_handle_update``
>> + * ``async_action_handle_query``
>> + * ``async_action_handle_query_update``
>> + * ``async_action_list_handle_create``
>> + * ``async_action_list_handle_destroy``
>> + * ``async_action_list_handle_query_update``
>
> [...]
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -71,6 +71,10 @@ struct rte_eth_dev {
>> struct rte_eth_dev_data *data;
>> void *process_private; /**< Pointer to per-process device data */
>> const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
>> +/**
>> + * Fast path flow API functions exported by PMD.
>> + */
>
> This comment may be on one single line.
>
>> +const struct rte_flow_fp_ops *flow_fp_ops;
>> struct rte_device *device; /**< Backing device */
>> struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
>
>> --- a/lib/ethdev/meson.build
>> +++ b/lib/ethdev/meson.build
>> +if get_option('buildtype').contains('debug')
>> +cflags += ['-DRTE_FLOW_DEBUG']
>> +endif
>
> This looks OK.
>
> Acked-by: Thomas Monjalon
>
>
Acked-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
[PATCH v2 00/13] net/ionic: miscellaneous fixes and improvements
This patchset provides miscellaneous fixes and improvements for the net/ionic driver used by AMD Pensando devices. V2: - Update device stop and device start patches to use compound literals as suggested by review. Akshay Dorwat (1): net/ionic: fix RSS query routine Andrew Boyer (8): net/ionic: add stat for completion queue entries processed net/ionic: increase max supported MTU to 9750 bytes net/ionic: don't auto-enable Rx scatter-gather a second time net/ionic: replace non-standard type in structure definition net/ionic: fix device close sequence to avoid crash net/ionic: optimize device close operation net/ionic: optimize device stop operation net/ionic: optimize device start operation Brad Larson (1): net/ionic: add flexible firmware xstat counters Neel Patel (2): net/ionic: fix missing volatile type for cqe pointers net/ionic: memcpy descriptors when using Q-in-CMB Vamsi Krishna Atluri (1): net/ionic: report 1G and 200G link speeds when applicable drivers/net/ionic/ionic.h | 3 + drivers/net/ionic/ionic_dev.c | 9 +- drivers/net/ionic/ionic_dev.h | 8 +- drivers/net/ionic/ionic_dev_pci.c | 2 +- drivers/net/ionic/ionic_ethdev.c | 81 ++--- drivers/net/ionic/ionic_if.h | 70 drivers/net/ionic/ionic_lif.c | 233 ++ drivers/net/ionic/ionic_lif.h | 19 ++- drivers/net/ionic/ionic_main.c| 17 +- drivers/net/ionic/ionic_rxtx.c| 160 +- drivers/net/ionic/ionic_rxtx.h| 80 - drivers/net/ionic/ionic_rxtx_sg.c | 28 ++-- drivers/net/ionic/ionic_rxtx_simple.c | 28 ++-- 13 files changed, 527 insertions(+), 211 deletions(-) -- 2.17.1
[PATCH v2 01/13] net/ionic: add stat for completion queue entries processed
When completion coalescing is turned on in the FW, there will be
fewer CQE than Tx packets. Expose the stat through debug logging.
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_lif.h | 1 +
drivers/net/ionic/ionic_rxtx.c| 3 +++
drivers/net/ionic/ionic_rxtx_sg.c | 2 ++
drivers/net/ionic/ionic_rxtx_simple.c | 2 ++
4 files changed, 8 insertions(+)
diff --git a/drivers/net/ionic/ionic_lif.h b/drivers/net/ionic/ionic_lif.h
index 36b3bcc5a9..cac7a4583b 100644
--- a/drivers/net/ionic/ionic_lif.h
+++ b/drivers/net/ionic/ionic_lif.h
@@ -32,6 +32,7 @@
struct ionic_tx_stats {
uint64_t packets;
uint64_t bytes;
+ uint64_t comps;
uint64_t drop;
uint64_t stop;
uint64_t no_csum;
diff --git a/drivers/net/ionic/ionic_rxtx.c b/drivers/net/ionic/ionic_rxtx.c
index b9e73b4871..d92b231f8f 100644
--- a/drivers/net/ionic/ionic_rxtx.c
+++ b/drivers/net/ionic/ionic_rxtx.c
@@ -117,6 +117,9 @@ ionic_dev_tx_queue_stop(struct rte_eth_dev *eth_dev,
uint16_t tx_queue_id)
stats = &txq->stats;
IONIC_PRINT(DEBUG, "TX queue %u pkts %ju tso %ju",
txq->qcq.q.index, stats->packets, stats->tso);
+ IONIC_PRINT(DEBUG, "TX queue %u comps %ju (%ju per)",
+ txq->qcq.q.index, stats->comps,
+ stats->comps ? stats->packets / stats->comps : 0);
return 0;
}
diff --git a/drivers/net/ionic/ionic_rxtx_sg.c
b/drivers/net/ionic/ionic_rxtx_sg.c
index ab8e56e91c..6c028a698c 100644
--- a/drivers/net/ionic/ionic_rxtx_sg.c
+++ b/drivers/net/ionic/ionic_rxtx_sg.c
@@ -26,6 +26,7 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
{
struct ionic_cq *cq = &txq->qcq.cq;
struct ionic_queue *q = &txq->qcq.q;
+ struct ionic_tx_stats *stats = &txq->stats;
struct rte_mbuf *txm;
struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
void **info;
@@ -72,6 +73,7 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
}
cq_desc = &cq_desc_base[cq->tail_idx];
+ stats->comps++;
}
}
diff --git a/drivers/net/ionic/ionic_rxtx_simple.c
b/drivers/net/ionic/ionic_rxtx_simple.c
index 5f81856256..5969287b66 100644
--- a/drivers/net/ionic/ionic_rxtx_simple.c
+++ b/drivers/net/ionic/ionic_rxtx_simple.c
@@ -26,6 +26,7 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
{
struct ionic_cq *cq = &txq->qcq.cq;
struct ionic_queue *q = &txq->qcq.q;
+ struct ionic_tx_stats *stats = &txq->stats;
struct rte_mbuf *txm;
struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
void **info;
@@ -67,6 +68,7 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
}
cq_desc = &cq_desc_base[cq->tail_idx];
+ stats->comps++;
}
}
--
2.17.1
[PATCH v2 02/13] net/ionic: increase max supported MTU to 9750 bytes
Some configurations want to use values this high internally. Allow them to do so without modifying the code. Signed-off-by: Andrew Boyer Signed-off-by: Bhuvan Mital --- drivers/net/ionic/ionic_dev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ionic/ionic_dev.h b/drivers/net/ionic/ionic_dev.h index b1e74fbd89..971c261b27 100644 --- a/drivers/net/ionic/ionic_dev.h +++ b/drivers/net/ionic/ionic_dev.h @@ -14,7 +14,7 @@ #define VLAN_TAG_SIZE 4 #define IONIC_MIN_MTU RTE_ETHER_MIN_MTU -#define IONIC_MAX_MTU 9378 +#define IONIC_MAX_MTU 9750 #define IONIC_ETH_OVERHEAD (RTE_ETHER_HDR_LEN + VLAN_TAG_SIZE) #define IONIC_MAX_RING_DESC32768 -- 2.17.1
[PATCH v2 03/13] net/ionic: don't auto-enable Rx scatter-gather a second time
The receive side will enable scatter-gather if required based on the
mbuf size. If the client already enabled it in the config, it does
not need to be enabled again. This reduces log output.
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_lif.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
index 25b490deb6..fe2112c057 100644
--- a/drivers/net/ionic/ionic_lif.c
+++ b/drivers/net/ionic/ionic_lif.c
@@ -768,7 +768,8 @@ ionic_rx_qcq_alloc(struct ionic_lif *lif, uint32_t
socket_id, uint32_t index,
max_mtu = rte_le_to_cpu_32(lif->adapter->ident.lif.eth.max_mtu);
/* If mbufs are too small to hold received packets, enable SG */
- if (max_mtu > hdr_seg_size) {
+ if (max_mtu > hdr_seg_size &&
+ !(lif->features & IONIC_ETH_HW_RX_SG)) {
IONIC_PRINT(NOTICE, "Enabling RX_OFFLOAD_SCATTER");
lif->eth_dev->data->dev_conf.rxmode.offloads |=
RTE_ETH_RX_OFFLOAD_SCATTER;
--
2.17.1
[PATCH v2 04/13] net/ionic: fix missing volatile type for cqe pointers
From: Neel Patel
This memory may be changed by the hardware, so the volatile
keyword is required for correctness.
Fixes: e86a6fcc7cf3 ("net/ionic: add optimized non-scattered Rx/Tx")
cc: sta...@dpdk.org
Signed-off-by: Andrew Boyer
Signed-off-by: Neel Patel
---
drivers/net/ionic/ionic_rxtx.c| 4 ++--
drivers/net/ionic/ionic_rxtx_sg.c | 8 +---
drivers/net/ionic/ionic_rxtx_simple.c | 8 +---
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ionic/ionic_rxtx.c b/drivers/net/ionic/ionic_rxtx.c
index d92b231f8f..d92fa1cca7 100644
--- a/drivers/net/ionic/ionic_rxtx.c
+++ b/drivers/net/ionic/ionic_rxtx.c
@@ -755,7 +755,7 @@ ionic_dev_rx_descriptor_status(void *rx_queue, uint16_t
offset)
{
struct ionic_rx_qcq *rxq = rx_queue;
struct ionic_qcq *qcq = &rxq->qcq;
- struct ionic_rxq_comp *cq_desc;
+ volatile struct ionic_rxq_comp *cq_desc;
uint16_t mask, head, tail, pos;
bool done_color;
@@ -794,7 +794,7 @@ ionic_dev_tx_descriptor_status(void *tx_queue, uint16_t
offset)
{
struct ionic_tx_qcq *txq = tx_queue;
struct ionic_qcq *qcq = &txq->qcq;
- struct ionic_txq_comp *cq_desc;
+ volatile struct ionic_txq_comp *cq_desc;
uint16_t mask, head, tail, pos, cq_pos;
bool done_color;
diff --git a/drivers/net/ionic/ionic_rxtx_sg.c
b/drivers/net/ionic/ionic_rxtx_sg.c
index 6c028a698c..1392342463 100644
--- a/drivers/net/ionic/ionic_rxtx_sg.c
+++ b/drivers/net/ionic/ionic_rxtx_sg.c
@@ -28,7 +28,8 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
struct ionic_queue *q = &txq->qcq.q;
struct ionic_tx_stats *stats = &txq->stats;
struct rte_mbuf *txm;
- struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+ struct ionic_txq_comp *cq_desc_base = cq->base;
+ volatile struct ionic_txq_comp *cq_desc;
void **info;
uint32_t i;
@@ -254,7 +255,7 @@ ionic_xmit_pkts_sg(void *tx_queue, struct rte_mbuf
**tx_pkts,
*/
static __rte_always_inline void
ionic_rx_clean_one_sg(struct ionic_rx_qcq *rxq,
- struct ionic_rxq_comp *cq_desc,
+ volatile struct ionic_rxq_comp *cq_desc,
struct ionic_rx_service *rx_svc)
{
struct ionic_queue *q = &rxq->qcq.q;
@@ -440,7 +441,8 @@ ionic_rxq_service_sg(struct ionic_rx_qcq *rxq, uint32_t
work_to_do,
struct ionic_cq *cq = &rxq->qcq.cq;
struct ionic_queue *q = &rxq->qcq.q;
struct ionic_rxq_desc *q_desc_base = q->base;
- struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+ struct ionic_rxq_comp *cq_desc_base = cq->base;
+ volatile struct ionic_rxq_comp *cq_desc;
uint32_t work_done = 0;
uint64_t then, now, hz, delta;
diff --git a/drivers/net/ionic/ionic_rxtx_simple.c
b/drivers/net/ionic/ionic_rxtx_simple.c
index 5969287b66..00152c885a 100644
--- a/drivers/net/ionic/ionic_rxtx_simple.c
+++ b/drivers/net/ionic/ionic_rxtx_simple.c
@@ -28,7 +28,8 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
struct ionic_queue *q = &txq->qcq.q;
struct ionic_tx_stats *stats = &txq->stats;
struct rte_mbuf *txm;
- struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
+ struct ionic_txq_comp *cq_desc_base = cq->base;
+ volatile struct ionic_txq_comp *cq_desc;
void **info;
cq_desc = &cq_desc_base[cq->tail_idx];
@@ -227,7 +228,7 @@ ionic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
*/
static __rte_always_inline void
ionic_rx_clean_one(struct ionic_rx_qcq *rxq,
- struct ionic_rxq_comp *cq_desc,
+ volatile struct ionic_rxq_comp *cq_desc,
struct ionic_rx_service *rx_svc)
{
struct ionic_queue *q = &rxq->qcq.q;
@@ -361,7 +362,8 @@ ionic_rxq_service(struct ionic_rx_qcq *rxq, uint32_t
work_to_do,
struct ionic_cq *cq = &rxq->qcq.cq;
struct ionic_queue *q = &rxq->qcq.q;
struct ionic_rxq_desc *q_desc_base = q->base;
- struct ionic_rxq_comp *cq_desc, *cq_desc_base = cq->base;
+ struct ionic_rxq_comp *cq_desc_base = cq->base;
+ volatile struct ionic_rxq_comp *cq_desc;
uint32_t work_done = 0;
uint64_t then, now, hz, delta;
--
2.17.1
[PATCH v2 05/13] net/ionic: replace non-standard type in structure definition
Use uint8_t instead of u_char. This simplifies the code. Signed-off-by: Andrew Boyer --- drivers/net/ionic/ionic_dev_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ionic/ionic_dev_pci.c b/drivers/net/ionic/ionic_dev_pci.c index 5e74a6da71..cbaac2c5bc 100644 --- a/drivers/net/ionic/ionic_dev_pci.c +++ b/drivers/net/ionic/ionic_dev_pci.c @@ -38,7 +38,7 @@ ionic_pci_setup(struct ionic_adapter *adapter) struct ionic_dev *idev = &adapter->idev; struct rte_pci_device *bus_dev = adapter->bus_dev; uint32_t sig; - u_char *bar0_base; + uint8_t *bar0_base; unsigned int i; /* BAR0: dev_cmd and interrupts */ -- 2.17.1
[PATCH v2 06/13] net/ionic: memcpy descriptors when using Q-in-CMB
From: Neel Patel
They can be batched together this way, reducing the number
of PCIe transactions. This improves transmit PPS by up to 50% in
some configurations.
Signed-off-by: Andrew Boyer
Signed-off-by: Neel Patel
---
drivers/net/ionic/ionic_dev.c | 9 +++--
drivers/net/ionic/ionic_dev.h | 6 ++-
drivers/net/ionic/ionic_lif.c | 26 +
drivers/net/ionic/ionic_rxtx.h| 56 +++
drivers/net/ionic/ionic_rxtx_sg.c | 18 -
drivers/net/ionic/ionic_rxtx_simple.c | 18 -
6 files changed, 101 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ionic/ionic_dev.c b/drivers/net/ionic/ionic_dev.c
index 70c14882ed..7f15914f74 100644
--- a/drivers/net/ionic/ionic_dev.c
+++ b/drivers/net/ionic/ionic_dev.c
@@ -369,17 +369,19 @@ ionic_q_init(struct ionic_queue *q, uint32_t index,
uint16_t num_descs)
q->index = index;
q->num_descs = num_descs;
q->size_mask = num_descs - 1;
- q->head_idx = 0;
- q->tail_idx = 0;
+ ionic_q_reset(q);
return 0;
}
void
-ionic_q_map(struct ionic_queue *q, void *base, rte_iova_t base_pa)
+ionic_q_map(struct ionic_queue *q, void *base, rte_iova_t base_pa,
+ void *cmb_base, rte_iova_t cmb_base_pa)
{
q->base = base;
q->base_pa = base_pa;
+ q->cmb_base = cmb_base;
+ q->cmb_base_pa = cmb_base_pa;
}
void
@@ -393,5 +395,6 @@ void
ionic_q_reset(struct ionic_queue *q)
{
q->head_idx = 0;
+ q->cmb_head_idx = 0;
q->tail_idx = 0;
}
diff --git a/drivers/net/ionic/ionic_dev.h b/drivers/net/ionic/ionic_dev.h
index 971c261b27..3a366247f1 100644
--- a/drivers/net/ionic/ionic_dev.h
+++ b/drivers/net/ionic/ionic_dev.h
@@ -145,11 +145,13 @@ struct ionic_queue {
uint16_t num_descs;
uint16_t num_segs;
uint16_t head_idx;
+ uint16_t cmb_head_idx;
uint16_t tail_idx;
uint16_t size_mask;
uint8_t type;
uint8_t hw_type;
void *base;
+ void *cmb_base;
void *sg_base;
struct ionic_doorbell __iomem *db;
void **info;
@@ -158,6 +160,7 @@ struct ionic_queue {
uint32_t hw_index;
rte_iova_t base_pa;
rte_iova_t sg_base_pa;
+ rte_iova_t cmb_base_pa;
};
#define IONIC_INTR_NONE(-1)
@@ -244,7 +247,8 @@ uint32_t ionic_cq_service(struct ionic_cq *cq, uint32_t
work_to_do,
int ionic_q_init(struct ionic_queue *q, uint32_t index, uint16_t num_descs);
void ionic_q_reset(struct ionic_queue *q);
-void ionic_q_map(struct ionic_queue *q, void *base, rte_iova_t base_pa);
+void ionic_q_map(struct ionic_queue *q, void *base, rte_iova_t base_pa,
+void *cmb_base, rte_iova_t cmb_base_pa);
void ionic_q_sg_map(struct ionic_queue *q, void *base, rte_iova_t base_pa);
static inline uint16_t
diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
index fe2112c057..2713f8aa24 100644
--- a/drivers/net/ionic/ionic_lif.c
+++ b/drivers/net/ionic/ionic_lif.c
@@ -572,10 +572,11 @@ ionic_qcq_alloc(struct ionic_lif *lif,
{
struct ionic_qcq *new;
uint32_t q_size, cq_size, sg_size, total_size;
- void *q_base, *cq_base, *sg_base;
+ void *q_base, *cmb_q_base, *cq_base, *sg_base;
rte_iova_t q_base_pa = 0;
rte_iova_t cq_base_pa = 0;
rte_iova_t sg_base_pa = 0;
+ rte_iova_t cmb_q_base_pa = 0;
size_t page_size = rte_mem_page_size();
int err;
@@ -666,19 +667,22 @@ ionic_qcq_alloc(struct ionic_lif *lif,
IONIC_PRINT(ERR, "Cannot reserve queue from NIC mem");
return -ENOMEM;
}
- q_base = (void *)
+ cmb_q_base = (void *)
((uintptr_t)lif->adapter->bars.bar[2].vaddr +
(uintptr_t)lif->adapter->cmb_offset);
/* CMB PA is a relative address */
- q_base_pa = lif->adapter->cmb_offset;
+ cmb_q_base_pa = lif->adapter->cmb_offset;
lif->adapter->cmb_offset += q_size;
+ } else {
+ cmb_q_base = NULL;
+ cmb_q_base_pa = 0;
}
IONIC_PRINT(DEBUG, "Q-Base-PA = %#jx CQ-Base-PA = %#jx "
"SG-base-PA = %#jx",
q_base_pa, cq_base_pa, sg_base_pa);
- ionic_q_map(&new->q, q_base, q_base_pa);
+ ionic_q_map(&new->q, q_base, q_base_pa, cmb_q_base, cmb_q_base_pa);
ionic_cq_map(&new->cq, cq_base, cq_base_pa);
*qcq = new;
@@ -1583,7 +1587,6 @@ ionic_lif_txq_init(struct ionic_tx_qcq *txq)
.flags = rte_cpu_to_le_16(IONIC_QINIT_F_ENA),
.intr_index = rte_cpu_to_le_16(IONIC_INTR_NONE),
.ring_size = rte_log2_u32(q->num_descs),
- .ring_base = rte_cpu_to_le_64(q->base_pa),
.cq_ring_base = rt
[PATCH v2 07/13] net/ionic: fix RSS query routine
From: Akshay Dorwat
The routine that copies out the RSS config can't use memcpy() because
'reta_conf->reta' is an array of uint16_t while 'lif->rss_ind_tbl' is
an array of uint8_t. Instead, copy the values individually.
Fixes: 22e7171bc63b ("net/ionic: support RSS")
Cc: cardigli...@ntop.org
Cc: sta...@dpdk.org
Signed-off-by: Akshay Dorwat
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_ethdev.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 340fd0cd59..008e50e0b9 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -561,7 +561,7 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
struct ionic_adapter *adapter = lif->adapter;
struct ionic_identity *ident = &adapter->ident;
- int i, num;
+ int i, j, num;
uint16_t tbl_sz = rte_le_to_cpu_16(ident->lif.eth.rss_ind_tbl_sz);
IONIC_PRINT_CALL();
@@ -582,9 +582,10 @@ ionic_dev_rss_reta_query(struct rte_eth_dev *eth_dev,
num = reta_size / RTE_ETH_RETA_GROUP_SIZE;
for (i = 0; i < num; i++) {
- memcpy(reta_conf->reta,
- &lif->rss_ind_tbl[i * RTE_ETH_RETA_GROUP_SIZE],
- RTE_ETH_RETA_GROUP_SIZE);
+ for (j = 0; j < RTE_ETH_RETA_GROUP_SIZE; j++) {
+ reta_conf->reta[j] =
+ lif->rss_ind_tbl[(i * RTE_ETH_RETA_GROUP_SIZE)
+ j];
+ }
reta_conf++;
}
--
2.17.1
[PATCH v2 08/13] net/ionic: report 1G and 200G link speeds when applicable
From: Vamsi Krishna Atluri
The hardware supports these speeds, so we should report them
correctly.
Signed-off-by: Andrew Boyer
Signed-off-by: Vamsi Krishna Atluri
---
drivers/net/ionic/ionic_ethdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 008e50e0b9..327f6b9de5 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -285,6 +285,9 @@ ionic_dev_link_update(struct rte_eth_dev *eth_dev,
link.link_status = RTE_ETH_LINK_UP;
link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
switch (adapter->link_speed) {
+ case 1000:
+ link.link_speed = RTE_ETH_SPEED_NUM_1G;
+ break;
case 1:
link.link_speed = RTE_ETH_SPEED_NUM_10G;
break;
@@ -300,6 +303,9 @@ ionic_dev_link_update(struct rte_eth_dev *eth_dev,
case 10:
link.link_speed = RTE_ETH_SPEED_NUM_100G;
break;
+ case 20:
+ link.link_speed = RTE_ETH_SPEED_NUM_200G;
+ break;
default:
link.link_speed = RTE_ETH_SPEED_NUM_NONE;
break;
--
2.17.1
[PATCH v2 09/13] net/ionic: add flexible firmware xstat counters
From: Brad Larson
Assign 32 counters for flexible firmware events. These can be used as
per-port or per-queue counters in certain firmware configurations.
They are displayed as fw_flex_eventX in xstats.
Signed-off-by: Andrew Boyer
Signed-off-by: Brad Larson
---
drivers/net/ionic/ionic_ethdev.c | 33 +++
drivers/net/ionic/ionic_if.h | 70
2 files changed, 68 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 327f6b9de5..7c55a26956 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -196,6 +196,39 @@ static const struct rte_ionic_xstats_name_off
rte_ionic_xstats_strings[] = {
tx_desc_fetch_error)},
{"tx_desc_data_error", offsetof(struct ionic_lif_stats,
tx_desc_data_error)},
+ /* Flexible firmware events */
+ {"fw_flex_event1", offsetof(struct ionic_lif_stats, flex1)},
+ {"fw_flex_event2", offsetof(struct ionic_lif_stats, flex2)},
+ {"fw_flex_event3", offsetof(struct ionic_lif_stats, flex3)},
+ {"fw_flex_event4", offsetof(struct ionic_lif_stats, flex4)},
+ {"fw_flex_event5", offsetof(struct ionic_lif_stats, flex5)},
+ {"fw_flex_event6", offsetof(struct ionic_lif_stats, flex6)},
+ {"fw_flex_event7", offsetof(struct ionic_lif_stats, flex7)},
+ {"fw_flex_event8", offsetof(struct ionic_lif_stats, flex8)},
+ {"fw_flex_event9", offsetof(struct ionic_lif_stats, flex9)},
+ {"fw_flex_event10", offsetof(struct ionic_lif_stats, flex10)},
+ {"fw_flex_event11", offsetof(struct ionic_lif_stats, flex11)},
+ {"fw_flex_event12", offsetof(struct ionic_lif_stats, flex12)},
+ {"fw_flex_event13", offsetof(struct ionic_lif_stats, flex13)},
+ {"fw_flex_event14", offsetof(struct ionic_lif_stats, flex14)},
+ {"fw_flex_event15", offsetof(struct ionic_lif_stats, flex15)},
+ {"fw_flex_event16", offsetof(struct ionic_lif_stats, flex16)},
+ {"fw_flex_event17", offsetof(struct ionic_lif_stats, flex17)},
+ {"fw_flex_event18", offsetof(struct ionic_lif_stats, flex18)},
+ {"fw_flex_event19", offsetof(struct ionic_lif_stats, flex19)},
+ {"fw_flex_event20", offsetof(struct ionic_lif_stats, flex20)},
+ {"fw_flex_event21", offsetof(struct ionic_lif_stats, flex21)},
+ {"fw_flex_event22", offsetof(struct ionic_lif_stats, flex22)},
+ {"fw_flex_event23", offsetof(struct ionic_lif_stats, flex23)},
+ {"fw_flex_event24", offsetof(struct ionic_lif_stats, flex24)},
+ {"fw_flex_event25", offsetof(struct ionic_lif_stats, flex25)},
+ {"fw_flex_event26", offsetof(struct ionic_lif_stats, flex26)},
+ {"fw_flex_event27", offsetof(struct ionic_lif_stats, flex27)},
+ {"fw_flex_event28", offsetof(struct ionic_lif_stats, flex28)},
+ {"fw_flex_event29", offsetof(struct ionic_lif_stats, flex29)},
+ {"fw_flex_event30", offsetof(struct ionic_lif_stats, flex30)},
+ {"fw_flex_event31", offsetof(struct ionic_lif_stats, flex31)},
+ {"fw_flex_event32", offsetof(struct ionic_lif_stats, flex32)},
};
#define IONIC_NB_HW_STATS RTE_DIM(rte_ionic_xstats_strings)
diff --git a/drivers/net/ionic/ionic_if.h b/drivers/net/ionic/ionic_if.h
index 79aa196345..7ca604a7bb 100644
--- a/drivers/net/ionic/ionic_if.h
+++ b/drivers/net/ionic/ionic_if.h
@@ -2592,41 +2592,41 @@ struct ionic_lif_stats {
__le64 rsvd16;
__le64 rsvd17;
- __le64 rsvd18;
- __le64 rsvd19;
- __le64 rsvd20;
- __le64 rsvd21;
- __le64 rsvd22;
- __le64 rsvd23;
- __le64 rsvd24;
- __le64 rsvd25;
-
- __le64 rsvd26;
- __le64 rsvd27;
- __le64 rsvd28;
- __le64 rsvd29;
- __le64 rsvd30;
- __le64 rsvd31;
- __le64 rsvd32;
- __le64 rsvd33;
-
- __le64 rsvd34;
- __le64 rsvd35;
- __le64 rsvd36;
- __le64 rsvd37;
- __le64 rsvd38;
- __le64 rsvd39;
- __le64 rsvd40;
- __le64 rsvd41;
-
- __le64 rsvd42;
- __le64 rsvd43;
- __le64 rsvd44;
- __le64 rsvd45;
- __le64 rsvd46;
- __le64 rsvd47;
- __le64 rsvd48;
- __le64 rsvd49;
+ __le64 flex1;
+ __le64 flex2;
+ __le64 flex3;
+ __le64 flex4;
+ __le64 flex5;
+ __le64 flex6;
+ __le64 flex7;
+ __le64 flex8;
+
+ __le64 flex9;
+ __le64 flex10;
+ __le64 flex11;
+ __le64 flex12;
+ __le64 flex13;
+ __le64 flex14;
+ __le64 flex15;
+ __le64 flex16;
+
+ __le64 flex17;
+ __le64 flex18;
+ __le64 flex19;
+ __le64 flex20;
+ __le64 flex21;
+ __le64 flex22;
+ __le64 flex23;
+ __le64 flex24;
+
+ __le64 flex25;
+ __le64 flex26;
+ __le64 flex27;
+ __le64 flex28;
+ __le64 flex29;
+ __le64 flex30;
+ __le64 flex31;
+ __le6
[PATCH v2 11/13] net/ionic: optimize device close operation
Use a single device reset command to speed up dev_close(). The LIF stop
and port reset commands are not needed.
This reduces the outage window when restarting the process by about 2ms
plus another 1ms per queue.
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_ethdev.c | 3 ---
drivers/net/ionic/ionic_lif.c| 8 +---
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index bedcf958e2..7e80751846 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -1007,13 +1007,10 @@ ionic_dev_close(struct rte_eth_dev *eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
- ionic_lif_stop(lif);
-
IONIC_PRINT(NOTICE, "Removing device %s", eth_dev->device->name);
if (adapter->intf->unconfigure_intr)
(*adapter->intf->unconfigure_intr)(adapter);
- ionic_port_reset(adapter);
ionic_reset(adapter);
ionic_lif_free_queues(lif);
diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
index 2713f8aa24..90efcc8cbb 100644
--- a/drivers/net/ionic/ionic_lif.c
+++ b/drivers/net/ionic/ionic_lif.c
@@ -1231,13 +1231,7 @@ ionic_lif_rss_setup(struct ionic_lif *lif)
static void
ionic_lif_rss_teardown(struct ionic_lif *lif)
{
- if (!lif->rss_ind_tbl)
- return;
-
- if (lif->rss_ind_tbl_z) {
- /* Disable RSS on the NIC */
- ionic_lif_rss_config(lif, 0x0, NULL, NULL);
-
+ if (lif->rss_ind_tbl) {
lif->rss_ind_tbl = NULL;
lif->rss_ind_tbl_pa = 0;
rte_memzone_free(lif->rss_ind_tbl_z);
--
2.17.1
[PATCH v2 10/13] net/ionic: fix device close sequence to avoid crash
The close routine should release all resources, but not
call rte_eth_dev_destroy(). As written this code will call
rte_eth_dev_release_port() twice and segfault.
Instead, move rte_eth_dev_destroy() to the remove routine.
eth_ionic_dev_uninit() will call close if necessary.
Fixes: 175e4e7ed760 ("net/ionic: complete release on close")
Cc: sta...@dpdk.org
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_ethdev.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index 7c55a26956..bedcf958e2 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -1009,19 +1009,21 @@ ionic_dev_close(struct rte_eth_dev *eth_dev)
ionic_lif_stop(lif);
- ionic_lif_free_queues(lif);
-
IONIC_PRINT(NOTICE, "Removing device %s", eth_dev->device->name);
if (adapter->intf->unconfigure_intr)
(*adapter->intf->unconfigure_intr)(adapter);
- rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
-
ionic_port_reset(adapter);
ionic_reset(adapter);
+
+ ionic_lif_free_queues(lif);
+ ionic_lif_deinit(lif);
+ ionic_lif_free(lif); /* Does not free LIF object */
+
if (adapter->intf->unmap_bars)
(*adapter->intf->unmap_bars)(adapter);
+ lif->adapter = NULL;
rte_free(adapter);
return 0;
@@ -1098,21 +1100,18 @@ eth_ionic_dev_init(struct rte_eth_dev *eth_dev, void
*init_params)
static int
eth_ionic_dev_uninit(struct rte_eth_dev *eth_dev)
{
- struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
- struct ionic_adapter *adapter = lif->adapter;
-
IONIC_PRINT_CALL();
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
- adapter->lif = NULL;
-
- ionic_lif_deinit(lif);
- ionic_lif_free(lif);
+ if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+ ionic_dev_close(eth_dev);
- if (!(lif->state & IONIC_LIF_F_FW_RESET))
- ionic_lif_reset(lif);
+ eth_dev->dev_ops = NULL;
+ eth_dev->rx_pkt_burst = NULL;
+ eth_dev->tx_pkt_burst = NULL;
+ eth_dev->tx_pkt_prepare = NULL;
return 0;
}
@@ -1267,17 +1266,18 @@ eth_ionic_dev_remove(struct rte_device *rte_dev)
{
char name[RTE_ETH_NAME_MAX_LEN];
struct rte_eth_dev *eth_dev;
+ int ret = 0;
/* Adapter lookup is using the eth_dev name */
snprintf(name, sizeof(name), "%s_lif", rte_dev->name);
eth_dev = rte_eth_dev_allocated(name);
if (eth_dev)
- ionic_dev_close(eth_dev);
+ ret = rte_eth_dev_destroy(eth_dev, eth_ionic_dev_uninit);
else
IONIC_PRINT(DEBUG, "Cannot find device %s", rte_dev->name);
- return 0;
+ return ret;
}
RTE_LOG_REGISTER_DEFAULT(ionic_logtype, NOTICE);
--
2.17.1
[PATCH v2 12/13] net/ionic: optimize device stop operation
Split the queue_stop operation into first-half and second-half helpers.
Move the command context from the stack into each Rx/Tx queue struct.
Expose some needed adminq interfaces.
This allows us to batch up the queue commands during dev_stop(), reducing
the outage window when restarting the process by about 1ms per queue.
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic.h | 3 ++
drivers/net/ionic/ionic_lif.c | 68 ++---
drivers/net/ionic/ionic_lif.h | 12 --
drivers/net/ionic/ionic_main.c | 17 +++-
drivers/net/ionic/ionic_rxtx.c | 78 --
drivers/net/ionic/ionic_rxtx.h | 14 +-
6 files changed, 138 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ionic/ionic.h b/drivers/net/ionic/ionic.h
index c479eaba74..cb4ea450a9 100644
--- a/drivers/net/ionic/ionic.h
+++ b/drivers/net/ionic/ionic.h
@@ -83,7 +83,10 @@ struct ionic_admin_ctx {
union ionic_adminq_comp comp;
};
+int ionic_adminq_post(struct ionic_lif *lif, struct ionic_admin_ctx *ctx);
int ionic_adminq_post_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx);
+int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx);
+uint16_t ionic_adminq_space_avail(struct ionic_lif *lif);
int ionic_dev_cmd_wait_check(struct ionic_dev *idev, unsigned long max_wait);
int ionic_setup(struct ionic_adapter *adapter);
diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
index 90efcc8cbb..45317590fa 100644
--- a/drivers/net/ionic/ionic_lif.c
+++ b/drivers/net/ionic/ionic_lif.c
@@ -31,11 +31,15 @@ static int ionic_lif_addr_add(struct ionic_lif *lif, const
uint8_t *addr);
static int ionic_lif_addr_del(struct ionic_lif *lif, const uint8_t *addr);
static int
-ionic_qcq_disable(struct ionic_qcq *qcq)
+ionic_qcq_disable_nowait(struct ionic_qcq *qcq,
+ struct ionic_admin_ctx *ctx)
{
+ int err;
+
struct ionic_queue *q = &qcq->q;
struct ionic_lif *lif = qcq->lif;
- struct ionic_admin_ctx ctx = {
+
+ *ctx = (struct ionic_admin_ctx) {
.pending_work = true,
.cmd.q_control = {
.opcode = IONIC_CMD_Q_CONTROL,
@@ -45,28 +49,39 @@ ionic_qcq_disable(struct ionic_qcq *qcq)
},
};
- return ionic_adminq_post_wait(lif, &ctx);
+ /* Does not wait for command completion */
+ err = ionic_adminq_post(lif, ctx);
+ if (err)
+ ctx->pending_work = false;
+ return err;
}
void
ionic_lif_stop(struct ionic_lif *lif)
{
- uint32_t i;
+ struct rte_eth_dev *dev = lif->eth_dev;
+ uint32_t i, j, chunk;
IONIC_PRINT_CALL();
lif->state &= ~IONIC_LIF_F_UP;
- for (i = 0; i < lif->nrxqcqs; i++) {
- struct ionic_rx_qcq *rxq = lif->rxqcqs[i];
- if (rxq->flags & IONIC_QCQ_F_INITED)
- (void)ionic_dev_rx_queue_stop(lif->eth_dev, i);
+ chunk = ionic_adminq_space_avail(lif);
+
+ for (i = 0; i < lif->nrxqcqs; i += chunk) {
+ for (j = 0; j < chunk && i + j < lif->nrxqcqs; j++)
+ ionic_dev_rx_queue_stop_firsthalf(dev, i + j);
+
+ for (j = 0; j < chunk && i + j < lif->nrxqcqs; j++)
+ ionic_dev_rx_queue_stop_secondhalf(dev, i + j);
}
- for (i = 0; i < lif->ntxqcqs; i++) {
- struct ionic_tx_qcq *txq = lif->txqcqs[i];
- if (txq->flags & IONIC_QCQ_F_INITED)
- (void)ionic_dev_tx_queue_stop(lif->eth_dev, i);
+ for (i = 0; i < lif->ntxqcqs; i += chunk) {
+ for (j = 0; j < chunk && i + j < lif->ntxqcqs; j++)
+ ionic_dev_tx_queue_stop_firsthalf(dev, i + j);
+
+ for (j = 0; j < chunk && i + j < lif->ntxqcqs; j++)
+ ionic_dev_tx_queue_stop_secondhalf(dev, i + j);
}
}
@@ -1240,21 +1255,42 @@ ionic_lif_rss_teardown(struct ionic_lif *lif)
}
void
-ionic_lif_txq_deinit(struct ionic_tx_qcq *txq)
+ionic_lif_txq_deinit_nowait(struct ionic_tx_qcq *txq)
{
- ionic_qcq_disable(&txq->qcq);
+ ionic_qcq_disable_nowait(&txq->qcq, &txq->admin_ctx);
txq->flags &= ~IONIC_QCQ_F_INITED;
}
void
-ionic_lif_rxq_deinit(struct ionic_rx_qcq *rxq)
+ionic_lif_txq_stats(struct ionic_tx_qcq *txq)
+{
+ struct ionic_tx_stats *stats = &txq->stats;
+
+ IONIC_PRINT(DEBUG, "TX queue %u pkts %ju tso %ju",
+ txq->qcq.q.index, stats->packets, stats->tso);
+ IONIC_PRINT(DEBUG, "TX queue %u comps %ju (%ju per)",
+ txq->qcq.q.index, stats->comps,
+ stats->comps ? stats->packets / stats->comps : 0);
+}
+
+void
+ionic_lif_rxq_deinit_nowait(struct ionic_rx_qcq *rxq)
{
- ionic_qcq_disable(&rxq->qcq);
+ ionic_qcq_disable_nowait(&rxq->qcq, &rxq->admin_ctx);
rxq->flags &= ~IONIC_QCQ_F_INITED;
}
+void
+ionic_lif_rxq_
[PATCH v2 13/13] net/ionic: optimize device start operation
Split the queue_start operation into first-half and second-half helpers.
This allows us to batch up the queue commands during dev_start(), reducing
the outage window when restarting the process by about 1ms per queue.
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_lif.c | 136 +++--
drivers/net/ionic/ionic_lif.h | 6 +-
drivers/net/ionic/ionic_rxtx.c | 81
drivers/net/ionic/ionic_rxtx.h | 10 +++
4 files changed, 176 insertions(+), 57 deletions(-)
diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
index 45317590fa..93a1011772 100644
--- a/drivers/net/ionic/ionic_lif.c
+++ b/drivers/net/ionic/ionic_lif.c
@@ -1601,13 +1601,16 @@ ionic_lif_set_features(struct ionic_lif *lif)
}
int
-ionic_lif_txq_init(struct ionic_tx_qcq *txq)
+ionic_lif_txq_init_nowait(struct ionic_tx_qcq *txq)
{
struct ionic_qcq *qcq = &txq->qcq;
struct ionic_queue *q = &qcq->q;
struct ionic_lif *lif = qcq->lif;
struct ionic_cq *cq = &qcq->cq;
- struct ionic_admin_ctx ctx = {
+ struct ionic_admin_ctx *ctx = &txq->admin_ctx;
+ int err;
+
+ *ctx = (struct ionic_admin_ctx) {
.pending_work = true,
.cmd.q_init = {
.opcode = IONIC_CMD_Q_INIT,
@@ -1621,32 +1624,41 @@ ionic_lif_txq_init(struct ionic_tx_qcq *txq)
.sg_ring_base = rte_cpu_to_le_64(q->sg_base_pa),
},
};
- int err;
if (txq->flags & IONIC_QCQ_F_SG)
- ctx.cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_SG);
+ ctx->cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_SG);
if (txq->flags & IONIC_QCQ_F_CMB) {
- ctx.cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_CMB);
- ctx.cmd.q_init.ring_base = rte_cpu_to_le_64(q->cmb_base_pa);
+ ctx->cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_CMB);
+ ctx->cmd.q_init.ring_base = rte_cpu_to_le_64(q->cmb_base_pa);
} else {
- ctx.cmd.q_init.ring_base = rte_cpu_to_le_64(q->base_pa);
+ ctx->cmd.q_init.ring_base = rte_cpu_to_le_64(q->base_pa);
}
IONIC_PRINT(DEBUG, "txq_init.index %d", q->index);
IONIC_PRINT(DEBUG, "txq_init.ring_base 0x%" PRIx64 "", q->base_pa);
IONIC_PRINT(DEBUG, "txq_init.ring_size %d",
- ctx.cmd.q_init.ring_size);
- IONIC_PRINT(DEBUG, "txq_init.ver %u", ctx.cmd.q_init.ver);
+ ctx->cmd.q_init.ring_size);
+ IONIC_PRINT(DEBUG, "txq_init.ver %u", ctx->cmd.q_init.ver);
ionic_q_reset(q);
ionic_cq_reset(cq);
- err = ionic_adminq_post_wait(lif, &ctx);
+ /* Caller responsible for calling ionic_lif_txq_init_done() */
+ err = ionic_adminq_post(lif, ctx);
if (err)
- return err;
+ ctx->pending_work = false;
+ return err;
+}
- q->hw_type = ctx.comp.q_init.hw_type;
- q->hw_index = rte_le_to_cpu_32(ctx.comp.q_init.hw_index);
+void
+ionic_lif_txq_init_done(struct ionic_tx_qcq *txq)
+{
+ struct ionic_lif *lif = txq->qcq.lif;
+ struct ionic_queue *q = &txq->qcq.q;
+ struct ionic_admin_ctx *ctx = &txq->admin_ctx;
+
+ q->hw_type = ctx->comp.q_init.hw_type;
+ q->hw_index = rte_le_to_cpu_32(ctx->comp.q_init.hw_index);
q->db = ionic_db_map(lif, q);
IONIC_PRINT(DEBUG, "txq->hw_type %d", q->hw_type);
@@ -1654,18 +1666,19 @@ ionic_lif_txq_init(struct ionic_tx_qcq *txq)
IONIC_PRINT(DEBUG, "txq->db %p", q->db);
txq->flags |= IONIC_QCQ_F_INITED;
-
- return 0;
}
int
-ionic_lif_rxq_init(struct ionic_rx_qcq *rxq)
+ionic_lif_rxq_init_nowait(struct ionic_rx_qcq *rxq)
{
struct ionic_qcq *qcq = &rxq->qcq;
struct ionic_queue *q = &qcq->q;
struct ionic_lif *lif = qcq->lif;
struct ionic_cq *cq = &qcq->cq;
- struct ionic_admin_ctx ctx = {
+ struct ionic_admin_ctx *ctx = &rxq->admin_ctx;
+ int err;
+
+ *ctx = (struct ionic_admin_ctx) {
.pending_work = true,
.cmd.q_init = {
.opcode = IONIC_CMD_Q_INIT,
@@ -1679,32 +1692,41 @@ ionic_lif_rxq_init(struct ionic_rx_qcq *rxq)
.sg_ring_base = rte_cpu_to_le_64(q->sg_base_pa),
},
};
- int err;
if (rxq->flags & IONIC_QCQ_F_SG)
- ctx.cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_SG);
+ ctx->cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_SG);
if (rxq->flags & IONIC_QCQ_F_CMB) {
- ctx.cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_CMB);
- ctx.cmd.q_init.ring_base = rte_cpu_to_le_64(q->cmb_base_pa);
+ ctx->cmd.q_init.flags |= rte_cpu_to_le_16(IONIC_QINIT_F_CMB);
+ ctx->cmd.q_init.ring_base = rte_cpu_to_le_64(q->cmb_base_pa);
RE: [EXT] [PATCH] cryptodev: add a simple mapping cache to speed up ops pool create
> From: Akhil Goyal [mailto:gak...@marvell.com]
> Sent: Tuesday, 6 February 2024 15.25
>
> > Cache the most recent VA -> PA mapping found so that we can skip
> > most of the system calls. With 4K pages this reduces pool create
> > time by about 90%.
> >
> > Signed-off-by: Andrew Boyer
>
> I believe there should be a generic solution for this in mempool
> if it is not there already.
> Here, you are adding cache in mempool priv
> which does not seem to be a correct place.
> This optimization would be needed across all types of mempools.
> Adding more people for comments.
>
>
> > ---
> > lib/cryptodev/rte_crypto.h| 5 +
> > lib/cryptodev/rte_cryptodev.c | 23 ++-
> > 2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
> > index dbc2700da5..ee6aa1e40e 100644
> > --- a/lib/cryptodev/rte_crypto.h
> > +++ b/lib/cryptodev/rte_crypto.h
> > @@ -220,6 +220,11 @@ struct rte_crypto_op_pool_private {
> > /**< Crypto op pool type operation. */
> > uint16_t priv_size;
> > /**< Size of private area in each crypto operation. */
> > +
> > + unsigned long vp_cache;
> > + /* Virtual page address of previous op. */
> > + rte_iova_t iovp_cache;
> > + /* I/O virtual page address of previous op. */
> > };
> >
> >
> > diff --git a/lib/cryptodev/rte_cryptodev.c
> b/lib/cryptodev/rte_cryptodev.c
> > index b233c0ecd7..d596f85a57 100644
> > --- a/lib/cryptodev/rte_cryptodev.c
> > +++ b/lib/cryptodev/rte_cryptodev.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -2568,12 +2569,32 @@ rte_crypto_op_init(struct rte_mempool
> *mempool,
> > {
> > struct rte_crypto_op *op = _op_data;
> > enum rte_crypto_op_type type = *(enum rte_crypto_op_type
> > *)opaque_arg;
> > + struct rte_crypto_op_pool_private *priv;
> > + unsigned long virt_addr = (unsigned long)(uintptr_t)_op_data;
> > +#ifdef RTE_EXEC_ENV_WINDOWS
> > + unsigned long page_mask = 4095;
> > +#else
> > + unsigned long page_mask = sysconf(_SC_PAGESIZE) - 1;
> > +#endif
> > + unsigned long virt_page = virt_addr & ~page_mask;
> >
> > memset(_op_data, 0, mempool->elt_size);
> >
> > __rte_crypto_op_reset(op, type);
> >
> > - op->phys_addr = rte_mem_virt2iova(_op_data);
This optimization is for rte_mem_virt2iova(_op_data) being slow.
If I'm not mistaken, _op_data is an object in a mempool, where the mempool
object headers have already been initialized.
In this case, it could simply be optimized as this:
- op->phys_addr = rte_mem_virt2iova(_op_data);
+ op->phys_addr = rte_mempool_virt2iova(_op_data);
Now going down a rat hole...
If the above is true, I wonder if struct rte_crypto_op is only instantiated as
objects in mempools... If it is, the op->mempool and op->phys_addr fields are
fundamentally redundant, and can be retrieved from the mempool object header
instead:
op->phys_addr === rte_mempool_virt2iova(op)
op->mempool === rte_mempool_from_obj(op)
Having these shadow variables in struct rte_crypto_op may provide performance
benefits when RTE_MEMPOOL_F_NO_CACHE_ALIGN is not set on the mempool, the
mempool object header is in the preceding cache line of the mempool object
(containing the struct rte_crypto_op op).
A better solution than these shadow variables would be to introduce another
mempool flag to cache align the mempool object header, but let the mempool
object itself directly follow the mempool object header, so the mempool object
header and the mempool object itself (if small enough) reside in the same cache
line. This would also use less memory.
> > + priv = (struct rte_crypto_op_pool_private *)
> > + rte_mempool_get_priv(mempool);
> > +
> > + if (virt_page == priv->vp_cache) {
> > + op->phys_addr = priv->iovp_cache + (virt_addr & page_mask);
> > + } else {
> > + op->phys_addr = rte_mem_virt2iova(_op_data);
> > +
> > + /* Update cached values */
> > + priv->vp_cache = virt_page;
> > + priv->iovp_cache = op->phys_addr & ~page_mask;
> > + }
> > +
> > op->mempool = mempool;
> > }
> >
> > --
> > 2.17.1
Re: [PATCH v2 3/3] config/arm: allow WFE to be enabled config time
> On Feb 1, 2024, at 3:57 PM, pbhagavat...@marvell.com wrote: > > From: Pavan Nikhilesh > > Allow RTE_ARM_USE_WFE to be enabled at meson configuration > time by passing it via c_args instead of modifying > `config/arm/meson.build`. > > Example usage: > meson build -Dc_args='-DRTE_ARM_USE_WFE' \ > --cross-file config/arm/arm64_cn10k_linux_gcc > > Signed-off-by: Pavan Nikhilesh > Acked-by: Chengwen Feng > Acked-by: Ruifeng Wang > --- > config/arm/meson.build | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/config/arm/meson.build b/config/arm/meson.build > index 6f2308f2fa..3467bef466 100644 > --- a/config/arm/meson.build > +++ b/config/arm/meson.build > @@ -17,7 +17,9 @@ flags_common = [ > #['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF], > #['RTE_ARM64_MEMCPY_STRICT_ALIGN', false], > > -['RTE_ARM_USE_WFE', false], > +# Enable use of ARM wait for event instruction. > +# ['RTE_ARM_USE_WFE', false], > + So, what is the default value for RTE_ARM_USE_WFE if the user does not pass the flag at the command line? Can we do it such a way that the flag passed on the command line takes precedence? > ['RTE_ARCH_ARM64', true], > ['RTE_CACHE_LINE_SIZE', 128] > ] > -- > 2.25.1 >
Re: [EXT] [PATCH] cryptodev: add a simple mapping cache to speed up ops pool create
On Feb 6, 2024, at 9:24 PM, Morten Brørup wrote:
Caution: This message originated from an External Source. Use proper caution
when opening attachments, clicking links, or responding.
From: Akhil Goyal [mailto:gak...@marvell.com]
Sent: Tuesday, 6 February 2024 15.25
Cache the most recent VA -> PA mapping found so that we can skip
most of the system calls. With 4K pages this reduces pool create
time by about 90%.
Signed-off-by: Andrew Boyer
I believe there should be a generic solution for this in mempool
if it is not there already.
Here, you are adding cache in mempool priv
which does not seem to be a correct place.
This optimization would be needed across all types of mempools.
Adding more people for comments.
---
lib/cryptodev/rte_crypto.h| 5 +
lib/cryptodev/rte_cryptodev.c | 23 ++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
index dbc2700da5..ee6aa1e40e 100644
--- a/lib/cryptodev/rte_crypto.h
+++ b/lib/cryptodev/rte_crypto.h
@@ -220,6 +220,11 @@ struct rte_crypto_op_pool_private {
/**< Crypto op pool type operation. */
uint16_t priv_size;
/**< Size of private area in each crypto operation. */
+
+ unsigned long vp_cache;
+ /* Virtual page address of previous op. */
+ rte_iova_t iovp_cache;
+ /* I/O virtual page address of previous op. */
};
diff --git a/lib/cryptodev/rte_cryptodev.c
b/lib/cryptodev/rte_cryptodev.c
index b233c0ecd7..d596f85a57 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
@@ -2568,12 +2569,32 @@ rte_crypto_op_init(struct rte_mempool
*mempool,
{
struct rte_crypto_op *op = _op_data;
enum rte_crypto_op_type type = *(enum rte_crypto_op_type
*)opaque_arg;
+ struct rte_crypto_op_pool_private *priv;
+ unsigned long virt_addr = (unsigned long)(uintptr_t)_op_data;
+#ifdef RTE_EXEC_ENV_WINDOWS
+ unsigned long page_mask = 4095;
+#else
+ unsigned long page_mask = sysconf(_SC_PAGESIZE) - 1;
+#endif
+ unsigned long virt_page = virt_addr & ~page_mask;
memset(_op_data, 0, mempool->elt_size);
__rte_crypto_op_reset(op, type);
- op->phys_addr = rte_mem_virt2iova(_op_data);
This optimization is for rte_mem_virt2iova(_op_data) being slow.
If I'm not mistaken, _op_data is an object in a mempool, where the mempool
object headers have already been initialized.
In this case, it could simply be optimized as this:
- op->phys_addr = rte_mem_virt2iova(_op_data);
+ op->phys_addr = rte_mempool_virt2iova(_op_data);
That certainly is shorter! Thanks, I was not aware of this function.
-Andrew
Re: [PATCH 1/2] net/txgbe: add vectorized functions for Rx/Tx
On 2/1/2024 3:00 AM, Jiawen Wu wrote:
> To optimize Rx/Tx burst process, add SSE/NEON vector instructions on
> x86/arm architecture.
>
Do you have any performance improvement number with vector
implementation, if so can you put it into commit log for record?
> Signed-off-by: Jiawen Wu
> ---
> drivers/net/txgbe/meson.build | 6 +
> drivers/net/txgbe/txgbe_ethdev.c | 6 +
> drivers/net/txgbe/txgbe_ethdev.h | 1 +
> drivers/net/txgbe/txgbe_ethdev_vf.c | 1 +
> drivers/net/txgbe/txgbe_rxtx.c| 150 -
> drivers/net/txgbe/txgbe_rxtx.h| 18 +
> drivers/net/txgbe/txgbe_rxtx_vec_common.h | 301 +
> drivers/net/txgbe/txgbe_rxtx_vec_neon.c | 604 ++
> drivers/net/txgbe/txgbe_rxtx_vec_sse.c| 736 ++
> 9 files changed, 1817 insertions(+), 6 deletions(-)
> create mode 100644 drivers/net/txgbe/txgbe_rxtx_vec_common.h
> create mode 100644 drivers/net/txgbe/txgbe_rxtx_vec_neon.c
> create mode 100644 drivers/net/txgbe/txgbe_rxtx_vec_sse.c
>
> diff --git a/drivers/net/txgbe/meson.build b/drivers/net/txgbe/meson.build
> index 14729a6cf3..ba7167a511 100644
> --- a/drivers/net/txgbe/meson.build
> +++ b/drivers/net/txgbe/meson.build
> @@ -24,6 +24,12 @@ sources = files(
>
> deps += ['hash', 'security']
>
> +if arch_subdir == 'x86'
> +sources += files('txgbe_rxtx_vec_sse.c')
> +elif arch_subdir == 'arm'
> +sources += files('txgbe_rxtx_vec_neon.c')
> +endif
> +
> includes += include_directories('base')
>
> install_headers('rte_pmd_txgbe.h')
> diff --git a/drivers/net/txgbe/txgbe_ethdev.c
> b/drivers/net/txgbe/txgbe_ethdev.c
> index 6bc231a130..2d5b935002 100644
> --- a/drivers/net/txgbe/txgbe_ethdev.c
> +++ b/drivers/net/txgbe/txgbe_ethdev.c
> @@ -1544,6 +1544,7 @@ txgbe_dev_configure(struct rte_eth_dev *dev)
>* allocation Rx preconditions we will reset it.
>*/
> adapter->rx_bulk_alloc_allowed = true;
> + adapter->rx_vec_allowed = true;
>
> return 0;
> }
> @@ -2735,6 +2736,11 @@ txgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev)
> dev->rx_pkt_burst == txgbe_recv_pkts_bulk_alloc)
> return txgbe_get_supported_ptypes();
>
> +#if defined(RTE_ARCH_X86) || defined(__ARM_NEON)
> + if (dev->rx_pkt_burst == txgbe_recv_pkts_vec ||
> + dev->rx_pkt_burst == txgbe_recv_scattered_pkts_vec)
> + return txgbe_get_supported_ptypes();
> +#endif
>
Sometimes the packet parsing capability of the device changes based on
vector Rx used, but above calls same function.
If there is no ptype parsing capability difference, why not just add
above checks to previous if block?
Btw, 'txgbe_get_supported_ptypes()' now gets a parameter, based on
changes in 'next-net', can you please rebase code on top of latest next-net?
<...>
> @@ -2198,8 +2220,15 @@ txgbe_set_tx_function(struct rte_eth_dev *dev, struct
> txgbe_tx_queue *txq)
> #endif
> txq->tx_free_thresh >= RTE_PMD_TXGBE_TX_MAX_BURST) {
> PMD_INIT_LOG(DEBUG, "Using simple tx code path");
> - dev->tx_pkt_burst = txgbe_xmit_pkts_simple;
> dev->tx_pkt_prepare = NULL;
> + if (txq->tx_free_thresh <= RTE_TXGBE_TX_MAX_FREE_BUF_SZ &&
> + (rte_eal_process_type() != RTE_PROC_PRIMARY ||
>
Why vector Tx enable only for secondary process?
<...>
> @@ -297,6 +299,12 @@ struct txgbe_rx_queue {
> #ifdef RTE_LIB_SECURITY
> uint8_tusing_ipsec;
> /**< indicates that IPsec RX feature is in use */
> +#endif
> + uint64_tmbuf_initializer; /**< value to init mbufs */
> + uint8_t rx_using_sse;
> +#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM)
>
RTE_ARCH_ARM, RTE_ARCH_ARM64 & __ARM_NEON seems used interchangable,
what do you think to stick one?
Similarly with RTE_ARCH_X86_64 & RTE_ARCH_X86.
<...>
> +++ b/drivers/net/txgbe/txgbe_rxtx_vec_neon.c
> @@ -0,0 +1,604 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2015-2024 Beijing WangXun Technology Co., Ltd.
> + * Copyright(c) 2010-2015 Intel Corporation
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "txgbe_ethdev.h"
> +#include "txgbe_rxtx.h"
> +#include "txgbe_rxtx_vec_common.h"
> +
> +#pragma GCC diagnostic ignored "-Wcast-qual"
> +
Is this pragma really required?
[PATCH v3 00/13] net/ionic: miscellaneous fixes and improvements
This patchset provides miscellaneous fixes and improvements for the net/ionic driver used by AMD Pensando devices. V3: - Resend to fix patchwork threading. V2: - Update device stop and device start patches to use compound literals as suggested by review. Akshay Dorwat (1): net/ionic: fix RSS query routine Andrew Boyer (8): net/ionic: add stat for completion queue entries processed net/ionic: increase max supported MTU to 9750 bytes net/ionic: don't auto-enable Rx scatter-gather a second time net/ionic: replace non-standard type in structure definition net/ionic: fix device close sequence to avoid crash net/ionic: optimize device close operation net/ionic: optimize device stop operation net/ionic: optimize device start operation Brad Larson (1): net/ionic: add flexible firmware xstat counters Neel Patel (2): net/ionic: fix missing volatile type for cqe pointers net/ionic: memcpy descriptors when using Q-in-CMB Vamsi Krishna Atluri (1): net/ionic: report 1G and 200G link speeds when applicable drivers/net/ionic/ionic.h | 3 + drivers/net/ionic/ionic_dev.c | 9 +- drivers/net/ionic/ionic_dev.h | 8 +- drivers/net/ionic/ionic_dev_pci.c | 2 +- drivers/net/ionic/ionic_ethdev.c | 81 ++--- drivers/net/ionic/ionic_if.h | 70 drivers/net/ionic/ionic_lif.c | 233 ++ drivers/net/ionic/ionic_lif.h | 19 ++- drivers/net/ionic/ionic_main.c| 17 +- drivers/net/ionic/ionic_rxtx.c| 160 +- drivers/net/ionic/ionic_rxtx.h| 80 - drivers/net/ionic/ionic_rxtx_sg.c | 28 ++-- drivers/net/ionic/ionic_rxtx_simple.c | 28 ++-- 13 files changed, 527 insertions(+), 211 deletions(-) -- 2.17.1
[PATCH v3 01/13] net/ionic: add stat for completion queue entries processed
When completion coalescing is turned on in the FW, there will be
fewer CQE than Tx packets. Expose the stat through debug logging.
Signed-off-by: Andrew Boyer
---
drivers/net/ionic/ionic_lif.h | 1 +
drivers/net/ionic/ionic_rxtx.c| 3 +++
drivers/net/ionic/ionic_rxtx_sg.c | 2 ++
drivers/net/ionic/ionic_rxtx_simple.c | 2 ++
4 files changed, 8 insertions(+)
diff --git a/drivers/net/ionic/ionic_lif.h b/drivers/net/ionic/ionic_lif.h
index 36b3bcc5a9..cac7a4583b 100644
--- a/drivers/net/ionic/ionic_lif.h
+++ b/drivers/net/ionic/ionic_lif.h
@@ -32,6 +32,7 @@
struct ionic_tx_stats {
uint64_t packets;
uint64_t bytes;
+ uint64_t comps;
uint64_t drop;
uint64_t stop;
uint64_t no_csum;
diff --git a/drivers/net/ionic/ionic_rxtx.c b/drivers/net/ionic/ionic_rxtx.c
index b9e73b4871..d92b231f8f 100644
--- a/drivers/net/ionic/ionic_rxtx.c
+++ b/drivers/net/ionic/ionic_rxtx.c
@@ -117,6 +117,9 @@ ionic_dev_tx_queue_stop(struct rte_eth_dev *eth_dev,
uint16_t tx_queue_id)
stats = &txq->stats;
IONIC_PRINT(DEBUG, "TX queue %u pkts %ju tso %ju",
txq->qcq.q.index, stats->packets, stats->tso);
+ IONIC_PRINT(DEBUG, "TX queue %u comps %ju (%ju per)",
+ txq->qcq.q.index, stats->comps,
+ stats->comps ? stats->packets / stats->comps : 0);
return 0;
}
diff --git a/drivers/net/ionic/ionic_rxtx_sg.c
b/drivers/net/ionic/ionic_rxtx_sg.c
index ab8e56e91c..6c028a698c 100644
--- a/drivers/net/ionic/ionic_rxtx_sg.c
+++ b/drivers/net/ionic/ionic_rxtx_sg.c
@@ -26,6 +26,7 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
{
struct ionic_cq *cq = &txq->qcq.cq;
struct ionic_queue *q = &txq->qcq.q;
+ struct ionic_tx_stats *stats = &txq->stats;
struct rte_mbuf *txm;
struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
void **info;
@@ -72,6 +73,7 @@ ionic_tx_flush_sg(struct ionic_tx_qcq *txq)
}
cq_desc = &cq_desc_base[cq->tail_idx];
+ stats->comps++;
}
}
diff --git a/drivers/net/ionic/ionic_rxtx_simple.c
b/drivers/net/ionic/ionic_rxtx_simple.c
index 5f81856256..5969287b66 100644
--- a/drivers/net/ionic/ionic_rxtx_simple.c
+++ b/drivers/net/ionic/ionic_rxtx_simple.c
@@ -26,6 +26,7 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
{
struct ionic_cq *cq = &txq->qcq.cq;
struct ionic_queue *q = &txq->qcq.q;
+ struct ionic_tx_stats *stats = &txq->stats;
struct rte_mbuf *txm;
struct ionic_txq_comp *cq_desc, *cq_desc_base = cq->base;
void **info;
@@ -67,6 +68,7 @@ ionic_tx_flush(struct ionic_tx_qcq *txq)
}
cq_desc = &cq_desc_base[cq->tail_idx];
+ stats->comps++;
}
}
--
2.17.1
