[dpdk-dev] [PATCH] examples/multi_process - fix crash in mp_client with sparse ports

[dpdk-dev] [PATCH] examples/multi_process - fix crash in mp_client with sparse ports

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The mp_client crashes if run on Azure or any system where ethdev
ports are owned. In that case, the tx_buffer and tx_stats for the
real port were initialized correctly, but the wrong port was used.

For example if the server has Ports 3 and 5. Then calling
rte_eth_tx_buffer_flush on any other buffer will dereference null
because the tx buffer for that port was not allocated.

Fixes: e2366e74e029 ("examples: use buffered Tx")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 examples/multi_process/client_server_mp/mp_client/client.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/examples/multi_process/client_server_mp/mp_client/client.c b/examples/multi_process/client_server_mp/mp_client/client.c
index c23dd3f378f7..c1d2d975b717 100644
--- a/examples/multi_process/client_server_mp/mp_client/client.c
+++ b/examples/multi_process/client_server_mp/mp_client/client.c
@@ -246,15 +246,16 @@ main(int argc, char *argv[])
 
 	for (;;) {
 		uint16_t i, rx_pkts;
-		uint16_t port;
 
 		rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
 				PKT_READ_SIZE, NULL);
 
 		if (unlikely(rx_pkts == 0)){
 			if (need_flush)
-				for (port = 0; port < ports->num_ports; port++) {
-					sent = rte_eth_tx_buffer_flush(ports->id[port], client_id,
+				for (i = 0; i < ports->num_ports; i++) {
+					uint16_t port = ports->id[i]'
+
+					sent = rte_eth_tx_buffer_flush(ports, client_id,
 							tx_buffer[port]);
 					if (unlikely(sent))
 						tx_stats->tx[port] += sent;
-- 
2.20.1

[dpdk-dev] Meson build failing in ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c

[Stephen Hemminger]

The Intel build is failing because of recent AES changes?


*Meson Failed Build #1:
OS: CENTOS76-64
Target:build-gcc-static
FAILED: drivers/a715181@@tmp_rte_pmd_aesni_mb at sta/crypto_aesni_mb_rte_aesni_mb_pmd.c.o 
gcc -Idrivers/a715181@@tmp_rte_pmd_aesni_mb at sta -Idrivers -I../drivers -Idrivers/crypto/aesni_mb -I../drivers/crypto/aesni_mb -Ilib/librte_cryptodev -I../lib/librte_cryptodev -I. -I../ -Iconfig -I../config -Ilib/librte_eal/common/include -I../lib/librte_eal/common/include -I../lib/librte_eal/linux/eal/include -Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal/common/include/arch/x86 -I../lib/librte_eal/common/include/arch/x86 -Ilib/librte_eal -I../lib/librte_eal -Ilib/librte_kvargs -I../lib/librte_kvargs -Ilib/librte_mbuf -I../lib/librte_mbuf -Ilib/librte_mempool -I../lib/librte_mempool -Ilib/librte_ring -I../lib/librte_ring -Idrivers/bus/vdev -I../drivers/bus/vdev -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wunused-parameter -Wsign-compare -Wcast-qual -fPIC -march=native -D_GNU_SOURCE -DALLOW_EXPERIMENTAL_API -MD -MQ 'drivers/a715181@@tmp_rte_pmd_aesni_mb at sta/crypto_aesni_mb_rte_aesni_mb_pmd.c.o' -MF 'drivers/a715181@@tmp_rte_pmd_aesni_mb at sta/crypto_aesni_mb_rte_aesni_mb_pmd.c.o.d' -o 'drivers/a715181@@tmp_rte_pmd_aesni_mb at sta/crypto_aesni_mb_rte_aesni_mb_pmd.c.o' -c ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c: In function ‘aesni_mb_set_session_auth_parameters’:
../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:60:19: error: ‘hash_oneblock_fn’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  (*one_block_hash)(opad_buf, opad);
                   ^
../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:107:19: note: ‘hash_oneblock_fn’ was declared here
  hash_one_block_t hash_oneblock_fn;
                   ^
cc1: all warnings being treated as errors
[1065/1660] Linking static target drivers/libtmp_rte_pmd_aesni_gcm.a.
[1066/1660] Compiling C object 'drivers/a715181@@tmp_rte_pmd_aesni_mb at sta/crypto_aesni_mb_rte_aesni_mb_pmd_ops.c.o'.
[1067/1660] Compiling C object 'drivers/a715181@@tmp_rte_pmd_vmxnet3 at sta/net_vmxnet3_vmxnet3_ethdev.c.o'.
[1068/1660] Compiling C object 'drivers/a715181@@tmp_rte_pmd_vmxnet3 at sta/net_vmxnet3_vmxnet3_rxtx.c.o'.
[1069/1660] Compiling C object 'drivers/a715181@@tmp_rte_pmd_virtio at sta/net_virtio_virtio_rxtx.c.o'.
ninja: build stopped: subcommand failed

Re: [dpdk-dev] [PATCH] examples/multi_process - fix crash in mp_client with sparse ports

[David Marchand]

On Tue, Jun 4, 2019 at 2:05 AM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> The mp_client crashes if run on Azure or any system where ethdev
> ports are owned. In that case, the tx_buffer and tx_stats for the
> real port were initialized correctly, but the wrong port was used.
>
> For example if the server has Ports 3 and 5. Then calling
> rte_eth_tx_buffer_flush on any other buffer will dereference null
> because the tx buffer for that port was not allocated.
>
> Fixes: e2366e74e029 ("examples: use buffered Tx")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  examples/multi_process/client_server_mp/mp_client/client.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/examples/multi_process/client_server_mp/mp_client/client.c
> b/examples/multi_process/client_server_mp/mp_client/client.c
> index c23dd3f378f7..c1d2d975b717 100644
> --- a/examples/multi_process/client_server_mp/mp_client/client.c
> +++ b/examples/multi_process/client_server_mp/mp_client/client.c
> @@ -246,15 +246,16 @@ main(int argc, char *argv[])
>
>         for (;;) {
>                 uint16_t i, rx_pkts;
> -               uint16_t port;
>
>                 rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
>                                 PKT_READ_SIZE, NULL);
>
>                 if (unlikely(rx_pkts == 0)){
>                         if (need_flush)
> -                               for (port = 0; port < ports->num_ports;
> port++) {
> -                                       sent =
> rte_eth_tx_buffer_flush(ports->id[port], client_id,
> +                               for (i = 0; i < ports->num_ports; i++) {
> +                                       uint16_t port = ports->id[i]'
>

Syntax error.

+
> +                                       sent =
> rte_eth_tx_buffer_flush(ports, client_id,
>

Not sure passing ports is intended.

                                                        tx_buffer[port]);
>                                         if (unlikely(sent))
>                                                 tx_stats->tx[port] += sent;
>
>


-- 
David Marchand

[dpdk-dev] [PATCH v2] examples/multi_process - fix crash in mp_client with sparse ports

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The mp_client crashes if run on Azure or any system where ethdev
ports are owned. This is because the flush loop is confusing the
index into the array of ports[] with the port id.

For example if the server has Ports 3 and 5. Then calling
rte_eth_tx_buffer_flush on any other buffer will dereference null
because the tx buffer for that port was not allocated.

Also:
   - the flush code is common enough that it should not be marked
     unlikely
   - combine conditions to reduce indentation
   - avoid unnecessary if() if sent is zero.

Fixes: e2366e74e029 ("examples: use buffered Tx")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 .../client_server_mp/mp_client/client.c       | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/examples/multi_process/client_server_mp/mp_client/client.c b/examples/multi_process/client_server_mp/mp_client/client.c
index c23dd3f378f7..db9ebbe2402a 100644
--- a/examples/multi_process/client_server_mp/mp_client/client.c
+++ b/examples/multi_process/client_server_mp/mp_client/client.c
@@ -246,19 +246,20 @@ main(int argc, char *argv[])
 
 	for (;;) {
 		uint16_t i, rx_pkts;
-		uint16_t port;
 
 		rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
 				PKT_READ_SIZE, NULL);
 
-		if (unlikely(rx_pkts == 0)){
-			if (need_flush)
-				for (port = 0; port < ports->num_ports; port++) {
-					sent = rte_eth_tx_buffer_flush(ports->id[port], client_id,
-							tx_buffer[port]);
-					if (unlikely(sent))
-						tx_stats->tx[port] += sent;
-				}
+		if (rx_pkts == 0 && need_flush) {
+			for (i = 0; i < ports->num_ports; i++) {
+				uint16_t port = ports->id[i];
+
+				sent = rte_eth_tx_buffer_flush(port,
+							       client_id,
+							       tx_buffer[port]);
+				if (unlikely(sent))
+					tx_stats->tx[port] += sent;
+			}
 			need_flush = 0;
 			continue;
 		}
-- 
2.20.1

[dpdk-dev] [PATCH v3] examples/multi_process - fix crash in mp_client with sparse ports

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The mp_client crashes if run on Azure or any system where ethdev
ports are owned. This is because the flush loop is confusing the
index into the array of ports[] with the port id.

For example if the server has Ports 3 and 5. Then calling
rte_eth_tx_buffer_flush on any other buffer will dereference null
because the tx buffer for that port was not allocated.

v2
   - fix typo
   - the flush code is common enough that it should not be marked unlikely
   - combine conditions to reduce indentation

v3
   - avoid unnecessary if() if sent is zero.

Fixes: e2366e74e029 ("examples: use buffered Tx")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 .../client_server_mp/mp_client/client.c        | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/examples/multi_process/client_server_mp/mp_client/client.c b/examples/multi_process/client_server_mp/mp_client/client.c
index c23dd3f378f7..361d90b54b2d 100644
--- a/examples/multi_process/client_server_mp/mp_client/client.c
+++ b/examples/multi_process/client_server_mp/mp_client/client.c
@@ -246,19 +246,19 @@ main(int argc, char *argv[])
 
 	for (;;) {
 		uint16_t i, rx_pkts;
-		uint16_t port;
 
 		rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
 				PKT_READ_SIZE, NULL);
 
-		if (unlikely(rx_pkts == 0)){
-			if (need_flush)
-				for (port = 0; port < ports->num_ports; port++) {
-					sent = rte_eth_tx_buffer_flush(ports->id[port], client_id,
-							tx_buffer[port]);
-					if (unlikely(sent))
-						tx_stats->tx[port] += sent;
-				}
+		if (rx_pkts == 0 && need_flush) {
+			for (i = 0; i < ports->num_ports; i++) {
+				uint16_t port = ports->id[i];
+
+				sent = rte_eth_tx_buffer_flush(port,
+							       client_id,
+							       tx_buffer[port]);
+				tx_stats->tx[port] += sent;
+			}
 			need_flush = 0;
 			continue;
 		}
-- 
2.20.1

Re: [dpdk-dev] [PATCH v3] examples/multi_process - fix crash in mp_client with sparse ports

[David Marchand]

On Tue, Jun 18, 2019 at 6:41 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> The mp_client crashes if run on Azure or any system where ethdev
> ports are owned. This is because the flush loop is confusing the
> index into the array of ports[] with the port id.
>
> For example if the server has Ports 3 and 5. Then calling
> rte_eth_tx_buffer_flush on any other buffer will dereference null
> because the tx buffer for that port was not allocated.
>
> v2
>    - fix typo
>    - the flush code is common enough that it should not be marked unlikely
>    - combine conditions to reduce indentation
>
> v3
>    - avoid unnecessary if() if sent is zero.
>

Annotations should be after the ---.



> Fixes: e2366e74e029 ("examples: use buffered Tx")
>

Cc: stable@dpdk.org


Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  .../client_server_mp/mp_client/client.c        | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/examples/multi_process/client_server_mp/mp_client/client.c
> b/examples/multi_process/client_server_mp/mp_client/client.c
> index c23dd3f378f7..361d90b54b2d 100644
> --- a/examples/multi_process/client_server_mp/mp_client/client.c
> +++ b/examples/multi_process/client_server_mp/mp_client/client.c
> @@ -246,19 +246,19 @@ main(int argc, char *argv[])
>
>         for (;;) {
>                 uint16_t i, rx_pkts;
> -               uint16_t port;
>
>                 rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
>                                 PKT_READ_SIZE, NULL);
>
> -               if (unlikely(rx_pkts == 0)){
> -                       if (need_flush)
> -                               for (port = 0; port < ports->num_ports;
> port++) {
> -                                       sent =
> rte_eth_tx_buffer_flush(ports->id[port], client_id,
> -                                                       tx_buffer[port]);
> -                                       if (unlikely(sent))
> -                                               tx_stats->tx[port] += sent;
> -                               }
> +               if (rx_pkts == 0 && need_flush) {
> +                       for (i = 0; i < ports->num_ports; i++) {
> +                               uint16_t port = ports->id[i];
> +
> +                               sent = rte_eth_tx_buffer_flush(port,
> +                                                              client_id,
> +
> tx_buffer[port]);
> +                               tx_stats->tx[port] += sent;
> +                       }
>                         need_flush = 0;
>                         continue;
>                 }
>
>
Woh, had to concentrate to see that the pb was when accessing tx_buffer[]
and tx_stats->tx[] arrays, good catch :-)

Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand