[PATCH v13 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Hanumanth Pothula]

Some of the HW has support for choosing memory pools based on
the packet's size. The pool sort capability allows PMD/NIC to
choose a memory pool based on the packet's length.

On multiple mempool support enabled, populate mempool array
accordingly. Also, print pool name on which packet is received.

Signed-off-by: Hanumanth Pothula 

v13:
 - Make sure protocol-based header split feature is not broken
   by updating changes with latest code base.
v12:
 - Process multi-segment configuration on number segments
   (rx_pkt_nb_segs) greater than 1 or buffer split offload
   flag (RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) set.
v11:
 - Resolve compilation and warning.
v10:
 - Populate multi-mempool array based on mbuf_data_size_n instead
   of rx_pkt_nb_segs.
---
 app/test-pmd/testpmd.c | 65 --
 app/test-pmd/testpmd.h |  3 ++
 app/test-pmd/util.c|  4 +--
 3 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5b0f0838dc..78ea19fcbb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2647,11 +2647,19 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
   struct rte_eth_rxconf *rx_conf, struct rte_mempool *mp)
 {
union rte_eth_rxseg rx_useg[MAX_SEGS_BUFFER_SPLIT] = {};
+   struct rte_mempool *rx_mempool[MAX_MEMPOOL] = {};
+   struct rte_mempool *mpx;
unsigned int i, mp_n;
int ret;
 
-   if (rx_pkt_nb_segs <= 1 ||
-   (rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) == 0) {
+   /* Verify Rx queue configuration is single pool and segment or
+* multiple pool/segment.
+* @see rte_eth_rxconf::rx_mempools
+* @see rte_eth_rxconf::rx_seg
+*/
+   if (!(mbuf_data_size_n > 1) && !(rx_pkt_nb_segs > 1 ||
+   ((rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) != 0))) {
+   /* Single pool/segment configuration */
rx_conf->rx_seg = NULL;
rx_conf->rx_nseg = 0;
ret = rte_eth_rx_queue_setup(port_id, rx_queue_id,
@@ -2659,33 +2667,46 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 rx_conf, mp);
goto exit;
}
-   for (i = 0; i < rx_pkt_nb_segs; i++) {
-   struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
-   struct rte_mempool *mpx;
-   /*
-* Use last valid pool for the segments with number
-* exceeding the pool index.
-*/
-   mp_n = (i >= mbuf_data_size_n) ? mbuf_data_size_n - 1 : i;
-   mpx = mbuf_pool_find(socket_id, mp_n);
-   /* Handle zero as mbuf data buffer size. */
-   rx_seg->offset = i < rx_pkt_nb_offs ?
-  rx_pkt_seg_offsets[i] : 0;
-   rx_seg->mp = mpx ? mpx : mp;
-   if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] == 0) {
-   rx_seg->proto_hdr = rx_pkt_hdr_protos[i];
-   } else {
-   rx_seg->length = rx_pkt_seg_lengths[i] ?
-   rx_pkt_seg_lengths[i] :
-   mbuf_data_size[mp_n];
+
+   if (rx_pkt_nb_segs > 1 ||
+   rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) {
+   for (i = 0; i < rx_pkt_nb_segs; i++) {
+   struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
+   /*
+* Use last valid pool for the segments with number
+* exceeding the pool index.
+*/
+   mp_n = (i >= mbuf_data_size_n) ? mbuf_data_size_n - 1 : 
i;
+   mpx = mbuf_pool_find(socket_id, mp_n);
+   /* Handle zero as mbuf data buffer size. */
+   rx_seg->offset = i < rx_pkt_nb_offs ?
+  rx_pkt_seg_offsets[i] : 0;
+   rx_seg->mp = mpx ? mpx : mp;
+   if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] 
== 0) {
+   rx_seg->proto_hdr = rx_pkt_hdr_protos[i];
+   } else {
+   rx_seg->length = rx_pkt_seg_lengths[i] ?
+   rx_pkt_seg_lengths[i] :
+   mbuf_data_size[mp_n];
+   }
}
-   }
rx_conf->rx_nseg = rx_pkt_nb_segs;
rx_conf->rx_seg = rx_useg;
+   } else {
+   /* multi-pool configuration */
+   for (i = 0; i < mbuf_data_size_n; i++) {
+   mpx = mbuf_pool_find(socket_id, i);
+   rx_mempool[i] = mpx ? mpx : mp;
+   }
+   rx_conf->rx_mempools = rx_mempool;
+   rx_c

[PATCH v5] app/testpmd: fix protocol header display for Rx buffer split

[Yuan Wang]

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows inner-ipv6 as inner-ipv4.

This patch fixes that by redefining rx_pkt_hdr_protos to hold
the full ptypes, and the show and set commands therefore
remain symmetrical.

Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang 
---
v5:
- remove unrelated changes.
v4:
- redefine rx_pkt_hdr_protos to hold the full ptypes.
- use single switch in get_ptype_str().
v3:
- use RTE_PTYPE_*_MASK as masks.
- refactor to use switch statement.
v2:
- add fixline.

---
 app/test-pmd/cmdline.c |   6 +-
 app/test-pmd/config.c  | 140 +
 app/test-pmd/testpmd.c |   4 +-
 3 files changed, 77 insertions(+), 73 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 8dc60e9388..8ffd62cac9 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3445,18 +3445,14 @@ parse_hdrs_list(const char *str, const char *item_name, 
unsigned int max_items,
unsigned int nb_item;
char *cur;
char *tmp;
-   unsigned int cur_item, prev_items = 0;
 
nb_item = 0;
char *str2 = strdup(str);
cur = strtok_r(str2, ",", &tmp);
while (cur != NULL) {
-   cur_item = get_ptype(cur);
-   cur_item &= ~prev_items;
-   parsed_items[nb_item] = cur_item;
+   parsed_items[nb_item] = get_ptype(cur);
cur = strtok_r(NULL, ",", &tmp);
nb_item++;
-   prev_items |= cur_item;
}
if (nb_item > max_items)
fprintf(stderr, "Number of %s = %u > %u (maximum items)\n",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e8a1b77c2a..84ebada8fd 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -5070,73 +5070,79 @@ show_rx_pkt_segments(void)
 
 static const char *get_ptype_str(uint32_t ptype)
 {
-   if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-   (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
-   return "ipv4-tcp";
-   else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-   (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
-   return "ipv4-udp";
-   else if ((ptype & (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) 
==
-   (RTE_PTYPE_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
-   return "ipv4-sctp";
-   else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP)) ==
-   (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_TCP))
-   return "ipv6-tcp";
-   else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP)) ==
-   (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_UDP))
-   return "ipv6-udp";
-   else if ((ptype & (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP)) 
==
-   (RTE_PTYPE_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_L4_SCTP))
-   return "ipv6-sctp";
-   else if ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP)
-   return "tcp";
-   else if ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)
-   return "udp";
-   else if ((ptype & RTE_PTYPE_L4_SCTP) == RTE_PTYPE_L4_SCTP)
-   return "sctp";
-   else if ((ptype & RTE_PTYPE_L3_IPV4_EXT_UNKNOWN) == 
RTE_PTYPE_L3_IPV4_EXT_UNKNOWN)
-   return "ipv4";
-   else if ((ptype & RTE_PTYPE_L3_IPV6_EXT_UNKNOWN) == 
RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
-   return "ipv6";
-   else if ((ptype & RTE_PTYPE_L2_ETHER) == RTE_PTYPE_L2_ETHER)
-   return "eth";
-
-   else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | 
RTE_PTYPE_INNER_L4_TCP)) ==
-   (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-   return "inner-ipv4-tcp";
-   else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | 
RTE_PTYPE_INNER_L4_UDP)) ==
-   (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-   return "inner-ipv4-udp";
-   else if ((ptype & (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | 
RTE_PTYPE_INNER_L4_SCTP)) ==
-   (RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP))
-   return "inner-ipv4-sctp";
-   else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | 
RTE_PTYPE_INNER_L4_TCP)) ==
-   (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_TCP))
-   return "inner-ipv6-tcp";
-   else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | 
RTE_PTYPE_INNER_L4_UDP)) ==
-   (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_UDP))
-   return "inner-ipv6-udp";
-   else if ((ptype & (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | 
RTE_PTYPE_INNER_L4_SCTP)) ==
-   (RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN | RTE_PTYPE_INNER_L4_SCTP)

[PATCH v2] net/iavf: fix taninted scalar

[Steve Yang]

tainted_data_downcast: Downcasting match_item->meta from void * to
struct virtchnl_proto_hdrs implies that the data that this pointer points
to is tainted.

var_assign_var: Assigning: proto_hdrs = match_item->meta.
Both are now tainted.

var_assign_var: Assigning: rss_meta->proto_hdrs = *proto_hdrs. Both are
now tainted.

Passing tainted expression "rss_meta->proto_hdrs.count" to
"iavf_refine_proto_hdrs", which uses it as a loop boundary.

Removed temporary variable 'proto_hdrs', and copied whole memory of
match_item meta with exact structure size to avoid data downcast.

Coverity issue: 381131

Fixes: 91f27b2e39ab ("net/iavf: refactor RSS")

Signed-off-by: Steve Yang 
---
 drivers/net/iavf/iavf_hash.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index 67b05313eb..ae6fb38594 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -992,10 +992,9 @@ iavf_refine_proto_hdrs_l234(struct virtchnl_proto_hdrs 
*proto_hdrs,
uint64_t rss_type)
 {
struct virtchnl_proto_hdr *hdr;
-   int phdrs_count = proto_hdrs->count;
int i;
 
-   for (i = 0; i < phdrs_count; i++) {
+   for (i = 0; i < proto_hdrs->count; i++) {
hdr = &proto_hdrs->proto_hdr[i];
switch (hdr->type) {
case VIRTCHNL_PROTO_HDR_ETH:
@@ -1184,13 +1183,12 @@ iavf_refine_proto_hdrs_gtpu(struct virtchnl_proto_hdrs 
*proto_hdrs,
uint64_t rss_type)
 {
struct virtchnl_proto_hdr *hdr;
-   int phdrs_count = proto_hdrs->count;
int i;
 
if (!(rss_type & RTE_ETH_RSS_GTPU))
return;
 
-   for (i = 0; i < phdrs_count; i++) {
+   for (i = 0; i < proto_hdrs->count; i++) {
hdr = &proto_hdrs->proto_hdr[i];
switch (hdr->type) {
case VIRTCHNL_PROTO_HDR_GTPU_IP:
@@ -1210,7 +1208,6 @@ iavf_refine_proto_hdrs_by_pattern(struct 
virtchnl_proto_hdrs *proto_hdrs,
struct virtchnl_proto_hdr *hdr2;
int i, shift_count = 1;
int tun_lvl = proto_hdrs->tunnel_level;
-   int phdrs_count = 0;
 
if (!(phint & IAVF_PHINT_GTPU_MSK) && !(phint & IAVF_PHINT_GRE))
return;
@@ -1219,9 +1216,8 @@ iavf_refine_proto_hdrs_by_pattern(struct 
virtchnl_proto_hdrs *proto_hdrs,
if (phint & IAVF_PHINT_LAYERS_MSK)
shift_count = 2;
 
-   phdrs_count = proto_hdrs->count;
/* shift headers layer */
-   for (i = phdrs_count - 1 + shift_count;
+   for (i = proto_hdrs->count - 1 + shift_count;
 i > shift_count - 1; i--) {
hdr1 = &proto_hdrs->proto_hdr[i];
hdr2 = &proto_hdrs->proto_hdr[i - shift_count];
@@ -1282,7 +1278,6 @@ iavf_refine_proto_hdrs_l2tpv2(struct virtchnl_proto_hdrs 
*proto_hdrs,
  uint64_t phint)
 {
struct virtchnl_proto_hdr *hdr, *hdr1;
-   int phdrs_count = proto_hdrs->count;
int i;
 
if (!(phint & IAVF_PHINT_L2TPV2) && !(phint & IAVF_PHINT_L2TPV2_LEN))
@@ -1290,7 +1285,7 @@ iavf_refine_proto_hdrs_l2tpv2(struct virtchnl_proto_hdrs 
*proto_hdrs,
 
if (proto_hdrs->tunnel_level == TUNNEL_LEVEL_INNER) {
/* shift headers layer */
-   for (i = phdrs_count; i > 0; i--)
+   for (i = proto_hdrs->count; i > 0; i--)
proto_hdrs->proto_hdr[i] = proto_hdrs->proto_hdr[i - 1];
 
/* adding outer ip header at layer 0 */
@@ -1303,7 +1298,7 @@ iavf_refine_proto_hdrs_l2tpv2(struct virtchnl_proto_hdrs 
*proto_hdrs,
else if (phint & IAVF_PHINT_OUTER_IPV6)
VIRTCHNL_SET_PROTO_HDR_TYPE(hdr1, IPV6);
} else {
-   for (i = 0; i < phdrs_count; i++) {
+   for (i = 0; i < proto_hdrs->count; i++) {
hdr = &proto_hdrs->proto_hdr[i];
if (hdr->type == VIRTCHNL_PROTO_HDR_L2TPV2) {
if (phint & IAVF_PHINT_L2TPV2) {
@@ -1427,7 +1422,6 @@ iavf_hash_parse_action(struct iavf_pattern_match_item 
*match_item,
   uint64_t pattern_hint, struct iavf_rss_meta *rss_meta,
   struct rte_flow_error *error)
 {
-   struct virtchnl_proto_hdrs *proto_hdrs;
enum rte_flow_action_type action_type;
const struct rte_flow_action_rss *rss;
const struct rte_flow_action *action;
@@ -1488,8 +1482,10 @@ iavf_hash_parse_action(struct iavf_pattern_match_item 
*match_item,
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION,
action, "RSS type not 
supported");
-   proto_hdrs = match_item->m

Re: [PATCH v5] app/testpmd: fix protocol header display for Rx buffer split

[Andrew Rybchenko]

On 11/10/22 11:20, Yuan Wang wrote:

The "show config rxhdrs" cmd displays the configured protocol headers
that are used for protocol-based buffer split.
However, it shows inner-ipv6 as inner-ipv4.

This patch fixes that by redefining rx_pkt_hdr_protos to hold
the full ptypes, and the show and set commands therefore
remain symmetrical.

Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang 


Reviewed-by: Andrew Rybchenko 

Applied to dpdk-next-net/main, thanks.

Re: [PATCH v13 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Andrew Rybchenko]

On 11/10/22 11:17, Hanumanth Pothula wrote:

Some of the HW has support for choosing memory pools based on
the packet's size. The pool sort capability allows PMD/NIC to
choose a memory pool based on the packet's length.

On multiple mempool support enabled, populate mempool array
accordingly. Also, print pool name on which packet is received.

Signed-off-by: Hanumanth Pothula 

v13:
  - Make sure protocol-based header split feature is not broken
by updating changes with latest code base.
v12:
  - Process multi-segment configuration on number segments
(rx_pkt_nb_segs) greater than 1 or buffer split offload
flag (RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) set.
v11:
  - Resolve compilation and warning.
v10:
  - Populate multi-mempool array based on mbuf_data_size_n instead
of rx_pkt_nb_segs.


I'm sorry for inconvenience, could you rebase the patch on
current next-net/main, please. I've decided to apply protocol
based buffer split fix first. Of course, I can rebase myself,
but I want result to be checked very carefully and tested
properly. Thanks.

RE: [PATCH v1 2/2] doc: increase python max line to 88

[Juraj Linkeš]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Friday, November 4, 2022 5:58 PM
> To: Juraj Linkeš 
> Cc: Honnappa Nagarahalli ; Owen Hilyard
> ; tho...@monjalon.net; Lijuan Tu
> ; Richardson, Bruce ;
> dev@dpdk.org
> Subject: Re: [PATCH v1 2/2] doc: increase python max line to 88
> 
> On Fri, 4 Nov 2022 09:16:13 +
> Juraj Linkeš  wrote:
> 
> > > +max_line_length = 88 #
> > >
> +https://black.readthedocs.io/en/stable/the_black_code_style/current_sty
> > > +le.html#li
> 
> Skip the comment, it caused your line break!

The fact that the line is a bit longer does not make the line less readable, as 
the link is there to be copy-pasted (and I don't think anyone reads the full 
hyperlinks - the knowledge of domain is enough). As such I think it's better to 
include the link as it serves as self-documentation (that we're deviating from 
the standard).

We could move the comment before or after the max_line_length option and that 
would result in lines below 100 characters (which is what .editorconfig 
prescribes). I used that in my one of my local versions, but it was less 
readable in my opinion. I'd rather break the rule and have it be more readable.

Of course, not having the comment is fine, since we document it in the coding 
style guide. I just think there's no (or very little) downside and some upside 
(more than downside) in adding the comment.

Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[Maxime Coquelin]

Hi Taekyung,

Adding Thomas and Ali who maintains the patchwork instance.

On 11/10/22 05:02, Taekyung Kim wrote:

On Thu, Nov 10, 2022 at 01:53:50AM +, Xia, Chenbo wrote:

Hi Kim,


-Original Message-
From: Taekyung Kim 
Sent: Tuesday, November 8, 2022 4:56 PM
To: dev@dpdk.org
Cc: Xia, Chenbo ; Pei, Andy ;
kim.tae.ky...@navercorp.com; maxime.coque...@redhat.com; sta...@dpdk.org;
Wang, Xiao W 
Subject: [PATCH v4] vdpa/ifc: fix update_datapath error handling

Stop and return the error code when update_datapath fails.
update_datapath prepares resources for the vdpa device.
The driver should not perform any further actions
if update_datapath returns an error.

Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
Cc: sta...@dpdk.org

Signed-off-by: Taekyung Kim 
---
v4:
* Add rte_vdpa_unregister_device in ifcvf_pci_probe

v3:
* Fix coding style

v2:
* Revert the prepared resources before returning an error
* Rebase to 22.11 rc2
* Add fixes and cc for backport

---
  drivers/vdpa/ifc/ifcvf_vdpa.c | 27 +++
  1 file changed, 23 insertions(+), 4 deletions(-)


I can't find your patch in patchwork:

http://patchwork.dpdk.org/project/dpdk/list/?series=&submitter=2877&state=*&q=&archive=both&delegate=

so it's difficult to review and merge. Do you know why or is it possible
that you send a new version to make it show on Patchwork today?

Thanks,
Chenbo



Hi Chenbo,

First, thanks for your review.
I will send a new version for this patch soon.

I think the mail for v4 is lost.
Whenever I send a patch, I received "Your message to dev awaits moderator 
approval"
from dev-ow...@dpdk.org with the reason "Post by non-member to a members-only 
list".
Maybe, the reason is that this is the first time that I submit a patch.


No, I don't think subscription is needed, there should be another issue.
Ali & Thomas, any idea why it happens?

Thanks,
Maxime


Thanks,
Taekyung

Is it correct to report checksum good when there is no checksum?

[Andrew Rybchenko]

Hi all,

some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
For me it looks strange, but I see some technical reasons behind.
Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
Should UNKNOWN or NONE be used instead?

Thanks,
Andrew.

RE: [PATCH] devtools: set DTS directory to format check

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Wednesday, November 9, 2022 6:09 PM
> To: dev@dpdk.org
> Cc: Juraj Linkeš ; Lijuan Tu
> ; Owen Hilyard 
> Subject: [PATCH] devtools: set DTS directory to format check
> 
> The script was running on the current directory.
> If not in the DTS directory, it would re-format every Python files.
> 
> A new positional argument is added to specify the directory to check.
> In most cases, the (new) default value should be enough.
> 
> While updating argument handling,
> the usage is printed in case of wrong argument.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  devtools/dts-check-format.sh | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
> index dc07150775..eb1bdd2a01 100755
> --- a/devtools/dts-check-format.sh
> +++ b/devtools/dts-check-format.sh
> @@ -3,11 +3,10 @@
>  # Copyright(c) 2022 University of New Hampshire
> 
>  usage() {
> - echo "Run formatting and linting programs for DTS. Usage:"
> -
> + echo 'Usage: $(basename $0) [options] [directory]'

Double quotes here, otherwise $0 won't be expanded.

> + echo 'Options:'
>   # Get source code comments after getopts arguments and print
> them both
>   grep -E '[a-zA-Z]+\) +#' "$0" | tr -d '#'
> - exit 0
>  }
> 
>  format=true
> @@ -17,7 +16,9 @@ lint=true
>  while getopts "hfl" arg; do
>   case $arg in
>   h) # Display this message
> + echo 'Run formatting and linting programs for DTS.'
>   usage
> + exit 0
>   ;;
>   f) # Don't run formatters
>   format=false
> @@ -25,10 +26,15 @@ while getopts "hfl" arg; do
>   l) # Don't run linter
>   lint=false
>   ;;
> - *)
> + ?)
> + usage
> + exit 1
>   esac
>  done
> +shift $(($OPTIND - 1))
> 
> +directory=${1:-$(dirname $0)/../dts}
> +cd $directory || exit 1
> 

I'd like to include the information of where we're doing the fomatting in the 
console output, e.g.:
echo "Formatting in $(pwd):"

We're silently chaning the directory, so this would be useful when running with 
no argument and the script doesn't change anything - as a confirmation that it 
ran over the files we wanted to.

>  errors=0
> 
> --
> 2.36.1
> 

Other than that,
Reviewed-by: Juraj Linkeš 
Tested-by: Juraj Linkeš 

RE: [EXT] Re: [PATCH v13 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Hanumanth Reddy Pothula]

> -Original Message-
> From: Andrew Rybchenko 
> Sent: Thursday, November 10, 2022 2:31 PM
> To: Hanumanth Reddy Pothula ; Aman Singh
> ; Yuying Zhang 
> Cc: dev@dpdk.org; tho...@monjalon.net; Jerin Jacob Kollanukkaran
> ; Nithin Kumar Dabilpuram
> 
> Subject: [EXT] Re: [PATCH v13 1/1] app/testpmd: support multiple mbuf
> pools per Rx queue
> 
> External Email
> 
> --
> On 11/10/22 11:17, Hanumanth Pothula wrote:
> > Some of the HW has support for choosing memory pools based on the
> > packet's size. The pool sort capability allows PMD/NIC to choose a
> > memory pool based on the packet's length.
> >
> > On multiple mempool support enabled, populate mempool array
> > accordingly. Also, print pool name on which packet is received.
> >
> > Signed-off-by: Hanumanth Pothula 
> >
> > v13:
> >   - Make sure protocol-based header split feature is not broken
> > by updating changes with latest code base.
> > v12:
> >   - Process multi-segment configuration on number segments
> > (rx_pkt_nb_segs) greater than 1 or buffer split offload
> > flag (RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) set.
> > v11:
> >   - Resolve compilation and warning.
> > v10:
> >   - Populate multi-mempool array based on mbuf_data_size_n instead
> > of rx_pkt_nb_segs.
> 
> I'm sorry for inconvenience, could you rebase the patch on current next-
> net/main, please. I've decided to apply protocol based buffer split fix first.
> Of course, I can rebase myself, but I want result to be checked very carefully
> and tested properly. Thanks.
Sure will do that.

RE: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[Ali Alnubani]

> -Original Message-
> From: Maxime Coquelin 
> Sent: Thursday, November 10, 2022 11:20 AM
> To: Taekyung Kim ; NBU-Contact-Thomas
> Monjalon (EXTERNAL) ; Ali Alnubani
> 
> Cc: dev@dpdk.org; Pei, Andy ; sta...@dpdk.org;
> Wang, Xiao W ; Xia, Chenbo
> 
> Subject: Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling
> 
> Hi Taekyung,
> 
> Adding Thomas and Ali who maintains the patchwork instance.
> 
> On 11/10/22 05:02, Taekyung Kim wrote:
> > On Thu, Nov 10, 2022 at 01:53:50AM +, Xia, Chenbo wrote:
> >> Hi Kim,
> >>
> >>> -Original Message-
> >>> From: Taekyung Kim 
> >>> Sent: Tuesday, November 8, 2022 4:56 PM
> >>> To: dev@dpdk.org
> >>> Cc: Xia, Chenbo ; Pei, Andy
> ;
> >>> kim.tae.ky...@navercorp.com; maxime.coque...@redhat.com;
> sta...@dpdk.org;
> >>> Wang, Xiao W 
> >>> Subject: [PATCH v4] vdpa/ifc: fix update_datapath error handling
> >>>
> >>> Stop and return the error code when update_datapath fails.
> >>> update_datapath prepares resources for the vdpa device.
> >>> The driver should not perform any further actions
> >>> if update_datapath returns an error.
> >>>
> >>> Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> >>> Cc: sta...@dpdk.org
> >>>
> >>> Signed-off-by: Taekyung Kim 
> >>> ---
> >>> v4:
> >>> * Add rte_vdpa_unregister_device in ifcvf_pci_probe
> >>>
> >>> v3:
> >>> * Fix coding style
> >>>
> >>> v2:
> >>> * Revert the prepared resources before returning an error
> >>> * Rebase to 22.11 rc2
> >>> * Add fixes and cc for backport
> >>>
> >>> ---
> >>>   drivers/vdpa/ifc/ifcvf_vdpa.c | 27 +++
> >>>   1 file changed, 23 insertions(+), 4 deletions(-)
> >>
> >> I can't find your patch in patchwork:
> >>
> >>
> http://patchwork.dpdk.org/project/dpdk/list/?series=&submitter=2877&sta
> te=*&q=&archive=both&delegate=
> >>
> >> so it's difficult to review and merge. Do you know why or is it possible
> >> that you send a new version to make it show on Patchwork today?
> >>
> >> Thanks,
> >> Chenbo
> >>
> >
> > Hi Chenbo,
> >
> > First, thanks for your review.
> > I will send a new version for this patch soon.
> >
> > I think the mail for v4 is lost.
> > Whenever I send a patch, I received "Your message to dev awaits
> moderator approval"
> > from dev-ow...@dpdk.org with the reason "Post by non-member to a
> members-only list".
> > Maybe, the reason is that this is the first time that I submit a patch.
> 
> No, I don't think subscription is needed, there should be another issue.
> Ali & Thomas, any idea why it happens?
> 

Hello,

Subscription to the dev mailing list is required for posting without moderator 
approval.
I see that Taekyung is a member only since Nov 08. Postings prior to his 
subscription are waiting moderation.

Thanks,
Ali

Re: [PATCH v3] vdpa/ifc: fix update_datapath error handling

[Taekyung Kim]

On Tue, Nov 08, 2022 at 07:56:18AM +, Xia, Chenbo wrote:
> > -Original Message-
> > From: Pei, Andy 
> > Sent: Tuesday, November 8, 2022 3:39 PM
> > To: Xia, Chenbo ; Taekyung Kim
> > ; dev@dpdk.org
> > Cc: sta...@dpdk.org; maxime.coque...@redhat.com; Wang, Xiao W
> > 
> > Subject: RE: [PATCH v3] vdpa/ifc: fix update_datapath error handling
> > 
> > Hi
> > 
> > See my reply inline.
> > 
> > > -Original Message-
> > > From: Xia, Chenbo 
> > > Sent: Tuesday, November 8, 2022 9:47 AM
> > > To: Taekyung Kim ; dev@dpdk.org
> > > Cc: sta...@dpdk.org; maxime.coque...@redhat.com; Wang, Xiao W
> > > 
> > > Subject: RE: [PATCH v3] vdpa/ifc: fix update_datapath error handling
> > >
> > > > -Original Message-
> > > > From: Taekyung Kim 
> > > > Sent: Monday, November 7, 2022 5:00 PM
> > > > To: dev@dpdk.org
> > > > Cc: sta...@dpdk.org; maxime.coque...@redhat.com; Xia, Chenbo
> > > > ; Wang, Xiao W ;
> > > > kim.tae.ky...@navercorp.com
> > > > Subject: [PATCH v3] vdpa/ifc: fix update_datapath error handling
> > > >
> > > > Stop and return the error code when update_datapath fails.
> > > > update_datapath prepares resources for the vdpa device.
> > > > The driver should not perform any further actions if update_datapath
> > > > returns an error.
> > > >
> > > > Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Taekyung Kim 
> > > > ---
> > > > v3:
> > > > * Fix coding style
> > > >
> > > > v2:
> > > > * Revert the prepared resources before returning an error
> > > > * Rebase to 22.11 rc2
> > > > * Add fixes and cc for backport
> > > >
> > > > ---
> > > >  drivers/vdpa/ifc/ifcvf_vdpa.c | 26 ++
> > > >  1 file changed, 22 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > > > b/drivers/vdpa/ifc/ifcvf_vdpa.c index 8dfd49336e..0396d49122 100644
> > > > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > > > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> > > > @@ -1098,7 +1098,12 @@ ifcvf_dev_config(int vid)
> > > > internal = list->internal;
> > > > internal->vid = vid;
> > > > rte_atomic32_set(&internal->dev_attached, 1);
> > > > -   update_datapath(internal);
> > > > +   if (update_datapath(internal) < 0) {
> > > > +   DRV_LOG(ERR, "failed to update datapath for vDPA device 
> > > > %s",
> > > > +   vdev->device->name);
> > > > +   rte_atomic32_set(&internal->dev_attached, 0);
> > > > +   return -1;
> > > > +   }
> > > >
> > > > hw = &internal->hw;
> > > > for (i = 0; i < hw->nr_vring; i++) { @@ -1146,7 +1151,12 @@
> > > > ifcvf_dev_close(int vid)
> > > > internal->sw_fallback_running = false;
> > > > } else {
> > > > rte_atomic32_set(&internal->dev_attached, 0);
> > > > -   update_datapath(internal);
> > > > +   if (update_datapath(internal) < 0) {
> > > > +   DRV_LOG(ERR, "failed to update datapath for vDPA
> > > > device %s",
> > > > +   vdev->device->name);
> > > > +   internal->configured = 0;
> > > > +   return -1;
> > > > +   }
> > > > }
> > > >
> > > > internal->configured = 0;
> > > > @@ -1752,7 +1762,14 @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv
> > > > __rte_unused,
> > > > }
> > > >
> > > > rte_atomic32_set(&internal->started, 1);
> > > > -   update_datapath(internal);
> > > > +   if (update_datapath(internal) < 0) {
> > > > +   DRV_LOG(ERR, "failed to update datapath %s", 
> > > > pci_dev->name);
> > > > +   rte_atomic32_set(&internal->started, 0);
> > > > +   pthread_mutex_lock(&internal_list_lock);
> > > > +   TAILQ_REMOVE(&internal_list, list, next);
> > > > +   pthread_mutex_unlock(&internal_list_lock);
> > > > +   goto error;
> > > > +   }
> > > >
> > 
> > Is it necessary to unregister vdpa device?
> 
> Good catch, yes it's needed.
> 
> Kim, please add the unregistration.
> 
> Thanks,
> Chenbo

Hi Andy and Chenbo,

Thanks for your comments.
I forgot to add `rte_vdpa_unregister_device(internal->vdev)`.
I will send a new patch soon.

By the way, it seems that deallocation for `ifcvf_vfio_setup(internal)`
is also ommitted in `ifcvf_pci_probe(...)`.
I will submit another commit to split `error:` into `error2:` and `error1:`,
which calls `rte_pci_unmap_device(...)` and `rte_vfio_container_destroy(...)`.

Thanks,
Taekyung

> 
> > 
> > > > rte_kvargs_free(kvlist);
> > > > return 0;
> > > > @@ -1781,7 +1798,8 @@ ifcvf_pci_remove(struct rte_pci_device *pci_dev)
> > > >
> > > > internal = list->internal;
> > > > rte_atomic32_set(&internal->started, 0);
> > > > -   update_datapath(internal);
> > > > +   if (update_datapath(internal) < 0)
> > > > + 

[PATCH v4] vdpa/ifc: fix update_datapath error handling

[Taekyung Kim]

Stop and return the error code when update_datapath fails.
update_datapath prepares resources for the vdpa device.
The driver should not perform any further actions
if update_datapath returns an error.

Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
Cc: sta...@dpdk.org

Signed-off-by: Taekyung Kim 
---
v4:
* Add rte_vdpa_unregister_device in ifcvf_pci_probe

v3:
* Fix coding style

v2:
* Revert the prepared resources before returning an error
* Rebase to 22.11 rc2
* Add fixes and cc for backport

---
 drivers/vdpa/ifc/ifcvf_vdpa.c | 27 +++
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 8dfd49336e..49d68ad1b1 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -1098,7 +1098,12 @@ ifcvf_dev_config(int vid)
internal = list->internal;
internal->vid = vid;
rte_atomic32_set(&internal->dev_attached, 1);
-   update_datapath(internal);
+   if (update_datapath(internal) < 0) {
+   DRV_LOG(ERR, "failed to update datapath for vDPA device %s",
+   vdev->device->name);
+   rte_atomic32_set(&internal->dev_attached, 0);
+   return -1;
+   }
 
hw = &internal->hw;
for (i = 0; i < hw->nr_vring; i++) {
@@ -1146,7 +1151,12 @@ ifcvf_dev_close(int vid)
internal->sw_fallback_running = false;
} else {
rte_atomic32_set(&internal->dev_attached, 0);
-   update_datapath(internal);
+   if (update_datapath(internal) < 0) {
+   DRV_LOG(ERR, "failed to update datapath for vDPA device 
%s",
+   vdev->device->name);
+   internal->configured = 0;
+   return -1;
+   }
}
 
internal->configured = 0;
@@ -1752,7 +1762,15 @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv 
__rte_unused,
}
 
rte_atomic32_set(&internal->started, 1);
-   update_datapath(internal);
+   if (update_datapath(internal) < 0) {
+   DRV_LOG(ERR, "failed to update datapath %s", pci_dev->name);
+   rte_atomic32_set(&internal->started, 0);
+   rte_vdpa_unregister_device(internal->vdev);
+   pthread_mutex_lock(&internal_list_lock);
+   TAILQ_REMOVE(&internal_list, list, next);
+   pthread_mutex_unlock(&internal_list_lock);
+   goto error;
+   }
 
rte_kvargs_free(kvlist);
return 0;
@@ -1781,7 +1799,8 @@ ifcvf_pci_remove(struct rte_pci_device *pci_dev)
 
internal = list->internal;
rte_atomic32_set(&internal->started, 0);
-   update_datapath(internal);
+   if (update_datapath(internal) < 0)
+   DRV_LOG(ERR, "failed to update datapath %s", pci_dev->name);
 
rte_pci_unmap_device(internal->pdev);
rte_vfio_container_destroy(internal->vfio_container_fd);
-- 
2.34.1

Re: [PATCH] maintainers: update for gve

[Rushil Gupta]

Thanks a lot Junfeng!

On Tue, Nov 8, 2022 at 11:26 PM Junfeng Guo  wrote:

> Add co-maintainers from Google team for gve (Google Virtual Ethernet).
>
> Signed-off-by: Junfeng Guo 
> ---
>  MAINTAINERS | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1c9922123e..d8c1d5272b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -698,6 +698,9 @@ F: doc/guides/nics/features/enic.ini
>
>  Google Virtual Ethernet
>  M: Junfeng Guo 
> +M: Jeroen de Borst 
> +M: Rushil Gupta 
> +M: Jordan Kimbrough 
>  F: drivers/net/gve/
>  F: doc/guides/nics/gve.rst
>  F: doc/guides/nics/features/gve.ini
> --
> 2.34.1
>
>

RE: [EXT] [PATCH v2] doc: support IPsec Multi-buffer lib v1.3

[Akhil Goyal]

> Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD
> documentation
> guides with information about the latest Intel IPSec Multi-buffer
> library supported.
> 
> Signed-off-by: Pablo de Lara 
> ---
Do you want to highlight in release notes as well?

Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[David Marchand]

On Thu, Nov 10, 2022 at 10:34 AM Ali Alnubani  wrote:
> > > I think the mail for v4 is lost.
> > > Whenever I send a patch, I received "Your message to dev awaits
> > moderator approval"
> > > from dev-ow...@dpdk.org with the reason "Post by non-member to a
> > members-only list".
> > > Maybe, the reason is that this is the first time that I submit a patch.
> >
> > No, I don't think subscription is needed, there should be another issue.
> > Ali & Thomas, any idea why it happens?
> >
>
> Hello,
>
> Subscription to the dev mailing list is required for posting without 
> moderator approval.
> I see that Taekyung is a member only since Nov 08. Postings prior to his 
> subscription are waiting moderation.

Indeed, I just flushed the queue.


-- 
David Marchand

Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[Maxime Coquelin]

On 11/10/22 10:34, Ali Alnubani wrote:

-Original Message-
From: Maxime Coquelin 
Sent: Thursday, November 10, 2022 11:20 AM
To: Taekyung Kim ; NBU-Contact-Thomas
Monjalon (EXTERNAL) ; Ali Alnubani

Cc: dev@dpdk.org; Pei, Andy ; sta...@dpdk.org;
Wang, Xiao W ; Xia, Chenbo

Subject: Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

Hi Taekyung,

Adding Thomas and Ali who maintains the patchwork instance.

On 11/10/22 05:02, Taekyung Kim wrote:

On Thu, Nov 10, 2022 at 01:53:50AM +, Xia, Chenbo wrote:

Hi Kim,


-Original Message-
From: Taekyung Kim 
Sent: Tuesday, November 8, 2022 4:56 PM
To: dev@dpdk.org
Cc: Xia, Chenbo ; Pei, Andy

;

kim.tae.ky...@navercorp.com; maxime.coque...@redhat.com;

sta...@dpdk.org;

Wang, Xiao W 
Subject: [PATCH v4] vdpa/ifc: fix update_datapath error handling

Stop and return the error code when update_datapath fails.
update_datapath prepares resources for the vdpa device.
The driver should not perform any further actions
if update_datapath returns an error.

Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
Cc: sta...@dpdk.org

Signed-off-by: Taekyung Kim 
---
v4:
* Add rte_vdpa_unregister_device in ifcvf_pci_probe

v3:
* Fix coding style

v2:
* Revert the prepared resources before returning an error
* Rebase to 22.11 rc2
* Add fixes and cc for backport

---
   drivers/vdpa/ifc/ifcvf_vdpa.c | 27 +++
   1 file changed, 23 insertions(+), 4 deletions(-)


I can't find your patch in patchwork:



http://patchwork.dpdk.org/project/dpdk/list/?series=&submitter=2877&sta
te=*&q=&archive=both&delegate=


so it's difficult to review and merge. Do you know why or is it possible
that you send a new version to make it show on Patchwork today?

Thanks,
Chenbo



Hi Chenbo,

First, thanks for your review.
I will send a new version for this patch soon.

I think the mail for v4 is lost.
Whenever I send a patch, I received "Your message to dev awaits

moderator approval"

from dev-ow...@dpdk.org with the reason "Post by non-member to a

members-only list".

Maybe, the reason is that this is the first time that I submit a patch.


No, I don't think subscription is needed, there should be another issue.
Ali & Thomas, any idea why it happens?



Hello,

Subscription to the dev mailing list is required for posting without moderator 
approval.
I see that Taekyung is a member only since Nov 08. Postings prior to his 
subscription are waiting moderation.


Ok, thanks. I did not know!

Maxime


Thanks,
Ali

Re: [PATCH v4] vdpa/ifc: fix update_datapath error handling

[Taekyung Kim]

On Thu, Nov 10, 2022 at 10:38:55AM +0100, David Marchand wrote:
> On Thu, Nov 10, 2022 at 10:34 AM Ali Alnubani  wrote:
> > > > I think the mail for v4 is lost.
> > > > Whenever I send a patch, I received "Your message to dev awaits
> > > moderator approval"
> > > > from dev-ow...@dpdk.org with the reason "Post by non-member to a
> > > members-only list".
> > > > Maybe, the reason is that this is the first time that I submit a patch.
> > >
> > > No, I don't think subscription is needed, there should be another issue.
> > > Ali & Thomas, any idea why it happens?
> > >
> >
> > Hello,
> >
> > Subscription to the dev mailing list is required for posting without 
> > moderator approval.
> > I see that Taekyung is a member only since Nov 08. Postings prior to his 
> > subscription are waiting moderation.
> 
> Indeed, I just flushed the queue.
> 
> 
> -- 
> David Marchand
> 

Hello,

I fully understand what was the problem. It was my mistake.
I will be more cautious before sending a patch.
Thanks for your detailed explanation.

Thanks,
Taekyung

RE: [PATCH] net/i40e: fix X722 NIC receives jumbo frame packets

[Zhang, Qi Z]

> -Original Message-
> From: Yuan, DukaiX 
> Sent: Thursday, November 10, 2022 2:00 PM
> To: Wang, Jie1X ; dev@dpdk.org
> Cc: Yang, SteveX ; Zhang, Qi Z
> ; Yang, Qiming ; Xing, Beilei
> ; Zhang, Yuying ; Wang,
> Jie1X ; sta...@dpdk.org
> Subject: RE: [PATCH] net/i40e: fix X722 NIC receives jumbo frame packets
> 
> > -Original Message-
> > From: Jie Wang 
> > Sent: 2022年11月10日 11:45
> > To: dev@dpdk.org
> > Cc: Yang, SteveX ; Zhang, Qi Z
> > ; Yang, Qiming ; Xing,
> > Beilei ; Zhang, Yuying
> > ; Wang, Jie1X ;
> > sta...@dpdk.org
> > Subject: [PATCH] net/i40e: fix X722 NIC receives jumbo frame packets
> >
> > For NIC I40E_10G-10G_BASE_T_X722, when the port is configured with
> > link speed, it cannot receive jumbo frame packets.
> >
> > Because it set maximum frame size failed when starts the port that the
> > port link status is still down.
> >
> > This patch fix the error that starts the port will force set maximum frame
> size.
> >
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > level")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Jie Wang 
> > ---
> Tested-by: Dukai Yuan

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [PATCH v1 1/1] baseband/acc: fix check after deref and dead code

[David Marchand]

On Thu, Nov 3, 2022 at 8:57 PM Hernan Vargas  wrote:
>
> Fix potential issue of dereferencing a pointer before null check.
> Remove null check for value that could never be null.
>
> Coverity issue: 381646, 381631
> Fixes: 989dec301a9 ("baseband/acc100: add ring companion address")
>
> Signed-off-by: Hernan Vargas 
> ---
>  drivers/baseband/acc/rte_acc100_pmd.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c 
> b/drivers/baseband/acc/rte_acc100_pmd.c
> index 96daef87bc..30a718916d 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -4122,15 +4122,11 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data 
> *q_data,
> struct rte_bbdev_enc_op *op;
> union acc_dma_desc *desc;
>
> -   if (q == NULL)
> -   return 0;

I guess this protects badly written applications that would do stuff
like pass an incorrect queue id, or call this callback while the queue
has not been configured yet.
This is something that should be caught at the bbdev layer (arguably
under the RTE_LIBRTE_BBDEV_DEBUG if the performance is that much
affected, though I'd like to see numbers).
(edit: I see Maxime replied a similar comment).

Back to this particular patch, rather than remove the check, the right
fix is to move acc_ring_avail_deq(q).
This is what Coverity reports.

And this same pattern is used in other parts of the driver.
It just happens that Coverity did not report them because some avec
under RTE_LIBRTE_BBDEV_DEBUG...



>  #ifdef RTE_LIBRTE_BBDEV_DEBUG
> if (unlikely(ops == 0))

And I also noticed this hunk.

DPDK coding style, ops should be compared against NULL, but see below...


> return 0;
>  #endif
> desc = q->ring_addr + (q->sw_ring_tail & q->sw_ring_wrap_mask);
> -   if (unlikely(desc == NULL))
> -   return 0;
> op = desc->req.op_addr;
> if (unlikely(ops == NULL || op == NULL))
> return 0;

... like here, so above check is redundant.

There is probably more cleanups to do in this driver.
This can be done later.


-- 
David Marchand

RE: Is it correct to report checksum good when there is no checksum?

[Morten Brørup]

> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 10.26
> 
> Hi all,
> 
> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> For me it looks strange, but I see some technical reasons behind.

Please note: IPv6 packets by definition have no IP checksum.

> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> Should UNKNOWN or NONE be used instead?

Certainly not NONE. Its description says: "the IP checksum is *not* correct in 
the packet [...]". But there is no incorrect IP checksum in the packet.

I will argue against UNKNOWN. Its description says: "no information about the 
RX IP checksum". But we do have information about it! We know that the IP 
checksum is not there (the value is "NULL"), and that it is not supposed to be 
there (the value is supposed to be "NULL").

So I consider GOOD the correct response here.

GOOD also means that the application can proceed processing the packet normally 
without further IP header checksum checking, so it's good for performance.

It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD that IPv6 
packets always return this value, because IPv6 packets have no IP header 
checksum, and that is what is expected of them.

-Morten

RE: [PATCH v3] net/iavf: revert fix VLAN insertion

[Zhang, Qi Z]

> -Original Message-
> From: Zhou, YidingX 
> Sent: Thursday, November 10, 2022 10:10 AM
> To: Zhang, Qi Z ; dev@dpdk.org
> Cc: ktray...@redhat.com; Yang, Qiming 
> Subject: RE: [PATCH v3] net/iavf: revert fix VLAN insertion
> 
> > > Subject: [PATCH v3] net/iavf: revert fix VLAN insertion
> > >
> > > The vector Tx path does not support VLAN insertion via the L2TAG2
> > > field, but the scalar path supports. The previous commit was to
> > > force to select scalar path as soon as kernel driver requests to use 
> > > L2TAG2.
> >
> > In which situation, that kernel driver will request to use L2TAG2?
> 
> According to my tests, this happens when the kernel driver version is newer
> than 1.8.9

> 
> > >
> > > That logic is incorrect. Because other case like VLAN offloading not
> > > required but scalar path selected would have a significant performance
> drop .
> > > Therefore the following commit needs to revert.
> >
> > What will happen, if kernel driver request to use L2TAG2, but still
> > vector path is selected?
> >
> The VLAN tag will be inserted to wrong location (inner of QinQ),  and this
> behavior is inconsistent with PF (outer).

Ok, I assume we will have above limitation after applying this patch, right?
If that's true we'd better claim this in the commit log as well as document it 
as a knowing issue.

RE: [PATCH v2] net/iavf: fix taninted scalar

[Zhang, Qi Z]

> -Original Message-
> From: Steve Yang 
> Sent: Thursday, November 10, 2022 4:31 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing ; Xing, Beilei 
> ;
> Yang, SteveX 
> Subject: [PATCH v2] net/iavf: fix taninted scalar
> 
> tainted_data_downcast: Downcasting match_item->meta from void * to
> struct virtchnl_proto_hdrs implies that the data that this pointer points to 
> is
> tainted.
> 
> var_assign_var: Assigning: proto_hdrs = match_item->meta.
> Both are now tainted.
> 
> var_assign_var: Assigning: rss_meta->proto_hdrs = *proto_hdrs. Both are
> now tainted.
> 
> Passing tainted expression "rss_meta->proto_hdrs.count" to
> "iavf_refine_proto_hdrs", which uses it as a loop boundary.
> 
> Removed temporary variable 'proto_hdrs', and copied whole memory of
> match_item meta with exact structure size to avoid data downcast.
> 
> Coverity issue: 381131
> 
> Fixes: 91f27b2e39ab ("net/iavf: refactor RSS")
> 
> Signed-off-by: Steve Yang 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [PATCH] memif: memif driver does not crashes when there's different N of TX and RX queues

[Huzaifa Rahman]

Hi,

Is there any other work/changes required for this patch to be submitted?

Thanks


On Tue, Oct 4, 2022 at 7:53 PM Andrew Rybchenko <
andrew.rybche...@oktetlabs.ru> wrote:

> On 8/8/22 13:39, Joyce Kong wrote:
> > Hi Huzaifa,
> >
> > This patch looks good to me.
> > And would you please help review my memif patches?
> >
> https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.k...@arm.com/
> >
> > Thanks,
> > Joyce
> >
> >> -Original Message-
> >> From: huzaifa.rahman 
> >> Sent: Tuesday, July 26, 2022 6:16 PM
> >> To: jgraj...@cisco.com
> >> Cc: dev@dpdk.org; huzaifa.rahman 
> >> Subject: [PATCH] memif: memif driver does not crashes when there's
> >> different N of TX and RX queues
> > net/memif: fix memif crash with different Tx Rx queues
> >
> >>
> >> Bugzilla ID: 734
> >>
> >> there's a bug in memif_stats_get() function due to confusion between C2S
> >> (client->server) and S2C (server->client) rings, causing a crash if
> there's a
> >> different number of RX and TX queues.
> >>
> >> this is fixed by selectiing the correct rings for RX and TX i.e for RX,
> S2C rings
> >> are selected and for TX, C2S rings are selected.
> >>
> > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> > Cc: sta...@dpdk.org
> >
> >> Signed-off-by: huzaifa.rahman 
> > Reviewed-by: Joyce Kong 
>
> Fixed above on applying.
>
> Applied to dpdk-next-net/main, thanks.
>
>
>

Re: [PATCH] mlx5: initially reading xstats does not cause seg fault

[Huzaifa Rahman]

Hi,

Is there any other work/changes required for this patch to be submitted?

Thanks


On Thu, Sep 22, 2022 at 3:39 PM Huzaifa Rahman 
wrote:

> The bugzilla ID of this bug is 701:
> https://bugs.dpdk.org/show_bug.cgi?id=701
>
> On Tue, Aug 23, 2022 at 12:33 PM Kamil Vojanec  wrote:
>
>> On 8/18/22 14:30, huzaifa.rahman wrote:
>>
>> Bugzilla ID: 296
>>
>> the size of counters array in mlx5_xstats_get() was smaller
>> than the memory we are setting for this array in
>> mlx5_os_read_dev_counters(). due to which the extra memory is
>> corrupted and thus corrupting the seemingly unrelated variables.
>> this happens at the first run only because the n function arg
>> of mlx5_xstats_get() which is used to init counters array is
>> initialized by adding the preceding statistics which in our case
>> (i.e first run) is zero. after the initialization in
>> mlx5_os_stats_init() the mlx5_stats_n is populated and thus from
>> then onward the counters array size is correct
>>
>> my changes will only affect the flow of the first run when we
>> need to initialize stats in mlx5_os_stats_init(). the size of the
>> counters array is set according the mlx5_stats_n variable. by doing
>> this we will avoid the memset corrupting other variables` memory
>>
>> Signed-off-by: huzaifa.rahman  
>> 
>>
>> Tested-by: Kamil Vojanec  
>>
>>

Re: Is it correct to report checksum good when there is no checksum?

[Andrew Rybchenko]

On 11/10/22 12:55, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 10.26

Hi all,

some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
For me it looks strange, but I see some technical reasons behind.


Please note: IPv6 packets by definition have no IP checksum.


Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
Should UNKNOWN or NONE be used instead?


Certainly not NONE. Its description says: "the IP checksum is *not* correct in the 
packet [...]". But there is no incorrect IP checksum in the packet.



Thanks, I should read the definition of none more careful.


I will argue against UNKNOWN. Its description says: "no information about the RX IP checksum". But 
we do have information about it! We know that the IP checksum is not there (the value is "NULL"), 
and that it is not supposed to be there (the value is supposed to be "NULL").



I thought that "no checksum" => "no information" => UNKNOWN


So I consider GOOD the correct response here.

GOOD also means that the application can proceed processing the packet normally 
without further IP header checksum checking, so it's good for performance.



It is very important point and would be nice to have in GOOD
case definition (both IP and L4 cases). It is the right
motivation why GOOD makes sense for IPv6.


It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD that IPv6 
packets always return this value, because IPv6 packets have no IP header 
checksum, and that is what is expected of them.



Could you make a patch?

Bonus question is UDP checksum 0 case. GOOD as well?
(just want to clarify the documentation while we're on it).

Thanks a lot,
Andrew.

RE: [RFC] mempool: zero-copy cache put bulk

[Morten Brørup]

> From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> Sent: Wednesday, 9 November 2022 23.46
> >
> > +To: Bruce also showed interest in this topic, and might have more
> insights.
> >
> > > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > > Sent: Wednesday, 9 November 2022 18.58
> > >
> > > 
> > >
> > > >
> > > > > From: Honnappa Nagarahalli
> [mailto:honnappa.nagaraha...@arm.com]
> > > > > Sent: Sunday, 6 November 2022 00.11
> > > > >
> > > > > + Akshitha, she is working on similar patch
> > > > >
> > > > > Few comments inline
> > > > >
> > > > > > From: Morten Brørup 
> > > > > > Sent: Saturday, November 5, 2022 8:40 AM
> > > > > >
> > > > > > Zero-copy access to the mempool cache is beneficial for PMD
> > > > > performance,
> > > > > > and must be provided by the mempool library to fix [Bug 1052]
> > > > > > without
> > > > > a
> > > > > > performance regression.
> > > > > >
> > > > > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052
> > > > > >
> > > > > >
> > > > > > This RFC offers a conceptual zero-copy put function, where
> the
> > > > > application
> > > > > > promises to store some objects, and in return gets an address
> > > where
> > > > > to store
> > > > > > them.
> > > > > >
> > > > > > I would like some early feedback.
> > > > > >
> > > > > > Notes:
> > > > > > * Allowing the 'cache' parameter to be NULL, and getting it
> from
> > > the
> > > > > > mempool instead, was inspired by rte_mempool_cache_flush().
> > > > > I am not sure why the 'cache' parameter is required for this
> API.
> > > This
> > > > > API should take the mem pool as the parameter.
> > > > >
> > > > > We have based our API on 'rte_mempool_do_generic_put' and
> removed
> > > > the
> > > > > 'cache' parameter.
> > > >
> > > > I thoroughly considered omitting the 'cache' parameter, but
> included
> > > it for
> > > > two reasons:
> > > >
> > > > 1. The function is a "mempool cache" function (i.e. primarily
> > > > working
> > > on the
> > > > mempool cache), not a "mempool" function.
> > > >
> > > > So it is appropriate to have a pointer directly to the structure
> it
> > > is working on.
> > > > Following this through, I also made 'cache' the first parameter
> and
> > > 'mp' the
> > > > second, like in rte_mempool_cache_flush().
> > > I am wondering if the PMD should be aware of the cache or not. For
> ex:
> > > in the case of pipeline mode, the RX and TX side of the PMD are
> > > running on different cores.
> >
> > In that example, the PMD can store two cache pointers, one for each
> of the
> > RX and TX side.
> I did not understand this. If RX core and TX core have their own per-
> core caches the logic would not work. For ex: the RX core cache would
> not get filled.
> 
> In the case of pipeline mode, there will not be a per-core cache. The
> buffers would be allocated and freed from a global ring or a global
> lockless stack.

Aha... Now I understand what you mean: You are referring to use cases where the 
mempool is configured to *not* have a mempool cache.

For a mempool without a mempool cache, the proposed "mempool cache" zero-copy 
functions can obviously not be used.

We need "mempool" zero-copy functions for the mempools that have no mempool 
cache.

However, those functions depend on the mempool's underlying backing store.

E.g. zero-copy access to a ring has certain requirements [1].

[1]: http://doc.dpdk.org/guides/prog_guide/ring_lib.html#ring-peek-zero-copy-api

For a stack, I think it is possible to locklessly zero-copy pop objects. But it 
is impossible to locklessly zero-copy push elements to a stack; another thread 
can race to pop some objects from the stack before the pushing thread has 
finished writing them into the stack.

Furthermore, the ring zero-copy get function cannot return a consecutive array 
of objects when wrapping, and PMD functions using vector instructions usually 
rely on handling chunks of e.g. 8 objects.

Just for a second, let me theorize into the absurd: Even worse, if a mempool's 
underlying backing store does not use an array of pointers as its internal 
storage structure, it is impossible to use a pointer to an array of pointers 
for zero-copy transactions. E.g. if the backing store uses a list or a tree 
structure for its storage, a pointer to somewhere in the list or tree structure 
is not an array of objects pointers.

Anyway, we could consider designing a generic API for zero-copy mempool 
get/put; but it should be compatible with all underlying backing stores - or 
return failure, so the PMD can fall back to the standard functions, if the 
mempool is in a state where zero-copy access to a contiguous burst cannot be 
provided. E.g. zero-copy get from a ring can return failure when zero-copy 
access to the ring is temporarily unavailable due to being at a point where it 
would wrap.

Here is a conceptual proposal for such an API.

/* Mempool zero-copy transaction state. Opaque outside the mempool API. */
struct rte_mempool_zc_transac

[PATCH v14 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Hanumanth Pothula]

Some of the HW has support for choosing memory pools based on
the packet's size. The pool sort capability allows PMD/NIC to
choose a memory pool based on the packet's length.

On multiple mempool support enabled, populate mempool array
accordingly. Also, print pool name on which packet is received.

Signed-off-by: Hanumanth Pothula 

v14:
 - Rebased on tip of next-net/main
v13:
 - Make sure protocol-based header split feature is not broken
   by updating changes with latest code base.
v12:
 - Process multi-segment configuration on number segments
   (rx_pkt_nb_segs) greater than 1 or buffer split offload
   flag (RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) set.
v11:
 - Resolve compilation and warning.
v10:
 - Populate multi-mempool array based on mbuf_data_size_n instead
   of rx_pkt_nb_segs.
---
 app/test-pmd/testpmd.c | 70 +++---
 app/test-pmd/testpmd.h |  3 ++
 app/test-pmd/util.c|  4 +--
 3 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d494870e59..ef281ccd20 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2653,12 +2653,20 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
   struct rte_eth_rxconf *rx_conf, struct rte_mempool *mp)
 {
union rte_eth_rxseg rx_useg[MAX_SEGS_BUFFER_SPLIT] = {};
+   struct rte_mempool *rx_mempool[MAX_MEMPOOL] = {};
+   struct rte_mempool *mpx;
unsigned int i, mp_n;
uint32_t prev_hdrs = 0;
int ret;
 
-   if (rx_pkt_nb_segs <= 1 ||
-   (rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) == 0) {
+   /* Verify Rx queue configuration is single pool and segment or
+* multiple pool/segment.
+* @see rte_eth_rxconf::rx_mempools
+* @see rte_eth_rxconf::rx_seg
+*/
+   if (!(mbuf_data_size_n > 1) && !(rx_pkt_nb_segs > 1 ||
+   ((rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) != 0))) {
+   /* Single pool/segment configuration */
rx_conf->rx_seg = NULL;
rx_conf->rx_nseg = 0;
ret = rte_eth_rx_queue_setup(port_id, rx_queue_id,
@@ -2666,34 +2674,48 @@ rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 rx_conf, mp);
goto exit;
}
-   for (i = 0; i < rx_pkt_nb_segs; i++) {
-   struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
-   struct rte_mempool *mpx;
-   /*
-* Use last valid pool for the segments with number
-* exceeding the pool index.
-*/
-   mp_n = (i >= mbuf_data_size_n) ? mbuf_data_size_n - 1 : i;
-   mpx = mbuf_pool_find(socket_id, mp_n);
-   /* Handle zero as mbuf data buffer size. */
-   rx_seg->offset = i < rx_pkt_nb_offs ?
-  rx_pkt_seg_offsets[i] : 0;
-   rx_seg->mp = mpx ? mpx : mp;
-   if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] == 0) {
-   rx_seg->proto_hdr = rx_pkt_hdr_protos[i] & ~prev_hdrs;
-   prev_hdrs |= rx_seg->proto_hdr;
-   } else {
-   rx_seg->length = rx_pkt_seg_lengths[i] ?
-   rx_pkt_seg_lengths[i] :
-   mbuf_data_size[mp_n];
+
+   if (rx_pkt_nb_segs > 1 ||
+   rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) {
+   /* multi-segment configuration */
+   for (i = 0; i < rx_pkt_nb_segs; i++) {
+   struct rte_eth_rxseg_split *rx_seg = &rx_useg[i].split;
+   /*
+* Use last valid pool for the segments with number
+* exceeding the pool index.
+*/
+   mp_n = (i >= mbuf_data_size_n) ? mbuf_data_size_n - 1 : 
i;
+   mpx = mbuf_pool_find(socket_id, mp_n);
+   /* Handle zero as mbuf data buffer size. */
+   rx_seg->offset = i < rx_pkt_nb_offs ?
+  rx_pkt_seg_offsets[i] : 0;
+   rx_seg->mp = mpx ? mpx : mp;
+   if (rx_pkt_hdr_protos[i] != 0 && rx_pkt_seg_lengths[i] 
== 0) {
+   rx_seg->proto_hdr = rx_pkt_hdr_protos[i] & 
~prev_hdrs;
+   prev_hdrs |= rx_seg->proto_hdr;
+   } else {
+   rx_seg->length = rx_pkt_seg_lengths[i] ?
+   rx_pkt_seg_lengths[i] :
+   mbuf_data_size[mp_n];
+   }
+   }
+   rx_conf->rx_nseg = rx_pkt_nb_segs;
+   rx_conf->rx_seg = rx_useg;
+   } else {
+   /* multi-pool conf

Re: [PATCH] devtools: set DTS directory to format check

[Thomas Monjalon]

10/11/2022 10:27, Juraj Linkeš:
> From: Thomas Monjalon 
> >  usage() {
> > -   echo "Run formatting and linting programs for DTS. Usage:"
> > -
> > +   echo 'Usage: $(basename $0) [options] [directory]'
> 
> Double quotes here, otherwise $0 won't be expanded.

I wonder how I tested it :)

> > +directory=${1:-$(dirname $0)/../dts}
> > +cd $directory || exit 1
> 
> I'd like to include the information of where we're doing the fomatting in the 
> console output, e.g.:
> echo "Formatting in $(pwd):"
> 
> We're silently chaning the directory, so this would be useful when running 
> with no argument and the script doesn't change anything - as a confirmation 
> that it ran over the files we wanted to.

Good comment, I'll improve in v2.

> Other than that,
> Reviewed-by: Juraj Linkeš 
> Tested-by: Juraj Linkeš 

No it does not work, so you should not add your Tested-by.
And in general, Reviewed-by is enough.
And really, give your reviewed-by only when it's perfect :)

Re: Is it correct to report checksum good when there is no checksum?

[Thomas Monjalon]

10/11/2022 11:08, Andrew Rybchenko:
> On 11/10/22 12:55, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> >> Sent: Thursday, 10 November 2022 10.26
> >>
> >> Hi all,
> >>
> >> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> >> For me it looks strange, but I see some technical reasons behind.
> > 
> > Please note: IPv6 packets by definition have no IP checksum.
> > 
> >> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >> Should UNKNOWN or NONE be used instead?
> > 
> > Certainly not NONE. Its description says: "the IP checksum is *not* correct 
> > in the packet [...]". But there is no incorrect IP checksum in the packet.
> > 
> 
> Thanks, I should read the definition of none more careful.
> 
> > I will argue against UNKNOWN. Its description says: "no information about 
> > the RX IP checksum". But we do have information about it! We know that the 
> > IP checksum is not there (the value is "NULL"), and that it is not supposed 
> > to be there (the value is supposed to be "NULL").
> > 
> 
> I thought that "no checksum" => "no information" => UNKNOWN
> 
> > So I consider GOOD the correct response here.
> > 
> > GOOD also means that the application can proceed processing the packet 
> > normally without further IP header checksum checking, so it's good for 
> > performance.
> > 
> 
> It is very important point and would be nice to have in GOOD
> case definition (both IP and L4 cases). It is the right
> motivation why GOOD makes sense for IPv6.
> 
> > It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD that 
> > IPv6 packets always return this value, because IPv6 packets have no IP 
> > header checksum, and that is what is expected of them.
> 
> Could you make a patch?

That would be perfect. I agree to use GOOD for IPv6 checksum.

> Bonus question is UDP checksum 0 case. GOOD as well?
> (just want to clarify the documentation while we're on it).

Good question :)

RE: Is it correct to report checksum good when there is no checksum?

[Morten Brørup]

> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 11.09
> 
> On 11/10/22 12:55, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> >> Sent: Thursday, 10 November 2022 10.26
> >>
> >> Hi all,
> >>
> >> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> >> For me it looks strange, but I see some technical reasons behind.
> >
> > Please note: IPv6 packets by definition have no IP checksum.
> >
> >> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >> Should UNKNOWN or NONE be used instead?
> >
> > Certainly not NONE. Its description says: "the IP checksum is *not*
> correct in the packet [...]". But there is no incorrect IP checksum in
> the packet.
> >
> 
> Thanks, I should read the definition of none more careful.
> 
> > I will argue against UNKNOWN. Its description says: "no information
> about the RX IP checksum". But we do have information about it! We know
> that the IP checksum is not there (the value is "NULL"), and that it is
> not supposed to be there (the value is supposed to be "NULL").
> >
> 
> I thought that "no checksum" => "no information" => UNKNOWN

That was my initial interpretation too, and it stuck with me for a while.

But then I tried hard to read it differently, tweaking it to support the 
conclusion I was looking for.

> 
> > So I consider GOOD the correct response here.
> >
> > GOOD also means that the application can proceed processing the
> packet normally without further IP header checksum checking, so it's
> good for performance.
> >
> 
> It is very important point and would be nice to have in GOOD
> case definition (both IP and L4 cases). It is the right
> motivation why GOOD makes sense for IPv6.
> 
> > It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD
> that IPv6 packets always return this value, because IPv6 packets have
> no IP header checksum, and that is what is expected of them.
> >
> 
> Could you make a patch?

Too busy right now, but I'll put it on my todo list. :-)

> 
> Bonus question is UDP checksum 0 case. GOOD as well?
> (just want to clarify the documentation while we're on it).

No. The UDP checksum is not optional in IPv6.

RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets are 
originated by an IPv6 node, the UDP checksum is not optional. [...] IPv6 
receivers must discard UDP packets containing a zero checksum, and should log 
the error."

> 
> Thanks a lot,
> Andrew.
> 

Re: Is it correct to report checksum good when there is no checksum?

[Andrew Rybchenko]

On 11/10/22 13:29, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 11.09

On 11/10/22 12:55, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 10.26

Hi all,

some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
For me it looks strange, but I see some technical reasons behind.


Please note: IPv6 packets by definition have no IP checksum.


Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
Should UNKNOWN or NONE be used instead?


Certainly not NONE. Its description says: "the IP checksum is *not*

correct in the packet [...]". But there is no incorrect IP checksum in
the packet.




Thanks, I should read the definition of none more careful.


I will argue against UNKNOWN. Its description says: "no information

about the RX IP checksum". But we do have information about it! We know
that the IP checksum is not there (the value is "NULL"), and that it is
not supposed to be there (the value is supposed to be "NULL").




I thought that "no checksum" => "no information" => UNKNOWN


That was my initial interpretation too, and it stuck with me for a while.

But then I tried hard to read it differently, tweaking it to support the 
conclusion I was looking for.




So I consider GOOD the correct response here.

GOOD also means that the application can proceed processing the

packet normally without further IP header checksum checking, so it's
good for performance.




It is very important point and would be nice to have in GOOD
case definition (both IP and L4 cases). It is the right
motivation why GOOD makes sense for IPv6.


It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD

that IPv6 packets always return this value, because IPv6 packets have
no IP header checksum, and that is what is expected of them.




Could you make a patch?


Too busy right now, but I'll put it on my todo list. :-)



Bonus question is UDP checksum 0 case. GOOD as well?
(just want to clarify the documentation while we're on it).


No. The UDP checksum is not optional in IPv6.

RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets are originated by 
an IPv6 node, the UDP checksum is not optional. [...] IPv6 receivers must discard UDP 
packets containing a zero checksum, and should log the error."



Yes I know, but I'm asking about IPv4 case with UDP checksum 0.

RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment

[Zhang, Qi Z]

> -Original Message-
> From: Ye, MingjinX 
> Sent: Wednesday, November 9, 2022 8:56 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org; Zhou, YidingX
> ; Ye, MingjinX ; Zhang, Qi
> Z ; Lu, Wenzhuo ; Wu,
> Jingjing ; Li, Xiaoyun ; Ferruh
> Yigit 
> Subject: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. If the last buffer
> packet length is '0', the scalar Tx path would send empty buffer that causes
> the Tx queue to overflow.

Please separate this patch into two, one for Rx and one for Tx, they are 
independent.

For the Tx implementation, I think we can move them into tx_prepare where is 
place to check Tx violation.
 
> 
> This patch adds a judgment for the last buffer length to fix this issue, so 
> that
> it would free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Mingjin Ye 
> 
> v2:
>   * Fix log level in ice_rxtx.c source file.
> ---
>  drivers/net/ice/ice_rxtx.c | 53 --
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> 0a2b0376ac..b181f66aad 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
>   } else
>   rxm->data_len = (uint16_t)(rx_packet_len -
> 
> RTE_ETHER_CRC_LEN);
> + } else if (rx_packet_len == 0) {
> + rte_pktmbuf_free_seg(rxm);
> + first_seg->nb_segs--;
> + last_seg->next = NULL;
>   }
> 
>   first_seg->port = rxq->port_id;
> @@ -2903,6 +2907,35 @@ ice_calc_pkt_desc(struct rte_mbuf *tx_pkt)
>   return count;
>  }
> 
> +/*Check the number of valid mbufs and free the invalid mbufs*/ static
> +inline uint16_t ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> + struct rte_mbuf *txd = tx_pkt;
> + struct rte_mbuf *txd_removal = NULL;
> + struct rte_mbuf *txd_pre = NULL;
> + uint16_t count = 0;
> + uint16_t removal = 0;
> +
> + while (txd != NULL) {
> + if (removal == 1 || txd->data_len == 0) {
> + txd_removal = txd;
> + txd = txd->next;
> + if (removal == 0) {
> + removal = 1;
> + txd_pre->next = NULL;
> + }
> + rte_pktmbuf_free_seg(txd_removal);
> + } else {
> + ++count;
> + txd_pre = txd;
> + txd = txd->next;
> + }
> + }
> +
> + return count;
> +}
> +
>  uint16_t
>  ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> { @@ -2960,11 +2993,27 @@ ice_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts, uint16_t nb_pkts)
>* the mbuf data size exceeds max data size that hw allows
>* per tx desc.
>*/
> - if (ol_flags & RTE_MBUF_F_TX_TCP_SEG)
> + if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
>   nb_used = (uint16_t)(ice_calc_pkt_desc(tx_pkt) +
>nb_ctx);
> - else
> + } else {
> + nb_used = ice_check_mbuf(tx_pkt);
> + if (nb_used == 0) {
> + PMD_TX_LOG(ERR,
> + "Check packets is empty "
> + "(port=%d queue=%d)\n",
> + txq->port_id, txq->queue_id);
> + continue;
> + } else if (nb_used < tx_pkt->nb_segs) {
> + PMD_TX_LOG(DEBUG,
> + "Check packets valid num ="
> + "%4u total num = %4u (port=%d
> queue=%d)\n",
> + nb_used, tx_pkt->nb_segs, txq->port_id, txq-
> >queue_id);
> + tx_pkt->nb_segs = nb_used;
> + }
>   nb_used = (uint16_t)(tx_pkt->nb_segs + nb_ctx);
> + }
> +
>   tx_last = (uint16_t)(tx_id + nb_used - 1);
> 
>   /* Circular ring */
> --
> 2.34.1

RE: [PATCH v2] doc: support IPsec Multi-buffer lib v1.3

[Power, Ciara]

Hi Pablo,

> -Original Message-
> From: Pablo de Lara 
> Sent: Wednesday 9 November 2022 18:39
> To: Ji, Kai 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo 
> Subject: [PATCH v2] doc: support IPsec Multi-buffer lib v1.3
> 
> Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD
> documentation guides with information about the latest Intel IPSec Multi-
> buffer library supported.
> 
> Signed-off-by: Pablo de Lara 
> ---
> 
> - v2: Removed repeated word 'the'
> 
> ---
>  doc/guides/cryptodevs/aesni_gcm.rst |  8 
> doc/guides/cryptodevs/aesni_mb.rst  | 18 --
>  doc/guides/cryptodevs/kasumi.rst| 15 +++
>  doc/guides/cryptodevs/snow3g.rst| 15 +++
>  doc/guides/cryptodevs/zuc.rst   | 14 ++
>  5 files changed, 48 insertions(+), 22 deletions(-)
> 

> --- a/doc/guides/cryptodevs/aesni_mb.rst
> +++ b/doc/guides/cryptodevs/aesni_mb.rst
> @@ -1,7 +1,7 @@
>  ..  SPDX-License-Identifier: BSD-3-Clause
>  Copyright(c) 2015-2018 Intel Corporation.
> 
> -AESN-NI Multi Buffer Crypto Poll Mode Driver
> +AES-NI Multi Buffer Crypto Poll Mode Driver
>  
> 
> 
> @@ -10,8 +10,6 @@ support for utilizing Intel multi buffer library, see the
> white paper  `Fast Multi-buffer IPsec Implementations on Intel® Architecture
> Processors
>  e-papers/fast-multi-buffer-ipsec-implementations-ia-processors-
> paper.pdf>`_.
> 
> -The AES-NI MB PMD has current only been tested on Fedora 21 64-bit with
> gcc.
> -
>  The AES-NI MB PMD supports synchronous mode of operation with
> ``rte_cryptodev_sym_cpu_crypto_process`` function call.
> 
> @@ -77,6 +75,14 @@ Limitations
>  * RTE_CRYPTO_CIPHER_DES_DOCSISBPI is not supported for combined
> Crypto-CRC
>DOCSIS security protocol.
> 
> +AESNI MB PMD selection over SNOW3G/ZUC/KASUMI PMDs
> +--
> +
> +This PMD supports wireless cipher suite (SNOW3G, ZUC and KASUMI).
> +On Intel processors, it is recommended to use this PMD instead of
> +SNOW3G, ZUC and KASUMI PMDs, as it enables algorithm mixing (e.g.
> +cipher algorithm SNOW3G-UEA2 with authentication algorithm
> +AES-CMAC-128) and performance over IMIX (packet size mix) traffic is
> significantly higher.
> 
>  Installation
>  
> @@ -84,8 +90,8 @@ Installation
>  To build DPDK with the AESNI_MB_PMD the user is required to download
> the multi-buffer  library from `here  mb>`_
>  and compile it on their user system before building DPDK.
> -The latest version of the library supported by this PMD is v1.2, which -can 
> be
> downloaded from ` mb/archive/v1.2.zip>`_.
> +The latest version of the library supported by this PMD is v1.3, which
> +can be downloaded from ` mb/archive/v1.3.zip>`_.
> 
>  .. code-block:: console
> 
> @@ -131,7 +137,7 @@ and the Multi-Buffer library version supported by
> them:
> 19.05 - 19.08   0.52
> 19.11 - 20.08   0.52 - 0.55
> 20.11 - 21.08   0.53 - 1.2*
> -   21.11+  1.0  - 1.2*
> +   21.11+  1.0  - 1.3*
> ==  
[CP] 

Should 20.11 - 21.08 be 0.53 - 1.3* also? 

Overall, looks good, thanks.

Acked-by: Ciara Power 

Re: [PATCH v14 1/1] app/testpmd: support multiple mbuf pools per Rx queue

[Andrew Rybchenko]

On 11/10/22 13:16, Hanumanth Pothula wrote:

Some of the HW has support for choosing memory pools based on
the packet's size. The pool sort capability allows PMD/NIC to
choose a memory pool based on the packet's length.

On multiple mempool support enabled, populate mempool array
accordingly. Also, print pool name on which packet is received.

Signed-off-by: Hanumanth Pothula 

v14:
  - Rebased on tip of next-net/main
v13:
  - Make sure protocol-based header split feature is not broken
by updating changes with latest code base.
v12:
  - Process multi-segment configuration on number segments
(rx_pkt_nb_segs) greater than 1 or buffer split offload
flag (RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) set.
v11:
  - Resolve compilation and warning.
v10:
  - Populate multi-mempool array based on mbuf_data_size_n instead
of rx_pkt_nb_segs.


Reviewed-by: Andrew Rybchenko 

Applied to dpdk-next-net/main, thanks.

Re: [PATCH] mlx5: initially reading xstats does not cause seg fault

[Kamil Vojanec]

Hello,

On 11/10/22 11:07, Huzaifa Rahman wrote:


Hi,

Is there any other work/changes required for this patch to be submitted?

Thanks


On Thu, Sep 22, 2022 at 3:39 PM Huzaifa Rahman
wrote:


The bugzilla ID of this bug is 701:
https://bugs.dpdk.org/show_bug.cgi?id=701

On Tue, Aug 23, 2022 at 12:33 PM Kamil Vojanec  wrote:


On 8/18/22 14:30, huzaifa.rahman wrote:

Bugzilla ID: 296

the size of counters array in mlx5_xstats_get() was smaller
than the memory we are setting for this array in
mlx5_os_read_dev_counters(). due to which the extra memory is
corrupted and thus corrupting the seemingly unrelated variables.
this happens at the first run only because the n function arg
of mlx5_xstats_get() which is used to init counters array is
initialized by adding the preceding statistics which in our case
(i.e first run) is zero. after the initialization in
mlx5_os_stats_init() the mlx5_stats_n is populated and thus from
then onward the counters array size is correct

my changes will only affect the flow of the first run when we
need to initialize stats in mlx5_os_stats_init(). the size of the
counters array is set according the mlx5_stats_n variable. by doing
this we will avoid the memset corrupting other variables` memory

Signed-off-by: huzaifa.rahman  


Tested-by: Kamil Vojanec  



Looks good to me


smime.p7s
Description: S/MIME Cryptographic Signature

RE: [PATCH] devtools: set DTS directory to format check

[Juraj Linkeš]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Thursday, November 10, 2022 11:26 AM
> To: Juraj Linkeš 
> Cc: dev@dpdk.org; Lijuan Tu ; Owen Hilyard
> 
> Subject: Re: [PATCH] devtools: set DTS directory to format check
> 
> 10/11/2022 10:27, Juraj Linkeš:
> > From: Thomas Monjalon 
> > >  usage() {
> > > - echo "Run formatting and linting programs for DTS. Usage:"
> > > -
> > > + echo 'Usage: $(basename $0) [options] [directory]'
> >
> > Double quotes here, otherwise $0 won't be expanded.
> 
> I wonder how I tested it :)
> 
> > > +directory=${1:-$(dirname $0)/../dts} cd $directory || exit 1
> >
> > I'd like to include the information of where we're doing the fomatting in
> the console output, e.g.:
> > echo "Formatting in $(pwd):"
> >
> > We're silently chaning the directory, so this would be useful when running
> with no argument and the script doesn't change anything - as a confirmation
> that it ran over the files we wanted to.
> 
> Good comment, I'll improve in v2.

One more thing, if we go with changing the Formatting echo, then we should also 
change the Linting echo. Or we could do it in some other place just once, I'm 
not sure which is better.

> 
> > Other than that,
> > Reviewed-by: Juraj Linkeš 
> > Tested-by: Juraj Linkeš 
> 
> No it does not work, so you should not add your Tested-by.
> And in general, Reviewed-by is enough.
> And really, give your reviewed-by only when it's perfect :)
> 

Thanks. It actually does work (by which I mean it does format and lint), but 
it's not perfect, so I guess Tested-by should be fine, but Reviewed-by isn't? 
:-)

Re: [RFC] mempool: zero-copy cache put bulk

[Bruce Richardson]

On Thu, Nov 10, 2022 at 11:15:27AM +0100, Morten Brørup wrote:
> > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > Sent: Wednesday, 9 November 2022 23.46
> > >
> > > +To: Bruce also showed interest in this topic, and might have more
> > insights.
> > >
> > > > From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > > > Sent: Wednesday, 9 November 2022 18.58
> > > >
> > > > 
> > > >
> > > > >
> > > > > > From: Honnappa Nagarahalli
> > [mailto:honnappa.nagaraha...@arm.com]
> > > > > > Sent: Sunday, 6 November 2022 00.11
> > > > > >
> > > > > > + Akshitha, she is working on similar patch
> > > > > >
> > > > > > Few comments inline
> > > > > >
> > > > > > > From: Morten Brørup 
> > > > > > > Sent: Saturday, November 5, 2022 8:40 AM
> > > > > > >
> > > > > > > Zero-copy access to the mempool cache is beneficial for PMD
> > > > > > performance,
> > > > > > > and must be provided by the mempool library to fix [Bug 1052]
> > > > > > > without
> > > > > > a
> > > > > > > performance regression.
> > > > > > >
> > > > > > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052
> > > > > > >
> > > > > > >
> > > > > > > This RFC offers a conceptual zero-copy put function, where
> > the
> > > > > > application
> > > > > > > promises to store some objects, and in return gets an address
> > > > where
> > > > > > to store
> > > > > > > them.
> > > > > > >
> > > > > > > I would like some early feedback.
> > > > > > >
> > > > > > > Notes:
> > > > > > > * Allowing the 'cache' parameter to be NULL, and getting it
> > from
> > > > the
> > > > > > > mempool instead, was inspired by rte_mempool_cache_flush().
> > > > > > I am not sure why the 'cache' parameter is required for this
> > API.
> > > > This
> > > > > > API should take the mem pool as the parameter.
> > > > > >
> > > > > > We have based our API on 'rte_mempool_do_generic_put' and
> > removed
> > > > > the
> > > > > > 'cache' parameter.
> > > > >
> > > > > I thoroughly considered omitting the 'cache' parameter, but
> > included
> > > > it for
> > > > > two reasons:
> > > > >
> > > > > 1. The function is a "mempool cache" function (i.e. primarily
> > > > > working
> > > > on the
> > > > > mempool cache), not a "mempool" function.
> > > > >
> > > > > So it is appropriate to have a pointer directly to the structure
> > it
> > > > is working on.
> > > > > Following this through, I also made 'cache' the first parameter
> > and
> > > > 'mp' the
> > > > > second, like in rte_mempool_cache_flush().
> > > > I am wondering if the PMD should be aware of the cache or not. For
> > ex:
> > > > in the case of pipeline mode, the RX and TX side of the PMD are
> > > > running on different cores.
> > >
> > > In that example, the PMD can store two cache pointers, one for each
> > of the
> > > RX and TX side.
> > I did not understand this. If RX core and TX core have their own per-
> > core caches the logic would not work. For ex: the RX core cache would
> > not get filled.
> > 
> > In the case of pipeline mode, there will not be a per-core cache. The
> > buffers would be allocated and freed from a global ring or a global
> > lockless stack.
> 
> Aha... Now I understand what you mean: You are referring to use cases where 
> the mempool is configured to *not* have a mempool cache.
> 
> For a mempool without a mempool cache, the proposed "mempool cache" zero-copy 
> functions can obviously not be used.
> 
> We need "mempool" zero-copy functions for the mempools that have no mempool 
> cache.
> 
> However, those functions depend on the mempool's underlying backing store.
> 
> E.g. zero-copy access to a ring has certain requirements [1].
> 
> [1]: 
> http://doc.dpdk.org/guides/prog_guide/ring_lib.html#ring-peek-zero-copy-api
> 
> For a stack, I think it is possible to locklessly zero-copy pop objects. But 
> it is impossible to locklessly zero-copy push elements to a stack; another 
> thread can race to pop some objects from the stack before the pushing thread 
> has finished writing them into the stack.
> 
> Furthermore, the ring zero-copy get function cannot return a consecutive 
> array of objects when wrapping, and PMD functions using vector instructions 
> usually rely on handling chunks of e.g. 8 objects.
> 
> Just for a second, let me theorize into the absurd: Even worse, if a 
> mempool's underlying backing store does not use an array of pointers as its 
> internal storage structure, it is impossible to use a pointer to an array of 
> pointers for zero-copy transactions. E.g. if the backing store uses a list or 
> a tree structure for its storage, a pointer to somewhere in the list or tree 
> structure is not an array of objects pointers.
> 
> Anyway, we could consider designing a generic API for zero-copy mempool 
> get/put; but it should be compatible with all underlying backing stores - or 
> return failure, so the PMD can fall back to the standard functions, if the 
> mempool is in a state where zero-copy access to a contiguous burst cannot be 
> p

RE: Is it correct to report checksum good when there is no checksum?

[Morten Brørup]

> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 11.34
> 
> On 11/10/22 13:29, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> >> Sent: Thursday, 10 November 2022 11.09
> >>
> >> On 11/10/22 12:55, Morten Brørup wrote:
>  From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
>  Sent: Thursday, 10 November 2022 10.26
> 
>  Hi all,
> 
>  some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
>  For me it looks strange, but I see some technical reasons behind.
> >>>
> >>> Please note: IPv6 packets by definition have no IP checksum.
> >>>
>  Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
>  Should UNKNOWN or NONE be used instead?
> >>>
> >>> Certainly not NONE. Its description says: "the IP checksum is *not*
> >> correct in the packet [...]". But there is no incorrect IP checksum
> in
> >> the packet.
> >>>
> >>
> >> Thanks, I should read the definition of none more careful.
> >>
> >>> I will argue against UNKNOWN. Its description says: "no information
> >> about the RX IP checksum". But we do have information about it! We
> know
> >> that the IP checksum is not there (the value is "NULL"), and that it
> is
> >> not supposed to be there (the value is supposed to be "NULL").
> >>>
> >>
> >> I thought that "no checksum" => "no information" => UNKNOWN
> >
> > That was my initial interpretation too, and it stuck with me for a
> while.
> >
> > But then I tried hard to read it differently, tweaking it to support
> the conclusion I was looking for.
> >
> >>
> >>> So I consider GOOD the correct response here.
> >>>
> >>> GOOD also means that the application can proceed processing the
> >> packet normally without further IP header checksum checking, so it's
> >> good for performance.
> >>>
> >>
> >> It is very important point and would be nice to have in GOOD
> >> case definition (both IP and L4 cases). It is the right
> >> motivation why GOOD makes sense for IPv6.
> >>
> >>> It should be added to the description of
> RTE_MBUF_F_RX_IP_CKSUM_GOOD
> >> that IPv6 packets always return this value, because IPv6 packets
> have
> >> no IP header checksum, and that is what is expected of them.
> >>>
> >>
> >> Could you make a patch?
> >
> > Too busy right now, but I'll put it on my todo list. :-)
> >
> >>
> >> Bonus question is UDP checksum 0 case. GOOD as well?
> >> (just want to clarify the documentation while we're on it).
> >
> > No. The UDP checksum is not optional in IPv6.
> >
> > RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
> are originated by an IPv6 node, the UDP checksum is not optional. [...]
> IPv6 receivers must discard UDP packets containing a zero checksum, and
> should log the error."
> >
> 
> Yes I know, but I'm asking about IPv4 case with UDP checksum 0.

It cannot be UNKNOWN, because we do have information: The checksum was 
intentionally omitted.

I would prefer GOOD, using the same logic as for the IPv6 header checksum.

Trying very hard to tweak the meaning of NONE's description ("the L4 checksum 
is not correct in the packet data, but the integrity of the L4 data is 
verified."), we could argue that "not correct" != "intentionally omitted" (and 
an intentional omission is absolutely correct), and conclude that it cannot be 
NONE. A seasoned politician would say this without blinking, but it is up to 
individual interpretation.

We should settle on either GOOD or NONE, and write it in the documentation.

In a perfect world, the PMD DPDK compliance tests should also check things like 
this.

[PATCH v3] doc: support IPsec Multi-buffer lib v1.3

[Pablo de Lara]

Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD documentation
guides with information about the latest Intel IPSec Multi-buffer
library supported.

Signed-off-by: Pablo de Lara 
Acked-by: Ciara Power 

---
-v3: Fixed library version from 1.2 to 1.3 in one line
-v2: Removed repeated word 'the'
---

 doc/guides/cryptodevs/aesni_gcm.rst |  8 
 doc/guides/cryptodevs/aesni_mb.rst  | 20 +---
 doc/guides/cryptodevs/kasumi.rst| 15 +++
 doc/guides/cryptodevs/snow3g.rst| 15 +++
 doc/guides/cryptodevs/zuc.rst   | 14 ++
 5 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/doc/guides/cryptodevs/aesni_gcm.rst 
b/doc/guides/cryptodevs/aesni_gcm.rst
index 6229392f58..5192287ed8 100644
--- a/doc/guides/cryptodevs/aesni_gcm.rst
+++ b/doc/guides/cryptodevs/aesni_gcm.rst
@@ -40,8 +40,8 @@ Installation
 To build DPDK with the AESNI_GCM_PMD the user is required to download the 
multi-buffer
 library from `here `_
 and compile it on their user system before building DPDK.
-The latest version of the library supported by this PMD is v1.2, which
-can be downloaded in 
``_.
+The latest version of the library supported by this PMD is v1.3, which
+can be downloaded in 
``_.
 
 .. code-block:: console
 
@@ -84,8 +84,8 @@ and the external crypto libraries supported by them:
17.08 - 18.02  Multi-buffer library 0.46 - 0.48
18.05 - 19.02  Multi-buffer library 0.49 - 0.52
19.05 - 20.08  Multi-buffer library 0.52 - 0.55
-   20.11 - 21.08  Multi-buffer library 0.53 - 1.2*
-   21.11+ Multi-buffer library 1.0  - 1.2*
+   20.11 - 21.08  Multi-buffer library 0.53 - 1.3*
+   21.11+ Multi-buffer library 1.0  - 1.3*
=  
 
 \* Multi-buffer library 1.0 or newer only works for Meson but not Make build 
system.
diff --git a/doc/guides/cryptodevs/aesni_mb.rst 
b/doc/guides/cryptodevs/aesni_mb.rst
index 599ed5698f..492c53f595 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -1,7 +1,7 @@
 ..  SPDX-License-Identifier: BSD-3-Clause
 Copyright(c) 2015-2018 Intel Corporation.
 
-AESN-NI Multi Buffer Crypto Poll Mode Driver
+AES-NI Multi Buffer Crypto Poll Mode Driver
 
 
 
@@ -10,8 +10,6 @@ support for utilizing Intel multi buffer library, see the 
white paper
 `Fast Multi-buffer IPsec Implementations on Intel® Architecture Processors
 
`_.
 
-The AES-NI MB PMD has current only been tested on Fedora 21 64-bit with gcc.
-
 The AES-NI MB PMD supports synchronous mode of operation with
 ``rte_cryptodev_sym_cpu_crypto_process`` function call.
 
@@ -77,6 +75,14 @@ Limitations
 * RTE_CRYPTO_CIPHER_DES_DOCSISBPI is not supported for combined Crypto-CRC
   DOCSIS security protocol.
 
+AESNI MB PMD selection over SNOW3G/ZUC/KASUMI PMDs
+--
+
+This PMD supports wireless cipher suite (SNOW3G, ZUC and KASUMI).
+On Intel processors, it is recommended to use this PMD instead of SNOW3G, ZUC 
and KASUMI PMDs,
+as it enables algorithm mixing (e.g. cipher algorithm SNOW3G-UEA2 with
+authentication algorithm AES-CMAC-128) and performance over IMIX (packet size 
mix) traffic
+is significantly higher.
 
 Installation
 
@@ -84,8 +90,8 @@ Installation
 To build DPDK with the AESNI_MB_PMD the user is required to download the 
multi-buffer
 library from `here `_
 and compile it on their user system before building DPDK.
-The latest version of the library supported by this PMD is v1.2, which
-can be downloaded from 
``_.
+The latest version of the library supported by this PMD is v1.3, which
+can be downloaded from 
``_.
 
 .. code-block:: console
 
@@ -130,8 +136,8 @@ and the Multi-Buffer library version supported by them:
18.05 - 19.02   0.49 - 0.52
19.05 - 19.08   0.52
19.11 - 20.08   0.52 - 0.55
-   20.11 - 21.08   0.53 - 1.2*
-   21.11+  1.0  - 1.2*
+   20.11 - 21.08   0.53 - 1.3*
+   21.11+  1.0  - 1.3*
==  
 
 \* Multi-buffer library 1.0 or newer only works for Meson but not Make build 
system.
diff --git a/doc/guides/cryptodevs/kasumi.rst b/doc/guides/cryptodevs/kasumi.rst
index d8128928f8..c8e8f1b847 100644
--- a/doc/guides/cryptodevs/kasumi.rst
+++ b/doc/guides/cryptodevs/kasumi.rst
@@ -30,14 +30,21 @@ Limitations
   (if length and/or offset of data to be ciphered is not byte-aligned).
 
 
+KASUMI PMD vs AESNI MB PMD
+--

Re: Is it correct to report checksum good when there is no checksum?

[Bruce Richardson]

On Thu, Nov 10, 2022 at 12:02:48PM +0100, Morten Brørup wrote:
> > From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> > Sent: Thursday, 10 November 2022 11.34
> > 
> > On 11/10/22 13:29, Morten Brørup wrote:
> > >> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> > >> Sent: Thursday, 10 November 2022 11.09
> > >>
> > >> On 11/10/22 12:55, Morten Brørup wrote:
> >  From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> >  Sent: Thursday, 10 November 2022 10.26
> > 
> >  Hi all,
> > 
> >  some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> >  For me it looks strange, but I see some technical reasons behind.
> > >>>
> > >>> Please note: IPv6 packets by definition have no IP checksum.
> > >>>
> >  Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >  Should UNKNOWN or NONE be used instead?
> > >>>
> > >>> Certainly not NONE. Its description says: "the IP checksum is *not*
> > >> correct in the packet [...]". But there is no incorrect IP checksum
> > in
> > >> the packet.
> > >>>
> > >>
> > >> Thanks, I should read the definition of none more careful.
> > >>
> > >>> I will argue against UNKNOWN. Its description says: "no information
> > >> about the RX IP checksum". But we do have information about it! We
> > know
> > >> that the IP checksum is not there (the value is "NULL"), and that it
> > is
> > >> not supposed to be there (the value is supposed to be "NULL").
> > >>>
> > >>
> > >> I thought that "no checksum" => "no information" => UNKNOWN
> > >
> > > That was my initial interpretation too, and it stuck with me for a
> > while.
> > >
> > > But then I tried hard to read it differently, tweaking it to support
> > the conclusion I was looking for.
> > >
> > >>
> > >>> So I consider GOOD the correct response here.
> > >>>
> > >>> GOOD also means that the application can proceed processing the
> > >> packet normally without further IP header checksum checking, so it's
> > >> good for performance.
> > >>>
> > >>
> > >> It is very important point and would be nice to have in GOOD
> > >> case definition (both IP and L4 cases). It is the right
> > >> motivation why GOOD makes sense for IPv6.
> > >>
> > >>> It should be added to the description of
> > RTE_MBUF_F_RX_IP_CKSUM_GOOD
> > >> that IPv6 packets always return this value, because IPv6 packets
> > have
> > >> no IP header checksum, and that is what is expected of them.
> > >>>
> > >>
> > >> Could you make a patch?
> > >
> > > Too busy right now, but I'll put it on my todo list. :-)
> > >
> > >>
> > >> Bonus question is UDP checksum 0 case. GOOD as well?
> > >> (just want to clarify the documentation while we're on it).
> > >
> > > No. The UDP checksum is not optional in IPv6.
> > >
> > > RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
> > are originated by an IPv6 node, the UDP checksum is not optional. [...]
> > IPv6 receivers must discard UDP packets containing a zero checksum, and
> > should log the error."
> > >
> > 
> > Yes I know, but I'm asking about IPv4 case with UDP checksum 0.
> 
> It cannot be UNKNOWN, because we do have information: The checksum was 
> intentionally omitted.
> 
> I would prefer GOOD, using the same logic as for the IPv6 header checksum.
> 
> Trying very hard to tweak the meaning of NONE's description ("the L4 checksum 
> is not correct in the packet data, but the integrity of the L4 data is 
> verified."), we could argue that "not correct" != "intentionally omitted" 
> (and an intentional omission is absolutely correct), and conclude that it 
> cannot be NONE. A seasoned politician would say this without blinking, but it 
> is up to individual interpretation.
> 
> We should settle on either GOOD or NONE, and write it in the documentation.
> 
> In a perfect world, the PMD DPDK compliance tests should also check things 
> like this.
> 

I would think that for cases where the checksum is intentionally omitted we
either add a new flag for "not applicable" or else just go with "good" as
you suggest. I think for simplicity to go with the latter.

Can we redefine "GOOD" to just mean "does not need to be checked by
software", rather than trying to define it in terms of what was done by
hardware?

/Bruce

RE: [PATCH v3] doc: support IPsec Multi-buffer lib v1.3

[Dooley, Brian]

Hi Pablo,

> -Original Message-
> From: Pablo de Lara 
> Sent: Thursday, November 10, 2022 11:07 AM
> To: Ji, Kai 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo ;
> Power, Ciara 
> Subject: [PATCH v3] doc: support IPsec Multi-buffer lib v1.3
> 
> Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD
> documentation guides with information about the latest Intel IPSec Multi-
> buffer library supported.
> 
> Signed-off-by: Pablo de Lara 
> Acked-by: Ciara Power 
> 
> ---
> -v3: Fixed library version from 1.2 to 1.3 in one line
> -v2: Removed repeated word 'the'
> ---
> 


Acked-by: Brian Dooley 

Re: Is it correct to report checksum good when there is no checksum?

[Andrew Rybchenko]

On 11/10/22 14:02, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 11.34

On 11/10/22 13:29, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 11.09

On 11/10/22 12:55, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 10.26

Hi all,

some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
For me it looks strange, but I see some technical reasons behind.


Please note: IPv6 packets by definition have no IP checksum.


Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
Should UNKNOWN or NONE be used instead?


Certainly not NONE. Its description says: "the IP checksum is *not*

correct in the packet [...]". But there is no incorrect IP checksum

in

the packet.




Thanks, I should read the definition of none more careful.


I will argue against UNKNOWN. Its description says: "no information

about the RX IP checksum". But we do have information about it! We

know

that the IP checksum is not there (the value is "NULL"), and that it

is

not supposed to be there (the value is supposed to be "NULL").




I thought that "no checksum" => "no information" => UNKNOWN


That was my initial interpretation too, and it stuck with me for a

while.


But then I tried hard to read it differently, tweaking it to support

the conclusion I was looking for.





So I consider GOOD the correct response here.

GOOD also means that the application can proceed processing the

packet normally without further IP header checksum checking, so it's
good for performance.




It is very important point and would be nice to have in GOOD
case definition (both IP and L4 cases). It is the right
motivation why GOOD makes sense for IPv6.


It should be added to the description of

RTE_MBUF_F_RX_IP_CKSUM_GOOD

that IPv6 packets always return this value, because IPv6 packets

have

no IP header checksum, and that is what is expected of them.




Could you make a patch?


Too busy right now, but I'll put it on my todo list. :-)



Bonus question is UDP checksum 0 case. GOOD as well?
(just want to clarify the documentation while we're on it).


No. The UDP checksum is not optional in IPv6.

RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets

are originated by an IPv6 node, the UDP checksum is not optional. [...]
IPv6 receivers must discard UDP packets containing a zero checksum, and
should log the error."




Yes I know, but I'm asking about IPv4 case with UDP checksum 0.


It cannot be UNKNOWN, because we do have information: The checksum was 
intentionally omitted.



I think that UNKNOWN definition should be updated to say that
it means that checksum is present and could be verified, but
NIC has not done it and application should do it itself.


I would prefer GOOD, using the same logic as for the IPv6 header checksum.



Yes, since it correct checksum from UDP over IPv4 protocol
definition. Application simply has no information to verify
checksum, so it cannot be UNKNOWN.

Since application gets entry packet in DPDK case, in GOOD case
it could check if checksum is 0 or not itself and do extra
checks in 0 case if it is possible (higher layer checksums etc)
and required.


Trying very hard to tweak the meaning of NONE's description ("the L4 checksum is not correct in the 
packet data, but the integrity of the L4 data is verified."), we could argue that "not 
correct" != "intentionally omitted" (and an intentional omission is absolutely correct), and 
conclude that it cannot be NONE. A seasoned politician would say this without blinking, but it is up to 
individual interpretation.

We should settle on either GOOD or NONE, and write it in the documentation.



NONE requires "but the integrity of the L4 data is verified".
Who said that NIC has verified L4 data integrity?


In a perfect world, the PMD DPDK compliance tests should also check things like 
this.



JFYI The initial IPv6 question comes from my attempt to
classify [1]. Now I understand that the test should be
fixed to expect GOOD in IPv6 case, not UNKNOWN.

[1] 
https://ts-factory.io/bublik/v2/log/163204?focusId=164090&mode=treeAndinfoAndlog

Re: Is it correct to report checksum good when there is no checksum?

[Andrew Rybchenko]

On 11/10/22 14:11, Bruce Richardson wrote:

On Thu, Nov 10, 2022 at 12:02:48PM +0100, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 11.34

On 11/10/22 13:29, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 11.09

On 11/10/22 12:55, Morten Brørup wrote:

From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
Sent: Thursday, 10 November 2022 10.26

Hi all,

some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
For me it looks strange, but I see some technical reasons behind.


Please note: IPv6 packets by definition have no IP checksum.


Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
Should UNKNOWN or NONE be used instead?


Certainly not NONE. Its description says: "the IP checksum is *not*

correct in the packet [...]". But there is no incorrect IP checksum

in

the packet.




Thanks, I should read the definition of none more careful.


I will argue against UNKNOWN. Its description says: "no information

about the RX IP checksum". But we do have information about it! We

know

that the IP checksum is not there (the value is "NULL"), and that it

is

not supposed to be there (the value is supposed to be "NULL").




I thought that "no checksum" => "no information" => UNKNOWN


That was my initial interpretation too, and it stuck with me for a

while.


But then I tried hard to read it differently, tweaking it to support

the conclusion I was looking for.





So I consider GOOD the correct response here.

GOOD also means that the application can proceed processing the

packet normally without further IP header checksum checking, so it's
good for performance.




It is very important point and would be nice to have in GOOD
case definition (both IP and L4 cases). It is the right
motivation why GOOD makes sense for IPv6.


It should be added to the description of

RTE_MBUF_F_RX_IP_CKSUM_GOOD

that IPv6 packets always return this value, because IPv6 packets

have

no IP header checksum, and that is what is expected of them.




Could you make a patch?


Too busy right now, but I'll put it on my todo list. :-)



Bonus question is UDP checksum 0 case. GOOD as well?
(just want to clarify the documentation while we're on it).


No. The UDP checksum is not optional in IPv6.

RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets

are originated by an IPv6 node, the UDP checksum is not optional. [...]
IPv6 receivers must discard UDP packets containing a zero checksum, and
should log the error."




Yes I know, but I'm asking about IPv4 case with UDP checksum 0.


It cannot be UNKNOWN, because we do have information: The checksum was 
intentionally omitted.

I would prefer GOOD, using the same logic as for the IPv6 header checksum.

Trying very hard to tweak the meaning of NONE's description ("the L4 checksum is not correct in the 
packet data, but the integrity of the L4 data is verified."), we could argue that "not 
correct" != "intentionally omitted" (and an intentional omission is absolutely correct), and 
conclude that it cannot be NONE. A seasoned politician would say this without blinking, but it is up to 
individual interpretation.

We should settle on either GOOD or NONE, and write it in the documentation.

In a perfect world, the PMD DPDK compliance tests should also check things like 
this.



I would think that for cases where the checksum is intentionally omitted we
either add a new flag for "not applicable" or else just go with "good" as
you suggest. I think for simplicity to go with the latter.

Can we redefine "GOOD" to just mean "does not need to be checked by
software", rather than trying to define it in terms of what was done by
hardware?


Yes, it is very good idea since we are providing the
information to application and make it clear what the
application should do.

RE: [PATCH v3] doc: support IPsec Multi-buffer lib v1.3

[Ji, Kai]

Do we need to add the section about chacha-poly PMD ? 
is that chacha-poly has a better performance  in AESNI PMD compare to Chacha 
PMD ?

regards

Kai 

> -Original Message-
> From: De Lara Guarch, Pablo 
> Sent: Thursday, November 10, 2022 11:07 AM
> To: Ji, Kai 
> Cc: dev@dpdk.org; De Lara Guarch, Pablo ;
> Power, Ciara 
> Subject: [PATCH v3] doc: support IPsec Multi-buffer lib v1.3
> 
> Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD
> documentation guides with information about the latest Intel IPSec 
> Multi-buffer
> library supported.
> 
> Signed-off-by: Pablo de Lara 
> Acked-by: Ciara Power 
> 
> ---
> -v3: Fixed library version from 1.2 to 1.3 in one line
> -v2: Removed repeated word 'the'
> ---

RE: Is it correct to report checksum good when there is no checksum?

[Morten Brørup]

> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 12.26
> 
> On 11/10/22 14:11, Bruce Richardson wrote:
> > On Thu, Nov 10, 2022 at 12:02:48PM +0100, Morten Brørup wrote:
> >>> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> >>> Sent: Thursday, 10 November 2022 11.34
> >>>
> >>> On 11/10/22 13:29, Morten Brørup wrote:
> > From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> > Sent: Thursday, 10 November 2022 11.09
> >
> > On 11/10/22 12:55, Morten Brørup wrote:
> >>> From: Andrew Rybchenko [mailto:andrew.rybche...@oktetlabs.ru]
> >>> Sent: Thursday, 10 November 2022 10.26
> >>>
> >>> Hi all,
> >>>
> >>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6
> packets.
> >>> For me it looks strange, but I see some technical reasons
> behind.
> >>
> >> Please note: IPv6 packets by definition have no IP checksum.
> >>
> >>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >>> Should UNKNOWN or NONE be used instead?
> >>
> >> Certainly not NONE. Its description says: "the IP checksum is
> *not*
> > correct in the packet [...]". But there is no incorrect IP
> checksum
> >>> in
> > the packet.
> >>
> >
> > Thanks, I should read the definition of none more careful.
> >
> >> I will argue against UNKNOWN. Its description says: "no
> information
> > about the RX IP checksum". But we do have information about it!
> We
> >>> know
> > that the IP checksum is not there (the value is "NULL"), and that
> it
> >>> is
> > not supposed to be there (the value is supposed to be "NULL").
> >>
> >
> > I thought that "no checksum" => "no information" => UNKNOWN
> 
>  That was my initial interpretation too, and it stuck with me for a
> >>> while.
> 
>  But then I tried hard to read it differently, tweaking it to
> support
> >>> the conclusion I was looking for.
> 
> >
> >> So I consider GOOD the correct response here.
> >>
> >> GOOD also means that the application can proceed processing the
> > packet normally without further IP header checksum checking, so
> it's
> > good for performance.
> >>
> >
> > It is very important point and would be nice to have in GOOD
> > case definition (both IP and L4 cases). It is the right
> > motivation why GOOD makes sense for IPv6.
> >
> >> It should be added to the description of
> >>> RTE_MBUF_F_RX_IP_CKSUM_GOOD
> > that IPv6 packets always return this value, because IPv6 packets
> >>> have
> > no IP header checksum, and that is what is expected of them.
> >>
> >
> > Could you make a patch?
> 
>  Too busy right now, but I'll put it on my todo list. :-)
> 
> >
> > Bonus question is UDP checksum 0 case. GOOD as well?
> > (just want to clarify the documentation while we're on it).
> 
>  No. The UDP checksum is not optional in IPv6.
> 
>  RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
> >>> are originated by an IPv6 node, the UDP checksum is not optional.
> [...]
> >>> IPv6 receivers must discard UDP packets containing a zero checksum,
> and
> >>> should log the error."
> 
> >>>
> >>> Yes I know, but I'm asking about IPv4 case with UDP checksum 0.
> >>
> >> It cannot be UNKNOWN, because we do have information: The checksum
> was intentionally omitted.
> >>
> >> I would prefer GOOD, using the same logic as for the IPv6 header
> checksum.
> >>
> >> Trying very hard to tweak the meaning of NONE's description ("the L4
> checksum is not correct in the packet data, but the integrity of the L4
> data is verified."), we could argue that "not correct" !=
> "intentionally omitted" (and an intentional omission is absolutely
> correct), and conclude that it cannot be NONE. A seasoned politician
> would say this without blinking, but it is up to individual
> interpretation.
> >>
> >> We should settle on either GOOD or NONE, and write it in the
> documentation.
> >>
> >> In a perfect world, the PMD DPDK compliance tests should also check
> things like this.
> >>
> >
> > I would think that for cases where the checksum is intentionally
> omitted we
> > either add a new flag for "not applicable" or else just go with
> "good" as
> > you suggest. I think for simplicity to go with the latter.
> >
> > Can we redefine "GOOD" to just mean "does not need to be checked by
> > software", rather than trying to define it in terms of what was done
> by
> > hardware?
> 
> Yes, it is very good idea since we are providing the
> information to application and make it clear what the
> application should do.
> 

We need to think this all the way through:

Eventually, this information will be carried over to the TX side, so we also 
need to consider TX handling.

E.g. GOOD means that the packet is good to go, and does not need the checksum 
to be updated on the way out.

RE: [PATCH] net/mlx5: fix port initialization with small LRO

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Gregory Etelson 
> Sent: Wednesday, November 9, 2022 6:51 PM
> To: dev@dpdk.org
> Cc: Gregory Etelson ; Matan Azrad
> ; Raslan Darawsheh ;
> sta...@dpdk.org; Slava Ovsiienko 
> Subject: [PATCH] net/mlx5: fix port initialization with small LRO
> 
> If application provided maximal LRO size was less than expected PMD
> minimum, the PMD either crashed with assert, if asserts were enabled,
> or proceeded with port initialization to set port private maximal
> LRO size below supported minimum.
> 
> The patch terminates port start if LRO size
> does not match PMD requirements and TCP LRO offload was requested
> at least for one Rx queue.
> 
> Fixes: 50c00baff763 ("net/mlx5: limit LRO size to maximum Rx packet")
> 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Gregory Etelson 
> Acked-by: Matan Azrad 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [PATCH v2] net/mlx5: fix port's event cleaning order

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Michael Baum 
> Sent: Thursday, November 10, 2022 12:30 AM
> To: dev@dpdk.org
> Cc: Matan Azrad ; Raslan Darawsheh
> ; Slava Ovsiienko ;
> dkozl...@nvidia.com; sta...@dpdk.org
> Subject: [PATCH v2] net/mlx5: fix port's event cleaning order
> 
> The shared IB device (sh) has per port data with filed for interrupt
> handler port_id. It used by shared interrupt handler to find the
> corresponding rte_eth device by IB port index.
> If value is equal or greater RTE_MAX_ETHPORTS it means there is no
> subhandler installed for specified IB port index.
> 
> When a few ports are created under same sh, the sh is created with the
> first port and the interrupt handler port_id is initialized to
> RTE_MAX_ETHPORTS for each port.
> In port creation, the interrupt handler port_id is updated with the
> correct value. Since this updating, the mlx5_dev_interrupt_nl_cb
> function uses this port and its priv structure.
> However, when the ports are closed, this filed isn't updated and the
> interrupt handler continue working until it is uninstalled in SH
> destruction.
> If mlx5_dev_interrupt_nl_cb is called between port closing and SH
> destruction, it uses invalid port causing a crash.
> 
> This patch adds interrupt handler port_id updating to the close function
> and add memory barrier to make sure it is done before priv reset.
> 
> Fixes: 655c3c26c11e ("net/mlx5: fix initial link status detection")
> Cc: dkozl...@nvidia.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Michael Baum 
> Acked-by: Matan Azrad 
> ---
> 
> v2: fix typo in commit message.
> 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [PATCH v1] net/mlx5: fix missing marks on received packets

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Rongwei Liu 
> Sent: Thursday, November 10, 2022 4:10 AM
> To: Matan Azrad ; Slava Ovsiienko
> ; Ori Kam ; NBU-Contact-
> Thomas Monjalon (EXTERNAL) ; Suanming Mou
> 
> Cc: dev@dpdk.org; Raslan Darawsheh ;
> sta...@dpdk.org
> Subject: [PATCH v1] net/mlx5: fix missing marks on received packets
> 
> If HW Steering is enabled, Rx queues were configured to receive MARKs
> when a table with MARK actions was created. After stopping the port, Rx
> queue configuration is released, but during starting the port the mark flag
> was not updated in the Rx queue configuration.
> 
> This patch introduces a reference count on the MARK action and it
> increases/decreases per template_table create/destroy.
> 
> When the port is stopped, Rx queue configuration is not cleared if reference
> count is not zero.
> 
> Fixes: 3a2f674b6aa8 ("net/mlx5: add queue and RSS HW steering action")
> Cc: sta...@dpdk.org
> Signed-off-by: Rongwei Liu 
> Acked-by: Matan Azrad 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [PATCH] net/mlx5: fix drop action validation

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Shun Hao 
> Sent: Thursday, November 10, 2022 8:59 AM
> To: Slava Ovsiienko ; Matan Azrad
> ; Ori Kam 
> Cc: dev@dpdk.org; Raslan Darawsheh ;
> sta...@dpdk.org
> Subject: [PATCH] net/mlx5: fix drop action validation
> 
> Currently there's limitation for Drop action that can only co-exist with
> Count action.
> 
> Sample and Age actions are also able to exist with Drop within the same
> flow, and this patch includes them in the Drop action validation.
> 
> Fixes: acb67cc8 ("net/mlx5: fix action flag data type")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Shun Hao 
> Acked-by: Matan Azrad 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

[PATCH] common/mlx5: fix DevX register read error severity

[Gregory Etelson]

PMD attempt to read HW UTC counter properties can fail because the feature
has no support in port FW or mlx5 kernel module.

In that case PMD still can produce correct time-stamps if it runs on core with
nanosecond time resolution.

Fixes: b0067860959d ("common/mlx5: update log for DevX general command failure")

Cc: sta...@dpdk.org

Signed-off-by: Gregory Etelson 
Acked-by: Matan Azrad 
---
 drivers/common/mlx5/mlx5_devx_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c 
b/drivers/common/mlx5/mlx5_devx_cmds.c
index 05b9429c7f..59cebb530f 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -111,7 +111,7 @@ mlx5_devx_cmd_register_read(void *ctx, uint16_t reg_id, 
uint32_t arg,
 MLX5_ST_SZ_BYTES(access_register_out) +
 sizeof(uint32_t) * dw_cnt);
if (rc || MLX5_FW_STATUS(out)) {
-   DEVX_DRV_LOG(ERR, out, "read access", "NIC register", reg_id);
+   DEVX_DRV_LOG(DEBUG, out, "read access", "NIC register", reg_id);
return MLX5_DEVX_ERR_RC(rc);
}
memcpy(data, &out[MLX5_ST_SZ_DW(access_register_out)],
-- 
2.34.1

RE: [PATCH] examples/ipsec-secgw: fix uninitialized variable access

[Akhil Goyal]

> Subject: [PATCH] examples/ipsec-secgw: fix uninitialized variable access
> 
> Fix uninitialized variable access of outbound offloads flags.
> 
> Coverity issue: 381669
> Fixes: 6938fc92c404 ("examples/ipsec-secgw: add lookaside event mode")
> 
> Signed-off-by: Volodymyr Fialko 
Applied to dpdk-next-crypto

Thanks.

[PATCH 1/6] doc: fix underlines too long in testpmd documentation

[Michael Baum]

In testpmd documentation, there are two underlines which should not
match the length of the text above.

This patch update them to be align with the guideline [1].

[1]
https://doc.dpdk.org/guides/contributing/documentation.html#section-headers

Fixes: a69c335d56b5 ("doc: add flow dump command in testpmd guide")
Fixes: 0e459ffa0889 ("app/testpmd: support flow aging")
Cc: jack...@mellanox.com
Cc: do...@mellanox.com
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 96c5ae0fe4..b5649d9d9a 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -4240,7 +4240,7 @@ Disabling isolated mode::
  testpmd>
 
 Dumping HW internal information
-
+~~~
 
 ``flow dump`` dumps the hardware's internal representation information of
 all flows. It is bound to ``rte_flow_dev_dump()``::
@@ -4256,7 +4256,7 @@ Otherwise, it will complain error occurred::
Caught error type [...] ([...]): [...]
 
 Listing and destroying aged flow rules
-
+~~
 
 ``flow aged`` simply lists aged flow rules be get from api 
``rte_flow_get_aged_flows``,
 and ``destroy`` parameter can be used to destroy those flow rules in PMD.
-- 
2.25.1

[PATCH 0/6] doc: some fixes

[Michael Baum]

Some doc fixes in testpmd doc and release notes.

Michael Baum (6):
  doc: fix underlines too long in testpmd documentation
  doc: fix the colon type in listing aged flow rules
  doc: fix miss blank line in testpmd flow syntax doc
  doc: fix miss blank line in release notes
  doc: add mlx5 HWS aging support to release notes
  doc: add ethdev pre-config flags to release notes

 doc/guides/rel_notes/release_22_11.rst  |  8 
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 ++-
 2 files changed, 14 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH 2/6] doc: fix the colon type in listing aged flow rules

[Michael Baum]

In testpmd documentation, for listing aged-out flow rules there is some
boxes of examples.

In Sphinx syntax, those boxes are achieved by "::" before. However,
in two places it uses ":" instead and the example looks like a regular
text.

This patch replace the ":" with "::" to get code box.

Fixes: 0e459ffa0889 ("app/testpmd: support flow aging")
Cc: do...@mellanox.com
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index b5649d9d9a..b5fea1396c 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -4259,7 +4259,7 @@ Listing and destroying aged flow rules
 ~~
 
 ``flow aged`` simply lists aged flow rules be get from api 
``rte_flow_get_aged_flows``,
-and ``destroy`` parameter can be used to destroy those flow rules in PMD.
+and ``destroy`` parameter can be used to destroy those flow rules in PMD::
 
flow aged {port_id} [destroy]
 
@@ -4294,7 +4294,7 @@ will be ID 3, ID 1, ID 0::
1   0   0   i--
0   0   0   i--
 
-If attach ``destroy`` parameter, the command will destroy all the list aged 
flow rules.
+If attach ``destroy`` parameter, the command will destroy all the list aged 
flow rules::
 
testpmd> flow aged 0 destroy
Port 0 total aged flows: 4
-- 
2.25.1

[PATCH 3/6] doc: fix miss blank line in testpmd flow syntax doc

[Michael Baum]

In flow syntax documentation, there is example for create pattern
template.

Before the example, miss a blank line causing it to look regular bold
text.
In addition, inside the example, it uses tab instead of spaces which
expand the indentation in one line.

This patch adds the blank line and replaces tab with spaces.

Fixes: 04cc665fab38 ("app/testpmd: add flow template management")
Cc: akozy...@nvidia.com
Cc: sta...@dpdk.org

Signed-off-by: Michael Baum 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index b5fea1396c..0037506a79 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2894,9 +2894,10 @@ following sections.
[meters_number {number}] [flags {number}]
 
 - Create a pattern template::
+
flow pattern_template {port_id} create [pattern_template_id {id}]
[relaxed {boolean}] [ingress] [egress] [transfer]
-  template {item} [/ {item} [...]] / end
+   template {item} [/ {item} [...]] / end
 
 - Destroy a pattern template::
 
-- 
2.25.1

[PATCH 6/6] doc: add ethdev pre-config flags to release notes

[Michael Baum]

Add to release notes:
1. Flags field in pre-configuration structure and strict-queue flag.

Fixes: dcc9a80c20b8 ("ethdev: add strict queue to pre-configuration flow hints")
Cc: michae...@nvidia.com

Signed-off-by: Michael Baum 
---
 doc/guides/rel_notes/release_22_11.rst | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_11.rst 
b/doc/guides/rel_notes/release_22_11.rst
index c27980a1aa..dfce601f8a 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -108,6 +108,12 @@ New Features
   Each flag has a corresponding capability flag
   in ``struct rte_eth_hairpin_queue_cap``.
 
+* **Added strict queue to pre-configuration flow hints.**
+
+  * Added flags option to ``rte_flow_configure`` and ``rte_flow_info_get``.
+  * Added ``RTE_FLOW_PORT_FLAG_STRICT_QUEUE`` flag to indicate all operations
+for a given flow rule will strictly happen on the same queue.
+
 * **Added configuration for asynchronous flow connection tracking.**
 
   Added connection tracking action number hint to ``rte_flow_configure``
-- 
2.25.1

[PATCH 4/6] doc: fix miss blank line in release notes

[Michael Baum]

The NVIDIA mlx5 driver inside 22.11 release notes, lists all features
support for queue-based async HW steering.

Before the list, miss a blank line causing it to look regular text line.

This patch adds the blank line as well.

Fixes: ddb68e47331e ("net/mlx5: add extended metadata mode for HWS")
Fixes: 0f4aa72b99da ("net/mlx5: support flow modify field with HWS")
Cc: bi...@nvidia.com
Cc: suanmi...@nvidia.com

Signed-off-by: Michael Baum 
---
 doc/guides/rel_notes/release_22_11.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/rel_notes/release_22_11.rst 
b/doc/guides/rel_notes/release_22_11.rst
index 61f7d4d0aa..7c50af38c6 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -185,6 +185,7 @@ New Features
 * **Updated NVIDIA mlx5 driver.**
 
   * Added full support for queue-based async HW steering.
+
 - Support of FDB.
 - Support of control flow and isolate mode.
 - Support of conntrack.
-- 
2.25.1

[PATCH 5/6] doc: add mlx5 HWS aging support to release notes

[Michael Baum]

Add to 22.11 release note the NVIDIA mlx5 HWS aging support.

Fixes: 04a4de756e14 ("net/mlx5: support flow age action with HWS")
Cc: michae...@nvidia.com

Signed-off-by: Michael Baum 
---
 doc/guides/rel_notes/release_22_11.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/rel_notes/release_22_11.rst 
b/doc/guides/rel_notes/release_22_11.rst
index 7c50af38c6..c27980a1aa 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -190,6 +190,7 @@ New Features
 - Support of control flow and isolate mode.
 - Support of conntrack.
 - Support of counter.
+- Support of aging.
 - Support of meter.
 - Support of modify fields.
 
-- 
2.25.1

Re: [PATCH] memif: memif driver does not crashes when there's different N of TX and RX queues

[Ferruh Yigit]

On 11/10/2022 10:02 AM, Huzaifa Rahman wrote:

Hi,

Is there any other work/changes required for this patch to be submitted?



Hi Huzaifa,

Patch seems already merged by Andrew and pulled to main repo:
https://git.dpdk.org/dpdk/commit/?id=231435a5e6c7fa915697d8f84a91b44176

So it will in oncoming 22.11 release.


Thanks


On Tue, Oct 4, 2022 at 7:53 PM Andrew Rybchenko 
mailto:andrew.rybche...@oktetlabs.ru>> 
wrote:


On 8/8/22 13:39, Joyce Kong wrote:
 > Hi Huzaifa,
 >
 > This patch looks good to me.
 > And would you please help review my memif patches?
 >

https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.k...@arm.com/
 

 >
 > Thanks,
 > Joyce
 >
 >> -Original Message-
 >> From: huzaifa.rahman mailto:huzaifa.rah...@emumba.com>>
 >> Sent: Tuesday, July 26, 2022 6:16 PM
 >> To: jgraj...@cisco.com 
 >> Cc: dev@dpdk.org ; huzaifa.rahman
mailto:huzaifa.rah...@emumba.com>>
 >> Subject: [PATCH] memif: memif driver does not crashes when there's
 >> different N of TX and RX queues
 > net/memif: fix memif crash with different Tx Rx queues
 >
 >>
 >> Bugzilla ID: 734
 >>
 >> there's a bug in memif_stats_get() function due to confusion
between C2S
 >> (client->server) and S2C (server->client) rings, causing a crash
if there's a
 >> different number of RX and TX queues.
 >>
 >> this is fixed by selectiing the correct rings for RX and TX i.e
for RX, S2C rings
 >> are selected and for TX, C2S rings are selected.
 >>
 > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
 > Cc: sta...@dpdk.org 
 >
 >> Signed-off-by: huzaifa.rahman mailto:huzaifa.rah...@emumba.com>>
 > Reviewed-by: Joyce Kong mailto:joyce.k...@arm.com>>

Fixed above on applying.

Applied to dpdk-next-net/main, thanks.

[PATCH v3 1/2] test/hash: fix coverity warning

[Vladimir Medvedkin]

Remove unnecessary variable assignment

Coverity issue: 336800
Fixes: 3f9aab961ed3 ("test/hash: check lock-free extendable bucket")
Cc: dharmik.thak...@arm.com
Cc: sta...@dpdk.org

Signed-off-by: Vladimir Medvedkin 
Reviewed-by: Ruifeng Wang 
---
 app/test/test_hash_readwrite_lf_perf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/test/test_hash_readwrite_lf_perf.c 
b/app/test/test_hash_readwrite_lf_perf.c
index 32f9ec9250..cf86046a2f 100644
--- a/app/test/test_hash_readwrite_lf_perf.c
+++ b/app/test/test_hash_readwrite_lf_perf.c
@@ -1102,7 +1102,6 @@ test_hash_multi_add_lookup(struct rwc_perf 
*rwc_perf_results, int rwc_lf,
rte_eal_remote_launch(test_rwc_reader,
(void *)(uintptr_t)read_type,
enabled_core_ids[i]);
-   write_type = WRITE_KEY_SHIFT;
pos_core = 0;
 
/* Launch writers */
-- 
2.25.1

[PATCH v3 2/2] test/hash: fix coverity warning

[Vladimir Medvedkin]

Check return value after bulk lookup

Coverity issue: 357746
Fixes: 14b8ab576235 ("hash: add bulk lookup with signatures array")
Cc: sta...@dpdk.org

Signed-off-by: Vladimir Medvedkin 
---
 app/test/test_hash_perf.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/test/test_hash_perf.c b/app/test/test_hash_perf.c
index 5d36c0f454..1a90acd1ba 100644
--- a/app/test/test_hash_perf.c
+++ b/app/test/test_hash_perf.c
@@ -475,6 +475,10 @@ timed_lookups_multi(unsigned int with_hash, unsigned int 
with_data,
(const void **)keys_burst,
&signatures[j * BURST_SIZE],
BURST_SIZE, positions_burst);
+   if (ret != 0) {
+   printf("rte_hash_lookup_with_hash_bulk 
failed with %d\n", ret);
+   return -1;
+   }
for (k = 0; k < BURST_SIZE; k++) {
if (positions_burst[k] !=
positions[j *
@@ -487,10 +491,14 @@ timed_lookups_multi(unsigned int with_hash, unsigned int 
with_data,
}
}
} else {
-   rte_hash_lookup_bulk(h[table_index],
+   ret = rte_hash_lookup_bulk(h[table_index],
(const void **) keys_burst,
BURST_SIZE,
positions_burst);
+   if (ret != 0) {
+   printf("rte_hash_lookup_bulk failed 
with %d\n", ret);
+   return -1;
+   }
for (k = 0; k < BURST_SIZE; k++) {
if (positions_burst[k] != positions[j * 
BURST_SIZE + k]) {
printf("Key looked up in %d, 
should be in %d\n",
-- 
2.25.1

Re: [PATCH v3 2/2] test/hash: fix coverity warning

[Bruce Richardson]

On Thu, Nov 10, 2022 at 03:13:34PM +, Vladimir Medvedkin wrote:
> Check return value after bulk lookup
> 
> Coverity issue: 357746
> Fixes: 14b8ab576235 ("hash: add bulk lookup with signatures array")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Vladimir Medvedkin 
> ---

Acked-by: Bruce Richardson 

[PATCH] common/mlx5: fix the sysfs port name translation

[Bing Zhao]

With some OFED or upstream kernel of mlx5, the port name fetched from
"/sys/class/net/[DEV]/phys_port_name" may have a tailing "\n" as the
EOL. The sscanf() will return the scanned items number with this EOL.

In such case, the "equal to" condition is considered as false and
the function mlx5_translate_port_name() will recognize the port type
wrongly with UNKNOWN result.

By changing the condition from "equal to" to "more than or equal
to", the port type can be recognized successfully.

Fixes: 654810b56828 ("common/mlx5: share Netlink commands")
Fixes: 420bbdae89f2 ("net/mlx5: fix host physical function representor naming")

Signed-off-by: Bing Zhao 
Acked-by: Viacheslav Ovsiienko 
---
 drivers/common/mlx5/linux/mlx5_common_os.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/common/mlx5/linux/mlx5_common_os.c 
b/drivers/common/mlx5/linux/mlx5_common_os.c
index aafff60eeb..655347a7c8 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.c
+++ b/drivers/common/mlx5/linux/mlx5_common_os.c
@@ -108,7 +108,7 @@ mlx5_translate_port_name(const char *port_name_in,
sc_items = sscanf(port_name_in, "%c%c%d%c%c%d%c",
  &pf_c1, &pf_c2, &port_info_out->pf_num,
  &vf_c1, &vf_c2, &port_info_out->port_name, &eol);
-   if (sc_items == 6 && pf_c1 == 'p' && pf_c2 == 'f') {
+   if (sc_items >= 6 && pf_c1 == 'p' && pf_c2 == 'f') {
if (vf_c1 == 'v' && vf_c2 == 'f') {
/* Kernel ver >= 5.0 or OFED ver >= 4.6 */
port_info_out->name_type =
@@ -128,7 +128,7 @@ mlx5_translate_port_name(const char *port_name_in,
 */
sc_items = sscanf(port_name_in, "%c%d%c",
  &pf_c1, &port_info_out->port_name, &eol);
-   if (sc_items == 2 && pf_c1 == 'p') {
+   if (sc_items >= 2 && pf_c1 == 'p') {
port_info_out->name_type = MLX5_PHYS_PORT_NAME_TYPE_UPLINK;
return;
}
@@ -138,7 +138,7 @@ mlx5_translate_port_name(const char *port_name_in,
 */
sc_items = sscanf(port_name_in, "%c%c%d%c",
  &pf_c1, &pf_c2, &port_info_out->pf_num, &eol);
-   if (sc_items == 3 && pf_c1 == 'p' && pf_c2 == 'f') {
+   if (sc_items >= 3 && pf_c1 == 'p' && pf_c2 == 'f') {
port_info_out->port_name = -1;
port_info_out->name_type = MLX5_PHYS_PORT_NAME_TYPE_PFHPF;
return;
-- 
2.21.0

Re: [PATCH v5] testpmd: cleanup cleanly from signal

[Stephen Hemminger]

> > +   stop_packet_forwarding();  
> 
> force_quit() calls stop_packet_forwarding() if test_done is 0.
> So, there is no difference in test_done == 0 case.
> If test_done is not zero, stop_packet_forwarding() just logs
> "Packet forwarding not started" and does nothing. So, the
> difference is only in error message. Is it intentional?
> 
> > +   force_quit();
> > }

Will fix in new version, it was a logic error.

Re: release candidate 22.11-rc2

[Thinh Tran]

Hi,

IBM - Power Systems
DPDK 22.11.0-rc2


* Basic PF on Mellanox: No new issues or regressions were seen.
* Performance: not tested.
* OS: RHEL 8.5  kernel: 4.18.0-348.el8.ppc64le
with gcc version 8.5.0 20210514 (Red Hat 8.5.0-10)
  RHEL 9.0  kernel: 5.14.0-70.13.1.el9_0.ppc64le
with gcc version 11.2.1 20220127 (Red Hat 11.2.1-9)

Systems tested:
 - IBM Power9 PowerNV 9006-22P
NICs:
 - Mellanox Technologies MT28800 Family [ConnectX-5 Ex]
 - firmware version: 16.34.1002
 - MLNX_OFED_LINUX-5.7-1.0.2.1 (OFED-5.7-1.0.2)

 - IBM Power10 PowerVM  IBM,9105-22A
NICs:
- Mellanox Technologies MT28800 Family [ConnectX-5 Ex]
- firmware version: 16.34.1002
- MLNX_OFED_LINUX-5.7-1.0.2.1 (OFED-5.7-1.0.2)

Regards,
Thinh Tran

On 10/31/2022 7:15 PM, Thomas Monjalon wrote:

A new DPDK release candidate is ready for testing:
https://git.dpdk.org/dpdk/tag/?id=v22.11-rc2

There are 422 new patches in this snapshot.

Release notes:
https://doc.dpdk.org/guides/rel_notes/release_22_11.html

There were a lot of updates in drivers, including 3 new drivers:
- GVE (Google Virtual Ethernet)
- IDPF (Intel DataPlane Function or Infrastructure DataPath Function)
- UADK (User Space Accelerator Development Kit) supporting HiSilicon 
crypto
The driver features should be frozen now.

Please test and report issues on bugs.dpdk.org.

Thank you everyone

Re: [PATCH] common/mlx5: fix DevX register read error severity

[David Marchand]

On Thu, Nov 10, 2022 at 2:06 PM Gregory Etelson  wrote:
>
> PMD attempt to read HW UTC counter properties can fail because the feature
> has no support in port FW or mlx5 kernel module.
>
> In that case PMD still can produce correct time-stamps if it runs on core with
> nanosecond time resolution.
>
> Fixes: b0067860959d ("common/mlx5: update log for DevX general command 
> failure")
>

No blank line here.

> Cc: sta...@dpdk.org
>

Reported-by: David Marchand 
> Signed-off-by: Gregory Etelson 
> Acked-by: Matan Azrad 

I had tested the same change to pass OVS unit tests.

I did not hit any other error level messages from functions changed in
b0067860959d ("common/mlx5: update log for DevX general command
failure").
So I guess only changing this one is enough.

Acked-by: David Marchand 


-- 
David Marchand

Re: [PATCH] common/mlx5: fix the sysfs port name translation

[Stephen Hemminger]

On Thu, 10 Nov 2022 17:57:26 +0200
Bing Zhao  wrote:

> With some OFED or upstream kernel of mlx5, the port name fetched from
> "/sys/class/net/[DEV]/phys_port_name" may have a tailing "\n" as the
> EOL. The sscanf() will return the scanned items number with this EOL.

Why not fix the DPDK driver to strip off the new line when the port
name is read? The code in mlx5_os.c should do it there.

Re: [PATCH v1 2/2] doc: increase python max line to 88

[Stephen Hemminger]

On Thu, 10 Nov 2022 09:15:16 +
Juraj Linkeš  wrote:

> > -Original Message-
> > From: Stephen Hemminger 
> > Sent: Friday, November 4, 2022 5:58 PM
> > To: Juraj Linkeš 
> > Cc: Honnappa Nagarahalli ; Owen Hilyard
> > ; tho...@monjalon.net; Lijuan Tu
> > ; Richardson, Bruce ;
> > dev@dpdk.org
> > Subject: Re: [PATCH v1 2/2] doc: increase python max line to 88
> > 
> > On Fri, 4 Nov 2022 09:16:13 +
> > Juraj Linkeš  wrote:
> >   
> > > > +max_line_length = 88 #
> > > >  
> > +https://black.readthedocs.io/en/stable/the_black_code_style/current_sty  
> > > > +le.html#li  
> > 
> > Skip the comment, it caused your line break!  
> 
> The fact that the line is a bit longer does not make the line less readable, 
> as the link is there to be copy-pasted (and I don't think anyone reads the 
> full hyperlinks - the knowledge of domain is enough). As such I think it's 
> better to include the link as it serves as self-documentation (that we're 
> deviating from the standard).
> 
> We could move the comment before or after the max_line_length option and that 
> would result in lines below 100 characters (which is what .editorconfig 
> prescribes). I used that in my one of my local versions, but it was less 
> readable in my opinion. I'd rather break the rule and have it be more 
> readable.
> 
> Of course, not having the comment is fine, since we document it in the coding 
> style guide. I just think there's no (or very little) downside and some 
> upside (more than downside) in adding the comment.

The choice of max line length is project specific. Referencing some other style 
guide
doesn't seem necessary.  My choice would be to use 100 like the other C code.

Re: [PATCH v5 3/3] mempool: use cache for frequently updated stats

[Thomas Monjalon]

09/11/2022 19:18, Morten Brørup:
> When built with stats enabled (RTE_LIBRTE_MEMPOOL_STATS defined), the
> performance of mempools with caches is improved as follows.

Series applied, thanks.

Re: [PATCH v3] doc: support IPsec Multi-buffer lib v1.3

[Zhang, Fan]

Hi Pablo,

On 11/10/2022 11:07 AM, Pablo de Lara wrote:

Updated AESNI MB and AESNI GCM, KASUMI, ZUC and SNOW3G PMD documentation
guides with information about the latest Intel IPSec Multi-buffer
library supported.

Signed-off-by: Pablo de Lara 
Acked-by: Ciara Power 

---
-v3: Fixed library version from 1.2 to 1.3 in one line
-v2: Removed repeated word 'the'
---

This to me is a great change that formally recommending one PMD over the 
other for performance reason.


Do you think release notes should be updated to shut out this change?


Regards,

Fan


Other than that

Acked-by: Fan Zhang 

[PATCH v6] testpmd: cleanup cleanly from signal

[Stephen Hemminger]

Do a clean shutdown of testpmd when a signal is received;
instead of having testpmd kill itself.
This fixes the problem where a signal could be received
in the middle of a PMD and then the signal handler would call
PMD's close routine leading to locking problems.

An added benefit is it gets rid of some Windows specific code.

Fixes: d9a191a00e81 ("app/testpmd: fix quitting in container")
Signed-off-by: Stephen Hemminger 
---
v5 - drop unnecessary print in signal handler.
 don't cleanup twice
 don't print message when select() is interrupted.

 app/test-pmd/testpmd.c | 72 --
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index cf5942d0c422..62d87f758ac8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,7 @@
 #include 
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include 
+#include 
 #endif
 #include 
 #include 
@@ -4224,13 +4225,6 @@ init_port(void)
memset(txring_numa, NUMA_NO_CONFIG, RTE_MAX_ETHPORTS);
 }
 
-static void
-force_quit(void)
-{
-   pmd_test_exit();
-   prompt_exit();
-}
-
 static void
 print_stats(void)
 {
@@ -4249,28 +4243,9 @@ print_stats(void)
 }
 
 static void
-signal_handler(int signum)
+signal_handler(int signum __rte_unused)
 {
-   if (signum == SIGINT || signum == SIGTERM) {
-   fprintf(stderr, "\nSignal %d received, preparing to exit...\n",
-   signum);
-#ifdef RTE_LIB_PDUMP
-   /* uninitialize packet capture framework */
-   rte_pdump_uninit();
-#endif
-#ifdef RTE_LIB_LATENCYSTATS
-   if (latencystats_enabled != 0)
-   rte_latencystats_uninit();
-#endif
-   force_quit();
-   /* Set flag to indicate the force termination. */
-   f_quit = 1;
-   /* exit with the expected status */
-#ifndef RTE_EXEC_ENV_WINDOWS
-   signal(signum, SIG_DFL);
-   kill(getpid(), signum);
-#endif
-   }
+   f_quit = 1;
 }
 
 int
@@ -4449,9 +4424,6 @@ main(int argc, char** argv)
} else
 #endif
{
-   char c;
-   int rc;
-
f_quit = 0;
 
printf("No commandline core given, start packet forwarding\n");
@@ -4476,15 +4448,41 @@ main(int argc, char** argv)
prev_time = cur_time;
rte_delay_us_sleep(US_PER_S);
}
-   }
+   } else {
+   char c;
+   fd_set fds;
+   int rc;
 
-   printf("Press enter to exit\n");
-   rc = read(0, &c, 1);
-   pmd_test_exit();
-   if (rc < 0)
-   return 1;
+   printf("Press enter to exit\n");
+
+   FD_ZERO(&fds);
+   FD_SET(0, &fds);
+
+   rc = select(1, &fds, NULL, NULL, NULL);
+   if (rc < 0 && errno != EINTR) {
+   fprintf(stderr, "Select failed: %s\n",
+   strerror(errno));
+   return 1;
+   }
+   if (rc > 0)
+   rc = read(0, &c, 1);
+
+   pmd_test_exit();
+   if (rc < 0)
+   return 1;
+   prompt_exit();
+   }
}
 
+#ifdef RTE_LIB_PDUMP
+   /* uninitialize packet capture framework */
+   rte_pdump_uninit();
+#endif
+#ifdef RTE_LIB_LATENCYSTATS
+   if (latencystats_enabled != 0)
+   rte_latencystats_uninit();
+#endif
+
ret = rte_eal_cleanup();
if (ret != 0)
rte_exit(EXIT_FAILURE,
-- 
2.35.1

RE: [PATCH] common/mlx5: fix DevX register read error severity

[Raslan Darawsheh]

Hi,

> -Original Message-
> From: Gregory Etelson 
> Sent: Thursday, November 10, 2022 3:05 PM
> To: dev@dpdk.org
> Cc: Gregory Etelson ; Matan Azrad
> ; Raslan Darawsheh ;
> sta...@dpdk.org; Slava Ovsiienko 
> Subject: [PATCH] common/mlx5: fix DevX register read error severity
> 
> PMD attempt to read HW UTC counter properties can fail because the
> feature
> has no support in port FW or mlx5 kernel module.
> 
> In that case PMD still can produce correct time-stamps if it runs on core with
> nanosecond time resolution.
> 
> Fixes: b0067860959d ("common/mlx5: update log for DevX general command
> failure")
> removed extra blank line
> Cc: sta...@dpdk.org
> 
added reported-by tag
> Signed-off-by: Gregory Etelson 
> Acked-by: Matan Azrad 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

[PATCH] failsafe: fix segfault on hotplug event

[Luc Pelletier]

When the failsafe PMD encounters a hotplug event, it switches its rx/tx
functions to "safe" ones that validate the sub-device's rx/tx functions
before calling them. It switches the rx/tx functions by changing the
function pointers in the rte_eth_dev structure.

Following commit 7a0935239b, the rx/tx functions of PMDs are no longer
called through the function pointers in the rte_eth_dev structure. They
are rather called through a flat array named rte_eth_fp_ops. The
function pointers in that array are initialized when the devices start
and are initialized.

When a hotplug event occurs, the function pointers in rte_eth_fp_ops
still point to the "unsafe" rx/tx functions in the failsafe PMD since
they haven't been updated. This results in a segmentation fault because
it ends up using the "unsafe" functions, when the "safe" functions
should have been used.

To fix the problem, the failsafe PMD code was changed to update the
function pointers in the rte_eth_fp_ops array when a hotplug event
occurs.

Fixes: 7a0935239b ("ethdev: make fast-path functions to use new flat array")
Cc: Konstantin Ananyev 
Cc: sta...@dpdk.org

Signed-off-by: Luc Pelletier 
---
 drivers/net/failsafe/failsafe_rxtx.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/failsafe/failsafe_rxtx.c 
b/drivers/net/failsafe/failsafe_rxtx.c
index fe67293299..34d59dfbb1 100644
--- a/drivers/net/failsafe/failsafe_rxtx.c
+++ b/drivers/net/failsafe/failsafe_rxtx.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -44,9 +45,13 @@ failsafe_set_burst_fn(struct rte_eth_dev *dev, int 
force_safe)
DEBUG("Using safe RX bursts%s",
  (force_safe ? " (forced)" : ""));
dev->rx_pkt_burst = &failsafe_rx_burst;
+   rte_eth_fp_ops[dev->data->port_id].rx_pkt_burst =
+   &failsafe_rx_burst;
} else if (!need_safe && safe_set) {
DEBUG("Using fast RX bursts");
dev->rx_pkt_burst = &failsafe_rx_burst_fast;
+   rte_eth_fp_ops[dev->data->port_id].rx_pkt_burst =
+   &failsafe_rx_burst_fast;
}
need_safe = force_safe || fs_tx_unsafe(TX_SUBDEV(dev));
safe_set = (dev->tx_pkt_burst == &failsafe_tx_burst);
@@ -54,9 +59,13 @@ failsafe_set_burst_fn(struct rte_eth_dev *dev, int 
force_safe)
DEBUG("Using safe TX bursts%s",
  (force_safe ? " (forced)" : ""));
dev->tx_pkt_burst = &failsafe_tx_burst;
+   rte_eth_fp_ops[dev->data->port_id].tx_pkt_burst =
+   &failsafe_tx_burst;
} else if (!need_safe && safe_set) {
DEBUG("Using fast TX bursts");
dev->tx_pkt_burst = &failsafe_tx_burst_fast;
+   rte_eth_fp_ops[dev->data->port_id].tx_pkt_burst =
+   &failsafe_tx_burst_fast;
}
rte_wmb();
 }
-- 
2.25.1

[PATCH v2] failsafe: fix segfault on hotplug event

[Luc Pelletier]

When the failsafe PMD encounters a hotplug event, it switches its rx/tx
functions to "safe" ones that validate the sub-device's rx/tx functions
before calling them. It switches the rx/tx functions by changing the
function pointers in the rte_eth_dev structure.

Following commit 7a0935239b9e, the rx/tx functions of PMDs are no longer
called through the function pointers in the rte_eth_dev structure. They
are rather called through a flat array named rte_eth_fp_ops. The
function pointers in that array are initialized when the devices start
and are initialized.

When a hotplug event occurs, the function pointers in rte_eth_fp_ops
still point to the "unsafe" rx/tx functions in the failsafe PMD since
they haven't been updated. This results in a segmentation fault because
it ends up using the "unsafe" functions, when the "safe" functions
should have been used.

To fix the problem, the failsafe PMD code was changed to update the
function pointers in the rte_eth_fp_ops array when a hotplug event
occurs.

Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
Cc: Konstantin Ananyev 
Cc: sta...@dpdk.org

Signed-off-by: Luc Pelletier 
---

v2:
* fixed git commit hashes in commit message

 drivers/net/failsafe/failsafe_rxtx.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/failsafe/failsafe_rxtx.c 
b/drivers/net/failsafe/failsafe_rxtx.c
index fe67293299..34d59dfbb1 100644
--- a/drivers/net/failsafe/failsafe_rxtx.c
+++ b/drivers/net/failsafe/failsafe_rxtx.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -44,9 +45,13 @@ failsafe_set_burst_fn(struct rte_eth_dev *dev, int 
force_safe)
DEBUG("Using safe RX bursts%s",
  (force_safe ? " (forced)" : ""));
dev->rx_pkt_burst = &failsafe_rx_burst;
+   rte_eth_fp_ops[dev->data->port_id].rx_pkt_burst =
+   &failsafe_rx_burst;
} else if (!need_safe && safe_set) {
DEBUG("Using fast RX bursts");
dev->rx_pkt_burst = &failsafe_rx_burst_fast;
+   rte_eth_fp_ops[dev->data->port_id].rx_pkt_burst =
+   &failsafe_rx_burst_fast;
}
need_safe = force_safe || fs_tx_unsafe(TX_SUBDEV(dev));
safe_set = (dev->tx_pkt_burst == &failsafe_tx_burst);
@@ -54,9 +59,13 @@ failsafe_set_burst_fn(struct rte_eth_dev *dev, int 
force_safe)
DEBUG("Using safe TX bursts%s",
  (force_safe ? " (forced)" : ""));
dev->tx_pkt_burst = &failsafe_tx_burst;
+   rte_eth_fp_ops[dev->data->port_id].tx_pkt_burst =
+   &failsafe_tx_burst;
} else if (!need_safe && safe_set) {
DEBUG("Using fast TX bursts");
dev->tx_pkt_burst = &failsafe_tx_burst_fast;
+   rte_eth_fp_ops[dev->data->port_id].tx_pkt_burst =
+   &failsafe_tx_burst_fast;
}
rte_wmb();
 }
-- 
2.25.1

RE: [PATCH v1 1/1] baseband/acc: fix check after deref and dead code

[Chautru, Nicolas]

Hi David, 

> -Original Message-
> From: David Marchand 
> Sent: Thursday, November 10, 2022 1:49 AM
> To: Vargas, Hernan 
> Cc: dev@dpdk.org; gak...@marvell.com; t...@redhat.com;
> maxime.coque...@redhat.com; Chautru, Nicolas ;
> Zhang, Qi Z 
> Subject: Re: [PATCH v1 1/1] baseband/acc: fix check after deref and dead code
> 
> On Thu, Nov 3, 2022 at 8:57 PM Hernan Vargas 
> wrote:
> >
> > Fix potential issue of dereferencing a pointer before null check.
> > Remove null check for value that could never be null.
> >
> > Coverity issue: 381646, 381631
> > Fixes: 989dec301a9 ("baseband/acc100: add ring companion address")
> >
> > Signed-off-by: Hernan Vargas 
> > ---
> >  drivers/baseband/acc/rte_acc100_pmd.c | 4 
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/baseband/acc/rte_acc100_pmd.c
> > b/drivers/baseband/acc/rte_acc100_pmd.c
> > index 96daef87bc..30a718916d 100644
> > --- a/drivers/baseband/acc/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> > @@ -4122,15 +4122,11 @@ acc100_dequeue_ldpc_enc(struct
> rte_bbdev_queue_data *q_data,
> > struct rte_bbdev_enc_op *op;
> > union acc_dma_desc *desc;
> >
> > -   if (q == NULL)
> > -   return 0;
> 
> I guess this protects badly written applications that would do stuff like 
> pass an
> incorrect queue id, or call this callback while the queue has not been 
> configured
> yet.
> This is something that should be caught at the bbdev layer (arguably under the
> RTE_LIBRTE_BBDEV_DEBUG if the performance is that much affected, though
> I'd like to see numbers).
> (edit: I see Maxime replied a similar comment).

That is not directly to that ticket but would be good to follow up. 
From previous discussion with Maxime, the new consensus was to avoid special 
check in debug mode (try to build the same code). It would be good to come up 
to a new consensus on this. 

> 
> Back to this particular patch, rather than remove the check, the right fix is 
> to
> move acc_ring_avail_deq(q).
> This is what Coverity reports.
> 
> And this same pattern is used in other parts of the driver.
> It just happens that Coverity did not report them because some avec under
> RTE_LIBRTE_BBDEV_DEBUG...

I believe that we don't want to create discrepancies : each dequeue function 
should behave the same way. Ie. acc100_dequeue_ldpc_enc should not do things 
differently from others dequeue functions. 
Currently there is a discrepancy which is being resolved in that patch.

Either we remove the check as in that commit which could be approved as is, 
or we move the check under the debug as for the other functions which hides the 
Coverity issue without in reality fully addressing it, 
or we remove these check from all functions (including under debug) which is 
what we do for other PMD. 

That 4th option you seem to suggest would consist in effect to do thing 
differently just for the dequeue function which would lacks consistency really. 

Is there any concern just to approve as is, again that q == NULL is not done in 
production code anywhere else as you pointed out.

I agree that in next release we can remove much of the code under DEBUG flag 
which is not adding value nor being built/used in practice. 

Thanks
Nic

> 
> 
> 
> >  #ifdef RTE_LIBRTE_BBDEV_DEBUG
> > if (unlikely(ops == 0))
> 
> And I also noticed this hunk.
> 
> DPDK coding style, ops should be compared against NULL, but see below...
> 
> 
> > return 0;
> >  #endif
> > desc = q->ring_addr + (q->sw_ring_tail & q->sw_ring_wrap_mask);
> > -   if (unlikely(desc == NULL))
> > -   return 0;
> > op = desc->req.op_addr;
> > if (unlikely(ops == NULL || op == NULL))
> > return 0;
> 
> ... like here, so above check is redundant.
> 
> There is probably more cleanups to do in this driver.
> This can be done later.
> 
> 
> --
> David Marchand

Re: [PATCH v4] testpmd: cleanup cleanly from signal

[Mattias Rönnblom]

On 2022-11-10 17:14, Stephen Hemminger wrote:

On Thu, 10 Nov 2022 08:50:40 +0100
Mattias Rönnblom  wrote:



Why is select() needed? Wouldn't a blocking read suffice? Or getchar().


On Linux, signal set SA_RESTART so a simple read is not interrupted.
One option was to use sigaction() which allows controlling flags, but that
won't work on Windows.  Using select() works on both.
   


OK, so select() is used because a signal might interrupt read() on Windows?

while (read(0, &c, 1) == -1 && errno == EINTR)
  ;

Would that work?


Try it. On Linux the read never gets interrupted.


I had no doubts about that, but I misunderstood the code and thought 
that was the required behavior.

Re: [PATCH v2] failsafe: fix segfault on hotplug event

[Stephen Hemminger]

On Thu, 10 Nov 2022 12:42:43 -0500
Luc Pelletier  wrote:

> When the failsafe PMD encounters a hotplug event, it switches its rx/tx
> functions to "safe" ones that validate the sub-device's rx/tx functions
> before calling them. It switches the rx/tx functions by changing the
> function pointers in the rte_eth_dev structure.
> 
> Following commit 7a0935239b9e, the rx/tx functions of PMDs are no longer
> called through the function pointers in the rte_eth_dev structure. They
> are rather called through a flat array named rte_eth_fp_ops. The
> function pointers in that array are initialized when the devices start
> and are initialized.
> 
> When a hotplug event occurs, the function pointers in rte_eth_fp_ops
> still point to the "unsafe" rx/tx functions in the failsafe PMD since
> they haven't been updated. This results in a segmentation fault because
> it ends up using the "unsafe" functions, when the "safe" functions
> should have been used.
> 
> To fix the problem, the failsafe PMD code was changed to update the
> function pointers in the rte_eth_fp_ops array when a hotplug event
> occurs.

Have it in both places might be breaking other drivers as well.
Shouldn't there be a ethdev function when changing rx/tx burst.

Also, changing a variable used by another thread needs to be
using __atomic_store and __atomic_load to guarantee that CPU
or compiler will no that it changed.

RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment

[Ye, MingjinX]

> -Original Message-
> From: Zhang, Qi Z 
> Sent: 2022年11月10日 18:37
> To: Ye, MingjinX ; dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org; Zhou, YidingX
> ; Lu, Wenzhuo ; Wu,
> Jingjing ; Li, Xiaoyun ; Ferruh
> Yigit 
> Subject: RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> 
> 
> 
> > -Original Message-
> > From: Ye, MingjinX 
> > Sent: Wednesday, November 9, 2022 8:56 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming ; sta...@dpdk.org; Zhou,
> > YidingX ; Ye, MingjinX
> > ; Zhang, Qi Z ; Lu,
> > Wenzhuo ; Wu, Jingjing ;
> > Li, Xiaoyun ; Ferruh Yigit
> > 
> > Subject: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> >
> > CRC is stripped by the hardware in the scattered Rx path. If the last
> > buffer packet length is '0', the scalar Tx path would send empty
> > buffer that causes the Tx queue to overflow.
> 
> Please separate this patch into two, one for Rx and one for Tx, they are
> independent.
> 
> For the Tx implementation, I think we can move them into tx_prepare where
> is place to check Tx violation.
Thanks for your suggestion, I will provide 2 new patches according to the new
scheme and promote it to the community.
> 
> >
> > This patch adds a judgment for the last buffer length to fix this
> > issue, so that it would free the mbuf associated to the last one if the last
> buffer is empty.
> >
> > Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Mingjin Ye 
> >
> > v2:
> > * Fix log level in ice_rxtx.c source file.
> > ---
> >  drivers/net/ice/ice_rxtx.c | 53
> > --
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index 0a2b0376ac..b181f66aad 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
> > } else
> > rxm->data_len = (uint16_t)(rx_packet_len -
> >
> > RTE_ETHER_CRC_LEN);
> > +   } else if (rx_packet_len == 0) {
> > +   rte_pktmbuf_free_seg(rxm);
> > +   first_seg->nb_segs--;
> > +   last_seg->next = NULL;
> > }
> >
> > first_seg->port = rxq->port_id;
> > @@ -2903,6 +2907,35 @@ ice_calc_pkt_desc(struct rte_mbuf *tx_pkt)
> > return count;
> >  }
> >
> > +/*Check the number of valid mbufs and free the invalid mbufs*/ static
> > +inline uint16_t ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> > +   struct rte_mbuf *txd = tx_pkt;
> > +   struct rte_mbuf *txd_removal = NULL;
> > +   struct rte_mbuf *txd_pre = NULL;
> > +   uint16_t count = 0;
> > +   uint16_t removal = 0;
> > +
> > +   while (txd != NULL) {
> > +   if (removal == 1 || txd->data_len == 0) {
> > +   txd_removal = txd;
> > +   txd = txd->next;
> > +   if (removal == 0) {
> > +   removal = 1;
> > +   txd_pre->next = NULL;
> > +   }
> > +   rte_pktmbuf_free_seg(txd_removal);
> > +   } else {
> > +   ++count;
> > +   txd_pre = txd;
> > +   txd = txd->next;
> > +   }
> > +   }
> > +
> > +   return count;
> > +}
> > +
> >  uint16_t
> >  ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t
> > nb_pkts) { @@ -2960,11 +2993,27 @@ ice_xmit_pkts(void *tx_queue,
> > struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> >  * the mbuf data size exceeds max data size that hw allows
> >  * per tx desc.
> >  */
> > -   if (ol_flags & RTE_MBUF_F_TX_TCP_SEG)
> > +   if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
> > nb_used = (uint16_t)(ice_calc_pkt_desc(tx_pkt) +
> >  nb_ctx);
> > -   else
> > +   } else {
> > +   nb_used = ice_check_mbuf(tx_pkt);
> > +   if (nb_used == 0) {
> > +   PMD_TX_LOG(ERR,
> > +   "Check packets is empty "
> > +   "(port=%d queue=%d)\n",
> > +   txq->port_id, txq->queue_id);
> > +   continue;
> > +   } else if (nb_used < tx_pkt->nb_segs) {
> > +   PMD_TX_LOG(DEBUG,
> > +   "Check packets valid num ="
> > +   "%4u total num = %4u (port=%d
> > queue=%d)\n",
> > +   nb_used, tx_pkt->nb_segs, txq->port_id,
> txq-
> > >queue_id);
> > +   tx_pkt->nb_segs = nb_used;
> > +   }
> > nb_used = (uint16_t)(tx_pkt->nb_segs + nb_ctx);
> > +   }
> > +
> > tx_last = (uint16_t)(tx_id + nb_used - 1);
> >
> > /* Circular ring */
> > --
> > 2.3

RE: [PATCH v5 1/2] net/ice: fix vlan offload

[Ye, MingjinX]

Hi ALL,

Could you please review and provide suggestions if any.

Thanks,
Mingjin

> -Original Message-
> From: Ye, MingjinX 
> Sent: 2022年11月8日 21:28
> To: dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org; Zhou, YidingX
> ; Ye, MingjinX ;
> Richardson, Bruce ; Konstantin Ananyev
> ; Zhang, Qi Z ; Lu,
> Wenzhuo ; Junyu Jiang ;
> Rong, Leyi ; Ajit Khaparde
> ; Jerin Jacob ; Xu,
> Rosen ; Hemant Agrawal
> ; Wisam Jaddo 
> Subject: [PATCH v5 1/2] net/ice: fix vlan offload
> 
> The vlan tag and flag in Rx descriptor are not processed on vector path, then
> the upper application can't fetch the tci from mbuf.
> 
> This patch is to add handling of vlan RX offloading.
> 
> Fixes: c68a52b8b38c ("net/ice: support vector SSE in Rx")
> Fixes: ece1f8a8f1c8 ("net/ice: switch to flexible descriptor in SSE path")
> Fixes: 12443386a0b0 ("net/ice: support flex Rx descriptor RxDID22")
> Fixes: 214f452f7d5f ("net/ice: add AVX2 offload Rx")
> Fixes: 7f85d5ebcfe1 ("net/ice: add AVX512 vector path")
> Fixes: 295968d17407 ("ethdev: add namespace")
> Fixes: 808a17b3c1e6 ("net/ice: add Rx AVX512 offload path")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Mingjin Ye 
> 
> v3:
>   * Fix macros in ice_rxtx_vec_sse.c source file.
> v4:
>   * Fix ice_rx_desc_to_olflags_v define in ice_rxtx_vec_sse.c source
> file.
> ---
>  drivers/net/ice/ice_rxtx_vec_avx2.c   | 135 +-
>  drivers/net/ice/ice_rxtx_vec_avx512.c | 154 +-
>  drivers/net/ice/ice_rxtx_vec_sse.c| 132 --
>  3 files changed, 332 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_rxtx_vec_avx2.c
> b/drivers/net/ice/ice_rxtx_vec_avx2.c
> index 31d6af42fd..bddfd6cf65 100644
> --- a/drivers/net/ice/ice_rxtx_vec_avx2.c
> +++ b/drivers/net/ice/ice_rxtx_vec_avx2.c
> @@ -474,7 +474,7 @@ _ice_recv_raw_pkts_vec_avx2(struct ice_rx_queue
> *rxq, struct rte_mbuf **rx_pkts,
>* will cause performance drop to get into this
> context.
>*/
>   if (rxq->vsi->adapter->pf.dev_data-
> >dev_conf.rxmode.offloads &
> - RTE_ETH_RX_OFFLOAD_RSS_HASH) {
> + (RTE_ETH_RX_OFFLOAD_RSS_HASH |
> RTE_ETH_RX_OFFLOAD_VLAN)) {
>   /* load bottom half of every 32B desc */
>   const __m128i raw_desc_bh7 =
>   _mm_load_si128
> @@ -529,33 +529,112 @@ _ice_recv_raw_pkts_vec_avx2(struct
> ice_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>* to shift the 32b RSS hash value to the
>* highest 32b of each 128b before mask
>*/
> - __m256i rss_hash6_7 =
> - _mm256_slli_epi64(raw_desc_bh6_7,
> 32);
> - __m256i rss_hash4_5 =
> - _mm256_slli_epi64(raw_desc_bh4_5,
> 32);
> - __m256i rss_hash2_3 =
> - _mm256_slli_epi64(raw_desc_bh2_3,
> 32);
> - __m256i rss_hash0_1 =
> - _mm256_slli_epi64(raw_desc_bh0_1,
> 32);
> -
> - __m256i rss_hash_msk =
> - _mm256_set_epi32(0x, 0, 0,
> 0,
> -  0x, 0, 0, 0);
> -
> - rss_hash6_7 = _mm256_and_si256
> - (rss_hash6_7, rss_hash_msk);
> - rss_hash4_5 = _mm256_and_si256
> - (rss_hash4_5, rss_hash_msk);
> - rss_hash2_3 = _mm256_and_si256
> - (rss_hash2_3, rss_hash_msk);
> - rss_hash0_1 = _mm256_and_si256
> - (rss_hash0_1, rss_hash_msk);
> -
> - mb6_7 = _mm256_or_si256(mb6_7,
> rss_hash6_7);
> - mb4_5 = _mm256_or_si256(mb4_5,
> rss_hash4_5);
> - mb2_3 = _mm256_or_si256(mb2_3,
> rss_hash2_3);
> - mb0_1 = _mm256_or_si256(mb0_1,
> rss_hash0_1);
> - } /* if() on RSS hash parsing */
> + if (rxq->vsi->adapter->pf.dev_data-
> >dev_conf.rxmode.offloads &
> +
>   RTE_ETH_RX_OFFLOAD_RSS_HASH) {
> + __m256i rss_hash6_7 =
> +
>   _mm256_slli_epi64(raw_desc_bh6_7, 32);
> + __m256i rss_hash4_5 =
> +
>   _mm256_slli_epi64(raw_desc_bh4_5, 32);
> + __m256i rss_hash2_3 =
> +
>   _mm256_slli_epi64(raw_desc_bh2_3, 32);
> + __m256i 

[PATCH v3 1/2] net/ice: fix scalar Rx path segment

[Mingjin Ye]

CRC is stripped by the hardware in the scattered Rx path. The last buffer
is invalid if it's packet length is zero.

This patch adds a judgment for the last buffer length to fix this issue,
it would free the mbuf associated to the last one if the last buffer is
empty.

Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
Cc: sta...@dpdk.org

Signed-off-by: Mingjin Ye 
---
 drivers/net/ice/ice_rxtx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0a2b0376ac..e6ddd2513d 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
} else
rxm->data_len = (uint16_t)(rx_packet_len -
   RTE_ETHER_CRC_LEN);
+   } else if (rx_packet_len == 0) {
+   rte_pktmbuf_free_seg(rxm);
+   first_seg->nb_segs--;
+   last_seg->next = NULL;
}
 
first_seg->port = rxq->port_id;
-- 
2.34.1

[PATCH v3 2/2] net/ice: fix scalar Tx path segment

[Mingjin Ye]

The scalar Tx path would send empty buffer that causes the Tx queue to
overflow.

This patch adds the last buffer length judgment in tx_prepare to fix this
issue, rte_errno will be set to EINVAL and returned if the last buffer is
empty.

Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
Cc: sta...@dpdk.org

Signed-off-by: Mingjin Ye 
---
 drivers/net/ice/ice_rxtx.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index e6ddd2513d..69358f6a3a 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct 
ice_tx_queue *txq)
 #define ICE_MIN_TSO_MSS64
 #define ICE_MAX_TSO_MSS9728
 #define ICE_MAX_TSO_FRAME_SIZE 262144
+
+/*Check for invalid mbuf*/
+static inline uint16_t
+ice_check_mbuf(struct rte_mbuf *tx_pkt)
+{
+   struct rte_mbuf *txd = tx_pkt;
+
+   while (txd != NULL) {
+   if (txd->data_len == 0)
+   return -1;
+   txd = txd->next;
+   }
+
+   return 0;
+}
+
 uint16_t
 ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
  uint16_t nb_pkts)
@@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct 
rte_mbuf **tx_pkts,
struct ice_tx_queue *txq = tx_queue;
struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
+   uint16_t nb_used;
 
for (i = 0; i < nb_pkts; i++) {
m = tx_pkts[i];
@@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct 
rte_mbuf **tx_pkts,
rte_errno = -ret;
return i;
}
+
+   if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
+   ice_check_mbuf(m)) {
+   rte_errno = EINVAL;
+   PMD_DRV_LOG(ERR, "INVALID mbuf: last mbuf 
data_len=[0]");
+   return i;
+   }
}
return i;
 }
-- 
2.34.1

RE: [RFC] mempool: zero-copy cache put bulk

[Honnappa Nagarahalli]

> > > > > > > From: Honnappa Nagarahalli
> > > [mailto:honnappa.nagaraha...@arm.com]
> > > > > > > Sent: Sunday, 6 November 2022 00.11
> > > > > > >
> > > > > > > + Akshitha, she is working on similar patch
> > > > > > >
> > > > > > > Few comments inline
> > > > > > >
> > > > > > > > From: Morten Br�rup 
> > > > > > > > Sent: Saturday, November 5, 2022 8:40 AM
> > > > > > > >
> > > > > > > > Zero-copy access to the mempool cache is beneficial for
> > > > > > > > PMD
> > > > > > > performance,
> > > > > > > > and must be provided by the mempool library to fix [Bug
> > > > > > > > 1052] without
> > > > > > > a
> > > > > > > > performance regression.
> > > > > > > >
> > > > > > > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052
> > > > > > > >
> > > > > > > >
> > > > > > > > This RFC offers a conceptual zero-copy put function, where
> > > the
> > > > > > > application
> > > > > > > > promises to store some objects, and in return gets an
> > > > > > > > address
> > > > > where
> > > > > > > to store
> > > > > > > > them.
> > > > > > > >
> > > > > > > > I would like some early feedback.
> > > > > > > >
> > > > > > > > Notes:
> > > > > > > > * Allowing the 'cache' parameter to be NULL, and getting
> > > > > > > > it
> > > from
> > > > > the
> > > > > > > > mempool instead, was inspired by rte_mempool_cache_flush().
> > > > > > > I am not sure why the 'cache' parameter is required for this
> > > API.
> > > > > This
> > > > > > > API should take the mem pool as the parameter.
> > > > > > >
> > > > > > > We have based our API on 'rte_mempool_do_generic_put' and
> > > removed
> > > > > > the
> > > > > > > 'cache' parameter.
> > > > > >
> > > > > > I thoroughly considered omitting the 'cache' parameter, but
> > > included
> > > > > it for
> > > > > > two reasons:
> > > > > >
> > > > > > 1. The function is a "mempool cache" function (i.e. primarily
> > > > > > working
> > > > > on the
> > > > > > mempool cache), not a "mempool" function.
> > > > > >
> > > > > > So it is appropriate to have a pointer directly to the
> > > > > > structure
> > > it
> > > > > is working on.
> > > > > > Following this through, I also made 'cache' the first
> > > > > > parameter
> > > and
> > > > > 'mp' the
> > > > > > second, like in rte_mempool_cache_flush().
> > > > > I am wondering if the PMD should be aware of the cache or not.
> > > > > For
> > > ex:
> > > > > in the case of pipeline mode, the RX and TX side of the PMD are
> > > > > running on different cores.
> > > >
> > > > In that example, the PMD can store two cache pointers, one for
> > > > each
> > > of the
> > > > RX and TX side.
> > > I did not understand this. If RX core and TX core have their own
> > > per- core caches the logic would not work. For ex: the RX core cache
> > > would not get filled.
> > >
> > > In the case of pipeline mode, there will not be a per-core cache.
> > > The buffers would be allocated and freed from a global ring or a
> > > global lockless stack.
> >
> > Aha... Now I understand what you mean: You are referring to use cases
> where the mempool is configured to *not* have a mempool cache.
> >
> > For a mempool without a mempool cache, the proposed "mempool cache"
> zero-copy functions can obviously not be used.
> >
> > We need "mempool" zero-copy functions for the mempools that have no
> mempool cache.
> >
> > However, those functions depend on the mempool's underlying backing
> store.
> >
> > E.g. zero-copy access to a ring has certain requirements [1].
> >
> > [1]:
> > http://doc.dpdk.org/guides/prog_guide/ring_lib.html#ring-peek-zero-cop
> > y-api
> >
> > For a stack, I think it is possible to locklessly zero-copy pop objects. 
> > But it is
> impossible to locklessly zero-copy push elements to a stack; another thread
> can race to pop some objects from the stack before the pushing thread has
> finished writing them into the stack.
> >
> > Furthermore, the ring zero-copy get function cannot return a consecutive
> array of objects when wrapping, and PMD functions using vector instructions
> usually rely on handling chunks of e.g. 8 objects.
> >
> > Just for a second, let me theorize into the absurd: Even worse, if a
> mempool's underlying backing store does not use an array of pointers as its
> internal storage structure, it is impossible to use a pointer to an array of
> pointers for zero-copy transactions. E.g. if the backing store uses a list or 
> a
> tree structure for its storage, a pointer to somewhere in the list or tree
> structure is not an array of objects pointers.
> >
> > Anyway, we could consider designing a generic API for zero-copy mempool
> get/put; but it should be compatible with all underlying backing stores - or
> return failure, so the PMD can fall back to the standard functions, if the
> mempool is in a state where zero-copy access to a contiguous burst cannot
> be provided. E.g. zero-copy get from a ring can return failure when zero-copy
> access to the ring is temporarily unavailable due to being at a point where

RE: [PATCH v3 1/2] net/ice: fix scalar Rx path segment

[Zhang, Qi Z]

> -Original Message-
> From: Ye, MingjinX 
> Sent: Friday, November 11, 2022 8:04 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org; Zhou, YidingX
> ; Ye, MingjinX ; Zhang, Qi
> Z ; Ferruh Yigit ; Wu, Jingjing
> ; Lu, Wenzhuo ; Li, Xiaoyun
> 
> Subject: [PATCH v3 1/2] net/ice: fix scalar Rx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. The last buffer is
> invalid if it's packet length is zero.
> 
> This patch adds a judgment for the last buffer length to fix this issue, it 
> would
> free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Mingjin Ye 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment

[Zhang, Qi Z]

> -Original Message-
> From: Ye, MingjinX 
> Sent: Friday, November 11, 2022 8:04 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org; Zhou, YidingX
> ; Ye, MingjinX ; Zhang, Qi
> Z ; Wu, Jingjing ; Lu,
> Wenzhuo ; Ferruh Yigit ; Li,
> Xiaoyun ; Liu, KevinX 
> Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> 
> The scalar Tx path would send empty buffer that causes the Tx queue to
> overflow.
> 
> This patch adds the last buffer length judgment in tx_prepare to fix this 
> issue,
> rte_errno will be set to EINVAL and returned if the last buffer is empty.
> 
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Mingjin Ye 
> ---
>  drivers/net/ice/ice_rxtx.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> e6ddd2513d..69358f6a3a 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev,
> struct ice_tx_queue *txq)
>  #define ICE_MIN_TSO_MSS64
>  #define ICE_MAX_TSO_MSS9728
>  #define ICE_MAX_TSO_FRAME_SIZE 262144
> +
> +/*Check for invalid mbuf*/
> +static inline uint16_t
> +ice_check_mbuf(struct rte_mbuf *tx_pkt) {

Better to name the function to exactly match what it does. 
e.g.: ice_check_emtpy_mbuf
and also declare it as inline.

> + struct rte_mbuf *txd = tx_pkt;
> +
> + while (txd != NULL) {
> + if (txd->data_len == 0)
> + return -1;
> + txd = txd->next;
> + }
> +
> + return 0;
> +}
> +
>  uint16_t
>  ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts)
> @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> struct rte_mbuf **tx_pkts,
>   struct ice_tx_queue *txq = tx_queue;
>   struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
>   uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> + uint16_t nb_used;
> 
>   for (i = 0; i < nb_pkts; i++) {
>   m = tx_pkts[i];
> @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> struct rte_mbuf **tx_pkts,
>   rte_errno = -ret;
>   return i;
>   }
> +
> + if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> + ice_check_mbuf(m)) {

Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
A empty mbuf with TSO enabled is still acceptable?

> + rte_errno = EINVAL;
> + PMD_DRV_LOG(ERR, "INVALID mbuf: last mbuf
> data_len=[0]");
> + return i;
> + }
>   }
>   return i;
>  }
> --
> 2.34.1

[dpdk-dev][PATCH] drivers: optimize the build time for cnxk

[kirankumark]

From: Kiran Kumar K 

While building cnxk, if build platform is cn9k, cn10k files
are also being compiled and vice versa. This is causing more
build time. Adding changes to avoid this by checking the
platform and compile only platform specific files. If no
platform is provided, both cn9k and cn10k files will be compiled.

Signed-off-by: Kiran Kumar K 
---
 drivers/event/cnxk/cn9k_eventdev.c | 16 
 drivers/event/cnxk/cnxk_eventdev.c | 14 ++
 drivers/event/cnxk/cnxk_eventdev.h |  1 +
 drivers/event/cnxk/meson.build | 22 ++
 drivers/net/cnxk/meson.build   | 14 ++
 5 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/event/cnxk/cn9k_eventdev.c 
b/drivers/event/cnxk/cn9k_eventdev.c
index f5a42a86f8..7b09f27644 100644
--- a/drivers/event/cnxk/cn9k_eventdev.c
+++ b/drivers/event/cnxk/cn9k_eventdev.c
@@ -6,7 +6,6 @@
 #include "cnxk_eventdev.h"
 #include "cnxk_worker.h"
 
-#define CN9K_DUAL_WS_NB_WS 2
 #define CN9K_DUAL_WS_PAIR_ID(x, id) (((x)*CN9K_DUAL_WS_NB_WS) + id)
 
 #define CN9K_SET_EVDEV_DEQ_OP(dev, deq_op, deq_ops)
\
@@ -239,21 +238,6 @@ cn9k_sso_hws_reset(void *arg, void *hws)
ws->swtag_req = 0;
 }
 
-void
-cn9k_sso_set_rsrc(void *arg)
-{
-   struct cnxk_sso_evdev *dev = arg;
-
-   if (dev->dual_ws)
-   dev->max_event_ports = dev->sso.max_hws / CN9K_DUAL_WS_NB_WS;
-   else
-   dev->max_event_ports = dev->sso.max_hws;
-   dev->max_event_queues =
-   dev->sso.max_hwgrp > RTE_EVENT_MAX_QUEUES_PER_DEV ?
- RTE_EVENT_MAX_QUEUES_PER_DEV :
- dev->sso.max_hwgrp;
-}
-
 static int
 cn9k_sso_rsrc_init(void *arg, uint8_t hws, uint8_t hwgrp)
 {
diff --git a/drivers/event/cnxk/cnxk_eventdev.c 
b/drivers/event/cnxk/cnxk_eventdev.c
index db62d32a81..efa9359ce6 100644
--- a/drivers/event/cnxk/cnxk_eventdev.c
+++ b/drivers/event/cnxk/cnxk_eventdev.c
@@ -623,3 +623,17 @@ cnxk_sso_remove(struct rte_pci_device *pci_dev)
 {
return rte_event_pmd_pci_remove(pci_dev, cnxk_sso_fini);
 }
+
+void
+cn9k_sso_set_rsrc(void *arg)
+{
+   struct cnxk_sso_evdev *dev = arg;
+
+   if (dev->dual_ws)
+   dev->max_event_ports = dev->sso.max_hws / CN9K_DUAL_WS_NB_WS;
+   else
+   dev->max_event_ports = dev->sso.max_hws;
+   dev->max_event_queues = dev->sso.max_hwgrp > 
RTE_EVENT_MAX_QUEUES_PER_DEV ?
+   RTE_EVENT_MAX_QUEUES_PER_DEV :
+   dev->sso.max_hwgrp;
+}
diff --git a/drivers/event/cnxk/cnxk_eventdev.h 
b/drivers/event/cnxk/cnxk_eventdev.h
index 738e335ea4..fdbcfb4640 100644
--- a/drivers/event/cnxk/cnxk_eventdev.h
+++ b/drivers/event/cnxk/cnxk_eventdev.h
@@ -56,6 +56,7 @@
 #define CNXK_TAG_IS_HEAD(x)(BIT_ULL(35) & x)
 
 #define CN9K_SSOW_GET_BASE_ADDR(_GW) ((_GW)-SSOW_LF_GWS_OP_GET_WORK0)
+#define CN9K_DUAL_WS_NB_WS  2
 
 #define CN10K_GW_MODE_NONE 0
 #define CN10K_GW_MODE_PREF 1
diff --git a/drivers/event/cnxk/meson.build b/drivers/event/cnxk/meson.build
index aa42ab3a90..227c6ae7a8 100644
--- a/drivers/event/cnxk/meson.build
+++ b/drivers/event/cnxk/meson.build
@@ -8,11 +8,17 @@ if not is_linux or not dpdk_conf.get('RTE_ARCH_64')
 subdir_done()
 endif
 
+if meson.is_cross_build()
+soc_type = meson.get_cross_property('platform', '')
+else
+soc_type = platform
+endif
+
+if soc_type != 'cn9k' and soc_type != 'cn10k'
+soc_type = 'all'
+endif
+
 sources = files(
-'cn9k_eventdev.c',
-'cn9k_worker.c',
-'cn10k_eventdev.c',
-'cn10k_worker.c',
 'cnxk_eventdev.c',
 'cnxk_eventdev_adptr.c',
 'cnxk_eventdev_selftest.c',
@@ -21,7 +27,10 @@ sources = files(
 'cnxk_tim_worker.c',
 )
 
+if soc_type == 'cn9k' or soc_type == 'all'
 sources += files(
+'cn9k_eventdev.c',
+'cn9k_worker.c',
 'deq/cn9k/deq_0_15_burst.c',
 'deq/cn9k/deq_16_31_burst.c',
 'deq/cn9k/deq_32_47_burst.c',
@@ -320,8 +329,12 @@ sources += files(
 'tx/cn9k/tx_96_111_dual_seg.c',
 'tx/cn9k/tx_112_127_dual_seg.c',
 )
+endif
 
+if soc_type == 'cn10k' or soc_type == 'all'
 sources += files(
+'cn10k_eventdev.c',
+'cn10k_worker.c',
 'deq/cn10k/deq_0_15_burst.c',
 'deq/cn10k/deq_16_31_burst.c',
 'deq/cn10k/deq_32_47_burst.c',
@@ -470,6 +483,7 @@ sources += files(
 'tx/cn10k/tx_96_111_seg.c',
 'tx/cn10k/tx_112_127_seg.c',
 )
+endif
 
 extra_flags = ['-flax-vector-conversions', '-Wno-strict-aliasing']
 foreach flag: extra_flags
diff --git a/drivers/net/cnxk/meson.build b/drivers/net/cnxk/meson.build
index c7ca24d437..99531c1917 100644
--- a/drivers/net/cnxk/meson.build
+++ b/drivers/net/cnxk/meson.build
@@ -8,6 +8,16 @@ if not dpdk_conf.get('RTE_ARCH_64')
 subdir_done()
 endif
 
+if meson.is_cross_build()
+   

[PATCH v2] doc: update matching list for i40e and ice driver

[Qiming Yang]

Add recommended matching list for ice PMD in DPDK 22.07 and
i40e PMD in DPDK 22.07 and 22.11.

Signed-off-by: Qiming Yang 
---
 doc/guides/nics/i40e.rst | 8 
 doc/guides/nics/ice.rst  | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index a0992dbc6c..a6c7dbd080 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -101,6 +101,10 @@ For X710/XL710/XXV710,
+--+---+--+
| DPDK version | Kernel driver version | Firmware version |
+==+===+==+
+   |22.11 | 2.20.12   |   9.01   |
+   +--+---+--+
+   |22.07 | 2.19.3|   8.70   |
+   +--+---+--+
|22.03 | 2.17.15   |   8.30   |
+--+---+--+
|21.11 | 2.17.4|   8.30   |
@@ -156,6 +160,10 @@ For X722,
+--+---+--+
| DPDK version | Kernel driver version | Firmware version |
+==+===+==+
+   |22.11 | 2.20.12   |   6.00   |
+   +--+---+--+
+   |22.07 | 2.19.3|   5.60   |
+   +--+---+--+
|22.03 | 2.17.15   |   5.50   |
+--+---+--+
|21.11 | 2.17.4|   5.30   |
diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index c7f82c261d..ce075e067c 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -64,6 +64,8 @@ The detailed information can refer to chapter Tested 
Platforms/Tested NICs in re

+---+---+-+---+--+---+
|22.03  | 1.8.3 |  1.3.28 |  1.3.35   |1.3.8 |  
  3.2|

+---+---+-+---+--+---+
+   |22.07  | 1.9.11|  1.3.30 |  1.3.37   |1.3.10|  
  4.0|
+   
+---+---+-+---+--+---+
 
 Pre-Installation Configuration
 --
-- 
2.25.1

RE: [PATCH] common/mlx5: fix the sysfs port name translation

[Bing Zhao]

Hi Stephen,

> -Original Message-
> From: Stephen Hemminger 
> Sent: Friday, November 11, 2022 12:22 AM
> To: Bing Zhao 
> Cc: Slava Ovsiienko ; Matan Azrad
> ; dev@dpdk.org; Raslan Darawsheh
> ; sta...@dpdk.org
> Subject: Re: [PATCH] common/mlx5: fix the sysfs port name
> translation
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 10 Nov 2022 17:57:26 +0200
> Bing Zhao  wrote:
> 
> > With some OFED or upstream kernel of mlx5, the port name fetched
> from
> > "/sys/class/net/[DEV]/phys_port_name" may have a tailing "\n" as
> the
> > EOL. The sscanf() will return the scanned items number with this
> EOL.
> 
> Why not fix the DPDK driver to strip off the new line when the port
> name is read? The code in mlx5_os.c should do it there.

Thanks for your comments. Yes, IF_NAMESIZE is large enough to read all the 
characters including the newline into the buffer.
After fgets(), the strlen() or strchr() can be used to remove the tailing 
newline character.


BR. Bing

RE: [PATCH v3 13/14] net/i40e: fix whitespace

[Zhang, Yuying]

Hi,

Could you add fix line and Cc: sta...@dpdk.org?
The fix looks good to me.

> -Original Message-
> From: Stephen Hemminger 
> Sent: 2022年11月10日 7:25
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Zhang, Yuying
> ; Xing, Beilei 
> Subject: [PATCH v3 13/14] net/i40e: fix whitespace
> 
> Add space after keywords.
> 
> Signed-off-by: Stephen Hemminger 
Reviewed-off-by: Yuying Zhang 


> ---
>  drivers/net/i40e/i40e_pf.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c index
> 15d9ff868f3a..7050e0057d8e 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf
> *vf,
> 
>   for (i = 0; i < vlan_filter_list->num_elements; i++) {
>   ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
> - if(ret != I40E_SUCCESS)
> + if (ret != I40E_SUCCESS)
>   goto send_msg;
>   }
> 
> @@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf
> *vf,
>   vid = vlan_filter_list->vlan_id;
>   for (i = 0; i < vlan_filter_list->num_elements; i++) {
>   ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
> - if(ret != I40E_SUCCESS)
> + if (ret != I40E_SUCCESS)
>   goto send_msg;
>   }
> 
> @@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
>* return if SRIOV not enabled, VF number not configured or
>* no queue assigned.
>*/
> - if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
> + if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps ==
> +0)
>   return I40E_SUCCESS;
> 
>   /* Allocate memory to store VF structure */
>   pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
> - if(pf->vfs == NULL)
> + if (pf->vfs == NULL)
>   return -ENOMEM;
> 
>   /* Disable irq0 for VFR event */
> --
> 2.35.1

RE: [PATCH v3 05/14] testpmd: fix whitespace

[Zhang, Yuying]

> -Original Message-
> From: Stephen Hemminger 
> Sent: 2022年11月10日 7:25
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Singh, Aman Deep
> ; Zhang, Yuying 
> Subject: [PATCH v3 05/14] testpmd: fix whitespace
> 
> Add space after keywords.
> 
> Signed-off-by: Stephen Hemminger 
Reviewed-off-by: Yuying Zhang 

> ---
>  app/test-pmd/cmdline.c| 31 ---
>  app/test-pmd/parameters.c | 10 ++
>  app/test-pmd/testpmd.c|  2 +-
>  3 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 8dc60e938830..7721006cc310 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2730,11 +2730,12 @@ parse_reta_config(const char *str,
> 
>   while ((p = strchr(p0,'(')) != NULL) {
>   ++p;
> - if((p0 = strchr(p,')')) == NULL)
> + p0 = strchr(p, ')');
> + if (p0 == NULL)
>   return -1;
> 
>   size = p0 - p;
> - if(size >= sizeof(s))
> + if (size >= sizeof(s))
>   return -1;
> 
>   snprintf(s, sizeof(s), "%.*s", size, p); @@ -3242,15 +3243,15
> @@ cmd_config_thresh_parsed(void *parsed_result,
> 
>   if (!strcmp(res->name, "txpt"))
>   tx_pthresh = res->value;
> - else if(!strcmp(res->name, "txht"))
> + else if (!strcmp(res->name, "txht"))
>   tx_hthresh = res->value;
> - else if(!strcmp(res->name, "txwt"))
> + else if (!strcmp(res->name, "txwt"))
>   tx_wthresh = res->value;
> - else if(!strcmp(res->name, "rxpt"))
> + else if (!strcmp(res->name, "rxpt"))
>   rx_pthresh = res->value;
> - else if(!strcmp(res->name, "rxht"))
> + else if (!strcmp(res->name, "rxht"))
>   rx_hthresh = res->value;
> - else if(!strcmp(res->name, "rxwt"))
> + else if (!strcmp(res->name, "rxwt"))
>   rx_wthresh = res->value;
>   else {
>   fprintf(stderr, "Unknown parameter\n"); @@ -4088,8 +4089,8
> @@ cmd_vlan_offload_parsed(void *parsed_result,
>   len = strnlen(str, STR_TOKEN_SIZE);
>   i = 0;
>   /* Get port_id first */
> - while(i < len){
> - if(str[i] == ',')
> + while (i < len) {
> + if (str[i] == ',')
>   break;
> 
>   i++;
> @@ -4097,7 +4098,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
>   str[i]='\0';
>   tmp = strtoul(str, NULL, 0);
>   /* If port_id greater that what portid_t can represent, return */
> - if(tmp >= RTE_MAX_ETHPORTS)
> + if (tmp >= RTE_MAX_ETHPORTS)
>   return;
>   port_id = (portid_t)tmp;
> 
> @@ -4108,17 +4109,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
> 
>   if (!strcmp(res->what, "strip"))
>   rx_vlan_strip_set(port_id,  on);
> - else if(!strcmp(res->what, "stripq")){
> + else if (!strcmp(res->what, "stripq")) {
>   uint16_t queue_id = 0;
> 
>   /* No queue_id, return */
> - if(i + 1 >= len) {
> + if (i + 1 >= len) {
>   fprintf(stderr, "must specify (port,queue_id)\n");
>   return;
>   }
>   tmp = strtoul(str + i + 1, NULL, 0);
>   /* If queue_id greater that what 16-bits can represent, return 
> */
> - if(tmp > 0x)
> + if (tmp > 0x)
>   return;
> 
>   queue_id = (uint16_t)tmp;
> @@ -7207,7 +7208,7 @@ static void cmd_mac_addr_parsed(void
> *parsed_result,
>   ret = rte_eth_dev_mac_addr_remove(res->port_num, &res-
> >address);
> 
>   /* check the return value and print it if is < 0 */
> - if(ret < 0)
> + if (ret < 0)
>   fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
> 
>  }
> @@ -7780,7 +7781,7 @@ static void cmd_vf_mac_addr_parsed(void
> *parsed_result,
>   res->vf_num);
>  #endif
> 
> - if(ret < 0)
> + if (ret < 0)
>   fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
> strerror(-ret));
> 
>  }
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> aed4cdcb8485..7fc6d91f0210 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -306,11 +306,12 @@ parse_portnuma_config(const char *q_arg)
>   /* reset from value set at definition */
>   while ((p = strchr(p0,'(')) != NULL) {
>   ++p;
> - if((p0 = strchr(p,')')) == NULL)
> + p0 = strchr(p, ')');
> + if (p0 == NULL)
>   return -1;
> 
>   size = p0 - p;
> - if(size >= sizeof(s))
> + if (size >= sizeof(s))
>   return -1;
> 
>   snprintf(s, sizeof(s), "%.*s", size, p); @@ -366,11 +367,12 @@
> parse_ringnuma_config(const char *q_arg)
>   /* res

[PATCH] net/ice: fix ice_interrupt_handler panic when stop

[Du, Frank]

rte_intr_callback_unregister may fail when irq cb is in handling,
use sync version to make sure unregister successfully.

Signed-off-by: Du, Frank 
---
 drivers/net/ice/ice_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 7294f38edc..93f572b251 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2596,8 +2596,8 @@ ice_dev_close(struct rte_eth_dev *dev)
rte_intr_disable(intr_handle);
 
/* unregister callback func from eal lib */
-   rte_intr_callback_unregister(intr_handle,
-ice_interrupt_handler, dev);
+   rte_intr_callback_unregister_sync(intr_handle,
+ ice_interrupt_handler, dev);
 
return ret;
 }
-- 
2.34.1

[PATCH v2] net/ice: fix ice_interrupt_handler panic when stop

[Du, Frank]

rte_intr_callback_unregister may fail when irq cb is in handling,
use sync version to make sure unregister successfully.

Signed-off-by: Du, Frank 
---
 drivers/net/ice/ice_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 7294f38edc..b7ce6b053f 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2596,8 +2596,8 @@ ice_dev_close(struct rte_eth_dev *dev)
rte_intr_disable(intr_handle);
 
/* unregister callback func from eal lib */
-   rte_intr_callback_unregister(intr_handle,
-ice_interrupt_handler, dev);
+   rte_intr_callback_unregister_sync(intr_handle,
+ ice_interrupt_handler, dev);
 
return ret;
 }
-- 
2.34.1

RE: [PATCH 3/6] doc: fix miss blank line in testpmd flow syntax doc

[Zhang, Yuying]

> -Original Message-
> From: Michael Baum 
> Sent: 2022年11月10日 22:15
> To: dev@dpdk.org
> Cc: Matan Azrad ; Thomas Monjalon
> ; Raslan Darawsheh ; Singh,
> Aman Deep ; Zhang, Yuying
> ; Andrew Rybchenko
> ; akozy...@nvidia.com; sta...@dpdk.org
> Subject: [PATCH 3/6] doc: fix miss blank line in testpmd flow syntax doc
> 
> In flow syntax documentation, there is example for create pattern template.
> 
> Before the example, miss a blank line causing it to look regular bold text.
> In addition, inside the example, it uses tab instead of spaces which expand 
> the
> indentation in one line.
> 
> This patch adds the blank line and replaces tab with spaces.
> 
> Fixes: 04cc665fab38 ("app/testpmd: add flow template management")
> Cc: akozy...@nvidia.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Michael Baum 
Acked-by: Yuying Zhang 

> ---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index b5fea1396c..0037506a79 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2894,9 +2894,10 @@ following sections.
> [meters_number {number}] [flags {number}]
> 
>  - Create a pattern template::
> +
> flow pattern_template {port_id} create [pattern_template_id {id}]
> [relaxed {boolean}] [ingress] [egress] [transfer]
> -template {item} [/ {item} [...]] / end
> +   template {item} [/ {item} [...]] / end
> 
>  - Destroy a pattern template::
> 
> --
> 2.25.1

[PATCH v2] net/ixgbe: fix error of drop queue index

[Kaiwen Deng]

The drop queue index was not set when adding internal Flow
Director Configuration copy in ixgbe device private data.
Therefore dropped packets would be received by queue 0
which is set to drop queue.

This commit sets drop queue index as IXGBE_FDIR_DROP_QUEUE
to fix this issue.

Fixes: 5007ac13189d ("ethdev: remove deprecated Flow Director configuration")

Signed-off-by: Kaiwen Deng 
---
 drivers/net/ixgbe/ixgbe_flow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index 1250c2dc12..110ff34fcc 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -2759,6 +2759,7 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
int ret;
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
+   fdir_conf->drop_queue = IXGBE_FDIR_DROP_QUEUE;
 
if (hw->mac.type != ixgbe_mac_82599EB &&
hw->mac.type != ixgbe_mac_X540 &&
-- 
2.34.1

RE: [PATCH 1/6] doc: fix underlines too long in testpmd documentation

[Zhang, Yuying]

> -Original Message-
> From: Michael Baum 
> Sent: 2022年11月10日 22:15
> To: dev@dpdk.org
> Cc: Matan Azrad ; Thomas Monjalon
> ; Raslan Darawsheh ; Singh,
> Aman Deep ; Zhang, Yuying
> ; Andrew Rybchenko
> ; jack...@mellanox.com;
> do...@mellanox.com; sta...@dpdk.org
> Subject: [PATCH 1/6] doc: fix underlines too long in testpmd documentation
> 
> In testpmd documentation, there are two underlines which should not match the
> length of the text above.
> 
> This patch update them to be align with the guideline [1].
> 
> [1]
> https://doc.dpdk.org/guides/contributing/documentation.html#section-headers
> 
> Fixes: a69c335d56b5 ("doc: add flow dump command in testpmd guide")
> Fixes: 0e459ffa0889 ("app/testpmd: support flow aging")
> Cc: jack...@mellanox.com
> Cc: do...@mellanox.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Michael Baum 
Acked-by: Yuying Zhang 

> ---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 96c5ae0fe4..b5649d9d9a 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -4240,7 +4240,7 @@ Disabling isolated mode::
>   testpmd>
> 
>  Dumping HW internal information
> -
> +~~~
> 
>  ``flow dump`` dumps the hardware's internal representation information of  
> all
> flows. It is bound to ``rte_flow_dev_dump()``::
> @@ -4256,7 +4256,7 @@ Otherwise, it will complain error occurred::
> Caught error type [...] ([...]): [...]
> 
>  Listing and destroying aged flow rules
> -
> +~~
> 
>  ``flow aged`` simply lists aged flow rules be get from api
> ``rte_flow_get_aged_flows``,  and ``destroy`` parameter can be used to destroy
> those flow rules in PMD.
> --
> 2.25.1

RE: [PATCH v3 05/14] testpmd: fix whitespace

[Zhang, Yuying]

fix

> -Original Message-
> From: Zhang, Yuying
> Sent: 2022年11月11日 14:30
> To: Stephen Hemminger ; dev@dpdk.org
> Cc: Singh, Aman Deep 
> Subject: RE: [PATCH v3 05/14] testpmd: fix whitespace
> 
> 
> 
> > -Original Message-
> > From: Stephen Hemminger 
> > Sent: 2022年11月10日 7:25
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger ; Singh, Aman Deep
> > ; Zhang, Yuying 
> > Subject: [PATCH v3 05/14] testpmd: fix whitespace
> >
> > Add space after keywords.
> >
> > Signed-off-by: Stephen Hemminger 
Reviewed-by: Yuying Zhang 

> > ---
> >  app/test-pmd/cmdline.c| 31 ---
> >  app/test-pmd/parameters.c | 10 ++
> >  app/test-pmd/testpmd.c|  2 +-
> >  3 files changed, 23 insertions(+), 20 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 8dc60e938830..7721006cc310 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -2730,11 +2730,12 @@ parse_reta_config(const char *str,
> >
> > while ((p = strchr(p0,'(')) != NULL) {
> > ++p;
> > -   if((p0 = strchr(p,')')) == NULL)
> > +   p0 = strchr(p, ')');
> > +   if (p0 == NULL)
> > return -1;
> >
> > size = p0 - p;
> > -   if(size >= sizeof(s))
> > +   if (size >= sizeof(s))
> > return -1;
> >
> > snprintf(s, sizeof(s), "%.*s", size, p); @@ -3242,15 +3243,15
> @@
> > cmd_config_thresh_parsed(void *parsed_result,
> >
> > if (!strcmp(res->name, "txpt"))
> > tx_pthresh = res->value;
> > -   else if(!strcmp(res->name, "txht"))
> > +   else if (!strcmp(res->name, "txht"))
> > tx_hthresh = res->value;
> > -   else if(!strcmp(res->name, "txwt"))
> > +   else if (!strcmp(res->name, "txwt"))
> > tx_wthresh = res->value;
> > -   else if(!strcmp(res->name, "rxpt"))
> > +   else if (!strcmp(res->name, "rxpt"))
> > rx_pthresh = res->value;
> > -   else if(!strcmp(res->name, "rxht"))
> > +   else if (!strcmp(res->name, "rxht"))
> > rx_hthresh = res->value;
> > -   else if(!strcmp(res->name, "rxwt"))
> > +   else if (!strcmp(res->name, "rxwt"))
> > rx_wthresh = res->value;
> > else {
> > fprintf(stderr, "Unknown parameter\n"); @@ -4088,8 +4089,8
> @@
> > cmd_vlan_offload_parsed(void *parsed_result,
> > len = strnlen(str, STR_TOKEN_SIZE);
> > i = 0;
> > /* Get port_id first */
> > -   while(i < len){
> > -   if(str[i] == ',')
> > +   while (i < len) {
> > +   if (str[i] == ',')
> > break;
> >
> > i++;
> > @@ -4097,7 +4098,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
> > str[i]='\0';
> > tmp = strtoul(str, NULL, 0);
> > /* If port_id greater that what portid_t can represent, return */
> > -   if(tmp >= RTE_MAX_ETHPORTS)
> > +   if (tmp >= RTE_MAX_ETHPORTS)
> > return;
> > port_id = (portid_t)tmp;
> >
> > @@ -4108,17 +4109,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
> >
> > if (!strcmp(res->what, "strip"))
> > rx_vlan_strip_set(port_id,  on);
> > -   else if(!strcmp(res->what, "stripq")){
> > +   else if (!strcmp(res->what, "stripq")) {
> > uint16_t queue_id = 0;
> >
> > /* No queue_id, return */
> > -   if(i + 1 >= len) {
> > +   if (i + 1 >= len) {
> > fprintf(stderr, "must specify (port,queue_id)\n");
> > return;
> > }
> > tmp = strtoul(str + i + 1, NULL, 0);
> > /* If queue_id greater that what 16-bits can represent, return 
> > */
> > -   if(tmp > 0x)
> > +   if (tmp > 0x)
> > return;
> >
> > queue_id = (uint16_t)tmp;
> > @@ -7207,7 +7208,7 @@ static void cmd_mac_addr_parsed(void
> > *parsed_result,
> > ret = rte_eth_dev_mac_addr_remove(res->port_num, &res-
> > >address);
> >
> > /* check the return value and print it if is < 0 */
> > -   if(ret < 0)
> > +   if (ret < 0)
> > fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
> >
> >  }
> > @@ -7780,7 +7781,7 @@ static void cmd_vf_mac_addr_parsed(void
> > *parsed_result,
> > res->vf_num);
> >  #endif
> >
> > -   if(ret < 0)
> > +   if (ret < 0)
> > fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
> > strerror(-ret));
> >
> >  }
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index
> > aed4cdcb8485..7fc6d91f0210 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -306,11 +306,12 @@ parse_portnuma_config(const char *q_arg)
> > /* reset from value set at definition */
> > while ((p = strchr(p0,'(')) != NULL) {
> > ++p;
> > -   if((p0 = strchr(p,')')) == NULL)
> > +   p0 = strchr(p, ')');
> > +   if (p0 == NULL)
> >  

RE: [PATCH v3 13/14] net/i40e: fix whitespace

[Zhang, Yuying]

fix

> -Original Message-
> From: Zhang, Yuying
> Sent: 2022年11月11日 14:24
> To: Stephen Hemminger ; dev@dpdk.org
> Cc: Xing, Beilei 
> Subject: RE: [PATCH v3 13/14] net/i40e: fix whitespace
> 
> Hi,
> 
> Could you add fix line and Cc: sta...@dpdk.org?
> The fix looks good to me.
> 
> > -Original Message-
> > From: Stephen Hemminger 
> > Sent: 2022年11月10日 7:25
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger ; Zhang, Yuying
> > ; Xing, Beilei 
> > Subject: [PATCH v3 13/14] net/i40e: fix whitespace
> >
> > Add space after keywords.
> >
> > Signed-off-by: Stephen Hemminger 
Reviewed-by: Yuying Zhang 

> > ---
> >  drivers/net/i40e/i40e_pf.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > index 15d9ff868f3a..7050e0057d8e 100644
> > --- a/drivers/net/i40e/i40e_pf.c
> > +++ b/drivers/net/i40e/i40e_pf.c
> > @@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct
> > i40e_pf_vf *vf,
> >
> > for (i = 0; i < vlan_filter_list->num_elements; i++) {
> > ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
> > -   if(ret != I40E_SUCCESS)
> > +   if (ret != I40E_SUCCESS)
> > goto send_msg;
> > }
> >
> > @@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct
> > i40e_pf_vf *vf,
> > vid = vlan_filter_list->vlan_id;
> > for (i = 0; i < vlan_filter_list->num_elements; i++) {
> > ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
> > -   if(ret != I40E_SUCCESS)
> > +   if (ret != I40E_SUCCESS)
> > goto send_msg;
> > }
> >
> > @@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
> >  * return if SRIOV not enabled, VF number not configured or
> >  * no queue assigned.
> >  */
> > -   if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
> > +   if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps ==
> > +0)
> > return I40E_SUCCESS;
> >
> > /* Allocate memory to store VF structure */
> > pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
> > -   if(pf->vfs == NULL)
> > +   if (pf->vfs == NULL)
> > return -ENOMEM;
> >
> > /* Disable irq0 for VFR event */
> > --
> > 2.35.1

[PATCH v3 0/3] Enable PMD power management on Arm

[Feifei Wang]

For Arm aarch, use WFE instructions to enable PMD power management.

Test Results:
dynamic instructions over 1sec  without wfe with wfepercentage
ampere-altra6,298,483,712   9,117,624   -99.855%
thunderx2   6,990,909,373   3,247,226   -99.954%

When power efficient PMD is enabled by using WFE on Arm, if no pkts
received, the instructions that CPU executes is reduced by 99%.

V2:
1. move rte_wake_up API out of signal_exit(David Marchand, Thomas, Stephen)
2. Add test results when using wfe on ARM server

v3:
1. make code cleaner (Stephen)

Feifei Wang (3):
  eal: add 8 bits case for wait scheme
  eal: add power mgmt support on Arm
  examples/l3fwd-power: enable PMD power monitor on Arm

 examples/l3fwd-power/main.c| 25 +++
 lib/eal/arm/include/rte_pause_64.h | 32 +++--
 lib/eal/arm/rte_cpuflags.c |  5 +++
 lib/eal/arm/rte_power_intrinsics.c | 72 --
 4 files changed, 127 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] eal: add 8 bits case for wait scheme

[Feifei Wang]

For wait scheme generic helper, add 8 bits case.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 lib/eal/arm/include/rte_pause_64.h | 27 ---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/lib/eal/arm/include/rte_pause_64.h 
b/lib/eal/arm/include/rte_pause_64.h
index fe4d42b1ea..c21600ca96 100644
--- a/lib/eal/arm/include/rte_pause_64.h
+++ b/lib/eal/arm/include/rte_pause_64.h
@@ -31,6 +31,25 @@ static inline void rte_pause(void)
 /* Put processor into low power WFE(Wait For Event) state. */
 #define __RTE_ARM_WFE() { asm volatile("wfe" : : : "memory"); }
 
+/*
+ * Atomic exclusive load from addr, it returns the 8-bit content of
+ * *addr while making it 'monitored', when it is written by someone
+ * else, the 'monitored' state is cleared and an event is generated
+ * implicitly to exit WFE.
+ */
+#define __RTE_ARM_LOAD_EXC_8(src, dst, memorder) {   \
+   if (memorder == __ATOMIC_RELAXED) {   \
+   asm volatile("ldxrb %w[tmp], [%x[addr]]"  \
+   : [tmp] "=&r" (dst)   \
+   : [addr] "r" (src)\
+   : "memory");  \
+   } else {  \
+   asm volatile("ldaxrb %w[tmp], [%x[addr]]" \
+   : [tmp] "=&r" (dst)   \
+   : [addr] "r" (src)\
+   : "memory");  \
+   } }
+
 /*
  * Atomic exclusive load from addr, it returns the 16-bit content of
  * *addr while making it 'monitored', when it is written by someone
@@ -111,9 +130,11 @@ static inline void rte_pause(void)
} } \
 
 #define __RTE_ARM_LOAD_EXC(src, dst, memorder, size) { \
-   RTE_BUILD_BUG_ON(size != 16 && size != 32 &&   \
-   size != 64 && size != 128);\
-   if (size == 16)\
+   RTE_BUILD_BUG_ON(size != 8 && size != 16 &&\
+   size != 32 && size != 64 && size != 128);  \
+   if (size == 8)\
+   __RTE_ARM_LOAD_EXC_8(src, dst, memorder)   \
+   else if (size == 16)   \
__RTE_ARM_LOAD_EXC_16(src, dst, memorder)  \
else if (size == 32)   \
__RTE_ARM_LOAD_EXC_32(src, dst, memorder)  \
-- 
2.25.1

[PATCH v3 2/3] eal: add power mgmt support on Arm

[Feifei Wang]

For Arm aarch, use WFE instruction to enable power monitor API, and use
SEV instruction to enable wake up API.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 lib/eal/arm/include/rte_pause_64.h |  5 ++-
 lib/eal/arm/rte_cpuflags.c |  5 +++
 lib/eal/arm/rte_power_intrinsics.c | 72 --
 3 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/lib/eal/arm/include/rte_pause_64.h 
b/lib/eal/arm/include/rte_pause_64.h
index c21600ca96..5f70e97481 100644
--- a/lib/eal/arm/include/rte_pause_64.h
+++ b/lib/eal/arm/include/rte_pause_64.h
@@ -25,9 +25,12 @@ static inline void rte_pause(void)
 
 #ifdef RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED
 
-/* Send an event to quit WFE. */
+/* Send a local event to quit WFE. */
 #define __RTE_ARM_SEVL() { asm volatile("sevl" : : : "memory"); }
 
+/* Send a global event to quit WFE for all cores. */
+#define __RTE_ARM_SEV() { asm volatile("sev" : : : "memory"); }
+
 /* Put processor into low power WFE(Wait For Event) state. */
 #define __RTE_ARM_WFE() { asm volatile("wfe" : : : "memory"); }
 
diff --git a/lib/eal/arm/rte_cpuflags.c b/lib/eal/arm/rte_cpuflags.c
index 93461191c7..90b80709fd 100644
--- a/lib/eal/arm/rte_cpuflags.c
+++ b/lib/eal/arm/rte_cpuflags.c
@@ -163,4 +163,9 @@ void
 rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics)
 {
memset(intrinsics, 0, sizeof(*intrinsics));
+
+#ifdef RTE_ARM_USE_WFE
+   intrinsics->power_monitor = 1;
+#endif
+
 }
diff --git a/lib/eal/arm/rte_power_intrinsics.c 
b/lib/eal/arm/rte_power_intrinsics.c
index 13f6a3264d..d7d8d7af2f 100644
--- a/lib/eal/arm/rte_power_intrinsics.c
+++ b/lib/eal/arm/rte_power_intrinsics.c
@@ -6,17 +6,75 @@
 
 #include "rte_power_intrinsics.h"
 
+#ifdef RTE_ARM_USE_WFE
+static inline int
+__check_val_size(const uint8_t sz)
+{
+   switch (sz) {
+   case sizeof(uint8_t):  /* fall-through */
+   case sizeof(uint16_t): /* fall-through */
+   case sizeof(uint32_t): /* fall-through */
+   case sizeof(uint64_t): /* fall-through */
+   return 0;
+   default:
+   /* unexpected size */
+   return -1;
+   }
+}
+#endif
+
 /**
- * This function is not supported on ARM.
+ * This function uses WFE instruction to make lcore suspend
+ * execution on ARM.
+ * Note that timestamp based timeout is not supported yet.
  */
 int
 rte_power_monitor(const struct rte_power_monitor_cond *pmc,
const uint64_t tsc_timestamp)
 {
-   RTE_SET_USED(pmc);
RTE_SET_USED(tsc_timestamp);
 
+#ifdef RTE_ARM_USE_WFE
+   const unsigned int lcore_id = rte_lcore_id();
+   uint64_t cur_value;
+
+   /* prevent non-EAL thread from using this API */
+   if (lcore_id >= RTE_MAX_LCORE)
+   return -EINVAL;
+
+   if (pmc == NULL)
+   return -EINVAL;
+
+   if (__check_val_size(pmc->size) < 0)
+   return -EINVAL;
+
+   if (pmc->fn == NULL)
+   return -EINVAL;
+
+   switch (pmc->size) {
+   case sizeof(uint8_t):
+   __RTE_ARM_LOAD_EXC_8(pmc->addr, cur_value, __ATOMIC_RELAXED);
+   __RTE_ARM_WFE()
+   break;
+   case sizeof(uint16_t):
+   __RTE_ARM_LOAD_EXC_16(pmc->addr, cur_value, __ATOMIC_RELAXED);
+   __RTE_ARM_WFE()
+   break;
+   case sizeof(uint32_t):
+   __RTE_ARM_LOAD_EXC_32(pmc->addr, cur_value, __ATOMIC_RELAXED);
+   __RTE_ARM_WFE()
+   break;
+   case sizeof(uint64_t):
+   __RTE_ARM_LOAD_EXC_64(pmc->addr, cur_value, __ATOMIC_RELAXED);
+   __RTE_ARM_WFE()
+   }
+
+   return 0;
+#else
+   RTE_SET_USED(pmc);
+
return -ENOTSUP;
+#endif
 }
 
 /**
@@ -31,14 +89,22 @@ rte_power_pause(const uint64_t tsc_timestamp)
 }
 
 /**
- * This function is not supported on ARM.
+ * This function uses SEV instruction to wake up all cores
+ * on ARM.
+ * Note that lcore_id is not used here.
  */
 int
 rte_power_monitor_wakeup(const unsigned int lcore_id)
 {
RTE_SET_USED(lcore_id);
 
+#ifdef RTE_ARM_USE_WFE
+   __RTE_ARM_SEV()
+
+   return 0;
+#else
return -ENOTSUP;
+#endif
 }
 
 int
-- 
2.25.1